[Buildroot] [PATCH] fs/iso9660: add dependencies for make source

[Buildroot] [PATCH] fs/iso9660: add dependencies for make source

[Karoly Kasza]

The "iso image" rootfs target is a special one, as it does not use the
ROOTFS_TARGET infrastructure. The absence of ROOTFS_*_DEPENDENCIES
variable makes "make source" to skip this target's dependencies
(namely host-cdrkit and it's children) obstructing an offline build.

Signed-off-by: Karoly Kasza <kaszak@gmail.com>
---
 fs/iso9660/iso9660.mk |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/iso9660/iso9660.mk b/fs/iso9660/iso9660.mk
index b0e755d..5137026 100644
--- a/fs/iso9660/iso9660.mk
+++ b/fs/iso9660/iso9660.mk
@@ -10,6 +10,8 @@
 ISO9660_TARGET_DIR = $(BUILD_DIR)/iso9660
 ISO9660_BOOT_MENU := $(call qstrip,$(BR2_TARGET_ROOTFS_ISO9660_BOOT_MENU))
 
+ROOTFS_ISO9660_DEPENDENCIES = host-cdrkit host-fakeroot linux rootfs-cpio grub
+
 $(BINARIES_DIR)/rootfs.iso9660: host-cdrkit host-fakeroot linux rootfs-cpio grub
 	@$(call MESSAGE,"Generating root filesystem image rootfs.iso9660")
 	mkdir -p $(ISO9660_TARGET_DIR)
-- 
1.7.10.4

[Buildroot] [PATCH] fs/iso9660: add dependencies for make source

[Maxime Hadjinlian]

Hi Karoly, all

On Fri, Oct 17, 2014 at 11:16 PM, Karoly Kasza <kaszak@gmail.com> wrote:
> The "iso image" rootfs target is a special one, as it does not use the
> ROOTFS_TARGET infrastructure. The absence of ROOTFS_*_DEPENDENCIES
> variable makes "make source" to skip this target's dependencies
> (namely host-cdrkit and it's children) obstructing an offline build.
>
> Signed-off-by: Karoly Kasza <kaszak@gmail.com>
> ---
>  fs/iso9660/iso9660.mk |    2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/fs/iso9660/iso9660.mk b/fs/iso9660/iso9660.mk
> index b0e755d..5137026 100644
> --- a/fs/iso9660/iso9660.mk
> +++ b/fs/iso9660/iso9660.mk
> @@ -10,6 +10,8 @@
>  ISO9660_TARGET_DIR = $(BUILD_DIR)/iso9660
>  ISO9660_BOOT_MENU := $(call qstrip,$(BR2_TARGET_ROOTFS_ISO9660_BOOT_MENU))
>
> +ROOTFS_ISO9660_DEPENDENCIES = host-cdrkit host-fakeroot linux rootfs-cpio grub
> +
>  $(BINARIES_DIR)/rootfs.iso9660: host-cdrkit host-fakeroot linux rootfs-cpio grub
>         @$(call MESSAGE,"Generating root filesystem image rootfs.iso9660")
>         mkdir -p $(ISO9660_TARGET_DIR)
> --
> 1.7.10.4
How about putting the dependencies variables on the top ?
Also, please keep the dependencies alphabetically sorted.
I know they are not ordered the line below, but it would be nice.
>
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

[Buildroot] [PATCH] fs/iso9660: add dependencies for make source

[Arnout Vandecappelle]

On 18/10/14 12:31, Maxime Hadjinlian wrote:
> Hi Karoly, all
> 
> On Fri, Oct 17, 2014 at 11:16 PM, Karoly Kasza <kaszak@gmail.com> wrote:
>> The "iso image" rootfs target is a special one, as it does not use the
>> ROOTFS_TARGET infrastructure. The absence of ROOTFS_*_DEPENDENCIES
>> variable makes "make source" to skip this target's dependencies
>> (namely host-cdrkit and it's children) obstructing an offline build.

 Good catch!

>>
>> Signed-off-by: Karoly Kasza <kaszak@gmail.com>
>> ---
>>  fs/iso9660/iso9660.mk |    2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/fs/iso9660/iso9660.mk b/fs/iso9660/iso9660.mk
>> index b0e755d..5137026 100644
>> --- a/fs/iso9660/iso9660.mk
>> +++ b/fs/iso9660/iso9660.mk
>> @@ -10,6 +10,8 @@
>>  ISO9660_TARGET_DIR = $(BUILD_DIR)/iso9660
>>  ISO9660_BOOT_MENU := $(call qstrip,$(BR2_TARGET_ROOTFS_ISO9660_BOOT_MENU))
>>
>> +ROOTFS_ISO9660_DEPENDENCIES = host-cdrkit host-fakeroot linux rootfs-cpio grub
>> +
>>  $(BINARIES_DIR)/rootfs.iso9660: host-cdrkit host-fakeroot linux rootfs-cpio grub

 You should use $(ROOTFS_ISO9660_DEPENDENCIES) here.

>>         @$(call MESSAGE,"Generating root filesystem image rootfs.iso9660")
>>         mkdir -p $(ISO9660_TARGET_DIR)
>> --
>> 1.7.10.4
> How about putting the dependencies variables on the top ?
> Also, please keep the dependencies alphabetically sorted.
> I know they are not ordered the line below, but it would be nice.

 Well, now they're ordered according to how they appear in the makefiles, i.e.
first the real packages, then the kernel, then the rootfs, then the bootloader.

 Hm, actually, the bootloader should come before the kernel in that reasoning...

 So alphabetically is probably better :-)


 Regards,
 Arnout

>>
>> _______________________________________________
>> buildroot mailing list
>> buildroot at busybox.net
>> http://lists.busybox.net/mailman/listinfo/buildroot
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
> 


-- 
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

[Buildroot] [PATCH] fs/iso9660: add dependencies for make source

[Károly Kasza]

Hi,

>>
> >> +ROOTFS_ISO9660_DEPENDENCIES = host-cdrkit host-fakeroot linux
> rootfs-cpio grub
> >> +
> >>  $(BINARIES_DIR)/rootfs.iso9660: host-cdrkit host-fakeroot linux
> rootfs-cpio grub
>
>  You should use $(ROOTFS_ISO9660_DEPENDENCIES) here.
>
>
Why? Non of the other rootfs target makefiles use that (fs/*/*.mk).


> > How about putting the dependencies variables on the top ?
> > Also, please keep the dependencies alphabetically sorted.
> > I know they are not ordered the line below, but it would be nice.
>
>  Well, now they're ordered according to how they appear in the makefiles,
> i.e.
> first the real packages, then the kernel, then the rootfs, then the
> bootloader.
>
>  Hm, actually, the bootloader should come before the kernel in that
> reasoning...
>
>  So alphabetically is probably better :-)
>

Technically, the dependencies variable is already at the top, before the
first make target.
Also, it's not on the exact top of the file in any other rootfs target
makefiles.
Regarding the order: I don't really think that matters.

Regards,
Karoly
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20141018/89f7f1cc/attachment.html>

[Buildroot] [PATCH] fs/iso9660: add dependencies for make source

[Thomas Petazzoni]

Dear K?roly Kasza,

On Sat, 18 Oct 2014 16:17:46 +0200, K?roly Kasza wrote:

> >  You should use $(ROOTFS_ISO9660_DEPENDENCIES) here.
> >
> Why? Non of the other rootfs target makefiles use that (fs/*/*.mk).

All the other rootfs target makefiles use <foo>_DEPENDENCIES. Except
that it's taken into account directly by the common filesystem logic in
fs/common.mk. However, iso9660 is special, and therefore we handle the
dependencies manually. But still, there's no need to repeat them twice.
So, in your patch, change:

ROOTFS_ISO9660_DEPENDENCIES = host-cdrkit host-fakeroot linux rootfs-cpio grub

$(BINARIES_DIR)/rootfs.iso9660: host-cdrkit host-fakeroot linux rootfs-cpio grub

To:

ROOTFS_ISO9660_DEPENDENCIES = host-cdrkit host-fakeroot linux rootfs-cpio grub

$(BINARIES_DIR)/rootfs.iso9660: $(ROOTFS_ISO9660_DEPENDENCIES)

> Technically, the dependencies variable is already at the top, before the
> first make target.

Agreed.

> Regarding the order: I don't really think that matters.

Well, it's indeed just a subjective thing, like most coding style
things. Still, when a core Buildroot contributor such as Arnout
suggests coding style changes, it's usually a good idea to follow suit
to get the patch merged quickly :)

That being said, I agree that we don't enforce any particular order for
the dependencies in the other packages.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH] fs/iso9660: add dependencies for make source

[Károly Kasza]

>
>
> > >  You should use $(ROOTFS_ISO9660_DEPENDENCIES) here.
> > >
> > Why? Non of the other rootfs target makefiles use that (fs/*/*.mk).
>
> All the other rootfs target makefiles use <foo>_DEPENDENCIES. Except
> that it's taken into account directly by the common filesystem logic in
> fs/common.mk. However, iso9660 is special, and therefore we handle the
> dependencies manually. But still, there's no need to repeat them twice.
> So, in your patch, change:
>
> ROOTFS_ISO9660_DEPENDENCIES = host-cdrkit host-fakeroot linux rootfs-cpio
> grub
>
> $(BINARIES_DIR)/rootfs.iso9660: host-cdrkit host-fakeroot linux
> rootfs-cpio grub
>
> To:
>
> ROOTFS_ISO9660_DEPENDENCIES = host-cdrkit host-fakeroot linux rootfs-cpio
> grub
>
> $(BINARIES_DIR)/rootfs.iso9660: $(ROOTFS_ISO9660_DEPENDENCIES)
>
> Oh, that's correct. I misunderstood the first proposal of Arnout.

> Regarding the order: I don't really think that matters.
>
> Well, it's indeed just a subjective thing, like most coding style
> things. Still, when a core Buildroot contributor such as Arnout
> suggests coding style changes, it's usually a good idea to follow suit
> to get the patch merged quickly :)
>
> That being said, I agree that we don't enforce any particular order for
> the dependencies in the other packages.
>

OK, but this means the changing of the order of dependencies is not at all
connected with
the original purpose of this patch, also everywhere else it should be
changed I guess.

I'll send a V2.

Best regards,
Karoly
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20141018/c653e863/attachment.html>

[Buildroot] [PATCH] fs/iso9660: add dependencies for make source

[Thomas Petazzoni]

Dear K?roly Kasza,

On Sat, 18 Oct 2014 19:13:11 +0200, K?roly Kasza wrote:

> OK, but this means the changing of the order of dependencies is not at all
> connected with the original purpose of this patch, also everywhere
> else it should be changed I guess.

I agree the order of the dependencies is a separate thing. And it's
also a bit strange to enforce an ordering for this case, while we do
not have such a rule for all other packages.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com