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

Add ArchGitChecksum template command in bashbrew cat #89

Merged
merged 2 commits into from
Jan 12, 2024

Conversation

tianon
Copy link
Member

@tianon tianon commented Jan 8, 2024

This also finally adds bashbrew context as an explicit subcommand so that issues with this code are easier to test/debug (so we can generate the actual tarball and compare it to previous versions of it, versions generated by git archive, etc).

As-is, this currently generates verbatim identical checksums to https://github.com/docker-library/meta-scripts/blob/0cde8de57dfe411ed5578feffe1b10f811e11dc2/sources.sh#L90-L96 (by design). We'll wait to do any cache bust there until we implement Dockerfile/context filtering:

$ bashbrew cat varnish:stable --format '{{ .TagEntry.GitCommit }} {{ .TagEntry.Directory }}'
0c295b528f28a98650fb2580eab6d34b30b165c4 stable/debian
$ git -C "$BASHBREW_CACHE/git" archive 0c295b528f28a98650fb2580eab6d34b30b165c4:stable/debian/ | ./tar-scrubber | sha256sum
3aef5ac859b23d65dfe5e9f2a47750e9a32852222829cfba762a870c1473fad6
$ bashbrew cat --format '{{ .ArchGitChecksum arch .TagEntry }}' varnish:stable
3aef5ac859b23d65dfe5e9f2a47750e9a32852222829cfba762a870c1473fad6

(Choosing varnish:stable there because it currently has some 100% valid dangling symlinks that tripped up my code beautifully 💕)

From a performance perspective (which was the original reason for looking into / implementing this), running the meta-scripts/sources.sh script against --all vs this, my local system gets ~18.5m vs ~4.5m (faster being this new pure-Go implementation).

@codecov-commenter
Copy link

codecov-commenter commented Jan 8, 2024

Codecov Report

Attention: 93 lines in your changes are missing coverage. Please review.

Comparison is base (4e0ea8d) 73.10% compared to head (795ff4b) 75.21%.

Files Patch % Lines
pkg/gitfs/fs.go 64.35% 55 Missing and 17 partials ⚠️
pkg/tarscrub/tarscrub.go 66.12% 15 Missing and 6 partials ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master      #89      +/-   ##
==========================================
+ Coverage   73.10%   75.21%   +2.10%     
==========================================
  Files           7        8       +1     
  Lines         714      920     +206     
==========================================
+ Hits          522      692     +170     
- Misses        162      173      +11     
- Partials       30       55      +25     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@tianon
Copy link
Member Author

tianon commented Jan 8, 2024

With that latest push, we have a net increase in overall code coverage (and it successfully limits our coverage gaps in the new code to just the error handling blocks, which are inherently difficult to test because they typically involve convincing other libraries/code to return errors).

This also finally adds `bashbrew context` as an explicit subcommand so that issues with this code are easier to test/debug (so we can generate the actual tarball and compare it to previous versions of it, versions generated by `git archive`, etc).

As-is, this currently generates verbatim identical checksums to https://github.com/docker-library/meta-scripts/blob/0cde8de57dfe411ed5578feffe1b10f811e11dc2/sources.sh#L90-L96 (by design).  We'll wait to do any cache bust there until we implement `Dockerfile`/context filtering:

```console
$ bashbrew cat varnish:stable --format '{{ .TagEntry.GitCommit }} {{ .TagEntry.Directory }}'
0c295b528f28a98650fb2580eab6d34b30b165c4 stable/debian
$ git -C "$BASHBREW_CACHE/git" archive 0c295b528f28a98650fb2580eab6d34b30b165c4:stable/debian/ | ./tar-scrubber | sha256sum
3aef5ac859b23d65dfe5e9f2a47750e9a32852222829cfba762a870c1473fad6
$ bashbrew cat --format '{{ .ArchGitChecksum arch .TagEntry }}' varnish:stable
3aef5ac859b23d65dfe5e9f2a47750e9a32852222829cfba762a870c1473fad6
```

(Choosing `varnish:stable` there because it currently has [some 100% valid dangling symlinks](https://github.com/varnish/docker-varnish/tree/6b1c6ffedcfececac71e46a85122c1adaef25868/stable/debian/scripts) that tripped up my code beautifully 💕)

From a performance perspective (which was the original reason for looking into / implementing this), running the `meta-scripts/sources.sh` script against `--all` vs this, my local system gets ~18.5m vs ~4.5m (faster being this new pure-Go implementation).
@tianon tianon merged commit 1a2b388 into docker-library:master Jan 12, 2024
6 checks passed
@tianon tianon deleted the context-checksum branch January 12, 2024 23:56
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