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.

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

Support for building Android platforms #90

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Changes from 1 commit
Commits
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
138 changes: 101 additions & 37 deletions build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use std::fs::read_dir;
use std::path;
use std::path::Path;
use std::process;
use std::process::ExitStatus;

use nix::fcntl;

Expand Down Expand Up @@ -151,10 +152,10 @@ fn main() {
pkg_check("flex");
pkg_check("bison");
pkg_check("gawk");
pkg_check("aclocal");
Copy link
Collaborator

Choose a reason for hiding this comment

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

What needs aclocal?

}

let (compiler, mut cflags) = if vendored_libbpf || vendored_libelf || vendored_zlib {
pkg_check("make");
pkg_check("pkg-config");

let compiler = cc::Build::new().try_get_compiler().expect(
Expand Down Expand Up @@ -213,45 +214,73 @@ fn main() {
}
}

fn make_zlib(compiler: &cc::Tool, src_dir: &path::Path, out_dir: &path::Path) {
let src_dir = src_dir.join("zlib");
fn make_zlib(compiler: &cc::Tool, src_dir: &path::Path, _: &path::Path) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

If you don't use a previously present argument, can you remove it?

// lock README such that if two crates are trying to compile
// this at the same time (eg libbpf-rs libbpf-cargo)
// they wont trample each other
let file = std::fs::File::open(src_dir.join("README")).unwrap();
let _lock = fcntl::Flock::lock(file, fcntl::FlockArg::LockExclusive).unwrap();

let status = process::Command::new("./configure")
.arg("--static")
.arg("--prefix")
.arg(".")
.arg("--libdir")
.arg(out_dir)
.env("CC", compiler.path())
.env("CFLAGS", compiler.cflags_env())
.current_dir(&src_dir)
.status()
.expect("could not execute make");

assert!(status.success(), "make failed");
let project_dir = src_dir.join("zlib");

let status = process::Command::new("make")
.arg("install")
.arg("-j")
.arg(&format!("{}", num_cpus()))
.current_dir(&src_dir)
.status()
.expect("could not execute make");
let file = std::fs::File::open(project_dir.join("README")).unwrap();
let _lock = fcntl::Flock::lock(file, fcntl::FlockArg::LockExclusive).unwrap();

assert!(status.success(), "make failed");
let project_dir = project_dir.to_str().unwrap();

let zlib_sources = [
"adler32.c",
"compress.c",
"crc32.c",
"deflate.c",
"gzclose.c",
"gzlib.c",
"gzread.c",
"gzwrite.c",
"infback.c",
"inffast.c",
"inflate.c",
"inftrees.c",
"trees.c",
"uncompr.c",
"zutil.c",
];

// These flags are only used in Android
// ref: https://android.googlesource.com/platform/external/zlib/+/refs/tags/android-11.0.0_r48/Android.bp
let android_cflags = [
// We do support hidden visibility, so turn that on.
"-DHAVE_HIDDEN",
// We do support const, so turn that on.
"-DZLIB_CONST",
// Enable -O3 as per chromium.
"-O3",
// "-Wall",
// "-Werror",
// "-Wno-deprecated-non-prototype",
// "-Wno-unused",
// "-Wno-unused-parameter",
Comment on lines +248 to +258
Copy link
Collaborator

Choose a reason for hiding this comment

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

Where are these flags coming from, all of a sudden? And what does chromium have to do with this?

Copy link
Collaborator

Choose a reason for hiding this comment

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

In general, I kind of fail to understand what we gain from building our own build logic. The general guideline I am aware of is that the original build system should be used. If it provided a value, perhaps, but what value is that? Is the existing zlib build system incapable, somehow? Perhaps if you finally wrote a proper commit description I wouldn't have to inquire about every single detail and it would even be available for future developers...

Copy link
Contributor

Choose a reason for hiding this comment

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

FYI Android keeps forks of elfutils(https://android.googlesource.com/platform/external/elfutils) and zlib(https://android.googlesource.com/platform/external/zlib/+/refs/heads/main) that uses the soong blueprint + ninja build system.

Clearly some flags are copied from there: https://android.googlesource.com/platform/external/zlib/+/refs/heads/main/Android.bp#20 . (That Android.bp file doesn't have a license header.)

Some of the flags used in the soong blueprint are known to be supported by the compiler prebuilt that the AOSP project distributes, but might not be supported by all compilers. I don't think it is appropriate to set those flags for non Android targets.

Copy link
Author

Choose a reason for hiding this comment

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

FYI Android keeps forks of elfutils(https://android.googlesource.com/platform/external/elfutils) and zlib(https://android.googlesource.com/platform/external/zlib/+/refs/heads/main) that uses the soong blueprint + ninja build system.

Clearly some flags are copied from there: https://android.googlesource.com/platform/external/zlib/+/refs/heads/main/Android.bp#20 . (That Android.bp file doesn't have a license header.)

Some of the flags used in the soong blueprint are known to be supported by the compiler prebuilt that the AOSP project distributes, but might not be supported by all compilers. I don't think it is appropriate to set those flags for non Android targets.

Yes, these flags are only used in Android

Copy link
Collaborator

@danielocfb danielocfb Aug 26, 2024

Choose a reason for hiding this comment

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

The larger question seems unanswered.

In general, I kind of fail to understand what we gain from building our own build logic. The general guideline I am aware of is that the original build system should be used. If it provided a value, perhaps, but what value is that? Is the existing zlib build system incapable, somehow? Perhaps if you finally wrote a proper commit description I wouldn't have to inquire about every single detail and it would even be available for future developers...

Putting that aside, please remove unused flags as well as references to Chromium. Perhaps these make some kind of sense in an Android build system context, but they have no business here.

];

configure(project_dir, vec![]);

let mut builder = cc::Build::new();

builder.include(project_dir).files({
zlib_sources
.iter()
.map(|source| format!("{project_dir}/{source}"))
});

if build_android() {
for flag in android_cflags {
builder.flag(flag);
}
} else {
for flag in compiler.args() {
builder.flag(flag);
}
}

let status = process::Command::new("make")
.arg("distclean")
.current_dir(&src_dir)
.status()
.expect("could not execute make");
builder.flag_if_supported("-w").warnings(false).compile("z");

assert!(status.success(), "make failed");
emit_rerun_directives_for_contents(&src_dir);
}

Expand Down Expand Up @@ -402,9 +431,44 @@ fn num_cpus() -> usize {
std::thread::available_parallelism().map_or(1, |count| count.get())
}


fn build_android() -> bool {
env::var("CARGO_CFG_TARGET_OS")
.expect("CARGO_CFG_TARGET_OS not set")
.eq("android")
}
.expect("CARGO_CFG_TARGET_OS not set")
.eq("android")
}

fn configure<'a, P, A>(project_dir: P, args: A)
where
P: AsRef<str>,
A: IntoIterator<Item = &'a str>,
{
let project_dir = project_dir.as_ref();

let prog = format!("{project_dir}/configure");

unsafe {
let prog = std::ffi::CString::new(prog.as_bytes()).unwrap();
nix::libc::chmod(prog.as_ptr(), 0o755);
}

let status = subproc(prog, project_dir, args);

assert!(
status.success(),
"configure({}) failed: {}",
project_dir,
status
);
}

fn subproc<'a, P, A>(prog: P, workdir: &str, args: A) -> ExitStatus
where
P: AsRef<str>,
A: IntoIterator<Item = &'a str>,
{
process::Command::new(prog.as_ref())
.current_dir(workdir)
.args(args)
.status()
.expect(&format!("could not execute `{}`", prog.as_ref()))
}