-
Notifications
You must be signed in to change notification settings - Fork 590
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
Pyup pulls fail when pip-compile changes whitespace #747
Comments
@DRMacIver, can you simply disable Pyup? It often causes conflict with the check-requirements job, and that job will ensure that everything is in order anyway whenever a PR is merged. I mean, we currently have four broken pulls by pyup and that's just silly. |
I propose we just disable check-requirements job entirely and deal with the misaligned comments (does anyone ever manually look at those files anyway?). It's not buying us anything that pyup doesn't already give us. We included it on a "might as well" basis but it turns out to be a bit of a pain. |
Ugh, right. OK. So here's a thing we could do: The pyup bot creates branches in this repo. We have the functionality to make changes from travis already. We could have check-requirements verify if it's on a pyup branch (easy to do: Their name starts with pyup) and if so rebuild and push the changed requirements file. Does that sound reasonable? |
The requirements check for #913 is now failing because of a Pyup/pip-compile clash, and only four days since the last instance in #905. IMO that makes it time to implement @DRMacIver's suggestion above - if the check-requirements job fails on a branch starting with |
Related, but I thought I'd changed pyup to weekly batching and it doesn't seem to have taken if so. |
...having taken a closer look at the commit-and-push proposal, I don't think we can do that in a way that doesn't expose commit access to someone sneaky who opens their own pull. This also hasn't been much of a problem since we went to weekly updates, so I suggest we just deal with it for a few minutes a month. |
I think we can! A distinguishing feature that prevents this is that the pyup branches are on this repo rather than an external fork, so anyone who can push to such a branch already has commit access. That being said, you and @alexwlchan are so much more on the ball than I am about picking up these pulls that it doesn't really matter to me either way, so I'm happy to leave it up to you. |
TBH I think it's less maintenance effort to do it by hand than to keep it working automatically, so there we are. The problem with checking branches is that any such protection would be defined in configuration that lives in this repo, and thus subject to change by a malicious pull. Or at least that's my understanding, and I'm not keen to get clever with security. |
Hey team, I saw your issue over a Pyup, and thought I'd chime in. I built Dependabot, a similar service that already has the ability to execute arbitrary code safely. As a result, I've been able to add Pipfile support for Python pretty straightforwardly, and am about to add pip-compile support. I'm going to use the setup in this repo as a reference point, so Dependabot will definitely work with it. Would you be up for giving it a try when it's done (should be tomorrow or over the weekend)? The service is already used by a few thousand people, including GitHub themselves. |
I'm certainly keen to give it a go. Especially given the current problems we're having where pyup's requirements updating completely ignores version compatibility issues and we have to manually run pip-compile each week ourselves anyway! |
Awesome! I just shipped support and ran it on my fork - looks like everything's working as expected so you should be good to go if you want to give it a try. |
See #738, #743, #745 for currently-open examples.
The short version is that when @pyup-bot edits the version number of a dependency, it doesn't account for the fixed-indent comment added by
pip-compile
. The check-requirements job therefore fails for updates that change the number of characters in a version identifier (eg 1.0.9 -> 1.0.10) because there is a diff when the spacing is restored.Our options seem to be (a) keep fixing this manually and hope it goes away, (b) ignore whitespace in the git diff for this check and accept uneven comments, (c) ask Pyup to recognize and handle pip-compile output, or (d) use something other than Pyup for version checks (eg a cron-job running pip-compile).
Assigned to @DRMacIver as he has both the relevant credentials and final decision on this kind of policy.
Update: we have another failure mode - if A requires B in a certain range, Pyup will try to update B to the latest version regardless. Ref: #945.
The text was updated successfully, but these errors were encountered: