Add support for plain URL in $(PKG)_PATCH variable

Submitted by Jérôme Pouiller on Jan. 18, 2013, 4:11 p.m.

Details

Message ID 1358525515-7295-1-git-send-email-jezz@sysmic.org
State Superseded
Headers show

Commit Message

Jérôme Pouiller Jan. 18, 2013, 4:11 p.m.
Until now, $(PKG)_PATCH allow only to download patches from same URL than tarball.
This patch allow to detect when plain URL are used in $(PKG)_PATCH and correctly
handle them.

Signed-off-by: Jérôme Pouiller <jezz@sysmic.org>
---
 package/pkg-generic.mk |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Yann E. MORIN Jan. 30, 2013, 6:59 p.m.
Jérôme, Peter, All,

On Friday 18 January 2013 Jérôme Pouiller wrote:
> Until now, $(PKG)_PATCH allow only to download patches from same URL than tarball.
> This patch allow to detect when plain URL are used in $(PKG)_PATCH and correctly
> handle them.
> 
> Signed-off-by: Jérôme Pouiller <jezz@sysmic.org>

I've added this patch to my tree, as I need it to cleanly add new packages,
and it will be included in my next pull-request.

I'll add my *-by tags at the same time.

Thank you! :-)

Regards,
Yann E. MORIN.
Yann E. MORIN Jan. 30, 2013, 7:23 p.m.
Jérôme, Thomas, All,

On Friday 18 January 2013 Jérôme Pouiller wrote:
> Until now, $(PKG)_PATCH allow only to download patches from same URL than tarball.
> This patch allow to detect when plain URL are used in $(PKG)_PATCH and correctly
> handle them.
> 
> Signed-off-by: Jérôme Pouiller <jezz@sysmic.org>
> ---
>  package/pkg-generic.mk |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index b0eca0a..37f3a29 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -40,7 +40,7 @@ ifeq ($(DL_MODE),DOWNLOAD)
>  	fi
>  endif
>  	$(if $($(PKG)_SOURCE),$(call DOWNLOAD,$($(PKG)_SITE)/$($(PKG)_SOURCE)))
> -	$(foreach p,$($(PKG)_PATCH),$(call DOWNLOAD,$($(PKG)_SITE)/$(p))$(sep))
> +	$(foreach p,$($(PKG)_PATCH),$(call DOWNLOAD,$(if $(findstring ://,$(p)),$(p),$($(PKG)_SITE)/$(p)))$(sep))
>  	$(foreach hook,$($(PKG)_POST_DOWNLOAD_HOOKS),$(call $(hook))$(sep))
>  ifeq ($(DL_MODE),DOWNLOAD)
>  	$(Q)mkdir -p $(@D)
> @@ -85,7 +85,7 @@ $(BUILD_DIR)/%/.stamp_patched: NAMEVER = $(RAWNAME)-$($(PKG)_VERSION)
>  $(BUILD_DIR)/%/.stamp_patched:
>  	@$(call MESSAGE,"Patching $($(PKG)_DIR_PREFIX)/$(RAWNAME)")
>  	$(foreach hook,$($(PKG)_PRE_PATCH_HOOKS),$(call $(hook))$(sep))
> -	$(foreach p,$($(PKG)_PATCH),support/scripts/apply-patches.sh $(@D) $(DL_DIR) $(p)$(sep))
> +	$(foreach p,$($(PKG)_PATCH),support/scripts/apply-patches.sh $(@D) $(DL_DIR) $(notdir $(p))$(sep))
>  	$(Q)( \
>  	if test -d $($(PKG)_DIR_PREFIX)/$(RAWNAME); then \
>  	  if test "$(wildcard $($(PKG)_DIR_PREFIX)/$(RAWNAME)/$(NAMEVER)*.patch*)"; then \

Hm. Not specific to the patch per-se, but:

Currently the (single) PKG_PATCH is saved to a file which name is the
plain filename ("notdir") of the patch.

In case one uses a custom directory to store all downloaded files (eg. I
use "${HOME}/src" so I don't have to download them again and again), this
directory gets cluttered with many files that are difficult to link to
the package they apply to.

It gets even worse with this change.

What about saving the patches to a file named thus:
   $(PKG_NAME)-$(PKG_VERSION)-$(PKG_PATCH_FILENAME).patch

Regards,
Yann E. MORIN.
Jérôme Pouiller Jan. 30, 2013, 10:37 p.m.
On Wednesday 30 January 2013 20:23:58 Yann E. MORIN wrote:
[...]
> In case one uses a custom directory to store all downloaded files (eg. I
> use "${HOME}/src" so I don't have to download them again and again), this
> directory gets cluttered with many files that are difficult to link to
> the package they apply to.
> 
> It gets even worse with this change.
> 
> What about saving the patches to a file named thus:
>    $(PKG_NAME)-$(PKG_VERSION)-$(PKG_PATCH_FILENAME).patches
Sure!

Patch follows, Looks doing the job. Nevertheless, I begin to found this feature a 
little difficult to read (euphemism...). Someone has an opinion about coding 
style?
Thomas Petazzoni Jan. 30, 2013, 10:50 p.m.
Dear Yann E. MORIN,

On Wed, 30 Jan 2013 20:23:58 +0100, Yann E. MORIN wrote:
> In case one uses a custom directory to store all downloaded files
> (eg. I use "${HOME}/src" so I don't have to download them again and
> again), this directory gets cluttered with many files that are
> difficult to link to the package they apply to.
> 
> It gets even worse with this change.
> 
> What about saving the patches to a file named thus:
>    $(PKG_NAME)-$(PKG_VERSION)-$(PKG_PATCH_FILENAME).patch

I am not sure because we don't do this for tarballs: we keep their
original name. For example Python-3.3.0 even though the package is
named python3, cJSONFiles.zip even though the package is named cjson,
etc.

So either we decide to keep the original name for everything, or we
decide to use our own name for everything.

Thomas
Yann E. MORIN Jan. 30, 2013, 10:57 p.m.
Thomas, All,

On Wednesday 30 January 2013 Thomas Petazzoni wrote:
> Dear Yann E. MORIN,
> 
> On Wed, 30 Jan 2013 20:23:58 +0100, Yann E. MORIN wrote:
> > In case one uses a custom directory to store all downloaded files
> > (eg. I use "${HOME}/src" so I don't have to download them again and
> > again), this directory gets cluttered with many files that are
> > difficult to link to the package they apply to.
> > 
> > It gets even worse with this change.
> > 
> > What about saving the patches to a file named thus:
> >    $(PKG_NAME)-$(PKG_VERSION)-$(PKG_PATCH_FILENAME).patch
> 
> I am not sure because we don't do this for tarballs: we keep their
> original name. For example Python-3.3.0 even though the package is
> named python3, cJSONFiles.zip even though the package is named cjson,
> etc.
> 
> So either we decide to keep the original name for everything, or we
> decide to use our own name for everything.

So, given two packages, 'foo' and 'bar', what will happen if both have
a patch file named 'mktemp.patch' ?

 1- only one will be downloaded
 2- it will be applied onto both packages

Err... :-(

Regards,
Yann E. MORIN.
Jérôme Pouiller Jan. 30, 2013, 11:04 p.m.
On Wednesday 30 January 2013 23:57:13 Yann E. MORIN wrote:
> On Wednesday 30 January 2013 Thomas Petazzoni wrote:
[...]
> > > What about saving the patches to a file named thus:
> > >    $(PKG_NAME)-$(PKG_VERSION)-$(PKG_PATCH_FILENAME).patch
> > 
> > I am not sure because we don't do this for tarballs: we keep their
> > original name. For example Python-3.3.0 even though the package is
> > named python3, cJSONFiles.zip even though the package is named cjson,
> > etc.
> > 
> > So either we decide to keep the original name for everything, or we
> > decide to use our own name for everything.
> 
> So, given two packages, 'foo' and 'bar', what will happen if both have
> a patch file named 'mktemp.patch' ?
> 
>  1- only one will be downloaded
>  2- it will be applied onto both packages
And what about
  $(PKG_NAME)-$(PKG_VERSION)/$(PKG_PATCH_FILENAME).patch
?
Yann E. MORIN Jan. 30, 2013, 11:10 p.m.
Jérôme, All,

On Thursday 31 January 2013 Jérôme Pouiller wrote:
> On Wednesday 30 January 2013 23:57:13 Yann E. MORIN wrote:
> > So, given two packages, 'foo' and 'bar', what will happen if both have
> > a patch file named 'mktemp.patch' ?
> > 
> >  1- only one will be downloaded
> >  2- it will be applied onto both packages
> And what about
>   $(PKG_NAME)-$(PKG_VERSION)/$(PKG_PATCH_FILENAME).patch

No, we have to keep the patch filename as-is, or apply-patches.sh will
not know it has to uncompress it. So, do not append '.patch' .

Besides, we do not want to create sub-dirs in the DL dir (which can be
a custom dir, as it is in my case).

So, my proposal still stands: ;-)
    $(PKG_NAME)-$(PKG_VERSION)-$(PKG_PATCH_FILENAME)

Regards,
Yann E. MORIN.

Patch hide | download patch | download mbox

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index b0eca0a..37f3a29 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -40,7 +40,7 @@  ifeq ($(DL_MODE),DOWNLOAD)
 	fi
 endif
 	$(if $($(PKG)_SOURCE),$(call DOWNLOAD,$($(PKG)_SITE)/$($(PKG)_SOURCE)))
-	$(foreach p,$($(PKG)_PATCH),$(call DOWNLOAD,$($(PKG)_SITE)/$(p))$(sep))
+	$(foreach p,$($(PKG)_PATCH),$(call DOWNLOAD,$(if $(findstring ://,$(p)),$(p),$($(PKG)_SITE)/$(p)))$(sep))
 	$(foreach hook,$($(PKG)_POST_DOWNLOAD_HOOKS),$(call $(hook))$(sep))
 ifeq ($(DL_MODE),DOWNLOAD)
 	$(Q)mkdir -p $(@D)
@@ -85,7 +85,7 @@  $(BUILD_DIR)/%/.stamp_patched: NAMEVER = $(RAWNAME)-$($(PKG)_VERSION)
 $(BUILD_DIR)/%/.stamp_patched:
 	@$(call MESSAGE,"Patching $($(PKG)_DIR_PREFIX)/$(RAWNAME)")
 	$(foreach hook,$($(PKG)_PRE_PATCH_HOOKS),$(call $(hook))$(sep))
-	$(foreach p,$($(PKG)_PATCH),support/scripts/apply-patches.sh $(@D) $(DL_DIR) $(p)$(sep))
+	$(foreach p,$($(PKG)_PATCH),support/scripts/apply-patches.sh $(@D) $(DL_DIR) $(notdir $(p))$(sep))
 	$(Q)( \
 	if test -d $($(PKG)_DIR_PREFIX)/$(RAWNAME); then \
 	  if test "$(wildcard $($(PKG)_DIR_PREFIX)/$(RAWNAME)/$(NAMEVER)*.patch*)"; then \