Bug 1495669: Share bindgen flags globally; r=emilio, r=froydnj
authorBenjamin Bouvier <benj@benj.me>
Tue, 09 Oct 2018 15:01:52 +0200
changeset 441237 1f5478a63db8a1f606ad2fd6ead34b5d91712357
parent 441236 ccf6f87d34302b01ff38c1934f70a906ab0e4184
child 441238 59345f79f963cb35478a1cab1f1aecfe76eaccb7
push id108943
push userbbouvier@mozilla.com
push dateMon, 15 Oct 2018 15:41:13 +0000
treeherdermozilla-inbound@1f5478a63db8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersemilio, froydnj
bugs1495669
milestone64.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1495669: Share bindgen flags globally; r=emilio, r=froydnj
build/moz.configure/android-ndk.configure
build/moz.configure/bindgen.configure
build/moz.configure/toolchain.configure
js/src/wasm/cranelift/build.rs
layout/style/ServoBindings.toml
servo/components/style/build_gecko.rs
--- a/build/moz.configure/android-ndk.configure
+++ b/build/moz.configure/android-ndk.configure
@@ -314,30 +314,30 @@ def android_toolchain_prefix(prefix_base
 
 imply_option('--with-toolchain-prefix', android_toolchain_prefix,
              reason='--with-android-ndk')
 
 
 @depends(extra_toolchain_flags, android_toolchain,
          android_toolchain_prefix_base, '--help')
 @imports(_from='os.path', _import='isdir')
-def bindgen_cflags_defaults(toolchain_flags, toolchain, toolchain_prefix, _):
+def bindgen_cflags_android(toolchain_flags, toolchain, toolchain_prefix, _):
     if not toolchain_flags:
         return
 
     gcc_include = os.path.join(
         toolchain, 'lib', 'gcc', toolchain_prefix, '4.9.x')
     if not isdir(gcc_include):
         gcc_include = os.path.join(
             toolchain, 'lib', 'gcc', toolchain_prefix, '4.9')
 
-    cflags_format = "%s -I%s -I%s"
-    return cflags_format % (' '.join(toolchain_flags),
-                            os.path.join(gcc_include, 'include'),
-                            os.path.join(gcc_include, 'include-fixed'))
+    return toolchain_flags + [
+        '-I%s' % os.path.join(gcc_include, 'include'),
+        '-I%s' % os.path.join(gcc_include, 'include-fixed'),
+    ]
 
 
 @depends(host, ndk)
 @imports(_from='os.path', _import='exists')
 @imports(_from='os.path', _import='isdir')
 def android_clang_compiler(host, ndk):
     if not ndk:
         return
--- a/build/moz.configure/bindgen.configure
+++ b/build/moz.configure/bindgen.configure
@@ -207,8 +207,136 @@ def bindgen_config_paths(llvm_config, li
 
     return namespace(
         libclang_path=libclang_path,
         clang_path=clang_resolved,
     )
 
 set_config('MOZ_LIBCLANG_PATH', bindgen_config_paths.libclang_path)
 set_config('MOZ_CLANG_PATH', bindgen_config_paths.clang_path)
+
+
+@depends(host, target, target_is_unix, c_compiler, bindgen_cflags_android)
+def basic_bindgen_cflags(host, target, is_unix, compiler_info, android_cflags):
+    args = [
+        '-x', 'c++', '-std=gnu++14', '-fno-sized-deallocation',
+        '-DTRACING=1', '-DIMPL_LIBXUL', '-DMOZILLA_INTERNAL_API',
+        '-DRUST_BINDGEN'
+    ]
+
+    if is_unix:
+        args += ['-DOS_POSIX=1']
+
+    if target.os == 'Android':
+        args += android_cflags
+
+    def handle_cpu(obj):
+        if 'cpu' in obj and target.cpu in obj['cpu']:
+            return obj['cpu'][target.cpu]
+        return []
+
+    if target.os == 'WINNT' and host.raw_os.startswith('gnu'):
+        args += handle_cpu({
+            'cpu': {
+                'x86': ['--target=i686-pc-mingw32'],
+                'x86_64': ['--target=x86_64-w64-mingw32'],
+            },
+        })
+
+    os_dict = {
+        'Android': {
+            'default': ['-DOS_ANDROID=1'],
+            'cpu': {
+                'aarch64': ['--target=aarch64-linux-android'],
+                'arm': ['--target=armv7-linux-androideabi'],
+                'x86': ['--target=i686-linux-android'],
+                'x86_64': ['--target=x86_64-linux-android'],
+            },
+        },
+        'DragonFly': {
+            'default': ['-DOS_BSD=1', '-DOS_DRAGONFLY=1'],
+        },
+        'FreeBSD': {
+            'default': ['-DOS_BSD=1', '-DOS_FREEBSD=1'],
+        },
+        'GNU': {
+            'default': ['-DOS_LINUX=1'],
+            'cpu': {
+                'x86': ['-m32'],
+                'x86_64': ['-m64'],
+            },
+        },
+        'NetBSD': {
+            'default': ['-DOS_BSD=1', '-DOS_NETBSD=1'],
+        },
+        'OpenBSD': {
+            'default': ['-DOS_BSD=1', '-DOS_OPENBSD=1'],
+        },
+        'OSX': {
+            'default': [
+                '-DOS_MACOSX=1',
+                '-stdlib=libc++',
+                # To disable the fixup bindgen applies which adds search
+                # paths from clang command line in order to avoid potential
+                # conflict with -stdlib=libc++.
+                '--target=x86_64-apple-darwin',
+            ],
+        },
+        'SunOS': {
+            'default': ['-DOS_SOLARIS=1'],
+        },
+        'WINNT': {
+            'default': [
+                '-DOS_WIN=1',
+                '-DWIN32=1',
+            ],
+            'compiler': {
+                'msvc': {
+                    'default': [
+                        # For compatibility with MSVC 2015
+                        '-fms-compatibility-version=19',
+                        # To enable the builtin __builtin_offsetof so that CRT wouldn't
+                        # use reinterpret_cast in offsetof() which is not allowed inside
+                        # static_assert().
+                        '-D_CRT_USE_BUILTIN_OFFSETOF',
+                        # Enable hidden attribute (which is not supported by MSVC and
+                        # thus not enabled by default with a MSVC-compatibile build)
+                        # to exclude hidden symbols from the generated file.
+                        '-DHAVE_VISIBILITY_HIDDEN_ATTRIBUTE=1',
+                    ],
+                    'cpu': {
+                        'x86': ['--target=i686-pc-win32'],
+                        'x86_64': ['--target=x86_64-pc-win32'],
+                        'aarch64': ['--target=aarch64-pc-windows-msvc'],
+                    },
+                },
+            },
+        },
+    }.get(target.os, {})
+
+    if 'default' in os_dict:
+        args += os_dict['default']
+
+    args += handle_cpu(os_dict)
+    if 'compiler' in os_dict and compiler_info and compiler_info in os_dict['compiler']:
+        compiler_dict = os_dict['compiler'].get(compiler_info)
+        if 'default' in compiler_dict:
+            args += compiler_dict['default']
+        args += handle_cpu(compiler_dict)
+
+    return args
+
+
+js_option(env='BINDGEN_CFLAGS',
+          nargs=1,
+          help='Options bindgen should pass to the C/C++ parser')
+
+
+@depends(basic_bindgen_cflags, 'BINDGEN_CFLAGS')
+@checking('bindgen cflags', lambda s: s if s else 'no')
+def bindgen_cflags(base_flags, extra_flags):
+    flags = base_flags
+    if extra_flags and len(extra_flags):
+        flags += extra_flags[0].split()
+    return ' '.join(flags)
+
+
+add_old_configure_assignment('_BINDGEN_CFLAGS', bindgen_cflags)
--- a/build/moz.configure/toolchain.configure
+++ b/build/moz.configure/toolchain.configure
@@ -1121,32 +1121,16 @@ include('compile-checks.configure')
          try_compile(body='static_assert(sizeof(void *) == 8, "")',
                      check_msg='for 64-bit OS'))
 def check_have_64_bit(have_64_bit, compiler_have_64_bit):
     if have_64_bit != compiler_have_64_bit:
         configure_error('The target compiler does not agree with configure '
                         'about the target bitness.')
 
 
-js_option(env='BINDGEN_CFLAGS',
-          nargs=1,
-          default=bindgen_cflags_defaults,
-          help='Options bindgen should pass to the C/C++ parser')
-
-
-@depends('BINDGEN_CFLAGS')
-@checking('bindgen cflags', lambda s: s if s else 'no')
-def bindgen_cflags(value):
-    if value and len(value):
-        return value[0].split()
-
-
-add_old_configure_assignment('_BINDGEN_CFLAGS', bindgen_cflags)
-
-
 @depends(c_compiler)
 def default_debug_flags(compiler_info):
     # Debug info is ON by default.
     if compiler_info.type in ('msvc', 'clang-cl'):
         return '-Zi'
     return '-g'
 
 
--- a/js/src/wasm/cranelift/build.rs
+++ b/js/src/wasm/cranelift/build.rs
@@ -21,23 +21,16 @@
 
 extern crate bindgen;
 
 use std::env;
 use std::fs::File;
 use std::io::prelude::*;
 use std::path::PathBuf;
 
-enum Arch {
-    X86,
-    X64,
-    Arm,
-    Aarch64
-}
-
 fn main() {
     // Tell Cargo to regenerate the bindings if the header file changes.
     println!("cargo:rerun-if-changed=baldrapi.h");
 
     let mut bindings = bindgen::builder()
         .disable_name_namespacing()
         // We whitelist the Baldr C functions and get the associated types for free.
         .whitelist_function("env_.*")
@@ -47,79 +40,16 @@ fn main() {
         .whitelist_type("Cranelift.*")
         // The enum classes defined in baldrapi.h and WasmBinaryConstants are all Rust-safe.
         .rustified_enum("BD_.*|Trap|TypeCode|FuncTypeIdDescKind")
         .whitelist_type("BD_.*|Trap|TypeCode|FuncTypeIdDescKind")
         .header("baldrapi.h")
         .clang_args(&["-x", "c++", "-std=gnu++14", "-fno-sized-deallocation", "-DRUST_BINDGEN"])
         .clang_arg("-I../..");
 
-    let arch = {
-        let target_arch = env::var("CARGO_CFG_TARGET_ARCH");
-        match target_arch.as_ref().map(|x| x.as_str()) {
-            Ok("aarch64") => Some(Arch::Aarch64),
-            Ok("arm") => Some(Arch::Arm),
-            Ok("x86") => Some(Arch::X86),
-            Ok("x86_64") => Some(Arch::X64),
-            _ => None
-        }
-    };
-
-    match env::var("CARGO_CFG_TARGET_OS").as_ref().map(|x| x.as_str()) {
-        Ok("android") => {
-            bindings = bindings.clang_arg("-DOS_ANDROID=1");
-            bindings = match arch.expect("unknown android architecture") {
-                Arch::Aarch64 => { bindings.clang_arg("--target=aarch64-linux-android") }
-                Arch::Arm => { bindings.clang_arg("--target=armv7-linux-androideabi") }
-                Arch::X86 => { bindings.clang_arg("--target=i686-linux-android") }
-                Arch::X64 => { bindings.clang_arg("--target=x86_64-linux-android") }
-            };
-        }
-
-        Ok("linux") | Ok("freebsd") | Ok("dragonfly") | Ok("openbsd") | Ok("bitrig") | Ok("netbsd")
-            | Ok("ios") => {
-            // Nothing to do in particular for these OSes, until proven the contrary.
-        }
-
-        Ok("macos") => {
-            bindings = bindings.clang_arg("-DOS_MACOSX=1");
-            bindings = bindings.clang_arg("-stdlib=libc++");
-            bindings = bindings.clang_arg("--target=x86_64-apple-darwin");
-        }
-
-        Ok("windows") => {
-            let arch = arch.expect("unknown Windows architecture");
-            bindings = bindings.clang_arg("-DOS_WIN=1")
-                .clang_arg("-DWIN32=1");
-            bindings = match env::var("CARGO_CFG_TARGET_ENV").as_ref().map(|x| x.as_str()) {
-                Ok("msvc") => {
-                    bindings = bindings.clang_arg("-fms-compatibility-version=19");
-                    bindings = bindings.clang_arg("-D_CRT_USE_BUILTIN_OFFSETOF");
-                    bindings = bindings.clang_arg("-DHAVE_VISIBILITY_HIDDEN_ATTRIBUTE=1");
-                    match arch {
-                        Arch::X86 => { bindings.clang_arg("--target=i686-pc-win32") },
-                        Arch::X64 => { bindings.clang_arg("--target=x86_64-pc-win32") },
-                        Arch::Aarch64 => { bindings.clang_arg("--target=aarch64-pc-windows-msvc") }
-                        _ => panic!("unknown Windows architecture for msvc build")
-                    }
-                }
-                Ok("gnu") => {
-                    match arch {
-                        Arch::X86 => { bindings.clang_arg("--target=i686-pc-mingw32") },
-                        Arch::X64 => { bindings.clang_arg("--target=x86_64-w64-mingw32") },
-                        _ => panic!("unknown Windows architecture for gnu build")
-                    }
-                }
-                _ => panic!("unknown Windows build environment")
-            };
-        }
-
-        os => panic!("unknown target os {:?}!", os)
-    }
-
     let path = PathBuf::from(env::var_os("MOZ_TOPOBJDIR").unwrap()).join("js/src/rust/extra-bindgen-flags");
 
     let mut extra_flags = String::new();
         File::open(&path)
             .expect("Failed to open extra-bindgen-flags file")
             .read_to_string(&mut extra_flags)
             .expect("Failed to read extra-bindgen-flags file");
 
--- a/layout/style/ServoBindings.toml
+++ b/layout/style/ServoBindings.toml
@@ -1,66 +1,8 @@
-[build]
-args = [
-    "-x", "c++", "-std=gnu++14", "-fno-sized-deallocation",
-    "-DTRACING=1", "-DIMPL_LIBXUL", "-DMOZ_STYLO_BINDINGS=1",
-    "-DMOZILLA_INTERNAL_API", "-DRUST_BINDGEN"
-]
-"family=unix" = ["-DOS_POSIX=1"]
-"os=solaris" = ["-DOS_SOLARIS=1"]
-"os=dragonfly" = ["-DOS_BSD=1", "-DOS_DRAGONFLY=1"]
-"os=freebsd" = ["-DOS_BSD=1", "-DOS_FREEBSD=1"]
-"os=netbsd" = ["-DOS_BSD=1", "-DOS_NETBSD=1"]
-"os=openbsd" = ["-DOS_BSD=1", "-DOS_OPENBSD=1"]
-"os=macos" = [
-    "-DOS_MACOSX=1", "-stdlib=libc++",
-    # To disable the fixup bindgen applies which adds search
-    # paths from clang command line in order to avoid potential
-    # conflict with -stdlib=libc++.
-    "--target=x86_64-apple-darwin",
-]
-
-[build."os=linux"]
-args = ["-DOS_LINUX=1"]
-"arch=x86" = ["-m32"]
-"arch=x86_64" = ["-m64"]
-
-[build."os=android"]
-args = ["-DOS_ANDROID=1"]
-"arch=aarch64" = ["--target=aarch64-linux-android"]
-"arch=arm" = ["--target=armv7-linux-androideabi"]
-"arch=x86" = ["--target=i686-linux-android"]
-"arch=x86_64" = ["--target=x86_64-linux-android"]
-
-[build."os=windows"]
-args = [
-    "-DOS_WIN=1", "-DWIN32=1",
-]
-
-[build."os=windows"."env=msvc"]
-args = [
-    # For compatibility with MSVC 2015
-    "-fms-compatibility-version=19",
-    # To enable the builtin __builtin_offsetof so that CRT wouldn't
-    # use reinterpret_cast in offsetof() which is not allowed inside
-    # static_assert().
-    "-D_CRT_USE_BUILTIN_OFFSETOF",
-    # Enable hidden attribute (which is not supported by MSVC and
-    # thus not enabled by default with a MSVC-compatibile build)
-    # to exclude hidden symbols from the generated file.
-    "-DHAVE_VISIBILITY_HIDDEN_ATTRIBUTE=1",
-]
-"arch=x86" = ["--target=i686-pc-win32"]
-"arch=x86_64" = ["--target=x86_64-pc-win32"]
-"arch=aarch64" = ["--target=aarch64-pc-windows-msvc"]
-
-[build."os=windows"."env=gnu"]
-"arch=x86" = ["--target=i686-pc-mingw32"]
-"arch=x86_64" = ["--target=x86_64-w64-mingw32"]
-
 [structs]
 headers = [
     "nsStyleStruct.h",
     "mozilla/StyleAnimationValue.h",
     "gfxFontConstants.h",
     "gfxFontFeatures.h",
     "nsStyleConsts.h",
     "mozilla/css/Loader.h",
--- a/servo/components/style/build_gecko.rs
+++ b/servo/components/style/build_gecko.rs
@@ -53,32 +53,20 @@ mod bindings {
         static ref CONFIG: Table = {
             // Load Gecko's binding generator config from the source tree.
             let path = PathBuf::from(env::var_os("MOZ_SRC").unwrap())
                 .join("layout/style/ServoBindings.toml");
             read_config(&path)
         };
         static ref BUILD_CONFIG: Table = {
             // Load build-specific config overrides.
-            // FIXME: We should merge with CONFIG above instead of
-            // forcing callers to do it.
             let path = PathBuf::from(env::var_os("MOZ_TOPOBJDIR").unwrap())
                 .join("layout/style/bindgen.toml");
             read_config(&path)
         };
-        static ref TARGET_INFO: HashMap<String, String> = {
-            const TARGET_PREFIX: &'static str = "CARGO_CFG_TARGET_";
-            let mut result = HashMap::new();
-            for (k, v) in env::vars() {
-                if k.starts_with(TARGET_PREFIX) {
-                    result.insert(k[TARGET_PREFIX.len()..].to_lowercase(), v);
-                }
-            }
-            result
-        };
         static ref INCLUDE_RE: Regex = Regex::new(r#"#include\s*"(.+?)""#).unwrap();
         static ref DISTDIR_PATH: PathBuf = {
             let path = PathBuf::from(env::var_os("MOZ_DIST").unwrap());
             if !path.is_absolute() || !path.is_dir() {
                 panic!("MOZ_DIST must be an absolute directory, was: {}", path.display());
             }
             path
         };
@@ -140,45 +128,16 @@ mod bindings {
     trait BuilderExt {
         fn get_initial_builder() -> Builder;
         fn include<T: Into<String>>(self, file: T) -> Builder;
         fn zero_size_type(self, ty: &str, structs_list: &HashSet<&str>) -> Builder;
         fn borrowed_type(self, ty: &str) -> Builder;
         fn mutable_borrowed_type(self, ty: &str) -> Builder;
     }
 
-    fn add_clang_args(mut builder: Builder, config: &Table, matched_os: &mut bool) -> Builder {
-        fn add_args(mut builder: Builder, values: &[toml::Value]) -> Builder {
-            for item in values.iter() {
-                builder = builder.clang_arg(item.as_str().expect("Expect string in list"));
-            }
-            builder
-        }
-        for (k, v) in config.iter() {
-            if k == "args" {
-                builder = add_args(builder, v.as_array().unwrap().as_slice());
-                continue;
-            }
-            let equal_idx = k.find('=').expect(&format!("Invalid key: {}", k));
-            let (target_type, target_value) = k.split_at(equal_idx);
-            if TARGET_INFO[target_type] != target_value[1..] {
-                continue;
-            }
-            if target_type == "os" {
-                *matched_os = true;
-            }
-            builder = match *v {
-                toml::Value::Table(ref table) => add_clang_args(builder, table, matched_os),
-                toml::Value::Array(ref array) => add_args(builder, array),
-                _ => panic!("Unknown type"),
-            };
-        }
-        builder
-    }
-
     impl BuilderExt for Builder {
         fn get_initial_builder() -> Builder {
             use bindgen::RustTarget;
 
             // Disable rust unions, because we replace some types inside of
             // them.
             let mut builder = Builder::default().rust_target(RustTarget::Stable_1_0);
 
@@ -202,26 +161,24 @@ mod bindings {
             }
 
             builder = builder.include(add_include("mozilla-config.h"));
 
             if env::var("CARGO_FEATURE_GECKO_DEBUG").is_ok() {
                 builder = builder.clang_arg("-DDEBUG=1").clang_arg("-DJS_DEBUG=1");
             }
 
-            let mut matched_os = false;
-            let build_config = CONFIG["build"].as_table().expect("Malformed config file");
-            builder = add_clang_args(builder, build_config, &mut matched_os);
             let build_config = BUILD_CONFIG["build"]
                 .as_table()
                 .expect("Malformed config file");
-            builder = add_clang_args(builder, build_config, &mut matched_os);
-            if !matched_os {
-                panic!("Unknown platform");
+            let extra_bindgen_flags = build_config["args"].as_array().unwrap().as_slice();
+            for item in extra_bindgen_flags.iter() {
+                builder = builder.clang_arg(item.as_str().expect("Expect string in list"));
             }
+
             builder
         }
         fn include<T: Into<String>>(self, file: T) -> Builder {
             self.clang_arg("-include").clang_arg(file)
         }
         // This makes an FFI-safe void type that can't be matched on
         // &VoidType is UB to have, because you can match on it
         // to produce a reachable unreachable. If it's wrapped in