skeleton: create include/ directory

Message ID 9873d2e0023b87545478d0615eb5d97bcbd82c2c.1526268236.git.baruch@tkos.co.il
State Accepted
Headers show
Series
  • skeleton: create include/ directory
Related show

Commit Message

Baruch Siach May 14, 2018, 3:23 a.m.
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 <maxime.hadjinlian@gmail.com>
Cc: Yann E. MORIN <yann.morin.1998@free.fr>
Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
 package/skeleton/skeleton.mk | 1 +
 1 file changed, 1 insertion(+)

Comments

Thomas Petazzoni May 14, 2018, 6:03 a.m. | #1
Hello,

On Mon, 14 May 2018 06:23:56 +0300, Baruch Siach wrote:
> 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 <maxime.hadjinlian@gmail.com>
> Cc: Yann E. MORIN <yann.morin.1998@free.fr>
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> ---
>  package/skeleton/skeleton.mk | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/package/skeleton/skeleton.mk b/package/skeleton/skeleton.mk
> index efcf420d7243..9d97f02f08bf 100644
> --- a/package/skeleton/skeleton.mk
> +++ b/package/skeleton/skeleton.mk
> @@ -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
Arnout Vandecappelle May 15, 2018, 9:39 p.m. | #2
On 14-05-18 08:03, Thomas Petazzoni wrote:
> Hello,
> 
> On Mon, 14 May 2018 06:23:56 +0300, Baruch Siach wrote:
>> 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 <maxime.hadjinlian@gmail.com>
>> Cc: Yann E. MORIN <yann.morin.1998@free.fr>
>> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
>> ---
>>  package/skeleton/skeleton.mk | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/package/skeleton/skeleton.mk b/package/skeleton/skeleton.mk
>> index efcf420d7243..9d97f02f08bf 100644
>> --- a/package/skeleton/skeleton.mk
>> +++ b/package/skeleton/skeleton.mk
>> @@ -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.

 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:

Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

 Regards,
 Arnout
Thomas Petazzoni May 19, 2018, 8:43 p.m. | #3
Hello,

On Mon, 14 May 2018 06:23:56 +0300, Baruch Siach wrote:
> 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 <maxime.hadjinlian@gmail.com>
> Cc: Yann E. MORIN <yann.morin.1998@free.fr>
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> ---
>  package/skeleton/skeleton.mk | 1 +
>  1 file changed, 1 insertion(+)

Applied to next, thanks.

Thomas

Patch

diff --git a/package/skeleton/skeleton.mk b/package/skeleton/skeleton.mk
index efcf420d7243..9d97f02f08bf 100644
--- a/package/skeleton/skeleton.mk
+++ b/package/skeleton/skeleton.mk
@@ -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;; \