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

Release 5.0.0 #613

Closed
6 tasks done
jonorossi opened this issue Apr 2, 2022 · 26 comments
Closed
6 tasks done

Release 5.0.0 #613

jonorossi opened this issue Apr 2, 2022 · 26 comments
Assignees
Milestone

Comments

@jonorossi
Copy link
Member

jonorossi commented Apr 2, 2022

@pmorelli92
Copy link

pmorelli92 commented Apr 14, 2022

Hey @jonorossi I don't really know how long will it take for the 5.0.0 to be released, but in the meantime can we get an intermediate version that targets the .NET Standard 2.0 instead of the 1.6 so we can get rid of the CVE-2017-0247 ?

It is a very old vulnerability, but tools are reporting it and security department is not happy.
Releasing the point of history after the merging of #485 should do it most likely.

Thanks!

@Jevonius
Copy link
Contributor

Jevonius commented May 1, 2022

How do we start ticking off more of the boxes?

I’ve updated a few of the test-related nuget packages as part of the net6.0 but what about the rest?

@jonorossi
Copy link
Member Author

jonorossi commented May 9, 2022

CI builds appear to be working well. The AppVeyor build is running tests again .NET Framework 4.?, .NET Core 3.1.24 and .NET 6.0.4.

I've regenerated the nuget.org API token for publishing, hopefully publishing will all still work well. I'll update the changelog before publishing.

Does anyone want to do any further changes or final testing before release?

@stakx
Copy link
Member

stakx commented May 9, 2022

Does anyone want to do any further changes or final testing before release?

I'd say we're good to go! 🚀

Getting the TFMs set up was perhaps the single most important work left, and that has now just been completed.

I'll be happy to start working on some of the reported bugs soon, but that can happen anytime and isn't tied to v5.

@jonorossi
Copy link
Member Author

Thanks @stakx.

Those that intend to use the .NET 6 build straight away, I'd be great if you could test the latest build:
https://ci.appveyor.com/project/castleproject/core/build/job/1ueqyrfx9hk9n67f/artifacts

In 24 hours I'll publish the release.

@Jevonius
Copy link
Contributor

Jevonius commented May 9, 2022

Is it feasible/easy to push out a 5.0.0-preview1 to NuGet? Much easier for me to pull that into various solutions and run tests against.

@jonorossi
Copy link
Member Author

Is it feasible/easy to push out a 5.0.0-preview1 to NuGet? Much easier for me to pull that into various solutions and run tests against.

Great idea. Just published v5.0.0-beta001.

It would be good to get at least one mocking library upgraded to run their test suite (@stakx any chance you've got time to create a branch for Moq so its test suite gets run before we publish final?).

@stakx
Copy link
Member

stakx commented May 10, 2022

@jonorossi, done. Moq's test suite still runs fine with v5, so there appear to be no regressions. I will still have to go through Moq's open DynamicProxy-related issues to see which ones are now fixed as expected, but so far so good. Update: Those open Moq issues that I expected to get fixed by v5 will indeed be fixed. All good. ✔️

@stakx
Copy link
Member

stakx commented May 10, 2022

NSubstitute's test suite also appears to run fine with v5 (assuming I've performed the version upgrade correctly).

@stakx
Copy link
Member

stakx commented May 10, 2022

I was going to give FakeItEasy a quick try as well, but even very basic, seemingly unrelated unit tests broke after the upgrade to v5, so I am assuming that I didn't perform the upgrade correctly; some problem with adjusting the TFMs, I guess. I'll have to skip this one.

@jonorossi
Copy link
Member Author

@stakx 🍻

@Jevonius Let me know when you've done your tests

@Jevonius
Copy link
Contributor

Don't wait on my account, turns out we make use of code that #563 got rid of, so fighting against that to get things building/running. Might take me a day or two.

@Jevonius
Copy link
Contributor

