Skip to contentSkip to conversation

fs: cleanups and enhancements (branch yem/fs)

PATCH New Yann E. MORIN 2 participants 23 messages Mar 11, 2016 – Mar 28, 2016

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

v2 cover fs: cleanups and enhancements (branch yem/fs)

Hello All!

This series introduces some cleaups and enhancements to the filesystem infrastructure.

Most notably, it gets rid of the post-target rules, i.e. rules defined by filesystems that were to be executed after the image was generated, in favour of post-gen hooks.

The reason for getting rid of post-target rules is that they were not really needed, and the way they were used was dubious when dealing with top-level parallel builds. Also, the sole user for which those rules were documented to be useful was not even using the filesystem infra.

To be noted: patch 8/9 was the beginning of a bigger plan to rework how images are generated. However, and quite sadly, that bigger plan is now evading my recollection. It's sad to get old... ;-] Nevertheless, the patch by itself still makes sense, so I kept it.

Finally, the series also introduces the concept of a "landing" area for the target directory.

This landing area is a copy of the currently-known target/ directory, copy in which all the target-finalize step and hooks are run, leaving the original target/ directory to contain only what packages have installed.

Please read the commit log for the final patch for more on that landing area. I also kept it in the series despite Thomas opoosition to it; at least it is now properly archived for posterity on the ML archives...

Changes v1 -> v2:
- rebased on top of master

Regards,
Yann E. MORIN.

The following changes since commit 0b7b84310c3781c346eea4c114c8cb4e97bbd8ea:

olimex_imx233_olinuxino_defconfig: genimage support (2016-03-11 13:14:54 +0100)

are available in the git repository at:

git://git.busybox.net/~ymorin/git/buildroot yem/fs

for you to fetch changes up to 8892913385099f562cba138ac3b4f762d8cc4d49:

[RFC] core: finalise target in its own location (2016-03-11 19:36:40 +0100)

----------------------------------------------------------------
Yann E. MORIN (9):
core: sort packages and eliminate duplicates before building
linux: split overly-long dependency line for readability
linux: meddle not in the affairs of filesystems, for you are tasty with bacon fs/initramfs: cleanup and enhance comments
fs/ext2: use a post-gen hook rather than a post-target rule fs/cpio: use a post-gen hook rather than a post-target rule fs/common: get rid of post-target rules
fs/common: move actions common to all filesystems to their own rule [RFC] core: finalise target in its own location

 Makefile                  | 34 +++++++++++++++++++++++-
 fs/common.mk              | 67 +++++++++++++++++++++++++----------------------
 fs/cpio/cpio.mk           | 11 ++++----
 fs/ext2/ext2.mk           | 10 +++----
 fs/initramfs/initramfs.mk | 22 +++++++++++-----
 linux/linux.mk            | 13 +++++----
 6 files changed, 99 insertions(+), 58 deletions(-)
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
#1 lore raw

PATCH v2 1/9 core: sort packages and eliminate duplicates before building

Currently, enabling more than one filesystem image will make 'show-targets' list a few host packages more than once.

This is because all filesystem images add the same set of host-packages to their dependencies, which are then added as-is to the package list.

Thus, host-fakeroot, host-makedevs and, if needed, host-mkpasswd will appear as many times as there are filesystem images enabled.

Fix that by sorting the package list, thus eliminating duplicates from that list.

Furthermore, and even though we're already sorting packages by scanning the .mk file in alphabetical order (because we $(sort) them), sorting the list itself will further guarantee the build order, thus enhancing reproducibility (and $(sort ...) is not dependent on the locale, it always sorts with the C locale).

Cc Thomas Petazzoni <[email protected]>
Cc Arnout Vandecappelle <[email protected]>
Cc Peter Korsgaard <[email protected]>

---
Changes v1 -> v2:
- fix typo in comment

---
 Makefile | 6 ++++++
 1 file changed, 6 insertions(+)
Makefile +6 -0
index 98c8dc7..0f16976 100644
@@ -600,6 +600,12 @@ endif
 
 $(TARGETS_ROOTFS): target-finalize
 
+# Filesystems may add the same packages more than once from their
+# dependency lists. For example, all filesystems include host-fakeroot,
+# host-makedevs and, if required, host-mkpasswd. They would appear
+# multiple times in the list returned by show-target.
+PACKAGES := $(sort $(PACKAGES))
+
 target-finalize: $(PACKAGES)
 	@$(call MESSAGE,"Finalizing target directory")
 	$(foreach hook,$(TARGET_FINALIZE_HOOKS),$($(hook))$(sep))
Signature · 1 line
1.9.1
#2 lore raw

PATCH v2 1/9 core: sort packages and eliminate duplicates before building

Hi Yann, All,

Le 11/03/2016 19:41, Yann E. MORIN a ?crit :

Quoted 36 lines

Currently, enabling more than one filesystem image will make 'show-targets' list a few host packages more than once.

This is because all filesystem images add the same set of host-packages to their dependencies, which are then added as-is to the package list.

Thus, host-fakeroot, host-makedevs and, if needed, host-mkpasswd will appear as many times as there are filesystem images enabled.

Fix that by sorting the package list, thus eliminating duplicates from that list.

Furthermore, and even though we're already sorting packages by scanning the .mk file in alphabetical order (because we $(sort) them), sorting the list itself will further guarantee the build order, thus enhancing reproducibility (and $(sort ...) is not dependent on the locale, it always sorts with the C locale).

Cc Thomas Petazzoni <[email protected]>
Cc Arnout Vandecappelle <[email protected]>
Cc Peter Korsgaard <[email protected]>

---
Changes v1 -> v2:
- fix typo in comment

---
 Makefile | 6 ++++++
 1 file changed, 6 insertions(+)
Makefile +0 -0
index 98c8dc7..0f16976 100644

Makefile endif · line 608

Quoted 9 lines
Makefile +6 -0
@@ -600,6 +600,12 @@ endif
 
 $(TARGETS_ROOTFS): target-finalize
 
+# Filesystems may add the same packages more than once from their
+# dependency lists. For example, all filesystems include host-fakeroot,
+# host-makedevs and, if required, host-mkpasswd. They would appear
+# multiple times in the list returned by show-target.
+PACKAGES := $(sort $(PACKAGES))
+

Indeed, now the content of PACKAGES is listed alphabetically and duplicate are removed.

Best regards,
Romain

Quoted 4 lines
Makefile +0 -0
 target-finalize: $(PACKAGES)
 	@$(call MESSAGE,"Finalizing target directory")
 	$(foreach hook,$(TARGET_FINALIZE_HOOKS),$($(hook))$(sep))
#3 lore raw

PATCH v2 2/9 linux: split overly-long dependency line for readability

In reply to Yann E. MORIN
Cc Thomas Petazzoni <[email protected]>
Cc Arnout Vandecappelle <[email protected]>

---
This is going to be useful to better understand the initramfs patch coming next.

---
 linux/linux.mk | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
linux/linux.mk +4 -1
index 7e20255..ea90918 100644
@@ -439,7 +439,10 @@ $(eval $(kconfig-package))
 
 # Support for rebuilding the kernel after the cpio archive has
 # been generated in $(BINARIES_DIR)/rootfs.cpio.
-$(LINUX_DIR)/.stamp_initramfs_rebuilt: $(LINUX_DIR)/.stamp_target_installed $(LINUX_DIR)/.stamp_images_installed $(BINARIES_DIR)/rootfs.cpio
+$(LINUX_DIR)/.stamp_initramfs_rebuilt: $(LINUX_DIR)/.stamp_target_installed
+$(LINUX_DIR)/.stamp_initramfs_rebuilt: $(LINUX_DIR)/.stamp_images_installed
+$(LINUX_DIR)/.stamp_initramfs_rebuilt: $(BINARIES_DIR)/rootfs.cpio
+$(LINUX_DIR)/.stamp_initramfs_rebuilt:
 	@$(call MESSAGE,"Rebuilding kernel with initramfs")
 	# Build the kernel.
 	$(LINUX_MAKE_ENV) $(MAKE) $(LINUX_MAKE_FLAGS) -C $(@D) $(LINUX_TARGET_NAME)
Signature · 1 line
1.9.1
#4 lore raw

PATCH v2 2/9 linux: split overly-long dependency line for readability

Hi Yann, All,

Le 11/03/2016 19:41, Yann E. MORIN a ?crit :

Quoted 16 lines
Cc Thomas Petazzoni <[email protected]>
Cc Arnout Vandecappelle <[email protected]>

---
This is going to be useful to better understand the initramfs patch coming next.

---
 linux/linux.mk | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
linux/linux.mk +0 -0
index 7e20255..ea90918 100644

linux/linux.mk $(eval $(kconfig-package)) · line 445

Quoted 8 lines
linux/linux.mk +4 -1
@@ -439,7 +439,10 @@ $(eval $(kconfig-package))
 
 # Support for rebuilding the kernel after the cpio archive has
 # been generated in $(BINARIES_DIR)/rootfs.cpio.
-$(LINUX_DIR)/.stamp_initramfs_rebuilt: $(LINUX_DIR)/.stamp_target_installed $(LINUX_DIR)/.stamp_images_installed $(BINARIES_DIR)/rootfs.cpio
+$(LINUX_DIR)/.stamp_initramfs_rebuilt: $(LINUX_DIR)/.stamp_target_installed
+$(LINUX_DIR)/.stamp_initramfs_rebuilt: $(LINUX_DIR)/.stamp_images_installed
+$(LINUX_DIR)/.stamp_initramfs_rebuilt: $(BINARIES_DIR)/rootfs.cpio
+$(LINUX_DIR)/.stamp_initramfs_rebuilt:

Best regards,
Romain

Quoted 4 lines
linux/linux.mk +0 -0
 	@$(call MESSAGE,"Rebuilding kernel with initramfs")
 	# Build the kernel.
 	$(LINUX_MAKE_ENV) $(MAKE) $(LINUX_MAKE_FLAGS) -C $(@D) $(LINUX_TARGET_NAME)
#5 lore raw

PATCH v2 3/9 linux: meddle not in the affairs of filesystems, for you are tasty with bacon

In reply to Yann E. MORIN

Currently, the rule to rebuild the Linux kernel with an initramfs directly depends on the filesystem image filename.

This is inherently "bad" from a purity point of view. linux.mk should not have to delve into the fs internals.

Rather, make it directly depend on the "frontal" rule that generates the cpio image.

Drop the comment for linux-rebuild-with-initramfs, it was misleading (talking about generating "the initramfs list of files", which is not what was done, since we use a cpio as source of initramfs, not a list of files).

Cc Thomas Petazzoni <[email protected]>
Cc Arnout Vandecappelle <[email protected]>

---
Changes v1 -> v2:
- drop intermediate, unneeded stamp-file
- drop misleading comment

---
This, too, will be useful for the initramfs patch coming next.

---
 linux/linux.mk | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)
linux/linux.mk +6 -10
index ea90918..c0d760a 100644
@@ -438,11 +438,12 @@ endif # BR_BUILDING
 $(eval $(kconfig-package))
 
 # Support for rebuilding the kernel after the cpio archive has
-# been generated in $(BINARIES_DIR)/rootfs.cpio.
-$(LINUX_DIR)/.stamp_initramfs_rebuilt: $(LINUX_DIR)/.stamp_target_installed
-$(LINUX_DIR)/.stamp_initramfs_rebuilt: $(LINUX_DIR)/.stamp_images_installed
-$(LINUX_DIR)/.stamp_initramfs_rebuilt: $(BINARIES_DIR)/rootfs.cpio
-$(LINUX_DIR)/.stamp_initramfs_rebuilt:
+# been generated.
+.PHONY: linux-rebuild-with-initramfs
+linux-rebuild-with-initramfs: $(LINUX_DIR)/.stamp_target_installed
+linux-rebuild-with-initramfs: $(LINUX_DIR)/.stamp_images_installed
+linux-rebuild-with-initramfs: rootfs-cpio
+linux-rebuild-with-initramfs:
 	@$(call MESSAGE,"Rebuilding kernel with initramfs")
 	# Build the kernel.
 	$(LINUX_MAKE_ENV) $(MAKE) $(LINUX_MAKE_FLAGS) -C $(@D) $(LINUX_TARGET_NAME)
@@ -451,8 +452,3 @@ $(LINUX_DIR)/.stamp_initramfs_rebuilt:
 	cp $(LINUX_IMAGE_PATH) $(BINARIES_DIR)
 	# If there is a .ub file copy it to the final destination
 	test ! -f $(LINUX_IMAGE_PATH).ub || cp $(LINUX_IMAGE_PATH).ub $(BINARIES_DIR)
-	$(Q)touch $@
-
-# The initramfs building code must make sure this target gets called
-# after it generated the initramfs list of files.
-linux-rebuild-with-initramfs: $(LINUX_DIR)/.stamp_initramfs_rebuilt
Signature · 1 line
1.9.1
#6 lore raw

PATCH v2 3/9 linux: meddle not in the affairs of filesystems, for you are tasty with bacon

Hi Yann, All,

Le 11/03/2016 19:41, Yann E. MORIN a ?crit :

Quoted 52 lines

Currently, the rule to rebuild the Linux kernel with an initramfs directly depends on the filesystem image filename.

This is inherently "bad" from a purity point of view. linux.mk should not have to delve into the fs internals.

Rather, make it directly depend on the "frontal" rule that generates the cpio image.

Drop the comment for linux-rebuild-with-initramfs, it was misleading (talking about generating "the initramfs list of files", which is not what was done, since we use a cpio as source of initramfs, not a list of files).

Cc Thomas Petazzoni <[email protected]>
Cc Arnout Vandecappelle <[email protected]>

---
Changes v1 -> v2:
- drop intermediate, unneeded stamp-file
- drop misleading comment

---
This, too, will be useful for the initramfs patch coming next.

---
 linux/linux.mk | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)
linux/linux.mk +6 -5
index ea90918..c0d760a 100644
@@ -438,11 +438,12 @@ endif # BR_BUILDING
 $(eval $(kconfig-package))
 
 # Support for rebuilding the kernel after the cpio archive has
-# been generated in $(BINARIES_DIR)/rootfs.cpio.
-$(LINUX_DIR)/.stamp_initramfs_rebuilt: $(LINUX_DIR)/.stamp_target_installed
-$(LINUX_DIR)/.stamp_initramfs_rebuilt: $(LINUX_DIR)/.stamp_images_installed
-$(LINUX_DIR)/.stamp_initramfs_rebuilt: $(BINARIES_DIR)/rootfs.cpio
-$(LINUX_DIR)/.stamp_initramfs_rebuilt:
+# been generated.
+.PHONY: linux-rebuild-with-initramfs
+linux-rebuild-with-initramfs: $(LINUX_DIR)/.stamp_target_installed
+linux-rebuild-with-initramfs: $(LINUX_DIR)/.stamp_images_installed
+linux-rebuild-with-initramfs: rootfs-cpio
+linux-rebuild-with-initramfs:
 	@$(call MESSAGE,"Rebuilding kernel with initramfs")
 	# Build the kernel.
 	$(LINUX_MAKE_ENV) $(MAKE) $(LINUX_MAKE_FLAGS) -C $(@D) $(LINUX_TARGET_NAME)

linux/linux.mk $(LINUX_DIR)/.stamp_initramfs_rebuilt: · line 454

Quoted 4 lines
linux/linux.mk +0 -1
@@ -451,8 +452,3 @@ $(LINUX_DIR)/.stamp_initramfs_rebuilt:
 	cp $(LINUX_IMAGE_PATH) $(BINARIES_DIR)
 	# If there is a .ub file copy it to the final destination
 	test ! -f $(LINUX_IMAGE_PATH).ub || cp $(LINUX_IMAGE_PATH).ub $(BINARIES_DIR)
-	$(Q)touch $@

IIUC, the stamp file can be removed since the target is now PHONY, right ?

But latter in the patch 4/9 in the comment there is a reference to this file:
# Note: ordering of the dependencies is not guaranteed here, but in # linux/linux.mk, via the .stamp_initramfs_rebuilt stamp file, which depends

# on the rootfs-cpio filesystem rule.

linux/linux.mk $(LINUX_DIR)/.stamp_initramfs_rebuilt: · line 458

Quoted 5 lines
linux/linux.mk +0 -4
-
-# The initramfs building code must make sure this target gets called
-# after it generated the initramfs list of files.
-linux-rebuild-with-initramfs: $(LINUX_DIR)/.stamp_initramfs_rebuilt

Something is wrong...
With this patch applied, I have the following error when rebuilding the kernel with initramfs:

make[2]: *** No rule to make target 'bzImage'. Arr?t. linux/linux.mk:447: recipe for target 'linux-rebuild-with-initramfs' failed

Best regards,
Romain

#7 lore raw

PATCH v2 3/9 linux: meddle not in the affairs of filesystems, for you are tasty with bacon

Yann, All,

Le 28/03/2016 19:36, Romain Naour a ?crit :

Quoted 39 lines

Hi Yann, All,

Le 11/03/2016 19:41, Yann E. MORIN a ?crit :

Quoted 36 lines

Currently, the rule to rebuild the Linux kernel with an initramfs directly depends on the filesystem image filename.

This is inherently "bad" from a purity point of view. linux.mk should not have to delve into the fs internals.

Rather, make it directly depend on the "frontal" rule that generates the cpio image.

Drop the comment for linux-rebuild-with-initramfs, it was misleading (talking about generating "the initramfs list of files", which is not what was done, since we use a cpio as source of initramfs, not a list of files).

Cc Thomas Petazzoni <[email protected]>
Cc Arnout Vandecappelle <[email protected]>

---
Changes v1 -> v2:
- drop intermediate, unneeded stamp-file
- drop misleading comment

---
This, too, will be useful for the initramfs patch coming next.

---
 linux/linux.mk | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)
linux/linux.mk +0 -0
index ea90918..c0d760a 100644
@@ -438,11 +438,12 @@ endif # BR_BUILDING
 $(eval $(kconfig-package))
 
Quoted 15 lines
Quoted 5 lines
linux/linux.mk +0 -4
 # Support for rebuilding the kernel after the cpio archive has
-# been generated in $(BINARIES_DIR)/rootfs.cpio.
-$(LINUX_DIR)/.stamp_initramfs_rebuilt: $(LINUX_DIR)/.stamp_target_installed
-$(LINUX_DIR)/.stamp_initramfs_rebuilt: $(LINUX_DIR)/.stamp_images_installed
-$(LINUX_DIR)/.stamp_initramfs_rebuilt: $(BINARIES_DIR)/rootfs.cpio

linux/linux.mk endif # BR_BUILDING · line 449

Quoted 10 lines
linux/linux.mk +6 -1
-$(LINUX_DIR)/.stamp_initramfs_rebuilt:
+# been generated.
+.PHONY: linux-rebuild-with-initramfs
+linux-rebuild-with-initramfs: $(LINUX_DIR)/.stamp_target_installed
+linux-rebuild-with-initramfs: $(LINUX_DIR)/.stamp_images_installed
+linux-rebuild-with-initramfs: rootfs-cpio
+linux-rebuild-with-initramfs:
 	@$(call MESSAGE,"Rebuilding kernel with initramfs")
 	# Build the kernel.
 	$(LINUX_MAKE_ENV) $(MAKE) $(LINUX_MAKE_FLAGS) -C $(@D) $(LINUX_TARGET_NAME)

The issue can be fixed by using LINUX_DIR instead of @D. ^^

Best regards,
Romain

Quoted 29 lines

linux/linux.mk $(LINUX_DIR)/.stamp_initramfs_rebuilt: · line 454

Quoted 5 lines
linux/linux.mk +0 -1
@@ -451,8 +452,3 @@ $(LINUX_DIR)/.stamp_initramfs_rebuilt:
 	cp $(LINUX_IMAGE_PATH) $(BINARIES_DIR)
 	# If there is a .ub file copy it to the final destination
 	test ! -f $(LINUX_IMAGE_PATH).ub || cp $(LINUX_IMAGE_PATH).ub $(BINARIES_DIR)
-	$(Q)touch $@

IIUC, the stamp file can be removed since the target is now PHONY, right ?

But latter in the patch 4/9 in the comment there is a reference to this file:
# Note: ordering of the dependencies is not guaranteed here, but in # linux/linux.mk, via the .stamp_initramfs_rebuilt stamp file, which depends

# on the rootfs-cpio filesystem rule.

linux/linux.mk $(LINUX_DIR)/.stamp_initramfs_rebuilt: · line 458

Quoted 5 lines
linux/linux.mk +0 -4
-
-# The initramfs building code must make sure this target gets called
-# after it generated the initramfs list of files.
-linux-rebuild-with-initramfs: $(LINUX_DIR)/.stamp_initramfs_rebuilt

Something is wrong...
With this patch applied, I have the following error when rebuilding the kernel with initramfs:

make[2]: *** No rule to make target 'bzImage'. Arr?t. linux/linux.mk:447: recipe for target 'linux-rebuild-with-initramfs' failed

Best regards,
Romain

#8 lore raw

PATCH v2 3/9 linux: meddle not in the affairs of filesystems, for you are tasty with bacon

Romain, All,

On 2016-03-28 19:47 +0200, Romain Naour spake thusly:

Quoted 38 lines

Le 28/03/2016 19:36, Romain Naour a ?crit :

Quoted 37 lines

Le 11/03/2016 19:41, Yann E. MORIN a ?crit :

Quoted 36 lines

Currently, the rule to rebuild the Linux kernel with an initramfs directly depends on the filesystem image filename.

This is inherently "bad" from a purity point of view. linux.mk should not have to delve into the fs internals.

Rather, make it directly depend on the "frontal" rule that generates the cpio image.

Drop the comment for linux-rebuild-with-initramfs, it was misleading (talking about generating "the initramfs list of files", which is not what was done, since we use a cpio as source of initramfs, not a list of files).

Cc Thomas Petazzoni <[email protected]>
Cc Arnout Vandecappelle <[email protected]>

---
Changes v1 -> v2:
- drop intermediate, unneeded stamp-file
- drop misleading comment

---
This, too, will be useful for the initramfs patch coming next.

---
 linux/linux.mk | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)
linux/linux.mk +0 -0
index ea90918..c0d760a 100644
@@ -438,11 +438,12 @@ endif # BR_BUILDING
 $(eval $(kconfig-package))
 
Quoted 16 lines
Quoted 7 lines
Quoted 7 lines
linux/linux.mk +1 -5
 # Support for rebuilding the kernel after the cpio archive has
-# been generated in $(BINARIES_DIR)/rootfs.cpio.
-$(LINUX_DIR)/.stamp_initramfs_rebuilt: $(LINUX_DIR)/.stamp_target_installed
-$(LINUX_DIR)/.stamp_initramfs_rebuilt: $(LINUX_DIR)/.stamp_images_installed
-$(LINUX_DIR)/.stamp_initramfs_rebuilt: $(BINARIES_DIR)/rootfs.cpio
-$(LINUX_DIR)/.stamp_initramfs_rebuilt:
+# been generated.
Quoted 8 lines

linux/linux.mk endif # BR_BUILDING · line 449

Quoted 8 lines
linux/linux.mk +5 -0
+.PHONY: linux-rebuild-with-initramfs
+linux-rebuild-with-initramfs: $(LINUX_DIR)/.stamp_target_installed
+linux-rebuild-with-initramfs: $(LINUX_DIR)/.stamp_images_installed
+linux-rebuild-with-initramfs: rootfs-cpio
+linux-rebuild-with-initramfs:
 	@$(call MESSAGE,"Rebuilding kernel with initramfs")
 	# Build the kernel.
 	$(LINUX_MAKE_ENV) $(MAKE) $(LINUX_MAKE_FLAGS) -C $(@D) $(LINUX_TARGET_NAME)

The issue can be fixed by using LINUX_DIR instead of @D. ^^

Indeed. It is in my local branch, though. I wonder why it did not end up in my tree, or why this patch does not have it... Weird...

Oh, I get it now... Damned. I should be more careful when I work on my remote machine. So, because buildiong can take quite some time, I ssh into a remote fast server for heavy builds. When there an issue, I usually fix locally, then push the branch to the remote machine. Except, I sometime forget that I'm remote, so I do the fixes remotely. And I can forget to git-fetch those...

Damn...

Thanks! :-)

Regards,
Yann E. MORIN.

Quoted 33 lines

Best regards,
Romain

Quoted 29 lines

linux/linux.mk $(LINUX_DIR)/.stamp_initramfs_rebuilt: · line 454

Quoted 5 lines
linux/linux.mk +0 -1
@@ -451,8 +452,3 @@ $(LINUX_DIR)/.stamp_initramfs_rebuilt:
 	cp $(LINUX_IMAGE_PATH) $(BINARIES_DIR)
 	# If there is a .ub file copy it to the final destination
 	test ! -f $(LINUX_IMAGE_PATH).ub || cp $(LINUX_IMAGE_PATH).ub $(BINARIES_DIR)
-	$(Q)touch $@

IIUC, the stamp file can be removed since the target is now PHONY, right ?

But latter in the patch 4/9 in the comment there is a reference to this file:
# Note: ordering of the dependencies is not guaranteed here, but in # linux/linux.mk, via the .stamp_initramfs_rebuilt stamp file, which depends

# on the rootfs-cpio filesystem rule.

linux/linux.mk $(LINUX_DIR)/.stamp_initramfs_rebuilt: · line 458

Quoted 5 lines
linux/linux.mk +0 -4
-
-# The initramfs building code must make sure this target gets called
-# after it generated the initramfs list of files.
-linux-rebuild-with-initramfs: $(LINUX_DIR)/.stamp_initramfs_rebuilt

Something is wrong...
With this patch applied, I have the following error when rebuilding the kernel with initramfs:

make[2]: *** No rule to make target 'bzImage'. Arr?t. linux/linux.mk:447: recipe for target 'linux-rebuild-with-initramfs' failed

Best regards,
Romain

Signature · 6 lines
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
#9 lore raw

PATCH v2 3/9 linux: meddle not in the affairs of filesystems, for you are tasty with bacon

In reply to Romain Naour

Romain, All,

On 2016-03-28 19:36 +0200, Romain Naour spake thusly:

Quoted 60 lines

Le 11/03/2016 19:41, Yann E. MORIN a ?crit :

Quoted 52 lines

Currently, the rule to rebuild the Linux kernel with an initramfs directly depends on the filesystem image filename.

This is inherently "bad" from a purity point of view. linux.mk should not have to delve into the fs internals.

Rather, make it directly depend on the "frontal" rule that generates the cpio image.

Drop the comment for linux-rebuild-with-initramfs, it was misleading (talking about generating "the initramfs list of files", which is not what was done, since we use a cpio as source of initramfs, not a list of files).

Cc Thomas Petazzoni <[email protected]>
Cc Arnout Vandecappelle <[email protected]>

---
Changes v1 -> v2:
- drop intermediate, unneeded stamp-file
- drop misleading comment

---
This, too, will be useful for the initramfs patch coming next.

---
 linux/linux.mk | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)
linux/linux.mk +6 -5
index ea90918..c0d760a 100644
@@ -438,11 +438,12 @@ endif # BR_BUILDING
 $(eval $(kconfig-package))
 
 # Support for rebuilding the kernel after the cpio archive has
-# been generated in $(BINARIES_DIR)/rootfs.cpio.
-$(LINUX_DIR)/.stamp_initramfs_rebuilt: $(LINUX_DIR)/.stamp_target_installed
-$(LINUX_DIR)/.stamp_initramfs_rebuilt: $(LINUX_DIR)/.stamp_images_installed
-$(LINUX_DIR)/.stamp_initramfs_rebuilt: $(BINARIES_DIR)/rootfs.cpio
-$(LINUX_DIR)/.stamp_initramfs_rebuilt:
+# been generated.
+.PHONY: linux-rebuild-with-initramfs
+linux-rebuild-with-initramfs: $(LINUX_DIR)/.stamp_target_installed
+linux-rebuild-with-initramfs: $(LINUX_DIR)/.stamp_images_installed
+linux-rebuild-with-initramfs: rootfs-cpio
+linux-rebuild-with-initramfs:
 	@$(call MESSAGE,"Rebuilding kernel with initramfs")
 	# Build the kernel.
 	$(LINUX_MAKE_ENV) $(MAKE) $(LINUX_MAKE_FLAGS) -C $(@D) $(LINUX_TARGET_NAME)

linux/linux.mk $(LINUX_DIR)/.stamp_initramfs_rebuilt: · line 454

Quoted 4 lines
linux/linux.mk +0 -1
@@ -451,8 +452,3 @@ $(LINUX_DIR)/.stamp_initramfs_rebuilt:
 	cp $(LINUX_IMAGE_PATH) $(BINARIES_DIR)
 	# If there is a .ub file copy it to the final destination
 	test ! -f $(LINUX_IMAGE_PATH).ub || cp $(LINUX_IMAGE_PATH).ub $(BINARIES_DIR)
-	$(Q)touch $@

IIUC, the stamp file can be removed since the target is now PHONY, right ?

Quoted 4 lines

But latter in the patch 4/9 in the comment there is a reference to this file:
# Note: ordering of the dependencies is not guaranteed here, but in # linux/linux.mk, via the .stamp_initramfs_rebuilt stamp file, which depends

# on the rootfs-cpio filesystem rule.

I'll fix patch 4, then. Thanks!

Quoted 11 lines

linux/linux.mk $(LINUX_DIR)/.stamp_initramfs_rebuilt: · line 458

Quoted 4 lines
linux/linux.mk +0 -3
-# The initramfs building code must make sure this target gets called
-# after it generated the initramfs list of files.
-linux-rebuild-with-initramfs: $(LINUX_DIR)/.stamp_initramfs_rebuilt

Something is wrong...
With this patch applied, I have the following error when rebuilding the kernel with initramfs:

make[2]: *** No rule to make target 'bzImage'. Arr?t. linux/linux.mk:447: recipe for target 'linux-rebuild-with-initramfs' failed

Yep, that's because there is no longer any directory component in the target path, so $(@D) is just '.' (a dot). But as I just explained in my previous mail, I had it already fixed, just not on the right machine... :-/

Thanks!

Regards,
Yann E. MORIN.

Signature · 6 lines
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
#10 lore raw

PATCH v2 4/9 fs/initramfs: cleanup and enhance comments

In reply to Yann E. MORIN

rootfs-initramfs is not using the generic fs infrastructure, because there is virtually nothing to do to build the initramfs image: there is no actual image to be built to begin with.

The only purpose of rootfs-initramfs is to ensure the rootfs.cpio image is built and then that the Linux kernel is rebuilt with that rootfs.cpio as initramfs source.

Using variables of the fs infra like if it were used is misleading. It looked nice as long as there was the possibility that rootfs-initramfs would one day use the fs infra. But there's no way that will happen any time soon.

Furthermore, the linux' rule linux-rebuild-with-initramfs now already depends on rootfs-cpio by itself, so we need not duplicate this dependency in rootfs-initramfs.

Still, we want to advertise that the dependency is on rootfs-cpio, so we get nice dependency graphs (and not expose the internal linux-rebuild-with-initramfs rule to the users).

So, remove the variables and directly define the rules.

Cc Thomas Petazzoni <[email protected]>
---
 fs/initramfs/initramfs.mk | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)
fs/initramfs/initramfs.mk +15 -7
index db50812..9aca9c8 100644
@@ -5,17 +5,25 @@
 #
 ################################################################################
 
-ROOTFS_INITRAMFS_DEPENDENCIES += rootfs-cpio
-
-ROOTFS_INITRAMFS_POST_TARGETS += linux-rebuild-with-initramfs
-
-
 # The generic fs infrastructure isn't very useful here.
+#
+# The initramfs image does not actually build an image; its only purpose is:
+# 1- to ensure rootfs.cpio is generated,
+# 2- to then rebuild the kernel with rootfs.cpio as initramfs
+#
+# Note: ordering of the dependencies is not guaranteed here, but in
+# linux/linux.mk, via the .stamp_initramfs_rebuilt stamp file, which depends
+# on the rootfs-cpio filesystem rule.
+#
+# Note: the trick here is that we directly depend on rebuilding the Linux
+# kernel image (which itself depends on the rootfs-cpio rule), while we
+# advertise that our dependency is on the rootfs-cpio rule, which is
+# cleaner in the dependency graph.
 
-rootfs-initramfs: $(ROOTFS_INITRAMFS_DEPENDENCIES) $(ROOTFS_INITRAMFS_POST_TARGETS)
+rootfs-initramfs: linux-rebuild-with-initramfs
 
 rootfs-initramfs-show-depends:
-	@echo $(ROOTFS_INITRAMFS_DEPENDENCIES)
+	@echo rootfs-cpio
 
 .PHONY: rootfs-initramfs rootfs-initramfs-show-depends
 
Signature · 1 line
1.9.1
#11 lore raw

PATCH v2 4/9 fs/initramfs: cleanup and enhance comments

Hi Yann, All,

Le 11/03/2016 19:41, Yann E. MORIN a ?crit :

Quoted 41 lines

rootfs-initramfs is not using the generic fs infrastructure, because there is virtually nothing to do to build the initramfs image: there is no actual image to be built to begin with.

The only purpose of rootfs-initramfs is to ensure the rootfs.cpio image is built and then that the Linux kernel is rebuilt with that rootfs.cpio as initramfs source.

Using variables of the fs infra like if it were used is misleading. It looked nice as long as there was the possibility that rootfs-initramfs would one day use the fs infra. But there's no way that will happen any time soon.

Furthermore, the linux' rule linux-rebuild-with-initramfs now already depends on rootfs-cpio by itself, so we need not duplicate this dependency in rootfs-initramfs.
LINUX_DIR
Still, we want to advertise that the dependency is on rootfs-cpio, so we get nice dependency graphs (and not expose the internal linux-rebuild-with-initramfs rule to the users).

So, remove the variables and directly define the rules.

Cc Thomas Petazzoni <[email protected]>
---
 fs/initramfs/initramfs.mk | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)
fs/initramfs/initramfs.mk +0 -4
index db50812..9aca9c8 100644
@@ -5,17 +5,25 @@
 #
 ################################################################################
 
-ROOTFS_INITRAMFS_DEPENDENCIES += rootfs-cpio
-
-ROOTFS_INITRAMFS_POST_TARGETS += linux-rebuild-with-initramfs
-

fs/initramfs/initramfs.mk line 16

Quoted 10 lines
fs/initramfs/initramfs.mk +8 -1
-
 # The generic fs infrastructure isn't very useful here.
+#
+# The initramfs image does not actually build an image; its only purpose is:
+# 1- to ensure rootfs.cpio is generated,
+# 2- to then rebuild the kernel with rootfs.cpio as initramfs
+#
+# Note: ordering of the dependencies is not guaranteed here, but in
+# linux/linux.mk, via the .stamp_initramfs_rebuilt stamp file, which depends
+# on the rootfs-cpio filesystem rule.

As said previously, there is no more .stamp_initramfs_rebuilt file.

Otherwise

Best regards,
Romain

Quoted 16 lines
fs/initramfs/initramfs.mk +7 -2
+#
+# Note: the trick here is that we directly depend on rebuilding the Linux
+# kernel image (which itself depends on the rootfs-cpio rule), while we
+# advertise that our dependency is on the rootfs-cpio rule, which is
+# cleaner in the dependency graph.
 
-rootfs-initramfs: $(ROOTFS_INITRAMFS_DEPENDENCIES) $(ROOTFS_INITRAMFS_POST_TARGETS)
+rootfs-initramfs: linux-rebuild-with-initramfs
 
 rootfs-initramfs-show-depends:
-	@echo $(ROOTFS_INITRAMFS_DEPENDENCIES)
+	@echo rootfs-cpio
 
 .PHONY: rootfs-initramfs rootfs-initramfs-show-depends
 
#12 lore raw

PATCH v2 4/9 fs/initramfs: cleanup and enhance comments

Romain, All,

On 2016-03-28 20:05 +0200, Romain Naour spake thusly:

Quoted 38 lines

Le 11/03/2016 19:41, Yann E. MORIN a ?crit :

Quoted 37 lines

rootfs-initramfs is not using the generic fs infrastructure, because there is virtually nothing to do to build the initramfs image: there is no actual image to be built to begin with.

The only purpose of rootfs-initramfs is to ensure the rootfs.cpio image is built and then that the Linux kernel is rebuilt with that rootfs.cpio as initramfs source.

Using variables of the fs infra like if it were used is misleading. It looked nice as long as there was the possibility that rootfs-initramfs would one day use the fs infra. But there's no way that will happen any time soon.

Furthermore, the linux' rule linux-rebuild-with-initramfs now already depends on rootfs-cpio by itself, so we need not duplicate this dependency in rootfs-initramfs.
LINUX_DIR
Still, we want to advertise that the dependency is on rootfs-cpio, so we get nice dependency graphs (and not expose the internal linux-rebuild-with-initramfs rule to the users).

So, remove the variables and directly define the rules.

Cc Thomas Petazzoni <[email protected]>
---
 fs/initramfs/initramfs.mk | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)
fs/initramfs/initramfs.mk +0 -0
index db50812..9aca9c8 100644
@@ -5,17 +5,25 @@
 #
 ################################################################################
 
Quoted 16 lines
Quoted 4 lines
fs/initramfs/initramfs.mk +0 -4
-ROOTFS_INITRAMFS_DEPENDENCIES += rootfs-cpio
-
-ROOTFS_INITRAMFS_POST_TARGETS += linux-rebuild-with-initramfs
-

fs/initramfs/initramfs.mk line 16

Quoted 10 lines
fs/initramfs/initramfs.mk +8 -1
-
 # The generic fs infrastructure isn't very useful here.
+#
+# The initramfs image does not actually build an image; its only purpose is:
+# 1- to ensure rootfs.cpio is generated,
+# 2- to then rebuild the kernel with rootfs.cpio as initramfs
+#
+# Note: ordering of the dependencies is not guaranteed here, but in
+# linux/linux.mk, via the .stamp_initramfs_rebuilt stamp file, which depends
+# on the rootfs-cpio filesystem rule.

As said previously, there is no more .stamp_initramfs_rebuilt file.

Fixed.

Quoted 2 lines

Otherwise

Thanks! :-)

Regards,
Yann E. MORIN.

Quoted 20 lines

Best regards,
Romain

Quoted 16 lines
fs/initramfs/initramfs.mk +7 -2
+#
+# Note: the trick here is that we directly depend on rebuilding the Linux
+# kernel image (which itself depends on the rootfs-cpio rule), while we
+# advertise that our dependency is on the rootfs-cpio rule, which is
+# cleaner in the dependency graph.
 
-rootfs-initramfs: $(ROOTFS_INITRAMFS_DEPENDENCIES) $(ROOTFS_INITRAMFS_POST_TARGETS)
+rootfs-initramfs: linux-rebuild-with-initramfs
 
 rootfs-initramfs-show-depends:
-	@echo $(ROOTFS_INITRAMFS_DEPENDENCIES)
+	@echo rootfs-cpio
 
 .PHONY: rootfs-initramfs rootfs-initramfs-show-depends
 
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
#13 lore raw

PATCH v2 5/9 fs/ext2: use a post-gen hook rather than a post-target rule

In reply to Yann E. MORIN

post-target rules are probably not resilient to parallel builds, given that they do not depend on the image being generated first.

Beside, we already have a mechanism for running stuff after the filesystem is generated, and that's called post-gen hooks.

Use those hooks.

Note: this basically reverts 75b6303 (rootfs-ext2: make the symlink as a _POST_TARGET) since we've now re-introduced post-gen hooks.

Cc Thomas Petazzoni <[email protected]>
Cc Arnout Vandecappelle <[email protected]>
---
 fs/ext2/ext2.mk | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)
fs/ext2/ext2.mk +4 -6
index 7417f81..afe44c7 100644
@@ -34,13 +34,11 @@ define ROOTFS_EXT2_CMD
 	PATH=$(BR_PATH) mke2img -d $(TARGET_DIR) $(EXT2_OPTS) -o $@
 endef
 
-rootfs-ext2-symlink:
-	ln -sf rootfs.ext2$(ROOTFS_EXT2_COMPRESS_EXT) $(BINARIES_DIR)/rootfs.ext$(BR2_TARGET_ROOTFS_EXT2_GEN)$(ROOTFS_EXT2_COMPRESS_EXT)
-
-.PHONY: rootfs-ext2-symlink
-
 ifneq ($(BR2_TARGET_ROOTFS_EXT2_GEN),2)
-ROOTFS_EXT2_POST_TARGETS += rootfs-ext2-symlink
+define ROOTFS_EXT2_SYMLINK
+	ln -sf rootfs.ext2$(ROOTFS_EXT2_COMPRESS_EXT) $(BINARIES_DIR)/rootfs.ext$(BR2_TARGET_ROOTFS_EXT2_GEN)$(ROOTFS_EXT2_COMPRESS_EXT)
+endef
+ROOTFS_EXT2_POST_GEN_HOOKS += ROOTFS_EXT2_SYMLINK
 endif
 
 $(eval $(call ROOTFS_TARGET,ext2))
Signature · 1 line
1.9.1
#14 lore raw

PATCH v2 5/9 fs/ext2: use a post-gen hook rather than a post-target rule

Hi Yann, All,

Le 11/03/2016 19:41, Yann E. MORIN a ?crit :

Quoted 28 lines

post-target rules are probably not resilient to parallel builds, given that they do not depend on the image being generated first.

Beside, we already have a mechanism for running stuff after the filesystem is generated, and that's called post-gen hooks.

Use those hooks.

Note: this basically reverts 75b6303 (rootfs-ext2: make the symlink as a _POST_TARGET) since we've now re-introduced post-gen hooks.

Cc Thomas Petazzoni <[email protected]>
Cc Arnout Vandecappelle <[email protected]>
---
 fs/ext2/ext2.mk | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)
fs/ext2/ext2.mk +0 -2
index 7417f81..afe44c7 100644
@@ -34,13 +34,11 @@ define ROOTFS_EXT2_CMD
 	PATH=$(BR_PATH) mke2img -d $(TARGET_DIR) $(EXT2_OPTS) -o $@
 endef
 
-rootfs-ext2-symlink:
-	ln -sf rootfs.ext2$(ROOTFS_EXT2_COMPRESS_EXT) $(BINARIES_DIR)/rootfs.ext$(BR2_TARGET_ROOTFS_EXT2_GEN)$(ROOTFS_EXT2_COMPRESS_EXT)

fs/ext2/ext2.mk define ROOTFS_EXT2_CMD · line 42

Quoted 10 lines
fs/ext2/ext2.mk +4 -4
-
-.PHONY: rootfs-ext2-symlink
-
 ifneq ($(BR2_TARGET_ROOTFS_EXT2_GEN),2)
-ROOTFS_EXT2_POST_TARGETS += rootfs-ext2-symlink
+define ROOTFS_EXT2_SYMLINK
+	ln -sf rootfs.ext2$(ROOTFS_EXT2_COMPRESS_EXT) $(BINARIES_DIR)/rootfs.ext$(BR2_TARGET_ROOTFS_EXT2_GEN)$(ROOTFS_EXT2_COMPRESS_EXT)
+endef
+ROOTFS_EXT2_POST_GEN_HOOKS += ROOTFS_EXT2_SYMLINK
 endif

Verified that the rootfs.ext3 symlink to rootfs.ext2 is still created.

Best regards,
Romain

Quoted 3 lines
fs/ext2/ext2.mk +0 -0
 $(eval $(call ROOTFS_TARGET,ext2))
 
#15 lore raw

PATCH v2 6/9 fs/cpio: use a post-gen hook rather than a post-target rule

In reply to Yann E. MORIN

We already have a mechanism for running stuff after the filesystem is generated, and that's called post-gen hooks.

Use those hooks.

Note: for cpio (and unlike ext2 previously), the dependency chain was correct, in that the post-target rule correctly depended on the image rule. Nonetheless, we still want to fix it for consistency.

Cc Thomas Petazzoni <[email protected]>
Cc Arnout Vandecappelle <[email protected]>
---
 fs/cpio/cpio.mk | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)
fs/cpio/cpio.mk +6 -5
index e82167e..c68e0bf 100644
@@ -31,12 +31,13 @@ define ROOTFS_CPIO_CMD
 	cd $(TARGET_DIR) && find . | cpio --quiet -o -H newc > $@
 endef
 
-$(BINARIES_DIR)/rootfs.cpio.uboot: $(BINARIES_DIR)/rootfs.cpio host-uboot-tools
-	$(MKIMAGE) -A $(MKIMAGE_ARCH) -T ramdisk \
-		-C none -d $<$(ROOTFS_CPIO_COMPRESS_EXT) $@
-
 ifeq ($(BR2_TARGET_ROOTFS_CPIO_UIMAGE),y)
-ROOTFS_CPIO_POST_TARGETS += $(BINARIES_DIR)/rootfs.cpio.uboot
+ROOTFS_CPIO_DEPENDENCIES += host-uboot-tools
+define ROOTFS_CPIO_UBOOT_MKIMAGE
+	$(MKIMAGE) -A $(MKIMAGE_ARCH) -T ramdisk \
+		-C none -d $@$(ROOTFS_CPIO_COMPRESS_EXT) [email protected]
+endef
+ROOTFS_CPIO_POST_GEN_HOOKS += ROOTFS_CPIO_UBOOT_MKIMAGE
 endif
 
 $(eval $(call ROOTFS_TARGET,cpio))
Signature · 1 line
1.9.1
#16 lore raw

PATCH v2 6/9 fs/cpio: use a post-gen hook rather than a post-target rule

Hi Yann, All,
Le 11/03/2016 19:41, Yann E. MORIN a ?crit :

Quoted 27 lines

We already have a mechanism for running stuff after the filesystem is generated, and that's called post-gen hooks.

Use those hooks.

Note: for cpio (and unlike ext2 previously), the dependency chain was correct, in that the post-target rule correctly depended on the image rule. Nonetheless, we still want to fix it for consistency.

Cc Thomas Petazzoni <[email protected]>
Cc Arnout Vandecappelle <[email protected]>
---
 fs/cpio/cpio.mk | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)
fs/cpio/cpio.mk +0 -3
index e82167e..c68e0bf 100644
@@ -31,12 +31,13 @@ define ROOTFS_CPIO_CMD
 	cd $(TARGET_DIR) && find . | cpio --quiet -o -H newc > $@
 endef
 
-$(BINARIES_DIR)/rootfs.cpio.uboot: $(BINARIES_DIR)/rootfs.cpio host-uboot-tools
-	$(MKIMAGE) -A $(MKIMAGE_ARCH) -T ramdisk \
-		-C none -d $<$(ROOTFS_CPIO_COMPRESS_EXT) $@

fs/cpio/cpio.mk define ROOTFS_CPIO_CMD · line 41

Quoted 10 lines
fs/cpio/cpio.mk +6 -2
-
 ifeq ($(BR2_TARGET_ROOTFS_CPIO_UIMAGE),y)
-ROOTFS_CPIO_POST_TARGETS += $(BINARIES_DIR)/rootfs.cpio.uboot
+ROOTFS_CPIO_DEPENDENCIES += host-uboot-tools
+define ROOTFS_CPIO_UBOOT_MKIMAGE
+	$(MKIMAGE) -A $(MKIMAGE_ARCH) -T ramdisk \
+		-C none -d $@$(ROOTFS_CPIO_COMPRESS_EXT) [email protected]
+endef
+ROOTFS_CPIO_POST_GEN_HOOKS += ROOTFS_CPIO_UBOOT_MKIMAGE
 endif

Verified that rootfs.cpio.uboot is still build.

Best regards,
Romain

Quoted 3 lines
fs/cpio/cpio.mk +0 -0
 $(eval $(call ROOTFS_TARGET,cpio))
 
#17 lore raw

PATCH v2 7/9 fs/common: get rid of post-target rules

In reply to Yann E. MORIN

The only users of post-target rules were ext2, cpio and initramfs.

Of those, ext2 and cpio were changed to use post-gen hooks, while initramfs was not even using the generic rootfs infra and was fixed to no longer reference post-target rules.

Besides, the comment in the infra was really misleading: it referenced initramfs implying it was the sole user of that feature, even though initramfs was not using the fs infra.

Furthermore, using post-target rules was inherently broken for top-level parallel builds, because filesystems had to ensure the ordering by themselves. Of the two real users of post-target rules (cpio and ext2), one did enforce rules ordering (apparently correctly), while the other forgot to do so.

We can get rid of post-target rules altogether, now.

Cc Thomas Petazzoni <[email protected]>
Cc Arnout Vandecappelle <[email protected]>
---
 fs/common.mk | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)
fs/common.mk +1 -5
index 3a424f6..0b8163c 100644
@@ -19,10 +19,6 @@
 #  ROOTFS_$(FSTYPE)_POST_GEN_HOOKS, a list of hooks to call after
 #  generating the filesystem image
 #
-#  ROOTFS_$(FSTYPE)_POST_TARGETS, the list of targets that should be
-#  run after running the main filesystem target. This is useful for
-#  initramfs, to rebuild the kernel once the initramfs is generated.
-#
 # In terms of configuration option, this macro assumes that the
 # BR2_TARGET_ROOTFS_$(FSTYPE) config option allows to enable/disable
 # the generation of a filesystem image of a particular type. If
@@ -104,7 +100,7 @@ endif
 rootfs-$(1)-show-depends:
 	@echo $$(ROOTFS_$(2)_DEPENDENCIES)
 
-rootfs-$(1): $$(BINARIES_DIR)/rootfs.$(1) $$(ROOTFS_$(2)_POST_TARGETS)
+rootfs-$(1): $$(BINARIES_DIR)/rootfs.$(1)
 
 .PHONY: rootfs-$(1) rootfs-$(1)-show-depends
 
Signature · 1 line
1.9.1
#18 lore raw

