1617217 - Python ImportHook issues when moving a Python file to a directory of the same name

Status: NEW

(Firefox Build System :: General, defect, P3)

Description

[Andrew Halberstadt [:ahal]]

Attachment: Attempted solution

I've run into this a few times now. Most recently in bug 1616902. The situation is this:

Commit A is our baseline, and has the following file:

- tools/tryselect/selectors/fuzzy.py

Commit B then moves this file to:

- tools/tryselect/selectors/fuzzy/__init__.py

The problem arises when someone updates to Commit B or later and runs mach try. So they have:

- tools/tryselect/selectors/fuzzy/__init__.py
- tools/tryselect/selectors/fuzzy/__init__.pyc

Then they update to Commit A or earlier, so they're back to:

- tools/tryselect/selectors/fuzzy.py
- tools/tryselect/selectors/fuzzy/__init__.pyc

At this point, our ImportHook will use the original Python import function to determine the module path. Python will find the __init__.pyc first rather than fuzzy.py. Since this bytecode module is from a future version of the file, chances are that it is incompatible (in fact, even just the move to a new directory itself often means relative import paths needed to change). The user then gets a confusing exception.

They can run ./mach clobber python to get back to a working state, but will need to do so every time they jump back and forth across the Commit A -> Commit B change. Last time I actually landed a change that did this, I probably had over a dozen annoyed developers running into this.

The easiest solution to this problem is to configure mach to not write bytecode. I'm not sure what the perf impacts of that are, but it might be worth swallowing to avoid things like this in the future.

I also tried to hack something up to handle this case in our ImportHook (see attached patch). I came pretty close and with some extra effort can probably get it working. I have no idea how risky it is though, and wouldn't feel very comfortable landing it.

Comment 1

[Mike Hommey [:glandium]]

Note that python 3.something (3.7?) allows to place the .pyc files in a different directory (it might even do it by default, I don't remember, and I don't have a recent python 3 handy to double check).

Comment 2

[Andrew Halberstadt [:ahal]]

With Python 3 we might also be able to use importlib.util.find_spec rather than pkgutil in my patch, which may work a little better (haven't tested).