| Message ID | 20181011161248.13457-5-francois.perrad@gadz.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | scancpan | expand |
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 >
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 <<a href="mailto:arnout@mind.be">arnout@mind.be</a>> 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> > Signed-off-by: Francois Perrad <<a href="mailto:francois.perrad@gadz.org" target="_blank">francois.perrad@gadz.org</a>><br> > ---<br> > package/<a href="http://pkg-generic.mk" rel="noreferrer" target="_blank">pkg-generic.mk</a> | 6 +++++-<br> > package/<a href="http://pkg-perl.mk" rel="noreferrer" target="_blank">pkg-perl.mk</a> | 11 +++++++++++<br> > 2 files changed, 16 insertions(+), 1 deletion(-)<br> > <br> > 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> > index 91b61c6de..f349c6c1f 100644<br> > --- a/package/<a href="http://pkg-generic.mk" rel="noreferrer" target="_blank">pkg-generic.mk</a><br> > +++ b/package/<a href="http://pkg-generic.mk" rel="noreferrer" target="_blank">pkg-generic.mk</a><br> > @@ -779,6 +779,9 @@ $(1)-external-deps:<br> > @echo "file://$$($(2)_OVERRIDE_SRCDIR)"<br> > endif<br> > <br> > +$(1)-upgrade:<br> > + +$$($(2)_UPGRADE_CMDS)<br> > +<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't think there should be a generic rule, only a<br> perl- and luarocks-specific one. So I've moved this to <a href="http://pkg-perl.mk" rel="noreferrer" target="_blank">pkg-perl.mk</a>.<br> <br> > $(1)-show-version:<br> > @echo $$($(2)_VERSION)<br> > <br> > @@ -1030,7 +1033,8 @@ DL_TOOLS_DEPENDENCIES += $$(call extractor-dependency,$$($(2)_SOURCE))<br> > $(1)-rsync \<br> > $(1)-show-depends \<br> > $(1)-show-version \<br> > - $(1)-source<br> > + $(1)-source \<br> > + $(1)-upgrade<br> > <br> > ifneq ($$($(2)_SOURCE),)<br> > ifeq ($$($(2)_SITE),)<br> > 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> > index 74a116075..53413b3eb 100644<br> > --- a/package/<a href="http://pkg-perl.mk" rel="noreferrer" target="_blank">pkg-perl.mk</a><br> > +++ b/package/<a href="http://pkg-perl.mk" rel="noreferrer" target="_blank">pkg-perl.mk</a><br> > @@ -61,6 +61,17 @@ ifeq ($(4),target)<br> > SCANCPAN_ALL_DISTRIB += $$($(2)_DISTNAME)<br> > endif<br> > <br> > +# Upgrade helper<br> > +ifeq ($(4),target)<br> > +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> > + 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've made the entire rule conditional on $(3)_DISTNAME not being empty.<br> <br> I've applied to master with all these changes. In the end, it doesn't resemble<br> what you submitted at all anymore, but I've kept you as the author :-).<br> <br> I've tried it out, and I think you'll want to improve the scancpan script a<br> little. For example, it would be better if it wouldn't touch a package if its<br> version hasn'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> > +endef<br> > +else<br> > +define $(2)_UPGRADE_CMDS<br> > + utils/scancpan -force -host $$($(3)_DISTNAME)<br> > +endef<br> > +endif<br> > +<br> > #<br> > # Configure step. Only define it if not already defined by the package<br> > # .mk file. And take care of the differences between host and target<br> > <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>
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
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(-)