Bug 1471096 - Choose OOM hooking version to use at build time rather than configure time. r=froydnj draft
authorMike Hommey <mh+mozilla@glandium.org>
Sat, 23 Jun 2018 07:28:26 +0900
changeset 811113 6b2f78aadfcce5f511466b7b2239b93c84083250
parent 811112 b6cab86b529857c0beeb235d740461f953e69516
child 811157 7b2a786caf13ce4ec0122b8f53abc75c1daafb82
push id114188
push userbmo:mh+mozilla@glandium.org
push dateTue, 26 Jun 2018 22:27:13 +0000
reviewersfroydnj
bugs1471096, 1458161
milestone63.0a1
Bug 1471096 - Choose OOM hooking version to use at build time rather than configure time. r=froydnj When I originally implemented bug 1458161, this is how it was done, but it was suggested to use a configure-time check. This turned out to not be great, because the rust compiler changes regularly, and we don't run the configure tests when the version changes. When people upgraded their rust compiler to 1.27, the code subsequently failed to build because the features were still set for the previous version they had installed.
build/moz.configure/rust.configure
toolkit/library/gtest/rust/Cargo.toml
toolkit/library/rust/Cargo.toml
toolkit/library/rust/gkrust-features.mozbuild
toolkit/library/rust/shared/Cargo.toml
toolkit/library/rust/shared/build.rs
--- a/build/moz.configure/rust.configure
+++ b/build/moz.configure/rust.configure
@@ -249,18 +249,16 @@ def rust_target_env_name(triple):
 # We need this to form various Cargo environment variables, as there is no
 # uppercase function in make, and we don't want to shell out just for
 # converting a string to uppercase.
 set_config('RUST_TARGET_ENV_NAME', rust_target_env_name)
 
 # This is used for putting source info into symbol files.
 set_config('RUSTC_COMMIT', depends(rustc_info)(lambda i: i.commit))
 
-set_config('RUSTC_VERSION', depends(rustc_info)(lambda i: str(i.version)))
-
 # Until we remove all the other Rust checks in old-configure.
 add_old_configure_assignment('RUSTC', rustc)
 add_old_configure_assignment('RUST_TARGET', rust_target_triple)
 
 # Rustdoc is required by Rust tests below.
 rustdoc = check_prog('RUSTDOC', add_rustup_path('rustdoc'), allow_missing=True)
 
 # This option is separate from --enable-tests because Rust tests are particularly
--- a/toolkit/library/gtest/rust/Cargo.toml
+++ b/toolkit/library/gtest/rust/Cargo.toml
@@ -8,18 +8,16 @@ description = "Testing code for libgkrus
 [features]
 bindgen = ["gkrust-shared/bindgen"]
 servo = ["gkrust-shared/servo"]
 quantum_render = ["gkrust-shared/quantum_render"]
 cubeb-remoting = ["gkrust-shared/cubeb-remoting"]
 cubeb_pulse_rust = ["gkrust-shared/cubeb_pulse_rust"]
 gecko_debug = ["gkrust-shared/gecko_debug"]
 simd-accel = ["gkrust-shared/simd-accel"]
-oom_with_global_alloc = ["gkrust-shared/oom_with_global_alloc"]
-oom_with_hook = ["gkrust-shared/oom_with_hook"]
 moz_memory = ["gkrust-shared/moz_memory"]
 
 [dependencies]
 mp4parse-gtest = { path = "../../../../dom/media/gtest" }
 nsstring-gtest = { path = "../../../../xpcom/rust/gtest/nsstring" }
 xpcom-gtest = { path = "../../../../xpcom/rust/gtest/xpcom" }
 gkrust-shared = { path = "../../rust/shared" }
 
--- a/toolkit/library/rust/Cargo.toml
+++ b/toolkit/library/rust/Cargo.toml
@@ -8,18 +8,16 @@ description = "Rust code for libxul"
 [features]
 bindgen = ["gkrust-shared/bindgen"]
 servo = ["gkrust-shared/servo"]
 quantum_render = ["gkrust-shared/quantum_render"]
 cubeb-remoting = ["gkrust-shared/cubeb-remoting"]
 cubeb_pulse_rust = ["gkrust-shared/cubeb_pulse_rust"]
 gecko_debug = ["gkrust-shared/gecko_debug"]
 simd-accel = ["gkrust-shared/simd-accel"]
