fs.createReadStream and --compress#1158
Conversation
erossignon
force-pushed
the
Feature/fs.createReadStream
branch
5 times, most recently
from
5c47a9f to
89eb14e
Compare
erossignon
force-pushed
the
Feature/fs.createReadStream
branch
2 times, most recently
from
4ded3af to
2bea2b8
Compare
erossignon
force-pushed
the
Feature/fs.createReadStream
branch
2 times, most recently
from
a22eaad to
61a465c
Compare
| const os = require('os'); | ||
| const tmpFolder = fs.mkdtempSync(path.join(os.tmpdir(), 'pkg-')); | ||
| process.on('beforeExit', () => { | ||
| removeTemporaryFolderAndContent(tmpFolder); |
There was a problem hiding this comment.
Is this really needed? Couldn't we keep files in temporary folder for quicker startup?
There was a problem hiding this comment.
- This is a boy scout attitude. Leave the place tidier than you found it.
- Those files potentially are exposed in clear,
- Arguably, those files could be corrupted later on somehow, the single source of truth is in the packed binary.
- Only files that are randomly accessed are copied uncompressed in this folder. I made sure that file that are read globally (readFileSync) or streamed (createReadStream) are processed in memory. I would be inclined to leave it this way unless users complain about potential performance issue.
- having a persistent cache would require that the temp subfolder name is stable and unique, based on the executable signature (shall we calculated the md5/sha1 of the entire pkg.exe ?)
There was a problem hiding this comment.
having a persistent cache would require that the temp subfolder name is stable and unique, based on the executable signature (shall we calculated the md5/sha1 of the entire pkg.exe ?)
Could be an idea but yeah those files could be someway corrupted. I would like to also know @jesec and @leerob opinion about this
There was a problem hiding this comment.
It might also be an option to generate a build-id uuid on build and bake that into the binary.
erossignon
force-pushed
the
Feature/fs.createReadStream
branch
from
61a465c to
2c49066
Compare
|
@erossignon Please rebase the PR. Apparently, I am not able to do this for you: Despite some concerns about race conditions, I think we can merge this PR, as the change appears to be self-contained, and should not hurt |
erossignon
force-pushed
the
Feature/fs.createReadStream
branch
from
2c49066 to
ce9b944
Compare
erossignon
force-pushed
the
Feature/fs.createReadStream
branch
from
ce9b944 to
65d3036
Compare
|
Could this be merged? |
|
Sure ! |
4 participants
@Hypfer , this should fix issue found with compressed pkg file and streaming file from snapshot.