[PATCH] gitlab-ci: fix "check" jobs with large merge requests

[PATCH] gitlab-ci: fix "check" jobs with large merge requests

[Patrick Steinhardt]

The "check-whitespace" and "check-style" jobs verify that the changes
proposed by a merge request don't introduce whitespace errors or style
violations. To do so, both of them compare against the base commit of
the merge request.

But both GitHub and GitLab use shallow clones to set up the repository
in CI jobs by default. So if the base commit to compare against is
beyond the shallow boundary then these checks will fail. To avoid that
error we thus configure GitHub CI to disable shallow clones via
`fetch-depth: 0`. But we don't do the equivalent for GitLab, so we see
such errors whenever we have a merge request with more than 50 commits,
which is the configured shallowness for gitlab-org/git.

Fix these issues by setting GIT_DEPTH to 0 for both jobs so that we
always fetch the full history.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
Hi,

one more day, one more CI annoyance. The "check-whitespace" job is
somewhat broken for us in case we have MRs that have more than 50
commits. And while that's not something that happens for patch series
that we send upstream, it does happen for some of our internal
development branches.

See [1] for a pipeline run with these changes.

Thanks!

Patrick

[1]: https://gitlab.com/gitlab-org/git/-/merge_requests/703
---
 .gitlab-ci.yml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 3f24835500..62737279d2 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -256,6 +256,8 @@ check-whitespace:
   image: ubuntu:latest
   stage: analyze
   needs: [ ]
+  variables:
+    GIT_DEPTH: 0
   before_script:
     - ./ci/install-dependencies.sh
   # Since $CI_MERGE_REQUEST_TARGET_BRANCH_SHA is only defined for merged
@@ -276,6 +278,7 @@ check-style:
   variables:
     CC: clang
     jobname: ClangFormat
+    GIT_DEPTH: 0
   before_script:
     - ./ci/install-dependencies.sh
   # Since $CI_MERGE_REQUEST_TARGET_BRANCH_SHA is only defined for merged

---
base-commit: f57c9a73884400380bddd172acc88b569834f909
change-id: 20261016-pks-ci-optional-whitespace-job-ff1effc3fec7

Re: [PATCH] gitlab-ci: fix "check" jobs with large merge requests

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

> The "check-whitespace" and "check-style" jobs verify that the changes
> proposed by a merge request don't introduce whitespace errors or style
> violations. To do so, both of them compare against the base commit of
> the merge request.
>
> But both GitHub and GitLab use shallow clones to set up the repository
> in CI jobs by default. So if the base commit to compare against is
> beyond the shallow boundary then these checks will fail. To avoid that
> error we thus configure GitHub CI to disable shallow clones via
> `fetch-depth: 0`. But we don't do the equivalent for GitLab, so we see
> such errors whenever we have a merge request with more than 50 commits,
> which is the configured shallowness for gitlab-org/git.
>
> Fix these issues by setting GIT_DEPTH to 0 for both jobs so that we
> always fetch the full history.

Hmph, that is annoying.  In theory, you only need to fetch from the
base that the patch series is designed to apply, but I guess that is
not a choice offered.

How much help are we really getting from these two jobs, by the way?
If we disabled them, would we suddenly start seeing uncontrollable
amount of crap patches full of whitespace breakages and style
violations?

Will queue, thanks.

>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> Hi,
>
> one more day, one more CI annoyance. The "check-whitespace" job is
> somewhat broken for us in case we have MRs that have more than 50
> commits. And while that's not something that happens for patch series
> that we send upstream, it does happen for some of our internal
> development branches.
>
> See [1] for a pipeline run with these changes.
>
> Thanks!
>
> Patrick
>
> [1]: https://gitlab.com/gitlab-org/git/-/merge_requests/703
> ---
>  .gitlab-ci.yml | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> index 3f24835500..62737279d2 100644
> --- a/.gitlab-ci.yml
> +++ b/.gitlab-ci.yml
> @@ -256,6 +256,8 @@ check-whitespace:
>    image: ubuntu:latest
>    stage: analyze
>    needs: [ ]
> +  variables:
> +    GIT_DEPTH: 0
>    before_script:
>      - ./ci/install-dependencies.sh
>    # Since $CI_MERGE_REQUEST_TARGET_BRANCH_SHA is only defined for merged
> @@ -276,6 +278,7 @@ check-style:
>    variables:
>      CC: clang
>      jobname: ClangFormat
> +    GIT_DEPTH: 0
>    before_script:
>      - ./ci/install-dependencies.sh
>    # Since $CI_MERGE_REQUEST_TARGET_BRANCH_SHA is only defined for merged
>
> ---
> base-commit: f57c9a73884400380bddd172acc88b569834f909
> change-id: 20261016-pks-ci-optional-whitespace-job-ff1effc3fec7

Re: [PATCH] gitlab-ci: fix "check" jobs with large merge requests

[Patrick Steinhardt]

On Fri, Oct 16, 2026 at 10:15:57AM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > The "check-whitespace" and "check-style" jobs verify that the changes
> > proposed by a merge request don't introduce whitespace errors or style
> > violations. To do so, both of them compare against the base commit of
> > the merge request.
> >
> > But both GitHub and GitLab use shallow clones to set up the repository
> > in CI jobs by default. So if the base commit to compare against is
> > beyond the shallow boundary then these checks will fail. To avoid that
> > error we thus configure GitHub CI to disable shallow clones via
> > `fetch-depth: 0`. But we don't do the equivalent for GitLab, so we see
> > such errors whenever we have a merge request with more than 50 commits,
> > which is the configured shallowness for gitlab-org/git.
> >
> > Fix these issues by setting GIT_DEPTH to 0 for both jobs so that we
> > always fetch the full history.
> 
> Hmph, that is annoying.  In theory, you only need to fetch from the
> base that the patch series is designed to apply, but I guess that is
> not a choice offered.

No, unfortunately it's not.

> How much help are we really getting from these two jobs, by the way?
> If we disabled them, would we suddenly start seeing uncontrollable
> amount of crap patches full of whitespace breakages and style
> violations?

I tend to ignore those myself:

  - "check-style" is not all that useful as we don't fully submit us to
    clang-format's style, so whatever it generates is not authoritative
    anyway.

  - "check-whitespace" was quite useless to me because it breaks rather
    frequently.

I still think that "check-style" _could_ be useful, but only if we fully
commit to clang-format. It would remove a whole class of nitpicks
and thus reduce the burden on both newcomers and reviewers. But that's a
discussion I don't really want to lead :)

Patrick

Re: [PATCH] gitlab-ci: fix "check" jobs with large merge requests

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 2648 bytes --]

Patrick Steinhardt <ps@pks.im> writes:

> On Fri, Oct 16, 2026 at 10:15:57AM -0700, Junio C Hamano wrote:
>> Patrick Steinhardt <ps@pks.im> writes:
>>
>> > The "check-whitespace" and "check-style" jobs verify that the changes
>> > proposed by a merge request don't introduce whitespace errors or style
>> > violations. To do so, both of them compare against the base commit of
>> > the merge request.
>> >
>> > But both GitHub and GitLab use shallow clones to set up the repository
>> > in CI jobs by default. So if the base commit to compare against is
>> > beyond the shallow boundary then these checks will fail. To avoid that
>> > error we thus configure GitHub CI to disable shallow clones via
>> > `fetch-depth: 0`. But we don't do the equivalent for GitLab, so we see
>> > such errors whenever we have a merge request with more than 50 commits,
>> > which is the configured shallowness for gitlab-org/git.
>> >
>> > Fix these issues by setting GIT_DEPTH to 0 for both jobs so that we
>> > always fetch the full history.

The patch itself looks good.

[snip]

>> How much help are we really getting from these two jobs, by the way?
>> If we disabled them, would we suddenly start seeing uncontrollable
>> amount of crap patches full of whitespace breakages and style
>> violations?
>
> I tend to ignore those myself:
>
>   - "check-style" is not all that useful as we don't fully submit us to
>     clang-format's style, so whatever it generates is not authoritative
>     anyway.
>
>   - "check-whitespace" was quite useless to me because it breaks rather
>     frequently.
>

I've used both as a guidance rather than an enforcement and while
neither have a bunch of false positives, they do help here and there.

> I still think that "check-style" _could_ be useful, but only if we fully
> commit to clang-format. It would remove a whole class of nitpicks
> and thus reduce the burden on both newcomers and reviewers. But that's a
> discussion I don't really want to lead :)
>
> Patrick

Sigh! I've tried to steer this for a while, but there are a bunch of
issues:
1. We need to commit and start enforcing.
2. We need to fixate on a version of clang-format, since we support a
   bunch of CI environments, we end up with the lowest common
   denominator. Older versions do lack a bunch of rules
3. There are certain rules which are written down, but we often allow
   some leeway. Like wrapping long lines.
4. There aren't rules for all the styles we want to enforce, which means
   we still have to do a manual check.

So I'm not sure we can ever come to a conclusion here. That's why the
job is also allowed to fail on both CI platforms.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

Re: [PATCH] gitlab-ci: fix "check" jobs with large merge requests

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

> I tend to ignore those myself:
>
>   - "check-style" is not all that useful as we don't fully submit us to
>     clang-format's style, so whatever it generates is not authoritative
>     anyway.
>
>   - "check-whitespace" was quite useless to me because it breaks rather
>     frequently.
>
> I still think that "check-style" _could_ be useful, but only if we fully
> commit to clang-format. It would remove a whole class of nitpicks
> and thus reduce the burden on both newcomers and reviewers. But that's a
> discussion I don't really want to lead :)

Hmph, I haven't seen check-whitespace giving useless false positive
myself.  I tend to agree that check-style would not be useful as it
came fairly late to the project after all the code have been written
long ago.

Re: [PATCH] gitlab-ci: fix "check" jobs with large merge requests

[Patrick Steinhardt]

On Mon, Oct 19, 2026 at 01:09:49PM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > I tend to ignore those myself:
> >
> >   - "check-style" is not all that useful as we don't fully submit us to
> >     clang-format's style, so whatever it generates is not authoritative
> >     anyway.
> >
> >   - "check-whitespace" was quite useless to me because it breaks rather
> >     frequently.
> >
> > I still think that "check-style" _could_ be useful, but only if we fully
> > commit to clang-format. It would remove a whole class of nitpicks
> > and thus reduce the burden on both newcomers and reviewers. But that's a
> > discussion I don't really want to lead :)
> 
> Hmph, I haven't seen check-whitespace giving useless false positive
> myself.  I tend to agree that check-style would not be useful as it
> came fairly late to the project after all the code have been written
> long ago.

Yeah, maybe check-whitespace will become more useful then for GitLab CI,
too, after this patch has landed. In its current form I started ignoring
it because it was broken way too often because of the shallow clones.

Patrick