Merge lp:~jameinel/goetveld/fix-log into lp:goetveld

[John A Meinel]

Handle some edge cases with 'bzr log'

You can alias 'bzr log' to set default arguments. Because of this, the output
doesn't conform to what rietveld expects. This just forces specific flags so
that the output will match expectations.

https://codereview.appspot.com/6822053/

[John A Meinel (jameinel)]

Reviewers: mp+132060_code.launchpad.net,

Message:
Please take a look.

Description:
Handle some edge cases with 'bzr log'

You can alias 'bzr log' to set default arguments. Because of this, the
output
doesn't conform to what rietveld expects. This just forces specific
flags so
that the output will match expectations.

https://code.launchpad.net/~jameinel/goetveld/fix-log/+merge/132060

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/6822053/

Affected files:
   A [revision details]
   M bazaar.go

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: <email address hidden>
+New revision: <email address hidden>

Index: bazaar.go
=== modified file 'bazaar.go'
--- bazaar.go 2012-03-15 02:02:17 +0000
+++ bazaar.go 2012-10-30 11:06:00 +0000
@@ -28,11 +28,11 @@
  // BazaarDiffBranches returns a Delta between the bazaar branch at
  // oldPath and the one at newPath.
  func BazaarDiffBranches(oldPath, newPath string) (Delta, error) {
- output1, _, err :=
run("bzr", "log", "-l1", "--show-ids", "-r", "ancestor:"+oldPath, newPath)
+ output1, _, err :=
run("bzr", "log", "--long", "-l1", "--show-ids", "-r", "ancestor:"+oldPath,
newPath)
   if err != nil {
    return nil, err
   }
- output2, _, err := run("bzr", "log", "-l1", "--show-ids", newPath)
+ output2, _, err :=
run("bzr", "log", "--long", "-l1", "--show-ids", "-r", "-1", newPath)
   if err != nil {
    return nil, err
   }

[lp:~jameinel/goetveld/fix-log]

42. By John A Meinel on 2012-10-30

rietveld/bazaar: use 'bzr revision-info' instead of 'bzr log'

We don't need to deal with lots of flags to 'bzr log', instead use
'bzr revision-info' which just gives us the data we actually care
about.
Also, 'strings.Fields' is a simpler way to parse then doing
string searching for the right context.

[John A Meinel (jameinel)]

Please take a look.

https://codereview.appspot.com/6822053/

[John A Meinel (jameinel)]

Poke about getting a review on this?

Unmerged revisions

42. By John A Meinel on 2012-10-30

rietveld/bazaar: use 'bzr revision-info' instead of 'bzr log'

We don't need to deal with lots of flags to 'bzr log', instead use
'bzr revision-info' which just gives us the data we actually care
about.
Also, 'strings.Fields' is a simpler way to parse then doing
string searching for the right context.

41. By John A Meinel on 2012-10-30

Change the use of 'bzr log' to force the format.

If you have any aliases defined, then bzr log can actually use
a different format, and the bazaar code won't be able to determine
the actual revision id. (eg --short puts ' revision-id:revid' rather
than 'revision-id: revid'.)

=== modified file 'bazaar.go'
--- bazaar.go	2012-03-15 02:02:17 +0000
+++ bazaar.go	2012-10-30 12:03:25 +0000
@@ -14,7 +14,6 @@
 
 import (
 	"bytes"
-	"errors"
 	"fmt"
 	"io"
 	"io/ioutil"
@@ -23,33 +22,36 @@
 	"strings"
 )
 
-var logRevId = []byte("\nrevision-id: ")
+func revisionInfoFromOutput(output string) (revisionId string, err error) {
+        fields := strings.Fields(output)
+        if len(fields) <= 1 || len(fields) > 2 {
+            err = fmt.Errorf("could not understand the output of bzr revision-info: %q", output)
+        } else {
+            // [0] is the revno, [1] is the revision-id
+            revisionId = fields[1]
+        }
+        return
+}
 
 // BazaarDiffBranches returns a Delta between the bazaar branch at
 // oldPath and the one at newPath.
 func BazaarDiffBranches(oldPath, newPath string) (Delta, error) {
-	output1, _, err := run("bzr", "log", "-l1", "--show-ids", "-r", "ancestor:"+oldPath, newPath)
-	if err != nil {
-		return nil, err
-	}
-	output2, _, err := run("bzr", "log", "-l1", "--show-ids", newPath)
-	if err != nil {
-		return nil, err
-	}
-	i1 := bytes.Index(output1, logRevId)
-	i2 := bytes.Index(output2, logRevId)
-	if i1 < 0 || i2 < 0 {
-		return nil, errors.New("no revision-id in bzr log output")
-	}
-	output1 = output1[i1+len(logRevId):]
-	output2 = output2[i2+len(logRevId):]
-	i1 = bytes.Index(output1, []byte{'\n'})
-	i2 = bytes.Index(output2, []byte{'\n'})
-	if i1 < 0 || i2 < 0 {
-		return nil, errors.New("bad revision-id in bzr log output")
-	}
-	oldRevision := string(output1[:i1])
-	newRevision := string(output2[:i2])
+        output1, _, err := run("bzr", "revision-info", "-r", "ancestor:"+oldPath, "-d", newPath)
+	if err != nil {
+		return nil, err
+	}
+        output2, _, err := run("bzr", "revision-info", "-r", "-1", "-d", newPath)
+	if err != nil {
+		return nil, err
+	}
+        oldRevision, err := revisionInfoFromOutput(string(output1))
+        if err != nil {
+            return nil, err
+        }
+        newRevision, err := revisionInfoFromOutput(string(output2))
+        if err != nil {
+            return nil, err
+        }
 	return &bzrBranches{oldPath, newPath, oldRevision, newRevision}, nil
 }