* [Buildroot] [PATCH] fs/iso9660: add dependencies for make source @ 2014-10-17 21:16 Karoly Kasza 2014-10-18 10:31 ` Maxime Hadjinlian 0 siblings, 1 reply; 7+ messages in thread From: Karoly Kasza @ 2014-10-17 21:16 UTC (permalink / raw) To: buildroot 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [Buildroot] [PATCH] fs/iso9660: add dependencies for make source 2014-10-17 21:16 [Buildroot] [PATCH] fs/iso9660: add dependencies for make source Karoly Kasza @ 2014-10-18 10:31 ` Maxime Hadjinlian 2014-10-18 13:31 ` Arnout Vandecappelle 0 siblings, 1 reply; 7+ messages in thread From: Maxime Hadjinlian @ 2014-10-18 10:31 UTC (permalink / raw) To: buildroot 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [Buildroot] [PATCH] fs/iso9660: add dependencies for make source 2014-10-18 10:31 ` Maxime Hadjinlian @ 2014-10-18 13:31 ` Arnout Vandecappelle 2014-10-18 14:17 ` Károly Kasza 0 siblings, 1 reply; 7+ messages in thread From: Arnout Vandecappelle @ 2014-10-18 13:31 UTC (permalink / raw) To: buildroot 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [Buildroot] [PATCH] fs/iso9660: add dependencies for make source 2014-10-18 13:31 ` Arnout Vandecappelle @ 2014-10-18 14:17 ` Károly Kasza 2014-10-18 16:47 ` Thomas Petazzoni 0 siblings, 1 reply; 7+ messages in thread From: Károly Kasza @ 2014-10-18 14:17 UTC (permalink / raw) To: buildroot 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> ^ permalink raw reply [flat|nested] 7+ messages in thread
* [Buildroot] [PATCH] fs/iso9660: add dependencies for make source 2014-10-18 14:17 ` Károly Kasza @ 2014-10-18 16:47 ` Thomas Petazzoni 2014-10-18 17:13 ` Károly Kasza 0 siblings, 1 reply; 7+ messages in thread From: Thomas Petazzoni @ 2014-10-18 16:47 UTC (permalink / raw) To: buildroot 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [Buildroot] [PATCH] fs/iso9660: add dependencies for make source 2014-10-18 16:47 ` Thomas Petazzoni @ 2014-10-18 17:13 ` Károly Kasza 2014-10-18 17:23 ` Thomas Petazzoni 0 siblings, 1 reply; 7+ messages in thread From: Károly Kasza @ 2014-10-18 17:13 UTC (permalink / raw) To: buildroot > > > > > 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> ^ permalink raw reply [flat|nested] 7+ messages in thread
* [Buildroot] [PATCH] fs/iso9660: add dependencies for make source 2014-10-18 17:13 ` Károly Kasza @ 2014-10-18 17:23 ` Thomas Petazzoni 0 siblings, 0 replies; 7+ messages in thread From: Thomas Petazzoni @ 2014-10-18 17:23 UTC (permalink / raw) To: buildroot 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2014-10-18 17:23 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-10-17 21:16 [Buildroot] [PATCH] fs/iso9660: add dependencies for make source Karoly Kasza 2014-10-18 10:31 ` Maxime Hadjinlian 2014-10-18 13:31 ` Arnout Vandecappelle 2014-10-18 14:17 ` Károly Kasza 2014-10-18 16:47 ` Thomas Petazzoni 2014-10-18 17:13 ` Károly Kasza 2014-10-18 17:23 ` Thomas Petazzoni
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox