unosquare / embedio

A tiny, cross-platform, module based web server for .NET
http://unosquare.github.io/embedio
Other
1.45k stars 175 forks source link

EmbedIO 3.5.0 NuGet package references wrong/old version of Unosquare.Swan.Lite #569

Closed rbdavison closed 1 year ago

rbdavison commented 1 year ago

The EmbedIO 3.5.0 package references Unosquare.Swan.Lite version 3.0.0 but the code requires 3.1.0

Errors similar to the following are reported at build time.

The type 'ConfiguredObject' is defined in an assembly that is not referenced. You must add a reference to assembly 'Swan.Lite, Version=3.1.0.0, Culture=neutral, PublicKeyToken=null'.

Installing Unosquare.Swan.Lite 3.1.0 resolves the build issue. But this shouldn't be required.

nathan130200 commented 1 year ago

Same problem

image

senbagaraman04 commented 1 year ago

Same issue !!, not able to update. Also, why there is no 3.5 in github while the nuget has the version ?

db2222 commented 1 year ago

@rdeago @geoperez I believe something got muddled up in the release 3.5.0. Seemingly 4.0.0rc was released under that number? See https://nuget.info/packages/EmbedIO/3.5.0. grafik

What I don't grasp at the moment is that Swan.Lite 3.1.0 is referenced but it still looks for >= 3.0.0. grafik

Could the package be released again? As far as I understand it should be based on the branch v3.X.

Furthermore I have 2 questions:

Thank you in advance!

db2222 commented 1 year ago

@rdeago @geoperez I believe I fixed it for both branches. Could this please be merged before a new version is uploaded.

db2222 commented 1 year ago

I only realized now that the project uses Directory.Packages.props. There the correct version is already defined. I have therefore no clue why version 3.0.0 is taken. But my fix attempt wasn't correct.

rbdavison commented 1 year ago

Perhaps this might be an appropriate time to review your build and verification process before packages are released. it sure sounds like there isn’t any verification process before release.

Also it might be noted that it’s been almost two months since the release was made and this issue raised.

rdeago commented 1 year ago

I believe something got muddled up in the release 3.5.0.

For sure. I just wish I knew exactly what.

Seemingly 4.0.0rc was released under that number?

Version 3.5.0 is a backport to branch V3.X of fixes that were already in master. See #565 for details.

rdeago commented 1 year ago

Unfortunately my hands are tied when it comes to the release process. @geoperez seems to be our only hope.

db2222 commented 1 year ago

Seemingly branch 4.0.0 was released. In that branch the older Swan.Lite version was referenced:

<ItemGroup>
    <PackageReference Include="Unosquare.Swan.Lite" Version="3.0.0" />
  </ItemGroup>

So this explains the issue from this ticket.

But what I cannot find is "4.0.0-rc1". So the "rc1" part. Could it be the case that at some point this year there was a other branch than v4-old for version 4? Maybe that was used for this release but now it got deleted.

In any case. I believe it all would be fixed if a new version got released based on branch v3.X. Or is master correct? It also is version 3.5.0 but there are different commits. Seemingly there are different expectations what belongs in which branch?

rdeago commented 1 year ago

@db2222 I'm sorry that the somewhat messed-up status of this repository (for which I take full blame) got you confused.

Seemingly branch 4.0.0 was released.

The released DLL is definitely version 3.5.0, as committed on the v3.X branch. What's wrong is that the package references the wrong version of a package. It's as if someone messed with the .nuspec file, which looks kind of weird - but then again, I know nothing about EmbedIO's release process.

But what I cannot find is "4.0.0-rc1".

There is no 4.0.0-rc1 except in my dreams, in which I have the time to finish it.

Could it be the case that at some point this year there was a other branch than v4-old for version 4?

I'm sure not. The version 4 work-in-progress has always been in master. This year I had to reboot my work on v4 completely, branching a new master from v3.X; v4-old is the old master branch, left there for reference, in case it still contains something valuable that I can port to the new master.

In any case. I believe it all would be fixed if a new version got released based on branch v3.X.

I agree with you here. We need a 3.5.1 package released, with no code changes except for the version bump, and the 3.5.0 package deprecated. Unfortunately, the most I am allowed to do is open a PR that updates the version number. The rest is up to @geoperez.

To recap:

Again, I'm sorry for the status of the branches. Version 3 should probably be in master and there should be a v4-develop instead of the current master.

geoperez commented 1 year ago

The new package https://www.nuget.org/packages/EmbedIO/3.5.1

And version 3.5.0 has been deprecated.

db2222 commented 1 year ago

@rdeago Thank you for your detailed response.

I don't want to put blame on anybody. Mistakes happen. I was simply intrigued if it could be figured out.

I don't think it could have been branch v3.X. See my screenshot from yesterday. It shows that the DLL inside of the NuGet package is of version 4.0.0.0. That is why I looked for 4.0.0-rc1 (see screenshot). Somewhere at some point that version must have been defined. But I cannot find it anywhere. Even if I go back in the Git history in the 3 different branches (version 3, 4 and master). That's why my idea was that maybe a deleted branch comes into play.

Furthermore just to prove that it's not branch v3.X. You did the following commit https://github.com/unosquare/embedio/commit/43e83682d41f454b8159c38a59c1e53b4053980b in that branch.

But it's not included in the decompiled code (check via dotPeek for instance). grafik

The NuGet package got released in August. So the above commit must have been contained in it if it was based on branch v3.X.


As I'm writing this I saw that a fix was uploaded. So it should hopefully be fixed and all good now πŸ˜€

db2222 commented 1 year ago

@rdeago @geoperez Just tested the new NuGet package 3.5.1. It still has the exact same issue.

db2222 commented 1 year ago

The issue definitely has to do with Directory.Packages.props. This then causes it not to be correctly applied on the NuGet package itself.

grafik

I haven't got any experience with doing it that way instead of defining the NuGet package references inside of the project file.

But according to the blog post from Microsoft (see https://devblogs.microsoft.com/nuget/introducing-central-package-management/) the flag ManagePackageVersionsCentrally should be set inside of this props file.

Currently it is set in Directory.Build.props. Maybe if you move this everything is ok. But I'm just guessing πŸ˜‰

rdeago commented 1 year ago

Just tested the new NuGet package 3.5.1. It still has the exact same issue.

I've been out all day, so I didn't even know there was a new package version.

I downloaded the new package from nuget.org and yeah, the issue is the same: Swan.Lite.dll v3.1.0 correctly referenced as assembly by the DLL, but v3.0.0 incorrectly referenced as package in the .nuspec file.

The issue definitely has to do with Directory.Packages.props.

It definitely does not.

Even if @geoperez was using an old version of the .NET SDK to pack, one that wouldn't recognize the ManagePackageVersionsCentrally flag, it would complain about missing package versions in PackageReference items.

Unless, of course, a recent .NET SDK is used to build and then an old version of nuget.exe is used to pack. Then I'm not sure what the behavior would be. Anyway, a missing package reference version should be interpreted as *, meaning the most recent version, not an arbitrary older version.

This is all mere speculation, of course, as I don't know how EmbedIO packages are built. Between dotnet pack, dotnet msbuild -t Pack, nuget.exe pack - with or without an additional .nuspec file alongside the .csproj - there are so many options, each with so many variables at play, that trying to debug the process abstractly, without even seeing the script / YAML / whatever file that drives it, is a waste of time.

the flag ManagePackageVersionsCentrally should be set inside of this props file

Uhm, no. The flag's purpose is to tell MSBuild (and NuGet) that Directory.Packages.props exists and shall be used. If you put it inside that same file, MSBuild will never read Directory.Packages.props, hence it will not even see the flag.

Currently it is set in Directory.Build.props.

Exactly, so all projects in the repo have the flag set. Otherwise, managing package versions centrally would make little sense.

geoperez commented 1 year ago

I'm using dotnet build -c Release but probably the nuspec is wrong. @rdeago what is the right version of SWAN? 3.1.0?

geoperez commented 1 year ago

I published 3.5.2 using Swan Lite 3.1.0: https://www.nuget.org/packages/EmbedIO/3.5.2

rdeago commented 1 year ago

@geoperez yes, 3.1.0 is the right version of Swan.Lite to use.

The new package (3.5.2) seems to have the correct reference btw.

probably the nuspec is wrong

I just don't understand where the wrong nuspec came from. It should have been automatically generated by nuspec.exe at pack time from the csproj, so why did it have a different version of Swan.Lite?

Or is there some nuspec file "injected" at some point in the release process? If so, it should be in the repository, so it can be kept in sync with dependency versions.

Anyway, now we have a working NuGet package. Thanks a lot @geoperez! As soon as someone else confirms that the new package is fine, I believe this issue can be closed.

rdeago commented 1 year ago

@geoperez please deprecate 3.5.1 on NuGet (and change the suggested alternative for 3.5.0 if possible).

rdeago commented 1 year ago

@db2222 please accept my apologies. Twice.

Once because I hadn't actually looked into the 3.5.0 package. I've done it just now and yes, you were right, the DLL version is 4.0.0, I have no clue as to why. Maybe it was built from the master branch of an out-of-date local copy; in that case, local master would be GitHub's v4-old (boy, did I mess things up! πŸ™ˆ πŸ€¦β€β™‚οΈ). Versions 3.5.1 and 3.5.2 on NuGet were definitely built from v3.X (you can see the version bump commits) so now everything should be fine.

Another due apology because you correctly reported what Jeff Kluge says in Introducing Central Package Management: he suggests to put the ManagePackageVersionsCentrally property in Directory.packages.props. I can assure you, though, that putting the property in Directory.build.props works fine: I've been using central package version management since it was a preview feature (.NET SDK v3.1.300).

BTW if you think about it, how can the SDK even know to import Directory.packages.props if it has to import it to know?[^1]

The answer is that, due to the somewhat convoluted way MSBuild evaluates projects and their imports, it has to import Directory.packages.props anyway and then decide later (i.e., after the first evaluation phase of all the imports) whether to use PackageVersion items or not.

If you want to see the actual MSBuild code that imports Directory.packages.props, look for a file named NuGet.props in your installed .NET SDK. It doesn't mention ManagePackageVersionsCentrally, which is instead used later, in NuGet.targets.

This means, of course, that you can put ManagePackageVersionsCentrally practically anywhere, including in Directory.build.props, or even in every project file; I'm not sure about Directory.build.targets, but it would be a weird place for it anyway. You can also put PackageVersion items practically anywhere, although for the sake of Visual Studio Package Manager you should probably leave them in their canonical place.

The first article about this feature I ever read, back in mid-2020, was this one by Stuart Lang. Stuart suggests to put ManagePackageVersionsCentrally in Directory.build.props, which makes way more sense to me. The very fact that you can put it in Directory.packages.props is sort of a workaround, as opposed to being part of the feature, so I prefer to pretend that I'm actually telling MSBuild whether to import Directory.packages.props or not. This way I can keep ManagePackageVersionsCentrally in the same ProjectGroup where I put other properties like Deterministic, LangVersion, and so on.

TL;DR @db2222 you were right, but what you were reporting is not the only way to manage package versions centrally (IMHO not the best, either). Anyway, kudos to you for the effort you put on this issue! πŸ‘ πŸ‘ I hope you will keep using EmbedIO and lend us a hand in answering issues when you can.

(And please excuse this old grumpy developer who sometimes sacrifices good manners on the altar of conciseness.)

[^1]: This reminds me of some EULAs of shrink-wrapped software (yeah, I'm that old, LOL) that you implicitly accepted by opening the package and couldn't even read without opening the package.

db2222 commented 1 year ago

Sorry, for the delayed response. I was busy with work.

I can confirm it's fixed. The issue can be closed.

The only remaining thing that would be nice is if the latest version is added to https://github.com/unosquare/embedio/releases. This was always done in the past πŸ™‚ Maybe with a hint to #565 and this issue as the release notes.

The other 2 flaky versions can be swept under the rug (nothing to see here, move along) πŸ˜‰

@rdeago Thank you for your detailed responses!

rdeago commented 1 year ago

@db2222 you're welcome.

I just discovered I have the permissions to create a release on this repo, so here it is: https://github.com/unosquare/embedio/releases/tag/v3.5.2