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

Fix CI two times #7898

Merged
merged 2 commits into from
Oct 9, 2020
Merged

Fix CI two times #7898

merged 2 commits into from
Oct 9, 2020

Conversation

piscisaureus
Copy link
Member

No description provided.

A recent change in rustc or cargo made it so that rusty_v8's `build.rs`,
which is responsible for downloading `librusty_v8.a`, does not get
rebuilt or re-run when its build output directory is restored from the
Github Actions cache.

However, rusty_v8's custom build script does not save the download to
its build output directory; it puts the file in
`target/debug|release/gn_out/obj` instead.

To get CI going again we opted to add `target/*/gn_out` to the Github
Actions cache.

A more robust fix would be make rusty_v8 save the download to the
cargo-designated output directory.
@piscisaureus piscisaureus changed the title Maybe fix CI ci: fix rusty_v8 binary download unavailable Oct 9, 2020
@piscisaureus piscisaureus changed the title ci: fix rusty_v8 binary download unavailable Fix CI Oct 9, 2020
@piscisaureus piscisaureus changed the title Fix CI Fix CI two times Oct 9, 2020
Copy link
Member

@ry ry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did an analysis of the speed up we're getting from cache in #7903 (comment) . It's not much. I would say if we run into more bugs or have to introduce more complexity, it would be probably better to simply remove the whole thing.

But it seems like this is working and is faster so LGTM. Thanks for putting out this fire.

@piscisaureus piscisaureus merged commit 265c257 into master Oct 9, 2020
@piscisaureus piscisaureus deleted the fix-ci branch October 9, 2020 16:19
@piscisaureus
Copy link
Member Author

I would say if we run into more bugs or have to introduce more complexity, it would be probably better to simply remove the whole thing.

It's a lot of stuff for a few minutes gain.

Maybe this experience will give us some hints on why our builds are so slow (and caching not effective) and it can be improved.

If not I agree then let's just turn it off.

BTW from-scratch release build & run tests in 10 minutes!
I need an XL at home. It usually takes at least 30 minutes for me...

iykekings pushed a commit to iykekings/deno that referenced this pull request Oct 10, 2020
A recent change in rustc or cargo made it so that rusty_v8's `build.rs`,
which is responsible for downloading `librusty_v8.a`, does not get
rebuilt or re-run when its build output directory is restored from the
Github Actions cache.

However, rusty_v8's custom build script does not save the download to
its build output directory; it puts the file in
`target/debug|release/gn_out/obj` instead.

To get CI going again we opted to add `target/*/gn_out` to the Github
Actions cache.

A more robust fix would be make rusty_v8 save the download to the
cargo-designated output directory.
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.

2 participants