{"thread":{"id":"52276","subject":"[RFC] Hook management via 'git hooks' command","startedAt":"2019-11-16T01:11:34Z","lastAt":"2019-12-02T23:46:18Z","messageCount":12,"participants":["Emily Shaffer","brian m. carlson"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"386352","messageId":"20191116011125.GG22855@google.com","threadId":"52276","inReplyTo":null,"subject":"[RFC] Hook management via 'git hooks' command","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2019-11-16T01:11:25Z","receivedAt":"2019-11-16T01:11:34Z","isPatch":false,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"Hi all.\n\nThe topic of multihook support has come up before; here are a handful of\nconversations I could find on it:\n\n brian m. carlson's proposal for multiple hooks via .git/hooks/pre-commit.d/:\n https://public-inbox.org/git/20190424004948.728326-1-sandals@crustytoothpaste.net/\n\n Jonathan Nieder's discussion of hooks as an attack vector:\n https://public-inbox.org/git/20171002234517.GV19555@aiede.mtv.corp.google.com/\n\n Ævar Arnfjörð Bjarmason's RFC for multihooks in a similar manner to brian's:\n https://public-inbox.org/git/CACBZZX6j6q2DUN_Z-Pnent1u714dVNPFBrL_PiEQyLmCzLUVxg@mail.gmail.com/\n\nAs I consider these conversations, I think about what goals we are aiming\nfor:\n\n 1. Let a user run more than one hook on a single event.\n 2. Let a user determine the order of those hooks.\n 3. Let a user version-control their hooks without kludging. (This is actually\n largely solved by core.hookName, thanks to Ævar.)\n 4. Make it hard for a user to run a hook they did not expect to run.\n 5. Don't break users who currently have hooks in $REPO/.git/hooks, but provide\n a path for them to stop using hooks from that directory.\n\nIt seems that brian's thread stalled out in v2, but as I understand it, the\nmethod was:\n\n - For each codepath which executes a hook:\n - Check whether .git/hooks/${hookname} is valid (exists, executable)\n - If not, check whether .git/hooks/${hookname}.d exists, and run each script\n within, in alphabetical order.\n\nThis approach seems similar to one that Ævar suggested in their RFC linked\nabove.\n\nBenefits of that approach include:\n - A familiar Unixlike interface\n - Previously configured hooks will Just Work\n\nDrawbacks of that approach include:\n - Still an attack vector, unless core.hooksDirectory is set\n - Still need to configure a hook for each repo, unless core.hooksDirectory is\n set\n - Hooks are still not version controlled, unless core.hooksDirectory is set\n\nThis meets goals 1, 2, and 5. Using core.hookPath makes this approach meet 3 as\nwell.\n\nAn alternate suggestion from Jonathan Nieder seems to look something like:\n\n - Learn a core.${hookname} config.\n - Teach all hook executing codepaths to check core.${hookname}\n - Either add multiple core.${hookname} entries and order ...TBD? or put lots of\n hooks on a single config entry and run in order on that line.\n\nBenefits of the config approach include:\n - Easy to configure global, repo-wide, worktree-wide hooks\n - Long-term, the decision of which hooks to run is left entirely to the user\n - The hooks themselves can be version controlled, even distributed with the\n repo (a user could specify ${repopath}/hooks/pre-push-lint if they wanted,\n and we chose to allow it)\n\nDrawbacks of the config approach include:\n - Different enough from the current approach to be unusual. Coming away from\n the transition period will require some thought.\n - The ordering of hooks may be confusing.\n - It may be difficult to choose _not_ to run a global hook from a local\n project.\n\nThis meets goals 3, 4, and 5; it could meet 1 and 2 but needs to be fleshed out\nmore.\n\nMeanwhile, I have some other concerns when I read through the source:\n\n - I don't think I saw a codified list of all the possible hooks. Meaning,\n find_hook() is called with a freeform string, not from some enum, and it's\n hard to programmatically list all places where a hook could be used. This\n came up when I was looking at bugreport; even the template\n ${hookname}.sample files prepopulated in .git/hooks aren't a comprehensive\n list of all hooks mentioned in `git help hooks`. This feels like a lack of\n assurance that all hooks are well-documented.\n\nPlease do let me know if I missed something in my attempt to frame the \"story so\nfar\".\n\nHere's my suggestion.\n\n - Let configs handle the hook list and ordering, via 'hook.{hookname}' which\n can be specified more than once.\n - global config: hook.update = /path/to/firsthook\n user config: hook.update = /path/to/secondhook\n worktree config: hook.update = -/path/to/firsthook #eliminate the firsthook\n call\n - global config: hook.update = /path/to/firsthook\n repo config: hook.update = /path/to/secondhook\n repo config: hook.update = ^/path/to/firsthook #move firsthook execution\n after secondhook for this project\n - Let users decide whether they want to check core.hookDir in addition to their\n config, instead of their config, or not at all, via a config\n 'hook.runHookDir' \"hookdir-first\", \"hookdir-only\", \"config-only\", etc.\n - Let users ask to be notified if running a hook from .git/hooks via a config\n 'hook.warnHookDir'. (Mentioning .git/hooks rather than core.hookDir is\n intentional here.) Alternatively, ask for 'hook.warn' with values \"hookdir\",\n \"all\" depending on how trusting the user feels.\n - If we want to phase out .git/hooks, we can make this notification opt-out\n instead of opt-in, someday.\n - between runHookDir and warnHookDir, users are able to phase out\n .git/hooks scripts at their own pace.\n - Abstract most of this via a 'git hooks' command.\n - 'git hooks add [--] ' to append another hook;\n let the scope setting match 'git config'.\n - 'git hooks show []' to indicate which hooks will be run, either\n on a specific event or for all events, and in which order, as well as each\n hook's config origin\n - 'git hooks edit ' to modify the hook order, or choose not to run\n a hook locally\n - By managing it through a command, we can reorder the contents of config\n files if it makes sense to do so.\n - Add a hook library.\n - Optionally specify all hook events via enum, so we aren't string-typing\n calls to find_hook() anymore.\n - Resolve configs into a list of hooks to run by concatenating them together\n in config precedence order.\n - Also allow configs formatted like \"-\" to remove an\n earlier-configured invocation of that hook, or \"^\" to move the\n earlier-configured invocation to this point in the execution order\n - Move the find_hook() implementation to here, to account for the multihook\n ordering\n\nI think it reaches the goals I mentioned:\n\n 1. User can specify more than one hook per event by adding more configs or\n running 'git hooks add '.\n 2. User can determine the order by reordering their configs in file, using\n \"^/hookpath\" notation, or using 'git hooks edit'.\n 3. User can point to a hook script which is checked into the repo they want it\n to run on, another repo in their filesystem, or anywhere at all. Contrary to\n the behavior of core.hookDir, users can manage each hook in a different repo\n if they wish (maybe the git-secrets hooks all live in the git-secrets repo,\n and the Gerrit Change-Id hook lives in the gerrit repo, and each project's\n pre-commit formatting hooks live in their own projects' repos).\n 4. Users who have \"bought in\" to the config-based hook management can ask to be\n warned if a hook is run from .git/hooks, and can turn off running hooks from\n .git/hooks entirely if they wish.\n 5. By defaulting to hook.runHookDir=hookdir-first or hookdir-only, we can avoid\n breaking users who are using .git/hooks/ today. The former gives value-add -\n the user can start using config hooks seamlessly - while the latter gives\n old functionality only.\n\nThe other benefits I can think of:\n - Having a friendly porcelain command to manage hooks makes life easier for the\n Git and Unix uninitiated.\n - If 'git hooks edit' UI looks like 'git rebase --interactive' UI, then users\n may already feel at home using it.\n - If organizations are already distributing global configs, they can also\n distribute hooks such as git-secrets or other internal hooks if they wish.\n - Users who want to use a hook that comes with a repo may still be subject to\n attack if the hook shipped by the repo changes under their feet (although\n they have explicitly asked Git to run that hook, so the threat doesn't feel\n different from running a shell script from a repo manually).\n\nThe drawbacks I can think of:\n - It is a much more complicated approach than adding .git/hooks/${hookname}.d.\n - The config syntax can clutter the output if users want to examine their\n config with 'git config --list'.\n - It is still possible for someone to attack via a zip file if they manipulate\n .git/config to use a hook they specified in the repo.\n\nThe issue with .git/config is still a pretty big one, but my own thought is that\nwe should treat it as a separate piece. Yes, those kinds of attacks are still\npossible as long as either .git/config or .git/hooks are present in the repo\ndirectory. But coding around .git/config will lead to some cruft that won't make\nsense once repo-local config files are made more secure (for example, one\nsolution might be to ignore local and worktree configs when evaluating the\nhooks list, which would be inconvenient and no longer make sense if those config\nscopes are secured).\n\nI look forward to the discussion. Thanks for your thoughts.\n - Emily\n"},{"id":"386362","messageId":"20191116054501.GA6538@camp.crustytoothpaste.net","threadId":"52276","inReplyTo":"20191116011125.GG22855@google.com","subject":"Re: [RFC] Hook management via 'git hooks' command","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2019-11-16T05:45:01Z","receivedAt":"2019-11-16T05:45:24Z","isPatch":false,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2019-11-16 at 01:11:25, Emily Shaffer wrote:\n> Here's my suggestion.\n> \n> - Let configs handle the hook list and ordering, via 'hook.{hookname}' which\n> can be specified more than once.\n> - global config: hook.update = /path/to/firsthook\n> user config: hook.update = /path/to/secondhook\n> worktree config: hook.update = -/path/to/firsthook #eliminate the firsthook\n> call\n> - global config: hook.update = /path/to/firsthook\n> repo config: hook.update = /path/to/secondhook\n> repo config: hook.update = ^/path/to/firsthook #move firsthook execution\n> after secondhook for this project\n\nI'd like to hear more about how we handle multiple hooks that are\nrepo-specific and don't live in the PATH. This is a common situation\nfor existing tools that handle multiple hooks, and it's one I think we\nshould support.\n\nPerhaps we add a \"git hook execute\" subcommand that executes scripts\nfrom the hook directory.\n\n> - Let users decide whether they want to check core.hookDir in addition to their\n> config, instead of their config, or not at all, via a config\n> 'hook.runHookDir' \"hookdir-first\", \"hookdir-only\", \"config-only\", etc.\n> - Let users ask to be notified if running a hook from .git/hooks via a config\n> 'hook.warnHookDir'. (Mentioning .git/hooks rather than core.hookDir is\n> intentional here.) Alternatively, ask for 'hook.warn' with values \"hookdir\",\n> \"all\" depending on how trusting the user feels.\n> - If we want to phase out .git/hooks, we can make this notification opt-out\n> instead of opt-in, someday.\n> - between runHookDir and warnHookDir, users are able to phase out\n> .git/hooks scripts at their own pace.\n> - Abstract most of this via a 'git hooks' command.\n> - 'git hooks add [--] ' to append another hook;\n> let the scope setting match 'git config'.\n> - 'git hooks show []' to indicate which hooks will be run, either\n> on a specific event or for all events, and in which order, as well as each\n> hook's config origin\n> - 'git hooks edit ' to modify the hook order, or choose not to run\n> a hook locally\n> - By managing it through a command, we can reorder the contents of config\n> files if it makes sense to do so.\n> - Add a hook library.\n> - Optionally specify all hook events via enum, so we aren't string-typing\n> calls to find_hook() anymore.\n> - Resolve configs into a list of hooks to run by concatenating them together\n> in config precedence order.\n> - Also allow configs formatted like \"-\" to remove an\n> earlier-configured invocation of that hook, or \"^\" to move the\n> earlier-configured invocation to this point in the execution order\n> - Move the find_hook() implementation to here, to account for the multihook\n> ordering\n\nI think this addresses most of the concerns that I had about ordering.\nIt is still a little suboptimal that we're relying on the ordering of\nthe config file, since it makes things equivalent to numbered files in\n.d directories hard.\n\nPossibly as an alternative to the ^ syntax, we could make the hook value\nbe of the form \"key program\", where key is a sort key (e.g., a number)\nand program is the program to run. We pick normal config file ordering\nif the keys are identical. Then if the system config wants to have a\nhook that always runs at the end, it can do so easily.\n\nIn addition, we should be sure that attempting to remove a hook which\ndoesn't exist isn't an error, since a user might want to set their\n~/.gitconfig file to always unset a global hook that may or may not\nexist.\n\nWe also need to address exit codes with multiple hooks and whether we\nfail fast or not. My series may provide some valuable options here, or\nwe may choose to go with a single, simpler approach. Whatever we do, we\nshould document the behavior clearly.\n\nOverall, though, I think this approach has a lot of potential and I feel\npositive about it. Thanks for bringing this up again in a productive\nand helpful way.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"386473","messageId":"20191118223819.GI22855@google.com","threadId":"52276","inReplyTo":"20191116054501.GA6538@camp.crustytoothpaste.net","subject":"Re: [RFC] Hook management via 'git hooks' command","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2019-11-18T22:38:19Z","receivedAt":"2019-11-18T22:38:33Z","isPatch":false,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"On Sat, Nov 16, 2019 at 05:45:01AM +0000, brian m. carlson wrote:\n> On 2019-11-16 at 01:11:25, Emily Shaffer wrote:\n> > Here's my suggestion.\n> > \n> > - Let configs handle the hook list and ordering, via 'hook.{hookname}' which\n> > can be specified more than once.\n> > - global config: hook.update = /path/to/firsthook\n> > user config: hook.update = /path/to/secondhook\n> > worktree config: hook.update = -/path/to/firsthook #eliminate the firsthook\n> > call\n> > - global config: hook.update = /path/to/firsthook\n> > repo config: hook.update = /path/to/secondhook\n> > repo config: hook.update = ^/path/to/firsthook #move firsthook execution\n> > after secondhook for this project\n> \n> I'd like to hear more about how we handle multiple hooks that are\n> repo-specific and don't live in the PATH. This is a common situation\n> for existing tools that handle multiple hooks, and it's one I think we\n> should support.\n\nI guess I'm confused about where PATH comes into play. Do you mean that\nthe hook being run relies on PATH to be set appropriately? I had\nenvisioned absolute paths in the config.\n\n> Perhaps we add a \"git hook execute\" subcommand that executes scripts\n> from the hook directory.\n\nCan you give an example of when you'd use it? I'm not understanding your\nconcern and I think an example use case would help me see what you mean.\n\n> \n> > - Let users decide whether they want to check core.hookDir in addition to their\n> > config, instead of their config, or not at all, via a config\n> > 'hook.runHookDir' \"hookdir-first\", \"hookdir-only\", \"config-only\", etc.\n> > - Let users ask to be notified if running a hook from .git/hooks via a config\n> > 'hook.warnHookDir'. (Mentioning .git/hooks rather than core.hookDir is\n> > intentional here.) Alternatively, ask for 'hook.warn' with values \"hookdir\",\n> > \"all\" depending on how trusting the user feels.\n> > - If we want to phase out .git/hooks, we can make this notification opt-out\n> > instead of opt-in, someday.\n> > - between runHookDir and warnHookDir, users are able to phase out\n> > .git/hooks scripts at their own pace.\n> > - Abstract most of this via a 'git hooks' command.\n> > - 'git hooks add [--] ' to append another hook;\n> > let the scope setting match 'git config'.\n> > - 'git hooks show []' to indicate which hooks will be run, either\n> > on a specific event or for all events, and in which order, as well as each\n> > hook's config origin\n> > - 'git hooks edit ' to modify the hook order, or choose not to run\n> > a hook locally\n> > - By managing it through a command, we can reorder the contents of config\n> > files if it makes sense to do so.\n> > - Add a hook library.\n> > - Optionally specify all hook events via enum, so we aren't string-typing\n> > calls to find_hook() anymore.\n> > - Resolve configs into a list of hooks to run by concatenating them together\n> > in config precedence order.\n> > - Also allow configs formatted like \"-\" to remove an\n> > earlier-configured invocation of that hook, or \"^\" to move the\n> > earlier-configured invocation to this point in the execution order\n> > - Move the find_hook() implementation to here, to account for the multihook\n> > ordering\n> \n> I think this addresses most of the concerns that I had about ordering.\n> It is still a little suboptimal that we're relying on the ordering of\n> the config file, since it makes things equivalent to numbered files in\n> .d directories hard.\n\nHm, I suppose I don't see why, if the final ordering is determined by\nthe .git/config (or future replacement for that). Can you explain what\nyou mean? I want to understand where you're coming from.\n\n> \n> Possibly as an alternative to the ^ syntax, we could make the hook value\n> be of the form \"key program\", where key is a sort key (e.g., a number)\n> and program is the program to run. We pick normal config file ordering\n> if the keys are identical. Then if the system config wants to have a\n> hook that always runs at the end, it can do so easily.\n\nInteresting. This way if you decide after you've set up all your configs\njust so that you really want something to run at the end of the update\nevent, you can change one place, not n=number of Git repos. (I do still\nwant to be able to say \"don't run that global hook in this project\"\nthough.)\n\n> \n> In addition, we should be sure that attempting to remove a hook which\n> doesn't exist isn't an error, since a user might want to set their\n> ~/.gitconfig file to always unset a global hook that may or may not\n> exist.\n\nI'd be comfortable with a warning when exiting 'git hook edit' mode and\nsilence when actually running the hook list.\n\n> \n> We also need to address exit codes with multiple hooks and whether we\n> fail fast or not. My series may provide some valuable options here, or\n> we may choose to go with a single, simpler approach. Whatever we do, we\n> should document the behavior clearly.\n\nAgree. I'll take a look at your series this week to see how you solved\nit; this feels like a place where it'd be easy to give too many knobs\nthat users would never pull.\n\nI was going to write my gut feeling about what we should do, but my gut\ncame up with reasonable arguments for lots of approaches. So I'll read\nwhat came first before I say anything more :)\n\n> \n> Overall, though, I think this approach has a lot of potential and I feel\n> positive about it. Thanks for bringing this up again in a productive\n> and helpful way.\n\nThanks for being open minded about it.\n\n - Emily\n"},{"id":"386488","messageId":"20191119005136.GA6430@camp.crustytoothpaste.net","threadId":"52276","inReplyTo":"20191118223819.GI22855@google.com","subject":"Re: [RFC] Hook management via 'git hooks' command","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2019-11-19T00:51:36Z","receivedAt":"2019-11-19T00:51:51Z","isPatch":false,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2019-11-18 at 22:38:19, Emily Shaffer wrote:\n> On Sat, Nov 16, 2019 at 05:45:01AM +0000, brian m. carlson wrote:\n> > On 2019-11-16 at 01:11:25, Emily Shaffer wrote:\n> > > Here's my suggestion.\n> > > \n> > > - Let configs handle the hook list and ordering, via 'hook.{hookname}' which\n> > > can be specified more than once.\n> > > - global config: hook.update = /path/to/firsthook\n> > > user config: hook.update = /path/to/secondhook\n> > > worktree config: hook.update = -/path/to/firsthook #eliminate the firsthook\n> > > call\n> > > - global config: hook.update = /path/to/firsthook\n> > > repo config: hook.update = /path/to/secondhook\n> > > repo config: hook.update = ^/path/to/firsthook #move firsthook execution\n> > > after secondhook for this project\n> > \n> > I'd like to hear more about how we handle multiple hooks that are\n> > repo-specific and don't live in the PATH. This is a common situation\n> > for existing tools that handle multiple hooks, and it's one I think we\n> > should support.\n> \n> I guess I'm confused about where PATH comes into play. Do you mean that\n> the hook being run relies on PATH to be set appropriately? I had\n> envisioned absolute paths in the config.\n\nIn past discussions, there's been an assumption that hooks in the config\nwill be found in PATH if they're not specified explicitly, and I assumed\n(apparently incorrectly) that the same would be true here.\n\nI do expect folks are going to want to use non-absolute paths, though.\nIf I'm invoking the git binary in a hook, I don't care whether it exists\nin /usr/bin, /usr/local/bin, ~/bin, or somewhere else entirely. That's\nmy shell's problem to figure out.\n\nIt's also common for folks to use something like \"bundle exec\" in a hook\nto run a linter that's installed by the local package manager, and in\norder to do that, you have to honor PATH to find the package manager's\nbinary. That could be in a variety of places, and it could end up\nchanging dynamically in a session due to a tool like RVM.\n\n> > Perhaps we add a \"git hook execute\" subcommand that executes scripts\n> > from the hook directory.\n> \n> Can you give an example of when you'd use it? I'm not understanding your\n> concern and I think an example use case would help me see what you mean.\n\nSure. Currently, if I have pre-push hook, it lives in\n.git/hooks/pre-push. Now, I want to have multiple hooks for that which\nare specific to my repo. Maybe I've stuffed them into\n.git/hooks/pre-push.d/hook1 and .git/hooks/pre-push.d/hook2, since\nthat's where my previous hook management system put them, but I now want\nto use those same hooks with the config style and drop the old tool.\n\nI'd like to use \"git hook execute pre-push.d/hook1\" and \"git hook\nexecute pre-push.d/hook2\" to automatically find the right hooks and\ninvoke them. Similarly, I could use \"git hook execute pre-push\" to\nexecute the old pre-push hook.\n\nI suppose if we continue to keep the existing behavior of changing the\ndirectory and we pass the config options to the shell, then we could\njust write \"$(git config core.hooksPath || echo\n.git/hooks)/pre-push.d/hook1\" instead, which, while ugly, gets the job\ndone. Then we wouldn't need such a command.\n\n> > I think this addresses most of the concerns that I had about ordering.\n> > It is still a little suboptimal that we're relying on the ordering of\n> > the config file, since it makes things equivalent to numbered files in\n> > .d directories hard.\n> \n> Hm, I suppose I don't see why, if the final ordering is determined by\n> the .git/config (or future replacement for that). Can you explain what\n> you mean? I want to understand where you're coming from.\n\nOne of the benefits to using numbered files in a .d directory is that\nyou can explicitly control ordering of operations. For example, maybe I\nhave a per-repo pre-push hook that performs some checks and rejects a\npush if something is off. I also have a pre-push hook for Git LFS that\npushes the Git LFS objects to the remote server if Git LFS is in use.\n\nIn this case, I'd always want my sanity-check hook to run first, and so\nI'd number it first. This is fine if both are per-repo, but if the LFS\nhook is global, then it's in the wrong order and my LFS objects are\npushed even though my sanity check failed.\n\n> > Possibly as an alternative to the ^ syntax, we could make the hook value\n> > be of the form \"key program\", where key is a sort key (e.g., a number)\n> > and program is the program to run. We pick normal config file ordering\n> > if the keys are identical. Then if the system config wants to have a\n> > hook that always runs at the end, it can do so easily.\n> \n> Interesting. This way if you decide after you've set up all your configs\n> just so that you really want something to run at the end of the update\n> event, you can change one place, not n=number of Git repos. (I do still\n> want to be able to say \"don't run that global hook in this project\"\n> though.)\n\nExactly. A global or per-user commit-msg hook may want to see the final\nmessage before approving or rejecting it, and that wouldn't be possible\nwithout some sort of ordering.\n\nI strongly agree that we should still allow removing higher-level hooks.\n\n> > In addition, we should be sure that attempting to remove a hook which\n> > doesn't exist isn't an error, since a user might want to set their\n> > ~/.gitconfig file to always unset a global hook that may or may not\n> > exist.\n> \n> I'd be comfortable with a warning when exiting 'git hook edit' mode and\n> silence when actually running the hook list.\n\nYeah, that's what I'm going for.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"386886","messageId":"20191123011924.GC101478@google.com","threadId":"52276","inReplyTo":"20191119005136.GA6430@camp.crustytoothpaste.net","subject":"Re: [RFC] Hook management via 'git hooks' command","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2019-11-23T01:19:24Z","receivedAt":"2019-11-23T01:19:33Z","isPatch":false,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"On Tue, Nov 19, 2019 at 12:51:36AM +0000, brian m. carlson wrote:\n> On 2019-11-18 at 22:38:19, Emily Shaffer wrote:\n> > On Sat, Nov 16, 2019 at 05:45:01AM +0000, brian m. carlson wrote:\n> > > On 2019-11-16 at 01:11:25, Emily Shaffer wrote:\n> > > > Here's my suggestion.\n> > > > \n> > > > - Let configs handle the hook list and ordering, via 'hook.{hookname}' which\n> > > > can be specified more than once.\n> > > > - global config: hook.update = /path/to/firsthook\n> > > > user config: hook.update = /path/to/secondhook\n> > > > worktree config: hook.update = -/path/to/firsthook #eliminate the firsthook\n> > > > call\n> > > > - global config: hook.update = /path/to/firsthook\n> > > > repo config: hook.update = /path/to/secondhook\n> > > > repo config: hook.update = ^/path/to/firsthook #move firsthook execution\n> > > > after secondhook for this project\n> > > \n> > > I'd like to hear more about how we handle multiple hooks that are\n> > > repo-specific and don't live in the PATH. This is a common situation\n> > > for existing tools that handle multiple hooks, and it's one I think we\n> > > should support.\n> > \n> > I guess I'm confused about where PATH comes into play. Do you mean that\n> > the hook being run relies on PATH to be set appropriately? I had\n> > envisioned absolute paths in the config.\n> \n> In past discussions, there's been an assumption that hooks in the config\n> will be found in PATH if they're not specified explicitly, and I assumed\n> (apparently incorrectly) that the same would be true here.\n\nAh, I think I see what you mean.\n\nhook.update = security-heuristic-runner\n\nwhere \"security-heuristic-runner\" is some compiled binary your employer\npurchased from some vendor and distributed directly to your `/bin/`.\n\nNo, I had imagined users would achieve that by writing:\n\nhook.update = /bin/security-heuristic-runner\n\n\n> I do expect folks are going to want to use non-absolute paths, though.\n> If I'm invoking the git binary in a hook, I don't care whether it exists\n> in /usr/bin, /usr/local/bin, ~/bin, or somewhere else entirely. That's\n> my shell's problem to figure out.\n\nHm. Do you mean:\n\nhook.update = git grep \"something\"\n\nOr do you mean:\n\nhook.update = ~/grephook.sh\n\ngrephook.sh:\n #!/bin/bash\n\n git grep \"something\" >output\n ... do something with output ...\n\nI suppose I need to understand better how $PATH works with the latter\nscenario, but my gut says \"if you didn't worry about where to find the\nGit binary from your script before, why are you going to start caring\nnow\".\n\nThis led me to wonder: \"Should we allow someone to pass arguments via\nthe hook config?\" Or to put it another way, \"Should we allow 'hook.update\n= grep -Rin blahblah >audit.log'?\" I think the answer is no - some hooks\ndo expect to be given arguments, for example\nsequencer.c:run_prepare_commit_msg_hook().\n\n> \n> It's also common for folks to use something like \"bundle exec\" in a hook\n> to run a linter that's installed by the local package manager, and in\n> order to do that, you have to honor PATH to find the package manager's\n> binary. That could be in a variety of places, and it could end up\n> changing dynamically in a session due to a tool like RVM.\n\nI imagine I'm missing something crucial here about why this isn't an\nissue in today's hook implementation, but it would be an issue with a\nconfig-based hook lookup. I don't think I see why the invocation would\nbe any different, except today find_hook() says \"is it in\n$GITDIR/hooks/\" and tomorrow find_hook() would say \"is it in config\".\n\n> \n> > > Perhaps we add a \"git hook execute\" subcommand that executes scripts\n> > > from the hook directory.\n> > \n> > Can you give an example of when you'd use it? I'm not understanding your\n> > concern and I think an example use case would help me see what you mean.\n> \n> Sure. Currently, if I have pre-push hook, it lives in\n> .git/hooks/pre-push. Now, I want to have multiple hooks for that which\n> are specific to my repo. Maybe I've stuffed them into\n> .git/hooks/pre-push.d/hook1 and .git/hooks/pre-push.d/hook2, since\n> that's where my previous hook management system put them, but I now want\n> to use those same hooks with the config style and drop the old tool.\n> \n> I'd like to use \"git hook execute pre-push.d/hook1\" and \"git hook\n> execute pre-push.d/hook2\" to automatically find the right hooks and\n> invoke them. Similarly, I could use \"git hook execute pre-push\" to\n> execute the old pre-push hook.\n\nHmmm. I think you're describing a scenario like this:\n\n1. I make .git/hooks/pre-push.d/hook1, .git/hooks/pre-push.d/hook2, and\n .git/hooks/pre-push.\n2. Hook magic happens upstream, and now instead of living in\n .git/hooks/, my hooks live in ~/.githooks//.\n3. I just want to run my hooks now, but where are they?\n\nI don't envision a Git-facilitated \"mv .git/hooks/\n~/.githooks//\" (or whatever). In fact, I think it could break\nyour scenario, because maybe you write \"pre-push\" to reference hook1 and\nhook2 by absolute path, or by relative path to the repo directory. I\ndon't like the idea of doing that very much at all, which is why I\nproposed hook.warnHookDir (or whatever it is I called it), which we can\nlater turn on by default if/when we want to chase people off using\n.git/hooks/.\n\nTo your point, though. The most hands-off way to keep your previous\nsetup would be to add:\n\n hook.pre-push = /path/to/myrepo/.git/hooks/pre-push\n\nwhich would invoke your springboard script and let you do whatever you\nwant with the contents of .git/hooks/pre-push.d/. The second most\nhands-off (but more correct) way would be to add:\n\n hook.pre-push = /path/to/myrepo/.git/hooks/pre-push.d/hook1\n hook.pre-push = /path/to/myrepo/.git/hooks/pre-push.d/hook2\n\nI think we may be talking past each other, because when I hear you\ndescribe what you want from 'git hook execute <...>', it sounds like you\nare asking for 'git hook' to just do:\n\n $ $(git config core.hookdir)/$3\n\nand that confuses me because I'm having trouble figuring out why you\ncouldn't just do that yourself. So I'm sure I'm still not understanding\nsomething correctly.\n\nThis is kind of the use case I was hoping to account for by providing\n'hook.runHookDir' - hooks in .git/hooks/ still \"just work\", by taking\nthe contents of that dir into account while composing the list of hooks\nto run.\n\n> \n> I suppose if we continue to keep the existing behavior of changing the\n> directory and we pass the config options to the shell, then we could\n> just write \"$(git config core.hooksPath || echo\n> .git/hooks)/pre-push.d/hook1\" instead, which, while ugly, gets the job\n> done. Then we wouldn't need such a command.\n\nYeah, I am wondering about when you want to run a hook generically (i.e.\nfrom a noninteractive script) but outside of the context of something in\nthe Git binary invoking a hook. Are you thinking of Git commands\nimplemented as scripts?\n\nI wonder if the script use case is better served by something like 'git\nhook list pre-push --porcelain' which could compose the full list of\nhooks to run, taking into account hook.runHookDir and provide them in a\nscript-readable way. Or, a simplification on your suggestion: 'git hook\nexecute pre-push\" which does the same thing, but runs them all for you\ntoo.\n\n> > > I think this addresses most of the concerns that I had about ordering.\n> > > It is still a little suboptimal that we're relying on the ordering of\n> > > the config file, since it makes things equivalent to numbered files in\n> > > .d directories hard.\n> > \n> > Hm, I suppose I don't see why, if the final ordering is determined by\n> > the .git/config (or future replacement for that). Can you explain what\n> > you mean? I want to understand where you're coming from.\n> \n> One of the benefits to using numbered files in a .d directory is that\n> you can explicitly control ordering of operations. For example, maybe I\n> have a per-repo pre-push hook that performs some checks and rejects a\n> push if something is off. I also have a pre-push hook for Git LFS that\n> pushes the Git LFS objects to the remote server if Git LFS is in use.\n> \n> In this case, I'd always want my sanity-check hook to run first, and so\n> I'd number it first. This is fine if both are per-repo, but if the LFS\n> hook is global, then it's in the wrong order and my LFS objects are\n> pushed even though my sanity check failed.\n\nYeah, this is really compelling, and also removes the somewhat wonky ^\nproposed just below here. I like this idea quite a lot:\n\nhook.pre-push = 001:/path/to/sanity-checker\n\nI'll have to ponder on the UX of a 'git hook'-facilitated interactive\nedit of the hook numbering, though. UX is not my strong point :)\n\n> \n> > > Possibly as an alternative to the ^ syntax, we could make the hook value\n> > > be of the form \"key program\", where key is a sort key (e.g., a number)\n> > > and program is the program to run. We pick normal config file ordering\n> > > if the keys are identical. Then if the system config wants to have a\n> > > hook that always runs at the end, it can do so easily.\n> > \n> > Interesting. This way if you decide after you've set up all your configs\n> > just so that you really want something to run at the end of the update\n> > event, you can change one place, not n=number of Git repos. (I do still\n> > want to be able to say \"don't run that global hook in this project\"\n> > though.)\n> \n> Exactly. A global or per-user commit-msg hook may want to see the final\n> message before approving or rejecting it, and that wouldn't be possible\n> without some sort of ordering.\n> \n> I strongly agree that we should still allow removing higher-level hooks.\n> \n> > > In addition, we should be sure that attempting to remove a hook which\n> > > doesn't exist isn't an error, since a user might want to set their\n> > > ~/.gitconfig file to always unset a global hook that may or may not\n> > > exist.\n> > \n> > I'd be comfortable with a warning when exiting 'git hook edit' mode and\n> > silence when actually running the hook list.\n> \n> Yeah, that's what I'm going for.\n> -- \n> brian m. carlson: Houston, Texas, US\n> OpenPGP: https://keybase.io/bk2204\n\nThanks for being patient as I wrapped my head around this enough to\nreply.\n\n - Emily\n"},{"id":"386967","messageId":"20191125030445.GB2404748@camp.crustytoothpaste.net","threadId":"52276","inReplyTo":"20191123011924.GC101478@google.com","subject":"Re: [RFC] Hook management via 'git hooks' command","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2019-11-25T03:04:45Z","receivedAt":"2019-11-25T03:05:03Z","isPatch":false,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2019-11-23 at 01:19:24, Emily Shaffer wrote:\n> Ah, I think I see what you mean.\n> \n> hook.update = security-heuristic-runner\n> \n> where \"security-heuristic-runner\" is some compiled binary your employer\n> purchased from some vendor and distributed directly to your `/bin/`.\n> \n> No, I had imagined users would achieve that by writing:\n> \n> hook.update = /bin/security-heuristic-runner\n\nYeah, that's what I'm looking for. The problem is that, for example,\nDebian does not guarantee where in PATH a file is. Having a newer Git\non RHEL or CentOS systems often involves hacking PATH and\nLD_LIBRARY_PATH.\n\n> Hm. Do you mean:\n> \n> hook.update = git grep \"something\"\n> \n> Or do you mean:\n> \n> hook.update = ~/grephook.sh\n> \n> grephook.sh:\n> #!/bin/bash\n> \n> git grep \"something\" >output\n> ... do something with output ...\n\nI had intended to include the latter case, but also allow valid hooks\nwith multiple argument support. For example, you could invoke \"git lfs\npre-push\" directly in your hook, and that is a fully functioning\npre-push hook, and would require a suitable PATH lookup to find your Git\nbinary. It accepts the additional arguments that pre-push hooks accept;\nright now we basically do the following instead:\n\n----\n#!/bin/sh\n\nexec git lfs pre-push \"$@\"\n----\n\n> I suppose I need to understand better how $PATH works with the latter\n> scenario, but my gut says \"if you didn't worry about where to find the\n> Git binary from your script before, why are you going to start caring\n> now\".\n> \n> This led me to wonder: \"Should we allow someone to pass arguments via\n> the hook config?\" Or to put it another way, \"Should we allow 'hook.update\n> = grep -Rin blahblah >audit.log'?\" I think the answer is no - some hooks\n> do expect to be given arguments, for example\n> sequencer.c:run_prepare_commit_msg_hook().\n\nI think what we want to do in this case is just invoke things in the\nshell with extra arguments, like we do with editors. This means we\ndon't have to handle PATH or anything else; we just invoke the shell and\nlet it handle it. That lets people provide multi-call binaries (like\ngit lfs) that include hooks inside them.\n\nI do, however, think we should require folks to have a suitable hook\nthat accepts the right arguments. So \"git grep blahblah\" isn't a valid\nhook in most cases, because it doesn't take the right arguments and read\nthe right data from stdin if necessary.\n\n> > I suppose if we continue to keep the existing behavior of changing the\n> > directory and we pass the config options to the shell, then we could\n> > just write \"$(git config core.hooksPath || echo\n> > .git/hooks)/pre-push.d/hook1\" instead, which, while ugly, gets the job\n> > done. Then we wouldn't need such a command.\n> \n> Yeah, I am wondering about when you want to run a hook generically (i.e.\n> from a noninteractive script) but outside of the context of something in\n> the Git binary invoking a hook. Are you thinking of Git commands\n> implemented as scripts?\n\nI'm just thinking about existing hook wrappers that invoke multiple\nscripts at the moment, something like how\nhttps://gist.github.com/mjackson/7e602a7aa357cfe37dadcc016710931b works\nat the moment and how we'd replace that with a config-based model.\n\nI think using the shell avoids the entire proposal, because it then\nbecomes trivial to script that in the command and port it over, since we\ncan use my ugly hack above. I think I like that better than \"git hook\nexecute\" because it's a little more flexible.\n\n> > One of the benefits to using numbered files in a .d directory is that\n> > you can explicitly control ordering of operations. For example, maybe I\n> > have a per-repo pre-push hook that performs some checks and rejects a\n> > push if something is off. I also have a pre-push hook for Git LFS that\n> > pushes the Git LFS objects to the remote server if Git LFS is in use.\n> > \n> > In this case, I'd always want my sanity-check hook to run first, and so\n> > I'd number it first. This is fine if both are per-repo, but if the LFS\n> > hook is global, then it's in the wrong order and my LFS objects are\n> > pushed even though my sanity check failed.\n> \n> Yeah, this is really compelling, and also removes the somewhat wonky ^\n> proposed just below here. I like this idea quite a lot:\n> \n> hook.pre-push = 001:/path/to/sanity-checker\n\nI think a colon is actually better than my proposal for a space in this\nregard, but I'm not picky: anything unambiguous is fine.\n\n> I'll have to ponder on the UX of a 'git hook'-facilitated interactive\n> edit of the hook numbering, though. UX is not my strong point :)\n\nI also find I'm not great at UX, so I can't be of much help.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"387020","messageId":"20191125222113.GA83137@google.com","threadId":"52276","inReplyTo":"20191125030445.GB2404748@camp.crustytoothpaste.net","subject":"Re: [RFC] Hook management via 'git hooks' command","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2019-11-25T22:21:13Z","receivedAt":"2019-11-25T22:21:23Z","isPatch":false,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"On Mon, Nov 25, 2019 at 03:04:45AM +0000, brian m. carlson wrote:\n> On 2019-11-23 at 01:19:24, Emily Shaffer wrote:\n> > Ah, I think I see what you mean.\n> > \n> > hook.update = security-heuristic-runner\n> > \n> > where \"security-heuristic-runner\" is some compiled binary your employer\n> > purchased from some vendor and distributed directly to your `/bin/`.\n> > \n> > No, I had imagined users would achieve that by writing:\n> > \n> > hook.update = /bin/security-heuristic-runner\n> \n> Yeah, that's what I'm looking for. The problem is that, for example,\n> Debian does not guarantee where in PATH a file is. Having a newer Git\n> on RHEL or CentOS systems often involves hacking PATH and\n> LD_LIBRARY_PATH.\n> \n> > Hm. Do you mean:\n> > \n> > hook.update = git grep \"something\"\n> > \n> > Or do you mean:\n> > \n> > hook.update = ~/grephook.sh\n> > \n> > grephook.sh:\n> > #!/bin/bash\n> > \n> > git grep \"something\" >output\n> > ... do something with output ...\n> \n> I had intended to include the latter case, but also allow valid hooks\n> with multiple argument support. For example, you could invoke \"git lfs\n> pre-push\" directly in your hook, and that is a fully functioning\n> pre-push hook, and would require a suitable PATH lookup to find your Git\n> binary. It accepts the additional arguments that pre-push hooks accept;\n> right now we basically do the following instead:\n> \n> ----\n> #!/bin/sh\n> \n> exec git lfs pre-push \"$@\"\n> ----\n> \n> > I suppose I need to understand better how $PATH works with the latter\n> > scenario, but my gut says \"if you didn't worry about where to find the\n> > Git binary from your script before, why are you going to start caring\n> > now\".\n> > \n> > This led me to wonder: \"Should we allow someone to pass arguments via\n> > the hook config?\" Or to put it another way, \"Should we allow 'hook.update\n> > = grep -Rin blahblah >audit.log'?\" I think the answer is no - some hooks\n> > do expect to be given arguments, for example\n> > sequencer.c:run_prepare_commit_msg_hook().\n> \n> I think what we want to do in this case is just invoke things in the\n> shell with extra arguments, like we do with editors. This means we\n> don't have to handle PATH or anything else; we just invoke the shell and\n> let it handle it. That lets people provide multi-call binaries (like\n> git lfs) that include hooks inside them.\n\nI think that you are saying we should do the nice equivalent of:\n\n system(\"git lfs pre-push\");\n\nand tack the args onto the end. (I suppose that's run-command.h, but I'm\ntrying to use a shorthand that is easy to understand.)\n\nIt seems like this would solve the PATH issue, yes. However, I feel\ntentative because of pushback on that approach in the bugreport review.\nHmmm. I think this is different, because the user understands that the\nhook they configure themselves will be invoked on the shell of their\nchoosing. That is, I think run-command.h with \"C:\\myhook.exe\" would\nstill work, no?\n\nWill this approach \"just work\" for Windows, et al.?\n\n> I do, however, think we should require folks to have a suitable hook\n> that accepts the right arguments. So \"git grep blahblah\" isn't a valid\n> hook in most cases, because it doesn't take the right arguments and read\n> the right data from stdin if necessary.\n\nI'm not sure how we would guarantee that. Are you suggesting we should\ntry dry running a hook somehow when it's added with 'git hook add'? Even\ndoing that won't stop someone from popping open ~/.gitconfig with nano\nand adding their hook that doesn't take the right args that way.\n\n> \n> > > I suppose if we continue to keep the existing behavior of changing the\n> > > directory and we pass the config options to the shell, then we could\n> > > just write \"$(git config core.hooksPath || echo\n> > > .git/hooks)/pre-push.d/hook1\" instead, which, while ugly, gets the job\n> > > done. Then we wouldn't need such a command.\n> > \n> > Yeah, I am wondering about when you want to run a hook generically (i.e.\n> > from a noninteractive script) but outside of the context of something in\n> > the Git binary invoking a hook. Are you thinking of Git commands\n> > implemented as scripts?\n> \n> I'm just thinking about existing hook wrappers that invoke multiple\n> scripts at the moment, something like how\n> https://gist.github.com/mjackson/7e602a7aa357cfe37dadcc016710931b works\n> at the moment and how we'd replace that with a config-based model.\n> \n> I think using the shell avoids the entire proposal, because it then\n> becomes trivial to script that in the command and port it over, since we\n> can use my ugly hack above.\n\nStill not sure I understand what the issue was before. Is it that the\ntrampoline script needs to know $PATH to be able to invoke the child\nhooks in hookname.d? Or it's because the current working directory\nisn't clear, so a relative path in the trampoline script may not be\nresolved well?\n\n> I think I like that better than \"git hook\n> execute\" because it's a little more flexible.\n> \n> > > One of the benefits to using numbered files in a .d directory is that\n> > > you can explicitly control ordering of operations. For example, maybe I\n> > > have a per-repo pre-push hook that performs some checks and rejects a\n> > > push if something is off. I also have a pre-push hook for Git LFS that\n> > > pushes the Git LFS objects to the remote server if Git LFS is in use.\n> > > \n> > > In this case, I'd always want my sanity-check hook to run first, and so\n> > > I'd number it first. This is fine if both are per-repo, but if the LFS\n> > > hook is global, then it's in the wrong order and my LFS objects are\n> > > pushed even though my sanity check failed.\n> > \n> > Yeah, this is really compelling, and also removes the somewhat wonky ^\n> > proposed just below here. I like this idea quite a lot:\n> > \n> > hook.pre-push = 001:/path/to/sanity-checker\n> \n> I think a colon is actually better than my proposal for a space in this\n> regard, but I'm not picky: anything unambiguous is fine.\n\nYeah, same. I'm a little worried about a colon because there was some\nconversation about being able to pick a hook in a repo from a specific\nref (i.e. I want to run 'update' hook from 'master' all the time even\nwhen I'm examining 'third-party-topic' branch or bisecting\n'nasty-bug-branch') and the colon seems to be used handily for ref +\npath, but I think these details will work themselves out during\nimplementation, so I'm not so very worried.\n> \n> > I'll have to ponder on the UX of a 'git hook'-facilitated interactive\n> > edit of the hook numbering, though. UX is not my strong point :)\n> \n> I also find I'm not great at UX, so I can't be of much help.\n\nWe're in luck on my end as it seems we have a UX team with open office\nhours who can give us an opinion. I'll try to meet with them next week\nThursday; thanks Jonathan Nieder for the pointer.\n\n\nThis sounds like we both are pretty close on the same page, so I think I\nwill get started in the coming weeks and see if we can get a mockup to\npick at with the implementation details in front of us.\n\n - Emily\n"},{"id":"387021","messageId":"20191125224533.GB83137@google.com","threadId":"52276","inReplyTo":"20191125222113.GA83137@google.com","subject":"Re: [RFC] Hook management via 'git hooks' command","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2019-11-25T22:45:33Z","receivedAt":"2019-11-25T22:45:41Z","isPatch":false,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"> This sounds like we both are pretty close on the same page, so I think I\n> will get started in the coming weeks and see if we can get a mockup to\n> pick at with the implementation details in front of us.\n\nHm. To elaborate (and partially as a reminder to myself) I will try to\nget it done in the following order:\n\n1. Implement 'git hook list ' which reads all the configs.\n(User would need to manually add the configs at this stage)\n\n2. (maybe) Implement 'git hook execute '. This may or\nmay not be useful; I suppose it would be pretty equivalent to:\n\n $ git hook list | xargs -I% sh % \n\n3. Implement config modifiers like 'git hook add', 'git hook edit' etc.\n\nMy thinking is that we will have a lot of time with 1. in front of us to\nnitpick how we want the config format to look, how the ordering should\ngo, etc. and it will be a fairly simple implementation. It'll also be\n\"usable\" although not in a particularly friendly way in case someone\nwants to try it and see, in a way that the config modifiers by\nthemselves wouldn't be.\n\n - Emily\n"},{"id":"387027","messageId":"20191126002806.GD2404748@camp.crustytoothpaste.net","threadId":"52276","inReplyTo":"20191125222113.GA83137@google.com","subject":"Re: [RFC] Hook management via 'git hooks' command","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2019-11-26T00:28:06Z","receivedAt":"2019-11-26T00:30:12Z","isPatch":false,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2019-11-25 at 22:21:13, Emily Shaffer wrote:\n> I think that you are saying we should do the nice equivalent of:\n> \n> system(\"git lfs pre-push\");\n> \n> and tack the args onto the end. (I suppose that's run-command.h, but I'm\n> trying to use a shorthand that is easy to understand.)\n\nYeah, I'm proposing we run the command using run-command.c with the\nuse_shell flag, which does something very much like that, except a\nlittle bit saner.\n\n> It seems like this would solve the PATH issue, yes. However, I feel\n> tentative because of pushback on that approach in the bugreport review.\n> Hmmm. I think this is different, because the user understands that the\n> hook they configure themselves will be invoked on the shell of their\n> choosing. That is, I think run-command.h with \"C:\\myhook.exe\" would\n> still work, no?\n\nThis will always use /bin/sh, as we do for editors.\n\nbash on Windows does understand the Windows paths and works correctly in\nthis case, AIUI, so that should be fine.\n\n> Will this approach \"just work\" for Windows, et al.?\n\nYes. Windows ships with bash (or in Portable Git, busybox sh), which is\nnot only required to run the testsuite, but required to invoke editors.\n\n> > I do, however, think we should require folks to have a suitable hook\n> > that accepts the right arguments. So \"git grep blahblah\" isn't a valid\n> > hook in most cases, because it doesn't take the right arguments and read\n> > the right data from stdin if necessary.\n> \n> I'm not sure how we would guarantee that. Are you suggesting we should\n> try dry running a hook somehow when it's added with 'git hook add'? Even\n> doing that won't stop someone from popping open ~/.gitconfig with nano\n> and adding their hook that doesn't take the right args that way.\n\nWe don't need to guarantee that. We just need to document it, so that\n(a) people write hooks in the expected way and (b) if people don't, we\ncan point them to the documentation and explain why their hooks don't\nwork. I can see people thinking of this as a set of commands that just\nchecks exit statuses, and there's likely to be confusion.\n\n> > I'm just thinking about existing hook wrappers that invoke multiple\n> > scripts at the moment, something like how\n> > https://gist.github.com/mjackson/7e602a7aa357cfe37dadcc016710931b works\n> > at the moment and how we'd replace that with a config-based model.\n> > \n> > I think using the shell avoids the entire proposal, because it then\n> > becomes trivial to script that in the command and port it over, since we\n> > can use my ugly hack above.\n> \n> Still not sure I understand what the issue was before. Is it that the\n> trampoline script needs to know $PATH to be able to invoke the child\n> hooks in hookname.d? Or it's because the current working directory\n> isn't clear, so a relative path in the trampoline script may not be\n> resolved well?\n\nI think we need to have either (a) a way to explicitly invoke hooks\nunderneath the hook directory or (b) a shell invocation to allow looking\nup the hook directory. People want to have per-repository hooks that\nare not a part of the project, and they need a place to store them,\nwhich is logically the hook directory.\n\nIf we want to allow people to have multiple hooks under something like\n.git/hooks/pre-push.d, then we need to have a way to wire them up in the\nconfiguration using the correct location of the hook directory instead\nof using a helper script like the one I linked above.\n\nWe don't know that the hook directory will necessarily be under\n.git/hooks, so if the user has moved it elsewhere, we'd want to follow\nthat. A relative path would be wrong if the user changed the hook\ndirectory to a different location.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"387031","messageId":"20191126005614.GA251804@google.com","threadId":"52276","inReplyTo":"20191126002806.GD2404748@camp.crustytoothpaste.net","subject":"Re: [RFC] Hook management via 'git hooks' command","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2019-11-26T00:56:14Z","receivedAt":"2019-11-26T00:56:57Z","isPatch":false,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"On Tue, Nov 26, 2019 at 12:28:06AM +0000, brian m. carlson wrote:\n> On 2019-11-25 at 22:21:13, Emily Shaffer wrote:\n> > I think that you are saying we should do the nice equivalent of:\n> > \n> > system(\"git lfs pre-push\");\n> > \n> > and tack the args onto the end. (I suppose that's run-command.h, but I'm\n> > trying to use a shorthand that is easy to understand.)\n> \n> Yeah, I'm proposing we run the command using run-command.c with the\n> use_shell flag, which does something very much like that, except a\n> little bit saner.\n> \n> > It seems like this would solve the PATH issue, yes. However, I feel\n> > tentative because of pushback on that approach in the bugreport review.\n> > Hmmm. I think this is different, because the user understands that the\n> > hook they configure themselves will be invoked on the shell of their\n> > choosing. That is, I think run-command.h with \"C:\\myhook.exe\" would\n> > still work, no?\n> \n> This will always use /bin/sh, as we do for editors.\n> \n> bash on Windows does understand the Windows paths and works correctly in\n> this case, AIUI, so that should be fine.\n> \n> > Will this approach \"just work\" for Windows, et al.?\n> \n> Yes. Windows ships with bash (or in Portable Git, busybox sh), which is\n> not only required to run the testsuite, but required to invoke editors.\n> \n> > > I do, however, think we should require folks to have a suitable hook\n> > > that accepts the right arguments. So \"git grep blahblah\" isn't a valid\n> > > hook in most cases, because it doesn't take the right arguments and read\n> > > the right data from stdin if necessary.\n> > \n> > I'm not sure how we would guarantee that. Are you suggesting we should\n> > try dry running a hook somehow when it's added with 'git hook add'? Even\n> > doing that won't stop someone from popping open ~/.gitconfig with nano\n> > and adding their hook that doesn't take the right args that way.\n> \n> We don't need to guarantee that. We just need to document it, so that\n> (a) people write hooks in the expected way and (b) if people don't, we\n> can point them to the documentation and explain why their hooks don't\n> work. I can see people thinking of this as a set of commands that just\n> checks exit statuses, and there's likely to be confusion.\n> \n> > > I'm just thinking about existing hook wrappers that invoke multiple\n> > > scripts at the moment, something like how\n> > > https://gist.github.com/mjackson/7e602a7aa357cfe37dadcc016710931b works\n> > > at the moment and how we'd replace that with a config-based model.\n> > > \n> > > I think using the shell avoids the entire proposal, because it then\n> > > becomes trivial to script that in the command and port it over, since we\n> > > can use my ugly hack above.\n> > \n> > Still not sure I understand what the issue was before. Is it that the\n> > trampoline script needs to know $PATH to be able to invoke the child\n> > hooks in hookname.d? Or it's because the current working directory\n> > isn't clear, so a relative path in the trampoline script may not be\n> > resolved well?\n> \n> I think we need to have either (a) a way to explicitly invoke hooks\n> underneath the hook directory or (b) a shell invocation to allow looking\n> up the hook directory. People want to have per-repository hooks that\n> are not a part of the project, and they need a place to store them,\n> which is logically the hook directory.\n> \n> If we want to allow people to have multiple hooks under something like\n> .git/hooks/pre-push.d, then we need to have a way to wire them up in the\n> configuration using the correct location of the hook directory instead\n> of using a helper script like the one I linked above.\n> \n> We don't know that the hook directory will necessarily be under\n> .git/hooks, so if the user has moved it elsewhere, we'd want to follow\n> that. A relative path would be wrong if the user changed the hook\n> directory to a different location.\n\nHopefully I am not beating a dead horse here but I really want to\nunderstand. Let me take another guess with examples at what you mean;\nplease correct me!\n\nFor our purposes, let's assume:\n\n.git/hooks/\n update\n update.d/\n update-1\n update-2\n\nupdate:\n #!/bin/bash\n\n ./git/hooks/update.d/update-1 &&\n ./git/hooks/update.d/update-2\n\nThe goal is to make sure update-1 and update-2 are run when other update\nhooks happen.\n\nWith my proposal as is, I see two options:\n\n1)\n.git/config:\n hook.runHookDir = true\n\nThis will run whatever else is in hook.update, and then it will run\n.git/hooks/update. This is not ideal because hookDir could change, and\nthen the content of update will be incorrect:\n\n git config --add core.hookdir=~/hooks/\n mv .git/hooks/update* ~/hooks/\n # call something which invokes update hooks\n # ~/hooks/update fails, .git/hooks/update-1 is gone :(\n\n2)\n.git/config:\n hook.update = 001:/project/path/.git/hooks/update.d/update-1\n hook.update = 002:/project/path/.git/hooks/update.d/update-2\n\nThis will run each update hook individually and have no notion of\nwhether they're in a \"hook dir\" or not. It sees a path, it hands it to\n'sh' to run, it checks the return code.\n\nNow I run 'mv .git/hooks/update* ~/hooks/'. Next time I invoke something\nwhich would run the 'update' hook, it fails, because the path in the\nconfig isn't pointing to anything. But this way is unrelated to hookdir.\n\nIt sounds like you might be asking for something like:\n\n.git/config:\n hook.update = 001:__HOOKDIR__/update.d/update-1\n\nI'm not sure that I like the idea of this. My own dream is to eliminate\nthe need for a hookdir entirely, so it's logically easy for my Gerrit\nhook to live in ~/gerrit-tools/ and my linter to live in ~/clang-tidy/\nand so on.\n\nI could see option 1 being alleviated by writing it as something like:\n\nupdate:\n $GIT_HOOKDIR/update.d/update-1 &&\n $GIT_HOOKDIR/update.d/update-2\n\nor\nupdate:\n $(git config core.hookdir)/update.d/update-1 &&\n $(git config core.hookdir)/update.d/update-2\n\nBut, I think once you \"buy in\" to the idea of providing a full path to\nthe final target - not to a trampoline script - in your config, you\nshould forget about the idea of \"core.hookdir\" having anything to do\nwith it.\n\nTo quote you out-of-order now:\n\n> If we want to allow people to have multiple hooks under something like\n> .git/hooks/pre-push.d, then we need to have a way to wire them up in the\n> configuration using the correct location of the hook directory instead\n> of using a helper script like the one I linked above.\n\nI think I may spot the misunderstanding. It sounds like you hope someone\ncan provide \"hook.update=001:~/update.d/\" and have all the contents of\nupdate.d executed. I'll be clear and say that I didn't have the\nintention to support that at all; instead I was hoping to support a case\nlike 2. above. Recursing through directories like that sounds scary to\norder.\n\nHmm. Maybe it makes slightly more sense to support something like 'git\nhook add update ~/update.d', which provides an editor where you can add\nordering numbers and config scope to each, but resolves to a config\nwhich looks like option 2. above.\n\nAre you pushing hard for this update.d case in the hopes that someone\ncan 'set and forget' a hook directory like that, and be able to add new\nhooks there without changing the configuration? It sounds tricky to me,\nas we then get to deal with more questions like:\n\n - how do we order hooks in that directory?\n - how should 'git hook list' display those hooks?\n - how would you use 'git hook' to reorder those hooks?\n - do we really want a user to 'git pull' and be running an entirely new script\n they didn't mean to, without any additional interaction? (and maybe\n we do - 'git pull' in a repo where you have asked to run hooks from,\n and then hook behavior changes, might not actually be surprising.)\n\n - Emily\n"},{"id":"387071","messageId":"20191126024147.GF2404748@camp.crustytoothpaste.net","threadId":"52276","inReplyTo":"20191126005614.GA251804@google.com","subject":"Re: [RFC] Hook management via 'git hooks' command","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2019-11-26T02:41:47Z","receivedAt":"2019-11-26T02:41:55Z","isPatch":false,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2019-11-26 at 00:56:14, Emily Shaffer wrote:\n> Hopefully I am not beating a dead horse here but I really want to\n> understand. Let me take another guess with examples at what you mean;\n> please correct me!\n> \n> For our purposes, let's assume:\n> \n> .git/hooks/\n> update\n> update.d/\n> update-1\n> update-2\n> \n> update:\n> #!/bin/bash\n> \n> ./git/hooks/update.d/update-1 &&\n> ./git/hooks/update.d/update-2\n> \n> The goal is to make sure update-1 and update-2 are run when other update\n> hooks happen.\n\n\n\n> With my proposal as is, I see two options:\n> \n> 1)\n> .git/config:\n> hook.runHookDir = true\n> \n> This will run whatever else is in hook.update, and then it will run\n> .git/hooks/update. This is not ideal because hookDir could change, and\n> then the content of update will be incorrect:\n> \n> git config --add core.hookdir=~/hooks/\n> mv .git/hooks/update* ~/hooks/\n> # call something which invokes update hooks\n> # ~/hooks/update fails, .git/hooks/update-1 is gone :(\n> \n\nThis is actually fine. We assume the user knows where they want to\nstore their hooks. If they change that, then that's intentional.\n\n> .git/config:\n> hook.update = 001:/project/path/.git/hooks/update.d/update-1\n> hook.update = 002:/project/path/.git/hooks/update.d/update-2\n> \n> This will run each update hook individually and have no notion of\n> whether they're in a \"hook dir\" or not. It sees a path, it hands it to\n> 'sh' to run, it checks the return code.\n\nCorrect. This is the logical porting of the above shell script to the\nconfig syntax if you use an absolute path.\n\n> Now I run 'mv .git/hooks/update* ~/hooks/'. Next time I invoke something\n> which would run the 'update' hook, it fails, because the path in the\n> config isn't pointing to anything. But this way is unrelated to hookdir.\n\nYes, that's correct.\n\nIt sounds like you're thinking of the config approach as completely\northogonal to the hook directory. However, I want to have multiple\nper-repository hooks that do not live within the repository but move\nwith it. The logical place to store those hooks is in the hook\ndirectory, even if it's not being invoked by Git explicitly. To use\nthat with the config approach so I can have multiple hooks in a useful\nway that's compatible with other tools, I need some way to refer to\nwhatever the hook directory is at a given point in time.\n\n> It sounds like you might be asking for something like:\n> \n> .git/config:\n> hook.update = 001:__HOOKDIR__/update.d/update-1\n> \n> I'm not sure that I like the idea of this. My own dream is to eliminate\n> the need for a hookdir entirely, so it's logically easy for my Gerrit\n> hook to live in ~/gerrit-tools/ and my linter to live in ~/clang-tidy/\n> and so on.\n\nUsing the config syntax eliminates per-repository storage for hooks. I,\npersonally, want to store multiple one-off hooks in my hooks directory.\nI want to use tools that install one-off hooks into my repository\nwithout needing to depend on the location of those tools in the future.\nI don't want those hooks to live elsewhere on my file system, since that\nmakes my repository no longer self contained.\n\nI want to store those hooks in the hook directory, wherever I've\nconfigured that, and whatever that happens to be at this point in time.\n\nI may additionally have tools that live in other locations as well and\nmay use other syntaxes to invoke them. For example, I may install a\nhook that's provided by a Debian package and refer to it using a bare\nprogram name.\n\nIf your goal is to eliminate the hook directory entirely, then I can't\nsay that I'm in support of that. A design which does that won't meet my\nneeds, and it likely won't meet the needs of other users.\n\nThe benefit of your proposed config syntax for me is that it provides a\nstandard way to configure multiple hooks. I still want many of those\nhooks to live in the hook directory, although others may live elsewhere.\n\n> I could see option 1 being alleviated by writing it as something like:\n> \n> update:\n> $GIT_HOOKDIR/update.d/update-1 &&\n> $GIT_HOOKDIR/update.d/update-2\n> \n> or\n> update:\n> $(git config core.hookdir)/update.d/update-1 &&\n> $(git config core.hookdir)/update.d/update-2\n\nThis is similar to what I want, and why I want to pass it to the shell.\nI can write the following, and everything just works:\n\n.git/config:\n hook.update = 001:$(git config core.hookdir || echo .git/hooks)/update.d/update-1\n hook.update = 002:$(git config core.hookdir || echo .git/hooks)/update.d/update-2\n\nWherever I have placed my hooks for this repository, I can refer to\nthem with a shell script. I can also add the following line as well:\n\n.git/config:\n hook.update = debian-package-hook update\n\n…and everything just works.\n\n> But, I think once you \"buy in\" to the idea of providing a full path to\n> the final target - not to a trampoline script - in your config, you\n> should forget about the idea of \"core.hookdir\" having anything to do\n> with it.\n\nI have no intention of providing a full path to any hook, since that's\nquite brittle. There are very few paths on my system which can be\nguaranteed not to change, and all of them are things like /bin/sh or\n/usr/bin/env. If my hooks are in the hook directory (or even in the\nworking tree) with a full path and I move that repository, they become\nbroken. If they're shipped by Debian and Debian decides to move the\ncommand, they're broken.\n\nI'm very interested in learning more about why you seem to want to\nspecify full paths. It seems very at odds with the way the rest of Git\nworks. If the goal is to support other syntaxes in the future, then\nlet's use a prefix character (e.g. !) to denote commands vs.\nnon-commands or something like that. If the goal is security, then I'd\nlike to hear more about the security model you're trying to achieve with\nthis design.\n\n> To quote you out-of-order now:\n> \n> > If we want to allow people to have multiple hooks under something like\n> > .git/hooks/pre-push.d, then we need to have a way to wire them up in the\n> > configuration using the correct location of the hook directory instead\n> > of using a helper script like the one I linked above.\n> \n> I think I may spot the misunderstanding. It sounds like you hope someone\n> can provide \"hook.update=001:~/update.d/\" and have all the contents of\n> update.d executed. I'll be clear and say that I didn't have the\n> intention to support that at all; instead I was hoping to support a case\n> like 2. above. Recursing through directories like that sounds scary to\n> order.\n\nI don't need a particular way to do that, since I can do it already, but\nI do need a way to wire up multiple hooks that are per-repository and\nmove with the repository but aren't in the repository. In other words,\nI need a functional replacement for that approach if we're not going to\nuse that approach itself.\n\nHopefully I've done a better job at explaining myself here.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"387423","messageId":"20191202234605.GA17687@google.com","threadId":"52276","inReplyTo":"20191126024147.GF2404748@camp.crustytoothpaste.net","subject":"Re: [RFC] Hook management via 'git hooks' command","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2019-12-02T23:46:05Z","receivedAt":"2019-12-02T23:46:18Z","isPatch":false,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"Thanks for waiting as I took half of last week off to host family for\nholiday. :)\n\nOn Tue, Nov 26, 2019 at 02:41:47AM +0000, brian m. carlson wrote:\n> On 2019-11-26 at 00:56:14, Emily Shaffer wrote:\n> > Hopefully I am not beating a dead horse here but I really want to\n> > understand. Let me take another guess with examples at what you mean;\n> > please correct me!\n> > \n> > For our purposes, let's assume:\n> > \n> > .git/hooks/\n> > update\n> > update.d/\n> > update-1\n> > update-2\n> > \n> > update:\n> > #!/bin/bash\n> > \n> > ./git/hooks/update.d/update-1 &&\n> > ./git/hooks/update.d/update-2\n> > \n> > The goal is to make sure update-1 and update-2 are run when other update\n> > hooks happen.\n> \n> \n> \n> > With my proposal as is, I see two options:\n> > \n> > 1)\n> > .git/config:\n> > hook.runHookDir = true\n> > \n> > This will run whatever else is in hook.update, and then it will run\n> > .git/hooks/update. This is not ideal because hookDir could change, and\n> > then the content of update will be incorrect:\n> > \n> > git config --add core.hookdir=~/hooks/\n> > mv .git/hooks/update* ~/hooks/\n> > # call something which invokes update hooks\n> > # ~/hooks/update fails, .git/hooks/update-1 is gone :(\n> > \n> \n> This is actually fine. We assume the user knows where they want to\n> store their hooks. If they change that, then that's intentional.\n> \n> > .git/config:\n> > hook.update = 001:/project/path/.git/hooks/update.d/update-1\n> > hook.update = 002:/project/path/.git/hooks/update.d/update-2\n> > \n> > This will run each update hook individually and have no notion of\n> > whether they're in a \"hook dir\" or not. It sees a path, it hands it to\n> > 'sh' to run, it checks the return code.\n> \n> Correct. This is the logical porting of the above shell script to the\n> config syntax if you use an absolute path.\n> \n> > Now I run 'mv .git/hooks/update* ~/hooks/'. Next time I invoke something\n> > which would run the 'update' hook, it fails, because the path in the\n> > config isn't pointing to anything. But this way is unrelated to hookdir.\n> \n> Yes, that's correct.\n> \n> It sounds like you're thinking of the config approach as completely\n> orthogonal to the hook directory. However, I want to have multiple\n> per-repository hooks that do not live within the repository but move\n> with it. The logical place to store those hooks is in the hook\n> directory, even if it's not being invoked by Git explicitly. To use\n> that with the config approach so I can have multiple hooks in a useful\n> way that's compatible with other tools, I need some way to refer to\n> whatever the hook directory is at a given point in time.\n\nThis sounds like a use case we can expect other users to want to work.\nIt is very different from the way I think about and use hooks, so I'm\nglad you explained it to me.\n\n(For the record, the way I think of hooks is something like this: I want\nall repositories to run git-secrets before I push; I want a handful of\nrepositories to run the Gerrit Change-Id hook after I commit; I want one\nrepo to run linter A, and three or four other repos to run linter B. So\nI tend to think \"how can I apply hooks I have downloaded a la carte to\nmy repositories?\" You seem to be thinking \"how can I write hooks for one\nrepository?\" They are both totally valid, of course.)\n\n> \n> > It sounds like you might be asking for something like:\n> > \n> > .git/config:\n> > hook.update = 001:__HOOKDIR__/update.d/update-1\n> > \n> > I'm not sure that I like the idea of this. My own dream is to eliminate\n> > the need for a hookdir entirely, so it's logically easy for my Gerrit\n> > hook to live in ~/gerrit-tools/ and my linter to live in ~/clang-tidy/\n> > and so on.\n> \n> Using the config syntax eliminates per-repository storage for hooks. I,\n> personally, want to store multiple one-off hooks in my hooks directory.\n> I want to use tools that install one-off hooks into my repository\n> without needing to depend on the location of those tools in the future.\n> I don't want those hooks to live elsewhere on my file system, since that\n> makes my repository no longer self contained.\n> \n> I want to store those hooks in the hook directory, wherever I've\n> configured that, and whatever that happens to be at this point in time.\n> \n> I may additionally have tools that live in other locations as well and\n> may use other syntaxes to invoke them. For example, I may install a\n> hook that's provided by a Debian package and refer to it using a bare\n> program name.\n> \n> If your goal is to eliminate the hook directory entirely, then I can't\n> say that I'm in support of that. A design which does that won't meet my\n> needs, and it likely won't meet the needs of other users.\n\nInternally, that is our goal, absolutely. However, you yourself are\nproving it's not a reasonable goal for the entire world. That was\nsomewhat expected, which is why I hoped \"hook.runHookDir\" and\n\"hook.warnHookDir\" would provide a path for users to decide by\nthemselves that they don't want to use the hook directory anymore. A\nself-paced hook directoy phase out seems like a good fit to make us both\nhappy, where your pace can be \"over my dead body\" if you so choose ;)\n\n> \n> The benefit of your proposed config syntax for me is that it provides a\n> standard way to configure multiple hooks. I still want many of those\n> hooks to live in the hook directory, although others may live elsewhere.\n> \n> > I could see option 1 being alleviated by writing it as something like:\n> > \n> > update:\n> > $GIT_HOOKDIR/update.d/update-1 &&\n> > $GIT_HOOKDIR/update.d/update-2\n> > \n> > or\n> > update:\n> > $(git config core.hookdir)/update.d/update-1 &&\n> > $(git config core.hookdir)/update.d/update-2\n> \n> This is similar to what I want, and why I want to pass it to the shell.\n> I can write the following, and everything just works:\n> \n> .git/config:\n> hook.update = 001:$(git config core.hookdir || echo .git/hooks)/update.d/update-1\n> hook.update = 002:$(git config core.hookdir || echo .git/hooks)/update.d/update-2\n> \n> Wherever I have placed my hooks for this repository, I can refer to\n> them with a shell script. I can also add the following line as well:\n> \n> .git/config:\n> hook.update = debian-package-hook update\n> \n> …and everything just works.\n> \n> > But, I think once you \"buy in\" to the idea of providing a full path to\n> > the final target - not to a trampoline script - in your config, you\n> > should forget about the idea of \"core.hookdir\" having anything to do\n> > with it.\n> \n> I have no intention of providing a full path to any hook, since that's\n> quite brittle. There are very few paths on my system which can be\n> guaranteed not to change, and all of them are things like /bin/sh or\n> /usr/bin/env. If my hooks are in the hook directory (or even in the\n> working tree) with a full path and I move that repository, they become\n> broken. If they're shipped by Debian and Debian decides to move the\n> command, they're broken.\n\nI think this is again stemming from the different ways you and I are\nthinking about our hooks, and that's fine. I'm open to the idea of\nrelative-path hooks, and a blurb in the interactive \"git hook edit\" mode\n(or whatever it ends up being) explaining where relative paths will be\nrelative to. Is that OK for you?\n\n> I'm very interested in learning more about why you seem to want to\n> specify full paths. It seems very at odds with the way the rest of Git\n> works.\n\nI hope my description closer to the top of this mail helps to explain.\n\n> If the goal is to support other syntaxes in the future, then\n> let's use a prefix character (e.g. !) to denote commands vs.\n> non-commands or something like that. If the goal is security, then I'd\n> like to hear more about the security model you're trying to achieve with\n> this design.\n\nAs for the security model, I think Jonathan explained it quite well in\nthe link I posted at the top of this thread. The tl;dr is that Malicious\nUser can hand Victim Sysadmin a .zip with a repo that is \"broken\",\nand when Victim Sysadmin starts to explore it, she inadvertently\ntriggers hooks which were packaged in that .zip and compromises her\nworkstation. Eliminating the use of .git/hooks/ alone doesn't protect\nfrom this - .git/config is also a vector, as one can include an alias\nthere - but I believe it's heading towards the right direction.\n\n> \n> > To quote you out-of-order now:\n> > \n> > > If we want to allow people to have multiple hooks under something like\n> > > .git/hooks/pre-push.d, then we need to have a way to wire them up in the\n> > > configuration using the correct location of the hook directory instead\n> > > of using a helper script like the one I linked above.\n> > \n> > I think I may spot the misunderstanding. It sounds like you hope someone\n> > can provide \"hook.update=001:~/update.d/\" and have all the contents of\n> > update.d executed. I'll be clear and say that I didn't have the\n> > intention to support that at all; instead I was hoping to support a case\n> > like 2. above. Recursing through directories like that sounds scary to\n> > order.\n> \n> I don't need a particular way to do that, since I can do it already, but\n> I do need a way to wire up multiple hooks that are per-repository and\n> move with the repository but aren't in the repository. In other words,\n> I need a functional replacement for that approach if we're not going to\n> use that approach itself.\n\nIt sounds like simply supporting relative paths in the config, with\nadequate explanation to the user about where the paths are relative to,\nwill solve this for you. What do you think?\n\n> \n> Hopefully I've done a better job at explaining myself here.\n\nYeah, I think I made a breakthrough understanding you from this mail.\nThanks :) :)\n\n - Emily\n\n"}]}