Patchwork [09,of,11] shelve: use fixed date for temporary commit

login
register
mail settings
Submitter Mads Kiilerich
Date Feb. 20, 2014, 1:43 a.m.
Message ID <6e1c0666f1f4fde109da.1392860613@localhost.localdomain>
Download mbox | patch
Permalink /patch/3715/
State Accepted
Headers show

Comments

Mads Kiilerich - Feb. 20, 2014, 1:43 a.m.
# HG changeset patch
# User Mads Kiilerich <madski@unity3d.com>
# Date 1392860341 -3600
#      Thu Feb 20 02:39:01 2014 +0100
# Node ID 6e1c0666f1f4fde109da045aedf26d93af9e7ed1
# Parent  a03c3f91bc52bad6796d83c9a723e64f74f8e9fa
shelve: use fixed date for temporary commit

Using a fixed date makes hashes stable and makes debugging simpler. The date
and hashes are normally not exposed.

The only slight disadvantage is that it perhaps in some cases when doing
forensics could be nice to see exactly when the temporary commit was made.
Sean Farley - Feb. 20, 2014, 5:37 a.m.
Mads Kiilerich <mads@kiilerich.com> writes:

> # HG changeset patch
> # User Mads Kiilerich <madski@unity3d.com>
> # Date 1392860341 -3600
> #      Thu Feb 20 02:39:01 2014 +0100
> # Node ID 6e1c0666f1f4fde109da045aedf26d93af9e7ed1
> # Parent  a03c3f91bc52bad6796d83c9a723e64f74f8e9fa
> shelve: use fixed date for temporary commit
>
> Using a fixed date makes hashes stable and makes debugging simpler. The date
> and hashes are normally not exposed.
>
> The only slight disadvantage is that it perhaps in some cases when doing
> forensics could be nice to see exactly when the temporary commit was made.

That seems like a major downside. Why not make an optional parameter for
setting the date through the tests?

> diff --git a/hgext/shelve.py b/hgext/shelve.py
> --- a/hgext/shelve.py
> +++ b/hgext/shelve.py
> @@ -546,7 +546,7 @@ def unshelve(ui, repo, *shelved, **opts)
>  
>                  try:
>                      return repo.commit(message, 'shelve@localhost',
> -                                       opts.get('date'), match)
> +                                       '0 0', match)
>                  finally:
>                      if hasmq:
>                          repo.mq.checkapplied = saved
> diff --git a/tests/test-shelve.t b/tests/test-shelve.t
> --- a/tests/test-shelve.t
> +++ b/tests/test-shelve.t
> @@ -586,16 +586,16 @@ unshelve and conflicts with untracked fi
>    merging f incomplete! (edit conflicts, then use 'hg resolve --mark')
>    unresolved conflicts (see 'hg resolve', then 'hg unshelve --continue')
>    [1]
> -  $ hg log -G --template '{rev}  {desc|firstline}  {author}'
> -  @  5  changes to 'commit stuff'  shelve@localhost
> +  $ hg log -G --template '{rev}  {desc|firstline}  {author}  {date|isodate}'
> +  @  5  changes to 'commit stuff'  shelve@localhost  1970-01-01 00:00 +0000
>    |
> -  | @  4  pending changes temporary commit  shelve@localhost
> +  | @  4  pending changes temporary commit  shelve@localhost  1970-01-01 00:00 +0000
>    |/
> -  o  3  commit stuff  test
> +  o  3  commit stuff  test  1970-01-01 00:00 +0000
>    |
> -  | o  2  c  test
> +  | o  2  c  test  1970-01-01 00:00 +0000
>    |/
> -  o  0  a  test
> +  o  0  a  test  1970-01-01 00:00 +0000

The date being listed here seems to contradict your earlier statement of
normally not being exposed. I would rather not see them if using log -G.

>    $ hg st
>    M f
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
Mads Kiilerich - Feb. 20, 2014, 2:22 p.m.
On 02/20/2014 06:37 AM, Sean Farley wrote:
> Mads Kiilerich <mads@kiilerich.com> writes:
>
>> # HG changeset patch
>> # User Mads Kiilerich <madski@unity3d.com>
>> # Date 1392860341 -3600
>> #      Thu Feb 20 02:39:01 2014 +0100
>> # Node ID 6e1c0666f1f4fde109da045aedf26d93af9e7ed1
>> # Parent  a03c3f91bc52bad6796d83c9a723e64f74f8e9fa
>> shelve: use fixed date for temporary commit
>>
>> Using a fixed date makes hashes stable and makes debugging simpler. The date
>> and hashes are normally not exposed.
>>
>> The only slight disadvantage is that it perhaps in some cases when doing
>> forensics could be nice to see exactly when the temporary commit was made.
> That seems like a major downside.

Why is that major?

> Why not make an optional parameter for
> setting the date through the tests?

That would introduce unnecessary complexity. I would not be able to 
argue why that would be relevant.

>> diff --git a/hgext/shelve.py b/hgext/shelve.py
>> --- a/hgext/shelve.py
>> +++ b/hgext/shelve.py
>> @@ -546,7 +546,7 @@ def unshelve(ui, repo, *shelved, **opts)
>>   
>>                   try:
>>                       return repo.commit(message, 'shelve@localhost',
>> -                                       opts.get('date'), match)
>> +                                       '0 0', match)
>>                   finally:
>>                       if hasmq:
>>                           repo.mq.checkapplied = saved
>> diff --git a/tests/test-shelve.t b/tests/test-shelve.t
>> --- a/tests/test-shelve.t
>> +++ b/tests/test-shelve.t
>> @@ -586,16 +586,16 @@ unshelve and conflicts with untracked fi
>>     merging f incomplete! (edit conflicts, then use 'hg resolve --mark')
>>     unresolved conflicts (see 'hg resolve', then 'hg unshelve --continue')
>>     [1]
>> -  $ hg log -G --template '{rev}  {desc|firstline}  {author}'
>> -  @  5  changes to 'commit stuff'  shelve@localhost
>> +  $ hg log -G --template '{rev}  {desc|firstline}  {author}  {date|isodate}'
>> +  @  5  changes to 'commit stuff'  shelve@localhost  1970-01-01 00:00 +0000
>>     |
>> -  | @  4  pending changes temporary commit  shelve@localhost
>> +  | @  4  pending changes temporary commit  shelve@localhost  1970-01-01 00:00 +0000
>>     |/
>> -  o  3  commit stuff  test
>> +  o  3  commit stuff  test  1970-01-01 00:00 +0000
>>     |
>> -  | o  2  c  test
>> +  | o  2  c  test  1970-01-01 00:00 +0000
>>     |/
>> -  o  0  a  test
>> +  o  0  a  test  1970-01-01 00:00 +0000
> The date being listed here seems to contradict your earlier statement of
> normally not being exposed.

This is the case where you look at  the DAG while you are in the process 
of resolving an unshelve conflict. I do not consider that 'normal'.

The commit also uses hardcoded unlocalized 'pending changes temporary 
commit' and 'shelve@localhost'. That shows that this commit just is an 
implementation detail.

> I would rather not see them if using log -G.

Them = the changesets? I kind of agree - it is an implementation detail. 
But it would be weird if you were merging changesets that you couldn't 
see at all. It shows an implementation detail, but also a detail that 
explains a lot about what unshelve really do.

/Mads
Sean Farley - Feb. 20, 2014, 4:12 p.m.
Mads Kiilerich <mads@kiilerich.com> writes:

> On 02/20/2014 06:37 AM, Sean Farley wrote:
>> Mads Kiilerich <mads@kiilerich.com> writes:
>>
>>> # HG changeset patch
>>> # User Mads Kiilerich <madski@unity3d.com>
>>> # Date 1392860341 -3600
>>> #      Thu Feb 20 02:39:01 2014 +0100
>>> # Node ID 6e1c0666f1f4fde109da045aedf26d93af9e7ed1
>>> # Parent  a03c3f91bc52bad6796d83c9a723e64f74f8e9fa
>>> shelve: use fixed date for temporary commit
>>>
>>> Using a fixed date makes hashes stable and makes debugging simpler. The date
>>> and hashes are normally not exposed.
>>>
>>> The only slight disadvantage is that it perhaps in some cases when doing
>>> forensics could be nice to see exactly when the temporary commit was made.
>> That seems like a major downside.
>
> Why is that major?

You said it yourself: debugging forensics.

>> Why not make an optional parameter for
>> setting the date through the tests?
>
> That would introduce unnecessary complexity. I would not be able to 
> argue why that would be relevant.

It doesn't seem that much more complex but I dunno. I'll let others
chime in to see if I'm alone on this one.

>>> diff --git a/hgext/shelve.py b/hgext/shelve.py
>>> --- a/hgext/shelve.py
>>> +++ b/hgext/shelve.py
>>> @@ -546,7 +546,7 @@ def unshelve(ui, repo, *shelved, **opts)
>>>   
>>>                   try:
>>>                       return repo.commit(message, 'shelve@localhost',
>>> -                                       opts.get('date'), match)
>>> +                                       '0 0', match)
>>>                   finally:
>>>                       if hasmq:
>>>                           repo.mq.checkapplied = saved
>>> diff --git a/tests/test-shelve.t b/tests/test-shelve.t
>>> --- a/tests/test-shelve.t
>>> +++ b/tests/test-shelve.t
>>> @@ -586,16 +586,16 @@ unshelve and conflicts with untracked fi
>>>     merging f incomplete! (edit conflicts, then use 'hg resolve --mark')
>>>     unresolved conflicts (see 'hg resolve', then 'hg unshelve --continue')
>>>     [1]
>>> -  $ hg log -G --template '{rev}  {desc|firstline}  {author}'
>>> -  @  5  changes to 'commit stuff'  shelve@localhost
>>> +  $ hg log -G --template '{rev}  {desc|firstline}  {author}  {date|isodate}'
>>> +  @  5  changes to 'commit stuff'  shelve@localhost  1970-01-01 00:00 +0000
>>>     |
>>> -  | @  4  pending changes temporary commit  shelve@localhost
>>> +  | @  4  pending changes temporary commit  shelve@localhost  1970-01-01 00:00 +0000
>>>     |/
>>> -  o  3  commit stuff  test
>>> +  o  3  commit stuff  test  1970-01-01 00:00 +0000
>>>     |
>>> -  | o  2  c  test
>>> +  | o  2  c  test  1970-01-01 00:00 +0000
>>>     |/
>>> -  o  0  a  test
>>> +  o  0  a  test  1970-01-01 00:00 +0000
>> The date being listed here seems to contradict your earlier statement of
>> normally not being exposed.
>
> This is the case where you look at  the DAG while you are in the process 
> of resolving an unshelve conflict. I do not consider that 'normal'.

Ah, I didn't realize that it's only for conflicts.

> The commit also uses hardcoded unlocalized 'pending changes temporary 
> commit' and 'shelve@localhost'. That shows that this commit just is an 
> implementation detail.
>
>> I would rather not see them if using log -G.
>
> Them = the changesets? I kind of agree - it is an implementation detail. 
> But it would be weird if you were merging changesets that you couldn't 
> see at all. It shows an implementation detail, but also a detail that 
> explains a lot about what unshelve really do.
>
> /Mads

Patch

diff --git a/hgext/shelve.py b/hgext/shelve.py
--- a/hgext/shelve.py
+++ b/hgext/shelve.py
@@ -546,7 +546,7 @@  def unshelve(ui, repo, *shelved, **opts)
 
                 try:
                     return repo.commit(message, 'shelve@localhost',
-                                       opts.get('date'), match)
+                                       '0 0', match)
                 finally:
                     if hasmq:
                         repo.mq.checkapplied = saved
diff --git a/tests/test-shelve.t b/tests/test-shelve.t
--- a/tests/test-shelve.t
+++ b/tests/test-shelve.t
@@ -586,16 +586,16 @@  unshelve and conflicts with untracked fi
   merging f incomplete! (edit conflicts, then use 'hg resolve --mark')
   unresolved conflicts (see 'hg resolve', then 'hg unshelve --continue')
   [1]
-  $ hg log -G --template '{rev}  {desc|firstline}  {author}'
-  @  5  changes to 'commit stuff'  shelve@localhost
+  $ hg log -G --template '{rev}  {desc|firstline}  {author}  {date|isodate}'
+  @  5  changes to 'commit stuff'  shelve@localhost  1970-01-01 00:00 +0000
   |
-  | @  4  pending changes temporary commit  shelve@localhost
+  | @  4  pending changes temporary commit  shelve@localhost  1970-01-01 00:00 +0000
   |/
-  o  3  commit stuff  test
+  o  3  commit stuff  test  1970-01-01 00:00 +0000
   |
-  | o  2  c  test
+  | o  2  c  test  1970-01-01 00:00 +0000
   |/
-  o  0  a  test
+  o  0  a  test  1970-01-01 00:00 +0000
   
   $ hg st
   M f