1581248 - mach vendor rust fails on Windows "Cannot mix UNC and non-UNC paths" since 1.37.0

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

Using the builtin cargo vendor in 1.37.0 causes this error on Windows:

$ ./mach vendor rust
    Updating crates.io index
    Updating git repository `https://github.com/alexcrichton/mio-named-pipes`
    Updating git repository `https://github.com/NikVolf/tokio-named-pipes`
Error running mach:
 
    ['vendor', 'rust']

The error occurred in code that was called by the mach command. This is either
a bug in the called code itself or in the way that mach is calling it.
You can invoke |./mach busted| to check if this issue is already on file. If it
isn't, please use |./mach busted file| to report it. If |./mach busted| is
misbehaving, you can also inspect the dependencies of bug 1543241.

If filing a bug, please include the full output of mach, including this error
message.

The details of the failure are as follows:

ValueError: Cannot mix UNC and non-UNC paths (//?/c:/mozilla-source/mozilla-central/third_party/rust and c:/mozilla-source/mozilla-central)

  File "c:\mozilla-source\mozilla-central\python/mozbuild/mozbuild/mach_commands.py", line 1089, in vendor_rust
    vendor_command.vendor(**kwargs)
  File "c:\mozilla-source\mozilla-central\python/mozbuild\mozbuild\vendor_rust.py", line 386, in vendor
    mozpath.normsep(os.path.normcase(self.topsrcdir)),
  File "c:\mozilla-source\mozilla-central\python/mozbuild\mozpack\path.py", line 41, in relpath
    rel = normsep(os.path.relpath(path, start))
  File "c:\mozilla-build\python\lib\ntpath.py", line 522, in relpath
    % (path, start))

I think this might be down to a Python relpath() bug, or missing feature; I believe it should be possible to work with those paths. Python 3 gives a different error but is still confused (ValueError: path is on mount 'C:', start on mount '\\\\?\\C:').

It shows up now because cargo-vendor 0.1.23 used to output:

directory = "c:\\mozilla-source\\mozilla-central\\third_party/rust/"

while cargo 1.37.0's vendor subcommand now does:

directory = "\\\\?\\C:\\mozilla-source\\mozilla-central\\third_party\\rust"

Probably because something is calling std::fs::canonicalize, which gives the full NT path (see this issue).

Comment 1

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

It looks like this was an issue in cargo-vendor here but fixed before merging, that must have gotten re-introduced somehow when it became a Cargo subcommand.

The path doesn't get transformed if the directory to vendor into doesn't exist, which makes sense as std::fs::canonicalize requires the file to exist so it can open the file and call GetFinalPathNameByHandleW. So a temporary workaround would be to delete third_party/rust, except there's at least one crate (coreaudio-sys) that pulls directly from there. Probably not worth the gymnastics.

Comment 2

[Michael Shal [:mshal]]

:agashlin, are you interested in us adding a workaround in the build system for these paths? Or are you confident it will be fixed upstream?

Comment 3

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

I don't know, it hasn't been acknowledged as a problem upstream, but there hasn't yet been a response to my post (yesterday) on the issue.

This isn't something I currently need to get work done, and :bytesized worked around it by running the vendoring step on Linux. It would be nice if mozpath.relpath() could handle these on Windows, though.

Comment 4

[Michael Shal [:mshal]]

Attachment: Bug 1581248 - Strip //?/ prefixes in mozpath.relpath(); r?#firefox-build-system-reviewers

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/caa26030e1f2
Strip //?/ prefixes in mozpath.relpath(); r=firefox-build-system-reviewers,chmanchester

Comment 6

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/caa26030e1f2

Comment 7

[Ryan VanderMeulen [:RyanVM]]

Should we uplift this to Beta?

Comment 8

[Michael Shal [:mshal]]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #7)

Should we uplift this to Beta?

I don't think it'd be particularly useful - or is it normal to run 'mach vendor rust' on beta? There shouldn't be a downside to uplifting if someone wants to do vendor directly to beta, though.