Skip to content

Commit 127ba36

Browse files
committed
fix(git-node): handle multi-line trailers
1 parent f0587fa commit 127ba36

2 files changed

Lines changed: 43 additions & 15 deletions

File tree

lib/landing_session.js

Lines changed: 23 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -316,35 +316,43 @@ export default class LandingSession extends Session {
316316

317317
// git has very specific rules about what is a trailer and what is not.
318318
// Instead of trying to implement those ourselves, let git parse the
319-
// original commit message and see if it outputs any trailers.
320-
const originalTrailers = runSync('git', [
319+
// commit message and see if it outputs any trailers.
320+
const interpretTrailers = commitMessage => runSync('git', [
321321
'interpret-trailers', '--parse', '--no-divider'
322322
], {
323-
input: `${original}\n`
324-
}).split('\n').map(trailer => {
325-
const separatorIndex = trailer.indexOf(':');
326-
return [trailer.slice(0, separatorIndex), trailer.slice(separatorIndex + 2)];
327-
}).slice(0, -1); // Remove the last line return entry
323+
input: `${commitMessage}\n`
324+
});
328325

326+
const originalTrailers = interpretTrailers(original);
329327
const containCVETrailer = CVE_RE.test(originalTrailers);
330328

331329
const filterTrailer = (line) => ([key]) =>
332330
line.startsWith(key) &&
333331
new RegExp(`^${RegExp.escape?.(key) ?? key}\\s*:`).test(line);
334-
const amended = original.trim().split('\n')
335-
.filter(line =>
336-
!line.includes(':') ||
337-
!originalTrailers.some(filterTrailer(line)));
338-
for (let i = amended.length - 1; amended[i] === ''; i--) {
332+
const amended = original.trim().split('\n');
333+
const stillInTrailers = () => {
334+
const result = interpretTrailers(amended.join('\n'));
335+
return result.length && originalTrailers.startsWith(result.trim());
336+
}
337+
for (let i = amended.length - 1; amended[i] === '' || stillInTrailers(); i--) {
338+
// Remove last line until git no longer detects any trailers
339339
amended.pop();
340340
}
341341

342342
// Only keep existing trailers that we won't add ourselves
343343
const trailersToFilterOut = ['BACKPORT-PR-URL', 'REVIEWED-BY'];
344344
if (!this.backport) trailersToFilterOut.push('PR-URL');
345-
const keptTrailers = originalTrailers.filter(
346-
([key]) => !trailersToFilterOut.includes(key.toUpperCase())
347-
);
345+
const keptTrailers =
346+
originalTrailers
347+
.split('\n')
348+
.map(trailer => {
349+
const separatorIndex = trailer.indexOf(':');
350+
return [trailer.slice(0, separatorIndex), trailer.slice(separatorIndex + 2)];
351+
})
352+
.slice(0, -1) // Remove the last line return entry
353+
.filter(
354+
([key]) => !trailersToFilterOut.includes(key.toUpperCase())
355+
);
348356
amended.push('', ...keptTrailers.map(([key, value]) => `${key}: ${value}`));
349357

350358
for (const line of metadata.trim().split('\n')) {

test/unit/amend.test.js

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,26 @@ describe('LandingSession.prototype.generateAmendedMessage', () => {
112112
t.assert.strictEqual(result, '\nPR-URL: http://example.com/123\nReviewed-By: user1 <[email protected]>');
113113
});
114114

115+
it('should handle multi-line trailers', async(t) => {
116+
const session = createSession();
117+
const result = await session.generateAmendedMessage(
118+
'subsystem: foobar\n\nSigned-off-by: Mike McCready\n <[email protected]>\n'
119+
);
120+
121+
t.assert.strictEqual(result,
122+
'subsystem: foobar\n\nSigned-off-by: Mike McCready <[email protected]>\nPR-URL: http://example.com/123\nReviewed-By: user1 <[email protected]>');
123+
});
124+
125+
it('should not remove lines that look like trailers in the commit body', async(t) => {
126+
const session = createSession();
127+
const result = await session.generateAmendedMessage(
128+
'subsystem: foobar\n\nNot-A-Trailer: http://example.com/\n\nSigned-off-by: Mike McCready\n <[email protected]>\n'
129+
);
130+
131+
t.assert.strictEqual(result,
132+
'subsystem: foobar\n\nNot-A-Trailer: http://example.com/\n\nSigned-off-by: Mike McCready <[email protected]>\nPR-URL: http://example.com/123\nReviewed-By: user1 <[email protected]>');
133+
});
134+
115135
it('should handle cherry-pick from upstream', async(t) => {
116136
const session = createSession({ metadata: 'Refs: https://github.com/v8/v8/commit/cf1bce40a5ef4c7c1da351754f5bf526c0c96463\n' });
117137
const result = await session.generateAmendedMessage(`deps: V8: cherry-pick cf1bce40a5ef

0 commit comments

Comments
 (0)