Closed Bug 1498215 Opened Last year Closed Last year

mach broken on systems with an old version of scandir installed


(Testing :: General, enhancement)

Version 3
Not set


(Not tracked)



(Reporter: jgraham, Unassigned)




(1 file)

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>
  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/", line 320, in bootstrap
    driver.load_commands_from_file(os.path.join(mozilla_dir, path))
  File "/home/jgraham/develop/gecko/python/mach/mach/", line 267, in load_commands_from_file
    imp.load_source(module_name, path)
  File "/home/jgraham/develop/gecko/tools/lint/", line 26, in <module>
    from scandir import scandir
  File "/home/jgraham/develop/gecko/build/", line 349, in __call__
    module = self._original_import(name, globals, locals, fromlist, level)
  File "/home/jgraham/develop/gecko/third_party/python/scandir/", 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?
Flags: needinfo?(dave.hunt)
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

* 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.
Duplicate of this bug: 1498284
Pushed by
Fix problem importing scandir when system scandir exists, r=davehunt
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.
Flags: needinfo?(james)
Just to note, I think using:

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!
Closed: Last year
Flags: needinfo?(dave.hunt)
Resolution: --- → WONTFIX
Duplicate of this bug: 1498200
You need to log in before you can comment on or make changes to this bug.