Switch to golang native error wrapping

[saschagrunert]

github.com/pkg/errors is deprecated since quite some time so we now use the native error wrapping for more idiomatic golang.

@containers/image-maintainers PTAL

[mtrmac]

Thanks!

Ref. containers/podman#14784 for an earlier discussion.

Notably this is hard-blocked at least,. on containers/common#1077 due to c/common/pkg/retry .

[mtrmac]

(Note to self: I didn’t actually read the patch.)

[flouthoc]

Just a friendly ping. I think this will be needed for final vendor and to make everything work.

[flouthoc]

LGTM

[mtrmac]

I think this will be needed for final vendor and to make everything work.

Um, is there anything that is currently broken and actually needs this PR to fix?

[flouthoc]

@mtrmac As of now don't think i have hit anything but today i hit a scenario where error generated from c/storage was not being understood by c/common as it should have been. So just a heads up here to prevent debugging time but I don't have any error for which this PR is needed :)

[mtrmac]

@flouthoc That… is vague enough to make me specifically nervous. Is the cause understood, and is it known whether it applies to the situation before applying this PR / after applying this PR / neither?

[flouthoc]

@mtrmac Sorry for confusion please ignore above comments since root cause of my issue was only on dependency between c/common/libimage and c/storage and I have notified here only for heads up just in-case similar scenarios exists in c/image dependency as well somewhere.

Is the cause understood

Yes cause of my issue i have shared here: containers/storage#1285 (comment) but just sharing it again in detail here libimage had a check like errors.Cause(err) != storage.ErrImageUnknown which only works if error is generated via errors.Wrapf(ErrImageUnknown, "error locating image with ID %q", id) so as soon as i updated my c/storage source of error was changed to fmt.Errorf("error locating image with ID %q: %w", id, ErrImageUnknown) causing all sort of failures in buildah vendor.I have only verified few failing test cases and all of them worked for me if we use latest c/common with latest c/storage. (PR: containers/buildah#4104)

is vague enough to make me specifically nervous

Again no c/image issue was seen while my testing so everything is chill but can confirm better once this is all green: containers/buildah#4104

[mtrmac]

@flouthoc Thanks very much for clarifying this. That’s the migration ordering discussed in containers/podman#14784 , and here about c/common needing to be updated last. The c/common change has now been merged, so we should be fine.

[mtrmac]

Thanks! Looks good overall.

[mtrmac]

The c/common change has now been merged, so we should be fine.

I’m sorry, I’m not thinking clearly today. That’s true for the c/image vs. {c/common, Buildah, Podman} interactions, but not for c/storage vs. c/image, where c/storage being migrated first might have broken something similarly — but we are fine because #1588 has updated the consumers in c/image already.

---

Given that, I’d weakly prefer for outstanding #1381 to be merged first — because this PR is the easier rebase, and because merging the feature one first would ensure that, at least in the short term, we don’t reintroduce another user. But that’s a weak preference and certainly not worth blocking this PR for a long time, if #1381 did not end up merged quickly.

[mtrmac]

LGTM. Thanks again!