Looks like Windsor gets broken. Not sure of the exact cause, but removed functionality (such as #563) seems a likely candidate.
Code that blows up:

var container = new WindsorContainer().Install(FromAssembly.Named("Does.Not.Matter"));

Gives an exception along the lines:

System.TypeLoadException : Could not load type 'Castle.Core.Internal.PermissionUtil' from assembly 'Castle.Core, Version=5.0.0.0, Culture=neutral, PublicKeyToken=407dd0808d44fbdc'.

Replicating in LinqPad gives a slightly different type failure:

Could not load type 'Castle.Core.Pair`2' from assembly 'Castle.Core, Version=5.0.0.0, Culture=neutral, PublicKeyToken=407dd0808d44fbdc'.

Tests affected by this issue in our codebase are the only ones that started failing upon upgrade to the new version.

@stakx
Copy link
Member

stakx commented May 10, 2022

@Jevonius, I'm surprised that you even got that far. Did you set up an assembly version binding redirect thingy for Castle.Core from assembly version 4 to 5? I thought Windsor would be compiled against v4, so I would've expected an "assembly not found" error of some kind due to the major version jump. (Though that's perhaps only enforced on the old .NET Framework these days.)

IIRC when we last discussed Windsor in the face of a major version upgrade of Castle.Core, our thinking was that people could stay on Castle.Core v4 until Windsor would get a version upgrade, too. Otherwise we'd have to keep Windsor in sync along with Castle.Core, which back then was out of the question due to our limited resources.

Not sure I remember that entirely accurately though; @jonorossi may have more details about the back story here.

@jonorossi
Copy link
Member Author

jonorossi commented May 11, 2022

IIRC when we last discussed Windsor in the face of a major version upgrade of Castle.Core, our thinking was that people could stay on Castle.Core v4 until Windsor would get a version upgrade, too. Otherwise we'd have to keep Windsor in sync along with Castle.Core, which back then was out of the question due to our limited resources.

@stakx yep, that's my recollection. It's up to Windsor to fix its usage of internal Castle Core classes. There is no expectation that Castle Core won't make breaking changes in a new major version, for example we've documented that Castle Core's Castle.Core.Pair<,> class has been removed, if it's too hard for Windsor to move to Tuple, then we can define the Pair class in Windsor's code base during the upgrade to Castle Core v5.

@Jevonius could you create an issue on the Windsor repo with your findings. Windsor is going to need its target frameworks upgraded too, as Castle Core is no longer supporting .NET 4.5.

The only question I think that remains here is, do we need to push a new patch release of Windsor that prevents use of Castle Core v5 so we don't get a heap of bug reports here? I'm surprised that NuGet would recommend a major version upgrade which is surely to result in breaking changes.

https://github.com/castleproject/Windsor/blob/28e7afaf04462d74d6ad05505dc426b81cf5d9a3/src/Castle.Windsor/Castle.Windsor.csproj#L27

@Jevonius
Copy link
Contributor

Jevonius commented May 11, 2022

According to Nuget, Windsor’s dependency is:
Castle.Core (>= 4.4.1)
It doesn’t have the upper limit of < 5.0.0, so a patch release with that limit might be worth considering until Windsor can be fixed.
Changing the version in your link to ”4.*” ought to do it.

@Jevonius
Copy link
Contributor

This might be heretical, but would there be any advantage to skipping 5.0.0, and jumping to 6.0.0? Windsor could then bump to 6 and make similar changes to its target frameworks.

I always assumed that Castle projects stayed in sync, so was surprised when Windsor jumped to 5. Is there a thread about that somewhere? I couldn’t find one when I looked.

@jonorossi
Copy link
Member Author

Changing the version in your link to ”4.*” ought to do it.

We wouldn't want 4.*, the current build of Windsor likely doesn't work with all 4.x versions which is why it was increased to 4.4.1. NuGet's preference for the lowest version but then always suggesting upgrades ignoring SemVer is always frustrating.

Interesting to see Roslyn and EntityFramework Versions.prop files don't specify a range.

Maybe Windsor should have [4.4.1,5.0) instead?

This might be heretical, but would there be any advantage to skipping 5.0.0, and jumping to 6.0.0? Windsor could then bump to 6 and make similar changes to its target frameworks.

I always assumed that Castle projects stayed in sync, so was surprised when Windsor jumped to 5. Is there a thread about that somewhere? I couldn’t find one when I looked.

I don't see any reason to do that, they are separate projects and have been for over a decade (web site news doesn't go that far back). There is currently no maintainer for Windsor, if no one steps forward to submit the dependency version patch or upgrade it, there won't be any more releases.

@Jevonius
Copy link
Contributor

Jevonius commented May 11, 2022

[4.4.1,5.0) was going to be my original suggestion, should have stuck with it 😆

There is currently no maintainer for Windsor, if no one steps forward to submit the dependency version patch or upgrade it, there won't be any more releases.

I’ll try and find some time to take a look, get an idea what’s involved. We use it for a large chunk of our codebase, and trying to maintain a split of old/new Core versions is going to be a pain as we shift towards net6+.

@stakx
Copy link
Member

stakx commented May 11, 2022

Maybe Windsor should have [4.4.1,5.0) instead?

NuGet's interpretation of semver is ridiculous. Why is this even necessary? 🤯

Sounds good to me, too. Doing a patch release of Windsor with the reference version thus adjusted at least properly documents the status quo.

People will perhaps still run into version conflicts, though; if NuGet updates both packages, one of those will have to be downgraded again in order to resolve the conflict; not sure NuGet is smart enough to see the major version bump as the likelier source of trouble and pick Castle.Core to be downgraded, instead of thinking, "Ooh! If I downgrade Windsor to the previous patch version, every Core version will work.. great!" 🙄

I think the odd issue report about this is almost a given. I'll also try to make some time to get Windsor off those removed types.

P.S.: In my opiniom, this shouldn't stop us here. My vote goes for releasing v5 first, then work on Windsor to make it compatible with v5 (and in the meantime deal with versioning issue reports with a canned response).

@jonorossi
Copy link
Member Author

P.S.: In my opiniom, this shouldn't stop us here. My vote goes for releasing v5 first, then work on Windsor to make it compatible with v5 (and in the meantime deal with versioning issue reports with a canned response).

Just saw your edit as I was writing a reply, I agree. Will release final 5.0.0 tonight.

@Jevonius
Copy link
Contributor

P.S.: In my opiniom, this shouldn't stop us here. My vote goes for releasing v5 first, then work on Windsor to make it compatible with v5 (and in the meantime deal with versioning issue reports with a canned response).

It might be worth a note somewhere, but I agree we shouldn't be blocking and waiting on Windsor. A 5.x bump of Windsor with the new upper bound will be quick to do, and gives a bit of breathing room for a proper fix.

@jonorossi
Copy link
Member Author

  • Tagged v5.0.0
  • Confirmed successfully pushed to nuget.org and indexed
  • Tweeted

@stakx
Copy link
Member

stakx commented May 12, 2022

Thank you @jonorossi for this release, I'm really excited about it! 🍺🍺 I'll soon resume work on some of the pending issues.

@stakx
Copy link
Member

stakx commented May 12, 2022

Also (last but not least!) thank you so much @Jevonius for your significant contributions to this release. That was some solid work there 💪, and you've really helped getting things moving once again. It's much appreciated.

@Jevonius
Copy link
Contributor

I fear you flatter me too much - I'm more cheerleader to your quarterback 😂

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

No branches or pull requests

4 participants