Re: What stands between us and branching 1.7?
On Thu, Jan 6, 2011 at 1:04 PM, C. Michael Pilato cmpil...@collab.net wrote: On 01/06/2011 03:48 PM, Stefan Küng wrote: On 06.01.2011 21:41, C. Michael Pilato wrote: I'm sorry if I asked this before -- I've been asking individual folks for over a month now, but I can't quickly find a public broadcast thread about it, at least -- but I've been wondering lately: What, exactly, stands in the way of us branching for 1.7 stabilization? ra_serf stabilization? No... that's fairly well taken care of, and would fit perfectly within in the scope of post-branch work anyway. At least on Windows, I doubt that ra_serf is even used right now. Because of the huge memory leak serf has/had (See here: http://code.google.com/p/serf/source/detail?r=1416). But even though the leak is fixed, there hasn't been another release yet. With the latest release without that fix, serf is not usable at all. To get more people to test ra_serf, serf itself first needs a new release which includes that fix. Stefan Xlnt feedback. I've noted this on our roadmap.html page. What else? We could cut a serf bug fix release that has a few fixes that Lieven committed shortly after we released 0.7.0. Greg or Lieven, any thoughts here? -- justin
Re: svn commit: r1058308 - /subversion/site/publish/docs/release-notes/1.7.html
Daniel Shahaf d...@daniel.shahaf.name writes: * The difference that's supposed to cause Philip's script to work as advertised is that svn_repos_fs_change_rev_prop4() uses the same propvalue to compute the ACTION parameter and to pass as the old propvalue to the FS for atomicity. I'm too sleepy right now to determine whether Philip's script will actually work as advertised given this server-side change, so just throwing it out there for now. The server side change hasn't been applied to the 1.6 branch AFAIK. That means that the ACTION parameter in the hook script is not reliable and so the script doesn't fix the problem. The obstacle to applying the server side fix to the 1.6 branch is that it changes the behaviour of server and so could be considered a regression, although that same behaviour change could also be considered an improvement. -- Philip
svn command line client's inconsistent behavior in handling multiple targets.
It is known that the commands which takes multiple targets does not behave in a consistent manner. I analyzed the behaviour of some of the commands, to see how they works when, multiple targets are passed and one of them is a non-existing target. Also various svn commands return inconsistent codes to the shell (See one such issue http://subversion.tigris.org/issues/show_bug.cgi?id=3713). Commands are executed with three existing targets 1.txt, 2.txt and 3.txt along with non-existing target 4.txt. ** add: noorul@noorul:/tmp/wc/repo1.7$ ~/projects/subversion/builds/trunk/bin/svn add 1.tx t 4.txt 2.txt 3.txt A 1.txt svn: warning: '/tmp/wc/repo1.7/4.txt' not found A 2.txt A 3.txt noorul@noorul:/tmp/wc/repo1.7$ echo $? 0 ** blame: noorul@noorul:/tmp/wc/repo1.7$ ~/projects/subversion/builds/trunk/bin/svn blame 1. txt 4.txt 2.txt 3.txt 2 noorul Adding one line svn: The node '/tmp/wc/repo1.7/4.txt' was not found. noorul@noorul:/tmp/wc/repo1.7$ echo $? 1 ** cat: noorul@noorul:/tmp/wc/repo1.7$ ~/projects/subversion/builds/trunk/bin/svn cat 1.tx t 4.txt 2.txt 3.txt Adding one line svn: warning: '/tmp/wc/repo1.7/4.txt' is not under version control noorul@noorul:/tmp/wc/repo1.7$ echo $? 0 ** changelist: noorul@noorul:/tmp/wc/repo1.7$ ~/projects/subversion/builds/trunk/bin/svn cl testl ist 1.txt 4.txt 2.txt 3.txt A [testlist] 1.txt svn: warning: The node '/tmp/wc/repo1.7/4.txt' was not found. noorul@noorul:/tmp/wc/repo1.7$ echo $? 0 ** commit: Changes in 1.txt and 2.txt noorul@noorul:/tmp/wc/repo1.7$ ~/projects/subversion/builds/trunk/bin/svn ci 1.txt 4.txt 2.txt 3.txt -m Commit svn: Commit failed (details follow): svn: '/tmp/wc/repo1.7/4.txt' is not under version control noorul@noorul:/tmp/wc/repo1.7$ echo $? 1 ** copy: noorul@noorul:/tmp/wc/repo1.7$ ~/projects/subversion/builds/trunk/bin/svn copy 1.t xt 4.txt 2.txt 3.txt temp svn: Path '/tmp/wc/repo1.7/4.txt' does not exist noorul@noorul:/tmp/wc/repo1.7$ echo $? 1 ** delete: noorul@noorul:/tmp/wc/repo1.7$ ~/projects/subversion/builds/trunk/bin/svn del 1.tx t 4.txt 2.txt 3.txt D 1.txt svn: '/tmp/wc/repo1.7/4.txt' does not exist noorul@noorul:/tmp/wc/repo1.7$ echo $? 1 ** diff: noorul@noorul:/tmp/wc/repo1.7$ ~/projects/subversion/builds/trunk/bin/svn diff 1.t xt 4.txt 2.txt 3.txt -c 2 Index: 1.txt === --- 1.txt (revision 1) +++ 1.txt (revision 2) @@ -0,0 +1 @@ +Adding one line svn: The node '/tmp/wc/repo1.7/4.txt' was not found. noorul@noorul:/tmp/wc/repo1.7$ echo $? 1 ** info: noorul@noorul:/tmp/wc/repo1.7$ ~/projects/subversion/builds/trunk/bin/svn info 1.t xt 4.txt 2.txt 3.txt Path: 1.txt Name: 1.txt Working Copy Root Path: /tmp/wc/repo1.7 URL: file:///tmp/repo1.7/1.txt Repository Root: file:///tmp/repo1.7 Repository UUID: 25579947-ca24-4ba8-ad04-23900ccc89dc Revision: 5 Node Kind: file Schedule: normal Last Changed Author: noorul Last Changed Rev: 3 Last Changed Date: 2011-01-11 12:05:08 +0530 (Tue, 11 Jan 2011) Text Last Updated: 2011-01-11 12:35:28 +0530 (Tue, 11 Jan 2011) Checksum: dd3cee37d491d7690669eb9a5f2c3744bd851253 svn: warning: The node '/tmp/wc/repo1.7/4.txt' was not found. Path: 2.txt Name: 2.txt Working Copy Root Path: /tmp/wc/repo1.7 URL: file:///tmp/repo1.7/2.txt Repository Root: file:///tmp/repo1.7 Repository UUID: 25579947-ca24-4ba8-ad04-23900ccc89dc Revision: 5 Node Kind: file Schedule: normal Last Changed Author: noorul Last Changed Rev: 3 Last Changed Date: 2011-01-11 12:05:08 +0530 (Tue, 11 Jan 2011) Text Last Updated: 2011-01-11 12:03:10 +0530 (Tue, 11 Jan 2011) Checksum: 9b22e741d575bdad23521c3cdbdbf93c818291ee Path: 3.txt Name: 3.txt Working Copy Root Path: /tmp/wc/repo1.7 URL: file:///tmp/repo1.7/3.txt Repository Root: file:///tmp/repo1.7 Repository UUID: 25579947-ca24-4ba8-ad04-23900ccc89dc Revision: 5 Node Kind: file Schedule: normal Last Changed Author: noorul Last Changed Rev: 1 Last Changed Date: 2011-01-11 11:51:39 +0530 (Tue, 11 Jan 2011) Text Last Updated: 2011-01-11 11:47:29 +0530 (Tue, 11 Jan 2011) Checksum: da39a3ee5e6b4b0d3255bfef95601890afd80709 svn: A problem occurred; see other errors for details noorul@noorul:/tmp/wc/repo1.7$ echo $? 1 ** list: noorul@noorul:/tmp/wc/repo1.7$ ~/projects/subversion/builds/trunk/bin/svn ls 1.txt 4.txt 2.txt 3.txt 1.txt svn: The node '/tmp/wc/repo1.7/4.txt' was not found. noorul@noorul:/tmp/wc/repo1.7$ echo $? 1 ** log: URL
Re: svn commit: r1058308 - /subversion/site/publish/docs/release-notes/1.7.html
On Thu, Jan 13, 2011 at 09:31:36AM +, Philip Martin wrote: Daniel Shahaf d...@daniel.shahaf.name writes: * The difference that's supposed to cause Philip's script to work as advertised is that svn_repos_fs_change_rev_prop4() uses the same propvalue to compute the ACTION parameter and to pass as the old propvalue to the FS for atomicity. I'm too sleepy right now to determine whether Philip's script will actually work as advertised given this server-side change, so just throwing it out there for now. The server side change hasn't been applied to the 1.6 branch AFAIK. That means that the ACTION parameter in the hook script is not reliable and so the script doesn't fix the problem. The obstacle to applying the server side fix to the 1.6 branch is that it changes the behaviour of server and so could be considered a regression, although that same behaviour change could also be considered an improvement. The scenario we're talking about is where people run a 1.7 server and use a 1.6 or earlier svnsync binary. I suppose in this case your hook script would help? If not, the current draft of the 1.7 release notes will need to be adjusted. Stefan
Re: svn command line client's inconsistent behavior in handling multiple targets.
On Thu, Jan 13, 2011 at 03:31:11PM +0530, Noorul Islam K M wrote: | Command| Return Code | Continue | |+-+--| | add| 0 | Y| | blame | 1 | N| | cat| 1 | Y| | changelist | 0 | N| | commit | 1 | N| | copy | 1 | N| | delete | 1 | N| | diff | 1 | N| | info | 1 | Y| | list | 1 | N| | log| 1 | N| | revert | 0 | Y| Nice overview, thanks for putting this together! The commands add, cat, info and revert continues execution even if it finds one of the targets non-existing. All of them returns 0 to the shell except 'info' which returns 1. Also info prints svn: A problem occurred; see other errors for details at the end of the execution which tells the user that something went wrong during execution. This is not the case with 'add', 'cat' and 'revert'. I think these three commands also should return 1 and print the message which 'info' does, so that users does not have to scroll up to see the warnings to decide something went wrong. +1 on making more commands print a problem occured message at the end. I also think that all commands should exit 1 if one of the specified targets could not be found. This makes it much easier to detect such errors in shell scripts. Among these revert prints 'Skipped' message and others print 'svn: warning: ..' message. Hmm... somewhat inconsistent, but I don't think we need consistency here. Either way of putting it (skipped or warning) is fine. I think commit operation is an atomic one, therefore it does make sense when it errors out when one of the target is invalid. I am not sure why all the other commands errors out when one of the targets is non-existing. I think any command that tries to make a commit should error out. This includes copy and delete, because they can make commits without a working copy. Commands that operate on the working copy (including copy and delete!) should continue, print a warning or skipped message, and print the problem occured message at the end. Stefan
Re: svn commit: r1058308 - /subversion/site/publish/docs/release-notes/1.7.html
On Thu, Jan 13, 2011 at 10:56:09AM +, Philip Martin wrote: Stefan Sperling s...@elego.de writes: The scenario we're talking about is where people run a 1.7 server and use a 1.6 or earlier svnsync binary. I suppose in this case your hook script would help? No, I don't think it does. Daniel's change introduced new protocol elements to get the new atomic behaviour, a old client using the old protocol still gets the old non-atomic behaviour. Right. I'll adjust the release notes then. Thanks. We could change the behaviour associated with the old protocol, and then we could backport that relatively small change to 1.6, and that would allow the script to work. That would be the regression/improvement I mentioned. I don't think that's necessary. Backporting this fix would give people one less reason to upgrade to 1.7 :) Anyone suffering from the race condition in 1.6 can use the known workaround (run svnsync jobs on one computer and synchronise the processes via file locks). Stefan
Re: SQLite and callbacks
On Wed, Jan 12, 2011 at 09:46:47AM -0500, C. Michael Pilato wrote: Nothing to add to the immediate discussion at this time. Just wanted to note that we have essentially the same problems to deal with in the BDB backend. The solution we've taken there is don't drive public API callbacks from inside BDB transactions. It's not always the best for performance, but the FS API is such a high profile API for third-party consumers (very unlike the WC API in that respect, I'd imagine) that simplicity of the API rules was valued. This is about callbacks in the libsvn_client(!) and libsvn_wc APIs. Those are widely used by third parties so we should really make the right decision. Do we trust third parties to heed restrictions we place on callbacks? There is no way can enforce the restrictions. We can only document them and hope that third party developers play along. However, this would give us the best performance we can theoretically get (based on the assumption that the number of sqlite queries we run is a major limiting factor). Or we could decide not to run callbacks while sqlite transactions are in progress, and pay some performance overhead. We'd have to run multiple queries and provide data to the callback in chunks (of, say, directory-sized units). I'm not sure if this would provide adequate performance, but I haven't taken any measurements either (yet). A recent change of this kind I made to the node walker didn't make to make any notable difference. We may need both approaches anyway, if we decide to use the second approach in backwards-compat code. Opinions? Stefan
Re: svn commit: r1058308 - /subversion/site/publish/docs/release-notes/1.7.html
Philip Martin wrote on Thu, Jan 13, 2011 at 10:56:09 +: Stefan Sperling s...@elego.de writes: The scenario we're talking about is where people run a 1.7 server and use a 1.6 or earlier svnsync binary. I suppose in this case your hook script would help? No, I don't think it does. Daniel's change introduced new protocol elements to get the new atomic behaviour, a old client using the old protocol still gets the old non-atomic behaviour. We're talking about the svn_repos_fs_change_rev_prop4() changes, not about the RA protocol changes. Briefly, svn_repos_fs_change_rev_prop4() (which is the function that computes the ACTION parameter to the pre-revprop-change hook) uses the *atomic* FS change revprop API regardless of the client version.
Re: headers in the body Re: [l10n] Translation status report for trunk r1057984
(Haven't tested, but) Thanks! Branko Čibej wrote on Wed, Jan 12, 2011 at 19:01:50 +0100: On 12.01.2011 18:43, Branko Čibej wrote: Doing that now, and I found the reason for that extra line -- it comes from the output of svnversion. Could be a difference between 1.6 and 1.7, I suppose. r1058257, local testing seems OK, but please double-check. -- Brane
Re: svn commit: r1058308 - /subversion/site/publish/docs/release-notes/1.7.html
Daniel Shahaf d...@daniel.shahaf.name writes: Philip Martin wrote on Thu, Jan 13, 2011 at 10:56:09 +: Stefan Sperling s...@elego.de writes: The scenario we're talking about is where people run a 1.7 server and use a 1.6 or earlier svnsync binary. I suppose in this case your hook script would help? No, I don't think it does. Daniel's change introduced new protocol elements to get the new atomic behaviour, a old client using the old protocol still gets the old non-atomic behaviour. We're talking about the svn_repos_fs_change_rev_prop4() changes, not about the RA protocol changes. Briefly, svn_repos_fs_change_rev_prop4() (which is the function that computes the ACTION parameter to the pre-revprop-change hook) uses the *atomic* FS change revprop API regardless of the client version. Ah! So we have changed the behaviour of the server for old clients. I believe that means that the ACTION parameter is now reliable within the hook, which means that the script should work as a solution when using a 1.7 server with older clients. In a mixed environment the script is also necessary to prevent old clients stealing the lock from well-behaved 1.7 clients. -- Philip
Re: SQLite and callbacks
On 01/13/2011 08:25 AM, Stefan Sperling wrote: On Wed, Jan 12, 2011 at 09:46:47AM -0500, C. Michael Pilato wrote: Nothing to add to the immediate discussion at this time. Just wanted to note that we have essentially the same problems to deal with in the BDB backend. The solution we've taken there is don't drive public API callbacks from inside BDB transactions. It's not always the best for performance, but the FS API is such a high profile API for third-party consumers (very unlike the WC API in that respect, I'd imagine) that simplicity of the API rules was valued. This is about callbacks in the libsvn_client(!) and libsvn_wc APIs. Those are widely used by third parties so we should really make the right decision. Do we trust third parties to heed restrictions we place on callbacks? There is no way can enforce the restrictions. Really? Our BDB code detects re-entry from within a transaction and errors out. :-) It does this by keeping a simple boolean state variable that is set whenever the code enters a BDB transaction, and cleared when the transaction is committed or aborted. If that variable is set, and some other code comes along to create a new (nested) transaction, and error is raised. Could we do something similar in our SQL logic? Or we could decide not to run callbacks while sqlite transactions are in progress, and pay some performance overhead. We'd have to run multiple queries and provide data to the callback in chunks (of, say, directory-sized units). I'm not sure if this would provide adequate performance, but I haven't taken any measurements either (yet). A recent change of this kind I made to the node walker didn't make to make any notable difference. So, yeah, we really need more information to answer questions of performance, it seems. In the meantime, we need to answer questions of correctness, maintainability, and ease-of-use. We may need both approaches anyway, if we decide to use the second approach in backwards-compat code. I don't see how we'd ever need both approaches. Can you explain? -- C. Michael Pilato cmpil...@collab.net CollabNet www.collab.net Distributed Development On Demand signature.asc Description: OpenPGP digital signature
Re: SQLite and callbacks
On Thu, Jan 13, 2011 at 09:03:10AM -0500, C. Michael Pilato wrote: On 01/13/2011 08:25 AM, Stefan Sperling wrote: On Wed, Jan 12, 2011 at 09:46:47AM -0500, C. Michael Pilato wrote: Nothing to add to the immediate discussion at this time. Just wanted to note that we have essentially the same problems to deal with in the BDB backend. The solution we've taken there is don't drive public API callbacks from inside BDB transactions. It's not always the best for performance, but the FS API is such a high profile API for third-party consumers (very unlike the WC API in that respect, I'd imagine) that simplicity of the API rules was valued. This is about callbacks in the libsvn_client(!) and libsvn_wc APIs. Those are widely used by third parties so we should really make the right decision. Do we trust third parties to heed restrictions we place on callbacks? There is no way can enforce the restrictions. Really? Our BDB code detects re-entry from within a transaction and errors out. :-) It does this by keeping a simple boolean state variable that is set whenever the code enters a BDB transaction, and cleared when the transaction is committed or aborted. If that variable is set, and some other code comes along to create a new (nested) transaction, and error is raised. Could we do something similar in our SQL logic? This is a nice idea. I'll think about it. Or we could decide not to run callbacks while sqlite transactions are in progress, and pay some performance overhead. We'd have to run multiple queries and provide data to the callback in chunks (of, say, directory-sized units). I'm not sure if this would provide adequate performance, but I haven't taken any measurements either (yet). A recent change of this kind I made to the node walker didn't make to make any notable difference. So, yeah, we really need more information to answer questions of performance, it seems. In the meantime, we need to answer questions of correctness, maintainability, and ease-of-use. There is no question that calling callbacks while sqlite transactions are running provides best performance. Instead of crawling a tree on disk and running a query for each node (or a set of nodes), we can simply walk the db and pass information to the callback. This only works for read-only operations like proplist and status, of course, but those are the ones that need to perform really well so they can efficiently be used by GUIs, for instance. We may need both approaches anyway, if we decide to use the second approach in backwards-compat code. I don't see how we'd ever need both approaches. Can you explain? If we decide that the new approach violates our backwards compat rules, we'd have to provide a separate implementation to continue supporting semantics of old APIs. Stefan
Re: SQLite and callbacks
On 01/13/2011 10:10 AM, Stefan Sperling wrote: If we decide that the new approach violates our backwards compat rules, we'd have to provide a separate implementation to continue supporting semantics of old APIs. Ah, okay. This is precise what I did in BDB recently with the svn_fs_get_locks() API. It's callback-ish, and the callbacks were being driven from way deep in the our BDB table cursor-crawling code. To make the API usable to third-parties (who almost *always* want to do something FS-ish with the lock information they get), I now -- just inside the API, squirrel the lock information from the BDB layer away to a tempfile, then read the tempfile back to drive the public callback system. It's not lovely, it's not fast, but it saves API consumers from having to essentially do the same thing anyway. -- C. Michael Pilato cmpil...@collab.net CollabNet www.collab.net Distributed Development On Demand signature.asc Description: OpenPGP digital signature
Re: svn commit: r1058503, 1058515
s...@apache.org wrote on Thu, Jan 13, 2011 at 11:07:27 -: -pWith a 1.7 server, it is possible to fix the svnsync race condition -even for pre-1.7 svnsync clients by installing the pre-revprop-change hook -script given in -a href=http://subversion.tigris.org/issues/show_bug.cgi?id=3546#desc12; - comment 12 of issue #3546/a./p This should be reverted, per Philip's recent comment. tda href=#svnrdumpsvnrdump/a/td td1.7/td -tdany/td +tdanynbsp;(dump)br1.7nbsp;(load)/td tdany/td tdcodesvnrdump load/code should only be used with 1.7 servers to avoid a race condition (see a href=#svnrdumpbelow/a)./td/tr Why not state just 1.7 in the table, since Philip's hook should prevent the race condition here too?
Re: svn commit: r1058503, 1058515
On Thu, Jan 13, 2011 at 05:27:52PM +0200, Daniel Shahaf wrote: s...@apache.org wrote on Thu, Jan 13, 2011 at 11:07:27 -: -pWith a 1.7 server, it is possible to fix the svnsync race condition -even for pre-1.7 svnsync clients by installing the pre-revprop-change hook -script given in -a href=http://subversion.tigris.org/issues/show_bug.cgi?id=3546#desc12; - comment 12 of issue #3546/a./p This should be reverted, per Philip's recent comment. Yes, I've seen it. Feel free to revert :) tda href=#svnrdumpsvnrdump/a/td td1.7/td -tdany/td +tdanynbsp;(dump)br1.7nbsp;(load)/td tdany/td tdcodesvnrdump load/code should only be used with 1.7 servers to avoid a race condition (see a href=#svnrdumpbelow/a)./td/tr Why not state just 1.7 in the table, since Philip's hook should prevent the race condition here too? Fine with me.
Subversion Issue: 'delete file' not transmitted to server when performing multiple merge-commit
Dear Users, My user reported that an extra file existed in the repository after performing a merge (claiming this file was not in his working copy prior to the commit). My initial thought was they had done the merges out of sequence causing the deletion of a file before it was added, the user was adamant this was not the case. I created a copy of the repository (up to the revision immediately before the merge) and reproduced the merge. The working copy was correct however once committed the extra file appeared in the repository. What appears to be the issue was when a revision containing and add folder with files is merged, then delete one of the files in a subsequent revision merge, the working copy is correct but the delete was not transmitted to the server (only the add folder contents). If each merge revision was committed separately then the issue does not appear (this is *not* a practical work-around since the merge-commit is one logical unit of work - i.e. all the changes related to one defect). We are currently using Subversion 1.6.12 (client server), Apache 2.2 (server) and TortoiseSVN 1.6.10 (clients) in a Windows only environment (Server 2003 / XP). I have created a test script that reproduces the issue (the file that should be deleted is 'trunk/folder3/file6.txt') This script should be run twice (copy it to two separate directories), once performing three merges with three commits (gives correct result) and once performing three merges with one commit. REM build starting structure md start md start\branches md start\tags md start\trunk md start\trunk\folder1 echo start\trunk\folder1\file1.txt md start\trunk\folder2 echo start\trunk\folder2\file2.txt echo start\trunk\folder2\file3.txt REM initiate repository (set repo= saves code) svnadmin create repo set repo=file:///%cd:\=/%/repo svn import start %repo% -m Import Starting Structure svn copy %repo%/trunk %repo%/branches/dev -m svn checkout %repo%/branches/dev dev REM add new files echo dev\folder1\file4.txt svn add dev/folder1/file4.txt echo dev\folder2\file5.txt svn add dev/folder2/file5.txt svn commit dev -m Add some new files REM add new folder md dev\folder3 echo dev\folder3\file6.txt echo dev\folder3\file7.txt svn add dev/folder3 REM modify a file echo dev\folder2\file3.txt svn commit dev -m Add new folder/files + change a file svn del dev/folder2/file5.txt svn del dev/folder3/file6.txt svn commit dev -m Delete some files svn checkout %repo%/trunk trunk svn merge %repo%/branches/dev@3 trunk REM comment-out on second run!!! svn commit trunk -m Merge changes back svn merge %repo%/branches/dev@4 trunk REM comment-out on second run!!! svn commit trunk -m Merge changes back svn merge %repo%/branches/dev@5 trunk svn commit trunk -m Merge changes back svn ls -R trunk The working copy (trunk) will be identical after both runs, however the repositories are different. The file file6.txt that was deleted from the branches/dev/folder3 in revision 5 is not removed from trunk/folder3 when merged at the same time that this folder was added in revision 4. Awaiting an acknowledgement (and a buddy to confirm). Regards Neil Tuffs Configuration Release Management RWE Supply Trading GmbH Windmill Hill Business Park Whitehill Way Swindon SN5 6PB Tel (Internal): 7 322 2705 Tel (External): +44 (0)1793 892705 Fax:+44 (0)1793 893560 E-mail: neil.tu...@rwe.com
Ref-counting for pristine texts
I have committed the ref counting for pristine texts (r1058523) and have had a bit more insight into when to perform the deletion. Deleting unreferenced texts on closing a wcroot is too late - too large a granularity - for the way the WC code is currently structured because this doesn't happen until a (long-lived) pool is cleaned up, as Bert pointed out. Deleting unreferenced texts after running the Work Queue is too soon - too fine a granularity - for the way commit is currently structured. A simple test of this approach showed that by the time the post-commit processing tries to install a reference to a pristine text whose checksum was noted earlier, in some cases that pristine row has already been purged. At the moment I think the practical solution is to insert calls to the pristine cleanup at the end of each main libsvn_wc public API that may have freed up one or more pristines. Wherever we do it, if it's at all frequent, the search for unreferenced pristines needs to be efficient. At present I use SELECT ... FROM PRISTINE WHERE refcount = 0. I believe that can most easily be made efficient by adding an index on the 'refcount' column, so that's what I intend to do. - Julian
Re: Subversion Issue: 'delete file' not transmitted to server when performing multiple merge-commit
neil.tu...@rwe.com writes: My user reported that an extra file existed in the repository after performing a merge (claiming this file was not in his working copy prior to the commit). My initial thought was they had done the merges out of sequence causing the deletion of a file before it was added, the user was adamant this was not the case. A simpler sequence: rm -rf repo wc file touch file url=file://`pwd`/repo svnadmin create repo svn mkdir -mm $url/trunk svn cp -mm $url/trunk $url/branch svn import -mm file $url/branch/folder/file svn rm -mm $url/branch/folder/file svn co $url/trunk wc svn merge $url/branch@3 wc svn merge $url/branch@4 wc The first merge adds a folder containing a file, the second merge deletes the file. After the two merges 1.6 shows status: Mwc A + wc/folder This is a problem because folder's history is a copy from a revision where it contains the file, so the file will exist in repository after the commit. Using 1.7 status shows: Mwc A + wc/folder D + wc/folder/file so 1.7 will delete explicitly the file when the merge is committed. -- Philip
Re: Ref-counting for pristine texts
On 13.01.2011 20:01, Julian Foad wrote: I have committed the ref counting for pristine texts (r1058523) and have had a bit more insight into when to perform the deletion. Deleting unreferenced texts on closing a wcroot is too late - too large a granularity - for the way the WC code is currently structured because this doesn't happen until a (long-lived) pool is cleaned up, as Bert pointed out. Deleting unreferenced texts after running the Work Queue is too soon - too fine a granularity - for the way commit is currently structured. A simple test of this approach showed that by the time the post-commit processing tries to install a reference to a pristine text whose checksum was noted earlier, in some cases that pristine row has already been purged. This would indicate that the reference counting happens too soon ... in other words, that a pristine can be dereferenced whilst some part of the code (or database) still refers to it. That breaks database consistency -- what happens if the user aborts a commit and then calls 'svn cleanup', for example? At the moment I think the practical solution is to insert calls to the pristine cleanup at the end of each main libsvn_wc public API that may have freed up one or more pristines. Mmm. Too many points of failure, don't you think? Of course, it's no worse than the cancellation handlers, however, and I suppose missing a chance to delete is better than having one too many. Wherever we do it, if it's at all frequent, the search for unreferenced pristines needs to be efficient. At present I use SELECT ... FROM PRISTINE WHERE refcount = 0. I believe that can most easily be made efficient by adding an index on the 'refcount' column, so that's what I intend to do. That's the only thing you can do, unless you want to enhance the triggers to populate a queue of to-be-deleted pristines. -- Brane
