net-commons / common-logging

A portable logging abstraction for .NET
http://net-commons.github.io/common-logging
Apache License 2.0
649 stars 204 forks source link

Could not load type 'Common.Logging.ILog' #74

Closed eman1986 closed 9 years ago

eman1986 commented 9 years ago

I recently updated Common.Logging from NuGet and I now get this error message no matter what I do:

Could not load type 'Common.Logging.ILog' from assembly 'Common.Logging, Version=3.0.0.0, Culture=neutral, PublicKeyToken=af08829b84f0328e'.

web.config is shown looking for the correct version so I'm just stuck on why it's complaining.

eman1986 commented 9 years ago

worth noting I'm using NLOG 3.2 if that is worth knowing.

sbohlen commented 9 years ago

Hmm...we don't (yet) support NLog 3.2, although that's more simply an oversight rather than a deliberate choice -- TBH, I wasn't aware that NLog 3.2 had been released; is this a (relatively) recent developement?

Are you indicating that you had been successfully using Common.Logging 2.x with NLog 3.2 prior and that this is no longer functioning now that you've updated to Common.Logging 3.0? Or only that you're unable to get any version of Common.Logging to work with NLog 3.2?

Either way, I'd expect the only way to get Common.Logging v-Anything to work with NLog 3.2 would be to use a binding redirect (http://msdn.microsoft.com/en-us/library/eftw1fys%28v=vs.110%29.aspx) to resolve the desired NLog 3.2 assembly from the Common.Logging.NLog31 assembly.

Can you advise...

  1. whether you had previously been able to get Common.Logging 2.x to work with NLog 3.2 and this is a new issue introduced by Common.Logging 3.0?
  2. whether you have attempted to use a binding redirect to resolve the NLog 3.2 assembly from the Common.Logging.NLog31 assembly?

Either way, if there is an NLog 3.2 available, we should probably plan to (quickly!) add formal support for it to Common.Logging...

sbohlen commented 9 years ago

Just FYI, we've added an issue (https://github.com/net-commons/common-logging/issues/75) specifically for adding formal support for NLog 3.2.

eman1986 commented 9 years ago

I recently updated common.logging which is when I noticed it.

eman1986 commented 9 years ago

So I reverted Common.Logging to 2.3.1 and I'm back in business. so it seems you not only need to create a NLog32 package you also need to see why version 3 is breaking NLog as the older version NLog 3.2 works

sbohlen commented 9 years ago

I tend to agree, we need to understand what's going on there. If possible, I'd like to try to repro both your success (with 2.3.1) and your failure (with 3.0.0).

Can you advise the following for me:

  1. For the working combination of NLog 3.2 + Common.Logging 2.3.1, did you introduce Assembly Binding Redirects? If not, what was your process for combining the two? Install Common.Logging.NLog31 (which would bring down the dep, on NLog 3.1) and then use NuGet to update the NLog 3.1 to NLog 3.2? Some other process/sequence of actions --?
  2. For the non-working combination of NLog 3.2 and Common.Logging 3.0, can you advise your sim. process for getting to that state?

Under Common.Logging 3.0, the interface ILog is actually not in the Common.Logging assembly so your specific error is probably to be expected. Under Common.Logging 3.0 the ILog interface was moved to the Common.Logging.Core assembly. Its entirely possible that the error you're getting is expected depending on exactly how you attempted to get the combination of Common.Logging and Nlog to the versions you attempted to combine ...

Thanks,

-Steve B.

eman1986 commented 9 years ago

I think that may be my problem, I'll have to adjust my code so its not thinking ILog is where it isn't.

sbohlen commented 9 years ago

Curious: are you trying to resolve ILog based on its assembly (e.g., via reflection) in your code?

Common.Logging.dll (3.0) has a reference to Common.Logging.Core.dll and the NuGet package should have ensured that you got both. The namespace for ILog shouldn't have changed even though the assembly did, so unless you're using reflection that cares about the assembly that contains the type, the intention with the 3.0 update was that you wouldn't have breaking changes in that regard (though of course you would have to recompile your code dependent upon ILog) ... can you advise?

If non-reflection-based access to ILog is broken for you, I fear its broken across the board for everyone and so I'd want to puzzle out the cause and try to release a quick-fix ASAP, of course...!

eman1986 commented 9 years ago

