search

ExtendedXmlSerializer / home

A configurable and eXtensible Xml serializer for .NET.
https://extendedxmlserializer.github.io/
MIT License
336 stars 47 forks source link

Installing ExtendedXmlSerializer into Android part of Xamarin.Forms app adds unneeded nuget packages #251

Closed MagicAndre1981 closed 5 years ago

MagicAndre1981 commented 5 years ago

Installing ExtendedXmlSerializer into Android part of Xamarin.Forms app, adds unneeded nuget packages.

I use .net standard 2.0 dll as shared codebase for Xamarin apps (Android, UWP 16299). And System.Xml.XmlSerializer is already part of .net standard 2.0 and Xamarin.Android, so why is this still referenced?

MagicAndre1981 commented 5 years ago

System.ComponentModel is also already part of .net standard 2.0

Mike-E-angelo commented 5 years ago

Hey good points @MagicAndre1981 ... to be sure, we started v2 before .NET Standard 2.0 was a thing. :) So this is leftover from 1.x and it hasn't been looked at until now.

Do note that we are also supporting .NET Framework 4.5+, so I am not sure of the deployment implications here. However, I did omit those references if the current build is .NET Standard 2.0.

I am not too familiar with the deployment ramifications, so @wojtpl2 might have to assist here before deploying to NuGet. However, you can check with our preview feed to see if that is working better for you now: https://www.myget.org/F/wojtpl2/api/v3/index.json

If not, @wojtpl2 will have to take over from here. 😁

MagicAndre1981 commented 5 years ago

For for full .net framework, they are already buildin for years and need no nugets. They were released to help users to migrate from .net framework to .net core.

ok, the major issue is now System.Reflection.Emit.Lightweight which is supposed to have no dependencies for MonoAndroid, but Nuget thinks it is .net standard 1.0 and install all other things into packages.config that are later not used in csproj 🤔

The issue is that I can't go away from packages.config for Android, because with "PackageReference Include" sometimes the reference assemblies are picked and not the real implementations from lib folder and so app crashes at start.

But with the 2.1.20 test build I see an small improvement, the XML thing is gone

Mike-E-angelo commented 5 years ago

WOW... so you get the actual ref assemblies rather than the runtime assemblies? That sounds like something that should never ever happen. :P If I understand correctly you are using VS2017 as well so that puts another wrinkle in your scenario.

I did look into System.Reflection.Emit.Lightweight and thought I was able to work around it with System.Memory instead, but it appears that LightInject is dependent on it. :(

I am not sure how to further assist in this particular issue for you, so I am open to suggestions on how to proceed.

MagicAndre1981 commented 5 years ago

Last year I was using VS2017 15.7 with PackageReference Include, but the ASP Net Core SignalR package only worked since 15.8 for Xamarin, so I installed the 15.8 preview in a new VM and app worked, later updated from 15.7.x to 15.8 final, but app crashed when compiled with final 15.8 at start due to PackageReference Include bug I linked. So I have to stay at packages.config.

I played now for several hours with it. I need to install the dependencies System.Collections.Immutable, System.Interactive, System.Reflection.Emit.Lightweight on my own into the Android part, now nuget doesn't install the additional packages when I try to add ExtendedXmlSerializer 2.1.20 to Android.

I have no idea why I get the additional packages are installed when I only try to install ExtendedXmlSerializer 🤔 🤷‍♂️

Mike-E-angelo commented 5 years ago

Ahhh sounds like you're running into legacy Nuget issues then in VS2017, if I understand correctly. I have yet to build an Android app, despite trying 2-3 times. Each time I attempt to install Visual Studio (for an entire day 😆) and never get past odd warnings and errors when I compile.

So, you're having better luck than I do. 😁

That said, am I understanding that you have fixed this issue when directly adding those dependencies? I am not sure why that is and it sounds like a Nuget limitation/bug.

MagicAndre1981 commented 5 years ago

That said, am I understanding that you have fixed this issue when directly adding those dependencies?

this is correct. Before install ExtendedXmlSerializer , install System.Collections.Immutable, System.Interactive and System.Reflection.Emit.Lightweight and the issue with the many useless added packages is gone.

Maybe add this information to the readme

MagicAndre1981 commented 5 years ago

btw, can you release the previews on nuget directly with beta suffix to avoid compile issue? I always get package downgrade from .20 to .19 and can't compile anything. And when .20 goes live, I need to delete the already cached one, otherwise I would not get the final bits. using the beta fixes this conflict

Mike-E-angelo commented 5 years ago

btw, can you release the previews on nuget directly with beta suffix to avoid compile issue

Ahhh @MagicAndre1981 that's a good question. We are actually using this scheme currently as preview feed users were getting errors/wrong packages on the other direction: e.g. on .20 preview feed they would get errors/inaccurate versioning because they had already installed .20 on the public feed.

Deployment is actually not my domain as I do not own the repo, and the automation is outside of my control. This is something for @wojtpl2 to consider and/or assist. 😁 Perhaps take advantage of the new GitHub feeds?

Maybe add this information to the readme

TBH I am not sure how valuable that will be, as I am not sure what we are doing that makes this happen, and from my understanding, this appears to be a Nuget issue with VS2017 packages.config on Xamarin projects. I would rather fix the issue on our end if possible but can also ultimately be talked into putting a quick link in our documentation pointing back to this issue for further information if that's what it takes, as well.

MagicAndre1981 commented 5 years ago

I also tried it in VS2019 Update 2 Preview 3 and same happens. So this is a NuGet issue with the old packages which were created before .net standard 2.0 was out.

I checked and saw that there are pre Release versions 4.6.0-preview6.19303.8 which includes monodroid and .net standard 2.0 definitions and doesn't install garbage.

The name indicates that Microsoft will release them as stable when .net core 3.0 gets stable. So release the new package with the fix you already made and after .net core 3.0 GA, update the nuget package dependencies to the updated versions of System.Collections.Immutable, System.Interactive and System.Reflection.Emit.Lightweight and my issue will be fixed.

btw, can you release the previews on nuget directly with beta suffix to avoid compile issue

Ahhh @MagicAndre1981 that's a good question. We are actually using this scheme currently as preview feed users were getting errors/wrong packages on the other direction: e.g. on .20 preview feed they would get errors/inaccurate versioning because they had already installed .20 on the public feed.

no, if you release a .20.beta1/2/3 , users can easily install .20 final without issues. Your .20 is detected as stable release, so if real stable is out on nuget.org, I would not get the final bits without deleting cached files from C:\Users\USERNAME.nuget\packages\extendedxmlserializer\ folder . I already had this in past for different package.

after several tries (deleting obj/bin folders) I was able to compile the app again.

Mike-E-angelo commented 5 years ago

So release the new package with the fix you already made and after .net core 3.0 GA, update the nuget package dependencies to the updated versions of System.Collections.Immutable, System.Interactive and System.Reflection.Emit.Lightweight and my issue will be fixed.

Ok cool! Sounds like this problem will eventually take care of itself. 😆

no, if you release a .20.beta1/2/3 , users can easily install .20 final without issues

I wasn't clear enough in my description of my decision/process there, apologies. I was attempting to fix a problem without the use of beta/preview monikers, as deployment is configured by repo owners and I am but a contributor for this repo. So, making best with the resources/powers available to me, if that makes sense. If we want to use a beta/preview moniker (which is indeed the preferred way to go IMO), @wojtpl2 will have to get involved to assist in doing so.

MagicAndre1981 commented 5 years ago

ok. @wojtpl2 can you please release the .20 with the implemented dependency fix as stable to nuget?

Mike-E-angelo commented 5 years ago

Ah, apologies for the confusion, @MagicAndre1981 ... @wojtpl2 would be needed for a preview/beta moniker. I can take care of regular/public deployments. I was going to do .20 over the weekend, but Real Life ™. 😆 I will take care of this now.

Mike-E-angelo commented 5 years ago

OK @MagicAndre1981 2.1.20 is now available for you: https://www.nuget.org/packages/ExtendedXmlSerializer/

MagicAndre1981 commented 5 years ago

thanks, now I can disable your preview feed in nuget.

I'll leave it open until .net 3.0 is RTM/RTW/GA to see how dependencies are organized in final bits.

Mike-E-angelo commented 5 years ago

Sounds good to me, @MagicAndre1981. Pretty much what I was thinking. 😊

MagicAndre1981 commented 5 years ago

ok, for normal .net framework 4.8 the unneeded System.ComponentModel and System.Xml.XmlSerializer are also added

When you look at packages\System.ComponentModel.4.3.0\lib\net45 you see it is empty placeholder _._

Your fix causing it. Also check for normal .net 4.6/4.7 or in my case 4.8

MagicAndre1981 commented 5 years ago

I think you can simply remove both dependencies. They are part of .net standard 2.0 and full .net framework.

Mike-E-angelo commented 5 years ago

Weird, @MagicAndre1981! I remember encountering build errors by not having those references present, but they appear to be working now. Might have been due to wonky project system state issues, which have been pretty prevalent since its release.

I've made a commit with those removed. Please continue letting me know of anything further that you find. 👍

MagicAndre1981 commented 5 years ago

.net core 3.0 is now out. Update the dependencies to latest versions and provide a test build to see how it looks now.

Mike-E-angelo commented 5 years ago

Ok cool! I have been swamped with Real Life™ lately... Will try to see if I can get something going tomorrow, and if not, on Wednesday for sure.

Mike-E-angelo commented 5 years ago

Alright @MagicAndre1981 check out our preview feed now: https://www.myget.org/F/wojtpl2/api/v3/index.json

See if that helps this issue any. Not sure if the build on the server needs to be updated to 3.0 or anything. If it does, that complicates this as I do not have access to it.

The other gotcha here is that I am in the middle of a feature with #264 and cannot release to public nugets until I get that to a place that I am happy with ... yeah, I am breaking some serious CD/CI rules with that. :P

In any case you should be able to get the build from the preview to see where this stands now.

MagicAndre1981 commented 5 years ago

I don't see an update in myget feed. Have you forgotten to increase version number from .22 to .23?

Mike-E-angelo commented 5 years ago

hah... maybe. 😁😇 I have posted a new commit that does so. You should see it available in a few minutes or so.

MagicAndre1981 commented 5 years ago

works fine 💪

I no longer get the old .net core 1.x packages offered. Only the real required packages in packages.config:

<package id="ExtendedXmlSerializer" version="2.1.23" targetFramework="monoandroid81" />
<package id="System.Collections.Immutable" version="1.6.0" targetFramework="monoandroid81" />
<package id="System.Interactive" version="3.2.0" targetFramework="monoandroid81" />
<package id="System.Reflection.Emit.Lightweight" version="4.6.0" targetFramework="monoandroid81" />

But only update System.Interactive to 4.0 which was released today and has .net standard 2.0 dependency metadata

Mike-E-angelo commented 5 years ago

Woohoo! That was a better result than I was expecting. 😆 I've gone ahead and updated System.Interactive as well.

The kicker here now is that I still need to finish #264 before I can publish this to the public feed. The kicker's kicker is that I am leaving on vacation next week for a couple weeks and not sure if I will be able to get it in a state where it can be "done" but we'll see...

MagicAndre1981 commented 5 years ago

you can make a new brunch which only includes the nuget updates and ignores the other changes and release those bits until the other thing is ready.

Mike-E-angelo commented 5 years ago

Yeah I thought of that... problem is my Git isn't too good and we deploy from master. So it would mean creating a new branch called Issue-264 or some such and moving the commits made around that (that are already in master) over to it. Sounds like a lot of work. 😉

MagicAndre1981 commented 5 years ago

ok, I did this and only reverted this 1 commit from master and moved this commit into its own brunch and merged the nuget changes from master.

Took me about 10 minutes with TortoiseGit

Mike-E-angelo commented 5 years ago

Cool... thanks for the inspiration, @MagicAndre1981. 😁 I use GitExtensions myself, and I looked into the revert. What I did was:

I think everything should be in order now? 😆 You should see a NuGet package soon: https://www.nuget.org/packages/ExtendedXmlSerializer/

MagicAndre1981 commented 5 years ago

I think everything should be in order now? You should see a NuGet package soon: https://www.nuget.org/packages/ExtendedXmlSerializer/

works fine. Now only the real required packages are added and no longer all the .net core 1.x packages. I'll close the issue now.

Mike-E-angelo commented 5 years ago

Woohoo! Good to hear. Thank you for your contribution to improve the quality and developer experience of this project, @MagicAndre1981!