Repository: google/autocxx
Pull request #792 (2022-02-13 10:54:18)

Implement an option for not generating `cxx::bridge` sources
It's documented for autocxx-gen, but confusingly didn't do anything. I
flipped the truth value of it so the default is the same as the old
behavior.

Also document another gotcha which combined with this one to confuse me
even more.

- [x] Tests pass
- [x] Appropriate changes to README are included in PR

adetaylor: Thanks, firstly for spotting the faulty option, and secondly for getting to grips with the convoluted test suite! Terrific to see a test added here for this option.

I'm happy to merge this as-is but the second commit that you pushed is authored by a different identity, and has therefore made our CLA bot sad. Please would you force-push to your branch to resolve that, then I'll go ahead and merge.

Patch:
diff --git a/engine/src/builder.rs b/engine/src/builder.rs
index 58906ea9..29876602 100644
--- a/engine/src/builder.rs
+++ b/engine/src/builder.rs
@@ -179,6 +179,13 @@ impl<CTX: BuilderContext> Builder<CTX> {
         self
     }
 
+    /// Whether to skip using [`cxx_gen`] to generate the C++ code,
+    /// so that some other process can handle that.
+    pub fn skip_cxx_gen(mut self, skip_cxx_gen: bool) -> Self {
+        self.cpp_codegen_options.skip_cxx_gen = skip_cxx_gen;
+        self
+    }
+
     /// Build autocxx C++ files and return a cc::Build you can use to build
     /// more from a build.rs file.
     pub fn build(self) -> Result<BuilderBuild, BuilderError> {
diff --git a/engine/src/lib.rs b/engine/src/lib.rs
index 669789e1..fee287cb 100644
--- a/engine/src/lib.rs
+++ b/engine/src/lib.rs
@@ -570,7 +570,9 @@ impl CppBuildable for IncludeCppEngine {
             State::NotGenerated => panic!("Call generate() first"),
             State::Generated(gen_results) => {
                 let rs = gen_results.item_mod.to_token_stream();
-                files.push(do_cxx_cpp_generation(rs, cpp_codegen_options)?);
+                if !cpp_codegen_options.skip_cxx_gen {
+                    files.push(do_cxx_cpp_generation(rs, cpp_codegen_options)?);
+                }
                 if let Some(cpp_file_pair) = &gen_results.cpp {
                     files.push(cpp_file_pair.clone());
                 }
@@ -642,4 +644,7 @@ pub struct CppCodegenOptions {
     /// An annotation optionally to include on each C++ function.
     /// For example to export the symbol from a library.
     pub cxx_impl_annotations: Option<String>,
+    /// Whether to skip using [`cxx_gen`] to generate the C++ code,
+    /// so that some other process can handle that.
+    pub skip_cxx_gen: bool,
 }
diff --git a/gen/cmd/src/main.rs b/gen/cmd/src/main.rs
index 0af32b45..c567367d 100644
--- a/gen/cmd/src/main.rs
+++ b/gen/cmd/src/main.rs
@@ -133,9 +133,9 @@ fn main() {
             .arg("gen-rs-include")
         )
         .arg(
-            Arg::with_name("cxx-gen")
-                .long("cxx-gen")
-                .help("Perform C++ codegen also for #[cxx::bridge] blocks. Only applies for --gen-cpp")
+            Arg::with_name("skip-cxx-gen")
+                .long("skip-cxx-gen")
+                .help("Skip performing C++ codegen for #[cxx::bridge] blocks. Only applies for --gen-cpp")
                 .requires("gen-cpp")
         )
         .arg(
@@ -211,6 +211,7 @@ fn main() {
     cpp_codegen_options.cxx_impl_annotations = get_option_string("cxx-impl-annotations", &matches);
     cpp_codegen_options.path_to_cxx_h = get_option_string("cxx-h-path", &matches);
     cpp_codegen_options.path_to_cxxgen_h = get_option_string("cxxgen-h-path", &matches);
+    cpp_codegen_options.skip_cxx_gen = matches.is_present("skip-cxx-gen");
     // In future, we should provide an option to write a .d file here
     // by passing a callback into the dep_recorder parameter here.
     // https://github.com/google/autocxx/issues/56


Test patch:
diff --git a/gen/cmd/src/cmd_test.rs b/gen/cmd/src/cmd_test.rs
index 2adbc3ab..6c3a1345 100644
--- a/gen/cmd/src/cmd_test.rs
+++ b/gen/cmd/src/cmd_test.rs
@@ -28,6 +28,15 @@ fn test_help() -> Result<(), Box<dyn std::error::Error>> {
 }
 
 fn base_test<F>(tmp_dir: &TempDir, arg_modifier: F) -> Result<(), Box<dyn std::error::Error>>
+where
+    F: FnOnce(&mut Command),
+{
+    let result = base_test_ex(tmp_dir, arg_modifier);
+    assert_contentful(&tmp_dir, "gen0.cc");
+    result
+}
+
+fn base_test_ex<F>(tmp_dir: &TempDir, arg_modifier: F) -> Result<(), Box<dyn std::error::Error>>
 where
     F: FnOnce(&mut Command),
 {
@@ -47,7 +56,6 @@ where
         .arg("--gen-rs-include")
         .assert()
         .success();
-    assert_contentful(&tmp_dir, "gen0.cc");
     Ok(())
 }
 
@@ -119,6 +127,25 @@ fn test_gen_repro() -> Result<(), Box<dyn std::error::Error>> {
     Ok(())
 }
 
+#[test]
+fn test_skip_cxx_gen() -> Result<(), Box<dyn std::error::Error>> {
+    let tmp_dir = TempDir::new("example")?;
+    base_test_ex(&tmp_dir, |cmd| {
+        cmd.arg("--generate-exact")
+            .arg("3")
+            .arg("--fix-rs-include-name")
+            .arg("--skip-cxx-gen");
+    })?;
+    assert_contentful(&tmp_dir, "autocxxgen_ffi.h");
+    assert_not_contentful(&tmp_dir, "gen0.cc");
+    assert_exists(&tmp_dir, "gen1.cc");
+    assert_exists(&tmp_dir, "gen2.cc");
+    assert_contentful(&tmp_dir, "gen0.include.rs");
+    assert_exists(&tmp_dir, "gen1.include.rs");
+    assert_exists(&tmp_dir, "gen2.include.rs");
+    Ok(())
+}
+
 fn write_to_file(dir: &Path, filename: &str, content: &[u8]) {
     let path = dir.join(filename);
     let mut f = File::create(&path).expect("Unable to create file");
@@ -130,7 +157,23 @@ fn assert_contentful(outdir: &TempDir, fname: &str) {
     if !p.exists() {
         panic!("File {} didn't exist", p.to_string_lossy());
     }
-    assert!(p.metadata().unwrap().len() > super::BLANK.len().try_into().unwrap());
+    assert!(
+        p.metadata().unwrap().len() > super::BLANK.len().try_into().unwrap(),
+        "File {} is empty",
+        fname
+    );
+}
+
+fn assert_not_contentful(outdir: &TempDir, fname: &str) {
+    let p = outdir.path().join(fname);
+    if !p.exists() {
+        panic!("File {} didn't exist", p.to_string_lossy());
+    }
+    assert!(
+        p.metadata().unwrap().len() <= super::BLANK.len().try_into().unwrap(),
+        "File {} is not empty",
+        fname
+    );
 }
 
 fn assert_exists(outdir: &TempDir, fname: &str) {
diff --git a/integration-tests/src/test_utils.rs b/integration-tests/src/test_utils.rs
index e2f576fb..a507122f 100644
--- a/integration-tests/src/test_utils.rs
+++ b/integration-tests/src/test_utils.rs
@@ -60,11 +60,7 @@ impl LinkableTryBuilder {
                 if dest.exists() {
                     std::fs::remove_file(&dest).unwrap();
                 }
-                if KEEP_TEMPDIRS {
-                    std::fs::copy(item.path(), dest).unwrap();
-                } else {
-                    std::fs::rename(item.path(), dest).unwrap();
-                }
+                std::fs::rename(item.path(), dest).unwrap();
             }
         }
     }
@@ -391,9 +387,6 @@ where
     b.include(tdir.path())
         .try_compile("autocxx-demo")
         .map_err(TestError::CppBuild)?;
-    if KEEP_TEMPDIRS {
-        println!("Generated .rs files: {:?}", generated_rs_files);
-    }
     // Step 8: use the trybuild crate to build the Rust file.
     let r = get_builder().lock().unwrap().build(
         &target_dir,
@@ -403,13 +396,13 @@ where
         &rs_path,
         generated_rs_files,
     );
-    if KEEP_TEMPDIRS {
-        println!("Tempdir: {:?}", tdir.into_path().to_str());
-    }
     if r.is_err() {
         return Err(TestError::RsBuild); // details of Rust panic are a bit messy to include, and
                                         // not important at the moment.
     }
+    if KEEP_TEMPDIRS {
+        println!("Tempdir: {:?}", tdir.into_path().to_str());
+    }
     Ok(())
 }
 
@@ -546,6 +539,17 @@ impl BuilderModifierFns for EnableAutodiscover {
     }
 }
 
+pub(crate) struct SkipCxxGen;
+
+impl BuilderModifierFns for SkipCxxGen {
+    fn modify_autocxx_builder(
+        &self,
+        builder: Builder<TestBuilderContext>,
+    ) -> Builder<TestBuilderContext> {
+        builder.skip_cxx_gen(true)
+    }
+}
+
 /// Searches generated C++ for strings we want to find, or want _not_ to find,
 /// or both.
 pub(crate) struct CppMatcher<'a> {
@@ -582,3 +586,28 @@ impl<'a> CodeCheckerFns for CppMatcher<'a> {
         }
     }
 }
+
+/// Counts the number of generated C++ files.
+pub(crate) struct CppCounter {
+    cpp_count: usize,
+}
+
+impl CppCounter {
+    pub(crate) fn new(cpp_count: usize) -> Self {
+        Self { cpp_count }
+    }
+}
+
+impl CodeCheckerFns for CppCounter {
+    fn check_cpp(&self, cpp: &[PathBuf]) -> Result<(), TestError> {
+        if cpp.len() == self.cpp_count {
+            Ok(())
+        } else {
+            Err(TestError::CppCodeExaminationFail)
+        }
+    }
+
+    fn skip_build(&self) -> bool {
+        true
+    }
+}
diff --git a/integration-tests/src/tests.rs b/integration-tests/src/tests.rs
index e78f93dd..31ef9f77 100644
--- a/integration-tests/src/tests.rs
+++ b/integration-tests/src/tests.rs
@@ -15,7 +15,8 @@
 use crate::test_utils::{
     directives_from_lists, do_run_test_manual, make_clang_arg_adder, make_error_finder,
     make_string_finder, run_test, run_test_ex, run_test_expect_fail, run_test_expect_fail_ex,
-    CppMatcher, EnableAutodiscover, NoSystemHeadersChecker, SetSuppressSystemHeaders,
+    CppCounter, CppMatcher, EnableAutodiscover, NoSystemHeadersChecker, SetSuppressSystemHeaders,
+    SkipCxxGen,
 };
 use indoc::indoc;
 use itertools::Itertools;
@@ -7911,6 +7912,27 @@ fn test_issue486_multi_types() {
     );
 }
 
+#[test]
+fn test_skip_cxx_gen() {
+    let cxx = indoc! {"
+        void do_nothing() {
+        }
+    "};
+    let hdr = indoc! {"
+        void do_nothing();
+    "};
+    let rs = quote! {};
+    run_test_ex(
+        cxx,
+        hdr,
+        rs,
+        directives_from_lists(&["do_nothing"], &[], None),
+        Some(Box::new(SkipCxxGen)),
+        Some(Box::new(CppCounter::new(1))),
+        None,
+    );
+}
+
 // Yet to test:
 // - Ifdef
 // - Out param pointers