[OpenWrt-Devel,RESEND,tools] toolchain: ensure tools are built and staged before preparing toolchain
diff mbox

Message ID 1418122055-5917-1-git-send-email-john@szakmeister.net
State Changes Requested
Headers show

Commit Message

John Szakmeister Dec. 9, 2014, 10:47 a.m. UTC
This fixes an issue where the toolchain/prepare step could run, but some
of the necessary host tools might be missing.

Signed-off-by: John Szakmeister <john@szakmeister.net>
---
This is a resend of a patch I sent earlier
(https://lists.openwrt.org/pipermail/openwrt-devel/2014-October/028422.html).
I didn't receive any feedback, but it has enabled me to build
correctly with a parallel build.

 Makefile           | 2 +-
 toolchain/Makefile | 4 ++++
 2 files changed, 5 insertions(+), 1 deletion(-)

Comments

Felix Fietkau Dec. 12, 2014, 5:03 p.m. UTC | #1
On 2014-12-09 11:47, John Szakmeister wrote:
> This fixes an issue where the toolchain/prepare step could run, but some
> of the necessary host tools might be missing.
> 
> Signed-off-by: John Szakmeister <john@szakmeister.net>
> ---
> This is a resend of a patch I sent earlier
> (https://lists.openwrt.org/pipermail/openwrt-devel/2014-October/028422.html).
> I didn't receive any feedback, but it has enabled me to build
> correctly with a parallel build.
> 
>  Makefile           | 2 +-
>  toolchain/Makefile | 4 ++++
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/Makefile b/Makefile
> index 91b6946..edb75fd 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -38,7 +38,7 @@ else
>    include tools/Makefile
>    include toolchain/Makefile
>  
> -$(toolchain/stamp-install): $(tools/stamp-install)
> +$(toolchain/stamp-prepare): $(tools/stamp-install)
>  $(target/stamp-compile): $(toolchain/stamp-install) $(tools/stamp-install) $(BUILD_DIR)/.prepared
>  $(package/stamp-compile): $(target/stamp-compile) $(package/stamp-cleanup)
>  $(package/stamp-install): $(package/stamp-compile)
> diff --git a/toolchain/Makefile b/toolchain/Makefile
> index 36c6ed3..b260a36 100644
> --- a/toolchain/Makefile
> +++ b/toolchain/Makefile
> @@ -82,6 +82,10 @@ ifndef DUMP_TARGET_DB
>  $(TOOLCHAIN_DIR)/stamp/.gcc-initial_installed:
>  endif
>  
> +$(eval $(call stampfile,$(curdir),toolchain,prepare))
>  $(eval $(call stampfile,$(curdir),toolchain,install,$(TOOLCHAIN_DIR)/stamp/.gcc-initial_installed,,$(TOOLCHAIN_DIR)))
> +
> +$($(curdir)/stamp-install): $($(curdir)/stamp-prepare)
This doesn't look right to me, I don't think we should add the
toolchain/prepare step as an intermediate target for the regular build
process.
How about just adding this line to Makefile and leaving out the rest:

toolchain/prepare: $(tools/stamp-install)

- Felix
John Szakmeister Dec. 13, 2014, 9:28 a.m. UTC | #2
On Fri, Dec 12, 2014 at 12:03 PM, Felix Fietkau <nbd@openwrt.org> wrote:
> On 2014-12-09 11:47, John Szakmeister wrote:
>> This fixes an issue where the toolchain/prepare step could run, but some
>> of the necessary host tools might be missing.
>>
>> Signed-off-by: John Szakmeister <john@szakmeister.net>
>> ---
>> This is a resend of a patch I sent earlier
>> (https://lists.openwrt.org/pipermail/openwrt-devel/2014-October/028422.html).
>> I didn't receive any feedback, but it has enabled me to build
>> correctly with a parallel build.
>>
>>  Makefile           | 2 +-
>>  toolchain/Makefile | 4 ++++
>>  2 files changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/Makefile b/Makefile
>> index 91b6946..edb75fd 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -38,7 +38,7 @@ else
>>    include tools/Makefile
>>    include toolchain/Makefile
>>
>> -$(toolchain/stamp-install): $(tools/stamp-install)
>> +$(toolchain/stamp-prepare): $(tools/stamp-install)
>>  $(target/stamp-compile): $(toolchain/stamp-install) $(tools/stamp-install) $(BUILD_DIR)/.prepared
>>  $(package/stamp-compile): $(target/stamp-compile) $(package/stamp-cleanup)
>>  $(package/stamp-install): $(package/stamp-compile)
>> diff --git a/toolchain/Makefile b/toolchain/Makefile
>> index 36c6ed3..b260a36 100644
>> --- a/toolchain/Makefile
>> +++ b/toolchain/Makefile
>> @@ -82,6 +82,10 @@ ifndef DUMP_TARGET_DB
>>  $(TOOLCHAIN_DIR)/stamp/.gcc-initial_installed:
>>  endif
>>
>> +$(eval $(call stampfile,$(curdir),toolchain,prepare))
>>  $(eval $(call stampfile,$(curdir),toolchain,install,$(TOOLCHAIN_DIR)/stamp/.gcc-initial_installed,,$(TOOLCHAIN_DIR)))
>> +
>> +$($(curdir)/stamp-install): $($(curdir)/stamp-prepare)
> This doesn't look right to me, I don't think we should add the
> toolchain/prepare step as an intermediate target for the regular build
> process.
> How about just adding this line to Makefile and leaving out the rest:
>
> toolchain/prepare: $(tools/stamp-install)

Who will depend on the toolchain/prepare to make sure that it
happens before toolchain/install?  I couldn't find this
happening automatically in the Makefiles, that's why I went
through the work of setting up the intermediate target.

Trying your version locally fails.

-John

Patch
diff mbox

diff --git a/Makefile b/Makefile
index 91b6946..edb75fd 100644
--- a/Makefile
+++ b/Makefile
@@ -38,7 +38,7 @@  else
   include tools/Makefile
   include toolchain/Makefile
 
-$(toolchain/stamp-install): $(tools/stamp-install)
+$(toolchain/stamp-prepare): $(tools/stamp-install)
 $(target/stamp-compile): $(toolchain/stamp-install) $(tools/stamp-install) $(BUILD_DIR)/.prepared
 $(package/stamp-compile): $(target/stamp-compile) $(package/stamp-cleanup)
 $(package/stamp-install): $(package/stamp-compile)
diff --git a/toolchain/Makefile b/toolchain/Makefile
index 36c6ed3..b260a36 100644
--- a/toolchain/Makefile
+++ b/toolchain/Makefile
@@ -82,6 +82,10 @@  ifndef DUMP_TARGET_DB
 $(TOOLCHAIN_DIR)/stamp/.gcc-initial_installed:
 endif
 
+$(eval $(call stampfile,$(curdir),toolchain,prepare))
 $(eval $(call stampfile,$(curdir),toolchain,install,$(TOOLCHAIN_DIR)/stamp/.gcc-initial_installed,,$(TOOLCHAIN_DIR)))
+
+$($(curdir)/stamp-install): $($(curdir)/stamp-prepare)
+
 $(eval $(call subdir,$(curdir)))