From: <rsbecker@nexbridge.com>
To: "'Kyle Lippincott'" <spectral@google.com>
Cc: "'David Aguilar'" <davvid@gmail.com>,
"'Junio C Hamano'" <gitster@pobox.com>, <git@vger.kernel.org>,
"'Ezekiel Newren'" <ezekielnewren@gmail.com>,
"'Josh Steadmon'" <steadmon@google.com>,
"'Calvin Wan'" <calvinwan@google.com>
Subject: RE: [PATCH] Makefile: build libgit-rs and libgit-sys serially
Date: Tue, 26 Aug 2025 14:05:16 -0400 [thread overview]
Message-ID: <014f01dc16b3$fdcc8b70$f965a250$@nexbridge.com> (raw)
In-Reply-To: <CAO_smViKBsfzvqAfu587V_FXp=OPgu3yeO-P8qw19jgwizuUhg@mail.gmail.com>
On August 26, 2025 2:03 PM, Kyle Lippincott wrote:
>On Tue, Aug 26, 2025 at 10:53 AM <rsbecker@nexbridge.com> wrote:
>>
>> On August 26, 2025 1:45 PM, Kyle Lippincott wrote:
>> >On Tue, Aug 26, 2025 at 9:04 AM David Aguilar <davvid@gmail.com> wrote:
>> >>
>> >> The "cargo build" invocations in contrib/ cannot be run in parallel.
>> >>
>> >> "make -JN" with INCLUDE_LIBGIT_RS enabled causes cargo lock
>> >> warnings and can trigger ld errors during the build.
>> >>
>> >> The build errors are caused by two inner "make" invocations getting
>> >> triggered concurrently: once inside of libgit-sys and another
>> >> inside of libgit-rs.
>> >>
>> >> Signed-off-by: David Aguilar <davvid@gmail.com>
>> >> ---
>> >> Makefile | 2 +-
>> >> 1 file changed, 1 insertion(+), 1 deletion(-)
>> >>
>> >> diff --git a/Makefile b/Makefile
>> >> index 29a53520fd..286d3ba3b2 100644
>> >> --- a/Makefile
>> >> +++ b/Makefile
>> >> @@ -3989,7 +3989,7 @@ libgit-sys libgit-rs:
>> >> cargo build \
>> >> )
>> >> ifdef INCLUDE_LIBGIT_RS
>> >> -all:: libgit-sys libgit-rs
>> >> +all:: libgit-sys .WAIT libgit-rs
>> >
>> >I'm not familiar enough with make or with rust, but do we need to
>> >depend on both of these here? Wouldn't it be sufficient to say
>> >libgit-rs depends on libgit-sys, and only explicitly depend on libgit-rs in `all::`?
>>
>> Not all platforms can build libgit-rs, so inserting it into as a
>> required component is not a particularly friendly idea.
>>
>
>That's not what I was suggesting. This is already in an `ifdef`, and the line I was
>quoting was changing `all:: libgit-sys libgit-rs` to
>`all:: libgit-sys .WAIT libgit-rs`. I'm wondering if we can instead split the `libgit-sys
>libgit-rs:` from a few lines earlier into `libgit-sys:` and `libgit-rs: libgit-sys` and then
>change this all line to `all:: libgit-rs` (still behind `ifdef INCLUDE_LIBGIT_RS`).
Thank you. I appreciate this and the clarity it provides.
Randall
prev parent reply other threads:[~2025-08-26 18:05 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-08-26 16:04 [PATCH] Makefile: build libgit-rs and libgit-sys serially David Aguilar
2025-08-26 16:46 ` Junio C Hamano
2025-08-26 23:35 ` [PATCH v2] " David Aguilar
2025-08-26 23:48 ` Kyle Lippincott
2025-08-27 0:01 ` Junio C Hamano
2025-08-26 17:44 ` [PATCH] " Kyle Lippincott
2025-08-26 17:53 ` rsbecker
2025-08-26 18:02 ` Kyle Lippincott
2025-08-26 18:05 ` rsbecker [this message]
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='014f01dc16b3$fdcc8b70$f965a250$@nexbridge.com' \
--to=rsbecker@nexbridge.com \
--cc=calvinwan@google.com \
--cc=davvid@gmail.com \
--cc=ezekielnewren@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=spectral@google.com \
--cc=steadmon@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;
as well as URLs for NNTP newsgroup(s).