-
-
Notifications
You must be signed in to change notification settings - Fork 84
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.
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
🐛 Fix verified commits #163
Conversation
async getBlobBase64Content(file) { | ||
const fileRelativePath = path.join(this.workingDir, file) | ||
const fileContent = await fs.promises.readFile(fileRelativePath) | ||
|
||
return fileContent.toString('base64') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of pushing the content of the file, this solution converts the file to base64.
Also, I have replaced the way of getting the content of the file. Previously I used the git show
command, but I found that this could be problematic because of the exec
command buffer size, and also the encoding that this command output produces.
|
||
const treeEntry = { | ||
mode, | ||
type, | ||
content: await this.getBlobContent(sha), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't upload the content of the file when creating the tree, the files/blobs are references by its SHA only.
async createGithubBlobs(commitSha) { | ||
core.debug('Creating missing blobs on GitHub') | ||
const [ previousTree, tree ] = await Promise.all([ this.getTree(`${ commitSha }~1`), this.getTree(commitSha) ]) | ||
const promisesGithubCreateBlobs = [] | ||
|
||
for (const treeEntry of tree) { | ||
// If the current treeEntry are in the previous tree, that means that the blob is uploaded and it doesn't need to be uploaded to GitHub again. | ||
if (previousTree.findIndex((entry) => entry.sha === treeEntry.sha) !== -1) { | ||
continue | ||
} | ||
|
||
const base64Content = await this.getBlobBase64Content(treeEntry.path) | ||
|
||
// Creates the blob. We don't need to store the response because the local sha is the same and we can use it to reference the blob | ||
const githubCreateBlobRequest = this.github.git.createBlob({ | ||
owner: this.repo.user, | ||
repo: this.repo.name, | ||
content: base64Content, | ||
encoding: 'base64' | ||
}) | ||
promisesGithubCreateBlobs.push(githubCreateBlobRequest) | ||
} | ||
|
||
// Wait for all the file uploads to be completed | ||
await Promise.all(promisesGithubCreateBlobs) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function compares the content of the tree to commit with the previous tree.
Iterates over each blob entry for the current commit checking if it was present on the previous tree. If it's not present, it uploads it to GitHub, if not just skip the upload.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, thanks!
🎉 This PR is included in version 1.17.4 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Hi again @BetaHuhn!
I was playing around with the changes merged in #153 and I proposing this PR to solve a potential problem that I found:
Encoding
It's possible to cause encoding issues with my previous way of uploading the files/blobs to GitHub. I was using the API https://octokit.github.io/rest.js/v18#git-create-tree but passing the tree[].content to create the git tree structure on GitHub. I found that this way could be problematic because it treats all the files as if they were utf-8. I found problems in utf-16le for example but in some other cases.
I will explain the solution with comments in the PR.