Enhance --cargo flag handling by allowing whitespace-separated values and adding tests for proper parsing

Author: isdaniel

cargo-pgrx/src/command/install.rs

@@ -57,8 +57,7 @@ pub(crate) struct Install {
     pub(crate) features: clap_cargo::Features,
     #[clap(long)]
     pub(crate) target: Option<String>,
-    /// Extra flag forwarded verbatim to every `cargo` invocation (repeatable).
-    /// e.g. `--cargo=--config=child/.cargo/config.toml --cargo=--offline`
+    /// Extra cargo flags forwarded to every `cargo` invocation. Repeatable and split on whitespace: `--cargo=--config=foo` or `--cargo "--offline --frozen"`.
     #[clap(long = "cargo", value_name = "FLAG")]
     pub(crate) cargo: Vec<String>,
     #[clap(from_global, action = ArgAction::Count)]
@@ -354,7 +353,7 @@ pub(crate) fn build_extension(
         command.arg(arg);
     }
 
-    for arg in cargo_flags {
+    for arg in crate::metadata::split_cargo_flags(cargo_flags) {
         command.arg(arg);
     }
 
@@ -513,9 +512,12 @@ pub(crate) fn get_version(manifest_path: &Path, cargo_flags: &[String]) -> eyre:
     let version = match get_property(manifest_path, "default_version")? {
         Some(v) => {
             if v == "@CARGO_VERSION@" {
-                let metadata =
-                    crate::metadata::metadata(&Default::default(), Some(manifest_path), cargo_flags)
-                        .wrap_err("couldn't get cargo metadata")?;
+                let metadata = crate::metadata::metadata(
+                    &Default::default(),
+                    Some(manifest_path),
+                    cargo_flags,
+                )
+                .wrap_err("couldn't get cargo metadata")?;
                 crate::metadata::validate(Some(manifest_path), &metadata)?;
                 let manifest_path = crate::manifest::manifest_path(&metadata, None)
                     .wrap_err("Couldn't get manifest path")?;

cargo-pgrx/src/command/package.rs

@@ -46,8 +46,7 @@ pub(crate) struct Package {
     pub(crate) features: clap_cargo::Features,
     #[clap(long)]
     pub(crate) target: Option<String>,
-    /// Extra flag forwarded verbatim to every `cargo` invocation (repeatable).
-    /// e.g. `--cargo=--config=child/.cargo/config.toml --cargo=--offline`
+    /// Extra cargo flags forwarded to every `cargo` invocation. Repeatable and on whitespace: `--cargo=--config=foo` or `--cargo "--offline --frozen"`.
     #[clap(long = "cargo", value_name = "FLAG")]
     pub(crate) cargo: Vec<String>,
     #[clap(from_global, action = ArgAction::Count)]

cargo-pgrx/src/command/run.rs

@@ -56,8 +56,7 @@ pub(crate) struct Run {
     install_only: bool,
     #[clap(long)]
     valgrind: bool,
-    /// Extra flag forwarded verbatim to every `cargo` invocation (repeatable).
-    /// e.g. `--cargo=--config=child/.cargo/config.toml --cargo=--offline`
+    /// Extra cargo flags forwarded to every `cargo` invocation. Repeatable and split on whitespace: `--cargo=--config=foo` or `--cargo "--offline --frozen"`.
     #[clap(long = "cargo", value_name = "FLAG")]
     cargo: Vec<String>,
 }

cargo-pgrx/src/command/test.rs

@@ -49,8 +49,7 @@ pub(crate) struct Test {
     pgdata: Option<PathBuf>,
     #[clap(flatten)]
     features: clap_cargo::Features,
-    /// Extra flag forwarded verbatim to every `cargo` invocation (repeatable).
-    /// e.g. `--cargo=--config=child/.cargo/config.toml --cargo=--offline`
+    /// Extra cargo flags forwarded to every `cargo` invocation. Repeatable and split on whitespace: `--cargo=--config=foo` or `--cargo "--offline --frozen"`.
     #[clap(long = "cargo", value_name = "FLAG")]
     cargo: Vec<String>,
     #[clap(from_global, action = clap::ArgAction::Count)]
@@ -176,7 +175,7 @@ pub fn test_extension(
         .env("PGRX_NO_SCHEMA", if no_schema { "true" } else { "false" });
 
     // The `--cargo` passthrough reaches every cargo invocation; here, `cargo test`.
-    for arg in cargo_flags {
+    for arg in crate::metadata::split_cargo_flags(cargo_flags) {
         command.arg(arg);
     }
 

cargo-pgrx/src/metadata.rs

@@ -26,14 +26,24 @@ pub fn metadata(
 
     // The `--cargo` passthrough must reach `cargo metadata` — the first cargo
     // invocation, where config issues (private registries, etc.) fail first.
+    let cargo_flags = split_cargo_flags(cargo_flags);
     if !cargo_flags.is_empty() {
-        metadata_command.other_options(cargo_flags.to_vec());
+        metadata_command.other_options(cargo_flags);
     }
     features.forward_metadata(&mut metadata_command);
     let metadata = metadata_command.exec()?;
     Ok(metadata)
 }
 
+/// Split each `--cargo` value on whitespace, so both the single-token form
+/// (`--cargo=--config=child/.cargo/config.toml`) and the quoted-string form
+/// (`--cargo "--config foo"`) work, forwarding each token as one argv element.
+// ponytail: naive whitespace split, same as PGRX_BUILD_FLAGS in cargo.rs; a
+// config path containing spaces needs the single-token `--cargo=--config=x` form.
+pub fn split_cargo_flags(cargo_flags: &[String]) -> Vec<String> {
+    cargo_flags.iter().flat_map(|f| f.split_ascii_whitespace().map(str::to_owned)).collect()
+}
+
 #[tracing::instrument(level = "error", skip_all)]
 pub fn validate(path: Option<&Path>, metadata: &Metadata) -> eyre::Result<()> {
     let cargo_pgrx_version = env!("CARGO_PKG_VERSION");
@@ -98,3 +108,27 @@ cargo-pgrx and pgrx library versions must be identical.
 
     Ok(())
 }
+
+#[cfg(test)]
+mod tests {
+    use super::split_cargo_flags;
+
+    #[test]
+    fn split_cargo_flags_handles_both_forms() {
+        // single-token form is passed through untouched
+        assert_eq!(
+            split_cargo_flags(&["--config=child/.cargo/config.toml".into()]),
+            vec!["--config=child/.cargo/config.toml"]
+        );
+        // quoted-string form is split into separate argv tokens
+        assert_eq!(
+            split_cargo_flags(&["--config foo --offline".into()]),
+            vec!["--config", "foo", "--offline"]
+        );
+        // repeatable `--cargo` and whitespace-splitting compose
+        assert_eq!(
+            split_cargo_flags(&["--offline".into(), "--config foo".into()]),
+            vec!["--offline", "--config", "foo"]
+        );
+    }
+}

cargo-pgrx/tests/cargo_flags_passthrough.rs

@@ -65,6 +65,19 @@ fn cargo_flag_reaches_cargo_metadata() {
     );
 }
 
+/// The quoted-string form (`--cargo "--flag-a --flag-b"`) is split on
+/// whitespace: two valid cargo flags packed into one `--cargo` value get past
+/// `cargo metadata`. Without splitting, the mashed-together token would be an
+/// unknown argument and `cargo metadata` would reject it.
+#[test]
+fn cargo_flag_whitespace_string_is_split() {
+    let out = install_with("split", &["--cargo", "--offline --color=never"], &[]);
+    assert!(
+        !out.contains("couldn't get cargo metadata"),
+        "two valid flags in one --cargo value should be split and pass `cargo metadata`, got:\n{out}"
+    );
+}
+
 /// `PGRX_BUILD_FLAGS` must stay build-only: a bogus flag there must NOT break
 /// `cargo metadata` (the run gets past it and fails later, for other reasons).
 /// This is the property that keeps `--cargo` distinct from build-only flags.