Merge tag 'common/v0.66.1' into main

[Luap99]

As normal we need to make the go module resolver aware that we are indeed newer on main.

[mtrmac]

Thanks!

[Luap99]

git-validate is validate is failing as it is linting some of the other merge commits that happen in between it seems, and our github ui merge commits all don't have a sign-off so it fails but somehow it only fails like that for one of the merge commits?

  * 85e98f3 "Merge tag 'common/v0.66.1' into merge-back" ... PASS
 * 0799dfd "Bump common to v0.66.1" ... PASS
 * fb1cbee "Merge pull request #526 from containers/renovate/golangci-golangci-lint-2.x" ... PASS
 * a657305 "chore(deps): update dependency golangci/golangci-lint to v2.7.1" ... PASS
 * e008c4c "Merge pull request #524 from containers/renovate/github.com-spf13-cobra-1.x" ... PASS
 * 9c98bf6 "Merge pull request #525 from Luap99/5.7-backport" ... PASS
 * 11033f8 "fix(deps): update module github.com/spf13/cobra to v1.10.2" ... PASS
 * 58ca294 "Merge pull request #523 from containers/renovate/golangci-golangci-lint-2.x" ... PASS
 * b134ef0 "common: clarify containers.conf doc for env" ... PASS
 * e128d7f "Merge pull request #517 from l0rd/win-default-helper-binaries-cp" ... PASS
 * df2167d "chore(deps): update dependency golangci/golangci-lint to v2.7.0" ... PASS
 * 424f1d7 "Merge pull request #514 from containers/renovate/github.com-klauspost-compress-1.x" ... FAIL
  - FAIL - has whitespace errors. See `git show --check 424f1d770ebcfc3c6a80a88f1085bc8a78c37ca1`.
 * a25a274 "fix(deps): update module github.com/klauspost/compress to v1.18.2" ... PASS
 * 75f8c40 "Merge pull request #520 from containers/renovate/github.com-docker-cli-29.x" ... FAIL
  - FAIL - does not have a valid DCO
 * 15e67a3 "Use $BINDIR as the default helper binary directory on Windows" ... PASS
 * 8163ca7 "Bump common to v0.66.0" ... PASS
 * 71fe077 "Bump storage to v1.61.0, image to v5.38.0 in common" ... PASS

I don't get the whitespace error either, git show --check 424f1d770ebcfc3c6a80a88f1085bc8a78c37ca1 doesn't show me anything problematic.

I guess I need to rework how the validate task works

[mtrmac]

OK but… why the change to fetch-depth (I assume) fix things?

The validation was passing for me locally.

---

Apparently if we fetch fewer commits, git show might show a commit (grafted) as adding a full tree instead of a diff.

… and the real underlying cause is that we do have whitespace errors in main:

% git show --check $(git log --oneline README.md | head -n 1 | cut -d ' ' -f 1)                    
commit ac7a4416ef6fd5069cd9a1ff4dc37cec3377bd59
…

README.md:3: trailing whitespace.
+This repository is a **monorepo** combining several core Go libraries and utilities from the [containers](https://github.com/containers) project.  
README.md:5: trailing whitespace.
+It brings together:  
README.md:7: trailing whitespace.
+- **[`common`](./common/)** → Shared Go code and configuration used across multiple containers projects.  
README.md:8: trailing whitespace.
+- **[`storage`](./storage/)** → A Go library for managing container images, layers, and containers.  
README.md:9: trailing whitespace.
+- **[`image`](./image/)** → A Go library for interacting with container images and registries.  

also on

README.md
SECURITY.md
common/pkg/config/testdata/containers_default.conf
common/pkg/config/testdata/containers_invalid.conf
common/pkg/config/testdata/containers_override2.conf
common/tools/rebuild_quay_ctr_images.sh
storage/.mailmap
storage/docs/containers-storage.md
storage/pkg/locker/README.md
storage/pkg/reexec/README.md
storage/types/storage_broken.conf

[Luap99]

OK but… why the change to fetch-depth (I assume) fix things?

I think it all comes down to git merge-base main HEAD working or not, with the sparse checkout there is no main so this fail. And with the current makefile if EPOCH_TEST_COMMIT is empty we use ..HEAD as range which has no commits so it passes without doing anything, so right now it is clearly broken.

We would have to fetch main and the PR to get this to work I believe or at least get enough commits so the sparse commit does not get associated with the other unrelated files in git show. I am currently just pushing some changes to test things.

The validation was passing for me locally.

right, we have the full history.

[mtrmac]

And with the current makefile if EPOCH_TEST_COMMIT is empty we use ..HEAD as range which has no commits so it passes without doing anything, so right now it is clearly broken.

Oh, so the version of this PR a few hours ago was not actually working, I didn’t notice.

---

So I think I have a complete enough hypothesis:

• Our main does not pass the whitespace check
• We were doing actions/checkout with fetch-depth: ${{ github.event.pull_request.commits }} and then validating all fetched commits.
• Looking at the outcome of such a clone, that seems to do a breath-first fetch of that depth. I.e., for a 7-commit PR, it also includes 7 commits from main
• We then validate all of them, including the commits from main, and that happens to include commits where we don’t have a parent
• So, git’s log --check’s (grafted) version of that commit looks like it contains the full tree of the repository, and --check reports all of the preexisting failures.

And the fix to that is to be precise about the set of incoming commits instead of relying on depth. Luckily Git does tell us: per #537, `git-validation -range "${{ github.event.pull_request.base.sha }}..HEAD" looks like the precise set we care about.

(This is me now paying down the debt from #33 (comment) )

[mtrmac]

(It wouldn’t also hurt to fix all of the whitespace errors in main, just for the aesthetics of the thing, but that’s not for this PR.)

[Luap99]

@mtrmac Thank you, it should work now.

Also double checked in #543 that we don't get anything odd if there is only a single commit.

[mtrmac]

Thanks!

[mtrmac]

(I’d be inclined to do make git-validation EPOCH_TEST_COMMIT=… to make the consumer and option passing explicit, but the comment + unique variable name work perfectly fine as is.)

[Luap99]

FWIW, I explicitly made the decision to do it that way because github actions is full of fotguns.

${{ github.event.pull_request.base.sha }} is resolved before it get passes to the shell if we do it in the shell line. That means if an attacker controls the value they can inject custom code, i.e. just end the quote in the value.

Now of course none of this applies here as, a) this is a pull request target where users can just modify the code anyway and b) it should be somewhat safe that we always get a hash there that an attcker can not swap. That said I think it is best practise to avoid the ${{ }} block inside the script section for this reason.

[mtrmac]

That means if an attacker controls the value they can inject custom code, i.e. just end the quote in the value

Oh… now I remember that coming up :) Thanks for the reminder.