| Message ID | CAHkwnC_exQsbt7e9CJrzO6FA8HQGjJMWmVOkevUxjSYpHbdvJw@mail.gmail.com |
|---|---|
| State | Not Applicable |
| Headers | show |
Dear Fabio Porcedda,
On Thu, 12 Jun 2014 09:32:39 +0200, Fabio Porcedda wrote:
> I think it's best to use PURGE_LOCALES_HOOK instead of TARGET_PURGE_LOCALES.
I think one of the concern with the hook thing was that the hooks would
be spread all over the Buildroot tree. To mitigate this concern, maybe
we could have a naming convention for those hooks that they should all
start with TARGET_FINALIZE_<something>, like
TARGET_FINALIZE_PURGE_LOCALES ?
This way, we can more easily grep through the tree those hooks?
Thomas
On Thu, Jun 12, 2014 at 9:56 AM, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote: > Dear Fabio Porcedda, > > On Thu, 12 Jun 2014 09:32:39 +0200, Fabio Porcedda wrote: > >> I think it's best to use PURGE_LOCALES_HOOK instead of TARGET_PURGE_LOCALES. > > I think one of the concern with the hook thing was that the hooks would > be spread all over the Buildroot tree. To mitigate this concern, maybe > we could have a naming convention for those hooks that they should all > start with TARGET_FINALIZE_<something>, like > TARGET_FINALIZE_PURGE_LOCALES ? Ok fine, i like it, maybe using TARGET_FINALIZE_HOOK_PURGE_LOCALES is too verbose? > This way, we can more easily grep through the tree those hooks? Sure. Best regards
Dear Fabio Porcedda, On Thu, 12 Jun 2014 10:05:41 +0200, Fabio Porcedda wrote: > > I think one of the concern with the hook thing was that the hooks would > > be spread all over the Buildroot tree. To mitigate this concern, maybe > > we could have a naming convention for those hooks that they should all > > start with TARGET_FINALIZE_<something>, like > > TARGET_FINALIZE_PURGE_LOCALES ? > > Ok fine, i like it, maybe using TARGET_FINALIZE_HOOK_PURGE_LOCALES is > too verbose? If anything, the "HOOK" should be at the end. Not sure it's needed though if we have all variables prefixed by TARGET_FINALIZE_<something>. That being said, now that I can think of it, we can anyway always grep all the hooks by grepping for "TARGET_FINALIZE_HOOKS", which will return all files doing: TARGET_FINALIZE_HOOKS += <something> So in the end, the name of make function being registered doesn't matter much, but still for consistency, I think it's good to have a common prefix for all of them. Thomas
On 06/12/14 10:23, Thomas Petazzoni wrote: > Dear Fabio Porcedda, > > On Thu, 12 Jun 2014 10:05:41 +0200, Fabio Porcedda wrote: > >>> I think one of the concern with the hook thing was that the hooks would >>> be spread all over the Buildroot tree. To mitigate this concern, maybe >>> we could have a naming convention for those hooks that they should all >>> start with TARGET_FINALIZE_<something>, like >>> TARGET_FINALIZE_PURGE_LOCALES ? >> >> Ok fine, i like it, maybe using TARGET_FINALIZE_HOOK_PURGE_LOCALES is >> too verbose? > > If anything, the "HOOK" should be at the end. Not sure it's needed > though if we have all variables prefixed by TARGET_FINALIZE_<something>. > > That being said, now that I can think of it, we can anyway always grep > all the hooks by grepping for "TARGET_FINALIZE_HOOKS", which will > return all files doing: > > TARGET_FINALIZE_HOOKS += <something> > > So in the end, the name of make function being registered doesn't > matter much, but still for consistency, I think it's good to have a > common prefix for all of them. Well, the main reason to use the hooks approach is to be able to spread it out over different makefiles, and in particular to be able to do package-specific target-finalize things. Therefore, the hooks themselves should be named <PKGNAME>_FOO_BAR. Also, for the hooks that are currently in use, we don't usually include HOOK in the name. Regards, Arnout
On Thu, Jun 12, 2014 at 2:54 PM, Arnout Vandecappelle <arnout@mind.be> wrote: > On 06/12/14 10:23, Thomas Petazzoni wrote: >> Dear Fabio Porcedda, >> >> On Thu, 12 Jun 2014 10:05:41 +0200, Fabio Porcedda wrote: >> >>>> I think one of the concern with the hook thing was that the hooks would >>>> be spread all over the Buildroot tree. To mitigate this concern, maybe >>>> we could have a naming convention for those hooks that they should all >>>> start with TARGET_FINALIZE_<something>, like >>>> TARGET_FINALIZE_PURGE_LOCALES ? >>> >>> Ok fine, i like it, maybe using TARGET_FINALIZE_HOOK_PURGE_LOCALES is >>> too verbose? >> >> If anything, the "HOOK" should be at the end. Not sure it's needed >> though if we have all variables prefixed by TARGET_FINALIZE_<something>. >> >> That being said, now that I can think of it, we can anyway always grep >> all the hooks by grepping for "TARGET_FINALIZE_HOOKS", which will >> return all files doing: >> >> TARGET_FINALIZE_HOOKS += <something> >> >> So in the end, the name of make function being registered doesn't >> matter much, but still for consistency, I think it's good to have a >> common prefix for all of them. > > Well, the main reason to use the hooks approach is to be able to spread it out > over different makefiles, and in particular to be able to do package-specific > target-finalize things. Therefore, the hooks themselves should be named > <PKGNAME>_FOO_BAR. > > Also, for the hooks that are currently in use, we don't usually include HOOK in > the name. Following what we usually done for hooks, i'm going to call it: PURGE_LOCALES SYSTEM_GENERIC_SECURETTY SYSTEM_GENERIC_HOSTNAME ... BTW: Wouldn't better to change the usual name we use for hooks, maybe adding "_HOOK" at the end of name? I've found only four hooks that have "HOOK" in the name. Best regards
diff --git a/Makefile b/Makefile index dc86060..bb51727 100644 --- a/Makefile +++ b/Makefile @@ -513,7 +513,7 @@ ifeq ($(BR2_ENABLE_LOCALE_PURGE),y) LOCALE_WHITELIST = $(BUILD_DIR)/locales.nopurge LOCALE_NOPURGE = $(call qstrip,$(BR2_ENABLE_LOCALE_WHITELIST)) -define TARGET_PURGE_LOCALES +define PURGE_LOCALES_HOOK rm -f $(LOCALE_WHITELIST) for i in $(LOCALE_NOPURGE); do echo $$i >> $(LOCALE_WHITELIST); done @@ -525,13 +525,14 @@ define TARGET_PURGE_LOCALES done; \ done endef +TARGET_FINALIZE_HOOKS += PURGE_LOCALES_HOOK endif $(TARGETS_ROOTFS): target-finalize target-finalize: $(TARGETS) @$(call MESSAGE,"Finalizing target directory") - $(TARGET_PURGE_LOCALES) + $(foreach hook,$(TARGET_FINALIZE_HOOKS),$($(hook))$(sep)) rm -rf $(TARGET_DIR)/usr/include $(TARGET_DIR)/usr/share/aclocal \ $(TARGET_DIR)/usr/lib/pkgconfig $(TARGET_DIR)/usr/share/pkgconfig \