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

Configure Renovate #654

Merged
merged 7 commits into from
Sep 13, 2018
Merged

Configure Renovate #654

merged 7 commits into from
Sep 13, 2018

Conversation

renovate[bot]
Copy link

@renovate renovate bot commented Sep 8, 2018

Welcome to Renovate! This is an onboarding PR to help you understand and configure settings before regular Pull Requests begin.

🚦 To activate Renovate, merge this Pull Request. To disable Renovate, simply close this Pull Request unmerged.


Detected Package Files

  • ci/rtd-requirements.txt (pip_requirements)
  • test-requirements.txt (pip_requirements)

Configuration

🔡 Renovate has detected a custom config for this PR. Feel free to post it to the Config Help repository if you have any doubts and would like it reviewed.

⚠️ This PR has a merge conflict, however Renovate is unable to automatically fix that due to edits in this branch. Please resolve the merge conflict manually.

What to Expect

With your current configuration, Renovate will create 1 Pull Request:


Update dependency sphinx to v1.8.0

  • Branch name: renovate/sphinx-1.x
  • Upgrade sphinx to ==1.8.0

❓ If you have any questions, try reading the Docs, particularly the Getting Started section.
Also, you can post questions about your config in Renovate's Config Help repository.


This PR has been generated by Renovate Bot.

pquentin
pquentin previously approved these changes Sep 8, 2018
Copy link
Member

@pquentin pquentin left a comment

Choose a reason for hiding this comment

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

I think we can try the default settings for now and reduce the noise later when we better understand how renovate affects us?

@renovate renovate bot force-pushed the renovate/configure branch from 6b695b4 to e7d973f Compare September 9, 2018 00:44
@codecov
Copy link

codecov bot commented Sep 9, 2018

Codecov Report

Merging #654 into master will decrease coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #654      +/-   ##
==========================================
- Coverage   99.31%   99.31%   -0.01%     
==========================================
  Files          94       94              
  Lines       11014    11002      -12     
  Branches      790      789       -1     
==========================================
- Hits        10939    10927      -12     
  Misses         56       56              
  Partials       19       19
Impacted Files Coverage Δ
trio/_core/_ki.py 98.36% <0%> (-0.03%) ⬇️
trio/_core/_multierror.py 99.37% <0%> (-0.02%) ⬇️
trio/tests/test_ssl.py 100% <0%> (ø) ⬆️
trio/_core/tests/test_multierror.py 100% <0%> (ø) ⬆️
trio/testing/_checkpoints.py 100% <0%> (ø) ⬆️
trio/_ssl.py 100% <0%> (ø) ⬆️
trio/_core/_run.py 99.7% <0%> (ø) ⬆️
trio/tests/test_signals.py 100% <0%> (ø) ⬆️
trio/_socket.py 100% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ca14ffe...ca50541. Read the comment docs.

@njsmith
Copy link
Member

njsmith commented Sep 9, 2018

Well, I enabled automerge: true anyway, so at least we won't have to manually click the button each time a new package is released (just like we don't right now).

