* [Buildroot] [PATCH v3] package/dosfstools: introduce custom install routine
@ 2019-05-30 23:42 Markus Mayer
2019-05-31 14:34 ` Thomas Petazzoni
0 siblings, 1 reply; 4+ messages in thread
From: Markus Mayer @ 2019-05-30 23:42 UTC (permalink / raw)
To: buildroot
We can't use dosfstools' install target, because it'll install *all*
binaries, even the disabled ones. Also, we can't just delete dosfstools
binaries from the target directory after installing them, because other
packages (specifically Busybox) may provide tools of the same name, and
we may end up deleting those instead.
To avoid any issues, we create our own install routine, which only
copies the enabled binaries into the target location.
Signed-off-by: Markus Mayer <mmayer@broadcom.com>
---
Changes since v2:
- use custom install routine that only copies the binaries we want
Changes since v1:
- don't bother removing man page files
- use rsync instead of tar to copy files to the destination
package/dosfstools/dosfstools.mk | 37 +++++++++++++++++++++-----------
1 file changed, 25 insertions(+), 12 deletions(-)
diff --git a/package/dosfstools/dosfstools.mk b/package/dosfstools/dosfstools.mk
index 6eb0851d0e23..d32f3e22c05c 100644
--- a/package/dosfstools/dosfstools.mk
+++ b/package/dosfstools/dosfstools.mk
@@ -24,26 +24,39 @@ DOSFSTOOLS_CONF_OPTS += LIBS="-liconv"
DOSFSTOOLS_DEPENDENCIES += libiconv
endif
-ifeq ($(BR2_PACKAGE_DOSFSTOOLS_FATLABEL),)
-define DOSFSTOOLS_REMOVE_FATLABEL
- rm -f $(addprefix $(TARGET_DIR)/sbin/,dosfslabel fatlabel)
+ifeq ($(BR2_PACKAGE_DOSFSTOOLS_FATLABEL),y)
+define DOSFSTOOLS_INSTALL_FATLABEL
+ install -c -p $(DOSFSTOOLS_BUILDDIR)src/fatlabel $(TARGET_DIR)/sbin
+ ln -sf fatlabel $(TARGET_DIR)/sbin/dosfslabel
endef
-DOSFSTOOLS_POST_INSTALL_TARGET_HOOKS += DOSFSTOOLS_REMOVE_FATLABEL
+DOSFSTOOLS_POST_INSTALL_TARGET_HOOKS += DOSFSTOOLS_INSTALL_FATLABEL
endif
-ifeq ($(BR2_PACKAGE_DOSFSTOOLS_FSCK_FAT),)
-define DOSFSTOOLS_REMOVE_FSCK_FAT
- rm -f $(addprefix $(TARGET_DIR)/sbin/,fsck.fat dosfsck fsck.msdos fsck.vfat)
+ifeq ($(BR2_PACKAGE_DOSFSTOOLS_FSCK_FAT),y)
+define DOSFSTOOLS_INSTALL_FSCK_FAT
+ install -c -p $(DOSFSTOOLS_BUILDDIR)src/fsck.fat $(TARGET_DIR)/sbin
+ ln -sf fsck.fat $(TARGET_DIR)/sbin/fsck.vfat
+ ln -sf fsck.fat $(TARGET_DIR)/sbin/fsck.msdos
+ ln -sf fsck.fat $(TARGET_DIR)/sbin/dosfsck
endef
-DOSFSTOOLS_POST_INSTALL_TARGET_HOOKS += DOSFSTOOLS_REMOVE_FSCK_FAT
+DOSFSTOOLS_POST_INSTALL_TARGET_HOOKS += DOSFSTOOLS_INSTALL_FSCK_FAT
endif
-ifeq ($(BR2_PACKAGE_DOSFSTOOLS_MKFS_FAT),)
-define DOSFSTOOLS_REMOVE_MKFS_FAT
- rm -f $(addprefix $(TARGET_DIR)/sbin/,mkfs.fat mkdosfs mkfs.msdos mkfs.vfat)
+ifeq ($(BR2_PACKAGE_DOSFSTOOLS_MKFS_FAT),y)
+define DOSFSTOOLS_INSTALL_MKFS_FAT
+ install -c -p $(DOSFSTOOLS_BUILDDIR)src/mkfs.fat $(TARGET_DIR)/sbin
+ ln -sf mkfs.fat $(TARGET_DIR)/sbin/mkdosfs
+ ln -sf mkfs.fat $(TARGET_DIR)/sbin/mkfs.msdos
+ ln -sf mkfs.fat $(TARGET_DIR)/sbin/mkfs.vfat
endef
-DOSFSTOOLS_POST_INSTALL_TARGET_HOOKS += DOSFSTOOLS_REMOVE_MKFS_FAT
+DOSFSTOOLS_POST_INSTALL_TARGET_HOOKS += DOSFSTOOLS_INSTALL_MKFS_FAT
endif
+# Actual installation happens via the post install hooks. Define a no-op install
+# command, so we don't end up calling "make install."
+define DOSFSTOOLS_INSTALL_TARGET_CMDS
+ @/bin/true
+endef
+
$(eval $(autotools-package))
$(eval $(host-autotools-package))
--
2.17.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [Buildroot] [PATCH v3] package/dosfstools: introduce custom install routine
2019-05-30 23:42 [Buildroot] [PATCH v3] package/dosfstools: introduce custom install routine Markus Mayer
@ 2019-05-31 14:34 ` Thomas Petazzoni
2019-05-31 15:32 ` Markus Mayer
0 siblings, 1 reply; 4+ messages in thread
From: Thomas Petazzoni @ 2019-05-31 14:34 UTC (permalink / raw)
To: buildroot
Hello,
Thanks for this patch.
On Thu, 30 May 2019 16:42:26 -0700
Markus Mayer <mmayer@broadcom.com> wrote:
> -ifeq ($(BR2_PACKAGE_DOSFSTOOLS_FATLABEL),)
> -define DOSFSTOOLS_REMOVE_FATLABEL
> - rm -f $(addprefix $(TARGET_DIR)/sbin/,dosfslabel fatlabel)
> +ifeq ($(BR2_PACKAGE_DOSFSTOOLS_FATLABEL),y)
> +define DOSFSTOOLS_INSTALL_FATLABEL
> + install -c -p $(DOSFSTOOLS_BUILDDIR)src/fatlabel $(TARGET_DIR)/sbin
Please use:
$(INSTALL) -D -m 0755 $(@D)/src/fatlabel $(TARGET_DIR)/sbin/fatlabel
since that's what we use everywhere for insatllation.
> +# Actual installation happens via the post install hooks. Define a no-op install
> +# command, so we don't end up calling "make install."
> +define DOSFSTOOLS_INSTALL_TARGET_CMDS
> + @/bin/true
Nope, that's not how we want to do it. Instead, we want this:
define DOSFSTOOLS_INSTALL_TARGET_CMDS
$(DOSFSTOOLS_INSTALL_FATLABEL)
$(DOSFSTOOLS_INSTALL_FSCK_FAT)
$(DOSFSTOOLS_INSTALL_MKFS_FAT)
endef
and of course, not register the DOSFSTOOLS_INSTALL_* defines as
post-install target hooks.
I'm also wondering if we could factorize the logic a bit, but I'm not
sure it's worth the effort. It would give something like this:
DOSFSTOOLS_INSTALL_FILES_$(BR2_PACKAGE_DOSFSTOOLS_FATLABEL) += fatlabel
DOSFSTOOLS_INSTALL_SYMLINKS_$(BR2_PACKAGE_DOSFSTOOLS_FATLABEL) += dosfslabel:fatlabel
DOSFSTOOLS_INSTALL_FILES_$(BR2_PACKAGE_DOSFSTOOLS_FSCK_FAT) += fsck.fat
DOSFSTOOLS_INSTALL_SYMLINKS_$(BR2_PACKAGE_DOSFSTOOLS_FSCK_FAT) += fsck.vfat:fsck.fat fsck.mkdos:fsck.fat dosfsck:fsck.fat
DOSFSTOOLS_INSTALL_FILES_$(BR2_PACKAGE_DOSFSTOOLS_MKFS_FAT) += mkfs.fat
DOSFSTOOLS_INSTALL_SYMLINKS_$(BR2_PACKAGE_DOSFSTOOLS_MKFS_FAT) += mkdosfs:mkfs.fat mkfs.msdos:mkfs.fat mkfs.vfat:mkfs.fat
define DOSFSTOOLS_INSTALL_TARGET_CMDS
$(foreach f,$(DOSFSTOOLS_INSTALL_FILES_y), \
$(INSTALL) -D -m 0744 $(@D)/src/$(f) $(TARGET_DIR)/sbin/$(f)
)
$(foreach l,$(DOSFSTOOLS_INSTALL_SYMLINKS_y), \
ln -s $(word 2,$(subst :, ,$(l))) $(TARGET_DIR)/sbin/$(word 1,$(subst :, ,$(l)))
)
endef
But admittedly, it becomes a bit hackish, and we don't do that anywhere
in Buildroot today. So probably sticking to three
DOSFSTOOLS_INSTALL_<foo> defines is better for now.
Thanks!
Thomas
--
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 4+ messages in thread
* [Buildroot] [PATCH v3] package/dosfstools: introduce custom install routine
2019-05-31 14:34 ` Thomas Petazzoni
@ 2019-05-31 15:32 ` Markus Mayer
2019-05-31 19:24 ` Thomas Petazzoni
0 siblings, 1 reply; 4+ messages in thread
From: Markus Mayer @ 2019-05-31 15:32 UTC (permalink / raw)
To: buildroot
On Fri, 31 May 2019 at 07:35, Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
>
> Hello,
>
> Thanks for this patch.
>
> On Thu, 30 May 2019 16:42:26 -0700
> Markus Mayer <mmayer@broadcom.com> wrote:
>
> > -ifeq ($(BR2_PACKAGE_DOSFSTOOLS_FATLABEL),)
> > -define DOSFSTOOLS_REMOVE_FATLABEL
> > - rm -f $(addprefix $(TARGET_DIR)/sbin/,dosfslabel fatlabel)
> > +ifeq ($(BR2_PACKAGE_DOSFSTOOLS_FATLABEL),y)
> > +define DOSFSTOOLS_INSTALL_FATLABEL
> > + install -c -p $(DOSFSTOOLS_BUILDDIR)src/fatlabel $(TARGET_DIR)/sbin
>
> Please use:
>
> $(INSTALL) -D -m 0755 $(@D)/src/fatlabel $(TARGET_DIR)/sbin/fatlabel
>
> since that's what we use everywhere for insatllation.
Sure.
> > +# Actual installation happens via the post install hooks. Define a no-op install
> > +# command, so we don't end up calling "make install."
> > +define DOSFSTOOLS_INSTALL_TARGET_CMDS
> > + @/bin/true
>
> Nope, that's not how we want to do it. Instead, we want this:
>
> define DOSFSTOOLS_INSTALL_TARGET_CMDS
> $(DOSFSTOOLS_INSTALL_FATLABEL)
> $(DOSFSTOOLS_INSTALL_FSCK_FAT)
> $(DOSFSTOOLS_INSTALL_MKFS_FAT)
> endef
>
> and of course, not register the DOSFSTOOLS_INSTALL_* defines as
> post-install target hooks.
Right. Won't I have to define the install macros as empty if they are
supposed to be noops?
Like this:
ifeq ($(BR2_PACKAGE_DOSFSTOOLS_FATLABEL),y)
define DOSFSTOOLS_REMOVE_FATLABEL
... install code here ...
endef
else
# empty declaration
define DOSFSTOOLS_REMOVE_FATLABEL
endef
endif
I had that before, but thought the if-else portion with an empty macro
definition might be frowned upon. Or does it just ignore unknown
macros and the above is not necessary?
> I'm also wondering if we could factorize the logic a bit, but I'm not
> sure it's worth the effort. It would give something like this:
>
> DOSFSTOOLS_INSTALL_FILES_$(BR2_PACKAGE_DOSFSTOOLS_FATLABEL) += fatlabel
> DOSFSTOOLS_INSTALL_SYMLINKS_$(BR2_PACKAGE_DOSFSTOOLS_FATLABEL) += dosfslabel:fatlabel
>
> DOSFSTOOLS_INSTALL_FILES_$(BR2_PACKAGE_DOSFSTOOLS_FSCK_FAT) += fsck.fat
> DOSFSTOOLS_INSTALL_SYMLINKS_$(BR2_PACKAGE_DOSFSTOOLS_FSCK_FAT) += fsck.vfat:fsck.fat fsck.mkdos:fsck.fat dosfsck:fsck.fat
>
> DOSFSTOOLS_INSTALL_FILES_$(BR2_PACKAGE_DOSFSTOOLS_MKFS_FAT) += mkfs.fat
> DOSFSTOOLS_INSTALL_SYMLINKS_$(BR2_PACKAGE_DOSFSTOOLS_MKFS_FAT) += mkdosfs:mkfs.fat mkfs.msdos:mkfs.fat mkfs.vfat:mkfs.fat
>
> define DOSFSTOOLS_INSTALL_TARGET_CMDS
> $(foreach f,$(DOSFSTOOLS_INSTALL_FILES_y), \
> $(INSTALL) -D -m 0744 $(@D)/src/$(f) $(TARGET_DIR)/sbin/$(f)
0755 I imagine? :-)
> )
>
> $(foreach l,$(DOSFSTOOLS_INSTALL_SYMLINKS_y), \
> ln -s $(word 2,$(subst :, ,$(l))) $(TARGET_DIR)/sbin/$(word 1,$(subst :, ,$(l)))
> )
> endef
>
> But admittedly, it becomes a bit hackish, and we don't do that anywhere
> in Buildroot today. So probably sticking to three
> DOSFSTOOLS_INSTALL_<foo> defines is better for now.
This is an interesting approach. But I'll likely stick with the other one.
> Thanks!
>
> Thomas
> --
> Thomas Petazzoni, CTO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
^ permalink raw reply [flat|nested] 4+ messages in thread
* [Buildroot] [PATCH v3] package/dosfstools: introduce custom install routine
2019-05-31 15:32 ` Markus Mayer
@ 2019-05-31 19:24 ` Thomas Petazzoni
0 siblings, 0 replies; 4+ messages in thread
From: Thomas Petazzoni @ 2019-05-31 19:24 UTC (permalink / raw)
To: buildroot
Hello Markus,
On Fri, 31 May 2019 08:32:54 -0700
Markus Mayer <mmayer@broadcom.com> wrote:
> Right. Won't I have to define the install macros as empty if they are
> supposed to be noops?
>
> Like this:
>
> ifeq ($(BR2_PACKAGE_DOSFSTOOLS_FATLABEL),y)
> define DOSFSTOOLS_REMOVE_FATLABEL
> ... install code here ...
> endef
> else
> # empty declaration
> define DOSFSTOOLS_REMOVE_FATLABEL
> endef
> endif
No: undefined variables in make are automatically empty, so there's no
need for this else condition. We use this principle all over Buildroot.
> > define DOSFSTOOLS_INSTALL_TARGET_CMDS
> > $(foreach f,$(DOSFSTOOLS_INSTALL_FILES_y), \
> > $(INSTALL) -D -m 0744 $(@D)/src/$(f) $(TARGET_DIR)/sbin/$(f)
>
> 0755 I imagine? :-)
Yes, indeed, sorry.
> > $(foreach l,$(DOSFSTOOLS_INSTALL_SYMLINKS_y), \
> > ln -s $(word 2,$(subst :, ,$(l))) $(TARGET_DIR)/sbin/$(word 1,$(subst :, ,$(l)))
> > )
> > endef
> >
> > But admittedly, it becomes a bit hackish, and we don't do that anywhere
> > in Buildroot today. So probably sticking to three
> > DOSFSTOOLS_INSTALL_<foo> defines is better for now.
>
> This is an interesting approach. But I'll likely stick with the other one.
Yes, as I said, I think it makes more sense to stick to the simpler
approach for now.
Thanks!
Thomas
--
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-05-31 19:24 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-05-30 23:42 [Buildroot] [PATCH v3] package/dosfstools: introduce custom install routine Markus Mayer
2019-05-31 14:34 ` Thomas Petazzoni
2019-05-31 15:32 ` Markus Mayer
2019-05-31 19:24 ` Thomas Petazzoni
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox