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

Switch tar verification to a manifest comparison. #4458

Merged
merged 4 commits into from
Oct 30, 2024

Conversation

jonmeow
Copy link
Contributor

@jonmeow jonmeow commented Oct 30, 2024

This removes the install marker-relative path checks, and replaces it with bidirectional verification: previously, files in the tar file had to be in install data, but there was no check that files in install data were all in the tar.

@chandlerc chandlerc requested review from chandlerc and removed request for zygoloid October 30, 2024 22:08
Copy link
Contributor

@chandlerc chandlerc left a comment

Choose a reason for hiding this comment

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

Very nice! Glad to see the TODO go away. Totally optional comment inline.

toolchain/install/toolchain_tar_test.py Outdated Show resolved Hide resolved
toolchain/install/toolchain_tar_test.py Outdated Show resolved Hide resolved
Copy link
Contributor

@chandlerc chandlerc left a comment

Choose a reason for hiding this comment

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

(and LG with new unittest module based code)

@jonmeow jonmeow added this pull request to the merge queue Oct 30, 2024
Merged via the queue into carbon-language:trunk with commit 85f6bf3 Oct 30, 2024
8 checks passed
@jonmeow jonmeow deleted the tar-test branch October 30, 2024 23:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants