Makefile: link osxkeychain helper against Rust by kiranani · Pull Request #2288 · git/git

kiranani · 2026-05-05T05:11:16Z

cc: "Kristoffer Haugsbakk" kristofferhaugsbakk@fastmail.com
cc: Shnatu snatu@google.com
cc: Koji Nakamaru koji.nakamaru@gree.net

gitgitgadget-git · 2026-05-05T05:11:48Z

Welcome to GitGitGadget

Hi @kiranani, and welcome to GitGitGadget, the GitHub App to send patch series to the Git mailing list from GitHub Pull Requests.

Please make sure that either:

Your Pull Request has a good description, if it consists of multiple commits, as it will be used as cover letter.
Your Pull Request description is empty, if it consists of a single commit, as the commit message should be descriptive enough by itself.

You can CC potential reviewers by adding a footer to the PR description with the following syntax:

CC: Revi Ewer <revi.ewer@example.com>, Ill Takalook <ill.takalook@example.net>

NOTE: DO NOT copy/paste your CC list from a previous GGG PR's description,
because it will result in a malformed CC list on the mailing list. See
example.

Also, it is a good idea to review the commit messages one last time, as the Git project expects them in a quite specific form:

the lines should not exceed 76 columns,
the first line should be like a header and typically start with a prefix like "tests:" or "revisions:" to state which subsystem the change is about, and
the commit messages' body should be describing the "why?" of the change.
Finally, the commit messages should end in a Signed-off-by: line matching the commits' author.

It is in general a good idea to await the automated test ("Checks") in this Pull Request before contributing the patches, e.g. to avoid trivial issues such as unportable code.

Contributing the patches

Before you can contribute the patches, your GitHub username needs to be added to the list of permitted users. Any already-permitted user can do that, by adding a comment to your PR of the form /allow. A good way to find other contributors is to locate recent pull requests where someone has been /allowed:

Search: is:pr is:open "/allow"

Both the person who commented /allow and the PR author are able to /allow you.

An alternative is the channel #git-devel on the Libera Chat IRC network:

<newcontributor> I've just created my first PR, could someone please /allow me? https://github.com/gitgitgadget/git/pull/12345
<veteran> newcontributor: it is done
<newcontributor> thanks!

Once on the list of permitted usernames, you can contribute the patches to the Git mailing list by adding a PR comment /submit.

If you want to see what email(s) would be sent for a /submit request, add a PR comment /preview to have the email(s) sent to you. You must have a public GitHub email address for this. Note that any reviewers CC'd via the list in the PR description will not actually be sent emails.

After you submit, GitGitGadget will respond with another comment that contains the link to the cover letter mail in the Git mailing list archive. Please make sure to monitor the discussion in that thread and to address comments and suggestions (while the comments and suggestions will be mirrored into the PR by GitGitGadget, you will still want to reply via mail).

If you do not want to subscribe to the Git mailing list just to be able to respond to a mail, you can download the mbox from the Git mailing list archive (click the (raw) link), then import it into your mail program. If you use GMail, you can do this via:

curl -g --user "<EMailAddress>:<Password>" \
    --url "imaps://imap.gmail.com/INBOX" -T /path/to/raw.txt

To iterate on your change, i.e. send a revised patch or patch series, you will first want to (force-)push to the same branch. You probably also want to modify your Pull Request description (or title). It is a good idea to summarize the revision by adding something like this to the cover letter (read: by editing the first comment on the PR, i.e. the PR description):

Changes since v1:
- Fixed a typo in the commit message (found by ...)
- Added a code comment to ... as suggested by ...
...

To send a new iteration, just add another PR comment with the contents: /submit.

Need help?

New contributors who want advice are encouraged to join git-mentoring@googlegroups.com, where volunteers who regularly contribute to Git are willing to answer newbie questions, give advice, or otherwise provide mentoring to interested contributors. You must join in order to post or view messages, but anyone can join.

You may also be able to find help in real time in the developer IRC channel, #git-devel on Libera Chat. Remember that IRC does not support offline messaging, so if you send someone a private message and log out, they cannot respond to you. The scrollback of #git-devel is archived, though.

dscho · 2026-05-05T09:36:09Z

/allow

gitgitgadget-git · 2026-05-05T09:36:53Z

User kiranani is now allowed to use GitGitGadget.

WARNING: kiranani has no public email address set on GitHub; GitGitGadget needs an email address to Cc: you on your contribution, so that you receive any feedback on the Git mailing list. Go to https://github.com/settings/profile to make your preferred email public to let GitGitGadget know which email address to use.

kiranani · 2026-05-05T17:25:07Z

/submit

gitgitgadget-git · 2026-05-05T17:26:34Z

Submitted as pull.2288.git.git.1778001976709.gitgitgadget@gmail.com

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git/ pr-git-2288/kiranani/next-v1

To fetch this version to local tag pr-git-2288/kiranani/next-v1:

git fetch --no-tags https://github.com/gitgitgadget/git/ tag pr-git-2288/kiranani/next-v1

WARNING: kiranani has no public email address set on GitHub; GitGitGadget needs an email address to Cc: you on your contribution, so that you receive any feedback on the Git mailing list. Go to https://github.com/settings/profile to make your preferred email public to let GitGitGadget know which email address to use.

gitgitgadget-git · 2026-05-05T19:54:58Z

"Kristoffer Haugsbakk" wrote on the Git mailing list (how to reply to this email):

On Tue, May 5, 2026, at 19:26, Shardul Natu via GitGitGadget wrote:
> From: Shnatu <snatu@google.com>
>
> 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.

This pararagraph is meaningless to those outside internal.

>
> Signed-off-by: Shnatu <snatu@google.com>
> ---
>[snip]

gitgitgadget-git · 2026-05-05T19:55:00Z

User "Kristoffer Haugsbakk" <kristofferhaugsbakk@fastmail.com> has been added to the cc: list.

gitgitgadget-git · 2026-05-06T23:54:52Z

There are issues in commit fde105f:
Makefile: link osxkeychain & support universal Rust

Commit not signed off
Lines in the body of the commit messages should be wrapped between 60 and 76 characters.
Indented lines, and lines without whitespace, are exempt

kiranani · 2026-05-06T23:57:44Z

"Kristoffer Haugsbakk" wrote on the Git mailing list (how to reply to this email):

On Tue, May 5, 2026, at 19:26, Shardul Natu via GitGitGadget wrote:
> From: Shnatu <snatu@google.com>
>
> 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.

This pararagraph is meaningless to those outside internal.

>
> Signed-off-by: Shnatu <snatu@google.com>
> ---
>[snip]

Updated the description to remove that paragraph too

dscho · 2026-05-07T00:02:24Z

"Kristoffer Haugsbakk" wrote on the Git mailing list (how to reply to this email):

On Tue, May 5, 2026, at 19:26, Shardul Natu via GitGitGadget wrote:
> From: Shnatu <snatu@google.com>
>
> 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.

This pararagraph is meaningless to those outside internal.

>
> Signed-off-by: Shnatu <snatu@google.com>
> ---
>[snip]

Updated the description to remove that paragraph too

@kiranani Please note that your reply is not going to reach the Git mailing list because the mirroring performed by GitGitGadget is only one way from the Git mailing list to this PR. You will have to follow the instructions under the label "reply to this" to make your answer reach Kristoffer.

gitgitgadget-git · 2026-05-07T01:18:41Z

Shnatu wrote on the Git mailing list (how to reply to this email):

I have remove the Google specific paragraph, in addition to updating the description

gitgitgadget-git · 2026-05-07T01:18:43Z

User Shnatu <snatu@google.com> has been added to the cc: list.

gitgitgadget-git · 2026-05-08T03:52:06Z

Junio C Hamano wrote on the Git mailing list (how to reply to this email):

"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

gitgitgadget-git · 2026-05-08T10:13:00Z

Koji Nakamaru wrote on the Git mailing list (how to reply to this email):

On Fri, May 8, 2026 at 11:54 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> "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

How about simply wrapping the RUST_LIB-related sections in ifndef
NO_RUST, as shown below? This way, we can avoid defining
RUST_LIBS.

diff --git a/Makefile b/Makefile
index f86173f93a..daa1691950 100644
--- a/Makefile
+++ b/Makefile
@@ -947,11 +947,13 @@ else
 RUST_TARGET_DIR = target/release
 endif

+ifndef NO_RUST
 ifeq ($(uname_S),Windows)
 RUST_LIB = $(RUST_TARGET_DIR)/gitcore.lib
 else
 RUST_LIB = $(RUST_TARGET_DIR)/libgitcore.a
 endif
+endif

 GITLIBS = common-main.o $(LIB_FILE)
 EXTLIBS =
@@ -3027,11 +3029,13 @@ scalar$X: scalar.o GIT-LDFLAGS $(GITLIBS)
 $(LIB_FILE): $(LIB_OBJS)
        $(QUIET_AR)$(RM) $@ && $(AR) $(ARFLAGS) $@ $^

+ifndef NO_RUST
 $(RUST_LIB): Cargo.toml $(RUST_SOURCES) $(LIB_FILE)
        $(QUIET_CARGO)cargo build $(CARGO_ARGS)

 .PHONY: rust
 rust: $(RUST_LIB)
+endif

 export DEFAULT_EDITOR DEFAULT_PAGER

@@ -4082,9 +4086,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_LIB) GIT-LDFLAGS
        $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) \
-               $(filter %.o,$^) $(LIB_FILE) $(EXTLIBS) -framework
Security -framework CoreFoundation
+               $(filter %.o,$^) $(LIB_FILE) $(RUST_LIB) $(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) $<

gitgitgadget-git · 2026-05-08T10:13:03Z

User Koji Nakamaru <koji.nakamaru@gree.net> has been added to the cc: list.

kiranani · 2026-05-08T17:46:52Z

Thank you for the suggestion! This is indeed a much cleaner approach. By wrapping `RUST_LIB` and the rust targets in `ifndef NO_RUST`, we can link `git-credential-osxkeychain` directly against `$(RUST_LIB)` without needing to introduce an intermediate `RUST_LIBS` variable. When `NO_RUST` is defined, `$(RUST_LIB)` evaluates to empty, and Make naturally links it as a pure C binary without any Rust dependencies. To integrate this with the universal build support (`RUST_TARGETS`/`lipo`) introduced in this PR, I have updated the changes to: - Wrap the `RUST_LIB` definition block (which resolves target-specific paths for universal builds) in ifndef `NO_RUST`. - Wrap the entire universal compilation and lipo combining block in ifndef `NO_RUST`. - Remove the `RUST_LIBS` helper variable and use `$(RUST_LIB)` directly in `git-credential-osxkeychain`. On Fri, May 8, 2026 at 3:13 AM gitgitgadget-git[bot] < ***@***.***> wrote:

…

