[PATCH 1 of 1] convert: add config option to use the local time zone

[Julian Cowley]

On Tue, Nov 13, 2012 at 7:23 AM, Bryan O'Sullivan <bos at serpentine.com> wrote:

[Finally had a chance to work on this and modify the patch to address
your concerns.  The updated patch follows in another post.]

> On Tue, Nov 13, 2012 at 1:54 AM, Julian Cowley <julian at lava.net> wrote:
>> IMPORTANT: the patch only applies to conversions from cvs or svn.
>> The documentation for the option only appears in those two sections
>> in the convert help text.
>
>
> If that's true, why is there a hunk applied to convert/hg.py?

That's because of an extra changeset that convert adds (see below).
I've reverted this change in the update.

>> +    :convert.localtime: use local time (as determined by the TZ
>> +        environment variable) for changeset date/times. The default
>> +        is False (use UTC).
>
>
> Maybe name this localtimezone instead. When I read "localtime", I
> incorrectly infer "oh, it uses the current local system time when doing a
> conversion".

Changed option to "convert.localtimezone".

>> --- a/hgext/convert/cvs.py
>> +++ b/hgext/convert/cvs.py
>> @@ -70,6 +70,8 @@
>>                  cs.author = self.recode(cs.author)
>>                  self.lastbranch[cs.branch] = id
>>                  cs.comment = self.recode(cs.comment)
>> +                if self.ui.configbool('convert', 'localtime', False):
>> +                    cs.date = util.makedate(cs.date[0])
>
>
> The default for configbool is already False, so don't supply a default when
> you use it.

OK.

>> diff --git a/hgext/convert/hg.py b/hgext/convert/hg.py
>> --- a/hgext/convert/hg.py
>> +++ b/hgext/convert/hg.py
>> @@ -203,7 +203,11 @@
>>              return context.memfilectx(f, data, False, False, None)
>>
>>          self.ui.status(_("updating tags\n"))
>> -        date = "%s 0" % int(time.mktime(time.gmtime()))
>> +        if self.ui.configbool('convert', 'localtime', False):
>> +            date = util.makedate()
>> +        else:
>> +            date = (util.makedate()[0], 0)
>> +        date = "%d %d" % date
>
>
> If only cvs and svn need a change in behaviour, it's a sign that you're
> doing something wrong if you're patching completely unrelated code.

It's not entirely unrelated (again see below).

>> -def makedate():
>> -    ct = time.time()
>> +def makedate(ct=time.time()):
>
>
> I'm afraid this is both ugly and wrong.
>
> Ugly because it adds a parameter that is only used by the convert
> extension, and only by svn and cvs conversion. As mentioned above, don't do
> this. If you need a custom date frobber for convert, do it in convert.
>
> Wrong because now the default date and time returned by makedate is set at
> the time the module is loaded by Python, which means that for any command
> that takes more than a fraction of a second to run, this will give wrong
> answers if called repeatedly.

Created a replacement function in hgext/convert/common.py called
makedatetimestamp() for this and reverted change to util.makedate().

>> +Check localtime option.
>
>
> Just fold this into an existing convert call within the same test file.
>
>>
>> +++ b/tests/test-convert-svn-source.t
>>
>> +Check localtime option.
>
>
> Ditto.

OK, done.

---

Regarding the change to hgext/convert/hg.py in the previous patch:

I changed this file since convert adds an extra changeset when
the source is cvs and when running it says it is "updating tags".
The extra changeset is added with a Zulu time zone:

    $ hg glog -R nolocaltime --template '{rev} {desc|firstline} {date|date}\n'
    o  7 update tags * +0000 (glob)
    ...

I'm perfectly fine with reverting my change since the changeset really
doesn't come from the caller's changesets which originated in the
local time zone, but at the time I wrote the patch it seemed like
this was another place where the local time could be used.