stage_executor: history should include heredoc summary correctly #6041
Conversation
imagebuildah/stage_executor.go
Outdated
| } | ||
| } | ||
| if buildArgs != "" { | ||
| return "|" + strconv.Itoa(len(strings.Split(buildArgs, " "))) + " " + buildArgs + " /bin/sh -c " + shArg + heredoc + appendCheckSum, nil |
There was a problem hiding this comment.
Correct result, but harder to follow than having this block compute a value that also gets combined with others and is returned from one place. It would be very easy to mistakenly introduce a similar bug in the future.
imagebuildah/stage_executor.go
Outdated
| if buildArgs != "" { | ||
| return "|" + strconv.Itoa(len(strings.Split(buildArgs, " "))) + " " + buildArgs + " /bin/sh -c " + shArg + heredoc + appendCheckSum, nil | ||
| } | ||
| result := "/bin/sh -c " + shArg + heredoc | ||
| return result + appendCheckSum, nil |
There was a problem hiding this comment.
Why concatenate multiple values to produce one string, then concatenate another with it in the return?
getCreatedBy ignores heredoc summary when build args are specified following PR makes sure the behaviour is correct. Also test is modified to make sure buildah correctly burst cache if heredoc content is changed. Closes: containers/podman#25469 Signed-off-by: flouthoc <[email protected]>
flouthoc
force-pushed
the
heredoc-buildarg
branch
from
019fdcf to
4dd300b
Compare
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: flouthoc, nalind 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 |
getCreatedByignores heredoc summary when build args are specified following PR makes sure the behaviour is correct.Also test is modified to make sure buildah correctly burst cache if heredoc content is changed.
Closes: containers/podman#25469
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?