Making deterministic .pc modules

[amyspark]

Hi,

Coming from https://gitlab.freedesktop.org/gstreamer/gst-plugins-rs/-/issues/599#note_2838913 --

Currently, the .pc generation in cargo-c is as simple as capturing the linker line for a given crate, its Requires and Requires.private entries, and string interpolating the two together into the file. While this certainly works, it does not allow for reproducibility:

• the order of libraries in the linker line is unpredictable (depends on the Cargo parallelization AFAIU)
• the Libs and Cflags entries are never pruned for flags provided in Requires etc.
• libraries provided by crates e.g. windows-targets (which does not install its .lib) or libsodium-sys (which does not provide a .pc file) break static usage of the .pc module

My proposal in https://gitlab.freedesktop.org/gstreamer/gst-plugins-rs/-/merge_requests/2162 was to handle it on our end by manually parsing and deduplicating entries, however it was determined that it was best to handle this at the source.

@lu-zero could this be handled in cargo-c in a reasonable way?

[sdroege]

In the worst case you could do the same in cargo-c that were doing in the gst-plugins-rs MR. I'm not sure there is a better way.

[lu-zero]

I need more information on why it happens. Probably we have to sort the link line fragments in cargo first to solve the first problem, the other two seem very outside of what we can solve within cargo or cargo-c though.

[amyspark]

I need more information on why it happens

From the looks of it, it's in rustc where it happens. I'm trying to track down the generation of native-static-libs to figure out why.

[amyspark]

So it seems that the code in compiler\rustc_codegen_ssa\src\back\link.rs does not control for duplicates at all when reducing the linker line. Something like this should work, given that at this stage the compiler is aware of each flag and hasn't yet joined them:

diff --git a/compiler/rustc_codegen_ssa/src/back/link.rs b/compiler/rustc_codegen_ssa/src/back/link.rs
index e2081ad7563..08cd2962ffc 100644
--- a/compiler/rustc_codegen_ssa/src/back/link.rs
+++ b/compiler/rustc_codegen_ssa/src/back/link.rs
@@ -12,7 +12,7 @@
 use regex::Regex;
 use rustc_arena::TypedArena;
 use rustc_ast::CRATE_NODE_ID;
-use rustc_data_structures::fx::{FxIndexMap, FxIndexSet};
+use rustc_data_structures::fx::{FxIndexMap, FxIndexSet, FxHashSet};
 use rustc_data_structures::memmap::Mmap;
 use rustc_data_structures::temp_dir::MaybeTempDir;
 use rustc_errors::DiagCtxtHandle;
@@ -1609,19 +1609,24 @@ fn print_native_static_libs(
         }
     }
 
+    let real_lib_args: Vec<_> = {
+        let mut x: FxHashSet<String> = FxHashSet::default();
+        lib_args.into_iter().filter(|i| x.insert(i.clone())).collect()
+    };
+
     match out {
         OutFileName::Real(path) => {
-            out.overwrite(&lib_args.join(" "), sess);
-            if !lib_args.is_empty() {
+            out.overwrite(&real_lib_args.join(" "), sess);
+            if !real_lib_args.is_empty() {
                 sess.dcx().emit_note(errors::StaticLibraryNativeArtifactsToFile { path });
             }
         }
         OutFileName::Stdout => {
-            if !lib_args.is_empty() {
+            if !real_lib_args.is_empty() {
                 sess.dcx().emit_note(errors::StaticLibraryNativeArtifacts);
                 // Prefix for greppability
                 // Note: This must not be translated as tools are allowed to depend on this exact string.
-                sess.dcx().note(format!("native-static-libs: {}", lib_args.join(" ")));
+                sess.dcx().note(format!("native-static-libs: {}", real_lib_args.join(" ")));
             }
         }
     }

For the correlation against Requires.private parsing of the Requires line is required, however pkg-config the crate is too high level (accepts version ranges programmatically, instead of deriving them from the string slice). That still doesn't solve pathological cases like libsodium, though.

[lu-zero]

I remember that this approach may not always work in some corner cases, wild is overhauling its command line parsing, we may look at this.

[amyspark]

Yeah, as soon as spaces are involved (the most basic case) all bets are off.