diff mbox series

support/bootlin-toolchains: don't depend on non-existent gcc version

Message ID 20231007205749.443600-1-yann.morin.1998@free.fr
State Changes Requested
Headers show
Series support/bootlin-toolchains: don't depend on non-existent gcc version | expand

Commit Message

Yann E. MORIN Oct. 7, 2023, 8:57 p.m. UTC
Commit a0d2a5cfec0a (support/scripts/gen-bootlin-toolchains: generate
BR2_ARCH_NEEDS_GCC_AT_LEAST_X guard) added a negative dependency to the
gcc version required by the configured architecture.

However, when the toolchain is using the latest gcc version currently
known to Buildroot, this generates a dependency on a non-existing gcc
version. For example, a toolchain using gcc 13, the most recent version
currently known to Buildroot, this would generate a dependency against
BR2_ARCH_NEEDS_GCC_AT_LEAST_14, which does not exist yet.

This dependency is in practice a no-op, because the symbol is missing,
so Kconfig evaluates it to false, and since it is negated, the
dependency is fulfilled. Still, this is spurious and semantically
incorrect.

We fix that by extracting the most recent gcc version that an
architecture may require, and use that to decide whether the guard is
needed for the toolchain.

Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
Cc: Vincent Fazio <vfazio@xes-inc.com>
---
 support/scripts/gen-bootlin-toolchains | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

Comments

Arnout Vandecappelle Oct. 8, 2023, 9:32 a.m. UTC | #1
On 07/10/2023 22:57, Yann E. MORIN wrote:
> Commit a0d2a5cfec0a (support/scripts/gen-bootlin-toolchains: generate
> BR2_ARCH_NEEDS_GCC_AT_LEAST_X guard) added a negative dependency to the
> gcc version required by the configured architecture.
> 
> However, when the toolchain is using the latest gcc version currently
> known to Buildroot, this generates a dependency on a non-existing gcc
> version. For example, a toolchain using gcc 13, the most recent version
> currently known to Buildroot, this would generate a dependency against
> BR2_ARCH_NEEDS_GCC_AT_LEAST_14, which does not exist yet.

  That means, however, that if we add GCC 14, we need to remember to add the 
condition to the Bootlin toolchains again... Of course, that can easily be 
solved by regenerating them, but we don't currently do that when adding a new 
GCC version.

  So really, I would feel more comfortable just adding the symbols for GCC 14 
support already.

  Regards,
  Arnout

> 
> This dependency is in practice a no-op, because the symbol is missing,
> so Kconfig evaluates it to false, and since it is negated, the
> dependency is fulfilled. Still, this is spurious and semantically
> incorrect.
> 
> We fix that by extracting the most recent gcc version that an
> architecture may require, and use that to decide whether the guard is
> needed for the toolchain.
> 
> Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
> Cc: Vincent Fazio <vfazio@xes-inc.com>
> ---
>   support/scripts/gen-bootlin-toolchains | 19 ++++++++++++++++---
>   1 file changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/support/scripts/gen-bootlin-toolchains b/support/scripts/gen-bootlin-toolchains
> index 4344221213..3b5f65515d 100755
> --- a/support/scripts/gen-bootlin-toolchains
> +++ b/support/scripts/gen-bootlin-toolchains
> @@ -305,7 +305,7 @@ class Toolchain:
>           return os.path.join(BASE_URL, self.arch, "fragments",
>                               self.fname_prefix + ".frag")
>   
> -    def gen_config_in_options(self, f):
> +    def gen_config_in_options(self, f, latest_gcc):
>           f.write("config %s\n" % self.option_name)
>           f.write("\tbool \"%s %s %s %s\"\n" %
>                   (self.arch, self.libc, self.variant, self.version))
> @@ -339,7 +339,8 @@ class Toolchain:
>                   assert m, "Cannot get gcc version for toolchain %s" % self.fname_prefix
>                   selects.append("BR2_TOOLCHAIN_GCC_AT_LEAST_%s" % m[1])
>                   # respect the GCC requirement for the selected CPU/arch tuning
> -                depends.append("!BR2_ARCH_NEEDS_GCC_AT_LEAST_%s" % str(int(m[1]) + 1))
> +                if int(m[1]) + 1 <= latest_gcc:
> +                    depends.append("!BR2_ARCH_NEEDS_GCC_AT_LEAST_%s" % str(int(m[1]) + 1))
>   
>               # kernel headers version
>               if frag.startswith("BR2_TOOLCHAIN_EXTERNAL_HEADERS_"):
> @@ -462,6 +463,17 @@ class Toolchain:
>               (self.arch, self.libc, self.variant, self.version, self.option_name)
>   
>   
> +def get_latest_gcc():
> +    match = "config BR2_ARCH_NEEDS_GCC_AT_LEAST_"
> +    with open("arch/Config.in", "r") as f:
> +        latest = sorted([
> +            int(line[len(match):].strip().split("_")[0])
> +            for line in f.readlines()
> +            if line.startswith(match)
> +        ])[-1]
> +    return latest
> +
> +
>   def get_toolchains():
>       toolchains = list()
>       for arch, details in arches.items():
> @@ -494,6 +506,7 @@ def get_toolchains():
>   
>   
>   def gen_config_in_options(toolchains, fpath):
> +    latest_gcc = get_latest_gcc()
>       with open(fpath, "w") as f:
>           f.write(AUTOGENERATED_COMMENT)
>   
> @@ -520,7 +533,7 @@ def gen_config_in_options(toolchains, fpath):
>           f.write("\tprompt \"Bootlin toolchain variant\"\n")
>   
>           for toolchain in toolchains:
> -            toolchain.gen_config_in_options(f)
> +            toolchain.gen_config_in_options(f, latest_gcc)
>   
>           f.write("endchoice\n")
>           f.write("endif\n")
Thomas Petazzoni Nov. 8, 2023, 10:05 p.m. UTC | #2
On Sun, 8 Oct 2023 11:32:38 +0200
Arnout Vandecappelle via buildroot <buildroot@buildroot.org> wrote:

