Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

composepost: apply add-determinism pyc-zero-mtime #5019

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

composepost: apply add-determinism pyc-zero-mtime #5019

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5c6b407
composepost: apply add-determinism pyc-zero-mtime
lukewarmtemp Jul 17, 2024
fff1ddb
wip: do not merge
lukewarmtemp Aug 8, 2024
5bc2034
WIP: do not merge
lukewarmtemp Aug 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion rust/src/composepost.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1201,7 +1201,18 @@ fn workaround_selinux_cross_labeling_recurse(
/// This is the nearly the last code executed before we run `ostree commit`.
pub fn compose_postprocess_final(rootfs_dfd: i32, _treefile: &Treefile) -> CxxResult<()> {
let rootfs = unsafe { &crate::ffiutil::ffi_dirfd(rootfs_dfd)? };

if std::process::Command::new("add-determinism").status().expect("Failed to find add-determinism on system.").success() {
Copy link
Member

Choose a reason for hiding this comment

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

This should probably be its own function like run_add_determinisim(rootfs);

Copy link
Member

Choose a reason for hiding this comment

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

Also this is not a great way to check for the existence of a binary. While the Rust stdlib doesn't have a single function for this I think it's trivial to do by combining

// add-determinism --handler pyc-zero-mtime
let r = std::process::Command::new("add-determinism")
.arg("--handler")
.arg("pyc-zero-mtime")
.arg("/usr")
Copy link
Member

Choose a reason for hiding this comment

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

This won't do what you expect or want. In an rpm-ostree build, the /usr is whatever happens to be the build environment, not the target.

In rpm-ostree we try hard to use fd-relative accesses generally via cap-std.

To do so with an external process, a typical pattern is to use e.g. cwd_dir and then pass . to the child process as the target.

.status()
.expect("Failed to normalize .pyc files using add-determinism");
if !r.success() {
return Err(anyhow!("Failed to execute add-determinism --handler pyc-zero-mtime: {:?}", r).into());
}
}
hardlink_rpmdb_base_location(rootfs, None)?;
Ok(())
}
Expand Down
Loading