diff mbox

[RFC] package: Redownload HEAD packages every build

Message ID 1380293015-20331-1-git-send-email-clshotwe@rockwellcollins.com
State Rejected
Headers show

Commit Message

Clayton Shotwell Sept. 27, 2013, 2:43 p.m. UTC
Adding a check to remove a downloaded package if the version is HEAD. This causes the package to be re-downloaded with updated software. This feature is very useful during package development.

Signed-off-by: Clayton Shotwell <clshotwe@rockwellcollins.com>
---
 package/pkg-generic.mk |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

Comments

Clayton Shotwell Sept. 27, 2013, 2:51 p.m. UTC | #1
Clayton Shotwell <clshotwe@rockwellcollins.com> wrote on 09/27/2013 
09:43:35 AM:

> Subject: [RFC] package: Redownload HEAD packages every build
> 
> Adding a check to remove a downloaded package if the version is 
> HEAD. This causes the package to be re-downloaded with updated 
> software. This feature is very useful during package development.

I apologize for messing up the commit message.  That will be fixed in the 
next version.

Thanks,
Clayton

Clayton Shotwell
Software Engineer
clshotwe@rockwellcollins.com
www.rockwellcollins.com
Thomas De Schampheleire Sept. 27, 2013, 3:11 p.m. UTC | #2
Clayton Shotwell <clshotwe@rockwellcollins.com> wrote:
>Adding a check to remove a downloaded package if the version is HEAD. This causes the package to be re-downloaded with updated software. This feature is very useful during package development.
>
>Signed-off-by: Clayton Shotwell <clshotwe@rockwellcollins.com>
>---
> package/pkg-generic.mk |    7 +++++++
> 1 files changed, 7 insertions(+), 0 deletions(-)
>
>diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
>index bfc4dc1..859c4a9 100644
>--- a/package/pkg-generic.mk
>+++ b/package/pkg-generic.mk
>@@ -27,6 +27,13 @@
> # Retrieve the archive
> $(BUILD_DIR)/%/.stamp_downloaded:
> ifeq ($(DL_MODE),DOWNLOAD)
>+# Remove the pre-downloaded tar file if the package is a HEAD version
>+	$(Q)( \
>+	if test $($(PKG)_VERSION) == HEAD -a -e $(DL_DIR)/$($(PKG)_SOURCE); then \
>+		rm $(DL_DIR)/$($(PKG)_SOURCE); \
>+		$(call MESSAGE,"Removing $(PKG) HEAD source file"); \
>+	fi; \
>+	)
> # Only show the download message if it isn't already downloaded
> 	$(Q)if test ! -e $(DL_DIR)/$($(PKG)_SOURCE); then \
> 		$(call MESSAGE,"Downloading") ; \


Not sure if this patch would be accepted, but in case it is I think you should also check for 'tip' which is the mercurial name for HEAD. Note that it may be safer to also check the FOO_SITE_METHOD variable for hg or git, to avoid that tip would match with a git tag or branch, and similarly that HEAD would match a mercurial tag or branch.

Best regards,
Thomas
Arnout Vandecappelle Oct. 1, 2013, 4:08 p.m. UTC | #3
On 09/27/13 16:43, Clayton Shotwell wrote:
> Adding a check to remove a downloaded package if the version is HEAD. This causes the package to be re-downloaded with updated software. This feature is very useful during package development.
>
> Signed-off-by: Clayton Shotwell <clshotwe@rockwellcollins.com>
> ---
>   package/pkg-generic.mk |    7 +++++++
>   1 files changed, 7 insertions(+), 0 deletions(-)
>
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index bfc4dc1..859c4a9 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -27,6 +27,13 @@
>   # Retrieve the archive
>   $(BUILD_DIR)/%/.stamp_downloaded:
>   ifeq ($(DL_MODE),DOWNLOAD)
> +# Remove the pre-downloaded tar file if the package is a HEAD version
> +	$(Q)( \
> +	if test $($(PKG)_VERSION) == HEAD -a -e $(DL_DIR)/$($(PKG)_SOURCE); then \
> +		rm $(DL_DIR)/$($(PKG)_SOURCE); \

  Use $(RM) instead of rm, then you don't need to check for existence.

> +		$(call MESSAGE,"Removing $(PKG) HEAD source file"); \

  I don't think this extra message is needed.

> +	fi; \
> +	)
>   # Only show the download message if it isn't already downloaded
>   	$(Q)if test ! -e $(DL_DIR)/$($(PKG)_SOURCE); then \
>   		$(call MESSAGE,"Downloading") ; \
>

  You'll also need to avoid touching the stamp file after the download, 
otherwise the rule is not even evaluated.

  Also I think I would prefer instead to have an explicit variable to 
indicate that it should be re-downloaded, e.g.

ifneq ($($(PKG)_FORCE_DOWNLOAD),)
	$(Q)$(RM) $(DL_DIR)/$($(PKG)_SOURCE)
endif

  This variable can be set either from the package's .mk file or from the 