-oom_with_global_alloc = ["gkrust-shared/oom_with_global_alloc"]
-oom_with_hook = ["gkrust-shared/oom_with_hook"]
 moz_memory = ["gkrust-shared/moz_memory"]
 
 [dependencies]
 gkrust-shared = { path = "shared" }
 
 [dev-dependencies]
 stylo_tests = { path = "../../../servo/ports/geckolib/tests/" }
 
--- a/toolkit/library/rust/gkrust-features.mozbuild
+++ b/toolkit/library/rust/gkrust-features.mozbuild
@@ -20,20 +20,8 @@ if CONFIG['MOZ_RUST_SIMD']:
 
 # This feature is only supported on Linux and macOS, and this check needs to
 # match MOZ_CUBEB_REMOTING in CubebUtils.cpp.
 if (CONFIG['OS_ARCH'] == 'Linux' and CONFIG['OS_TARGET'] != 'Android') or CONFIG['OS_ARCH'] == 'Darwin':
     gkrust_features += ['cubeb-remoting']
 
 if CONFIG['MOZ_MEMORY']:
     gkrust_features += ['moz_memory']
-
-# See details in toolkit/library/rust/shared/lib.rs
-# A string test is not the best thing, but it works well enough here.
-if CONFIG['RUSTC_VERSION'] < "1.27":
-    gkrust_features += ['oom_with_global_alloc']
-elif CONFIG['RUSTC_VERSION'] >= "1.28" and CONFIG['RUSTC_VERSION'] < "1.29":
-    gkrust_features += ['oom_with_hook']
-elif CONFIG['MOZ_AUTOMATION']:
-    # We don't want builds on automation to unwittingly stop annotating OOM
-    # crash reports from rust.
-    error('Builds on automation must use a version of rust that supports OOM '
-          'hooking')
--- a/toolkit/library/rust/shared/Cargo.toml
+++ b/toolkit/library/rust/shared/Cargo.toml
@@ -37,18 +37,16 @@ rustc_version = "=0.2.1"
 default = []
 bindgen = ["geckoservo/bindgen"]
 servo = ["geckoservo"]
 quantum_render = ["webrender_bindings"]
 cubeb-remoting = ["cubeb-sys", "audioipc-client", "audioipc-server"]
 cubeb_pulse_rust = ["cubeb-sys", "cubeb-pulse"]
 gecko_debug = ["geckoservo/gecko_debug", "nsstring/gecko_debug"]
 simd-accel = ["encoding_c/simd-accel", "encoding_glue/simd-accel"]
-oom_with_global_alloc = []
-oom_with_hook = []
 moz_memory = ["mp4parse_capi/mp4parse_fallible"]
 
 [lib]
 path = "lib.rs"
 test = false
 doctest = false
 bench = false
 doc = false
--- a/toolkit/library/rust/shared/build.rs
+++ b/toolkit/library/rust/shared/build.rs
@@ -1,8 +1,25 @@
+extern crate rustc_version;
+
+use rustc_version::{version, Version};
+
 fn main() {
+    let ver = version().unwrap();
+    let mut bootstrap = false;
+
+    if ver >= Version::parse("1.24.0").unwrap() && ver < Version::parse("1.27.0").unwrap() {
+        println!("cargo:rustc-cfg=feature=\"oom_with_global_alloc\"");
+        bootstrap = true;
+    } else if ver >= Version::parse("1.28.0-alpha").unwrap() && ver < Version::parse("1.29.0").unwrap() {
+        println!("cargo:rustc-cfg=feature=\"oom_with_hook\"");
+        bootstrap = true;
+    } else if std::env::var("MOZ_AUTOMATION").is_ok() {
+        panic!("Builds on automation must use a version of rust that supports OOM hooking")
+    }
+
     // This is a rather awful thing to do, but we're only doing it on
-    // versions of rustc, >= 1.24 < 1.27, that are not going to change
-    // the unstable APIs we use from under us (1.26 being a beta as of
-    // writing, and close to release).
-    #[cfg(any(feature = "oom_with_global_alloc", feature = "oom_with_hook"))]
-    println!("cargo:rustc-env=RUSTC_BOOTSTRAP=1");
+    // versions of rustc that are not going to change the unstable APIs
+    // we use from under us, all being already released or beta.
+    if bootstrap {
+        println!("cargo:rustc-env=RUSTC_BOOTSTRAP=1");
+    }
 }