Skip to content
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.

Sign up for GitHub

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

Use npmignore file on fetch #8010

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Use npmignore file on fetch #8010

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3364dbc
Add del package to delete files vith respect glob
Mar 22, 2020
249cb27
Import/Export del module on fs util
Mar 22, 2020
a5ba07e
Add npmignore filename on constants
Mar 22, 2020
50456b9
Create deleter function to delete files on npmignore
Mar 22, 2020
28296fb
Write test for unlinkIgnoredFiles
Mar 22, 2020
063a231
Refactor deleter code and avoid line errors
Mar 22, 2020
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
23 changes: 23 additions & 0 deletions __tests__/fetchers.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import mkdir from './_temp.js';
import * as fs from '../src/util/fs.js';
import {readdirSync} from 'fs';

import {unlinkIgnoredFiles} from '../src/package-fetcher';

const path = require('path');
const ssri = require('ssri');

Expand Down Expand Up @@ -78,6 +80,27 @@ test('GitFetcher.fetch', async () => {
expect(name).toBe('beeper');
});

test.only('Use .npmignore file after install a module', async () => {
const dir = await mkdir('git-fetcher');
const fetcher = new GitFetcher(
dir,
{
type: 'git',
reference: 'https://github.com/facebook/flux',
hash: '4f0af648c2bdcca7088c4935898f51aeb2d9507f',
registry: 'npm',
},
await Config.create(),
);
await fetcher.fetch();
await unlinkIgnoredFiles(dir);
expect(await fs.exists(path.join(dir, 'src/'))).toBe(false);
expect(await fs.exists(path.join(dir, 'Makefile'))).toBe(false);
expect(await fs.exists(path.join(dir, 'Flux.js'))).toBe(false);
expect(await fs.exists(path.join(dir, 'CONTRIBUTING.md'))).toBe(false);
expect(await fs.exists(path.join(dir, '.flowconfig'))).toBe(false);
});

test('GitFetcher.getTarballMirrorPath without slashes in the repo path', async () => {
const dir = await mkdir('git-fetcher');
const config = await Config.create();
Expand Down
1 change: 1 addition & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
"death": "^1.0.0",
"debug": "^3.0.0",
"deep-equal": "^1.0.1",
"del": "^5.1.0",
"detect-indent": "^5.0.0",
"dnscache": "^1.0.1",
"glob": "^7.1.1",
Expand Down
1 change: 1 addition & 0 deletions src/constants.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ export const CLEAN_FILENAME = '.yarnclean';

export const NPM_LOCK_FILENAME = 'package-lock.json';
export const NPM_SHRINKWRAP_FILENAME = 'npm-shrinkwrap.json';
export const NPM_IGNORE_FILENAME = '.npmignore';

export const DEFAULT_INDENT = ' ';
export const SINGLE_INSTANCE_PORT = 31997;
Expand Down
21 changes: 20 additions & 1 deletion src/package-fetcher.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ import * as fetchers from './fetchers/index.js';
import * as fs from './util/fs.js';
import * as promise from './util/promise.js';

import {NPM_IGNORE_FILENAME} from '../src/constants.js';

const path = require('path');
const ssri = require('ssri');

async function fetchCache(
Expand Down Expand Up @@ -73,10 +76,12 @@ export async function fetchOneRemote(
await fs.unlink(dest);

try {
return await fetcher.fetch({
const fetchingProccess = await fetcher.fetch({
name,
version,
});
await unlinkIgnoredFiles(dest);
return fetchingProccess;
} catch (err) {
try {
await fs.unlink(dest);
Expand All @@ -87,6 +92,20 @@ export async function fetchOneRemote(
}
}

export async function unlinkIgnoredFiles(dest: string): Promise<void> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for working on this! I'm not entirely sure if this is the right place to apply this filter. Could we instead apply this while we unpack so that ignored files are never even written? If we scope it to git dependencies only it shouldn't have an impact on install time.

Copy link
Author

Choose a reason for hiding this comment

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

Your welcome! Okay, I will work on it.

Copy link

Choose a reason for hiding this comment

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

Thanks for this, how'd you get on? :)

Copy link
Author

@wethil wethil Jul 23, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

hey! sorry, i have been super busy, dealing with the conditions that comes with Covid, but I started to working on again

const ignoreFilePath = path.join(dest, NPM_IGNORE_FILENAME);
const ignoringPatternFileExist = await fs.exists(ignoreFilePath);

if (ignoringPatternFileExist) {
const ignoringPatternFileAsRaw = await fs.readFile(ignoreFilePath);
const filesWillBeUnlikedAsArray = ignoringPatternFileAsRaw
.toString()
.split(/\r?\n/)
.filter(line => line.trim() !== '' && line.charAt(0) !== '#');
await fs.del(filesWillBeUnlikedAsArray.map(file => path.join(dest, file)), {force: true});
}
}

function fetchOne(ref: PackageReference, config: Config): Promise<FetchedMetadata> {
const dest = config.generateModuleCachePath(ref);

Expand Down
2 changes: 2 additions & 0 deletions src/util/fs.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import type {CopyFileAction} from './fs-normalized.js';

import fs from 'fs';
import globModule from 'glob';
import delModule from 'del';
import os from 'os';
import path from 'path';

Expand Down Expand Up @@ -43,6 +44,7 @@ export const lstat: (path: string) => Promise<fs.Stats> = promisify(fs.lstat);
export const chmod: (path: string, mode: number | string) => Promise<void> = promisify(fs.chmod);
export const link: (src: string, dst: string) => Promise<fs.Stats> = promisify(fs.link);
export const glob: (path: string, options?: Object) => Promise<Array<string>> = promisify(globModule);
export const del: (pathArray: Array<string>, options?: Object) => Promise<void> = delModule;
export {unlink};

// fs.copyFile uses the native file copying instructions on the system, performing much better
Expand Down
Loading