Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bootstrap/configure.py: targets with a dot in the name prevent finding the correct target section in config.toml.example #130602

Closed
kiike opened this issue Sep 20, 2024 · 4 comments · Fixed by #132635
Labels
C-bug Category: This is a bug. E-help-wanted Call for participation: Help is requested to fix this issue. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Comments

@kiike
Copy link
Contributor

kiike commented Sep 20, 2024

When passing --set arguments to the configure script, the targets will be incorrectly detected when they include a dot in the name. An example would be thumbv8m.main. Thus, when using a command line such as

./configure.py --target=thumb8vm.main-none-eabi --set=target.thumbv8m.main-none-eabi.linker=/path/to/linker

this error will be triggered:

configure: processing command line
configure: 
configure: build.target         := ['thumb8vm.main-none-eabi']
configure: target.thumbv8m.main-none-eabi.linker := /path/to/linker  ...
configure: build.configure-args := ['--target=thumb8vm.main-none-eabi', '--set=ta ...
Traceback (most recent call last):
  File "/home/kiike/projects/rust/src/bootstrap/./configure.py", line 469, in <module>
    configure_section(targets[target], section_config[target])
  File "/home/kiike/projects/rust/src/bootstrap/./configure.py", line 459, in configure_section
    raise RuntimeError("failed to find config line for {}".format(key))
RuntimeError: failed to find config line for main-none-eabi

This is related to #97263 and #105920. I think there might be three different solutions:

  • overwrite a wrongly-detected target:
diff --git a/src/bootstrap/configure.py b/src/bootstrap/configure.py
index 49d564642bd..e4e1f15e80b 100755
--- a/src/bootstrap/configure.py
+++ b/src/bootstrap/configure.py
@@ -297,6 +297,12 @@ def set(key, value, config):
 
     arr = config
     parts = key.split('.')
+
+    if len(parts) > 2 and parts[1] == "thumbv8m" and parts[2].endswith("none-eabi"):
+        print("Old `parts`:", parts)
+        parts[1] += "." + parts.pop(2)
+        print("New `parts`:", parts)
+
     for i, part in enumerate(parts):
         if i == len(parts) - 1:
             if is_value_list(part) and isinstance(value, str):

Output:

$ python3 ./configure.py --target=thumb8vm.main-none-eabi --set=target.thumbv8m.main-none-eabi.linker=/path/to/linker
configure: processing command line
configure: 
configure: build.configure-args := ['--target=thumb8vm.main-none-eabi', '--set=ta ...
configure: build.target         := ['thumb8vm.main-none-eabi']
configure: target.thumbv8m.main-none-eabi.linker := /path/to/linker
Old `parts`: ['target', 'thumbv8m', 'main-none-eabi', 'linker']
New `parts`: ['target', 'thumbv8m.main-none-eabi', 'linker']
configure: profile              := dist
configure: 
configure: writing `config.toml` in current directory
configure: 
configure: run `python /home/kiike/projects/rust/x.py --help`
  • implement some kind of quoting, so that dots will only be interpreted as separators when not preceded by a character, eg. a backslash:
diff --git a/src/bootstrap/configure.py b/src/bootstrap/configure.py
index 49d564642bd..1945945a990 100755
--- a/src/bootstrap/configure.py
+++ b/src/bootstrap/configure.py
@@ -5,6 +5,7 @@
 from __future__ import absolute_import, division, print_function
 import sys
 import os
+import re
 rust_dir = os.path.dirname(os.path.abspath(__file__))
 rust_dir = os.path.dirname(rust_dir)
 rust_dir = os.path.dirname(rust_dir)
@@ -296,8 +297,12 @@ def set(key, value, config):
         p(s[:70] + " ...")
 
     arr = config
-    parts = key.split('.')
+    parts = re.split(r"(?<!\\)\.", key)
     for i, part in enumerate(parts):
+        if re.search(r"\\\.", part):
+            print("Old part:", part)
+            part = re.sub(r"\\\.", ".", part)
+            print("New part:", part)
         if i == len(parts) - 1:
             if is_value_list(part) and isinstance(value, str):
                 value = value.split(',')

Resulting in:

python3 ./configure.py --target=thumb8vm.main-none-eabi --set=target.thumbv8m\\.main-none-eabi.linker=/path/to/linker
configure: processing command line
configure: 
configure: build.configure-args := ['--target=thumb8vm.main-none-eabi', '--set=ta' ...
configure: build.target         := ['thumb8vm.main-none-eabi']
configure: target.thumbv8m\.main-none-eabi.linker := /path/to/linker
Old part: thumbv8m\.main-none-eabi
New part: thumbv8m.main-none-eabi
configure: profile              := dist
configure: 
configure: writing `config.toml` in current directory
configure: 
configure: run `python /home/kiike/projects/rust/x.py --help`
  • provide the splitting logic with a list of items that can't be split. In other words, split around the dots, except when the dot is between "thumv8m" and "main", etc.

I'm wiling to work on this issue but I'd need to know what approach would be preferred.

@kiike kiike added the C-bug Category: This is a bug. label Sep 20, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Sep 20, 2024
@kiike
Copy link
Contributor Author

kiike commented Sep 20, 2024

Related: https://rust-lang.zulipchat.com/#narrow/stream/326414-t-infra.2Fbootstrap/topic/specifying.20linker.20for.20cross-compiling

If this is out-of-scope for configure.py and manual editing of the config.toml is required, I'd like to know too.

@jieyouxu jieyouxu added the O-Arm Target: 32-bit Arm processors (armv6, armv7, thumb...), including 64-bit Arm in AArch32 state label Sep 20, 2024
@kiike
Copy link
Contributor Author

kiike commented Sep 24, 2024

@rustbot label T-bootstrap

@rustbot rustbot added the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Sep 24, 2024
@onur-ozkan onur-ozkan added E-help-wanted Call for participation: Help is requested to fix this issue. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. and removed O-Arm Target: 32-bit Arm processors (armv6, armv7, thumb...), including 64-bit Arm in AArch32 state needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Sep 25, 2024
@kiike
Copy link
Contributor Author

kiike commented Oct 7, 2024

@onur-ozkan, I'd like to work on this. As the lead for the bootstrap phase, could you provide a couple hints on how you envision the change, i.e. the approach you would like this to take? I provided some patches in the first comment to show a couple of ways I could tackle this issue.

@onur-ozkan
Copy link
Member

@onur-ozkan, I'd like to work on this. As the lead for the bootstrap phase, could you provide a couple hints on how you envision the change, i.e. the approach you would like this to take? I provided some patches in the first comment to show a couple of ways I could tackle this issue.

Hi, sorry for the late response. I wouldn't add any special logic for target names but would instead solve this more generally by handling dots within quotes (e.g., making it so that --set="target.thumbv8m.main-none-eabi.linker=/path/to/linker" works without issues).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. E-help-wanted Call for participation: Help is requested to fix this issue. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants