Incremental compilation creates file names too long for Windows

[SimonSapin]

Today I made a Servo try build with a rustc/cargo version that, if I understand correctly, enable incremental compilation by default. This left on CI builders some files with long names. On every subsequent builds, Buildbot would first try to clean the target directory and fail to remove files with too long names/paths such as c:\buildbot\slave\windows-msvc-dev\build\target\debug\incremental\script-1g52totqj6qac\s-ex1p8h0sua-1w9qs9v-hbvaqqvv1zrk\cgu-script-dom-bindings-codegen-Bindings-BluetoothCharacteristicPropertiesBinding-BluetoothCharacteristicPropertiesBinding.volatile.bc-compressed:

exception from rmdirRecursive
Traceback (most recent call last):
  File "c:\Python27\lib\threading.py", line 801, in __bootstrap_inner
    self.run()
  File "c:\Python27\lib\threading.py", line 754, in run
    self.__target(*self.__args, **self.__kwargs)
  File "c:\Python27\lib\site-packages\twisted-16.1.1-py2.7-win-amd64.egg\twisted\_threads\_threadworker.py", line 46, in work
    task()
  File "c:\Python27\lib\site-packages\twisted-16.1.1-py2.7-win-amd64.egg\twisted\_threads\_team.py", line 190, in doWork
    task()
--- <exception caught here> ---
  File "c:\Python27\lib\site-packages\twisted-16.1.1-py2.7-win-amd64.egg\twisted\python\threadpool.py", line 246, in inContext
    result = inContext.theWork()
  File "c:\Python27\lib\site-packages\twisted-16.1.1-py2.7-win-amd64.egg\twisted\python\threadpool.py", line 262, in <lambda>
    inContext.theWork = lambda: context.call(ctx, func, *args, **kw)
  File "c:\Python27\lib\site-packages\twisted-16.1.1-py2.7-win-amd64.egg\twisted\python\context.py", line 118, in callWithContext
    return self.currentContext().callWithContext(ctx, func, *args, **kw)
  File "c:\Python27\lib\site-packages\twisted-16.1.1-py2.7-win-amd64.egg\twisted\python\context.py", line 81, in callWithContext
    return func(*args,**kw)
  File "c:\Python27\lib\site-packages\buildbot_slave-0.9.0b7-py2.7.egg\buildslave\commands\utils.py", line 92, in rmdirRecursive
    rmdirRecursive(full_name)
  File "c:\Python27\lib\site-packages\buildbot_slave-0.9.0b7-py2.7.egg\buildslave\commands\utils.py", line 92, in rmdirRecursive
    rmdirRecursive(full_name)
  File "c:\Python27\lib\site-packages\buildbot_slave-0.9.0b7-py2.7.egg\buildslave\commands\utils.py", line 92, in rmdirRecursive
    rmdirRecursive(full_name)
  File "c:\Python27\lib\site-packages\buildbot_slave-0.9.0b7-py2.7.egg\buildslave\commands\utils.py", line 92, in rmdirRecursive
    rmdirRecursive(full_name)
  File "c:\Python27\lib\site-packages\buildbot_slave-0.9.0b7-py2.7.egg\buildslave\commands\utils.py", line 92, in rmdirRecursive
    rmdirRecursive(full_name)
  File "c:\Python27\lib\site-packages\buildbot_slave-0.9.0b7-py2.7.egg\buildslave\commands\utils.py", line 87, in rmdirRecursive
    os.chmod(full_name, 0o600)
exceptions.WindowsError: [Error 3] The system cannot find the path specified: u'c:\\buildbot\\slave\\windows-msvc-dev\\build\\target\\debug\\incremental\\script-1g52totqj6qac\\s-ex1p8h0sua-1w9qs9v-hbvaqqvv1zrk\\cgu-script-dom-bindings-codegen-Bindings-BluetoothCharacteristicPropertiesBinding-BluetoothCharacteristicPropertiesBinding.volatile.bc-compressed'
program finished with exit code -1

Could the file names be made to not contain full Rust paths? Maybe using a hash instead?

CC @michaelwoerister @larsbergstrom @jonathandturner

[mdaniel]

as a friendly reminder, the Windows API has a separate "unicode" filename, which I confirmed works as expected on the install of Python 2.7 I had lying around (on both NTFS and ExFAT filesystems):

>>> fh = open(r'\\?\c:\buildbot\slave\windows-msvc-dev\build\target\debug\incremental\script-1g52totqj6qac\s-ex1p8h0sua-1w9qs9v-hbvaqqvv1zrk\cgu-script-dom-bindings-codegen-Bindings-BluetoothCharacteristicPropertiesBinding-BluetoothCharacteristicPropertiesBinding.volatile.bc-compressed', 'w')
>>> fh.close()

I'd have to dig in MSDN to find the gory details, but can do so if it would be helpful

[chris-morgan]

At first I was going to say “buildbot bug”, and indeed it could be resolved there with things like the \\?\ trick; but the fact of the matter is that that’s just a patch for this particular symptom, leaving the underlying issue still present.

Individual path components on most file systems currently in use (e.g. NTFS, ext*) can only be 255 characters, so as soon as you get a module path with a long enough name, it’s going to break regardless; therefore this isn’t solely a Windows bug—it simply showed up on Windows first!

[mdaniel]

whoa, that was some creepy timing :-)

[michaelwoerister]

Thanks for the bug report. We fixed this problem in the compiler itself more than a year ago by using the proper path format (the one @mdaniel mentions).

We could probably add a flag that makes the compiler generate shorter, non-human-readable filenames for object files. Not sure about the priority of that though. Do you have a use case where you want incremental compilation in combination with buildbot?

[SimonSapin]

Right now Servo CI cleans the target directory for every build, so we’ll probably want to disable incremental compilation for now. But it’d be nice if we can enable it at some point and reduce the CI cycle time.

We can try to patch this specific Buildbot code, but it’s just one example of possibly many tools that might break in the presence of such files.

Why make this configurable? Are these file names ever read by humans, other than when working on the compiler itself? And for that case, do you actually manipulate these files directly rather than through some tool? Could that tool extract the Rust path from the contents of the file rather than its name?

[michaelwoerister]

Incremental compilation has been a two-man-show so far, we don't have fancy tools like that :)

The human-readable filenames have been useful for debugging so far and we need them for some of our auto-tests. However, we could default to short, mangled names and only have human-readable/predictable file names for our test suites.

Note, however, that this does not completely solve the problem: We can only control the length of the last three components of these paths. Given a long enough prefix the path as a whole can still be longer than 260 characters.

[chris-morgan]

@michaelwoerister Is I remarked, it’s fundamentally a more serious, not-Windows-specific issue, because file names are capped at 255 characters on most filesystems, but long module paths could lead to an excessively long file name on almost any platform, regardless of the whole prefix aspect.

[larsbergstrom]

@froydnj & @glandium @nnethercote , is m-c friendlier than Servo to > MAX_PATH internal paths on Windows, or will this affect use of incremental compilation for Firefox, too?

[froydnj]

@larsbergstrom Eyeballing logs from m-c suggests that the paths would be roughly as long as the ones Servo uses, probably longer:

c:\buildbot\slave\windows-msvc-dev\build\target\
z:/build/build/src/obj-firefox/toolkit/library\x86_64-pc-windows-msvc\

are the path prefixes for where debug//release//etc. go. That on its own is not reassuring. Path prefixes are liable to be a little shorter on developer machines, but still...

Whether things would actually break would depend on the particular files and whatnot being compiled, and I don't have a good feel for things to say definitively one way or the other; I guess we should just try it and see? (Note too that adding more code--particularly generated code like @mystor wants to add in m-c--is liable to change whether things break, so just because things would work now doesn't mean they'd keep working...)

[michaelwoerister]

Alright, it looks like this will keep causing complications if we don't do anything and fixing it should be relatively straightforward, so I'll just do that next week.

[SimonSapin]

To be fair incremental compilation doesn’t create long file names by itself. It creates file names based on Rust paths, and Servo’s DOM implementation happens to have a script::dom::bindings::codegen::Bindings::BluetoothCharacteristicPropertiesBinding::BluetoothCharacteristicPropertiesBinding module. We could also work on our side to avoid repeating that last component.

Firefox doesn’t use Servo’s DOM implementation, and so would not include this particular module.

[jgraham]

FWIW we had problems with overlong paths when running web-platform-tests on Windows in mozilla-central, so it's certainly an issue there. We ended up capping the upstream paths at 150 characters and the in-tree path prefix is about 30, so we are allowing about 75 characters for the build path.

This character counting is obviously terrible, but I don't know of a better solution that works with all the tooling that's used during a build.

[michaelwoerister]

The latest nightly contains a fix for this issue. Can you test that, @SimonSapin? It would be good to confirm that this actually helps before doing a backport.

[michaelwoerister]

The fix has been backported to the beta branch. Not sure if we release a new beta version yet though.

[SimonSapin]

I haven’t reproduced the actual buildbot failure (it breaks the CI infra until manual clean up that I don’t have access to do) but when removing the path prefix and starting from target/debug/, the longest path after building Servo went from 225 bytes to 175, and that path is inside a Python virtualenv inside a Cargo $OUT_DIR. Within incremental/ it’s 119. Thanks!

[PeterCook1]

I suggest that you try the LongPathTool program , I think it will be very helpful in your case .

[SimonSapin]

Thanks @PeterCook1, but in case you meant this for me or Servo this is already not an issue for us anymore as we’ve explicitly disabled incremental compilation on Servo’s CI servers. (It wouldn’t help since we clean the target directory between builds. This issue occurred when we upgraded rustc to a version that started enabling incremental by default.)