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.

Sign up for GitHub

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

Jump to bottom

Issue 5180 tempfiles are leak in tmp #5210

Merged
merged 12 commits into from
Aug 3, 2022
Merged

Issue 5180 tempfiles are leak in tmp #5210

Changes from 2 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
d729880
Don't create .json files when getting paths
dqkqd Jul 30, 2022
7453542
Merge branch 'pypa:main' into issue-5180-tempfiles-are-leak-in-tmp
dqkqd Jul 30, 2022
9581ec8
Merge branch 'pypa:main' into issue-5180-tempfiles-are-leak-in-tmp
dqkqd Jul 31, 2022
837292c
Use sysconfig instead of distutils.sysconfig
dqkqd Jul 31, 2022
61ee6e7
Add news fragment.
dqkqd Aug 2, 2022
228672f
Fix linting
dqkqd Aug 2, 2022
48ab50d
Add newline
dqkqd Aug 2, 2022
3a05e37
Remove newline, add dot.
dqkqd Aug 2, 2022
3ca7c96
Remove unnecessary exception handling
dqkqd Aug 2, 2022
029d187
Revert "Remove newline, add dot."
dqkqd Aug 2, 2022
0438677
Revert "Remove unnecessary exception handling"
dqkqd Aug 2, 2022
d99f18e
Revert "Revert "Remove newline, add dot.""
dqkqd Aug 2, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 18 additions & 34 deletions pipenv/environment.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -342,9 +342,8 @@ def build_command(
pylib_lines = []
pyinc_lines = []
py_command = (
"import sysconfig, distutils.sysconfig, io, json, sys; paths = {{"
"%s }}; value = u'{{0}}'.format(json.dumps(paths));"
"fh = io.open('{0}', 'w'); fh.write(value); fh.close()"
"import sysconfig, distutils.sysconfig, io, json, sys; paths = {"
"%s }; value = u'{0}'.format(json.dumps(paths)); print(value)"
)
distutils_line = "distutils.sysconfig.get_python_{0}(plat_specific={1})"
sysconfig_line = "sysconfig.get_path('{0}')"
Expand All @@ -354,24 +353,24 @@ def build_command(
# XXX: We need to get 'stdlib' or 'platstdlib'
sys_prefix = "{}stdlib".format("" if key == "pure" else key)
pylib_lines.append(
f"u'{dist_prefix}': u'{{{{0}}}}'.format({distutils_line.format(var, val)})"
f"u'{dist_prefix}': u'{{0}}'.format({distutils_line.format(var, val)})"
)
pylib_lines.append(
f"u'{sys_prefix}': u'{{{{0}}}}'.format({sysconfig_line.format(sys_prefix)})"
f"u'{sys_prefix}': u'{{0}}'.format({sysconfig_line.format(sys_prefix)})"
)
if python_inc:
for key, var, val in (("include", "inc", "0"), ("platinclude", "inc", "1")):
pylib_lines.append(
f"u'{key}': u'{{{{0}}}}'.format({distutils_line.format(var, val)})"
f"u'{key}': u'{{0}}'.format({distutils_line.format(var, val)})"
)
lines = pylib_lines + pyinc_lines
if scripts:
lines.append(
"u'scripts': u'{{0}}'.format(%s)" % sysconfig_line.format("scripts")
"u'scripts': u'{0}'.format(%s)" % sysconfig_line.format("scripts")
)
if py_version:
lines.append(
"u'py_version_short': u'{{0}}'.format(distutils.sysconfig.get_python_version()),"
"u'py_version_short': u'{0}'.format(distutils.sysconfig.get_python_version()),"
)
lines_as_str = ",".join(lines)
py_command = py_command % lines_as_str
Expand All @@ -384,18 +383,13 @@ def get_paths(self) -> Optional[Dict[str, str]]:
:return: The python paths for the environment
:rtype: Dict[str, str]
"""
tmpfile = vistir.path.create_tracked_tempfile(suffix=".json")
Copy link
Contributor

Choose a reason for hiding this comment

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

Bonus points for removing usage of vistir 💯

tmpfile.close()
tmpfile_path = make_posix(tmpfile.name)
py_command = self.build_command(
python_lib=True, python_inc=True, scripts=True, py_version=True
)
command = [self.python, "-c", py_command.format(tmpfile_path)]
command = [self.python, "-c", py_command]
c = subprocess_run(command)
if c.returncode == 0:
paths = {}
with open(tmpfile_path, "r", encoding="utf-8") as fh:
paths = json.load(fh)
paths = json.loads(c.stdout)
if "purelib" in paths:
paths["libdir"] = paths["purelib"] = make_posix(paths["purelib"])
for key in (
Expand All @@ -420,17 +414,12 @@ def get_lib_paths(self) -> Dict[str, str]:
:return: The python include path for the environment
:rtype: Dict[str, str]
"""
tmpfile = vistir.path.create_tracked_tempfile(suffix=".json")
tmpfile.close()
tmpfile_path = make_posix(tmpfile.name)
py_command = self.build_command(python_lib=True)
command = [self.python, "-c", py_command.format(tmpfile_path)]
command = [self.python, "-c", py_command]
c = subprocess_run(command)
paths = None
if c.returncode == 0:
paths = {}
with open(tmpfile_path, "r", encoding="utf-8") as fh:
paths = json.load(fh)
paths = json.loads(c.stdout)
if "purelib" in paths:
paths["libdir"] = paths["purelib"] = make_posix(paths["purelib"])
for key in ("platlib", "platstdlib", "stdlib"):
Expand Down Expand Up @@ -476,22 +465,17 @@ def get_include_path(self) -> Optional[Dict[str, str]]:
:return: The python include path for the environment
:rtype: Dict[str, str]
"""
tmpfile = vistir.path.create_tracked_tempfile(suffix=".json")
tmpfile.close()
tmpfile_path = make_posix(tmpfile.name)
py_command = (
"import distutils.sysconfig, io, json, sys; paths = {{u'include': "
"u'{{0}}'.format(distutils.sysconfig.get_python_inc(plat_specific=0)), "
"u'platinclude': u'{{0}}'.format(distutils.sysconfig.get_python_inc("
"plat_specific=1)) }}; value = u'{{0}}'.format(json.dumps(paths));"
"fh = io.open('{0}', 'w'); fh.write(value); fh.close()"
"import distutils.sysconfig, io, json, sys; paths = {u'include': "
Copy link
Member

Choose a reason for hiding this comment

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

I realize you are just refactoring existing code, but now I notice this usage of distutils which is totally deprecated. I wonder what the correct transformation of this usage would be.

Copy link
Contributor Author

@dqkqd dqkqd Jul 30, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Instead of writing to a file, I refactor so it print out to the stdout directly.
Because the filename should not be passed to string as variable anymore, I deleted some unnecessary curly braces.
The ouptut code should look like this

import distutils.sysconfig, io, json, sys 
paths = {
    u'include': u'{0}'.format(distutils.sysconfig.get_python_inc(plat_specific=0)), 
    u'platinclude': u'{0}'.format(distutils.sysconfig.get_python_inc(plat_specific=1)) 
}
value = u'{0}'.format(json.dumps(paths))
print(value)

Sorry, I was copied the ouptut from build_command instead of get_include_path

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As you said, I tried to remove distutils.sysconfig and use sysconfig instead.

Copy link
Contributor Author

@dqkqd dqkqd Jul 31, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Since sysconfig.get_path does not raise exception like distutils.sysconfig, the c.returncode is always 0. Therefore, those lines below could be refactored

pipenv/pipenv/environment.py

Lines 393 to 415 in 75b77eb

command = [self.python, "-c", py_command.format(tmpfile_path)]
c = subprocess_run(command)
if c.returncode == 0:
paths = {}
with open(tmpfile_path, "r", encoding="utf-8") as fh:
paths = json.load(fh)
if "purelib" in paths:
paths["libdir"] = paths["purelib"] = make_posix(paths["purelib"])
for key in (
"platlib",
"scripts",
"platstdlib",
"stdlib",
"include",
"platinclude",
):
if key in paths:
paths[key] = make_posix(paths[key])
return paths
else:
click.secho(f"Failed to load paths: {c.stderr}", fg="yellow")
click.secho(f"Output: {c.stdout}", fg="yellow")
return None

"u'{0}'.format(distutils.sysconfig.get_python_inc(plat_specific=0)), "
"u'platinclude': u'{0}'.format(distutils.sysconfig.get_python_inc("
"plat_specific=1)) }; value = u'{0}'.format(json.dumps(paths));"
"print(value)"
)
command = [self.python, "-c", py_command.format(tmpfile_path)]
command = [self.python, "-c", py_command]
c = subprocess_run(command)
if c.returncode == 0:
paths = []
with open(tmpfile_path, "r", encoding="utf-8") as fh:
paths = json.load(fh)
paths = json.loads(c.stdout)
for key in ("include", "platinclude"):
if key in paths:
paths[key] = make_posix(paths[key])
Expand Down