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

[Feature] Refactor CKAN-core to use .NET standard 2.0 #2799

Closed
jbrot opened this issue Jun 24, 2019 · 8 comments
Closed

[Feature] Refactor CKAN-core to use .NET standard 2.0 #2799

jbrot opened this issue Jun 24, 2019 · 8 comments
Labels
Architecture Build Issues affecting the build system Core (ckan.dll) Issues affecting the core part of CKAN macOS Issues specific for macOS

Comments

@jbrot
Copy link
Contributor

jbrot commented Jun 24, 2019

I am considering creating a new GUI for CKAN which is native to macOS. I would like to make the GUI using Xamarin.Mac and Xamarin.Forms. In order to compile CKAN for Xamarin.Mac, CKAN-core must be moved to .NET Standard 2.0 (instead of .NET Framework 4.5).

If I make a pull request refactoring CKAN-core, would the project be open to accepting it? Furthermore, if I were to go further and actually create the Xamarin GUI, would the project be open to accepting that as well?

I have done some preliminary work towards such a refactor already, and so my understanding is that the refactor involves the following changes:

  1. Core/CKAN-core.csproj must be entirely rewritten.
  2. Within CKAN-core, SHA1Cng must be replaced with another SHA1 provider because SHA1Cng is not supported in .NET Standard 2.0. The other .NET providers are supported, so this change is effectively trivial.
  3. Several dependencies must be swapped out for drop-in replacements on Nuget which target .NET Standard
  4. ChinhDo.Transactions.FileManager must be replaced/rewritten to support .NET Standard 2.0 (A blueprint for how to do this can be found here: Find replacement or rewrite on .NET Core ChinhDo.Transactions.FileManager VirtoCommerce/vc-platform-core#51)
  5. The remaining CKAN components must be moved from .NET 4.5 to .NET 4.6.1.
@techman83
Copy link
Member

At a high level (I'll leave the .NET commentary to those with that expertise) anything that makes KSP-CKAN better will accepted, you can see more in our Developers Guide.

Some things we should add to that:

  • All dependencies are license compatible with the projects license
  • Runs on all the platforms we support (Linux/Mac/Windows)
  • Consider our Mono version targets
  • Minimal end user disruption (if CKAN suddenly stops working it will cause a significant support load we don't have the resources to address).

On the last point, that's not to say we can't make a huge GUI/architecture change. Rather maybe put it behind a separate feature flag or different build. As in say continue to build CKAN 1 + CKAN 2 for a period of time, but those are just implementation details :-)

@DasSkelett DasSkelett added Architecture Build Issues affecting the build system Core (ckan.dll) Issues affecting the core part of CKAN Discussion needed macOS Issues specific for macOS labels Jun 24, 2019
@DasSkelett
Copy link
Member

DasSkelett commented Jun 24, 2019

Core/CKAN-core.csproj must be entirely rewritten.

Can you elaborate why? I suspect it's due to missing System.* libraries?
"Rewrite entirely" sounds quite a bit scary.


Furthermore, if I were to go further and actually create the Xamarin GUI, would the project be open to accepting that as well?

Would this be platform independent?
But I think that's less of a problem, if this doesn't work at first, nobody depends on it and there's no stress getting it fixed.

I agree with @techman83, best would be to ship both separately in the beginning. Maybe keep it even as a separate branch? Or just as separate projects.


ChinhDo.Transactions.FileManager must be replaced/rewritten to support .NET Standard 2.0 (A blueprint for how to do this can be found here: VirtoCommerce/vc-platform-core#51)

That linked software is licensed under a customized Open Software License. That's a copyleft license:

with the proviso that copies of Original Work or Derivative Works that You distribute or communicate shall be licensed under this Virto Commerce Open Source License;

If we use those linked commits as guide, we end up with the same problem as in #2027 (with my understanding that 'Derivative Work' only means our implementation of it, not the full CKAN project):
Furthermore, if I understand the wikipedia right:

The restriction contained in Section 9 of the OSL reads:

If You distribute or communicate copies of the Original Work or a Derivative Work, You must make a reasonable effort under the circumstances to obtain the express assent of recipients to the terms of this License.

In its analysis of the OSL the Free Software Foundation claims that "this requirement means that distributing OSL software on ordinary FTP sites, sending patches to ordinary mailing lists, or storing the software in an ordinary version control system, can arguably be a violation of the license and would subject violators to possible termination of the license. Thus, the OSL makes it challenging to develop software using the ordinary tools of Free Software development."

it's not possible at all fur us to include something licensed under OSL/VCOSL.
But somebody with better licence knowledge should take a look at it.

@jbrot
Copy link
Contributor Author

jbrot commented Jun 24, 2019

  1. CKAN-Core.csproj would be rewritten because .NET Standard 2.0 introduces sensible defaults. About 90% of the current csproj would be removed, and the functionality would be maintained due to the defaults. This is actually a good thing, because it makes the file much easier to understand, reducing technical debt.

  2. The Xamarin GUI has two components. The first component is. cross platform Xamarin.Forms project which is platform independent. Most of the GUI would be laid out here, and most of the supporting logic would also be contained here.

The second component is a platform dependent shim. One of these would be made for each supported platform, and is responsible for starting the Xamarin.Forms GUI as well as maintaining any platform-specific quirks. I intend to just make a Xamarin.Mac project at first, but if that goes well, I will look into making a Xamarin.UWP project bringing the GUI to windows and a Xamarin.Gtk# project bringing the GUI to Linux. Adding these extra platforms should be require relatively little work after the Mac GUI is fully operational.

  1. I did not think about licensing. I will look into this further and see what other solutions are possible.

Furthermore, I just looked into the Mono target. Mono 4.0 supports .NET 4.5 and .NET Standard 1.1. Since .NET Standard is forward compatible, if I refactor CKAN-core for .NET Standard 1.1 instead of 2.0, this should allow the rest of the project to remain more or less unchanged, while still allowing CKAN-core to be used in Xamarin.Mac.

I will do more research over the next few days to see what changes will be needed to refactor for .NET Standard 1.1 instead of 2.0, and then report back to see if it seems reasonable.

@DasSkelett
Copy link
Member

Ohhh, I misread that, I didn't read the .csproj, I thought you meant the whole project code...
So scratch that, that sounds good indeed.

@jbrot
Copy link
Contributor Author

jbrot commented Jun 25, 2019

I've done some more research, and I've conclude that CKAN-core cannot be converted to .NET Standard 1.1 because .NET Standard 1.1 doesn't support several APIs CKAN uses. However, as discussed earlier, to convert CKAN-core to .NET Standard 2.0 requires relatively few changes: the Sha1 provider needs to be swapped out, and the TxFileManager classes would need to be brought into the project, as the package on NuGet does not support .NET Standard 2.0.

I've found a blog post here which describes how to set up a project which can be built for both .NET Framework 4.5 and .NET Standard 2.0. Fortunately, the code linked in the blog post (https://github.com/RickStrahl/Westwind.Utilities) is MIT licensed.

So, my current proposal is to modify CKAN-core so that it can be compiled both for .NET Framework 4.5, maintaining compatibility with Mono 4.0, but also allow it to be compiled for .NET Standard 2.0 so that I can use it with Xamarin.Mac.

With respect to the licensing issues of https://github.com/VirtoCommerce/vc-platform-core, I will just not adapt any of the code found there. Instead, I will adapt the relevant portions from the original source at https://github.com/rsevil/Transactions, which is MIT licensed.

@techman83
Copy link
Member

So, my current proposal is to modify CKAN-core so that it can be compiled both for .NET Framework 4.5, maintaining compatibility with Mono 4.0, but also allow it to be compiled for .NET Standard 2.0 so that I can use it with Xamarin.Mac.

I have one primary concern here, Xamarin is not and will never be usable on linux. I'm very much not in favour in moving in a direction that puts cross platform last.

If the project can be cleaned up so that it will compile for .NET Standard without introducing any technical debt and has good test coverage, that would be an ok middle ground. As for the client, personally I would need convincing that producing platform specific clients are a good idea. But I'm also conscious that WinForms + Mac = Pain.

@jbrot
Copy link
Contributor Author

jbrot commented Jun 25, 2019

The situation with respect to Xamarin and Linux has actually changed in the past two years. Xamarin now has a GTK# client which directly supports making GNOME GUIs. Ideally, a Xamarin GUI will allow for a native Windows, Linux, and Mac OS GUI to be made with minimal platform-specific code, which I would not describe as putting cross-platform last. However, I do acknowledge that skepticism for cross-platform products from Microsoft is well warranted, and I completely understand your concerns.

@DasSkelett
Copy link
Member

Done in #2820.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Architecture Build Issues affecting the build system Core (ckan.dll) Issues affecting the core part of CKAN macOS Issues specific for macOS
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants