Change in osmo-dev[master]: gits: use @{u} to get upstream branch, not origin/%s

2018-11-20 Thread Neels Hofmeyr
Neels Hofmeyr has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/11756 )

Change subject: gits: use @{u} to get upstream branch, not origin/%s
..

gits: use @{u} to get upstream branch, not origin/%s

git has an internal concept of a branch's upstream branch, and the remote need
not be 'origin', and also, the upstream branch name may differ completely. So,
use git's {u} keyword to obtain the upstream branch name.

This removes all 'origin' strings from gits. Also, git_branch_exists() is no
longer needed, drop it.

Also remove a couple of default arguments which aren't ever used because we
always pass arguments anyway.

In the case of git_ahead_behind(), we would have use for a default
branch_upstream=None to imply calling git_branch_upstream(), but then during
rebase, if no upstream exists, we would invoke it twice to get None a second
time. So just call the function explicitly. I thought about returning an empty
string instead of None, but it's too convoluted.

In git_output(), pipe STDERR to STDOUT, because every time we parse a remote
ref (git ref-parse --abbrev-ref '%s{u}') and there is no remote branch, git
prints 'Fatal: there is no remote bla bla' on stdout, and that error is
expected / ok to happen, so it just clutters the 'gits' output. The easiest way
to achieve silence then is to pipe to STDOUT, IIUC otherwise we'd have to use
Popen() and communicate()... In the case of error, subprocess raises an
exception and we see that an error happens. In the ref-parse case we can simply
catch the exception and be quiet.

Change-Id: Ife146903ae1323a4e568587ccfd4018725e9d719
---
M src/gits
1 file changed, 30 insertions(+), 22 deletions(-)

Approvals:
  osmith: Looks good to me, but someone else must approve



diff --git a/src/gits b/src/gits
index cb2cf59..95b722e 100755
--- a/src/gits
+++ b/src/gits
@@ -58,7 +58,7 @@


 def git_output(git_dir, *args):
-return subprocess.check_output(['git', '-C', git_dir, ] + 
list(args)).decode('utf-8')
+return subprocess.check_output(['git', '-C', git_dir, ] + list(args), 
stderr=subprocess.STDOUT).decode('utf-8')


 def git_bool(git_dir, *args):
@@ -69,20 +69,16 @@
 return False


-def git_branch_exists(git_dir, branch='origin/master'):
-return git_bool(git_dir, 'rev-parse', '--quiet', '--verify', branch)
-
-
-def git_ahead_behind(git_dir, branch='master', remote='origin'):
+def git_ahead_behind(git_dir, branch, branch_upstream):
 ''' Count revisions ahead/behind of the remote branch.
 returns: (ahead, behind) (e.g. (0, 5)) '''

 # Missing remote branch
-if not git_branch_exists(git_dir, remote + '/' + branch):
+if not branch_upstream:
 return (0, 0)

-behind = git_output(git_dir, 'rev-list', '--count', '%s..%s/%s' % (branch, 
remote, branch))
-ahead = git_output(git_dir, 'rev-list', '--count', '%s/%s..%s' % (remote, 
branch, branch))
+behind = git_output(git_dir, 'rev-list', '--count', '%s..%s' % (branch, 
branch_upstream))
+ahead = git_output(git_dir, 'rev-list', '--count', '%s..%s' % 
(branch_upstream, branch))
 return (int(ahead.rstrip()), int(behind.rstrip()))


@@ -98,12 +94,20 @@
 return ret


+def git_branch_upstream(git_dir, branch_name='HEAD'):
+'''Return an upstream branch name, or an None if there is none.'''
+try:
+return git_output(git_dir, 'rev-parse', '--abbrev-ref', '%s@{u}' % 
branch_name).rstrip()
+except subprocess.CalledProcessError:
+return None
+
+
 def git_has_modifications(git_dir):
 return not git_bool(git_dir, 'diff', '--quiet', 'HEAD')


-def git_can_fast_forward(git_dir, branch='master', remote='origin'):
-return git_bool(git_dir, 'merge-base', '--is-ancestor', branch, remote + 
'/' + branch)
+def git_can_fast_forward(git_dir, branch, branch_upstream):
+return git_bool(git_dir, 'merge-base', '--is-ancestor', branch, 
branch_upstream)


 def format_branch_ahead_behind(branch, ahead, behind):
@@ -142,12 +146,14 @@
 if not is_current and branch not in interesting_branch_names:
 continue

-ahead, behind = git_ahead_behind(git_dir, branch)
+ahead, behind = git_ahead_behind(git_dir, branch,
+ git_branch_upstream(git_dir, branch))
+
 if not ahead and not behind and not is_current:
 # skip branches that are "not interesting"
 continue

-# Branch with ahead/behind origin info ("master[+1|-5]")
+# Branch with ahead/behind upstream info ("master[+1|-5]")
 strs.append(format_branch_ahead_behind(branch, ahead, behind))
 return strs

@@ -238,8 +244,10 @@
 print('Not on a branch: %s' % git_dir)
 raise SkipThisRepo()

-print('Rebasing branch: ' + orig_branch)
-ahead, behind = git_ahead_behind(git_dir, orig_branch)
+upstream_branch = git_branch_upstream(git_dir, orig_branch)
+
+

Change in osmo-dev[master]: gits: use @{u} to get upstream branch, not origin/%s

