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

requirement: Close temporary files before passing them to pip #551

Merged
merged 4 commits into from
Mar 17, 2023

Conversation

tetsuo-cpp
Copy link
Contributor

Closes #548

for filename in self._filenames:
tmp_file = stack.enter_context(NamedTemporaryFile(mode="w"))
# Deliberately pass `delete=False` so that our temporary file doesn't get
# automatically deleted on close. We need to close it so that `pip` can
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm referring to pip here instead of the requirements parser (where the actual error is at the moment) since we're about to remove the parser from this code path.

@tetsuo-cpp
Copy link
Contributor Author

tetsuo-cpp commented Mar 17, 2023

@woodruffw Actually, now that I think of it more, when we remove the requirements parser from this code path, perhaps this issue will disappear since we're no longer opening the file again in the same Python process. I don't think pip opening the file should matter since it's happening in a child process, it's really only the parser that's going to cause this problem.

We can probably merge this to get a patch fix out the door but we can revert these changes as part of #540. Does that seem right to you? I might be able to confirm that #540 fixes the issue by asking the original reporter to try that branch.

@woodruffw
Copy link
Member

We can probably merge this to get a patch fix out the door but we can revert these changes as part of #540. Does that seem right to you? I might be able to confirm that #540 fixes the issue by asking the original reporter to try that branch.

That sounds good to me -- let's merge here, confirm that #540 independently fixes, and then revert here if necessary.

@woodruffw woodruffw added the component:dep-sources Dependency sources label Mar 17, 2023
Signed-off-by: William Woodruff <[email protected]>
@woodruffw
Copy link
Member

LGTM; I'll make a patch release after this.

@woodruffw woodruffw merged commit ed69b62 into main Mar 17, 2023
@woodruffw woodruffw deleted the alex/tmp-file-reopen branch March 17, 2023 14:44
@woodruffw woodruffw mentioned this pull request Mar 17, 2023
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Mar 19, 2023
## [2.5.1]

### Fixed

* Fixed a crash on Windows caused by multiple open file handles to
  input requirements ([#551](pypa/pip-audit#551))

## [2.5.0]

### Changed

* Improved error messaging when a requirements input or indirect dependency
  has an invalid (non-PEP 440) requirements specifier
  ([#507](pypa/pip-audit#507))

* `pip-audit`'s handling of dependency resolution has been significantly
  refactored and simplified ([#523](pypa/pip-audit#523))

### Fixed

* Fixed a potential crash on invalid unicode in subprocess streams
  ([#536](pypa/pip-audit#536))

## [2.4.15]

**YANKED**

### Fixed

* Fixed an issue where hash checking would fail when using third-party indices
  ([#462](pypa/pip-audit#462))

* Fixed the behavior of the `--skip-editable` flag, which had regressed
  with an internal API change
  ([#499](pypa/pip-audit#499))

* Fixed a dependency resolution bug that can potentially be triggered when
  multiple packages have the same subdependency
  ([#488](pypa/pip-audit#488))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:dep-sources Dependency sources
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pip_requirements_parser.InstallationError on Windows
2 participants