classification
Title: Move Windows external libs from \..\ to \externals
Type: enhancement Stage: resolved
Components: Build, Extension Modules, Windows Versions: Python 3.5, Python 3.4, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: zach.ware Nosy List: BreamoreBoy, db3l, jeremy.kloth, loewis, python-dev, steve.dower, terry.reedy, tim.golden, trent, zach.ware
Priority: normal Keywords: patch

Created on 2013-05-03 17:18 by zach.ware, last changed 2015-02-09 07:05 by python-dev. This issue is now closed.

Files
File name Uploaded Description Edit
issue17896-default.diff zach.ware, 2014-10-29 06:00 review
issue17896-3.4.diff zach.ware, 2014-10-29 06:01 review
issue17896-2.7.diff zach.ware, 2014-10-29 06:01 review
Messages (20)
msg188309 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2013-05-03 17:18
PCbuild\readme.txt has a comment from Trent Nelson dated 2-April-2008 suggesting moving the external library location for Windows from "<cpython checkout>\..\" to "<cpython checkout>\external\" to make switching between versions easier.  I've gotten rather annoyed with having all of the external libs cluttering the folder holding my cpython checkout, so I've written a patch that moves them :).

The patch actually moves the external libs into "<checkout>\external-34", because in most cases it's best to have separately built externals for each Python version.  The idea is to then have an "external-35" after 3.4 is released, and possibly "external-27" and "external-33" if it is permissible to backport this kind of change.  There are only four places that would need to be changed in other versions; PCbuild/pyproject.props:19, PCbuild/rt.bat:41, Tools/buildbot/external-common.bat:4 and 5, and Lib/tkinter/_fix.py:52.  All four are simple replacements which I believe could be automated by the release script.  PCbuild/build_ssl.py is patched to find the externals dir in PCbuild/pyproject.props while finding the OpenSSL version there.

A few of the benefits to this that have occurred to me are:
- the fact that the dir holding your cpython checkout doesn't get cluttered up with external libs
- it becomes very easy to rebuild all of the externals (just `rmdir /s /q external-34` and run external(-amd64).bat again)
- no possibility of problems arising from one version trying to use an external lib built by/for another version

The only real downsides I'm aware of are that it is a change from what everyone is accustomed to, and that it will use up a bit more disk space (which could be mitigated by using junctions, as Trent suggested in the comment in readme.txt).  Also, committing this would cause an extra long build time on the buildbots due to having to recompile Tcl/Tk and OpenSSL.

Everything builds and works for me using the buildbot scripts after the patch.  All comments welcome :)

Thanks,

Zach
msg223484 - (view) Author: Mark Lawrence (BreamoreBoy) * Date: 2014-07-19 22:33
I also find the location annoying and would be happy to see it moved.

Regarding the "extra long build time on the buildbots due to having to recompile Tcl/Tk and OpenSSL" is this a one off the first time you run after the commit, or does it always happen because the buildbots run some form of clean?  Sorry if this sounds like a daft question but I haven't the faintest idea how the various buildbots are configured.
msg223496 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2014-07-20 03:01
After the most recent changes to the buildbot scripts, it would be every time without a patch to the clean script, but at the time I only meant a single time (which really isn't a big deal).

The patch is well out of date, I can update it if we can come to a consensus that this kind of change is a good idea.
msg225812 - (view) Author: Mark Lawrence (BreamoreBoy) * Date: 2014-08-24 11:52
I see that this has also been raised on #22262.  As nobody has objected can't we just get on with it?
msg230088 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2014-10-27 18:55
Not so keen on having separate folders for Python version, mostly because I want to avoid having the current version in too many locations. I've settled on patchlevel.h as the canonical source of the version number for my VC14 branch - everything else should read it from there.

If the patch is updated to read it from patchlevel.h then I'm +1. Otherwise +0, and I'll probably update it later myself :)
msg230092 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2014-10-27 19:15
I'm ambivalent on per-version externals dirs by now; I've since found it much easier to maintain hg-shared repos per branch.  I'll go ahead with this shortly on all three branches, unless there are objections to it on 2.7 and 3.4.
msg230102 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2014-10-27 20:19
Sounds good to me. I thought about the shared repo approach, but I don't understand fully how merging between 2.7, 3.4 and default works in that scenario (simply because I've never tried it - maybe it's really easy?)
msg230108 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2014-10-27 21:46
Basically, after:

hg clone h.p.o/cpython default
hg share default 3.4
hg share default 2.7
hg -R 3.4 update 3.4
hg -R 2.7 update 2.7

the 2.7, 3.4, and default directories are separate working copies created from the same history, each at a different revision.  As soon as a changeset is created in one, it's available in the others (but the other working copies don't change).  So after commiting to 2.7 (from within the 2.7 dir), `cd ..\3.4 && hg graft 2.7` will try to graft your new changeset to 3.4, then `cd ..\default && hg merge 3.4` will merge it with default.  hg push will push the same thing from any of the three directories.
msg230109 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2014-10-27 22:07
Since all other dependency directories are dependency version specific, the only problem with a common external directory is the unversioned tcltk/.  What do you plan to do with that?
msg230110 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2014-10-27 22:20
There's no change from the status quo on that front: the only change
is that the $(externalsDir) VS variable becomes "..\externals" instead
of "..\..".  Is there an open issue for versioning the tcltk[64] dirs?
msg230122 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2014-10-28 06:30
I could not find an issue specifically for the tcltk problem.  I just explained the problem as I know it in msg230120 of #17896.  Some sort of fix is required before we can merge multiple tcltk directories in isolated build directories of the sort you diagrammed in msg214138.
msg230199 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2014-10-29 06:00
I don't think we're on the same page here, Terry, so here's some patches and a wall of text to hopefully make me clearer.

In particular, I don't understand what you mean by "merge multiple tcltk directories in isolated build directories", as the "merge" and "isolated" seem mutually exclusive to me.

The layout I diagrammed in that message is basically what I use currently.  These patches would allow me to go from this:


root
  |_ default
  |   |_ cpython (clone of https://hg.python.org/cpython)
  |   |   |_ Doc
  |   |   |_ Lib
  |   |   |_ PCbuild
  |   |   |_ ...
  |   |_ tcl-8.6.1.0
  |   |_ tk-8.6.1.0
  |   |_ tcltk
  |   |_ ...
  |
  |_ 2.7
      |_ cpython (clone of https://hg.python.org/cpython#2.7)
      |   |_ Doc
      |   |_ Lib
      |   |_ PCbuild
      |   |_ ...
      |_ tcl-8.5.15.0
      |_ tk-8.5.15.0
      |_ tcltk
      |_ ...


To this:

root
  |_ default (clone of https://hg.python.org/cpython)
  |   |_ Doc
  |   |_ externals
  |   |   |_ tcl-8.6.1.0
  |   |   |_ tk-8.6.1.0
  |   |   |_ tcltk
  |   |   |_ ...
  |   |_ Lib
  |   |_ PCbuild
  |   |_ ...
  |
  |_ 2.7 (clone of https://hg.python.org/cpython#2.7)
      |_ Doc
      |_ externals
      |   |_ tcl-8.5.15.0
      |   |_ tk-8.5.15.0
      |   |_ tcltk
      |   |_ ...
      |_ Lib
      |_ PCbuild
      |_ ...


I don't think it's wise to try to use the same directory for builds of multiple Python versions without rebuilding all externals anyway.  The reason you can't use the same tcltk install between 2.7 and 3.4/default is because of the different compiler used for each branch.  These days, you'll run into the same issue with OpenSSL, since we now use the same version of it on all branches.  I suppose we could try to make the OpenSSL build put the compiler version in the output directory name and similarly version the tcltk install dirs, but I think that's a lot more effort than it's worth especially considering the other benefits of using separate checkouts for each branch (like being able to test more than one interpreter simultaneously).

These patches do make it significantly easier to switch between versions in the same checkout properly, though; an 'hg purge --all' after update will clear out all the externals as well as all build artifacts (though that would require re-downloading the externals).
msg230252 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2014-10-30 03:02
After seeing your new diagram and glancing at the patch, we are pretty much in agreement.  I would like to see this change so I can delete the extra layer of directories.  I will try to do a review and test tomorrow.  I will also check the devguide to see if I think it needs revising too.
msg230477 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2014-11-02 02:50
The patch looks pretty straightforward.  Running the revised external.bat and then pcbuild.sln seem to give the same result as before.  So this looks ready to me.

As to Steve's merge question: since externals is ignored and not part of the python.org repository, it should have no effect on merge or graft.
msg230479 - (view) Author: Roundup Robot (python-dev) Date: 2014-11-02 03:50
New changeset 62ce0f623154 by Zachary Ware in branch '2.7':
Issue #17896: Move Windows external lib sources from .. to externals.
https://hg.python.org/cpython/rev/62ce0f623154

New changeset b5e9bc4352e1 by Zachary Ware in branch '3.4':
Issue #17896: Move Windows external lib sources from .. to externals.
https://hg.python.org/cpython/rev/b5e9bc4352e1

New changeset 64a54f0c87d7 by Zachary Ware in branch 'default':
Issue #17896: Move Windows external lib sources from .. to externals.
https://hg.python.org/cpython/rev/64a54f0c87d7
msg230480 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2014-11-02 03:57
Thanks for the test, Terry.  Committed.
msg230518 - (view) Author: David Bolen (db3l) Date: 2014-11-02 18:55
I noticed this issue when checking on some recent 2.7 branch failures on my buildbot.  It might be worth noting this change to any Windows buildbot owners since we all have existing trees now with a lot of stranded external folders that can be removed.

For what it's worth - and it's a minor concern - in a buildbot context, I prefer the old way.  I am much more likely to have to manipulate the build folder (the hg checkout/build folder on a buildbot) to deal with issues than any of the external folders, so it's always been very convenient to be able to do so easily without affecting those externals (for which causing a rebuild is very time consuming)

-- David
msg230574 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2014-11-04 04:46
Good point, David.  Jeremy, Trent, you're the only other Windows buildbot operators as far as I know; feel free to clean up the old externals locations as you like.

Also, sorry to make it a bit hairier to operate, but I think this is a big enough improvement for day-to-day development that I definitely don't want to change back.  For the record, it is fairly easy to just move out the externals folder, do whatever you need to with the repository, and move it back in without having to rebuild OpenSSL or Tcl/Tk/Tix.
msg235552 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2015-02-08 10:13
I noticed this issue today when trying to make the 3.4.3rc1 build; this broke Tools/msi.py, which fails to find the license files and the tcltk files.
msg235595 - (view) Author: Roundup Robot (python-dev) Date: 2015-02-09 07:05
New changeset 7d22dbf3a0dc by Martin v. Löwis in branch '3.4':
Issue #17896: Update msi.py to new externals dir.
https://hg.python.org/cpython/rev/7d22dbf3a0dc
History
Date User Action Args
2015-02-09 07:05:13python-devsetmessages: + msg235595
2015-02-08 10:13:14loewissetmessages: + msg235552
2014-11-04 04:46:26zach.waresetnosy: + jeremy.kloth
messages: + msg230574
2014-11-02 18:55:15db3lsetnosy: + db3l
messages: + msg230518
2014-11-02 03:57:12zach.waresetstatus: open -> closed
messages: + msg230480

assignee: zach.ware
resolution: fixed
stage: commit review -> resolved
2014-11-02 03:50:12python-devsetnosy: + python-dev
messages: + msg230479
2014-11-02 02:50:34terry.reedysetmessages: + msg230477
stage: patch review -> commit review
2014-10-30 03:02:52terry.reedysetmessages: + msg230252
2014-10-29 06:01:26zach.waresetfiles: - move_externals.diff
2014-10-29 06:01:17zach.waresetfiles: + issue17896-2.7.diff
2014-10-29 06:01:05zach.waresetfiles: + issue17896-3.4.diff
2014-10-29 06:00:50zach.waresetfiles: + issue17896-default.diff

messages: + msg230199
stage: needs patch -> patch review
2014-10-28 06:30:19terry.reedysetmessages: + msg230122
stage: patch review -> needs patch
2014-10-27 22:20:35zach.waresetmessages: + msg230110
2014-10-27 22:07:11terry.reedysetmessages: + msg230109
2014-10-27 21:46:12zach.waresetmessages: + msg230108
2014-10-27 20:19:26steve.dowersetmessages: + msg230102
2014-10-27 19:15:45zach.waresetmessages: + msg230092
versions: + Python 2.7, Python 3.4
2014-10-27 18:55:38steve.dowersetmessages: + msg230088
2014-08-24 11:52:00BreamoreBoysetmessages: + msg225812
2014-08-24 01:52:17zach.warelinkissue22262 superseder
2014-07-20 03:01:32zach.waresetstage: patch review
messages: + msg223496
versions: + Python 3.5, - Python 3.4
2014-07-19 22:33:56brian.curtinsetnosy: - brian.curtin
2014-07-19 22:33:19BreamoreBoysetnosy: + BreamoreBoy, terry.reedy, steve.dower
messages: + msg223484
2013-05-03 17:55:32pitrousetnosy: + loewis
2013-05-03 17:18:26zach.warecreate