husky npm package doesn't work when installed with esy

[giltho]

Hello,

I was using esy to work on a small project, and I have a bad habit of writing terrible commit messages, which is why I installed husky + commitlint through esy. However, it does not seem that the hooks were properly installed (as it would be the case when installing through npm).

The thing is, I have no idea when the _install script of husky is called. And I don't think devinstall is one of the npm scripts...

[bryphe]

Hey @giltho , thanks for the issue!

Just curious - what version of esy are you using, and which platform?

I was using esy to work on a small project, and I have a bad habit of writing terrible commit messages,

I know the feeling 😄 I was curious if husky worked too, so I could run refmt prior to my commits.

I looked at this briefly - esy actually runs the lifecycle script husky uses. That happens here:

esy/esyi/FetchStorage.ml

Line 294 in d59ec6c

let%bind () =

(husky uses the 'install' script to hook everything up).

Husky makes some assumptions about the path - it tries to look upward to see where it needs to set up the hooks:

/**
 * @param huskyDir - e.g. /home/typicode/project/node_modules/husky/
 * @param requireRunNodePath - path to run-node resolved by require e.g. /home/typicode/project/node_modules/.bin/run-node
 * @param isCI - true if running in CI
 */
function install(huskyDir, requireRunNodePath = require.resolve('.bin/run-node'), isCI) {
    console.log('husky > setting up git hooks');
    // First directory containing user's package.json
    const userPkgDir = pkgDir.sync(path.join(huskyDir, '..'));
    // Get conf from package.json or .huskyrc
    const conf = getConf_1.default(userPkgDir);
    // Get directory containing .git directory or in the case of Git submodules, the .git file
    const gitDirOrFile = findUp.sync('.git', { cwd: userPkgDir });
    // Resolve git directory (e.g. .git/ or .git/modules/path/to/submodule)
    const resolvedGitDir = resolveGitDir_1.default(userPkgDir);

I suspect that these assumptions might not be valid in the install environment we use for esy.

One starting point we need is to log the output of the lifecycle scripts - it doesn't look like we do that today. I don't see it written to a file anywhere but @andreypopp might know for sure. There's some information in the logging of husky that could help guide us to see exactly what's wrong.

[andreypopp]

Yeah, I think husky isn't "pnp-enabled".

I've tried to add husky as a devDependency along with simplest husky config from its README into yarnpkg/pnp-sample-app repository:

  "husky": {
    "hooks": {
      "pre-commit": "echo pre-commit"
    }
  },

Then after installing it with yarn — hooks weren't activated for me.

I've tried to find if husky has some kind of command to install hooks manually and not during its postinstall phase but wasn't able to find it. By the way I don't think this is a good idea to mess with user's repository on postinstall — instead I'd prefer to have a speciall command like husky-install which would install hooks specified in package.json — I'd put such command in "bootstrap" script or something like that.

In the future I think we need to add sandboxing to npm's lifecycle (install/postinstall/...) like we do for builds on macOS — so that lifecycle can only modify files inside their corresponding package's directory. It is much safer.

[giltho]

Sorry indeed, I forgot my configuration :

• esy -> 0.3.1
• MacBook Pro 2016 with MacOS Mojave 10.14.1

The weird thing that I don't understand is how the _install script is called. I know that install is called in the npm lifecycle, but there is not such script, they renamed it _install so I don't know when it is called

[giltho]

By the way I don't think this is a good idea to mess with user's repository on postinstall

Indeed but it is husky's philosophy right ? The point is to force users to install your hooks so they cannot commit without going through hooks. You lose that if they have to manually execute husky

[andreypopp]

@giltho

The weird thing that I don't understand is how the _install script is called. I know that install is called in the npm lifecycle, but there is not such script, they renamed it _install so I don't know when it is called

The published npm package has _install renamed to install:

% npm view husky scripts
{ test: 'npm run lint && jest',
  install: 'node husky install',
  preuninstall: 'node husky uninstall',
  devinstall: 'npm run build && npm run _install -- node_modules/husky && node scripts/dev-fix-path',
  devuninstall: 'npm run build && npm run preuninstall -- node_modules/husky',
  build: 'del-cli lib && tsc',
  version: 'jest -u && git add -A src/installer/__tests__/__snapshots__',
  postversion: 'git push && git push --tags',
  prepublishOnly: 'npm run test && npm run build && pinst --enable && pkg-ok',
  postpublish: 'pinst --disable',
  lint: 'tslint \'src/**/*.ts\'',
  fix: 'npm run lint -- --fix' }

Indeed but it is husky's philosophy right ? The point is to force users to install your hooks so they cannot commit without going through hooks. You lose that if they have to manually execute husky

Maybe, I still think hooks are for convenience — you'd still want to validate invariants hooks are trying to force on CI or something like that.

[giltho]

I understand your point, although I don't think husky is going to change. Do you believe esy should implement commands that execute install scripts for packages ?
esy package-install -> esy husky-install as you said ?

[andreypopp]

I can't say for sure at this point.

Maybe husky can be changed to work with yarn's pnp — then it will be working with esy automatically (before the point we add sandboxing but then we can implement whitelist or something like that).

[giltho]

To come back on @bryphe point, how should I proceed if I want to make a PR that logs the output of lifecycle scripts ?
Redirect to stdout ? Write to a file ? use Logs_lwt.
The last option would mean (I think) that things would only be logged at the end of the execution of the script, and that stdin/stderr would be separate.

[andreypopp]

We really have to add sandboxing to npm lifecycle:

An npm module named husky destructively added pre- and post-commit hooks to my dotfiles repo (literally ~/.git!). Or maybe some other module told it to do that. I never asked for that. I don't understand why the JavaScript tool ecosystem is like this!

https://twitter.com/garybernhardt/status/1062760973457514496

[andreypopp]

@giltho I suggest checking out esy@0.4.0

Then look at https://github.com/esy/esy/blob/master/esyi/FetchStorage.ml#L295 for how it executes hooks.

I think the problem is that husky expects itself to be in some node_modules dir down the project dir so it could recurse up a find a directory with .git. It doesn't happen in our case because packages are installed into ~/.esy/source/i and not into project subdirectory like in node_modules-style installation.

[giltho]

Yes sorry I misspoke, I've read a bit about plug'n'play and I understood better how husky is installing hooks after your comments. And indeed, the issue seems to be on husky's side.

What I meant is that it is still interesting to have the logs of lifecycle scripts printed out when executing esy. I've started modifying FetchStorage on my side and I have something that I believe works. I simply redirect the output of the process to stdout & stderr.

The thing is that I do not know what you think about that. I do not like to use pervasives constants like stdin or stdout (or Lwt_io.stdin / out) when I have modules written for logging.

[andreypopp]

We use logs library for logging.

You can add logging by adding lines like

Logs_lwt.debug (fun m -> m "stdout: %s" stdou);%lwt

I think debug level is the right one for that kind of log messages.

[andreypopp]

I understood better how husky is installing hooks after your comments

I don't really know how it works, I was just speculating.

[giltho]

Logs_lwt.debug (fun m -> m "stdout: %s" stdout);%lwt

That would mean we do not write things as they happen but only once everything happens.
If there are things in stderr happening as the same time as things in stdout and we print them separately it might be more difficult to understand what happened

(I'll ping you on discord)

[giltho]

For the sake of completeness on this issue. Husky is now compatible with yarn 2 (and therefore pnp), but there's a bit of yarn-specific configuration that happens there. I.e, there's a configuration in Husky to use it with Yarn, when it is activated, nice things happen that are compatible with yarn pnp. Mostly, it tells husky to call scripts through yarn instead of npm (typicode/husky#511 and yarnpkg/berry#102).
One solution to this issue could be to add something similar for esy ?

[dylanirlbeck]

In general, how difficult would it be to build a pre-commit hook solution that's perfectly compatible (maybe even recommended) by esy? My hacky solution in a project was to use esy as a package manager, and yarn solely for husky + lint-staged. Ideally there'd be an OCaml/NPM package I can install with esy add that "just works" (and also works with an esy.json!).

I have some time in the next few weeks, so I'd love to investigate building a sort of "official" pre-commit formaatting hook for Reason/OCaml, though I'd definitely need some input before starting.

cc @giltho @andreypopp

[giltho]

Hey @dylanirlbeck, I wanted to answer that 9 days ago when you pinged me and I felt like this wasn't really constructive, but I changed my mind:
I have no idea what a good solution is 😅 Sandboxing makes it much more difficult for a tool like this to work, and some may even defend that this kind of things should not be happening automatically.
However, I am myself very interested :) Here are some random ideas:

• You could have the tool detect the limit of the project in the same way ocamlformat does, so that it never goes and modify a .git folder it's not supposed to
• I think this may require changes in esy, because post-install may not be executed every time you add it to a project, since dependencies are only ever installed once.
• If you're working on this, please do come on the #esy channel of the Ocaml discord, I'm not the biggest esy expert, but I'll try to answer, and some people there are extremely good. Also everybody's nice 😄 (If you want to join and can't find an invite link, ping me :) )