Page MenuHomePhabricator

convert: set date and time for svn commits
ClosedPublic

Authored by nslsrv on Fri, Jan 8, 6:40 PM.

Details

Summary

Committing to subversion repository is changed to use commit dates from
the source. Subversion always uses commit dates with UTC timezone, so only
timestamps are used.

Test test-convert-svn-sink.t uses svnxml.py script to dump history of svn
repositories. Atm the script is not printing date field from svn log. This
patch changes this to allow checks on correctness of date and time convertion.

Diff Detail

Repository
rHG Mercurial
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

nslsrv created this revision.Fri, Jan 8, 6:40 PM
pulkit added a subscriber: pulkit.Sat, Jan 9, 5:18 AM
pulkit added inline comments.
tests/test-convert-svn-sink.t
57

It seems to me that the seconds part of this timestamp can change depending how loaded the machine is where tests are run. This will result in flaky tests. Hence, all these occurrences should be replaced with * (glob)[1] patterns.

[1]: grep-ing for (glob) in tests/ should help you.

nslsrv added inline comments.Sat, Jan 9, 5:28 AM
tests/test-convert-svn-sink.t
57

I thought about it, but I believe that commit times in this test are set in a deterministic way. See line 36:

hg --cwd a ci -d '1 0' -m 'modify a file'

Date is set to be "1 second past the beginning of time", and svn date is just a representation of what time is specified in hg commit.

pulkit accepted this revision.Sat, Jan 9, 5:40 AM

Queued this, many thanks for the patch.

tests/test-convert-svn-sink.t
57

Yeah, seems like we are covered.

This revision is now accepted and ready to land.Sat, Jan 9, 5:40 AM
nslsrv marked 2 inline comments as done.Sat, Jan 9, 5:50 AM
This revision was automatically updated to reflect the committed changes.

This is wrong. Subversion does not, by default, allow you to propset things like the date unless you're the repo administrator, and even if it does, you shouldn't do that. LOTS of things in Subversion have hard dependencies on the commit timestamps monotonically increasing, so this isn't something we should ever attempt.

I'll drop this patch.

This is wrong. Subversion does not, by default, allow you to propset things like the date unless you're the repo administrator, and even if it does, you shouldn't do that. LOTS of things in Subversion have hard dependencies on the commit timestamps monotonically increasing, so this isn't something we should ever attempt.
I'll drop this patch.

Hi, Augie. Indeed, dy default subversion allows to change only svn:log and doesn't even allow for custom revprops setting. But I saw (and used) a number of svn repos with commit dates not increasing monotonically.
Could you please provide an example of effects one can face when this contract is broken? I can only think of date-range log (i.e. svn log -r {2008-09-19}:{2008-09-26}) which I can cofirm is broken in these cases. But other than that – I'm curious: have you faced any other issues?

Anyway, as non-monotonical svn repos are present, and this patch was made specifically to convert dates as is – I suggest to include behaviour I proposed under a configuration option, set to false by default of course.

Or, a bit more complex solution here: to allow dates translation, but only if dates are increasing. And an option to override the restriction if the user is ok with arbitrary dates.

Sent new revision of these changes here: D9721

marmoute updated this revision to Diff 24723.Tue, Jan 12, 5:38 PM