| Message ID | 20161123111404.20661-1-arnout@mind.be |
|---|---|
| State | Changes Requested |
| Headers | show |
Hello, On Wed, 23 Nov 2016 12:14:04 +0100, Arnout Vandecappelle (Essensium/Mind) wrote: > We should also remove the 4.3 and 4.4 options, because they are > currently unused and also untested in the autobuilders. The Blackfin ADI toolchains use gcc 4.3, if I remember correctly. Thomas
On 23-11-16 13:22, Thomas Petazzoni wrote: > Hello, > > On Wed, 23 Nov 2016 12:14:04 +0100, Arnout Vandecappelle > (Essensium/Mind) wrote: > >> We should also remove the 4.3 and 4.4 options, because they are >> currently unused and also untested in the autobuilders. > > The Blackfin ADI toolchains use gcc 4.3, if I remember correctly. Sorry, let me correct my statement: because they are currently unused by any package and will very soon be untested in the autobuilders. Regards, Arnout
Hello, On Wed, 23 Nov 2016 13:27:38 +0100, Arnout Vandecappelle wrote: > > The Blackfin ADI toolchains use gcc 4.3, if I remember correctly. > > Sorry, let me correct my statement: because they are currently unused by any > package and will very soon be untested in the autobuilders. Huh? toolchain/toolchain-external/toolchain-external-blackfin-uclinux/Config.in: select BR2_TOOLCHAIN_GCC_AT_LEAST_4_3 So it's definitely used, at least until we remove the ADI Blackfin toolchain. Bets regards, Thomas
Hello, On Wed, 23 Nov 2016 12:14:04 +0100, Arnout Vandecappelle (Essensium/Mind) wrote: > This patch conflicts with the external toolchain rework - I'll rebase > the one which "looses" in the first-to-be-committed contest. external toolchain rework committed :) > > We should also remove the 4.3 and 4.4 options, because they are > currently unused and also untested in the autobuilders. > > I think something similar should be done for kernel headers. We already have something like this: we have BR2_TOOLCHAIN_EXTERNAL_HEADERS_REALLY_OLD. > toolchain/helpers.mk | 3 +-- > toolchain/toolchain-external/Config.in | 11 +++++++++++ > 2 files changed, 12 insertions(+), 2 deletions(-) > > diff --git a/toolchain/helpers.mk b/toolchain/helpers.mk > index 3991bc1..72e7292 100644 > --- a/toolchain/helpers.mk > +++ b/toolchain/helpers.mk > @@ -145,8 +145,7 @@ check_kernel_headers_version = \ > check_gcc_version = \ > expected_version="$(strip $2)" ; \ > if [ -z "$${expected_version}" ]; then \ > - printf "Internal error, gcc version unknown (no GCC_AT_LEAST_X_Y selected)\n"; \ > - exit 1 ; \ > + exit 0 ; \ Why? Assuming you will respin, I've marked this one as Changes Requested. Thanks! Thomas
On 23-11-16 23:19, Thomas Petazzoni wrote: > Hello, > > On Wed, 23 Nov 2016 13:27:38 +0100, Arnout Vandecappelle wrote: > >>> The Blackfin ADI toolchains use gcc 4.3, if I remember correctly. >> >> Sorry, let me correct my statement: because they are currently unused by any >> package and will very soon be untested in the autobuilders. > > Huh? > > toolchain/toolchain-external/toolchain-external-blackfin-uclinux/Config.in: select BR2_TOOLCHAIN_GCC_AT_LEAST_4_3 > > So it's definitely used, at least until we remove the ADI Blackfin > toolchain. When I say package, I mean: no package depends on _AT_LEAST_4_3 or _4_4. And the ADI Blackfin removal patch is coming up :-) Regards, Arnout > > Bets regards, > > Thomas >
On 23-11-16 23:21, Thomas Petazzoni wrote: [snip] >> diff --git a/toolchain/helpers.mk b/toolchain/helpers.mk >> index 3991bc1..72e7292 100644 >> --- a/toolchain/helpers.mk >> +++ b/toolchain/helpers.mk >> @@ -145,8 +145,7 @@ check_kernel_headers_version = \ >> check_gcc_version = \ >> expected_version="$(strip $2)" ; \ >> if [ -z "$${expected_version}" ]; then \ >> - printf "Internal error, gcc version unknown (no GCC_AT_LEAST_X_Y selected)\n"; \ >> - exit 1 ; \ >> + exit 0 ; \ > > Why? As stated in the commit log: When [BR2_TOOLCHAIN_GCC_AT_LEAST] is empty, we don't do a version check at all in check_gcc_version. Perhaps I should add to that "(previously we errored out)". Regards, Arnout > > Assuming you will respin, I've marked this one as Changes Requested. > > Thanks! > > Thomas >
Hello, On Thu, 24 Nov 2016 00:38:33 +0100, Arnout Vandecappelle wrote: > On 23-11-16 23:21, Thomas Petazzoni wrote: > [snip] > >> diff --git a/toolchain/helpers.mk b/toolchain/helpers.mk > >> index 3991bc1..72e7292 100644 > >> --- a/toolchain/helpers.mk > >> +++ b/toolchain/helpers.mk > >> @@ -145,8 +145,7 @@ check_kernel_headers_version = \ > >> check_gcc_version = \ > >> expected_version="$(strip $2)" ; \ > >> if [ -z "$${expected_version}" ]; then \ > >> - printf "Internal error, gcc version unknown (no GCC_AT_LEAST_X_Y selected)\n"; \ > >> - exit 1 ; \ > >> + exit 0 ; \ > > > > Why? > > As stated in the commit log: > > When [BR2_TOOLCHAIN_GCC_AT_LEAST] is > empty, we don't do a version check at all in check_gcc_version. > > Perhaps I should add to that "(previously we errored out)". Well, we're still erroring out, but with a 0 exit code. So I must be missing something. Thomas
On 24-11-16 09:01, Thomas Petazzoni wrote: > Hello, > > On Thu, 24 Nov 2016 00:38:33 +0100, Arnout Vandecappelle wrote: >> On 23-11-16 23:21, Thomas Petazzoni wrote: >> [snip] >>>> diff --git a/toolchain/helpers.mk b/toolchain/helpers.mk >>>> index 3991bc1..72e7292 100644 >>>> --- a/toolchain/helpers.mk >>>> +++ b/toolchain/helpers.mk >>>> @@ -145,8 +145,7 @@ check_kernel_headers_version = \ >>>> check_gcc_version = \ >>>> expected_version="$(strip $2)" ; \ >>>> if [ -z "$${expected_version}" ]; then \ >>>> - printf "Internal error, gcc version unknown (no GCC_AT_LEAST_X_Y selected)\n"; \ >>>> - exit 1 ; \ >>>> + exit 0 ; \ >>> >>> Why? >> >> As stated in the commit log: >> >> When [BR2_TOOLCHAIN_GCC_AT_LEAST] is >> empty, we don't do a version check at all in check_gcc_version. >> >> Perhaps I should add to that "(previously we errored out)". > > Well, we're still erroring out, but with a 0 exit code. So I must be > missing something. 0 is not an error, right? Too early in the morning? Regards, Arnout
Hello, On Thu, 24 Nov 2016 09:39:13 +0100, Arnout Vandecappelle wrote: > >> When [BR2_TOOLCHAIN_GCC_AT_LEAST] is > >> empty, we don't do a version check at all in check_gcc_version. > >> > >> Perhaps I should add to that "(previously we errored out)". > > > > Well, we're still erroring out, but with a 0 exit code. So I must be > > missing something. > > 0 is not an error, right? Hum, right. I think what confused me is that "exit 0" doesn't abort the build. It exists the sub-shell started by make, with a zero exit code, so make continues the build. I was thinking it would abort the build completely, which is why I was confused. Thanks, Thomas
diff --git a/toolchain/helpers.mk b/toolchain/helpers.mk index 3991bc1..72e7292 100644 --- a/toolchain/helpers.mk +++ b/toolchain/helpers.mk @@ -145,8 +145,7 @@ check_kernel_headers_version = \ check_gcc_version = \ expected_version="$(strip $2)" ; \ if [ -z "$${expected_version}" ]; then \ - printf "Internal error, gcc version unknown (no GCC_AT_LEAST_X_Y selected)\n"; \ - exit 1 ; \ + exit 0 ; \ fi; \ real_version=`$(1) --version | sed -r -e '1!d; s/^[^)]+\) ([^[:space:]]+).*/\1/;'` ; \ if [[ ! "$${real_version}" =~ ^$${expected_version}\. ]] ; then \ diff --git a/toolchain/toolchain-external/Config.in b/toolchain/toolchain-external/Config.in index 5324599..7a3c504 100644 --- a/toolchain/toolchain-external/Config.in +++ b/toolchain/toolchain-external/Config.in @@ -712,6 +712,17 @@ config BR2_TOOLCHAIN_EXTERNAL_GCC_4_3 bool "4.3.x" select BR2_TOOLCHAIN_GCC_AT_LEAST_4_3 +config BR2_TOOLCHAIN_EXTERNAL_GCC_OLD + bool "older" + help + Use this option if your GCC version is older than any of the + above. + + Note that the Buildroot community doesn't do any testing with + such old toolchains. Some packages may fail to build in + surprising ways, or the generated root filesystem may not + work at all. Use such old toolchains at your own risk. + endchoice choice
We currently support gcc as old as 4.3. However, Buildroot works perfectly well with even older gcc versions (tested with 4.1). So we can add an option BR2_TOOLCHAIN_EXTERNAL_GCC_OLD to support that. The help text of this option is written with plenty of discouragement. We use _OLD and not something like _PRE_4_3, because at some point we will likely remove the 4.3 option and what would then require a name change. We don't set any _AT_LEAST option in this case because it's no use - there is no lower bound on the version in this case. We therefore leave BR2_TOOLCHAIN_GCC_AT_LEAST empty (the implicit default). When it is empty, we don't do a version check at all in check_gcc_version. Indeed, it makes perfect sense to not set any BR2_TOOLCHAIN_GCC_AT_LEAST option. Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be> --- This patch conflicts with the external toolchain rework - I'll rebase the one which "looses" in the first-to-be-committed contest. We should also remove the 4.3 and 4.4 options, because they are currently unused and also untested in the autobuilders. I think something similar should be done for kernel headers. --- toolchain/helpers.mk | 3 +-- toolchain/toolchain-external/Config.in | 11 +++++++++++ 2 files changed, 12 insertions(+), 2 deletions(-)