Closed Bug 1498215 Opened Last year Closed Last year
mach broken on systems with an old version of scandir installed
46 bytes, text/x-phabricator-request
|Details | Review|
jgraham@flitwick:~/develop/gecko$ pip install 'scandir==1.6' Collecting scandir==1.6 Installing collected packages: scandir Successfully installed scandir-1.6 jgraham@flitwick:~/develop/gecko$ ./mach help Traceback (most recent call last): File "./mach", line 86, in <module> main(sys.argv[1:]) File "./mach", line 78, in main mach = get_mach() File "./mach", line 68, in get_mach mach = check_and_get_mach(dir_path) File "./mach", line 42, in check_and_get_mach return load_mach(dir_path, mach_path) File "./mach", line 30, in load_mach return mach_bootstrap.bootstrap(dir_path) File "/home/jgraham/develop/gecko/build/mach_bootstrap.py", line 320, in bootstrap driver.load_commands_from_file(os.path.join(mozilla_dir, path)) File "/home/jgraham/develop/gecko/python/mach/mach/main.py", line 267, in load_commands_from_file imp.load_source(module_name, path) File "/home/jgraham/develop/gecko/tools/lint/mach_commands.py", line 26, in <module> from scandir import scandir File "/home/jgraham/develop/gecko/build/mach_bootstrap.py", line 349, in __call__ module = self._original_import(name, globals, locals, fromlist, level) File "/home/jgraham/develop/gecko/third_party/python/scandir/scandir.py", line 584, in <module> DirEntry_c = _scandir.DirEntry AttributeError: 'module' object has no attribute 'DirEntry' This totally breaks everything until you realise what the problem is. We either need to ensure that we get the order of paths correct, or a workaround, or to back out bug 1494069 until it's fixed.
davehunt: Since ahal is ill, can you look at this please?
So I think what's happening is that scandir tries to import _scandir which it doesn't find in the in-tree version because nothing yet had a chance to compile the c extension (maybe nothing ever will?) but it does find _scandir from the previously installed version, which doesn't end well.
If scandir is already present on the system the attempt to import the c helper library will currently find the c helper from the system install which may well be an outdated verion, so causing mach to break. To solve this this patch does two things: * Stops importing scandir in files that are run unconditionally when invoking mach. This is generally considered good for performance reasons. * Installs the vendored scandir into the virtualenv for `mach lint` rather than trying to import it directly from the source tree and so not getting the c helper library.
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/b01876f4f16e Fix problem importing scandir when system scandir exists, r=davehunt
Backed out for bustages on test_pathutils.py There were also mozlint perma failures - https://treeherder.mozilla.org/logviewer.html#?job_id=204869316&repo=autoland&lineNumber=357 Backout link: https://hg.mozilla.org/integration/autoland/rev/2099bd3c295d191c41d0015adc986ebe4407faef Push link: https://hg.mozilla.org/integration/autoland/rev/b01876f4f16eddbcfdc203e99f32356a81763863 Log link: https://treeherder.mozilla.org/logviewer.html#?job_id=204869317&repo=autoland&lineNumber=47237
Can we then also backout bug 1494069 until we have a better solution here?
Flags: needinfo?(james) → needinfo?(sheriffs)
@jgraham we are going to backout bug 1494069 from m-c. Is it going to affect anything, as it will be merged around to the integration branches?
Flags: needinfo?(sheriffs) → needinfo?(james)
I believe it's mostly a performance fix and therefore I'm hopeful that it won't break anything to back it out.
Just to note, I think using: setup.py:third_party/python/scandir:install in virtualenv_packages.txt would have worked instead of using the mozilla.pth (we'd still need to be careful not to import scandir from the global mach context though). But in the end I decided to re-implement my patch to use os.listdir to avoid the headache (turns out that the perf benefit of scandir might be a bit overstated for the case I was using it for). We may eventually want to vendor scandir for other purposes (or we may want it if we make my 'collapse' function more general purpose). Sorry for all the trouble!
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.