1266123 - Windows builds fail due to moz.build file being considered to be outside of topsrcdir due to path library not treating drive letter as case insensitive

Status: RESOLVED INACTIVE

(Firefox Build System :: General, defect)

Description

[Pete Moore [:pmoore][:pete]]

11:00:34     INFO -  js\src\ctypes\libffi> ********************************************************
11:00:34     INFO -  js\src\ctypes\libffi> * WARNING: Don't know the best CFLAGS for this system  *
11:00:34     INFO -  js\src\ctypes\libffi> * Use ./configure CFLAGS=... to specify your own flags *
11:00:34     INFO -  js\src\ctypes\libffi> * (otherwise, a default of CFLAGS=-O3 will be used)    *
11:00:34     INFO -  js\src\ctypes\libffi> ********************************************************
11:00:34     INFO -  js\src\ctypes\libffi>
11:00:34     INFO -  js\src\ctypes\libffi> checking whether C compiler accepts -O3... yes
11:00:34     INFO -  js\src\ctypes\libffi> checking CFLAGS for maximum warnings... -Wall
11:00:34     INFO -  js\src\ctypes\libffi> checking whether to enable maintainer-specific portions of Makefiles... no
11:00:34     INFO -  js\src\ctypes\libffi> checking sys/mman.h usability... no
11:00:34     INFO -  js\src\ctypes\libffi> checking sys/mman.h presence... no
11:00:34     INFO -  js\src\ctypes\libffi> checking for sys/mman.h... no
11:00:34     INFO -  js\src\ctypes\libffi> checking for mmap... no
11:00:34     INFO -  js\src\ctypes\libffi> checking for sys/mman.h... (cached) no
11:00:34     INFO -  js\src\ctypes\libffi> checking for mmap... (cached) no
11:00:34     INFO -  js\src\ctypes\libffi> checking for ANSI C header files... (cached) yes
11:00:34     INFO -  js\src\ctypes\libffi> checking for memcpy... no
11:00:34     INFO -  js\src\ctypes\libffi> checking for size_t... yes
11:00:34     INFO -  js\src\ctypes\libffi> checking for working alloca.h... no
11:00:34     INFO -  js\src\ctypes\libffi> checking for alloca... yes
11:00:34     INFO -  js\src\ctypes\libffi> checking size of double... 8
11:00:34     INFO -  js\src\ctypes\libffi> checking size of long double... 8
11:00:34     INFO -  js\src\ctypes\libffi> checking whether byte ordering is bigendian... no
11:00:34     INFO -  js\src\ctypes\libffi> checking assembler .cfi pseudo-op support... no
11:00:34     INFO -  js\src\ctypes\libffi> checking for _ prefix in compiled symbols... no
11:00:34     INFO -  js\src\ctypes\libffi> configure: updating cache c:/home/worker/workspace/build/src/obj-firefox/js/src/ctypes/libffi/config.cache
11:00:34     INFO -  js\src\ctypes\libffi> checking that generated files are newer than configure... done
11:00:34     INFO -  js\src\ctypes\libffi> configure: creating ./config.status
11:00:34     INFO -  js\src\ctypes\libffi> config.status: creating include/Makefile
11:00:34     INFO -  js\src\ctypes\libffi> config.status: creating include/ffi.h
11:00:34     INFO -  js\src\ctypes\libffi> config.status: creating Makefile
11:00:34     INFO -  js\src\ctypes\libffi> config.status: creating testsuite/Makefile
11:00:34     INFO -  js\src\ctypes\libffi> config.status: creating man/Makefile
11:00:34     INFO -  js\src\ctypes\libffi> config.status: creating libffi.pc
11:00:34     INFO -  js\src\ctypes\libffi> config.status: creating fficonfig.h
11:00:34     INFO -  js\src\ctypes\libffi> config.status: linking C:/home/worker/workspace/build/src/js/src/ctypes/libffi/src/x86/ffitarget.h to include/ffitarget.h
11:00:34     INFO -  js\src\ctypes\libffi> config.status: executing buildir commands
11:00:34     INFO -  js\src\ctypes\libffi> config.status: skipping top_srcdir/Makefile - not created
11:00:34     INFO -  js\src\ctypes\libffi> config.status: executing depfiles commands
11:00:34     INFO -  js\src\ctypes\libffi> config.status: executing libtool commands
11:00:34     INFO -  js\src\ctypes\libffi> config.status: executing include commands
11:00:34     INFO -  js\src\ctypes\libffi> config.status: executing src commands
11:00:36     INFO -  Reticulating splines...
11:00:36     INFO -  Traceback (most recent call last):
11:00:36     INFO -    File "config.status", line 992, in <module>
11:00:36     INFO -      config_status(**args)
11:00:36     INFO -    File "c:\home\worker\workspace\build\src\python\mozbuild\mozbuild\config_status.py", line 156, in config_status
11:00:36     INFO -      definitions = list(definitions)
11:00:36     INFO -    File "c:\home\worker\workspace\build\src\python\mozbuild\mozbuild\frontend\emitter.py", line 165, in emit
11:00:36     INFO -      for out in output:
11:00:36     INFO -    File "c:\home\worker\workspace\build\src\python\mozbuild\mozbuild\frontend\reader.py", line 1066, in read_mozbuild
11:00:36     INFO -      raise bre
11:00:36     INFO -  mozbuild.frontend.reader.BuildReaderError:
11:00:36     INFO -  ==============================
11:00:36     INFO -  ERROR PROCESSING MOZBUILD FILE
11:00:36     INFO -  ==============================
11:00:36     INFO -  The error occurred while processing the following file:
11:00:36     INFO -      C:/home/worker/workspace/build/src/moz.build
11:00:36     INFO -  The underlying problem is an illegal file access. This is likely due to trying to access a file outside of the top source directory.
11:00:36     INFO -  The path whose access was denied is:
11:00:36     INFO -      C:/home/worker/workspace/build/src/js/src/moz.build
11:00:36     INFO -  Modify the script to not access this file and try again.
11:00:36     INFO -  Creating config.status
11:00:36     INFO -  *** Fix above errors and then restart with\
11:00:36     INFO -                 "C:/home/worker/workspace/build/src/mozmake.EXE -f client.mk build"
11:00:36     INFO -  C:/home/worker/workspace/build/src/client.mk:370: recipe for target 'configure' failed
11:00:36     INFO -  mozmake.EXE[2]: *** [configure] Error 1
11:00:36     INFO -  mozmake.EXE[2]: Leaving directory 'C:/home/worker/workspace/build/src'
11:00:36     INFO -  C:/home/worker/workspace/build/src/client.mk:384: recipe for target 'C:/home/worker/workspace/build/src/obj-firefox/Makefile' failed
11:00:36     INFO -  mozmake.EXE[1]: *** [C:/home/worker/workspace/build/src/obj-firefox/Makefile] Error 2
11:00:36     INFO -  mozmake.EXE[1]: Leaving directory 'C:/home/worker/workspace/build/src'
11:00:36     INFO -  client.mk:168: recipe for target 'build' failed
11:00:36     INFO -  mozmake.EXE: *** [build] Error 2
11:00:36     INFO -  1 compiler warnings present.
11:00:36     INFO -  2
11:00:36    ERROR - Return code: 1
11:00:36  WARNING - setting return code to 2
11:00:36    FATAL - 'mach build' did not run successfully. Please check log for errors.

