Skip to content

feat: unplug packages with native files #853

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Jan 30, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
"comma-dangle": ["error", "always-multiline"],
"computed-property-spacing": 2,
"generator-star-spacing": ["error", {"before": true, "after": true}],
"space-before-blocks": 2,
"keyword-spacing": 2,
"no-multiple-empty-lines": 2,
"no-tabs": 2,
Expand Down
6 changes: 3 additions & 3 deletions .pnp.js

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

32 changes: 32 additions & 0 deletions .yarn/versions/325ffac3.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
releases:
"@yarnpkg/fslib": prerelease
"@yarnpkg/plugin-pnp": prerelease
"@yarnpkg/pnpify": prerelease

declined:
- "@yarnpkg/plugin-constraints"
- "@yarnpkg/plugin-dlx"
- "@yarnpkg/plugin-essentials"
- "@yarnpkg/plugin-exec"
- "@yarnpkg/plugin-file"
- "@yarnpkg/plugin-git"
- "@yarnpkg/plugin-github"
- "@yarnpkg/plugin-http"
- "@yarnpkg/plugin-init"
- "@yarnpkg/plugin-link"
- "@yarnpkg/plugin-node-modules"
- "@yarnpkg/plugin-npm"
- "@yarnpkg/plugin-npm-cli"
- "@yarnpkg/plugin-pack"
- "@yarnpkg/plugin-patch"
- "@yarnpkg/plugin-stage"
- "@yarnpkg/plugin-version"
- "@yarnpkg/plugin-workspace-tools"
- vscode-zipfs
- "@yarnpkg/builder"
- "@yarnpkg/cli"
- "@yarnpkg/core"
- "@yarnpkg/doctor"
- "@yarnpkg/json-proxy"
- "@yarnpkg/pnp"
- "@yarnpkg/shell"
18 changes: 16 additions & 2 deletions packages/plugin-pnp/sources/PnpLinker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,17 @@ const FORCED_UNPLUG_PACKAGES = new Set([
structUtils.makeIdent(null, `fsevents`).identHash,
]);

const FORCED_UNPLUG_FILETYPES = new Set([
// Windows can't execute exe files inside zip archives
'.exe',
// The c/c++ compiler can't read files from zip archives
'.h', '.hh', '.hpp', '.c', '.cc', '.cpp',
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
'.h', '.hh', '.hpp', '.c', '.cc', '.cpp',
'.h', '.hh', '.hpp', '.c', '.cc', '.cpp',
// iOS (react-native)
'.m', '.mm', '.swift',
// Android (react-native)
'.kt',

React-native is a popular node/yarn environment, and react-native often comes with native code. These should be unplugged as well.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just realized this was a PR from a year ago, so I don't expect anybody to see the above comment!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'd be surprised 🙂 I'm not expert in RN but that seems reasonable enough, feel free to open a PR 👍

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@arcanis thanks for the feedback!

I checked up on the main (master) branch and found this:

if (typeof dependencyMeta.unplugged !== `undefined`)
return dependencyMeta.unplugged;
if (FORCED_UNPLUG_PACKAGES.has(pkg.identHash))
return true;
if (customPackageData.manifest.preferUnplugged !== null)
return customPackageData.manifest.preferUnplugged;
if (jsInstallUtils.extractBuildScripts(pkg, customPackageData, dependencyMeta, {configuration: this.opts.project.configuration}).length > 0 || customPackageData.misc.extractHint)
return true;
return false;

Which appears to not use a file extension system at all anymore. I think packages have to add a preferUnplugged option to my package.json files, but that's not super convenient for 3rd party packages I don't control :(

https://next.yarnpkg.com/configuration/manifest#preferUnplugged

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems reasonable indeed, PR welcome :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fbartho it got moved here:

const FORCED_EXTRACT_FILETYPES = new Set([
// Windows can't execute exe files inside zip archives
`.exe`,
// The c/c++ compiler can't read files from zip archives
`.h`, `.hh`, `.hpp`, `.c`, `.cc`, `.cpp`,
// The java runtime can't read files from zip archives
`.java`, `.jar`,
// Node opens these through dlopen
`.node`,
]);

// The java runtime can't read files from zip archives
'.java',
// Node opens these through dlopen
'.node',
]);

export class PnpLinker implements Linker {
supportsPackage(pkg: Package, opts: MinimalLinkOptions) {
return opts.project.configuration.get('nodeLinker') === 'pnp';
Expand Down Expand Up @@ -83,7 +94,7 @@ class PnpInstaller extends AbstractPnpInstaller {
}

async transformPackage(locator: Locator, dependencyMeta: DependencyMeta, packageFs: FakeFS<PortablePath>, {hasBuildScripts}: {hasBuildScripts: boolean}) {
if (hasBuildScripts || this.isUnplugged(locator, dependencyMeta)) {
if (hasBuildScripts || this.isUnplugged(locator, dependencyMeta, packageFs)) {
return this.unplugPackage(locator, packageFs);
} else {
return packageFs;
Expand Down Expand Up @@ -184,13 +195,16 @@ class PnpInstaller extends AbstractPnpInstaller {
return new CwdFS(unplugPath);
}

private isUnplugged(ident: Ident, dependencyMeta: DependencyMeta) {
private isUnplugged(ident: Ident, dependencyMeta: DependencyMeta, packageFs: FakeFS<PortablePath>) {
if (dependencyMeta.unplugged)
return true;

if (FORCED_UNPLUG_PACKAGES.has(ident.identHash))
return true;

if (packageFs.getExtractHint({relevantExtensions:FORCED_UNPLUG_FILETYPES}))
return true;

return false;
}
}
8 changes: 7 additions & 1 deletion packages/yarnpkg-fslib/sources/FakeFS.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,10 @@ export type Watcher = {
close: () => void;
};

export type ExtractHintOptions = {
relevantExtensions: Set<string>;
}

export abstract class FakeFS<P extends Path> {
static DEFAULT_TIME = 315532800;

Expand All @@ -60,6 +64,8 @@ export abstract class FakeFS<P extends Path> {
this.pathUtils = pathUtils;
}

abstract getExtractHint(hints: ExtractHintOptions): boolean;

abstract getRealPath(): P;

abstract resolve(p: P): P;
Expand Down Expand Up @@ -528,6 +534,6 @@ function getEndOfLine(content: string) {
return crlf > lf ? `\r\n` : `\n`;
}

export function normalizeLineEndings(originalContent: string, newContent: string){
export function normalizeLineEndings(originalContent: string, newContent: string) {
return newContent.replace(/\r?\n/g, getEndOfLine(originalContent));
}
4 changes: 4 additions & 0 deletions packages/yarnpkg-fslib/sources/NoFS.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ export class NoFS extends FakeFS<PortablePath> {
super(ppath);
}

getExtractHint(): never {
throw makeError();
}

getRealPath(): never {
throw makeError();
}
Expand Down
4 changes: 4 additions & 0 deletions packages/yarnpkg-fslib/sources/NodeFS.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ export class NodeFS extends BasePortableFakeFS {
this.realFs = realFs;
}

getExtractHint() {
return false;
}

getRealPath() {
return PortablePath.root;
}
Expand Down
12 changes: 8 additions & 4 deletions packages/yarnpkg-fslib/sources/ProxiedFS.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import {CreateReadStreamOptions, CreateWriteStreamOptions, FakeFS} from './FakeFS';
import {Dirent} from './FakeFS';
import {MkdirOptions, WriteFileOptions, WatchCallback, WatchOptions, Watcher} from './FakeFS';
import {FSPath, Filename, Path} from './path';
import {CreateReadStreamOptions, CreateWriteStreamOptions, FakeFS, ExtractHintOptions} from './FakeFS';
import {Dirent} from './FakeFS';
import {MkdirOptions, WriteFileOptions, WatchCallback, WatchOptions, Watcher} from './FakeFS';
import {FSPath, Filename, Path} from './path';

export abstract class ProxiedFS<P extends Path, IP extends Path> extends FakeFS<P> {
protected abstract readonly baseFs: FakeFS<IP>;
Expand All @@ -16,6 +16,10 @@ export abstract class ProxiedFS<P extends Path, IP extends Path> extends FakeFS<
*/
protected abstract mapFromBase(path: IP): P;

getExtractHint(hints: ExtractHintOptions) {
return this.baseFs.getExtractHint(hints);
}

resolve(path: P) {
return this.mapFromBase(this.baseFs.resolve(this.mapToBase(path)));
}
Expand Down
6 changes: 5 additions & 1 deletion packages/yarnpkg-fslib/sources/VirtualFS.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {FakeFS} from './FakeFS';
import {FakeFS, ExtractHintOptions} from './FakeFS';
import {NodeFS} from './NodeFS';
import {ProxiedFS} from './ProxiedFS';
import {Filename, PortablePath, ppath} from './path';
Expand Down Expand Up @@ -66,6 +66,10 @@ export class VirtualFS extends ProxiedFS<PortablePath, PortablePath> {
this.baseFs = baseFs;
}

getExtractHint(hints: ExtractHintOptions) {
return this.baseFs.getExtractHint(hints);
}

getRealPath() {
return this.baseFs.getRealPath();
}
Expand Down
33 changes: 22 additions & 11 deletions packages/yarnpkg-fslib/sources/ZipFS.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
import {Libzip} from '@yarnpkg/libzip';
import {ReadStream, Stats, WriteStream, constants} from 'fs';
import {PassThrough} from 'stream';
import {isDate} from 'util';

import {CreateReadStreamOptions, CreateWriteStreamOptions, BasePortableFakeFS} from './FakeFS';
import {FakeFS, MkdirOptions, WriteFileOptions} from './FakeFS';
import {WatchOptions, WatchCallback, Watcher} from './FakeFS';
import {NodeFS} from './NodeFS';
import * as errors from './errors';
import {FSPath, PortablePath, npath, ppath, Filename} from './path';
import {Libzip} from '@yarnpkg/libzip';
import {ReadStream, Stats, WriteStream, constants} from 'fs';
import {PassThrough} from 'stream';
import {isDate} from 'util';

import {CreateReadStreamOptions, CreateWriteStreamOptions, BasePortableFakeFS, ExtractHintOptions} from './FakeFS';
import {FakeFS, MkdirOptions, WriteFileOptions} from './FakeFS';
import {WatchOptions, WatchCallback, Watcher} from './FakeFS';
import {NodeFS} from './NodeFS';
import * as errors from './errors';
import {FSPath, PortablePath, npath, ppath, Filename} from './path';

const S_IFMT = 0o170000;

Expand Down Expand Up @@ -251,6 +251,17 @@ export class ZipFS extends BasePortableFakeFS {
this.ready = true;
}

getExtractHint(hints: ExtractHintOptions) {
for (const fileName of this.entries.keys()) {
const ext = this.pathUtils.extname(fileName);
if (hints.relevantExtensions.has(ext)) {
return true;
}
}

return false;
}

getAllFiles() {
return Array.from(this.entries.keys());
}
Expand Down
22 changes: 13 additions & 9 deletions packages/yarnpkg-fslib/sources/ZipOpenFS.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import {Libzip} from '@yarnpkg/libzip';
import {constants} from 'fs';
import {Libzip} from '@yarnpkg/libzip';
import {constants} from 'fs';

import {CreateReadStreamOptions, CreateWriteStreamOptions, BasePortableFakeFS} from './FakeFS';
import {Dirent} from './FakeFS';
import {FakeFS, MkdirOptions, WriteFileOptions} from './FakeFS';
import {WatchOptions, WatchCallback, Watcher} from './FakeFS';
import {NodeFS} from './NodeFS';
import {ZipFS} from './ZipFS';
import {Filename, FSPath, PortablePath} from './path';
import {CreateReadStreamOptions, CreateWriteStreamOptions, BasePortableFakeFS, ExtractHintOptions} from './FakeFS';
import {Dirent} from './FakeFS';
import {FakeFS, MkdirOptions, WriteFileOptions} from './FakeFS';
import {WatchOptions, WatchCallback, Watcher} from './FakeFS';
import {NodeFS} from './NodeFS';
import {ZipFS} from './ZipFS';
import {Filename, FSPath, PortablePath} from './path';

const ZIP_FD = 0x80000000;

Expand Down Expand Up @@ -64,6 +64,10 @@ export class ZipOpenFS extends BasePortableFakeFS {
this.notZip = new Set();
}

getExtractHint(hints: ExtractHintOptions) {
return this.baseFs.getExtractHint(hints);
}

getRealPath() {
return this.baseFs.getRealPath();
}
Expand Down
1 change: 1 addition & 0 deletions packages/yarnpkg-fslib/sources/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ export {WatchCallback} from './FakeFS';
export {Watcher} from './FakeFS';
export {WriteFileOptions} from './FakeFS';
export {normalizeLineEndings} from './FakeFS';
export {ExtractHintOptions} from './FakeFS';

export {FSPath, Path, PortablePath, NativePath, Filename} from './path';
export {ParsedPath, PathUtils, FormatInputPathObject} from './path';
Expand Down
4 changes: 2 additions & 2 deletions packages/yarnpkg-pnp/sources/generateSerializedState.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import {LocationBlacklistData, LocationLengthData, PackageRegistryData} from './types';
import {PackageStoreData, PnpSettings, SerializedState} from './types';
import {LocationBlacklistData, PackageRegistryData} from './types';
import {PackageStoreData, PnpSettings, SerializedState} from './types';

// Keep this function is sync with its implementation in:
// @yarnpkg/core/sources/miscUtils.ts
Expand Down
6 changes: 5 additions & 1 deletion packages/yarnpkg-pnpify/sources/NodeModulesFS.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {Dirent, Filename, MkdirOptions} from '@yarnpkg/fslib';
import {Dirent, Filename, MkdirOptions,ExtractHintOptions} from '@yarnpkg/fslib';
import {FSPath, NativePath, PortablePath, npath, ppath} from '@yarnpkg/fslib';
import {WatchOptions, WatchCallback, Watcher, toFilename} from '@yarnpkg/fslib';
import {NodeFS, FakeFS, WriteFileOptions, ProxiedFS} from '@yarnpkg/fslib';
Expand Down Expand Up @@ -86,6 +86,10 @@ export class PortableNodeModulesFS extends FakeFS<PortablePath> {
}
}

getExtractHint(hints: ExtractHintOptions) {
return this.baseFs.getExtractHint(hints);
}

resolve(path: PortablePath) {
return this.baseFs.resolve(this.resolvePath(path).resolvedPath!);
}
Expand Down