openvsa.inc has no OPENVSA_LOG_DIR target, and considering that $(SOURCE_DIR)/$(OPENVSA_TARBALL) creates the LOG_DIR we don't need to add dependency on the LOG_DIR.
Thanks.
Index: geodevsa/openvsa.inc =================================================================== --- geodevsa/openvsa.inc (revision 255) +++ geodevsa/openvsa.inc (working copy) @@ -17,7 +17,7 @@ OPENVSA_FETCH_LOG=$(OPENVSA_LOG_DIR)/fetch.log endif
-$(SOURCE_DIR)/$(OPENVSA_TARBALL): | $(OPENVSA_LOG_DIR) +$(SOURCE_DIR)/$(OPENVSA_TARBALL): @ mkdir -p $(SOURCE_DIR)/openvsa @ mkdir -p $(OPENVSA_LOG_DIR) @ echo "Fetching openvsa..."
On Wed, Dec 10, 2008 at 09:50:12AM -0700, Abhishek Kulkarni wrote:
openvsa.inc has no OPENVSA_LOG_DIR target, and considering that $(SOURCE_DIR)/$(OPENVSA_TARBALL) creates the LOG_DIR we don't need to add dependency on the LOG_DIR.
Actually, let's do it the other way around and remove that
@ mkdir -p $(OPENVSA_LOG_DIR)
while keeping the dependency on $(OPENVSA_LOG_DIR).
That's cleaner. OK?
Thanks, Ward.
On Wed, Dec 10, 2008 at 10:00 AM, Ward Vandewege ward@gnu.org wrote:
On Wed, Dec 10, 2008 at 09:50:12AM -0700, Abhishek Kulkarni wrote:
openvsa.inc has no OPENVSA_LOG_DIR target, and considering that $(SOURCE_DIR)/$(OPENVSA_TARBALL) creates the LOG_DIR we don't need to
add
dependency on the LOG_DIR.
Actually, let's do it the other way around and remove that
@ mkdir -p $(OPENVSA_LOG_DIR)
while keeping the dependency on $(OPENVSA_LOG_DIR).
That's cleaner. OK?
Sure, makes sense.
Also, most of the main targets already have a dependency on the LOG_DIR targets. Is there a specific reason to add it to the TARBALL_TARGET too?
Corrected patch below:
Thanks Abhishek
Index: geodevsa/openvsa.inc =================================================================== --- geodevsa/openvsa.inc (revision 255) +++ geodevsa/openvsa.inc (working copy) @@ -19,7 +19,6 @@
$(SOURCE_DIR)/$(OPENVSA_TARBALL): | $(OPENVSA_LOG_DIR) @ mkdir -p $(SOURCE_DIR)/openvsa - @ mkdir -p $(OPENVSA_LOG_DIR) @ echo "Fetching openvsa..." @ $(BIN_DIR)/fetchsvn.sh $(OPENVSA_URL) $(SOURCE_DIR)/openvsa \ $(OPENVSA_TAG) $@ > $(OPENVSA_FETCH_LOG) 2>&1 @@ -31,7 +30,6 @@ @ touch $@
$(OPENVSA_SRC_DIR)/vsa_lx.bin: $(OPENVSA_STAMP_DIR)/.unpacked - @ mkdir -p $(OPENVSA_LOG_DIR) @ echo "Building openvsa..." @(unset LDFLAGS; $(MAKE) -C $(OPENVSA_SRC_DIR) \ > $(OPENVSA_BUILD_LOG) 2>&1) @@ -40,6 +38,9 @@ @ mkdir -p $(shell dirname $(GEODE_UNCOMPRESSED_VSA)) @ cp $< $@
+$(OPENVESA_STAMP_DIR) $(OPENVESA_LOG_DIR): + @ mkdir -p $@ + openvsa: $(OPENVSA_SRC_DIR)/vsa_lx.bin
openvsa-clean:
Thanks, Ward.
-- Ward Vandewege ward@fsf.org Free Software Foundation - Senior Systems Administrator
On Wed, Dec 10, 2008 at 10:06:24AM -0700, Abhishek Kulkarni wrote:
On Wed, Dec 10, 2008 at 10:00 AM, Ward Vandewege ward@gnu.org wrote:
On Wed, Dec 10, 2008 at 09:50:12AM -0700, Abhishek Kulkarni wrote: > openvsa.inc has no OPENVSA_LOG_DIR target, and considering that > $(SOURCE_DIR)/$(OPENVSA_TARBALL) creates the LOG_DIR we don't need to add > dependency on the LOG_DIR. Actually, let's do it the other way around and remove that @ mkdir -p $(OPENVSA_LOG_DIR) while keeping the dependency on $(OPENVSA_LOG_DIR). That's cleaner. OK?
Sure, makes sense.
Also, most of the main targets already have a dependency on the LOG_DIR targets. Is there a specific reason to add it to the TARBALL_TARGET too?
Yes:
@ $(BIN_DIR)/fetchsvn.sh $(OPENVSA_URL) $(SOURCE_DIR)/openvsa \ $(OPENVSA_TAG) $@ > $(OPENVSA_FETCH_LOG) 2>&1
$(OPENVSA_FETCH_LOG) is made up from $(OPENVSA_LOG_DIR), so it has to exist before this target is called.
Corrected patch below:
Can you sign it off as per the guidelines at
http://www.coreboot.org/Development_Guidelines#Sign-off_Procedure
In fact that entire page is good reading :)
Thanks, Ward.
On Wed, Dec 10, 2008 at 10:34 AM, Ward Vandewege ward@gnu.org wrote:
On Wed, Dec 10, 2008 at 10:06:24AM -0700, Abhishek Kulkarni wrote:
On Wed, Dec 10, 2008 at 10:00 AM, Ward Vandewege ward@gnu.org
wrote:
On Wed, Dec 10, 2008 at 09:50:12AM -0700, Abhishek Kulkarni wrote: > openvsa.inc has no OPENVSA_LOG_DIR target, and considering that > $(SOURCE_DIR)/$(OPENVSA_TARBALL) creates the LOG_DIR we don't
need
to add > dependency on the LOG_DIR. Actually, let's do it the other way around and remove that @ mkdir -p $(OPENVSA_LOG_DIR) while keeping the dependency on $(OPENVSA_LOG_DIR). That's cleaner. OK?
Sure, makes sense.
Also, most of the main targets already have a dependency on the
LOG_DIR
targets. Is there a specific reason to add it to the TARBALL_TARGET
too?
Yes:
@ $(BIN_DIR)/fetchsvn.sh $(OPENVSA_URL) $(SOURCE_DIR)/openvsa \ $(OPENVSA_TAG) $@ > $(OPENVSA_FETCH_LOG) 2>&1
$(OPENVSA_FETCH_LOG) is made up from $(OPENVSA_LOG_DIR), so it has to exist before this target is called.
Corrected patch below:
Can you sign it off as per the guidelines at
http://www.coreboot.org/Development_Guidelines#Sign-off_Procedure
In fact that entire page is good reading :)
Yes, indeed :) I figured I don't have commit access, so I skipped the SOB part. Here it goes again:
Minor buildrom Makefile fix for geodvesa/openvesa.inc. This patch adds a OPENVSA_LOG_DIR target and removes the redundant calls which create OPENVESA_LOG_DIR.
Signed-off-by: Abhishek Kulkarni adkulkar@cs.indiana.edu
----
Index: geodevsa/openvsa.inc =================================================================== --- geodevsa/openvsa.inc (revision 255) +++ geodevsa/openvsa.inc (working copy) @@ -19,7 +19,6 @@
$(SOURCE_DIR)/$(OPENVSA_TARBALL): | $(OPENVSA_LOG_DIR) @ mkdir -p $(SOURCE_DIR)/openvsa - @ mkdir -p $(OPENVSA_LOG_DIR) @ echo "Fetching openvsa..." @ $(BIN_DIR)/fetchsvn.sh $(OPENVSA_URL) $(SOURCE_DIR)/openvsa \ $(OPENVSA_TAG) $@ > $(OPENVSA_FETCH_LOG) 2>&1 @@ -31,7 +30,6 @@ @ touch $@
$(OPENVSA_SRC_DIR)/vsa_lx.bin: $(OPENVSA_STAMP_DIR)/.unpacked - @ mkdir -p $(OPENVSA_LOG_DIR) @ echo "Building openvsa..." @(unset LDFLAGS; $(MAKE) -C $(OPENVSA_SRC_DIR) \ > $(OPENVSA_BUILD_LOG) 2>&1) @@ -40,6 +38,9 @@ @ mkdir -p $(shell dirname $(GEODE_UNCOMPRESSED_VSA)) @ cp $< $@
+$(OPENVESA_STAMP_DIR) $(OPENVESA_LOG_DIR): + @ mkdir -p $@ + openvsa: $(OPENVSA_SRC_DIR)/vsa_lx.bin
openvsa-clean:
Thanks, Ward.
-- Ward Vandewege ward@fsf.org Free Software Foundation - Senior Systems Administrator