`mach vendor rust` has problems on Windows
Categories
(Firefox Build System :: General, defect, P3)
Tracking
(firefox82 fixed)
Tracking | Status | |
---|---|---|
firefox82 | --- | fixed |
People
(Reporter: n.nethercote, Assigned: Gankra)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
(deleted),
text/x-phabricator-request
|
Details |
Bug 1619788 was a problem, but it just got fixed.
Also, symlinks aren't handled correctly. There's a Cargo issue on file. This seems to be at root a minGW/Windows filesystem issue, though it might be possible for Cargo to work around it?
Reporter | ||
Comment 1•4 years ago
|
||
Note: once mach vendor rust
works fully on Windows, we should remove the note in build/docs/rust.rst
about it not working fully on Windows.
Reporter | ||
Updated•4 years ago
|
Comment 2•4 years ago
|
||
Not triaging this since according to comment 0 this appears to just be a cargo bug.
Reporter | ||
Comment 3•4 years ago
|
||
If I run ./mach vendor rust
on an unchanged m-c, on Linux and Mac I get no changes. But on Windows I get huge numbers of changes:
966 files changed, 281660 insertions(+), 281919 deletions(-)
But most of those changes are just changing line endings of files within third_party/rust/
. Ricky, do you know how to fix that?
Comment 4•4 years ago
|
||
This smells like cargo's libgit2 doing crlf conversions it shouldn't be doing. Which could be because of the global git configuration.
Comment 5•4 years ago
|
||
@njn confirmed on Matrix that this is related to core.autocrlf being true on his machine. It's the kinda default when you install Git for Windows, but you can actually choose something else in the installer. What this also means is that this should be reproducible on other OSes by setting core.eol to crlf. It seems to me cargo should preserve whatever end-of-lines are in the original crates.
Reporter | ||
Comment 6•4 years ago
|
||
glandium suggested:
- Run
git config --global core.autocrlf false
- Delete
~/.cargo/git
and ~/.cargo/registry` - Re-run
./mach vendor rust
That gave much better results. The line endings problem went away. Six dot files were erroneously modified, though:
third_party/rust/lucet-wasi/examples/.gitignore
was deletedthird_party/rust/lucet-wasi/.cargo-checksum.json
was modified to account for the above deletionthird_party/rust/spirv-cross-internal/src/vendor/SPIRV-Cross/.clang-format
was deletedthird_party/rust/spirv-cross-internal/src/vendor/SPIRV-Cross/.gitignore
was deletedthird_party/rust/spirv-cross-internal/src/vendor/SPIRV-Cross/.travis.yml
was deletedthird_party/rust/spirv-cross-internal/.cargo-checksum.json
was modified to account for the above deletions.
The Cargo in my PATH is 1.44.1.
So, two questions:
- Is the line endings thing affecting other people?
- How to fix the dot file problems?
Reporter | ||
Comment 7•4 years ago
|
||
gankra: are you interested in looking into this? I'd start just by running ./mach vendor rust
on an unchanged m-c repository and see what happens, whether you get the line-ending weirdness that I did. Beyond that, for the dot file issues, I guess you'd need to work out exactly what cargo commands are being run by ./mach vendor rust
, and see if they're working as they should. Bug 1644371 also dealt with dot file issues, and may be relevant.
Updated•4 years ago
|
Assignee | ||
Comment 8•4 years ago
|
||
I cleaned off the dust on my windows dev machine, updated the OS, pulled central (538532:f0ac79e1ed53), ran rustup, reconfigured, rebuilt, and then ran mach vendor rust
.
It ran for a bit and at the end, hg diff
shows no changes to my project. So, at least on my laptop, everything seems to be working perfectly? My copy of mozillabuild is a bit old, dating back to May 3, 2018 (VERSION 3.2). Maybe this is a more recent regression? There's only one release after it (3.3), so idk. Could also have been fixed upstream in cargo or something?
Reporter | ||
Comment 9•4 years ago
|
||
Updating mozbuild sounds wise, can you try that?
Also, what is the output of git config core.autocrlf
on your machine?
Finally, you could try making a change (e.g. add a fake dependency to a crate, e.g. add a dependency on bitflags 1.0 to modules/libpref/parser/Cargo.toml
) and see if it does the same thing as a Linux or Mac machine.
Assignee | ||
Comment 10•4 years ago
|
||
Still not getting anything bizarre on my windows machine.
Things done:
- Updated to mozillabuild 3.3 (following the instructions on its wiki page)
- Reran
mach bootstrap
stuff / clobbered git config core.autocrlf
was false (possible I once set this in the past 3 years)- note: I'm not using cinnibar for gecko on windows, although of course git is still involved for cargo
- Added a bitflags 1.0.0 dep to libpref/parser
- Added the same dep on macos
- ran
mach vendor rust
result: both machines produced the same trivial/clean changes
possibly you should run a vendor yourself to see if things were magically fixed in the last few weeks?
Reporter | ||
Comment 11•4 years ago
|
||
I'm still getting the changes to the six dot files mentioned in comment 12. Hmm.
Assignee | ||
Comment 12•4 years ago
|
||
Update: I built a new work machine, and my basically pristine new install of windows + mozillabuild reproduces njn's issues. Investigating now.
Assignee | ||
Comment 13•4 years ago
|
||
Note: the windows git installer prompts you to set core.autocrlf
, but the default is true
(as noted above, we prefer false
).
Assignee | ||
Comment 14•4 years ago
|
||
ok so the non-autocrlf issue is a bug in cargo vendor
which was fixed in Rust 1.45, the latest stable release as of 3 days ago.
So maybe we add a version check to cargo vendor
and the is fixed?
Assignee | ||
Comment 15•4 years ago
|
||
Ok final summary of this bug. It is in fact two bugs:
cargo vendor
behaves poorly whengit config core.autocrlf=true
, needlessly rewriting the vendored library's newlinescargo vendor
in versions of rust earlier than 1.45 (the latest stable) will deterministically but nonsensically delete and modify some files when run on windows (msys?) (for historical record, you can reproduce this on gecko commit 540166:426eecf976c0).
Issue 2 ends up being fairly straightforward: at most we just need to add a version check to mach vendor rust
. We could make it conditional on windows, but honestly we might as well make it universal for simplicity, since it's the latest stable release, and vendor
is an ~advanced developer feature.
That said, it would be nice to know why it was fixed in 1.45. Nothing in the cargo release notes suggests an answer.
Issue 1 is more subtle. Some observations:
- If you have not installed git, cargo's libgit2 will default to
autocrlf=false
(good!) (presumably it's the absence of standard git config files more than anything) - If you do install git with the official wizard, it will prompt you for the value you want, but defaults to
autocrlf=true
(bad!)
Glandium asserts that true
is actually a good value for end user development on windows (no opinion), and therefore believes we should regard this as a bug in cargo/libgit2/mach-vendor-rust to set/ignore the value properly.
Alternatively, we can regard this as just a "bad configuration" for firefox development, and teach mach vcs-setup
and/or mach bootstrap
to prompt you to reconfigure it.
Assignee | ||
Updated•4 years ago
|
Comment 16•4 years ago
|
||
Glandium asserts that true is actually a good value for end user development on windows
In general. Not too sure about Firefox specifically. But there are two problems with tweaking the git configuration:
- we can't change the config for cargo vendor only, any change will affect all uses of git on the machine.
- mach vcs-setup/mach bootstrap tweak the configuration of the tool used to clone and update the Firefox tree, but in the case of cargo vendor, the problem is always with the git configuration, whether Firefox is cloned from git or mercurial.
Comment 17•4 years ago
|
||
+1 to autocrlf=true
being a good thing that we don't want to start forbidding or discouraging. It's especially useful for cross-platform, heavily distributed projects where Windows development occurs -- i.e., exactly what Firefox is.
Really it's entirely possible to have a perfectly sensible Git workflow on Windows with either value of autocrlf
. cargo vendor
should be aware of this and support any vaue autocrlf
might have, either by default or via a command-line option that we can use.
Reporter | ||
Comment 18•4 years ago
|
||
Could mach vendor
temporarily set autocrlf
to false and then set it back if necessary?
Assignee | ||
Comment 19•4 years ago
|
||
I have submitted a fix for issue 1 upstream: https://github.com/rust-lang/cargo/pull/8523
Assignee | ||
Comment 20•4 years ago
|
||
The upstream fix has landed, so if you run a recent rust/cargo nightly, all of the issues reported here should be fixed. So the only remaining question is if we wish to put a check in to cargo vendor
that you're running a recently nightly (1.47.0, 2020-07-26 should suffice), or if it's just a documentation issue?
Comment 21•4 years ago
|
||
(In reply to Alexis Beingessner [:Gankra] from comment #20)
The upstream fix has landed, so if you run a recent rust/cargo nightly, all of the issues reported here should be fixed. So the only remaining question is if we wish to put a check in to
cargo vendor
that you're running a recently nightly (1.47.0, 2020-07-26 should suffice), or if it's just a documentation issue?
I am not wild about the idea of requiring nightly rust/cargo to effectively develop Firefox, but maybe if the check is Windows-only, and we made it so mach vendor
would transparently use the nightly version, that would be OK?
Assignee | ||
Comment 22•4 years ago
|
||
Comment 23•4 years ago
|
||
There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:Gankra, could you have a look please?
For more information, please visit auto_nag documentation.
Comment 24•4 years ago
|
||
Comment 25•4 years ago
|
||
bugherder |
Assignee | ||
Updated•4 years ago
|
Description
•