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.

Sign up for GitHub

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

Jump to bottom

Make DigestSubset symlink-aware #18963

Merged
merged 3 commits into from
May 10, 2023
Merged

Make DigestSubset symlink-aware #18963

merged 3 commits into from
May 10, 2023

Conversation

thejcannon
Copy link
Member

@thejcannon thejcannon commented May 10, 2023 •
edited
Loading

Fixes #18956

Added a new test. Fails before, passes after!

@thejcannon thejcannon added needs-cherrypick category:bugfix Bug fixes for released features labels May 10, 2023
@thejcannon thejcannon added this to the 2.16.x milestone May 10, 2023
@thejcannon thejcannon requested review from stuhood, tdyas and huonw May 10, 2023 01:44
@thejcannon
Copy link
Member Author

I should add @huonw laid out all the breadcrumbs on this one <3

@huonw
Copy link
Contributor

huonw commented May 10, 2023

There seems to be several calls of .subset in src/rust/engine/fs/store/src/snapshot_ops_tests.rs, and the names are somewhat suggestive that subset is the item under test. It'd probably be good to add a test for this sort of foundational object?

@tdyas
Copy link
Contributor

tdyas commented May 10, 2023

I agree, even a small regression test of just symlink support would be good.

@thejcannon
Copy link
Member Author

Oh haha I was looking in snapshot_tests.rs not snapshot_ops_tests.rs 😅

@thejcannon thejcannon merged commit bee75a9 into pantsbuild:main May 10, 2023
@thejcannon thejcannon deleted the fix18956 branch May 11, 2023 00:00
thejcannon added a commit to thejcannon/pants that referenced this pull request May 11, 2023
@thejcannon
Make DigestSubset symlink-aware (pantsbuild#18963)
1b42621 
Fixes pantsbuild#18956

Added a new test. Fails before, passes after!
@huonw huonw mentioned this pull request May 11, 2023
Merged
thejcannon added a commit that referenced this pull request May 11, 2023
@thejcannon
Make DigestSubset symlink-aware (Cherry-pick of #18963) (#18970)
e22b418 
Fixes #18956

Added a new test. Fails before, passes after!
@huonw huonw mentioned this pull request Jun 3, 2023
Merged
@huonw huonw mentioned this pull request Sep 1, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tdyas tdyas tdyas left review comments

@huonw huonw huonw approved these changes

@stuhood stuhood Awaiting requested review from stuhood

Assignees
No one assigned
Labels
category:bugfix Bug fixes for released features
Projects
None yet
Milestone
2.16.x
Development

Successfully merging this pull request may close these issues.

"Path path/to/thing was a file rather than a symlink."
3 participants
@thejcannon @huonw @tdyas