[V2,4/4,RFC] pkg-perl: add per package upgrade target

Message ID 20181011161248.13457-5-francois.perrad@gadz.org
State Superseded
Headers show
Series
  • scancpan
Related show

Commit Message

Francois Perrad Oct. 11, 2018, 4:12 p.m.
Signed-off-by: Francois Perrad <francois.perrad@gadz.org>
---
 package/pkg-generic.mk |  6 +++++-
 package/pkg-perl.mk    | 11 +++++++++++
 2 files changed, 16 insertions(+), 1 deletion(-)

Comments

Arnout Vandecappelle Oct. 23, 2018, 11:57 p.m. | #1
On 10/11/18 5:12 PM, Francois Perrad wrote:
> Signed-off-by: Francois Perrad <francois.perrad@gadz.org>
> ---
>  package/pkg-generic.mk |  6 +++++-
>  package/pkg-perl.mk    | 11 +++++++++++
>  2 files changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index 91b61c6de..f349c6c1f 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -779,6 +779,9 @@ $(1)-external-deps:
>  	@echo "file://$$($(2)_OVERRIDE_SRCDIR)"
>  endif
>  
> +$(1)-upgrade:
> +			+$$($(2)_UPGRADE_CMDS)
> +

 Oh, I misremembered - I thought you had put this bit in pkg-perl.mk. As noted
in the scanrock series, I don't think there should be a generic rule, only a
perl- and luarocks-specific one. So I've moved this to pkg-perl.mk.

>  $(1)-show-version:
>  			@echo $$($(2)_VERSION)
>  
> @@ -1030,7 +1033,8 @@ DL_TOOLS_DEPENDENCIES += $$(call extractor-dependency,$$($(2)_SOURCE))
>  	$(1)-rsync \
>  	$(1)-show-depends \
>  	$(1)-show-version \
> -	$(1)-source
> +	$(1)-source \
> +	$(1)-upgrade
>  
>  ifneq ($$($(2)_SOURCE),)
>  ifeq ($$($(2)_SITE),)
> diff --git a/package/pkg-perl.mk b/package/pkg-perl.mk
> index 74a116075..53413b3eb 100644
> --- a/package/pkg-perl.mk
> +++ b/package/pkg-perl.mk
> @@ -61,6 +61,17 @@ ifeq ($(4),target)
>  SCANCPAN_ALL_DISTRIB += $$($(2)_DISTNAME)
>  endif
>  
> +# Upgrade helper
> +ifeq ($(4),target)
> +define $(2)_UPGRADE_CMDS

 With this in pkg-perl.mk, there is no longer a need for the variable.

> +	utils/scancpan -force -target $$($(3)_DISTNAME)

 The condition is also not needed, since we can use -$(4) directly here.

 Instead, I've made the entire rule conditional on $(3)_DISTNAME not being empty.

 I've applied to master with all these changes. In the end, it doesn't resemble
what you submitted at all anymore, but I've kept you as the author :-).

 I've tried it out, and I think you'll want to improve the scancpan script a
little. For example, it would be better if it wouldn't touch a package if its
version hasn't changed. Printing the entire package/Config.in hunk is also
inconvenient. So just for your own sanity, I think you can improve things there
a little :-), e.g. passing an -upgrade $(PKG_VERSION) argument instead of -force.

 Regards,
 Arnout


> +endef
> +else
> +define $(2)_UPGRADE_CMDS
> +	utils/scancpan -force -host $$($(3)_DISTNAME)
> +endef
> +endif
> +
>  #
>  # Configure step. Only define it if not already defined by the package
>  # .mk file. And take care of the differences between host and target
>
François Perrad Oct. 24, 2018, 10:28 a.m. | #2
Le mer. 24 oct. 2018 à 01:57, Arnout Vandecappelle <arnout@mind.be> a
écrit :

>
>
> On 10/11/18 5:12 PM, Francois Perrad wrote:
> > Signed-off-by: Francois Perrad <francois.perrad@gadz.org>
> > ---
> >  package/pkg-generic.mk |  6 +++++-
> >  package/pkg-perl.mk    | 11 +++++++++++
> >  2 files changed, 16 insertions(+), 1 deletion(-)
> >
> > diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> > index 91b61c6de..f349c6c1f 100644
> > --- a/package/pkg-generic.mk
> > +++ b/package/pkg-generic.mk
> > @@ -779,6 +779,9 @@ $(1)-external-deps:
> >       @echo "file://$$($(2)_OVERRIDE_SRCDIR)"
> >  endif
> >
> > +$(1)-upgrade:
> > +                     +$$($(2)_UPGRADE_CMDS)
> > +
>
>  Oh, I misremembered - I thought you had put this bit in pkg-perl.mk. As
> noted
> in the scanrock series, I don't think there should be a generic rule, only
> a
> perl- and luarocks-specific one. So I've moved this to pkg-perl.mk.
>
> >  $(1)-show-version:
> >                       @echo $$($(2)_VERSION)
> >
> > @@ -1030,7 +1033,8 @@ DL_TOOLS_DEPENDENCIES += $$(call
> extractor-dependency,$$($(2)_SOURCE))
> >       $(1)-rsync \
> >       $(1)-show-depends \
> >       $(1)-show-version \
> > -     $(1)-source
> > +     $(1)-source \
> > +     $(1)-upgrade
> >
> >  ifneq ($$($(2)_SOURCE),)
> >  ifeq ($$($(2)_SITE),)
> > diff --git a/package/pkg-perl.mk b/package/pkg-perl.mk
> > index 74a116075..53413b3eb 100644
> > --- a/package/pkg-perl.mk
> > +++ b/package/pkg-perl.mk
> > @@ -61,6 +61,17 @@ ifeq ($(4),target)
> >  SCANCPAN_ALL_DISTRIB += $$($(2)_DISTNAME)
> >  endif
> >
> > +# Upgrade helper
> > +ifeq ($(4),target)
> > +define $(2)_UPGRADE_CMDS
>
>  With this in pkg-perl.mk, there is no longer a need for the variable.
>
> > +     utils/scancpan -force -target $$($(3)_DISTNAME)
>
>  The condition is also not needed, since we can use -$(4) directly here.
>
>  Instead, I've made the entire rule conditional on $(3)_DISTNAME not being
> empty.
>
>  I've applied to master with all these changes. In the end, it doesn't
> resemble
> what you submitted at all anymore, but I've kept you as the author :-).
>
>  I've tried it out, and I think you'll want to improve the scancpan script
> a
> little. For example, it would be better if it wouldn't touch a package if
> its
> version hasn't changed. Printing the entire package/Config.in hunk is also
> inconvenient. So just for your own sanity, I think you can improve things
> there
> a little :-), e.g. passing an -upgrade $(PKG_VERSION) argument instead of
> -force.
>
>
Thanks for this remark. See https://patchwork.ozlabs.org/patch/988493/

François


>  Regards,
>  Arnout
>
>
> > +endef
> > +else
> > +define $(2)_UPGRADE_CMDS
> > +     utils/scancpan -force -host $$($(3)_DISTNAME)
> > +endef
> > +endif
> > +
> >  #
> >  # Configure step. Only define it if not already defined by the package
> >  # .mk file. And take care of the differences between host and target
> >
> _______________________________________________
> buildroot mailing list
> buildroot@busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
>
<div dir="ltr"><div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">Le mer. 24 oct. 2018 à 01:57, Arnout Vandecappelle &lt;<a href="mailto:arnout@mind.be">arnout@mind.be</a>&gt; a écrit :<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br>
<br>
On 10/11/18 5:12 PM, Francois Perrad wrote:<br>
&gt; Signed-off-by: Francois Perrad &lt;<a href="mailto:francois.perrad@gadz.org" target="_blank">francois.perrad@gadz.org</a>&gt;<br>
&gt; ---<br>
&gt;  package/<a href="http://pkg-generic.mk" rel="noreferrer" target="_blank">pkg-generic.mk</a> |  6 +++++-<br>
&gt;  package/<a href="http://pkg-perl.mk" rel="noreferrer" target="_blank">pkg-perl.mk</a>    | 11 +++++++++++<br>
&gt;  2 files changed, 16 insertions(+), 1 deletion(-)<br>
&gt; <br>
&gt; diff --git a/package/<a href="http://pkg-generic.mk" rel="noreferrer" target="_blank">pkg-generic.mk</a> b/package/<a href="http://pkg-generic.mk" rel="noreferrer" target="_blank">pkg-generic.mk</a><br>
&gt; index 91b61c6de..f349c6c1f 100644<br>
&gt; --- a/package/<a href="http://pkg-generic.mk" rel="noreferrer" target="_blank">pkg-generic.mk</a><br>
&gt; +++ b/package/<a href="http://pkg-generic.mk" rel="noreferrer" target="_blank">pkg-generic.mk</a><br>
&gt; @@ -779,6 +779,9 @@ $(1)-external-deps:<br>
&gt;       @echo &quot;file://$$($(2)_OVERRIDE_SRCDIR)&quot;<br>
&gt;  endif<br>
&gt;  <br>
&gt; +$(1)-upgrade:<br>
&gt; +                     +$$($(2)_UPGRADE_CMDS)<br>
&gt; +<br>
<br>
 Oh, I misremembered - I thought you had put this bit in <a href="http://pkg-perl.mk" rel="noreferrer" target="_blank">pkg-perl.mk</a>. As noted<br>
in the scanrock series, I don&#39;t think there should be a generic rule, only a<br>
perl- and luarocks-specific one. So I&#39;ve moved this to <a href="http://pkg-perl.mk" rel="noreferrer" target="_blank">pkg-perl.mk</a>.<br>
<br>
&gt;  $(1)-show-version:<br>
&gt;                       @echo $$($(2)_VERSION)<br>
&gt;  <br>
&gt; @@ -1030,7 +1033,8 @@ DL_TOOLS_DEPENDENCIES += $$(call extractor-dependency,$$($(2)_SOURCE))<br>
&gt;       $(1)-rsync \<br>
&gt;       $(1)-show-depends \<br>
&gt;       $(1)-show-version \<br>
&gt; -     $(1)-source<br>
&gt; +     $(1)-source \<br>
&gt; +     $(1)-upgrade<br>
&gt;  <br>
&gt;  ifneq ($$($(2)_SOURCE),)<br>
&gt;  ifeq ($$($(2)_SITE),)<br>
&gt; diff --git a/package/<a href="http://pkg-perl.mk" rel="noreferrer" target="_blank">pkg-perl.mk</a> b/package/<a href="http://pkg-perl.mk" rel="noreferrer" target="_blank">pkg-perl.mk</a><br>
&gt; index 74a116075..53413b3eb 100644<br>
&gt; --- a/package/<a href="http://pkg-perl.mk" rel="noreferrer" target="_blank">pkg-perl.mk</a><br>
&gt; +++ b/package/<a href="http://pkg-perl.mk" rel="noreferrer" target="_blank">pkg-perl.mk</a><br>
&gt; @@ -61,6 +61,17 @@ ifeq ($(4),target)<br>
&gt;  SCANCPAN_ALL_DISTRIB += $$($(2)_DISTNAME)<br>
&gt;  endif<br>
&gt;  <br>
&gt; +# Upgrade helper<br>
&gt; +ifeq ($(4),target)<br>
&gt; +define $(2)_UPGRADE_CMDS<br>
<br>
 With this in <a href="http://pkg-perl.mk" rel="noreferrer" target="_blank">pkg-perl.mk</a>, there is no longer a need for the variable.<br>
<br>
&gt; +     utils/scancpan -force -target $$($(3)_DISTNAME)<br>
<br>
 The condition is also not needed, since we can use -$(4) directly here.<br>
<br>
 Instead, I&#39;ve made the entire rule conditional on $(3)_DISTNAME not being empty.<br>
<br>
 I&#39;ve applied to master with all these changes. In the end, it doesn&#39;t resemble<br>
what you submitted at all anymore, but I&#39;ve kept you as the author :-).<br>
<br>
 I&#39;ve tried it out, and I think you&#39;ll want to improve the scancpan script a<br>
little. For example, it would be better if it wouldn&#39;t touch a package if its<br>
version hasn&#39;t changed. Printing the entire package/Config.in hunk is also<br>
inconvenient. So just for your own sanity, I think you can improve things there<br>
a little :-), e.g. passing an -upgrade $(PKG_VERSION) argument instead of -force.<br>
<br></blockquote><div><br></div><div>Thanks for this remark. See <a href="https://patchwork.ozlabs.org/patch/988493/">https://patchwork.ozlabs.org/patch/988493/</a><br></div><div><br></div><div>François<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
 Regards,<br>
 Arnout<br>
<br>
<br>
&gt; +endef<br>
&gt; +else<br>
&gt; +define $(2)_UPGRADE_CMDS<br>
&gt; +     utils/scancpan -force -host $$($(3)_DISTNAME)<br>
&gt; +endef<br>
&gt; +endif<br>
&gt; +<br>
&gt;  #<br>
&gt;  # Configure step. Only define it if not already defined by the package<br>
&gt;  # .mk file. And take care of the differences between host and target<br>
&gt; <br>
_______________________________________________<br>
buildroot mailing list<br>
<a href="mailto:buildroot@busybox.net" target="_blank">buildroot@busybox.net</a><br>
<a href="http://lists.busybox.net/mailman/listinfo/buildroot" rel="noreferrer" target="_blank">http://lists.busybox.net/mailman/listinfo/buildroot</a><br>
</blockquote></div></div></div>

Patch

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 91b61c6de..f349c6c1f 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -779,6 +779,9 @@  $(1)-external-deps:
 	@echo "file://$$($(2)_OVERRIDE_SRCDIR)"
 endif
 
+$(1)-upgrade:
+			+$$($(2)_UPGRADE_CMDS)
+
 $(1)-show-version:
 			@echo $$($(2)_VERSION)
 
@@ -1030,7 +1033,8 @@  DL_TOOLS_DEPENDENCIES += $$(call extractor-dependency,$$($(2)_SOURCE))
 	$(1)-rsync \
 	$(1)-show-depends \
 	$(1)-show-version \
-	$(1)-source
+	$(1)-source \
+	$(1)-upgrade
 
 ifneq ($$($(2)_SOURCE),)
 ifeq ($$($(2)_SITE),)
diff --git a/package/pkg-perl.mk b/package/pkg-perl.mk
index 74a116075..53413b3eb 100644
--- a/package/pkg-perl.mk
+++ b/package/pkg-perl.mk
@@ -61,6 +61,17 @@  ifeq ($(4),target)
 SCANCPAN_ALL_DISTRIB += $$($(2)_DISTNAME)
 endif
 
+# Upgrade helper
+ifeq ($(4),target)
+define $(2)_UPGRADE_CMDS
+	utils/scancpan -force -target $$($(3)_DISTNAME)
+endef
+else
+define $(2)_UPGRADE_CMDS
+	utils/scancpan -force -host $$($(3)_DISTNAME)
+endef
+endif
+
 #
 # Configure step. Only define it if not already defined by the package
 # .mk file. And take care of the differences between host and target