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

Log a warning if the user specifies -P and the output file is present but empty #1822

Merged
merged 8 commits into from
May 15, 2023
Merged

Log a warning if the user specifies -P and the output file is present but empty #1822

merged 8 commits into from
May 15, 2023

Conversation

davidmreed
Copy link
Contributor

@davidmreed davidmreed commented Mar 14, 2023

When the user runs a command like

pip-compile -P some_package requirements.in

the expected behavior is that only some_package should be updated to a new version.

If the user errs by redirecting pip-compile's stdout to the target file:

pip-compile -P some_package requirements.in > requirements.txt

instead of either using -o or allowing pip-compile to infer the target file name, the target file is truncated by the shell before pip-compile executes. This results in pip-compile actually updating all dependencies, because while the target file is present, it's not a valid source of version constraints.

This PR adds a check in the code to detect this situation, which is almost certainly undesired, and redirect the user to a more appropriate command.

Contributor checklist
  • Provided the tests for the changes.
  • Assure PR title is short, clear, and good to be included in the user-oriented changelog
Maintainer checklist
  • Assure one of these labels is present: backwards incompatible, feature, enhancement, deprecation, bug, dependency, docs or skip-changelog as they determine changelog listing.
  • Assign the PR to an existing or new milestone for the target version (following Semantic Versioning).

@chrysle chrysle added logging Related to log or console output enhancement Improvements to functionality labels May 13, 2023
tests/test_cli_compile.py Outdated Show resolved Hide resolved
Copy link
Contributor

@chrysle chrysle left a comment

Choose a reason for hiding this comment

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

LGTM, but I'd like another person to proof-read this.

@chrysle chrysle requested a review from webknjaz May 14, 2023 10:01
@webknjaz
Copy link
Member

@chrysle no wording suggestions from me, but I've got a few nitpicks on the engineering side of things… Though, they are not mandatory, and it's okay to merge if @davidmreed chooses to leave things as is.

@webknjaz webknjaz enabled auto-merge May 15, 2023 17:44
@webknjaz webknjaz merged commit 41f4b66 into jazzband:main May 15, 2023
@webknjaz webknjaz changed the title Log a warning if the user specifies -P and the output file is present but empty Log a warning if the user specifies -P and the output file is present but empty Jun 13, 2023
@atugushev atugushev added this to the 6.14.0 milestone Jun 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvements to functionality logging Related to log or console output
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants