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

pip-tools --upgrade --dry-run --output-file requirements.pc should not erase requirements.pc when --dry-run is specified #841

Closed
shipmints opened this issue Jul 3, 2019 · 6 comments · Fixed by #842
Labels
bug Something is not working

Comments

@shipmints
Copy link

Describe the issue briefly here.

Environment Versions
  1. OS Type

$ uname -v
Darwin Kernel Version 16.7.0: Thu Dec 20 21:53:35 PST 2018; root:xnu-3789.73.31~1/RELEASE_X86_64

  1. Python version: $ python -V

$ python -V
Python 3.7.3

  1. pip version: $ pip --version

$ pip --version
pip 19.0.3[snip]

  1. pip-tools version: $ pip-compile --version

$ pip-compile --version
pip-compile, version 3.8.0

Steps to replicate
  1. Run pip-compile --output-file requirements.pc requirements.in
  2. Run pip-compile --upgrade --dry-run --output-file requirements.pc
  3. requirements.pc is recreated as an empty file rather than no effects taking place via --dry-run
Expected result

requirements.pc should be left alone if --upgrade is used with --dry-run

Actual result

requirements.pc is replaced with an empty file

Commentary

I'd first like to say thank you to everyone for this wonderful tool. I appreciate the sheer amount of work and attention to detail. Apologies if this is a repeat issue; I did look to see if this was already reported.

I suspect this behavior is due to click's file handling behavior being left turned on even for --dry-run?

As for why I'm using this mode at all and why other people may not have seen this...

I'm trying to run the command to let our developers know what would be upgraded vs. the existing requirements.pc and my expectation is that I'd be able to use the --output-file - stdout function and pipe to diff vs. the existing requirements.pc file with no damage caused. BUT, because when run with --dry-run no output is created so stdout is empty. When attempting to use the existing file and perhaps run git diff to highlight differences, the file disappears (bad). Another option, and one I'll take for the time being, is to run the upgrade command without --run-run into a dummy file and diff that vs. the real one. Unless someone else has a better idea?

Thanks, again, and warm regards.

@shipmints
Copy link
Author

One thing to add is that we run pip-tools with --quiet to avoid the terminal noise. I should have put --quiet in the examples. I find automatically spewing to stdout kind of ill advised when --output-file is specified.

@atugushev
Copy link
Member

Hello @shipmints,

Thank you for the very detailed report and good catch! I guess that's because the lazy output file has been accidentally opened for some reason. Investigating.

@atugushev atugushev added the bug Something is not working label Jul 4, 2019
@atugushev
Copy link
Member

atugushev commented Jul 4, 2019

Found it. If you add --no-header flag it won't erase output file:

$ pip-compile --upgrade --dry-run --output-file requirements.pc --no-header

Lazy output file opens when pip-compile tries to get output's file name on rendering this header:

#
# This file is autogenerated by pip-compile
# To update, run:
#
#    pip-compile --output-file=requirements.pc requirements.in
#

Here is the line causes the bug:

pip-tools/piptools/utils.py

Lines 341 to 347 in de874f4

# Use a file name for file-like objects
if (
hasattr(value, "write")
and hasattr(value, "read")
and hasattr(value, "name")
):
value = value.name

And here is the output file opens on click's side:

70   class LazyFile(object):
         ...
97       def __getattr__(self, name):
98           return getattr(self.open(), name)

Unfortunately, there is no a proper test for dry-run, that could have helped to catch that bug in advance :(

@shipmints
Copy link
Author

Excellent @atugushev and thank you. There was another bug mentioned in the paragraph at the bottom wherein when one says --dry-run --quiet --output-file - i.e., stdout, it is not honored as it should have been. Removing --quiet obviously reverts to pip-compiles (less than optimal) behavior to dump to stdout. It is precisely the "-" mode I'd prefer so I can avoid creating temporary files for diffing vs. the existing output file. Thanks, again.

@atugushev
Copy link
Member

Excellent @atugushev and thank you. There was another bug mentioned in the paragraph at the bottom wherein when one says --dry-run --quiet --output-file - i.e., stdout, it is not honored as it should have been. Removing --quiet obviously reverts to pip-compiles (less than optimal) behavior to dump to stdout. It is precisely the "-" mode I'd prefer so I can avoid creating temporary files for diffing vs. the existing output file. Thanks, again.

@shipmints i would appreciate if you create a different issue for that bug. Thanks!

@shipmints
Copy link
Author

Good idea. Will do that now. Thank you.

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

Successfully merging a pull request may close this issue.

2 participants