Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH] Revert "toolchain/toolchain-external: error if BR2_TOOLCHAIN_EXTERNAL_PATH is not set"
@ 2022-04-28 21:20 Thomas Petazzoni via buildroot
  2022-04-28 21:50 ` Yann E. MORIN
  2022-04-28 22:04 ` Yann E. MORIN
  0 siblings, 2 replies; 4+ messages in thread
From: Thomas Petazzoni via buildroot @ 2022-04-28 21:20 UTC (permalink / raw)
  To: James Hilliard, Arnout Vandecappelle, Peter Korsgaard,
	Kristof Havasi, Buildroot List
  Cc: Giulio Benetti, Romain Naour, Thomas De Schampheleire,
	Thomas Petazzoni

This reverts commit 8945ba49480a7885d104445ca855ef7681a374ff.

This commit is incorrect, as it is perfectly valid for
BR2_TOOLCHAIN_EXTERNAL_PATH to be empty. The help text of
BR2_TOOLCHAIN_EXTERNAL_PATH even documents it as a supported case:

          If empty, the compiler will be searched in $PATH.

Commit 392b0a26f5fd8d3aedce7e0c15f9762a79312f01 ("toolchain-external:
default BR2_TOOLCHAIN_EXTERNAL_PATH to empty") even made that the
default saying "In addition, it in fact works correctly when it is
empty. In that case, the toolchain will be searched in PATH."

A user has reported that commit
8945ba49480a7885d104445ca855ef7681a374ff breaks his use-case:

  https://lore.kernel.org/buildroot/CADBnMvhgaozAgZgy3njckjL1i0U6bZ0fLrq-kdFF-qpGhFWgmw@mail.gmail.com/

Reported-by: Kristof Havasi <havasiefr@gmail.com>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
This should be backported to 2022.02.x
---
 toolchain/toolchain-external/pkg-toolchain-external.mk | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/toolchain/toolchain-external/pkg-toolchain-external.mk b/toolchain/toolchain-external/pkg-toolchain-external.mk
index c7f4175c9e..299b6008aa 100644
--- a/toolchain/toolchain-external/pkg-toolchain-external.mk
+++ b/toolchain/toolchain-external/pkg-toolchain-external.mk
@@ -69,12 +69,7 @@ ifeq ($(BR2_TOOLCHAIN_EXTERNAL_DOWNLOAD),y)
 TOOLCHAIN_EXTERNAL_INSTALL_DIR = $(TOOLCHAIN_EXTERNAL_DOWNLOAD_INSTALL_DIR)
 else
 TOOLCHAIN_EXTERNAL_INSTALL_DIR = $(abspath $(call qstrip,$(BR2_TOOLCHAIN_EXTERNAL_PATH)))
-ifeq ($(BR2_TOOLCHAIN_EXTERNAL)$(BR_BUILDING),yy)
-ifeq ($(TOOLCHAIN_EXTERNAL_INSTALL_DIR),)
-$(error No external toolchain path set, check your BR2_TOOLCHAIN_EXTERNAL_PATH setting)
-endif
 endif
-endif # BR2_TOOLCHAIN_EXTERNAL_DOWNLOAD
 
 ifeq ($(TOOLCHAIN_EXTERNAL_INSTALL_DIR),)
 ifneq ($(TOOLCHAIN_EXTERNAL_PREFIX),)
-- 
2.35.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [Buildroot] [PATCH] Revert "toolchain/toolchain-external: error if BR2_TOOLCHAIN_EXTERNAL_PATH is not set"
  2022-04-28 21:20 [Buildroot] [PATCH] Revert "toolchain/toolchain-external: error if BR2_TOOLCHAIN_EXTERNAL_PATH is not set" Thomas Petazzoni via buildroot
@ 2022-04-28 21:50 ` Yann E. MORIN
  2022-04-28 21:55   ` Thomas Petazzoni via buildroot
  2022-04-28 22:04 ` Yann E. MORIN
  1 sibling, 1 reply; 4+ messages in thread
From: Yann E. MORIN @ 2022-04-28 21:50 UTC (permalink / raw)
  To: Thomas Petazzoni
  Cc: James Hilliard, Kristof Havasi, Buildroot List, Giulio Benetti,
	Romain Naour, Thomas De Schampheleire

Thomas, All,

On 2022-04-28 23:20 +0200, Thomas Petazzoni via buildroot spake thusly:
> This reverts commit 8945ba49480a7885d104445ca855ef7681a374ff.

This is a commit on the 2022.02.x branch; it is a backport of
8e91385a2cb81c5f8114354f4494230e2aebb93a on master.

Did you meant this patch to be applied only to 2022.02.x?

I would believe it should be applied to master and backported to 2022.02
instead, no?

Regards,
Yann E. MORIN.

> This commit is incorrect, as it is perfectly valid for
> BR2_TOOLCHAIN_EXTERNAL_PATH to be empty. The help text of
> BR2_TOOLCHAIN_EXTERNAL_PATH even documents it as a supported case:
> 
>           If empty, the compiler will be searched in $PATH.
> 
> Commit 392b0a26f5fd8d3aedce7e0c15f9762a79312f01 ("toolchain-external:
> default BR2_TOOLCHAIN_EXTERNAL_PATH to empty") even made that the
> default saying "In addition, it in fact works correctly when it is
> empty. In that case, the toolchain will be searched in PATH."
> 
> A user has reported that commit
> 8945ba49480a7885d104445ca855ef7681a374ff breaks his use-case:
> 
>   https://lore.kernel.org/buildroot/CADBnMvhgaozAgZgy3njckjL1i0U6bZ0fLrq-kdFF-qpGhFWgmw@mail.gmail.com/
> 
> Reported-by: Kristof Havasi <havasiefr@gmail.com>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> ---
> This should be backported to 2022.02.x
> ---
>  toolchain/toolchain-external/pkg-toolchain-external.mk | 5 -----
>  1 file changed, 5 deletions(-)
> 
> diff --git a/toolchain/toolchain-external/pkg-toolchain-external.mk b/toolchain/toolchain-external/pkg-toolchain-external.mk
> index c7f4175c9e..299b6008aa 100644
> --- a/toolchain/toolchain-external/pkg-toolchain-external.mk
> +++ b/toolchain/toolchain-external/pkg-toolchain-external.mk
> @@ -69,12 +69,7 @@ ifeq ($(BR2_TOOLCHAIN_EXTERNAL_DOWNLOAD),y)
>  TOOLCHAIN_EXTERNAL_INSTALL_DIR = $(TOOLCHAIN_EXTERNAL_DOWNLOAD_INSTALL_DIR)
>  else
>  TOOLCHAIN_EXTERNAL_INSTALL_DIR = $(abspath $(call qstrip,$(BR2_TOOLCHAIN_EXTERNAL_PATH)))
> -ifeq ($(BR2_TOOLCHAIN_EXTERNAL)$(BR_BUILDING),yy)
> -ifeq ($(TOOLCHAIN_EXTERNAL_INSTALL_DIR),)
> -$(error No external toolchain path set, check your BR2_TOOLCHAIN_EXTERNAL_PATH setting)
> -endif
>  endif
> -endif # BR2_TOOLCHAIN_EXTERNAL_DOWNLOAD
>  
>  ifeq ($(TOOLCHAIN_EXTERNAL_INSTALL_DIR),)
>  ifneq ($(TOOLCHAIN_EXTERNAL_PREFIX),)
> -- 
> 2.35.1
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Buildroot] [PATCH] Revert "toolchain/toolchain-external: error if BR2_TOOLCHAIN_EXTERNAL_PATH is not set"
  2022-04-28 21:50 ` Yann E. MORIN
@ 2022-04-28 21:55   ` Thomas Petazzoni via buildroot
  0 siblings, 0 replies; 4+ messages in thread
From: Thomas Petazzoni via buildroot @ 2022-04-28 21:55 UTC (permalink / raw)
  To: Yann E. MORIN
  Cc: James Hilliard, Kristof Havasi, Buildroot List, Giulio Benetti,
	Romain Naour, Thomas De Schampheleire

On Thu, 28 Apr 2022 23:50:03 +0200
"Yann E. MORIN" <yann.morin.1998@free.fr> wrote:

> On 2022-04-28 23:20 +0200, Thomas Petazzoni via buildroot spake thusly:
> > This reverts commit 8945ba49480a7885d104445ca855ef7681a374ff.  
> 
> This is a commit on the 2022.02.x branch; it is a backport of
> 8e91385a2cb81c5f8114354f4494230e2aebb93a on master.
> 
> Did you meant this patch to be applied only to 2022.02.x?

No.

> I would believe it should be applied to master and backported to 2022.02
> instead, no?

Yes, absolutely. I referred to 8e91385a2cb81c5f8114354f4494230e2aebb93a
because that's what the bug reporter pointed to as the offending
commit, as the bug report is on the 2022.02.x branch.

Though I agree the commit log should probably refer to the commit in
master. I can send a v2 if needed.

Thanks for the review!

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Buildroot] [PATCH] Revert "toolchain/toolchain-external: error if BR2_TOOLCHAIN_EXTERNAL_PATH is not set"
  2022-04-28 21:20 [Buildroot] [PATCH] Revert "toolchain/toolchain-external: error if BR2_TOOLCHAIN_EXTERNAL_PATH is not set" Thomas Petazzoni via buildroot
  2022-04-28 21:50 ` Yann E. MORIN
@ 2022-04-28 22:04 ` Yann E. MORIN
  1 sibling, 0 replies; 4+ messages in thread
From: Yann E. MORIN @ 2022-04-28 22:04 UTC (permalink / raw)
  To: Thomas Petazzoni
  Cc: James Hilliard, Kristof Havasi, Buildroot List, Giulio Benetti,
	Romain Naour, Thomas De Schampheleire

Thomas, All,

On 2022-04-28 23:20 +0200, Thomas Petazzoni via buildroot spake thusly:
> This reverts commit 8945ba49480a7885d104445ca855ef7681a374ff.

I've changes this to the corresponding sha1 on master.

> This commit is incorrect, as it is perfectly valid for
> BR2_TOOLCHAIN_EXTERNAL_PATH to be empty. The help text of
> BR2_TOOLCHAIN_EXTERNAL_PATH even documents it as a supported case:
> 
>           If empty, the compiler will be searched in $PATH.
> 
> Commit 392b0a26f5fd8d3aedce7e0c15f9762a79312f01 ("toolchain-external:
> default BR2_TOOLCHAIN_EXTERNAL_PATH to empty") even made that the
> default saying "In addition, it in fact works correctly when it is
> empty. In that case, the toolchain will be searched in PATH."
> 
> A user has reported that commit
> 8945ba49480a7885d104445ca855ef7681a374ff breaks his use-case:

And here I kept that hash and added a comment that it's a backpor of the
commit on master.

>   https://lore.kernel.org/buildroot/CADBnMvhgaozAgZgy3njckjL1i0U6bZ0fLrq-kdFF-qpGhFWgmw@mail.gmail.com/
> 
> Reported-by: Kristof Havasi <havasiefr@gmail.com>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>

Applied to master, thanks.

> ---
> This should be backported to 2022.02.x

Ah, I missed that post-cpmmit-log mesage earlier.

Regards,
Yann E. MORIN.

> ---
>  toolchain/toolchain-external/pkg-toolchain-external.mk | 5 -----
>  1 file changed, 5 deletions(-)
> 
> diff --git a/toolchain/toolchain-external/pkg-toolchain-external.mk b/toolchain/toolchain-external/pkg-toolchain-external.mk
> index c7f4175c9e..299b6008aa 100644
> --- a/toolchain/toolchain-external/pkg-toolchain-external.mk
> +++ b/toolchain/toolchain-external/pkg-toolchain-external.mk
> @@ -69,12 +69,7 @@ ifeq ($(BR2_TOOLCHAIN_EXTERNAL_DOWNLOAD),y)
>  TOOLCHAIN_EXTERNAL_INSTALL_DIR = $(TOOLCHAIN_EXTERNAL_DOWNLOAD_INSTALL_DIR)
>  else
>  TOOLCHAIN_EXTERNAL_INSTALL_DIR = $(abspath $(call qstrip,$(BR2_TOOLCHAIN_EXTERNAL_PATH)))
> -ifeq ($(BR2_TOOLCHAIN_EXTERNAL)$(BR_BUILDING),yy)
> -ifeq ($(TOOLCHAIN_EXTERNAL_INSTALL_DIR),)
> -$(error No external toolchain path set, check your BR2_TOOLCHAIN_EXTERNAL_PATH setting)
> -endif
>  endif
> -endif # BR2_TOOLCHAIN_EXTERNAL_DOWNLOAD
>  
>  ifeq ($(TOOLCHAIN_EXTERNAL_INSTALL_DIR),)
>  ifneq ($(TOOLCHAIN_EXTERNAL_PREFIX),)
> -- 
> 2.35.1
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2022-04-28 22:05 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-04-28 21:20 [Buildroot] [PATCH] Revert "toolchain/toolchain-external: error if BR2_TOOLCHAIN_EXTERNAL_PATH is not set" Thomas Petazzoni via buildroot
2022-04-28 21:50 ` Yann E. MORIN
2022-04-28 21:55   ` Thomas Petazzoni via buildroot
2022-04-28 22:04 ` Yann E. MORIN

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox