search

brackets-archive / bracketsIssues

Archive of issues in brackets.
0 stars 0 forks source link

[CLOSED] _performInstall writes files to subdirectories that don't exist yet #3495

Open core-ai-bot opened 3 years ago

core-ai-bot commented 3 years ago

Issue by alltom Tuesday May 07, 2013 at 05:52 GMT Originally opened as https://github.com/adobe/brackets/issues/3734

File > Install Extension failed on my extension sometimes with the symptom that some files wouldn't be extracted. I narrowed it down to a race condition in ExtensionManagerDomain.js.

When _performInstall encounters a directory entry, it attempts to pause the unzip stream so that it won't try to write files to the directory until the directory has been created:

extractStream.pause();
fs.mkdirs(installDirectory + "/" + installpath, function (err) {
    if (err) {
        if (!callbackCalled) {
            callback(err);
            callbackCalled = true;
        }
        extractStream.close();
        return;
    }
    extractStream.resume();
});

However, that doesn't seem to work. I created a separate test script so I could call _performInstall in isolation and added log statements for when 1) it started to create a directory, 2) finished creating a directory, and 3) started writing a file. When I repeatedly ran the script, I saw log entries indicating that it was writing to a sub-directory that didn't exist yet, and throwing ENOENT errors when that happened.

core-ai-bot commented 3 years ago

Comment by alltom Tuesday May 07, 2013 at 05:59 GMT

It's pretty late, but it seems like Brackets could use the one-liner from unzip's README:

fs.createReadStream('path/to/archive.zip').pipe(unzip.Extract({ path: 'output/path' }));
core-ai-bot commented 3 years ago

Comment by peterflynn Tuesday May 07, 2013 at 21:16 GMT

@dangoor to take a look after we're back from MAX

core-ai-bot commented 3 years ago

Comment by njx Friday May 10, 2013 at 02:06 GMT

Nominating for sprint 25

core-ai-bot commented 3 years ago

Comment by dangoor Tuesday May 14, 2013 at 18:33 GMT

@alltom Thanks for the good report and looking into possible solutions. Unfortunately, that one-liner from unzip's README doesn't work in this context because we need to strip out the common install prefix, rather than just extracting the files without change.

Re-reading some of the info about the new streams in Node 0.10, it looks like stream.pause is "advisory" in some streams in 0.8, and that's probably what we're seeing here.

core-ai-bot commented 3 years ago

Comment by redmunds Thursday May 16, 2013 at 16:06 GMT

Closing.@alltom Let us know if this is not fixed for you.