1603273 - Mozilla-Build could expose cargo on $PATH

Status: RESOLVED FIXED

(Firefox Build System :: Toolchains, enhancement, P3)

Description

[Marc Streckfuß [:MeFisto94]]

When using Mozilla-Build Environment on Windows and wanting to contribute to servo (or generally using it as well-set-up environment), one has most tools available (git, hg, ...), but the cargo toolchain is not exposed, even though it is installed into ~/.rustup/toolchains/stable-x86_64-pc-windows-msvc/bin/cargo.

Now I know that there are multiple toolchains available (nightly, older versions), thus I don't know how we could always have the latest version available, but we could maybe symlink it into the mozilla-build main directory?

I get that this issue is non-trivial, because this would have to be done from mach, which should be unrelated from such mozilla-build changes, but maybe someone has some input on that?

Comment 1

[Marc Streckfuß [:MeFisto94]]

I revise that statement:
Couldn't we just append ~/.cargo/bin to the path, or does this have undesired side effects?

Comment 2

[Marc Streckfuß [:MeFisto94]]

Attachment: cargo.patch

Keep https://searchfox.org/mozilla-central/rev/8d04c3f5332d470eeae5aa3dc0ed132359a339c1/build/moz.configure/toolchain.configure#722 in mind, this would add the path explictly (i.e. it's twice on the searchpath for tools), but I don't think it's a problem.

Comment 3

[Marc Streckfuß [:MeFisto94]]

Attachment: cargo.patch

The old patch relied on expanding ~, which at that stage is mounted onto msys' home folder, not the real windows user home. Thus it had a wrong path and didn't work.

Comment 4

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 9117454 [details] [diff] [review]
cargo.patch

Review of attachment 9117454 [details] [diff] [review]:
-----------------------------------------------------------------

Ryan, does this change seem reasonable?

Comment 5

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9117454 [details] [diff] [review]
cargo.patch

Sounds fine to me assuming this doesn't somehow interact badly with the Firefox build system (which I'm not qualified to answer on). That said, glob is the maintainer of MozillaBuild now, so redirecting this his way.

Comment 6

[:glob ✱]

Comment on attachment 9117454 [details] [diff] [review]
cargo.patch

Review of attachment 9117454 [details] [diff] [review]:
-----------------------------------------------------------------

lgtm, r=glob

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/mozilla-build/rev/3b8c537ca3c8
Expose Cargo on $PATH, r=glob

Comment 8

[Mike Hommey [:glandium]]

I'm not sure we should keep this. MozillaBuild doesn't come with cargo, and installing rust explicitly gives instructions to add the cargo directory to the path (or does it itself, I'm not sure).

Comment 9

[Mitchell Hentges [:mhentges] 🦀]

installing rust explicitly gives instructions to add the cargo directory to the path (or does it itself, I'm not sure).

It automatically adds it to the Windows "User" Path environment variable, but not the "System" Path environment variable.
Either way, today's MozillaBuild doesn't have cargo on the path. I'm guessing that's because we're not including the User's Path configurations?

To be fully unambiguous, even if we back out this bug's patch, I think that cargo should be on the PATH within MozillaBuild.

Comment 10

[Mike Hommey [:glandium]]

Either way, today's MozillaBuild doesn't have cargo on the path. I'm guessing that's because we're not including the User's Path configurations?

The patch landed after the 3.3 release.

Comment 11

[Mitchell Hentges [:mhentges] 🦀]

The patch landed after the 3.3 release.

True, but also, if starting a new cmd populates the PATH with both System and User path entries, then why are user path entries missing from MozillaBuild?