Message ID | 1424816770-14271-1-git-send-email-clayton.shotwell@rockwellcollins.com |
---|---|
State | Superseded |
Headers | show |
Dear Clayton Shotwell, On Tue, 24 Feb 2015 16:26:10 -0600, Clayton Shotwell wrote: > In the case when a package has a host version, the package is dependent > on the host version, and the version contains a '/', the host version > does not evaluate properly. The host version will contain a '_' instead > of a '/', resulting in a failed download. To solve this corner case, add > a check to see if the _DL_VERSION of the package has been defined before > defining the host _DL_VERSION. If the package _DL_VERSION has not been > defined yet, then the version string has not been formatted yet and is > still good to use. > > This error occured on a package in a BR2_EXTERNAL that uses a git repo > for its remote storage with '/' in the tag names. I do not believe this > affects any packages in the Buildroot mainline but it could in the > future. > > Signed-off-by: Clayton Shotwell <clayton.shotwell@rockwellcollins.com> So, I had a look at the issue, and I can confirm it. I did the following stupid change: -LIBGLIB2_VERSION = $(LIBGLIB2_VERSION_MAJOR).0 +LIBGLIB2_VERSION = $(LIBGLIB2_VERSION_MAJOR).0/bar (which obviously doesn't work, but we don't care) And then dumped the interesting variables: $ make printvars 2> /dev/null | grep LIBGLIB2 | grep VERSION HOST_LIBGLIB2_DL_VERSION=2.42.0_bar (2.42.0_bar) HOST_LIBGLIB2_VERSION=2.42.0_bar (2.42.0_bar) LIBGLIB2_DL_VERSION=2.42.0/bar (2.42.0/bar) LIBGLIB2_VERSION=2.42.0_bar (2.42.0_bar) So indeed, HOST_LIBGLIB2_DL_VERSION is wrong, it should be '2.42.0/bar'. > diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk > index 1b09955..fcef461 100644 > --- a/package/pkg-generic.mk > +++ b/package/pkg-generic.mk > @@ -323,7 +323,11 @@ $(2)_RAWNAME = $$(patsubst host-%,%,$(1)) > # version control system branch or tag, for example remotes/origin/1_10_stable. > ifndef $(2)_VERSION > ifdef $(3)_VERSION > - $(2)_DL_VERSION := $$(strip $$($(3)_VERSION)) > + ifdef $(3)_DL_VERSION > + $(2)_DL_VERSION := $$(strip $$($(3)_DL_VERSION)) > + else > + $(2)_DL_VERSION := $$(strip $$($(3)_VERSION)) > + endif However, I haven't yet made up my mind on whether this proposed solution is the best one, or if we have a chance of doing something clearer/nicer for this _VERSION vs. _DL_VERSION thing. Thomas
Thomas, On Wed, Mar 4, 2015 at 4:56 PM, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote: > $ make printvars 2> /dev/null | grep LIBGLIB2 | grep VERSION > HOST_LIBGLIB2_DL_VERSION=2.42.0_bar (2.42.0_bar) > HOST_LIBGLIB2_VERSION=2.42.0_bar (2.42.0_bar) > LIBGLIB2_DL_VERSION=2.42.0/bar (2.42.0/bar) > LIBGLIB2_VERSION=2.42.0_bar (2.42.0_bar) > > So indeed, HOST_LIBGLIB2_DL_VERSION is wrong, it should be '2.42.0/bar'. That is exactly what I was running into. > >> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk >> index 1b09955..fcef461 100644 >> --- a/package/pkg-generic.mk >> +++ b/package/pkg-generic.mk >> @@ -323,7 +323,11 @@ $(2)_RAWNAME = $$(patsubst host-%,%,$(1)) >> # version control system branch or tag, for example remotes/origin/1_10_stable. >> ifndef $(2)_VERSION >> ifdef $(3)_VERSION >> - $(2)_DL_VERSION := $$(strip $$($(3)_VERSION)) >> + ifdef $(3)_DL_VERSION >> + $(2)_DL_VERSION := $$(strip $$($(3)_DL_VERSION)) >> + else >> + $(2)_DL_VERSION := $$(strip $$($(3)_VERSION)) >> + endif > > However, I haven't yet made up my mind on whether this proposed > solution is the best one, or if we have a chance of doing something > clearer/nicer for this _VERSION vs. _DL_VERSION thing. This was the simplest solution I could come up with without reworking a lot of the pkg-download logic. I figured that if the _DL_VERSION of non-host version was already set then it should be used, otherwise the _VERSION string should be modified. Thanks, Clayton Clayton Shotwell Senior Software Engineer, Rockwell Collins
Dear Clayton Shotwell, On Wed, 4 Mar 2015 17:17:50 -0600, Clayton Shotwell wrote: > >> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk > >> index 1b09955..fcef461 100644 > >> --- a/package/pkg-generic.mk > >> +++ b/package/pkg-generic.mk > >> @@ -323,7 +323,11 @@ $(2)_RAWNAME = $$(patsubst host-%,%,$(1)) > >> # version control system branch or tag, for example remotes/origin/1_10_stable. > >> ifndef $(2)_VERSION > >> ifdef $(3)_VERSION > >> - $(2)_DL_VERSION := $$(strip $$($(3)_VERSION)) > >> + ifdef $(3)_DL_VERSION > >> + $(2)_DL_VERSION := $$(strip $$($(3)_DL_VERSION)) > >> + else > >> + $(2)_DL_VERSION := $$(strip $$($(3)_VERSION)) > >> + endif > > > > However, I haven't yet made up my mind on whether this proposed > > solution is the best one, or if we have a chance of doing something > > clearer/nicer for this _VERSION vs. _DL_VERSION thing. > > This was the simplest solution I could come up with without reworking > a lot of the pkg-download logic. I figured that if the _DL_VERSION of > non-host version was already set then it should be used, otherwise the > _VERSION string should be modified. Note that I was not saying that your proposal is not good. It's just that it was already almost midnight my time, and it was too late to really think about the right solution :-) Thanks, Thomas
diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk index 1b09955..fcef461 100644 --- a/package/pkg-generic.mk +++ b/package/pkg-generic.mk @@ -323,7 +323,11 @@ $(2)_RAWNAME = $$(patsubst host-%,%,$(1)) # version control system branch or tag, for example remotes/origin/1_10_stable. ifndef $(2)_VERSION ifdef $(3)_VERSION - $(2)_DL_VERSION := $$(strip $$($(3)_VERSION)) + ifdef $(3)_DL_VERSION + $(2)_DL_VERSION := $$(strip $$($(3)_DL_VERSION)) + else + $(2)_DL_VERSION := $$(strip $$($(3)_VERSION)) + endif $(2)_VERSION := $$(subst /,_,$$(strip $$($(3)_VERSION))) else $(2)_VERSION = undefined
In the case when a package has a host version, the package is dependent on the host version, and the version contains a '/', the host version does not evaluate properly. The host version will contain a '_' instead of a '/', resulting in a failed download. To solve this corner case, add a check to see if the _DL_VERSION of the package has been defined before defining the host _DL_VERSION. If the package _DL_VERSION has not been defined yet, then the version string has not been formatted yet and is still good to use. This error occured on a package in a BR2_EXTERNAL that uses a git repo for its remote storage with '/' in the tag names. I do not believe this affects any packages in the Buildroot mainline but it could in the future. Signed-off-by: Clayton Shotwell <clayton.shotwell@rockwellcollins.com> --- package/pkg-generic.mk | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)