Problem: mr.developer broken by CLI arg to avoid endless loop #536
Conversation
src/zc/buildout/buildout.py
Outdated
| if not __debug__: | ||
| args.insert(0, '-O') | ||
| args.insert(0, sys.executable) | ||
| sys.exit(subprocess.call(args)) | ||
| env = {'BUILDOUT_RESTART_AFTER_UPGRADE': '1'} |
There was a problem hiding this comment.
Shouldn't this be something like
env = dict(os.environ, BUILDOUT_RESTART_AFTER_UPGRADE='1')to also preserve environment from current process variables ?
There was a problem hiding this comment.
@perrinjerome Right, I missed that.
| @@ -2161,8 +2161,6 @@ def main(args=None): | |||
| _help() | |||
| elif orig_op == '--version': | |||
| _version() | |||
| elif orig_op == '--restart-after-upgrade': | |||
| options.append(('buildout', 'restart-after-upgrade', "true")) | |||
There was a problem hiding this comment.
I'm curious as to what happens if a zc.buildout before this patch upgrades itself to a version after this patch. Will the old buildout add --restart-after-upgrade to the command line, causing the new buildout to fail with an unknown command line argument error?
There was a problem hiding this comment.
@mgedmin --restart-after-upgrade only exist in 3.0.0a1 and 3.0.0a2
I feel we do not need to cover those alpha versions. What is your take on that ?
There was a problem hiding this comment.
Good point! I forgot when it was introduced; seemed like ages ago.
|
Do you have a link to a mr.developer bug, or a traceback, or something for us to see how mr. developer fails? |
|
The CI failures I looked at seem to be caused by Python version deprecation warnings. Didn't we have a fix for that in some other PR? Perhaps, since the fix consisted of putting an environment variable asking pip not to print those warnings, the fix may have broken for the reason @perrinjerome noticed: that environment variable is now lost when you replace the entire environment. |
Ah, not yet: #534. Still, the CI fix will need the environment to be preserved. |
|
I added the link to #515 in the description. |
gotcha
force-pushed
the
refactor-no-endless-loop
branch
from
260e77e
to
9a0ae4f
Compare
Solution: use environment variable instead
gotcha
force-pushed
the
refactor-no-endless-loop
branch
from
9a0ae4f
to
0ce2aee
Compare
|
Is there still something needed to be done before merging this PR? |
I second this. Just found this issue rearing its ugly head in my workflows. |
Solution: use environment variable instead
See #515