PATCH v2 7/9 fs/common: get rid of post-target rules

Hi Yann, All,

Le 11/03/2016 19:41, Yann E. MORIN a ?crit :

Quoted 41 lines

The only users of post-target rules were ext2, cpio and initramfs.

Of those, ext2 and cpio were changed to use post-gen hooks, while initramfs was not even using the generic rootfs infra and was fixed to no longer reference post-target rules.

Besides, the comment in the infra was really misleading: it referenced initramfs implying it was the sole user of that feature, even though initramfs was not using the fs infra.

Furthermore, using post-target rules was inherently broken for top-level parallel builds, because filesystems had to ensure the ordering by themselves. Of the two real users of post-target rules (cpio and ext2), one did enforce rules ordering (apparently correctly), while the other forgot to do so.

We can get rid of post-target rules altogether, now.

Cc Thomas Petazzoni <[email protected]>
Cc Arnout Vandecappelle <[email protected]>
---
 fs/common.mk | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)
fs/common.mk +0 -4
index 3a424f6..0b8163c 100644
@@ -19,10 +19,6 @@
 #  ROOTFS_$(FSTYPE)_POST_GEN_HOOKS, a list of hooks to call after
 #  generating the filesystem image
 #
-#  ROOTFS_$(FSTYPE)_POST_TARGETS, the list of targets that should be
-#  run after running the main filesystem target. This is useful for
-#  initramfs, to rebuild the kernel once the initramfs is generated.
-#
 # In terms of configuration option, this macro assumes that the
 # BR2_TARGET_ROOTFS_$(FSTYPE) config option allows to enable/disable
 # the generation of a filesystem image of a particular type. If

fs/common.mk endif · line 103

Quoted 5 lines
fs/common.mk +1 -1
@@ -104,7 +100,7 @@ endif
 rootfs-$(1)-show-depends:
 	@echo $$(ROOTFS_$(2)_DEPENDENCIES)
 
-rootfs-$(1): $$(BINARIES_DIR)/rootfs.$(1) $$(ROOTFS_$(2)_POST_TARGETS)
+rootfs-$(1): $$(BINARIES_DIR)/rootfs.$(1)

Best regards,
Romain

Quoted 4 lines
fs/common.mk +0 -0
 .PHONY: rootfs-$(1) rootfs-$(1)-show-depends
 
 
#19 lore raw

PATCH v2 8/9 fs/common: move actions common to all filesystems to their own rule

In reply to Yann E. MORIN

A lot of actions are common to generating the various images. Currently, they are all done for each image being generated.

However, we can do them once and for all.

Also add a sha-bang to the fakeroot script, for completeness.

Cc Thomas Petazzoni <[email protected]>

---
Note: when I wrote that patch, it was only a preparatory work for something bigger and clever about how images are generated. However, I can't recall what it was... :-( Yet, I believe that patch to still be valid on its own.

---
 fs/common.mk | 57 ++++++++++++++++++++++++++++++++-------------------------
 1 file changed, 32 insertions(+), 25 deletions(-)
fs/common.mk +32 -25
index 0b8163c..d3733b5 100644
@@ -27,13 +27,36 @@
 # BR2_TARGET_ROOTFS_$(FSTYPE)_LZMA exist and are enabled, then the
 # macro will automatically generate a compressed filesystem image.
 
-FAKEROOT_SCRIPT = $(BUILD_DIR)/_fakeroot.fs
-FULL_DEVICE_TABLE = $(BUILD_DIR)/_device_table.txt
+FS_DIR = $(BUILD_DIR)/fs-common
+FAKEROOT_SCRIPT = $(FS_DIR)/fakeroot.common-base
+FULL_DEVICE_TABLE = $(FS_DIR)/device_table.txt
 ROOTFS_DEVICE_TABLES = $(call qstrip,$(BR2_ROOTFS_DEVICE_TABLE) \
 	$(BR2_ROOTFS_STATIC_DEVICE_TABLE))
-USERS_TABLE = $(BUILD_DIR)/_users_table.txt
+USERS_TABLE = $(FS_DIR)/users_table.txt
 ROOTFS_USERS_TABLES = $(call qstrip,$(BR2_ROOTFS_USERS_TABLES))
 
+.PHONY: rootfs-common
+rootfs-common: target-finalize
+	mkdir -p "$(FS_DIR)"
+	rm -f $(FAKEROOT_SCRIPT)
+	rm -f $(USERS_TABLE)
+	echo "#!/bin/sh" > $(FAKEROOT_SCRIPT)
+	echo "set -e" >> $(FAKEROOT_SCRIPT)
+	echo "chown -h -R 0:0 $(TARGET_DIR)" >> $(FAKEROOT_SCRIPT)
+ifneq ($(ROOTFS_USERS_TABLES),)
+	cat $(ROOTFS_USERS_TABLES) >> $(USERS_TABLE)
+endif
+	printf '$(subst $(sep),\n,$(PACKAGES_USERS))' >> $(USERS_TABLE)
+	PATH=$(BR_PATH) $(TOPDIR)/support/scripts/mkusers $(USERS_TABLE) $(TARGET_DIR) >> $(FAKEROOT_SCRIPT)
+ifneq ($(ROOTFS_DEVICE_TABLES),)
+	cat $(ROOTFS_DEVICE_TABLES) > $(FULL_DEVICE_TABLE)
+ifeq ($(BR2_ROOTFS_DEVICE_CREATION_STATIC),y)
+	printf '$(subst $(sep),\n,$(PACKAGES_DEVICES_TABLE))' >> $(FULL_DEVICE_TABLE)
+endif
+	printf '$(subst $(sep),\n,$(PACKAGES_PERMISSIONS_TABLE))' >> $(FULL_DEVICE_TABLE)
+	echo "$(HOST_DIR)/usr/bin/makedevs -d $(FULL_DEVICE_TABLE) $(TARGET_DIR)" >> $(FAKEROOT_SCRIPT)
+endif
+
 # Since this function will be called from within an $(eval ...)
 # all variable references except the arguments must be $$-quoted.
 define ROOTFS_TARGET_INTERNAL
@@ -66,32 +89,16 @@ ROOTFS_$(2)_COMPRESS_EXT = .xz
 ROOTFS_$(2)_COMPRESS_CMD = xz -9 -C crc32 -c
 endif
 
-$$(BINARIES_DIR)/rootfs.$(1): target-finalize $$(ROOTFS_$(2)_DEPENDENCIES)
+ROOTFS_$(2)_FAKEROOT_SCRIPT = $$(FS_DIR)/fakeroot.$(1)
+
+$$(BINARIES_DIR)/rootfs.$(1): rootfs-common $$(ROOTFS_$(2)_DEPENDENCIES)
 	@$$(call MESSAGE,"Generating root filesystem image rootfs.$(1)")
 	$$(foreach hook,$$(ROOTFS_$(2)_PRE_GEN_HOOKS),$$(call $$(hook))$$(sep))
-	rm -f $$(FAKEROOT_SCRIPT)
+	$$(INSTALL) -D -m 0755 $$(FAKEROOT_SCRIPT) $$(ROOTFS_$(2)_FAKEROOT_SCRIPT)
+	echo "$$(ROOTFS_$(2)_CMD)" >> $$(ROOTFS_$(2)_FAKEROOT_SCRIPT)
 	rm -f $$(TARGET_DIR_WARNING_FILE)
-	rm -f $$(USERS_TABLE)
-	echo "set -e" >> $$(FAKEROOT_SCRIPT)
-	echo "chown -h -R 0:0 $$(TARGET_DIR)" >> $$(FAKEROOT_SCRIPT)
-ifneq ($$(ROOTFS_USERS_TABLES),)
-	cat $$(ROOTFS_USERS_TABLES) >> $$(USERS_TABLE)
-endif
-	printf '$$(subst $$(sep),\n,$$(PACKAGES_USERS))' >> $$(USERS_TABLE)
-	PATH=$$(BR_PATH) $$(TOPDIR)/support/scripts/mkusers $$(USERS_TABLE) $$(TARGET_DIR) >> $$(FAKEROOT_SCRIPT)
-ifneq ($$(ROOTFS_DEVICE_TABLES),)
-	cat $$(ROOTFS_DEVICE_TABLES) > $$(FULL_DEVICE_TABLE)
-ifeq ($$(BR2_ROOTFS_DEVICE_CREATION_STATIC),y)
-	printf '$$(subst $$(sep),\n,$$(PACKAGES_DEVICES_TABLE))' >> $$(FULL_DEVICE_TABLE)
-endif
-	printf '$$(subst $$(sep),\n,$$(PACKAGES_PERMISSIONS_TABLE))' >> $$(FULL_DEVICE_TABLE)
-	echo "$$(HOST_DIR)/usr/bin/makedevs -d $$(FULL_DEVICE_TABLE) $$(TARGET_DIR)" >> $$(FAKEROOT_SCRIPT)
-endif
-	echo "$$(ROOTFS_$(2)_CMD)" >> $$(FAKEROOT_SCRIPT)
-	chmod a+x $$(FAKEROOT_SCRIPT)
-	PATH=$$(BR_PATH) $$(HOST_DIR)/usr/bin/fakeroot -- $$(FAKEROOT_SCRIPT)
+	PATH=$$(BR_PATH) $$(HOST_DIR)/usr/bin/fakeroot -- $$(ROOTFS_$(2)_FAKEROOT_SCRIPT)
 	$$(INSTALL) -m 0644 support/misc/target-dir-warning.txt $$(TARGET_DIR_WARNING_FILE)
-	- at rm -f $$(FAKEROOT_SCRIPT) $$(FULL_DEVICE_TABLE)
 ifneq ($$(ROOTFS_$(2)_COMPRESS_CMD),)
 	PATH=$$(BR_PATH) $$(ROOTFS_$(2)_COMPRESS_CMD) $$@ > $$@$$(ROOTFS_$(2)_COMPRESS_EXT)
 endif
Signature · 1 line
1.9.1
#20 lore raw

PATCH v2 8/9 fs/common: move actions common to all filesystems to their own rule

Hi Yann, All,

Le 11/03/2016 19:41, Yann E. MORIN a ?crit :

Quoted 7 lines

A lot of actions are common to generating the various images. Currently, they are all done for each image being generated.

However, we can do them once and for all.

Also add a sha-bang to the fakeroot script, for completeness.

Quoted 7 lines
Cc Thomas Petazzoni <[email protected]>

---
Note: when I wrote that patch, it was only a preparatory work for something bigger and clever about how images are generated. However, I can't recall what it was... :-( Yet, I believe that patch to still be valid on its own.

Maybe this patch was done to remove some $$ in the code :p

Quoted 74 lines
---
 fs/common.mk | 57 ++++++++++++++++++++++++++++++++-------------------------
 1 file changed, 32 insertions(+), 25 deletions(-)
fs/common.mk +31 -17
index 0b8163c..d3733b5 100644
@@ -27,13 +27,36 @@
 # BR2_TARGET_ROOTFS_$(FSTYPE)_LZMA exist and are enabled, then the
 # macro will automatically generate a compressed filesystem image.
 
-FAKEROOT_SCRIPT = $(BUILD_DIR)/_fakeroot.fs
-FULL_DEVICE_TABLE = $(BUILD_DIR)/_device_table.txt
+FS_DIR = $(BUILD_DIR)/fs-common
+FAKEROOT_SCRIPT = $(FS_DIR)/fakeroot.common-base
+FULL_DEVICE_TABLE = $(FS_DIR)/device_table.txt
 ROOTFS_DEVICE_TABLES = $(call qstrip,$(BR2_ROOTFS_DEVICE_TABLE) \
 	$(BR2_ROOTFS_STATIC_DEVICE_TABLE))
-USERS_TABLE = $(BUILD_DIR)/_users_table.txt
+USERS_TABLE = $(FS_DIR)/users_table.txt
 ROOTFS_USERS_TABLES = $(call qstrip,$(BR2_ROOTFS_USERS_TABLES))
 
+.PHONY: rootfs-common
+rootfs-common: target-finalize
+	mkdir -p "$(FS_DIR)"
+	rm -f $(FAKEROOT_SCRIPT)
+	rm -f $(USERS_TABLE)
+	echo "#!/bin/sh" > $(FAKEROOT_SCRIPT)
+	echo "set -e" >> $(FAKEROOT_SCRIPT)
+	echo "chown -h -R 0:0 $(TARGET_DIR)" >> $(FAKEROOT_SCRIPT)
+ifneq ($(ROOTFS_USERS_TABLES),)
+	cat $(ROOTFS_USERS_TABLES) >> $(USERS_TABLE)
+endif
+	printf '$(subst $(sep),\n,$(PACKAGES_USERS))' >> $(USERS_TABLE)
+	PATH=$(BR_PATH) $(TOPDIR)/support/scripts/mkusers $(USERS_TABLE) $(TARGET_DIR) >> $(FAKEROOT_SCRIPT)
+ifneq ($(ROOTFS_DEVICE_TABLES),)
+	cat $(ROOTFS_DEVICE_TABLES) > $(FULL_DEVICE_TABLE)
+ifeq ($(BR2_ROOTFS_DEVICE_CREATION_STATIC),y)
+	printf '$(subst $(sep),\n,$(PACKAGES_DEVICES_TABLE))' >> $(FULL_DEVICE_TABLE)
+endif
+	printf '$(subst $(sep),\n,$(PACKAGES_PERMISSIONS_TABLE))' >> $(FULL_DEVICE_TABLE)
+	echo "$(HOST_DIR)/usr/bin/makedevs -d $(FULL_DEVICE_TABLE) $(TARGET_DIR)" >> $(FAKEROOT_SCRIPT)
+endif
+
 # Since this function will be called from within an $(eval ...)
 # all variable references except the arguments must be $$-quoted.
 define ROOTFS_TARGET_INTERNAL
@@ -66,32 +89,16 @@ ROOTFS_$(2)_COMPRESS_EXT = .xz
 ROOTFS_$(2)_COMPRESS_CMD = xz -9 -C crc32 -c
 endif
 
-$$(BINARIES_DIR)/rootfs.$(1): target-finalize $$(ROOTFS_$(2)_DEPENDENCIES)
+ROOTFS_$(2)_FAKEROOT_SCRIPT = $$(FS_DIR)/fakeroot.$(1)
+
+$$(BINARIES_DIR)/rootfs.$(1): rootfs-common $$(ROOTFS_$(2)_DEPENDENCIES)
 	@$$(call MESSAGE,"Generating root filesystem image rootfs.$(1)")
 	$$(foreach hook,$$(ROOTFS_$(2)_PRE_GEN_HOOKS),$$(call $$(hook))$$(sep))
-	rm -f $$(FAKEROOT_SCRIPT)
+	$$(INSTALL) -D -m 0755 $$(FAKEROOT_SCRIPT) $$(ROOTFS_$(2)_FAKEROOT_SCRIPT)
+	echo "$$(ROOTFS_$(2)_CMD)" >> $$(ROOTFS_$(2)_FAKEROOT_SCRIPT)
 	rm -f $$(TARGET_DIR_WARNING_FILE)
-	rm -f $$(USERS_TABLE)
-	echo "set -e" >> $$(FAKEROOT_SCRIPT)
-	echo "chown -h -R 0:0 $$(TARGET_DIR)" >> $$(FAKEROOT_SCRIPT)
-ifneq ($$(ROOTFS_USERS_TABLES),)
-	cat $$(ROOTFS_USERS_TABLES) >> $$(USERS_TABLE)
-endif
-	printf '$$(subst $$(sep),\n,$$(PACKAGES_USERS))' >> $$(USERS_TABLE)
-	PATH=$$(BR_PATH) $$(TOPDIR)/support/scripts/mkusers $$(USERS_TABLE) $$(TARGET_DIR) >> $$(FAKEROOT_SCRIPT)
-ifneq ($$(ROOTFS_DEVICE_TABLES),)
-	cat $$(ROOTFS_DEVICE_TABLES) > $$(FULL_DEVICE_TABLE)
-ifeq ($$(BR2_ROOTFS_DEVICE_CREATION_STATIC),y)
-	printf '$$(subst $$(sep),\n,$$(PACKAGES_DEVICES_TABLE))' >> $$(FULL_DEVICE_TABLE)

fs/common.mk ROOTFS_$(2)_COMPRESS_EXT = .xz · line 94

Quoted 10 lines
fs/common.mk +1 -8
-endif
-	printf '$$(subst $$(sep),\n,$$(PACKAGES_PERMISSIONS_TABLE))' >> $$(FULL_DEVICE_TABLE)
-	echo "$$(HOST_DIR)/usr/bin/makedevs -d $$(FULL_DEVICE_TABLE) $$(TARGET_DIR)" >> $$(FAKEROOT_SCRIPT)
-endif
-	echo "$$(ROOTFS_$(2)_CMD)" >> $$(FAKEROOT_SCRIPT)
-	chmod a+x $$(FAKEROOT_SCRIPT)
-	PATH=$$(BR_PATH) $$(HOST_DIR)/usr/bin/fakeroot -- $$(FAKEROOT_SCRIPT)
+	PATH=$$(BR_PATH) $$(HOST_DIR)/usr/bin/fakeroot -- $$(ROOTFS_$(2)_FAKEROOT_SCRIPT)
 	$$(INSTALL) -m 0644 support/misc/target-dir-warning.txt $$(TARGET_DIR_WARNING_FILE)
-	- at rm -f $$(FAKEROOT_SCRIPT) $$(FULL_DEVICE_TABLE)

I think it's a good idea to keep all fakeroot.* script used during rootfs images build.

It seems that there is no need to add PATH=$(BR_PATH) in ROOTFS_EXT2_CMD since it's already present in fakeroot environment ? (see 89e16118b45c1bb8d112e8829d34e0883a6450be)

Best regards,
Romain

Quoted 4 lines
fs/common.mk +0 -0
 ifneq ($$(ROOTFS_$(2)_COMPRESS_CMD),)
 	PATH=$$(BR_PATH) $$(ROOTFS_$(2)_COMPRESS_CMD) $$@ > $$@$$(ROOTFS_$(2)_COMPRESS_EXT)
 endif
#21 lore raw

PATCH v2 8/9 fs/common: move actions common to all filesystems to their own rule

Romain, All,

On 2016-03-28 23:12 +0200, Romain Naour spake thusly:

Quoted 11 lines

Le 11/03/2016 19:41, Yann E. MORIN a ?crit :

Quoted 10 lines

A lot of actions are common to generating the various images. Currently, they are all done for each image being generated.

However, we can do them once and for all.

Also add a sha-bang to the fakeroot script, for completeness.

Cc Thomas Petazzoni <[email protected]>
Quoted 6 lines
Quoted 4 lines

---
Note: when I wrote that patch, it was only a preparatory work for something bigger and clever about how images are generated. However, I can't recall what it was... :-( Yet, I believe that patch to still be valid on its own.

Maybe this patch was done to remove some $$ in the code :p

Hehe! However, who would complain for having too many $ ? ;-]

[--SNIP--]

Quoted 40 lines
Quoted 26 lines
fs/common.mk +5 -14
@@ -66,32 +89,16 @@ ROOTFS_$(2)_COMPRESS_EXT = .xz
 ROOTFS_$(2)_COMPRESS_CMD = xz -9 -C crc32 -c
 endif
 
-$$(BINARIES_DIR)/rootfs.$(1): target-finalize $$(ROOTFS_$(2)_DEPENDENCIES)
+ROOTFS_$(2)_FAKEROOT_SCRIPT = $$(FS_DIR)/fakeroot.$(1)
+
+$$(BINARIES_DIR)/rootfs.$(1): rootfs-common $$(ROOTFS_$(2)_DEPENDENCIES)
 	@$$(call MESSAGE,"Generating root filesystem image rootfs.$(1)")
 	$$(foreach hook,$$(ROOTFS_$(2)_PRE_GEN_HOOKS),$$(call $$(hook))$$(sep))
-	rm -f $$(FAKEROOT_SCRIPT)
+	$$(INSTALL) -D -m 0755 $$(FAKEROOT_SCRIPT) $$(ROOTFS_$(2)_FAKEROOT_SCRIPT)
+	echo "$$(ROOTFS_$(2)_CMD)" >> $$(ROOTFS_$(2)_FAKEROOT_SCRIPT)
 	rm -f $$(TARGET_DIR_WARNING_FILE)
-	rm -f $$(USERS_TABLE)
-	echo "set -e" >> $$(FAKEROOT_SCRIPT)
-	echo "chown -h -R 0:0 $$(TARGET_DIR)" >> $$(FAKEROOT_SCRIPT)
-ifneq ($$(ROOTFS_USERS_TABLES),)
-	cat $$(ROOTFS_USERS_TABLES) >> $$(USERS_TABLE)
-endif
-	printf '$$(subst $$(sep),\n,$$(PACKAGES_USERS))' >> $$(USERS_TABLE)
-	PATH=$$(BR_PATH) $$(TOPDIR)/support/scripts/mkusers $$(USERS_TABLE) $$(TARGET_DIR) >> $$(FAKEROOT_SCRIPT)
-ifneq ($$(ROOTFS_DEVICE_TABLES),)
-	cat $$(ROOTFS_DEVICE_TABLES) > $$(FULL_DEVICE_TABLE)
-ifeq ($$(BR2_ROOTFS_DEVICE_CREATION_STATIC),y)
-	printf '$$(subst $$(sep),\n,$$(PACKAGES_DEVICES_TABLE))' >> $$(FULL_DEVICE_TABLE)

fs/common.mk ROOTFS_$(2)_COMPRESS_EXT = .xz · line 94

Quoted 10 lines
fs/common.mk +1 -8
-endif
-	printf '$$(subst $$(sep),\n,$$(PACKAGES_PERMISSIONS_TABLE))' >> $$(FULL_DEVICE_TABLE)
-	echo "$$(HOST_DIR)/usr/bin/makedevs -d $$(FULL_DEVICE_TABLE) $$(TARGET_DIR)" >> $$(FAKEROOT_SCRIPT)
-endif
-	echo "$$(ROOTFS_$(2)_CMD)" >> $$(FAKEROOT_SCRIPT)
-	chmod a+x $$(FAKEROOT_SCRIPT)
-	PATH=$$(BR_PATH) $$(HOST_DIR)/usr/bin/fakeroot -- $$(FAKEROOT_SCRIPT)
+	PATH=$$(BR_PATH) $$(HOST_DIR)/usr/bin/fakeroot -- $$(ROOTFS_$(2)_FAKEROOT_SCRIPT)
 	$$(INSTALL) -m 0644 support/misc/target-dir-warning.txt $$(TARGET_DIR_WARNING_FILE)
-	- at rm -f $$(FAKEROOT_SCRIPT) $$(FULL_DEVICE_TABLE)

I think it's a good idea to keep all fakeroot.* script used during rootfs images build.

Quoted 5 lines

It seems that there is no need to add PATH=$(BR_PATH) in ROOTFS_EXT2_CMD since it's already present in fakeroot environment ? (see 89e16118b45c1bb8d112e8829d34e0883a6450be)

Indeed, but that should be done in a separate patch (since it was not added in my series). I'll add such a patch to the series.

Thanks! :-)

Regards,
Yann E. MORIN.

Quoted 8 lines

Best regards,
Romain

Quoted 4 lines
fs/common.mk +0 -0
 ifneq ($$(ROOTFS_$(2)_COMPRESS_CMD),)
 	PATH=$$(BR_PATH) $$(ROOTFS_$(2)_COMPRESS_CMD) $$@ > $$@$$(ROOTFS_$(2)_COMPRESS_EXT)
 endif
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
#22 lore raw

RFC PATCH v2 9/9 core: finalise target in its own location

In reply to Yann E. MORIN

Currently, after all packages have been installed into target/ , we run a sanitising pass called 'target-finalize' on that directory, to:
- apply overlays
- remove unnecessary files (man, .h, .a ...)
- strip files
as well as a few other miscellanous cleanups.

This means that target/ no longer contains only package-installed files, and that target-finalize might not be idempotent (i.e. sucessive runs of target-finalize may yield different results in target/ ). We're trying pretty hard that all the internal target-finalize hooks are idempotent, whether they are from the core (e.g. installing glibc locales) or provided by packages (e.g. cleaning up perl files).

However, that might not be the case for packages from br2-external for example, or under complex situations where a combination of packages does not yield an idempotent sequence (quoting Wikipedia: "a combination of idempotent methods or subroutines is not necessrily idempotent"; see:
https://en.wikipedia.org/wiki/Idempotence#Examples ).

Address this issue by copying target/ to a "landing" area where the finalising takes place.

This keeps the user-visible target/ directory to contain only what packages have installed, helps keeping target-finalize be idempotent by allowing packages to provide simpler target-finalize hooks.

For simplicity for packages, we have to allow them to use $(TARGET_DIR) everywhere, be it in target install commands or target finalize commands, without requiring them to know whether to use $(TARGET_DIR) or $(FINAL_TARGET_DIR). $(TARGET_DIR) always points to the directory in which to act.

So, for target-finalize, we override TARGET_DIR to point to the landing area. But since packages are dependencies of target-finalize, they would also inherit from this override. So, we over-override TARGET_DIR for packages, to point to the usual and currently used $(O)/target/ directory.

Finally, filesystem images are generated from that landing area, of course. Similarly, we need to override TARGET_DIR for them.

Cc Thomas Petazzoni <[email protected]>

---
Note: Thomas already express his dislike about that patch. Yet, for posterity, here it goes updated to current master and tagged as RFC...

---
 Makefile     | 28 +++++++++++++++++++++++++++-
 fs/common.mk |  4 ++--
 2 files changed, 29 insertions(+), 3 deletions(-)
Makefile +27 -1
index 0f16976..a401788 100644
@@ -183,7 +183,9 @@ BR_GRAPH_OUT := $(or $(BR2_GRAPH_OUT),pdf)
 
 BUILD_DIR := $(BASE_DIR)/build
 BINARIES_DIR := $(BASE_DIR)/images
-TARGET_DIR := $(BASE_DIR)/target
+BUILD_TARGET_DIR := $(BASE_DIR)/target
+TARGET_DIR := $(BUILD_TARGET_DIR)
+FINAL_TARGET_DIR := $(BUILD_DIR)/target-finalize
 # initial definition so that 'make clean' works for most users, even without
 # .config. HOST_DIR will be overwritten later when .config is included.
 HOST_DIR := $(BASE_DIR)/host
@@ -606,8 +608,32 @@ $(TARGETS_ROOTFS): target-finalize
 # multiple times in the list returned by show-target.
 PACKAGES := $(sort $(PACKAGES))
 
+# Finalizing the target directory involves:
+#  - applying overlays,
+#  - removing unecessary files (man, .h, .a ...)
+#  - tweaking files installed by packages (like stripping)
+# and miscellanous cleanups.
+#
+# We want to keep the $(O)/target/ directory as a perfect image of
+# what packages have actually installed, so we copy it to a landing
+# location where we'll do those cleanups.
+#
+# Still, we only document $(TARGET_DIR) so packages that want to provide
+# target-finalize hooks will be using that. Thus, we just override that
+# variable with the landing location just for target-finalize.
+#
+# However, since rule-overriden variables are inherited by the dependencies
+# of that rule, we must re-override TARGET_DIR with its original location
+# for packages.
+#
+$(PACKAGES): TARGET_DIR=$(BUILD_TARGET_DIR)
+target-finalize: TARGET_DIR=$(FINAL_TARGET_DIR)
+
 target-finalize: $(PACKAGES)
 	@$(call MESSAGE,"Finalizing target directory")
+	$(Q)rm -rf $(TARGET_DIR)
+	$(Q)cp -a $(BUILD_TARGET_DIR) $(TARGET_DIR)
+	$(Q)rm -f $(TARGET_DIR_WARNING_FILE)
 	$(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 \
fs/common.mk +2 -2
index d3733b5..197bee4 100644
@@ -36,6 +36,7 @@ USERS_TABLE = $(FS_DIR)/users_table.txt
 ROOTFS_USERS_TABLES = $(call qstrip,$(BR2_ROOTFS_USERS_TABLES))
 
 .PHONY: rootfs-common
+rootfs-common: TARGET_DIR=$(FINAL_TARGET_DIR)
 rootfs-common: target-finalize
 	mkdir -p "$(FS_DIR)"
 	rm -f $(FAKEROOT_SCRIPT)
@@ -91,14 +92,13 @@ endif
 
 ROOTFS_$(2)_FAKEROOT_SCRIPT = $$(FS_DIR)/fakeroot.$(1)
 
+$$(BINARIES_DIR)/rootfs.$(1): TARGET_DIR=$(FINAL_TARGET_DIR)
 $$(BINARIES_DIR)/rootfs.$(1): rootfs-common $$(ROOTFS_$(2)_DEPENDENCIES)
 	@$$(call MESSAGE,"Generating root filesystem image rootfs.$(1)")
 	$$(foreach hook,$$(ROOTFS_$(2)_PRE_GEN_HOOKS),$$(call $$(hook))$$(sep))
 	$$(INSTALL) -D -m 0755 $$(FAKEROOT_SCRIPT) $$(ROOTFS_$(2)_FAKEROOT_SCRIPT)
 	echo "$$(ROOTFS_$(2)_CMD)" >> $$(ROOTFS_$(2)_FAKEROOT_SCRIPT)
-	rm -f $$(TARGET_DIR_WARNING_FILE)
 	PATH=$$(BR_PATH) $$(HOST_DIR)/usr/bin/fakeroot -- $$(ROOTFS_$(2)_FAKEROOT_SCRIPT)
-	$$(INSTALL) -m 0644 support/misc/target-dir-warning.txt $$(TARGET_DIR_WARNING_FILE)
 ifneq ($$(ROOTFS_$(2)_COMPRESS_CMD),)
 	PATH=$$(BR_PATH) $$(ROOTFS_$(2)_COMPRESS_CMD) $$@ > $$@$$(ROOTFS_$(2)_COMPRESS_EXT)
 endif
Signature · 1 line
1.9.1