2018-11-20 Thread osmith
osmith has posted comments on this change. ( https://gerrit.osmocom.org/11756 )

Change subject: gits: use @{u} to get upstream branch, not origin/%s
..


Patch Set 4: Code-Review+1


--
To view, visit https://gerrit.osmocom.org/11756
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-dev
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ife146903ae1323a4e568587ccfd4018725e9d719
Gerrit-Change-Number: 11756
Gerrit-PatchSet: 4
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: osmith 
Gerrit-Comment-Date: Tue, 20 Nov 2018 09:31:52 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes


Change in osmo-dev[master]: gits: use @{u} to get upstream branch, not origin/%s

2018-11-19 Thread Neels Hofmeyr
Hello osmith,

I'd like you to reexamine a change. Please visit

https://gerrit.osmocom.org/11756

to look at the new patch set (#4).

Change subject: gits: use @{u} to get upstream branch, not origin/%s
..

gits: use @{u} to get upstream branch, not origin/%s

git has an internal concept of a branch's upstream branch, and the remote need
not be 'origin', and also, the upstream branch name may differ completely. So,
use git's {u} keyword to obtain the upstream branch name.

This removes all 'origin' strings from gits. Also, git_branch_exists() is no
longer needed, drop it.

Also remove a couple of default arguments which aren't ever used because we
always pass arguments anyway.

In the case of git_ahead_behind(), we would have use for a default
branch_upstream=None to imply calling git_branch_upstream(), but then during
rebase, if no upstream exists, we would invoke it twice to get None a second
time. So just call the function explicitly. I thought about returning an empty
string instead of None, but it's too convoluted.

In git_output(), pipe STDERR to STDOUT, because every time we parse a remote
ref (git ref-parse --abbrev-ref '%s{u}') and there is no remote branch, git
prints 'Fatal: there is no remote bla bla' on stdout, and that error is
expected / ok to happen, so it just clutters the 'gits' output. The easiest way
to achieve silence then is to pipe to STDOUT, IIUC otherwise we'd have to use
Popen() and communicate()... In the case of error, subprocess raises an
exception and we see that an error happens. In the ref-parse case we can simply
catch the exception and be quiet.

Change-Id: Ife146903ae1323a4e568587ccfd4018725e9d719
---
M src/gits
1 file changed, 30 insertions(+), 22 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-dev refs/changes/56/11756/4
--
To view, visit https://gerrit.osmocom.org/11756
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-dev
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ife146903ae1323a4e568587ccfd4018725e9d719
Gerrit-Change-Number: 11756
Gerrit-PatchSet: 4
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: osmith 


Change in osmo-dev[master]: gits: use @{u} to get upstream branch, not origin/%s

2018-11-19 Thread Neels Hofmeyr
Neels Hofmeyr has posted comments on this change. ( 
https://gerrit.osmocom.org/11756 )

Change subject: gits: use @{u} to get upstream branch, not origin/%s
..


Patch Set 3:

(1 comment)

https://gerrit.osmocom.org/#/c/11756/1/src/gits
File src/gits:

https://gerrit.osmocom.org/#/c/11756/1/src/gits@61
PS1, Line 61: return subprocess.check_output(['git', '-C', git_dir, ] + 
list(args), stderr=subprocess.STDOUT).decode('utf-8')
> why is this needed?
because every time we parse a remote ref (git ref-parse --abbrev-ref '%s{u}') 
and there is no remote branch, git prints 'Fatal: there is no remote bla bla' 
on stdout, and that error is expected / ok to happen, so it just clutters the 
'gits' output.

In the other cases, if there is an error, subprocess raises an exception and we 
see that an error happens. In the ref-parse case we can simply catch the 
exception and be quiet.



--
To view, visit https://gerrit.osmocom.org/11756
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-dev
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ife146903ae1323a4e568587ccfd4018725e9d719
Gerrit-Change-Number: 11756
Gerrit-PatchSet: 3
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: osmith 
Gerrit-Comment-Date: Mon, 19 Nov 2018 15:54:43 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: No


Change in osmo-dev[master]: gits: use @{u} to get upstream branch, not origin/%s

2018-11-14 Thread osmith
osmith has posted comments on this change. ( https://gerrit.osmocom.org/11756 )

Change subject: gits: use @{u} to get upstream branch, not origin/%s
..


Patch Set 1: Code-Review-1


--
To view, visit https://gerrit.osmocom.org/11756
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-dev
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ife146903ae1323a4e568587ccfd4018725e9d719
Gerrit-Change-Number: 11756
Gerrit-PatchSet: 1
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: osmith 
Gerrit-Comment-Date: Wed, 14 Nov 2018 09:23:15 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes


Change in osmo-dev[master]: gits: use @{u} to get upstream branch, not origin/%s

2018-11-13 Thread osmith
osmith has posted comments on this change. ( https://gerrit.osmocom.org/11756 )

Change subject: gits: use @{u} to get upstream branch, not origin/%s
..


Patch Set 1:

(1 comment)

Nice! besides catching the git stderr this looks good to me.

https://gerrit.osmocom.org/#/c/11756/1/src/gits
File src/gits:

https://gerrit.osmocom.org/#/c/11756/1/src/gits@61
PS1, Line 61: return subprocess.check_output(['git', '-C', git_dir, ] + 
list(args), stderr=subprocess.STDOUT).decode('utf-8')
why is this needed?



--
To view, visit https://gerrit.osmocom.org/11756
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-dev
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ife146903ae1323a4e568587ccfd4018725e9d719
Gerrit-Change-Number: 11756
Gerrit-PatchSet: 1
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: osmith 
Gerrit-Comment-Date: Tue, 13 Nov 2018 10:31:59 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: No


Change in osmo-dev[master]: gits: use @{u} to get upstream branch, not origin/%s

2018-11-12 Thread Neels Hofmeyr
Neels Hofmeyr has uploaded this change for review. ( 
https://gerrit.osmocom.org/11756


Change subject: gits: use @{u} to get upstream branch, not origin/%s
..

gits: use @{u} to get upstream branch, not origin/%s

git has an internal concept of a branch's upstream branch, and the remote need
not be 'origin', and also, the upstream branch name may differ completely. So,
use git's {u} keyword to obtain the upstream branch name.

This removes all 'origin' strings from gits. Also, git_branch_exists() is no
longer needed, drop it.

Also remove a couple of default arguments which aren't ever used because we
always pass arguments anyway.

In the case of git_ahead_behind(), we would have use for a default
branch_upstream=None to imply calling git_branch_upstream(), but then during
rebase, if no upstream exists, we would invoke it twice to get None a second
time. So just call the function explicitly. I thought about returning an empty
string instead of None, but it's too convoluted.

Change-Id: Ife146903ae1323a4e568587ccfd4018725e9d719
---
M src/gits
1 file changed, 30 insertions(+), 22 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/osmo-dev refs/changes/56/11756/1

diff --git a/src/gits b/src/gits
index 490a6af..681082d 100755
--- a/src/gits
+++ b/src/gits
@@ -58,7 +58,7 @@


 def git_output(git_dir, *args):
-return subprocess.check_output(['git', '-C', git_dir, ] + 
list(args)).decode('utf-8')
+return subprocess.check_output(['git', '-C', git_dir, ] + list(args), 
stderr=subprocess.STDOUT).decode('utf-8')


 def git_bool(git_dir, *args):
@@ -69,20 +69,16 @@
 return False


-def git_branch_exists(git_dir, branch='origin/master'):
-return git_bool(git_dir, 'rev-parse', '--quiet', '--verify', branch)
-
-
-def git_ahead_behind(git_dir, branch='master', remote='origin'):
+def git_ahead_behind(git_dir, branch, branch_upstream):
 ''' Count revisions ahead/behind of the remote branch.
 returns: (ahead, behind) (e.g. (0, 5)) '''

 # Missing remote branch
-if not git_branch_exists(git_dir, remote + '/' + branch):
+if not branch_upstream:
 return (0, 0)

-behind = git_output(git_dir, 'rev-list', '--count', '%s..%s/%s' % (branch, 
remote, branch))
-ahead = git_output(git_dir, 'rev-list', '--count', '%s/%s..%s' % (remote, 
branch, branch))
+behind = git_output(git_dir, 'rev-list', '--count', '%s..%s' % (branch, 
branch_upstream))
+ahead = git_output(git_dir, 'rev-list', '--count', '%s..%s' % 
(branch_upstream, branch))
 return (int(ahead.rstrip()), int(behind.rstrip()))


@@ -98,12 +94,20 @@
 return ret


+def git_branch_upstream(git_dir, branch_name='HEAD'):
+'''Return an upstream branch name, or an None if there is none.'''
+try:
+return git_output(git_dir, 'rev-parse', '--abbrev-ref', '%s@{u}' % 
branch_name).rstrip()
+except subprocess.CalledProcessError:
+return None
+
+
 def git_has_modifications(git_dir):
 return not git_bool(git_dir, 'diff-index', '--quiet', 'HEAD')


-def git_can_fast_forward(git_dir, branch='master', remote='origin'):
-return git_bool(git_dir, 'merge-base', '--is-ancestor', branch, remote + 
'/' + branch)
+def git_can_fast_forward(git_dir, branch, branch_upstream):
+return git_bool(git_dir, 'merge-base', '--is-ancestor', branch, 
branch_upstream)


 def format_branch_ahead_behind(branch, ahead, behind):
@@ -142,12 +146,14 @@
 if not is_current and branch not in interesting_branch_names:
 continue

-ahead, behind = git_ahead_behind(git_dir, branch)
+ahead, behind = git_ahead_behind(git_dir, branch,
+ git_branch_upstream(git_dir, branch))
+
 if not ahead and not behind and not is_current:
 # skip branches that are "not interesting"
 continue

-# Branch with ahead/behind origin info ("master[+1|-5]")
+# Branch with ahead/behind upstream info ("master[+1|-5]")
 strs.append(format_branch_ahead_behind(branch, ahead, behind))
 return strs

@@ -238,8 +244,10 @@
 print('Not on a branch: %s' % git_dir)
 raise SkipThisRepo()

-print('Rebasing branch: ' + orig_branch)
-ahead, behind = git_ahead_behind(git_dir, orig_branch)
+upstream_branch = git_branch_upstream(git_dir, orig_branch)
+
+print('Rebasing %r onto %r' % (orig_branch, upstream_branch))
+ahead, behind = git_ahead_behind(git_dir, orig_branch, upstream_branch)

 if git_has_modifications(git_dir):
 do_commit = ask(git_dir, 'Local mods.',
@@ -262,24 +270,24 @@
 raise SkipThisRepo()

 # Missing upstream branch
-if not git_branch_exists(git_dir, 'origin/' + orig_branch):
+if not upstream_branch:
 print('there is no upstream branch for %r' % orig_branch)

 # Diverged
 elif ahead and behind:
 do_reset = ask(git_dir, 'Diverged.',
-