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

Rerun cbindgen on changes to src #7708

Merged
merged 1 commit into from May 13, 2019

Conversation

Projects
None yet
2 participants
@illicitonion
Copy link
Contributor

commented May 13, 2019

Currently, if you only modify a signature in src, and don't otherwise
happen to modify native.py, we won't rerun cbindgen, and you'll get an
error that the Python and Rust code had different expectations.

@illicitonion illicitonion requested a review from blorente May 13, 2019

@blorente

This comment has been minimized.

Copy link
Contributor

commented May 13, 2019

I can't accept from my phone, but. Looks good :) is there a noticeable perf impact on noop runs? Not sure exactly how rerun-if-changed is implemented or what guarantees it offers.

@illicitonion

This comment has been minimized.

Copy link
Contributor Author

commented May 13, 2019

I can't accept from my phone, but. Looks good :) is there a noticeable perf impact on noop runs? Not sure exactly how rerun-if-changed is implemented or what guarantees it offers.

If no files that affect rust compilation have changed, we'll hit the ~/.cache/pants/bin/native-engine cache, and not invoke cargo.
Otherwise the way rerun-if-changed currently works is by inspecting timestamps, so we'll perform a stat-per-file which is trivial.
Then, if we miss that cache, cbindgen only updates its output file if it actually changed, and so does native.py (which consumes it), so we won't trigger re-builds or re-links unless actually necessary.

Rerun cbindgen on changes to src
Currently, if you only modify a signature in src, and don't otherwise
happen to modify native.py, we won't rerun cbindgen, and you'll get an
error that the Python and Rust code had different expectations.

@illicitonion illicitonion force-pushed the twitter:dwagnerhall/cbindgen branch from 8d3eff2 to 7d922fb May 13, 2019

@blorente

This comment has been minimized.

Copy link
Contributor

commented May 13, 2019

Thanks! The timestamp part was what I wasn't so sure about :)

@illicitonion illicitonion merged commit 55529b0 into pantsbuild:master May 13, 2019

1 check was pending

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details

@illicitonion illicitonion deleted the twitter:dwagnerhall/cbindgen branch May 13, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.