Closed Bug 1508828 Opened 11 months ago Closed 9 months ago

Stand up webrender CI on Firefox CI (Windows edition)


(Core :: Graphics: WebRender, defect, P2)

Other Branch



Tracking Status
firefox66 --- fixed


(Reporter: kats, Assigned: kats)


(Depends on 1 open bug, Blocks 1 open bug)


(Whiteboard: [gfx-noted])


(1 file)

+++ This bug was initially created as a clone of Bug #1507884 +++

Same deal as bug 1507884, but for windows
Depends on: 1503756
Priority: -- → P2
Quick update here: I've been plugging away on this for a while now [1] and making slow but steady progress. Latest attempt is at - this has the taskcluster job running the windows test commands (after going through a nightmare of a time ensuring msvc is set up, because rustc requires link.exe to link) but now it's failing because we need cmake for servo-freetype-sys.

Looks like the cmake 3.6.2 repack that's currently on tooltool isn't new enough to generate stuff for Visual Studio 2017. I tried using a newer cmake and that worked better, but it couldn't actually find the Visual Studio "installation" (probably because it's looking for registry keys or some such while all we have is a tarball from tooltool). I'm not entirely sure where to go from here, the cmake documentation doesn't provide much useful information in the way of telling it where to find Visual Studio.
A couple of possible options:
1) Disable the pathfinder build, which should eliminate the servo-freetype-sys build and avoid this whole problem. That will at least get me unblocked to whatever the next problem is, and we can circle back to the pathfinder build later
2) Try to have cmake generate a different build (e.g. ninja + cl.exe instead of VisualStudio + cl.exe) that we can actually set up and execute. Right now the cmake-rs crate assumes that if the rust target is an msvc target, then we must want to use VisualStudio to do the build. Which isn't necessarily true - it should be possible to use a different build system with the cl.exe compiler to still generate msvc ABI binaries. So removing the cmake-rs assumption and allowing cmake to fall back to some other build system should in theory work.
With the pathfinder stuff removed, things seem to generally work. There are some reftest failures though, with what looks like fuzzable differences:
Using ninja+MSVC seems to work. But to do that we'll need a patch to either cmake-rs or servo-freetype-sys. I've filed an issue to figure out which is better. Now it's just the reftests that need to be dealt with, I think.
Switching to a non-GPU worker (and using the PowerShell script to set the screen resolution to what we use in appveyor) seems to fix the reftest differences.
Unfortunately after rebasing I'm getting intermittent windows build failures for the pathfinder build:

link.exe sometimes fails to find advapi32.dll. All the environment paths and files should be the exact same between passing and failing runs, so I'm not really sure what's going on here.
Even doing a `cargo clean` before the pathfinder build doesn't help:

And just to make sure I'm not crazy I did some retriggers on a push from a few days ago:

and that's all green. Could just be lucky, or could be something regressed. I think we moved from rustc 1.30 to 1.31 in that interval which might be related.
Interesting.. try push at shows that in passing runs the LIB environment variable has windows paths but in failing runs it has msys-style paths:

LIB=z:\task_1545096103\build\src\vs2017_15.8.4\VC\lib\x64;z:\task_1545096103\build\src\vs2017_15.8.4\VC\atlmfc\lib\x64;z:\task_1545096103\build\src\vs2017_15.8.4\SDK\Lib\10.0.17134.0\ucrt\x64;z:\task_1545096103\build\src\vs2017_15.8.4\SDK\Lib\10.0.17134.0\um\x64;z:\task_1545096103\build\src\vs2017_15.8.4\DIA SDK\lib\amd64

LIB=/z/task_1545096211/build/src/vs2017_15.8.4/VC/lib/x64:/z/task_1545096211/build/src/vs2017_15.8.4/VC/atlmfc/lib/x64:/z/task_1545096211/build/src/vs2017_15.8.4/SDK/Lib/10.0.17134.0/ucrt/x64:/z/task_1545096211/build/src/vs2017_15.8.4/SDK/Lib/10.0.17134.0/um/x64:/z/task_1545096211/build/src/vs2017_15.8.4/DIA SDK/lib/amd64

AFAICT the last `export LIB=` that we actually run sets the msys path so it's not clear to me what's doing the conversion into windows paths, and why it doesn't happen every time. Must be a race condition of some sort.
Still happening:

:ahal, do you have any insight here? I'm at a bit of a loss as to where to look. In a nutshell: I have a task that I'm trying to run on windows via run-task, the meat of which can be seen at [1]. What's happening is that when it gets to running the cmd.exe lines towards the end, the LIB environment variable (and INCLUDE) is sometimes of the form z:\... and sometimes of the form /z/... (see try push logs and search for "LIB is" if you want the full thing). When it's in the windows format it passes and when it's in the other format the linker can't find the DLL it needs and the build job fails.

I am doing some dubious stuff via the setup-msvc-env script, which I copied from [2]. That's the most likely source of problems, but I don't see why it would be non-deterministic like this.

:glandium, also if you have any feedback on this approach or suggestions for alternatives that would bypass this problem I'm all ears.

(Also this is not super urgent, so don't let it interrupt any PTO/holiday stuff. I just wanted to write it down while it's fresh)

Flags: needinfo?(mh+mozilla)
Flags: needinfo?(ahal)
That's pretty strange! The only thing I can think of is that it must have something to do with the state of the hosts, e.g maybe some of them have an artifact lying around from a previous task that is somehow causing a different code path to get hit.

Since your modified VSPATH shows up in the LIB env, it looks like this must be where that env is getting set:

I'm not familiar with how this stuff works, but maybe glandium will have a bit of extra insight.
Flags: needinfo?(ahal)
Oh wait, I guess it's here:

That mk_export_correct_style LIB at the bottom looks suspicious.
So the mk_export_correct_style is at [1] and invokes mk_add_options, which I've gutted via [2], so that should effectively be a no-op. It should really just set the env var from the line you linked and that's it. If mk_export_correct_style is responsible, I still don't see why it's nondeterministic. :(

This is some kind of madness. In a fresh local mozilla build shell:

MozillaBuild Install Directory: C:\mozilla-build\
kats@kgupta-win ~$ export LIB=$PWD
kats@kgupta-win ~$ echo $LIB; cmd /c 'echo %LIB%'; cmd.exe /c 'echo %LIB%'
kats@kgupta-win ~$ echo $SHELL
kats@kgupta-win ~$ bash  # nested shell
kats@kgupta-win ~$ echo $SHELL
kats@kgupta-win ~$ echo $LIB; cmd /c 'echo %LIB%'; cmd.exe /c 'echo %LIB%'
kats@kgupta-win ~$ exit
kats@kgupta-win ~$ echo $LIB; cmd /c 'echo %LIB%'; cmd.exe /c 'echo %LIB%'
kats@kgupta-win ~$ export LIB=$PWD:$PWD/foo
kats@kgupta-win ~$ echo $LIB; cmd /c 'echo %LIB%'; cmd.exe /c 'echo %LIB%'
kats@kgupta-win ~$ bash  # nested shell
kats@kgupta-win ~$ echo $LIB; cmd /c 'echo %LIB%'; cmd.exe /c 'echo %LIB%'
kats@kgupta-win ~$

AFAICT the rules here seem to be:
a) The local bash variable is always fine
b) Echoing via `cmd` gives the same result as (a). Note that `cmd` is a wrapper that just runs "$COMSPEC" "$@"
c) Echoing via `cmd.exe` converts to c:/ format (forward slashes), except if there are multiple paths, in which case it converts to c:\ format (backslashes)
d) In a nested bash shell `cmd.exe` produces the same result as `cmd`

Further experimentation shows that the PATH variable doesn't follow rule (b) and always emits c:/ format (forward slashes) via `cmd`. In nested bash shells `cmd.exe` also emits forward slashes for %PATH%
Welcome in msys hell.
Flags: needinfo?(mh+mozilla)

As far as I can tell from digging around in the source, there are 6 environment variables that are supposed to go undergo automatic conversion [1] - PATH, HOME, LD_LIBRARY_PATH, TMP, TEMP, and TMPDIR. In Cygwin this works as described. In msys (which AFAICT is a fork of cygwin, but with useful stuff removed and bugs sprinkled liberally) this does not work as described, and for some reason works as described in comment 15 instead.

So my options here are:
1) Skip the pathfinder build, but that's just really kicking the can down the road and we'll have to deal with this again later
2) Hard-code the path in Windows format in my task description
3) Generate the path in Unix format and convert it myself to Windows format either using a regex or some other mechanism
4) ???

I tried making a toolchain out of cygpath.exe and cygwin1.dll from a cygwin installation and using that to do path conversion, but it doesn't work quite right. It converts something like "/z/foo/bar:/z/baz" into "z;Z:\foo\bar;z;Z:\baz" - while that does technically work it seems like it might be a footgun later, and maybe mixing cygwin stuff with msys isn't the best idea.

I did get it working with sed to do the path conversion. Will clean up the patches and do retriggers to ensure it's green before posting for review.
Pushed by
Add a task to run standalone WebRender CI scripts on Windows. r=glandium,jrmuizel
Closed: 9 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
You need to log in before you can comment on or make changes to this bug.