Traverse directories in stable order when building a PEX #2220
Conversation
testing/__init__.py
Outdated
| @@ -791,7 +791,7 @@ def __call__(self, *args, **kwargs): | |||
| def _increment_counter(self, counter_key): | |||
| # type: (str) -> int | |||
| self._counters[counter_key] += 1 | |||
| return self._counters[counter_key] | |||
| return cast(int, self._counters[counter_key]) | |||
There was a problem hiding this comment.
It would be better to type self._counters = defaultdict(int) # type: DefaultDict[str, int] where you declare / initialize, but it would be arguably even better to use collections.Counter (with similar declaration site typing) which is exactly built for this.
tests/test_common.py
Outdated
| root_dir = os.path.join(tmp, "root") | ||
| os.mkdir(root_dir) | ||
| dir_a = os.path.join(root_dir, "a") | ||
| os.mkdir(dir_a) |
There was a problem hiding this comment.
The os.mkdirs are fine, but touch takes care of this.
There was a problem hiding this comment.
Ah, forgot to follow up here. Looking into it now
There was a problem hiding this comment.
Done. Fingers crossed the new commit doesn't end up needing multiple CI runs.
Also thanks for the other review comments - I learned a few things.
There was a problem hiding this comment.
Fingers crossed the new commit doesn't end up needing multiple CI runs.
The change in #2198, which just landed aheaded of this PR, should help with things going forward. If you continue to do development on Pex, you too can take advantage with tox -e<test env> -- --devpi.
pex/common.py
Outdated
| # type: (*Any, **Any) -> Iterator[Tuple[str, List[str], List[str]]] | ||
| """Walk the specified directory tree in deterministic order. | ||
|
|
||
| Takes the same parameters as os.walk and yields tuples of the same shape. |
There was a problem hiding this comment.
It seems like calling out the topdown restriction, would be good.
|
Thanks again @ento. This should go out in 2.1.144 tomorrow. |
|
Well, tomorrow came and went, but this is now released @ento: |
Addresses one part of #2155.
Adds a wrapper for
os.walkthat sorts directory entries as it traverses directories. As discussed in the issue,os.walkmay yield directory entries in a different order depending on (possibly) the file system.I don't know if the two
os.walkcalls that this PR replaces are enough for addressing this class of non-determinism in pex, but these were sufficient for solving the issue in my case. Happy to expand the use of the wrapper if there are other places that it should be used.