Skip to contentSkip to conversation

Replace DL_DIR refs in the GIT/SVN/HG download

PATCH New Danomi Manchego 3 participants 4 messages Aug 27, 2012 – Sep 10, 2012

lore
Share & tools
lore

Danomi Manchego

author
#0 lore raw

PATCH 1/1 Replace DL_DIR refs in the GIT/SVN/HG download

If DL_DIR is set to a relative path, then the download helpers for GIT, SVN, and HG fail, because they reference DL_DIR or $(PKG)_DL_DIR from within the DL_DIR directory. (But the relative path is relative to TOPDIR.) All of these cases can be avoided, since the location of the cloned repo is always $(DL_DIR)/$($(PKG)_BASE_NAME), which can be used to make the helpers simpler, as well as more friendly to the relative DL_DIR definition.

Signed-off-by Danomi Manchego <[email protected]>
---
 package/pkg-download.mk |   16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)
package/pkg-download.mk +8 -8
index 9e98581..a7cef0e 100644
@@ -68,11 +68,11 @@ define DOWNLOAD_GIT
 	test -e $(DL_DIR)/$($(PKG)_SOURCE) || \
 	(pushd $(DL_DIR) > /dev/null && \
 	$(GIT) clone --bare $($(PKG)_SITE) $($(PKG)_BASE_NAME) && \
-	pushd $($(PKG)_BASE_NAME) > /dev/null && \
+	cd $($(PKG)_BASE_NAME) && \
 	$(GIT) archive --format=tar --prefix=$($(PKG)_BASE_NAME)/ $($(PKG)_DL_VERSION) | \
-		gzip -c > $(DL_DIR)/$($(PKG)_SOURCE) && \
-	popd > /dev/null && \
-	rm -rf $($(PKG)_DL_DIR) && \
+		gzip -c > ../$($(PKG)_SOURCE) && \
+	cd .. && \
+	rm -rf $($(PKG)_BASE_NAME) && \
 	popd > /dev/null)
 endef
 

@@ -104,9 +104,9 @@ endef
 define DOWNLOAD_SVN
 	test -e $(DL_DIR)/$($(PKG)_SOURCE) || \
 	(pushd $(DL_DIR) > /dev/null && \
-	$(SVN) export -r $($(PKG)_DL_VERSION) $($(PKG)_SITE) $($(PKG)_DL_DIR) && \
+	$(SVN) export -r $($(PKG)_DL_VERSION) $($(PKG)_SITE) $($(PKG)_BASE_NAME) && \
 	$(TAR) czf $($(PKG)_SOURCE) $($(PKG)_BASE_NAME)/ && \
-	rm -rf $($(PKG)_DL_DIR) && \
+	rm -rf $($(PKG)_BASE_NAME) && \
 	popd > /dev/null)
 endef
 

@@ -140,8 +140,8 @@ define DOWNLOAD_HG
 	(pushd $(DL_DIR) > /dev/null && \
 	$(HG) clone --noupdate --rev $($(PKG)_DL_VERSION) $($(PKG)_SITE) $($(PKG)_BASE_NAME) && \
 	$(HG) archive --repository $($(PKG)_BASE_NAME) --type tgz --prefix $($(PKG)_BASE_NAME)/ \
-	              --rev $($(PKG)_DL_VERSION) $(DL_DIR)/$($(PKG)_SOURCE) && \
-	rm -rf $($(PKG)_DL_DIR) && \
+	              --rev $($(PKG)_DL_VERSION) ./$($(PKG)_SOURCE) && \
+	rm -rf $($(PKG)_BASE_NAME) && \
 	popd > /dev/null)
 endef
Signature · 1 line
1.7.9.5
#1 lore raw

PATCH 1/1 Replace DL_DIR refs in the GIT/SVN/HG download

Hi Danomi, all,

2012/8/27 Danomi Manchego <[email protected]>:

Quoted 31 lines

If DL_DIR is set to a relative path, then the download helpers for GIT, SVN, and HG fail, because they reference DL_DIR or $(PKG)_DL_DIR from within the DL_DIR directory. (But the relative path is relative to TOPDIR.) All of these cases can be avoided, since the location of the cloned repo is always $(DL_DIR)/$($(PKG)_BASE_NAME), which can be used to make the helpers simpler, as well as more friendly to the relative DL_DIR definition.

Signed-off-by Danomi Manchego <[email protected]>
---
 package/pkg-download.mk |   16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)
