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

only hash Cargo.toml/Cargo.lock that belong to a configured workspace #90

Merged
merged 4 commits into from
Nov 5, 2022

Conversation

lucasfernog
Copy link
Contributor

This improves the lock hash to not include Cargo files that are not relevant to the cache by only checking the defined workspace folders.

@lucasfernog
Copy link
Contributor Author

Closes #89

Copy link
Owner

@Swatinem Swatinem left a comment

Choose a reason for hiding this comment

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

I have two comments:

I believe rust-toolchain is workspace specific, though I’m not entirely sure.

You are overwriting keyFiles in the globHash function, though it is later being printed, so you would lose half the output in the cache summary if you specify multiple workspaces. Also, the hash now depends on the order of how you define the workspaces.

I believe a better way would be first glob/collect all the files into keyFiles, sort them once and then create a hash from that one.

src/config.ts Outdated
}
}

await globHash("rust-toolchain\nrust-toolchain.toml");
Copy link
Owner

Choose a reason for hiding this comment

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

I’m wondering, is the rust-toolchain file something workspace specific? I think so?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right, cargo uses the closest one but it'll search all the way to the root.

@Swatinem Swatinem merged commit ccdddcc into Swatinem:master Nov 5, 2022
@lucasfernog lucasfernog deleted the feat/glob-workspace branch November 6, 2022 17:45
lucasfernog added a commit to tauri-apps/tauri that referenced this pull request Nov 6, 2022
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