Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prism is now commercial software #872

Closed
rauhs opened this issue Oct 6, 2023 · 9 comments · Fixed by #875
Closed

Prism is now commercial software #872

rauhs opened this issue Oct 6, 2023 · 9 comments · Fixed by #875
Assignees

Comments

@rauhs
Copy link
Contributor

rauhs commented Oct 6, 2023

Hi *,

just wanted to drop a note: Please be wary of upgrading to any new Prism nuget. They'll likely be commercial software: https://github.com/PrismLibrary/Prism

Or maybe even consider copying the code for the Weak Conditional table...

Cheers and thanks for all the great work!

@rclabo
Copy link
Contributor

rclabo commented Oct 6, 2023

That's unfortunate. We appreciate the heads up. Seems like the suggestion to copy the older Weak Conditional Table code (under the prior license) might be the way to go?

What do other's think?

@NightOwl888
Copy link
Contributor

NightOwl888 commented Oct 8, 2023

This will only affect .NET Framework/.NET Standard 2.0 users - so this is all legacy. Previously the code was under MIT and now it has either a Community or Commercial license.

ConditionalWeakTable is missing the enumerator prior to .NET Standard 2.1 and newer versions cannot be back ported because it ties directly into the GC in native code. This is the reason why we went with weak events in the first place. See #613. Porting it is a dead end.

There are at least 3 options:

  1. Leave the current dependency. If someone wants to upgrade, let them deal with the licensing. Presumably, they won't be upgrading unless they have a license, anyway.
  2. Go to a commit in Prism that had an MIT license and copy the weak event code into the Support/Util/Events folder (in the namespace Lucene.Net.Util.Events) and add the headers as specified in the prior license. There are only a few types that we depend on and it is all pure C# code. All of the types should be made internal.
  3. Adapt the weak events to use WeakEventManager in .NET Framework (See Weak event patterns) and remove the dependency on the Prism.Core package. This would require us to drop support for .NET Standard 2.0, but this is something we are considering doing anyway.

@eladmarg
Copy link
Contributor

eladmarg commented Oct 8, 2023

Thanks @NightOwl888 for the detailed response and options,
AFAIK most of the users moving anyway from .NET standard 2.0, so its a good bet.

@rauhs
Copy link
Contributor Author

rauhs commented Oct 9, 2023

I'd not use WeakEventManager. It's tied to WPF/WindowsBase, which is something a lot of projects do not want to pull in.

@NightOwl888
Copy link
Contributor

I'd not use WeakEventManager. It's tied to WPF/WindowsBase, which is something a lot of projects do not want to pull in.

Agreed. WeakEventManager seems over-engineered, anyway. IMO, the best option would be to bring in the types from Prism into our codebase under a commit with the former MIT license. We haven't had any complaints with this solution from .NET Framework users. Removing the dependency on Prism will avoid any "gotcha" issues with people upgrading and being out of compliance with the license.

@rauhs - It is Hacktoberfest month, so if you wish to submit a PR it will apply toward your free swag for participation.

@jeme
Copy link
Contributor

jeme commented Oct 9, 2023

This will only affect .NET Framework/.NET Standard 2.0 users - so this is all legacy. Previously the code was under MIT and now it has either a Community or Commercial license.

ConditionalWeakTable is missing the enumerator prior to .NET Standard 2.1 and newer versions cannot be back ported because it ties directly into the GC in native code. This is the reason why we went with weak events in the first place. See #613. Porting it is a dead end.

There are at least 3 options:

  1. Leave the current dependency. If someone wants to upgrade, let them deal with the licensing. Presumably, they won't be upgrading unless they have a license, anyway.
  2. Go to a commit in Prism that had an MIT license and copy the weak event code into the Support/Util/Events folder (in the namespace Lucene.Net.Util.Events) and add the headers as specified in the prior license. There are only a few types that we depend on and it is all pure C# code. All of the types should be made internal.
  3. Adapt the weak events to use WeakEventManager in .NET Framework (See Weak event patterns) and remove the dependency on the Prism.Core package. This would require us to drop support for .NET Standard 2.0, but this is something we are considering doing anyway.

Just wanted to add my thoughts on the .NET Framework/Standard statement.