I'm not seeing anywhere I'm trying to use ILog, so that stumps me as to why its complaining. I do have both of those packages installed.

sbohlen commented 9 years ago

Hmmm...have you (re)compiled all the assemblies of your own that contain references to ILog? As mentioned, since its moved from one ass'y to another you would have to recompile your own code that references Common.Logging.dll in order for the type to resolve correctly.

Since all types in .NET are assembly-qualified, ILog from Common.Logging.dll isn't binary-compatible with ILog from Common.Logging.Core.dll even if the shape (interface) of ILog may be the same to a human reading the code. Since the namespace for ILog hadn't changed however, a re-compilation of your code should permit ILog to be resolved again.

Another thought (just brainstorming here):are you certain that its your own code that's raising the issue? Lots of other OSS projects take a dependency on Common.Logging so is it possible that another of your dependencies is expecting the Common.Logging.dll from 2.3.1 (and is sim. expecting it to contain the ILog interface when in fact it no longer does under Common.Logging 3.0)?

For better or for worse, NuGet does some rather "creative" things under the hood with "automatic" binding redirects and its poss. that its added a binding redirect that makes one of your other dependencies redirect its dependency on CL 2.3.1 to CL 3.0, and this would definitely cause your issue because CL 3.0 is not binary-compatible with CL 2.x (due to the introduction of Common.Logging.Core and a few other things)...

rbirkby commented 9 years ago
"The type initializer for 'Quartz.Impl.StdSchedulerFactory' threw an exception."
"Method not found: 'Common.Logging.ILog Common.Logging.LogManager.GetLogger(System.Type)'."

simply because I updated all packages in nuget and got Common.Logging 3

rbirkby commented 9 years ago

Wondering if this would have fixed it?

[assembly:TypeForwardedTo(typeof(ILog))]
sbohlen commented 9 years ago

@rbirkby

Common.Logging 3.0 is the first release since 1.2 that breaks binary reverse-compatibility. This was by design in order to properly address the need to support PCL as well introduce several new (breaking) features.

Following semantic versioning rules, there's no guarantee that a 2.x package can be replaced with a 3.x package (major version changes indicate at least in part an expectation that such a substitution isn't a supported scenario). Specifically re: Quartz.NET, this is a known issue https://github.com/quartznet/quartznet/issues/226 which I understand that they are working on currently (or at least will be shortly).

In the interim, if you are dependent upon any project that is in turn dependent upon Common.Logging 2.x, updating to Common.Logging 3.x is not supported until those other projects similarly update their own dependencies to reflect Common.Logging 3.x's changes.

sbohlen commented 9 years ago

@rbirkby

That actually would have solved that one particular issue (the relocation of the ILog interface (and several others) to the new Common.Logging.Core assembly, but since there are actually other breaking changes that go beyond just relocating these several interface types, we decided not to bother with the type-forwarders since consumers of Common.Logging 2.x would have to recompile anyway to accommodate the other breaking changes.

sbohlen commented 9 years ago

@rbirkby

When you say "I updated all packages in NuGet and got Common.Logging 3.0", I'm curious: did your code have a direct dependency on Common.Logging or only on Quartz.NET (and through that, an indirect dependency on Common.Logging)?

The reason I ask is that NuGet semantic versioning rules should prevent an update to Quartz.NET from cascading through to Common.Logging 3.0 unless you had explicitly added the Common.Logging package to your project for other reasons (e.g. you wanted to use it directly). NuGet should 'respect' the major-version-boundary for Common.Logging (from 2.x to 3.x) and not silently update the Quartz.NET dependency on Common.Logging past the 2.x boundary for you (again, unless you directly added the Common.Logging package to your project yourself for other reasons).

Can you advise --?

eman1986 commented 9 years ago

I looked through my packages and I found that two of them are requiring Common.Logging 2.3.1

I'm upgrading them to version 3 to see if that fixes things, which I'm hoping it does.

rbirkby commented 9 years ago

I've just checked the packages.config, and there is an entry for Common.Logging in it, even though it was never used directly.

sbohlen commented 9 years ago

@eman1986

Thanks, please do circle back and let us know the results; if still not a success we may have a larger issue to address :)

sbohlen commented 9 years ago

@rbirkby

Thanks; let me do some investigating to understand just how SemVer for dependent packages is supposed to work. My understanding based on http://docs.nuget.org/docs/reference/versioning is that its not necessary to state an 'upper bound' on the version of dependent packages because the semantic versioning boundary (e.g., major-version value) is always an implied upper-bound.

So when saying something like this (which is the 'common' format for dependencies that most NuGet package authors tend to stick with) ...

<dependency id="ExamplePackage" version="1.3.2" />

...its implied that it won't accept/update to ExamplePackage versions of 2.x and so on (because semantic versioning rules imply that v2.x would likely contain breaking changes). However, based on your observations, it sounds as if this is not how updates interpret the "=" in the version-spec for dependencies.

Since this honestly makes me wonder the value/role of semantic versioning in NuGet in the first place (i.e., , if not as a default way to puzzle-out what can and can't be updated without introducing a breaking change then what is its purpose?), I want to do a bit of investigation on my end to understand why you're seeing the update behavior you are reporting here ...

eman1986 commented 9 years ago

updating the helper packages I use fixed my problem.

sbohlen commented 9 years ago

@eman1986

Excellent -- so more of a run-of-the-mill .NET assembly-resolution-version-conflict than anything wrong with Common.Logging 3.0 + packaging per se. Happy to hear that you solved your issue (and that nothing is more broadly wrong with Common.Logging 3.0 in this case as well).

Cheers,

-Steve B.

rbirkby commented 9 years ago

Just to confirm, my code had a direct dependency on Common.Logging (which was not used directly) in addition to a direct dependency on Quartz.

From packages.config:

  <package id="Common.Logging" version="2.3.1" targetFramework="net45" />
  <package id="Quartz" version="2.3" targetFramework="net45" />
sbohlen commented 9 years ago

@rbirkby

Thanks for the additional info. My research has (apparently) turned up that there is a difference in behavior WRT dependency version-handling during updates depending on the version of the NuGet client you're using.

My initial research suggests that NuGet 2.2 and prior will update all dependencies to "whatever is the latest version available in the NuGet package feed, disregarding sematic versioning rules" whereas NuGet 2.5 and later will properly respect semantic versioning and shouldn't be giving you Common.Logging 3.0 so long as Quartz.NET states a dependency on only Common.Logging 2.3.1.

What version of NuGet are you using to perform the updates?

rbirkby commented 9 years ago

@sbohlen 2.8

If my project directly references Common.Logging, why wouldn't it offer the update?

PM> get-package -Updates

Id                             Version              Description/Release Notes                                                                                                                                                                                                               
--                             -------              -------------------------                                                                                                                                                                                                               
Common.Logging                 3.0.0                Common.Logging library introduces a simple abstraction to allow you to select a specific logging implementation at runtime.   
sbohlen commented 9 years ago

Ok, that's clearly current enough to be supposed to be respecting SemVer rules, as far as my research turned up.

TBH my expectation was that NuGet should detect that you have another package (e.g., Quartz.NET in this case) that has an upper-bound on the version of Common.Logging that it will support of <3.0 based on SemVer rules and that updating to Common.Logging >=3.0 will break that dependency (again, based on SemVer rules) so it shouldn't offer it. Since packages don't get referenced in a vacuum and often have shared- / cross-dependencies (as is the case in your scenario), only by analyzing the entirety of the package dependency graph for your project/solution can NuGet make an 'informed decision' about what to update.

IMO there's little/no point in even bothering to include version info in the dependencies graph of the NuSpec files if NuGet isn't going to use that info to puzzle out what is and isn't a valid update for packages...after all, as a human being I could read each of the NuSpec files myself and determine that updating Common.Logging to anything >= 3.0 was going to be a bad idea based on the dependencies of others of my packages I'd referenced. I'd expect NuGet to do this for me using a (relatively) straightforward algorithm to analyze the NuSpec files already on disk in the /packages/ folder and only offer me 'valid' updates (or, at a minimum, to at least WARN ME IN BIG GIANT TEXT that what I'm about to do is likely to result in a broken state for my code).

TBH, my understanding was that this exact scenario (updating packages and avoiding breaking changes) was the whole point of NuGet supporting SemVer in the first place...somewhat disappointing that this appears to not actually be the case and to discover that NuGet is really just a (comparatively) brainless way to get binaries over the internet wrapped in ZIP files :-/

Ugh.

rbirkby commented 9 years ago

Quartz has an upper bound? https://www.nuget.org/packages/Quartz/ says:

Dependencies

Common.Logging (≥ 2.3.1)

sbohlen commented 9 years ago

You can't trust the NuGet.org website; it interprets a NuSpec entry of "=2.3.1" as ">=2.3.1" for the sake of display on the site (which, frankly, reinforces the impression that SemVer isn't considered -- essentially the behavior you are seeing here). However, if you look at the actual NuSpec file for Quartz.NET https://github.com/quartznet/quartznet/blob/master/Quartz.nuspec you'll see that its stated as "=" (although of course since they just updated to CL 3.0, it now reads "=3.0" ).

Based on SemVer rules, my expectation is that "=2.3.1" should be interpreted as ">=2.3.1, <3.0".

rbirkby commented 9 years ago

Quartz (before yesterday) said:

<dependency id="Common.Logging" version="2.3.1" />

According to Nuget docs, this is a lowerbound. It is not an exact version.

sbohlen commented 9 years ago

Right, it used to say 2.3.1 until very recently, but its always been stated as "=". You're right of course that NuGet has always interpreted "=" to mean ">=" (e.g., a lower-bound rather than an exact equivalency) but this isn't an issue about the lower-bound, its an issue about an implicit upper-bound (that should be implied by Semantic Versioning rules).

That same doc you point to references the SemVer spec (http://semver.org/) which offers the following:

Given a version number MAJOR.MINOR.PATCH, increment the:

MAJOR version when you make incompatible API changes,
MINOR version when you add functionality in a backwards-compatible manner, and
PATCH version when you make backwards-compatible bug fixes.
Additional labels for pre-release and build metadata are available as extensions to the MAJOR.MINOR.PATCH format.

If NuGet supports SemVer and I increment the MAJOR version number, per the spec this is my ability to signal via the version number to all consumers of my package (both human and machine) that I've introduced an incompatible API change. If I've rev-ed the MAJOR version number, there's not only no expectation of compatibility, there's a manifest expectation of INCOMPATIBILITY, else I'd not have incremented the MAJOR version. In this context, NuGet updating a package that says "=2.3.1" to 3.0 is (IMO) not something that should happen (again, at least not without some seriously-invasive UI "are you really sure you want to do this, its likely that things won't work if you proceed!" element presented to the user.

After further investigation, I'm approaching the conclusion that what I now think the NuGet team meant in referencing the SemVer spec was just that last part re: Additional labels for pre-release and build metadata are available as extensions to the MAJOR.MINOR.PATCH format. and not in fact the rest of the SemVer spec.

This means that package authors really interested in respecting SemVer for packages upon which their packages depend should get into the habit of specifying version dependencies according to the following scheme:

<dependency id="DependencyPackage" version="[n,n+1)" />

...which means "anything >= n but also < n+1 and so then NuGet would start to behave per the full SemVer spec (as far as I can tell in re: reviewing the docs more carefully).

If everyone defined their dependency on Common.Logging 2.3.1 as follows ...

<dependency id="Common.Logging" version="[2.3.1,3)" />

...then problems like the Quartz.NET issue would be avoided (as far as I can tell). Unfortunately, of course, this becomes an education issues for devs of packages that reference other packages that are following SemVer rules rather than anything that we on this project can control/fix per se :(

I believe the error here is on my part in re: misunderstanding that NuGet doesn't support SemVer per se but has instead selectively adopted just the part about suffixes to version numbers being used to indicate a pre-release state. This is somewhat disappointing to me as respecting the rest of the SemVer spec would make for a much 'smarter' NuGet client able to make much better-informed decisions in re: what packages can/can't be updated safely, but as this thread seems to have validated, that's not (apparently) a feature that the NuGet client offers (at least not as of today).

If I had to guess, I'd say that this is probably because NuGet hasn't had even the nominal support for the "suffix=pre-release" behavior since its inception and so not all package authors use SemVer in their versioning schemes. This makes having NuGet respect SemVer in its entirety at this point a difficult breaking change for package authors and consumers to have to deal with.

The problem, however, is that right now the onus is on the authors of packages that in turn depend on packages that follow SemVer to make the changes rather than the author of the package that supports SemVer to make the proper changes. It would (probably) be a better strategy for there to be a sort of an optional <ThisPackageRespectsSemVer="true" /> node in the NuSpec file that package authors who support SemVer in their packages could add as a signal to the NuGet client dependency-resolver that any packages that consume this package should NOT update past their MAJOR version value.

I suspect that's a bit pie-in-the-sky, but at least it would permit authors of packages wishing to support the full SemVer spec to 'opt-in' to having the NuGet client cooperate with their intentions when resolving packages that in turn depend upon their own package.

rbirkby commented 9 years ago

That's a great write-up. Implied by what you say, I guess the nuspec for Common.Logging.Log4Net1213 is also wrong and instead of:

<dependency id="Log4Net" version="1.2.13" />

should use:

<dependency id="Log4Net" version="[1.2.13, 2)" />
sbohlen commented 9 years ago

@rbirkby

I think you might be correct ... I'm doing some additional due-diligence right now to attempt to (re)validate that my understanding of the behavior of NuGet in re: SemVer is as outlined above. If I'm able to confirm that to be the case then I think we should consider making a change to the dependency entries in our own NuSpec files.

The trick, however, is that AFAICT from past experience Log4Net itself doesn't have a history of following SemVer, making a hard-coded SemVer-based upper-bound like that somewhat problematic (at least in re: Log4Net).

The more I think about it, the more I start to actually think that the entirety of the dependencies stated by Common.Logging. packages is wrong-headed on its face: since each package is stated to work with only the target adapter version, I wonder if we should consider a NuGet-enforced expression of that restriction in re: something more like this ...

<dependency id="Log4Net" version="[1.2.13]" />

...which expresses "I only accept exactly version 1.2.13". Since that really is the intent behind the Common.Logging.Log4Net1213 package anyway (?)

Wonder what consumers would think of that more restrictive/explicit approach? In theory users could introduce their own Assembly Binding Redirect statements if they wanted to "manually" shim in e.g., Log4Net 1.2.14 at some future pt betw. the time that Log4Net 1.2.14 was released and Common.Logging.Log4Net11214 was released to support it formally, but at least this proposed constraint would prevent NuGet from trying to do so on their behalf if/when they try to just update Log4Net 1.2.13 to Log4Net 1.2.14 directly...

Thoughts on this, anyone --?

sbohlen commented 9 years ago

Interestingly, I just did a cursory look through a few of the NuSpec files for some of our adapters that support older (not latest) logging frameworks. The Log4Net1210 dependency https://github.com/net-commons/common-logging/blob/master/src/Common.Logging.Log4Net1210/Common.Logging.Log4Net1210.nuspec actually does use the `=[1.2.10]" exact match, presumably because we'd figured as we moved forward to later adapters for later versions we wanted to 'lock in' that dependency at a specific version. However we failed to do the same thing for Log4Net1211 or Log4Net1212 when they too became superseded by later adapters.

So we're pretty inconsistent ourselves here, it appears :( At a minimum, we should (probably) go back to all of the adapter NuSpec files for superseded versions of logging frameworks and update their version dependencies to use the =[...] exact version match syntax.

Whether or not we choose to do the same for the 'latest' adapters is perhaps a slightly different question, I guess ...

lahma commented 9 years ago

I think log4net is quite the different beast here. It has notorious history of breaking changes between versions, like the strong name change fiasco. Usually I'd be fine with updating to newer minor release with binding redirect but in case of log4net it could be a truly breaking change. That could be one (?) of reasons why the explicit restriction was made.

I've ran happily log4net1211 integration with log4net 1213 via binding redirect, less hassles and no need to uninstall/install new version of integration package.

Two-sided coin with pluses and minuses. In a perfect world I could use CL.NLog3 with all version of NLog 3.x without constant need to push updates via NuGet / waiting for updated integration lib. The first breaking release that would require CL.NLog33 of course would be awkward... :smile:

lahma commented 9 years ago

As a side note, Quartz 2.3.1 was released yesterday and contains the new Common Logging 3.0.0 dependency (open upper-bound).

mperdeck commented 9 years ago

I am the author of JSNLog (jsnlog.com; github.com/mperdeck/jsnlog; nuget.org/packages/JSNLog). My package needs to do logging, which it does via Common.Logging. Most users of JSNLog use the (precompiled) NuGet package, rather than (re)compiling the source from Github.

I guess not surprisingly, my users are now reporting the same "Method not found: 'Common.Logging.ILog Common.Logging.LogManager.GetLogger(System.String)'" problem: https://github.com/mperdeck/jsnlog/issues/55

I'm sure the same goes for all third party NuGet packages that use Common.Logging to do their logging. I've always seen Common.Logging as ideal for this situation, because it allows the NuGet package to do its logging while giving its users the flexibility to send the log messages to whatever logging package they happen to be using.

I would like to suggest to release a new temporary minor version of Common.Logging 3 that is binary compatible with Common.Logging 2. I understand that one way to achieve this quickly would be to reintroduce ILog in Common.Logging.

I appreciate this is not necessarily the direction you want to take longer term. However, it will solve the immediate problem of breaking packages while you work out a long term solution. Common.Logging 2 has had over 600,000 downloads, which potentially represents a lot of affected people.

sbohlen commented 9 years ago

Yeah, unfortunately even if we wanted to relocate ILog (and the other interfaces) back into the Common.Logging assembly that wouldn't by itself solve the issue (for you or other consumers of Common.Logging).

The "cannot find interface ILog"-related run-time errors are only the first level of breaks with compatibility that users encounter (and so it understandably appears that just relocating the interface back would 'solve' the problem). However, moving the interfaces out of Common.Logging.Core and back into Common.Logging would only result in a "you failed to implement method blah-blah on interface so-and-so" errors. This is the reasoning behind our choice not to use the TypeForwardedTo attribute (http://msdn.microsoft.com/en-us/library/ms404275%28v=vs.110%29.aspx). Forwarding the type resolution would only solve the initial incompatibility. Ultimately, there's no getting around the fact that Common.Logging 3.0 is not a binding-redirectable/binary-compatible replacement for Common.Logging < 3.0.

We tried to make these changes in the most responsible/least-disruptive manner possible. These breaking changes were intentionally minimized so that (in almost all mainstream usage cases) only a recompile is required by present consumers of Common.Logging < 3.0 when moving to 3.x. Unfortunately, the nature of these breaking changes precludes our making them in such a manner that binary compatibly could be maintained (thus permitting a swap-in replacement by only a binding redirect).

Based on my own investigations along with the empirical evidence of both package authors such as yourself that depend upon Common.Logging as well as direct consumers of Common.Logging, NuGet clients do not presently provide any protection (by following Semantic Versioning rules) against users updating packages and ending up with breaking changes.

Until the NuGet clients properly respect Semantic Versioning in some meaningful manner that properly supports introducing breaking changes across MAJOR version numbers (as we have done here), the only advice we can provide to package authors such as yourself is that if you are not interested in releasing an update to your own package that is recompiled for Common.Logging 3.x, you should instead release an updated package of your own with an explicit upper-bound on the version of Common.Logging which your package supports.

In your case, something like the following would solve the problem for your users:

<dependency id="Common.Logging" version="1.2, 3)" />

...which effectively signals to the NuGet client that your package support Common.Logging >= 1.2 but < 3.0. Since this actually is what it sounds as if you intend to support for the time being, stating it explicitly as above is the best option we can offer you given the seeming constraints in the present behavior of the NuGet client in re: Semantic Versioning.

Note that my actual advice to you would be to instead update your own package to Common.Logging 3.0 and simply recompile it to release an update that does work with Common.Logging 3.x (as was mentioned earlier in this thread was done by e.g., Quartz.NET). But if you choose not do so then updating your package dependency graph as suggested above to match explicitly the range of Common.Logging versions you do support is literally the only other viable option.

rbirkby commented 9 years ago

Interestingly, the nuget recommendation seems to have been not to specify an upper bound:

In most cases, simply specifying a minimum version is the way to go, as illustrated above. This does not imply that upper bounds shouldn't be specified in some cases.

In fact, an upper bound should be specified whenever a component is known not to work past a certain version of a dependency.

...

The rule of thumb here is that a dependency version is “innocent until proven guilty”, and not the other way around.

sbohlen commented 9 years ago

@rbirkby

Yes, that would seem on its face to also imply that its the responsibility of the consumer of a package to guard against breaking changes by specifying an upper-bound when stating a dependency.

FWIW, I've attempted to condense much of this thread into a single discussion item on the NuGet CodePlex site here https://nuget.codeplex.com/discussions/578411 so perhaps we'll get an official response from the NuGet team re: just what the thinking is in re: how package authors are anticipated to best deal with this (and whether the many assumptions cataloged in this thread are in fact correct).

alexminza commented 9 years ago

@sbohlen

For types that were simply moved to the Common.Logging.Core assembly (like ILog) .NET Type forwarding could be used to minimize the breaking of existing deployed assemblies/packages: https://msdn.microsoft.com/en-us/library/ms404275.aspx

rbirkby commented 9 years ago

@alexminza Already discussed. https://github.com/net-commons/common-logging/issues/74#issuecomment-69961388

sbohlen commented 9 years ago

Just to update this thread in the interests of being comprehensive, I've now determined/confirmed that there is in fact no way that NuGet can protect against introducing breaking changes during an UPDATE.

The package dependency graph (e.g., as identified in the .nuspec file for each package) is only inspected at initial package install. At that time, if someone had specified e.g.

<dependency id="Common.Logging" version="1.2, 3)" />

for their package, then that upper-bound would have been respected.

However, once the 'root package' and all its dependencies are first installed, they are each (all of them, regardless of parent-child position in the dependency hierarchy) added to the packages.config file. The packages.config does contain the current version of each package, but doesn't contain any version constraints information on each of the packages.

When you ask NuGet to update packages, it does so by inspecting only the packages.config file rather than the individual .nuspec files in each installed package. Since the packages.config file lacks any understanding of upper-bound constraints, it not surprising that this may introduce breaking changes for consumers of these packages -- even if upper-bounds were specified, they are only interrogated during install and not update.

In other words, the dependency hierarchy is only 'understood; by NuGet at initial install; after that, the hierarchy is 'flattened' into the packages.config file and NuGet effectively pretends that version constraints don't exist/haven't been expressed anywhere.

Thus, even if NuGet did understand Semantic Versioining, there's no way for a package author to express that they are following it in such a way that NuGet clients could properly respect that when searching for 'valid' updates to packages.

As such, I'm going to close this issue since effectively NuGet is behaving as designed and there's nothing that we as package authors can do to mitigate this behavior when introducing breaking changes to our packages.

alexminza commented 9 years ago

Here's how Microsoft does it - https://www.nuget.org/packages?q=Microsoft.AspNet.WebApi Basically, they create new packages for major versions to avoid breaking compatibility.

sbohlen commented 9 years ago

Its interesting that you identify this approach, because in a sense its the same thing we here concluded some time ago when we switched from e.g., Common.Logging.NLog (which didn't indicate which specific version of NLog it requires) to e.g., Common.Logging.NLog20, Common.Logging.NLog30 which were version-specific (see https://github.com/net-commons/common-logging/wiki/Common.Logging-Packaging-and-Versions).

I am a bit uncertain that I can confirm your observation though... based on e.g., this package from the listing to which you're referring https://www.nuget.org/packages/Microsoft.AspNet.WebApi/5.2.3-beta1 it seems (to me) as if its actually not the case that MSFT is doing what you suggest (?)

If I look at the Version History section of that page it looks to me as if this same package used to be "just WebAPI" (presumably the original WebAPI with an implicit "v1") when it had a package versions of e.g. 4.0.20710. Are you saying that there weren't any breaking changes between e.g., 4.0.20710 and any of the WebAPI packages listed there as "WebAPI 2.2" (e.g., package version 5.2.2)? I'm not a (frequent) user of WebAPI, but along the way from "v1" to "v2" didn't Microsoft rev the .NET framework version required for using this package? Surely even if the API didn't change, that would constitute a breaking change...?

Even if you're right and I'm misunderstanding the version history of this specific package I've picked as an example, I'm not certain this approach would ever really be feasible for package authors other than Microsoft. The basic issue I'd see package authors struggle with in this scenario is discoverability of updates. If/when MSFT releases a new version/package of e.g., WebAPI, there's a lot of ways people will become aware that an update is available which they'd want to search for. Other package authors cannot of course muster a similar public-relations effort around their releases :)

If I look at another NuGet package with significant adoption like e.g., JSON.NET (https://www.nuget.org/packages/Newtonsoft.Json/ with 10M downloads to our 680K) I don't see that team following this strategy of new package for new major version (probably for that very reason -- consumers of even a package that popular aren't visiting the project website frequently enough to learn of updates other than through their NuGet clients).

In the end, I think the only conclusion here is that NuGet is useful as a package fetcher that pulls DLLs wrapped in ZIP files from a centralized, searchable online repository but that its unfortunately not really a terribly effective package manager, useful in playing much of a role once you have used it to acquire the initial assemblies :(