simplify checkWorkingCopyClean to make backporting easier?

[rmuir]

Description

I run into trouble when backporting, the checkWorkingCopyClean check always wants to fail on things that are actually in gitignore, from switching branches. IMO, working copy is clean as "git status" has no output.

Should we implement this differently? at my dayjob, I use the following one-liner to implement this check, which will respect .gitignore:

check-working-copy:
  @test -z "`git status --porcelain`" || (echo "working copy is dirty" && git status --porcelain && exit 2)

[rmuir]

I do this on build side, rather than locally. so you might want to tweak it if you want to ignore "explicitly git-added files" which I think is our use-case. git status has a lot of options to do just that?

[dweiss]

Ok, I see what's wrong. The script doesn't fail on things that are in .gitignore - it fails on directories that are not empty and contain git-ignored things. For example:

buildSrc/build
buildSrc/.gradle

Both build and .gradle are .gitignore-d so buildSrc is "empty" for git. We handle this check manually for some reason - I'll have to take a look as to why.

We already use git status --porcelain to detect changes in github actions [1]. I think the reason for not using the command-line git in gradle scripts was to keep it within Java... but I don't think it matters much. We can switch to forking an external command. We can also move it to CI builds only.

[1] https://github.com/apache/lucene/blob/main/.github/workflows/run-checks-gradle-upgrade.yml#L67

[rmuir]

@dweiss yes that's the case i hit, it is just the "switching branches from main" use-case and it seems to trip on things such as buildSrc and benchmark-jmh, i end out manually rm -rf'ing them after switching.

in my uses of git-status checks, I actually run the git-status in CI after all the build logic runs, to make sure no tasks/tests/etc modify the tree. It is a different concern maybe than what we might be doing with it (I'm not sure). e.g. I'm not trying to detect "you forgot to git-add"

[dweiss]

I think this logic is flawed:

        // git ignores any folders which are empty (this includes folders with recursively empty sub-folders).
        def untrackedNonEmptyFolders = status.untrackedFolders.findAll { path ->
          File location = file("${rootProject.projectDir}/${path}")
          boolean hasFiles = false
          Files.walkFileTree(location.toPath(), new SimpleFileVisitor<Path>() {
            @Override
            FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException {
              hasFiles = true
              // Terminate early.
              return FileVisitResult.TERMINATE
            }
          })
          return hasFiles
        }

we shouldn't do anything about untrackedFolders. Looking at jgit's implementation, I don't even see the reason it's there and I can't remember why I (or somebody else) added it.

[dweiss]

#13728 ?

[uschindler]

Thanks.

The purpose of this whole task was the following: We wanted to check on Jenkins that after starting with a clean checkout, running all tests,... the working copy is not dirty. This was introduced before we had security manager in tests.

The check on a developers computer was not really wanted, so originally the task was only executed on CI runs.

But anyways the check should have two modes:

• very hard: complain about everything not clean (on Jenkins/CI): A modified and already added file is a violation, too. This shozld be a separate task only executed on Jenkins/CI.
• developer mode: Just check that all files were added correctly and there are no untracked files. This check should be possible to run before committing! IMHO "gradlew validate"

We should really restore this old behaviour with the two modes: CI only after running tests and "gradlew validate" (only check sanity of checkout.

[dweiss]

The check on a developers computer was not really wanted, so originally the task was only executed on CI runs.

I don't know about that - I think it's one of the original reasons I added it to precommit... But it's been a while.

Also, both modes you mention are actually very similar: nothing in the code ever calls "git add" so anything touched will be either untracked or modified by definition. I'm not sure I understand the difference. A notable exception is if something created a directory (empty or with something that is in .gitignore) - then this change would go unnoticed.

[uschindler]

The check on a developers computer was not really wanted, so originally the task was only executed on CI runs.

I don't know about that - I think it's one of the original reasons I added it to precommit... But it's been a while.

Also, both modes you mention are actually very similar: nothing in the code ever calls "git add" so anything touched will be either untracked or modified by definition. I'm not sure I understand the difference. A notable exception is if something created a directory (empty or with something that is in .gitignore) - then this change would go unnoticed.

The check was previously done on Subversion, so it was just ported over long ago.

If the current code works for detecting any changes after a clean checkout and running tests, we are fine.

P.S.: Sorry for my ignorance: I never do "git add" manually, because Tortoise does it in a more "subversion-like" way which I prefer. You just click some checkboxes, enter a log message and cxommit. Very easy. I never understood the horrible git cli, I hate it because it is completely unintuitive (subversion was). Maybe its like with US showers at Mike's home.

[dweiss]

Come on, Uwe - you're brilliant, I don't think git is beyond your ability to grasp. You just want those shiny icons and buttons, don't you... :)

[rmuir]

I never understood the horrible git cli, I hate it because it is completely unintuitive

I agree. latest version has 161 subcommands but you need to stick to your guns and restrict yourself to only 5 or 10 of its commands, then it is useful. and definitely only use commands from the ones listed as "common" in git -h output. that is the trick to dealing with its CLI.

[dweiss]

About right. These are my top three:

git status
git add -A .
git commit -m "foo"

and these are used much, much more rarely:

git cherry-pick
git branch
git log
git reset (--hard)
git merge --no-ff ...
git reflog

This about covers my git knowledge. Anything else I have to go and RTFM.

latest version has 161 subcommands

Isn't it like TeX and LaTeX - that there is a very fundamental core and lots of other commands built around it? I remember this was amusing when I came across it a while ago: https://github.com/chrisdickinson/git-rs