nicojs / node-link-parent-bin

A tool to link the bins of the your npm dependencies to child packages in a node multi-package repository
43 stars 11 forks source link

Mac - Problem with multiple dependencies importing same bin #5

Open jonas-lauber opened 6 years ago

jonas-lauber commented 6 years ago

I just moved my project to Mac yesterday (was using Windows before) and I ran into another small problem (not blocking though). In have 2 dependencies importing the same bin (jest and jest-cli both importing .bin/jest.js) The first time I run the command I get ugly exception from the symlink command.

[2018-04-03 16:41:17.788] [ERROR] ParentBinLinker - Could not link bin jest for child components-demo. { Error: EEXIST: file already exists, symlink '[removed]/node_modules/jest-cli/bin/jest.js' -> '[removed]/.bin/jest' errno: -17, code: 'EEXIST', syscall: 'symlink', path: '[removed]/node_modules/jest-cli/bin/jest.js', dest: '/[removed]/.bin/jest' }

The second time everything works fine and I get the expected warning.

[2018-04-03 16:26:18.707] [INFO] link - Different link at '[removed]/.bin/jest' already exists. Leaving it alone, the package is probably already installed in the child package.

For what I saw this is because everything runs asynch and they both do the check before the symlink is actually done. Then the first do it and everything goes fine and the second tries and it fails. What I did by me, is to systematically call symlink and if it fails checkout why it failed. => https://github.com/jonas-lauber/node-link-parent-bin/commit/211fed660cc3bdf0d64f62ef1bcee48f35f7bea6 A small optimisation would be to check if to exists first to handle the case where it was created by a previous run (the case where the same run has multiple times the same bin remains).

Something like:

 function linkIfExists(from: string, to: string) {
    if (fs.existsSync(to)) {
        return handleExistingSymlink(from, to);
    }

    return symlink(from, to)
        .catch(() => handleExistingSymlink(from, to));
}

But I am actually not sure it is much more efficient... So I kept it simple.

Once again, I would love to make a PR and not do my stuff in a fork, but my skills lack to do the tests properly...

nicojs commented 6 years ago

I'll help you running tests in #4 :v:

nicojs commented 5 years ago

@jonas-lauber I think we can close this one?

Aqours commented 5 years ago

Should we add some filters, like include and exclude to include root packages or exclude packages?

nicojs commented 5 years ago

@Aqours what do you mean exactly?

Aqours commented 5 years ago

@nicojs

17:45:06 (5012) INFO ParentBinLinker Linking dependencies ["@commitlint/cli","@commitlint/config-conventional","husky","lerna","link-parent-bin","ts-loader","tslint","typescript","webpack","webpack-cli"] under children ["base-1","demo","test"]
17:45:06 (5012) INFO link Link at 'packages\base-1\node_modules\.bin\lerna' already exists. Leaving it alone.
17:45:06 (5012) INFO link Link at 'packages\demo\node_modules\.bin\lerna' already exists. Leaving it alone.
17:45:06 (5012) INFO link Link at 'packages\test\node_modules\.bin\lerna' already exists. Leaving it alone.
17:45:06 (5012) INFO link Link at 'packages\base-1\node_modules\.bin\link-parent-bin' already exists. Leaving it alone.
17:45:06 (5012) INFO link Link at 'packages\demo\node_modules\.bin\link-parent-bin' already exists. Leaving it alone.
17:45:06 (5012) INFO link Link at 'packages\test\node_modules\.bin\link-parent-bin' already exists. Leaving it alone.
17:45:06 (5012) INFO link Link at 'packages\base-1\node_modules\.bin\tslint' already exists. Leaving it alone.

This package link every binary to sub-project, but these packages(["@commitlint/cli","@commitlint/config-conventional","husky","lerna","link-parent-bin","ts-loader"]) seems unnecessary symlink.