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

Do not set TMPDIR when exporting to requirements.txt #298

Merged
merged 4 commits into from
Mar 14, 2021
Merged

Conversation

cjolowicz
Copy link
Owner

@cjolowicz cjolowicz commented Mar 13, 2021

Do not set or modify TMPDIR in Nox sessions. nox-poetry should not modify the environment of users. Fiddling with TMPDIR can also lead to issues with other tools, especially since it happened at unpredictable points in the build process.

Previously, nox-poetry set TMPDIR indirectly by invoking session.create_tmp from export_requirements. We continue to use <venv>/tmp but create it ourselves instead of using session.create_tmp.

Closes #249

@cjolowicz cjolowicz added the bug Something isn't working label Mar 13, 2021
Base automatically changed from master to main March 13, 2021 14:06
@cjolowicz cjolowicz force-pushed the tmpdir-fix branch 2 times, most recently from f4ec5de to 0acc3e7 Compare March 13, 2021 14:54
@brechtm
Copy link

brechtm commented Mar 14, 2021

Previously, nox-poetry set TMPDIR indirectly by invoking session.create_tmp from export_requirements. We continue to use /tmp but create it ourselves instead of using session.create_tmp.

Just a small note. TMPDIR used to be .nox/tmp. Now it's .nox/<session>/tmp.

This was referenced Mar 15, 2021
@cjolowicz
Copy link
Owner Author

cjolowicz commented Mar 17, 2021

Previously, nox-poetry set TMPDIR indirectly by invoking session.create_tmp from export_requirements. We continue to use /tmp but create it ourselves instead of using session.create_tmp.

Just a small note. TMPDIR used to be .nox/tmp. Now it's .nox/<session>/tmp.

@brechtm I don't think so. Try this with the previous version:

from nox_poetry import session

@session
def test(session):
    session.poetry.export_requirements()
    session.run("python", "-c", """import os; print(f"{os.environ['TMPDIR'] = }")""")

Here's the output I get:

os.environ['TMPDIR'] = '.nox/test/tmp'

We just used nox.Session.create_tmp which uses .nox/<session>/tmp.

@brechtm
Copy link

brechtm commented Mar 18, 2021

Ah, yes. During my testing I adjusted the nox-poetry code to create and use the .nox/tmp directory, which I why I remember seeing it :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Installation of sdist is (silently) failing
2 participants