Refactor archiving to go through single determinstic function #310
Conversation
| '1x1.jpg', // this is in the root because the css is always served from the root on happo workers | ||
| 'one.jpg', | ||
| 'inlineResources/1x1.png', | ||
| 'one.jpg', |
There was a problem hiding this comment.
The order changes here because we are now sorting alphabetically by path at the end, and o is after i.
| if (isDir) { | ||
| const files = await fs.promises.readdir(dirOrFile, { | ||
| withFileTypes: true, | ||
| recursive: true, |
There was a problem hiding this comment.
Node 18 adds support for recursive: true here, so we can use that and simplify our directory scanning.
src/deterministicArchive.js
Outdated
| for (const file of contentToArchiveSorted) { | ||
| archive.append(file.content, { | ||
| name: file.name, | ||
| prefix: '', |
There was a problem hiding this comment.
We didn't have prefix when using archive.append before, but since we are using it with archive.file I figured we might as well add it here for consistency.
| }); | ||
|
|
||
| return deterministicArchive([], files.filter(Boolean)); |
There was a problem hiding this comment.
Since we are generating an archive with a file structure that is different than what is on the filesystem, we need to use the second argument for everything so we can control the paths more specifically.
| resolve(pathToZipFile); | ||
| }); | ||
| archive.pipe(output); | ||
| archive.directory(dir, false, { date: FILE_CREATION_DATE }); |
There was a problem hiding this comment.
I believe archive.directory may be nondeterministic, so this should be a good improvement for folks using remote runner.
lencioni
force-pushed
the
readdir-recursive
branch
from
f9619fd to
ca9bc41
Compare
| 'static/', | ||
| 'static/media/', |
There was a problem hiding this comment.
We no longer add directory entries to this zip file, just files.
lencioni
force-pushed
the
readdir-recursive
branch
2 times, most recently
from
32c7ae8 to
37e8689
Compare
We had three places that were interfacing with the archiver package, and they all need to do basically the same thing--create a deterministic zip file. If the zip file is nondeterministic, then downstream problems with caching will arise. Since creating a deterministic zip requires some special handling, I decided that it would be best to refactor this code to all run through the same function that guarantees a deterministic output. While working on this I noticed that the remoteRunner was using archiver's directory function, which I believe can by nondeterministic, so this should help to stabilize those zip files.
lencioni
force-pushed
the
readdir-recursive
branch
from
37e8689 to
32ac8b4
Compare
We had three places that were interfacing with the archiver package, and they all need to do basically the same thing--create a deterministic zip file. If the zip file is nondeterministic, then downstream problems with caching will arise. Since creating a deterministic zip requires some special handling, I decided that it would be best to refactor this code to all run through the same function that guarantees a deterministic output.
While working on this I noticed that the remoteRunner was using archiver's directory function, which I believe can by nondeterministic, so this should help to stabilize those zip files.