build: return ExecErrorCodeGeneric when git operation fails instead of relaying error code directly from git
#6092
Conversation
|
Please fill out the description, describing why this is being changed, and describing the user-facing change in terms of what someone running the binary would observe when running it. |
|
@nalind Updated commit and PR body. |
|
My second request was about
I wouldn't expect someone who wasn't familiar with the implementation to be able to make sense of this. |
flouthoc
changed the title
ExecErrorCodeGeneric when git operation fails instead of relaying error code directly from git
@nalind How about this |
| @@ -267,14 +267,14 @@ func cloneToDirectory(url, dir string) ([]byte, string, error) { | |||
| cmd = exec.Command("git", "init", dir) | |||
| combinedOutput, err := cmd.CombinedOutput() | |||
| if err != nil { | |||
| return combinedOutput, gitSubdir, fmt.Errorf("failed while performing `git init`: %w", err) | |||
| return combinedOutput, gitSubdir, fmt.Errorf("failed while performing `git init`: %s", err.Error()) | |||
There was a problem hiding this comment.
It would likely makes sense to add a comment here (once at least for all four cases) why this is done as this seems like an anti pattern.
Only propagate error message from git and let buildah reflect error code `125`. Reason: Buildah should return predicatable error code from the set of defined error codes in exec_codes.go at https://github.com/containers/buildah/blob/main/pkg/cli/exec_codes.go#L6 anything other that predefined error codes introduces inconsistency thus making testing difficult in CI and podman. Users should expect buildah to refect ExecErrorCodeGeneric with error message kept intact from the underlying `git` commands. Signed-off-by: flouthoc <[email protected]>
|
@containers/buildah-maintainers PTAL |
I wouldn't expect someone who wasn't familiar with the implementation to know what |
Done edited the release note. |
|
LGTM |
|
[APPROVALNOTIFIER] This PR is APPROVED Approval requirements bypassed by manually added approval. This pull-request has been approved by: flouthoc, Luap99 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
I found this here containers/podman#25756 |
Only propagate error message from git and let buildah reflect error code
125.Reason: Buildah should return predicatable error code from the set of
defined error codes in exec_codes.go at https://github.com/containers/buildah/blob/main/pkg/cli/exec_codes.go#L6
anything other that predefined error codes introduces inconsistency thus making testing difficult in CI and podman.
Users should expect buildah to refect ExecErrorCodeGeneric with error message kept intact from the underlying
gitcommands.
What type of PR is this?
What this PR does / why we need it:
How to verify it
Which issue(s) this PR fixes:
Special notes for your reviewer:
Does this PR introduce a user-facing change?