> > However, when the toolchain is using the latest gcc version currently
> > known to Buildroot, this generates a dependency on a non-existing gcc
> > version. For example, a toolchain using gcc 13, the most recent version
> > currently known to Buildroot, this would generate a dependency against
> > BR2_ARCH_NEEDS_GCC_AT_LEAST_14, which does not exist yet.  
> 
>   That means, however, that if we add GCC 14, we need to remember to add the 
> condition to the Bootlin toolchains again... Of course, that can easily be 
> solved by regenerating them, but we don't currently do that when adding a new 
> GCC version.
> 
>   So really, I would feel more comfortable just adding the symbols for GCC 14 
> support already.

Yes, I think I agree too. Otherwise, we will never remember that we
need to re-generate the Bootlin toolchain files when the next
BR2_ARCH_NEEDS_GCC_AT_LEAST_xyz. And actually, it kind of makes sense
to have the BR2_ARCH_NEEDS_GCC_AT_LEAST_14 option available early on:
gcc 13.x toolchains should indeed say that they are not relevant if the
arch needs gcc 14. gcc 14.x may not exist yet, but the gcc 13.x
toolchain exists, and should say "I'm not good if your arch is so new
that it needs a new compiler than the one I provide".

Best regards,

Thomas
diff mbox series

Patch

diff --git a/support/scripts/gen-bootlin-toolchains b/support/scripts/gen-bootlin-toolchains
index 4344221213..3b5f65515d 100755
--- a/support/scripts/gen-bootlin-toolchains
+++ b/support/scripts/gen-bootlin-toolchains
@@ -305,7 +305,7 @@  class Toolchain:
         return os.path.join(BASE_URL, self.arch, "fragments",
                             self.fname_prefix + ".frag")
 
-    def gen_config_in_options(self, f):
+    def gen_config_in_options(self, f, latest_gcc):
         f.write("config %s\n" % self.option_name)
         f.write("\tbool \"%s %s %s %s\"\n" %
                 (self.arch, self.libc, self.variant, self.version))
@@ -339,7 +339,8 @@  class Toolchain:
                 assert m, "Cannot get gcc version for toolchain %s" % self.fname_prefix
                 selects.append("BR2_TOOLCHAIN_GCC_AT_LEAST_%s" % m[1])
                 # respect the GCC requirement for the selected CPU/arch tuning
-                depends.append("!BR2_ARCH_NEEDS_GCC_AT_LEAST_%s" % str(int(m[1]) + 1))
+                if int(m[1]) + 1 <= latest_gcc:
+                    depends.append("!BR2_ARCH_NEEDS_GCC_AT_LEAST_%s" % str(int(m[1]) + 1))
 
             # kernel headers version
             if frag.startswith("BR2_TOOLCHAIN_EXTERNAL_HEADERS_"):
@@ -462,6 +463,17 @@  class Toolchain:
             (self.arch, self.libc, self.variant, self.version, self.option_name)
 
 
+def get_latest_gcc():
+    match = "config BR2_ARCH_NEEDS_GCC_AT_LEAST_"
+    with open("arch/Config.in", "r") as f:
+        latest = sorted([
+            int(line[len(match):].strip().split("_")[0])
+            for line in f.readlines()
+            if line.startswith(match)
+        ])[-1]
+    return latest
+
+
 def get_toolchains():
     toolchains = list()
     for arch, details in arches.items():
@@ -494,6 +506,7 @@  def get_toolchains():
 
 
 def gen_config_in_options(toolchains, fpath):
+    latest_gcc = get_latest_gcc()
     with open(fpath, "w") as f:
         f.write(AUTOGENERATED_COMMENT)
 
@@ -520,7 +533,7 @@  def gen_config_in_options(toolchains, fpath):
         f.write("\tprompt \"Bootlin toolchain variant\"\n")
 
         for toolchain in toolchains:
-            toolchain.gen_config_in_options(f)
+            toolchain.gen_config_in_options(f, latest_gcc)
 
         f.write("endchoice\n")
         f.write("endif\n")