From b1a2a3121ff68bb978ab0eeb0247478d125f0864 Mon Sep 17 00:00:00 2001 From: Marc Herbert Date: Fri, 30 Aug 2024 21:24:28 +0000 Subject: [PATCH] app: project: fix git diff --exit-code logic Fixes 2020 commit a53ec10cf28b ("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 73aee322788c). 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. Signed-off-by: Marc Herbert --- src/west/app/project.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/west/app/project.py b/src/west/app/project.py index 7d998228..54c35493 100644 --- a/src/west/app/project.py +++ b/src/west/app/project.py @@ -89,6 +89,7 @@ def _handle_failed(self, args, failed): # and report errors if anything failed. if not failed: + self.dbg(f'git {self.name} failed for zero project') return elif len(failed) < 20: s = 's:' if len(failed) > 1 else '' @@ -804,13 +805,20 @@ def do_run(self, args, user_args): extra_args=user_args, capture_stdout=True, capture_stderr=True, check=False) - if cp.returncode == 0: + + # We cannot trust --exit-code alone, for instance merge + # conflicts return 0 with (at least) git version 2.46.0. See + # west issue #731 + some_diff = cp.returncode == 1 or (cp.returncode == 0 and len(cp.stdout) > 0) + if not some_diff: no_diff += 1 - if cp.returncode == 1 or self.verbosity >= Verbosity.DBG: + if some_diff or self.verbosity >= Verbosity.DBG: self.banner(f'diff for {project.name_and_path}:') self.inf(cp.stdout.decode('utf-8')) - elif cp.returncode: + self.inf(cp.stderr.decode('utf-8')) + if cp.returncode > 1: failed.append(project) + if failed: self._handle_failed(args, failed) elif self.verbosity <= Verbosity.INF: