diff mbox

mtd: restore installation of libmtd to staging

Message ID 20170628035610.9945-1-yurovsky@gmail.com
State Accepted
Headers show

Commit Message

Andrey Yurovsky June 28, 2017, 3:56 a.m. UTC
Commit 67117adc891e06154d23a01b8c8ee348c63e78eb backed out the
deployment of libmtd headers (libmtd.h, libubi.h, ubi-media.h) to
staging and these may be needed by packages that work directly with MTD,
for example swupdate. mtd also produces libmtd.a and libubi.a which are
needed by those packages.

Tested by configuring swupdate to use MTD and verifying that swupdate
now compiles and links.

Signed-off-by: Andrey Yurovsky <yurovsky@gmail.com>
---
 package/mtd/mtd.mk | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

Thomas Petazzoni June 28, 2017, 6:29 a.m. UTC | #1
Hello,

Thanks for your patch!

On Tue, 27 Jun 2017 20:56:10 -0700, Andrey Yurovsky wrote:

> +define MTD_INSTALL_STAGING_CMDS
> +	$(INSTALL) -D -m 0755 $(@D)/include/libmtd.h $(STAGING_DIR)/usr/include/mtd/libmtd.h
> +	$(INSTALL) -D -m 0755 $(@D)/include/libubi.h $(STAGING_DIR)/usr/include/mtd/libubi.h
> +	$(INSTALL) -D -m 0755 $(@D)/include/mtd/ubi-media.h $(STAGING_DIR)/usr/include/mtd/ubi-media.h
> +	$(INSTALL) -D -m 0755 $(@D)/libmtd.a $(STAGING_DIR)/usr/lib/libmtd.a
> +	$(INSTALL) -D -m 0755 $(@D)/libubi.a $(STAGING_DIR)/usr/lib/libubi.a
> +endef
> +
>  $(eval $(autotools-package))
>  $(eval $(host-autotools-package))

Since it's an autotools package, it has a "make install" step that
should install those libraries. I believe that if the upstream mtd
doesn't install such useful libraries as part of its "make install"
step, then we should instead patch the upstream Makefile.am, and
contribute it upstream.

Best regards,

Thomas Petazzoni
Andrey Yurovsky June 28, 2017, 5 p.m. UTC | #2
On Tue, Jun 27, 2017 at 11:29 PM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Since it's an autotools package, it has a "make install" step that
> should install those libraries. I believe that if the upstream mtd
> doesn't install such useful libraries as part of its "make install"
> step, then we should instead patch the upstream Makefile.am, and
> contribute it upstream.

I can discuss it with them but the libmtd archives are part of
noinst_LIBRARIES in their build system so I suspect they don't intend
for external software like swupdate to actually use them (or are
otherwise not interested in supporting them as libraries).

I'm not sure what the right way to solve this is, for instance
swupdate could instead pull libmtd as a git submodule, build those
libraries, and link against them directly. I couldn't find anyone
beside swupdate needing them (for example rauc, which is roughly
equivalent to swupdate, actually forks off and runs the mtd-utils
shell commands to do the same job) so I wouldn't be surprised if
swupdate's usage is not standard. Either way, swupdate is effectively
broken in buildroot (if you want to talk to NAND Flash, UBI, etc) so I
wanted to have it working again but I am not sure where and how it
should be addressed.
Thomas Petazzoni July 1, 2017, 4:54 p.m. UTC | #3
Hello,

On Tue, 27 Jun 2017 20:56:10 -0700, Andrey Yurovsky wrote:

> +define MTD_INSTALL_STAGING_CMDS
> +	$(INSTALL) -D -m 0755 $(@D)/include/libmtd.h $(STAGING_DIR)/usr/include/mtd/libmtd.h
> +	$(INSTALL) -D -m 0755 $(@D)/include/libubi.h $(STAGING_DIR)/usr/include/mtd/libubi.h
> +	$(INSTALL) -D -m 0755 $(@D)/include/mtd/ubi-media.h $(STAGING_DIR)/usr/include/mtd/ubi-media.h
> +	$(INSTALL) -D -m 0755 $(@D)/libmtd.a $(STAGING_DIR)/usr/lib/libmtd.a
> +	$(INSTALL) -D -m 0755 $(@D)/libubi.a $(STAGING_DIR)/usr/lib/libubi.a
> +endef

I've applied, but I've changed this to be a post install staging hook.
This way, the normal "make install" is done for staging, and then those
additional libraries are installed as well. I've also added a comment
in the code explaining why we're doing this.

It would still be good to let upstream know that those libraries are
used by other programs, and see if they would accept to adjust the
Makefile.am logic to install such libraries.

Thanks!

Thomas
diff mbox

Patch

diff --git a/package/mtd/mtd.mk b/package/mtd/mtd.mk
index 3df641e..bcd2363 100644
--- a/package/mtd/mtd.mk
+++ b/package/mtd/mtd.mk
@@ -97,5 +97,13 @@  define MTD_INSTALL_TARGET_CMDS
 	)
 endef
 
+define MTD_INSTALL_STAGING_CMDS
+	$(INSTALL) -D -m 0755 $(@D)/include/libmtd.h $(STAGING_DIR)/usr/include/mtd/libmtd.h
+	$(INSTALL) -D -m 0755 $(@D)/include/libubi.h $(STAGING_DIR)/usr/include/mtd/libubi.h
+	$(INSTALL) -D -m 0755 $(@D)/include/mtd/ubi-media.h $(STAGING_DIR)/usr/include/mtd/ubi-media.h
+	$(INSTALL) -D -m 0755 $(@D)/libmtd.a $(STAGING_DIR)/usr/lib/libmtd.a
+	$(INSTALL) -D -m 0755 $(@D)/libubi.a $(STAGING_DIR)/usr/lib/libubi.a
+endef
+
 $(eval $(autotools-package))
 $(eval $(host-autotools-package))