Skip to content

issue 169: Python 3.4 compatibility. #170

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

Closed
wants to merge 3 commits into from
Closed

issue 169: Python 3.4 compatibility. #170

wants to merge 3 commits into from

Conversation

JulienPalard
Copy link
Member

See #169

@Mariatta
Copy link
Member

Mariatta commented Aug 4, 2017

If this is accepted, please also update readme and flit.ini files.

@larryhastings
Copy link
Contributor

I realize this feels like a dumb question, but I just want to 100% confirm: with these changes merged, blurb runs under Python 3.4, and in particular it runs fine with the python3 installed on docs.iad1.psf.io?

@JulienPalard
Copy link
Member Author

@larryhastings on docs.iad1.psf.io we have a Python 3.4.3.

I fixed what's were obvious, and tested what I could with a Python 3.4.4 I had on my laptop: I ran blurb on a cpython clone and it poped an editor, asked for things, and generated a file. I'm sure it does not cover 100% of the lines. so I may have missed something.

@larryhastings
Copy link
Contributor

Fair enough.

Really I just want to make sure that we solve the problem. So I'll make you a deal: you don't have to guarantee 100% coverage for 3.4, you just have to guarantee that it makes "blurb merge" work on docs.iad1.psf.io for both 3.4 and 3.5 (our entire test case right now). As long as the resulting diff looks reasonable, I'll merge it. (FWIW the current diff is entirely reasonable and if that solved the problem I'd happily merge it.)

Deal?

@JulienPalard
Copy link
Member Author

"blurb merge"? Deal, I'm testing this and I'll tell you. Thanks for providing a test case, I was not sure how to test this "correctly".

@larryhastings
Copy link
Contributor

Right now, developers use "blurb add", but I don't think that has to support 3.4. The doc build process uses "blurb merge" and that's what's hurting you on docs.iad1.psf.io. In the future, release managers will use "blurb release" and "blurb export", but again I think they're fine on 3.5+. And it's remotely possible that future workflows for some people (e.g. the Debian package maintainer) will use "blurb split", but again they can probably live with 3.5+. So, yeah, I think all we need to worry about for docs.iad1.psf.io is "blurb merge".

"blurb merge" slurps in all the disparate blurb entries and spits out a NEWS file. Test it by running from anywhere inside a CPython source tree. It takes an optional argument, which is the file to write to (e.g. "blurb merge foo" will write to "foo"). Default is to write to Misc/NEWS from the root of the current CPython tree.

@JulienPalard
Copy link
Member Author

For the 3.5 cpython branch I have not a single difference in the resulting NEWS file between a blurb merge unpatched ran with Python 3.5 and a partched blurb merge ran with Python 3.4.

Same experience with cpython 3.4 branch result in a single difference:

$ diff Misc/NEWS /tmp/NEWS
8905c8905
<   OS X. (Note, the python.org 32-bit-only Pythons use gcc-4.0 and the 10.4u
---
>   OS X. (Note, the python.org 32 -bit-only Pythons use gcc-4.0 and the 10.4u

Would you like me to open an issue on this subject and investigate?

@larryhastings
Copy link
Contributor

Well... that's interesting! I certainly wouldn't complain if you opened a separate issue on that subject and investigated.

My hunch is that it's a change in behavior (maybe even a "bugfix") in textwrap.wrap. blurb uses that to "aggressively" reflow text.

You might also grep through the blurb data files (Misc/NEWS.d) to see what the original line looked like. I'm guessing that it didn't have the space between "32" and "-bit".

@JulienPalard
Copy link
Member Author

I did, I expected a newline or something like this between 32 and -bit but no:

$ git grep -C1 'Pythons use gcc-4.0 and the 10.4u' Misc/NEWS.d/
Misc/NEWS.d/3.3.0a1.rst-Distutils LDSHARED configuration variable for OS X. (Note, the python.org
Misc/NEWS.d/3.3.0a1.rst:32-bit-only Pythons use gcc-4.0 and the 10.4u SDK, neither of which are
Misc/NEWS.d/3.3.0a1.rst-available in Xcode 4.  This change does not attempt to override settings to

Opening an issue for this one so I'll remember to investigate / document it.

@JulienPalard
Copy link
Member Author

I didn't have to change the code more that previously submitted in this PR, but as we only tested blurb and blurb merge, I think we should either revert 4234d78 or at least be more precise.

In one hand if we dont write explicitly that it's Python 3.4 compatible, someone will break compatibility, breaking documentation builds. In the other hand I don't want to pollute the "overview" paragraph with this. I propose a new paragraph, far from the overview, maybe like:

Compatibility
-------------

blurb is compatible with Python 3.5+, expect for `blurb merge` which is compatible with Python 3.4+.

@larryhastings
Copy link
Contributor

See the "Unwanted space" issue (#171). textwrap is a little broken in 3.4, and maybe this means blurb just shouldn't support 3.4.

@JulienPalard
Copy link
Member Author

Let's see if there's a chance for this machine to be upgraded python/psf-salt#109 (comment)

@larryhastings
Copy link
Contributor

larryhastings commented Aug 5, 2017

How about this we change the text to:

blurb is supported on Python 3.5+. (blurb merge runs under Python 3.4 but is known to produce incorrect output.)

and we go ahead and merge your diff.

@terryjreedy
Copy link
Member

'is known to produce incorrect output' could mean that something is always wrong. 'may produce slightly incorrect output' seems to covers the discussion above.

@larryhastings
Copy link
Contributor

Well, blurb merge consistently produces incorrect output for the notes for 3.3.0b1. 3.5 actually dropped the release notes for 3.3 (they were moved to Misc/HISTORY) and naturally that's true going forward as well. So depending on which version you run blurb merge under, you may or you may not get consistently incorrect output. I guess I don't care that much about the wording, but at this point I don't want to support blurb on 3.4.

@JulienPalard
Copy link
Member Author

I'm closing this PR as due to the textwrap bug in 3.4 and we now have Python 3.6 on docs.python.org.

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

Successfully merging this pull request may close these issues.

5 participants