package/pkg-download.mk +3 -4
index 9e98581..a7cef0e 100644
@@ -68,11 +68,11 @@ define DOWNLOAD_GIT
        test -e $(DL_DIR)/$($(PKG)_SOURCE) || \
        (pushd $(DL_DIR) > /dev/null && \
        $(GIT) clone --bare $($(PKG)_SITE) $($(PKG)_BASE_NAME) && \
-       pushd $($(PKG)_BASE_NAME) > /dev/null && \
+       cd $($(PKG)_BASE_NAME) && \
        $(GIT) archive --format=tar --prefix=$($(PKG)_BASE_NAME)/ $($(PKG)_DL_VERSION) | \
-               gzip -c > $(DL_DIR)/$($(PKG)_SOURCE) && \
-       popd > /dev/null && \
-       rm -rf $($(PKG)_DL_DIR) && \
+               gzip -c > ../$($(PKG)_SOURCE) && \
 
+       cd $($(PKG)_BASE_NAME) && \

[...]

Quoted 1 line

+ cd .. && \

If pushd/popd does not do the job, rather than these cd calls, I'd prefer:
( cd somewhere ; do something )

Quoted 17 lines
package/pkg-download.mk +3 -2
+       rm -rf $($(PKG)_BASE_NAME) && \
        popd > /dev/null)
 endef

@@ -104,9 +104,9 @@ endef
 define DOWNLOAD_SVN
        test -e $(DL_DIR)/$($(PKG)_SOURCE) || \
        (pushd $(DL_DIR) > /dev/null && \
-       $(SVN) export -r $($(PKG)_DL_VERSION) $($(PKG)_SITE) $($(PKG)_DL_DIR) && \
+       $(SVN) export -r $($(PKG)_DL_VERSION) $($(PKG)_SITE) $($(PKG)_BASE_NAME) && \
        $(TAR) czf $($(PKG)_SOURCE) $($(PKG)_BASE_NAME)/ && \
-       rm -rf $($(PKG)_DL_DIR) && \
+       rm -rf $($(PKG)_BASE_NAME) && \
        popd > /dev/null)
 endef

package/pkg-download.mk define DOWNLOAD_HG · line 147

Quoted 10 lines
package/pkg-download.mk +2 -2
@@ -140,8 +140,8 @@ define DOWNLOAD_HG
        (pushd $(DL_DIR) > /dev/null && \
        $(HG) clone --noupdate --rev $($(PKG)_DL_VERSION) $($(PKG)_SITE) $($(PKG)_BASE_NAME) && \
        $(HG) archive --repository $($(PKG)_BASE_NAME) --type tgz --prefix $($(PKG)_BASE_NAME)/ \
-                     --rev $($(PKG)_DL_VERSION) $(DL_DIR)/$($(PKG)_SOURCE) && \
-       rm -rf $($(PKG)_DL_DIR) && \
+                     --rev $($(PKG)_DL_VERSION) ./$($(PKG)_SOURCE) && \
+       rm -rf $($(PKG)_BASE_NAME) && \
        popd > /dev/null)
 endef

Overall, I'd prefer a patch like the following, resolving a absolute path for DL_DIR, than a patch fixing everywhere DL_DIR is used.

package/pkg-download.mk +1 -0
@@ -26,6 +26,7 @@ DL_DIR=$(call qstrip,$(BR2_DL_DIR))
 ifeq ($(DL_DIR),)
 DL_DIR:=$(TOPDIR)/dl
 endif
+DL_DIR:=$(shell readlink -f $(DL_DIR))

or
+DL_DIR:=$(shell cd $(DL_DIR); pwd)

Regards,

Signature · 1 line
Sam
author
#2 lore raw

PATCH 1/1 Replace DL_DIR refs in the GIT/SVN/HG download

Hi Samuel,

Quoted 2 lines

If pushd/popd does not do the job, rather than these cd calls, I'd prefer:
( cd somewhere ; do something )

No, I can restore the pushd/popd. I just didn't think they were necessary, since the relationship between DL_DIR and $(PKG)_DL_DIR is fixed.

Quoted 3 lines

+DL_DIR:=$(shell readlink -f $(DL_DIR))
or
+DL_DIR:=$(shell cd $(DL_DIR); pwd)

I like the "readlink -f" trick. But I'm not sure either of these will work. The readlink
option requires all but the last component to already exist, and the cd/pwd requires
the whole path to exist. The top-level Makefile can create DL_DIR (with mkdir -p),
to support directory creation if needed, so I think we need to avoid assuming that
the directory pre-exists. Tell me if I've misunderstood.

Basically, Arnout's reminder that the default DL_DIR value is "$(TOPDIR)/dl" was enough to convince me to recommend adding $(TOPDIR) to all our defconfigs - fixing my particular situation. So I'm not 100% invested in this
patch, if people would rather leave the download helpers as-is ...

Danomi -

Quoted 88 lines · Samuel Martin

Hi Danomi, all,

2012/8/27 Danomi Manchego <[email protected]>:

Quoted 19 lines

If DL_DIR is set to a relative path, then the download helpers for GIT, SVN, and HG fail, because they reference DL_DIR or $(PKG)_DL_DIR from within the DL_DIR directory. (But the relative path is relative to TOPDIR.) All of these cases can be avoided, since the location of the cloned repo is always $(DL_DIR)/$($(PKG)_BASE_NAME), which can be used to make the helpers simpler, as well as more friendly to the relative DL_DIR definition.

Signed-off-by Danomi Manchego <[email protected]>
---
 package/pkg-download.mk |   16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)
package/pkg-download.mk +0 -0
index 9e98581..a7cef0e 100644

package/pkg-download.mk define DOWNLOAD_GIT · line 72

Quoted 6 lines
package/pkg-download.mk +1 -1
@@ -68,11 +68,11 @@ define DOWNLOAD_GIT
        test -e $(DL_DIR)/$($(PKG)_SOURCE) || \
        (pushd $(DL_DIR) > /dev/null && \
        $(GIT) clone --bare $($(PKG)_SITE) $($(PKG)_BASE_NAME) && \
-       pushd $($(PKG)_BASE_NAME) > /dev/null && \
+       cd $($(PKG)_BASE_NAME) && \
        $(GIT) archive --format=tar --prefix=$($(PKG)_BASE_NAME)/

$($(PKG)_DL_VERSION) | \

Quoted 6 lines
package/pkg-download.mk +2 -3
-               gzip -c > $(DL_DIR)/$($(PKG)_SOURCE) && \
-       popd > /dev/null && \
-       rm -rf $($(PKG)_DL_DIR) && \
+               gzip -c > ../$($(PKG)_SOURCE) && \
 
+       cd $($(PKG)_BASE_NAME) && \

[...]

Quoted 1 line

+ cd .. && \

If pushd/popd does not do the job, rather than these cd calls, I'd prefer:
( cd somewhere ; do something )

package/pkg-download.mk endef · line 107

Quoted 9 lines
package/pkg-download.mk +1 -1
+       rm -rf $($(PKG)_BASE_NAME) && \
        popd > /dev/null)
 endef

@@ -104,9 +104,9 @@ endef
 define DOWNLOAD_SVN
        test -e $(DL_DIR)/$($(PKG)_SOURCE) || \
        (pushd $(DL_DIR) > /dev/null && \
-       $(SVN) export -r $($(PKG)_DL_VERSION) $($(PKG)_SITE)

$($(PKG)_DL_DIR) && \

package/pkg-download.mk endef · line 107

Quoted 1 line
package/pkg-download.mk +1 -0
+       $(SVN) export -r $($(PKG)_DL_VERSION) $($(PKG)_SITE)

$($(PKG)_BASE_NAME) && \

package/pkg-download.mk define DOWNLOAD_HG · line 141

Quoted 9 lines
package/pkg-download.mk +1 -1
        $(TAR) czf $($(PKG)_SOURCE) $($(PKG)_BASE_NAME)/ && \
-       rm -rf $($(PKG)_DL_DIR) && \
+       rm -rf $($(PKG)_BASE_NAME) && \
        popd > /dev/null)
 endef
 

@@ -140,8 +140,8 @@ define DOWNLOAD_HG
        (pushd $(DL_DIR) > /dev/null && \
        $(HG) clone --noupdate --rev $($(PKG)_DL_VERSION) $($(PKG)_SITE)

$($(PKG)_BASE_NAME) && \

package/pkg-download.mk define DOWNLOAD_HG · line 142

Quoted 1 line
package/pkg-download.mk +0 -0
        $(HG) archive --repository $($(PKG)_BASE_NAME) --type tgz

--prefix $($(PKG)_BASE_NAME)/ \

package/pkg-download.mk define DOWNLOAD_HG · line 143

Quoted 1 line
package/pkg-download.mk +0 -1
-                     --rev $($(PKG)_DL_VERSION)

$(DL_DIR)/$($(PKG)_SOURCE) && \

package/pkg-download.mk define DOWNLOAD_HG · line 147

Quoted 6 lines
package/pkg-download.mk +2 -1
-       rm -rf $($(PKG)_DL_DIR) && \
+                     --rev $($(PKG)_DL_VERSION) ./$($(PKG)_SOURCE) && \
+       rm -rf $($(PKG)_BASE_NAME) && \
        popd > /dev/null)
 endef

Overall, I'd prefer a patch like the following, resolving a absolute path for DL_DIR, than a patch fixing everywhere DL_DIR is used.

package/pkg-download.mk +1 -0
@@ -26,6 +26,7 @@ DL_DIR=$(call qstrip,$(BR2_DL_DIR))
 ifeq ($(DL_DIR),)
 DL_DIR:=$(TOPDIR)/dl
 endif
+DL_DIR:=$(shell readlink -f $(DL_DIR))

or
+DL_DIR:=$(shell cd $(DL_DIR); pwd)

Quoted 5 lines

Regards,

--
Sam

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20120827/38dc48a8/attachment-0001.html>

#3 lore raw

PATCH 1/1 Replace DL_DIR refs in the GIT/SVN/HG download

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Danomi" == Danomi Manchego <[email protected]> writes:

Hi,

Danomi> Basically, Arnout's reminder that the default DL_DIR value Danomi> is?"$(TOPDIR)/dl" was enough to convince me to recommend adding Danomi> $(TOPDIR) to all our defconfigs - fixing my particular Danomi> situation. ?So I'm not 100% invested in this patch, if people Danomi> would rather?leave the download helpers as-is ...

I've just pushed a change to use mkdir -p $(DL_DIR) && cd $(DL_DIR) && pwd similiar to how we do for the output directory for out of tree builds.

Signature · 1 line
Bye, Peter Korsgaard