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

Run 'cargo package' in CI #646

Closed
wants to merge 4 commits into from
Closed

Run 'cargo package' in CI #646

wants to merge 4 commits into from

Conversation

dbr
Copy link
Contributor

@dbr dbr commented Jun 2, 2022

Should verify the .crate file sent to crates.io can at least be built, which should catch things like the docs.rs build failure, as discussed in #645 and #631

Should verify the .crate file sent to crates.io can at least be built, which should catch things like the docs.rs build failure, as discussed in imgui-rs#645 and imgui-rs#631
@thomcc
Copy link
Member

thomcc commented Jun 2, 2022

Would this actually catch the docs.rs failure? I'm surprised. Cargo does cargo package as part of publishing, so it doesn't make sense to me that it would, when it (presumably) wasn't caught as part of the publish.

@dbr
Copy link
Contributor Author

dbr commented Jun 2, 2022

Ah, sorry, I should have explained better: it would catch the specific failure that caused #631

The problem was some files needed for the freetype weren't included in the .crate package

Then given we build the docs with various features enabled, the first thing to notice the problem was the docs.rs build (since that pulls from crates.io, not the git repo)

However the problem was also happen if someone used imgui like imgui = {version = "0.8.2", features = ["freetype"]}

Cargo does cargo package as part of publishing, so it doesn't make sense to me that it would, when it (presumably) wasn't caught as part of the publish

This is correct, but only if there are no features 😢 (i.e publishing only checks cargo build, not cargo build --features $x)

@dbr
Copy link
Contributor Author

dbr commented Aug 18, 2022

It is not easy to do cargo package -p imgui without publishing the dependant imgui-sys to crates.io (adding the --no-verify flag defeats the purpose of this since it skips building the crate)

There's an existing issue at on rust-lang/cargo#9260 which I'll add a note to

There's probably workarounds (like setting up a temporary private package registry or something), but not worthwhile given the complexity versus the minor severity of the problem it might catch

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.

None yet

2 participants