| Message ID | 1346600635-30946-11-git-send-email-yann.morin.1998@free.fr |
|---|---|
| State | Superseded |
| Headers | show |
On 09/02/12 17:43, Yann E. MORIN wrote: > If the gettext package is selected, it means that the toolchain does not > provide gettext functionality, and that at least one package needs it. > Thus, the gettext package needs to be built before any such package. > > So far, packages that need gettext declare an explicit conditional > build-dependency upon gettext, thus ensuring that gettext be built > before they are. > > But if gettext was built first, packages would no longer need to declare > such an explicit build-dependency. > > As it is not easy to ensure that gettext be the first package, we can also > declare it as a dependency of all packages (except itself, of course). I don't like this. I don't like the idea of having redundant dependencies, I don't like to have a single thing that everything depends on, I don't like the $(filter-out ...) construct. I also don't like that it breaks the build when the toolchain depends on some package (sstrip, ccache, ...). Possible alternatives: * Use a construct similar to patch 11/12: $(gettext-dependency) which is nothing unless gettext is enabled. * Move this dependency into the toolchain logic - which means duplicating it three times so maybe not such a good idea... Regards, Arnout
Arnout, All, On Sunday 02 September 2012 22:30:48 Arnout Vandecappelle wrote: > On 09/02/12 17:43, Yann E. MORIN wrote: > > As it is not easy to ensure that gettext be the first package, we can also > > declare it as a dependency of all packages (except itself, of course). > > I don't like this. Well, it's not ideal. I am not too fond of it either. The reason for this patch was to avoid adding gettext to _DEPENDENCIES, because packages have different ways of doing so: ifeq ($(BR2_PACKAGE_GETTEXT),y) xxx_DEPENDENCIES += gettext endif or: xxx_DEPENDENCIES += $(if $(BR2_PACKAGE_GETTEXT),gettext) Without counting packages that test: BR2_PACKAGE_GETTEXT BR2_NEEDS_GETTEXT BR2_NEEDS_GETTEXT_IF_LOCALE and packages that need to add explicit link against -lintl But I'll drop that patch. > I also don't like that it breaks the build when the toolchain depends on some > package (sstrip, ccache, ...). Ah, I did test with an external toolchain. > Possible alternatives: > * Use a construct similar to patch 11/12: $(gettext-dependency) which is > nothing unless gettext is enabled. Will do that. > * Move this dependency into the toolchain logic - which means duplicating > it three times so maybe not such a good idea... Not sure I understand this... O_o Thanks for the review! Regards, Yann E. MORIN.
On 09/02/12 23:05, Yann E. MORIN wrote: >> > * Move this dependency into the toolchain logic - which means duplicating >> > it three times so maybe not such a good idea... > Not sure I understand this... O_o All the more reason not to do it :-) I'm currently running a test build with the whole patch series (barring 10/12 and 12/12) - result in an hour or so. Regards, Arnout
Le Sun, 02 Sep 2012 22:30:48 +0200, Arnout Vandecappelle <arnout@mind.be> a écrit : > I don't like this. I don't like the idea of having redundant > dependencies, I don't like to have a single thing that everything > depends on, I don't like the $(filter-out ...) construct. > > I also don't like that it breaks the build when the toolchain > depends on some package (sstrip, ccache, ...). > > Possible alternatives: > > * Use a construct similar to patch 11/12: $(gettext-dependency) > which is nothing unless gettext is enabled. > > * Move this dependency into the toolchain logic - which means > duplicating it three times so maybe not such a good idea... I don't like it either, and I don't think I like those alternatives either. I prefer keeping the explicit dependency in the packages, because I don't like things that are too magic. Best regards, Thomas
On 09/03/12 11:09, Thomas Petazzoni wrote: > Le Sun, 02 Sep 2012 22:30:48 +0200, > Arnout Vandecappelle<arnout@mind.be> a écrit : > >> I don't like this. I don't like the idea of having redundant >> dependencies, I don't like to have a single thing that everything >> depends on, I don't like the $(filter-out ...) construct. >> >> I also don't like that it breaks the build when the toolchain >> depends on some package (sstrip, ccache, ...). >> >> Possible alternatives: >> >> * Use a construct similar to patch 11/12: $(gettext-dependency) >> which is nothing unless gettext is enabled. >> >> * Move this dependency into the toolchain logic - which means >> duplicating it three times so maybe not such a good idea... > > I don't like it either, and I don't think I like those alternatives > either. > > I prefer keeping the explicit dependency in the packages, because I > don't like things that are too magic. With $(gettext-dependency), the dependency is explicit. You'll see in Yann's next patch series :-) Regards, Arnout
diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk index ffe7dfb..b731ff0 100644 --- a/package/pkg-generic.mk +++ b/package/pkg-generic.mk @@ -367,7 +367,8 @@ $(1)-patch: $(1)-extract $$($(2)_TARGET_PATCH) $(1)-extract: $(1)-source \ $$($(2)_TARGET_EXTRACT) -$(1)-depends: $$($(2)_DEPENDENCIES) +$(1)-depends: $(filter-out $(1),$(if $(BR2_PACKAGE_GETTEXT),gettext)) \ + $$($(2)_DEPENDENCIES) $(1)-source: $$($(2)_TARGET_SOURCE) else @@ -378,7 +379,9 @@ else $(1)-configure: $(1)-depends \ $$($(2)_TARGET_CONFIGURE) -$(1)-depends: $(1)-rsync $$($(2)_DEPENDENCIES) +$(1)-depends: $(1)-rsync \ + $(filter-out $(1),$(if $(BR2_PACKAGE_GETTEXT),gettext)) \ + $$($(2)_DEPENDENCIES) $(1)-rsync: $$($(2)_TARGET_RSYNC) @@ -386,7 +389,7 @@ $(1)-source: $$($(2)_TARGET_RSYNC_SOURCE) endif $(1)-show-depends: - @echo $$($(2)_DEPENDENCIES) + @echo $(filter-out $(1),$(if $(BR2_PACKAGE_GETTEXT),gettext)) $$($(2)_DEPENDENCIES) $(1)-uninstall: $(1)-configure $$($(2)_TARGET_UNINSTALL)
If the gettext package is selected, it means that the toolchain does not provide gettext functionality, and that at least one package needs it. Thus, the gettext package needs to be built before any such package. So far, packages that need gettext declare an explicit conditional build-dependency upon gettext, thus ensuring that gettext be built before they are. But if gettext was built first, packages would no longer need to declare such an explicit build-dependency. As it is not easy to ensure that gettext be the first package, we can also declare it as a dependency of all packages (except itself, of course). Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr> --- package/pkg-generic.mk | 9 ++++++--- 1 files changed, 6 insertions(+), 3 deletions(-)