composepost: apply add-determinism pyc-zero-mtime #5019
Conversation
Fixes: ostreedev/ostree#1469 Assuming that add-determinism is installed on the system, apply add-determinism to set the embedded mtime in all .pyc files to zero. Signed-off-by: Luke Yang <[email protected]> Co-authored-by: Steven Presti <[email protected]>
|
Thanks for this! add-determinism is generally a build tool, and I think it'd be nicer to support a model where it's not shipped onto the end system. However, right now we don't quite expose such a "multi stage" mechanism via treefile-based builds. If we did it'd be useful for a whole lot of other things too (for example, in theory one could drop |
|
From what we understand, the main concern is that As an alternative to having |
Yes, this is a notable point.
It's an option; the advantage is it keeps |
rust/src/composepost.rs
Outdated
| @@ -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() { | |||
There was a problem hiding this comment.
This should probably be its own function like run_add_determinisim(rootfs);
There was a problem hiding this comment.
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
let path = std::env::var_os("PATH")- https://doc.rust-lang.org/std/env/fn.split_paths.html
rust/src/composepost.rs
Outdated
| let r = std::process::Command::new("add-determinism") | ||
| .arg("--handler") | ||
| .arg("pyc-zero-mtime") | ||
| .arg("/usr") |
There was a problem hiding this comment.
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.
Yeah, so it really sounds like it needs to be on the image.
Right. Currently, the binary is 2.3MB. I think this is totally fine for build environments, but it's a bit heavy-weight for being installed on final images. Using it as a crate is one option. Another option would be to provide a stripped-down separate binary build… |
Mobbing WIP. Contains pseudo code.
rpm-ostree already drags in a lot of stuff transitively, most notably the libdnf stack, etc. The use cases we have today tend to have at least 400+MB images, commonly GB or more when you add desktops in the mix, libvirt, etc. Was thinking about this more and I have no problem just making rpm-ostree soft depend on a separate |
Fixes: ostreedev/ostree#1469
Assuming that add-determinism is installed on the system, apply add-determinism to set the embedded mtime in all .pyc files to zero.