Set BigFilesTemporaryDir to GetEnv(TMPDIR) if set or /var/tmp #628
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rhatdan 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 |
|
@mheon @vrothberg This is a fairly big regression in podman 3.2 and will need to be backported. |
rhatdan
changed the title
|
LGTM. Another candidate for 3.2.2, then. |
|
Yes, I want to wait until @vrothberg approves. |
libimage/runtime.go
Outdated
| var systemContext types.SystemContext | ||
| if options.SystemContext != nil { | ||
| systemContext = *options.SystemContext | ||
| if systemContext.BigFilesTemporaryDir == "" { | ||
| systemContext.BigFilesTemporaryDir = tmpdir() | ||
| } | ||
| } else { | ||
| systemContext = types.SystemContext{} | ||
| systemContext = types.SystemContext{ | ||
| BigFilesTemporaryDir: tmpdir(), | ||
| } | ||
| } |
There was a problem hiding this comment.
systemContext := types.SystemContext{}
if options.SystemContext != nil {
systemContext = *options.SystemContext
}
if systemContext.BigFilesTemporaryDir == "" {
systemContext.BigFilesTemporaryDir = tmpdir()
}might be a bit easier to follow and less repetitive.
(Personally I’d prefer a more explicit than “handle correctly” — to make it clear what was broken before and how it is fixed now. Are the two changes fixing two different bugs? But that’s depending on standards of this repo.)
There was a problem hiding this comment.
Fixed. and Updated commit message.
Currently if the caller does not specify the BigFilesTemporaryDir, Podman and Buildah users expect this to default TMPDIR environment variable or /var/tmp if not set. Moving to libimage caused a regression in this functionality. Signed-off-by: Daniel J Walsh <[email protected]>
rhatdan
changed the title
rhatdan
changed the title
|
/lgtm |
|
@rhatdan, can you backport to 0.38? I am under time pressure today to create a presentation for Moday. |
Currently if the caller does not specify the BigFilesTemporaryDir,
Podman and Buildah users expect this to default TMPDIR environment
variable or /var/tmp if not set.
Moving to libimage caused a regression in this functionality.
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1972282
Signed-off-by: Daniel J Walsh [email protected]