Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH 2/4] Implement basic non-wget download methods
  2010-07-13  7:15 [Buildroot] [PATCH/RFC] Git/Svn downloaders Maxime Petazzoni
@ 2010-07-13  7:15 ` Maxime Petazzoni
  0 siblings, 0 replies; 7+ messages in thread
From: Maxime Petazzoni @ 2010-07-13  7:15 UTC (permalink / raw)
  To: buildroot

Packages can now be sourced from Git and Subversion repositories by
setting a _SITE_METHOD variable to 'git' or 'svn', respectively and
without the quotes.

The package's _VERSION variable defines which commit, revision, tag or
branch should be checked out. For Git, it can be HEAD, a commit ID, a
tag name or branch name (anything that can be checked out with `git
checkout`). For Subversion, it must be a revision number, or HEAD.

Signed-off-by: Maxime Petazzoni <maxime.petazzoni@bulix.org>
---
 Config.in                   |    4 ++
 Makefile                    |    1 +
 package/Makefile.package.in |   70 ++++++++++++++++++++++++++++++++++++++++--
 3 files changed, 71 insertions(+), 4 deletions(-)

diff --git a/Config.in b/Config.in
index 6adfe49..0fe4cd1 100644
--- a/Config.in
+++ b/Config.in
@@ -41,6 +41,10 @@ config BR2_GIT
 	string "Git command to download source tree"
 	default "git clone"
 
+config BR2_GIT_CO
+	string "Git command to checkout a given commit or tag"
+	default "git checkout"
+
 config BR2_ZCAT
 	string "zcat command"
 	default "gzip -d -c"
diff --git a/Makefile b/Makefile
index 2e49a6b..65b43f1 100644
--- a/Makefile
+++ b/Makefile
@@ -249,6 +249,7 @@ SVN_UP:=$(call qstrip,$(BR2_SVN_UP)) $(QUIET)
 BZR_CO:=$(call qstrip,$(BR2_BZR_CO)) $(QUIET)
 BZR_UP:=$(call qstrip,$(BR2_BZR_UP)) $(QUIET)
 GIT:=$(call qstrip,$(BR2_GIT)) $(QUIET)
+GIT_CO:=$(call qstrip,$(BR2_GIT_CO)) $(QUIET)
 ZCAT:=$(call qstrip,$(BR2_ZCAT))
 BZCAT:=$(call qstrip,$(BR2_BZCAT))
 TAR_OPTIONS=$(call qstrip,$(BR2_TAR_OPTIONS)) -xf
diff --git a/package/Makefile.package.in b/package/Makefile.package.in
index b21741f..624d006 100644
--- a/package/Makefile.package.in
+++ b/package/Makefile.package.in
@@ -69,6 +69,38 @@ TERM_BOLD := $(shell tput smso)
 TERM_RESET := $(shell tput rmso)
 
 ################################################################################
+# The DOWNLOAD_{GIT,SVN,BZR} helpers are in charge of getting a working copy of
+# the source repository for their corresponding SCM, checkouting the requested
+# version / commit / tag, and create an archive out of it.
+################################################################################
+
+define VCS_PACK_SOURCE
+	$(TAR) czf $($(PKG)_SOURCE) --exclude-vcs $($(PKG)_BASE_NAME)/ && \
+	rm -rf $($(PKG)_DL_DIR)
+endef
+
+define DOWNLOAD_GIT
+	pushd $(DL_DIR) > /dev/null && \
+	$(GIT) $($(PKG)_SITE) $($(PKG)_DL_DIR) && \
+	pushd $($(PKG)_DL_DIR) > /dev/null && \
+	$(GIT_CO) $($(PKG)_DL_VERSION) && \
+	popd > /dev/null && \
+	$(VCS_PACK_SOURCE) && \
+	popd > /dev/null
+endef
+
+define DOWNLOAD_SVN
+	pushd $(DL_DIR) > /dev/null && \
+	$(SVN_CO) -r $($(PKG)_DL_VERSION) $($(PKG)_SITE) $($(PKG)_DL_DIR) && \
+	$(VCS_PACK_SOURCE) && \
+	popd > /dev/null
+endef
+
+define DOWNLOAD_WGET
+	$(WGET) -P $(DL_DIR) $(call qstrip,$(1))/$(2)
+endef
+
+################################################################################
 # DOWNLOAD -- Download helper. Will try to download source from:
 # 1) BR2_PRIMARY_SITE if enabled
 # 2) Download site
@@ -83,8 +115,20 @@ TERM_RESET := $(shell tput rmso)
 
 define DOWNLOAD
 	$(Q)test -e $(DL_DIR)/$(2) || \
-	for site in $(call qstrip,$(BR2_PRIMARY_SITE)) $(1) $(call qstrip,$(BR2_BACKUP_SITE)); \
-	do $(WGET) -P $(DL_DIR) $$site/$(2) && exit; done
+	(if test -n "$(call qstrip,$(BR2_PRIMARY_SITE))" ; then \
+		$(call DOWNLOAD_WGET,$(BR2_PRIMARY_SITE),$(2)) && exit ; \
+	fi ; \
+	if test -n "$(1)" ; then \
+		case "$($(PKG)_SITE_METHOD)" in \
+			git) $(DOWNLOAD_GIT) && exit ;; \
+			svn) $(DOWNLOAD_SVN) && exit ;; \
+			*) $(call DOWNLOAD_WGET,$(1),$(2)) && exit ;; \
+		esac ; \
+	fi ; \
+	if test -n "$(call qstrip,$(BR2_BACKUP_SITE))" ; then \
+		$(call DOWNLOAD_WGET,$(BR2_BACKUP_SITE),$(2)) && exit ; \
+	fi ; \
+	exit 1)
 endef
 
 # Utility programs used to build packages
@@ -244,13 +288,23 @@ ifndef $(2)_VERSION
  endif
 endif
 
-$(2)_DIR			=  $$(BUILD_DIR)/$(1)-$$($(2)_VERSION)
+# Keep the package version that may contain forward slashes in the _DL_VERSION
+# variable, then replace all forward slashes ('/') by underscores ('_') to
+# sanitize the package version that is used in paths, directory and file names.
+# Forward slashes may appear in the package's version when pointing to a
+# version control system branch or tag, for example remotes/origin/1_10_stable.
+$(2)_DL_VERSION	= $($(2)_VERSION)
+$(2)_VERSION = $(subst /,_,$($(2)_VERSION))
+
+$(2)_BASE_NAME	=  $(1)-$$($(2)_VERSION)
+$(2)_DL_DIR	=  $$(DL_DIR)/$$($(2)_BASE_NAME)
+$(2)_DIR	=  $$(BUILD_DIR)/$$($(2)_BASE_NAME)
 
 ifndef $(2)_SOURCE
  ifdef $(3)_SOURCE
   $(2)_SOURCE = $($(3)_SOURCE)
  else
-  $(2)_SOURCE			?= $(1)-$$($(2)_VERSION).tar.gz
+  $(2)_SOURCE			?= $$($(2)_BASE_NAME).tar.gz
  endif
 endif
 
@@ -269,6 +323,14 @@ ifndef $(2)_SITE
  endif
 endif
 
+ifndef $(2)_SITE_METHOD
+ ifdef $(3)_SITE_METHOD
+  $(2)_SITE_METHOD = $($(3)_SITE_METHOD)
+ else
+	$(2)_SITE_METHOD = wget
+ endif
+endif
+
 $(2)_DEPENDENCIES		?=
 $(2)_INSTALL_STAGING		?= NO
 $(2)_INSTALL_TARGET		?= YES
-- 
1.6.3.3.346.g8a41d

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

* [Buildroot] [PATCH 2/4] Implement basic non-wget download methods
@ 2010-07-13  9:51 Luca Ceresoli
  2010-07-13 11:30 ` Maxime Petazzoni
  0 siblings, 1 reply; 7+ messages in thread
From: Luca Ceresoli @ 2010-07-13  9:51 UTC (permalink / raw)
  To: buildroot

Maxime Petazzoni wrote:
> Packages can now be sourced from Git and Subversion repositories by
> setting a _SITE_METHOD variable to 'git' or 'svn', respectively and
> without the quotes.
> 
> The package's _VERSION variable defines which commit, revision, tag or
> branch should be checked out. For Git, it can be HEAD, a commit ID, a
> tag name or branch name (anything that can be checked out with `git
> checkout`). For Subversion, it must be a revision number, or HEAD.
> 
> Signed-off-by: Maxime Petazzoni<maxime.petazzoni@bulix.org>
> ---
>   Config.in                   |    4 ++
>   Makefile                    |    1 +
>   package/Makefile.package.in |   70 ++++++++++++++++++++++++++++++++++++++++--
>   3 files changed, 71 insertions(+), 4 deletions(-)
> 
> diff --git a/Config.in b/Config.in
> index 6adfe49..0fe4cd1 100644
> --- a/Config.in
> +++ b/Config.in
> @@ -41,6 +41,10 @@ config BR2_GIT
>   	string "Git command to download source tree"
>   	default "git clone"
> 
> +config BR2_GIT_CO
> +	string "Git command to checkout a given commit or tag"
> +	default "git checkout"
> +
>   config BR2_ZCAT
>   	string "zcat command"
>   	default "gzip -d -c"
> diff --git a/Makefile b/Makefile
> index 2e49a6b..65b43f1 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -249,6 +249,7 @@ SVN_UP:=$(call qstrip,$(BR2_SVN_UP)) $(QUIET)
>   BZR_CO:=$(call qstrip,$(BR2_BZR_CO)) $(QUIET)
>   BZR_UP:=$(call qstrip,$(BR2_BZR_UP)) $(QUIET)
>   GIT:=$(call qstrip,$(BR2_GIT)) $(QUIET)
> +GIT_CO:=$(call qstrip,$(BR2_GIT_CO)) $(QUIET)
Now I would rename GIT to GIT_CLONE to make the difference clear.

>   ZCAT:=$(call qstrip,$(BR2_ZCAT))
>   BZCAT:=$(call qstrip,$(BR2_BZCAT))
>   TAR_OPTIONS=$(call qstrip,$(BR2_TAR_OPTIONS)) -xf
> diff --git a/package/Makefile.package.in b/package/Makefile.package.in
> index b21741f..624d006 100644
> --- a/package/Makefile.package.in
> +++ b/package/Makefile.package.in
> @@ -69,6 +69,38 @@ TERM_BOLD := $(shell tput smso)
>   TERM_RESET := $(shell tput rmso)
> 
>   ################################################################################
> +# The DOWNLOAD_{GIT,SVN,BZR} helpers are in charge of getting a working copy of
> +# the source repository for their corresponding SCM, checkouting the requested
checkouting -> checking out

> +# version / commit / tag, and create an archive out of it.
> +################################################################################
> +
> +define VCS_PACK_SOURCE
> +	$(TAR) czf $($(PKG)_SOURCE) --exclude-vcs $($(PKG)_BASE_NAME)/&&  \
> +	rm -rf $($(PKG)_DL_DIR)
> +endef
> +
> +define DOWNLOAD_GIT
> +	pushd $(DL_DIR)>  /dev/null&&  \
> +	$(GIT) $($(PKG)_SITE) $($(PKG)_DL_DIR)&&  \
> +	pushd $($(PKG)_DL_DIR)>  /dev/null&&  \
> +	$(GIT_CO) $($(PKG)_DL_VERSION)&&  \
> +	popd>  /dev/null&&  \
> +	$(VCS_PACK_SOURCE)&&  \
> +	popd>  /dev/null
> +endef
> +
> +define DOWNLOAD_SVN
> +	pushd $(DL_DIR)>  /dev/null&&  \
> +	$(SVN_CO) -r $($(PKG)_DL_VERSION) $($(PKG)_SITE) $($(PKG)_DL_DIR)&&  \
> +	$(VCS_PACK_SOURCE)&&  \
> +	popd>  /dev/null
> +endef
> +
> +define DOWNLOAD_WGET
> +	$(WGET) -P $(DL_DIR) $(call qstrip,$(1))/$(2)
> +endef

Your overall approach is very clean and easy to extend to other VCSs.

The flip side of this generality is that for each new version that one
wants to download it requires a new git clone or svn checkout. This
takes much more bandwidth (and disk space) than git pull or svn update.
Think about big repos such as Linux.

> +
> +################################################################################
>   # DOWNLOAD -- Download helper. Will try to download source from:
>   # 1) BR2_PRIMARY_SITE if enabled
>   # 2) Download site
> @@ -83,8 +115,20 @@ TERM_RESET := $(shell tput rmso)
> 
>   define DOWNLOAD
>   	$(Q)test -e $(DL_DIR)/$(2) || \
> -	for site in $(call qstrip,$(BR2_PRIMARY_SITE)) $(1) $(call qstrip,$(BR2_BACKUP_SITE)); \
> -	do $(WGET) -P $(DL_DIR) $$site/$(2)&&  exit; done
> +	(if test -n "$(call qstrip,$(BR2_PRIMARY_SITE))" ; then \
> +		$(call DOWNLOAD_WGET,$(BR2_PRIMARY_SITE),$(2))&&  exit ; \
> +	fi ; \
> +	if test -n "$(1)" ; then \
> +		case "$($(PKG)_SITE_METHOD)" in \
> +			git) $(DOWNLOAD_GIT)&&  exit ;; \
> +			svn) $(DOWNLOAD_SVN)&&  exit ;; \
> +			*) $(call DOWNLOAD_WGET,$(1),$(2))&&  exit ;; \
> +		esac ; \
> +	fi ; \
> +	if test -n "$(call qstrip,$(BR2_BACKUP_SITE))" ; then \
> +		$(call DOWNLOAD_WGET,$(BR2_BACKUP_SITE),$(2))&&  exit ; \
> +	fi ; \
> +	exit 1)
>   endef
> 
>   # Utility programs used to build packages
> @@ -244,13 +288,23 @@ ifndef $(2)_VERSION
>    endif
>   endif
> 
> -$(2)_DIR			=  $$(BUILD_DIR)/$(1)-$$($(2)_VERSION)
> +# Keep the package version that may contain forward slashes in the _DL_VERSION
> +# variable, then replace all forward slashes ('/') by underscores ('_') to
> +# sanitize the package version that is used in paths, directory and file names.
> +# Forward slashes may appear in the package's version when pointing to a
> +# version control system branch or tag, for example remotes/origin/1_10_stable.
> +$(2)_DL_VERSION	= $($(2)_VERSION)
> +$(2)_VERSION = $(subst /,_,$($(2)_VERSION))
> +
> +$(2)_BASE_NAME	=  $(1)-$$($(2)_VERSION)
> +$(2)_DL_DIR	=  $$(DL_DIR)/$$($(2)_BASE_NAME)
I don't like saving in $(DL_DIR). That is a directory that I keep with
care so I don't have to re-download everything if I wipe buildroot and
start off again, and I also appreciate the chance of having it on a
shared storage so that different developers or different hosts save
bandwidth and time. So I wouldn't like it to be polluted with repository
clones.

IMHO in your proposed infrastructure the clones should go in a place
where `make clean` removes them. After all you never reuse them (you
reuse the tarball instead).

Luca


> +$(2)_DIR	=  $$(BUILD_DIR)/$$($(2)_BASE_NAME)
> 
>   ifndef $(2)_SOURCE
>    ifdef $(3)_SOURCE
>     $(2)_SOURCE = $($(3)_SOURCE)
>    else
> -  $(2)_SOURCE			?= $(1)-$$($(2)_VERSION).tar.gz
> +  $(2)_SOURCE			?= $$($(2)_BASE_NAME).tar.gz
>    endif
>   endif
> 
> @@ -269,6 +323,14 @@ ifndef $(2)_SITE
>    endif
>   endif
> 
> +ifndef $(2)_SITE_METHOD
> + ifdef $(3)_SITE_METHOD
> +  $(2)_SITE_METHOD = $($(3)_SITE_METHOD)
> + else
> +	$(2)_SITE_METHOD = wget
> + endif
> +endif
> +
>   $(2)_DEPENDENCIES		?=
>   $(2)_INSTALL_STAGING		?= NO
>   $(2)_INSTALL_TARGET		?= YES

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

* [Buildroot] [PATCH 2/4] Implement basic non-wget download methods
  2010-07-13  9:51 [Buildroot] [PATCH 2/4] Implement basic non-wget download methods Luca Ceresoli
@ 2010-07-13 11:30 ` Maxime Petazzoni
  2010-07-14 12:02   ` Quotient Remainder
  0 siblings, 1 reply; 7+ messages in thread
From: Maxime Petazzoni @ 2010-07-13 11:30 UTC (permalink / raw)
  To: buildroot

Hi,

* Luca Ceresoli <list@lucaceresoli.net> [2010-07-13 11:51:42]:

> > diff --git a/Makefile b/Makefile
> > index 2e49a6b..65b43f1 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -249,6 +249,7 @@ SVN_UP:=$(call qstrip,$(BR2_SVN_UP)) $(QUIET)
> >   BZR_CO:=$(call qstrip,$(BR2_BZR_CO)) $(QUIET)
> >   BZR_UP:=$(call qstrip,$(BR2_BZR_UP)) $(QUIET)
> >   GIT:=$(call qstrip,$(BR2_GIT)) $(QUIET)
> > +GIT_CO:=$(call qstrip,$(BR2_GIT_CO)) $(QUIET)
> Now I would rename GIT to GIT_CLONE to make the difference clear.

I aimed for the least modifications here. But if the consensus is on having
GIT_CLONE and GIT_CO, I'll change it.

> >   ################################################################################
> > +# The DOWNLOAD_{GIT,SVN,BZR} helpers are in charge of getting a working copy of
> > +# the source repository for their corresponding SCM, checkouting the requested
> checkouting -> checking out

Fixed

> Your overall approach is very clean and easy to extend to other VCSs.
> 
> The flip side of this generality is that for each new version that one
> wants to download it requires a new git clone or svn checkout. This
> takes much more bandwidth (and disk space) than git pull or svn update.
> Think about big repos such as Linux.

Indeed. But as I understand it, versions for packages don't change that
often. I agree that when they do, you end up downloading more stuff to
clone the full repository again instead of just updating.

But support for git fetch and svn update can easily be added later down
the road if this becomes a real and frequent issue.

> > +$(2)_BASE_NAME	=  $(1)-$$($(2)_VERSION)
> > +$(2)_DL_DIR	=  $$(DL_DIR)/$$($(2)_BASE_NAME)
> I don't like saving in $(DL_DIR). That is a directory that I keep with
> care so I don't have to re-download everything if I wipe buildroot and
> start off again, and I also appreciate the chance of having it on a
> shared storage so that different developers or different hosts save
> bandwidth and time. So I wouldn't like it to be polluted with repository
> clones.
> 
> IMHO in your proposed infrastructure the clones should go in a place
> where `make clean` removes them. After all you never reuse them (you
> reuse the tarball instead).

I think you missed the 'rm' line in VCS_PACK_SOURCE. Working copies are
not kept around. I archive them with tar --exclude-vcs, and delete the
working copy directory so only the archive remains for the extract phase
to use.

