Dvc: Do we need `exec` in git hooks?

[lrob]

I'm using the dvc hooks for git. I would like to add some lines to the post-checkout script but it cannot be done (as far as I know) because the dvc command is executed using the exec command. What is the reason of this choice? Can the exec command be removed?

[pared]

Related #679.
@lrob It seems to me that you can simply edit the hooks for your repo and remove the exec part. I am not sure if and what was the reasoning for using exec originally. @efiop do you, by any chance, remember?

[efiop]

Hi @lrob . There is no particular reason for exec, we just wanted to launch our hooks directly without spawning a yet another process. As @pared noted, you should be ok to remove it and modify as you wish. But I also have to point out that it is probably better to use --use-pre-commit-tool flag, which uses https://pre-commit.com/ as a framework for git hooks, which makes it much easier to manage. You could create your own hooks for it too.

[lrob]

Thanks @efiop. I'll proceed as you suggest.

However, I think that if not strictly required the exec command should be removed. In my experience I was adding code to the hooks and at first sight I did not realized that it was not being executed because of the exec. After noticing the exec I was in doubt about the possibility to remove it safely. I think it presence has been quite annoying compared to the advantages it gives.

[efiop]

@lrob Without the exec you'll need more flags/logic to handle the failure properly, as just removing exec will make the script not error-out on dvc command failure. So the hook will become uglier and might also not match your expectectations for further modification. Using exec right now clearly signals that our installed auto-generated hooks are standalone, we don't expect you to add anything yourself to them. But you could modify it at your own risk. So I'm not seeing a clear advantage to removing it compared to the alternative implementation.