Skip to content

KAFKA-21327: Replace JGit calls in build.gradle with the git CLI via providers.exec - #23905

Open
dejan2609 wants to merge 3 commits into
apache:trunkfrom
dejan2609:KAFKA-21327
Open

KAFKA-21327: Replace JGit calls in build.gradle with the git CLI via providers.exec#23905
dejan2609 wants to merge 3 commits into
apache:trunkfrom
dejan2609:KAFKA-21327

Conversation

@dejan2609

@dejan2609 dejan2609 commented Oct 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Notes:

  • JIRA ticket: KAFKA-21327
  • Rationale: this is one of the prerequisites for KAFKA-21326 (Enable Gradle configuration cache) which blocks upgrade to Gradle 10: KAFKA-20102 Preparations for Gradle 10 update: see this jira comment
  • ℹ️ this PR does not enable the configuration cache
  • drafted and smoke tested in a session with Claude Opus 5.5
  • requires more manual (human) testing; testing steps will be added ASAP additional testing has been performed, see testing comment below

Drafted with Claude, then reviewed and edited.

❌ Pitfall:

  • build.gradle opens the repository with JGit at configuration time to compute the commit id and the RAT excludes.
  • With the configuration cache enabled, Git.open starts external processes (git --version, git config --system ...) that Gradle cannot track, so it reports configuration cache problems and fails the build by default.

💡 Solution:

  • build.gradle now runs git rev-parse HEAD and git ls-files -z --others --directory through providers.exec, so Gradle tracks their output as configuration inputs. A new commit or a change in untracked files invalidates the cache.
  • Git runs with --git-dir=.git --work-tree=., so it always reads the root directory's own repository, regardless of GIT_DIR or a repository in a parent directory. This also skips git's safe.directory check, but Gradle is already running that checkout's build script.

Behaviour changes:

  • In a git worktree, the build now gets the real commit id and runs RAT, instead of using unknown and skipping RAT.
  • If git cannot read HEAD, for example because it is not installed, the build logs a warning, uses unknown and skips RAT, as it does for a source tarball. With JGit, a missing git still gave the real commit id and ran RAT, while cases such as an empty .git directory failed the build.
  • An untracked nested git repository is now excluded from RAT.

The contents of the Kafka distribution are unchanged.

This also removes an implicit dependency: since KAFKA-21086 the org.eclipse.jgit import in build.gradle relied on JGit being on the build script classpath transitively via the Spotless plugin; related PR ⬇️

Note: :generator submodule keeps its own JGit dependency (fetched from gradle/dependencies.gradle).

Co-Authored-By: Claude Opus 5.5 noreply@anthropic.com

@github-actions github-actions Bot added triage PRs from the community build Gradle build or GitHub Actions small Small PRs labels Oct 21, 2026
@github-actions

github-actions Bot commented Oct 29, 2026

Copy link
Copy Markdown

A label of 'needs-attention' was automatically added to this PR in order to raise the
attention of the committers. Once this issue has been triaged, the triage label
should be removed to prevent this automation from happening again.

dejan2609 and others added 2 commits November 7, 2026 17:23
@dejan2609 @claude
KAFKA-21327: Replace JGit calls in build.gradle with the git CLI via …
9c9cc30 
…providers.exec

build.gradle opened the repository with JGit at configuration time to
compute the commit id and the RAT excludes. With the configuration cache
enabled, Git.open starts external processes (`git --version`,
`git config --system ...`) that Gradle cannot track, so it reports
configuration cache problems and fails the build by default.

Use providers.exec to run `git rev-parse HEAD` and
`git ls-files --others --directory` instead. Gradle tracks their output
as configuration inputs, so a new commit or a new untracked file
correctly invalidates the cache. The RAT exclude set is unchanged: on a
working tree, JGit's clean().setDryRun(true) output matches
`git ls-files --others --directory` exactly.

This also removes an implicit dependency: since KAFKA-21086 the
org.eclipse.jgit import in build.gradle relied on JGit being on the
build script classpath transitively via the Spotless plugin (:generator
keeps its own JGit dependency).

.git is now checked with exists() instead of isDirectory(), so builds in
a git worktree (where .git is a file) also get the real commit id and
run RAT.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@dejan2609 @claude
KAFKA-21327: Handle unusual file names and unusable git checkouts
31cf99c 
Moving build.gradle from JGit to the git CLI made the build depend on
how git formats its output, on git's environment and on a working git
command. This fixes the resulting regressions.

Changes:
- Read `git ls-files` with -z and split on NUL. Without -z, git quotes
  and escapes names with non-ASCII characters or double quotes, so those
  RAT excludes matched nothing and RAT could fail on an untracked file.
- Run git with --git-dir=.git --work-tree=. so that it reads the root
  directory's own repository. This overrides GIT_DIR and GIT_WORK_TREE,
  stops the search through parent directories and still works in git
  worktrees. Previously, a GIT_DIR pointing to another repository
  silently gave that repository's commit id and made RAT exclude almost
  every file.
- Read HEAD once during configuration. If git cannot read it, log a
  warning with the reason and handle the tree like a source tarball: the
  commit id is `unknown` and RAT is disabled. isGitCheckout is now true
  only when HEAD can be read.

Decisions:
- Fall back rather than fail, so that builds keep working without a
  usable git, for example in a JDK-only container. The cost is that a
  broken git skips RAT with only a warning.
- Use explicit git paths rather than removing variables from the
  environment: removing them through the providers.exec environment had
  no effect when tested.
- An explicit --git-dir skips git's safe.directory check. Gradle already
  runs that checkout's build script, so this adds no meaningful risk.
- Keep git's own message in the warning, although reading it takes a few
  more lines.

Commit id and RAT in each .git state:
- Normal clone, git worktree, -PcommitId, GIT_DIR set: the expected
  commit id, RAT active.
- Dubious ownership, simulated with GIT_TEST_ASSUME_DIFFERENT_OWNER: the
  real commit id, RAT active.
- No .git, also inside a parent repository: `unknown`, RAT skipped, no
  warning.
- Dangling .git file, empty .git directory (also inside a parent
  repository), no commits yet, git not installed: `unknown`, RAT skipped,
  with a warning.

RAT, compared with JGit on the same tree:
- Clean tree, and untracked non-ASCII names, double quotes, spaces,
  empty and untracked directories, a file in a tracked directory, ignored
  files: same report apart from the timestamp. The excludes were also
  compared directly on a tree with the non-ASCII and double quote names,
  and were identical and in the same order.
- Untracked nested git repository (without commits): RAT now passes
  because the directory is excluded. JGit scanned its files and failed.

Distribution: comparing a clean releaseTarGz with the JGit build gave the
same 225 files. Jar entries differ only in the commitId of the 9 jars
with a version file, jar entry timestamps also differ, and the site-docs
contents are identical.

Configuration cache: the entry is reused when nothing changed and after
an unrelated environment variable, GIT_DIR, a new ref or git gc. A new
commit or a change in untracked files invalidates it.

Known limitations:
- With the configuration cache enabled and git not installed, the build
  fails, because Gradle reports the process that could not start as a
  configuration cache problem. This branch does not enable the cache.
- A backslash in an untracked name fails RAT with `Cannot read header`,
  as it did with JGit.
- With LC_ALL=C on JDK 25, a non-ASCII untracked name fails with
  FileNotFoundException, while JGit passes. LC_ALL=C on JDK 17 passes.
  These locale checks ran on the -z change, before the rest of this
  commit was added.
- GIT_INDEX_FILE is still respected.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@dejan2609 @claude
KAFKA-21327: Replace JGit in build.gradle with the git CLI
fd4fad7 
build.gradle used JGit at configuration time to compute the commit id
and the RAT excludes. Git.open starts external processes (`git --version`
and `git config --system`) that the configuration cache reports as
problems, which is one of the things preventing the cache from being
enabled. This change does not enable it. It also removes the script's
implicit dependency on the JGit jar that the Spotless plugin brings in.

build.gradle now runs `git rev-parse HEAD` and
`git ls-files -z --others --directory` through providers.exec, so that
Gradle tracks their output as configuration inputs. Git runs with
--git-dir=.git --work-tree=. so that it always reads the root
directory's own repository.

