Consider changing [AssemblyVersion] to major version only (4.0.0.0) to save users from having to configure assembly version redirects

[stakx]

Detailed description will follow later. For now, see the discussion in #418 and castleproject/Core#288.

/cc @skovsende, @JohanLarsson

[JohanLarsson]

Do you mean pinning the Castle.Core dependency to 4.0.0?

[stakx]

No, I meant setting Moq's own assembly version to 4.0.0.0 to effectively make .NET's assembly resolution algorithm less fussy, and let NuGet deal with picking correct versions.

I'm still not convinced that pinning NuGet package dependency versions would be a good idea.

[skovsende]

Pinning NuGet versions definitively sounds like a bad idea, and it shouldn't be necessary when Castle.Core sets their AssemblyVersion correct.

[JohanLarsson]

I don't know enough about assembly resolution to know if it will work. Will be nice if it works without redirects for the simple case where the test project has reference to Moq and xUnit and nothing other than Moq references Castle.Core. A pretty common case I think.

[stakx]

After giving this some more thought, I think it would be prudent to change [AssemblyVersion] only to major.minor.0.0 instead of major.0.0.0, and at the same time increase the minor version by 1. This will help prevent worst-case assembly binding redirects such as the following two:

4.0.0.0 - 4.7.127.0  ->  4.0.0.0
4.0.0.0 - 4.7.127.0  ->  4.7.0.0

In both cases, the upper bound of the source version exceeds the target version, thereby potentially giving the (wrong) impression that the binding is set up incorrectly.

If we, for now, increase the minor version and do major.minor.0.0, worst-case assembly binding redirects will look more sane:

4.0.0.0 - 4.7.127.0  ->  4.8.0.0

Moq 5 will be able to safely make the switch to major.0.0.0, that is, [AssemblyVersion("5.0.0.0")].

(The necessary changes should roughly look like stakx@d27688f.)

@skovsende: Does the above make sense to you?

[skovsende]

Depends a little on the timeframe for a Moq 5 release. If it is close, it would make sense, but if you expect there to be a 4.9 and maybe even 4.10, things would start getting weird. Either we keep the 4.8 assemblyversion for those, which would seem weird or we bump the assemblyversion to 4.9 and possibly 4.10 - but then we loose some of the advantage of fixing the assemblyversion.

So in short, unless 4.8 is expected to be the last minor version in the 4-series, I'd just get it over with and set it to 4.0.0.0 - and then people will have to live with some weird assemblyredirects until the offfending packages updates to reference 4.0.0.0.

[stakx]

@skovsende: 4.8 likely won't be the last 4.x release. And true, it would be nice to go all the way to major.0.0.0 but there's two arguments against that:

• 5.0.0.0 is reserved for Moq 5 which is developed as a separate code base, so that's blocked.

• I'm still very reluctant to backstep / decrease the assembly version to 4.0.0.0, the Castle Core folks have done this and it's apparently backfired already with some tooling: see Release 4.2.0 castleproject/Core#306 (comment) -- note how VS or NuGet obviously didn't create a correct binding redirect. I'd like to avoid this happening to Moq users.

So major only isn't an option for the moment, IMHO. You're saying major-minor would lead to weird situations. I agree if this means to stay with 4.8.0.0 forever (until 5, that is). But would it be so bad to have assembly versions 4.8.0.0, 4.9.0.0, 4.10.0.0, etc.? Admittedly it will cause more binding redirects than just a 4.0.0.0 but less than 4.8.0.0, 4.8.1.0, 4.9.0.0, 4.9.1.0, and so on. Wouldn't major-minor only still be a little better than what we have now?

[kzu]

Keeping this in mind, v5 will keep stable assembly version number based on the "base version" defined for GitInfo. See https://github.com/moq/moq/blob/master/src/Version.targets#L56

[stakx]

@kzu - Understood. Of course, you're free to version Moq 5 assemblies that way instead of adopting a 'major.0.0.0' or 'major.minor.0.0' scheme.

I made the decision to adopt 'major.minor.0.0' for Moq 4 after we ran through a longish series of people reporting assembly version / binding redirect problems. (See e.g. castleproject/Core#288.) The Castle.Core folks got it worse than we did, simply because there are probably more libraries sitting on top of Castle.Core than there are libraries sitting on top of Moq.

Also, according to https://github.com/dotnet/coreclr/issues/14263#issuecomment-335975464 the assembly binding policy for the .NET Framework might be relaxed in the future, meaning that a 'major.minor.patch.0' scheme might then not cause as much trouble as it does today.

As things stand today, however, I'd very much advise to go with a 'major.0.0.0' scheme for the assembly version. Seems even Microsoft is doing this.

[kzu]

Well, Roslyn doesn't, for one :). Properly incremented major.minor.patch (as in, not in every commit/build, but explicitly when bumping versions should be fine.

I know newtonsoft.json does it precisely because a lot of libraries depend on it, so it sticks to major.0.0. I wonder why the automatically generated binding redirects wouldn't work. Are the test runner(s) not picking them up properly?

I use the following attributes and they work quite consistently fine (with xunit+TDD.NET anyway):

  <PropertyGroup Condition="'$(IncludeXunit)' == 'true'">
    <AutoUnifyAssemblyReferences>true</AutoUnifyAssemblyReferences>
    <AutoGenerateBindingRedirects>true</AutoGenerateBindingRedirects>
    <GenerateBindingRedirectsOutputType>true</GenerateBindingRedirectsOutputType>
  </PropertyGroup>

[stakx]

I wonder why the automatically generated binding redirects wouldn't work.

@kzu - IIRC, the problems with assembly binding redirects were manifold:

• VS / NuGet in some cases generated wrong assembly binding redirects, or they weren't removed / updated properly when people upgraded packages. It then doesn't help that many people apparently don't really understand how those redirects work, nor how .NET resolves assembly versions (can't really blame them, it's difficult to understand if you've never given ot some thought), so those people got trapped by this.

• Depending on whether you use packages.config or <PackageReference>, VS shows you a flat list of all installed NuGet packages, or (transitive) sub dependencies are hidden (i.e. you'd only see your own, direct package dependencies). Most people probably aren't aware of those two modes. (I wasn't.) In the former case, that would lead to some people trying to explicitly manage Castle.Core (which they have no direct business with), updating it without also updating Moq (or Moq didn't yet have an updated version against the new Castle release), thereby creating the need for a binding redirect, which often VS / NuGet didn't create properly (see above).

• Both of the above problems were made worse by some people still using VS 2015 (for which there are many valid reasons), where NuGet simply doesn't work as well as in VS 2017.

Those issues could theoretically have been solved in part by educating people, which would have quickly started to feel like a Sisyphean task. Making .NET assembly resolution less finicky through major.minor.0.0 was an easy way out. :)

Well, Roslyn doesn't, for one :). Properly incremented major.minor.patch (as in, not in every commit/build, but explicitly when bumping versions should be fine.

Glad to hear Moq 5 won't run into DLL hell!

Btw. do you think it would make sense for Moq 4 to adopt explicitly set patch versions (instead of letting commit count determine them)?