libimage: Add input to error context when normalizing

[cgwalters]

Before, here's what happens if you forget a -v in your bind mount for example:

$ podman run /dev:/dev docker.io/busybox echo hello
Error: invalid reference format
$

After:

$ podman run /dev:/dev docker.io/busybox echo hello
Error: parsing reference "/dev:/dev": invalid reference format

This error prefixing is common in other callers.

None

[rhatdan]

/approve
LGTM

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cgwalters, 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:

• OWNERS [rhatdan]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[Luap99]

This makes a lot of sense, however there are more reference.Parse() callers here that do not wrap the error so I think they should be fixed too.

[Luap99]

I also also wonder if this breaks podman tests that hard code certain error messages. Not a blocker of course just something that may needs chnages when vendoring the next time into podman

[cgwalters]

This makes a lot of sense, however there are more reference.Parse() callers here that do not wrap the error so I think they should be fixed too.

I looked at that, but my worry is we end up with double context somewhere; it's hard to trace through all these call chains (hmm, I wonder if someone wrote a tool for this).

I decided to change the implementation so that just the two callers here of normalizeTaggedDigestedString add context, which feels like it more closely matches what happens elsewhere.

I also also wonder if this breaks podman tests that hard code certain error messages. Not a blocker of course just something that may needs chnages when vendoring the next time into podman

Yeah, that seems possible.

[vrothberg]

An alternative could be adding the context directly to the reference package in c/image?
@mtrmac WDYT?

[Luap99]

An alternative could be adding the context directly to the reference package in c/image? @mtrmac WDYT?

I was thinking about this too but then chances of including the path twice is higher as there seem to be many places already that wrap the error accordingly that would need to be changed back.

[mtrmac]

Yes, this is “pay a little cost every time a new ParseNormalizedNamed caller is added” vs. “invest more now to solve it for good”.

(And pedantically we might worry about starting to return a wrapped error , but nothing in Podman actually cares.)

It’s ~50 non-test calls across Podman, so quite doable, but also not something I can prioritize working on short-term. Perhaps a good first PR for a new contributor.

[rhatdan]

Lets get this fix in, and then we can look at moving it to containers/image later.

[Luap99]

Seems like a good idea to at least fix the bad podman run/create message for now.
/lgtm