Perform 'blurb split' for master. #2719

larryhastings · 2017-07-15T14:27:02Z

No description provided.

brettcannon

A quick perusal of the output LGTM.

brettcannon · 2017-07-15T17:21:16Z

It looks like Sphinx snuck in a new deprecation warning in Sphinx 1.6.3 which this is triggering. If you can easily fix it by replacing any use of sphinx.util.compat.Directive with docutils.parsers.rst.Directive (I think) then please do, else pin Sphinx to 1.6.2 in the Travis configuration.

brettcannon · 2017-07-15T17:23:51Z

And I think the culprit might be

cpython/Doc/tools/extensions/pyspecific.py

Line 26 in 272d888

from sphinx.util.compat import Directive

, but that's just from a quick search for [Directive].

ned-deily · 2017-07-15T19:25:58Z

For the deprecation warning, I documented the issue in bpo-30939 and PR #2721 (for master), also needed in other branches.

ned-deily · 2017-07-15T19:36:30Z

LIke Brett, I didn't thoroughly review the changes but, in general, LGTM. One concern: as is, making this change will immediately break all doc builds of Doc/whatsnew/changelog.rst (for example, https://docs.python.org/dev/whatsnew/changelog.html). Are you planning to change Doc/Makefile to automatically do a 'blurb' run? I think we need to.

ned-deily · 2017-07-15T20:34:14Z

I tried to restart the Travis doc build but it is now getting a 3/333 unused rules warning for make suspicious which is suspicious because I don't get that when doing a clean make suspicious locally.

…-split-master

larryhastings · 2017-07-16T12:08:56Z

So... make suspicious is itself suspicious? GREAT SUCCESS

I experimented with having Doc/Makefile run blurb merge. That works fine, but it's a bit uncomfortable having the doc build process output a file in ../Misc. I propose to change the documentation so blurb merge outputs to build/NEWS and update the references (e.g. whatsnew/changelog.rst).

This does make the Doc build process dependent on blurb, which I hadn't anticipated. But that's a natural enough outcome of the process. And we can easily update the make venv target of the doc makefile to also install blurb. I don't think this is a big deal.

larryhastings · 2017-07-16T14:28:57Z

@brettcannon: It looks to me like Travis built against an old version. Did I hit some sort of workflow bug?

I changed Doc/Makefile and pushed. This was local revision dc85bd9, which AFAICT was auto-merged on GitHub to revision 8163ffa44849 . This kicked off Travis CI build 7463 ( https://travis-ci.org/python/cpython/builds/254150043 ).

The build didn't work because I had to fix .travis.yaml too. So I pushed a second change, which on my side is revision a8f8d57 . That kicked off Travis CI build 7464 ( https://travis-ci.org/python/cpython/builds/254151401 ). But Travis build 7464 was run against 8163ffa44849 ! Both Travis builds 7463 and 7464 were based on the same revision.

What's strange is that on Travis's "requests" dashboard: https://travis-ci.org/python/cpython/requests
it shows the latest checkin comment for build 7464. But both builds were clearly based on the same revision. It even shows you that in the requests dashboard.

What's going on?

I may try to fool Travis into running another build by uploading a no-op revision.

vstinner · 2017-07-16T22:40:46Z

The docs job fails on Travis CI because this job only clones the last 50 commits:

git clone --depth=50 https://github.com/python/cpython.git python/cpython

whereas blurb looks for a specific commit:

run("git log -r 7f777ed95a19224294949e1b4ce56bbffcb1fe9f")

I don't think that we can change how Travis CI clones a Git repository, so we should change how blurb checks if if's running in a Git repository. Maybe it should catch subprocess.SubprocessError on:

git_dir = run("git rev-parse --git-dir").strip()

To check if it's running in Git or not?

vstinner

LGTM.

brettcannon · 2017-07-17T21:58:28Z

Looks like @Haypo spotted why the revision check is failing. A bit annoying but understandable since Travis doesn't want to pay the CPU/network costs of downloading the entire CPython repo.

As for having the build process depend on blurb, that's just life and I have no issue with it since we already depend on sphinx. If it becomes a really serious problem we can talk about just putting blurb into the cpython repo to essentially vendor blurb in Python itself.

ned-deily

Not surprisingly, trying to add blurb into the doc build has twisty little passages. I ran into two major problems reviewing and testing this:

At the moment, the most recent version of blurb that is published on PyPI and the first one that works as expected was published as 1.0.2.dev1. Because it's a dev version, by default pip won't try to install it and falls back to the last "production" release 1.0.1.post1 which has bugs. I think the best solution is to specify a version >=1.0.2dev1 with Travis (not tested) and pip.
Installing blurb into the venv produced by make venv is good except that the rest of the Doc Makefile does not use the venv by default. The (undocumented, other than in the original checkin message d5ea39d) intended use was to then explicitly pass the venv Python into Doc makefile targets by setting the PYTHON variable, in part, because trying to use venv activate in Makefile recipes can be problematic. But even then, the blurb command isn't found because using PYTHON= doesn't put the venv bin directory (where the blurb script is installed) on the process PATH. I've suggested one way around it which is to invoke blurb via $(PYTHON) -m blurb; that way, the lack of blurb sort of fails the same way the lack of sphinx-build will. It might be nice to fail with a more informative message. I think it mandatory to document the venv target in Doc/README.rst and something about the need to use PYTHON=venv/bin/python when using venv. BTW, it appears that the 2.7 Doc/Makefile doesn't have a venv target so that will have to be backported for the 2.7 PR.

ned-deily · 2017-07-19T07:33:51Z

.travis.yml

@@ -42,6 +42,7 @@ matrix:
        # Sphinx is pinned so that new versions that introduce new warnings won't suddenly cause build failures.
        # (Updating the version is fine as long as no warnings are raised by doing so.)
        - python -m pip install sphinx~=1.6.1
+        - python -m pip install blurb


Suggest something like:
- python -m pip install 'blurb>=1.0.2dev1'

@larryhastings did you push this version? If so, was keeping the dev version on purpose or an accident? If the latter we can just fix the version number and do another release as 1.0.2 so the dependency can be >=1.0.2.

I pushed this version. I don't remember what I changed. I didn't keep the "dev" version on purpose, but then again it is still a little bit under development.

Blurb is under development but is the 1.0.2 release specifically under development? My assumption is you mean the former and not the latter, which means I can push a 1.0.2 release so that the changes in that bugfix release go public because right now no one is getting access since pip considers a dev release unusable without opting in.

ned-deily · 2017-07-19T07:35:38Z

Doc/Makefile

@@ -38,6 +38,8 @@ help:
 	@echo "  serve      to serve the documentation on the localhost (8000)"

 build:
+	-mkdir build


A more common idiom for this is mkdir -p build. That has the advantage of not producing any messages if the directory already exists.

ned-deily · 2017-07-19T07:49:01Z

Doc/Makefile

@@ -38,6 +38,8 @@ help:
 	@echo "  serve      to serve the documentation on the localhost (8000)"

 build:
+	-mkdir build
+	blurb merge -f build/NEWS


To get around the PATH problem, suggest:

Adding
BLURB = $(PYTHON) -m blurb
after line 8 (SPHINXBUILD = sphinx-build) in the definitions at the top of the file and then

changing the line above to:
$(BLURB) merge -f build/NEWS
That way, if users do a make venv ; make html PYTHON=venv/bin/python, blurb will be found there but, if they need to use a different version of blurb, they can override it: make html BLURB=/usr/local/bin/blurb.

ned-deily · 2017-07-19T07:52:54Z

Doc/Makefile

@@ -107,7 +109,7 @@ clean:

 venv:
 	$(PYTHON) -m venv venv
-	./venv/bin/python3 -m pip install -U Sphinx
+	./venv/bin/python3 -m pip install -U Sphinx blurb


As above, suggest setting a version requirement here:
./venv/bin/python3 -m pip install -U Sphinx 'blurb>=1.0.2dev1'
That should be future-proof.

brettcannon · 2017-07-19T17:46:46Z

I suspect @larryhastings did a release on Sunday of blurb and simply forgot to bump the version number to 1.0.2 (i.e. drop the dev1 part). Once that's confirmed it wasn't on purpose Larry or me can quickly fix the version number and do a non-dev release.

…aster

… into blurb-split-master

larryhastings · 2017-07-22T22:39:34Z

Booyah! We now pass CI testing in a post-blurb world. @ned-deily, care to review again?

ned-deily

Thanks for making the changes. Other than the spurious change noted, I still think we need to document how to use make venv and the need to add PYTHON=venv/bin/python to the doc make steps. And I need to update the mac installer build accordingly.

ned-deily · 2017-07-23T20:13:11Z

Objects/weakrefobject.c

@@ -959,3 +959,4 @@ PyObject_ClearWeakRefs(PyObject *object)
        PyErr_Restore(err_type, err_value, err_tb);
    }
 }
+


This seems spurious.

brettcannon · 2017-07-24T19:42:11Z

@ned-deily where are you suggesting to document the use of make venv? https://devguide.python.org/docquality/#helping-with-the-developer-s-guide ?

