Merge lp:~widelands-dev/widelands/detect_revision_update into lp:widelands

Proposed by Tino
Status: Merged
Merged at revision: 8862
Proposed branch: lp:~widelands-dev/widelands/detect_revision_update
Merge into: lp:widelands
Diff against target: 64 lines (+21/-16)
1 file modified
utils/detect_revision.py (+21/-16)
To merge this branch: bzr merge lp:~widelands-dev/widelands/detect_revision_update
Reviewer Review Type Date Requested Status
Tino Approve
GunChleoc Approve
Review via email: mp+355590@code.launchpad.net

Commit message

use git rev-parse instead of show with head
remove use of deprecated os.open
avoid leaking of stderr into cmake output on windows

To post a comment you must log in.
Revision history for this message
GunChleoc (gunchleoc) wrote :

Code LGTM. Not tested.

review: Approve
Revision history for this message
bunnybot (widelandsofficial) wrote :

Continuous integration builds have changed state:

Travis build 4038. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/432860634.
Appveyor build 3834. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_detect_revision_update-3834.

Revision history for this message
GunChleoc (gunchleoc) wrote :

Appveyor builds are working, compiling from Git on Linux will not. We have to wait for https://code.launchpad.net/~widelands-dev/widelands/cmake_errors_WL_VERSION/+merge/355728, merge in trunk and then test again.

Revision history for this message
Tino (tino79) wrote :

No, the cmake warning is not causing the failure.
I've updated the whole function, should work on every os...

Revision history for this message
Tino (tino79) wrote :

Ok, currently at least on travis (MacOS and Linux) and on Travis (Windows) CMake + DetectRevision.py can determine the git version.
On my local machine (windows) both git and bzr work.

So this can go in from my side.

review: Approve
Revision history for this message
GunChleoc (gunchleoc) wrote :

I have tested both git and bzr. Linux is fine, so is git on MSys2. I could not it to work with bazaar on MSys2, but that's because the bzr pypi package is buggy, and it's accordingly already broken in trunk.

@bunnybot merge

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'utils/detect_revision.py'
2--- utils/detect_revision.py 2018-09-26 08:47:26 +0000
3+++ utils/detect_revision.py 2018-09-28 08:20:49 +0000
4@@ -42,20 +42,19 @@
5
6
7 def detect_git_revision():
8- if not sys.platform.startswith('linux') and \
9- not sys.platform.startswith('darwin'):
10- git_revnum = os.popen(
11- 'git show --pretty=format:%h | head -n 1').read().rstrip()
12+ try:
13+ cmd = subprocess.Popen(
14+ ['git','rev-parse','--short','HEAD'],
15+ stdout=subprocess.PIPE,
16+ stderr=subprocess.PIPE,cwd=base_path
17+ )
18+ stdout,stderr = cmd.communicate()
19+ git_revnum = stdout.rstrip()
20 if git_revnum:
21- return 'unofficial-git-%s' % (git_revnum,)
22- else:
23- return None
24-
25- is_git_workdir = os.system('git show >/dev/null 2>&1') == 0
26- if is_git_workdir:
27- git_revnum = os.popen(
28- 'git show --pretty=format:%h | head -n 1').read().rstrip()
29- return 'unofficial-git-%s' % (git_revnum,)
30+ return 'unofficial-git-%s' % git_revnum
31+ except Exception as e:
32+ pass
33+ return None
34
35
36 def check_for_explicit_version():
37@@ -72,8 +71,12 @@
38
39 def detect_bzr_revision():
40 if __has_bzrlib:
41- b = BzrDir.open(base_path).open_branch()
42- revno, nick = b.revno(), b.nick
43+ try:
44+ b = BzrDir.open(base_path).open_branch()
45+ revno, nick = b.revno(), b.nick
46+ return 'bzr%s[%s]' % (revno, nick)
47+ except:
48+ return None
49 else:
50 # Windows stand alone installer do not come with bzrlib. We try to
51 # parse the output of bzr then directly
52@@ -83,9 +86,11 @@
53 ).stdout.read().strip().decode('utf-8')
54 revno = run_bzr('revno')
55 nick = run_bzr('nick')
56+ return 'bzr%s[%s]' % (revno, nick)
57 except OSError:
58 return None
59- return 'bzr%s[%s]' % (revno, nick)
60+ return None
61+
62
63
64 def detect_revision():

Subscribers

People subscribed via source and target branches

to status/vote changes: