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.

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

Fixed path to binaries in postinstall script #86

Merged
merged 1 commit into from
Nov 9, 2019

Conversation

schibrikov
Copy link
Contributor

Attempt to resolve issue #85

@schibrikov
Copy link
Contributor Author

schibrikov commented Nov 8, 2019

@Arkweid check this out, works good from the first sight
I don't understand why was cwd used there, because binaries are always located next to postinstall script, so my approach should work I believe

@Arkweid
Copy link
Collaborator

Arkweid commented Nov 8, 2019

@Arkweid check this out, works good from the first sight
I don't understand why was cwd used there, because binaries are always located next to postinstall script, so my approach should work I believe

Cool! Will check today.

@Arkweid Arkweid merged commit c85ec9d into evilmartians:master Nov 9, 2019
@Arkweid
Copy link
Collaborator

Arkweid commented Nov 9, 2019

Unfortenuly it doen't work :(

  1. One underline missed
    https://github.com/Arkweid/lefthook/pull/86/files#diff-57f92dec60c72c04bad912182b175f12R18
binpath = join(_dirname, 'bin', binary);
  1. Postinstal script will install config and hooks into .node_modules directory.

Could you recheck it?

@schibrikov
Copy link
Contributor Author

Yes, of course.

Postinstal script will install config and hooks into .node_modules directory.

I don't completely understand what you mean here. All files inside .npm directory will be collocated inside node_modules as I know. So why do we care about configs/hooks/whatever?

@schibrikov
Copy link
Contributor Author

I found the previous version of your comment and understood what it's about :)
So what if we just return process.chdir(process.env.INIT_CWD);?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants