gitlint is showing all files as changed in github CI

[jorisroovers]

Gitlint is incorrectly listing all files as changed in CI, not only those that have changed:
https://github.com/jorisroovers/gitlint/actions/runs/3014300199/jobs/4935936697#step:14:104

Is this perhaps due to a misuse of refspec? Should we be passing a -1 argument like we do with log?

[jorisroovers]

Found the issue.

By default the checkout action only checks out the commit being modified.
This behavior can be modified using the fetch-depth option (setting it to 0 checks out the entire history).

In gitlint, we basically invoke git diff-tree <some flags> --root <sha>:

gitlint/gitlint-core/gitlint/git.py

Line 369 in 8276aec

"diff-tree", "--no-commit-id", "--numstat", "-r", "--root", self.sha, _cwd=self.context.repository_path

It's the --root flag that is the culprit:

When --root is specified the initial commit will be shown as a big creation event. This is equivalent to a diff against the NULL tree.

We added the --root flag in 32d8cc2, in order to be able to detect the changed files of an initial commit on a repo. This works as intended when the entire git history is checked out, but when only checking out a single commit (as happens in CI), this will cause git to return all files as having changed.

While this is easily fixed in CI by setting fetch-depth:0 as part of the checkout action (example fix), I think gitlint's behavior is actually incorrect here. Gitlint should only show files changed in the commit itself, not including any files from the initial commit.

Need to brainstorm and research a bit whether that means we shouldn't show any changed files when linting the initial commit (i.e. remove --root from the diff-tree invocation), or whether we should treat the initial commit as a special case and only include --root then.

[jorisroovers]

So when a single commit is checked out from git (a shallow clone), its parent information is lost.

Typically, the way to determine whether a commit is one of the initial commits on a repo, is by checking whether the commit has parents using a variation of:

> git rev-list --max-parents=0 HEAD
765a4427f80ccdda61bc78e2680bda14787b2a4e # this is the sha of the initial commit

We could have used the output of this to figure out when to include the --root flag in the our diff-tree command: only include it when the current commit is an initial commit, i.e. it has no parents. But so it turns out this won’t work because that parent information is also lost in a shallow commit.

I did find out that such shallow commits that are not initial commits have a so-call grafted annotation, to indicate they weren’t really an initial commit within their commit history.

While technically it might be possible to use this grafted annotation to figure out whether any given commit without parents is an initial commit or not, and depending on that use the --root flag in the git diff-tree command, I think I’m starting to go down a rabbit hole of complexity here for an edge case that might not even be used by anyone.

In addition, it’s worth calling out that gitlint actually doesn’t adhere to the same semantics as git does by default, i.e. git by default won’t show you files that were part of the initial commit as being changed files.

As such, I’m inclined to revert gitlint behavior on this to align with default git semantics. In other words, gitlint won’t list files that were added in the initial commit as part of its changed_files list. Ideally, we might want add a flag to toggle this behavior, but I’m not sure whether that’s a level of implementation and maintenance complexity I want to add right now. Will let this rest for a bit…

[chbndrhnns]

The same behaviour applies to Gitlab CI.

[chbndrhnns]

As a side note, I am stuck on 0.15 now because of that issue and this means I cannot run a recent version of black which depends on click>=8 while gitlint 0.15 pins click to version 7.

[webknjaz]

@jorisroovers FYI here's an example of using GitHub API to figure out how many commits a PR has (up to a certain limit, w/o pagination) cherrypy/cheroot#579. That PR is only implemented for pull_request event triggers, but maybe there's enough details in the context for retrieving this information for the push events too?

[chbndrhnns]

FYI here's an example of using GitHub API

I would very much want this to work agnostic of the Git provider as I am using GitLab in a corporate environment.

[webknjaz]

@chbndrhnns this won't be possible for as long as you want to only lint commits from PRs/MRs. Each of the providers needs to be handled separately. Figuring out what to lint may be embedded into gitlint, technically, but this would only work with envs for which it'll be implemented, any new/exotic cases would need to be handled in the job definitions outside the call to gitlint.

[webknjaz]

FWIW, I don't use GitLab actively these days, but I'm happy to consult on/review the GHA-related things @jorisroovers.