Raise the file limit for debug artifacts by producing zip64 files where necessary#2842
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR updates the debug artifacts creation process by replacing the AdmZip library with archiver to accommodate zip64 files for large debug artifacts.
- Replaces AdmZip with archiver in both TypeScript and JavaScript files
- Implements asynchronous zip creation through archiver to support a higher file limit
Reviewed Changes
Copilot reviewed 36 out of 37 changed files in this pull request and generated 2 comments.
Files not reviewed (1)
- package.json: Language not supported
Tip: Copilot only keeps its highest confidence comments to reduce noise and keep you focused. Learn more
| zip.addLocalFolder(databasePath); | ||
| zip.writeZip(databaseBundlePath); | ||
| const output = fs.createWriteStream(databaseBundlePath); | ||
| const zip = (0, archiver_1.default)("zip"); |
There was a problem hiding this comment.
The PR title indicates support for large numbers of files with zip64; consider enabling zip64 explicitly by passing {forceZip64: true} to archiver to ensure the archive correctly supports large file sets.
| const zip = (0, archiver_1.default)("zip"); | |
| const zip = (0, archiver_1.default)("zip", { zlib: { level: 9 }, forceZip64: true }); |
There was a problem hiding this comment.
Suggestion in compiled code.
henrymercer
added
the
Update dependencies
github-actions
Bot
removed
the
Update dependencies
|
Pushed a commit to update the checked-in dependencies. Please mark the PR as ready for review to trigger PR checks. |
| zip.on("warning", (err) => { | ||
| throw err; | ||
| }); |
There was a problem hiding this comment.
Two questions:
- In what cases will there be a warning and is this really something we want to raise an error on?
- What will happen if an error is thrown? Where will it be caught and will it be properly handled?
There was a problem hiding this comment.
Looking at the sample code in the packge, the recommendation is something like this:
if (err.code === "ENOENT") {
// log warning
} else {
// throw error
throw err;
}
I'll update.
| zip.on("warning", (err) => { | ||
| throw err; | ||
| }); |
There was a problem hiding this comment.
Looking at the sample code in the packge, the recommendation is something like this:
if (err.code === "ENOENT") {
// log warning
} else {
// throw error
throw err;
}
I'll update.
| }); | ||
|
|
||
| zip.on("warning", (err) => { | ||
| throw err; |
There was a problem hiding this comment.
| throw err; | |
| // Ignore ENOENT warnings. There's nothing anyone can do about it. | |
| if (err.code !== "ENOENT") { | |
| throw err; | |
| } |
aeisenberg
force-pushed
the
henrymercer/zip64
branch
from
117d1f1 to
fd8685f
Compare
The previous library only supported the original zip format, which is limited to 65535 files. This caused issues producing debug artifacts for codebases with large numbers of files.
Merge / deployment checklist