-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Fix building from http or '-' options #7121
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 |
An alternative to #7113 |
fixes #7107 |
test/system/070-build.bats
Outdated
fi | ||
|
||
PODMAN_TIMEOUT=240 run_podman build -t build_test - << EOF | ||
FROM alpine |
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.
Could you make this $IMAGE (instead of alpine), to avoid hitting a registry?
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.
If that works, sure.
RUN /bin/echo 'Test' | ||
EOF | ||
is "$output" ".*STEP 3: COMMIT" "COMMIT seen in log" | ||
|
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.
Why not add a podman run
test? Verify that is "$output" "Test" "blah blah"
? Costs nothing, and could (maybe??) catch some weird future error.
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.
Added
When copying from a URL, podman will download and create a context directory in a temporary file. The problem was that this directory was being removed as soon as the function that created it was returned. Later the build code would look for content in the temporary directory and fail to find it, blowing up the build. By pulling the extraction code back into the build function, we keep the temporary directory around until the build completes. Signed-off-by: Daniel J Walsh <[email protected]>
Looks like a panic in one of the tests, I don't know if it's a known issue or not:
|
It's a known flake, #6518. It is really hammering us hard lately. I've restarted the bindings test. |
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.
/lgtm
This patch also fixes https://bugzilla.redhat.com/show_bug.cgi?id=1858862 |
When copying from a URL, podman will download and create a context
directory in a temporary file. The problem was that this directory
was being removed as soon as the function that created it was returned.
Later the build code would look for content in the temporary directory
and fail to find it, blowing up the build.
By pulling the extraction code back into the build function, we keep the
temporary directory around until the build completes.
Signed-off-by: Daniel J Walsh [email protected]