local override file.


  And finally, the documentation should be updated as well.


  Regards,
  Arnout
Thomas Petazzoni Oct. 1, 2013, 6:51 p.m. UTC | #4
Dear Clayton Shotwell,

On Fri, 27 Sep 2013 09:43:35 -0500, Clayton Shotwell wrote:

> Adding a check to remove a downloaded package if the version is HEAD. This causes the 
> package to be re-downloaded with updated software. This feature is very
> useful during package development.

I haven't made up my mind yet on this, but in general, I'd like to
avoid having several "mechanisms" achieving the same goal. I believe
the <pkg>_OVERRIDE_SRCDIR mechanism is already here to easily allow to
rebuild the latest version of a component source code during its
development.

It seems like the mechanism you're proposing overlaps quite
significantly with that, while being a bit less flexible. Have you
tried the <pkg>_OVERRIDE_SRCDIR mechanism?

Regarding the implementation, I'm not sure to understand how
<pkg>_VERSION = HEAD is supposed to be used. For example, I have a
software, whose source code is handled in Git, in its 'master' branch.
How would I tell your mechanism that I want the latest source code to
be redownloaded each time?

Moreover, it's going to be redownloaded each time completely, which is
quite annoying during development. The <pkg>_OVERRIDE_SRCDIR allows you
to point to a local directory, which avoids re-downloading the entire
source code everytime.

Thanks!

Thomas
Clayton Shotwell Oct. 1, 2013, 8:01 p.m. UTC | #5
Thomas and Arnout,

Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote on 10/01/2013 
01:51:54 PM:

> It seems like the mechanism you're proposing overlaps quite
> significantly with that, while being a bit less flexible. Have you
> tried the <pkg>_OVERRIDE_SRCDIR mechanism?

I did a little digging into the <pkg>_OVERRIDE_SRCDIR mechanism but I 
could not find any documentation and the implementation in the make 
structure is a little unclear.  What is the full intent of that mechanism?

> Regarding the implementation, I'm not sure to understand how
> <pkg>_VERSION = HEAD is supposed to be used. For example, I have a
> software, whose source code is handled in Git, in its 'master' branch.
> How would I tell your mechanism that I want the latest source code to
> be redownloaded each time?

I failed to take git and mercurial into account with this patch. 
Subversion uses "HEAD" for its latest source code.

> Moreover, it's going to be redownloaded each time completely, which is
> quite annoying during development. The <pkg>_OVERRIDE_SRCDIR allows you
> to point to a local directory, which avoids re-downloading the entire
> source code everytime.

Arnout Vandecappelle <arnout@mind.be> wrote on 10/01/2013 11:08:23 AM:

>   Also I think I would prefer instead to have an explicit variable to 
> indicate that it should be re-downloaded, e.g.
> 
> ifneq ($($(PKG)_FORCE_DOWNLOAD),)
>    $(Q)$(RM) $(DL_DIR)/$($(PKG)_SOURCE)
> endif

I like this approach much better.  It would make this mechanism very 
explicit and avoid having to handle all of the different repository 
nuances. I would like to create a new patch that implements this method.

>   This variable can be set either from the package's .mk file or from 
the 
> local override file.
> 
> 
>   And finally, the documentation should be updated as well.

Agreed.

Thanks,
Clayton

Clayton Shotwell
Software Engineer
clshotwe@rockwellcollins.com
www.rockwellcollins.com
Thomas Petazzoni Oct. 1, 2013, 9:37 p.m. UTC | #6
Hello,

On Tue, 1 Oct 2013 15:01:52 -0500, clshotwe@rockwellcollins.com wrote:

> > It seems like the mechanism you're proposing overlaps quite
> > significantly with that, while being a bit less flexible. Have you
> > tried the <pkg>_OVERRIDE_SRCDIR mechanism?
> 
> I did a little digging into the <pkg>_OVERRIDE_SRCDIR mechanism but I 
> could not find any documentation and the implementation in the make 
> structure is a little unclear.  What is the full intent of that mechanism?

Either in your package .mk file or in a local.mk file, you can add:

FOO_OVERRIDE_SRCDIR = /path/to/foo/sources/

If you do that, then Buildroot will no longer download/extract/patch
the sources for the foo package, it will directly assume the sources
are located in /path/to/foo/sources/. It will rsync them to the build
directory, and build from there. Whenever you do:

	make foo-reconfigure

or

	make foo-rebuild

Buildroot will rsync again the source code, and restart the build at
the configuration step, or the build step of the foo package.

Usually, /path/to/foo/sources/ will be the directory where you did your
Subversion checkout or Git clone of your "foo" source code.

Best regards,

Thomas
Ryan Barnett Oct. 1, 2013, 9:54 p.m. UTC | #7
Thomas,

Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote on 
10/01/2013 04:37:39 PM:

> Hello,
> 
> On Tue, 1 Oct 2013 15:01:52 -0500, clshotwe@rockwellcollins.com wrote:
> 
> > > It seems like the mechanism you're proposing overlaps quite
> > > significantly with that, while being a bit less flexible. Have you
> > > tried the <pkg>_OVERRIDE_SRCDIR mechanism?
> > 
> > I did a little digging into the <pkg>_OVERRIDE_SRCDIR mechanism but I 
> > could not find any documentation and the implementation in the make 
> > structure is a little unclear.  What is the full intent of that 
mechanism?
> 
> Either in your package .mk file or in a local.mk file, you can add:
> 
> FOO_OVERRIDE_SRCDIR = /path/to/foo/sources/
> 
> If you do that, then Buildroot will no longer download/extract/patch
> the sources for the foo package, it will directly assume the sources
> are located in /path/to/foo/sources/. It will rsync them to the build
> directory, and build from there. Whenever you do:
> 
>    make foo-reconfigure
> 
> or
> 
>    make foo-rebuild
> 
> Buildroot will rsync again the source code, and restart the build at
> the configuration step, or the build step of the foo package.
> 
> Usually, /path/to/foo/sources/ will be the directory where you did your
> Subversion checkout or Git clone of your "foo" source code.

Could you add this explanation to buildroot manual since it looks like
<pkg>_OVERRIDE_SRCDIR mechanism is your feature and it is currently
lacking in documentation in the manual? I too have tried to understand
how this feature worked and it was a bit unclear.

The only mention of OVERRIDE_SRCDIR in the documentation is under the
rebuild and reconfigure commands. Where is says that it that it only
makes sense to use these features with OVERRIDE_SRCDIR but doesn't
explain what you just explained above.

Thanks,
-Ryan

------------------------------------------------------------------------------------------
Ryan J Barnett / Software Engineer / Platform SW
MS 137-157, 855 35th St NE, Cedar Rapids, IA, 52498-3161, US
rjbarnet@rockwellcollins.com
www.rockwellcollins.com
Thomas Petazzoni Oct. 2, 2013, 7:13 a.m. UTC | #8
Dear Ryan Barnett,

On Tue, 1 Oct 2013 16:54:04 -0500, Ryan Barnett wrote:

> > Buildroot will rsync again the source code, and restart the build at
> > the configuration step, or the build step of the foo package.
> > 
> > Usually, /path/to/foo/sources/ will be the directory where you did your
> > Subversion checkout or Git clone of your "foo" source code.
> 
> Could you add this explanation to buildroot manual since it looks like
> <pkg>_OVERRIDE_SRCDIR mechanism is your feature and it is currently
> lacking in documentation in the manual? I too have tried to understand
> how this feature worked and it was a bit unclear.
> 
> The only mention of OVERRIDE_SRCDIR in the documentation is under the
> rebuild and reconfigure commands. Where is says that it that it only
> makes sense to use these features with OVERRIDE_SRCDIR but doesn't
> explain what you just explained above.

Right, I will submit a documentation patch.

Thanks!

Thomas
Clayton Shotwell Oct. 2, 2013, 6:34 p.m. UTC | #9
Thomas,

Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote on 10/02/2013 
02:13:49 AM:

> > > Buildroot will rsync again the source code, and restart the build at
> > > the configuration step, or the build step of the foo package.
> > > 
> > > Usually, /path/to/foo/sources/ will be the directory where you did 
your
> > > Subversion checkout or Git clone of your "foo" source code.
> > 
> > Could you add this explanation to buildroot manual since it looks like
> > <pkg>_OVERRIDE_SRCDIR mechanism is your feature and it is currently
> > lacking in documentation in the manual? I too have tried to understand
> > how this feature worked and it was a bit unclear.
> > 
> > The only mention of OVERRIDE_SRCDIR in the documentation is under the
> > rebuild and reconfigure commands. Where is says that it that it only
> > makes sense to use these features with OVERRIDE_SRCDIR but doesn't
> > explain what you just explained above.
> 
> Right, I will submit a documentation patch.

This sounds like a much better method for my development.  Thanks for 
explaining it.

Thanks,
Clayton

Clayton Shotwell
Software Engineer
clshotwe@rockwellcollins.com
www.rockwellcollins.com
diff mbox

Patch

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index bfc4dc1..859c4a9 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -27,6 +27,13 @@ 
 # Retrieve the archive
 $(BUILD_DIR)/%/.stamp_downloaded:
 ifeq ($(DL_MODE),DOWNLOAD)
+# Remove the pre-downloaded tar file if the package is a HEAD version
+	$(Q)( \
+	if test $($(PKG)_VERSION) == HEAD -a -e $(DL_DIR)/$($(PKG)_SOURCE); then \
+		rm $(DL_DIR)/$($(PKG)_SOURCE); \
+		$(call MESSAGE,"Removing $(PKG) HEAD source file"); \
+	fi; \
+	)
 # Only show the download message if it isn't already downloaded
 	$(Q)if test ! -e $(DL_DIR)/$($(PKG)_SOURCE); then \
 		$(call MESSAGE,"Downloading") ; \