Skip to content

Conversation

@rhatdan
Copy link
Member

@rhatdan rhatdan commented Mar 11, 2019

These patches remove libpod from buildah and mv some pkg/rootless and pkg/chrootuser from libpod to buildah repo.

@giuseppe
Copy link
Member

it looks all we need in Buildah from rootless is rootless.IsRootless() and rootless.GetRootlessUID().

Let's just duplicate these two functions instead of moving the package. It will be a lot more difficult to fix rootless stuff in Podman if the implementation lives in another repository.

@TomSweeneyRedHat
Copy link
Member

Are the chroot and rootless pkgs also in libpod still? If so could/should we make a new project in containers and vendor them from there? Maybe a jira card for later, it would be nice to untangle this sooner.
Otherwise, LGTM

@rhatdan rhatdan force-pushed the storage branch 2 times, most recently from b77c45d to 9816729 Compare March 11, 2019 17:36
@rhatdan rhatdan force-pushed the storage branch 3 times, most recently from 202bbc1 to bdb7728 Compare March 12, 2019 18:15
@rhatdan
Copy link
Member Author

rhatdan commented Mar 12, 2019

bot, retest this please

@giuseppe
Copy link
Member

the error seems related to files that are still under vendor/ but not present in vendor.conf

@rh-atomic-bot
Copy link
Collaborator

☔ The latest upstream changes (presumably 939de6f) made this pull request unmergeable. Please resolve the merge conflicts.

@rhatdan rhatdan force-pushed the storage branch 3 times, most recently from 4d40d8a to e34e041 Compare March 19, 2019 15:21
@rhatdan
Copy link
Member Author

rhatdan commented Mar 19, 2019

@giuseppe Yes I need to get this vendored into containers/image first.

@rh-atomic-bot
Copy link
Collaborator

☔ The latest upstream changes (presumably d1c75ea) made this pull request unmergeable. Please resolve the merge conflicts.

@rhatdan rhatdan changed the title [WIP] Stop vendoring libpod into buildah Stop vendoring libpod into buildah Mar 25, 2019
@rhatdan
Copy link
Member Author

rhatdan commented Mar 25, 2019

Completes: #1437

We don't want to vendor anything from libpod into Buildah.
We want to switch this around.  Moving pkg content from libpod
to Buildah allows us to fix this.

Signed-off-by: Daniel J Walsh <[email protected]>
@vrothberg
Copy link
Member

We need this PR merged to make others pass again (that vendor recent storage).
LGTM, @giuseppe PTAL.

@TomSweeneyRedHat
Copy link
Member

LGTM, but would like a head nod from @giuseppe to make sure his rootless concerns are addressed.

@vrothberg
Copy link
Member

On a general note.

We should not merge PRs over at containers/storage (or any other place) when breaking the API while the follow-up ones on our consumers are not yet ready. In this specific case we cannot update containers/storage in libpod (and soon in CRI-O) because they depend on buildah which is not yet updated to the API breakages in containers/storage. This does not only block work (in this case me) but also blocks any fixes we might need to perform that require quick vendoring.

I think it's more the exception than the rule but I was blocked fairly often by such cases in the past that I want to share my troubles. Especially API breakages should be treated with care and those responsible should do the vendoring into all the consumers.

@giuseppe
Copy link
Member

@rh-atomic-bot r+

@rh-atomic-bot
Copy link
Collaborator

📌 Commit 9fbc0f3 has been approved by giuseppe

@rh-atomic-bot
Copy link
Collaborator

⌛ Testing commit 9fbc0f3 with merge 3d74031...

@rh-atomic-bot
Copy link
Collaborator

☀️ Test successful - status-papr, status-travis
Approved by: giuseppe
Pushing 3d74031 to master...

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants