Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug 1697100: Don't let environment affect subprocess module search path #1542

Merged
merged 1 commit into from
Nov 22, 2021

Conversation

mdboom
Copy link
Contributor

@mdboom mdboom commented Mar 9, 2021

@cpeterso: Can you confirm this addresses your issue by copying _process_dispatcher.py on this branch to /Users/chris/Code/mozilla/firefox/obj-x86_64-apple-darwin20.3.0-clang-mozbuild/_virtualenvs/init_py3/lib/python3.8/site-packages/glean/?

@auto-assign auto-assign bot requested a review from brizental March 9, 2021 16:31
@mdboom mdboom requested a review from badboy March 9, 2021 16:31
@badboy badboy removed the request for review from brizental March 9, 2021 16:37
Copy link
Member

@badboy badboy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awaiting confirmation that it solves the issue.

@cpeterso
Copy link

@cpeterso: Can you confirm this addresses your issue by copying _process_dispatcher.py on this branch to /Users/chris/Code/mozilla/firefox/obj-x86_64-apple-darwin20.3.0-clang-mozbuild/_virtualenvs/init_py3/lib/python3.8/site-packages/glean/?

I copied your new https://github.com/mozilla/glean/blob/95fb6783762608800d70c2e071b665a8c094e8a6/glean-core/python/glean/_process_dispatcher.py to my objdir obj-x86_64-apple-darwin20.3.0-clang-mozbuild/_virtualenvs/init_py3/lib/python3.8/site-packages/glean/_process_dispatcher.py, but I still hit the same Python error:

firefox % mach mozregression
...
 0:00.78 INFO: Running mozilla-central build for 2020-03-09
Traceback (most recent call last):
  File "/Users/chris/Code/mozilla/firefox/obj-x86_64-apple-darwin20.3.0-clang-mozbuild/_virtualenvs/init_py3/lib/python3.8/site-packages/glean/_subprocess/_process_dispatcher_helper.py", line 36, in <module>
    success = func(*args)
TypeError: _process() missing 1 required positional argument: 'throttle_backoff_time_s'

@cpeterso
Copy link

Looks like I just needed to re-run mach bootstrap. So you probably close this issue as WONTFIX. Sorry for the wild goose chase!

I don't have a PYTHONPATH environment variable set, but it looks like mach is picking up glean.py from my mach bootstrap directory here: $HOME/.mozbuild/_virtualenvs/mach/lib/python3.8/site-packages/glean/glean.py. That copy of glean.py still refers to throttle_backoff_time_s.

I reran mach bootstrap and then mach mozregression worked without hitting the throttle_backoff_time_s error.

@cpeterso
Copy link

It would be helpful if mach had a clobber status so it could warn users when they need to re-run mach bootstrap.

@badboy
Copy link
Member

badboy commented Mar 10, 2021

Looks like I just needed to re-run mach bootstrap. So you probably close this issue as WONTFIX. Sorry for the wild goose chase!

I don't have a PYTHONPATH environment variable set, but it looks like mach is picking up glean.py from my mach bootstrap directory here: $HOME/.mozbuild/_virtualenvs/mach/lib/python3.8/site-packages/glean/glean.py. That copy of glean.py still refers to throttle_backoff_time_s.

I reran mach bootstrap and then mach mozregression worked without hitting the throttle_backoff_time_s error.

Hmpf, sounds like bad interaction between the 2 python programs.

It would be helpful if mach had a clobber status so it could warn users when they need to re-run mach bootstrap.

IMO worth reporting on bugzilla. Would also play well with ac_add_options --enable-bootstrap then I guess.
Though I'm not sure it would have helped here because probably no one would have flagged this as clobber-worthy.

@mdboom
Copy link
Contributor Author

mdboom commented Mar 10, 2021

Thanks @cpeterso. It feels like it should at least theoretically be possible to ensure that the code in the subprocess is the same as in the parent process somehow (even if this shot-in-the-dark didn't work). Then we wouldn't have to worry about clobbering issues etc. So, I'll close this PR as WONTFIX, but might keep the parent bug open for a while to investigate further.

@mdboom mdboom closed this Mar 10, 2021
@kvark
Copy link

kvark commented Nov 18, 2021

I was having a similar issue on NixOS, and grabbing the patch appears to fix it.
@mdboom can we land this please?

@badboy badboy reopened this Nov 18, 2021
@badboy badboy force-pushed the fix-throttle-backoff-time-s branch 2 times, most recently from da7f7c2 to a1a443f Compare November 18, 2021 13:29
@badboy badboy force-pushed the fix-throttle-backoff-time-s branch from a1a443f to cbbe413 Compare November 18, 2021 13:34
Copy link
Contributor Author

@mdboom mdboom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is still fine. We had originally chosen to pass on it because we didn't full understand the root cause (and we still don't). But it's definitely helping the minority that falls into this -- we should keep an eye out on any regressions when this lands in m-c, though.l

@badboy badboy merged commit 9b1ffd4 into mozilla:main Nov 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants