See the attached patch for a simple repro. If a repo has a prechangegroup hook, it causes future pretxnclose hooks to run without being able to see the changelog.a file, which means they cannot see the new commits that are supposed to be tested in the hook. The root cause is that changelog.delayupdate() must be called after every changelog write, but in changegroup.py it is called *before* the changelog writes. delayupdate() adds a subscription to the transaction that says "on the next writepending() write to the .a file". When prechangegroup is called, it uses that subscription then throws the subscription away, so future hooks don't execute it and consequently don't see the commits. This bisects to "520defbc0335 hook: centralize passing HG_PENDING to external hook process", but that change just exposes the issue (by making every hook call tr.writepending()). The actual problem is from over a year ago (when transaction writepending() was added), since it wasn't ensured that the delayupdate subscription was added appropriately everywhere.
Created attachment 1870 [details] Repro test change
I believe the fix is to just move the delayupdate call to be down below the changelog writes. I'm testing it, but this is exposing another issue where the HG_PENDING environment variable only gets put in the hook if writepending() returns true, but it will only return true for the first invocation of it, so I may need to fix that as well (by having HG_PENDING always added if a transaction exists)
Patches are out
Fixed by https://selenic.com/repo/hg/rev/e7c618cee8df Durham Goode <durham@fb.com> hooks: fix hooks not firing if prechangegroup was set (issue4934) We need to call delayupdate again after writing to the changelog. Otherwise the prechangegroup hook consumes the delayupdate subscription and future hooks don't see the pending changes (see issue 4934 for more details). Adds a test that triggers the prechangegroup hook before the pretxnchangegroup hook and verifies that the output of pretxnchangegroup doesn't change. (please test the fix)
Fixed by https://selenic.com/repo/hg/rev/c4895f9b8ab1 Pierre-Yves David <pierre-yves.david@fb.com> changegroup: back code change of e7c618cee8df out The previous changeset is a simpler way of fixing issue4934 without changing the spirit of the code. We can remove the dual call to 'delayupdate' but we keep the tests to show that the issue is still fixed. (please test the fix)
Bug was set to TESTING for 7 days, resolving