Skip to content

Commit 8856415

Browse files
cory-millercory-miller
authored
Implement branch list using callbacks from exec function (#1045)
When trying to list local branches to figure out what needs cleaned up during runs on non-ephemeral Actions Runners, we use git rev-parse --symbolic-full-name to get a list of branches. This can lead to ambiguous ref name errors when there are branches and tags with similar names. Part of the reason we use rev-parse --symbolic-full-name vs git branch --list or git rev-parse --symbolic seems to related to a bug in Git 2.18. Until we can deprecate our usage of Git 2.18, I think we need to keep --symbolic-full-name. Since part of the problem is that these ambiguous ref name errors clog the Actions annotation limits, this is a mitigation to suppress those messages until we can get rid of the workaround.
1 parent 755da8c commit 8856415

File tree

5 files changed

+240
-54
lines changed

5 files changed

+240
-54
lines changed

‎.licenses/npm/qs.dep.yml‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎__test__/git-command-manager.test.ts‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
import * as exec from '@actions/exec'
2+
import * as fshelper from '../lib/fs-helper'
3+
import * as commandManager from '../lib/git-command-manager'
4+
5+
let git: commandManager.IGitCommandManager
6+
let mockExec = jest.fn()
7+
8+
describe('git-auth-helper tests', () => {
9+
beforeAll(async () => {})
10+
11+
beforeEach(async () => {
12+
jest.spyOn(fshelper, 'fileExistsSync').mockImplementation(jest.fn())
13+
jest.spyOn(fshelper, 'directoryExistsSync').mockImplementation(jest.fn())
14+
})
15+
16+
afterEach(() => {
17+
jest.restoreAllMocks()
18+
})
19+
20+
afterAll(() => {})
21+
22+
it('branch list matches', async () => {
23+
mockExec.mockImplementation((path, args, options) => {
24+
console.log(args, options.listeners.stdout)
25+
26+
if (args.includes('version')) {
27+
options.listeners.stdout(Buffer.from('2.18'))
28+
return 0
29+
}
30+
31+
if (args.includes('rev-parse')) {
32+
options.listeners.stdline(Buffer.from('refs/heads/foo'))
33+
options.listeners.stdline(Buffer.from('refs/heads/bar'))
34+
return 0
35+
}
36+
37+
return 1
38+
})
39+
jest.spyOn(exec, 'exec').mockImplementation(mockExec)
40+
const workingDirectory = 'test'
41+
const lfs = false
42+
git = await commandManager.createCommandManager(workingDirectory, lfs)
43+
44+
let branches = await git.branchList(false)
45+
46+
expect(branches).toHaveLength(2)
47+
expect(branches.sort()).toEqual(['foo', 'bar'].sort())
48+
})
49+
50+
it('ambiguous ref name output is captured', async () => {
51+
mockExec.mockImplementation((path, args, options) => {
52+
console.log(args, options.listeners.stdout)
53+
54+
if (args.includes('version')) {
55+
options.listeners.stdout(Buffer.from('2.18'))
56+
return 0
57+
}
58+
59+
if (args.includes('rev-parse')) {
60+
options.listeners.stdline(Buffer.from('refs/heads/foo'))
61+
// If refs/tags/v1 and refs/heads/tags/v1 existed on this repository
62+
options.listeners.errline(
63+
Buffer.from("error: refname 'tags/v1' is ambiguous")
64+
)
65+
return 0
66+
}
67+
68+
return 1
69+
})
70+
jest.spyOn(exec, 'exec').mockImplementation(mockExec)
71+
const workingDirectory = 'test'
72+
const lfs = false
73+
git = await commandManager.createCommandManager(workingDirectory, lfs)
74+
75+
let branches = await git.branchList(false)
76+
77+
expect(branches).toHaveLength(1)
78+
expect(branches.sort()).toEqual(['foo'].sort())
79+
})
80+
})

‎dist/index.js‎

Lines changed: 96 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -7441,27 +7441,53 @@ class GitCommandManager {
74417441
const result = [];
74427442
// Note, this implementation uses "rev-parse --symbolic-full-name" because the output from
74437443
// "branch --list" is more difficult when in a detached HEAD state.
7444-
// Note, this implementation uses "rev-parse --symbolic-full-name" because there is a bug
7445-
// in Git 2.18 that causes "rev-parse --symbolic" to output symbolic full names.
7444+
// TODO(https://github.com/actions/checkout/issues/786): this implementation uses
7445+
// "rev-parse --symbolic-full-name" because there is a bug
7446+
// in Git 2.18 that causes "rev-parse --symbolic" to output symbolic full names. When
7447+
// 2.18 is no longer supported, we can switch back to --symbolic.
74467448
const args = ['rev-parse', '--symbolic-full-name'];
74477449
if (remote) {
74487450
args.push('--remotes=origin');
74497451
}
74507452
else {
74517453
args.push('--branches');
74527454
}
7453-
const output = yield this.execGit(args);
7454-
for (let branch of output.stdout.trim().split('\n')) {
7455+
const stderr = [];
7456+
const errline = [];
7457+
const stdout = [];
7458+
const stdline = [];
7459+
const listeners = {
7460+
stderr: (data) => {
7461+
stderr.push(data.toString());
7462+
},
7463+
errline: (data) => {
7464+
errline.push(data.toString());
7465+
},
7466+
stdout: (data) => {
7467+
stdout.push(data.toString());
7468+
},
7469+
stdline: (data) => {
7470+
stdline.push(data.toString());
7471+
}
7472+
};
7473+
// Suppress the output in order to avoid flooding annotations with innocuous errors.
7474+
yield this.execGit(args, false, true, listeners);
7475+
core.debug(`stderr callback is: ${stderr}`);
7476+
core.debug(`errline callback is: ${errline}`);
7477+
core.debug(`stdout callback is: ${stdout}`);
7478+
core.debug(`stdline callback is: ${stdline}`);
7479+
for (let branch of stdline) {
74557480
branch = branch.trim();
7456-
if (branch) {
7457-
if (branch.startsWith('refs/heads/')) {
7458-
branch = branch.substr('refs/heads/'.length);
7459-
}
7460-
else if (branch.startsWith('refs/remotes/')) {
7461-
branch = branch.substr('refs/remotes/'.length);
7462-
}
7463-
result.push(branch);
7481+
if (!branch) {
7482+
continue;
7483+
}
7484+
if (branch.startsWith('refs/heads/')) {
7485+
branch = branch.substring('refs/heads/'.length);
7486+
}
7487+
else if (branch.startsWith('refs/remotes/')) {
7488+
branch = branch.substring('refs/remotes/'.length);
74647489
}
7490+
result.push(branch);
74657491
}
74667492
return result;
74677493
});
@@ -7712,7 +7738,7 @@ class GitCommandManager {
77127738
return result;
77137739
});
77147740
}
7715-
execGit(args, allowAllExitCodes = false, silent = false) {
7741+
execGit(args, allowAllExitCodes = false, silent = false, customListeners = {}) {
77167742
return __awaiter(this, void 0, void 0, function* () {
77177743
fshelper.directoryExistsSync(this.workingDirectory, true);
77187744
const result = new GitOutput();
@@ -7723,20 +7749,24 @@ class GitCommandManager {
77237749
for (const key of Object.keys(this.gitEnv)) {
77247750
env[key] = this.gitEnv[key];
77257751
}
7752+
const defaultListener = {
7753+
stdout: (data) => {
7754+
stdout.push(data.toString());
7755+
}
7756+
};
7757+
const mergedListeners = Object.assign(Object.assign({}, defaultListener), customListeners);
77267758
const stdout = [];
77277759
const options = {
77287760
cwd: this.workingDirectory,
77297761
env,
77307762
silent,
77317763
ignoreReturnCode: allowAllExitCodes,
7732-
listeners: {
7733-
stdout: (data) => {
7734-
stdout.push(data.toString());
7735-
}
7736-
}
7764+
listeners: mergedListeners
77377765
};
77387766
result.exitCode = yield exec.exec(`"${this.gitPath}"`, args, options);
77397767
result.stdout = stdout.join('');
7768+
core.debug(result.exitCode.toString());
7769+
core.debug(result.stdout);
77407770
return result;
77417771
});
77427772
}
@@ -13947,6 +13977,7 @@ var encode = function encode(str, defaultEncoder, charset, kind, format) {
1394713977

1394813978
i += 1;
1394913979
c = 0x10000 + (((c & 0x3FF) << 10) | (string.charCodeAt(i) & 0x3FF));
13980+
/* eslint operator-linebreak: [2, "before"] */
1395013981
out += hexTable[0xF0 | (c >> 18)]
1395113982
+ hexTable[0x80 | ((c >> 12) & 0x3F)]
1395213983
+ hexTable[0x80 | ((c >> 6) & 0x3F)]
@@ -17572,7 +17603,7 @@ var parseObject = function (chain, val, options, valuesParsed) {
1757217603
) {
1757317604
obj = [];
1757417605
obj[index] = leaf;
17575-
} else {
17606+
} else if (cleanRoot !== '__proto__') {
1757617607
obj[cleanRoot] = leaf;
1757717608
}
1757817609
}
@@ -34704,6 +34735,7 @@ var arrayPrefixGenerators = {
3470434735
};
3470534736

3470634737
var isArray = Array.isArray;
34738+
var split = String.prototype.split;
3470734739
var push = Array.prototype.push;
3470834740
var pushToArray = function (arr, valueOrArray) {
3470934741
push.apply(arr, isArray(valueOrArray) ? valueOrArray : [valueOrArray]);
@@ -34740,10 +34772,13 @@ var isNonNullishPrimitive = function isNonNullishPrimitive(v) {
3474034772
|| typeof v === 'bigint';
3474134773
};
3474234774

34775+
var sentinel = {};
34776+
3474334777
var stringify = function stringify(
3474434778
object,
3474534779
prefix,
3474634780
generateArrayPrefix,
34781+
commaRoundTrip,
3474734782
strictNullHandling,
3474834783
skipNulls,
3474934784
encoder,
@@ -34759,8 +34794,23 @@ var stringify = function stringify(
3475934794
) {
3476034795
var obj = object;
3476134796

34762-
if (sideChannel.has(object)) {
34763-
throw new RangeError('Cyclic object value');
34797+
var tmpSc = sideChannel;
34798+
var step = 0;
34799+
var findFlag = false;
34800+
while ((tmpSc = tmpSc.get(sentinel)) !== void undefined && !findFlag) {
34801+
// Where object last appeared in the ref tree
34802+
var pos = tmpSc.get(object);
34803+
step += 1;
34804+
if (typeof pos !== 'undefined') {
34805+
if (pos === step) {
34806+
throw new RangeError('Cyclic object value');
34807+
} else {
34808+
findFlag = true; // Break while
34809+
}
34810+
}
34811+
if (typeof tmpSc.get(sentinel) === 'undefined') {
34812+
step = 0;
34813+
}
3476434814
}
3476534815

3476634816
if (typeof filter === 'function') {
@@ -34787,6 +34837,14 @@ var stringify = function stringify(
3478734837
if (isNonNullishPrimitive(obj) || utils.isBuffer(obj)) {
3478834838
if (encoder) {
3478934839
var keyValue = encodeValuesOnly ? prefix : encoder(prefix, defaults.encoder, charset, 'key', format);
34840+
if (generateArrayPrefix === 'comma' && encodeValuesOnly) {
34841+
var valuesArray = split.call(String(obj), ',');
34842+
var valuesJoined = '';
34843+
for (var i = 0; i < valuesArray.length; ++i) {
34844+
valuesJoined += (i === 0 ? '' : ',') + formatter(encoder(valuesArray[i], defaults.encoder, charset, 'value', format));
34845+
}
34846+
return [formatter(keyValue) + (commaRoundTrip && isArray(obj) && valuesArray.length === 1 ? '[]' : '') + '=' + valuesJoined];
34847+
}
3479034848
return [formatter(keyValue) + '=' + formatter(encoder(obj, defaults.encoder, charset, 'value', format))];
3479134849
}
3479234850
return [formatter(prefix) + '=' + formatter(String(obj))];
@@ -34801,32 +34859,36 @@ var stringify = function stringify(
3480134859
var objKeys;
3480234860
if (generateArrayPrefix === 'comma' && isArray(obj)) {
3480334861
// we need to join elements in
34804-
objKeys = [{ value: obj.length > 0 ? obj.join(',') || null : undefined }];
34862+
objKeys = [{ value: obj.length > 0 ? obj.join(',') || null : void undefined }];
3480534863
} else if (isArray(filter)) {
3480634864
objKeys = filter;
3480734865
} else {
3480834866
var keys = Object.keys(obj);
3480934867
objKeys = sort ? keys.sort(sort) : keys;
3481034868
}
3481134869

34812-
for (var i = 0; i < objKeys.length; ++i) {
34813-
var key = objKeys[i];
34814-
var value = typeof key === 'object' && key.value !== undefined ? key.value : obj[key];
34870+
var adjustedPrefix = commaRoundTrip && isArray(obj) && obj.length === 1 ? prefix + '[]' : prefix;
34871+
34872+
for (var j = 0; j < objKeys.length; ++j) {
34873+
var key = objKeys[j];
34874+
var value = typeof key === 'object' && typeof key.value !== 'undefined' ? key.value : obj[key];
3481534875

3481634876
if (skipNulls && value === null) {
3481734877
continue;
3481834878
}
3481934879

3482034880
var keyPrefix = isArray(obj)
34821-
? typeof generateArrayPrefix === 'function' ? generateArrayPrefix(prefix, key) : prefix
34822-
: prefix + (allowDots ? '.' + key : '[' + key + ']');
34881+
? typeof generateArrayPrefix === 'function' ? generateArrayPrefix(adjustedPrefix, key) : adjustedPrefix
34882+
: adjustedPrefix + (allowDots ? '.' + key : '[' + key + ']');
3482334883

34824-
sideChannel.set(object, true);
34884+
sideChannel.set(object, step);
3482534885
var valueSideChannel = getSideChannel();
34886+
valueSideChannel.set(sentinel, sideChannel);
3482634887
pushToArray(values, stringify(
3482734888
value,
3482834889
keyPrefix,
3482934890
generateArrayPrefix,
34891+
commaRoundTrip,
3483034892
strictNullHandling,
3483134893
skipNulls,
3483234894
encoder,
@@ -34850,7 +34912,7 @@ var normalizeStringifyOptions = function normalizeStringifyOptions(opts) {
3485034912
return defaults;
3485134913
}
3485234914

34853-
if (opts.encoder !== null && opts.encoder !== undefined && typeof opts.encoder !== 'function') {
34915+
if (opts.encoder !== null && typeof opts.encoder !== 'undefined' && typeof opts.encoder !== 'function') {
3485434916
throw new TypeError('Encoder has to be a function.');
3485534917
}
3485634918

@@ -34923,6 +34985,10 @@ module.exports = function (object, opts) {
3492334985
}
3492434986

3492534987
var generateArrayPrefix = arrayPrefixGenerators[arrayFormat];
34988+
if (opts && 'commaRoundTrip' in opts && typeof opts.commaRoundTrip !== 'boolean') {
34989+
throw new TypeError('`commaRoundTrip` must be a boolean, or absent');
34990+
}
34991+
var commaRoundTrip = generateArrayPrefix === 'comma' && opts && opts.commaRoundTrip;
3492634992

3492734993
if (!objKeys) {
3492834994
objKeys = Object.keys(obj);
@@ -34943,6 +35009,7 @@ module.exports = function (object, opts) {
3494335009
obj[key],
3494435010
key,
3494535011
generateArrayPrefix,
35012+
commaRoundTrip,
3494635013
options.strictNullHandling,
3494735014
options.skipNulls,
3494835015
options.encode ? options.encoder : null,

‎package-lock.json‎

Lines changed: 6 additions & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)