Behaviour changes:
- In a git worktree, the build now gets the real commit id and runs RAT,
  instead of using `unknown` and skipping RAT.
- If git cannot read HEAD, for example because it is not installed, the
  build logs a warning, uses `unknown` and skips RAT, as it does for a
  source tarball. Some of these cases used to fail the build, and a
  missing git used to work with JGit.
- An untracked nested git repository is now excluded from RAT.

The Kafka distribution content is unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@dejan2609

dejan2609 commented Nov 7, 2026

Copy link
Copy Markdown
Contributor Author

Testing notes

Tests were run in a session with Claude, on throwaway clones, using Linux, Gradle 9.8.2, git 2.43.0 and JDK 25 (plus JDK 17 where noted). The baseline is trunk ef6a6c1edb with JGit, plus a temporary doLast block in the rat task that prints its includes and excludes. The final code is the tree of fd4fad7 (this commit is empty and basically serves as a placeholder for the final commit message).

RAT

Untracked entries depend on the tree state when Gradle configures the build, so each comparison needs the same tree state. For example, run the first commit twice and compare its second run.

Untracked content JGit This PR
None, or names with non-ASCII characters, " or spaces pass pass, same report
Empty directory, untracked directory, file in a tracked directory, ignored files pass pass, same report
Nested git repository (no commits) fail, scans its files pass, directory excluded
Name with a backslash fail, Cannot read header fail, Cannot read header

"Same report" means identical apart from the timestamp. On a tree with the non-ASCII and " names, the excludes were also identical and in the same order.

Commit id and RAT by .git state

Checked with clients:createVersionFile rat:

State JGit This PR
Normal clone real id, RAT active same
-PcommitId=... given value, RAT active same
No .git, also inside a parent repository unknown, RAT skipped same
Git worktree unknown, RAT skipped real id, RAT active
GIT_DIR set to another repository real id, RAT active same
Dubious ownership (simulated with GIT_TEST_ASSUME_DIFFERENT_OWNER) real id, RAT active same
git not installed real id, RAT active unknown, RAT skipped, warning
Dangling .git file unknown, RAT skipped unknown, RAT skipped, warning
Empty .git directory build fails unknown, RAT skipped, warning
git init without commits build fails unknown, RAT skipped, warning
Empty .git directory inside a parent repository not tested unknown, RAT skipped, warning

Distribution

Compared a clean releaseTarGz from the baseline and the final code:

  • Both have the same 225 files.
  • Jar entries differ only in commitId, in the 9 jars with a version file. Jar entry timestamps also differ.
  • The 196 site-docs files have identical contents.
  • kafka-transactions.sh --version prints the expected commit id.

Configuration cache

Checked with --configuration-cache rat only. This PR does not enable the cache, and the build still fails with it enabled on other task graphs.

Change between runs Cache entry
None, an unrelated environment variable, GIT_DIR, a new ref, git gc reused
New commit, added or removed untracked file invalidated
git not installed build fails with a configuration cache problem, even with the fallback

Known gaps

  • With LC_ALL=C, a non-ASCII untracked name passes on JDK 17 but fails on JDK 25 with FileNotFoundException, while JGit passes on both. This ran on the -z change, before the --git-dir and fallback code, and was not rerun.
  • Without the temporary doLast block, only clients:createVersionFile rat was rerun, and it passed.
  • GIT_INDEX_FILE is still respected by git.

@dejan2609
dejan2609 marked this pull request as ready for review November 7, 2026 16:55
@dejan2609

dejan2609 commented Nov 7, 2026

Copy link
Copy Markdown
Contributor Author

➡️ This PR is ready for review, but please review #23424 (RAT upgrade) first.

Both PRs change the same rat configuration in build.gradle (with a small one-line conflict).

Once #23424 is merged, I will:

  • rebase this PR
  • rerun the RAT tests on the new RAT version
  • update the testing notes comment above

Small note for this PR: last commit is empty (it serves as a placeholder for the final commit message).

@dejan2609 dejan2609 mentioned this pull request Nov 7, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

build Gradle build or GitHub Actions needs-attention small Small PRs triage PRs from the community

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@dejan2609