Skip to content

Use showFileAtRefBase64 to read per-commit file contents #3744

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Feb 24, 2025
Merged

Use showFileAtRefBase64 to read per-commit file contents #3744

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
bf28f82
GitCommandManager: add a function to get a file's contents at a speci…
grahamc Feb 20, 2025
c2125cf
use showFileAtRef instead of readFileBase64
grahamc Feb 20, 2025
65c4628
Teach GitCommandManager.exec about an object of exec parameters so we…
grahamc Feb 20, 2025
4e9b418
Encode the showFiletRef output as base64 out of the gate
grahamc Feb 20, 2025
af869eb
Fix missing async for function
gustavderdrache Feb 20, 2025
805b174
Use Buffer.concat to avoid issues with partial data streams
gustavderdrache Feb 20, 2025
4ae5e43
formatting
gustavderdrache Feb 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 31 additions & 28 deletions dist/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ var WorkingBaseType;
})(WorkingBaseType || (exports.WorkingBaseType = WorkingBaseType = {}));
function getWorkingBaseAndType(git) {
return __awaiter(this, void 0, void 0, function* () {
const symbolicRefResult = yield git.exec(['symbolic-ref', 'HEAD', '--short'], true);
const symbolicRefResult = yield git.exec(['symbolic-ref', 'HEAD', '--short'], { allowAllExitCodes: true });
if (symbolicRefResult.exitCode == 0) {
// A ref is checked out
return [symbolicRefResult.stdout.trim(), WorkingBaseType.Branch];
Expand Down Expand Up @@ -194,7 +194,7 @@ function createOrUpdateBranch(git, commitMessage, base, branch, branchRemoteName
else {
aopts.push('-A');
}
yield git.exec(aopts, true);
yield git.exec(aopts, { allowAllExitCodes: true });
const popts = ['-m', commitMessage];
if (signoff) {
popts.push('--signoff');
Expand Down Expand Up @@ -517,7 +517,7 @@ function createPullRequest(inputs) {
// Create signed commits via the GitHub API
const stashed = yield git.stashPush(['--include-untracked']);
yield git.checkout(inputs.branch);
const pushSignedCommitsResult = yield ghBranch.pushSignedCommits(result.branchCommits, result.baseCommit, repoPath, branchRepository, inputs.branch);
const pushSignedCommitsResult = yield ghBranch.pushSignedCommits(git, result.branchCommits, result.baseCommit, repoPath, branchRepository, inputs.branch);
outputs.set('pull-request-head-sha', pushSignedCommitsResult.sha);
outputs.set('pull-request-commits-verified', pushSignedCommitsResult.verified.toString());
yield git.checkout('-');
Expand Down Expand Up @@ -704,7 +704,7 @@ class GitCommandManager {
if (options) {
args.push(...options);
}
return yield this.exec(args, allowAllExitCodes);
return yield this.exec(args, { allowAllExitCodes: allowAllExitCodes });
});
}
commit(options_1) {
Expand All @@ -716,7 +716,7 @@ class GitCommandManager {
if (options) {
args.push(...options);
}
return yield this.exec(args, allowAllExitCodes);
return yield this.exec(args, { allowAllExitCodes: allowAllExitCodes });
});
}
config(configKey, configValue, globalConfig, add) {
Expand All @@ -738,7 +738,7 @@ class GitCommandManager {
'--get-regexp',
configKey,
configValue
], true);
], { allowAllExitCodes: true });
return output.exitCode === 0;
});
}
Expand Down Expand Up @@ -835,7 +835,7 @@ class GitCommandManager {
if (options) {
args.push(...options);
}
const output = yield this.exec(args, true);
const output = yield this.exec(args, { allowAllExitCodes: true });
return output.exitCode === 1;
});
}
Expand Down Expand Up @@ -892,6 +892,13 @@ class GitCommandManager {
return output.stdout.trim();
});
}
showFileAtRefBase64(ref, path) {
return __awaiter(this, void 0, void 0, function* () {
const args = ['show', `${ref}:${path}`];
const output = yield this.exec(args, { encoding: 'base64' });
return output.stdout.trim();
});
}
stashPush(options) {
return __awaiter(this, void 0, void 0, function* () {
const args = ['stash', 'push'];
Expand Down Expand Up @@ -939,13 +946,13 @@ class GitCommandManager {
'--unset',
configKey,
configValue
], true);
], { allowAllExitCodes: true });
return output.exitCode === 0;
});
}
tryGetRemoteUrl() {
return __awaiter(this, void 0, void 0, function* () {
const output = yield this.exec(['config', '--local', '--get', 'remote.origin.url'], true);
const output = yield this.exec(['config', '--local', '--get', 'remote.origin.url'], { allowAllExitCodes: true });
if (output.exitCode !== 0) {
return '';
}
Expand All @@ -957,30 +964,34 @@ class GitCommandManager {
});
}
exec(args_1) {
return __awaiter(this, arguments, void 0, function* (args, allowAllExitCodes = false) {
return __awaiter(this, arguments, void 0, function* (args, { encoding = 'utf8', allowAllExitCodes = false } = {}) {
const result = new GitOutput();
const env = {};
for (const key of Object.keys(process.env)) {
env[key] = process.env[key];
}
const stdout = [];
let stdoutLength = 0;
const stderr = [];
let stderrLength = 0;
const options = {
cwd: this.workingDirectory,
env,
ignoreReturnCode: allowAllExitCodes,
listeners: {
stdout: (data) => {
stdout.push(data.toString());
stdout.push(data);
stdoutLength += data.length;
},
stderr: (data) => {
stderr.push(data.toString());
stderr.push(data);
stderrLength += data.length;
}
}
};
result.exitCode = yield exec.exec(`"${this.gitPath}"`, args, options);
result.stdout = stdout.join('');
result.stderr = stderr.join('');
result.stdout = Buffer.concat(stdout, stdoutLength).toString(encoding);
result.stderr = Buffer.concat(stderr, stderrLength).toString(encoding);
return result;
});
}
Expand Down Expand Up @@ -1400,21 +1411,21 @@ class GitHubHelper {
return pull;
});
}
pushSignedCommits(branchCommits, baseCommit, repoPath, branchRepository, branch) {
pushSignedCommits(git, branchCommits, baseCommit, repoPath, branchRepository, branch) {
return __awaiter(this, void 0, void 0, function* () {
let headCommit = {
sha: baseCommit.sha,
tree: baseCommit.tree,
verified: false
};
for (const commit of branchCommits) {
headCommit = yield this.createCommit(commit, headCommit, repoPath, branchRepository);
headCommit = yield this.createCommit(git, commit, headCommit, repoPath, branchRepository);
}
yield this.createOrUpdateRef(branchRepository, branch, headCommit.sha);
return headCommit;
});
}
createCommit(commit, parentCommit, repoPath, branchRepository) {
createCommit(git, commit, parentCommit, repoPath, branchRepository) {
return __awaiter(this, void 0, void 0, function* () {
const repository = this.parseRepository(branchRepository);
// In the case of an empty commit, the tree references the parent's tree
Expand All @@ -1436,7 +1447,9 @@ class GitHubHelper {
let sha = null;
if (status === 'A' || status === 'M') {
try {
const { data: blob } = yield blobCreationLimit(() => this.octokit.rest.git.createBlob(Object.assign(Object.assign({}, repository), { content: utils.readFileBase64([repoPath, path]), encoding: 'base64' })));
const { data: blob } = yield blobCreationLimit(() => __awaiter(this, void 0, void 0, function* () {
return this.octokit.rest.git.createBlob(Object.assign(Object.assign({}, repository), { content: yield git.showFileAtRefBase64(commit.sha, path), encoding: 'base64' }));
}));
sha = blob.sha;
}
catch (error) {
Expand Down Expand Up @@ -1763,7 +1776,6 @@ exports.randomString = randomString;
exports.parseDisplayNameEmail = parseDisplayNameEmail;
exports.fileExistsSync = fileExistsSync;
exports.readFile = readFile;
exports.readFileBase64 = readFileBase64;
exports.getErrorMessage = getErrorMessage;
const core = __importStar(__nccwpck_require__(7484));
const fs = __importStar(__nccwpck_require__(9896));
Expand Down Expand Up @@ -1853,15 +1865,6 @@ function fileExistsSync(path) {
function readFile(path) {
return fs.readFileSync(path, 'utf-8');
}
function readFileBase64(pathParts) {
const resolvedPath = path.resolve(...pathParts);
if (fs.lstatSync(resolvedPath).isSymbolicLink()) {
return fs
.readlinkSync(resolvedPath, { encoding: 'buffer' })
.toString('base64');
}
return fs.readFileSync(resolvedPath).toString('base64');
}
/* eslint-disable @typescript-eslint/no-explicit-any */
function hasErrorCode(error) {
return typeof (error && error.code) === 'string';
Expand Down
4 changes: 2 additions & 2 deletions src/create-or-update-branch.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export async function getWorkingBaseAndType(
): Promise<[string, WorkingBaseType]> {
const symbolicRefResult = await git.exec(
['symbolic-ref', 'HEAD', '--short'],
true
{allowAllExitCodes: true}
)
if (symbolicRefResult.exitCode == 0) {
// A ref is checked out
Expand Down Expand Up @@ -200,7 +200,7 @@ export async function createOrUpdateBranch(
} else {
aopts.push('-A')
}
await git.exec(aopts, true)
await git.exec(aopts, {allowAllExitCodes: true})
const popts = ['-m', commitMessage]
if (signoff) {
popts.push('--signoff')
Expand Down
1 change: 1 addition & 0 deletions src/create-pull-request.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ export async function createPullRequest(inputs: Inputs): Promise<void> {
const stashed = await git.stashPush(['--include-untracked'])
await git.checkout(inputs.branch)
const pushSignedCommitsResult = await ghBranch.pushSignedCommits(
git,
result.branchCommits,
result.baseCommit,
repoPath,
Expand Down
44 changes: 31 additions & 13 deletions src/git-command-manager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@ export type Commit = {
unparsedChanges: string[]
}

export type ExecOpts = {
allowAllExitCodes?: boolean
encoding?: 'utf8' | 'base64'
}

export class GitCommandManager {
private gitPath: string
private workingDirectory: string
Expand Down Expand Up @@ -66,7 +71,7 @@ export class GitCommandManager {
args.push(...options)
}

return await this.exec(args, allowAllExitCodes)
return await this.exec(args, {allowAllExitCodes: allowAllExitCodes})
}

async commit(
Expand All @@ -82,7 +87,7 @@ export class GitCommandManager {
args.push(...options)
}

return await this.exec(args, allowAllExitCodes)
return await this.exec(args, {allowAllExitCodes: allowAllExitCodes})
}

async config(
Expand Down Expand Up @@ -113,7 +118,7 @@ export class GitCommandManager {
configKey,
configValue
],
true
{allowAllExitCodes: true}
)
return output.exitCode === 0
}
Expand Down Expand Up @@ -222,7 +227,7 @@ export class GitCommandManager {
if (options) {
args.push(...options)
}
const output = await this.exec(args, true)
const output = await this.exec(args, {allowAllExitCodes: true})
return output.exitCode === 1
}

Expand Down Expand Up @@ -278,6 +283,12 @@ export class GitCommandManager {
return output.stdout.trim()
}

async showFileAtRefBase64(ref: string, path: string): Promise<string> {
const args = ['show', `${ref}:${path}`]
const output = await this.exec(args, {encoding: 'base64'})
return output.stdout.trim()
}

async stashPush(options?: string[]): Promise<boolean> {
const args = ['stash', 'push']
if (options) {
Expand Down Expand Up @@ -326,15 +337,15 @@ export class GitCommandManager {
configKey,
configValue
],
true
{allowAllExitCodes: true}
)
return output.exitCode === 0
}

async tryGetRemoteUrl(): Promise<string> {
const output = await this.exec(
['config', '--local', '--get', 'remote.origin.url'],
true
{allowAllExitCodes: true}
)

if (output.exitCode !== 0) {
Expand All @@ -349,34 +360,41 @@ export class GitCommandManager {
return stdout
}

async exec(args: string[], allowAllExitCodes = false): Promise<GitOutput> {
async exec(
args: string[],
{encoding = 'utf8', allowAllExitCodes = false}: ExecOpts = {}
): Promise<GitOutput> {
const result = new GitOutput()

const env = {}
for (const key of Object.keys(process.env)) {
env[key] = process.env[key]
}

const stdout: string[] = []
const stderr: string[] = []
const stdout: Buffer[] = []
let stdoutLength = 0
const stderr: Buffer[] = []
let stderrLength = 0

const options = {
cwd: this.workingDirectory,
env,
ignoreReturnCode: allowAllExitCodes,
listeners: {
stdout: (data: Buffer) => {
stdout.push(data.toString())
stdout.push(data)
stdoutLength += data.length
},
stderr: (data: Buffer) => {
stderr.push(data.toString())
stderr.push(data)
stderrLength += data.length
}
}
}

result.exitCode = await exec.exec(`"${this.gitPath}"`, args, options)
result.stdout = stdout.join('')
result.stderr = stderr.join('')
result.stdout = Buffer.concat(stdout, stdoutLength).toString(encoding)
result.stderr = Buffer.concat(stderr, stderrLength).toString(encoding)
Comment on lines -360 to +397
Copy link
Contributor Author

@grahamc grahamc Feb 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note these changes are necessary to avoid accidentally incorrectly handling split chunks. In utf-8 this is a problem if the chunk is in the middle of a code point, corrupting the data. In base64 this is a problem because it can end up with padding in the middle of the stream, which effectively truncates the file early.

Reading it all into a single buffer and then converting retains the correct behavior, without letting the executing child process block on a full output stream.

return result
}
}
Expand Down
9 changes: 6 additions & 3 deletions src/github-helper.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import * as core from '@actions/core'
import {Inputs} from './create-pull-request'
import {Commit} from './git-command-manager'
import {Commit, GitCommandManager} from './git-command-manager'
import {Octokit, OctokitOptions, throttleOptions} from './octokit-client'
import pLimit from 'p-limit'
import * as utils from './utils'
Expand Down Expand Up @@ -220,6 +220,7 @@ export class GitHubHelper {
}

async pushSignedCommits(
git: GitCommandManager,
branchCommits: Commit[],
baseCommit: Commit,
repoPath: string,
Expand All @@ -233,6 +234,7 @@ export class GitHubHelper {
}
for (const commit of branchCommits) {
headCommit = await this.createCommit(
git,
commit,
headCommit,
repoPath,
Expand All @@ -244,6 +246,7 @@ export class GitHubHelper {
}

private async createCommit(
git: GitCommandManager,
commit: Commit,
parentCommit: CommitResponse,
repoPath: string,
Expand All @@ -269,10 +272,10 @@ export class GitHubHelper {
let sha: string | null = null
if (status === 'A' || status === 'M') {
try {
const {data: blob} = await blobCreationLimit(() =>
const {data: blob} = await blobCreationLimit(async () =>
this.octokit.rest.git.createBlob({
...repository,
content: utils.readFileBase64([repoPath, path]),
content: await git.showFileAtRefBase64(commit.sha, path),
encoding: 'base64'
})
)
Expand Down
Loading