diff mbox

[1/2] external/pflash: Add quiet rules and fix race

Message ID 1448508139-19206-1-git-send-email-joel@jms.id.au
State Accepted
Headers show

Commit Message

Joel Stanley Nov. 26, 2015, 3:22 a.m. UTC
Adding quiet rules to make our output a bit cleaner. Building now looks
like this:

$ make
    LN  libflash
    LN  common
    LN  ccan
    CC  pflash.o
    CC  version.o
    LD  common-arch_flash.o
    CC  pflash

You can see the full build ouput by doing a "make V=1".

By doing this, we build fractionally faster, exposing arace condition
between running the make_version.sh script and the link existing for it.
As we run it when creating the variable, there is no way to ensure it
exists first.

Solved this by not creating the symlink and simply running
make_version.sh from the root.

Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 external/common/rules.mk | 26 ++++++++++++++++++--------
 external/pflash/Makefile | 23 +++++++++++++++--------
 external/pflash/rules.mk | 13 +++++++------
 3 files changed, 40 insertions(+), 22 deletions(-)

Comments

Joel Stanley Nov. 26, 2015, 3:32 a.m. UTC | #1
Hi Stewart,

On Thu, Nov 26, 2015 at 1:52 PM, Joel Stanley <joel@jms.id.au> wrote:
>  external/common/rules.mk | 26 ++++++++++++++++++--------
>  external/pflash/Makefile | 23 +++++++++++++++--------
>  external/pflash/rules.mk | 13 +++++++------
>  3 files changed, 40 insertions(+), 22 deletions(-)

This one depends on "[PATCH 1/4] pflash: Remove ast.h".

Cheers,

Joel
Stewart Smith Nov. 26, 2015, 11:08 p.m. UTC | #2
Joel Stanley <joel@jms.id.au> writes:
> Adding quiet rules to make our output a bit cleaner. Building now looks
> like this:
>
> $ make
>     LN  libflash
>     LN  common
>     LN  ccan
>     CC  pflash.o
>     CC  version.o
>     LD  common-arch_flash.o
>     CC  pflash
>
> You can see the full build ouput by doing a "make V=1".
>
> By doing this, we build fractionally faster, exposing arace condition
> between running the make_version.sh script and the link existing for it.
> As we run it when creating the variable, there is no way to ensure it
> exists first.
>
> Solved this by not creating the symlink and simply running
> make_version.sh from the root.
>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
>  external/common/rules.mk | 26 ++++++++++++++++++--------
>  external/pflash/Makefile | 23 +++++++++++++++--------
>  external/pflash/rules.mk | 13 +++++++------

Series merged as of 34901e08da043bf464775292c0c292e52e217121
diff mbox

Patch

diff --git a/external/common/rules.mk b/external/common/rules.mk
index 7ebf48531e19..4d94fe8f96f7 100644
--- a/external/common/rules.mk
+++ b/external/common/rules.mk
@@ -17,19 +17,29 @@  endif
 endif
 endif
 
-.PHONY: arch_links
-arch_links:
-	ln -sf ../../hw/ast-bmc/ast-sf-ctrl.c common/ast-sf-ctrl.c
-	ln -sf ../../include/ast.h common/ast.h
-	ln -sf arch_flash_$(arch)_io.h common/io.h
+
+# Arch links are like this so we can have dependencies work (so that we don't
+# run the rule when the links exist), pretty build output (knowing the target
+# name) and a list of the files so we can clean them up.
+ARCH_LINKS = common/ast-sf-ctrl.c common/ast.h common/io.h
+
+arch_links: $(ARCH_LINKS)
+common/ast.h : ../../include/ast.h | common
+	$(Q_LN)ln -sf ../../include/ast.h common/ast.h
+
+common/io.h : ../common/arch_flash_$(arch)_io.h | common
+	$(Q_LN)ln -sf arch_flash_$(arch)_io.h common/io.h
+
+common/ast-sf-ctrl.c : ../../hw/ast-bmc/ast-sf-ctrl.c | common
+	$(Q_LN)ln -sf ../../hw/ast-bmc/ast-sf-ctrl.c common/ast-sf-ctrl.c
 
 .PHONY: arch_clean
 arch_clean:
-	rm -rf $(ARCH_OBJS)
+	rm -rf $(ARCH_OBJS) $(ARCH_LINKS)
 
 $(ARCH_OBJS): common-%.o: common/%.c
-	$(CROSS_COMPILE)gcc $(CFLAGS) -c $< -o $@
+	$(Q_CC)$(CROSS_COMPILE)gcc $(CFLAGS) -c $< -o $@
 
 common-arch_flash.o: $(ARCH_OBJS)
-	$(CROSS_COMPILE)ld $(LDFLAGS) -r $(ARCH_OBJS) -o $@
+	$(Q_LD)$(CROSS_COMPILE)ld $(LDFLAGS) -r $(ARCH_OBJS) -o $@
 
diff --git a/external/pflash/Makefile b/external/pflash/Makefile
index f0c3773c5d0d..e2aa33db18e2 100644
--- a/external/pflash/Makefile
+++ b/external/pflash/Makefile
@@ -1,3 +1,11 @@ 
+# Use make V=1 for a verbose build.
+ifndef V
+        Q_CC=	@echo '    CC ' $@;
+        Q_LD=	@echo '    LD ' $@;
+        Q_LN=   @echo '    LN ' $@;
+        Q_MKDIR=@echo ' MKDIR ' $@;
+endif
+
 include rules.mk
 GET_ARCH = ../../external/common/get_arch.sh
 include ../../external/common/rules.mk
@@ -5,19 +13,16 @@  include ../../external/common/rules.mk
 all: $(EXE)
 
 .PHONY: links
-links: libflash ccan common make_version.sh
+links: libflash ccan common
 
 libflash:
-	ln -sf ../../libflash .
+	$(Q_LN)ln -sf ../../libflash .
 
 ccan:
-	ln -sf ../../ccan .
+	$(Q_LN)ln -sf ../../ccan .
 
 common:
-	ln -sf ../common .
-
-make_version.sh:
-	ln -sf ../../make_version.sh
+	$(Q_LN)ln -sf ../common .
 
 $(OBJS): | links arch_links
 
@@ -38,9 +43,11 @@  dist: links .version
 		../pflash/.version ../pflash/make_version.sh \
 		../pflash/common/*
 
+.PHONY: clean
 clean: arch_clean
 	rm -f $(OBJS) $(EXE) *.o *.d libflash/test/test_flash libflash/test/*.o
+.PHONY: distclean
 distclean: clean
 	rm -f *.c~ *.h~ *.sh~ Makefile~ config.mk~ libflash/*.c~ libflash/*.h~
 	rm -f libflash ccan .version .version.tmp
-	rm -f common io.h make_version.sh
+	rm -f common io.h
diff --git a/external/pflash/rules.mk b/external/pflash/rules.mk
index 96f6a4f6e809..f49e438cedf2 100644
--- a/external/pflash/rules.mk
+++ b/external/pflash/rules.mk
@@ -2,16 +2,17 @@ 
 
 override CFLAGS  += -O2 -Wall -I.
 OBJS    = pflash.o progress.o version.o
-LIBFLASH_OBJS += libflash-libflash.o libflash-libffs.o libflash-ecc.o libflash-blocklevel.o libflash-file.o
+LIBFLASH_OBJS += libflash-libflash.o libflash-libffs.o libflash-ecc.o \
+		 libflash-blocklevel.o libflash-file.o
 OBJS	+= $(LIBFLASH_OBJS)
 OBJS	+= common-arch_flash.o
 EXE     = pflash
 
 CC	= $(CROSS_COMPILE)gcc
 
-PFLASH_VERSION ?= $(shell ./make_version.sh $(EXE))
+PFLASH_VERSION ?= $(shell ../../make_version.sh $(EXE))
 
-version.c: make_version.sh .version
+version.c: .version
 	@(if [ "a$(PFLASH_VERSION)" = "a" ]; then \
 	echo "#error You need to set PFLASH_VERSION environment variable" > $@ ;\
 	else \
@@ -19,11 +20,11 @@  version.c: make_version.sh .version
 	fi) > $@
 
 %.o : %.c
-	$(CC) $(CFLAGS) -c $< -o $@
+	$(Q_CC)$(CC) $(CFLAGS) -c $< -o $@
 
 $(LIBFLASH_OBJS): libflash-%.o : libflash/%.c
-	$(CC) $(CFLAGS) -c $< -o $@
+	$(Q_CC)$(CC) $(CFLAGS) -c $< -o $@
 
 $(EXE): $(OBJS)
-	$(CC) $(CFLAGS) $^ -lrt -o $@
+	$(Q_CC)$(CC) $(CFLAGS) $^ -lrt -o $@