This is an archive of the discontinued Mercurial Phabricator instance.

log: add bookmark option to "hg log"
ClosedPublic

Authored by sebhtml on Nov 18 2020, 5:58 PM.

Details

Summary

Before pushing a bookmark with "hg push origin -B 'my-topic'", it is useful to inspect
the list of commits that are ancestors of the bookmark.

By relying on scmutil.bookmarkrevs(), "hg log -B topic" has the same bookmark semantics
found in other commands like hg export, hg email, hg strip.

This is my V3.

Test Plan

[atlantis:/tank/home/sebhtml/projects/mercurial/sebhtml-hg-log-bookmark/hg-committed]
$ uname -a
FreeBSD atlantis 12.2-RELEASE FreeBSD 12.2-RELEASE r366954 GENERIC amd64

[atlantis:/tank/home/sebhtml/projects/mercurial/sebhtml-hg-log-bookmark/hg-committed]
$ ./tests/run-tests.py -t 600
running 946 tests using 12 parallel processes

Failed test-lfs-serve.t#lfsremote-on: output changed
Failed test-transaction-rollback-on-sigpipe.t: output changed

Ran 876 tests, 70 skipped, 2 failed.

python hash seed: 3187862483

test-lfs-serve.t

OUTPUT:--- /tank/home/sebhtml/projects/mercurial/sebhtml-hg-log-bookmark/hg-committed/tests/test-lfs-serve.t
OUTPUT:+++ /tank/home/sebhtml/projects/mercurial/sebhtml-hg-log-bookmark/hg-committed/tests/test-lfs-serve.t#lfsremote-on.err
OUTPUT:@@ -603,24 +603,26 @@
OUTPUT:   */hg-8374dc4052cb.patch (glob)
OUTPUT:   lfs: found bed80f00180ac404b843628ab56a1c1984d6145c391cd1628a7dd7d2598d71fc in the local lfs store
OUTPUT:   */hg-9640b57e77b1.patch (glob)
OUTPUT:-  --- */hg-8374dc4052cb.patch  * (glob)
OUTPUT:-  +++ */hg-9640b57e77b1.patch  * (glob)
OUTPUT:-  @@ -2,12 +2,7 @@
OUTPUT:-   # User test
OUTPUT:-   # Date 0 0
OUTPUT:-   #      Thu Jan 01 00:00:00 1970 +0000
OUTPUT:-  -# Node ID 8374dc4052cbd388e79d9dc4ddb29784097aa354
OUTPUT:-  -# Parent  1477875038c60152e391238920a16381c627b487
OUTPUT:-  -lfs
OUTPUT:-  +# Node ID 9640b57e77b14c3a0144fb4478b6cc13e13ea0d1
OUTPUT:-  +# Parent  d3b84d50eacbd56638e11abce6b8616aaba54420
OUTPUT:-  +add lfs pair
OUTPUT:-
OUTPUT:-  -diff -r 1477875038c6 -r 8374dc4052cb lfs.bin
OUTPUT:-  ---- /dev/null       Thu Jan 01 00:00:00 1970 +0000
OUTPUT:-  -+++ b/lfs.bin       Thu Jan 01 00:00:00 1970 +0000
OUTPUT:-  -@@ -0,0 +1,1 @@
OUTPUT:-  -+this is a big lfs file
OUTPUT:+  error: conflicting output format options.
OUTPUT:+  usage: diff [-aBbdilpTtw] [-c | -e | -f | -n | -q | -u] [--ignore-case]
OUTPUT:+              [--no-ignore-case] [--normal] [--strip-trailing-cr] [--tabsize]
OUTPUT:+              [-I pattern] [-L label] file1 file2
OUTPUT:+         diff [-aBbdilpTtw] [-I pattern] [-L label] [--ignore-case]
OUTPUT:+              [--no-ignore-case] [--normal] [--strip-trailing-cr] [--tabsize]
OUTPUT:+              -C number file1 file2
OUTPUT:+              [--no-ignore-case] [--normal] [--strip-trailing-cr] [--tabsize]                                                                                [120/1886]
OUTPUT:+              -C number file1 file2
OUTPUT:+         diff [-aBbdiltw] [-I pattern] [--ignore-case] [--no-ignore-case]
OUTPUT:+              [--normal] [--strip-trailing-cr] [--tabsize] -D string file1 file2
OUTPUT:+         diff [-aBbdilpTtw] [-I pattern] [-L label] [--ignore-case]
OUTPUT:+              [--no-ignore-case] [--normal] [--tabsize] [--strip-trailing-cr]
OUTPUT:+              -U number file1 file2
OUTPUT:+         diff [-aBbdilNPprsTtw] [-c | -e | -f | -n | -q | -u] [--ignore-case]
OUTPUT:+              [--no-ignore-case] [--normal] [--tabsize] [-I pattern] [-L label]
OUTPUT:+              [-S name] [-X file] [-x pattern] dir1 dir2
OUTPUT:+         diff [-aBbditwW] [--expand-tabs] [--ignore-all-blanks]
OUTPUT:+              [--ignore-blank-lines] [--ignore-case] [--minimal]
OUTPUT:+              [--no-ignore-file-name-case] [--strip-trailing-cr]
OUTPUT:+              [--suppress-common-lines] [--tabsize] [--text] [--width]
OUTPUT:+              -y | --side-by-side file1 file2
OUTPUT:   cleaning up temp directory
OUTPUT:   [1]
OUTPUT:
OUTPUT:
OUTPUT:ERROR: test-lfs-serve.t#lfsremote-on output changed
OUTPUT:

test-transaction-rollback-on-sigpipe.t

OUTPUT:--- /tank/home/sebhtml/projects/mercurial/sebhtml-hg-log-bookmark/hg-committed/tests/test-transaction-rollback-on-sigpipe.t
OUTPUT:+++ /tank/home/sebhtml/projects/mercurial/sebhtml-hg-log-bookmark/hg-committed/tests/test-transaction-rollback-on-sigpipe.t.err
OUTPUT:@@ -56,7 +56,8 @@
OUTPUT:   $ cd local
OUTPUT:   $ echo foo > foo ; hg commit -qAm "commit"
OUTPUT:   $ hg push -q -e "\"$PYTHON\" \"$TESTDIR/dummyssh\"" --remotecmd $remotecmd 2>&1 | grep -v $killable_pipe
OUTPUT:-  abort: stream ended unexpectedly (got 0 bytes, expected 4)
OUTPUT:+  remote: sh: $TESTTMP/remotecmd.sh: not found
OUTPUT:+  abort: no suitable response from remote hg!
OUTPUT:
OUTPUT:   $ check_for_abandoned_transaction
OUTPUT:   [1]
OUTPUT:
OUTPUT:ERROR: test-transaction-rollback-on-sigpipe.t output changed
OUTPUT:
OUTPUT:

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

sebhtml created this revision.Nov 18 2020, 5:58 PM
sebhtml edited the test plan for this revision. (Show Details)Nov 18 2020, 6:07 PM
sebhtml added a reviewer: martinvonz.
sebhtml edited the summary of this revision. (Show Details)Nov 18 2020, 6:15 PM

Do test-lfs-serve.t and test-transaction-rollback-on-sigpipe.t consistently fail with this patch? If they do, you should fix them :) If they don't (i.e. it was some flakiness you were experiencing), just don't mention that in the commit message and instead ask on #mercurial or send an email to mercurial-devel to see if others have seen the flakiness and ask how to fix it.

Do test-lfs-serve.t and test-transaction-rollback-on-sigpipe.t consistently fail with this patch? If they do, you should fix them :) If they don't (i.e. it was some flakiness you were experiencing), just don't mention that in the commit message and instead ask on #mercurial or send an email to mercurial-devel to see if others have seen the flakiness and ask how to fix it.

Also, including diff output (such as from failing tests) without indentation makes the patch fail to apply:

$ hg phabimport D9341
applying patch from D9341
unable to find 'tank/home/sebhtml/projects/mercurial/sebhtml-hg-log-bookmark/hg-committed/tests/test-lfs-serve.t' for patching
(use '--prefix' to apply patch relative to the current directory)
abort: bad hunk #1 @@ -603,24 +603,26 @@
 (24 24 28 26)

Do test-lfs-serve.t and test-transaction-rollback-on-sigpipe.t consistently fail with this patch? If they do, you should fix them :) If they don't (i.e. it was some flakiness you were experiencing), just don't mention that in the commit message and instead ask on #mercurial or send an email to mercurial-devel to see if others have seen the flakiness and ask how to fix it.

I think that these two tests fail because they are not portable on FreeBSD.
I will run them on 'default' on FreeBSD and on 'default' on Debian Linux and post results here.

Also, including diff output (such as from failing tests) without indentation makes the patch fail to apply:
unable to find 'tank/home/sebhtml/projects/mercurial/sebhtml-hg-log-bookmark/hg-committed/tests/test-lfs-serve.t' for patching

I think that the problem is that "hg phabimport" is fetching what I wrote in the test plan. I did not commit anything to tests/test-lfs-serve.t
This is weird.

Also, including diff output (such as from failing tests) without indentation makes the patch fail to apply:
unable to find 'tank/home/sebhtml/projects/mercurial/sebhtml-hg-log-bookmark/hg-committed/tests/test-lfs-serve.t' for patching

I think that the problem is that "hg phabimport" is fetching what I wrote in the test plan. I did not commit anything to tests/test-lfs-serve.t
This is weird.

That's what I meant: you need to indent diffs that you include in the commit message, otherwise tools can't figure out where the commit message ends and the diff starts. But I still think you can just remove that whole thing from the commit message; it's implied that the test plan is to run the test suite, so you only need to describe your test plan if you did some other testing and/or did not run the test suite.

sebhtml edited the test plan for this revision. (Show Details)Nov 23 2020, 5:22 PM

That's what I meant: you need to indent diffs that you include in the commit message, otherwise tools can't figure out where the commit message ends and the diff starts. But I still think you can just remove that whole thing from the commit message; it's implied that the test plan is to run the test suite, so you only need to describe your test plan if you did some other testing and/or did not run the test suite.

I did not include any diff in my commit message, as far as I know.

Given what you recommended, I added the prefix OUTPUT: in my test results in my test plan.

That's what I meant: you need to indent diffs that you include in the commit message, otherwise tools can't figure out where the commit message ends and the diff starts. But I still think you can just remove that whole thing from the commit message; it's implied that the test plan is to run the test suite, so you only need to describe your test plan if you did some other testing and/or did not run the test suite.

I did not include any diff in my commit message, as far as I know.
Given what you recommended, I added the prefix OUTPUT: in my test results in my test plan.

The test failure has a diff. I still think that's just noise that will confuse much more than it helps. Okay if I drop the test plan from the commit message in flight?

The test failure has a diff. I still think that's just noise that will confuse much more than it helps. Okay if I drop the test plan from the commit message in flight?

Sure. In my opinion, the test plan is not part of my commit message.

I am currently running the tests in a VM-BHYVE running Debian Linux 10. I except all tests to pass.
I will keep you posted.

martinvonz accepted this revision.Nov 23 2020, 6:47 PM

The test failure has a diff. I still think that's just noise that will confuse much more than it helps. Okay if I drop the test plan from the commit message in flight?

Sure. In my opinion, the test plan is not part of my commit message.
I am currently running the tests in a VM-BHYVE running Debian Linux 10. I except all tests to pass.
I will keep you posted.

I've run the tests on whatever kind of Debian I have and only test-check-format.t failed. I'll fix the formatting error in flight. Queuing this now, thanks!

This revision is now accepted and ready to land.Nov 23 2020, 6:47 PM
This revision was automatically updated to reflect the committed changes.
sebhtml added a comment.EditedNov 23 2020, 7:14 PM

Do test-lfs-serve.t and test-transaction-rollback-on-sigpipe.t consistently fail with this patch? If they do, you should fix them :) If they don't (i.e. it was some flakiness you were experiencing), just don't mention that in the commit message and instead ask on #mercurial or send an email to mercurial-devel to see if others have seen the flakiness and ask how to fix it.

I think that the problem is that the 2 failing tests of Mercurial are not portable on FreeBSD 12.2.

Proof:

Tests in 'default' on Debian GNU/Linux 10 (buster)

$ uname -a
Linux capside 4.19.0-12-amd64 #1 SMP Debian 4.19.152-1 (2020-10-18) x86_64 GNU/Linux
$ python --version
Python 3.7.3
$ hg id
f96059fa519c tip @
$ ./tests/run-tests.py -t 600
running 945 tests using 12 parallel processes
...
# Ran 870 tests, 75 skipped, 0 failed.
$ ./tests/run-tests.py -t 600 tests/test-lfs-serve.t                                                         
running 2 tests using 2 parallel processes 
..
# Ran 2 tests, 0 skipped, 0 failed.
$ ./tests/run-tests.py -t 600 tests/test-transaction-rollback-on-sigpipe.t
running 1 tests using 1 parallel processes 
.
# Ran 1 tests, 0 skipped, 0 failed.

Tests in 'default' on FreeBSD 12.2

$ uname -a
FreeBSD atlantis 12.2-RELEASE FreeBSD 12.2-RELEASE r366954 GENERIC  amd64
$ python --version
Python 3.7.6
$ pwd
/tank/home/sebhtml/projects/mercurial/mercurial-scm.org/hg-committed
$ hg id
f96059fa519c tip @
$ ./tests/run-tests.py -t 600
running 945 tests using 12 parallel processes                  
...
Failed test-lfs-serve.t#lfsremote-on: output changed
Failed test-transaction-rollback-on-sigpipe.t: output changed
# Ran 875 tests, 70 skipped, 2 failed.
python hash seed: 864460952

Do test-lfs-serve.t and test-transaction-rollback-on-sigpipe.t consistently fail with this patch? If they do, you should fix them :) If they don't (i.e. it was some flakiness you were experiencing), just don't mention that in the commit message and instead ask on #mercurial or send an email to mercurial-devel to see if others have seen the flakiness and ask how to fix it.

I think that the problem is that the 2 failing tests of Mercurial are not portable on FreeBSD 12.2.

I don't have FreeBSD so it's hard for me to test. We're happy to take patches for making the tests more portable :)