*gitgitgadget-git[bot]* left a comment (git/git#2288) <#2288 (comment)> *Koji Nakamaru* wrote on the Git mailing list ***@***.***> (how to reply to this email <https://gitgitgadget.github.io/reply-to-this> ): On Fri, May 8, 2026 at 11:54 AM Junio C Hamano ***@***.***> wrote:>> "Shardul Natu via GitGitGadget" ***@***.***> writes:>> > From: Shnatu ***@***.***>>> 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 ***@***.***>> > ---> > 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: #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: 4f69b47 How about simply wrapping the RUST_LIB-related sections in ifndef NO_RUST, as shown below? This way, we can avoid defining RUST_LIBS. diff --git a/Makefile b/Makefile index f86173f93a..daa1691950 100644--- a/Makefile +++ b/Makefile @@ -947,11 +947,13 @@ else RUST_TARGET_DIR = target/release endif +ifndef NO_RUST ifeq ($(uname_S),Windows) RUST_LIB = $(RUST_TARGET_DIR)/gitcore.lib else RUST_LIB = $(RUST_TARGET_DIR)/libgitcore.a endif +endif GITLIBS = common-main.o $(LIB_FILE) EXTLIBS = @@ -3027,11 +3029,13 @@ scalar$X: scalar.o GIT-LDFLAGS $(GITLIBS) $(LIB_FILE): $(LIB_OBJS) $(QUIET_AR)$(RM) $@ && $(AR) $(ARFLAGS) $@ $^ +ifndef NO_RUST $(RUST_LIB): Cargo.toml $(RUST_SOURCES) $(LIB_FILE) $(QUIET_CARGO)cargo build $(CARGO_ARGS) .PHONY: rust rust: $(RUST_LIB) +endif export DEFAULT_EDITOR DEFAULT_PAGER @@ -4082,9 +4086,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_LIB) GIT-LDFLAGS $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) \ - $(filter %.o,$^) $(LIB_FILE) $(EXTLIBS) -framework Security -framework CoreFoundation + $(filter %.o,$^) $(LIB_FILE) $(RUST_LIB) $(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) $< — Reply to this email directly, view it on GitHub <#2288 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AKLJEVXT4YS6K2JYCE3UCKD4ZWXMJAVCNFSM6AAAAACYRCBHB6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHM2DIMBVGU4TOMRQGA> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. You are receiving this because you were mentioned.Message ID: ***@***.***>

gitgitgadget-git · 2026-05-08T17:51:46Z

Shnatu wrote on the Git mailing list (how to reply to this email):

Thank you for the suggestion! This is indeed a much cleaner approach.

By wrapping `RUST_LIB` and the rust targets in `ifndef NO_RUST`, we can link `git-credential-osxkeychain` directly against `$(RUST_LIB)` without needing to introduce an intermediate `RUST_LIBS` variable. When `NO_RUST` is defined, `$(RUST_LIB)` evaluates to empty, and Make naturally links it as a pure C binary without any Rust dependencies.

To integrate this with the universal build support (`RUST_TARGETS`/`lipo`) introduced in this PR, I have updated the changes to:

- Wrap the `RUST_LIB` definition block (which resolves target-specific paths for universal builds) in ifndef `NO_RUST`.
- Wrap the entire universal compilation and lipo combining block in ifndef `NO_RUST`.
- Remove the `RUST_LIBS` helper variable and use `$(RUST_LIB)` directly in `git-credential-osxkeychain`.

gitgitgadget-git · 2026-05-13T21:28:35Z

Invalid author email in f7d9a4b: "43422294+kiranani@users.noreply.github.com"

gitgitgadget-git · 2026-05-13T21:28:36Z

There is a merge commit in this Pull Request:

f7d9a4b003ad181dec830c3248b358b16407a701

Please rebase the branch and force-push.

When Rust is enabled, ensure that the git-credential-osxkeychain helper is linked with the necessary Rust libraries. Also, introduce native support for macOS Universal Binaries (multi-architecture builds) in the Git build system by allowing the user to specify a list of target triples in the RUST_TARGETS environment variable. To implement this cleanly without complex shell scripting in recipes: 1. We introduce a declarative Make pattern rule (target/%/...) to compile each target-specific library slice (e.g., target/aarch64-apple-darwin/...). 2. We update the $(RUST_LIB) recipe to depend on the list of compiled target-specific member libraries ($(RUST_MEMBER_LIBS)). 3. On macOS, if multiple targets are specified, we use lipo to combine them into a single Universal static library at target/release/libgitcore.a. 4. If only one target is specified, we copy it to the standard path. 5. We enforce that building for multiple targets requires macOS (as lipo is only available there), raising a clear make error on other platforms. This is a highly elegant and native Makefile solution that avoids complex shell scripting in recipes and fully supports macOS Universal Binaries. Signed-off-by: Shardul Natu <snatu@google.com>

KojiNakamaru · 2026-05-21T00:12:17Z

@kiranani You will need to resubmit this for the next iteration.

github-project-automation Bot added this to Op-Amp Frequency Response and Applications Suite May 5, 2026

github-project-automation Bot moved this to Todo in Op-Amp Frequency Response and Applications Suite May 5, 2026

gitgitgadget-git Bot added the new user label May 5, 2026

kiranani force-pushed the next branch from 57046d2 to fde105f Compare May 6, 2026 23:54

kiranani force-pushed the next branch from fde105f to 5fddcfe Compare May 6, 2026 23:59

kiranani force-pushed the next branch 2 times, most recently from a9ce4c1 to 4c8e9ab Compare May 8, 2026 17:37

kiranani force-pushed the next branch from f7d9a4b to 7491bbc Compare May 13, 2026 22:11

Conversation

kiranani commented May 5, 2026 • edited by gitgitgadget-git Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gitgitgadget-git Bot commented May 5, 2026

Welcome to GitGitGadget

Contributing the patches

Need help?

Uh oh!

dscho commented May 5, 2026

Uh oh!

gitgitgadget-git Bot commented May 5, 2026

Uh oh!

kiranani commented May 5, 2026

Uh oh!

gitgitgadget-git Bot commented May 5, 2026

Uh oh!

gitgitgadget-git Bot commented May 5, 2026

Uh oh!

gitgitgadget-git Bot commented May 5, 2026

Uh oh!

gitgitgadget-git Bot commented May 6, 2026

Uh oh!

kiranani commented May 6, 2026

Uh oh!

dscho commented May 7, 2026

Uh oh!

gitgitgadget-git Bot commented May 7, 2026

Uh oh!

gitgitgadget-git Bot commented May 7, 2026

Uh oh!

gitgitgadget-git Bot commented May 8, 2026

Uh oh!

gitgitgadget-git Bot commented May 8, 2026

Uh oh!

gitgitgadget-git Bot commented May 8, 2026

Uh oh!

kiranani commented May 8, 2026 via email

Uh oh!

gitgitgadget-git Bot commented May 8, 2026

Uh oh!

gitgitgadget-git Bot commented May 13, 2026

Uh oh!

gitgitgadget-git Bot commented May 13, 2026

Uh oh!

KojiNakamaru commented May 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

kiranani commented May 5, 2026 •

edited by gitgitgadget-git Bot

Loading