I don't think .NET Framework is as legacy as one would think, that is a really sad fact but it turns out that its more difficult to migrate than what many could hope for :(. So I think there are more allot of projects out there stuck on .NET 4.X that are still maintained.

With regards to .NET Standard 2.0, I think there are many cases that one may not be aware of, I was certainly made aware as part of a Castle.Windsor discussion a while back that it's a really useful target for e.g. Library authors or if you where sitting on a "Core" version one step prior to a supported one, e.g. if one would choose to support .NET 6, 7 etc but not 5 and down, people on 5 and down could use that. So I would not cut that away so hastily.

I know I was at least convinced by that discussion that it was to soon to discard support for it, and that perhaps one should discard support for .NET 4.X before that. (It would become supported as part of supporting .NET Standard)

Ofc. this has to be weighed against the burden to maintain it.

@rauhs
Copy link
Contributor Author

rauhs commented Oct 9, 2023

@rauhs - It is Hacktoberfest month, so if you wish to submit a PR it will apply toward your free swag for participation.

I do not plan to create a PR for this. Sorry. The past few contributions (not to this repo) have been too frustrating for me: About 20min for the coding work and then fighting 3-4h with git until the PR was merged... $dayjob is still on SVN :/

@NightOwl888
Copy link
Contributor

NightOwl888 commented Oct 9, 2023

Just wanted to add my thoughts on the .NET Framework/Standard statement.

I don't think .NET Framework is as legacy as one would think, that is a really sad fact but it turns out that its more difficult to migrate than what many could hope for :(. So I think there are more allot of projects out there stuck on .NET 4.X that are still maintained.

Present company included :).

But I wasn't thinking of dropping .NET Framework yet (although many people have suggested it). There were huge investments on .NET Framework and it will be a decade or so until it is no longer in wide use due to problems upgrading (i.e. WPF, ASP.NET WebForms, etc).

With regards to .NET Standard 2.0, I think there are many cases that one may not be aware of, I was certainly made aware as part of a Castle.Windsor discussion a while back that it's a really useful target for e.g. Library authors or if you where sitting on a "Core" version one step prior to a supported one, e.g. if one would choose to support .NET 6, 7 etc but not 5 and down, people on 5 and down could use that. So I would not cut that away so hastily.

I know I was at least convinced by that discussion that it was to soon to discard support for it, and that perhaps one should discard support for .NET 4.X before that. (It would become supported as part of supporting .NET Standard)

Ofc. this has to be weighed against the burden to maintain it.

Well you have a point. I just looked at the details and although Xamarin is going out of support very soon, .NET Standard 2.1 doesn't support UWP. So, dropping .NET Standard 2.0 would be a problem for users of that platform.

We support .NET Standard 2.1, which will handle any users lagging to upgrade from .NET Core 2.x - .NET 5.x and we have a .NET Framework 4.6.2 target to handle that platform. If we drop .NET Standard 2.0, we would also lose Xamarin.Android < 10.0, Xamarin.iOS < 12.16 which are now out of support and Unity < 2021.2.

.NET Standard 2.0 is the least efficient platform we support. There are performance advantages for being on a native platform (such as on net6.0 we call the System.Numerics.BitOperations class that calls hardware intrinsics). As a result, 3rd party extensions to Lucene.NET (such as Lucene.Net.Store.Azure) that target netstandard2.0 that are consumed by .NET 6.x - .NET 8.x will take a performance hit to use because although a net6.0 target is available, when referencing the component it will downgrade to netstandard2.0 and skip the hardware intrinsic code (or at least I think it will - need to test). They would also be stuck with this hacky weak event code to workaround the missing ConditionalWeakTable enumerator and would use ConcurrentDictionary instead of Dictionary in some cases to work around the lack of support for deletes while iterating forward. But I guess that is more of a "don't do that" bug that we need to add a note on the README telling NuGet library developers to multi-target to avoid the slower code.

@rclabo
Copy link
Contributor

rclabo commented Oct 18, 2023

Awesome. Thanks Shad! Much appreciated.

NightOwl888 added a commit that referenced this issue Oct 18, 2023
* Lucene.Net.Util: Renamed Events class > WeakEvents

* SWEEP: Removed dependency on Prism.Core and brought the Prism.Events namespace into Lucene.Net.Util.Events including tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants