Skip to content
This repository has been archived by the owner on Nov 7, 2024. It is now read-only.

Use rustix instead of nix #201

Merged
merged 2 commits into from
Jan 6, 2022
Merged

Use rustix instead of nix #201

merged 2 commits into from
Jan 6, 2022

Conversation

cgwalters
Copy link
Member

In this actually, the owned/borrowed FD stuff is definitely nicer. I still need to investigate whether the rustix ecosystem has something like this fd passing-to-Command that we need.

@cgwalters
Copy link
Member Author

Depends #200 which is a clean prep patch that can be merged now.

@cgwalters cgwalters marked this pull request as ready for review January 6, 2022 15:24
@cgwalters
Copy link
Member Author

OK lifting draft on this one. This also relates to coreos/rpm-ostree#3311 in that we want to drop nix/openat from the ostree dependency chain ideally first.

@cgwalters
Copy link
Member Author

(We should use cap-std here too, but that's a larger churn)

Copy link
Member

@jmarrero jmarrero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@jmarrero
Copy link
Member

jmarrero commented Jan 6, 2022

Changes lgtm, I wonder why CI is failing on both PRs with the same issue, do we need to force these in?

@cgwalters
Copy link
Member Author

I clearly should have tested #203 locally - we need that first because we're pulling containers-image-proxy from git now, so CI started failing after containers/containers-image-proxy-rs#24 was merged.

(I did actually test that PR but with a different branch)

@jmarrero
Copy link
Member

jmarrero commented Jan 6, 2022

I guess we need a rebase now.

This is safer and may actually fix a race condition I've seen sometimes
in CI runs.  Part of investigating using rustix (and cap-std) in
our section of the ecosystem (xref rust-lang/rfcs#2610).
By using rustix for uname.
@cgwalters
Copy link
Member Author

Rebased 🏄

@jmarrero jmarrero merged commit ec01d70 into ostreedev:main Jan 6, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants