-
-
Notifications
You must be signed in to change notification settings - Fork 31.7k
Deprecate sys._mercurial and create sys._git #71780
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
Comments
This is part of the GitHub migration. And by "deprecate" sys._mercurial I mean fill it with default values (https://www.python.org/dev/peps/pep-0512/#deprecate-sys-mercurial). |
Get the current branch: |
Get the current revision: |
https://github.com/python/cpython/blob/master/Modules/getbuildinfo.c will need to be updated on top of configure.ac and Makefile.pre.in. |
Here is a patch to add sys._git and update Py_GetBuildInfo() as necessary. I would like to see this in Python 3.6 as 3.6.0 will be released using Mercurial but (hopefully) subsequent releases will be on git, so any reviews will be appreciated. I will hold off on removing sys._mercurial until 3.7 as that will be the first release that doesn't span version control systems. |
Thanks for the patch, Brett. Having done a quick test, I'd like to review this in more detail before applying but that will have to wait until after b1. If you want, you can reassign this to me. |
Sounds good. On Sun, Sep 11, 2016, 11:13 Ned Deily <report@bugs.python.org> wrote:
|
Hello Brett, after my review, I propose this patch where I have added the missing indents and remove the ./configure file (because this one can be generated by autoreconf) |
The inclusion of changes to ./configure is on purpose since the file is in the repository and not everyone has autoconf installed to necessarily re-generate the file (and we have had issues with people using really old versions of autoconf in the past). |
I'll get to this one shortly. |
Hi Brett, With your comment, I have added the modified ./configure. Here is the last patch including the indentation. |
This is now blocking all releases for all versions. I'll try and make some time to update the Windows build scripts and project files, but if someone else gets there first feel free to post a PR. |
Yeah, I'll get to the non-Windows parts shortly. I'd been waiting for the transition to happen. |
It looks to me like we want: branch= Unless we're planning on leaving out the tag? My PR 262 makes the Windows build changes in master, but doesn't change getbuildinfo.c. |
I purposefully left the tag out because I don't think it's useful (a tag is just a conveniently named hash and all of our tags are version numbers and that's already available in other parts of the sys module). |
Just updated my PR to remove the GITTAG variable, so we'll just go with GITBRANCH and GITVERSION. (Any value in trying to extract the URL of the remote? That's probably going to be a bit flimsy, but might help more clearly identify forks.) |
I had thought about trying to pull down the remote but it did seems unnecessarily messy when the SHA1 hash should match across various remotes only when the commits actually match. |
I finally took a close look at this and I think the approach Brett advocated here is a bit too simple and does not cover the use case I had in mind when I suggested we needed to have a replacement for sys._mercurial. Previously when building from a source repo, if the repo was checked out to a tag (like a release tag, say, "v3.6.0"), that tag showed up in sys._mercurial and sys.version making it very easy to identify release builds. For example, with the macOS pythons we provide on python.org, you see versions like this: $ /usr/local/bin/python3.5 -c 'import sys;print(sys.version)'
3.5.2 (v3.5.2:4def2a2901a5, Jun 26 2016, 10:
$ /usr/local/bin/python3.5
Python 3.5.2 (v3.5.2:4def2a2901a5, Jun 26 2016, 10:47:25)
[GCC 4.2.1 (Apple Inc. build 5666) (dot 3)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> ^D Having the tag, if available, is very important, IMO. Also, the original patch didn't deal with the platform module which also uses the SCM info. Since we've now made the transition to git, it seems to me there's no reason to complicate the code by trying to support either hg or git; we don't support svn anymore. So the PR is simpler and follows very much the hg support. I think this also means that Steve's PR for the Windows installer should be changed back to use tags again. I'd like to get this into 3.6.1 if possible. Better late than never! |
Here's some sample output with the change: C:\build\cpython36>PCbuild\win32\python_d.exe
Python 3.6.0+ (3.6:95c50e5aed9e5683676e18349dd94b11901a66b3, Mar 4 2017, 06:08:54) [MSC v.1900 32 bit (Intel)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> import sys
>>> sys._git
('CPython', '3.6', '95c50e5aed9e5683676e18349dd94b11901a66b3') I wonder whether we should shorten the revision hash for the copyright string? I think sys._git[2] should keep the whole output. |
copyright==>version string (sys.version) Also, that part is clearly not release blocking. My PR for 2.7 is waiting for Travis to catch up, so once that's done and Ned has finished his backports (or Larry and Ben have backported to their versions?) we can lower the priority on this. |
Yeah, shortening the hash in the version string might be nice. Feel free to do so. It can probably wait for 3.6.2. |
GitHub trims to the first 7 characters. I see no harm in doing that for sys.version (but not tonight) |
git actually does not always shorten to 7 characters. In git 2.11 (I think) they shorten to the shortest length to guarantee uniqueness. So |
I just did my build for 3.6.1rc1 and it looks like Perhaps "git describe" is the better command to use here? From what I've seen of that, it should do "tag if tagged, else short hash" by default, and we can do "git describe --dirty=<suffix>" to get an extra marker for when it has been modified. (Though this may mean taking GITTAG back out of our scripts.) Otherwise, I think the releases are no longer blocked on this, and we can take a bit of time to figure out the right format. |
Based on Brett's and Steve's feedback, I've pushed some tweaks to the captured git info. With the changes, we now use --short form of git hash. And we use output from "git describe --all --always --dirty" for the tag. I added --all and --always for better results with development builds; the results should be the same for release (tagged) builds - I think. I'm going to cherry pick this into 3.6.1 final and we can see how this works for development builds and further tweak as necessary. Steve, note that I was brave and modified the Windows builds; yay, AppVeyor. I'll also cherrypick these to 2.7 soon. Expected outputs:
Release (tagged) build:
Development build:
"dirty" means the working tree has uncommitted changes. |
The output LGTM. |
The 3.5, 3.6 and master branches of Python have a new sys._git attribute and lost their sys._mercurial attribute. But the 2.7 branch is still stuck at sys._mercurial: version and tag are empty. Should we backport sys._git from master to 2.7 and remove sys._mercurial? Maybe on 2.7 we can keep sys._mercurial. |
It needs to be backported there too, or we need to unmigrate 2.7 back to mercurial. Otherwise we can't make a release. |
I'll be doing backports to 2.7 for this and some other things shortly. |
Ned Deily: "I'll be doing backports to 2.7 for this and some other things shortly." It seems like Ned was busy or forgot 2.7, so I wrote a change: #1327 My change doesn't touch platform.py. platform._sys_version() uses sys.subversion but not sys._hg. If someone wants to enhance the platform module of Python 2.7, I suggest to open a new issue. There is a non-zero risk of breaking the backward compatibility, so I skipped my turn for that one :-) |
With #1392 Python 2.7 should up to date with Python master, and this issue should be fixed. Or did I miss something? |
I think that covers it! |
Cool, good job ;-) |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: