Repository: google/autocxx (rust)

Issue:
Tweaks needed to command-line generation tool
`gen/cmd` turns out to need the following tweaks in order to work in the Chrome `gn`/`ninja` build system. I'm working on them.

* *Do not canonize paths*. It turns out that sometimes compilers are passed `-I` flags to entirely non-existent directories, which confuses canonizers. Canonizing seems to be unnecessary anyway; `bindgen` seems happy with relative paths.
* *Do not write out unchanged files*. If a file hasn't changed, don't write it out, so as to avoid changing timestamps and causing downstream work.
* *Use the same command-line syntax as compilers for includes.* That means `-I`
* *Don't rely on rewriting the entire .rs file*. Currently the command-line utility takes the _entire_ Rust file and writes out a new version with the `include_cpp!` macro substituted out with replacement code. The resulting `rs` file would need to go into a `gen` directory in a `gn`/`ninja` world, and the only way we can accomplish that for a Rust library is _also_ to copy _all other_ `rs` files into that `gen` directory (as far as I know). It would be better to rely on use of the `autocxx_macro` crate to `include!` an extra generated file, just as we do for `gen/build`. This in turn requires a few extra changes since the dynamic filenames are probably not compatible with statically resolved dependency trees as expected by `gn`/`ninja`. Completely predictable output filenames are needed.

We should also do #56 but that's beyond the scope of this issue.

Pull request:
Redo autocxx-gen syntax.
This allows us to generate the include! files required by autocxx_macro.
It introduces an AUTOCXX_RS_FILE to educate the macro about how to find
such files.

This is necessary for build systems which rely on tracking completely
fixed input and output file names for each build action.

Fixes #248.

Patch:
diff --git a/README.md b/README.md
index 9e3e428d..074a9a67 100644
--- a/README.md
+++ b/README.md
@@ -125,7 +125,8 @@ variable such that the macro can discover the location of the .rs file which was
 If you use the `build.rs` cargo integration, this happens automatically. You'll also need
 to ensure that you build and link against the C++ code. Again, if you use the Cargo integrationm
 and follow the pattern of the `demo` example, this is fairly automatic because we use
-`cc` for this.
+`cc` for this. (There's also the option of `AUTOCXX_RS_FILE` if your build system needs to
+specify the precise file name used for the `.rs` file which is `include!`ed).
 
 You'll also want to ensure that the code generation (both Rust and C++ code) happens whenever
 any included header file changes. This is now handled automatically by our
diff --git a/gen/cmd/src/main.rs b/gen/cmd/src/main.rs
index c9380eea..f392ac25 100644
--- a/gen/cmd/src/main.rs
+++ b/gen/cmd/src/main.rs
@@ -16,7 +16,7 @@
 mod cmd_test;
 
 use autocxx_engine::parse_file;
-use clap::{crate_authors, crate_version, App, Arg, SubCommand};
+use clap::{crate_authors, crate_version, App, Arg, ArgGroup};
 use indoc::indoc;
 use proc_macro2::TokenStream;
 use quote::ToTokens;
@@ -24,20 +24,60 @@ use std::io::{Read, Write};
 use std::path::PathBuf;
 use std::{fs::File, path::Path};
 
+pub(crate) static BLANK: &str = "// Blank autocxx placeholder";
+
+static LONG_HELP: &str = indoc! {"
+Command line utility to expand the Rust 'autocxx' include_cpp! directive.
+
+This tool can generate both the C++ and Rust side binding code for
+a Rust file containing an include_cpp! directive.
+
+If you're using cargo, don't use this: use autocxx_build instead,
+which is much easier to include in build.rs build scripts. You'd likely
+use this tool only if you're using some non-Cargo build system. If
+that's you, read on.
+
+This tool has three modes: generate the C++; generate a new Rust file where
+the include_cpp! directive is *replaced* with bindings, or generate
+a Rust file which can be included by the autocxx_macro. You may specify
+multiple modes, or of course, invoke the tool multiple times.
+
+In any mode, you'll need to pass the source Rust file name and the C++
+include path.
+
+For generation of the Rust side bindings, here's how to choose between
+the two modes. If you're copying the entire Rust crate to a different
+location during your build process, you may as well use --gen-rs-complete
+to generate a whole new replacement .rs file with the autocxx
+include_cpp! macro expanded.
+
+But in most build systems, you won't be copying all the crate source
+to a new location. In such a case, you should use --gen-rs-include
+which will generate a file that will be included by the autocxx_macro
+crate.
+
+The second decision you must make is naming of the output files.
+If your build system is able to cope with autocxx_gen building
+unpredictable filenames, then:
+a) set AUTOCXX_RS when using autocxx_macro
+b) build all *.cc files produced by this tool.
+
+If your build system requires each build rule to make precise filenames
+known in advance, then use --generate-exact 2
+and --fix-rs-include-name. If you do this, you'll need to:
+a) Have exactly one include_cpp directive in each source .rs file
+   (or use different numbers with --generate-exact)
+b) Set AUTOCXX_RS_FILE when using autocxx_macro.
+c) Teach your build system always that the outputs of this tool
+   are always guaranteed to be gen0.include.rs, gen0.cc and gen1.cc.
+"};
+
 fn main() {
     let matches = App::new("autocxx-gen")
         .version(crate_version!())
         .author(crate_authors!())
-        .about("Generates C++ files from Rust files that contain include_cpp! macros")
-        .long_about(indoc! {"
-                Command line utility to expand the Rust 'autocxx'
-                include_cpp macro. Normal usage here is to use the gen-cpp
-                subcommand to generate the .cpp and .h side of the bindings,
-                such that you can build them and link them to Rust code.
-                You can also use this utility
-                to expand the Rust code if you wish to avoid a dependency
-                on a C++ include path within your Rust build process.
-        "})
+        .about("Generates bindings files from Rust files that contain include_cpp! macros")
+        .long_about(LONG_HELP)
         .arg(
             Arg::with_name("INPUT")
                 .help("Sets the input .rs file to use")
@@ -63,34 +103,55 @@ fn main() {
                 .help("include path")
                 .takes_value(true),
         )
-        .subcommand(
-            SubCommand::with_name("gen-cpp")
-                .help("Generate C++ .cpp and .h files. Normal mode of operation.")
-                .arg(
-                    Arg::with_name("pattern")
-                        .long("pattern")
-                        .value_name("PATTERN")
-                        .help(".h and .cpp output pattern")
-                        .default_value("gen")
-                        .takes_value(true),
-                )
-                .arg(
-                    Arg::with_name("cpp-extension")
-                        .long("cpp-extension")
-                        .value_name("EXTENSION")
-                        .default_value("cc")
-                        .help("include path")
-                        .takes_value(true),
-                )
-                .arg(
-                    Arg::with_name("generate-exact")
-                        .long("generate-exact")
-                        .value_name("NUM")
-                        .help("always generate this number of .cc and .h files")
-                        .takes_value(true),
-                ),
+        .arg(
+            Arg::with_name("cpp-extension")
+                .long("cpp-extension")
+                .value_name("EXTENSION")
+                .default_value("cc")
+                .help("C++ filename extension")
+                .takes_value(true),
+        )
+        .arg(
+            Arg::with_name("gen-cpp")
+                .long("gen-cpp")
+                .help("whether to generate C++ implementation and header files")
+        )
+        .arg(
+            Arg::with_name("gen-rs-complete")
+                .long("gen-rs-complete")
+                .help("whether to generate a Rust file replacing the original file (suffix will be .complete.rs)")
+        )
+        .arg(
+            Arg::with_name("gen-rs-include")
+                .long("gen-rs-include")
+                .help("whether to generate Rust files for inclusion using autocxx_macro (suffix will be .include.rs)")
+        )
+        .group(ArgGroup::with_name("mode")
+            .required(true)
+            .multiple(true)
+            .arg("gen-cpp")
+            .arg("gen-rs-complete")
+            .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")
+                .requires("gen-cpp")
+        )
+        .arg(
+            Arg::with_name("generate-exact")
+                .long("generate-exact")
+                .value_name("NUM")
+                .help("assume and ensure there are exactly NUM bridge blocks in the file. Only applies for --gen-cpp or --gen-rs-include")
+                .takes_value(true),
+        )
+        .arg(
+            Arg::with_name("fix-rs-include-name")
+                .long("fix-rs-include-name")
+                .help("Make the name of the .rs file predictable. You must set AUTOCXX_RS_FILE during Rust build time to educate autocxx_macro about your choice.")
+                .requires("gen-rs-include")
         )
-        .subcommand(SubCommand::with_name("gen-rs").help("Generate expanded Rust file."))
         .get_matches();
     let mut parsed_file = parse_file(matches.value_of("INPUT").unwrap())
         .expect("Unable to parse Rust file and interpret autocxx macro");
@@ -106,48 +167,67 @@ fn main() {
         .resolve_all(incs, None)
         .expect("Unable to resolve macro");
     let outdir: PathBuf = matches.value_of_os("outdir").unwrap().into();
-    if let Some(matches) = matches.subcommand_matches("gen-cpp") {
-        let pattern = matches.value_of("pattern").unwrap_or("gen");
-        let cpp = matches.value_of("cpp-extension").unwrap_or("cc");
-        let desired_number = matches
-            .value_of("generate-exact")
-            .map(|s| s.parse::<usize>().unwrap());
+    let desired_number = matches
+        .value_of("generate-exact")
+        .map(|s| s.parse::<usize>().unwrap());
+    if matches.is_present("gen-cpp") {
+        let cpp = matches.value_of("cpp-extension").unwrap();
         let mut counter = 0usize;
         for include_cxx in parsed_file.get_autocxxes() {
             let generations = include_cxx
                 .generate_h_and_cxx()
                 .expect("Unable to generate header and C++ code");
             for pair in generations.0 {
-                let cppname = format!("{}{}.{}", pattern, counter, cpp);
+                let cppname = format!("gen{}.{}", counter, cpp);
                 write_to_file(&outdir, cppname, &pair.implementation);
                 write_to_file(&outdir, pair.header_name, &pair.header);
                 counter += 1;
             }
         }
-        if let Some(desired_number) = desired_number {
-            while counter < desired_number {
-                write_cpp_file(
-                    &outdir,
-                    pattern,
-                    cpp,
-                    counter,
-                    "// Blank C++ file generated by autocxx".as_bytes(),
-                );
-                counter += 1;
-            }
-        }
-    } else if matches.subcommand_matches("gen-rs").is_some() {
+        write_placeholders(&outdir, counter, desired_number, cpp);
+    }
+    if matches.is_present("gen-rs-complete") {
         let mut ts = TokenStream::new();
         parsed_file.to_tokens(&mut ts);
-        write_to_file(&outdir, "gen.rs".to_string(), ts.to_string().as_bytes());
-    } else {
-        panic!("Must specify a subcommand");
+        write_to_file(
+            &outdir,
+            "gen.complete.rs".to_string(),
+            ts.to_string().as_bytes(),
+        );
+    }
+    if matches.is_present("gen-rs-include") {
+        let autocxxes = parsed_file.get_autocxxes();
+        let mut counter = 0usize;
+        for include_cxx in autocxxes {
+            let ts = include_cxx.generate_rs();
+            let fname = if matches.is_present("fix-rs-include-name") {
+                format!("gen{}.include.rs", counter)
+            } else {
+                include_cxx.get_rs_filename()
+            };
+            write_to_file(&outdir, fname, ts.to_string().as_bytes());
+            counter += 1;
+        }
+        write_placeholders(&outdir, counter, desired_number, "include.rs");
     }
 }
 
-fn write_cpp_file(outdir: &Path, pattern: &str, cpp: &str, counter: usize, content: &[u8]) {
-    let cppname = format!("{}{}.{}", pattern, counter, cpp);
-    write_to_file(outdir, cppname, content);
+fn write_placeholders(
+    outdir: &Path,
+    mut counter: usize,
+    desired_number: Option<usize>,
+    extension: &str,
+) {
+    if let Some(desired_number) = desired_number {
+        if counter > desired_number {
+            panic!("More include_cpp! sections were found than expected");
+        }
+        while counter < desired_number {
+            let fname = format!("gen{}.{}", counter, extension);
+            write_to_file(&outdir, fname, BLANK.as_bytes());
+            counter += 1;
+        }
+    }
 }
 
 fn write_to_file(dir: &Path, filename: String, content: &[u8]) {
diff --git a/parser/src/file_locations.rs b/parser/src/file_locations.rs
index 2df24f44..98506c8e 100644
--- a/parser/src/file_locations.rs
+++ b/parser/src/file_locations.rs
@@ -23,8 +23,16 @@ use std::path::PathBuf;
 /// not available. We need to give provision for custom locations
 /// and we need our code generator build script to be able to pass
 /// locations through to the proc macro by env vars.
+///
+/// On the whole this class concerns itself with directory names
+/// and allows the actual file name to be determined elsewhere
+/// (based on a hash of the contents of `include_cpp!`.) But
+/// some types of build system need to know the precise file _name_
+/// produced by the codegen phase and passed into the macro phase,
+/// so we have a `AUTOCXX_RS_FILE` option for that.
 pub enum FileLocationStrategy {
     Custom(PathBuf),
+    FromAutocxxRsFile(PathBuf),
     FromAutocxxRs(PathBuf),
     FromOutDir(PathBuf),
     UnknownMaybeFromOutdir,
@@ -33,15 +41,19 @@ pub enum FileLocationStrategy {
 static BUILD_DIR_NAME: &str = "autocxx-build-dir";
 static RS_DIR_NAME: &str = "rs";
 static AUTOCXX_RS: &str = "AUTOCXX_RS";
+static AUTOCXX_RS_FILE: &str = "AUTOCXX_RS_FILE";
 
 impl FileLocationStrategy {
     pub fn new() -> Self {
-        match std::env::var_os(AUTOCXX_RS) {
-            None => match std::env::var_os("OUT_DIR") {
-                None => FileLocationStrategy::UnknownMaybeFromOutdir,
-                Some(od) => FileLocationStrategy::FromOutDir(PathBuf::from(od)),
+        match std::env::var_os(AUTOCXX_RS_FILE) {
+            Some(of) => FileLocationStrategy::FromAutocxxRsFile(PathBuf::from(of)),
+            None => match std::env::var_os(AUTOCXX_RS) {
+                None => match std::env::var_os("OUT_DIR") {
+                    None => FileLocationStrategy::UnknownMaybeFromOutdir,
+                    Some(od) => FileLocationStrategy::FromOutDir(PathBuf::from(od)),
+                },
+                Some(acrs) => FileLocationStrategy::FromAutocxxRs(PathBuf::from(acrs)),
             },
-            Some(acrs) => FileLocationStrategy::FromAutocxxRs(PathBuf::from(acrs)),
         }
     }
 
@@ -72,6 +84,14 @@ impl FileLocationStrategy {
                     include!(concat!(env!("OUT_DIR"), #fname));
                 }
             }
+            FileLocationStrategy::FromAutocxxRsFile(fname) => {
+                let fname = fname
+                    .to_str()
+                    .expect("AUTOCXX_RS_FILE environment variable contained non-UTF8 characters");
+                quote! {
+                    include!( #fname )
+                }
+            }
         }
     }
 
@@ -83,6 +103,9 @@ impl FileLocationStrategy {
             FileLocationStrategy::UnknownMaybeFromOutdir => {
                 panic!("Could not determine OUT_DIR or AUTOCXX_RS dir")
             }
+            FileLocationStrategy::FromAutocxxRsFile(_) => {
+                panic!("It's invalid to set AUTOCXX_RS_FILE during the codegen phase.")
+            }
         };
         root.join(suffix)
     }
diff --git a/src/lib.rs b/src/lib.rs
index 33eb83c4..c83b3341 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -63,7 +63,10 @@ use autocxx_engine::IncludeCppEngine;
 ///   implementation files. This will also generate `.rs` side bindings.
 /// * Educate the procedural macro about where to find the generated `.rs` bindings. Set the
 ///   `AUTOCXX_RS` environment variable to a list of directories to search.
-///   If you use `autocxx-build`, this happens automatically.
+///   If you use `autocxx-build`, this happens automatically. (You can alternatively
+///   specify `AUTOCXX_RS_FILE` to give a precise filename as opposed to a directory to search,
+///   though this isn't recommended unless your build system specifically requires it
+///   because it allows only a single `include_cpp!` block per `.rs` file.)
 ///
 /// ```mermaid
 /// flowchart TB


Test patch:
diff --git a/gen/cmd/src/cmd_test.rs b/gen/cmd/src/cmd_test.rs
index 96f68888..b034b162 100644
--- a/gen/cmd/src/cmd_test.rs
+++ b/gen/cmd/src/cmd_test.rs
@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-use std::{fs::File, io::Write, path::PathBuf};
+use std::{convert::TryInto, fs::File, io::Write, path::PathBuf};
 
 use assert_cmd::Command;
 use tempdir::TempDir;
@@ -41,9 +41,41 @@ fn test_gen() -> Result<(), Box<dyn std::error::Error>> {
         .arg(demo_rs)
         .arg("--outdir")
         .arg(tmp_dir.path().to_str().unwrap())
-        .arg("gen-cpp")
+        .arg("--gen-cpp")
+        .arg("--gen-rs-include")
         .assert()
         .success();
+    assert_contentful(&tmp_dir, "gen0.cc");
+    Ok(())
+}
+
+#[test]
+fn test_gen_fixed_num() -> Result<(), Box<dyn std::error::Error>> {
+    let tmp_dir = TempDir::new("example")?;
+    let demo_code_dir = tmp_dir.path().join("demo");
+    std::fs::create_dir(&demo_code_dir).unwrap();
+    write_to_file(&demo_code_dir, "input.h", INPUT_H.as_bytes());
+    write_to_file(&demo_code_dir, "main.rs", MAIN_RS.as_bytes());
+    let demo_rs = demo_code_dir.join("main.rs");
+    let mut cmd = Command::cargo_bin("autocxx-gen")?;
+    cmd.arg("-I")
+        .arg(demo_code_dir.to_str().unwrap())
+        .arg(demo_rs)
+        .arg("--outdir")
+        .arg(tmp_dir.path().to_str().unwrap())
+        .arg("--gen-cpp")
+        .arg("--gen-rs-include")
+        .arg("--generate-exact")
+        .arg("3")
+        .arg("--fix-rs-include-name")
+        .assert()
+        .success();
+    assert_contentful(&tmp_dir, "gen0.cc");
+    assert_contentful(&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(())
 }
 
@@ -52,3 +84,18 @@ fn write_to_file(dir: &PathBuf, filename: &str, content: &[u8]) {
     let mut f = File::create(&path).expect("Unable to create file");
     f.write_all(content).expect("Unable to write file");
 }
+
+fn assert_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());
+}
+
+fn assert_exists(outdir: &TempDir, fname: &str) {
+    let p = outdir.path().join(fname);
+    if !p.exists() {
+        panic!("File {} didn't exist", p.to_string_lossy());
+    }
+}