[Buildroot] [PATCH 0/2] toolchain/external: fix installing libs to target (branch yem-ext-toolchain-fixes)

[Buildroot] [PATCH 0/2] toolchain/external: fix installing libs to target (branch yem-ext-toolchain-fixes)

[Yann E. MORIN]

Hello All!

This series is a proposal to fix a regresion in the way we copy the
libraries from an external toolchain to our target directory.

The issue at stake here is that, when a custom, local toolchain is used,
that has lib{32,64} symlinks to lib/ , we would copy too much into the
target directory.

I'm not entirely sure about the first patch; it might be dropped, as it
is not required to fix the reported problem.

The real fix is the second patch.

See the individual commits for explanations.

Regards,
Yann E. MORIN.


The following changes since commit 02302d21536b9c19cd87733f4eb1bd62065416bb

  core/pkg-kconfig: pass host PKG_CONFIG_PATH env var (2016-05-28 19:46:47 +0200)


are available in the git repository at:

  https://git.busybox.net/~ymorin/git/buildroot

for you to fetch changes up to 16dd334fb29148489afa344c652208a4c512f445

  toolchain/helper: don't follow symlinks when copying libs to target (2016-05-29 17:15:41 +0200)


----------------------------------------------------------------
Yann E. MORIN (2):
      toolchain/external: fix arch-subdir
      toolchain/helper: don't follow symlinks when copying libs to target

 toolchain/helpers.mk                               | 2 +-
 toolchain/toolchain-external/toolchain-external.mk | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 1/2] toolchain/external: fix arch-subdir

[Yann E. MORIN]

When there is no architecture-specific sub-directory in the sysroot, we
expect ARCGH_SUBDIR to be empty.

However, with the current code, it is not (with an intrumented
copy_toolchain_sysroot, when copying a ct-ng toolchain):

    >>> toolchain-external undefined Copying external toolchain sysroot to
    >>> staging...
    main sysroot: /home/ymorin/x-tools/armv6-unknown-linux-gnueabihf/armv6-unknown-linux-gnueabihf/sysroot/
    arch sysroot: /home/ymorin/x-tools/armv6-unknown-linux-gnueabihf/armv6-unknown-linux-gnueabihf/sysroot/
    arch subdir:  /home/ymorin/x-tools/armv6-unknown-linux-gnueabihf/armv6-unknown-linux-gnueabihf/sysroot/
    lib dir:      lib
    support dir:

... when we would have expected 'arch subdir' to be empty.

Fix the matching pattern to correctly return an empty string if there is
no arch subdir.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: Arnout Vandecappelle <arnout@mind.be>
Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
Cc: Peter Korsgaard <peter@korsgaard.com>
---
 toolchain/toolchain-external/toolchain-external.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/toolchain/toolchain-external/toolchain-external.mk b/toolchain/toolchain-external/toolchain-external.mk
index 47b951d..3af56b8 100644
--- a/toolchain/toolchain-external/toolchain-external.mk
+++ b/toolchain/toolchain-external/toolchain-external.mk
@@ -662,7 +662,7 @@ define TOOLCHAIN_EXTERNAL_INSTALL_SYSROOT_LIBS
 			SUPPORT_LIB_DIR=`readlink -f $${LIBSTDCPP_A_LOCATION} | sed -r -e 's:libstdc\+\+\.a::'` ; \
 		fi ; \
 	fi ; \
-	ARCH_SUBDIR=`echo $${ARCH_SYSROOT_DIR} | sed -r -e "s:^$${SYSROOT_DIR}(.*)/$$:\1:"` ; \
+	ARCH_SUBDIR=`echo $${ARCH_SYSROOT_DIR} | sed -r -e "s:^$${SYSROOT_DIR}((.*[^/])/?)?$$:\2:"` ; \
 	$(call MESSAGE,"Copying external toolchain sysroot to staging...") ; \
 	$(call copy_toolchain_sysroot,$${SYSROOT_DIR},$${ARCH_SYSROOT_DIR},$${ARCH_SUBDIR},$${ARCH_LIB_DIR},$${SUPPORT_LIB_DIR})
 endef
-- 
2.7.4

[Buildroot] [PATCH 2/2] toolchain/helper: don't follow symlinks when copying libs to target

[Yann E. MORIN]

In 2a87b64 (toolchain-external: align library locations in target and
staging dir), copying the libraries from the sysroot to the target was
changed to a simple find-based solution.

To be sure that the staging directory was entered to find the libraries,
in case the variable was pointing to a symlink, the -L clause to find
was used.

However, that causes extraneous libraries to be copied over.

For example, a ct-ng toolchain would have this sysroot (e.g for an arm
32-bit toolchain):

    .../sysroot/lib/
    .../sysroot/lib32 -> lib
    .../sysroot/lib64 -> lib
    .../sysroot/usr/lib/
    .../sysroot/usr/lib32 -> lib
    .../sysroot/usr/lib64 -> lib

Which we would carry as-is to our own sysroot.

But then, in target, our skeleton creates the /lib/ and /usr/lib
directories, with the necessary lib32 or lib64 symlink pointing to it.
In this case, a lib32->lib symlink is created, but no lib64 symlink
since this is a 32-bit architecture.

So, when we copy over the libraries from our staging to the target
directory, we end creating a /usr/lib64/ directory.

This was very difficult to observe, as no /lib64/ directory is created
and this only happens with a merged /usr.

Since the reason to use -L was to be sure to enter our staging
directory, we just need to ensure that the path ends up with a slash, as
was already talked about in this thread:
    http://lists.busybox.net/pipermail/buildroot/2016-April/159737.html

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: Arnout Vandecappelle <arnout@mind.be>
Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
Cc: Peter Korsgaard <peter@korsgaard.com>
---
 toolchain/helpers.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/toolchain/helpers.mk b/toolchain/helpers.mk
index 5512759..b3510b6 100644
--- a/toolchain/helpers.mk
+++ b/toolchain/helpers.mk
@@ -11,7 +11,7 @@
 copy_toolchain_lib_root = \
 	LIB="$(strip $1)"; \
 \
-	LIBPATHS=`find -L $(STAGING_DIR) -name "$${LIB}" 2>/dev/null` ; \
+	LIBPATHS=`find $(STAGING_DIR)/ -name "$${LIB}" 2>/dev/null` ; \
 	for LIBPATH in $${LIBPATHS} ; do \
 		DESTDIR=`echo $${LIBPATH} | sed "s,^$(STAGING_DIR)/,," | xargs dirname` ; \
 		mkdir -p $(TARGET_DIR)/$${DESTDIR}; \
-- 
2.7.4

[Buildroot] [PATCH 2/2] toolchain/helper: don't follow symlinks when copying libs to target

[Thomas De Schampheleire]

On Sun, May 29, 2016 at 5:17 PM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> In 2a87b64 (toolchain-external: align library locations in target and
> staging dir), copying the libraries from the sysroot to the target was
> changed to a simple find-based solution.
>
> To be sure that the staging directory was entered to find the libraries,
> in case the variable was pointing to a symlink, the -L clause to find
> was used.
>
> However, that causes extraneous libraries to be copied over.
>
> For example, a ct-ng toolchain would have this sysroot (e.g for an arm
> 32-bit toolchain):
>
>     .../sysroot/lib/
>     .../sysroot/lib32 -> lib
>     .../sysroot/lib64 -> lib
>     .../sysroot/usr/lib/
>     .../sysroot/usr/lib32 -> lib
>     .../sysroot/usr/lib64 -> lib
>
> Which we would carry as-is to our own sysroot.
>
> But then, in target, our skeleton creates the /lib/ and /usr/lib
> directories, with the necessary lib32 or lib64 symlink pointing to it.
> In this case, a lib32->lib symlink is created, but no lib64 symlink
> since this is a 32-bit architecture.

Until here, the explanation is clear to me.

>
> So, when we copy over the libraries from our staging to the target
> directory, we end creating a /usr/lib64/ directory.

But this could be expanded upon, I think, as this is the actual
problem statement.

>
> This was very difficult to observe, as no /lib64/ directory is created
> and this only happens with a merged /usr.
>
> Since the reason to use -L was to be sure to enter our staging
> directory, we just need to ensure that the path ends up with a slash, as
> was already talked about in this thread:
>     http://lists.busybox.net/pipermail/buildroot/2016-April/159737.html

While the trailing slash is indeed a fine solution, it is kind of dirty.
I wonder if -H would do the trick too:

       -H     Do  not  follow symbolic links, except while processing the com?
              mand line arguments.  When find examines or  prints  information
              about  files, the information used shall be taken from the prop?
              erties of the symbolic link itself.   The only exception to this
              behaviour is when a file specified on the command line is a sym?
              bolic link, and the link can be resolved.  For  that  situation,
              the  information  used is taken from whatever the link points to
              (that is, the link is followed).  The information about the link
              itself  is used as a fallback if the file pointed to by the sym?
              bolic link cannot be examined.  If -H is in effect  and  one  of
              the  paths specified on the command line is a symbolic link to a
              directory, the contents  of  that  directory  will  be  examined
              (though of course -maxdepth 0 would prevent this).

It should make sure that only STAGING_DIR is resolved, not any other
symbolic link encountered.

/Thomas

[Buildroot] [PATCH 2/2] toolchain/helper: don't follow symlinks when copying libs to target

[Yann E. MORIN]

Thomas DS, All,

On 2016-05-29 20:54 +0200, Thomas De Schampheleire spake thusly:
> On Sun, May 29, 2016 at 5:17 PM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> > In 2a87b64 (toolchain-external: align library locations in target and
> > staging dir), copying the libraries from the sysroot to the target was
> > changed to a simple find-based solution.
> >
> > To be sure that the staging directory was entered to find the libraries,
> > in case the variable was pointing to a symlink, the -L clause to find
> > was used.
> >
> > However, that causes extraneous libraries to be copied over.
> >
> > For example, a ct-ng toolchain would have this sysroot (e.g for an arm
> > 32-bit toolchain):
> >
> >     .../sysroot/lib/
> >     .../sysroot/lib32 -> lib
> >     .../sysroot/lib64 -> lib
> >     .../sysroot/usr/lib/
> >     .../sysroot/usr/lib32 -> lib
> >     .../sysroot/usr/lib64 -> lib
> >
> > Which we would carry as-is to our own sysroot.
> >
> > But then, in target, our skeleton creates the /lib/ and /usr/lib
> > directories, with the necessary lib32 or lib64 symlink pointing to it.
> > In this case, a lib32->lib symlink is created, but no lib64 symlink
> > since this is a 32-bit architecture.
> 
> Until here, the explanation is clear to me.
> 
> >
> > So, when we copy over the libraries from our staging to the target
> > directory, we end creating a /usr/lib64/ directory.
> 
> But this could be expanded upon, I think, as this is the actual
> problem statement.

OK, here's what happens:

    $ ls -lF /home/ymorin/dev/buildroot/O/host/usr/arm-buildroot-linux-gnueabihf/sysroot \
             /home/ymorin/dev/buildroot/O/host/usr/arm-buildroot-linux-gnueabihf/sysroot/usr
    /home/ymorin/dev/buildroot/O/host/usr/arm-buildroot-linux-gnueabihf/sysroot:
    total 8
    lrwxrwxrwx 1 ymorin ymorin    7 May 29 23:03 bin -> usr/bin/
    drwxr-xr-x 2 ymorin ymorin 4096 May 16 23:59 etc/
    lrwxrwxrwx 1 ymorin ymorin    7 May 29 23:03 lib -> usr/lib/
    lrwxrwxrwx 1 ymorin ymorin    3 May 29 23:03 lib32 -> lib/
    lrwxrwxrwx 1 ymorin ymorin    8 May 29 23:03 sbin -> usr/sbin/
    drwxr-xr-x 8 ymorin ymorin 4096 May 16 23:58 usr/

    /home/ymorin/dev/buildroot/O/host/usr/arm-buildroot-linux-gnueabihf/sysroot/usr:
    total 24
    drwxr-xr-x  2 ymorin ymorin 4096 May 16 23:59 bin/
    drwxr-xr-x 33 ymorin ymorin 4096 May 16 23:59 include/
    drwxr-xr-x  5 ymorin ymorin 4096 May 17 00:02 lib/
    lrwxrwxrwx  1 ymorin ymorin    3 May 16 23:39 lib32 -> lib/
    lrwxrwxrwx  1 ymorin ymorin    3 May 16 23:39 lib64 -> lib/
    drwxr-xr-x  3 ymorin ymorin 4096 May 16 23:58 libexec/
    drwxr-xr-x  2 ymorin ymorin 4096 May 16 23:59 sbin/
    drwxr-xr-x  4 ymorin ymorin 4096 May 17 00:11 share/

    $ find -L /home/ymorin/dev/buildroot/O/host/usr/arm-buildroot-linux-gnueabihf/sysroot -name 'libatomic.so.*'
    /home/ymorin/dev/buildroot/O/host/usr/arm-buildroot-linux-gnueabihf/sysroot/lib/libatomic.so.1.1.0
    /home/ymorin/dev/buildroot/O/host/usr/arm-buildroot-linux-gnueabihf/sysroot/lib/libatomic.so.1
    /home/ymorin/dev/buildroot/O/host/usr/arm-buildroot-linux-gnueabihf/sysroot/lib32/libatomic.so.1.1.0
    /home/ymorin/dev/buildroot/O/host/usr/arm-buildroot-linux-gnueabihf/sysroot/lib32/libatomic.so.1
    /home/ymorin/dev/buildroot/O/host/usr/arm-buildroot-linux-gnueabihf/sysroot/usr/lib/libatomic.so.1.1.0
    /home/ymorin/dev/buildroot/O/host/usr/arm-buildroot-linux-gnueabihf/sysroot/usr/lib/libatomic.so.1
    /home/ymorin/dev/buildroot/O/host/usr/arm-buildroot-linux-gnueabihf/sysroot/usr/lib32/libatomic.so.1.1.0
    /home/ymorin/dev/buildroot/O/host/usr/arm-buildroot-linux-gnueabihf/sysroot/usr/lib32/libatomic.so.1
    /home/ymorin/dev/buildroot/O/host/usr/arm-buildroot-linux-gnueabihf/sysroot/usr/lib64/libatomic.so.1.1.0
    /home/ymorin/dev/buildroot/O/host/usr/arm-buildroot-linux-gnueabihf/sysroot/usr/lib64/libatomic.so.1

