git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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


      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).