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

Implement support for requiring dependency version ranges #82

Merged
merged 1 commit into from
Oct 31, 2023

Conversation

sdroege
Copy link

@sdroege sdroege commented Oct 27, 2023

This now supports expressions in the form

  • "1.2" or ">= 1.2" for at least version 1.2
  • ">= 1.2, < 2.0" for at least version 1.2 and less than version 2.0

Fixes #60


@gdesmott Does this look reasonable to you? As discussed, semver can't really be used here because pkg-config versions don't follow semver. A version of "1.2" is semver ">= 1.2, < 2.0" and that is more semantics than pkg-config versions have.

src/lib.rs Show resolved Hide resolved
@gdesmott
Copy link
Owner

Does this look reasonable to you?

I think so. Let's go for that then. I didn't look at the implementation yet but the syntax is ok.

Could you please add some tests as well?

@sdroege
Copy link
Author

sdroege commented Oct 30, 2023

Could you please add some tests as well?

Sure, I was waiting for an OK for the syntax before spending time on writing tests :)

@gdesmott
Copy link
Owner

This MR does not break API right? Just so I know how to version the release once it's merged.

@sdroege
Copy link
Author

sdroege commented Oct 30, 2023

It doesn't break API. If a crate starts using more complex version expressions then it needs to be able to actually handle them though: it will get them passed as version to the closure for building the dependency internally. That doesn't seem like API breakage though as it only happens once the crate uses the new feature.

@sdroege
Copy link
Author

sdroege commented Oct 30, 2023

Added two simple tests for this.

@gdesmott
Copy link
Owner

Looks like this test is failing on CI: https://github.com/gdesmott/system-deps/actions/runs/6695863075/job/18192357484?pr=82

(Yeah I need the fix the other jobs)

@gdesmott
Copy link
Owner

(Yeah I need the fix the other jobs)

I just did so you can rebase your branch for easy CI testing.

This now supports expressions in the form

  * "1.2" or ">= 1.2" for at least version 1.2
  * ">= 1.2, < 2.0" for at least version 1.2 and less than version 2.0

Fixes gdesmott#60
@sdroege
Copy link
Author

sdroege commented Oct 31, 2023

Looks like this test is failing on CI

Because pkgconf and pkg-config have completely different error messages. I've added both of them now.

@gdesmott gdesmott merged commit 45c8685 into gdesmott:master Oct 31, 2023
6 checks passed
@gdesmott
Copy link
Owner

Thanks for the patch. I just released 6.2.0 with it.

@sdroege sdroege deleted the version-ranges branch October 31, 2023 11:09
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 this pull request may close these issues.

Add support for providing version ranges to pkg-config
2 participants