All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH] Replace DL_DIR refs in the GIT/SVN/HG download
@ 2012-08-27  0:12 Danomi Manchego
  2012-08-27  4:54 ` Samuel Martin
  0 siblings, 1 reply; 4+ messages in thread
From: Danomi Manchego @ 2012-08-27  0:12 UTC (permalink / raw)
  To: buildroot

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 <danomimanchego123@gmail.com>
---
 package/pkg-download.mk |   16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/package/pkg-download.mk b/package/pkg-download.mk
index 9e98581..a7cef0e 100644
--- a/package/pkg-download.mk
+++ b/package/pkg-download.mk
@@ -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
 
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [Buildroot] [PATCH] Replace DL_DIR refs in the GIT/SVN/HG download
  2012-08-27  0:12 [Buildroot] [PATCH] Replace DL_DIR refs in the GIT/SVN/HG download Danomi Manchego
@ 2012-08-27  4:54 ` Samuel Martin
  2012-08-28  1:13   ` Danomi Manchego
  0 siblings, 1 reply; 4+ messages in thread
From: Samuel Martin @ 2012-08-27  4:54 UTC (permalink / raw)
  To: buildroot

Hi Danomi, all,

2012/8/27 Danomi Manchego <danomimanchego123@gmail.com>:
> 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 <danomimanchego123@gmail.com>
> ---
>  package/pkg-download.mk |   16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/package/pkg-download.mk b/package/pkg-download.mk
> index 9e98581..a7cef0e 100644
> --- a/package/pkg-download.mk
> +++ b/package/pkg-download.mk
> @@ -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) && \
[...]
> +       cd .. && \
If pushd/popd does not do the job, rather than these cd calls, I'd prefer:
( cd somewhere ;  do something )


> +       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
>


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.
--- a/package/pkg-download.mk
+++ b/package/pkg-download.mk
@@ -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,

-- 
Sam

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [Buildroot] [PATCH] Replace DL_DIR refs in the GIT/SVN/HG download
  2012-08-27  4:54 ` Samuel Martin
@ 2012-08-28  1:13   ` Danomi Manchego
  2012-09-10 14:27     ` Peter Korsgaard
  0 siblings, 1 reply; 4+ messages in thread
From: Danomi Manchego @ 2012-08-28  1:13 UTC (permalink / raw)
  To: buildroot

Hi Samuel,

> 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.

> +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 -


On Mon, Aug 27, 2012 at 12:54 AM, Samuel Martin <s.martin49@gmail.com>wrote:

> Hi Danomi, all,
>
> 2012/8/27 Danomi Manchego <danomimanchego123@gmail.com>:
> > 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 <danomimanchego123@gmail.com>
> > ---
> >  package/pkg-download.mk |   16 ++++++++--------
> >  1 file changed, 8 insertions(+), 8 deletions(-)
> >
> > diff --git a/package/pkg-download.mk b/package/pkg-download.mk
> > index 9e98581..a7cef0e 100644
> > --- a/package/pkg-download.mk
> > +++ b/package/pkg-download.mk
> > @@ -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) && \
> [...]
> > +       cd .. && \
> If pushd/popd does not do the job, rather than these cd calls, I'd prefer:
> ( cd somewhere ;  do something )
>
>
> > +       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
> >
>
>
> 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.
> --- a/package/pkg-download.mk
> +++ b/package/pkg-download.mk
> @@ -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,
>
> --
> Sam
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20120827/38dc48a8/attachment-0001.html>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [Buildroot] [PATCH] Replace DL_DIR refs in the GIT/SVN/HG download
  2012-08-28  1:13   ` Danomi Manchego
@ 2012-09-10 14:27     ` Peter Korsgaard
  0 siblings, 0 replies; 4+ messages in thread
From: Peter Korsgaard @ 2012-09-10 14:27 UTC (permalink / raw)
  To: buildroot

>>>>> "Danomi" == Danomi Manchego <danomimanchego123@gmail.com> 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.

-- 
Bye, Peter Korsgaard

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2012-09-10 14:27 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-08-27  0:12 [Buildroot] [PATCH] Replace DL_DIR refs in the GIT/SVN/HG download Danomi Manchego
2012-08-27  4:54 ` Samuel Martin
2012-08-28  1:13   ` Danomi Manchego
2012-09-10 14:27     ` Peter Korsgaard

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.