| Message ID | 20180518104524.4630-1-arnout@mind.be |
|---|---|
| State | Superseded |
| Headers | show |
| Series | pkg-generic: error out with 'local' site method and no _SITE | expand |
Hello, On Fri, 18 May 2018 12:45:24 +0200, Arnout Vandecappelle (Essensium/Mind) wrote: > +ifeq ($$($(2)_OVERRIDE_SRCDIR),) > +$$(error $(1) has local site method, but `$(2)_SITE_METHOD` is not defined) > +endif Are you sure it shouldn't be "$(2)_SITE" is not defined ? Thomas
On 19-05-18 10:38, Thomas Petazzoni wrote: > Hello, > > On Fri, 18 May 2018 12:45:24 +0200, Arnout Vandecappelle > (Essensium/Mind) wrote: > >> +ifeq ($$($(2)_OVERRIDE_SRCDIR),) >> +$$(error $(1) has local site method, but `$(2)_SITE_METHOD` is not defined) >> +endif > > Are you sure it shouldn't be "$(2)_SITE" is not defined ? That was actually intentional. The idea is to allow 'local' packages to set either _SITE or _OVERRIDE_SRCDIR. The check comes after the assignment to _OVERRIDE_SRCDIR. The reason for this is: currently, 'local' packages work correctly if you set _OVERRIDE_SRCDIR instead of _SITE, and I don't see a reason to make them fail. I probably should have explained this in the commit log. It was a bit short :-) Regards, Arnout
>>>>> "Arnout" == Arnout Vandecappelle <arnout@mind.be> writes: > On 19-05-18 10:38, Thomas Petazzoni wrote: >> Hello, >> >> On Fri, 18 May 2018 12:45:24 +0200, Arnout Vandecappelle >> (Essensium/Mind) wrote: >> >>> +ifeq ($$($(2)_OVERRIDE_SRCDIR),) >>> +$$(error $(1) has local site method, but `$(2)_SITE_METHOD` is not defined) >>> +endif >> >> Are you sure it shouldn't be "$(2)_SITE" is not defined ? > That was actually intentional. The idea is to allow 'local' packages to set > either _SITE or _OVERRIDE_SRCDIR. The check comes after the assignment to > _OVERRIDE_SRCDIR. The reason for this is: currently, 'local' packages work > correctly if you set _OVERRIDE_SRCDIR instead of _SITE, and I don't see a reason > to make them fail. > I probably should have explained this in the commit log. It was a bit short :-) Indeed! Will you send an update?
diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk index 8a3b5f90a9..988f2d34e3 100644 --- a/package/pkg-generic.mk +++ b/package/pkg-generic.mk @@ -520,6 +520,9 @@ ifeq ($$($(2)_SITE_METHOD),local) ifeq ($$($(2)_OVERRIDE_SRCDIR),) $(2)_OVERRIDE_SRCDIR = $$($(2)_SITE) endif +ifeq ($$($(2)_OVERRIDE_SRCDIR),) +$$(error $(1) has local site method, but `$(2)_SITE_METHOD` is not defined) +endif endif ifndef $(2)_LICENSE
The 'local' site method is easily confused with the 'file' site method, making people create packages like this: FOO_SITE_METHOD = local FOO_SOURCE = foo.tar.gz $(eval $(generic-package)) Due to the intricacies of the generic package infra, this does not cause an error; instead, the foo.tar.gz tarball that happens to be present in the download directory will be used. This behaviour differs greatly from what is specified in the manual. Instead, error out immediately if a package specifies the 'local' site method but does not specify a _SITE. See also https://stackoverflow.com/questions/50364655/including-patches-to-build-root Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be> --- package/pkg-generic.mk | 3 +++ 1 file changed, 3 insertions(+)