Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
f760030 to
5cad37a
Compare
f2b6634 to
de08013
Compare
e80ce99 to
6368721
Compare
4 tasks
Some builders want to specify extra cargo or rustc flags when building, so making this be built-in is far more preferable. Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
When doing cross-compliation (for runc), the binaries are put into a different directory and we need to support using those alternative build directories as sources of installation. Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
Outputting .symver entries for staticlib builds leads to "multiple definition" errors when linking statically, so we need to only output .symver (and our version script) when building a cdylib. Unfortunately, Rust provides no built-in mechanism to do this nicely. There is no way to get the crate type using #[cfg(...)] and even an explicit --cfg on the command-line does not show up in build.rs. The only real "Rust-y" solution would be to create a separate feature for cdylib but that is really ugly. The solution I went with was passing an environment variable during the build (LIBPATHRS_CAPI_BUILDMODE) which acts as a proxy for --crate-type (in the hopes that this might be supported properly in the future). build.rs then configures the #[cfg(cdylib)] used to control the output of .symver and only outputs a version script. We even get to use cargo:rerun-if-env-changed to make sure that the build script is actually rebuilt for every crate type change. An unset LIBPATHRS_CAPI_BUILDMODE (such as during tests) is treated like staticlib. Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
There were two separate issues when building on i386: * We used C.ulong rather than C.size_t internally when calling into readlink-related libpathrs APIs, which are different widths on i386. The solution is to just use C.size_t, which we should've used in the first place. * There was a comparison between int and 1<<31, which would overflow on i386. The solution is to cast to uint64, though this happens to cause a spurious G115 lint error from gosec which we need to mask. Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
If we assign ProcfsHandleRef::open_base to a variable then Rust will only drop it once we exit the scope, which is fine in general but causes issues with runc's CI. runc's CI checks for fd leaks in "runc create" (which blocks on a re-open of a FIFO) and so having an additional temporary file descriptor open at that stage leads to regressions in runc's CI. We could add this file descriptor to the allow-list but it's much nicer to just always use it as a temporary (sadly there isn't a way to mark the return value in a way that would cause a clippy lint, as far as I can see). Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
It turns out our builds were not working well with what runc needs (in particular,
staticlibcontained duplicate symbols because of our version scripts).