Page MenuHomePhabricator

Further correct and disambigutate ref selectors passed to Git on the CLI
ClosedPublic

Authored by epriestley on Jan 20 2021, 7:44 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Apr 6, 10:05 PM
Unknown Object (File)
Wed, Apr 3, 1:23 PM
Unknown Object (File)
Sat, Mar 30, 9:45 AM
Unknown Object (File)
Sat, Mar 30, 9:45 AM
Unknown Object (File)
Sat, Mar 30, 9:45 AM
Unknown Object (File)
Sat, Mar 30, 9:45 AM
Unknown Object (File)
Wed, Mar 27, 5:18 PM
Unknown Object (File)
Wed, Mar 20, 10:26 PM
Subscribers
None

Details

Summary

Ref T13589. In D21510, not every ref selector got touched, and this isn't a valid construction in Git:

$ git ls-tree ... -- ''

Thus:

  • Disambiguate more (all?) ref selectors.
  • Correct the construction of "git ls-tree" when there is no path.
  • Clean some stuff up: make the construction of some flags and arguments more explicit, get rid of a needless "%C", prefer "%Ls" over acrobatics, etc.
Test Plan

Browsed/updated a local Git repository. (This change is somewhat difficult to test exhaustively, as evidenced by the "ls-tree" issue in D21510.)

Diff Detail

Repository
rP Phabricator
Branch
git2
Lint
Lint Passed
Unit
Test Failures
Build Status
Buildable 25013
Build 34510: Run Core Tests
Build 34509: arc lint + arc unit

Unit TestsFailed

TimeTest
55 msPhabricatorWorkingCopyDiscoveryTestCase::testSubversionCommitDiscovery
EXCEPTION (CommandException): Command failed with error #127! COMMAND svn --non-interactive info --xml file:///var/folders/72/x4dt4z152d79y3wf2khgbgtr0000gn/T/6rivnpj9ofwg8sgk
0 msAlmanacNamesTestCase::testServiceOrDeviceNames
30 assertions passed.
0 msAlmanacServiceTypeTestCase::testGetAllServiceTypes
1 assertion passed.
0 msAphrontHTTPHeaderParserTestCase::testHeaderParser
18 assertions passed.
0 msAphrontHTTPSinkTestCase::testHTTPHeaderNames
2 assertions passed.
View Full Test Results (1 Failed · 429 Passed · 4 Skipped)

Event Timeline

Harbormaster completed remote builds in B25013: Diff 51200.
  • Test for "svn" before running an SVN test to fix the local test failure, since this new machine doesn't have "svn" installed yet.
This revision was not accepted when it landed; it landed in state Needs Review.Jan 20 2021, 8:07 PM
This revision was automatically updated to reflect the committed changes.