Wrong inferred Git workspace status in Go

[flelli]

The command line version sometimes fails when inferring the workspace status.

There are times when git status returns a clean workspace but Nyx gets it dirty from the underlying implementation, and this makes the release type match fail.

[flelli]

The wrong behavior is also outlined in #129

[flelli]

Even when the git status command returns a clean repository, under some circumstances the underlying library returns false when querying the workspace status clean check.

[flelli]

This seems to be a known issue in go-git. Another similar (wrong) behavior has been documented here.

It looks like it fails to check for file changes due to CRLF line endings (maybe it's Windows specific). When the repository contains files with CRLF, git status may return a clean status, while go-git fails to honour the git behavior and returns false.

To confirm this behavior one can remove CRLF endings and the right value is returned.

[Prototik]

I encountered the same issue on Linux, so it's not Windows-specific.

Also I scanned my repo with find -type f -exec file {} \; | grep CRLF, it gives no results, so no any file with CRLF line endings.

Probably another issue caused by go-git/go-git#643, but not sure
Yeah, it's totally caused by LFS, when I get rid off it go-nyx correctly match release type with clean workspace status.

[bergtwvd]

In my repo all text files have LF ending. There is no CRLF ending in any file.

I also get a clean git status from the container shell:

/project # apk add git
fetch https://dl-cdn.alpinelinux.org/alpine/v3.16/main/x86_64/APKINDEX.tar.gz
fetch https://dl-cdn.alpinelinux.org/alpine/v3.16/community/x86_64/APKINDEX.tar.gz
(1/7) Installing ca-certificates (20220614-r0)
(2/7) Installing brotli-libs (1.0.9-r6)
(3/7) Installing nghttp2-libs (1.47.0-r0)
(4/7) Installing libcurl (7.83.1-r5)
(5/7) Installing expat (2.5.0-r0)
(6/7) Installing pcre2 (10.40-r0)
(7/7) Installing git (2.36.3-r0)
Executing busybox-1.35.0-r17.trigger
Executing ca-certificates-20220614-r0.trigger
OK: 19 MiB in 21 packages
/project #
/project #
/project #
/project # git status
On branch main
Your branch is up to date with 'origin/main'.

nothing to commit, working tree clean
/project # ls
README.md  test2.txt
/project #
/project # nyx --info
INFO[0000] Version: '0.2.0-internal.1+timestamp.19700120084829'
/project #
/project # cat .nyx.yml
{ "preset":"extended" }
/project #

[flelli]

Thanks both.

I'm trying to find a convenient workaround as it looks those bugs on go-git have been there forever with no fix.

[flelli]

I still couldn't find a workaround, but it seems the issue with go-git is also about .gitignore (see here and here) and empty folders (see here).

[Prototik]

As a kinda alternative to go-git I propose create cli with jgit (so on java) and package it as docker image, what do you think about it?

[flelli]

Well, if I end up executing an external library I'd rather run git. It'd be cleaner after all, and git is basically available on all hosts whwre Nyx is used (statistically speaking).

[flelli]

I just published an hotfix as 1.3.1-hotfix130.2.

Since there was no suitable library alternative I had to back up to use the local git executable. If it's not available the workaround is not effective.

Reproducing the bug was quite hairy so I'd like to double check with you the workaround works. @bergtwvd and @Prototik can you please give it a try before I merge to the main branch and make it generally available?

In case it doesn't work as expected please send the debug log output. You will find some rows with the workaround #130 prefix and they are significant for this workaround.

Thanks

[flelli]

Workaround summary

I'm leaving notes for my future self about the triage and the actions taken.

Findings

• the bug only affects the Go version because it's due to a bug in the go-git library that under some circumstances returns false when invoking IsClean() (meaning the repository is DIRTY, containing uncommitted changes) even when it should return true
• the proof of the above is running the git status command in the same repository at the same point in time and it returns no pending changes (repository CLEAN)
• the go-git bug is confirmed by the issues listed below
• although the bug seems to show up in different circumstances (see below) the only way we could reproduce it with Nyx was running a sequence of manual steps using the git executable to bring a reporitory to a certain state and only then run nyx infer --debug (the --debug flag is needed to see when the repository is considered DIRTY due to the bug); the same sequence of steps executed using go-git doesn't reproduce the error and this is why it was hard to build automatic tests to reproduce the bug; this:
  • leads to thinking that committing files using go-git makes it run some specific internal operations or apply some tags/flags which prevent the bug to happen
  • imposes using the git executable in automatic tests in order to reproduce the bug
• although the bug was reproduced using the git executable, the automatic integration tests we have now only detect the bug when running on Windows, while running on Linux and Mac runs withouth the bug; this can be seen by the Go integration test matrix here and here
• even considering the above, the bug doesn't show up if the repository only has binary files or the text files do not contain linefeeds (either LF or CRLF) so it must have something to do with text files containing linefeeds
• using Nyx as a standalone executable or within a Docker container doesn't make any difference (which also means that environment variables or global Git configuration files, not local to the repository, are not relevant)

The workaround

Since no other native Go alternative library was available, the only option left was to run the git executable from within Nyx.

The IsClean() method in go_git_repository.go has been changed so that, when go-git's IsClean() returns false (and only in that case, to narrow the impact of the workaround), the git executable is also executed (when available in the local PATH) and if it returns CLEAN for the repository status, it is considered more authoritative and Nyx will take that into accound, discarding go-git's result.

Two additional tests (TestGoGitRepositoryIsCleanWithTextFileContainingLineFeedsUsingEmbeddedLibrary and TestGoGitRepositoryIsCleanWithTextFileContainingLineFeedsUsingGitCommand) have been created in go_git_repository_test.go to reproduce the bug. Likewise, just to make sure the bug did not affect the Java version, the same tests (isCleanWithTextFileContainingLineFeedsUsingEmbeddedLibraryTest and isCleanWithTextFileContainingLineFeedsUsingGitCommandTest) have been replicated in JGitRepositoryTests.java.

All changes required for the workaround have been marked with references (see below) to explain the reason behind the change and make it easy to remove the workaround as soon as possible.

For the workaround to work the git executable must be present in the local PATH. If it's not Nyx won't break but will just output a warning message informing the user that the repository status, when detected as DIRTY may be wrong.

Debug logs regarding the workaround have the workaround #130.

Relevant bugs to monitor

The following bugs are open in go-git and should be monitored so that, when they're fixed, we can remove the workaround:

• worktree.Status not honouring .gitignore
• worktree.Status takes into account line endings
• Worktree.Status() and git status differ on newlines
• worktree.Status() not respecting .gitignore

What to do to remove the workaround

When the above bugs are resolved, or when go-git can be replaced with a more reliable library, the things to di are:

1. edit go_git_repository.go and scan all the TODO: comments I left referencing https://github.com/mooltiverse/nyx/issues/130; in particular the IsClean() method can be cleaned up from the workaround
2. optionally edit go_git_repository_test.go and scan all references to https://github.com/mooltiverse/nyx/issues/130 to find the tests that can be removed (TestGoGitRepositoryIsCleanWithTextFileContainingLineFeedsUsingEmbeddedLibrary and TestGoGitRepositoryIsCleanWithTextFileContainingLineFeedsUsingGitCommand), or just leave them to check against regressions
3. likewise, since the Java version had the same tests (to double check, although the bug doesn't affect the Java version), optionally edit JGitRepositoryTests.java and scan all references to https://github.com/mooltiverse/nyx/issues/130 to find the tests that can be removed (isCleanWithTextFileContainingLineFeedsUsingEmbeddedLibraryTest and isCleanWithTextFileContainingLineFeedsUsingGitCommandTest), or just leave them to check against regressions

[flelli]

Fix is generally available in release 1.3.3.

[bergtwvd]

I was away for work, but just a belated response to confirm that this solution works for me.

Thanks for the effort.

I just published an hotfix as 1.3.1-hotfix130.2.

Since there was no suitable library alternative I had to back up to use the local git executable. If it's not available the workaround is not effective.

Reproducing the bug was quite hairy so I'd like to double check with you the workaround works. @bergtwvd and @Prototik can you please give it a try before I merge to the main branch and make it generally available?

In case it doesn't work as expected please send the debug log output. You will find some rows with the workaround #130 prefix and they are significant for this workaround.

Thanks