-
Notifications
You must be signed in to change notification settings - Fork 0
Fix build hash inconsistency #41
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Fixes inconsistent build hash generation across different systems by standardizing file sorting and improving file inclusion logic. The build hash is used to uniquely identify the version of the installer scripts.
- Enforces consistent sorting across systems using
LC_ALL=Clocale - Excludes non-essential directories (
exp,.github, hidden files) from hash calculation - Includes symlinked files and directories in hash calculation
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
69bc7b3 to
bb5cdd2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
The build hash was including some files that were not intended, and not sorting the files in a consistent way, so this led to inconsistent hashes between systems. Both `exp` and `.github` directories were allowed to be included in the hash calculation, even though these directories are not part of the live code. Incidentally, different locales would handle sorting `.github` differently, leading to different hashes. So, we prune those directories from the file search, and we set `LC_ALL=C` to ensure consistent sorting. Additionally, we were not considering symlinks when identifying files and directories to include in the hash calculation. This wasn't causing a problem but it could result in unchanged hashes even when symlinks are changed or omitted. So, we improve the file search to include symlinks in the hash calculation.
4e6c7b9 to
df2bf02
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
This bug doesn't actually impact behavior, but negates the purpose of the build hash. Due to inconsistencies with the way sorting occurs on different systems, the build hash may differ between systems.
To fix this, we use
LC_ALL=Cwhich consistently sorts files on all systems regardless of their locale, which influences the sorting algorithm.While looking at the code, though, I realized that we were (a) Including some code that isn't "live" to the installer, like the
expand.githubdirectories, and (b) NOT including symlinked directories and files, which could potentially result in the hash not changing when symlinked directories and files are added or omitted.To demonstrate what files will be included in the hash calculation now, run this script from the
docker-hostproject checkout:It should return:
You can also test this on other systems by asking for the version from this branch like this:
bash <(curl -H 'Cache-Control: no-cache, no-store' -o- https://raw.githubusercontent.com/uicpharm/docker-host/jcurt/hash/init.sh) -b jcurt/hash --versionCurrently, it reports:
Docker Host version 1.0.0 build 4036ef92