Extract raw commit messages from Git more faithfully across Git versions

Summary:
Fixes T5028. Older versions of Git (apparently, from before 2010) did not provide a way to extract the raw body of a commit message from "git log", so we approximate it with "subject" and "wrapped body".

In newer versions of Git, the raw body can be extracted exactly.

Adjust how we extract messages based on the version of Git, and try to be more faithful to edge cases: particularly, be more careful to extract the correct number of trailing newlines.

Test Plan:
- Added "var_dump()" + "die(1)" later in this method, then pushed various commit messages. Used "&& false" to force execution down the old path (either path should work in modern Git).
- Observed more faithful extraction of messages, including a more faithful extraction of the number of trailing newlines. Extraction is fully faithful if we can go down the "%B" path, which we should be able to in nearly all modern cases.
- Not all messages extract faithfully or consistently across the old and new versions, but the old extraction is destructive so this is likely about as close as we can realistically ever get.

Maniphest Tasks: T5028

Differential Revision: https://secure.phabricator.com/D21027

epriestley 6 years ago 785f3c98 d3f4af4a

+59 -11

2 changed files

expand all

resources

celerity

map.php

src

applications

diffusion

query

lowlevel

DiffusionLowLevelCommitQuery.php

+7 -7

resources/celerity/map.php

··· 9 9 'names' => array( 10 10 'conpherence.pkg.css' => '3c8a0668', 11 11 'conpherence.pkg.js' => '020aebcf', 12 - 'core.pkg.css' => '5edb4679', 12 + 'core.pkg.css' => '7ef29af5', 13 13 'core.pkg.js' => '705aec2c', 14 14 'differential.pkg.css' => '607c84be', 15 15 'differential.pkg.js' => '1b97518d', ··· 31 31 'rsrc/css/aphront/panel-view.css' => '46923d46', 32 32 'rsrc/css/aphront/phabricator-nav-view.css' => 'f8a0c1bf', 33 33 'rsrc/css/aphront/table-view.css' => '0bb61df1', 34 - 'rsrc/css/aphront/tokenizer.css' => 'b52d0668', 34 + 'rsrc/css/aphront/tokenizer.css' => '34e2a838', 35 35 'rsrc/css/aphront/tooltip.css' => 'e3f2412f', 36 36 'rsrc/css/aphront/typeahead-browse.css' => 'b7ed02d2', 37 37 'rsrc/css/aphront/typeahead.css' => '8779483d', ··· 537 537 'aphront-multi-column-view-css' => 'fbc00ba3', 538 538 'aphront-panel-view-css' => '46923d46', 539 539 'aphront-table-view-css' => '0bb61df1', 540 - 'aphront-tokenizer-control-css' => 'b52d0668', 540 + 'aphront-tokenizer-control-css' => '34e2a838', 541 541 'aphront-tooltip-css' => 'e3f2412f', 542 542 'aphront-typeahead-control-css' => '8779483d', 543 543 'application-search-view-css' => '0f7c06d8', ··· 1220 1220 'javelin-stratcom', 1221 1221 'javelin-workflow', 1222 1222 ), 1223 + '34e2a838' => array( 1224 + 'aphront-typeahead-control-css', 1225 + 'phui-tag-view-css', 1226 + ), 1223 1227 '37b8a04a' => array( 1224 1228 'javelin-install', 1225 1229 'javelin-util', ··· 1944 1948 ), 1945 1949 'b517bfa0' => array( 1946 1950 'phui-oi-list-view-css', 1947 - ), 1948 - 'b52d0668' => array( 1949 - 'aphront-typeahead-control-css', 1950 - 'phui-tag-view-css', 1951 1951 ), 1952 1952 'b58d1a2a' => array( 1953 1953 'javelin-behavior',

+52 -4

src/applications/diffusion/query/lowlevel/DiffusionLowLevelCommitQuery.php

··· 41 41 private function loadGitCommitRef() { 42 42 $repository = $this->getRepository(); 43 43 44 - // NOTE: %B was introduced somewhat recently in git's history, so pull 45 - // commit message information with %s and %b instead. 44 + // See T5028. The "%B" (raw body) mode is not present in very old versions 45 + // of Git. Use "%s" and "%b" ("subject" and "wrapped body") as an 46 + // approximation. 47 + 48 + $git_binary = PhutilBinaryAnalyzer::getForBinary('git'); 49 + $git_version = $git_binary->getBinaryVersion(); 50 + if (version_compare($git_version, '1.7.2', '>=')) { 51 + $body_format = '%B'; 52 + $split_body = false; 53 + } else { 54 + $body_format = '%s%x00%b'; 55 + $split_body = true; 56 + } 46 57 47 58 // Even though we pass --encoding here, git doesn't always succeed, so 48 59 // we try a little harder, since git *does* tell us what the actual encoding ··· 52 63 'UTF-8', 53 64 implode( 54 65 '%x00', 55 - array('%e', '%cn', '%ce', '%an', '%ae', '%T', '%at', '%s%n%n%b')), 66 + array( 67 + '%e', 68 + '%cn', 69 + '%ce', 70 + '%an', 71 + '%ae', 72 + '%T', 73 + '%at', 74 + $body_format, 75 + 76 + // The "git log" output includes a trailing newline. We want to 77 + // faithfully capture only the exact text of the commit message, 78 + // so include an explicit terminator: this makes sure the exact 79 + // body text is surrounded by "\0" characters. 80 + '~', 81 + )), 56 82 $this->identifier); 57 83 58 84 $parts = explode("\0", $info); ··· 82 108 $author_epoch = null; 83 109 } 84 110 111 + if ($split_body) { 112 + // Here, the body is: "subject", "\0", "wrapped body". Stitch the 113 + // pieces back together by putting a newline between them if both 114 + // parts are nonempty. 115 + 116 + $head = $parts[6]; 117 + $tail = $parts[7]; 118 + 119 + if (strlen($head) && strlen($tail)) { 120 + $body = $head."\n\n".$tail; 121 + } else if (strlen($head)) { 122 + $body = $head; 123 + } else if (strlen($tail)) { 124 + $body = $tail; 125 + } else { 126 + $body = ''; 127 + } 128 + } else { 129 + // Here, the body is the raw unwrapped body. 130 + $body = $parts[6]; 131 + } 132 + 85 133 return id(new DiffusionCommitRef()) 86 134 ->setCommitterName($parts[0]) 87 135 ->setCommitterEmail($parts[1]) ··· 89 137 ->setAuthorEmail($parts[3]) 90 138 ->setHashes($hashes) 91 139 ->setAuthorEpoch($author_epoch) 92 - ->setMessage($parts[6]); 140 + ->setMessage($body); 93 141 } 94 142 95 143 private function loadMercurialCommitRef() {

Configure Feed

Configure Feed