Updating versioneer to 0.17+ #5622

Merged
merged 7 commits into from Jan 11, 2017

Projects

None yet

3 participants

@azjps
Contributor
azjps commented Dec 28, 2016

First cut, still need to check local changes. Somehow windows is expanding "*" as a glob in subprocess.Popen([.., "*", ..], shell=False), I'm hoping its just an issue with my environment setup 😦 .

Rebuilt versioneer using latest version, which has some windows
compatibility fixes and so forth. The main migration is moving
configuration for versioneer from setup.py to setup.cfg.

All pull requests must have an associated issue in the issue tracker. If there
isn't one, please go open an issue describing the defect, deficiency or desired
feature. You can read more about our issue and PR processes in the
wiki.

  • issues: fixes #5613
  • tests added / passed
  • release document entry (if new feature or API change)
.gitignore
@@ -113,3 +113,6 @@ win-64
# generated coffeescript
/bokehjs/test/common/generated_defaults/models_defaults.coffee
/bokehjs/test/common/generated_defaults/widgets_defaults.coffee
+
@azjps
azjps Dec 28, 2016 Contributor

not supposed to be committed, i'll remove

@mattpap mattpap added the status: WIP label Dec 28, 2016
-"""versioneer.py
+# Version: 0.17
@mattpap
mattpap Dec 28, 2016 Contributor

Shouldn't this be installed with conda?

@azjps
azjps Dec 28, 2016 Contributor

This isn't the versioneer library itself, python-versioneer generates a versioneer.py to the root of the project repository, so that setup.py can use it to compute versions (I guess it was easiest to do this way in order to perform heuristics with the repository directory). In fact versioneer.py doesn't exist in the python-versioneer repo. I'll add some additional context to this PR's commit message.

.gitignore
@@ -113,3 +113,6 @@ win-64
# generated coffeescript
/bokehjs/test/common/generated_defaults/models_defaults.coffee
/bokehjs/test/common/generated_defaults/widgets_defaults.coffee
+
+*.styl
+*.png
@mattpap
mattpap Dec 28, 2016 Contributor

Missing trailing newline.

@azjps
azjps Dec 28, 2016 Contributor

Yeah, fallout from my attempts to figure out how to use git effectively on windows. This wasn't even supposed to be committed, it was auto-added by the github UI 😐

@bryevdv
bryevdv Dec 28, 2016 Member

no worries, we are here to help :D

@bryevdv
Member
bryevdv commented Dec 28, 2016

The main migration is moving configuration for versioneer from setup.py to setup.cfg.

Ah, that's nice

@azjps
Contributor
azjps commented Dec 29, 2016 edited

Okay, doesn't seem like there were any local changes made to _version.py that are still needed, besides maybe some minor whitespace tweaks. The first commit in PR is direct result of versioneer install, the second were some minor changes that I added as a workaround for myself.

The only other side effect I know of this change is that the version string uses a "+" now to conform to PEP440.

@bryevdv
Member
bryevdv commented Dec 29, 2016 edited

The only other side effect I know of this change is that the version string uses a "+" now to conform to PEP440.

Oh, that's actually important, unfortunately. There is a significant amount of internal tooling with expectations around the existing format. Additionally it has implications for how BokehJS is loaded from CDN, and I am not keen to disturb that (not worth the risk) I would not be opposed to patching our checked in _version.py to change reported versions to the old format, however.

@bryevdv
Member
bryevdv commented Dec 29, 2016

Or alternatively instead of patching _version.py, perhaps a conversion step could be applied in

https://github.com/bokeh/bokeh/blob/master/bokeh/util/version.py

which converts the new format from versioneer to the old format.

@azjps
Contributor
azjps commented Dec 29, 2016 edited

versioneer has a style=git-describe config which uses 0.12.4dev10-2-g902c066-dirty instead of the versioneer PEP440-compliant default 0.12.4dev10+2.g902c066.dirty. How does that sound? It should probably get tested for a couple of use cases first though, the docs on versioneer aren't up-to-date.

Edit: tested below for local import, setup.py version, and import after setup.py install.

bokeh/_version.py
- return {"version": "0+unknown", "full-revisionid": None,
- "dirty": None,
- "error": "unable to compute version", "date": None}
+
@azjps
azjps Dec 29, 2016 Contributor

Uh, I'm not sure why git doesn't collapse the diff here. I re-generated this file with versioneer to make sure I didn't leave anything out, maybe versioneer does a git remove + git add.

versioneer.py
@@ -1043,10 +1050,15 @@ def git_pieces_from_vcs(tag_prefix, root, verbose, run_command=run_command):
# if there is a tag matching tag_prefix, this yields TAG-NUM-gHEX[-dirty]
# if there isn't one, this yields HEX[-dirty] (no NUM)
- describe_out, rc = run_command(GITS, ["describe", "--tags", "--dirty",
- "--always", "--long",
- "--match", "%%s*" %% tag_prefix],
@azjps
azjps Dec 29, 2016 Contributor

note to self -- %% accidentally copy+pasted in prior commit

bokeh/_version.py
+ # shell=False! As a temporary workaround, I've remove the --match
+ # argument since tag_prefix is empty in the bokeh setup config.
+ describe_args = ["describe", "--tags", "--dirty",
+ "--always", "--long"]
@bryevdv
bryevdv Dec 31, 2016 Member

comparing to our old set of flags that were used, this adds --long. That might be fine but we should investigate what exactly it does

@bryevdv
Member
bryevdv commented Dec 31, 2016

@azjps this is looking good, just an FYI re: schedule I'd love to merge this right after the 0.12.4 release or thereabouts (there's enough other build-changes going into 0.12.4 I don't want to add this on top)

@bryevdv
Member
bryevdv commented Jan 9, 2017

@azjps I would love to merge this soon, I think the only outstanding issue is the file endings in _version.py. Our checker is reporting :

File contains carriage returns at end of line: bokeh/_version.py, line 512

for every line. Maybe there is an editor setting you can change to unix-style (just line feed) endings, then save and push?

Alternatively, this SO answer suggests a .gitattributes file can be set that will have git automatically fix up line endings, which might be nice / worth trying:

http://stackoverflow.com/questions/10418975/how-to-change-line-ending-settings

azjps added some commits Dec 28, 2016
@azjps @azjps azjps Updating versioneer to 0.17+
Rebuilt versioneer using latest version, which has some windows
compatibility fixes and so forth. The main migration is moving
configuration for versioneer from setup.py to setup.cfg.

Refer to the python-versioneer repository for documentation;
roughly it installs a versioneer.py to the root directory of
the project which is used by setup.py to build a _version.py
file (a default _version.py is also provided when setup.py
is not used). The _version.py is used to retrieve the version
based on the VCS tag.
05b0076
@azjps azjps Don't match tag_prefix in git describe
Not sure why but on windows, the asterisk is getting expanded
in subprocess.Popen(shell=False). Doing this as a temporary
workaround.
5e50bed
@azjps azjps Use style=git-describe for versioneer
To keep the __version__ string backwards-compatible, use the
git-describe style offered by versioneer. (By default, versioneer
uses a PEP440-compliant __version__ string which uses "+"
as a delimiter for local versioning information.)
8e5853d
@azjps
Contributor
azjps commented Jan 10, 2017 edited

So that's why the git diff wasn't collapsed before (yay windows).

Edit: travis still failed, is it the same error?

@bryevdv
Member
bryevdv commented Jan 11, 2017

@azjps No, now it's just:

E       AssertionError: Code quality issues:
E         File starts with more than 1 empty line: bokeh/_version.py, line 1
E       assert 1 == 0
E        +  where 1 = len(['File starts with more than 1 empty line: bokeh/_version.py, line 1'])

This one I can fix in two seconds on the GH UI, so I will do that, and then when it goes green it should!) I will merge

@bryevdv
Member
bryevdv commented Jan 11, 2017 edited

@azjps I did look into that --long option, and the help had this to say:

--long
           Always output the long format (the tag, the number of commits and the abbreviated commit name) even when it
           matches a tag. This is useful when you want to see parts of the commit object name in "describe" output, even
           when the commit in question happens to be a tagged version. Instead of just emitting the tag name, it will
           describe such a commit as v1.2-0-gdeadbee (0th commit since tag v1.2 that points at object deadbee....).

given that, I think we actual must remove that option to match the previous behaviour. Tagged versions should specifically not have that extra info:

[bryan:~/work/bokeh]$  git describe --tags --dirty --always                                        (760396a)
0.12.4
[bryan:~/work/bokeh]$  git describe --tags --dirty --always --long                                 (760396a)
0.12.4-0-g760396a
@bryevdv
Member
bryevdv commented Jan 11, 2017

@azjps I went ahead and made that small change, I'd like to merge this today and get a dev build out to get some early experience with it on more realistic deploys

@bryevdv
Member
bryevdv commented Jan 11, 2017

Actually this might need more tweaking, unfortunately. I tested locally and clean, dirty, and __conda_version__.py override all worked as expected. But if I made a local tag:

[bryan:~/work/bokeh]$ python                                                                                            (vers)
Python 3.4.4 |Anaconda 2.3.0 (x86_64)| (default, Jan  9 2016, 17:30:09)
[GCC 4.2.1 (Apple Inc. build 5577)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import bokeh
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/bryan/work/bokeh/bokeh/__init__.py", line 16, in <module>
    from .util.version import __version__; __version__
  File "/Users/bryan/work/bokeh/bokeh/util/version.py", line 36, in <module>
    __base_version__ = base_version()
  File "/Users/bryan/work/bokeh/bokeh/util/version.py", line 34, in base_version
    return VERSION_PAT.search(__version__).group(1)
AttributeError: 'NoneType' object has no attribute 'group'
>>>
@bryevdv
Member
bryevdv commented Jan 11, 2017

OK it actually looksl like my last change was too hasty and is what broke things, will revert it

@bryevdv
Member
bryevdv commented Jan 11, 2017

OK! checking locally:

[bryan:~/work/bokeh]$ vi setup.py                                                                                       (vers)
[bryan:~/work/bokeh]$ python -c "import bokeh; print(bokeh.__version__)"                                               (vers✱)
0.12.4-11-g6dd1f0f-dirty
[bryan:~/work/bokeh]$ git checkout -- setup.py                                                                         (vers✱)
[bryan:~/work/bokeh]$ python -c "import bokeh; print(bokeh.__version__)"                                                (vers)
0.12.4-11-g6dd1f0f
[bryan:~/work/bokeh]$ git tag 1.2.3                                                                                     (vers)
[bryan:~/work/bokeh]$ python -c "import bokeh; print(bokeh.__version__)"                                                (vers)
1.2.3
[bryan:~/work/bokeh]$ git tag -d 1.2.3                                                                                  (vers)
Deleted tag '1.2.3' (was 6dd1f0f)
[bryan:~/work/bokeh]$ vi bokeh/__conda_version__.py                                                                     (vers)
[bryan:~/work/bokeh]$ python -c "import bokeh; print(bokeh.__version__)"                                                (vers)
2.3.4

All looks great. Sorry for the noise earlier I will merge when green.

@bryevdv bryevdv merged commit e952791 into bokeh:master Jan 11, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@bryevdv
Member
bryevdv commented Jan 11, 2017

Thanks @azjps ! sorry again for the noise edits.

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