Add --cargo passthrough support for cargo commands#2343
Conversation
|
What's the use-case for this? I'd like to see a real-world example of where this is needed. It's curious to me that after all these years this is now a concern. EDIT: I see the original bug report, of course. @hakashya can you provide more details about your project layout? |
|
Hi @eeeebbbbrrrr I don't know what's the use case from @hakashya, but below is the used case which I noticed that why I create this PR, please kindly check Real use-case: private-registry crate in a monorepo submoduleAn extension is a submodule of a larger monorepo and depends on an internal crate from a private registry. Per cargo convention, that registry is declared in the submodule's own .cargo/config.toml (so the submodule stays buildable standalone, and to avoid clashing with other submodules in the shared root config). Cargo resolves .cargo/config.toml from the CWD upward, so building from the monorepo root inherits the parent config which doesn't know the private registry — and cargo pgrx install fails at its first step, cargo metadata: A runnable end-to-end repro lives at repro/install-config-2135 , we could refer more detail from this commit (39a2fa0) cargo build solves this with --config; this PR gives cargo pgrx install (and package/run) the same flag, forwarded to the cargo invocations install makes: cargo metadata, cargo rustc, the get_version metadata, and the schema-gen cargo run. Output: Generally speaking, When the root project's |
|
I guess what I don't like about this is that it's plumbing through specialized support for just one Should we take a step back and try to generalize this so that What I don't want to end up with is a situation where I say that this particular approach is fine and in a week/month someone PRs another and then another and then another and we've ended up making a mess of pgrx's primary user-facing tool. If this is a general problem class, perhaps we ought to solve it generally. |
39a2fa0 to
cce3fc4
Compare
|
Understood and I agree with your point, and I also noticed there is an Env What: a new env var I also change the repro example to demonstrate real use case which we need |
cce3fc4 to
8f10674
Compare
|
I'm not sure I like having another envvar for this. Can we not support something like I think this will be easier for robots and humans and also easier for cargo-pgrx to validate, if it needs to. |
bf72856 to
34c780b
Compare
--config passthrough support for cargo commands--cargo passthrough support for cargo commands
34c780b to
26d849a
Compare
|
Ok, I changed mechanism to parameter flag as you mentioned, please let me know if you have any question about that. --cargo on install / package / run / test, forwarded to every cargo call those commands make (cargo metadata, cargo rustc, get_version, cargo test). cargo metadata matters most — it's the first call and where the private-registry case fails. Both forms you suggested work:
Supported commands
|
…tralfoundation#2135 Monorepo/submodule layouts fail because cargo resolves .cargo/config.toml from the CWD upward, so building from the parent repo picks up the wrong config and cargo metadata (the first cargo call) dies — e.g. a private registry declared only in the submodule's config. Add a repeatable --cargo <FLAG> that is forwarded verbatim to every cargo invocation cargo-pgrx makes (cargo metadata, cargo rustc, get_version, cargo test), so global flags like --config/--offline/--locked reach all of them: cargo pgrx install --cargo=--config=child/.cargo/config.toml
26d849a to
3f4a931
Compare
|
But I like where this is going! |
f80bc8a to
4471b67
Compare
|
Good call regress builds too, so I've added --cargo to it and wired it through both cargo paths it hits.
|
|
Hi @eeeebbbbrrrr hope you are doing well, please let me know if you have any question regarding this PR, thanks. |
Motivation
Fixes #2135.
In monorepo/submodule layouts, cargo resolves
.cargo/config.tomlby walking up from the CWD, socargo pgrx installpicks up the parent repo's config and fails.cargo buildsolves this with--config, butcargo pgrx installhad no equivalent. (PGRX_BUILD_FLAGSonly reaches the build step, not thecargo metadatacall that runs first — so source/registry/proxy config still breaks it.)How to fix
Add a repeatable
--config <KEY=VALUE | path>flag tocargo pgrx install, mirroring cargo's own flag, and forward it to every cargo call install makes:cargo metadata(viaMetadataCommand::other_options),cargo rustc, and the schema generator'scargo run.Results
cargo pgrx installnow accepts a repeatable--config <KEY=VALUE | path>, forwarded to every cargo call install, run.Example: a dependency from a private registry
A child extension depends on a crate from an internal registry, defined only in the child's
.cargo/config.toml:Before
building from the parent repo picks up the parent's config (which lacks [registries.mycorp]), and install dies in the very first step, cargo metadata:
(PGRX_BUILD_FLAGS can't help here — it only reaches the build step, never cargo metadata.)
After
point install at the right config:
cargo metadata now resolves internal-crate against the correct registry and install proceeds.