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

Do not use an upper bound on dependencies, unless you have to #253

Closed
abelbraaksma opened this issue Nov 5, 2023 · 5 comments · Fixed by #254
Closed

Do not use an upper bound on dependencies, unless you have to #253

abelbraaksma opened this issue Nov 5, 2023 · 5 comments · Fixed by #254

Comments

@abelbraaksma
Copy link
Member

abelbraaksma commented Nov 5, 2023

Description

Whenever xUnit is updated beyond what this package assumes, NU1608 warnings are raised. This issue was introduced in the latest v5.5.0, it was not present in v5.4.0.

(side note, I don't see any PR for the last 8 days, and the new version is 8 days old, so I can't really find what caused this...)

Repro steps

Just include this library through Nuget and upgrade the xUnit dependency to the latest version 2.6.1

Expected behavior

No warnings.

Actual behavior

Warning:

Warning NU1608: Detected package version outside of dependency constraint: FsUnit.xUnit 5.5.0 requires xunit (>= 2.5.3 && < 2.6.0) but version xunit 2.6.1 was resolved.

Known workarounds

Either ignore the warning, or downgrade to FsUnit.xUnit 5.4.0, which does not have this issue.

Related information

Version FsUnit.xUnit 5.5.0

Maybe there's a good reason for not supporting the 2.6+ range, but so far, it works "just fine" if I ignore the warning. In general, dependencies should have a lowest bound, ideally as low as possible for backward compatibility, but no upper bound, for the simple reason that we cannot predict what future versions will bring.

Note that this is how the FSharp.Core dependency was set, no upperbound:

@CaptnCodr
Copy link
Member

Hey @abelbraaksma,
thank you for opening this issue. I'm currently looking into it.
I will probably release a new version this week.

@CaptnCodr
Copy link
Member

CaptnCodr commented Nov 6, 2023

The reason why xunit (>= 2.5.3 && < 2.6.0) is the mechanism in paket, which packs the nuget package.
When there is a bugfix-version (2.5.3) then it ties it to the next minor version (2.6.0).
NHamcrest is tied to >= 3.3.0 && < 4.0.0 because there is no bugfix release else there would be the tie e.g.: >= 3.3.1 && < 3.4.0.

I will update the dependencies in the next PR and tie them to >= LOCKEDVERSION where LOCKEDVERSION is the newest version from nuget.

Edit: or tie the LOCKEDVERSION to the next major version, e.g.: xunit (>= 2.6.1 && < 3.0.0).

@abelbraaksma
Copy link
Member Author

abelbraaksma commented Nov 7, 2023

Actually, you should make LOCKEDVERSION the lowest common denominator.

If you make it high, you force everybody using your library to also update xUnit, which, as we all know, introduces incompatibilities. You may need to, in which case there's no way out, but in any case, stay as low as you're code allows it.

If you can, don't give an upper bound. You don't want to run in the same situation again some day.

PS good to know that it's Paket making these decisions. They may make sense in company code, but not in shared libraries. In fact, with quite some public libs, I've seen Paket making weird decisions more often. You want the compatible version as long as possible as low as possible.

else there would be the tie e.g.: >= 3.3.1 && < 3.4.0.

So the same would happen again with FSharp.Core or NHamcrest. Maybe Paket is a little too much automation here?

@CaptnCodr
Copy link
Member

This has been fixed in v5.6.0 which is now available.

@abelbraaksma
Copy link
Member Author

@CaptnCodr many thanks for the quick resolution! I just upgraded and indeed, the issue doesn't repro anymore.

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

Successfully merging a pull request may close this issue.

2 participants