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

[CD] Prevent unnecessary building during crate publish process #2591

Closed
tjtelan opened this issue Aug 25, 2022 · 8 comments
Closed

[CD] Prevent unnecessary building during crate publish process #2591

tjtelan opened this issue Aug 25, 2022 · 8 comments
Labels
bug Something isn't working build-release CD DX/GENERAL Developer Experience General enhancement New feature or request good first issue Good for newcomers help wanted Good issue for community involvement no-issue-activity Test Infrastructure Testing infrastructure value/high

Comments

@tjtelan
Copy link
Contributor

tjtelan commented Aug 25, 2022

Related to: #2590

Our publish process is not efficient. It is driven by these two items.

The complexity comes from the latency from when a new crate is published to crates.io, and the when it is made available. This delay causes publish of crates further down the list to fail due to their dependencies not being available.

(The delay is at least a minute, but I'm not quite sure what it is anymore at the time of this writing.)

Typically the issue resolves itself when the build cycle loops back from the beginning since the builds provide further delay for crates.io to refresh.

But a consequence is when there is an issue that won't resolve by waiting (like #2550), it will take a long time to fail (>1 hr) because the script will rebuild all the crates 3 times before failing.


We want this process to fail fast if we're aware of the conditions. And to minimize the building of crates if we know the publish is going to fail because no publish is required.

Extra points if we can accommodate the crates.io refresh delay before attempting to push crates with dependencies that have been updated.

@tjtelan tjtelan added bug Something isn't working enhancement New feature or request help wanted Good issue for community involvement good first issue Good for newcomers build-release Test Infrastructure Testing infrastructure Priority/low value/high CD DX/GENERAL Developer Experience General labels Aug 25, 2022
@mfdorst
Copy link
Contributor

mfdorst commented Aug 25, 2022

Do you think this is also a candidate for rewriting in Rust, or do you think it makes more sense as a shell script?

@tjtelan
Copy link
Contributor Author

tjtelan commented Aug 25, 2022

Do you think this is also a candidate for rewriting in Rust, or do you think it makes more sense as a shell script?

The only nice thing about the shell script is not needing to compile it. But I don't have a preference for this being in Rust or shell script.

@mfdorst
Copy link
Contributor

mfdorst commented Aug 26, 2022

Given that the script will take a lot of time regardless I don't see that as a huge problem. I'll work on it a bit and try to decide which way is better.

@mfdorst
Copy link
Contributor

mfdorst commented Aug 26, 2022

And to minimize the building of crates if we know the publish is going to fail because no publish is required.

Under what circumstances would no publish be required?

@mfdorst
Copy link
Contributor

mfdorst commented Aug 26, 2022

Also, do you know of a good way to test this code that doesn't involve actually publishing crates?

@sehz
Copy link
Contributor

sehz commented Aug 26, 2022

I believe you can test publishing by performing dry run

@tjtelan
Copy link
Contributor Author

tjtelan commented Aug 27, 2022

And to minimize the building of crates if we know the publish is going to fail because no publish is required.

Under what circumstances would no publish be required?

  • If a crate version is already published, and the crate in the repo is the "exact same" as the latest crate available on crates.io.
    • So source code hasn't changed, and no Cargo.toml changes.
  • If a crate's Cargo.toml version is 0.0.0 and/or the crate is marked publish = false

And this last case is an error case. The publish is needed, but it will never succeed.

  • If a crate has a dependency that has failed, and it is unlikely it will be resolved by trying again.
    • If we're unable to compile
    • If we're unsuccessful with publishing a dependency

Also, do you know of a good way to test this code that doesn't involve actually publishing crates?

cargo publish --dry-run will work for most of the testing.

Otherwise, maybe you can try to publish a crate you're not authorized to publish for testing internal dependencies. (See #2550)

I'm not sure if this is how cargo publish would work, but maybe you can go through all the build process but the last step would fail bc you'd be unable to complete the publish. Though it's a little bit of a hack.

I did the initial testing as part of our typical release, with publishing involved.

@github-actions
Copy link

Stale issue message

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Nov 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working build-release CD DX/GENERAL Developer Experience General enhancement New feature or request good first issue Good for newcomers help wanted Good issue for community involvement no-issue-activity Test Infrastructure Testing infrastructure value/high
Projects
None yet
Development

No branches or pull requests

3 participants