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

cargo-release doesn't prevent publishing a package when it has unpublished registry dependencies #624

Open
kornelski opened this issue Dec 2, 2022 · 4 comments
Labels
bug Not as expected

Comments

@kornelski
Copy link

kornelski commented Dec 2, 2022

If I run cargo release in the root of: https://github.com/kornelski/cavif-rs commit b2f707ce08bf758048cb967998df92d8dd3d2c7c

https://github.com/kornelski/cavif-rs/tree/b2f707ce08bf758048cb967998df92d8dd3d2c7c/

it chooses not to publish workspace's leaf dependency first (ravif), and then the root crate fails validation (cavif), because its dependency isn't published yet.

$ cargo release
warning: disabled by user, skipping ravif which has files changed since v1.4.0: [
    "./cavif/ravif/Cargo.toml",
    "./cavif/ravif/src/av1encoder.rs",
    "./cavif/ravif/src/dirtyalpha.rs",
    "./cavif/ravif/src/error.rs",
    "./cavif/ravif/src/lib.rs",
]
warning: disabled by user, skipping ravif v0.9.3 despite being unpublished
  Publishing cavif
warning: profiles for the non root package will be ignored, specify profiles at the workspace root:
package:   ./cavif/ravif/Cargo.toml
workspace: ./cavif/Cargo.toml
    Updating crates.io index
   Packaging cavif v1.4.1 (./cavif)
error: failed to prepare local package for uploading

Caused by:
  failed to select a version for the requirement `ravif = "^0.9.2"`
  candidate versions found which didn't match: 0.9.1, 0.9.0, 0.8.9, ...
  location searched: crates.io index
  required by package `cavif v1.4.1 (./cavif)`

I don't see why cargo-release thinks the dependency is "disabled by user".

Using cargo-release 0.24.0 and rustc 1.65

@epage
Copy link
Collaborator

epage commented Dec 2, 2022

cargo release mimics most cargo command when selecting packages. Running in a virtual workspace will select everything but a workspaced with a package will select just that package. You need to either say -p ravif, --workspace, or --unpublished.

@kornelski
Copy link
Author

That's disappointing. I'd expect it to be smarter than cargo publish and figure out it needs to publish dependencies first.

@epage epage reopened this Dec 2, 2022
@epage
Copy link
Collaborator

epage commented Dec 2, 2022

Actually, that is why I hadn't closed this yet.

We should check all registry dependencies of the item we are publishing and ensure they are published as well.

Unsure whether this should be a warning, error, or that we implicitly publish them. I lean towards error. A warning is good if there could be a valid reason for someone not doing it but this will always fail. If we implicitly publish, the user might not have prepared that package for publishing and we could be doing something unexpected. An error gives the user the ability to choose whether it is right or not. We still have --unpublished to make it easier to publish them

@epage epage changed the title Verification builds fail fetching (non-dev) dependency that isn't published yet cargo-release doesn't prevent publishing a package when it has unpublished registry dependencies Dec 2, 2022
@epage
Copy link
Collaborator

epage commented Dec 2, 2022

One thing I'll add is that we have the following warning

warning: disabled by user, skipping ravif v0.9.3 despite being unpublished

This is generic and doesn't deal with dependencies. While useful to help users identify crates they might have meant to also include, relying solely on this warning to resolve this issue means the user has to mentally track the dependency tree.

@epage epage added the bug Not as expected label Dec 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Not as expected
Projects
None yet
Development

No branches or pull requests

2 participants