Hi,
It appears that custom mercurial hooks are not supported by Phabricator.
I looked at all the Herald rules and unfortunately, we cannot enforce our company's workflow by using them.
Is there a way to create new custom Herald rules or work a way through the custom hooks ?
Thanks for the help,
Can you provide some examples of what you want the rules to do? There are probably three general ways forward here:
The path forward which makes the most sense depends on what you actually want to do.
Most of the rules we are implementing are directly related to Differential.
We are for example, reporting or blocking pushes to the repositories based on:
Any of these items can be activated of de-activated based on the author.
One last thing, not related to Differential, is that we are watching the size of the files being added to the repositories.
Ah, thanks! That's helpful.
The filesize condition is one that I think could make sense in the upstream. It seems like a reasonable general-purpose rule with broad applicability.
The "revision exists" condition should already be supported, as you note.
The other checks are probably not good candidates for upstreaming, since they're more specialized than the filesize condition, not things we're philosophically excited about right now, and relatively difficult to build as general-purpose controls (because "last update for any open item" and "status" aren't necessarily simple values).
Implementing these as either hooks or Herald conditions seems reasonable, conceptually. Currently:
Hooks: The path to getting hooks working is probably not especially long, but we've seen no other requests for Mercurial hooks, so this is difficult to prioritize. The major issue here is that in Git and Subversion, we can just have users move existing hooks (which are shell scripts) into a directory and they keep working as-is. We send them extra data (acting user, e.g.) via environment variables.
However, the way Mercurial hooks work is more complex, and it's harder to have our hook fire the other hooks after it finishes and hand them the data they need in the general case. I don't immediately see a reasonable way that we can support Mercurial in-process hooks. However, I think we can do something similar to Git/Subversion and support a directory for shell-script-style hooks -- although we could parse, wrap, and execute them, I think we can not reasonably expect to hand them the arguments they expect (ui, repo, etc).
I think shell-style hooks should be reasonable in your case though, since it doesn't sound like in-process hooks are necessary (you aren't performing complex operations on the Mercurial internals) but you do need access to Phabricator metadata (acting user account, e.g.). This dovetails somewhat with T7959, which is a change in a related area of the codebase; both changes could easily be tested at the same time.
Herald Conditions: The path to modularizing Herald conditions is longer. This is something I'd like to do, and we've made general progress in modularizing Herald (for example, D12957 improves robustness in the face of missing conditions and actions) but it's still a fair amount of work. Until the upstream groundwork happens, it's not practical to add new conditions without heavy local patching. This isn't very easy, nor is it entwined with other planned work, so it's hard to prioritize even though it's something I'd like to see done. It will probably take us a while to be able to find time to build this.
Herald Actions: You can technical cheat today by writing an action instead of a condition. So your rule would work like this:
When conditions are met:
[Always]
Take these actions:
[Run special workflow checks]
Where "Run special workflow checks" is a custom action which evaluates all the stuff you care about and either does nothing or blocks the commit -- roughly, you write your entire commit hook as a custom action, and just always run it.
This is possible today but not well-documented (or at-all-documented); T5194 covers writing documentation and links to T5571, which has more discussion. Depending on your level of ambition, this might be the shortest path to getting a reasonable solution in place.
Wow, thanks a lot for that insightful feedback.
I understand that some of these actions we are taking are definitely not conventional and should not be integrated as is.
Conceptually, I would have loved for my repositories to be migrated as is, i.e. allow custom hooks to be run independently of the Phabricator framework. It looks like the .hgrc file is generated and cannot be manually edited anyway. From a pure integration standpoint, it would make migration a lot easier. But maybe, I am the only one with that kind of requirements :)
Customs Herald Conditions seem the way to go in the future indeed. They make a lot of sense, and managing my rules through them is definitely convenient.
In the meantime, I am OK with implementing my own custom Actions. I have taken a look at the referenced Tasks, and it does not seem insurmountable at all. Unfortunately, I am currently giving it a try and encounter some issues.
I started writing an action that applies to HeraldPreCommitContentAdapter. I am trying to implement "getActionKey" to return HeraldAdapter::BLOCK as it is apparently the only action that would reject a push (src/applications/diffusion/engine/DiffusionCommitHookEngine.php:339). When I do so, my custom action does not appear in the list of actions upon creating the rule. I believe this is due to the fact there can exist only one action of a particular type (getActionNameMap at src/applications/herald/adapter/HeraldAdapter.php:615). Am I mistaken ?
What is the correct way of achieving what I want:
I'm going to close this since it was mostly answered and the remaining questions (about custom extension development) are outside the scope of modern support. See T13039 for a followup about numeric fields in Herald.