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

Prevent sessions from modifying each other's posargs #439

Merged
merged 5 commits into from
Jun 6, 2021

Conversation

cjolowicz
Copy link
Collaborator

@cjolowicz cjolowicz commented Jun 5, 2021

Fixes #400
Follow-up to #397

This PR gives each session its own copy of posargs, thereby preventing mutations in one session to affect posargs in another session.

import nox

@nox.session
def test(session):
    session.posargs.extend(["-x"])
    session.install("pytest")
    session.run("pytest", *session.posargs)

@nox.session
def lint(session):
    args = [*session.posargs, "src"]
    session.install("flake8")
    session.run("flake8", *args)

# flake8: error: unrecognized arguments: -x

A test is added to check that posargs does not bleed through between sessions. Some existing tests are updated to fix breakage due to the change. There are three groups of tests that broke:

  • Tests that compare posargs using is instead of ==
  • Tests that construct a namespace manually without including posargs
  • Tests that construct a namespace manually and set posargs to a mock sentinel

Rationale:

#397 introduced the ability to invoke session.notify with posargs, allowing one session to schedule another session with its own set of positional arguments. As a consequence, every session conceptually now has its own posargs; these arguments are no longer necessarily those passed on the command-line. Therefore it makes sense to prevent one session from accidentally mutating the posargs of other sessions.

session.posargs was already mutable before #397. But as all sessions received the same set of positional arguments, typical uses of positional arguments would restrict the run to a specific session. In such cases, mutating the shared copy of posargs would not lead to issues.

While it's conceivable that users would intentionally modify posargs in one session to affect another session, this technique would rely on an undocumented implementation detail. So I don't see a backwards compatibility issue here.

Rejected Ideas:

Using a tuple for session.posargs. This would break existing use patterns which rely on session.posargs supporting list operations, for example in the pip project. See #400 (comment) for more details.

@theacodes theacodes merged commit bcaa5ff into wntrblm:main Jun 6, 2021
@cjolowicz cjolowicz deleted the posargs-copy branch June 6, 2021 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Session.posargs should be immutable
2 participants