Before we can merge this I think we need to at least:

  • Tell travis and appveyor not to build renovate/* branches in the main repo
  • Extend our requirements files to have a complete list of packages + pins (maybe pip-tools can do this?)

@Fuyukai
Copy link
Member

Fuyukai commented Sep 10, 2018

I note this doesn't support setup.py dependencies.

@njsmith
Copy link
Member

njsmith commented Sep 10, 2018

I note this doesn't support setup.py dependencies.

Right, this is a limitation of all services like this, because there's no way to get those without actually executing setup.py. Hopefully someday python will have a somewhat less tangled way of handling this kind of package metadata, but in the mean time the workaround is that we list our general runtime dependencies in setup.py, and then list the exact versions that we want to test against in CI in a requirements file. So there's some redundancy, but it seems manageable. (And if we forget to add something to the requirements file, then we just end up testing against the latest version, not a big deal.)

@pquentin
Copy link
Member

Using pip-compile from pip-tools is the best way to pin all indirect dependencies, but I think we need to make sure that renovate runs pip-compile itself? For example, say dependency A starts depending on dependency A1: you want to pin both A and A1 when we start depending on a new A version, and that's only possible with a new pip-compile run.

If that's not an option, I guess only pinning direct dependencies is a better compromise?

@pquentin
Copy link
Member

From the todo list:

Tell travis and appveyor not to build renovate/* branches in the main repo

I've added 8289d5b to this effect. renovate/* branches are now ignored in all repos. I don't think this can be configured per repository, but there's no reason to have renovate/* branches in forks, so that's OK?

@pquentin
Copy link
Member

I think the coverage failure comes from #656. Maybe some kind of race condition where the coverage result of merging #656 had not been taken into account yet when computing the coverage of this PR?

@njsmith
Copy link
Member

njsmith commented Sep 10, 2018

Yeah, codecov can get very confused when one PR is merged while another's CI is running. (Partly this is the fault of the CI services: they actually pick a base revision for each job at the time the job starts, so if master moves while the CI is running, then you can end up with e.g. the py35 and the py36 jobs actually testing different trees...) We know nothing here should be touching coverage and the codecov results make no sense, so oh well, ignore it. There's a reason codecov isn't marked as mandatory for merge :-)

Using pip-compile from pip-tools is the best way to pin all indirect dependencies, but I think we need to make sure that renovate runs pip-compile itself? For example, say dependency A starts depending on dependency A1: you want to pin both A and A1 when we start depending on a new A version, and that's only possible with a new pip-compile run.

Interesting point. I don't know if renovate (or any similar tools) pull in new pins when necessary. But, it's also not the end of the world if we have some deps missing from our requirements file... basically it just means that we continue to automatically test the latest version, and are at risk of being broken by their releases. Either this never happens, and it's fine, or else it eventually happens, and then we notice and add them to the requirements.txt-file-of-shame :-)

@Zac-HD
Copy link
Member

Zac-HD commented Sep 11, 2018

Re: pip-compile - Hypothesis has a test in CI that running pip-compile doesn't change our pinned dependencies, which keeps everything in sync. (we also automatically push any changes back to the PR if it's coming from pyup 😉)

@njsmith
Copy link
Member

njsmith commented Sep 11, 2018

@Zac-HD that does sound like some fancy tech! should we delegate all this to you? :-)

@pquentin
Copy link
Member

Yeah, what Hypothesis is doing is impressive. The 'push back to the PR magic' PR is HypothesisWorks/hypothesis#1562.

So I moved test-requirements.txt and ci/rtd-requirements.txt to the pip-compile format, but maybe we should keep a .in version in order to 1/ keep track of the direct dependencies and 2/ keep the comments we had until now? Of course, that's what Hypothesis does: https://github.com/HypothesisWorks/hypothesis/tree/master/requirements :)

And yeah, I would be happy to remove my commits here and let @Zac-HD work on this! 😄

@Zac-HD
Copy link
Member

Zac-HD commented Sep 11, 2018

  • Yes to the .in requirements files, they're great.
  • Don't bother with push-to-PR until it's actually needed; we didn't for more than a year.
  • The Hypothesis system is... baroque (and not my code), so I'm very happy to explain tactics that worked there and leave the implementation to someone else 😄
    (i.e you don't get out of it that easy, @pquentin)

@pquentin
Copy link
Member

pquentin commented Sep 12, 2018

@njsmith any preferences regarding the location of .in and .txt requirement files?

Oh, and the PyPy builds have all failed with:

  Running setup.py bdist_wheel for typed-ast ... error
  Complete output from command /home/travis/build/python-trio/trio/testenv/bin/python -u -c "import setuptools, tokenize;__file__='/tmp/pip-install-e2kfi_hj/typed-ast/setup.py';f=getattr(tokenize, 'open', open)(__file__);code=f.read().replace('\r\n', '\n');f.close();exec(compile(code, __file__, 'exec'))" bdist_wheel -d /tmp/pip-wheel-dd9twx1x --python-tag pp361:
  running bdist_wheel
  running build
  running build_py
  creating build
  creating build/lib.linux-x86_64-3.6
  creating build/lib.linux-x86_64-3.6/typed_ast
  copying typed_ast/__init__.py -> build/lib.linux-x86_64-3.6/typed_ast
  copying typed_ast/ast27.py -> build/lib.linux-x86_64-3.6/typed_ast
  copying typed_ast/ast3.py -> build/lib.linux-x86_64-3.6/typed_ast
  copying typed_ast/conversions.py -> build/lib.linux-x86_64-3.6/typed_ast
  running build_ext
  building '_ast27' extension
  creating build/temp.linux-x86_64-3.6
  creating build/temp.linux-x86_64-3.6/ast27
  creating build/temp.linux-x86_64-3.6/ast27/Parser
  creating build/temp.linux-x86_64-3.6/ast27/Python
  creating build/temp.linux-x86_64-3.6/ast27/Custom
  gcc -pthread -DNDEBUG -O2 -fPIC -Iast27/Include -I/home/travis/build/python-trio/trio/pypy-c-jit-95085-dd78db026ae0-linux64/include -c ast27/Parser/acceler.c -o build/temp.linux-x86_64-3.6/ast27/Parser/acceler.o
  ast27/Parser/acceler.c:13:25: fatal error: pgenheaders.h: No such file or directory
   #include "pgenheaders.h"
                           ^
  compilation terminated.
  error: command 'gcc' failed with exit status 1
  ----------------------------------------
  Failed building wheel for typed-ast

...

  Running setup.py install for typed-ast ... error
    Complete output from command /home/travis/build/python-trio/trio/testenv/bin/python -u -c "import setuptools, tokenize;__file__='/tmp/pip-install-e2kfi_hj/typed-ast/setup.py';f=getattr(tokenize, 'open', open)(__file__);code=f.read().replace('\r\n', '\n');f.close();exec(compile(code, __file__, 'exec'))" install --record /tmp/pip-record-su__se3r/install-record.txt --single-version-externally-managed --compile --install-headers /home/travis/build/python-trio/trio/testenv/include/site/python3.6/typed-ast:
    running install
    running build
    running build_py
    creating build
    creating build/lib.linux-x86_64-3.6
    creating build/lib.linux-x86_64-3.6/typed_ast
    copying typed_ast/__init__.py -> build/lib.linux-x86_64-3.6/typed_ast
    copying typed_ast/ast27.py -> build/lib.linux-x86_64-3.6/typed_ast
    copying typed_ast/ast3.py -> build/lib.linux-x86_64-3.6/typed_ast
    copying typed_ast/conversions.py -> build/lib.linux-x86_64-3.6/typed_ast
    running build_ext
    building '_ast27' extension
    creating build/temp.linux-x86_64-3.6
    creating build/temp.linux-x86_64-3.6/ast27
    creating build/temp.linux-x86_64-3.6/ast27/Parser
    creating build/temp.linux-x86_64-3.6/ast27/Python
    creating build/temp.linux-x86_64-3.6/ast27/Custom
    gcc -pthread -DNDEBUG -O2 -fPIC -Iast27/Include -I/home/travis/build/python-trio/trio/pypy-c-jit-95085-dd78db026ae0-linux64/include -c ast27/Parser/acceler.c -o build/temp.linux-x86_64-3.6/ast27/Parser/acceler.o
    ast27/Parser/acceler.c:13:25: fatal error: pgenheaders.h: No such file or directory
     #include "pgenheaders.h"
                             ^
    compilation terminated.
    error: command 'gcc' failed with exit status 1

And indeed, astroid (a pylint dependency) only depends on typed_ast with CPython: https://github.com/PyCQA/astroid/blob/6acd49c33e0143da5ab8aaf8caca49d1d564467c/astroid/__pkginfo__.py#L33. I guess pip-compile should have been able to pick that up?

@njsmith
Copy link
Member

njsmith commented Sep 12, 2018

No strong preferences on the .in files... they can go next to the .txt files I guess, unless someone has a better idea?

That is unfortunate that pip-compile didn't preserve the environment marker... reading issues like jazzband/pip-tools#635 , it sounds like it is intended to preserve it? Maybe if we manually add it to requirements.in, and then re-run it?

@pquentin
Copy link
Member

@Zac-HD okay, at least we tried. 😛

@njsmith Yeah, this is noted in the pip-tools 2.0.0 release notes:

The pip environment markers on top-level requirements in the source file (requirements.in) are now properly handled and will only be processed in the right environment (#647). Thanks @JoergRittinger

So, yeah, thanks @JoergRittinger. :)

I added typed_ast with its environment markers in the .in file. This has fixed the PyPy builds. Please take a look.

Copy link
Member

@Zac-HD Zac-HD left a comment

Choose a reason for hiding this comment

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

I think we need to run pip-compile in CI, but otherwise looking good to me.

@@ -0,0 +1,11 @@
pytest == 3.7.4 # for catchlog fixture
Copy link
Member

Choose a reason for hiding this comment

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

I think this pin is an artifact from recent pytest trouble; and should just be >= 3.3.

Copy link
Member

Choose a reason for hiding this comment

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

Wow, nice catch! Fixed. I made sure there were no others invalid changes like that in the .in files. Thanks.


branches:
except:
- /renovate/*/
Copy link
Member

Choose a reason for hiding this comment

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

Problem here, actually: if we skip CI for these branches, we'll update to incompatible versions of our dependencies too!

We should just delete the exclusion. Ideally we would also rewrite our CI scripts to use the pinned versions everywhere and check they're consistent, but you might want to leave that to a future PR (or even push them onto me 😉)

Copy link
Member

Choose a reason for hiding this comment

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

The problem is that renovate puts these branches directly into the main repo, so the CI services want to run CI twice – once b/c it's a branch in the main repo, and a second time b/c it's a PR. I think this should eliminate the first CI, but keep the second?

Copy link
Member

Choose a reason for hiding this comment

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

@Zac-HD what @njsmith says. :) Notice that the PR is indeed tested below, but the branch itself is not.

Copy link
Member

Choose a reason for hiding this comment

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

...Derp, yeah, that makes sense. (in my defence- no, wait, pyup does the same 🤦‍♂️)

@pquentin pquentin dismissed their stale review September 12, 2018 07:56

code changed

@pquentin
Copy link
Member

(I'd be happy to leave the pip-compile check in CI for another PR.)

@njsmith njsmith merged commit 0d3dd6d into master Sep 13, 2018
@njsmith njsmith deleted the renovate/configure branch September 13, 2018 06:31
@njsmith
Copy link
Member

njsmith commented Sep 13, 2018

OK, let's see what happens :-)

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

Successfully merging this pull request may close these issues.

5 participants