Comment 1

[Pete Moore [:pmoore][:pete]]

With a bit more debugging, we get:

CAUSE OF FAILURE: C:/home/worker/workspace/build/src/js/src/moz.build falls outside of c:/home/worker/workspace/build/src

The logging was added with:

https://hg.mozilla.org/try/rev/912139a9c6624160c64a55ca2a09328213218290

Comment 2

[Pete Moore [:pmoore][:pete]]

The failure seems due to the case of the drive letter being different in the two paths (`C:/` vs `c:/`). The path library does not normalise the drive letter case when checking if a path falls underneath a different path.

Comment 3

[Pete Moore [:pmoore][:pete]]

It looks like the build system parses argv[0] of the mach command to establish the path, and will keep the case of the drive letter as seen there. However, the moz.build file seems to take onboard a lower case drive letter.

When calling mach directly, a workaround can be used, to use a relative command for the mach path.

When calling mach from mozharness, this is trickier, as mozharness invokes mach, rather than the shell.

Comment 4

[Pete Moore [:pmoore][:pete]]

In any case the path library should treat windows drive letters as case insensitive - that is the correct fix, anything else to tweak config is just a workaround.

Please note, we invoke mozharness here, and even with a lowercase c: we are not able to work around the issue:
https://hg.mozilla.org/try/file/7fae21293326/testing/taskcluster/scripts/builder/build-windows.cmd#l63

Comment 5

[Pete Moore [:pmoore][:pete]]

Specifically, this function doesn't respect drive letter case insensitivity:

https://hg.mozilla.org/try/file/7fae21293326/python/mozbuild/mozpack/path.py#l73

This function needs to be fixed.

Comment 6

[Pete Moore [:pmoore][:pete]]

