add the ability to blacklist mercurial extensions to moz-phab
Categories
(Conduit :: moz-phab, enhancement, P3)
Tracking
(Not tracked)
People
(Reporter: valentin, Unassigned)
Details
(Keywords: conduit-triaged)
So I recently installed the bookbinder mercurial extension again and noticed that when uploading a stack with moz-phab submit, the commits were actually submitted in reverse order.
[1] https://bitbucket.org/halbersa/bookbinder
I use the extension to get the equivalent of hg qapplied:
[extensions]
histedit =
rebase =
share =
purge =
evolve = ~/.mozbuild/evolve/hgext3rd/evolve
blackbox =
firefoxtree = ~/.mozbuild/version-control-tools/hgext/firefoxtree
clang-format = ~/.mozbuild/version-control-tools/hgext/clang-format
format-source = ~/.mozbuild/version-control-tools/hgext/format-source
push-to-try = ~/.mozbuild/version-control-tools/hgext/push-to-try
mq = !
bookbinder = ~/workspace/bookbinder
[alias]
blog = log --rev 'feature(tip)'
I noticed the problem because sometimes the command errors out like this:
abort: this rebase will cause divergences from: d2f53839e4b1
(to force the rebase please set experimental.evolution.allowdivergence=True)
CalledProcessError: Command '['hg', '--config', 'extensions.rebase=', 'rebase', '--source', 'd2f53839e4b15187fe1eb85beea053ed2acca8b3', '--dest', '3bb3381ff56d00d1807db7c42b8b4e434dc07407']' returned non-zero exit status 255
or
abort: source and destination form a cycle
CalledProcessError: Command '['hg', '--config', 'extensions.rebase=', 'rebase', '--source', '66b5f646a592adf8205b2c0e2d034e7006cbae7a', '--dest', '1bffc850d56e655bdb47cba90aad4e23e94177cd']' returned non-zero exit status 255
The errors don't happen without the extension.
I'm not sure how moz-phab works, and if this is a bug in the extension causing subtle differences in how it outputs the history or in how moz-phab is using that history.
|
||
Comment 1•6 months ago
|
||
mercurial extensions have the ability to change pretty well any aspect of how mercurial works; it isn't surprising that this sort of thing happens unfortunately.
in bookbinder's case they override log's output to reverse the ordering, which is exciting.
https://bitbucket.org/halbersa/bookbinder/src/40ddfc43535fea55f71e592b19b40f6ba82155a8/bookbinder.py?at=default&fileviewer=file-view-default#bookbinder.py-156
moz-phab used to use a whitelist of hg extensions to avoid this, however we ran into issues where some useful extensions were not being loaded (especially experimental extensions or those relating to performance).
so now moz-phab loads all configured extensions, with a --safe-mode switch to fall back to the whitelisting approach. until this bug is resolved you'll have to pass --safe-mode to moz-phab submit in order for things to work.
looks like we'll need to add the ability to blacklist an extension:
hg --config extensions.bookbinder=! ..
|
|
||
Comment 2•6 months ago
|
||
|
||
Comment 3•6 months ago
|
||
Commented on that issue. Basically I need to add log to the reversewrap in order to match the behaviour of mercurial. I.e, without reversing, hg log -r "ancestors(.)" -T "{rev}\n" results in:
0
1
2
3
...
Fwiw, I use bookbinder + moz-phab without issue (unless moz-phab only very recently stopped using a whitelist and I haven't upgraded yet).
One possible thing to try is to set HGPLAIN when running moz-phab submit? Just a shot in the dark, haven't tested or anything.
|
|
||
Comment 4•6 months ago
|
||
Valentin, might also help to know what version of mercurial you're using.
|
|
||
Comment 5•6 months ago
|
||
(In reply to Andrew Halberstadt [:ahal] from comment #3)
One possible thing to try is to set HGPLAIN when running
moz-phab submit? Just a shot in the dark, haven't tested or anything.
HGPLAIN impacts encoding, verbosity, localisation; it doesn't override extensions.
|
Reporter | |
Comment 6•6 months ago
|
||
(In reply to Andrew Halberstadt [:ahal] from comment #4)
Valentin, might also help to know what version of mercurial you're using.
Mercurial Distributed SCM (version 4.5.3)
|
|
||
Comment 7•6 months ago
|
||
From the issue:
I think I see the issue. Here is mercurial's logic for handling log: https://www.mercurial-scm.org/repo/hg/file/stable/mercurial/logcmdutil.py#l693
So essentially if you pass in -r <revset> mercurial does not reverse the output. But if you don't pass in -r <revset>, it does reverse the output.
Bookbinder only operates when you pass in -r, so that is why I needed to reverse in order to match the default behaviour. The bug here is that I'm reversing all revsets, whether or not bookbinder actually expanded them or not. I should only reverse revsets that bookbinder expands.
(In reply to Byron Jones ‹:glob› 🎈 from comment #5)
HGPLAINimpacts encoding, verbosity, localisation; it doesn't override extensions.
It does impact bookbinder:
https://bitbucket.org/halbersa/bookbinder/src/40ddfc43535fea55f71e592b19b40f6ba82155a8/bookbinder.py?at=default&fileviewer=file-view-default#lines-51
But I think it would be irrelevant in this particular case.
|
|
||
Comment 8•6 months ago
|
||
Note: I'm still confused about why I'm seeing moz-phab submit revisions in the right order. Valentin and I must have something different in our local hg configs.
|
|
||
Comment 9•6 months ago
|
||
I think I pushed a fix to bookbinder, though I'm not sure as I wasn't able to reproduce your issue. Can you try updating bookbinder and pushing to review again?
|
|
Reporter | |
Comment 10•6 months ago
|
||
(In reply to Andrew Halberstadt [:ahal] from comment #9)
I think I pushed a fix to bookbinder, though I'm not sure as I wasn't able to reproduce your issue. Can you try updating bookbinder and pushing to review again?
moz-phab seems to work correctly now.
hg blog does list the commits in reverse order compared to hg log -l 4 but this is a lot better than submitting them in reverse order :)
Thanks a lot for the fix.
|
|
||
Comment 11•6 months ago
|
||
I'm not sure what hg blog is, but if that's a custom alias using -r, yes it will start logging in the reverse order as before (which is what mercurial does by design). You can re-align it by wrapping whatever your revset is with reverse(<revset>).
|
|
||
Comment 12•6 months ago
|
||
Oh I see it in your ~/.hgrc, yes you'll need to change it to reverse(feature(tip)).
|
|
Reporter | |
Comment 13•6 months ago
|
||
(In reply to Andrew Halberstadt [:ahal] from comment #12)
Oh I see it in your
~/.hgrc, yes you'll need to change it toreverse(feature(tip)).
Thanks! It's perfect now.
|
|
||
Comment 14•6 months ago
|
||
thanks for the super quick fix ahal!
as it might be useful in future i'll make this bug about a generic blacklisting feature.
might be best implemented as a configuration option so users can self-manage (as opposed to a hard-coded list in moz-phab).
Description
•