| Message ID | fe19a41362f17e9435b0da14097ce9fecda38780.1446577223.git.yann.morin.1998@free.fr |
|---|---|
| State | Changes Requested |
| Headers | show |
On 03-11-15 20:06, Yann E. MORIN wrote: > Some packages misbehave, and install files in either $(STAGING_DIR)/$(O) > or in $(TARGET_DIR)/$(O) > > One common reason for that is that pkgconf now prepends the sysroot path > to all the paths it returns. Other reasons vary, but are mostly due to > poorly writen generic-packages. > > And a new step hooks to check that no file gets installed in either > location, called after the install-target and install-staging steps. > > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr> > Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> > Cc: Gustavo Zacarias <gustavo@zacarias.com.ar> > Cc: Peter Seiderer <ps.report@gmx.net> > Cc: Romain Naour <romain.naour@openwide.fr> > > --- > Note that, in the current state of Buildroot, this *will* cause a lot of > build failures until all those packages are fixed. Among the known to > break are quite a few Xorg packages, plus many packages that need to > call moc (from Qt4 for sure, possibly from Qt5 too). Well, the latter will not cause _new_ build failures, because there the problem is that the moc path returned by pkgconfig is incorrect, while it is installed in the right place. There are a few things in here that I don't like, but I don't want to block the urgency. > --- > package/pkg-generic.mk | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk > index 3db616c..59b0938 100644 > --- a/package/pkg-generic.mk > +++ b/package/pkg-generic.mk > @@ -87,6 +87,21 @@ define step_pkg_size > endef > GLOBAL_INSTRUMENTATION_HOOKS += step_pkg_size > > +define step_check_build_dir_one > + if [ -d $(2) ]; then \ > + printf "%s: installs files in %s\n" $(1) $(2) >&2; \ > + exit 1; \ > + fi I think this could also be $(if $(wildcard $(2)),$(error $(1) installs files in $(2))) but not sure if that is better readable. > +endef > + > +define step_check_build_dir > + $(if $(filter install-staging,$(2)),\ > + $(if $(filter end,$(1)),$(call step_check_build_dir_one,$(3),$(STAGING_DIR)/$(O)))) Is this correct? Shouldn't that be $(HOST_DIR) instead of $(O)? If a custom BR2_HOST_DIR is configured, then the wrong pkgconfig stuff will put stuff into $(STAGING_DIR)/$(HOST_DIR), no? > + $(if $(filter install-target,$(2)),\ > + $(if $(filter end,$(1)),$(call step_check_build_dir_one,$(3),$(TARGET_DIR)/$(O)))) Actually, I wonder if it shouldn't be HOST_DIR here as well. Perhaps, to be sure, both should be checked. > +endef > +GLOBAL_INSTRUMENTATION_HOOKS += step_check_build_dir I don't see much point to add this as instrumentation hooks, it could be done directly in the .stamp rule. Note that this patch will make it impossible to have $(O) equal to some path that happens to exist on the target, e.g. O=/tmp/build if /tmp/build happens to exist in the custom skeleton or rootfs overlay. (Same story with HOST_DIR of O is replaced by HOST_DIR above). Granted, it's not very likely. Regards, Arnout > + > # User-supplied script > ifneq ($(BR2_INSTRUMENTATION_SCRIPTS),) > define step_user >
Arnout, All, On 2015-11-03 23:31 +0100, Arnout Vandecappelle spake thusly: > On 03-11-15 20:06, Yann E. MORIN wrote: > > Some packages misbehave, and install files in either $(STAGING_DIR)/$(O) > > or in $(TARGET_DIR)/$(O) > > > > One common reason for that is that pkgconf now prepends the sysroot path > > to all the paths it returns. Other reasons vary, but are mostly due to > > poorly writen generic-packages. > > > > And a new step hooks to check that no file gets installed in either > > location, called after the install-target and install-staging steps. > > > > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr> > > Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> > > Cc: Gustavo Zacarias <gustavo@zacarias.com.ar> > > Cc: Peter Seiderer <ps.report@gmx.net> > > Cc: Romain Naour <romain.naour@openwide.fr> > > > > --- > > Note that, in the current state of Buildroot, this *will* cause a lot of > > build failures until all those packages are fixed. Among the known to > > break are quite a few Xorg packages, plus many packages that need to > > call moc (from Qt4 for sure, possibly from Qt5 too). > > Well, the latter will not cause _new_ build failures, because there the problem > is that the moc path returned by pkgconfig is incorrect, while it is installed > in the right place. Right. That would not be new failures. We would not even catch it with this patch, since moc *is* installed in the correct place. We would just notice when a moc-using package tries to use (like the failure noticed by Peter S.). > There are a few things in here that I don't like, but I don't want to block the > urgency. > > > --- > > package/pkg-generic.mk | 15 +++++++++++++++ > > 1 file changed, 15 insertions(+) > > > > diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk > > index 3db616c..59b0938 100644 > > --- a/package/pkg-generic.mk > > +++ b/package/pkg-generic.mk > > @@ -87,6 +87,21 @@ define step_pkg_size > > endef > > GLOBAL_INSTRUMENTATION_HOOKS += step_pkg_size > > > > +define step_check_build_dir_one > > + if [ -d $(2) ]; then \ > > + printf "%s: installs files in %s\n" $(1) $(2) >&2; \ > > + exit 1; \ > > + fi > > I think this could also be > > $(if $(wildcard $(2)),$(error $(1) installs files in $(2))) > > but not sure if that is better readable. Well, I'm always a bit uneasy at using a Makefile construct in a place where commands are expected (i.e. part of a rule). So I'd prefer we stick to using shell code here. And as you said, it is not muh mor ereadable. > > +endef > > + > > +define step_check_build_dir > > + $(if $(filter install-staging,$(2)),\ > > + $(if $(filter end,$(1)),$(call step_check_build_dir_one,$(3),$(STAGING_DIR)/$(O)))) > > Is this correct? Shouldn't that be $(HOST_DIR) instead of $(O)? If a custom > BR2_HOST_DIR is configured, then the wrong pkgconfig stuff will put stuff into > $(STAGING_DIR)/$(HOST_DIR), no? Hmmm... It looks like you are right. I'll do a few tests here, with various values for HOST_DIR (in $(O) and elsewhere) and wee what we get as a result. Still, we may also want to catch packages that use PREFIX=$(TARGET_DIR) and DESTDIR=$(TARGET_DIR) which would ultimately lead to $(TARGET_DIR)/$(TARGET_DIR) (and since $(TARGET_DIR) is a sub-dir of $(O), we'd catch that as well. So I think we should check both XXX/$(O) and XXX/$(HOST_DIR) in both the staging and target installs. > > + $(if $(filter install-target,$(2)),\ > > + $(if $(filter end,$(1)),$(call step_check_build_dir_one,$(3),$(TARGET_DIR)/$(O)))) > > Actually, I wonder if it shouldn't be HOST_DIR here as well. Ditto. > Perhaps, to be sure, both should be checked. Yes. > > +endef > > +GLOBAL_INSTRUMENTATION_HOOKS += step_check_build_dir > > I don't see much point to add this as instrumentation hooks, it could be done > directly in the .stamp rule. > > > Note that this patch will make it impossible to have $(O) equal to some path > that happens to exist on the target, e.g. O=/tmp/build if /tmp/build happens to > exist in the custom skeleton or rootfs overlay. (Same story with HOST_DIR of O > is replaced by HOST_DIR above). Granted, it's not very likely. Hmm.. That's indeed a bit annoying... So, if we have HOST_DIR=/tmp then the tests above are broken indeed... OK, I'll investigate more... Thanks! :-) Regards, Yann E. MORIN. > Regards, > Arnout > > > + > > # User-supplied script > > ifneq ($(BR2_INSTRUMENTATION_SCRIPTS),) > > define step_user > > > > > -- > Arnout Vandecappelle arnout at mind be > Senior Embedded Software Architect +32-16-286500 > Essensium/Mind http://www.mind.be > G.Geenslaan 9, 3001 Leuven, Belgium BE 872 984 063 RPR Leuven > LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle > GPG fingerprint: 7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF
diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk index 3db616c..59b0938 100644 --- a/package/pkg-generic.mk +++ b/package/pkg-generic.mk @@ -87,6 +87,21 @@ define step_pkg_size endef GLOBAL_INSTRUMENTATION_HOOKS += step_pkg_size +define step_check_build_dir_one + if [ -d $(2) ]; then \ + printf "%s: installs files in %s\n" $(1) $(2) >&2; \ + exit 1; \ + fi +endef + +define step_check_build_dir + $(if $(filter install-staging,$(2)),\ + $(if $(filter end,$(1)),$(call step_check_build_dir_one,$(3),$(STAGING_DIR)/$(O)))) + $(if $(filter install-target,$(2)),\ + $(if $(filter end,$(1)),$(call step_check_build_dir_one,$(3),$(TARGET_DIR)/$(O)))) +endef +GLOBAL_INSTRUMENTATION_HOOKS += step_check_build_dir + # User-supplied script ifneq ($(BR2_INSTRUMENTATION_SCRIPTS),) define step_user
Some packages misbehave, and install files in either $(STAGING_DIR)/$(O) or in $(TARGET_DIR)/$(O) One common reason for that is that pkgconf now prepends the sysroot path to all the paths it returns. Other reasons vary, but are mostly due to poorly writen generic-packages. And a new step hooks to check that no file gets installed in either location, called after the install-target and install-staging steps. Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> Cc: Gustavo Zacarias <gustavo@zacarias.com.ar> Cc: Peter Seiderer <ps.report@gmx.net> Cc: Romain Naour <romain.naour@openwide.fr> --- Note that, in the current state of Buildroot, this *will* cause a lot of build failures until all those packages are fixed. Among the known to break are quite a few Xorg packages, plus many packages that need to call moc (from Qt4 for sure, possibly from Qt5 too). --- package/pkg-generic.mk | 15 +++++++++++++++ 1 file changed, 15 insertions(+)