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

Prevent unnecessary rebuilds using Cargo in root #520

Merged
merged 1 commit into from
Sep 6, 2022

Conversation

syvb
Copy link
Member

@syvb syvb commented Sep 6, 2022

Currently .cargo/config and extension/.cargo/config are the same file (except for whitespace differences).

The Cargo docs specify that Cargo config files are looked for in each parent directory, so including the exact same config file in the extension/ directory and its parent directory is unnecessary.

The pgx README says:

Workspace users: cargo pgx new $NAME will create a $NAME/.cargo/config, you should move this into your workspace root as .cargo./config.

If you don't, you may experience unnecessary rebuilds using tools like Rust-Analyzer, as it will use the wrong rustflags option.

Config file duplication results in cargo test in the root directory building Toolkit from scratch twice, since Cargo invalidates previously built dependencies whenever it uses a new config file (even if they are semantically identical).

The [Cargo docs](https://doc.rust-lang.org/cargo/reference/config.html#hierarchical-structure)
specify that Cargo config files are looked for in each parent directory,
so including the exact same config file in the extension directory and
its parent directory is unnecessary.

Config file duplication results in `cargo test` in the root directory
building Toolkit from scratch twice, since Cargo invalidates previously
built dependencies whenever it uses a new config file (even if they are
semantically identical).
Copy link
Contributor

@rtwalker rtwalker left a comment

Choose a reason for hiding this comment

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

Just tried this out locally and it definitely works! Nice!

@epgts
Copy link
Contributor

epgts commented Sep 6, 2022

Just tried this out locally and it definitely works! Nice!

Me too! THANK YOU!!!

@syvb
Copy link
Member Author

syvb commented Sep 6, 2022

bors r+

@bors
Copy link
Contributor

bors bot commented Sep 6, 2022

@bors bors bot merged commit fb8a094 into main Sep 6, 2022
@bors bors bot deleted the smittyvb/fix-cargo-rebuilds branch September 6, 2022 20:56
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.

3 participants