New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 737688 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Long OOO (go/where-is-mgiuca)
Closed: Jul 2017
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug-Regression



Sign in to add a comment

tests/git_hyper_blame_test.py broken in depot_tools

Project Member Reported by thakis@chromium.org, Jun 28 2017

Issue description

On current depot_tools master:

thakis@thakis:~/src/depot_tools$ python tests/git_hyper_blame_test.py 
.....F................
======================================================================
FAIL: testBadFilename (__main__.GitHyperBlameMainTest)
Tests the main function (bad filename).
----------------------------------------------------------------------
Traceback (most recent call last):
  File "tests/git_hyper_blame_test.py", line 136, in testBadFilename
    self.repo['C'], stderr.getvalue())
AssertionError: 'fatal: no such path some/files/xxxx in a3084d72cb26667a133652f99ca4f3fb2a7e0eac\n' != 'fatal: no such path some/files/xxxx in /usr/share/locale/en_US/LC_MESSAGES/git.mo\n'

----------------------------------------------------------------------
Ran 22 tests in 1.455s

FAILED (failures=1)

 
Labels: Restrict-View-SecurityTeam
Status: ExternalDependency (was: Unconfirmed)
Well... this is weird. Seems like a regression in git.

$ git blame `git rev-parse HEAD` -- xyz
fatal: no such path xyz in /usr/share/locale/en_US/LC_MESSAGES/git.mo

This makes no sense.

$ git blame HEAD -- xyz
fatal: no such path xyz in ȷs?�?

This makes less sense.

Confirmed on git tip of tree (5116f791c1, v2.13).

I'll file a bug upstream. In the meantime, do you think we can leave this test as failing or do you want me to temporarily disable it?
I'll do a full bisect later. For now...

Last known good: e572fef9d459497de2bd719747d5625a27c9b41d (Jan 2016)
First known bad: dc076ae5d9fe8c2fbf238b265c1bc1f6f089fd16 (May 2017)

(That's a big bisect.)
Bisect shows 835c49f7d16c5f0307e71da19618fcfe500c2430 is the culprit ("blame: rework methods that determine 'final' commit").

Comment 4 by mgiuca@chromium.org, Jul 11 2017

Update: I have a patch for this which I am going to send to the Git mailing list. I'm just checking whether there is any special process for a potential security vulnerability (since this is a use-after-free in git-blame).

Comment 5 by thakis@chromium.org, Jul 11 2017

Nice!

Re comment 0: I touch depot_tools maybe once a year, so personally I don't care much. But having passing tests on presubmit seems fairly basic on a project's hierarchy of needs, so disabling it might be nice.
Project Member

Comment 6 by bugdroid1@chromium.org, Jul 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/depot_tools/+/6bac8a849974017b8a78c27b97cd52f668bd033f

commit 6bac8a849974017b8a78c27b97cd52f668bd033f
Author: Matt Giuca <mgiuca@chromium.org>
Date: Thu Jul 13 17:18:50 2017

GitHyperBlameMainTest.testBadFilename: Work around git-blame bug.

A regression in git-blame prints an incorrect error message which causes
this test case to fail. Alter the test to only check the start of the
string, until the bug is fixed upstream.

Bug:  737688 
Change-Id: I4045cb8792d8abe984215c7198e213b23e9f6f5d
Reviewed-on: https://chromium-review.googlesource.com/567778
Commit-Queue: Nico Weber <thakis@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>

[modify] https://crrev.com/6bac8a849974017b8a78c27b97cd52f668bd033f/tests/git_hyper_blame_test.py

Comment 7 by mgiuca@chromium.org, Jul 14 2017

Labels: -Restrict-View-SecurityTeam
Status: Fixed (was: ExternalDependency)
Fixed test. I will post more info about the upstreaming of the fix to git-blame here.

Also removing security since the fix is posted to a public mailing list.

Sign in to add a comment