Skip to contentSkip to conversation

Makefile: target-purgelocales: add dependencies

PATCH New Fabio Porcedda 4 participants 10 messages Apr 24, 2014 – Jun 11, 2014 Kbuild

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Makefile: target-purgelocales: add dependencies

The "target-purgelocales" target must be executed after all the other targets so add dependencies to ensure that.
This also fix the support to top-level parallel make.

Cc Arnout Vandecappelle <[email protected]>
Cc Thomas De Schampheleire <[email protected]>

---

When both BR2_ENABLE_LOCALE_PURGE BR2_GENERATE_LOCALE are defined it swap the execution order of target-purgelocales and target-generatelocales, but as far as i can understand it does not arm to do that.

Regards
Fabio Porcedda

 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Makefile +1 -1
index eff408b..6b24482 100644
@@ -579,7 +579,7 @@ ifeq ($(BR2_ENABLE_LOCALE_PURGE),y)
 LOCALE_WHITELIST = $(BUILD_DIR)/locales.nopurge
 LOCALE_NOPURGE = $(call qstrip,$(BR2_ENABLE_LOCALE_WHITELIST))
 
-target-purgelocales:
+target-purgelocales: $(filter-out target-purgelocales,$(TARGETS))
 	rm -f $(LOCALE_WHITELIST)
 	for i in $(LOCALE_NOPURGE); do echo $$i >> $(LOCALE_WHITELIST); done
 
Signature · 1 line
1.8.3.2
#1 lore raw

PATCH 1/1 Makefile: target-purgelocales: add dependencies

Dear Fabio Porcedda,

Makefile ifeq ($(BR2_ENABLE_LOCALE_PURGE),y) · line 584

Quoted 4 lines · Fabio Porcedda
Makefile +1 -1
-target-purgelocales:
+target-purgelocales: $(filter-out target-purgelocales,$(TARGETS))
 	rm -f $(LOCALE_WHITELIST)
 	for i in $(LOCALE_NOPURGE); do echo $$i >> $(LOCALE_WHITELIST); done

Don't we have several other targets that need to be executed only after all packages have been built and installed? Wouldn't it make sense to have a common solution for these? Like maybe a dedicated target?

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#2 lore raw

PATCH 1/1 Makefile: target-purgelocales: add dependencies

Quoted 4 lines · Thomas Petazzoni

Dear Fabio Porcedda,

On Thu, 24 Apr 2014 18:24:39 +0200, Fabio Porcedda wrote:

Quoted 8 lines

Makefile ifeq ($(BR2_ENABLE_LOCALE_PURGE),y) · line 584

Quoted 4 lines
Makefile +1 -1
-target-purgelocales:
+target-purgelocales: $(filter-out target-purgelocales,$(TARGETS))
      rm -f $(LOCALE_WHITELIST)
      for i in $(LOCALE_NOPURGE); do echo $$i >> $(LOCALE_WHITELIST); done

Don't we have several other targets that need to be executed only after all packages have been built and installed? Wouldn't it make sense to have a common solution for these? Like maybe a dedicated target?

Can you please give some examples? I know only tatget-purgelocales and target-finalize.

About the common solution, i see two possible solutions of the problem:

1) all those targets must be listed in a variable like TARGETS_PRE_ROOTFS, but those targets must be able to be executed in parallel without a specific order.

2) all those targets must be converted in hooks and added to a variable like PRE_ROOTFS_HOOKS, so those steps are going to be executed in serial observing a specific order.

What is the more appropriate solution? The easiest and fastest one is the first, but i'm not sure if those targets can be executed in parallel.

Regards
--
Fabio Porcedda

#3 lore raw

PATCH 1/1 Makefile: target-purgelocales: add dependencies

Quoted 24 lines · Fabio Porcedda
Quoted 4 lines · Thomas Petazzoni

Dear Fabio Porcedda,

On Thu, 24 Apr 2014 18:24:39 +0200, Fabio Porcedda wrote:

Quoted 8 lines

Makefile ifeq ($(BR2_ENABLE_LOCALE_PURGE),y) · line 584

Quoted 4 lines
Makefile +1 -1
-target-purgelocales:
+target-purgelocales: $(filter-out target-purgelocales,$(TARGETS))
      rm -f $(LOCALE_WHITELIST)
      for i in $(LOCALE_NOPURGE); do echo $$i >> $(LOCALE_WHITELIST); done

Don't we have several other targets that need to be executed only after all packages have been built and installed? Wouldn't it make sense to have a common solution for these? Like maybe a dedicated target?

Can you please give some examples? I know only tatget-purgelocales and target-finalize.

About the common solution, i see two possible solutions of the problem:

1) all those targets must be listed in a variable like TARGETS_PRE_ROOTFS, but those targets must be able to be executed in parallel without a specific order.

Quoted 7 lines

2) all those targets must be converted in hooks and added to a variable like PRE_ROOTFS_HOOKS, so those steps are going to be executed in serial observing a specific order.

What is the more appropriate solution? The easiest and fastest one is the first, but i'm not sure if those targets can be executed in parallel.

My personal preference is to have a single rule (e.g. target-finalize)
that performs everything that is post-targets and pre-rootfs. There isn't much that needs to be done so parallelisation doesn't make sense. And I think it's much easier to understand which steps are executed and in which order if they are all put together in a single rule rather than spread out over several.

To make things more readable, we can put the commands into separate
variables. For instance:

define TARGET_PURGE_LOCALES

	rm -f $(LOCALE_WHITELIST)
	...

endef

define TARGET_PURGE_DEVFILES

	rm -rf $(TARGET_DIR)/usr/include ...

...
endef

ifneq ($(BR2_PACKAGE_GDB),y)
define TARGET_PURGE_GDB

        rm -rf $(TARGET_DIR)/usr/share/gdb

endef
endif

target-finalize: $(TARGETS)

	$(TARGET_PURGE_LOCALES)
	$(TARGET_PURGE_DEVFILES)
	$(TARGET_PURGE_GDB)
	$(TARGET_PURGE_DOC)

...

I'm giving the content of target-finalize as an example here, but it's
not immediately needed to convert those into variables. The different target-* steps that are currently added to TARGETS are rather more important.

Regards,
Arnout

Signature · 6 lines
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:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F
#4 lore raw

PATCH 1/1 Makefile: target-purgelocales: add dependencies

Quoted 64 lines · Arnout Vandecappelle
Quoted 24 lines · Fabio Porcedda
Quoted 4 lines · Thomas Petazzoni

Dear Fabio Porcedda,

On Thu, 24 Apr 2014 18:24:39 +0200, Fabio Porcedda wrote:

Quoted 8 lines

Makefile ifeq ($(BR2_ENABLE_LOCALE_PURGE),y) · line 584

Quoted 4 lines
Makefile +1 -1
-target-purgelocales:
+target-purgelocales: $(filter-out target-purgelocales,$(TARGETS))
      rm -f $(LOCALE_WHITELIST)
      for i in $(LOCALE_NOPURGE); do echo $$i >> $(LOCALE_WHITELIST); done

Don't we have several other targets that need to be executed only after all packages have been built and installed? Wouldn't it make sense to have a common solution for these? Like maybe a dedicated target?

Can you please give some examples? I know only tatget-purgelocales and target-finalize.

About the common solution, i see two possible solutions of the problem:

1) all those targets must be listed in a variable like TARGETS_PRE_ROOTFS, but those targets must be able to be executed in parallel without a specific order.

Quoted 7 lines

2) all those targets must be converted in hooks and added to a variable like PRE_ROOTFS_HOOKS, so those steps are going to be executed in serial observing a specific order.

What is the more appropriate solution? The easiest and fastest one is the first, but i'm not sure if those targets can be executed in parallel.

My personal preference is to have a single rule (e.g. target-finalize)
that performs everything that is post-targets and pre-rootfs. There isn't much that needs to be done so parallelisation doesn't make sense. And I think it's much easier to understand which steps are executed and in which order if they are all put together in a single rule rather than spread out over several.

To make things more readable, we can put the commands into separate
variables. For instance:

define TARGET_PURGE_LOCALES

        rm -f $(LOCALE_WHITELIST)
        ...

endef

define TARGET_PURGE_DEVFILES

        rm -rf $(TARGET_DIR)/usr/include ...

...
endef

ifneq ($(BR2_PACKAGE_GDB),y)
define TARGET_PURGE_GDB

        rm -rf $(TARGET_DIR)/usr/share/gdb

endef
endif

target-finalize: $(TARGETS)

        $(TARGET_PURGE_LOCALES)
        $(TARGET_PURGE_DEVFILES)
        $(TARGET_PURGE_GDB)
        $(TARGET_PURGE_DOC)
Quoted 6 lines

...

I'm giving the content of target-finalize as an example here, but it's
not immediately needed to convert those into variables. The different target-* steps that are currently added to TARGETS are rather more important.

It's fine for me.
So you prefer to list those variables inside the target instead of adding them to a hook variable? like e.g. PRE_ROOTFS_HOOK?

Regards

Signature · 1 line
Fabio Porcedda
#5 lore raw

PATCH 1/1 Makefile: target-purgelocales: add dependencies

Quoted 68 lines · Fabio Porcedda
Quoted 67 lines · Arnout Vandecappelle
Quoted 24 lines · Fabio Porcedda
Quoted 4 lines · Thomas Petazzoni

Dear Fabio Porcedda,

On Thu, 24 Apr 2014 18:24:39 +0200, Fabio Porcedda wrote:

Quoted 8 lines

Makefile ifeq ($(BR2_ENABLE_LOCALE_PURGE),y) · line 584

Quoted 4 lines
Makefile +1 -1
-target-purgelocales:
+target-purgelocales: $(filter-out target-purgelocales,$(TARGETS))
      rm -f $(LOCALE_WHITELIST)
      for i in $(LOCALE_NOPURGE); do echo $$i >> $(LOCALE_WHITELIST); done

Don't we have several other targets that need to be executed only after all packages have been built and installed? Wouldn't it make sense to have a common solution for these? Like maybe a dedicated target?

Can you please give some examples? I know only tatget-purgelocales and target-finalize.

About the common solution, i see two possible solutions of the problem:

1) all those targets must be listed in a variable like TARGETS_PRE_ROOTFS, but those targets must be able to be executed in parallel without a specific order.

Quoted 7 lines

2) all those targets must be converted in hooks and added to a variable like PRE_ROOTFS_HOOKS, so those steps are going to be executed in serial observing a specific order.

What is the more appropriate solution? The easiest and fastest one is the first, but i'm not sure if those targets can be executed in parallel.

My personal preference is to have a single rule (e.g. target-finalize)
that performs everything that is post-targets and pre-rootfs. There isn't much that needs to be done so parallelisation doesn't make sense. And I think it's much easier to understand which steps are executed and in which order if they are all put together in a single rule rather than spread out over several.

To make things more readable, we can put the commands into separate
variables. For instance:

define TARGET_PURGE_LOCALES

        rm -f $(LOCALE_WHITELIST)
        ...

endef

define TARGET_PURGE_DEVFILES

        rm -rf $(TARGET_DIR)/usr/include ...

...
endef

ifneq ($(BR2_PACKAGE_GDB),y)
define TARGET_PURGE_GDB

        rm -rf $(TARGET_DIR)/usr/share/gdb

endef
endif

target-finalize: $(TARGETS)

        $(TARGET_PURGE_LOCALES)
        $(TARGET_PURGE_DEVFILES)
        $(TARGET_PURGE_GDB)
        $(TARGET_PURGE_DOC)

...

Quoted 7 lines
Quoted 3 lines

I'm giving the content of target-finalize as an example here, but it's
not immediately needed to convert those into variables. The different target-* steps that are currently added to TARGETS are rather more important.

It's fine for me.
So you prefer to list those variables inside the target instead of adding them to a hook variable? like e.g. PRE_ROOTFS_HOOK?

Indeed. The HOOKS way of working is not very clear. The code for calling
the hooks is not easy to understand at first sight, because it contains this $(foreach ...) and $(sep) stuff. Also, the assignments to the HOOKS variable are spread out over the Makefiles so it's hard to see which things will get executed and in which order. And finally, it's not needed here because we can enumerate all the possible steps - that's different from the package infrastructure, where the HOOK contents differs from package to package.

Regards,
Arnout

Signature · 6 lines
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:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F
#6 lore raw

PATCH 1/1 Makefile: target-purgelocales: add dependencies

Quoted 77 lines · Arnout Vandecappelle
Quoted 68 lines · Fabio Porcedda
Quoted 67 lines · Arnout Vandecappelle
Quoted 24 lines · Fabio Porcedda
Quoted 4 lines · Thomas Petazzoni

Dear Fabio Porcedda,

On Thu, 24 Apr 2014 18:24:39 +0200, Fabio Porcedda wrote:

Quoted 8 lines

Makefile ifeq ($(BR2_ENABLE_LOCALE_PURGE),y) · line 584

Quoted 4 lines
Makefile +1 -1
-target-purgelocales:
+target-purgelocales: $(filter-out target-purgelocales,$(TARGETS))
      rm -f $(LOCALE_WHITELIST)
      for i in $(LOCALE_NOPURGE); do echo $$i >> $(LOCALE_WHITELIST); done

Don't we have several other targets that need to be executed only after all packages have been built and installed? Wouldn't it make sense to have a common solution for these? Like maybe a dedicated target?

Can you please give some examples? I know only tatget-purgelocales and target-finalize.

About the common solution, i see two possible solutions of the problem:

1) all those targets must be listed in a variable like TARGETS_PRE_ROOTFS, but those targets must be able to be executed in parallel without a specific order.

Quoted 7 lines

2) all those targets must be converted in hooks and added to a variable like PRE_ROOTFS_HOOKS, so those steps are going to be executed in serial observing a specific order.

What is the more appropriate solution? The easiest and fastest one is the first, but i'm not sure if those targets can be executed in parallel.

My personal preference is to have a single rule (e.g. target-finalize)
that performs everything that is post-targets and pre-rootfs. There isn't much that needs to be done so parallelisation doesn't make sense. And I think it's much easier to understand which steps are executed and in which order if they are all put together in a single rule rather than spread out over several.

To make things more readable, we can put the commands into separate
variables. For instance:

define TARGET_PURGE_LOCALES

        rm -f $(LOCALE_WHITELIST)
        ...

endef

define TARGET_PURGE_DEVFILES

        rm -rf $(TARGET_DIR)/usr/include ...

...
endef

ifneq ($(BR2_PACKAGE_GDB),y)
define TARGET_PURGE_GDB

        rm -rf $(TARGET_DIR)/usr/share/gdb

endef
endif

target-finalize: $(TARGETS)

        $(TARGET_PURGE_LOCALES)
        $(TARGET_PURGE_DEVFILES)
        $(TARGET_PURGE_GDB)
        $(TARGET_PURGE_DOC)

...

Quoted 7 lines
Quoted 3 lines

I'm giving the content of target-finalize as an example here, but it's
not immediately needed to convert those into variables. The different target-* steps that are currently added to TARGETS are rather more important.

It's fine for me.
So you prefer to list those variables inside the target instead of adding them to a hook variable? like e.g. PRE_ROOTFS_HOOK?

Quoted 8 lines

Indeed. The HOOKS way of working is not very clear. The code for calling
the hooks is not easy to understand at first sight, because it contains this $(foreach ...) and $(sep) stuff. Also, the assignments to the HOOKS variable are spread out over the Makefiles so it's hard to see which things will get executed and in which order. And finally, it's not needed here because we can enumerate all the possible steps - that's different from the package infrastructure, where the HOOK contents differs from package to package.

Ok fine, i will do like that.

Thanks and regards

Signature · 1 line
Fabio Porcedda
#7 lore raw

PATCH 1/1 Makefile: target-purgelocales: add dependencies

In reply to Arnout Vandecappelle
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Arnout" == Arnout Vandecappelle <[email protected]> writes:

Hi,

Quoted 30 lines
Quoted 3 lines

What is the more appropriate solution? The easiest and fastest one is the first, but i'm not sure if those targets can be executed in parallel.

My personal preference is to have a single rule (e.g. target-finalize)
that performs everything that is post-targets and pre-rootfs. There isn't much that needs to be done so parallelisation doesn't make sense. And I think it's much easier to understand which steps are executed and in which order if they are all put together in a single rule rather than spread out over several.

To make things more readable, we can put the commands into separate
variables. For instance:

define TARGET_PURGE_LOCALES

	rm -f $(LOCALE_WHITELIST)
	...

endef

define TARGET_PURGE_DEVFILES

	rm -rf $(TARGET_DIR)/usr/include ...

...
endef

ifneq ($(BR2_PACKAGE_GDB),y)
define TARGET_PURGE_GDB

        rm -rf $(TARGET_DIR)/usr/share/gdb

endef
endif

Quoted 6 lines

target-finalize: $(TARGETS)

	$(TARGET_PURGE_LOCALES)
	$(TARGET_PURGE_DEVFILES)
	$(TARGET_PURGE_GDB)
	$(TARGET_PURGE_DOC)

...

Yes, that looks nice and clear to me too.

Signature · 1 line
Bye, Peter Korsgaard
#8 lore raw

PATCH 1/1 Makefile: target-purgelocales: add dependencies

Peter, Arnout,

Quoted 26 lines · Peter Korsgaard
Quoted 25 lines

My personal preference is to have a single rule (e.g. target-finalize)
that performs everything that is post-targets and pre-rootfs. There isn't much that needs to be done so parallelisation doesn't make sense. And I think it's much easier to understand which steps are executed and in which order if they are all put together in a single rule rather than spread out over several.

To make things more readable, we can put the commands into separate
variables. For instance:

define TARGET_PURGE_LOCALES

	rm -f $(LOCALE_WHITELIST)
	...

endef

define TARGET_PURGE_DEVFILES

	rm -rf $(TARGET_DIR)/usr/include ...

...
endef

ifneq ($(BR2_PACKAGE_GDB),y)
define TARGET_PURGE_GDB

        rm -rf $(TARGET_DIR)/usr/share/gdb

endef
endif

Quoted 8 lines
Quoted 6 lines

target-finalize: $(TARGETS)

	$(TARGET_PURGE_LOCALES)
	$(TARGET_PURGE_DEVFILES)
	$(TARGET_PURGE_GDB)
	$(TARGET_PURGE_DOC)

...

Yes, that looks nice and clear to me too.

Yes, agreed, this looks a lot nicer than a long chain of targets that simply depend on each other to avoid any parallelization.

Thanks!

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#9 lore raw

PATCH 1/1 Makefile: target-purgelocales: add dependencies

Quoted 37 lines · Thomas Petazzoni

Peter, Arnout,

Quoted 33 lines · Peter Korsgaard
Quoted 32 lines

My personal preference is to have a single rule (e.g. target-finalize)
that performs everything that is post-targets and pre-rootfs. There isn't much that needs to be done so parallelisation doesn't make sense. And I think it's much easier to understand which steps are executed and in which order if they are all put together in a single rule rather than spread out over several.

To make things more readable, we can put the commands into separate
variables. For instance:

define TARGET_PURGE_LOCALES
rm -f $(LOCALE_WHITELIST)
...
endef

define TARGET_PURGE_DEVFILES
rm -rf $(TARGET_DIR)/usr/include ...
...
endef

ifneq ($(BR2_PACKAGE_GDB),y)
define TARGET_PURGE_GDB

        rm -rf $(TARGET_DIR)/usr/share/gdb

endef
endif

target-finalize: $(TARGETS)
$(TARGET_PURGE_LOCALES)
$(TARGET_PURGE_DEVFILES)
$(TARGET_PURGE_GDB)
$(TARGET_PURGE_DOC)
...

Quoted 4 lines
Quoted 1 line

Yes, that looks nice and clear to me too.

Yes, agreed, this looks a lot nicer than a long chain of targets that simply depend on each other to avoid any parallelization.

Just to let you know, I've sent a patch following the above idea:
http://patchwork.ozlabs.org/patch/357792/

Best regards

Signature · 1 line
Fabio Porcedda