build: use libpathrs by default#5103
Conversation
d8dd6b0 to
11671a9
Compare
.github/workflows/test.yml
Outdated
| run: cargo install --locked cargo-auditable | ||
| - name: install libpathrs | ||
| env: | ||
| LIBPATHRS_VERSION: "0.2.3" |
There was a problem hiding this comment.
This should be in the matrix?
There was a problem hiding this comment.
Will do, I'm just trying to validate that cyphar/libpathrs#327 is sufficient to make runc builds happy (both with make releaseall and in GHA). I also still need to set up signed release archives and switch script/build-libpathrs.sh back to pulling from a release as well.
2e26652 to
c21dc79
Compare
This comment was marked as resolved.
This comment was marked as resolved.
af83d54 to
d25c6d6
Compare
.github/workflows/test.yml
Outdated
| - uses: dtolnay/rust-toolchain@stable | ||
| - name: install libpathrs v${{ matrix.libpathrs-version }} | ||
| env: | ||
| LIBPATHRS_VERSION: (${{ matrix.libpathrs-version }}) | ||
| run: | | ||
| sudo -E PATH="$PATH" ./script/build-libpathrs.sh "$LIBPATHRS_VERSION" /usr |
There was a problem hiding this comment.
Perhaps you can set up OBS to build (and serve) libpathrs deb packages, similar to how it's currently done for criu? This will save a lot of hassle (in this repo, not for you I'm afraid). We can probably also reuse this from Dockerfile, too, eliminating the need for script/build-libpathrs.sh (the seccomp one is really needed as we have to include the sources, and there's no such requirement for libpathrs IMO).
There was a problem hiding this comment.
Yeah, I was thinking about doing that to be honest, but was hoping to be able to do this later rather than upfront (I don't mind maintaining unofficial packages for this stuff eventually but learning Rust packaging for 3-4 different distros as a prerequisite for this PR seems a little harsh 😅).
eliminating the need for script/build-libpathrs.sh (the seccomp one is really needed as we have to include the sources, and there's no such requirement for libpathrs IMO).
libpathrs is MPLv2 (or LGPLv3), so we do need to include a copy of the sources.
rata
left a comment
There was a problem hiding this comment.
This mostly LGTM. I like that it is basically contained on the CI/release files. Ping me when you want another review.
I left some comments. I think I'd nto try to figure out the "correct" location for a library on each distro, if something like /usr/local works on all. I'd let the right destdir to when we have an OBS package for each distro or so. But I feel usr/local probably doesn't work?
Left some other comments too, mostly nits.
d25c6d6 to
dbdd4c2
Compare
dbdd4c2 to
cf68c86
Compare
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
Debian 13 (trixie) was released a few months ago and it's probably prudent to just upgrade. This is also necessary to get access to riscv64 repositories when we build libpathrs for inclusion in our runc binaries. Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
The intention of commit 531e29e ("script/lib.sh: set GOARM=5 for armel, GOARM=6 for armhf") was to properly support older ARM platforms with our release builds. However, we have never been able to support ARMv6 for our builds because we use the Debian compiler to build the libseccomp we statically compile into our binaries and (as per the now-deleted comment itself) Debian treats armhf as being ARMv7 so the final binaries we produced were always only ever compatible with ARMv7+. This was a bit of an oddity before but when building libpathrs for releases we will need to use Rust which makes the target more explicit (and while it does support armhf, we are using the Debian-packaged Rust cross-compiler and thus are in the same dilemma with what Debian considers "armhf" to be). All-in-all, it's better to just bite the bullet and just follow Debian here properly. Fixes: 531e29e ("script/lib.sh: set GOARM=5 for armel, GOARM=6 for armhf") Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
This name is far more descriptive. Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
35783a2 to
95a8a72
Compare
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
95a8a72 to
c92bbc6
Compare
|
What the heck... 🤨 Hmmmm, this is probably caused by the annoying procfs behaviour with user namespaces and non-dumpability... ( I guess in the case where we would accept no |
c92bbc6 to
6a0a073
Compare
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
6a0a073 to
bd15ac1
Compare
Draft until these PRs / issues are resolved and libpathrs has a new release:
pathrs-litesupports transparently switching tolibpathrs.soas a backendwith the
libpathrsbuild tag. In order to permit us to eventually requirelibpathrs.so(and get rid of some of the duplicated wrappers ininternal/pathrs), we need to makelibpathrsopt-out for at least onerelease before making it mandatory.
Signed-off-by: Aleksa Sarai cyphar@cyphar.com