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

Wildcard version warnings generated git dependencies with fixed commit #488

Closed
TimJentzsch opened this issue Jan 2, 2023 · 6 comments · Fixed by #599
Closed

Wildcard version warnings generated git dependencies with fixed commit #488

TimJentzsch opened this issue Jan 2, 2023 · 6 comments · Fixed by #599
Labels
bug Something isn't working

Comments

@TimJentzsch
Copy link

Describe the bug

When specifying a git dependency with a rev (i.e., fixed commit hash) entry, the version wildcard warning is triggered.
However, because of the commit hash, an exact version has been specified.

To Reproduce
Steps to reproduce the behavior:

  1. Add a git dependency with a commit specified, e.g.:

    [dependencies]
    bevy_ecs_tilemap = { git = "https://github.com/StarArawn/bevy_ecs_tilemap", rev = "2967a394dc59c29fac14cb8cb187d601ea604a1e" }
  2. Ban wildcard dependencies in deny.toml:

    [bans]
    wildcards = "deny"
  3. Run cargo deny check. A wildcard error is triggered for the git dependency.
    In our case, it looks like this (with two git dependencies):

    error[wildcard]: found 3 wildcard dependencies for crate 'emergence_lib'
      ┌─ /home/tim/Documents/bevyengine/Emergence/emergence_lib/Cargo.toml:2:1
      │
    2 │ bevy-trait-query = '*'
      │ ^^^^^^^^^^^^^^^^^^^^^^ wildcard crate entry
    3 │ bevy_ecs_tilemap = '*'
    4 │ bevy_ecs_tilemap = '*'
      │ ^^^^^^^^^^^^^^^^^^^^^^
      │ │
      │ wildcard crate entry
      │ wildcard crate entry
      │
      = emergence_lib v0.1.0
        └── emergence_game v0.1.0
    
    

Expected behavior

The wildcard warning is only triggered for git dependencies if the commit hash is not fixed, i.e. when the rev attribute is missing.

Device:

  • OS: Linux (Pop!_OS)
  • Version 0.13.5
@Jake-Shadle
Copy link
Member

This was fixed in #487

@TimJentzsch
Copy link
Author

I'm still getting the same error with cargo-deny 0.13.6, is that intentional?

@Jake-Shadle Jake-Shadle reopened this Jan 11, 2023
@Jake-Shadle
Copy link
Member

Sorry, thought this was fixed, I guess I need another edge case when git dependencies are direct like that instead of a patch on top of a version, which is the only way I use them.

@BatmanAoD
Copy link

I believe I'm seeing this error, or a related one. I have a private package that is importing some other private packages from GitLab. Even with allow-wildcard-paths, I get this message:

error[wildcard]: found 2 wildcard dependencies for crate 'behavioral-tests'. allow-wildcard-paths is enabled, but does not apply to public crates as crates.io disallows path dependencies.

There's no indication of why cargo deny thinks there's a "public crate" involved here; is it saying that the two packages I've specified as git dependencies are public, or that the top-level package is public?

@kpreid
Copy link
Contributor

kpreid commented Feb 4, 2024

There's no indication of why cargo deny thinks there's a "public crate" involved here

I took a look, and the code producing that error message doesn't actually check whether the allow-wildcard-paths option affected the result, just that you had it enabled. The actual reason for the failure is that whether or not the package is private, the git dependency doesn't count as a path dependency, and doesn't have any version requirements, so the wildcard ban always rejects it and can't be configured otherwise.

I created a PR #599 which changes the behavior to treat git dependencies like path dependencies, which seems like the cleanest solution short of larger changes to how the ban works and is configured.

@BatmanAoD
Copy link

@kpreid That looks like exactly what I hoped for from that option! Thanks!

@mergify mergify bot closed this as completed in #599 Feb 5, 2024
mergify bot pushed a commit that referenced this issue Feb 5, 2024
…ildcard-paths`. (#599)

Fixes #488, making it possible to ban wildcards without also banning
git-only dependencies.

This may not be a perfect fit for some use cases — arguably `git`
dependencies are less implicitly-versioned than `path` dependencies
since `path` dependencies are typically always the same revision of the
same repo, but `git` dependencies might be `cargo update`d to totally
different code. But I can't think of an alternative that's
equal-or-better in correctness short of introducing even more
configuration.

(I suspect that the whole idea of counting path-only or git-only deps as
wildcard versions *ever* is wrong, because [the Cargo documentation
says](https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#specifying-path-dependencies)
that “…the version key always implies that the package is available in a
registry. version, git, and path keys are considered [separate
locations](https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#multiple-locations)
for resolving the dependency” — which implies that a dep *without*
`version` is different from a dep with a wildcard version. However,
figuring out Cargo's behavior there and how cargo-deny should treat it
feels like a rabbit hole I don't want to go down just to fix #488. I
left a TODO comment suggesting further consideration.)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants