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
Use csproj information for NuGet pack #1189
base: devel
Are you sure you want to change the base?
Conversation
Yeah but that is what I feared. We distinguish here, because I'm fine if this is the only change here; but then we should maybe change "Florian Rappl" to either "AngleSharp contributors" or "Florian Rappl et al." (or something else that seems to reflect this). Rest seems fine to me. Final question: How to make the |
Well if you look at the NuGet page https://www.nuget.org/packages/Quartz you can see that the owner is me and the Quartz organization, however the NuGet package metadata term is author https://nuget.info/packages/Quartz/3.8.1 . So NuGet experience considers author the owner.
So changing this would also change (at least) the NuGet web UI to show contributors as owners. I get the idea if distinction, but not sure if it's worth the hassle. I haven't seen packages listing contributors on NuGet pages, see NUnit for example https://www.nuget.org/packages/NUnit . Only specific people are listed as authors/owners, but the copyright notice includes the contributors.
👍🏻
This is definitely something hard to achieve with just the |
|
The range is restore guidance for library's own compilation that won't reflect to produced nuget package/its internal nuspec definition. |
Yes I know this - I was not talking about how NuGet interprets it, but how we use these fields (and that for me its important that the contributors are mentioned in some way in the metadata; how that is interpreted / represented on the page then is secondary.
But authors are not owners; NuGet made that clear IIRC. The owner (deprecated owners field) is associated with the token / publisher account(s) and all other accounts that have a right to publish new versions. Previously, we also used the authors field to indicate "AngleSharp" and my point was to keep doing that.
I am not sure here; my experience with restore is that a range such as [1.0, 2.0) is not automatically used when a dependency on 1.1 was specified. So if you (as a user) would want to go for the lower end (1.0) you'd get in trouble here. I mean, that makes sense as NuGet does not know that the 1.1 can be downleveled to a 1.0; so either one would need to work with an outdated version locally when publishing (not great) or need to remain at an implicit [1.1, 2.0).
It would, if it would be considered for package creation. AFAIK it is not. |
For me the default logic feels quite right. Say we have Jint with version 3.0.0, 3.0.1 and 3.1.0. Have library
The fixed version ranges with DotNetZip were a real PITA due to Paket's handling and it requiring these explicit ranges, always needed a new release when dependency versions didn't match anymore to other consumed libraries, even though there even were no runtime breaking changes. |
The described cases are not the ones that we are worried about. It's the case where you have an explicit dependency to, e.g., Jint 3.0.2 but you now add a dependency to lib B - making this a 3.1.0 which should be a 3.0.2 still. |
IMO if you add the dependency to B it's an informed decision that you cannot no longer use older version as you want to use a library that needs a newer version. Basically you would want to prevent using B? |
Well, first of all not everyone is aware of the dependencies of a lib, but even with that awareness - why should we limit the combinations if we know that the lower one is supported, too? It does not make much sense. We should deliver the library in a way that it can be consumed in the broadest / most flexible way. That's why it is a library. A package manager should help with this. The version range has been introduced to help with this - by giving the library means for telling the package manager what is supported. |
I think we just have different viewpoints on this, and I respect your view. My look on the matter is more about me as a software developer making the decision if the functionality works with the combination of libraries I've chosen. This is one things I've worked recently on - package lock files and reporting used versions and their licenses for auditing purposes. So I guess the difference is that while I'm leaning towards getting the lowest supported version and taking steps from that, you would like to opt out from supporting something that might not work. Major versions of course are the ones here to worry about with their freedom to break things. Locking the upper bound is just a sure way to get issues opened about inability to use the library as NuGet won't allow the update 🙂 |
Also as a sidenote, we did intentionally break public API surface with Jint 3.1 for some advanced cases that I had vetted for against the public libraries using Jint (like yours). As Jint doesn't promise semantic versioning it would be correct to lock against very specific version (3.0.1 allowed, not 3.0.2). Of course in ideal world it would be semantic versioning but in this case we paved the road for v4 changes - where I would expect that your integration would be actually binary compatible with the API surface you are using even though we are replacing the whole JS parsing library. I try my best check downstream consumers when they are public OSS libraries. |
Yeah I fully agree - the thing is that only the version a lib was developed against would therefore work ("work" in the sense of "works as intended / covered by the test cases"). Because any other version might break; even if semver is tried to be followed there could be something in that would break API or even ABI compatibility. That being written it's a hard problem without any good solution. The direction we want to take with AngleSharp is to allow the broadest set. All libs of AngleSharp (e.g., AngleSharp.Io, AngleSharp.Xml, ...) are released with support for the same major version of AngleSharp (AngleSharp.Io 1.x requires / supports AngleSharp 1.x etc.). To reflect this support on NuGet install we use the semver ranges. I know that this is a discussion that cannot be solved / where the POV really depends on what your target / desire / mindset / whatever (taste?) is. I fully get your point, too - I just want that we can continue on semver ranges moving forward. Maybe a post-build step that modifies the NuGet package could be introduced? On the other side that somewhat counters the simplification that we aim for with this PR. (Sure, in this repo the PR would make sense, but usually we use the same infrastructure in all repos, which means this change cannot be transported; counter-acting the simplification) |
Maybe it boils down to whether you would be willing to have specific project (repos) to have the nuspec in use to achieve the goal you want. If the library you are publishing only needs core MS dependencies I would argue that it's safe to allow any range without upper bound. So if the JS integration is just "exception that confirms the rule" it would justifiable (maybe) to have specific packaging logic there. I know it's a pain to know that any specific repo needs nuspec file updates when dependencies change, but in the other hand I feel the automation from csproj very compelling. |
Well it's all the repos except this one; as all the repos have at least a dependency to AngleSharp - and (as explained) this one is major-version only. The JS-one is IIRC the only with with a non-AngleSharp dependency (Jint); MS-dependencies excluded. |
Types of Changes
Prerequisites
Please make sure you can check the following two boxes:
Contribution Type
What types of changes does your code introduce? Put an
x
in all the boxes that apply:Description
It seems I was able to drop NET6/7/8 targets with my earlier changes to
.nuspec
so here's something to make amends. This in my opinion simplifies the packaging logic quite a bit. All is in propertyes/csproj anddotnet pack
produces the output as expectHere's the produced nuspec in package file, you can see
<owners>
as empty but it's deprecated and now replaced with proper<authors>
.Files in package: