# [PATCH] Makefile: build libgit-rs and libgit-sys serially 9 messages from 2025-08-26 to 2025-08-27. Participants: David Aguilar, Junio C Hamano, Kyle Lippincott, rsbecker@nexbridge.com. Thread: https://gitlist.dev/t/64038 ## David Aguilar, 2025-08-26 16:04 Subject: [PATCH] Makefile: build libgit-rs and libgit-sys serially Message-ID: <20250826160437.2539113-1-davvid@gmail.com> URL: https://gitlist.dev/e/20250826160437.2539113-1-davvid%40gmail.com ``` 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 --- 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 endif LIBGIT_PUB_OBJS += contrib/libgit-sys/public_symbol_export.o -- 2.50.0.7.gec2f25360c ``` ## Junio C Hamano, 2025-08-26 16:46 Subject: Re: [PATCH] Makefile: build libgit-rs and libgit-sys serially Message-ID: URL: https://gitlist.dev/e/xmqq7byqkp3p.fsf%40gitster.g In-Reply-To: <20250826160437.2539113-1-davvid@gmail.com> ``` David Aguilar writes: > 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 > --- > Makefile | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Don't we need a similar change to t/Makefile, or "cargo test" does fine while "cargo build" cannot be run in parallel? > > 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 > endif > > LIBGIT_PUB_OBJS += contrib/libgit-sys/public_symbol_export.o ``` ## Kyle Lippincott, 2025-08-26 17:44 Subject: Re: [PATCH] Makefile: build libgit-rs and libgit-sys serially Message-ID: URL: https://gitlist.dev/e/CAO_smVjviMdpZyHFp4zJc62DJYAZxLAc5yw68C3U%2Bc5wbwRziA%40mail.gmail.com In-Reply-To: <20250826160437.2539113-1-davvid@gmail.com> ``` On Tue, Aug 26, 2025 at 9:04 AM David Aguilar 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 > --- > 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::`? > endif > > LIBGIT_PUB_OBJS += contrib/libgit-sys/public_symbol_export.o > -- > 2.50.0.7.gec2f25360c > ``` ## rsbecker@nexbridge.com, 2025-08-26 17:53 Subject: RE: [PATCH] Makefile: build libgit-rs and libgit-sys serially Message-ID: <014b01dc16b2$4a1dd0d0$de597270$@nexbridge.com> URL: https://gitlist.dev/e/014b01dc16b2%244a1dd0d0%24de597270%24%40nexbridge.com In-Reply-To: ``` On August 26, 2025 1:45 PM, Kyle Lippincott wrote: >On Tue, Aug 26, 2025 at 9:04 AM David Aguilar 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 >> --- >> 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. ``` ## Kyle Lippincott, 2025-08-26 18:02 Subject: Re: [PATCH] Makefile: build libgit-rs and libgit-sys serially Message-ID: URL: https://gitlist.dev/e/CAO_smViKBsfzvqAfu587V_FXp%3DOPgu3yeO-P8qw19jgwizuUhg%40mail.gmail.com In-Reply-To: <014b01dc16b2$4a1dd0d0$de597270$@nexbridge.com> ``` On Tue, Aug 26, 2025 at 10:53 AM wrote: > > On August 26, 2025 1:45 PM, Kyle Lippincott wrote: > >On Tue, Aug 26, 2025 at 9:04 AM David Aguilar 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 > >> --- > >> 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`). ``` ## rsbecker@nexbridge.com, 2025-08-26 18:05 Subject: RE: [PATCH] Makefile: build libgit-rs and libgit-sys serially Message-ID: <014f01dc16b3$fdcc8b70$f965a250$@nexbridge.com> URL: https://gitlist.dev/e/014f01dc16b3%24fdcc8b70%24f965a250%24%40nexbridge.com In-Reply-To: ``` On August 26, 2025 2:03 PM, Kyle Lippincott wrote: >On Tue, Aug 26, 2025 at 10:53 AM wrote: >> >> On August 26, 2025 1:45 PM, Kyle Lippincott wrote: >> >On Tue, Aug 26, 2025 at 9:04 AM David Aguilar 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 >> >> --- >> >> 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 ``` ## David Aguilar, 2025-08-26 23:35 Subject: [PATCH v2] Makefile: build libgit-rs and libgit-sys serially Message-ID: <20250826233525.2635432-1-davvid@gmail.com> URL: https://gitlist.dev/e/20250826233525.2635432-1-davvid%40gmail.com In-Reply-To: ``` "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. Make libgit-rs depend on libgit-sys so that "make" prevents them from running concurrently. Apply the same logic to the test invocations. Use cargo's "--manifest-path" option instead of "cd" in the recipes. Signed-off-by: David Aguilar --- Differences since v0: * The targets have been split apart into separate targets so that the libgit-rs targets can be made to depend on the libgit-sys targets. * cargo build/test --manifest-path is being used to simplify the build recipe by eliminating the "cd" step, which would have been duplicated in the split-out target. * t/Makefile has been updated to apply the same logic. Makefile | 11 +++++------ t/Makefile | 14 ++++---------- 2 files changed, 9 insertions(+), 16 deletions(-) diff --git a/Makefile b/Makefile index 29a53520fd..539e6907b4 100644 --- a/Makefile +++ b/Makefile @@ -3983,13 +3983,12 @@ unit-tests: $(UNIT_TEST_PROGS) $(CLAR_TEST_PROG) t/helper/test-tool$X $(MAKE) -C t/ unit-tests .PHONY: libgit-sys libgit-rs -libgit-sys libgit-rs: - $(QUIET)(\ - cd contrib/$@ && \ - cargo build \ - ) +libgit-sys: + $(QUIET)cargo build --manifest-path contrib/libgit-sys/Cargo.toml +libgit-rs: libgit-sys + $(QUIET)cargo build --manifest-path contrib/libgit-rs/Cargo.toml ifdef INCLUDE_LIBGIT_RS -all:: libgit-sys libgit-rs +all:: libgit-rs endif LIBGIT_PUB_OBJS += contrib/libgit-sys/public_symbol_export.o diff --git a/t/Makefile b/t/Makefile index 791e0a0978..29dd226c7d 100644 --- a/t/Makefile +++ b/t/Makefile @@ -190,15 +190,9 @@ perf: .PHONY: libgit-sys-test libgit-rs-test libgit-sys-test: - $(QUIET)(\ - cd ../contrib/libgit-sys && \ - cargo test \ - ) -libgit-rs-test: - $(QUIET)(\ - cd ../contrib/libgit-rs && \ - cargo test \ - ) + $(QUIET)cargo test --manifest-path ../contrib/libgit-sys/Cargo.toml +libgit-rs-test: libgit-sys-test + $(QUIET)cargo test --manifest-path ../contrib/libgit-rs/Cargo.toml ifdef INCLUDE_LIBGIT_RS -all:: libgit-sys-test libgit-rs-test +all:: libgit-rs-test endif -- 2.50.0.7.ge90cf88798 ``` ## Kyle Lippincott, 2025-08-26 23:48 Subject: Re: [PATCH v2] Makefile: build libgit-rs and libgit-sys serially Message-ID: URL: https://gitlist.dev/e/CAO_smViX%2BEVyq5AzO3dwfcBGdenuZ1w89ksse%3D6MXYv8xi%2Bq1g%40mail.gmail.com In-Reply-To: <20250826233525.2635432-1-davvid@gmail.com> ``` On Tue, Aug 26, 2025 at 4:35 PM David Aguilar wrote: > > "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. > > Make libgit-rs depend on libgit-sys so that "make" prevents them > from running concurrently. Apply the same logic to the test invocations. > Use cargo's "--manifest-path" option instead of "cd" in the recipes. > > Signed-off-by: David Aguilar > --- > > Differences since v0: > > * The targets have been split apart into > separate targets so that the libgit-rs targets can be made to > depend on the libgit-sys targets. > > * cargo build/test --manifest-path is being used to simplify > the build recipe by eliminating the "cd" step, which would > have been duplicated in the split-out target. > > * t/Makefile has been updated to apply the same logic. > > Makefile | 11 +++++------ > t/Makefile | 14 ++++---------- > 2 files changed, 9 insertions(+), 16 deletions(-) > > diff --git a/Makefile b/Makefile > index 29a53520fd..539e6907b4 100644 > --- a/Makefile > +++ b/Makefile > @@ -3983,13 +3983,12 @@ unit-tests: $(UNIT_TEST_PROGS) $(CLAR_TEST_PROG) t/helper/test-tool$X > $(MAKE) -C t/ unit-tests > > .PHONY: libgit-sys libgit-rs > -libgit-sys libgit-rs: > - $(QUIET)(\ > - cd contrib/$@ && \ > - cargo build \ > - ) > +libgit-sys: > + $(QUIET)cargo build --manifest-path contrib/libgit-sys/Cargo.toml > +libgit-rs: libgit-sys > + $(QUIET)cargo build --manifest-path contrib/libgit-rs/Cargo.toml > ifdef INCLUDE_LIBGIT_RS > -all:: libgit-sys libgit-rs > +all:: libgit-rs > endif > > LIBGIT_PUB_OBJS += contrib/libgit-sys/public_symbol_export.o > diff --git a/t/Makefile b/t/Makefile > index 791e0a0978..29dd226c7d 100644 > --- a/t/Makefile > +++ b/t/Makefile > @@ -190,15 +190,9 @@ perf: > > .PHONY: libgit-sys-test libgit-rs-test > libgit-sys-test: > - $(QUIET)(\ > - cd ../contrib/libgit-sys && \ > - cargo test \ > - ) > -libgit-rs-test: > - $(QUIET)(\ > - cd ../contrib/libgit-rs && \ > - cargo test \ > - ) > + $(QUIET)cargo test --manifest-path ../contrib/libgit-sys/Cargo.toml > +libgit-rs-test: libgit-sys-test > + $(QUIET)cargo test --manifest-path ../contrib/libgit-rs/Cargo.toml > ifdef INCLUDE_LIBGIT_RS > -all:: libgit-sys-test libgit-rs-test > +all:: libgit-rs-test > endif > -- > 2.50.0.7.ge90cf88798 This version looks good to me, thanks! ``` ## Junio C Hamano, 2025-08-27 00:01 Subject: Re: [PATCH v2] Makefile: build libgit-rs and libgit-sys serially Message-ID: URL: https://gitlist.dev/e/xmqqh5xtfx9p.fsf%40gitster.g In-Reply-To: ``` Kyle Lippincott writes: > On Tue, Aug 26, 2025 at 4:35 PM David Aguilar wrote: >> >> "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. >> >> Make libgit-rs depend on libgit-sys so that "make" prevents them >> from running concurrently. Apply the same logic to the test invocations. >> Use cargo's "--manifest-path" option instead of "cd" in the recipes. >> .... >> + $(QUIET)cargo test --manifest-path ../contrib/libgit-sys/Cargo.toml >> +libgit-rs-test: libgit-sys-test >> + $(QUIET)cargo test --manifest-path ../contrib/libgit-rs/Cargo.toml >> ifdef INCLUDE_LIBGIT_RS >> -all:: libgit-sys-test libgit-rs-test >> +all:: libgit-rs-test >> endif >> -- >> 2.50.0.7.ge90cf88798 > > This version looks good to me, thanks! Thanks, both. Will apply. ```