rust: Ensure cargo executable path is fetched for custom targets when bootstrapping rust compiler

[Yashinde145]

When rust compiler is bootstrapped and built in custom sources, for example poky,
the build fails to fetch cargo exe path and gives error as follows.

This PR changes fix the error.

====================================
 ERROR: test_cargoflags (bootstrap_test.BuildBootstrap)
-------------------------------------------------------------
  Traceback (most recent call last):
  File "/home/build-st/tmp/work/cortexa57-poky-linux/rust/1.74.1/rustc-1.74.1-src/src/bootstrap/bootstrap_test.py", line 157, in test_cargoflags
     args, _ = self.build_args(env={"CARGOFLAGS": "--timings"})
  File "/home/build-st/tmp/work/cortexa57-poky-linux/rust/1.74.1/rustc-1.74.1-src/src/bootstrap/bootstrap_test.py", line 154, in build_args
     return build.build_bootstrap_cmd(env), env
  File "/home/build-st/tmp/work/cortexa57-poky-linux/rust/1.74.1/rustc-1.74.1-src/src/bootstrap/bootstrap.py", line 960, in build_bootstrap_cmd
    raise Exception("no cargo executable found at `{}`".format(
Exception: no cargo executable found at /home/build-st/tmp/work/cortexa57-poky-linux/rust/1.74.1/rustc-1.74.1-src/build/x86_64-unknown-linux-gnu/stage0/bin/cargo

[rustbot]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @clubby789 (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

• @rustbot author: the review is finished, PR author should check the comments and take action accordingly
• @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

[Yashinde145]

@clubby789 , can you please review the changes?

[Yashinde145]

@clubby789, gentle reminder to review the changes.

[onur-ozkan]

We don't include custom paths in the source code directly. Why not adding it to "PATH" in your environment?

[Yashinde145]

Thanks for looking into the PR.

I thought of including the path set by RUST_TARGET_PATH + "rust-snapshot/bin/cargo" and adding it as cargo bin path.

Why not adding it to "PATH" in your environment?

While doing rustc build I don't know the path of cargo bin beforehand and the build gives error as mentioned in #122069 (comment)

Should I do any changes in the above patch?

[clubby789]

If you know that Cargo will be at $RUST_TARGET_PATH + "rust-snapshot/bin/cargo", can't you just add that to your PATH in your build process? Or set cargo in config.toml to that

[Yashinde145]

Actually, I am running the build process with a wrapper script. $RUST_TARGET_PATH is set after the rust sources are generated and while build is running. If I add it in the PATH before build, it would fail as $RUST_TARGET_PATH is undefined path. Still, I will give it a try and check as you mentioned and get back.

[onur-ozkan]

I am quite sure there are plenty of ways to solve this problem without adding hard-coded custom paths in the source code. If not, then your build flow is obviously doing something very wrong. We can't accept this change to upstream.

[Yashinde145]

Does the following changes look good to you?

Reverting the changes on line no 978. Then using that cargo_bin_path value to handle cases where cargo bin is not found(as in my case, since this is been handled at very last else case).

diff --git a/src/bootstrap/bootstrap.py b/src/bootstrap/bootstrap.py
index 0d604c0d3e5..e7224ab45b8 100644
--- a/src/bootstrap/bootstrap.py
+++ b/src/bootstrap/bootstrap.py
@@ -977,9 +977,15 @@ class RustBuild(object):
 
         env["PATH"] = os.path.join(self.bin_root(), "bin") + \
             os.pathsep + env["PATH"]
+        cargo_bin_path = os.path.join(self.bin_root(), "bin") + \
+            os.pathsep + env["PATH"]
         if not os.path.isfile(self.cargo()):
-            raise Exception("no cargo executable found at `{}`".format(
+            if cargo_bin_path is None:
+                raise Exception("no cargo executable found at `{}`".format(
                 self.cargo()))
+            else:
+                cargo_bin_path = os.getenv("RUST_TARGET_PATH") + "rust-snapshot/bin/cargo"
+                env["PATH"] = os.path.dirname(cargo_bin_path) + os.pathsep + env["PATH"]
         args = [self.cargo(), "build", "--manifest-path",
                 os.path.join(self.rust_root, "src/bootstrap/Cargo.toml")]
         args.extend("--verbose" for _ in range(self.verbose))

[clubby789]

As mentioned earlier, we can't accept hard-coded paths for specific projects like this rust-snapshot path. This really sounds like this would be better solved within your build flow

[onur-ozkan]

@clubby789 , can you please review the changes?
@clubby789, gentle reminder to review the changes.

Just so you know, there are other PRs waiting for reviews too. We aim to review PRs promptly, but depending on the developer's workload it may take up to 2 weeks for the initial review. You don't need to ping the developers beforehand, but if you do, one ping will be enough.

[Yashinde145]

Just so you know, there are other PRs waiting for reviews too. We aim to review PRs promptly, but depending on the developer's workload it may take up to 2 weeks for the initial review. You don't need to ping the developers beforehand, but if you do, one ping will be enough.

Thanks for the information. I didn't wanted to bother but just remind in case of a miss for reviewing PR. I will follow the process as mentioned.

[Dylan-DPC]

@Yashinde145 any updates on this? thanks

[Yashinde145]

@Dylan-DPC , I will close this PR.
Thanks.