Closed
Bug 1498215
Opened 6 years ago
Closed 6 years ago
mach broken on systems with an old version of scandir installed
Categories
(Testing :: General, enhancement)
Tracking
(Not tracked)
RESOLVED
WONTFIX
People
(Reporter: jgraham, Unassigned)
References
Details
Attachments
(1 file)
(deleted),
text/x-phabricator-request
|
Details |
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.
Reporter | ||
Comment 1•6 years ago
|
||
davehunt: Since ahal is ill, can you look at this please?
Flags: needinfo?(dave.hunt)
Reporter | ||
Comment 2•6 years ago
|
||
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.
Reporter | ||
Comment 3•6 years ago
|
||
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.
Reporter | ||
Comment 4•6 years ago
|
||
Pushed by james@hoppipolla.co.uk:
https://hg.mozilla.org/integration/autoland/rev/b01876f4f16e
Fix problem importing scandir when system scandir exists, r=davehunt
Comment 7•6 years ago
|
||
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
Flags: needinfo?(james)
Reporter | ||
Comment 8•6 years ago
|
||
Can we then also backout bug 1494069 until we have a better solution here?
Flags: needinfo?(james) → needinfo?(sheriffs)
Comment 9•6 years ago
|
||
@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)
Reporter | ||
Comment 10•6 years ago
|
||
I believe it's mostly a performance fix and therefore I'm hopeful that it won't break anything to back it out.
Flags: needinfo?(james)
Comment 11•6 years ago
|
||
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: 6 years ago
Flags: needinfo?(dave.hunt)
Resolution: --- → WONTFIX
You need to log in
before you can comment on or make changes to this bug.
Description
•