So you can still have a shared download directories with the archives
you need in there, even for VCS sourced packages, and Buildroot will
know where to find them and will use them. For example for Tremor, a
tremor-16259.tar.gz archive will be created in DL_DIR, and subsequent
builds will use this directly (unless TREMOR_VERSION changed of course).


Regards,
- Maxime
-- 
Maxime Petazzoni <http://www.bulix.org>
 ``One by one, the penguins took away my sanity.''
Linux kernel and software developer at MontaVista Software
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20100713/c53b59b3/attachment.pgp>

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

* [Buildroot] [PATCH 2/4] Implement basic non-wget download methods
  2010-07-13 11:30 ` Maxime Petazzoni
@ 2010-07-14 12:02   ` Quotient Remainder
  2010-07-16 10:39     ` Maxime Petazzoni
  0 siblings, 1 reply; 7+ messages in thread
From: Quotient Remainder @ 2010-07-14 12:02 UTC (permalink / raw)
  To: buildroot

On Tue, 2010-07-13 at 13:30 +0200, Maxime Petazzoni wrote:
> Hi,
> 
> * Luca Ceresoli <list@lucaceresoli.net> [2010-07-13 11:51:42]:
> 
> > > +GIT_CO:=$(call qstrip,$(BR2_GIT_CO)) $(QUIET)
> > Now I would rename GIT to GIT_CLONE to make the difference clear.
> 
> I aimed for the least modifications here. But if the consensus is on having
> GIT_CLONE and GIT_CO, I'll change it.

In a broader sense what are these configuration options for?  Is it to
allow adding switches (e.g. git checkout -f) to the commands or is it to
allow specifying the path to the executable
(e.g. /usr/bin/svn, /opt/git4.0-pre/bin/git)?
If it's the first case, I'd side with GIT_CLONE and GIT_CO.  If it's the
second case, I'd leave it as GIT but get rid of the GIT_CO and just put
the actual command in Makefile.package.in.  Is is likely that they will
ever differ from "clone" or "checkout" anyway?

> Indeed. But as I understand it, versions for packages don't change that
> often. I agree that when they do, you end up downloading more stuff to
> clone the full repository again instead of just updating.
> 
> But support for git fetch and svn update can easily be added later down
> the road if this becomes a real and frequent issue.
> 
> > > +$(2)_BASE_NAME	=  $(1)-$$($(2)_VERSION)
> > > +$(2)_DL_DIR	=  $$(DL_DIR)/$$($(2)_BASE_NAME)
> > I don't like saving in $(DL_DIR). That is a directory that I keep with
> > care so I don't have to re-download everything if I wipe buildroot and
> > start off again, and I also appreciate the chance of having it on a
> > shared storage so that different developers or different hosts save
> > bandwidth and time. So I wouldn't like it to be polluted with repository
> > clones.
> > 
> > IMHO in your proposed infrastructure the clones should go in a place
> > where `make clean` removes them. After all you never reuse them (you
> > reuse the tarball instead).
> 
> I think you missed the 'rm' line in VCS_PACK_SOURCE. Working copies are
> not kept around. I archive them with tar --exclude-vcs, and delete the
> working copy directory so only the archive remains for the extract phase
> to use.
> 
> So you can still have a shared download directories with the archives
> you need in there, even for VCS sourced packages, and Buildroot will
> know where to find them and will use them. For example for Tremor, a
> tremor-16259.tar.gz archive will be created in DL_DIR, and subsequent
> builds will use this directly (unless TREMOR_VERSION changed of course).
> 
I have to agree with Maxime here, $(DL_DIR) seems the right place for
these directories and the creation of the tarball in $(DL_DIR)
integrates nicely with the existing infrastructure.  I would prefer the
use of "git archive"/"svn export" instead of plain "tar" here though.  I
feel it's better to use the VCS native facility than relying on the
installed version of tar to know about the particulars of the VCS
metainformation.
In addition, when tar is not used, the name of the cloned directory need
not be tied to the version; a prefix can be specified in the archive
command or exporting done to a temporary directory.  $($(PKG)_BASE_NAME)
can just be $($(PKG)_NAME).$($(PKG)_SITE_METHOD) or similar.


The result is that the working copy can be left in $(DL_DIR) so it's
available for updating at a later time; I would leave out the "rm" line
so re-use is possible.  It could be regarded as a kind of hook to help
the possible future feature (discussed in the other thread) of using BR
as a development environment.

Another feature not present in this is the guessing of the download
scheme/protocol from the URI (in Luca's patch).  It seems to fit in
nicely with this patch without significant modification and adds that
extra bit of flexibility while falling back to sensible defaults, in my
opinion.

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

* [Buildroot] [PATCH 2/4] Implement basic non-wget download methods
  2010-07-14 12:02   ` Quotient Remainder
@ 2010-07-16 10:39     ` Maxime Petazzoni
  2010-07-18 21:22       ` Peter Korsgaard
  0 siblings, 1 reply; 7+ messages in thread
From: Maxime Petazzoni @ 2010-07-16 10:39 UTC (permalink / raw)
  To: buildroot

* Quotient Remainder <quotientvremainder@gmail.com> [2010-07-14 13:02:00]:

> > > > +GIT_CO:=$(call qstrip,$(BR2_GIT_CO)) $(QUIET)
> > > Now I would rename GIT to GIT_CLONE to make the difference clear.
> > 
> > I aimed for the least modifications here. But if the consensus is on having
> > GIT_CLONE and GIT_CO, I'll change it.
> 
> In a broader sense what are these configuration options for?  Is it to
> allow adding switches (e.g. git checkout -f) to the commands or is it to
> allow specifying the path to the executable
> (e.g. /usr/bin/svn, /opt/git4.0-pre/bin/git)?
> If it's the first case, I'd side with GIT_CLONE and GIT_CO.  If it's the
> second case, I'd leave it as GIT but get rid of the GIT_CO and just put
> the actual command in Makefile.package.in.  Is is likely that they will
> ever differ from "clone" or "checkout" anyway?

I don't really know what they are for. Maybe some "older" Buildroot
developers could give us some details there. Because you're right, git
clone will most likely remain git clone. But having the option to use a
specific path is usefull, in which case I'd leave only GIT="git". But
then the same would apply to SVN_CO/SVN_UP where we should only keep
SVN="svn".

Thoughts?

> I have to agree with Maxime here, $(DL_DIR) seems the right place for
> these directories and the creation of the tarball in $(DL_DIR)
> integrates nicely with the existing infrastructure.  I would prefer the
> use of "git archive"/"svn export" instead of plain "tar" here though.  I
> feel it's better to use the VCS native facility than relying on the
> installed version of tar to know about the particulars of the VCS
> metainformation.

Makes sense. I'm more used to --exclude-vcs because I don't have to
remember the different syntax of all VCS, especially for the ones I
don't use frequently like Bzr, Mercurial, etc.

I'll change that in my next resend.

> In addition, when tar is not used, the name of the cloned directory need
> not be tied to the version; a prefix can be specified in the archive
> command or exporting done to a temporary directory.  $($(PKG)_BASE_NAME)
> can just be $($(PKG)_NAME).$($(PKG)_SITE_METHOD) or similar.
> 
> The result is that the working copy can be left in $(DL_DIR) so it's
> available for updating at a later time; I would leave out the "rm" line
> so re-use is possible.  It could be regarded as a kind of hook to help
> the possible future feature (discussed in the other thread) of using BR
> as a development environment.

Although this would work fairly well for Git repository, it is not the
case for Subversion repositories, especially if the PGK_SITE changes to
point to a different tag, in which case you need to svn switch
--relocate, etc.

I think it's best for now to go with the current method of checking out,
creating the tarball and removing the working copy, and later work on
how/where to keep the working copies and using them efficiently for
updates. This makes the incremental changes smaller and easier to deal
with. Not necessarily in terms of code, but also in terms of the
discussions around it.

> Another feature not present in this is the guessing of the download
> scheme/protocol from the URI (in Luca's patch).  It seems to fit in
> nicely with this patch without significant modification and adds that
> extra bit of flexibility while falling back to sensible defaults, in my
> opinion.

It does indeed fit very nicely with my patch, so I'll go ahead and
integrate it in my next resend.

- Maxime
-- 
Maxime Petazzoni <http://www.bulix.org>
 ``One by one, the penguins took away my sanity.''
Linux kernel and software developer at MontaVista Software
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20100716/091183f7/attachment.pgp>

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

* [Buildroot] [PATCH 2/4] Implement basic non-wget download methods
  2010-07-16 10:39     ` Maxime Petazzoni
@ 2010-07-18 21:22       ` Peter Korsgaard
  2010-07-20  5:48         ` Maxime Petazzoni
  0 siblings, 1 reply; 7+ messages in thread
From: Peter Korsgaard @ 2010-07-18 21:22 UTC (permalink / raw)
  To: buildroot

>>>>> "Maxime" == Maxime Petazzoni <maxime.petazzoni@bulix.org> writes:

Hi,

 >> In a broader sense what are these configuration options for?  Is it to
 >> allow adding switches (e.g. git checkout -f) to the commands or is it to
 >> allow specifying the path to the executable
 >> (e.g. /usr/bin/svn, /opt/git4.0-pre/bin/git)?
 >> If it's the first case, I'd side with GIT_CLONE and GIT_CO.  If it's the
 >> second case, I'd leave it as GIT but get rid of the GIT_CO and just put
 >> the actual command in Makefile.package.in.  Is is likely that they will
 >> ever differ from "clone" or "checkout" anyway?

 Maxime> I don't really know what they are for. Maybe some "older" Buildroot
 Maxime> developers could give us some details there. Because you're right, git
 Maxime> clone will most likely remain git clone. But having the option to use a
 Maxime> specific path is usefull, in which case I'd leave only GIT="git". But
 Maxime> then the same would apply to SVN_CO/SVN_UP where we should only keep
 Maxime> SVN="svn".

Yes, they are really old and not used (besides SVN_CO) - Feel free to
clean this up while making these changes.

-- 
Bye, Peter Korsgaard

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

* [Buildroot] [PATCH 2/4] Implement basic non-wget download methods
  2010-07-18 21:22       ` Peter Korsgaard
@ 2010-07-20  5:48         ` Maxime Petazzoni
  0 siblings, 0 replies; 7+ messages in thread
From: Maxime Petazzoni @ 2010-07-20  5:48 UTC (permalink / raw)
  To: buildroot

* Peter Korsgaard <jacmet@uclibc.org> [2010-07-18 23:22:46]:

> Yes, they are really old and not used (besides SVN_CO) - Feel free to
> clean this up while making these changes.

Ok. I'll try to resend an updated series today or tomorrow.

Thanks for the review,
- Maxime

-- 
Maxime Petazzoni <http://www.bulix.org>
 ``One by one, the penguins took away my sanity.''
Linux kernel and software developer at MontaVista Software
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20100720/8fddfc8d/attachment-0001.pgp>

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

end of thread, other threads:[~2010-07-20  5:48 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-07-13  9:51 [Buildroot] [PATCH 2/4] Implement basic non-wget download methods Luca Ceresoli
2010-07-13 11:30 ` Maxime Petazzoni
2010-07-14 12:02   ` Quotient Remainder
2010-07-16 10:39     ` Maxime Petazzoni
2010-07-18 21:22       ` Peter Korsgaard
2010-07-20  5:48         ` Maxime Petazzoni
  -- strict thread matches above, loose matches on Subject: below --
2010-07-13  7:15 [Buildroot] [PATCH/RFC] Git/Svn downloaders Maxime Petazzoni
2010-07-13  7:15 ` [Buildroot] [PATCH 2/4] Implement basic non-wget download methods Maxime Petazzoni

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox