Re: multiple targets for 'svnlook propget'
Johan Corveleyn wrote on Mon, Nov 19, 2012 at 09:43:30 +0100: (on a related note, I currently have a patch sitting here for adding --diff-cmd to 'svnlook diff', but those changes are local to the I wonder what's the minimal change we could make to the cmdline client such that it can operate on transactions (and thus void the need to reimplement every svn proplist/diff/cat/info switch in svnlook). (Read-only, at least initially.) Is this something Julian's tree-read-api branch would address? Maybe we need to implement svn_ra_local_txn (like ra_local, but with HEAD being a transaction instead of a revision)? Other ideas?
Re: Serf crashes on fork
Stefan Fuhrmann stefan.fuhrm...@wandisco.com writes: As it turns out, your commit has only be the trigger but not the root cause. serf_trunk/allocator.c, serf_bucket_allocator_create(), line 147: /* ### this implies buckets cannot cross a fork/exec. desirable? * * ### hmm. it probably also means that buckets cannot be AROUND * ### during a fork/exec. the new process will try to clean them * ### up and figure out there are unfreed blocks... */ apr_pool_cleanup_register(pool, allocator, allocator_cleanup, allocator_cleanup); Since we fork() for hooks, we can't use hooks in ra_local while there is an open serf connection. Otherwise, we get into trouble with pool cleanups: Does it ever make sense for the child process to run that handler? Is that to allow a parent process to allocate a serf connection and then fork off a child process to use the connection? -- Certified Supported Apache Subversion Downloads: http://www.wandisco.com/subversion/download
Re: Serf crashes on fork
On Mon, Nov 19, 2012 at 12:24 PM, Philip Martin philip.mar...@wandisco.comwrote: Stefan Fuhrmann stefan.fuhrm...@wandisco.com writes: As it turns out, your commit has only be the trigger but not the root cause. serf_trunk/allocator.c, serf_bucket_allocator_create(), line 147: /* ### this implies buckets cannot cross a fork/exec. desirable? * * ### hmm. it probably also means that buckets cannot be AROUND * ### during a fork/exec. the new process will try to clean them * ### up and figure out there are unfreed blocks... */ apr_pool_cleanup_register(pool, allocator, allocator_cleanup, allocator_cleanup); Since we fork() for hooks, we can't use hooks in ra_local while there is an open serf connection. Otherwise, we get into trouble with pool cleanups: Does it ever make sense for the child process to run that handler? Is that to allow a parent process to allocate a serf connection and then fork off a child process to use the connection? From the comments in APR/threadproc/unix/proc.c, it seems that apr_proc_create runs *all* pool cleanups in the child process to clean up duplicated file handles and such. -- Stefan^2. -- Certified Supported Apache Subversion Downloads: * http://www.wandisco.com/subversion/download *
Re: multiple targets for 'svnlook propget'
On Mon, Nov 19, 2012 at 11:38 AM, Daniel Shahaf d...@daniel.shahaf.name wrote: Johan Corveleyn wrote on Mon, Nov 19, 2012 at 09:43:30 +0100: (on a related note, I currently have a patch sitting here for adding --diff-cmd to 'svnlook diff', but those changes are local to the I wonder what's the minimal change we could make to the cmdline client such that it can operate on transactions (and thus void the need to reimplement every svn proplist/diff/cat/info switch in svnlook). (Read-only, at least initially.) Is this something Julian's tree-read-api branch would address? Maybe we need to implement svn_ra_local_txn (like ra_local, but with HEAD being a transaction instead of a revision)? Other ideas? I'd like to note that the output of 'svnlook diff' is slightly different from 'svn diff', and I'd like to preserve that different behavior (or at least preserve the svnlook behavior here). IMO the output of 'svnlook diff' is better suited for post-commit emails. Some of the differences are (comparing svn 1.7.7 with svnlook 1.5.4): - 'svnlook diff' shows the differences in the same order as 'svnlook changed'. - 'svnlook diff' shows headers like 'Modified:', 'Added:', 'Copied: XXX (from YYY)', 'Deleted:', instead of the boring 'Index:' of 'svn diff'. - they both have the option --no-diff-deleted, but 'svnlook diff' then doesn't even show the header of the deleted file, while 'svn diff' does. (here I like 'svn diff's behavior better ... I think 'svnlook diff' should show the header line 'Deleted:' even with --no-diff-deleted (note: my svnlook is version 1.5.4, maybe that's already changed ...?)). - 'svnlook diff' has --diff-copy-from, while 'svn diff' has --show-copies-as-adds. Sound like these two are the reverse of each other (so maybe the default behavior is reversed). However, I just quickly tested an 'svn diff -c XXX' of some revision which has a move, and it always shows the moved file in full as a delete and an add, no matter if I use --show-copies-as-adds or not. So it seems that detecting copies with their sources is broken in 'svn diff'. - 'svnlook diff' has --no-diff-added, while 'svn diff' doesn't seem to have that option. Again, with --no-diff-added, 'svnlook diff' doesn't even show the header line, which I don't really like, so that can be improved IMO. - 'svn diff' has --notice-ancestry, but 'svnlook diff' doesn't (and I don't know what that option actually does). However, this doesn't mean that both behaviors can't converge, and end up with a single implementation. I just want to say that, in my opinion, this means porting some of the 'better behavior' from 'svnlook diff' to this generic implementation, and not just plainly re-using the (in my opinion) inferior behavior of 'svn diff'. -- Johan
Re: Serf crashes on fork
Stefan Fuhrmann stefan.fuhrm...@wandisco.com writes: On Mon, Nov 19, 2012 at 12:24 PM, Philip Martin philip.mar...@wandisco.comwrote: Stefan Fuhrmann stefan.fuhrm...@wandisco.com writes: As it turns out, your commit has only be the trigger but not the root cause. serf_trunk/allocator.c, serf_bucket_allocator_create(), line 147: /* ### this implies buckets cannot cross a fork/exec. desirable? * * ### hmm. it probably also means that buckets cannot be AROUND * ### during a fork/exec. the new process will try to clean them * ### up and figure out there are unfreed blocks... */ apr_pool_cleanup_register(pool, allocator, allocator_cleanup, allocator_cleanup); Since we fork() for hooks, we can't use hooks in ra_local while there is an open serf connection. Otherwise, we get into trouble with pool cleanups: Does it ever make sense for the child process to run that handler? Is that to allow a parent process to allocate a serf connection and then fork off a child process to use the connection? From the comments in APR/threadproc/unix/proc.c, it seems that apr_proc_create runs *all* pool cleanups in the child process to clean up duplicated file handles and such. apr_proc_create runs the child cleanup handlers. Note that two handlers are passed to _register, one for the parent one for the child. I'm asking why serf installs a child handler rather than passing apr_pool_cleanup_null. -- Certified Supported Apache Subversion Downloads: http://www.wandisco.com/subversion/download
Re: [RFC] svn propset should require 'force' to set unknown svn: propnames
On 11/18/2012 10:56 PM, Daniel Shahaf wrote: Julian Foad wrote on Mon, Nov 19, 2012 at 02:50:46 +: Thoughts, objections? Another related trap is setting a revprop as a nodeprop, or vice-versa: svn commit --with-revprop=svn:eol-style=native -mm foo.c svn propset svn:log x foo.c You might want to require --force for these too. +1 to both suggestions. I can never remember if it's svn:ignore[s] or svn:keyword[s]. Don't mind getting smacked for choosing wrongly, but it's always irritating for the propset to succeed yet the behavior not change. -- C. Michael Pilato cmpil...@collab.net CollabNet www.collab.net Enterprise Cloud Development signature.asc Description: OpenPGP digital signature
Re: Serf crashes on fork
Philip Martin philip.mar...@wandisco.com writes: Stefan Fuhrmann stefan.fuhrm...@wandisco.com writes: On Mon, Nov 19, 2012 at 12:24 PM, Philip Martin philip.mar...@wandisco.comwrote: Stefan Fuhrmann stefan.fuhrm...@wandisco.com writes: As it turns out, your commit has only be the trigger but not the root cause. serf_trunk/allocator.c, serf_bucket_allocator_create(), line 147: /* ### this implies buckets cannot cross a fork/exec. desirable? * * ### hmm. it probably also means that buckets cannot be AROUND * ### during a fork/exec. the new process will try to clean them * ### up and figure out there are unfreed blocks... */ apr_pool_cleanup_register(pool, allocator, allocator_cleanup, allocator_cleanup); Since we fork() for hooks, we can't use hooks in ra_local while there is an open serf connection. Otherwise, we get into trouble with pool cleanups: Does it ever make sense for the child process to run that handler? Is that to allow a parent process to allocate a serf connection and then fork off a child process to use the connection? If the processes were behaving like that the child cleanup handlers would not be involved. From the comments in APR/threadproc/unix/proc.c, it seems that apr_proc_create runs *all* pool cleanups in the child process to clean up duplicated file handles and such. apr_proc_create runs the child cleanup handlers. Note that two handlers are passed to _register, one for the parent one for the child. I'm asking why serf installs a child handler rather than passing apr_pool_cleanup_null. The cleanup handler is just releasing memory via apr_allocator_free. I see no reason for it to be installed as a child cleanup handler. -- Certified Supported Apache Subversion Downloads: http://www.wandisco.com/subversion/download
'svn diff' vs 'svnlook diff' [was: multiple targets for 'svnlook propget']
Johan Corveleyn wrote: Daniel Shahaf wrote: Johan Corveleyn wrote: I currently have a patch sitting here for adding --diff-cmd to 'svnlook diff', I wonder what's the minimal change we could make to the cmdline client such that it can operate on transactions (and thus void the need to reimplement every svn proplist/diff/cat/info switch in svnlook). (Read-only, at least initially.) Is this something Julian's tree-read-api branch would address? Yes my tree-read-api work would make this sort of thing easier. Maybe we need to implement svn_ra_local_txn (like ra_local, but with HEAD being a transaction instead of a revision)? Other ideas? Move the core tree-diffing functionality down a layer from libsvn_client into libsvn_diff. Let 'svn' pass some kinds of 'tree description' inputs to it (from the WC and RA interfaces) and let 'svnlook' pass other kinds of 'tree description' inputs (revisions and txns, from the repos layer). I'd like to note that the output of 'svnlook diff' is slightly different from 'svn diff', and I'd like to preserve that different behavior (or at least preserve the svnlook behavior here). IMO the output of 'svnlook diff' is better suited for post-commit emails. Ugh. Most of these differences are (IMO) unwanted. I basically agree with your comments below about which ones are better. Some of the differences are (comparing svn 1.7.7 with svnlook 1.5.4): - 'svnlook diff' shows the differences in the same order as 'svnlook changed'. - 'svnlook diff' shows headers like 'Modified:', 'Added:', 'Copied: XXX (from YYY)', 'Deleted:', instead of the boring 'Index:' of 'svn diff'. - they both have the option --no-diff-deleted, but 'svnlook diff' then doesn't even show the header of the deleted file, while 'svn diff' does. (here I like 'svn diff's behavior better ... I think 'svnlook diff' should show the header line 'Deleted:' even with --no-diff-deleted (note: my svnlook is version 1.5.4, maybe that's already changed ...?)). - 'svnlook diff' has --diff-copy-from, while 'svn diff' has --show-copies-as-adds. Sound like these two are the reverse of each other (so maybe the default behavior is reversed). However, I just quickly tested an 'svn diff -c XXX' of some revision which has a move, and it always shows the moved file in full as a delete and an add, no matter if I use --show-copies-as-adds or not. So it seems that detecting copies with their sources is broken in 'svn diff'. - 'svnlook diff' has --no-diff-added, while 'svn diff' doesn't seem to have that option. Again, with --no-diff-added, 'svnlook diff' doesn't even show the header line, which I don't really like, so that can be improved IMO. - 'svn diff' has --notice-ancestry, but 'svnlook diff' doesn't (and I don't know what that option actually does). However, this doesn't mean that both behaviors can't converge, and end up with a single implementation. I just want to say that, in my opinion, this means porting some of the 'better behavior' from 'svnlook diff' to this generic implementation, and not just plainly re-using the (in my opinion) inferior behavior of 'svn diff'. Agreed. - Julian
Re: [RFC] svn propset should require 'force' to set unknown svn: propnames
C. Michael Pilato wrote: On 11/18/2012 10:56 PM, Daniel Shahaf wrote: Julian Foad wrote on Mon, Nov 19, 2012 at 02:50:46 +: Thoughts, objections? Another related trap is setting a revprop as a nodeprop, or vice-versa: svn commit --with-revprop=svn:eol-style=native -mm foo.c svn propset svn:log x foo.c You might want to require --force for these too. +1 to both suggestions. I can never remember if it's svn:ignore[s] or svn:keyword[s]. Don't mind getting smacked for choosing wrongly, but it's always irritating for the propset to succeed yet the behavior not change. I agree with Daniel's suggestion too. I have filed enhancement issue #4261 Setting unknown svn: propnames should require 'force', which covers both suggestions together. Anybody want to work on it? Please do. I can get to it some time, but I don't know when, if nobody else picks it up. - Julian
Re: [Issue 4261] Setting unknown svn: propnames should require 'force'
On 11/19/2012 09:12 AM, julianf...@tigris.org wrote: http://subversion.tigris.org/issues/show_bug.cgi?id=4261 User julianfoad changed the following: What|Old value |New value Target milestone|--- |unscheduled --- Additional comments from julianf...@tigris.org Mon Nov 19 06:12:26 -0800 2012 --- Setting the target milestone to '1.8-consider' because the introduction of the 'svn:global-ignores' property name in 1.8 is a motivating factor. Other than that there is no particular reason to schedule this for a particular release. Er... nope. Try again on the milestone? -- C. Michael Pilato cmpil...@collab.net CollabNet www.collab.net Enterprise Cloud Development signature.asc Description: OpenPGP digital signature
Re: [RFC] svn propset should require 'force' to set unknown svn: propnames
Julian Foad wrote on Mon, Nov 19, 2012 at 14:08:34 +: C. Michael Pilato wrote: On 11/18/2012 10:56 PM, Daniel Shahaf wrote: Julian Foad wrote on Mon, Nov 19, 2012 at 02:50:46 +: Thoughts, objections? Another related trap is setting a revprop as a nodeprop, or vice-versa: svn commit --with-revprop=svn:eol-style=native -mm foo.c svn propset svn:log x foo.c You might want to require --force for these too. +1 to both suggestions. I can never remember if it's svn:ignore[s] or svn:keyword[s]. Don't mind getting smacked for choosing wrongly, but it's always irritating for the propset to succeed yet the behavior not change. I agree with Daniel's suggestion too. I have filed enhancement issue #4261 Setting unknown svn: propnames should require 'force', which covers both suggestions together. Another thing we could do is warn when unrecognised options are found in the config file (~/.subversion/*) or in the --config-option command-line option. This would be an independent improvement, i.e., it neither blocks nor is blocked by the propset improvements in #4261. Anybody want to work on it? Please do. I can get to it some time, but I don't know when, if nobody else picks it up. - Julian
Re: [RFC] svn propset should require 'force' to set unknown svn: propnames
On 11/19/2012 09:13 AM, Daniel Shahaf wrote: Another thing we could do is warn when unrecognised options are found in the config file (~/.subversion/*) or in the --config-option command-line option. This would be an independent improvement, i.e., it neither blocks nor is blocked by the propset improvements in #4261. As regards the config files, -0 (tending towards -1). The runtime configuration area is not release-version-specific. Folks like myself who keep their ~/.subversion under version control and use it across many different clients (which may be running different versions of Subversion) could be constantly irritated by such warnings. And of course, we devs would be likely to run into such things often, too. Besides, the config files are generated as templates -- you'd have to work extra hard to botch the spelling of an option name there. :-) As for warning on bogus --config-option input ... I'm closer to +0 or +1 there. -- C. Michael Pilato cmpil...@collab.net CollabNet www.collab.net Enterprise Cloud Development signature.asc Description: OpenPGP digital signature
Re: [Issue 4261] Setting unknown svn: propnames should require 'force'
C. Michael Pilato wrote: User julianfoad changed the following: What |Old value |New value === Target milestone|--- |unscheduled --- Additional comments from julianf...@tigris.org Setting the target milestone to '1.8-consider' because the introduction of the 'svn:global-ignores' property name in 1.8 is a motivating factor. Other than that there is no particular reason to schedule this for a particular release. Er... nope. Try again on the milestone? At first I thought you meant you disagreed with my comment, but you just meant I failed to set the milestone that I claimed to set in this update of the issue. However, I fixed it immediately afterwards. - Julian
Re: r1397773 - rep-sharing in a txn - subversion/libsvn_fs_fs/fs_fs.c
On Thu, Nov 15, 2012 at 6:16 PM, Julian Foad julianf...@btopenworld.comwrote:
Hi Stefan. Some review questions and comments...
stef...@apache.org wrote:
URL: http://svn.apache.org/viewvc?rev=1397773view=rev
Log:
Due to public request: apply rep-sharing to equal data reps within
the same transaction.
The idea is simple. When writing a noderev to the txn folder,
write another file named by the rep's SHA1 and store the rep
struct in there. Lookup is then straight-forward.
Please
could you update the FSFS structure document. I assume it says what
files are stored in the txn dir; if it doesn't it should.
Oops - hadn't thought about that! Done in r1411202.
All other changes in r1411209.
* subversion/libsvn_fs_fs/fs_fs.c
(svn_fs_fs__put_node_revision): also look for SHA1-named files
(get_shared_rep): write SHA1-named files
[...]
@@ -2603,7 +2607,32 @@ svn_fs_fs__put_node_revision(svn_fs_t *f
- return svn_io_file_close(noderev_file, pool);
+ SVN_ERR(svn_io_file_close(noderev_file, pool));
+
+ /* if rep sharing has been enabled and the noderev has a data rep
+ * and its SHA-1 is known, store the rep struct under its SHA1. */
It
looks like we re-enter this 'if' block, and re-write the rep-reference
file, every time we store a node-rev with the same representation --
even if the rep we're storing was already found from this file. This
might not be much of an overhead, but it seems ugly.
Could we
avoid re-writing it in those cases? Maybe by refactoring such that we
write the rep-reference file immediately after writing the rep, instead
of here in put_node_revision.
Yep. It turns out that rep_write_contents_close is the
only relevant user of that functionality. There, we can
also decide whether to store the sha1-rep mapping
or not. It also addresses the locking problem below.
+ if (sha1)
+ {
+apr_file_t *rep_file;
+const char *file_name = svn_dirent_join(path_txn_dir(fs, txn_id,\
+pool), sha1, pool);
Creating
the file name ('checksum_to_cstring' + 'path_txn_dir' +
'svn_dirent_join') is common to both this function and the function
below where we read the file. Factor it out, like the existing
path_txn_node_rev() and similar functions, to make the commonality
clear.
Done.
+const char *rep_string = representation_string(noderev-data_rep,
+ ffd-format,
+ (noderev-kind
+== svn_node_dir),
+ FALSE,
+ pool);
+SVN_ERR(svn_io_file_open(rep_file, file_name,
+ APR_WRITE | APR_CREATE | APR_TRUNCATE
+ | APR_BUFFERED, APR_OS_DEFAULT, pool));
+
+SVN_ERR(svn_io_file_write_full(rep_file, rep_string,
+ strlen(rep_string), NULL, pool));
+
+SVN_ERR(svn_io_file_close(rep_file, pool));
+ }
+
+ return SVN_NO_ERROR;
}
@@ -7083,6 +7112,30 @@ get_shared_rep(representation_t **old_re
+ /* look for intra-revision matches (usually data reps but not limited
+ to them in case props happen to look like some data rep)
+ */
+ if (*old_rep == NULL rep-txn_id)
+{
+ svn_node_kind_t kind;
+ const char *file_name
+= svn_dirent_join(path_txn_dir(fs, rep-txn_id, pool),
+ svn_checksum_to_cstring(rep-sha1_checksum,\
+ pool), pool);
+
+ /* in our txn, is there a rep file named with the wanted SHA1?
+ If so, read it and use that rep. */
+ SVN_ERR(svn_io_check_path(file_name, kind, pool));
Instead
of stat'ing the file to decide whether to open it, it's more efficient
and more 'robust' to just try to open it and then handle the potential
failure, isn't it?
Access is serialized (see below). Checking for existence
should be more efficient because actual matches are
relatively rare and constructing an error object can be
expensive (NLS). svn_stringbuf_from_file2 is also a very
convenient function to use.
+ if (kind == svn_node_file)
+{
+ svn_stringbuf_t *rep_string;
+ SVN_ERR(svn_stringbuf_from_file2(rep_string, file_name,\
+ pool));
I'm
sure the answer is obvious to those familiar with FSFS, but just to be
clear please could you tell me which locking
mechanism guarantees that the opening and reading of this file cannot be
happening at the same time as this file is being created and written
(by another thread or process)?
Writing representations is serialized using the protorev lock.
All access to the sha1-rep mapping is done from
rep_write_contents_close() now, which in
Re: fork/exec for hooks scripts with a large FSFS cache
On 14 nov 2012, at 01:44, Daniel Shahaf wrote: Philip Martin wrote on Tue, Nov 13, 2012 at 21:30:00 +: Perhaps we could start up a separate hook script process before allocating the large FSFS cache and then delegate the fork/exec to that smaller process? If so, let's have that daemon handle all forks we might do, not just those related to hook scripts. (Otherwise we'll run into the same problem as soon as we have another use-case for fork() in the server code --- such as calling svn_sysinfo__release_name().) Looking at it from another perspective, perhaps the cache should live within a separate daemon? That would address not only the hook script problem but also the challenges of prefork processes typically required when combining Subversion and PHP. Just a thought, /Thomas Å.
Re: AW: duplicate externals cause update error -- thoughts?
Thanks for your input, Markus! As so often with externals, things are more complex than one would think. - When an externals error occurs, the entire externals property remains unhandled. That needn't be so. - When an existing repos has duplicates, doing a simple update/checkout with the new error check would be tiresome, as one would first need to edit all those props before the update completes. - To be able to issue a warning, the parsing function needs a new argument with a callback function (or something). Like this it can notify about more than one duplicate. It could still parse everything and return the parsed data, and externals updating could continue to work the same (stupid) way it does now: fifo style. Just adding a flag to the function to select between error and warning won't work, as that function does not have any way to issue warnings. Right now I'm thinking: - add a callback arg to svn_wc_parse_externals_description3(). - the callback fn usually tries to punch a warning thru to the user. - maybe during propset/propedit, that callback causes an error. Maybe all the other externals parse errors should be handled in the same way? We'll see about that later. Maybe we should even ... reimplement externals from scratch ;) ~Neels On 2012-11-19 08:17, Markus Schaber wrote: Could you give a flag to the parsing code whether it should error or just warn on that issue? So on an update, we can use the warning, while the code can error out on propset/propedit. And (of course), for backwards compatibility, the behavior in case of an collision should resemble the old behavior (which of the two entries wins...) Best regards Markus Schaber CODESYS(r) a trademark of 3S-Smart Software Solutions GmbH Inspiring Automation Solutions 3S-Smart Software Solutions GmbH Dipl.-Inf. Markus Schaber | Product Development Core Technology Memminger Str. 151 | 87439 Kempten | Germany Tel. +49-831-54031-979 | Fax +49-831-54031-50 E-Mail: m.scha...@codesys.com | Web: http://www.codesys.com CODESYS internet forum: http://forum.codesys.com Managing Directors: Dipl.Inf. Dieter Hess, Dipl.Inf. Manfred Werner | Trade register: Kempten HRB 6186 | Tax ID No.: DE 167014915 signature.asc Description: OpenPGP digital signature
Re: Serf crashes on fork
On Mon, Nov 19, 2012 at 2:44 PM, Philip Martin philip.mar...@wandisco.comwrote: Philip Martin philip.mar...@wandisco.com writes: Stefan Fuhrmann stefan.fuhrm...@wandisco.com writes: On Mon, Nov 19, 2012 at 12:24 PM, Philip Martin philip.mar...@wandisco.comwrote: Stefan Fuhrmann stefan.fuhrm...@wandisco.com writes: As it turns out, your commit has only be the trigger but not the root cause. serf_trunk/allocator.c, serf_bucket_allocator_create(), line 147: /* ### this implies buckets cannot cross a fork/exec. desirable? * * ### hmm. it probably also means that buckets cannot be AROUND * ### during a fork/exec. the new process will try to clean them * ### up and figure out there are unfreed blocks... */ apr_pool_cleanup_register(pool, allocator, allocator_cleanup, allocator_cleanup); Since we fork() for hooks, we can't use hooks in ra_local while there is an open serf connection. Otherwise, we get into trouble with pool cleanups: Does it ever make sense for the child process to run that handler? Is that to allow a parent process to allocate a serf connection and then fork off a child process to use the connection? If the processes were behaving like that the child cleanup handlers would not be involved. From the comments in APR/threadproc/unix/proc.c, it seems that apr_proc_create runs *all* pool cleanups in the child process to clean up duplicated file handles and such. apr_proc_create runs the child cleanup handlers. Note that two handlers are passed to _register, one for the parent one for the child. I'm asking why serf installs a child handler rather than passing apr_pool_cleanup_null. The cleanup handler is just releasing memory via apr_allocator_free. I see no reason for it to be installed as a child cleanup handler. Simply patching serf fixes the problem for me. -- Stefan^2. [[[ Index: buckets/allocator.c === --- buckets/allocator.c(revision 1685) +++ buckets/allocator.c(working copy) @@ -151,7 +151,7 @@ * ### up and figure out there are unfreed blocks... */ apr_pool_cleanup_register(pool, allocator, - allocator_cleanup, allocator_cleanup); + allocator_cleanup, apr_pool_cleanup_null); return allocator; } ]]] -- Certified Supported Apache Subversion Downloads: * http://www.wandisco.com/subversion/download *
Re: r1397773 - rep-sharing in a txn - subversion/libsvn_fs_fs/fs_fs.c
Stefan Fuhrmann wrote: On Thu, Nov 15, 2012 at 6:16 PM, Julian Foad julianf...@btopenworld.com wrote: stef...@apache.org wrote: URL: http://svn.apache.org/viewvc?rev=1397773view=rev Log: Due to public request: apply rep-sharing to equal data reps within the same transaction. The idea is simple. When writing a noderev to the txn folder, write another file named by the rep's SHA1 and store the rep struct in there. Lookup is then straight-forward. Please could you update the FSFS structure document. I assume it says what files are stored in the txn dir; if it doesn't it should. Oops - hadn't thought about that! Done in r1411202. All other changes in r1411209. Thanks, Stefan. Looks good. + /* in our txn, is there a rep file named with the wanted SHA1? + If so, read it and use that rep. */ + SVN_ERR(svn_io_check_path(file_name, kind, pool)); Instead of stat'ing the file to decide whether to open it, it's more efficient and more 'robust' to just try to open it and then handle the potential failure, isn't it? Access is serialized (see below). Checking for existence should be more efficient because actual matches are relatively rare and constructing an error object can be expensive (NLS). svn_stringbuf_from_file2 is also a very convenient function to use. I wonder if we could change our generic error handling scheme to defer NLS look-up until the time when we print the error. That would eliminate that overhead in cases where we handle and clear an error without printing it. Something to think about for the future. [...] Thanks for the review! The code should be multi-thread safe. Thanks. - Julian
Re: fork/exec for hooks scripts with a large FSFS cache
On Mon, Nov 19, 2012 at 3:45 PM, Thomas Åkesson tho...@akesson.cc wrote: On 14 nov 2012, at 01:44, Daniel Shahaf wrote: Philip Martin wrote on Tue, Nov 13, 2012 at 21:30:00 +: Perhaps we could start up a separate hook script process before allocating the large FSFS cache and then delegate the fork/exec to that smaller process? If so, let's have that daemon handle all forks we might do, not just those related to hook scripts. (Otherwise we'll run into the same problem as soon as we have another use-case for fork() in the server code --- such as calling svn_sysinfo__release_name().) Looking at it from another perspective, perhaps the cache should live within a separate daemon? That would address not only the hook script problem but also the challenges of prefork processes typically required when combining Subversion and PHP. For non-obvious reasons listed at the end of http://svn.haxx.se/dev/archive-2012-11/0148.shtml, this is not trivial to do. I may give it a try in 1.9 but there might be lock cleanup edge cases that simply can't be handled in a portable way. -- Stefan^2. -- Certified Supported Apache Subversion Downloads: * http://www.wandisco.com/subversion/download *
Re: AW: duplicate externals cause update error -- thoughts?
Neels J Hofmeyr ne...@elego.de writes: - To be able to issue a warning, the parsing function needs a new argument with a callback function (or something). Like this it can notify about more than one duplicate. It could still parse everything and return the parsed data, and externals updating could continue to work the same (stupid) way it does now: fifo style. Just adding a flag to the function to select between error and warning won't work, as that function does not have any way to issue warnings. Perhaps the caller could catch the error, issue the warning, and skip processing that svn:externals. The caller already has notification support. svn_wc_parse_externals_description3 is also used in upgrade. What do we do there? I suppose we could do the same, issue a warning and skip processing the svn:externals. -- Certified Supported Apache Subversion Downloads: http://www.wandisco.com/subversion/download
Re: AW: duplicate externals cause update error -- thoughts?
Neels J Hofmeyr wrote: Thanks for your input, Markus! As so often with externals, things are more complex than one would think. - When an externals error occurs, the entire externals property remains unhandled. That needn't be so. - When an existing repos has duplicates, doing a simple update/checkout with the new error check would be tiresome, as one would first need to edit all those props before the update completes. - To be able to issue a warning, the parsing function needs a new argument with a callback function (or something). Like this it can notify about more than one duplicate. It could still parse everything and return the parsed data, and externals updating could continue to work the same (stupid) way it does now: fifo style. Just adding a flag to the function to select between error and warning won't work, as that function does not have any way to issue warnings. Right now I'm thinking: - add a callback arg to svn_wc_parse_externals_description3(). - the callback fn usually tries to punch a warning thru to the user. - maybe during propset/propedit, that callback causes an error. Sounds icky. Maybe that's the wrong level to detect duplicates. Instead, let this parse function return a list describing exactly what it parsed, and let the *caller* check for duplicates. - Julian Maybe all the other externals parse errors should be handled in the same way? We'll see about that later. Maybe we should even ... reimplement externals from scratch ;)
Re: r1397773 - rep-sharing in a txn - subversion/libsvn_fs_fs/fs_fs.c
On Mon, Nov 19, 2012 at 4:16 PM, Julian Foad julianf...@btopenworld.comwrote: Stefan Fuhrmann wrote: Thanks for the review! The code should be multi-thread safe. hm. That should read should be multi-thread safe *now*. -- Stefan^2. -- Certified Supported Apache Subversion Downloads: * http://www.wandisco.com/subversion/download *
Re: svn commit: r1411296 - /subversion/branches/1.6.x/STATUS
stef...@apache.org wrote on Mon, Nov 19, 2012 at 16:45:30 -: @@ -74,7 +74,7 @@ Candidate changes: Branch: ^/subversion/branches/1.6.x-rep_write_cleanup Votes: - +1: breser, danielsh + +1: breser, danielsh, stefan2 If you move this paragraph below the Approved changes header, the backport.pl cron will merge it. Veto-blocked changes: =
Re: svn commit: r1411296 - /subversion/branches/1.6.x/STATUS
On Mon, Nov 19, 2012 at 5:54 PM, Daniel Shahaf d...@daniel.shahaf.namewrote: stef...@apache.org wrote on Mon, Nov 19, 2012 at 16:45:30 -: @@ -74,7 +74,7 @@ Candidate changes: Branch: ^/subversion/branches/1.6.x-rep_write_cleanup Votes: - +1: breser, danielsh + +1: breser, danielsh, stefan2 If you move this paragraph below the Approved changes header, the backport.pl cron will merge it. Thanks for the hint! Done in r1411308. -- Stefan^2. -- Certified Supported Apache Subversion Downloads: * http://www.wandisco.com/subversion/download *
named_atomic review
A few questions about named_atomic.c:
Index: subversion/libsvn_subr/named_atomic.c
===
--- subversion/libsvn_subr/named_atomic.c (revision 1411269)
+++ subversion/libsvn_subr/named_atomic.c (working copy)
@@ -309,6 +309,7 @@ delete_lock_file(void *arg)
/* locks have already been cleaned up. Simply close the file */
apr_file_close(mutex-lock_file);
+ /* ### check the return value? */
/* Remove the file from disk. This will fail if there ares still other
* users of this lock file, i.e. namespace. */
@@ -324,6 +325,7 @@ delete_lock_file(void *arg)
static svn_error_t *
validate(svn_named_atomic__t *atomic)
{
+ /* ### What is the purpose of this validation? What does it do? */
return atomic atomic-data atomic-mutex
? SVN_NO_ERROR
: svn_error_create(SVN_ERR_BAD_ATOMIC, 0, _(Not a valid atomic));
@@ -394,6 +396,7 @@ svn_atomic_namespace__create(svn_atomic_namespace_
const char *shm_name, *lock_name;
apr_finfo_t finfo;
+ /* ### Use a scratch_pool provided by caller? */
apr_pool_t *subpool = svn_pool_create(result_pool);
/* allocate the namespace data structure
@@ -421,10 +424,15 @@ svn_atomic_namespace__create(svn_atomic_namespace_
apr_pool_cleanup_register(result_pool, new_ns-mutex,
delete_lock_file,
apr_pool_cleanup_null);
+ /* ### If svn_atomic_namespace__create() is called twice, the cleanup
+ ### handler will be installed twice - so the lock will be removed
+ ### as soon as _either_ if them is fired. Problem? */
/* Prevent concurrent initialization.
*/
SVN_ERR(lock(new_ns-mutex));
+ /* ### What if two functions reach this line of code concurrently? Should
+ ### the rest of the function be an svn_atomic__init_once() callback? */
/* First, make sure that the underlying file exists. If it doesn't
* exist, create one and initialize its content.
Index: subversion/include/private/svn_named_atomic.h
===
--- subversion/include/private/svn_named_atomic.h (revision 1411269)
+++ subversion/include/private/svn_named_atomic.h (working copy)
@@ -104,8 +104,10 @@ svn_atomic_namespace__cleanup(const char *name,
* characters and an error will be returned if the specified name is longer
* than supported.
*
- * @note The lifetime of the atomic is bound to the lifetime
+ * @note The lifetime of the atomic object is bound to the lifetime
* of the @a ns object, i.e. the pool the latter was created in.
+ * The namespace persists as long as at least one process holds
+ * an #svn_atomic_namespace__t object corresponding to it.
*/
svn_error_t *
svn_named_atomic__get(svn_named_atomic__t **atomic,
I've skipped some details, but those that I haven't appear to be
straightforward.
Cheers
Daniel
Re: r1397773 - rep-sharing in a txn - subversion/libsvn_fs_fs/fs_fs.c
Julian Foad wrote on Mon, Nov 19, 2012 at 15:16:44 +: Stefan Fuhrmann wrote: relatively rare and constructing an error object can be expensive (NLS). svn_stringbuf_from_file2 is also a very convenient function to use. I wonder if we could change our generic error handling scheme to defer NLS look-up until the time when we print the error. That would eliminate that overhead in cases where we handle and clear an error without printing it. Something to think about for the future. I thought we considered error object creation to be expensive for other reasons, too, not just for NLS reasons? (eg, using a global pool) Daniel
Re: AW: duplicate externals cause update error -- thoughts?
On 2012-11-19 16:20, Julian Foad wrote: let the *caller* check for duplicates. I was, actually, also thinking about that at some point... I guess that's the best call after all. I'll limit to propset/propedit and provide a function to do the check. /me still working on it ~Neels signature.asc Description: OpenPGP digital signature
Re: r1397773 - rep-sharing in a txn - subversion/libsvn_fs_fs/fs_fs.c
On Mon, Nov 19, 2012 at 6:43 AM, Stefan Fuhrmann stefan.fuhrm...@wandisco.com wrote: A crashed writer process may leave a corrupt protorev and / or other incomplete files. There is no atomic incremental change here. The caller (client) using the crashed process is supposed to detect the crash and abandon the transaction. I don't think that supposed to is documented anywhere (unless you want to consider our clients behavior as documenation). After dealing with the stuck being_written flag that produced rep_write_cleanup, I don't think we should assume that going forward. It may not be worth dealing with in fsfs but within fsfs2 I think we should make representation operations even to the protorev files (or whatever their equivalent is) be atomic. Especially since these operations are usually driven across separate HTTP requests when we're using DAV. Then again, beyond improving repo consistency in some edge cases it'd also allow us to support parallelizing the PUTs. There may be other issues I'm not aware of off the top of my head in doing that right now, but at least in fsfs it's absolutely not supported since for the entire duration of the PUT you have the protorev file locked.
reposurgeon now has full Subversion support
Some of you will recall that I did a lot of work early this year on the dump stream documentation as part of an effort to enable reposurgeon to read Subversion dump files directly, translating Subversion histories into a DVCS-style commit DAG that can then be exported into git, hg, or bzr. I am pleased to be able to announce that this effort has been (somewhat belatedly) successful. The project had stalled for six months, but production-quality Subversion support in reposurgeon has now been verified by successful lift to DVCS of a large, old Subversion repository with lots of ugly metadata corner cases in it (the repo of the Network UPS Tools project). Not only does it work, it even works *fast*. The dumpfile analyzer cranks through more than a thousand commits per minute on vanilla desktop hardware. Subversion contributor Greg Hudson deserves credit for this, as he contributed a performance patch implementing copy-on-write filemaps that tremendously sped up processing. More than that, a slight extension of Greg's idea enabled me to abolish some code that I had suspected (correctly, it turns out) of harboring the subtle bug that had stalled the project for half a year - eliminating another O(n**2) lookup that Greg hadn't been originally targeting in the process. It may be of interest that the bug involved incorrect translation of two successive copies in opposite directions across a pair of branches. Another case that gave me trouble was a branch delete followed by a copy to the same branch name. A third was a directory copy followed by a file change in one of the copied files *before* commit. There are also various mixtures of file system copies with Subversion copy and commit operations that a tool like this needs to detect and patch so the history looks as though proper Subversion operations were used throughout, otherwise the commit DAG will be missing some ancestry links that semantically ought to be there. For example, if you (a) create a branch directory, (b) use file system copy to populate it from another branch, and (c) commit, the DAG builder needs to detect this and treat step (b) as though it had been done with Subversion directory and/or file copies. Fortunately this is a relatively simple exercise in hash matching. I'm still polishing; one thing that needs more work is interpretation of mergeinfo properties. The cherry-picking model Subversion uses doesn't match the way git/hg/bzr want to do things. Simple mergeinfos translate well but there are complex cases that yield perverse-looking merge links. Still, all that wrestling with strange corner cases paid off - reposurgeon is now better at translating Subversion repos to DVCS histories than anything else out there. It even handles cross-branch mixed commits without breaking stride. But it doesn't try to do everything. One of the philosophical premises behind reposurgeon was that repository translation is more like literary translation than people who write repository-conversion tools normally understand. That is, low-level mechanical translations don't work very well - they need to be cleaned up by a human who understands the ontological mismatches between VCSes and the idioms of both source and target VCS. A very simple example of this requirement is: what should be done with Subversion revision references like r456 in commit comments? Not just what should they be translated into? but how can we even *recognize* them reliably? Humans come up with lots of variant ways to write these even within the same repo, and mechanical translators have trouble spotting them all. reposurgeon was built with the goal of amplifying human judgment (making it as easy as possible for a human to improve on reposurgeon's basic mechanical translation) rather than trying to eliminate human judgment. This choice now seems well vindicated. -- a href=http://www.catb.org/~esr/;Eric S. Raymond/a The only purpose for which power can be rightfully exercised over any member of a civilized community, against his will, is to prevent harm to others. His own good, either physical or moral, is not a sufficient warrant.-- John Stuart Mill, On Liberty, 1859
Re: reposurgeon now has full Subversion support
On 11/19/2012 01:45 PM, Eric S. Raymond wrote: Some of you will recall that I did a lot of work early this year on the dump stream documentation as part of an effort to enable reposurgeon to read Subversion dump files directly, translating Subversion histories into a DVCS-style commit DAG that can then be exported into git, hg, or bzr. I am pleased to be able to announce that this effort has been (somewhat belatedly) successful. Congratulations, Eric! -- C. Michael Pilato cmpil...@collab.net CollabNet www.collab.net Enterprise Cloud Development signature.asc Description: OpenPGP digital signature
[RFC]: Make svnlook (pg|pl) output format mimic svn (pg|pl) add --show-inherited-props option (Was: multiple targets for 'svnlook propget')
Recently Johan proposed supporting multiple targets for svnlook propget: http://svn.haxx.se/dev/archive-2012-11/0439.shtml One of the ideas that came of this was to make the output of 'svnlook (pl|pg)' mimic the output of 'svn (pl|pg)'. This dovetails nicely with my desire to add the ''show-inherited-props option to svnlook (pl|pg): http://svn.haxx.se/dev/archive-2012-11/0432.shtml The question is, how far do we go with the backwards incompatible changes to the output of svnlook (pg|pl)? Here's what we have today (skip ahead to PROPOSED CHANGES: if you already know all this): ### svn proplist Today: ### svn pl A\B Properties on 'A\B': svn:auto-props svn:global-ignores svn pl A\B -v Properties on 'A\B': svn:auto-props *.c=svn:eol-style=native *.h=svn:eol-style=native svn:global-ignores *.pyc svn pl A\B --show-inherited-props Properties inherited from 'C:\SVN\src-trunk-2\Debug\subversion\tests\cmdline\svn-test-work\working_copies\autoprop_tests-30': svn:auto-props Properties on 'A\B': svn:auto-props svn:global-ignores svn pl A\B --show-inherited-props -v Properties inherited from 'C:\SVN\src-trunk-2\Debug\subversion\tests\cmdline\svn-test-work\working_copies\autoprop_tests-30': svn:auto-props *.py=svn:eol-style=native *.cpp=svn:eol-style=native Properties on 'A\B': svn:auto-props *.c=svn:eol-style=native *.h=svn:eol-style=native svn:global-ignores *.pyc ### svnlook proplist Today: ### svnlook pl autoprop_tests-30 A/B svn:global-ignores svn:auto-props svnlook pl autoprop_tests-30 A/B -v svn:global-ignores : *.pyc svn:auto-props : *.c=svn:eol-style=native *.h=svn:eol-style=native ### svn propget Today: ### svn pg svn:auto-props ^^/A/B *.c=svn:eol-style=native *.h=svn:eol-style=native svn pg svn:auto-props ^^/A/B -v Properties on 'file:///C:/SVN/src-trunk-2/Debug/subversion/tests/cmdline/svn-test-work/repositories/autoprop_tests-30/A/B': svn:auto-props *.c=svn:eol-style=native *.h=svn:eol-style=native svn pg svn:auto-props A\B --show-inherited-props . - *.py=svn:eol-style=native *.cpp=svn:eol-style=native A\B - *.c=svn:eol-style=native *.h=svn:eol-style=native svn pg svn:auto-props A\B --show-inherited-props -v Properties inherited from '.': svn:auto-props *.py=svn:eol-style=native *.cpp=svn:eol-style=native Properties on 'A\B': svn:auto-props *.c=svn:eol-style=native *.h=svn:eol-style=native ### svnlook propget Today: ### svnlook pg autoprop_tests-30 svn:auto-props A/B *.c=svn:eol-style=native *.h=svn:eol-style=native (svnlook pg -v is not currently supported) PROPOSED CHANGES: ### FORMAT CHANGE: 'svnlook pl' outputs results similar to 'svn pl': svnlook pl autoprop_tests-30 A/B Properties on '/A/B': svn:auto-props svn:global-ignores ### FORMAT CHANGE: 'svnlook pl -v' outputs results like 'svn pl -v' svnlook pl autoprop_tests-30 A/B -v Properties on '/A/B': svn:auto-props *.c=svn:eol-style=native *.h=svn:eol-style=native svn:global-ignores *.pyc ### NEW OPTION: Add support for 'svnlook pl --show-inherited-props' with output similar to 'svn pl --show-inherited-props': svnlook pl autoprop_tests-30 A/B --show-inherited-props Properties inherited from '/': svn:auto-props Properties on '/A/B': svn:auto-props svn:global-ignores ### NEW OPTION: As above, but with -v: svnlook pl autoprop_tests-30 A/B --show-inherited-props -v Properties inherited from '/': svn:global-ignores *.pyc Properties on '/A/B': svn:auto-props *.c=svn:eol-style=native *.h=svn:eol-style=native Properties on '/A/B': svn:auto-props *.c=svn:eol-style=native *.h=svn:eol-style=native ### NO CHANGE: 'svnlook pg' stays the same: svnlook pg autoprop_tests-30 svn:auto-props A/B *.c=svn:eol-style=native *.h=svn:eol-style=native ### NEW OPTION: Add support for 'svnlook pg -v' with output similar to 'svn pg -v': svnlook pg autoprop_tests-30 svn:auto-props A/B -v Properties on '/A/B': svn:auto-props *.c=svn:eol-style=native *.h=svn:eol-style=native ### NEW OPTION: Add support for 'svnlook pg --show-inherited-props' with output similar to 'svn pg --show-inherited-props': svnlook pg autoprop_tests-30 svn:auto-props A/B --show-inherited-props . - *.py=svn:eol-style=native *.cpp=svn:eol-style=native A\B - *.c=svn:eol-style=native *.h=svn:eol-style=native ### As above, but with new -v option: svnlook pg autoprop_tests-30 svn:auto-props A/B --show-inherited-props -v Properties inherited from '/': svn:auto-props *.py=svn:eol-style=native *.cpp=svn:eol-style=native Properties on '/A/B': svn:auto-props *.c=svn:eol-style=native *.h=svn:eol-style=native So the only backwards incompatible changes we'd be making compared to 1.7 are for 'svnlook pl' and 'svnlook pl -v', everything else proposed above is the result of a new option. Is this acceptable? -- Paul T. Burba CollabNet, Inc. -- www.collab.net -- Enterprise Cloud Development Skype: ptburba
Re: reposurgeon now has full Subversion support
On Mon, Nov 19, 2012 at 01:50:56PM -0500, C. Michael Pilato wrote: On 11/19/2012 01:45 PM, Eric S. Raymond wrote: Some of you will recall that I did a lot of work early this year on the dump stream documentation as part of an effort to enable reposurgeon to read Subversion dump files directly, translating Subversion histories into a DVCS-style commit DAG that can then be exported into git, hg, or bzr. I am pleased to be able to announce that this effort has been (somewhat belatedly) successful. Congratulations, Eric! Indeed, very nice! Combined with svnrdump this should allow anyone to convert any publically accessible SVN repository to git/hg/bzr with history preservation :) Would reposurgeon in theory be able to use SVN dump files as a destination as well as a source? Or perhaps the answer is to teach 'svnadmin load' about third-party formats, such as git's fast-import (which would then need to solve a rather similar data model mapping problem)?
Re: [RFC]: Make svnlook (pg|pl) output format mimic svn (pg|pl) add --show-inherited-props option
On Mon, Nov 19, 2012 at 9:04 PM, Paul Burba ptbu...@gmail.com wrote:
Recently Johan proposed supporting multiple targets for svnlook
propget: http://svn.haxx.se/dev/archive-2012-11/0439.shtml
One of the ideas that came of this was to make the output of 'svnlook
(pl|pg)' mimic the output of 'svn (pl|pg)'. This dovetails nicely
with my desire to add the ''show-inherited-props option to svnlook
(pl|pg): http://svn.haxx.se/dev/archive-2012-11/0432.shtml
+1 in general. Thanks for taking this on.
I have a couple of questions though ... see below.
The question is, how far do we go with the backwards incompatible
changes to the output of svnlook (pg|pl)? Here's what we have today
(skip ahead to PROPOSED CHANGES: if you already know all this):
### svn proplist Today: ###
svn pl A\B
Properties on 'A\B':
svn:auto-props
svn:global-ignores
svn pl A\B -v
Properties on 'A\B':
svn:auto-props
*.c=svn:eol-style=native
*.h=svn:eol-style=native
svn:global-ignores
*.pyc
svn pl A\B --show-inherited-props
Properties inherited from
'C:\SVN\src-trunk-2\Debug\subversion\tests\cmdline\svn-test-work\working_copies\autoprop_tests-30':
svn:auto-props
Properties on 'A\B':
svn:auto-props
svn:global-ignores
So this means that you don't show which node is actually the target
of these inherited props. What happens when you provide multiple
arguments ('svn pl A/B C/D --show-inherited-props')? Can a user see
(or a tool parse) which inherited props apply to which, or do they
have to depend on the order? What if one of the targets doesn't have
properties of its own (so no 'Properties on X' line), but does have
inherited props ... how will you know on which node they apply?
Maybe the output should become something like:
Properties on 'A\B' inherited from
'C:\SVN\src-trunk-2\Debug\subversion\tests\cmdline\svn-test-work\working_copies\autoprop_tests-30':
svn:auto-props
Properties on 'A\B':
svn:auto-props
svn:global-ignores
?
The same applies to propget.
svn pl A\B --show-inherited-props -v
Properties inherited from
'C:\SVN\src-trunk-2\Debug\subversion\tests\cmdline\svn-test-work\working_copies\autoprop_tests-30':
svn:auto-props
*.py=svn:eol-style=native
*.cpp=svn:eol-style=native
Properties on 'A\B':
svn:auto-props
*.c=svn:eol-style=native
*.h=svn:eol-style=native
svn:global-ignores
*.pyc
### svnlook proplist Today: ###
svnlook pl autoprop_tests-30 A/B
svn:global-ignores
svn:auto-props
svnlook pl autoprop_tests-30 A/B -v
svn:global-ignores : *.pyc
svn:auto-props : *.c=svn:eol-style=native
*.h=svn:eol-style=native
### svn propget Today: ###
svn pg svn:auto-props ^^/A/B
*.c=svn:eol-style=native
*.h=svn:eol-style=native
Note that this output (without -v) changes when there are multiple targets:
svn pg svn:auto-props A/B C/D
A/B - *.c=svn:eol-style=native
*.h=svn:eol-style=native
C/D - *.py=svn:eol-style=native
*.pl=svn:eol-style=native
Or with a single-line prop:
svn pg svn:eol-style foo.txt bar.txt
foo.txt - native
bar.txt - native
This output isn't exactly great (not parseable for multiline props),
but it's okay for single-line props. In any case, we should also
define what this (multi-arg but without -v) looks like in the case of
'svnlook pg'.
svn pg svn:auto-props ^^/A/B -v
Properties on
'file:///C:/SVN/src-trunk-2/Debug/subversion/tests/cmdline/svn-test-work/repositories/autoprop_tests-30/A/B':
svn:auto-props
*.c=svn:eol-style=native
*.h=svn:eol-style=native
svn pg svn:auto-props A\B --show-inherited-props
. - *.py=svn:eol-style=native
*.cpp=svn:eol-style=native
A\B - *.c=svn:eol-style=native
*.h=svn:eol-style=native
svn pg svn:auto-props A\B --show-inherited-props -v
Properties inherited from '.':
svn:auto-props
*.py=svn:eol-style=native
*.cpp=svn:eol-style=native
Properties on 'A\B':
svn:auto-props
*.c=svn:eol-style=native
*.h=svn:eol-style=native
### svnlook propget Today: ###
svnlook pg autoprop_tests-30 svn:auto-props A/B
*.c=svn:eol-style=native
*.h=svn:eol-style=native
(svnlook pg -v is not currently supported)
PROPOSED CHANGES:
### FORMAT CHANGE: 'svnlook pl' outputs results similar to 'svn pl':
svnlook pl autoprop_tests-30 A/B
Properties on '/A/B':
svn:auto-props
svn:global-ignores
### FORMAT CHANGE: 'svnlook pl -v' outputs results like 'svn pl -v'
svnlook pl autoprop_tests-30 A/B -v
Properties on '/A/B':
svn:auto-props
*.c=svn:eol-style=native
*.h=svn:eol-style=native
svn:global-ignores
*.pyc
### NEW OPTION: Add support for 'svnlook pl --show-inherited-props'
with output similar to 'svn pl --show-inherited-props':
svnlook pl autoprop_tests-30 A/B --show-inherited-props
Properties inherited from '/':
svn:auto-props
Properties on '/A/B':
svn:auto-props
svn:global-ignores
### NEW OPTION: As above, but with -v:
svnlook pl autoprop_tests-30 A/B --show-inherited-props -v
Properties
Re: 'svn diff' vs 'svnlook diff'
On Mon, Nov 19, 2012 at 2:47 PM, Julian Foad julianf...@btopenworld.com wrote: Johan Corveleyn wrote: Daniel Shahaf wrote: Johan Corveleyn wrote: I currently have a patch sitting here for adding --diff-cmd to 'svnlook diff', I wonder what's the minimal change we could make to the cmdline client such that it can operate on transactions (and thus void the need to reimplement every svn proplist/diff/cat/info switch in svnlook). (Read-only, at least initially.) Is this something Julian's tree-read-api branch would address? Yes my tree-read-api work would make this sort of thing easier. Maybe we need to implement svn_ra_local_txn (like ra_local, but with HEAD being a transaction instead of a revision)? Other ideas? Move the core tree-diffing functionality down a layer from libsvn_client into libsvn_diff. Let 'svn' pass some kinds of 'tree description' inputs to it (from the WC and RA interfaces) and let 'svnlook' pass other kinds of 'tree description' inputs (revisions and txns, from the repos layer). Am I right in thinking that this is something that will probably take some time to complete (i.e. not for 1.8)? I'm just wondering whether I should commit my patch for adding --diff-cmd to 'svnlook diff'. If you (or anyone else) intend to merge both diff drivers soonish, it doesn't make much sense to work specifically on --diff-cmd. I'd like to note that the output of 'svnlook diff' is slightly different from 'svn diff', and I'd like to preserve that different behavior (or at least preserve the svnlook behavior here). IMO the output of 'svnlook diff' is better suited for post-commit emails. Ugh. Most of these differences are (IMO) unwanted. I basically agree with your comments below about which ones are better. Agreed. Would be nice indeed if both implementations could converge, and take on the best format. -- Johan
Re: reposurgeon now has full Subversion support
Stefan Sperling s...@elego.de: Would reposurgeon in theory be able to use SVN dump files as a destination as well as a source? I have considered this possibility. It would be difficult, and it would expose me to support issues I don't think I want to deal with. Or perhaps the answer is to teach 'svnadmin load' about third-party formats, such as git's fast-import (which would then need to solve a rather similar data model mapping problem)? You guys (the Subversion dev group) would be better equipped for the job. I think there are decent reasons for you to try it. But beware that there are some boojums among the snarks here. The basic problem is the nature of the mismatch between Subversion's ontology and the gitspace ontology of fast-import streams. Here are several of Subversion's ontological premises: 1. Names of containers matter 2. Branches are copies 3. The basic merge operation is cherry-picking 4. User identities are local to an assumed namespace 5. Versions are strictly time-ordered and there is only one clock In gitspace, every one of those premises is wrong. I was going to write about this at more length but I have a kung-fu class to get to. Perhaps I'll start one later tonight. -- a href=http://www.catb.org/~esr/;Eric S. Raymond/a