So, the 'find -L' follows symlinks to directories, and thus finds the
required libraries in multiple locations.

I'll see to improve that part of the commit log.

> > This was very difficult to observe, as no /lib64/ directory is created
> > and this only happens with a merged /usr.
> >
> > Since the reason to use -L was to be sure to enter our staging
> > directory, we just need to ensure that the path ends up with a slash, as
> > was already talked about in this thread:
> >     http://lists.busybox.net/pipermail/buildroot/2016-April/159737.html
> 
> While the trailing slash is indeed a fine solution, it is kind of dirty.
> I wonder if -H would do the trick too:
[--SNIP--]
> It should make sure that only STAGING_DIR is resolved, not any other
> symbolic link encountered.

Well, appending a slash is a sure mean to make it sure the target of the
symlink (if it is a symlink to start with) is entered. I don't see where
it is dirty.

Yes, -H does the job, too. But I think it is better that we append a
slash: it makes it explicit we want to treat it as a directory, not a
potential symlink.

However, why would we need to handle the symlink case, to begin with? We
are controlling the variable passed in this case and it points to a copy
of the sysroot we did make. I.e. that variable is not provided by the
user; it is always filled by our infra.

Surely, if we really, really, like really-really, want to make it
explicit we want the directory pointed-to by the symlink, then, we
should probably do:

    find $$(readlink -f $(STAGING_DIR)) -name blabla

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 2/2] toolchain/helper: don't follow symlinks when copying libs to target

[Arnout Vandecappelle]

On 05/29/16 23:12, Yann E. MORIN wrote:
> Thomas DS, All,
> 
> On 2016-05-29 20:54 +0200, Thomas De Schampheleire spake thusly:
[snip]
>> While the trailing slash is indeed a fine solution, it is kind of dirty.
>> I wonder if -H would do the trick too:
> [--SNIP--]
>> It should make sure that only STAGING_DIR is resolved, not any other
>> symbolic link encountered.
> 
> Well, appending a slash is a sure mean to make it sure the target of the
> symlink (if it is a symlink to start with) is entered. I don't see where
> it is dirty.

 I should have realized this before, but: when is $(STAGING_DIR) ever a symlink?
It's defined as $(HOST_DIR)/usr/$(GNU_TARGET_NAME)/sysroot and it's explicitly
mkdir'ed in the Makefile. It's not a symlink. $(O)/staging is a symlink, but
that's not what's used here.

 That makes me think there must have been a different reason why Thomas added
the -L. Thomas originally wrote:

  I used -follow just to make sure that STAGING_DIR is entered. It seems
  to work without just because STAGING_DIR happens to be defined with a
  trailing slash, in which case its contents are evaluated anyhow by
  find. I considered this dependency too fragile and wanted it to work
  even if someone optimizes away the trailing slash. Another solution
  would be to add a trailing slash manually as $(STAGING_DIR)/ but -L is
  much cleaner.

Note that there never has been a trailing / in STAGING_DIR.

 Thomas, any idea what is going on here?

 I did a quick try with just removing -L and it seems to work fine.


> 
> Yes, -H does the job, too. But I think it is better that we append a
> slash: it makes it explicit we want to treat it as a directory, not a
> potential symlink.
> 
> However, why would we need to handle the symlink case, to begin with? We
> are controlling the variable passed in this case and it points to a copy
> of the sysroot we did make. I.e. that variable is not provided by the
> user; it is always filled by our infra.

 Yann, did you mean the same thing as I described above?


 Regards,
 Arnout

> 
> Surely, if we really, really, like really-really, want to make it
> explicit we want the directory pointed-to by the symlink, then, we
> should probably do:
> 
>     find $$(readlink -f $(STAGING_DIR)) -name blabla
> 
> Regards,
> Yann E. MORIN.
> 


-- 
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

[Buildroot] [PATCH 2/2] toolchain/helper: don't follow symlinks when copying libs to target

[Thomas De Schampheleire]

On Mon, May 30, 2016 at 12:33 AM, Arnout Vandecappelle <arnout@mind.be> wrote:
> On 05/29/16 23:12, Yann E. MORIN wrote:
>> Thomas DS, All,
>>
>> On 2016-05-29 20:54 +0200, Thomas De Schampheleire spake thusly:
> [snip]
>>> While the trailing slash is indeed a fine solution, it is kind of dirty.
>>> I wonder if -H would do the trick too:
>> [--SNIP--]
>>> It should make sure that only STAGING_DIR is resolved, not any other
>>> symbolic link encountered.
>>
>> Well, appending a slash is a sure mean to make it sure the target of the
>> symlink (if it is a symlink to start with) is entered. I don't see where
>> it is dirty.
>
>  I should have realized this before, but: when is $(STAGING_DIR) ever a symlink?
> It's defined as $(HOST_DIR)/usr/$(GNU_TARGET_NAME)/sysroot and it's explicitly
> mkdir'ed in the Makefile. It's not a symlink. $(O)/staging is a symlink, but
> that's not what's used here.
>
>  That makes me think there must have been a different reason why Thomas added
> the -L. Thomas originally wrote:
>
>   I used -follow just to make sure that STAGING_DIR is entered. It seems
>   to work without just because STAGING_DIR happens to be defined with a
>   trailing slash, in which case its contents are evaluated anyhow by
>   find. I considered this dependency too fragile and wanted it to work
>   even if someone optimizes away the trailing slash. Another solution
>   would be to add a trailing slash manually as $(STAGING_DIR)/ but -L is
>   much cleaner.
>
> Note that there never has been a trailing / in STAGING_DIR.
>
>  Thomas, any idea what is going on here?
>

I think the correct term here is 'mass hysteria' :-D

I can imagine where the imagined need comes from: while testing the
necessary commands on the command-line I will have compared
output/target with output/staging, not with
output/host/usr/<tuple>/sysroot. When coding this into Buildroot I did
not consider that STAGING_DIR was not pointing to output/staging
(although I actually know that, of course).

I'm baffled by the statement "It seems to work without just because
STAGING_DIR happens to be defined with a trailing slash". This must
have been derived from the observation that things worked without
-L/-follow and making the assumption that it must have been due to a
trailing slash without actually verifying that.

Anyway, if it works with plain find, all the better.
Sorry for all this confusion.
Let's consider it a good lesson in not making assumptions :-)

Thanks,
Thomas

[Buildroot] [PATCH 2/2] toolchain/helper: don't follow symlinks when copying libs to target

[Peter Korsgaard]

>>>>> "Thomas" == Thomas De Schampheleire <patrickdepinguin@gmail.com> writes:

Hi,

 > I'm baffled by the statement "It seems to work without just because
 > STAGING_DIR happens to be defined with a trailing slash". This must
 > have been derived from the observation that things worked without
 > -L/-follow and making the assumption that it must have been due to a
 > trailing slash without actually verifying that.

 > Anyway, if it works with plain find, all the better.
 > Sorry for all this confusion.
 > Let's consider it a good lesson in not making assumptions :-)

Heh ;)

Ok, I'll commit a simplified version of the patch (just dropping -L)
with an updated description - Thanks all!

-- 
Venlig hilsen,
Peter Korsgaard 

[Buildroot] [PATCH 2/2] toolchain/helper: don't follow symlinks when copying libs to target

[Yann E. MORIN]

Arnout, All,

On 2016-05-30 00:33 +0200, Arnout Vandecappelle spake thusly:
> On 05/29/16 23:12, Yann E. MORIN wrote:
> > Thomas DS, All,
> > 
> > On 2016-05-29 20:54 +0200, Thomas De Schampheleire spake thusly:
> [snip]
> >> While the trailing slash is indeed a fine solution, it is kind of dirty.
> >> I wonder if -H would do the trick too:
> > [--SNIP--]
> >> It should make sure that only STAGING_DIR is resolved, not any other
> >> symbolic link encountered.
> > 
> > Well, appending a slash is a sure mean to make it sure the target of the
> > symlink (if it is a symlink to start with) is entered. I don't see where
> > it is dirty.
> 
>  I should have realized this before, but: when is $(STAGING_DIR) ever a symlink?
> It's defined as $(HOST_DIR)/usr/$(GNU_TARGET_NAME)/sysroot and it's explicitly
> mkdir'ed in the Makefile. It's not a symlink. $(O)/staging is a symlink, but
> that's not what's used here.
> 
>  That makes me think there must have been a different reason why Thomas added
> the -L. Thomas originally wrote:
> 
>   I used -follow just to make sure that STAGING_DIR is entered. It seems
>   to work without just because STAGING_DIR happens to be defined with a
>   trailing slash, in which case its contents are evaluated anyhow by
>   find. I considered this dependency too fragile and wanted it to work
>   even if someone optimizes away the trailing slash. Another solution
>   would be to add a trailing slash manually as $(STAGING_DIR)/ but -L is
>   much cleaner.
> 
> Note that there never has been a trailing / in STAGING_DIR.
> 
>  Thomas, any idea what is going on here?
> 
>  I did a quick try with just removing -L and it seems to work fine.
> 
> 
> > 
> > Yes, -H does the job, too. But I think it is better that we append a
> > slash: it makes it explicit we want to treat it as a directory, not a
> > potential symlink.
> > 
> > However, why would we need to handle the symlink case, to begin with? We
> > are controlling the variable passed in this case and it points to a copy
> > of the sysroot we did make. I.e. that variable is not provided by the
> > user; it is always filled by our infra.
> 
>  Yann, did you mean the same thing as I described above?

Basically, yes.

Regards,
Yann E. MORIN.

>  Regards,
>  Arnout
> 
> > 
> > Surely, if we really, really, like really-really, want to make it
> > explicit we want the directory pointed-to by the symlink, then, we
> > should probably do:
> > 
> >     find $$(readlink -f $(STAGING_DIR)) -name blabla
> > 
> > Regards,
> > Yann E. MORIN.
> > 
> 
> 
> -- 
> 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

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 2/2] toolchain/helper: don't follow symlinks when copying libs to target

[Arnout Vandecappelle]

On 05/30/16 09:03, Peter Korsgaard wrote:
>>>>>> "Thomas" == Thomas De Schampheleire <patrickdepinguin@gmail.com> writes:
> 
> Hi,
> 
>  > I'm baffled by the statement "It seems to work without just because
>  > STAGING_DIR happens to be defined with a trailing slash". This must
>  > have been derived from the observation that things worked without
>  > -L/-follow and making the assumption that it must have been due to a
>  > trailing slash without actually verifying that.
> 
>  > Anyway, if it works with plain find, all the better.
>  > Sorry for all this confusion.
>  > Let's consider it a good lesson in not making assumptions :-)
> 
> Heh ;)
> 
> Ok, I'll commit a simplified version of the patch (just dropping -L)
> with an updated description - Thanks all!

 Did you forget to push? Or should Yann still send a v3?

 Regards,
 Arnout


-- 
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

[Buildroot] [PATCH 2/2] toolchain/helper: don't follow symlinks when copying libs to target

[Peter Korsgaard]

>>>>> "Arnout" == Arnout Vandecappelle <arnout@mind.be> writes:

 > On 05/30/16 09:03, Peter Korsgaard wrote:
 >>>>>>> "Thomas" == Thomas De Schampheleire <patrickdepinguin@gmail.com> writes:
 >> 
 >> Hi,
 >> 
 >> > I'm baffled by the statement "It seems to work without just because
 >> > STAGING_DIR happens to be defined with a trailing slash". This must
 >> > have been derived from the observation that things worked without
 >> > -L/-follow and making the assumption that it must have been due to a
 >> > trailing slash without actually verifying that.
 >> 
 >> > Anyway, if it works with plain find, all the better.
 >> > Sorry for all this confusion.
 >> > Let's consider it a good lesson in not making assumptions :-)
 >> 
 >> Heh ;)
 >> 
 >> Ok, I'll commit a simplified version of the patch (just dropping -L)
 >> with an updated description - Thanks all!

 >  Did you forget to push? Or should Yann still send a v3?

Sorry, I was caught up with other stuff, I've pushed it now.

-- 
Venlig hilsen,
Peter Korsgaard 

[Buildroot] [PATCH 1/2] toolchain/external: fix arch-subdir

[Arnout Vandecappelle]

On 05/29/16 17:17, Yann E. MORIN wrote:
> When there is no architecture-specific sub-directory in the sysroot, we
> expect ARCGH_SUBDIR to be empty.
> 
> However, with the current code, it is not (with an intrumented
> copy_toolchain_sysroot, when copying a ct-ng toolchain):
> 
>     >>> toolchain-external undefined Copying external toolchain sysroot to
>     >>> staging...
>     main sysroot: /home/ymorin/x-tools/armv6-unknown-linux-gnueabihf/armv6-unknown-linux-gnueabihf/sysroot/
>     arch sysroot: /home/ymorin/x-tools/armv6-unknown-linux-gnueabihf/armv6-unknown-linux-gnueabihf/sysroot/
>     arch subdir:  /home/ymorin/x-tools/armv6-unknown-linux-gnueabihf/armv6-unknown-linux-gnueabihf/sysroot/
>     lib dir:      lib
>     support dir:
> 
> ... when we would have expected 'arch subdir' to be empty.

 So, I checked, and it doesn't hurt that it's not empty, because ARCH_SUBDIR is
only used within this condition:

if [ $${SYSROOT_DIR_CANON} != $${ARCH_SYSROOT_DIR_CANON} ]

i.e. if SYSROOT == ARCH_SYSROOT, ARCH_SUBDIR will not be used. Makes sense too,
of course: if there is no arch-specific sysroot directory, then there's no need
to create the relative symlinks.

 That said, for consistency it's of course nicer to make ARCH_SUBDIR empty. But
that's not needed for 2016.05.

> 
> Fix the matching pattern to correctly return an empty string if there is
> no arch subdir.
> 
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Cc: Arnout Vandecappelle <arnout@mind.be>
> Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
> Cc: Peter Korsgaard <peter@korsgaard.com>
> ---
>  toolchain/toolchain-external/toolchain-external.mk | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/toolchain/toolchain-external/toolchain-external.mk b/toolchain/toolchain-external/toolchain-external.mk
> index 47b951d..3af56b8 100644
> --- a/toolchain/toolchain-external/toolchain-external.mk
> +++ b/toolchain/toolchain-external/toolchain-external.mk
> @@ -662,7 +662,7 @@ define TOOLCHAIN_EXTERNAL_INSTALL_SYSROOT_LIBS
>  			SUPPORT_LIB_DIR=`readlink -f $${LIBSTDCPP_A_LOCATION} | sed -r -e 's:libstdc\+\+\.a::'` ; \
>  		fi ; \
>  	fi ; \
> -	ARCH_SUBDIR=`echo $${ARCH_SYSROOT_DIR} | sed -r -e "s:^$${SYSROOT_DIR}(.*)/$$:\1:"` ; \
> +	ARCH_SUBDIR=`echo $${ARCH_SYSROOT_DIR} | sed -r -e "s:^$${SYSROOT_DIR}((.*[^/])/?)?$$:\2:"` ; \

 Simpler alternative (just tested in the shell, not in an actual build):

ARCH_SUBDIR=`echo $${ARCH_SYSROOT_DIR} | sed -n -r -e
"\:^$${SYSROOT_DIR}(.*)/$$:s::\1:p"`

Well, at least I find this regex easier to parse. Also, your version still gives
a newline (which admittedly will be stripped inside the funciton) while mine is
really an empty string.


 Regards,
 Arnout


>  	$(call MESSAGE,"Copying external toolchain sysroot to staging...") ; \
>  	$(call copy_toolchain_sysroot,$${SYSROOT_DIR},$${ARCH_SYSROOT_DIR},$${ARCH_SUBDIR},$${ARCH_LIB_DIR},$${SUPPORT_LIB_DIR})
>  endef
> 


-- 
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

[Buildroot] [PATCH 1/2] toolchain/external: fix arch-subdir

[Yann E. MORIN]

Arnout, All,

