All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Shardul Natu via GitGitGadget" <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org,  Shnatu <snatu@google.com>,
	"brian m. carlson" <sandals@crustytoothpaste.net>,
	Koji Nakamaru <koji.nakamaru@gree.net>
Subject: Re: [PATCH] Makefile: link osxkeychain helper against Rust
Date: Fri, 08 May 2026 11:54:54 +0900	[thread overview]
Message-ID: <xmqqlddufw5d.fsf@gitster.g> (raw)
In-Reply-To: <pull.2288.git.git.1778001976709.gitgitgadget@gmail.com> (Shardul Natu via GitGitGadget's message of "Tue, 05 May 2026 17:26:16 +0000")

"Shardul Natu via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Shnatu <snatu@google.com>

If your name is "Shardul Natu", we'd prefer (not 'require', but
'prefer') that the patches authored by you also identify with that
name, both on "From:" and "Signed-off-by:"..

> When Rust is enabled, ensure that the git-credential-osxkeychain
> helper is linked with the necessary Rust libraries.
>
> Introduce the RUST_LIBS variable inside ifndef NO_RUST block
> to hold the Rust library dependency, and use it in the helper's
> build target. This cleanly handles cases where Rust is disabled,
> making it a no-op and avoiding any build failures on systems
> without Cargo.
>
> This addresses reviewer feedback from internal CL 910223487
> by simplifying the variables and avoiding confusing "LINK"
> terminology.
>
> Signed-off-by: Shnatu <snatu@google.com>
> ---
>     Makefile: link osxkeychain helper against Rust

Thanks.  I've added to CC: a few folks who may be more clueful in
the affected area than I am.  It somehow feels strange that we have
to have RUST_LIB and RUST_LIBS separately, and apparently with the
new definition the latter is expected to be a superset of the
former, and it is unclear what are the things that should be added
to the latter without getting added to the former.

> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2288%2Fkiranani%2Fnext-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2288/kiranani/next-v1
> Pull-Request: https://github.com/git/git/pull/2288
>
>  Makefile | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index f86173f93a..a17dca22b1 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1593,6 +1593,7 @@ ALL_LDFLAGS = $(LDFLAGS) $(LDFLAGS_APPEND)
>  ifndef NO_RUST
>  BASIC_CFLAGS += -DWITH_RUST
>  GITLIBS += $(RUST_LIB)
> +RUST_LIBS = $(RUST_LIB)
>  ifeq ($(uname_S),Windows)
>  EXTLIBS += -luserenv
>  endif
> @@ -4082,9 +4083,9 @@ $(LIBGIT_HIDDEN_EXPORT): $(LIBGIT_PARTIAL_EXPORT)
>  contrib/libgit-sys/libgitpub.a: $(LIBGIT_HIDDEN_EXPORT)
>  	$(AR) $(ARFLAGS) $@ $^
>  
> -contrib/credential/osxkeychain/git-credential-osxkeychain: contrib/credential/osxkeychain/git-credential-osxkeychain.o $(LIB_FILE) GIT-LDFLAGS
> +contrib/credential/osxkeychain/git-credential-osxkeychain: contrib/credential/osxkeychain/git-credential-osxkeychain.o $(LIB_FILE) $(RUST_LIBS) GIT-LDFLAGS
>  	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) \
> -		$(filter %.o,$^) $(LIB_FILE) $(EXTLIBS) -framework Security -framework CoreFoundation
> +		$(filter %.o,$^) $(LIB_FILE) $(RUST_LIBS) $(EXTLIBS) -framework Security -framework CoreFoundation
>  
>  contrib/credential/osxkeychain/git-credential-osxkeychain.o: contrib/credential/osxkeychain/git-credential-osxkeychain.c GIT-CFLAGS
>  	$(QUIET_LINK)$(CC) -o $@ -c $(dep_args) $(compdb_args) $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) $<
>
> base-commit: 4f69b47b940100b02630f745a52f9d9850f122b2

  parent reply	other threads:[~2026-05-08  2:54 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-05 17:26 [PATCH] Makefile: link osxkeychain helper against Rust Shardul Natu via GitGitGadget
2026-05-05 19:08 ` Kristoffer Haugsbakk
2026-05-07  0:39   ` Shnatu
2026-05-08  2:54 ` Junio C Hamano [this message]
2026-05-08  9:33   ` Koji Nakamaru
2026-05-08 17:44     ` Shnatu

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=xmqqlddufw5d.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=koji.nakamaru@gree.net \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.