[Buildroot] [PATCH 0/7] More HOST_DIR/usr fixes

[Buildroot] [PATCH 0/7] More HOST_DIR/usr fixes

[Arnout Vandecappelle]

Thomas started rebuilding the toolchains after the HOST_DIR/usr removal
and noticed a corner case that didn't work. While investigating that, I
uncovered a slew of other issues.

I was also in the process of adding a test for
BR2_TOOLCHAIN_EXTERNAL_PREINSTALLED=y but that turns out to require a
lot of refactoring of the testing infra. So it's for another day.

Regards,
Arnout

[Buildroot] [PATCH 1/7] Makefile: properly create $(HOST_DIR)/usr compatibility symlink

[Arnout Vandecappelle]

Up to now we created the $(HOST_DIR)/usr compatibility symlink as part
of the creation of $(HOST_DIR) itself. However, when the user specifies
a custom BR2_HOST_DIR, it is possible that the directory already exists
so this rule will never trigger.

Therefore, add an explicit rule for creating $(HOST_DIR)/usr and add
this rule to the dependencies of the dirs target. HOST_DIR itself goes
back to the standard rule for directories. The order-only dependency of
STAGING_DIR isn't needed any more either: HOST_DIR is implicitly
created if needed by mkdir -p, and we don't need to trigger the
HOST_DIR rule any more if the directory already exists.

Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
 Makefile | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/Makefile b/Makefile
index a22e87f29e..188ce9adc7 100644
--- a/Makefile
+++ b/Makefile
@@ -237,7 +237,7 @@ LEGAL_REPORT = $(LEGAL_INFO_DIR)/README
 # dependencies anywhere else
 #
 ################################################################################
-$(BUILD_DIR) $(TARGET_DIR) $(BINARIES_DIR) $(LEGAL_INFO_DIR) $(REDIST_SOURCES_DIR_TARGET) $(REDIST_SOURCES_DIR_HOST):
+$(BUILD_DIR) $(TARGET_DIR) $(HOST_DIR) $(BINARIES_DIR) $(LEGAL_INFO_DIR) $(REDIST_SOURCES_DIR_TARGET) $(REDIST_SOURCES_DIR_HOST):
 	@mkdir -p $@
 
 BR2_CONFIG = $(CONFIG_DIR)/.config
@@ -541,7 +541,7 @@ endif
 
 .PHONY: dirs
 dirs: $(BUILD_DIR) $(STAGING_DIR) $(TARGET_DIR) \
-	$(HOST_DIR) $(BINARIES_DIR)
+	$(HOST_DIR) $(HOST_DIR)/usr $(BINARIES_DIR)
 
 $(BUILD_DIR)/buildroot-config/auto.conf: $(BR2_CONFIG)
 	$(MAKE1) $(EXTRAMAKEARGS) HOSTCC="$(HOSTCC_NOCCACHE)" HOSTCXX="$(HOSTCXX_NOCCACHE)" silentoldconfig
@@ -552,13 +552,12 @@ prepare: $(BUILD_DIR)/buildroot-config/auto.conf
 .PHONY: world
 world: target-post-image
 
-# When creating HOST_DIR, also symlink usr -> .
-$(HOST_DIR):
-	@mkdir -p $@
-	@ln -snf . $@/usr
+# Compatibility symlink in case a post-build script still uses $(HOST_DIR)/usr
+$(HOST_DIR)/usr: $(HOST_DIR)
+	@ln -snf . $@
 
 # Populating the staging with the base directories is handled by the skeleton package
-$(STAGING_DIR): | $(HOST_DIR)
+$(STAGING_DIR):
 	@mkdir -p $(STAGING_DIR)
 	@ln -snf $(STAGING_DIR) $(BASE_DIR)/staging
 
-- 
2.13.2

[Buildroot] [PATCH 1/7] Makefile: properly create $(HOST_DIR)/usr compatibility symlink

[Thomas Petazzoni]

Hello,

On Mon, 10 Jul 2017 01:21:17 +0200, Arnout Vandecappelle
(Essensium/Mind) wrote:
> Up to now we created the $(HOST_DIR)/usr compatibility symlink as part
> of the creation of $(HOST_DIR) itself. However, when the user specifies
> a custom BR2_HOST_DIR, it is possible that the directory already exists
> so this rule will never trigger.
> 
> Therefore, add an explicit rule for creating $(HOST_DIR)/usr and add
> this rule to the dependencies of the dirs target. HOST_DIR itself goes
> back to the standard rule for directories. The order-only dependency of
> STAGING_DIR isn't needed any more either: HOST_DIR is implicitly
> created if needed by mkdir -p, and we don't need to trigger the
> HOST_DIR rule any more if the directory already exists.
> 
> Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> ---
>  Makefile | 13 ++++++-------
>  1 file changed, 6 insertions(+), 7 deletions(-)

Applied to master, thanks.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 2/7] toolchain-wrapper: remove remaining references to HOST_DIR/usr

[Arnout Vandecappelle]

Commit 14151d77af20ec50eeba6e30465debf87b35faaa that eliminated
$(HOST_DIR)/usr seriously missed the toolchain-wrapper - only a single
reference was updated, the other three were missed. Commit
015d68c84c9c6ad6f6d41f181d19d813f309088b removed one more. This commit
finally removes the two remaining ones.

Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
 toolchain/toolchain-wrapper.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/toolchain/toolchain-wrapper.c b/toolchain/toolchain-wrapper.c
index a73e6d6f94..c048992656 100644
--- a/toolchain/toolchain-wrapper.c
+++ b/toolchain/toolchain-wrapper.c
@@ -208,14 +208,14 @@ int main(int argc, char **argv)
 #elif defined(BR_CROSS_PATH_ABS)
 	ret = snprintf(path, sizeof(path), BR_CROSS_PATH_ABS "/%s" BR_CROSS_PATH_SUFFIX, basename);
 #else
-	ret = snprintf(path, sizeof(path), "%s/usr/bin/%s" BR_CROSS_PATH_SUFFIX, absbasedir, basename);
+	ret = snprintf(path, sizeof(path), "%s/bin/%s" BR_CROSS_PATH_SUFFIX, absbasedir, basename);
 #endif
 	if (ret >= sizeof(path)) {
 		perror(__FILE__ ": overflow");
 		return 3;
 	}
 #ifdef BR_CCACHE
-	ret = snprintf(ccache_path, sizeof(ccache_path), "%s/usr/bin/ccache", absbasedir);
+	ret = snprintf(ccache_path, sizeof(ccache_path), "%s/bin/ccache", absbasedir);
 	if (ret >= sizeof(ccache_path)) {
 		perror(__FILE__ ": overflow");
 		return 3;
-- 
2.13.2

[Buildroot] [PATCH 2/7] toolchain-wrapper: remove remaining references to HOST_DIR/usr

[Thomas Petazzoni]

Hello,

On Mon, 10 Jul 2017 01:21:18 +0200, Arnout Vandecappelle
(Essensium/Mind) wrote:
> Commit 14151d77af20ec50eeba6e30465debf87b35faaa that eliminated
> $(HOST_DIR)/usr seriously missed the toolchain-wrapper - only a single
> reference was updated, the other three were missed. Commit
> 015d68c84c9c6ad6f6d41f181d19d813f309088b removed one more. This commit
> finally removes the two remaining ones.
> 
> Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> ---
>  toolchain/toolchain-wrapper.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Applied to master, thanks.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 3/7] support/testing: strip /usr/ part from HOST_DIR

[Arnout Vandecappelle]

Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
 support/testing/infra/__init__.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/support/testing/infra/__init__.py b/support/testing/infra/__init__.py
index a55ad7fdf5..27e2a2708d 100644
--- a/support/testing/infra/__init__.py
+++ b/support/testing/infra/__init__.py
@@ -55,7 +55,7 @@ def get_elf_arch_tag(builddir, prefix, fpath, tag):
     v5TEJ
     >>>
     """
-    cmd = ["host/usr/bin/{}-readelf".format(prefix),
+    cmd = ["host/bin/{}-readelf".format(prefix),
            "-A", os.path.join("target", fpath)]
     out = subprocess.check_output(cmd, cwd=builddir, env={"LANG": "C"})
     regexp = re.compile("^  {}: (.*)$".format(tag))
@@ -80,7 +80,7 @@ def get_elf_prog_interpreter(builddir, prefix, fpath):
     /lib/ld-linux-armhf.so.3
     >>>
     """
-    cmd = ["host/usr/bin/{}-readelf".format(prefix),
+    cmd = ["host/bin/{}-readelf".format(prefix),
            "-l", os.path.join("target", fpath)]
     out = subprocess.check_output(cmd, cwd=builddir, env={"LANG": "C"})
     regexp = re.compile("^ *\[Requesting program interpreter: (.*)\]$")
-- 
2.13.2

[Buildroot] [PATCH 3/7] support/testing: strip /usr/ part from HOST_DIR

[Thomas Petazzoni]

Hello,

On Mon, 10 Jul 2017 01:21:19 +0200, Arnout Vandecappelle
(Essensium/Mind) wrote:
> Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> ---
>  support/testing/infra/__init__.py | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Applied to master, thanks.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 4/7] support/testing: move BRTest initialisation to __init__

[Arnout Vandecappelle]

BRTest's setUp() method contains a few assignments that initialize its
member variables. Since we will want to use these in test case
overrides, move them to the __init__ function.

Also allow the config member to be overridden, rather than always
taking the class member.

Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
 support/testing/infra/basetest.py | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/support/testing/infra/basetest.py b/support/testing/infra/basetest.py
index d75458a02c..2a5c9ec939 100644
--- a/support/testing/infra/basetest.py
+++ b/support/testing/infra/basetest.py
@@ -36,15 +36,18 @@ class BRTest(unittest.TestCase):
     keepbuilds = False
     jlevel = None
 
+    def __init__(self, names):
+        super(BRTest, self).__init__(names)
+        self.testname = self.__class__.__name__
+        self.builddir = os.path.join(self.__class__.outputdir, self.testname)
+        self.emulator = None
+
     def show_msg(self, msg):
         print "{} {:40s} {}".format(datetime.datetime.now().strftime("%H:%M:%S"),
                                     self.testname, msg)
     def setUp(self):
-        self.testname = self.__class__.__name__
-        self.builddir = os.path.join(self.__class__.outputdir, self.testname)
-        self.emulator = None
         self.show_msg("Starting")
-        config = self.__class__.config
+        config = self.config
         if self.jlevel:
             config += "BR2_JLEVEL={}\n".format(self.jlevel)
         self.b = Builder(config, self.builddir, self.logtofile)
-- 
2.13.2

[Buildroot] [PATCH 4/7] support/testing: move BRTest initialisation to __init__

[Thomas Petazzoni]

Hello,

On Mon, 10 Jul 2017 01:21:20 +0200, Arnout Vandecappelle
(Essensium/Mind) wrote:
> BRTest's setUp() method contains a few assignments that initialize its
> member variables. Since we will want to use these in test case
> overrides, move them to the __init__ function.
> 
> Also allow the config member to be overridden, rather than always
> taking the class member.
> 
> Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> ---
>  support/testing/infra/basetest.py | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)

Applied to master, thanks.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 5/7] support/testing: add test of BR2_CCACHE with an external toolchain

[Arnout Vandecappelle]

We piggy-back on an existing test.

Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
 support/testing/tests/toolchain/test_external.py | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/support/testing/tests/toolchain/test_external.py b/support/testing/tests/toolchain/test_external.py
index afb4bb0b50..c315ef8055 100644
--- a/support/testing/tests/toolchain/test_external.py
+++ b/support/testing/tests/toolchain/test_external.py
@@ -207,9 +207,13 @@ BR2_TOOLCHAIN_EXTERNAL_CXX=y
         self.emulator.login()
 
 class TestExternalToolchainBuildrootuClibc(TestExternalToolchain):
+    # On this test we piggy-back a test of BR2_CCACHE in combination
+    # with an external toolchain.
     config = BASIC_CONFIG + \
 """
 BR2_arm=y
+BR2_CCACHE=y
+BR2_CCACHE_DIR="{builddir}/ccache-dir"
 BR2_TOOLCHAIN_EXTERNAL=y
 BR2_TOOLCHAIN_EXTERNAL_CUSTOM=y
 BR2_TOOLCHAIN_EXTERNAL_DOWNLOAD=y
@@ -222,6 +226,10 @@ BR2_TOOLCHAIN_EXTERNAL_CXX=y
 """
     toolchain_prefix = "arm-linux"
 
+    def __init__(self, names):
+        super(TestExternalToolchainBuildrootuClibc, self).__init__(names)
+        self.config = self.config.format(builddir = self.builddir)
+
     def test_run(self):
         TestExternalToolchain.common_check(self)
         img = os.path.join(self.builddir, "images", "rootfs.cpio")
-- 
2.13.2

[Buildroot] [PATCH 5/7] support/testing: add test of BR2_CCACHE with an external toolchain

[Thomas Petazzoni]

Hello,

On Mon, 10 Jul 2017 01:21:21 +0200, Arnout Vandecappelle
(Essensium/Mind) wrote:
> We piggy-back on an existing test.
> 
> Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> ---
>  support/testing/tests/toolchain/test_external.py | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/support/testing/tests/toolchain/test_external.py b/support/testing/tests/toolchain/test_external.py
> index afb4bb0b50..c315ef8055 100644
> --- a/support/testing/tests/toolchain/test_external.py
> +++ b/support/testing/tests/toolchain/test_external.py
> @@ -207,9 +207,13 @@ BR2_TOOLCHAIN_EXTERNAL_CXX=y
>          self.emulator.login()
>  
>  class TestExternalToolchainBuildrootuClibc(TestExternalToolchain):
> +    # On this test we piggy-back a test of BR2_CCACHE in combination
> +    # with an external toolchain.

I'm not sure I'm a big fan of "hijacking" an existing test to validate
ccache. Shouldn't we have a separate test for that, clearly identified
as being ccache related?

Could be a test that inherits from this one, and simply adds more
options. But at least in case of failure it will be clearly identified
as a CCache related test in the Gitlab CI results.

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[Buildroot] [PATCH 5/7] support/testing: add test of BR2_CCACHE with an external toolchain

[Arnout Vandecappelle]

On 10-07-17 18:01, Thomas Petazzoni wrote:
> Hello,
> 
> On Mon, 10 Jul 2017 01:21:21 +0200, Arnout Vandecappelle
> (Essensium/Mind) wrote:
>> We piggy-back on an existing test.
>>
>> Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
>> ---
>>  support/testing/tests/toolchain/test_external.py | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/support/testing/tests/toolchain/test_external.py b/support/testing/tests/toolchain/test_external.py
>> index afb4bb0b50..c315ef8055 100644
>> --- a/support/testing/tests/toolchain/test_external.py
>> +++ b/support/testing/tests/toolchain/test_external.py
>> @@ -207,9 +207,13 @@ BR2_TOOLCHAIN_EXTERNAL_CXX=y
>>          self.emulator.login()
>>  
>>  class TestExternalToolchainBuildrootuClibc(TestExternalToolchain):
>> +    # On this test we piggy-back a test of BR2_CCACHE in combination
>> +    # with an external toolchain.
> 
> I'm not sure I'm a big fan of "hijacking" an existing test to validate
> ccache. Shouldn't we have a separate test for that, clearly identified
> as being ccache related?
> 
> Could be a test that inherits from this one, and simply adds more
> options. But at least in case of failure it will be clearly identified
> as a CCache related test in the Gitlab CI results.

 I didn't want to duplicate stuff, that's why. Inheriting is a great idea.

 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 6/7] Makefile: add check that $(HOST_DIR)/usr is not a directory

[Arnout Vandecappelle]

We create a compatibility symlink for $(HOST_DIR)/usr. However, if
that exists already as a directory, the link can't be created. Make
will not even try since it exists already and has no dependencies. If
it exists as a directory, any post-build script that is still using
$(HOST_DIR)/usr will fail to find what it needs. Therefore, add a
check that it is not a directory.

This check has to be made as part of some PHONY target. We can reuse
the dirs target, on which all packages depend.

Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
Cc: Yann E. MORIN <yann.morin.1998@free.fr>
---
Personally I don't think it's important enough to add such a check.
If someone ends up in this situation, they'll be motivated to fix
their post-build scripts :-)
---
 Makefile | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Makefile b/Makefile
index 188ce9adc7..a94555d130 100644
--- a/Makefile
+++ b/Makefile
@@ -542,6 +542,11 @@ endif
 .PHONY: dirs
 dirs: $(BUILD_DIR) $(STAGING_DIR) $(TARGET_DIR) \
 	$(HOST_DIR) $(HOST_DIR)/usr $(BINARIES_DIR)
+	@if [ -d "$(HOST_DIR)/usr" -a ! -L "$(HOST_DIR)/usr" ]; then \
+		printf '%s is not allowed to be a directory\n' "$(HOST_DIR)/usr" 1>&2; \
+		printf 'Please remove this directory and rebuild\n' 1>&2; \
+		exit 1; \
+	fi
 
 $(BUILD_DIR)/buildroot-config/auto.conf: $(BR2_CONFIG)
 	$(MAKE1) $(EXTRAMAKEARGS) HOSTCC="$(HOSTCC_NOCCACHE)" HOSTCXX="$(HOSTCXX_NOCCACHE)" silentoldconfig
-- 
2.13.2

[Buildroot] [PATCH 6/7] Makefile: add check that $(HOST_DIR)/usr is not a directory

[Thomas Petazzoni]

Hello,

On Mon, 10 Jul 2017 01:21:22 +0200, Arnout Vandecappelle
(Essensium/Mind) wrote:
> We create a compatibility symlink for $(HOST_DIR)/usr. However, if
> that exists already as a directory, the link can't be created. Make
> will not even try since it exists already and has no dependencies. If
> it exists as a directory, any post-build script that is still using
> $(HOST_DIR)/usr will fail to find what it needs. Therefore, add a
> check that it is not a directory.
> 
> This check has to be made as part of some PHONY target. We can reuse
> the dirs target, on which all packages depend.
> 
> Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> Cc: Yann E. MORIN <yann.morin.1998@free.fr>
> ---
> Personally I don't think it's important enough to add such a check.
> If someone ends up in this situation, they'll be motivated to fix
> their post-build scripts :-)

I also don't like very much adding such checks. In which situation can
our users fall into this problem?

They have an existing Buildroot tree, they do "git pull" and do a
partial rebuild ?

> diff --git a/Makefile b/Makefile
> index 188ce9adc7..a94555d130 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -542,6 +542,11 @@ endif
>  .PHONY: dirs
>  dirs: $(BUILD_DIR) $(STAGING_DIR) $(TARGET_DIR) \
>  	$(HOST_DIR) $(HOST_DIR)/usr $(BINARIES_DIR)
> +	@if [ -d "$(HOST_DIR)/usr" -a ! -L "$(HOST_DIR)/usr" ]; then \
> +		printf '%s is not allowed to be a directory\n' "$(HOST_DIR)/usr" 1>&2; \
> +		printf 'Please remove this directory and rebuild\n' 1>&2; \

What about saying "Please run make clean all" ?

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[Buildroot] [PATCH 6/7] Makefile: add check that $(HOST_DIR)/usr is not a directory

[Yann E. MORIN]

Thomas, All,

On 2017-07-10 18:02 +0200, Thomas Petazzoni spake thusly:
> On Mon, 10 Jul 2017 01:21:22 +0200, Arnout Vandecappelle
> (Essensium/Mind) wrote:
> > We create a compatibility symlink for $(HOST_DIR)/usr. However, if
> > that exists already as a directory, the link can't be created. Make
> > will not even try since it exists already and has no dependencies. If
> > it exists as a directory, any post-build script that is still using
> > $(HOST_DIR)/usr will fail to find what it needs. Therefore, add a
> > check that it is not a directory.
> > 
> > This check has to be made as part of some PHONY target. We can reuse
> > the dirs target, on which all packages depend.
> > 
> > Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> > Cc: Yann E. MORIN <yann.morin.1998@free.fr>
> > ---
> > Personally I don't think it's important enough to add such a check.
> > If someone ends up in this situation, they'll be motivated to fix
> > their post-build scripts :-)
> 
> I also don't like very much adding such checks. In which situation can
> our users fall into this problem?

I've explained it in details when you initially reported the issue of
regenerating the toolchains:

    http://lists.busybox.net/pipermail/buildroot/2017-July/197456.html

> They have an existing Buildroot tree, they do "git pull" and do a
> partial rebuild ?

No, that occurs when a user points HOST_DIR outside, like you do for the
toolchains, and that host-dir already exists, and it already contains a
usr/ directory.

My opinion is that we should not allow using HOST_DIR if it is not empty
at all.

Regards,
Yann E. MORIN.

> > diff --git a/Makefile b/Makefile
> > index 188ce9adc7..a94555d130 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -542,6 +542,11 @@ endif
> >  .PHONY: dirs
> >  dirs: $(BUILD_DIR) $(STAGING_DIR) $(TARGET_DIR) \
> >  	$(HOST_DIR) $(HOST_DIR)/usr $(BINARIES_DIR)
> > +	@if [ -d "$(HOST_DIR)/usr" -a ! -L "$(HOST_DIR)/usr" ]; then \
> > +		printf '%s is not allowed to be a directory\n' "$(HOST_DIR)/usr" 1>&2; \
> > +		printf 'Please remove this directory and rebuild\n' 1>&2; \
> 
> What about saying "Please run make clean all" ?
> 
> Thomas
> -- 
> Thomas Petazzoni, CTO, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com

-- 
.-----------------.--------------------.------------------.--------------------.
|  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 6/7] Makefile: add check that $(HOST_DIR)/usr is not a directory

[Arnout Vandecappelle]

On 10-07-17 18:12, Yann E. MORIN wrote:
> Thomas, All,
> 
> On 2017-07-10 18:02 +0200, Thomas Petazzoni spake thusly:
[snip]
>> I also don't like very much adding such checks. In which situation can
>> our users fall into this problem?
> 
> I've explained it in details when you initially reported the issue of
> regenerating the toolchains:
> 
>     http://lists.busybox.net/pipermail/buildroot/2017-July/197456.html
> 
>> They have an existing Buildroot tree, they do "git pull" and do a
>> partial rebuild ?
> 
> No, that occurs when a user points HOST_DIR outside, like you do for the
> toolchains, and that host-dir already exists, and it already contains a
> usr/ directory.

 Actually, I realize now: when people do a git pull, we expect them to do "make
clean all" afterwards. And "make clean" will delete HOST_DIR - so no
HOST_DIR/usr will exist anymore.

 So the original report from Thomas only happened because Thomas didn't do a
"make clean" like he should have...

 Or am I missing something?


> My opinion is that we should not allow using HOST_DIR if it is not empty
> at all.

 Not sure what you mean with "using HOST_DIR". Do you mean "using a non-default
HOST_DIR"? Do you mean that we should force people to do "make clean all" *all
the time* when using a non-default HOST_DIR?

 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 6/7] Makefile: add check that $(HOST_DIR)/usr is not a directory

[Arnout Vandecappelle]

On 10-07-17 21:51, Arnout Vandecappelle wrote:
> 
> 
> On 10-07-17 18:12, Yann E. MORIN wrote:
>> Thomas, All,
>>
>> On 2017-07-10 18:02 +0200, Thomas Petazzoni spake thusly:
> [snip]
>>> I also don't like very much adding such checks. In which situation can
>>> our users fall into this problem?
>>
>> I've explained it in details when you initially reported the issue of
>> regenerating the toolchains:
>>
>>     http://lists.busybox.net/pipermail/buildroot/2017-July/197456.html
>>
>>> They have an existing Buildroot tree, they do "git pull" and do a
>>> partial rebuild ?
>>
>> No, that occurs when a user points HOST_DIR outside, like you do for the
>> toolchains, and that host-dir already exists, and it already contains a
>> usr/ directory.
> 
>  Actually, I realize now: when people do a git pull, we expect them to do "make
> clean all" afterwards. And "make clean" will delete HOST_DIR - so no
> HOST_DIR/usr will exist anymore.
> 
>  So the original report from Thomas only happened because Thomas didn't do a
> "make clean" like he should have...
> 
>  Or am I missing something?

 By the way, since I think this patch is useless, I'm not going to respin it.

 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 6/7] Makefile: add check that $(HOST_DIR)/usr is not a directory

[Thomas Petazzoni]

Hello,

On Mon, 10 Jul 2017 21:51:50 +0200, Arnout Vandecappelle wrote:

>  Actually, I realize now: when people do a git pull, we expect them to do "make
> clean all" afterwards. And "make clean" will delete HOST_DIR - so no
> HOST_DIR/usr will exist anymore.
> 
>  So the original report from Thomas only happened because Thomas didn't do a
> "make clean" like he should have...

Not correct, each time my output directory and host directories were
empty. The only thing that was different is that the host directory is
not set to the default of $(BASE_DIR)/host, but to /opt/something, and
this /opt/something is created (empty) before I start the build.

So my build was perfectly clean, it is just that I created the host
directory prior to starting the build, which IMO isn't crazy,
especially when such host directory was customized in the Buildroot
configuration.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[Buildroot] [PATCH 6/7] Makefile: add check that $(HOST_DIR)/usr is not a directory

[Arnout Vandecappelle]

On 10-07-17 23:27, Thomas Petazzoni wrote:
> Hello,
> 
> On Mon, 10 Jul 2017 21:51:50 +0200, Arnout Vandecappelle wrote:
> 
>>  Actually, I realize now: when people do a git pull, we expect them to do "make
>> clean all" afterwards. And "make clean" will delete HOST_DIR - so no
>> HOST_DIR/usr will exist anymore.
>>
>>  So the original report from Thomas only happened because Thomas didn't do a
>> "make clean" like he should have...
> 
> Not correct, each time my output directory and host directories were
> empty. The only thing that was different is that the host directory is
> not set to the default of $(BASE_DIR)/host, but to /opt/something, and
> this /opt/something is created (empty) before I start the build.

 The part about a non-standard HOST_DIR is irrelevant, it's doing

make clean
mkdir -p output/host
make

that triggered the issue. Which obviously makes no sense at all in the normal
situation, but does make some kind of twisted sense in the non-standard HOST_DIR
situation.

> So my build was perfectly clean, it is just that I created the host
> directory prior to starting the build, which IMO isn't crazy,
> especially when such host directory was customized in the Buildroot
> configuration.

 Well, it doesn't make *that* much sense to me, because pre-populating that
directory with stuff is kind of fragile, given that it will all be removed again
by "make clean".

 Anyway, I still don't think that this patch is very useful :-)


 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 6/7] Makefile: add check that $(HOST_DIR)/usr is not a directory

[Yann E. MORIN]

Arnout, all,

On 2017-07-10 23:36 +0200, Arnout Vandecappelle spake thusly:
> On 10-07-17 23:27, Thomas Petazzoni wrote:
> > Hello,
> > 
> > On Mon, 10 Jul 2017 21:51:50 +0200, Arnout Vandecappelle wrote:
> > 
> >>  Actually, I realize now: when people do a git pull, we expect them to do "make
> >> clean all" afterwards. And "make clean" will delete HOST_DIR - so no
> >> HOST_DIR/usr will exist anymore.
> >>
> >>  So the original report from Thomas only happened because Thomas didn't do a
> >> "make clean" like he should have...
> > 
> > Not correct, each time my output directory and host directories were
> > empty. The only thing that was different is that the host directory is
> > not set to the default of $(BASE_DIR)/host, but to /opt/something, and
> > this /opt/something is created (empty) before I start the build.
> 
>  The part about a non-standard HOST_DIR is irrelevant, it's doing
> 
> make clean
> mkdir -p output/host
> make
> 
> that triggered the issue. Which obviously makes no sense at all in the normal
> situation, but does make some kind of twisted sense in the non-standard HOST_DIR
> situation.
> 
> > So my build was perfectly clean, it is just that I created the host
> > directory prior to starting the build, which IMO isn't crazy,
> > especially when such host directory was customized in the Buildroot
> > configuration.
> 
>  Well, it doesn't make *that* much sense to me, because pre-populating that
> directory with stuff is kind of fragile, given that it will all be removed again
> by "make clean".

It is not about running "make clean" but having stuff pre-isntalled in
there, from a clean build.

I.e. people would do:
  mkdir /my/host-dir
  tar xf mystuff.tar -C /my/host-dir
  make -C buildroot my_toolchain_defconfig
  make -C buildroot toolchain

and then their stuff is poulting the host-dir, which we should try to
avoid.

Granted, there is nothing that would prevent them from extracting it
after the build...

>  Anyway, I still don't think that this patch is very useful :-)

Until we have weird build errors that are reported, because some weirdo
will have already extracted a few toolchains in there. Or tries to
re-use the same host dir to isntall many toolchains in there... ;-)

(Note: I take it that Thomas uses different host dirs for each of the
toolchains, ot that the host dir is empty at the start of each toolchain
build.)

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 7/7] toolchain-external: default BR2_TOOLCHAIN_EXTERNAL_PATH to empty

[Arnout Vandecappelle]

It makes no sense to default to an arbitrary path. In addition, it in
fact works correctly when it is empty. In that case, the toolchain will
be searched in PATH.

Update the help text to explain the above, and also that the compiler
is supposed to be in the bin subdirectory.

Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
 toolchain/toolchain-external/Config.in | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/toolchain/toolchain-external/Config.in b/toolchain/toolchain-external/Config.in
index 4cd9ee4752..c4063b177b 100644
--- a/toolchain/toolchain-external/Config.in
+++ b/toolchain/toolchain-external/Config.in
@@ -67,10 +67,14 @@ endchoice
 
 config BR2_TOOLCHAIN_EXTERNAL_PATH
 	string "Toolchain path"
-	default "/path/to/toolchain/usr"
+	default ""
 	depends on BR2_TOOLCHAIN_EXTERNAL_PREINSTALLED
 	help
-	  Path to where the external toolchain is installed.
+	  Path to where the external toolchain is installed. The
+	  compiler itself is expected to be in the "bin" subdirectory
+	  of this path.
+
+	  If empty, the compiler will be searched in $PATH.
 
 config BR2_TOOLCHAIN_EXTERNAL_GLIBC
 	bool
-- 
2.13.2

[Buildroot] [PATCH 7/7] toolchain-external: default BR2_TOOLCHAIN_EXTERNAL_PATH to empty

[Thomas Petazzoni]

Hello,

On Mon, 10 Jul 2017 01:21:23 +0200, Arnout Vandecappelle
(Essensium/Mind) wrote:
> It makes no sense to default to an arbitrary path. In addition, it in
> fact works correctly when it is empty. In that case, the toolchain will
> be searched in PATH.
> 
> Update the help text to explain the above, and also that the compiler
> is supposed to be in the bin subdirectory.
> 
> Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> ---
>  toolchain/toolchain-external/Config.in | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)

Applied to master, thanks.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com