On 2016-05-30 23:01 +0200, Arnout Vandecappelle spake thusly:
> On 05/29/16 17:17, Yann E. MORIN wrote:
> > When there is no architecture-specific sub-directory in the sysroot, we
> > expect ARCGH_SUBDIR to be empty.
> > 
> > However, with the current code, it is not (with an intrumented
> > copy_toolchain_sysroot, when copying a ct-ng toolchain):
> > 
> >     >>> toolchain-external undefined Copying external toolchain sysroot to
> >     >>> staging...
> >     main sysroot: /home/ymorin/x-tools/armv6-unknown-linux-gnueabihf/armv6-unknown-linux-gnueabihf/sysroot/
> >     arch sysroot: /home/ymorin/x-tools/armv6-unknown-linux-gnueabihf/armv6-unknown-linux-gnueabihf/sysroot/
> >     arch subdir:  /home/ymorin/x-tools/armv6-unknown-linux-gnueabihf/armv6-unknown-linux-gnueabihf/sysroot/
> >     lib dir:      lib
> >     support dir:
> > 
> > ... when we would have expected 'arch subdir' to be empty.
> 
>  So, I checked, and it doesn't hurt that it's not empty, because ARCH_SUBDIR is
> only used within this condition:
> 
> if [ $${SYSROOT_DIR_CANON} != $${ARCH_SYSROOT_DIR_CANON} ]
> 
> i.e. if SYSROOT == ARCH_SYSROOT, ARCH_SUBDIR will not be used. Makes sense too,
> of course: if there is no arch-specific sysroot directory, then there's no need
> to create the relative symlinks.
> 
>  That said, for consistency it's of course nicer to make ARCH_SUBDIR empty. But
> that's not needed for 2016.05.
> 
> > 
> > Fix the matching pattern to correctly return an empty string if there is
> > no arch subdir.
> > 
> > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> > Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> > Cc: Arnout Vandecappelle <arnout@mind.be>
> > Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
> > Cc: Peter Korsgaard <peter@korsgaard.com>
> > ---
> >  toolchain/toolchain-external/toolchain-external.mk | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/toolchain/toolchain-external/toolchain-external.mk b/toolchain/toolchain-external/toolchain-external.mk
> > index 47b951d..3af56b8 100644
> > --- a/toolchain/toolchain-external/toolchain-external.mk
> > +++ b/toolchain/toolchain-external/toolchain-external.mk
> > @@ -662,7 +662,7 @@ define TOOLCHAIN_EXTERNAL_INSTALL_SYSROOT_LIBS
> >  			SUPPORT_LIB_DIR=`readlink -f $${LIBSTDCPP_A_LOCATION} | sed -r -e 's:libstdc\+\+\.a::'` ; \
> >  		fi ; \
> >  	fi ; \
> > -	ARCH_SUBDIR=`echo $${ARCH_SYSROOT_DIR} | sed -r -e "s:^$${SYSROOT_DIR}(.*)/$$:\1:"` ; \
> > +	ARCH_SUBDIR=`echo $${ARCH_SYSROOT_DIR} | sed -r -e "s:^$${SYSROOT_DIR}((.*[^/])/?)?$$:\2:"` ; \
> 
>  Simpler alternative (just tested in the shell, not in an actual build):
> 
> ARCH_SUBDIR=`echo $${ARCH_SYSROOT_DIR} | sed -n -r -e
> "\:^$${SYSROOT_DIR}(.*)/$$:s::\1:p"`
> 
> Well, at least I find this regex easier to parse. Also, your version still gives
> a newline (which admittedly will be stripped inside the funciton) while mine is
> really an empty string.

OK, so, after a bit of discussion with Arnout on IRC, we came up to the
conclusion that we were a little bit too focused.

First, my proposal only works when there is an arch subdir that is a
single path component. So it is broken.

SAecond, Arnout's solution is a bit better, but like me he did not see
the full picture in this regexp.

All that this regexp tries to do is to capture whatever is *after* the
main sysroot. That can be achieved quite easily, in fact, and does not
need a convoluted regexp:

    ARCH_SUBDIR=`echo $${ARCH_SYSROOT_DIR} | sed -r -e "s:^$${SYSROOT_DIR}::"



However, there might be a gotcha.

Both SYSROOT_DIR and ARCH_SYSROOT_DIR end with a slash (as can be seen
in my examples).

The previous expression look like it would try to extract a relative
path that would neither start nor end with a slash; we would have gotten
something like:
    armv4
    armv4/thumb
    or/what/ever

This new regexp would extract somthing that would end up with a slash,
however. The number of slashes is important, because that is what is
used to construct a relative path in toolchain/helper.mk:

  123     nbslashs=`printf $${ARCH_SUBDIR} | sed 's%[^/]%%g' | wc -c` ; \
  124     for slash in `seq 1 $${nbslashs}` ; do \
  125         relpath=$${relpath}"../" ; \
  126     done ; \


So we need to do a second pass at it:

    ARCH_SUBDIR=`echo $${ARCH_SYSROOT_DIR} | sed -r -e "s:^$${SYSROOT_DIR}::; s:/+$$::"

And now we should be back to what is expected.

Absolutely untested, it's too late here, and we all know well what
happens when I test things too late in the evening. Gremlins! ;-]

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'