Support merging with git LFS hooks

[jdtzmn]

Disclaimer: I don't think that this will handle updating hooks if git lfs hooks are present. I would consider adding functionality to check if there is git-lfs and husky installed and then only update husky, but that might be more advanced than this pr should be.

[shaodahong]

If local git lfs after husky install...

[jdtzmn]

Hmm that's a good point. I could see two ways of fixing that problem:

1. Providing documentation for that case
2. Including a comment about that that will appear when installing git-lfs. If hooks are present already, git-lfs shows the current hook that would be overwritten; a link to documentation could be presented there.

[shaodahong]

❯ git lfs install               
Hook already exists: pre-push

        #!/bin/sh
        # husky

        # Created by Husky v4.2.5 (https://github.com/typicode/husky#readme)
        #   At: 6/1/2020, 6:14:22 PM
        #   From: /ant-design/node_modules/husky (https://github.com/typicode/husky#readme)

        . "$(dirname "$0")/husky.sh"

To resolve this, either:
  1: run `git lfs update --manual` for instructions on how to merge hooks.
  2: run `git lfs update --force` to overwrite your hook.

[jdtzmn]

What if it was like this instead:

❯ git lfs install               
Hook already exists: pre-push

        #!/bin/sh
        # husky

        # Created by Husky v4.2.5 (https://github.com/typicode/husky#readme)
        #   At: 6/1/2020, 6:14:22 PM
        #   From: /ant-design/node_modules/husky (https://github.com/typicode/husky#readme)

        # See the following documentation for using husky with git lfs: [url to documentation]
        . "$(dirname "$0")/husky.sh"

To resolve this, either:
  1: run `git lfs update --manual` for instructions on how to merge hooks.
  2: run `git lfs update --force` to overwrite your hook.

Note the added comment above . "$(dirname "$0")/husky.sh"

[shaodahong]

@typicode Look like good. Please help check, thanks

[mijay]

Any chances this can be merged?

[kawazoe]

@jdtzmn Any chance you could resolve the merge soon? I can't wait to see this PR land 😀

[jdtzmn]

@jdtzmn Any chance you could resolve the merge soon? I can't wait to see this PR land 😀

Yeah I can resolve the merge conflicts. The PR was originally waiting on a review from @typicode; the merge conflicts are new.

[jdtzmn]

@kawazoe Done!

[kawazoe]

Thanks! I hope that with a passing build, we have more chances of the PR getting merged quickly. ;)

EDIT: Eh, talked too quickly...

[jdtzmn]

Oops- I forgot to update the snapshots. I'll fix that.

[jdtzmn]

@kawazoe I'm not sure why the checks are failing. Running npm run test locally works as expected.

[kawazoe]

@jdtzmn I took a quick peak at the CI logs and it looks like the only issue is with prettier's linting:

/home/travis/build/typicode/husky/src/installer/checkGitVersion.ts
  11:15  error  Delete `,`  prettier/prettier
  12:11  error  Delete `,`  prettier/prettier

[jdtzmn]

@kawazoe I saw that as well, but the error is caused by the eslint . --ext .js,.ts --ignore-path .gitignore command and that command does not error when I run it locally.

Normally I would believe the CI over my local environment, but I've checked every file in the project called checkGitVersion.ts and the line numbers don't match at all. I've also run prettier on those files individually to no avail as they pass without any errors.

Let me know what you think.

[kawazoe]

@jdtzmn from what I'm seeing, checks have been failing on this file for a while now: https://github.com/typicode/husky/commits/master/src/installer/checkGitVersion.ts

@typicode could you please look into it?

[typicode]

Sorry for the late reply and thanks for the PR!

I didn't run the code, but wouldn't it append husky's shell script multiple time if husky installer is run multiple times (for example when upgrading from v4.x.y to v4.x.z or with npm rebuild)?

That said, it should be easier to have LFS work with husky 5 as hooks are exposed in .husky and can be directly edited.

[jaytonic]

I've this exact same issue, but it isn't solved by this PR. The difference is that mine is on our CI pipeline on Azure Devop. On each run, when git-lfs is enabled for this pipeline, it does run git lfs install and complains that a git hook is already defined.

Any idea how I can workaround this?