{"thread":{"id":"28391","subject":"Fwd: vcs-svn and friends","startedAt":"2011-09-15T01:53:53Z","lastAt":"2011-09-23T18:45:51Z","messageCount":10,"participants":["David Michael Barr","Jonathan Nieder","Dmitry Ivankov","Stephen Bash"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"175532","messageId":"CAFfmPPOBZ6cXG51mDHbj2VRDzjvH46Q7=_LvUWeMq0SGR40S1g@mail.gmail.com","threadId":"28391","inReplyTo":null,"subject":"Fwd: vcs-svn and friends","fromName":"David Michael Barr","fromEmail":"davidbarr@google.com","sentAt":"2011-09-15T01:53:53Z","receivedAt":"2011-09-15T01:53:53Z","isPatch":false,"sender":{"key":"davidbarr@google.com","avatar":"https://avatars.githubusercontent.com/u/220594?v=4"},"body":"Hi,\n\nThanks to the work of Dmitry, we now have a simple front-end\nthat exercises the yet unmerged changes to vcs-svn that Jonathan\nand I authored a few months ago. I think there's still some work\nto be done before we can bless an integrated branch for inclusion.\nI'd like to bring attention to just how far we have diverged; see the\nemail below.\n\n--\nDavid Barr\n\n---------- Forwarded message ----------\nFrom: David Michael Barr \nDate: Thu, Sep 15, 2011 at 11:37 AM\nSubject: Re: vcs-svn and friends\nTo: Dmitry Ivankov \nCc: Jonathan Nieder \n\n\nWe now have 56 interesting commits pending:\n\nvcs-svn: add fast_export_note to create notes\nvcs-svn,svn-fe: add --incremental option\nvcs-svn,svn-fe: allow to disable 'progress' lines\nvcs-svn,svn-fe: convert REPORT_FILENO to an option\nvcs-svn,svn-fe: allow to specify dump destination ref\nvcs-svn: move commit parameters logic to svndump.c\nvcs-svn: make svndump_init parameters a struct\nsvn-fe,test-svn-fe: use parse-options\nfast-import: allow top directory as an argument for some commands\nfast-import: be saner with temporary trees\nsvn-fe: reuse import-marks in remote-svn-alpha\nsvn-fe: import incrementally in svn-remote-alpha\nsvn-fe: write svnrev notes in remote-svn-alpha\nsvn-fe: use proper refspec in remote-svn-alpha\nsvn-fe: use svn-fe --no-progress in remote-svn-alpha\nsvn-fe: add a test for remote-svn-alpha\nsvn-fe: allow svnadmin instead of svnrdump in remote-svn-alpha\nsvn-fe: avoid error on no-op imports in remote-svn-alpha\nsvn-fe: add man target to Makefile\nsvn-fe: use svnrdump --quiet in remote-svn-alpha\nvcs-svn: reset first_commit_done in fast_export_init\nsvn-fe: use tabs to indent in remote helper script\nsvn-fe: do not rely on /bin/env utility to launch remote helper\nAdd alpha version of remote-svn helper\nArrange a backflow pipe from fast-importer to remote helper stdin\nvcs-svn: do not initialize report_buffer twice\nvcs-svn: avoid hangs from corrupt deltas\nvcs-svn: guard against overflow when computing preimage length\nvcs-svn: cap number of bytes read from sliding view\ntest-svn-fe: split off \"test-svn-fe -d\" into a separate function\nvcs-svn: implement text-delta handling\nvcs-svn: let deltas use data from preimage\nvcs-svn: let deltas use data from postimage\nvcs-svn: verify that deltas consume all inline data\nvcs-svn: implement copyfrom_data delta instruction\nvcs-svn: read instructions from deltas\nvcs-svn: read inline data from deltas\nvcs-svn: read the preimage when applying deltas\nvcs-svn: parse svndiff0 window header\nvcs-svn: skeleton of an svn delta parser\nvcs-svn: make buffer_read_binary API more convenient\nvcs-svn: learn to maintain a sliding view of a file\nMakefile: list one vcs-svn/xdiff object or header per line\nvcs-svn: avoid using ls command twice\nvcs-svn: drop obj_pool\nvcs-svn: drop treap\nvcs-svn: drop string_pool\nvcs-svn: pass paths through to fast-import\nvcs-svn: use mark from previous import for parent commit\nvcs-svn: handle filenames with dq correctly\nvcs-svn: quote paths correctly for ls command\nvcs-svn: eliminate repo_tree structure\nvcs-svn: add a comment before each commit\nvcs-svn: save marks for imported commits\nvcs-svn: use higher mark numbers for blobs\nvcs-svn: set up channel to read fast-import cat-blob response\n\nThere are a lot of svn-fe tests failing on my integration branch.\nOne upside is that gph/master..db/svn-fe-pu only contains\nrelevant commits. I think a little more polish is needed before\nwe can suggest a pull to jch. In particular, I think we should\ninclude remote-svn-alpha and the test should work out of the\nbox.\n\n--\nDavid Barr\n"},{"id":"175567","messageId":"20110915100106.GB2328@elie","threadId":"28391","inReplyTo":"CAFfmPPOBZ6cXG51mDHbj2VRDzjvH46Q7=_LvUWeMq0SGR40S1g@mail.gmail.com","subject":"Re: Fwd: vcs-svn and friends","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-09-15T10:01:06Z","receivedAt":"2011-09-15T10:01:06Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"David Michael Barr wrote:\n\n> Thanks to the work of Dmitry, we now have a simple front-end\n> that exercises the yet unmerged changes to vcs-svn that Jonathan\n> and I authored a few months ago. I think there's still some work\n> to be done before we can bless an integrated branch for inclusion.\n> I'd like to bring attention to just how far we have diverged; see the\n> email below.\n\nQuick thoughts:\n\n - everything up to c5bcbcdc looks good to me (as you might expect)\n\n - later patches seem to be missing your sign-off. Is this deliberate\n (as in: \"withholding sign-off as a hint that these haven't received\n their final review yet\") or an oversight?\n\n - f4472ae61 (\"fast-import: be saner with temporary trees\"): when I\n looked it over in $gmane/178043, I acked the patch, but not the\n change description. Re-reading the patch, I've completely forgotten\n what it does and the commit message doesn't help. What user-visible\n effect would the patch have, if any?\n\n Reading over $gmane/178043, I learn:\n\n - new_tree_entry() returns a tree entry from a stack of trees used\n as temporaries. Initializing them before use is indeed the\n caller's responsibility.\n\n - parse_ls() uses the following idiom to retrieve content named by\n a tree-ish and path within it:\n\n\tstruct tree_entry result = {0};\n\tstruct tree_entry *root;\n\n\troot = new_tree_entry();\n\thashcpy(root->versions[1].sha1, treeish_name);\n\tload_tree(root);\n\ttree_content_get(root, path, &result);\n\trelease_tree_entry(root);\n\n This method that populates \"root\" only to free it moments later\n is somewhat wasteful --- it would be nicer to stop parsing each\n tree when the appropriate entry is found, which would speed up\n commands in the input stream like\n\n\tls 78a7c87aabc78acb7887c89a98c87ca87ca8ca89 a/a/a/a/a/a/a\n\n when the relevant trees have many entries. Oh well.\n\n This patch is about a detail in that sequence --- the temporary\n tree entry \"root\" just mentioned has uninitialized fields, such\n as versions[0].sha1. Nobody accesses them, though, and the result\n from tree_content_get() which is the important thing has no\n uninitialized fields. So this patch is about futureproofing or\n code clarity rather than an actual functional change.\n\n Would it be possible to suggest a new change description that\n clarifies that?\n\n - 3bba32e9 (\"fast-import: allow top directory as an argument for some\n commands\"): I'm not sure what the motivation is --- is this just\n about the principle of least surprise, or did it come up in practice\n somewhere?\n\n The change description could use some examples and a reference to\n the earlier related work it seems to be inspired by (\"fast-import:\n Allow filemodify to set the root\"). It would also be nice to\n update the manpage to document the change at the same time.\n\n - e9e480e7 (\"vcs-svn,svn-fe: convert REPORT_FILENO to an option\") has\n nested quote marks in the test.\n\n The motivating comment \"Moreover it may require noticeable effort\n to setup this descriptor, if number 3 is already taken for example\"\n is unjustified --- system(\"foo 3>wherever\") or { fork();\n dup2(wherever, 3); execlp(\"foo\", ...) } does not look noticeably\n difficult to me, though maybe there is some unexplained detail that\n makes this require more effort in some circumstance.\n\nOk, I notice I am starting to nitpick. Better to make a global\ncomment: the change descriptions do not currently motivate each change\nin a straightforward way. I'd be glad to help with that by providing\nfeedback and examples where appropriate if help is needed. I believe\nthat fixing this can make other pieces that need fixing easier to find\nwhen they exist.\n\nThanks for your help,\nJonathan\n"},{"id":"175578","messageId":"CA+gfSn9KVN2iDCevd0s+TjYHNupDez8NiKZycP3pgBCkYiraFQ@mail.gmail.com","threadId":"28391","inReplyTo":"20110915100106.GB2328@elie","subject":"Re: Fwd: vcs-svn and friends","fromName":"Dmitry Ivankov","fromEmail":"divanorama@gmail.com","sentAt":"2011-09-15T13:00:04Z","receivedAt":"2011-09-15T13:00:04Z","isPatch":false,"sender":{"key":"divanorama@gmail.com","avatar":"https://avatars.githubusercontent.com/u/158999?v=4"},"body":">  - 3bba32e9 (\"fast-import: allow top directory as an argument for some\n>   commands\"): I'm not sure what the motivation is --- is this just\n>   about the principle of least surprise, or did it come up in practice\n>   somewhere?\n(to ease one's reading, commands are ls, copy and move top directory)\n\nHaven't seen them in practice. It seemed possible with svn import: if there were\nno branches at start, and then someone did svn mv . trunk. But it\nturns out that my\nsvn client doesn't allow such move. So more like a least surprise purpose.\n"},{"id":"175580","messageId":"27874151.17649.1316095596748.JavaMail.root@mail.hq.genarts.com","threadId":"28391","inReplyTo":"CAFfmPPOBZ6cXG51mDHbj2VRDzjvH46Q7=_LvUWeMq0SGR40S1g@mail.gmail.com","subject":"Re: vcs-svn and friends","fromName":"Stephen Bash","fromEmail":"bash@genarts.com","sentAt":"2011-09-15T14:06:36Z","receivedAt":"2011-09-15T14:06:36Z","isPatch":false,"sender":{"key":"bash@genarts.com","avatar":null},"body":"----- Original Message -----\n> From: \"David Michael Barr\" \n> Sent: Wednesday, September 14, 2011 9:53:53 PM\n> Subject: Fwd: vcs-svn and friends\n> \n> Thanks to the work of Dmitry, we now have a simple front-end\n> that exercises the yet unmerged changes to vcs-svn that Jonathan\n> and I authored a few months ago.\n\nFor those of us interested but out of the loop, does this mean you have a working example where I can point it at a SVN repo and see what happens? Having done our SVN to Git conversion last year, I know our repo has a lot of the common SVN screw cases (non-branching copies, partial merges, mis-merges, *lots* of retagging, changes committed to tags, etc.) so if it's relatively easy to setup a test I'm happy to run one.\n\nThanks,\nStephen\n"},{"id":"175605","messageId":"20110915204815.GA31508@elie","threadId":"28391","inReplyTo":"27874151.17649.1316095596748.JavaMail.root@mail.hq.genarts.com","subject":"Re: vcs-svn and friends","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-09-15T20:48:15Z","receivedAt":"2011-09-15T20:48:15Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Stephen Bash wrote:\n\n> For those of us interested but out of the loop, does this mean you\n> have a working example where I can point it at a SVN repo and see\n> what happens? Having done our SVN to Git conversion last year, I\n> know our repo has a lot of the common SVN screw cases (non-branching\n> copies, partial merges, mis-merges, *lots* of retagging, changes\n> committed to tags, etc.) so if it's relatively easy to setup a test\n> I'm happy to run one.\n\nThanks. It's very bare-bones at the moment: it just imports each\nrevision as a whole tree, with no branch and merge tracking at all.\nSo it would be very interesting to get this basic stuff out into the\nwild and then add some code implementing those things for you to break\non top of it.\n"},{"id":"175618","messageId":"3435337.18574.1316121232672.JavaMail.root@mail.hq.genarts.com","threadId":"28391","inReplyTo":"20110915204815.GA31508@elie","subject":"Re: vcs-svn and friends","fromName":"Stephen Bash","fromEmail":"bash@genarts.com","sentAt":"2011-09-15T21:13:52Z","receivedAt":"2011-09-15T21:13:52Z","isPatch":false,"sender":{"key":"bash@genarts.com","avatar":null},"body":"----- Original Message -----\n> From: \"Jonathan Nieder\" \n> Sent: Thursday, September 15, 2011 4:48:15 PM\n> Subject: Re: vcs-svn and friends\n>\n> Stephen Bash wrote:\n> \n> > For those of us interested but out of the loop, does this mean you\n> > have a working example where I can point it at a SVN repo and see\n> > what happens? Having done our SVN to Git conversion last year, I\n> > know our repo has a lot of the common SVN screw cases (non-branching\n> > copies, partial merges, mis-merges, *lots* of retagging, changes\n> > committed to tags, etc.) so if it's relatively easy to setup a test\n> > I'm happy to run one.\n> \n> Thanks. It's very bare-bones at the moment: it just imports each\n> revision as a whole tree, with no branch and merge tracking at all.\n> So it would be very interesting to get this basic stuff out into the\n> wild and then add some code implementing those things for you to break\n> on top of it.\n\nOkay, that matches my last known state of the project, guess I just got optimistic ;) \n\nSince last summer I've been meaning to take a crack at svn-filter-root.py, but it keeps sliding off the priority list. If you guys don't beat me to it, maybe I'll eventually get around to it...\n\nThanks,\nStephen\n"},{"id":"175968","messageId":"20110921234008.GA23439@elie","threadId":"28391","inReplyTo":"CA+gfSn9KVN2iDCevd0s+TjYHNupDez8NiKZycP3pgBCkYiraFQ@mail.gmail.com","subject":"Re: Fwd: vcs-svn and friends","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-09-21T23:40:08Z","receivedAt":"2011-09-21T23:40:08Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Dmitry Ivankov wrote:\n\n>>  - 3bba32e9 (\"fast-import: allow top directory as an argument for some\n>>   commands\"): I'm not sure what the motivation is --- is this just\n>>   about the principle of least surprise, or did it come up in practice\n>>   somewhere?\n>\n> (to ease one's reading, commands are ls, copy and move top directory)\n>\n> Haven't seen them in practice. It seemed possible with svn import: if there were\n> no branches at start, and then someone did svn mv . trunk. But it\n> turns out that my\n> svn client doesn't allow such move. So more like a least surprise purpose.\n\nWith that information in mind, it sounds like a reasonable change in\nprinciple (though I haven't looked over the code at all). Could you\npropose a log message explaining it (both the original motivation and\nthe actual impact)?\n\nThanks,\nJonathan\n"},{"id":"176062","messageId":"CA+gfSn8UeqiLu3trecPzzqSLsUr3eNT0yVN=-991sL6oJHar2g@mail.gmail.com","threadId":"28391","inReplyTo":"CA+gfSn9KVN2iDCevd0s+TjYHNupDez8NiKZycP3pgBCkYiraFQ@mail.gmail.com","subject":"Re: Fwd: vcs-svn and friends","fromName":"Dmitry Ivankov","fromEmail":"divanorama@gmail.com","sentAt":"2011-09-23T13:27:27Z","receivedAt":"2011-09-23T13:27:27Z","isPatch":false,"sender":{"key":"divanorama@gmail.com","avatar":"https://avatars.githubusercontent.com/u/158999?v=4"},"body":"On Thu, Sep 15, 2011 at 7:00 PM, Dmitry Ivankov wrote:\n>>  - 3bba32e9 (\"fast-import: allow top directory as an argument for some\n>>   commands\"): I'm not sure what the motivation is --- is this just\n>>   about the principle of least surprise, or did it come up in practice\n>>   somewhere?\n> (to ease one's reading, commands are ls, copy and move top directory)\n>\n> Haven't seen them in practice. It seemed possible with svn import: if there were\n> no branches at start, and then someone did svn mv . trunk. But it\n> turns out that my\n> svn client doesn't allow such move. So more like a least surprise purpose.\nI think now that this commit should go separately if at all.\nEspecially considering\nmy other activity on fast-import (and thus possible merge conflicts) that isn't\nstrictly necessary for vcs-svn and friends.\n"},{"id":"176063","messageId":"CA+gfSn_OVBB16pvyWtARzp_9=WPu+Q0WPM=h=4zEePH_WnW4pA@mail.gmail.com","threadId":"28391","inReplyTo":"CA+gfSn8UeqiLu3trecPzzqSLsUr3eNT0yVN=-991sL6oJHar2g@mail.gmail.com","subject":"Re: Fwd: vcs-svn and friends","fromName":"Dmitry Ivankov","fromEmail":"divanorama@gmail.com","sentAt":"2011-09-23T13:29:20Z","receivedAt":"2011-09-23T13:29:20Z","isPatch":false,"sender":{"key":"divanorama@gmail.com","avatar":"https://avatars.githubusercontent.com/u/158999?v=4"},"body":"On Fri, Sep 23, 2011 at 7:27 PM, Dmitry Ivankov wrote:\n> On Thu, Sep 15, 2011 at 7:00 PM, Dmitry Ivankov wrote:\n>>>  - 3bba32e9 (\"fast-import: allow top directory as an argument for some\n>>>   commands\"): I'm not sure what the motivation is --- is this just\n>>>   about the principle of least surprise, or did it come up in practice\n>>>   somewhere?\n>> (to ease one's reading, commands are ls, copy and move top directory)\n>>\n>> Haven't seen them in practice. It seemed possible with svn import: if there were\n>> no branches at start, and then someone did svn mv . trunk. But it\n>> turns out that my\n>> svn client doesn't allow such move. So more like a least surprise purpose.\n> I think now that this commit should go separately if at all.\n> Especially considering\n> my other activity on fast-import (and thus possible merge conflicts) that isn't\n> strictly necessary for vcs-svn and friends.\nAnd the same for 3bba32e9^ \"fast-import: be saner with temporary trees\", which\nis only needed for 3bba32e9.\n"},{"id":"176085","messageId":"20110923184550.GA12721@elie","threadId":"28391","inReplyTo":"CA+gfSn8UeqiLu3trecPzzqSLsUr3eNT0yVN=-991sL6oJHar2g@mail.gmail.com","subject":"Re: Fwd: vcs-svn and friends","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-09-23T18:45:51Z","receivedAt":"2011-09-23T18:45:51Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Dmitry Ivankov wrote:\n> On Thu, Sep 15, 2011 at 7:00 PM, Dmitry Ivankov wrote:\n\n>> Haven't seen them in practice. It seemed possible with svn import:\n>> if there were no branches at start, and then someone did svn mv .\n>> trunk. But it turns out that my svn client doesn't allow such\n>> move. So more like a least surprise purpose.\n>\n> I think now that this commit should go separately if at all.\n> Especially considering my other activity on fast-import (and thus\n> possible merge conflicts) that isn't strictly necessary for vcs-svn\n> and friends.\n\nWell, I think it's a good patch, and merge conflicts are really not a\nbig deal when one has a good patch, especially when it's clear how to\ntest that the tool still works after resolving. It's just missing a\nclear description...\n"}]}