Skip to content

Commit

Permalink
chore(get): refactor unzip symlink implementation (#1030)
Browse files Browse the repository at this point in the history
* Do not mix Promise and Callback APIs.
* Remove `createSymlinks` function workaround

Refs:
overlookmotel/yauzl-promise#39 (comment)
  • Loading branch information
ayushmanchhabra authored Feb 15, 2024
1 parent f9ae7cd commit 69661c3
Show file tree
Hide file tree
Showing 7 changed files with 30 additions and 114 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,7 @@ nwbuild({

### Bugs

- Managed Manifest is broken. If glob is disabled and srcDir has no package.json, build fails.
- MacOS fails to unzip MacOS NW.js binaries consistently
- Add back error, info, warn and debug logs

Expand Down
33 changes: 0 additions & 33 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 1 addition & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
"lint": "eslint ./src/**/*.js ./test/**/*.js",
"lint:fix": "eslint --fix ./**/*.{js,md} && markdownlint --fix ./README.md",
"docs": "jsdoc -d docs ./README.md ./src/index.js ./src/get.js ./src/run.js ./src/bld.js",
"test": "vitest",
"test": "vitest run",
"demo": "cd test/fixture && node demo.js"
},
"devDependencies": {
Expand All @@ -63,7 +63,6 @@
"axios": "^1.6.7",
"cli-progress": "^3.12.0",
"compressing": "^1.10.0",
"fs-extra": "^11.2.0",
"glob": "^10.3.10",
"node-gyp": "^10.0.1",
"plist": "^3.1.0",
Expand Down
72 changes: 26 additions & 46 deletions src/get/decompress.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import stream from "node:stream";

import tar from "tar";
import yauzl from "yauzl-promise";
import {ensureSymlink} from "fs-extra";

/**
* Decompresses a file at `filePath` to `cacheDir` directory.
Expand All @@ -24,72 +23,53 @@ export default async function decompress(filePath, cacheDir) {
}

/**
* Wrapper for unzipping using `yauzl-promise`.
* Get file mode from entry. Reference implementation is [here](https://github.com/fpsqdb/zip-lib/blob/ac447d269218d396e05cd7072d0e9cd82b5ec52c/src/unzip.ts#L380).
*
* @async
* @function
* @param {string} zippedFile - file path to .zip file
* @param {string} cacheDir - directory to unzip in
* @return {Promise<void>}
* @param {yauzl.Entry} entry - Yauzl entry
* @return {number} - entry's file mode
*/
async function unzip(zippedFile, cacheDir) {
await unzipInternal(zippedFile, cacheDir, false).then(() => {
unzipInternal(zippedFile, cacheDir, true);
})
function modeFromEntry(entry) {
const attr = entry.externalFileAttributes >> 16 || 33188;

return [448 /* S_IRWXU */, 56 /* S_IRWXG */, 7 /* S_IRWXO */]
.map(mask => attr & mask)
.reduce((a, b) => a + b, attr & 61440 /* S_IFMT */);
}

/**
* Method for unzip with symlink. Workaround for not being able to handle symlinks. Tracking in linked issue.
* Unzip `zippedFile` to `cacheDir`.
*
* @async
* @function
* @param {string} zippedFile - file path to .zip file
* @param {string} cacheDir - directory to unzip in
* @param {boolean} unzipSymlink - Using or not symlink
* @param {string} zippedFile - file path to .zip file
* @param {string} cacheDir - directory to unzip in
* @return {Promise<void>}
*/
async function unzipInternal(zippedFile, cacheDir, unzipSymlink ) {
async function unzip(zippedFile, cacheDir) {
const zip = await yauzl.open(zippedFile);

let entry = await zip.readEntry();

while (entry !== null) {
// console.log(entry)
let entryPathAbs = path.join(cacheDir, entry.filename);
// Create the directory beforehand to prevent `ENOENT: no such file or directory` errors.
await fs.promises.mkdir(path.dirname(entryPathAbs), {recursive: true});
await fs.promises.mkdir(path.dirname(entryPathAbs), { recursive: true });
// Pipe read to write stream
const readStream = await entry.openReadStream();
const writeStream = fs.createWriteStream(entryPathAbs);
await stream.promises.pipeline(readStream, writeStream);
// Get file mode
let fileMode = modeFromEntry(entry);
const isSymlink = ((fileMode & 0o170000) === 0o120000);

try {
if (!unzipSymlink) {
// Regular method and silent error at this point
const writeStream = fs.createWriteStream(entryPathAbs);
await stream.promises.pipeline(readStream, writeStream);
} else {
// Need check before if file is a symlink or not at this point
const pathContent = await fs.promises.lstat(entryPathAbs);

if (pathContent.isSymbolicLink()) {
const chunks = [];
readStream.on('data', (chunk) => chunks.push(chunk));
await stream.promises.finished(readStream);
// need fetch value of current symlink here
const linkTarget = Buffer.concat(chunks).toString('utf8').trim();
await ensureSymlink(entryPathAbs, path.join(path.dirname(entryPathAbs), linkTarget));
}else{
// Regular method and silent error at this point
const writeStream = fs.createWriteStream(entryPathAbs);
await stream.promises.pipeline(readStream, writeStream);
}
}
} catch (error) {
if (unzipSymlink) {
console.error(error);
}
if (isSymlink) {
const buffer = await fs.promises.readFile(entryPathAbs);
const link = buffer.toString();
await fs.promises.rm(entryPathAbs);
await fs.promises.symlink(link, entryPathAbs);
}

// Read next entry
entry = await zip.readEntry();
}

await zip.close();
}
32 changes: 0 additions & 32 deletions src/get/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,12 +88,6 @@ async function get(options) {

await decompress(nwFilePath, options.cacheDir);

// TODO: remove once linked issue is resolved.
// https://github.com/overlookmotel/yauzl-promise/issues/39
if (options.platform === "osx") {
await createSymlinks(options);
}

if (options.ffmpeg === true) {

/**
Expand Down Expand Up @@ -211,30 +205,4 @@ async function get(options) {
}
}

/**
* Workaround for manually creating symbolic links for MacOS builds.
*
* @param {object} options - options config
*/
const createSymlinks = async (options) => {
let frameworksPath = path.resolve(process.cwd(), options.cacheDir, `nwjs${options.flavor === "sdk" ? "-sdk" : ""}-v${options.version}-${options.platform}-${options.arch}`, "nwjs.app", "Contents", "Frameworks", "nwjs Framework.framework")
// Allow resolve cacheDir from another directory for prevent crash
if (!fs.lstatSync(frameworksPath).isDirectory()) {
frameworksPath = path.resolve(options.cacheDir, `nwjs${options.flavor === "sdk" ? "-sdk" : ""}-v${options.version}-${options.platform}-${options.arch}`, "nwjs.app", "Contents", "Frameworks", "nwjs Framework.framework")
}
const symlinks = [
path.join(frameworksPath, "Helpers"),
path.join(frameworksPath, "Libraries"),
path.join(frameworksPath, "nwjs Framework"),
path.join(frameworksPath, "Resources"),
path.join(frameworksPath, "Versions", "Current"),
];
for await (const symlink of symlinks) {
const buffer = await fs.promises.readFile(symlink);
const link = buffer.toString();
await fs.promises.rm(symlink);
await fs.promises.symlink(link, symlink);
}
};

export default get;
1 change: 1 addition & 0 deletions test/fixture/demo.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import nwbuild from "../../src/index.js";

await nwbuild({
mode: "get",
platform:"osx",
flavor: "sdk",
srcDir: "app",
});
2 changes: 1 addition & 1 deletion test/specs/get.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import get from '../../src/get/index.js';

describe("get", async () => {
const nwOptions = {
version: "0.83.0",
version: "0.84.0",
flavor: "sdk",
platform: "osx",
arch: "x64",
Expand Down

0 comments on commit 69661c3

Please sign in to comment.