-
Notifications
You must be signed in to change notification settings - Fork 58
Ignore exit code of clang-format-diff #35
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
This helps debug what is happening when a git command fails in a test.
Newer versions of git don't allow this by default. Specify the appropriate config option on the command line to allow it.
Starting with 18.x, clang-format-diff exits with a nonzero exit code if there was a diff to be applied. This breaks the apply-format script because it will exit without printing the diff. Instead, ignore the exit code and proceed to print the diff. Closes: barisione#33
Note, clang-format-diff 18.x has another regression: llvm/llvm-project#86776 This causes the |
> "$patch_dest" \ | ||
|| exit 1 | ||
> "$patch_dest" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it makes sense to ignore all exit codes. From version 18, clang-format-diff exits with status 1 if there are diffs, but other statuses are possible, for example, if there is an invalid command-line option then the exit status is 2:
$ /usr/share/clang/clang-format-18/clang-format-diff.py --no-such-option
usage: clang-format-diff.py [-h] [-i] [-p NUM] [-regex PATTERN] [-iregex PATTERN] [-sort-includes]
[-v] [-style STYLE] [-fallback-style FALLBACK_STYLE] [-binary BINARY]
clang-format-diff.py: error: unrecognized arguments: --no-such-option
$ echo $?
2
So we ought to follow the clang-format-diff pipeline with something like:
[ $? -gt 1 ] && exit $?
return subprocess.check_output(args, **kwargs) | ||
try: | ||
return subprocess.check_output(args, **kwargs) | ||
except subprocess.CalledProcessError as exc: | ||
exc.add_note(exc.output) | ||
raise exc |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change seems to be unrelated to the clang-format-diff exit status. I suggest making separate PR(s) for this and the other changes.
Adapt to breaking change in clang-format-diff 18.x. The PR also includes a few unrelated cleanups.
Closes: #33