Skip to contentSkip to conversation

skeleton: create include/ directory

PATCH New Baruch Siach 3 participants 4 messages May 14, 2018 – May 19, 2018

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 skeleton: create include/ directory

Don't rely on a random host package to create the include/ directory for us. Some packages do the wrong thing since they implicitly assume that this directory exists already. Commit a557aedad2 (zstd: fix host headers installation) shows an example of that.

Cc Maxime Hadjinlian <[email protected]>
Cc Yann E. MORIN <[email protected]>
---
 package/skeleton/skeleton.mk | 1 +
 1 file changed, 1 insertion(+)
package/skeleton/skeleton.mk +1 -0
index efcf420d7243..9d97f02f08bf 100644
@@ -16,6 +16,7 @@ SKELETON_ADD_SKELETON_DEPENDENCY = NO
 define HOST_SKELETON_INSTALL_CMDS
 	$(Q)ln -snf . $(HOST_DIR)/usr
 	$(Q)mkdir -p $(HOST_DIR)/lib
+	$(Q)mkdir -p $(HOST_DIR)/include
 	$(Q)case $(HOSTARCH) in \
 		(*64) ln -snf lib $(HOST_DIR)/lib64;; \
 		(*)   ln -snf lib $(HOST_DIR)/lib32;; \
Signature · 1 line
2.17.0
#1 lore raw

PATCH 1/1 skeleton: create include/ directory

Hello,

Quoted 17 lines · Baruch Siach

Don't rely on a random host package to create the include/ directory for us. Some packages do the wrong thing since they implicitly assume that this directory exists already. Commit a557aedad2 (zstd: fix host headers installation) shows an example of that.

Cc Maxime Hadjinlian <[email protected]>
Cc Yann E. MORIN <[email protected]>
---
 package/skeleton/skeleton.mk | 1 +
 1 file changed, 1 insertion(+)
package/skeleton/skeleton.mk +0 -0
index efcf420d7243..9d97f02f08bf 100644

package/skeleton/skeleton.mk SKELETON_ADD_SKELETON_DEPENDENCY = NO · line 19

Quoted 4 lines
package/skeleton/skeleton.mk +1 -0
@@ -16,6 +16,7 @@ SKELETON_ADD_SKELETON_DEPENDENCY = NO
 define HOST_SKELETON_INSTALL_CMDS
 	$(Q)ln -snf . $(HOST_DIR)/usr
 	$(Q)mkdir -p $(HOST_DIR)/lib
+	$(Q)mkdir -p $(HOST_DIR)/include

Following the zstd patch, I was discussing this with Yann on IRC, and my thinking is that we normally shouldn't have to do something like this. Packages that install stuff in ${prefix}/include should create this folder as needed.

Now, it is true that we already create $(STAGING_DIR)/include, and it doesn't cost much to create such a directory ahead of time.

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
#2 lore raw

PATCH 1/1 skeleton: create include/ directory

Quoted 30 lines · Thomas Petazzoni

Hello,

Quoted 17 lines · Baruch Siach

Don't rely on a random host package to create the include/ directory for us. Some packages do the wrong thing since they implicitly assume that this directory exists already. Commit a557aedad2 (zstd: fix host headers installation) shows an example of that.

Cc Maxime Hadjinlian <[email protected]>
Cc Yann E. MORIN <[email protected]>
---
 package/skeleton/skeleton.mk | 1 +
 1 file changed, 1 insertion(+)
package/skeleton/skeleton.mk +0 -0
index efcf420d7243..9d97f02f08bf 100644

package/skeleton/skeleton.mk SKELETON_ADD_SKELETON_DEPENDENCY = NO · line 19

Quoted 4 lines
package/skeleton/skeleton.mk +1 -0
@@ -16,6 +16,7 @@ SKELETON_ADD_SKELETON_DEPENDENCY = NO
 define HOST_SKELETON_INSTALL_CMDS
 	$(Q)ln -snf . $(HOST_DIR)/usr
 	$(Q)mkdir -p $(HOST_DIR)/lib
+	$(Q)mkdir -p $(HOST_DIR)/include

Following the zstd patch, I was discussing this with Yann on IRC, and my thinking is that we normally shouldn't have to do something like this. Packages that install stuff in ${prefix}/include should create this folder as needed.

Quoted 2 lines

Now, it is true that we already create $(STAGING_DIR)/include, and it doesn't cost much to create such a directory ahead of time.

Well, basically, for any problem that is really due to the package build system
that is wrong (which is 90% of the problems we are dealing with :-), we have two options:
- an upstreamable patch;
- a sweeping catch-all change to Buildroot that fixes all packages.

Although upstreamable patches are nice because we improve the world, there are
a couple of disadvantages:
- it's a Sisyphean task, upstreams will make the same mistakes all over;
- it's more work for us;
- we have more breakage;
- we have a significant number of packages without upstream (dead, or never there cfr. vendor-supplied code dumps).

Therefore, the sweeping catch-all fix is pretty attractive. So I would actually
say we prefer such a solution iff:

1. it is simple (maintainable);
2. it is effective (guaranteed to fix the problem in all cases);
3. it is harmless (guaranteed not to break anything that used to work);
4. it is logical (when you see it, you don't think wtf?);
5. it is needed for several packages.

We already have quite a number of sweeping catch-alls that don't satisfy these
criteria, e.g. toolchain wrapper, libtool patching.

This patch, on the other hand, fits the criteria nicely. Except for the 5th
one, but IMO that one is less important.

Bottom line:

Regards,
Arnout

Signature · 6 lines
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:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF
#3 lore raw

PATCH 1/1 skeleton: create include/ directory

In reply to Baruch Siach

Hello,

Quoted 11 lines · Baruch Siach

Don't rely on a random host package to create the include/ directory for us. Some packages do the wrong thing since they implicitly assume that this directory exists already. Commit a557aedad2 (zstd: fix host headers installation) shows an example of that.

Cc Maxime Hadjinlian <[email protected]>
Cc Yann E. MORIN <[email protected]>
---
 package/skeleton/skeleton.mk | 1 +
 1 file changed, 1 insertion(+)

Applied to next, thanks.

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com