This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

Author zach.ware
Recipients ezio.melotti, jkloth, kathweaver, michael.kearney, r.david.murray, terry.reedy, zach.ware
Date 2014-03-19.21:20:36
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1395264036.89.0.0427500585465.issue17846@psf.upfronthosting.co.za>
In-reply-to
Content
Review of kw-issue17846.diff:

setup.rst
- hunks 1 & 2 (@@ -49,9 and @@ -216,7):
  Both changes should be reverted.  The change from 3.4 -> 3.3 reverts a very
  recent change made due to 3.4 becoming the newest maintenance branch.  If
  I'm not mistaken, the blank line after the Windows heading is important.

- hunk 3 (@@ -240,11)
  I think the added sentence here would be better tacked onto the end of the
  previous paragraph:
  "[limitations are] here (2010).  Not listed on those pages, Visual C++ Express
  does not support Solution Folders.  Because the Python solution file uses
  Solution Folders, VS Express will warn you about that fact every time you
  open the ``pcbuild.sln`` file.  You can safely dismiss the warning with no
  impact on your ability to build Python."

  If you can improve on my English or brevity there, please do :)

  Also, there's a lot of trailing whitespace in that paragraph.  I'd suggest
  using an editor that can strip trailing spaces for you (I like Notepad++).

setupwindows.rst
- throughout
  Lots of trailing whitespace.

- line 7
  Seems a bit pointless to point back and forth between the two sections.  I'm
  open to suggestions on how best to avoid that.

- line 11
  A suggestion of TortoiseHg and a link thereto wouldn't be amiss.

- line 12
  Source for what?  What download page?

- line 13-14
  Doesn't fit here in the narrative flow.  This should be in the section about
  Subversion.

- line 17-19
  Subversion and Perl should be capitalized.  Also, this isn't true:  svn is
  necessary to pull the external library sources from svn.python.org, but you
  can actually download your sources directly from their respective vendors,
  provided you put them where the VS project files expect them to be.  Perl is
  not necessary at all if you pulled your OpenSSL sources from svn.python.org,
  it is only required if you're using sources you got straight from the
  OpenSSL folks.

- line 21-30
  This should be rewritten to explain the problem, then suggest a solution.
  Something like:

  """
  The VS project files expect external libraries that they need to be in
  folders on the same level as the cpython source checkout, for example, at
  ``PCbuild\..\..\openssl-1.0.1e``.  Since different versions of Python use
  different versions (or different builds) of the various external libraries,
  it is recommended to build different Python versions in isolated folders.
  For example: 

     C:\
      |_ Source
            |_ python3.4
            |      |_ cpython
            |      |_ tcl-8.6.1.0
            |      |_ tk-8.6.1.0
            |      |_ tcltk
            |      |_ ...
            |
            |_ python3.3
                   |_ cpython
                   |_ tcl-8.5.11.0
                   |_ tk-8.5.11.0
                   |_ tcltk
                   |_ ...
  """

  Either way, the term "buildbots" is used incorrectly here, referring to the
  scripts in the Tools/buildbot directory, which our CI system ("the
  buildbots", see buildbot.python.org) use to build and test Python.

- line 36
  Subversion should be capitalized.  This would also be the place to explain
  why svn is needed, and also to link to a place to get it.  TortoiseSVN is
  probably a good choice to link to, and mention its installer option to add
  svn to PATH.

- line 42
  Perl should be capitalized.  It should be explained that Perl is not
  absolutely necessary (and the cases in which it is).  It would also be good
  to link to ActivePerl, which is what the build_ssl.py script actually looks
  for.

- line 48-50
  Grammar issues.  Also, this is covered in the _windows-compiling section.

- line 52
  This is not true.

- line 58
  There should be an explanation of why.

- line 64
  Incorrect use of the term "buildbots".

- line 70
  Those scripts download the sources for every external project, but only
  build Tcl and Tk.

- line 71
  Sentence fragment.  Unfinished thought?

- line 78-93
  Poor grammar.  Also, none of this is true: I have never had any problems
  letting the solution build OpenSSL.  There is no need to jump through those
  hoops.  If building with pcbuild.sln doesn't work correctly, it's a bug that
  should be fixed, not documented as being broken.

- line 96
  Incorrect use of the term "buildbot".

- line 125-126
  I would rephrase this as "`Python Tools for Visual Studio <...>`_ is an
  add-on published by Microsoft for editing Python in Visual Studio.  It is
  not usable with Express editions."


Frankly, I don't see much added benefit from the new file.  It's mostly
duplication of PCbuild/readme.txt, and what little isn't already there should
be.  I would much rather see a patch making sure PCbuild/readme.txt is up to
date, and a link to http://hg.python.org/cpython/file/default/PCbuild/readme.txt
added to the devguide's existing compilation on Windows section.
History
Date User Action Args
2014-03-19 21:20:36zach.waresetrecipients: + zach.ware, terry.reedy, jkloth, ezio.melotti, r.david.murray, michael.kearney, kathweaver
2014-03-19 21:20:36zach.waresetmessageid: <1395264036.89.0.0427500585465.issue17846@psf.upfronthosting.co.za>
2014-03-19 21:20:36zach.warelinkissue17846 messages
2014-03-19 21:20:36zach.warecreate