more CI improvements #33
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
File renamed without changes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,96 @@ | ||
| name: validate | ||
|
|
||
| on: | ||
| push: | ||
| branches: | ||
| - main | ||
| pull_request: | ||
| branches: | ||
| - main | ||
|
|
||
| permissions: read-all | ||
|
|
||
| env: | ||
| LINT_VERSION: v2.1.6 | ||
|
|
||
| jobs: | ||
| codespell: | ||
| runs-on: ubuntu-24.04 | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| - name: install deps | ||
| # Version of codespell bundled with Ubuntu is way old, so use pip. | ||
| run: pip install --break-system-packages codespell==v2.4.1 | ||
| - name: run codespell | ||
| run: codespell --dictionary=- | ||
|
|
||
| lint: | ||
| runs-on: ubuntu-24.04 | ||
| defaults: | ||
| run: | ||
| working-directory: ./common | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| with: | ||
| fetch-depth: 2 | ||
| - uses: actions/setup-go@v5 | ||
| with: | ||
| go-version: 1.25.x | ||
| # By default the go cache will only use go.sum in the root which we don't have, | ||
| # make it use for all checksum files. | ||
| # https://github.com/actions/setup-go?tab=readme-ov-file#caching-dependency-files-and-build-outputs | ||
| cache-dependency-path: "**/go.sum" | ||
| - name: install deps | ||
| run: | | ||
| sudo apt-get -qq update | ||
| sudo apt-get -qq install libseccomp-dev libgpgme-dev libbtrfs-dev libsubid-dev | ||
| - name: lint-common | ||
| uses: golangci/golangci-lint-action@v8 | ||
| with: | ||
| version: "${{ env.LINT_VERSION }}" | ||
| args: --verbose | ||
| working-directory: ./common | ||
| # Extra linters, only checking new code from a pull request. | ||
| - name: lint-common-extra | ||
| uses: golangci/golangci-lint-action@v8 | ||
| with: | ||
| args: --config=.golangci-extra.yml | ||
| version: "${{ env.LINT_VERSION }}" | ||
| only-new-issues: true | ||
| working-directory: ./common | ||
| - name: lint-image | ||
| uses: golangci/golangci-lint-action@v8 | ||
| with: | ||
| version: "${{ env.LINT_VERSION }}" | ||
| args: --verbose | ||
| working-directory: ./image | ||
| - name: lint-storage | ||
| uses: golangci/golangci-lint-action@v8 | ||
| with: | ||
| version: "${{ env.LINT_VERSION }}" | ||
| args: --verbose | ||
| working-directory: ./storage | ||
|
|
||
| - name: validate seccomp | ||
| run: ./tools/validate_seccomp.sh ./pkg/seccomp | ||
|
|
||
| git-validate: | ||
| # only run this on PRs | ||
| if: github.event_name == 'pull_request' | ||
| runs-on: ubuntu-24.04 | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| with: | ||
| # By default github actions creates a merge commit which fails the validation, | ||
| # we only must validate the actual commits of the author. | ||
| ref: ${{ github.event.pull_request.head.sha }} | ||
| fetch-depth: ${{ github.event.pull_request.commits }} | ||
| - uses: actions/setup-go@v5 | ||
| with: | ||
| go-version: 1.25.x | ||
| # See comment on lint task | ||
| cache-dependency-path: "**/go.sum" | ||
| - name: install deps | ||
| run: go install github.com/vbatts/git-validation@v1.2.2 | ||
| - name: run git-validation | ||
| run: git-validation -q -run DCO,short-subject,dangling-whitespace -range "HEAD~${{ github.event.pull_request.commits }}..HEAD" | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,7 +9,6 @@ pwd | |
| ls -l | ||
| make install.tools | ||
| showrun make local-binary | ||
|
|
||
| case $TEST_DRIVER in | ||
| overlay) | ||
|
|
||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Low-priority: Does this correctly handle merge requests and the like?
I think conceptually we should include all commits not already present on the target branch, and it’s not obvious to me that counting commits is an accurate way to achieve that.
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.
Why would counting not work? This should return the number of commits in the PR (well what github reports) and then we lint the diff between that commit in HEAD which should aalways check all commits in any given PR.
Do you mean like merge commits within a PR? They should still be reported and checked as normal commit I think. I mean feel free to tests the scenario you can think of out.
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.
Yes, I was thinking something like containers/image#2876 which goes
i.e the branch is “rebased on top of upstream/main” but it starts with a merge of an older commit. If I read the specification of
~correctly, this will follow the “first parent” of the merge when following the ancestor chain, i.e. the incoming PR gets to choose which of the merge parents is included in-range, by choosing how to order the parents of the merge commit.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.
checking out that pr with git pr I see this:
So yeah it gets more commits but is also got all commits from the PR so I think that seems fine then?
I mean of course there is the risk that it fails on a commit not in that PR but that should not happen if all PRs were validated before I hope.
Although I need to double check how this interacts with the checkout action as I also have to say checkout X commits there.
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.
(I’ll try to set up a definite reproducer, but I want to clean my inbox of the re-filed issues first.)