From: Patrick Steinhardt <ps@pks.im>
To: Andy Koppe <andy.koppe@gmail.com>
Cc: git@vger.kernel.org,
"brian m . carlson " <sandals@crustytoothpaste.net>,
"Shardul Natu " <snatu@google.com>
Subject: Re: [PATCH] Makefile: pin cargo --target-dir to ./target
Date: Mon, 19 Oct 2026 08:45:54 +0200 [thread overview]
Message-ID: <atW8otvbLGDtDBG9@pks.im> (raw)
In-Reply-To: <20261018205305.2634339-1-andy.koppe@gmail.com>
On Sun, Oct 18, 2026 at 09:53:05PM +0100, Andy Koppe wrote:
> The Makefile assumes that cargo places build artifacts under ./target.
> That assumption breaks if the user has a custom target dir setting, e.g.
> in their ~/.cargo/config.toml file or in the CARGO_TARGET_DIR env var.
> This results in a link error due to failing to find the gitcore library
> built with cargo.
>
> To fix this, pass '--target-dir target' to the cargo commands that build
> gitcore. Add it directly to the recipes rather than to CARGO_ARGS, so
> that it isn't lost if CARGO_ARGS is overridden.
>
> Leave the contrib/libgit-{sys,rs} targets alone, as nothing depends on
> the location of their build output.
Right. I sometimes wonder whether we maybe want to start merging this
into the main Rust library. But that's certainly a topic for another
day.
> The Meson build of gitcore already passes --target-dir explicitly.
Indeed it does.
> diff --git a/Makefile b/Makefile
> index c649c93c51..aea99a3639 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -3039,7 +3039,7 @@ $(LIB_FILE): $(LIB_OBJS)
> ifndef NO_RUST
> ifeq ($(RUST_TARGETS),)
> $(RUST_LIB): Cargo.toml $(RUST_SOURCES) $(LIB_FILE)
> - $(QUIET_CARGO)cargo build $(CARGO_ARGS)
> + $(QUIET_CARGO)cargo build $(CARGO_ARGS) --target-dir target
> else
> ifneq ($(words $(RUST_TARGETS)),1)
> ifneq ($(uname_S),Darwin)
> @@ -3049,7 +3049,7 @@ endif
>
> RUST_MEMBER_LIBS = $(foreach target,$(RUST_TARGETS),target/$(target)/$(RUST_BUILD_CONFIG)/$(RUST_LIB_NAME))
> $(RUST_MEMBER_LIBS): target/%/$(RUST_BUILD_CONFIG)/$(RUST_LIB_NAME): Cargo.toml $(RUST_SOURCES) $(LIB_FILE)
> - $(QUIET_CARGO)cargo build $(CARGO_ARGS) --target $*
> + $(QUIET_CARGO)cargo build $(CARGO_ARGS) --target-dir target --target $*
>
> $(RUST_LIB): $(RUST_MEMBER_LIBS)
> $(call mkdir_p_parent_template)
Makes sense. I would have preferred to use the "--target-dir=target"
form, but that really only is a nit that does not warrant a reroll in my
opinion.
So overall this patch looks good to me, thanks!
Patrick
next prev parent reply other threads:[~2026-10-19 6:46 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-10-18 20:53 [PATCH] Makefile: pin cargo --target-dir to ./target Andy Koppe
2026-10-19 6:45 ` Patrick Steinhardt [this message]
2026-10-19 16:07 ` Junio C Hamano
2026-10-19 19:43 ` Andy Koppe
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=atW8otvbLGDtDBG9@pks.im \
--to=ps@pks.im \
--cc=andy.koppe@gmail.com \
--cc=git@vger.kernel.org \
--cc=sandals@crustytoothpaste.net \
--cc=snatu@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox