app: project: fix git diff --exit-code logic

[marc-hb]

Fixes 2020 commit a53ec10 ("west diff: only print output for projects with nonempty diffs")

Fixes #731

At least with git version 2.46.0, --exit-code is 0 when there has been a merge conflict! git bug or feature? Either way this causes west diff to wrongly assume there is no diff in that repo.

Two other issues:

1. if returncode is higher than 1 (= a git diff failure) AND -v is used, then failed.append(project) is never run.
2. stderr is never, ever printed

Try for instance west diff --fubar and west -v --fubar (which are possible since commit 73aee32). One fails and the other does not. Neither prints any useful error.

The verbosity level should never have any side-effect.

To fix:

• Assume there is a diff when --exit-code is zero AND stdout is not empty. Treat this the same as --exit-code=1
• Always failed.append(project) when --exit-code > 1
• Print stderr when there is a diff or in verbose mode.

This drops one elif which also simplifies the logic.

[tejlmand]

At least with git version 2.46.0, --exit-code is 0 when there has been a merge conflict! git bug or feature? Either way this causes west diff to wrongly assume there is no diff in that repo.

Have not tested with git 2.46.0 but could it be that somehow the conflicting files has been staged ?

One thing I have observed is that if there are staged changes, then git diff --exit-code will also return 0.
To check if there are staged changes, one must actually run git diff --cached --exit-code.

Is not considering staged files perhaps another potential bug.

[marc-hb]

Thanks @tejlmand for taking a look!

One thing I have observed is that if there are staged changes, then git diff --exit-code will also return 0.
To check if there are staged changes, one must actually run git diff --cached --exit-code.

Yes and I think that Works As Intended. In either case, the west diff output and status will follow the git diff output. Note you can pass west diff --staged now thanks to @pdgendt .

This --exit-code bug is about something different.

Have not tested with git 2.46.0 but could it be that somehow the conflicting files has been staged ?

git merge (or git stash pop) will stage non-conflicting changes and NOT stage conflicting changes. So let's consider a single, conflicting change to keep things simple. In that case nothing is staged. The git bug/feature is then "simply" that the --exit-code option is ignored when the --cc "combined diff" option is used. Also, the --cc combined diff option is automatically added when running a plain git diff when "you have unmerged paths" (-cc is not automatically added when running git diff HEAD; then --exit-code works if "you have unmerged paths")

It's not too surprising that --exit-code is not compatible with a "combined diff" when you think about it because the combined diff is comparing not just 2 but 3 different versions.

[tejlmand]

Have not tested with git 2.46.0 but could it be that somehow the conflicting files has been staged ?

git merge (or git stash pop) will stage non-conflicting changes and NOT stage conflicting changes.

I know it wont, but the user can. Have you ever experienced someone committing code containing conflict markers 🙈
So I was just being a bit curios.

I actually tried to play a little, and can inform that same behavior is seen with git v2.34.1.
conflict markers are shown in the diff, but will still give an exit code of 0.

[tejlmand]

as a side-note, git diff --check will return an exit value != 0 when there are conflict markers, but imo we only want to run git diff once, so --check is not really useful in this context.