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

hack: use double space in ytt checksum check #333

Merged
merged 1 commit into from
Nov 12, 2021

Conversation

cirocosta
Copy link
Contributor

@cirocosta cirocosta commented Nov 11, 2021

even though standard gnu coreutils accepts taking a single space as the
separator, that's not true for busybox.

by making sure we're passing a double space everywhere we compute a
checksum, we """guarantee""" to be good not only w/ gnu, but also
busybox-based ones (like alpine etc).

				filename_ptr = strstr(line, "  ");          // double space
				/* handle format for binary checksums */
				if (filename_ptr == NULL) {
					filename_ptr = strstr(line, " *");

(see https://github.com/mirror/busybox/blob/15f7d618ea7f8c3a0277c98309268b709e20d77c/coreutils/md5_sha1_sum.c#L296)

ps.: in case you're wondering why this isn't captured by CI, it's because
we have github running based of ubuntu:

jobs:
  build:
    runs-on: ubuntu-latest

even though standard gnu coreutils accepts taking a single space as the
separator, that's not true for busybox.

by making sure we're passing a double space everywhere we compute a
checksum, we """guarantee""" to be good not only w/ gnu, but also
busybox-based ones (like alpine etc).

see https://github.com/mirror/busybox/blob/15f7d618ea7f8c3a0277c98309268b709e20d77c/coreutils/md5_sha1_sum.c#L296

Signed-off-by: Ciro S. Costa <ciroscosta@vmware.com>
@netlify
Copy link

netlify bot commented Nov 11, 2021

✔️ Deploy Preview for elated-stonebraker-105904 canceled.

🔨 Explore the source changes: 9de1744

🔍 Inspect the deploy log: https://app.netlify.com/sites/elated-stonebraker-105904/deploys/618d3912fcae130007617c99

@emmjohnson
Copy link
Contributor

How to ensure we wont have a regression here?

@waciumawanjohi
Copy link
Contributor

How to ensure we wont have a regression here?

We could add a self-hosted busybox runner and have our workflows randomly assign the job to the default runner or the self-hosted runner...

Note: Github discourages self-hosted runners for public repos.

@cirocosta
Copy link
Contributor Author

yeah that's a good point - internally we make use of it in GitLab, so there's some form of coverage there that would surface here for sure

@cirocosta cirocosta merged commit 37d9fd1 into main Nov 12, 2021
@cirocosta cirocosta deleted the hack/checksum-double-space branch November 12, 2021 11:51
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