This is an archive of the discontinued Mercurial Phabricator instance.

Differential D9715

convert: set date and time for svn commits
ClosedPublic

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

Details

Reviewers
pulkit
Group Reviewers
hg-reviewers
Commits
rHG3dc886e66f07: convert: set date and time for svn commits
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.Jan 8 2021, 6:40 PM
pulkit added a subscriber: pulkit.Jan 9 2021, 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.Jan 9 2021, 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.Jan 9 2021, 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.Jan 9 2021, 5:40 AM
nslsrv marked 2 inline comments as done.Jan 9 2021, 5:50 AM
nslsrv added a commit: rHG3dc886e66f07: convert: set date and time for svn commits.Jan 9 2021, 6:12 AM
Closed by commit rHG3dc886e66f07: convert: set date and time for svn commits (authored by nslsrv). · Explain Why
This revision was automatically updated to reflect the committed changes.
durin42 added a subscriber: durin42.Jan 11 2021, 2:04 PM

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.

nslsrv added a comment.Jan 11 2021, 2:57 PM
In D9715#146745, @durin42 wrote:

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.

nslsrv added a comment.Jan 11 2021, 5:56 PM

Sent new revision of these changes here: D9721

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

Revision Contents
Changeset List

Diff 24723

hgext/convert/subversion.py

REPOS="$1" REPOS="$1"
REV="$2" REV="$2"
USER="$3" USER="$3"
PROPNAME="$4" PROPNAME="$4"
ACTION="$5" ACTION="$5"
if [ "$ACTION" = "M" -a "$PROPNAME" = "svn:log" ]; then exit 0; fi if [ "$ACTION" = "M" -a "$PROPNAME" = "svn:log" ]; then exit 0; fi
if [ "$ACTION" = "M" -a "$PROPNAME" = "svn:date" ]; then exit 0; fi
if [ "$ACTION" = "A" -a "$PROPNAME" = "hg:convert-branch" ]; then exit 0; fi if [ "$ACTION" = "A" -a "$PROPNAME" = "hg:convert-branch" ]; then exit 0; fi
if [ "$ACTION" = "A" -a "$PROPNAME" = "hg:convert-rev" ]; then exit 0; fi if [ "$ACTION" = "A" -a "$PROPNAME" = "hg:convert-rev" ]; then exit 0; fi
echo "Changing prohibited revision property" >&2 echo "Changing prohibited revision property" >&2
exit 1 exit 1
''' '''
if self.setexec: if self.setexec:
self.xargs(self.setexec, b'propset', b'svn:executable', b'*') self.xargs(self.setexec, b'propset', b'svn:executable', b'*')
self.setexec = [] self.setexec = []
fd, messagefile = pycompat.mkstemp(prefix=b'hg-convert-') fd, messagefile = pycompat.mkstemp(prefix=b'hg-convert-')
fp = os.fdopen(fd, 'wb') fp = os.fdopen(fd, 'wb')
fp.write(util.tonativeeol(commit.desc)) fp.write(util.tonativeeol(commit.desc))
fp.close() fp.close()
# Subverson always uses UTC to represent date and time
date = dateutil.parsedate(commit.date)
svndate = dateutil.datestr((date[0], 0), b'%Y-%m-%dT%H:%M:%S.000000Z')
try: try:
output = self.run0( output = self.run0(
b'commit', b'commit',
username=stringutil.shortuser(commit.author), username=stringutil.shortuser(commit.author),
file=messagefile, file=messagefile,
encoding=b'utf-8', encoding=b'utf-8',
) )
try: try:
if commit.branch and commit.branch != b'default': if commit.branch and commit.branch != b'default':
self.run( self.run(
b'propset', b'propset',
b'hg:convert-branch', b'hg:convert-branch',
commit.branch, commit.branch,
revprop=True, revprop=True,
revision=rev, revision=rev,
) )
# The only way to set date and time for svn commit is to use propset after commit is done
self.run(
b'propset',
b'svn:date',
svndate,
revprop=True,
revision=rev,
)
for parent in parents: for parent in parents:
self.addchild(parent, rev) self.addchild(parent, rev)
return self.revid(rev) return self.revid(rev)
finally: finally:
os.unlink(messagefile) os.unlink(messagefile)
def puttags(self, tags): def puttags(self, tags):
self.ui.warn(_(b'writing Subversion tags is not yet implemented\n')) self.ui.warn(_(b'writing Subversion tags is not yet implemented\n'))

tests/svnxml.py

return ''.join(c.data for c in e.childNodes if c.nodeType == c.TEXT_NODE) return ''.join(c.data for c in e.childNodes if c.nodeType == c.TEXT_NODE)
def parseentry(entry): def parseentry(entry):
e = {} e = {}
e['revision'] = entry.getAttribute('revision') e['revision'] = entry.getAttribute('revision')
e['author'] = xmltext(entry.getElementsByTagName('author')[0]) e['author'] = xmltext(entry.getElementsByTagName('author')[0])
e['msg'] = xmltext(entry.getElementsByTagName('msg')[0]) e['msg'] = xmltext(entry.getElementsByTagName('msg')[0])
e['date'] = xmltext(entry.getElementsByTagName('date')[0])
e['paths'] = [] e['paths'] = []
paths = entry.getElementsByTagName('paths') paths = entry.getElementsByTagName('paths')
if paths: if paths:
paths = paths[0] paths = paths[0]
for p in paths.getElementsByTagName('path'): for p in paths.getElementsByTagName('path'):
action = p.getAttribute('action').encode('utf-8') action = p.getAttribute('action').encode('utf-8')
path = xmltext(p).encode('utf-8') path = xmltext(p).encode('utf-8')
frompath = p.getAttribute('copyfrom-path').encode('utf-8') frompath = p.getAttribute('copyfrom-path').encode('utf-8')
def printentries(entries): def printentries(entries):
try: try:
fp = sys.stdout.buffer fp = sys.stdout.buffer
except AttributeError: except AttributeError:
fp = sys.stdout fp = sys.stdout
for e in entries: for e in entries:
for k in ('revision', 'author', 'msg'): for k in ('revision', 'author', 'date', 'msg'):
fp.write(('%s: %s\n' % (k, e[k])).encode('utf-8')) fp.write(('%s: %s\n' % (k, e[k])).encode('utf-8'))
for path, action, fpath, frev in sorted(e['paths']): for path, action, fpath, frev in sorted(e['paths']):
frominfo = b'' frominfo = b''
if frev: if frev:
frominfo = b' (from %s@%s)' % (fpath, frev) frominfo = b' (from %s@%s)' % (fpath, frev)
p = b' %s %s%s\n' % (action, path, frominfo) p = b' %s %s%s\n' % (action, path, frominfo)
fp.write(p) fp.write(p)
if __name__ == '__main__': if __name__ == '__main__':
data = sys.stdin.read() data = sys.stdin.read()
entries = parselog(data) entries = parselog(data)
printentries(entries) printentries(entries)

tests/test-convert-hg-svn.t

> >
> REPOS="$1" > REPOS="$1"
> REV="$2" > REV="$2"
> USER="$3" > USER="$3"
> PROPNAME="$4" > PROPNAME="$4"
> ACTION="$5" > ACTION="$5"
> >
> if [ "$ACTION" = "M" -a "$PROPNAME" = "svn:log" ]; then exit 0; fi > if [ "$ACTION" = "M" -a "$PROPNAME" = "svn:log" ]; then exit 0; fi
> if [ "$ACTION" = "M" -a "$PROPNAME" = "svn:date" ]; then exit 0; fi
> if [ "$ACTION" = "A" -a "$PROPNAME" = "hg:convert-branch" ]; then exit 0; fi > if [ "$ACTION" = "A" -a "$PROPNAME" = "hg:convert-branch" ]; then exit 0; fi
> if [ "$ACTION" = "A" -a "$PROPNAME" = "hg:convert-rev" ]; then exit 0; fi > if [ "$ACTION" = "A" -a "$PROPNAME" = "hg:convert-rev" ]; then exit 0; fi
> >
> echo "Changing prohibited revision property" >&2 > echo "Changing prohibited revision property" >&2
> exit 1 > exit 1
> EOF > EOF
$ chmod +x "$SVNREPOPATH"/hooks/pre-revprop-change $ chmod +x "$SVNREPOPATH"/hooks/pre-revprop-change
$ svn co "$SVNREPOURL" "$SVNREPOPATH"-wc $ svn co "$SVNREPOURL" "$SVNREPOPATH"-wc

tests/test-convert-svn-sink.t

$ svnupanddisplay a-hg-wc 2 $ svnupanddisplay a-hg-wc 2
2 1 test d1 2 1 test d1
2 1 test d1/d2 2 1 test d1/d2
2 1 test d1/d2/b 2 1 test d1/d2/b
2 2 test . 2 2 test .
2 2 test a 2 2 test a
revision: 2 revision: 2
author: test author: test
date: 1970-01-01T00:00:01.000000Z
pulkitUnsubmitted
Done

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.

pulkit: It seems to me that the seconds part of this timestamp can change depending how loaded the…
nslsrvAuthorUnsubmitted
Done

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.

nslsrv: I thought about it, but I believe that commit times in this test are set in a deterministic way.
pulkitUnsubmitted
Done

Yeah, seems like we are covered.

pulkit: Yeah, seems like we are covered.
msg: modify a file msg: modify a file
M /a M /a
revision: 1 revision: 1
author: test author: test
date: 1970-01-01T00:00:00.000000Z
msg: add a file msg: add a file
A /a A /a
A /d1 A /d1
A /d1/d2 A /d1/d2
A /d1/d2/b A /d1/d2/b
$ ls a a-hg-wc $ ls a a-hg-wc
a: a:
a a
$ svnupanddisplay a-hg-wc 1 $ svnupanddisplay a-hg-wc 1
3 1 test d1 3 1 test d1
3 1 test d1/d2 3 1 test d1/d2
3 1 test d1/d2/b 3 1 test d1/d2/b
3 3 test . 3 3 test .
3 3 test b 3 3 test b
revision: 3 revision: 3
author: test author: test
date: 1970-01-01T00:00:02.000000Z
msg: rename a file msg: rename a file
D /a D /a
A /b (from /a@2) A /b (from /a@2)
$ ls a a-hg-wc $ ls a a-hg-wc
a: a:
b b
d1 d1
4 1 test d1 4 1 test d1
4 1 test d1/d2 4 1 test d1/d2
4 1 test d1/d2/b 4 1 test d1/d2/b
4 3 test b 4 3 test b
4 4 test . 4 4 test .
4 4 test c 4 4 test c
revision: 4 revision: 4
author: test author: test
date: 1970-01-01T00:00:03.000000Z
msg: copy a file msg: copy a file
A /c (from /b@3) A /c (from /b@3)
$ ls a a-hg-wc $ ls a a-hg-wc
a: a:
b b
c c
d1 d1
$ svnupanddisplay a-hg-wc 1 $ svnupanddisplay a-hg-wc 1
5 1 test d1 5 1 test d1
5 1 test d1/d2 5 1 test d1/d2
5 1 test d1/d2/b 5 1 test d1/d2/b
5 4 test c 5 4 test c
5 5 test . 5 5 test .
revision: 5 revision: 5
author: test author: test
date: 1970-01-01T00:00:04.000000Z
msg: remove a file msg: remove a file
D /b D /b
$ ls a a-hg-wc $ ls a a-hg-wc
a: a:
c c
d1 d1
a-hg-wc: a-hg-wc:
$ svnupanddisplay a-hg-wc 1 $ svnupanddisplay a-hg-wc 1
6 1 test d1 6 1 test d1
6 1 test d1/d2 6 1 test d1/d2
6 1 test d1/d2/b 6 1 test d1/d2/b
6 6 test . 6 6 test .
6 6 test c 6 6 test c
revision: 6 revision: 6
author: test author: test
date: 1970-01-01T00:00:05.000000Z
msg: make a file executable msg: make a file executable
M /c M /c
#if execbit #if execbit
$ test -x a-hg-wc/c $ test -x a-hg-wc/c
#endif #endif
#if symlink #if symlink
8 1 test d1 8 1 test d1
8 1 test d1/d2 8 1 test d1/d2
8 1 test d1/d2/b 8 1 test d1/d2/b
8 6 test c 8 6 test c
8 8 test . 8 8 test .
8 8 test newlink 8 8 test newlink
revision: 8 revision: 8
author: test author: test
date: 1970-01-01T00:00:00.000000Z
msg: move symlink msg: move symlink
D /link D /link
A /newlink (from /link@7) A /newlink (from /link@7)
Make sure our changes don't affect the rest of the test cases Make sure our changes don't affect the rest of the test cases
$ hg --cwd a up 5 $ hg --cwd a up 5
0 files updated, 0 files merged, 1 files removed, 0 files unresolved 0 files updated, 0 files merged, 1 files removed, 0 files unresolved
converting... converting...
0 f 0 f
$ svnupanddisplay a-hg-wc 1 $ svnupanddisplay a-hg-wc 1
7 7 test . 7 7 test .
7 7 test d 7 7 test d
7 7 test f 7 7 test f
revision: 7 revision: 7
author: test author: test
date: 1970-01-01T00:00:00.000000Z
msg: f msg: f
D /c D /c
A /d A /d
D /d1 D /d1
A /f A /f
$ rm -rf a a-hg a-hg-wc $ rm -rf a a-hg a-hg-wc
converting... converting...
0 add executable file in new directory 0 add executable file in new directory
$ svnupanddisplay a-hg-wc 1 $ svnupanddisplay a-hg-wc 1
1 1 test . 1 1 test .
1 1 test d1 1 1 test d1
1 1 test d1/a 1 1 test d1/a
revision: 1 revision: 1
author: test author: test
date: 1970-01-01T00:00:00.000000Z
msg: add executable file in new directory msg: add executable file in new directory
A /d1 A /d1
A /d1/a A /d1/a
#if execbit #if execbit
$ test -x a-hg-wc/d1/a $ test -x a-hg-wc/d1/a
#endif #endif
Copy to new directory Copy to new directory
$ svnupanddisplay a-hg-wc 1 $ svnupanddisplay a-hg-wc 1
2 1 test d1 2 1 test d1
2 1 test d1/a 2 1 test d1/a
2 2 test . 2 2 test .
2 2 test d2 2 2 test d2
2 2 test d2/a 2 2 test d2/a
revision: 2 revision: 2
author: test author: test
date: 1970-01-01T00:00:01.000000Z
msg: copy file to new directory msg: copy file to new directory
A /d2 A /d2
A /d2/a (from /d1/a@1) A /d2/a (from /d1/a@1)
Branchy history Branchy history
$ hg init b $ hg init b
$ echo base > b/b $ echo base > b/b
4 2 test left-1 4 2 test left-1
4 3 test b 4 3 test b
4 3 test left-2 4 3 test left-2
4 4 test . 4 4 test .
4 4 test right-1 4 4 test right-1
4 4 test right-2 4 4 test right-2
revision: 4 revision: 4
author: test author: test
date: 1970-01-01T00:00:05.000000Z
msg: merge msg: merge
A /right-1 A /right-1
A /right-2 A /right-2
revision: 3 revision: 3
author: test author: test
date: 1970-01-01T00:00:02.000000Z
msg: left-2 msg: left-2
M /b M /b
A /left-2 A /left-2
revision: 2 revision: 2
author: test author: test
date: 1970-01-01T00:00:01.000000Z
msg: left-1 msg: left-1
M /b M /b
A /left-1 A /left-1
revision: 1 revision: 1
author: test author: test
date: 1970-01-01T00:00:00.000000Z
msg: base msg: base
A /b A /b
Tags are not supported, but must not break conversion Tags are not supported, but must not break conversion
$ rm -rf a a-hg a-hg-wc $ rm -rf a a-hg a-hg-wc
$ hg init a $ hg init a
$ echo a > a/a $ echo a > a/a
0 Tagged as v1.0 0 Tagged as v1.0
writing Subversion tags is not yet implemented writing Subversion tags is not yet implemented
$ svnupanddisplay a-hg-wc 2 $ svnupanddisplay a-hg-wc 2
2 1 test a 2 1 test a
2 2 test . 2 2 test .
2 2 test .hgtags 2 2 test .hgtags
revision: 2 revision: 2
author: test author: test
date: 1970-01-01T00:00:01.000000Z
msg: Tagged as v1.0 msg: Tagged as v1.0
A /.hgtags A /.hgtags
revision: 1 revision: 1
author: test author: test
date: 1970-01-01T00:00:00.000000Z
msg: Add file a msg: Add file a
A /a A /a
$ rm -rf a a-hg a-hg-wc $ rm -rf a a-hg a-hg-wc
#if execbit #if execbit
Executable bit removal Executable bit removal
converting... converting...
1 create executable 1 create executable
0 remove executable bit 0 remove executable bit
$ svnupanddisplay a-hg-wc 0 $ svnupanddisplay a-hg-wc 0
2 2 test . 2 2 test .
2 2 test exec 2 2 test exec
revision: 2 revision: 2
author: test author: test
date: 1970-01-01T00:00:02.000000Z
msg: remove executable bit msg: remove executable bit
M /exec M /exec
revision: 1 revision: 1
author: test author: test
date: 1970-01-01T00:00:01.000000Z
msg: create executable msg: create executable
A /exec A /exec
$ test ! -x a-hg-wc/exec $ test ! -x a-hg-wc/exec
$ rm -rf a a-hg a-hg-wc $ rm -rf a a-hg a-hg-wc
#endif #endif
0 Another change 0 Another change
$ svnupanddisplay a-hg-wc 0 $ svnupanddisplay a-hg-wc 0
2 1 test a 2 1 test a
2 2 test . 2 2 test .
2 2 test b 2 2 test b
revision: 2 revision: 2
author: test author: test
date: 1970-01-01T00:00:00.000000Z
msg: Another change msg: Another change
A /b A /b
revision: 1 revision: 1
author: test author: test
date: 1970-01-01T00:00:00.000000Z
msg: Some change msg: Some change
A /a A /a
$ rm -rf a a-hg a-hg-wc $ rm -rf a a-hg a-hg-wc