ned-deily · 2017-07-24T22:35:31Z

@brettcannon The primary documentation for Doc builds is Doc/README.rst. All of the make targets should be described there. Adding something to that section of the devguide would be good, too.

brettcannon · 2017-07-30T20:40:01Z

I created GH-2953 to document make venv to help keep this moving forward.

Perform 'blurb split' for master.

b5ae220

larryhastings added the skip issue label Jul 15, 2017

larryhastings self-assigned this Jul 15, 2017

larryhastings requested review from brettcannon and ned-deily July 15, 2017 14:27

the-knights-who-say-ni added the CLA signed label Jul 15, 2017

brettcannon approved these changes Jul 15, 2017

View reviewed changes

This was referenced Jul 15, 2017

Perform 'blurb split' for 3.6. #2718

Closed

Perform 'blurb split' for 3.5. #2717

Closed

ned-deily mentioned this pull request Jul 15, 2017

Perform 'blurb split' for 3.3. #2715

Closed

Merge branch 'master' of https://github.com/python/cpython into blurb…

38c8fde

…-split-master

larryhastings added 2 commits July 16, 2017 15:36

Fix Doc build for use with blurb.

dc85bd9

Modify Travis configuration to pre-install blurb.

a8f8d57

Add newlines to convince Travis to re-run tests.

c20bbb9

vstinner approved these changes Jul 16, 2017

View reviewed changes

ned-deily requested changes Jul 19, 2017

View reviewed changes

larryhastings added 4 commits July 22, 2017 01:30

Trying to diagnose build failure.

d84931b

More debugging stuff.

64cf545

Force Travis CI retry, as I've updated blurb.

3d5d159

Update build info to force latest version of blurb.

b7f7d37

larryhastings added 4 commits July 22, 2017 14:03

Touch a C file to *force* a Travis CI build.

8377d6a

Merge branch 'master' of github.com:python/cpython into blurb-split-m…

43ac140

…aster

Merge branch 'blurb-split-master' of github.com:larryhastings/cpython…

cbc8e93

… into blurb-split-master

Update ignored suspicious markup for doc build.

a7fb9e9

ned-deily requested changes Jul 23, 2017

View reviewed changes

brettcannon mentioned this pull request Jul 30, 2017

Document the make venv documentation build target #2953

Closed

larryhastings closed this Sep 4, 2017

larryhastings deleted the blurb-split-master branch September 4, 2017 20:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Perform 'blurb split' for master. #2719

Perform 'blurb split' for master. #2719

larryhastings commented Jul 15, 2017

brettcannon left a comment

brettcannon commented Jul 15, 2017

brettcannon commented Jul 15, 2017

ned-deily commented Jul 15, 2017

ned-deily commented Jul 15, 2017

ned-deily commented Jul 15, 2017

larryhastings commented Jul 16, 2017

larryhastings commented Jul 16, 2017

vstinner commented Jul 16, 2017

vstinner left a comment

brettcannon commented Jul 17, 2017

ned-deily left a comment

ned-deily Jul 19, 2017

brettcannon Jul 19, 2017

larryhastings Jul 22, 2017

brettcannon Jul 22, 2017 •

edited

Loading

ned-deily Jul 19, 2017

larryhastings Jul 22, 2017

ned-deily Jul 19, 2017

larryhastings Jul 22, 2017

ned-deily Jul 19, 2017

larryhastings Jul 22, 2017

brettcannon commented Jul 19, 2017

larryhastings commented Jul 22, 2017

ned-deily left a comment

ned-deily Jul 23, 2017

brettcannon commented Jul 24, 2017

ned-deily commented Jul 24, 2017

brettcannon commented Jul 30, 2017

Perform 'blurb split' for master. #2719

Perform 'blurb split' for master. #2719

Conversation

larryhastings commented Jul 15, 2017

brettcannon left a comment

Choose a reason for hiding this comment

brettcannon commented Jul 15, 2017

brettcannon commented Jul 15, 2017

ned-deily commented Jul 15, 2017

ned-deily commented Jul 15, 2017

ned-deily commented Jul 15, 2017

larryhastings commented Jul 16, 2017

larryhastings commented Jul 16, 2017

vstinner commented Jul 16, 2017

vstinner left a comment

Choose a reason for hiding this comment

brettcannon commented Jul 17, 2017

ned-deily left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brettcannon Jul 22, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brettcannon commented Jul 19, 2017

larryhastings commented Jul 22, 2017

ned-deily left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brettcannon commented Jul 24, 2017

ned-deily commented Jul 24, 2017

brettcannon commented Jul 30, 2017

brettcannon Jul 22, 2017 •

edited

Loading