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

login
register
mail settings
Submitter Jérôme Pouiller
Date Jan. 18, 2013, 4:11 p.m.
Message ID <1358525515-7295-1-git-send-email-jezz@sysmic.org>
Download mbox | patch
Permalink /patch/213668/
State Superseded
Headers show

Comments

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(-)
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

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 \