1498215 - mach broken on systems with an old version of scandir installed

Status: RESOLVED WONTFIX

(Testing :: General, enhancement)

Description

[James Graham [:jgraham]]

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.

Comment 1

[James Graham [:jgraham]]

davehunt: Since ahal is ill, can you look at this please?

Comment 2

[James Graham [:jgraham]]

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.

Comment 3

[James Graham [:jgraham]]

Attachment: Bug 1498215 - Fix problem importing scandir when system scandir exists,

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.

Comment 4

[James Graham [:jgraham]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=46e656270d799715591edccbd5dfae1c4913545b

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b01876f4f16e
Fix problem importing scandir when system scandir exists, r=davehunt

Comment 7

[Narcis Beleuzu [:NarcisB]]

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

Comment 8

[James Graham [:jgraham]]

Can we then also backout bug 1494069 until we have a better solution here?

Comment 9

[Natalia Csoregi [:nataliaCs]]

@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?

Comment 10

[James Graham [:jgraham]]

I believe it's mostly a performance fix and therefore I'm hopeful that it won't break anything to back it out.

Comment 11

[Andrew Halberstadt [:ahal]]

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!