Patchwork D6026: lock: Improve the waiting for lock message which will help newcomers(issue6081)

login
register
mail settings
Submitter phabricator
Date Feb. 26, 2019, 9:30 a.m.
Message ID <differential-rev-PHID-DREV-kdrxik2xxrts4x4ifet5-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/38937/
State New
Headers show

Comments

phabricator - Feb. 26, 2019, 9:30 a.m.
akshjain.jain74 created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D6026

AFFECTED FILES
  tests/test-lock-badness.t

CHANGE DETAILS




To: akshjain.jain74, #hg-reviewers
Cc: mercurial-devel
phabricator - Feb. 26, 2019, 2:21 p.m.
pulkit added a comment.


  This patch does not changes any code. It only changes some test output. You need to change the error message in the code.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D6026

To: akshjain.jain74, #hg-reviewers
Cc: pulkit, mercurial-devel
phabricator - Feb. 26, 2019, 2:23 p.m.
akshjain.jain74 added a comment.


  Ya I am sending that  just trying to find out the file

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D6026

To: akshjain.jain74, #hg-reviewers
Cc: pulkit, mercurial-devel
phabricator - Feb. 28, 2019, 9:31 p.m.
pulkit added inline comments.

INLINE COMMENTS

> journal.py:310
>              self.ui.warn(
> -                _("waiting for lock on %s held by %r\n") % (desc, inst.locker))
> +                _("waiting for lock on %s held by %r\n" "on host %r\n"
> +                    """Another Mercurial process seems to be running in

This is only for locking namejournal.lock and not for working directory lock and store lock. We need to improve message there too.

> journal.py:316
> +                      may have crashed in this repository earlier:
> +  remove the file manually to continue.\n""") % (desc, inst.locker))
>              # default to 600 seconds timeout

remove which file manually to continue?

I think we should delete the `If it still fails, ..... remove the file manually to continue` part.

> test-lock-badness.t:18
>    > from mercurial import error, registrar
> -  > 
>    > cmdtable = {}

unrelated changes in the test file here and below.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D6026

To: akshjain.jain74, #hg-reviewers
Cc: pulkit, mercurial-devel
phabricator - Feb. 28, 2019, 10:14 p.m.
akshjain.jain74 marked an inline comment as done.
akshjain.jain74 added a comment.


  @pulkit done changes as per your suggestion in lock file too but what did you meant by storelock?

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D6026

To: akshjain.jain74, #hg-reviewers
Cc: pulkit, mercurial-devel
phabricator - March 6, 2019, 1:40 p.m.
pulkit added a comment.


  I am not sure the new message is much helpful. As JordiGH and others pointed out on IRC, it will definitely be helpful if we can show which operation has the lock right now.
  
  Something like:
  
  `waiting for lock on working directory of b held by process '*' running <cmd-name> on host '*'`

INLINE COMMENTS

> test-extdiff.t:155
> +  diffing */extdiff.*/a.46c0e4daeb72/b a.81906f2b98ac/b (glob)
>    diffing */extdiff.*/a.46c0e4daeb72/a a.81906f2b98ac/a (glob)
>    making snapshot of 2 files from rev 46c0e4daeb72

unrelated changes.

> pulkit wrote in test-lock-badness.t:18
> unrelated changes in the test file here and below.

still here are unrealted changes.

> test-lock-badness.t:62
>    waiting for lock on working directory of b held by process '*' on host '*' (glob)
> +  Another Mercurial process seems to be running in
> +                       this repository, e.g. an editor opened by 'hg commit'.

The output is not correctly aligned.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D6026

To: akshjain.jain74, #hg-reviewers
Cc: pulkit, mercurial-devel
phabricator - March 6, 2019, 5:31 p.m.
akshjain.jain74 added a comment.


  okay @pulkit let me work upon your suggestion and

INLINE COMMENTS

> pulkit wrote in test-lock-badness.t:18
> still here are unrealted changes.

I accept these changes while running test,  if i am not changing this then it will cause test failing

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D6026

To: akshjain.jain74, #hg-reviewers
Cc: pulkit, mercurial-devel
phabricator - March 6, 2019, 7:31 p.m.
akshjain.jain74 added a comment.


  In https://phab.mercurial-scm.org/D6026#88552, @pulkit wrote:
  
  > I am not sure the new message is much helpful. As JordiGH and others pointed out on IRC, it will definitely be helpful if we can show which operation has the lock right now.
  >
  > Something like:
  >
  > `waiting for lock on working directory of b held by process '*' running <cmd-name> on host '*'`
  
  
  So for which commands we can show this message, like i got message when i run hg commit --amend sometimes

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D6026

To: akshjain.jain74, #hg-reviewers
Cc: pulkit, mercurial-devel

Patch

diff --git a/tests/test-lock-badness.t b/tests/test-lock-badness.t
--- a/tests/test-lock-badness.t
+++ b/tests/test-lock-badness.t
@@ -15,17 +15,17 @@ 
 
   $ cat > testlock.py << EOF
   > from mercurial import error, registrar
-  > 
+  >
   > cmdtable = {}
   > command = registrar.command(cmdtable)
-  > 
+  >
   > def acquiretestlock(repo, releaseexc):
   >     def unlock():
   >         if releaseexc:
   >             raise error.Abort(b'expected release exception')
   >     l = repo._lock(repo.vfs, b'testlock', False, unlock, None, b'test lock')
   >     return l
-  > 
+  >
   > @command(b'testlockexc')
   > def testlockexc(ui, repo):
   >     testlock = acquiretestlock(repo, True)
@@ -106,6 +106,12 @@ 
   calling hook pre-update: hghook_pre-update.sleephalf
   waiting for lock on working directory of b held by process '*' on host '*' (glob)
   got lock after * seconds (glob)
+
+  Another Mercurial process seems to be running in this repository, e.g.
+  an editor opened by 'hg commit'. Please make sure all processes
+  are terminated then try again. If it still fails, a Mercurial process
+  may have crashed in this repository earlier:
+  remove the file manually to continue.
   0 files updated, 0 files merged, 0 files removed, 0 files unresolved
   $ cat preup-stderr
   $ cat stdout
@@ -123,6 +129,12 @@ 
   calling hook pre-update: hghook_pre-update.sleephalf
   waiting for lock on working directory of b held by process '*' on host '*' (glob)
   got lock after * seconds (glob)
+
+  Another Mercurial process seems to be running in this repository, e.g.
+  an editor opened by 'hg commit'. Please make sure all processes
+  are terminated then try again. If it still fails, a Mercurial process
+  may have crashed in this repository earlier:
+  remove the file manually to continue.
   0 files updated, 0 files merged, 0 files removed, 0 files unresolved
   $ cat preup-stderr
   $ cat stdout