(In reply to Pete Moore [:pmoore][:pete] from comment #4)
> In any case the path library should treat windows drive letters as case
> insensitive - that is the correct fix, anything else to tweak config is just
> a workaround.
> 
> Please note, we invoke mozharness here, and even with a lowercase c: we are
> not able to work around the issue:
> https://hg.mozilla.org/try/file/7fae21293326/testing/taskcluster/scripts/
> builder/build-windows.cmd#l63


In other words, there is not a trivial workaround of changing a c:\ to a C:\ somewhere in our config, since the C:\ isn't coming from any of our config. Therefore the only workaround we can do is to comment out checks in the build system, but this leads to test failures. Therefore the fix suggested in comment 5 is the only real way to workaround this at the moment. I also tried calling mozharness from inside bash, but in my first attempt didn't get that working either.

Comment 7

[Joel Maher ( :jmaher ) (UTC -8)]

I don't understand why we are not hitting this in our current builders?  Did the toolchain change or the shell we are running this in?  it sounds like if we are mixing the case of c:\ and C:\ that we have an updated library or toolchain somewhere that is new.

Comment 8

[Joel Maher ( :jmaher ) (UTC -8)]

:pmoore, I should have ni'd you regarding comment 7.  I thought about this more and developers do not hit this when following the build steps to build on their local windows machines.

I assume we have manually logged onto this box and have verified a build works prior to running this in automation?

Comment 9

[Mike Hommey [:glandium]]

See bug 1244750 comment 49.

Comment 10

[Joel Maher ( :jmaher ) (UTC -8)]

looks like this general issue is already discussed.  I am still curious to the answers of my previous questions.

Comment 11

[Pete Moore [:pmoore][:pete]]

(In reply to Joel Maher (:jmaher) from comment #7)
> I don't understand why we are not hitting this in our current builders?  Did
> the toolchain change or the shell we are running this in?  it sounds like if
> we are mixing the case of c:\ and C:\ that we have an updated library or
> toolchain somewhere that is new.

Since migrating from buildbot to taskcluster, there has been a lot of cleanup. Not only that, but the context from which the builds are executed is now different. For example, there are not buildbot properties, and the builds do not execute under the cltbld user account. The toolchains should be the same, but I suspect the change that caused this is probably the context in which the mozharness script is invoked in taskcluster vs buildbot. Probably in buildbot the mozharness script is called from an msys context rather than a batch script calling python. In any case, it is clear that since drive letters in Windows are not case sensitive, the best fix would be for our mozilla path libraries to respect that, as this would catch more errors than just this one, as it is ultimately our path library that evaluates if a file sits inside a given directory, and decides it doesn't, when it does, due to the different casing.

I'll take a look at glandium's reference, I'm sure we'll be able to put a fix in this morning.

Comment 12

[Pete Moore [:pmoore][:pete]]

(In reply to Joel Maher (:jmaher) from comment #8)
> :pmoore, I should have ni'd you regarding comment 7.  I thought about this
> more and developers do not hit this when following the build steps to build
> on their local windows machines.
> 
> I assume we have manually logged onto this box and have verified a build
> works prior to running this in automation?

This is because developers call `mach build` rather than run the mozharness script for desktop builds. I firmly believe at a point in the future we should do the same, however for now we've agreed to run a mozharness script, and I will leave that fight until later. ;)

But yes, we may continue to get differences while developers run different steps to automation.

Ultimately I'd like to see `mach bootstrap` implemented for Windows (bug 774115, nearly 4 years old) and then propose in automation, builds run from a pristine Windows environment, first executing `mach bootstrap` to bootstrap the environment, and then `mach build` using the appropriate in-tree mozconfig. This would then be an accurate reflection of what developers do, and they should be able to build the same builds we do in automation.

Another blocker for doing this is at the moment, the mozconfigs set both build switches (e.g. --enable-rust) and environment paths. In my opinion, configuring a build environment (e.g. settings paths) should be independent from choosing build switches, so that people can build with the same build switches, but have their toolchains installed in different locations, and have different drive letters, for example. Once we have achieved this, it will be much more trivial for devs to reproduce builds, and for us to maintain our build environments / taskcluster task definitions, and we can remove the mozharness layer.

But not now, as we need to get the jobs ported over first, before we start optimising...

Comment 13

[Joel Maher ( :jmaher ) (UTC -8)]

Thanks Pete for the great information.

Comment 14

[Pete Moore [:pmoore][:pete]]

https://hg.mozilla.org/try/rev/7cdb8b7ae68fbc53c71c05f84d75c7d645e3f6d0

Comment 15

[Adrian Kalla [:adriank]]

(In reply to Pete Moore [:pmoore][:pete] - PTO 22 July 2016 from comment #14)
> https://hg.mozilla.org/try/rev/7cdb8b7ae68fbc53c71c05f84d75c7d645e3f6d0

This bug has been marked as fixed but unfortunately the patch from comment #14 never landed on central -> reopening.

Besides: I've just run into the problem here and 've spent countless hours on trying to find out what is wrong... Only thanks to the patch from bug 1279027 I've found out, that I've run into this bug here...
Applying the patch from comment #14 seems to fix the problem for me.

Comment 16

[Pete Moore [:pmoore][:pete]]

Not actively working on this at the moment.