1595104 - moz-phab truncates long branch names in output with --apply-to

Status: RESOLVED FIXED

(Conduit :: moz-phab, defect, P3)

Description

[Julien Wajsberg [:julienw]]

This is the command I used:
moz-phab patch https://phabricator.services.mozilla.com/D50667 --apply-to mozilla/central
I got:

Checked out mozilla/cent

Similarly for mozilla/autoland I got:

Checked out mozilla/auto

I'm on a git setup.

Comment 1

[:glob ✱]

When displaying the base_node, moz-phab assumes that it's a SHA and truncates. The full name is used when performing operations so this is a cosmetic issue (but still important to fix as it appears to be broken).

# Checkout sha
if node:
    with wait_message("Checking out %s.." % node[:12]):
        self.checkout(node)
    if not self.args.raw:
        logger.info("Checked out %s" % node[:12])

We probably need a short_node function which only shortens strings that look like a commit SHA (/^[0-9a-f]{40}$/)

Comment 2

[:glob ✱]

Attachment: Bug 1595104 - Move style tests from .circleci to test suite; r=zalun

To make it easier to run and catch style issues, move the checks from
circle-ci only into the test suite.

Comment 3

[:glob ✱]

Attachment: Bug 1595104 - Only truncate commit SHAs to 12 chars in messages; r=zalun

Depends on D52500

Comment 4

[Julien Wajsberg [:julienw]]

(In reply to Byron Jones ‹:glob› 🎈 from comment #1)

We probably need a short_node function which only shortens strings that look like a commit SHA (/^[0-9a-f]{40}$/)

Maybe use {12,40} instead of 40.

But maybe better is to use git commands to know if some revision info is a reference or a sha.
For example:

# This errors if ref isn't a valid reference. With --quiet it doesn't output anything.
git show-ref --verify --quiet <ref>

# This errors if ref-or-sha isn't a valid reference or a valid sha in this repository
# Otherwise it prints the sha for this ref.
git rev-parse --verify <ref-or-sha>

Maybe there are others I don't know, or some attributes for these commands that could be more useful.
Hope this helps!

Comment 5

[Julien Wajsberg [:julienw]]

Maybe use {12,40} instead of 40.

This suggestion because a start hash is valid if it unambigously matches the start of the full hash.

Comment 6

[Julien Wajsberg [:julienw]]

BTW, git rev-parse --short <ref-or-hash> will get you the shortest matching hash, too. Slicing to 12 characters may still be ambiguous if we're unlucky.

Comment 7

[:glob ✱]

I don't think the performance hit for calling a git command to ensure we display a SHA correctly is worth it.

Comment 8

[:glob ✱]

https://github.com/mozilla-conduit/review/commit/3fe4d196c6c0c109f38f688b673bb06639601fa1
https://github.com/mozilla-conduit/review/commit/abe41a3a5c2cf5ef8828aa8c9d83e60ecbdabe0d

Comment 9

[Julien Wajsberg [:julienw]]

Despite the size of the mozilla-central unified git directory, I find these commands to be super fast, 4ms in my case. (Other commands are indeed slow in this huge repo, maybe half a second, to give a point of comparison).

Is this context performance-sensitive enough to be especially careful about 4ms?

BTW on my repository, git rev-parse --short actually returns a 13-character hash. 12 character doesn't seem long enough to be unambiguous after all, and the git command would actually help. Or maybe let's not slice at all? :-)