diff mbox

FW: [PATCH] external toolchain: use relative path to external toolchain.

Message ID 884EA965490E3C4D8E66AEF41E98025006BF60@ezex10.ezchip.com
State Rejected
Headers show

Commit Message

Noam Camus Feb. 23, 2014, 8:29 a.m. UTC
Now configuration may include relative path to external toolchain.

Signed-off-by: Noam Camus <noamc@ezchip.com>
---
 toolchain/toolchain-external/toolchain-external.mk |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Comments

Yann E. MORIN March 4, 2014, 10:29 p.m. UTC | #1
Noam, All,

On 2014-02-23 08:29 +0000, Noam Camus spake thusly:
> Now configuration may include relative path to external toolchain.

What is the use-case for providing a relative path?

Do not forget that relative paths are always relative to the top dir of
the Buildroot source tree, not the current directory.

For example, if you build out-of-tree, then the relative paths are still
relative to Buildroot's tree, not $(O), which can have some strange
consequences:

    # ls -lF
    buildroot/
    ext-toolchain/
    $ mkdir -p my/own/output/dir
    $ cd my/own/output/dir
    $ make -C ../../../../buildroot O=$(pwd) my_defconfig all

So, you would have to set BR2_TOOLCHAIN_EXTERNAL_PATH=../ext-toolchain,
which is not straightforward, as most users would expect to set a path
relative to the current working directory. [0]

But, if you want to use relative paths, you can do use $(TOPDIR) or
$(CONFIG_DIR) as a prefix.

So, I'm not really sure we want that.

[0] This is not Buildroot's fault, but a standard behaviour of make,
that prevents us from knowing the directory make was run from in the
first place. This information is lost, and can not be retrieved from
inside a Makefile.

Regards,
Yann E. MORIN.
Thomas Petazzoni April 20, 2014, 9:12 a.m. UTC | #2
Dear Noam Camus,

On Sun, 23 Feb 2014 08:29:34 +0000, Noam Camus wrote:
> Now configuration may include relative path to external toolchain.
> 
> Signed-off-by: Noam Camus <noamc@ezchip.com>
> ---
>  toolchain/toolchain-external/toolchain-external.mk |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)

A week or two after you submitted this patch, Yann E. Morin asked some
questions about why this is needed (e-mail from March, 4th), and you
never came back with answers to Yann's questions. Since Yann basically
questions the usefulness of the patch, I'm going to mark this patch as
"Rejected" for now in our patchwork tool. If, despite Yann comments,
you feel that this patch is still useful for you, please resubmit with
a more detailed commit log that explains *why* it is needed.

Thanks!

Thomas
diff mbox

Patch

diff --git a/toolchain/toolchain-external/toolchain-external.mk b/toolchain/toolchain-external/toolchain-external.mk
index a7bfafc..46f33a9 100644
--- a/toolchain/toolchain-external/toolchain-external.mk
+++ b/toolchain/toolchain-external/toolchain-external.mk
@@ -131,7 +131,7 @@  TOOLCHAIN_EXTERNAL_PREFIX=$(call qstrip,$(BR2_TOOLCHAIN_EXTERNAL_PREFIX))
 ifeq ($(BR2_TOOLCHAIN_EXTERNAL_DOWNLOAD),y)
 TOOLCHAIN_EXTERNAL_INSTALL_DIR=$(HOST_DIR)/opt/ext-toolchain
 else
-TOOLCHAIN_EXTERNAL_INSTALL_DIR=$(call qstrip,$(BR2_TOOLCHAIN_EXTERNAL_PATH))
+TOOLCHAIN_EXTERNAL_INSTALL_DIR=$(abspath $(call qstrip,$(BR2_TOOLCHAIN_EXTERNAL_PATH)))
 endif
 
 ifeq ($(TOOLCHAIN_EXTERNAL_INSTALL_DIR),)