Ensure all --extern crates are listed as dependencies in Ninja.
Any Rust crate B that is visible for use as a dependency of another
Rust crate A gets added to the compile of A with an `--extern` flag,
such as `--extern B`. This ensures that B is accessible from A, so that
it can use the functions and types in B.
When A makes use of something in B, it means A needs to be rebuilt when
B changes, however. This is not expressed to ninja through the
`externs` set. Currently, if A deps on B deps on C, and B is an
order-only dependency (such as a group), ninja does not know that A
should be rebuilt when C is changed. For direct dependencies,
ninja hears about them since rlibs are also collected as linkable_deps
in NinjaBinaryTargetWriter::ClassifyDependency(). But transitive
dependencies need to be propagated to the set of implicit dependencies
of a target in order to cause it to rebuild, if the dependency can be
accessed from the target.
We ensure transitive deps get a dependency relationship in ninja if
and only if they appear in `externs`, since transitive dependencies
that are not visible to a crate can not require it to be recompiled.
Bug: 282
Change-Id: I9f30274c65cccabcaed0181d7fe3b95539fb674b
Reviewed-on: https://gn-review.googlesource.com/c/gn/+/13380
Reviewed-by: Tyler Mandry <tmandry@google.com>
Reviewed-by: Brett Wilson <brettw@chromium.org>
Commit-Queue: Brett Wilson <brettw@chromium.org>
diff --git a/src/gn/ninja_rust_binary_target_writer.cc b/src/gn/ninja_rust_binary_target_writer.cc
index f3ff21f..9263a58 100644
--- a/src/gn/ninja_rust_binary_target_writer.cc
+++ b/src/gn/ninja_rust_binary_target_writer.cc
@@ -187,6 +187,12 @@
if (dep->source_types_used().RustSourceUsed() &&
RustValues::IsRustLibrary(dep)) {
transitive_crates.push_back({dep, has_direct_access});
+ // If the current crate can directly acccess the `dep` crate, then the
+ // current crate needs an implicit dependency on `dep` so it will be
+ // rebuilt if `dep` changes.
+ if (has_direct_access) {
+ implicit_deps.push_back(dep->dependency_output_file());
+ }
}
}
diff --git a/src/gn/ninja_rust_binary_target_writer_unittest.cc b/src/gn/ninja_rust_binary_target_writer_unittest.cc
index 68d5970..d224193 100644
--- a/src/gn/ninja_rust_binary_target_writer_unittest.cc
+++ b/src/gn/ninja_rust_binary_target_writer_unittest.cc
@@ -457,7 +457,8 @@
"target_output_name = bar\n"
"\n"
"build ./foo_bar: rust_bin ../../foo/main.rs | ../../foo/source.rs "
- "../../foo/main.rs obj/foo/libdirect.so obj/bar/libmylib.so\n"
+ "../../foo/main.rs obj/foo/libdirect.so obj/bar/libmylib.so "
+ "obj/baz/libinside.rlib\n"
" source_file_part = main.rs\n"
" source_name_part = main\n"
" externs = --extern direct=obj/foo/libdirect.so "
@@ -518,6 +519,13 @@
EXPECT_EQ(expected, out_str) << expected << "\n" << out_str;
}
+ // A group produces an order-only dependency in ninja:
+ // https://ninja-build.org/manual.html#ref_dependencies.
+ //
+ // If a crate D inside the group is visible to a crate C depending on the
+ // group, the crate C needs to be rebuilt when D is changed. The group
+ // dependency does not guarantee that it would, so we test that C has an
+ // indirect dependency on D through this group.
Target group(setup.settings(), Label(SourceDir("//baz/"), "group"));
group.set_output_type(Target::GROUP);
group.visibility().SetPublic();
@@ -543,6 +551,10 @@
NinjaRustBinaryTargetWriter writer(&rlib, out);
writer.Run();
+ // libmymacro.so is inside the obj/baz/group, so would be built before
+ // libmylib.rlib. However it must also cause libmylib.rlib to be recompiled
+ // when changed, so we expect an implicit dependency (appearing after `|` on
+ // the build line) from libmylib.rlib to libmymacro.so.
const char expected[] =
"crate_name = mylib\n"
"crate_type = rlib\n"
@@ -555,7 +567,7 @@
"target_output_name = libmylib\n"
"\n"
"build obj/bar/libmylib.rlib: rust_rlib ../../bar/lib.rs | "
- "../../bar/mylib.rs ../../bar/lib.rs || "
+ "../../bar/mylib.rs ../../bar/lib.rs obj/bar/libmymacro.so || "
"obj/baz/group.stamp\n"
" source_file_part = lib.rs\n"
" source_name_part = lib\n"
@@ -597,7 +609,8 @@
"target_output_name = bar\n"
"\n"
"build ./foo_bar: rust_bin ../../foo/main.rs | "
- "../../foo/source.rs ../../foo/main.rs obj/bar/libmylib.rlib\n"
+ "../../foo/source.rs ../../foo/main.rs "
+ "obj/bar/libmylib.rlib obj/bar/libmymacro.so\n"
" source_file_part = main.rs\n"
" source_name_part = main\n"
" externs = --extern mylib=obj/bar/libmylib.rlib "
@@ -1055,7 +1068,8 @@
"obj/staticlib/libstaticlib.a "
"obj/dylib/libdylib.so "
"obj/pub_in_staticlib/libpub_in_staticlib.rlib "
- "obj/priv_in_staticlib/libpriv_in_staticlib.rlib || "
+ "obj/priv_in_staticlib/libpriv_in_staticlib.rlib "
+ "obj/pub_in_dylib/libpub_in_dylib.rlib || "
"obj/pub_sset_in_staticlib/pub_sset_in_staticlib.stamp "
"obj/priv_sset_in_staticlib/priv_sset_in_staticlib.stamp\n"
" source_file_part = main.rs\n"
