search

containers / podman

Podman: A tool for managing OCI containers and pods.
https://podman.io
Apache License 2.0
22.43k stars 2.31k forks source link

refactor(build): improve err when file specified by -f does not exist #22972

Closed BlackHole1 closed 5 days ago

BlackHole1 commented 3 weeks ago

When the user specifies a Containerfile or Dockfile with the -f flag in podman build, if the file does not exist, the error should be intuitive to the user.

Fixed: #22940

Does this PR introduce a user-facing change?

Improve error message when file specified by -f does not exist in build command
BlackHole1 commented 3 weeks ago

@mheon Modification completed, request re-review

mheon commented 3 weeks ago

LGTM

On Fri, Jun 14, 2024 at 05:43 Kevin Cui @.***> wrote:

@mheon https://github.com/mheon Modification completed, request re-review

— Reply to this email directly, view it on GitHub https://github.com/containers/podman/pull/22972#issuecomment-2167154080, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB3AOCBET23OQYW2P2VDGOLZHJRGBAVCNFSM6AAAAABJFQKSL2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNRXGE2TIMBYGA . You are receiving this because you were mentioned.Message ID: @.***>

baude commented 3 weeks ago

code lgtm, please consider a release note.

baude commented 2 weeks ago

please rebase

baude commented 2 weeks ago

do we have a test that uses -f for a non-existent file ?

BlackHole1 commented 2 weeks ago

please rebase

Done

do we have a test that uses -f for a non-existent file ?

I checked, and currently, there isn't one. Also, because this logic is within a large function, writing a unit test is quite difficult. Maybe we could add an e2e test?

rhatdan commented 2 weeks ago

Looks like some tests need to be fixed.

BlackHole1 commented 2 weeks ago

@mheon @baude @rhatdan I added e2e tests and updated the code (which is now a bit more complex than before), please review the PR again.

mheon commented 2 weeks ago

LGTM

mheon commented 2 weeks ago

/approve

baude commented 2 weeks ago

code is fine, i did add a nit comment wondering if we can clean up readability ... will let @Luap99 break the tie.

packit-as-a-service[bot] commented 1 week ago

Ephemeral COPR build failed. @containers/packit-build please check.

BlackHole1 commented 1 week ago

@Luap99 @baude Done

BlackHole1 commented 5 days ago

Friendly ping :)

openshift-ci[bot] commented 5 days ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: BlackHole1, Luap99, mheon

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: - ~~[OWNERS](https://github.com/containers/podman/blob/main/OWNERS)~~ [Luap99,mheon] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment