| pulkit |
| hg-reviewers |
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.
| No Linters Available |
| No Unit Test Coverage |
| 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. | |
| 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. | |
Queued this, many thanks for the patch.
| tests/test-convert-svn-sink.t | ||
|---|---|---|
| 57 | Yeah, seems like we are covered. | |
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.