You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Kean Johnston <jk...@sco.com> on 2005/11/19 21:27:52 UTC
[PATCH]: Change sort order for 'svn diff'
All,
Attached is my first patch for Subversion, so go easy with the
flames :) This is to change the order in which files are shown
by and 'svn diff', such that all files in a directory are shown
first, alphabetically, then all directories, also alphabetically.
I put the function svn_repos_sort_by_type_and_path() in
libsvn_repos, since that is the only consumer of the function.
However, it sorts entries of type svn_fs_dirent_t, so perhaps
a better location would have been libsvn_fs. I am not sure this
is a generically useful function outside of libsvn_repos which
is why it is where it currently is. I originally had the function
as a static inside both delta.c and reporter.c, but thats a bit
wasteful so I make it a visible function.
A similar function was needed in libsvn_wc, but there there is
only a single consumer of it, so I made it static instead of
adding a new API. That version differs from the one in libsvn_repos
only in the data types it sorts on (svn_wc_entry_t).
Please feel free to whack me upside the head with a big SVN
cluestick if this patch is not up to par.
Kean
Re: [PATCH]: Change sort order for 'svn diff'
Posted by Daniel Berlin <db...@dberlin.org>.
On Sun, 2005-11-20 at 08:51 +0100, Branko Čibej wrote:
> Kean Johnston wrote:
> > All,
> >
> > Attached is my first patch for Subversion, so go easy with the
> > flames :) This is to change the order in which files are shown
> > by and 'svn diff', such that all files in a directory are shown
> > first, alphabetically, then all directories, also alphabetically.
> I'm worried that this patch introduces sorting for diff output, but
> ignores other commands. If we start sorting the output we produce, then
> it should be sorted for all commands -- checkout, update, status, list,
> merge, switch, ...
>
Well, considering we are already in the position where some commands are
sorted, and others not, i'm not sure this is so much of a concern.
In particular, list's output is already sorted, but the rest are not.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH]: Change sort order for 'svn diff'
Posted by Julian Foad <ju...@btopenworld.com>.
Kean Johnston wrote:
>> My point is that there /might/ be a better place to do this than in
>> the diff driver, such that more commands would benefit from the
>> change. I'm not saying that you should do everything at once, but if
>> we can get a bit more benefit for the same effort, I'd say that's a
>> good thing.
>
> Yup I agree 100%. I suspect Julian found that place a while back.
Due to the fact that many of our interfaces return things in a hash, there is
no getting away from having to do the sorting in lots of places (unless we
change the interfaces or design a hash iterator that iterates in sorted order).
> Unfortunately he's posted a LOT but I am trying to find mail from
> him about it :)
If you're looking for my old mail, the threads were:
2003-10-13 "Sorted order for output of "status" etc."
2004-05-31 "PATCH: Perform multi-file operations in sorted order"
And these are related:
2004-05-30 "RFC: Hash dumps in sorted order" (Greg Hudson)
2005-02-22 "Sorted output from Subversion commands" (Branko Cibej)
(Sorry for not providing links to the archives. I looked them up in my mailbox.)
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH]: Change sort order for 'svn diff'
Posted by Kean Johnston <jk...@sco.com>.
> My point is that there /might/ be a better place to do this than in the
> diff driver, such that more commands would benefit from the change. I'm
> not saying that you should do everything at once, but if we can get a
> bit more benefit for the same effort, I'd say that's a good thing.
Yup I agree 100%. I suspect Julian found that place a while back.
Unfortunately he's posted a LOT but I am trying to find mail from
him about it :)
Kean
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH]: Change sort order for 'svn diff'
Posted by Branko Čibej <br...@xbc.nu>.
Kean Johnston wrote:
>> I'm worried that this patch introduces sorting for diff output, but
>> ignores other commands. If we start sorting the output we produce,
>> then it should be sorted for all commands -- checkout, update,
>> status, list, merge, switch, ...
> Maybe it might already. I didnt look at every single
> command but I made the change in the place Dan Berlin suggested,
> in libsvn_repos/reporter.c, and that seemed to affect checkout
> at the very least.
>
> However, I will say, that the order in which things are checked
> out or status reported is considerably less important than the
> order in which files are diff'ed. One doesn't use those commands
> for preparing patches and for producing editable output (in
> general). However, producing a patch, especially one which you
> may need to edit or split up into groups of files, is a far more
> common task and a part of daily development. I'm having a hard
> time thinking of a case where the order in which files are updated
> will really matter, but I'm sure someone can think of one.
>
>> IIRC, the last time we discussed this, there was a fairly good
>> proposal as to where such a change should be made.
> Ok I wasn't aware of previous discussions as I only recently started
> reading this list again. However, when I originally posted about
> this no-one other than Julian Foad mentioned any earlier work, and
> he mentioned that there was only mild enthusiasm for making everything
> sorted. Perhaps Julian and I should work together to see if we can
> extend his earlier work to take node type (file vs dir) into account?
> Julian?
My point is that there /might/ be a better place to do this than in the
diff driver, such that more commands would benefit from the change. I'm
not saying that you should do everything at once, but if we can get a
bit more benefit for the same effort, I'd say that's a good thing.
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH]: Change sort order for 'svn diff'
Posted by Julian Foad <ju...@btopenworld.com>.
Kean Johnston wrote:
>> I'm worried that this patch introduces sorting for diff output, but
>> ignores other commands. If we start sorting the output we produce,
>> then it should be sorted for all commands -- checkout, update, status,
>> list, merge, switch, ...
>
> Maybe it might already. I didnt look at every single
> command but I made the change in the place Dan Berlin suggested,
> in libsvn_repos/reporter.c, and that seemed to affect checkout
> at the very least.
>
> However, I will say, that the order in which things are checked
> out or status reported is considerably less important than the
> order in which files are diff'ed.
Yes, in general: diff is quite important for the reasons you stated. But
actually I find unsorted "status" output to be quite obnoxious too.
>> IIRC, the last time we discussed this, there was a fairly good
>> proposal as to where such a change should be made.
>
> Ok I wasn't aware of previous discussions as I only recently started
> reading this list again. However, when I originally posted about
> this no-one other than Julian Foad mentioned any earlier work, and
> he mentioned that there was only mild enthusiasm for making everything
> sorted. Perhaps Julian and I should work together to see if we can
> extend his earlier work to take node type (file vs dir) into account?
> Julian?
Choosing the sort order is the easy bit. The difficulties I had were in trying
to make the wc-and-repos combined operations sorted. However, you might learn
something useful by looking at what I did for getting wc-only and repos-only
things sorted.
Here's the log message from the last version of my patch, so you can see in
which functions I put the sorting. I'll try to update the patch so that it
applies to trunk, and then post it, later today. The table of which commands
are sorted is out of date because the project source code has changed since I
last updated it over a year ago. Some parts of the patch will be broken as a
result.
[[[
Perform multi-file operations in the sorted order of the file names.
This makes the output of operations like "svn diff" and "svn status"
predictable, repeatable, easy to read, etc., for the user's benefit, and
also provides a better foundation for machine processing of Subversion
output and even internal operations.
This patch addresses the following operations (+=yes, .=n/a, -=no, ?=was sorted
by this patch in the past, but needs re-checking):
- add
. blame
. cat
- checkout
. cleanup
? commit
. copy
? delete [1]
? diff (locally) [1]
- diff (contacting repository) [1]
? export
. help
? import
? info
? list
. log
? merge [1]
. mkdir
? move
. propedit
? propget,proplist
? propset,propdel
? resolved
? revert
? status (locally)
- status (contacting repository)
? switch,update [1]
[1] Sorted except that directories can be reported after their children.
File and directory names are sorted in the numerical order of their Unicode
representations. This order is fixed (because it is implemented on the server
as well as the client) and is not affected by locale or case-insensitive file
systems.
This patch sorts sequences of path names that are generated within Subversion.
It does not, for instance, sort a list of targets specified by the user.
Tweak a regression test so that it does not depend on "svn status" reporting
unversioned files before ".".
* subversion/clients/cmdline/propget-cmd.c (svn_cl__propget)
Sort the properties by filename before printing them.
* subversion/libsvn_client/commit.c (import_dir)
Call svn_io_get_dirents instead of doing the hard work manually.
Sort the dir entries before processing and recursing.
* subversion/libsvn_client/export.c (copy_versioned_files)
Sort the dir entries before processing and recursing.
* subversion/libsvn_client/prop_commands.c (remote_propget, remote_proplist)
Sort the dir entries before processing and recursing.
* subversion/libsvn_repos/delta.c (delta_dirs)
Bring together all additions, changes and deletions rather than leaving
deletions until afterwards.
Sort the FS dir entries before processing and recursing, so as to
emit the editing operations in sorted order.
* subversion/libsvn_wc/adm_crawler.c (report_revisions)
* subversion/libsvn_wc/adm_ops.c (svn_wc_revert, mark_tree)
* subversion/libsvn_wc/diff.c (directory_elements_diff)
* subversion/libsvn_wc/entries.c (walker_helper)
Sort the WC entries before processing and recursing.
* subversion/libsvn_wc/status.c
(get_dir_status): Bring together all versioned and unversioned items from
a single directory, instead of handling all the unversioned items first.
Sort them before recursing and reporting.
(handle_statii): Sort the entries before recursing and reporting.
(close_directory): Report "." before its children, not after them.
* subversion/tests/clients/cmdline/stat_tests.py
(status_for_unignored_file): Commit the property change so that no status is
reported for "this dir", to avoid depending on the reporting order.
]]]
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH]: Change sort order for 'svn diff'
Posted by Kean Johnston <jk...@sco.com>.
> I'm worried that this patch introduces sorting for diff output, but
> ignores other commands. If we start sorting the output we produce, then
> it should be sorted for all commands -- checkout, update, status, list,
> merge, switch, ...
Maybe it might already. I didnt look at every single
command but I made the change in the place Dan Berlin suggested,
in libsvn_repos/reporter.c, and that seemed to affect checkout
at the very least.
However, I will say, that the order in which things are checked
out or status reported is considerably less important than the
order in which files are diff'ed. One doesn't use those commands
for preparing patches and for producing editable output (in
general). However, producing a patch, especially one which you
may need to edit or split up into groups of files, is a far more
common task and a part of daily development. I'm having a hard
time thinking of a case where the order in which files are updated
will really matter, but I'm sure someone can think of one.
> IIRC, the last time we discussed this, there was a fairly good proposal
> as to where such a change should be made.
Ok I wasn't aware of previous discussions as I only recently started
reading this list again. However, when I originally posted about
this no-one other than Julian Foad mentioned any earlier work, and
he mentioned that there was only mild enthusiasm for making everything
sorted. Perhaps Julian and I should work together to see if we can
extend his earlier work to take node type (file vs dir) into account?
Julian?
Kean
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH]: Change sort order for 'svn diff'
Posted by Branko Čibej <br...@xbc.nu>.
Kean Johnston wrote:
> All,
>
> Attached is my first patch for Subversion, so go easy with the
> flames :) This is to change the order in which files are shown
> by and 'svn diff', such that all files in a directory are shown
> first, alphabetically, then all directories, also alphabetically.
I'm worried that this patch introduces sorting for diff output, but
ignores other commands. If we start sorting the output we produce, then
it should be sorted for all commands -- checkout, update, status, list,
merge, switch, ...
IIRC, the last time we discussed this, there was a fairly good proposal
as to where such a change should be made.
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH]: Change sort order for 'svn diff'
Posted by Malcolm Rowe <ma...@farside.org.uk>.
On Sun, Nov 20, 2005 at 10:58:44PM +0100, Peter N. Lundblad wrote:
> I'm not opposed to making output of svn commands sorted (OTOH; I like it a
> lot). I just want to point out something that might not be obvious to
> everyone.
>
> The deterministic order will be localce dependent. That's a good thing,
> IMO. The only problem is that if you do sorting on the server, you will
> depend on the locale of the server process..
Ah, hadn't thought of that, yes. We could presumably sort everything
using a non-collation aware sort, couldn't we (i.e., codepoint by
codepoint)?
I'm happy enough if we make the output of 'svn diff' deterministic,
even if it's not actually well-defined.
Regards,
Malcolm
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH]: Change sort order for 'svn diff'
Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
On Sun, 20 Nov 2005, Malcolm Rowe wrote:
> On Sun, Nov 20, 2005 at 12:59:18PM -0800, Kean Johnston wrote:
> > >I am curious about the ordering you picked: was there any
> > >(performance? usability?) reason for it? I guess if I had to come up
> > >with an ordering for diff myself, I'd just have picked 'sort by name',
> > >not 'files first, directories second, sorted by name'.
> > >
> > [...]
> >
> > Lets take a real-world example. Suppose I am working on gcc,
> > and I have made changes that span most of its sub-trees. If
> > we were to sort the entire list alphabetically, I would have
> > my diff to calls.c followed by patches to files in config/i386
> > followed by the diff to toplev.c. But patches in subdirectories
> > tend to be logically grouped.
>
> Ah, yes. That does make perfect sense, thanks!
>
I'm not opposed to making output of svn commands sorted (OTOH; I like it a
lot). I just want to point out something that might not be obvious to
everyone.
The deterministic order will be localce dependent. That's a good thing,
IMO. The only problem is that if you do sorting on the server, you will
depend on the locale of the server process.. That might not be a big
problem, but it will be confusing to some people. I honestly don't know
what to do about it. Communicating the locale information from client to
server isn't trivial to do portably and changing the server process locale
as per the client's preferences isn't always an option. Think
multitheading servers on Unix.
Just wanted to make sure people are aware of this.
Regards,
//Peter
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH]: Change sort order for 'svn diff'
Posted by Malcolm Rowe <ma...@farside.org.uk>.
On Sun, Nov 20, 2005 at 12:59:18PM -0800, Kean Johnston wrote:
> >I am curious about the ordering you picked: was there any
> >(performance? usability?) reason for it? I guess if I had to come up
> >with an ordering for diff myself, I'd just have picked 'sort by name',
> >not 'files first, directories second, sorted by name'.
> >
> [...]
>
> Lets take a real-world example. Suppose I am working on gcc,
> and I have made changes that span most of its sub-trees. If
> we were to sort the entire list alphabetically, I would have
> my diff to calls.c followed by patches to files in config/i386
> followed by the diff to toplev.c. But patches in subdirectories
> tend to be logically grouped.
Ah, yes. That does make perfect sense, thanks!
Regards,
Malcolm
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH]: Change sort order for 'svn diff'
Posted by Kean Johnston <jk...@sco.com>.
> I am curious about the ordering you picked: was there any
> (performance? usability?) reason for it? I guess if I had to come up
> with an ordering for diff myself, I'd just have picked 'sort by name',
> not 'files first, directories second, sorted by name'.
Actually there was a good reason (well, thats a subjective statement,
*I* believe its a good reason, you be the judge).
It is very common practice, when contributing large patches,
to have the maintainers ask you to split the patch up into
multiple smaller pieces, so that they can be more effectively
reviewed. There are two ways of doing that:
a) run svn diff fon specific sets of files for each patch
b) run wvn diff on the whole tree, and use the generated output
to work on a CHangeLong, and split the large diff into its
smaller parts.
It is in light of option (b) that I chose the ordering I did.
Lets take a real-world example. Suppose I am working on gcc,
and I have made changes that span most of its sub-trees. If
we were to sort the entire list alphabetically, I would have
my diff to calls.c followed by patches to files in config/i386
followed by the diff to toplev.c. But patches in subdirectories
tend to be logically grouped. Thus, the patches in config/* are
likely to be target-specific patches, whereas the patches in the
main gcc directory are generic. I would normally want to put
all of the generic fixes in one patch, and platform or target
sepcific ones in another. A simple lexical sort would foil that.
If you are not familiar with the gcc tree, pretend its the svn
tree itself. If I made patches in libsvn_fs_base, I am much
more likely to want to group all of the *.[ch] files into a
patch, then all the bdb/* into another, then util/* into yet
another. This is a poorer example becuase the layout of svn
actually lends itself better to things being "sensible" if
a simple lexical ordering was used, but you get the idea.
Kean
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH]: Change sort order for 'svn diff'
Posted by Malcolm Rowe <ma...@farside.org.uk>.
On Sat, Nov 19, 2005 at 01:27:52PM -0800, Kean Johnston wrote:
> Attached is my first patch for Subversion, so go easy with the
> flames :) This is to change the order in which files are shown
> by and 'svn diff', such that all files in a directory are shown
> first, alphabetically, then all directories, also alphabetically.
I like the idea of making the output from 'svn diff' deterministic, and
I'm personally not too bothered about the precise order, or whether it
matches the ordering of other svn subcommands (though it might be nice
if it did).
I am curious about the ordering you picked: was there any
(performance? usability?) reason for it? I guess if I had to come up
with an ordering for diff myself, I'd just have picked 'sort by name',
not 'files first, directories second, sorted by name'.
For repos->wc diffs with local modifications, we'll actually report the
repos->wc diffs followed by the wc->wc diffs, with each group sorted,
won't we? (instead of sorting the whole output, which would be hard).
I've not really reviewed the patch itself, since I'm not familiar enough
with libsvn_repos, but it looks generally good to me.
> Index: subversion/libsvn_repos/delta.c
> ===================================================================
> --- subversion/libsvn_repos/delta.c (revision 17442)
> +++ subversion/libsvn_repos/delta.c (working copy)
> void *val;
> @@ -970,8 +978,11 @@
> svn_pool_clear (subpool);
>
> /* KEY is the entry name in target, VAL the dirent */
> - apr_hash_this (hi, &key, &klen, &val);
> - t_entry = val;
> + val = item->value;
> + t_entry = item->value;
> + key = item->key;
> + klen = item->klen;
> +
I think you can probably delete the 'val' variable entirely: it's not
used anywhere else in this function.
> Index: subversion/libsvn_repos/repos.h
> ===================================================================
> --- subversion/libsvn_repos/repos.h (revision 17442)
> +++ subversion/libsvn_repos/repos.h (working copy)
> @@ -240,6 +241,14 @@
> apr_pool_t *pool);
>
>
> +/* Sort an svn_fs_direct_t hash first by file type (such that nodes of type
> + svn_node_file appear before svn_node_dir nodes), and then alphabetically
> + as defined by svn_path_compare_paths.
> +
> + This function is intended as a callback for svn_sort__hash(). */
> +int
> +svn_repos_sort_by_type_and_path (const svn_sort__item_t *item1,
> + const svn_sort__item_t *item2);
> #ifdef __cplusplus
> }
> #endif /* __cplusplus */
That should probably be svn_repos__sort_by_type_and_path(). (And isn't it
'type and name'?)
> Index: subversion/libsvn_wc/diff.c
> ===================================================================
> --- subversion/libsvn_wc/diff.c (revision 17442)
> +++ subversion/libsvn_wc/diff.c (working copy)
> @@ -691,9 +725,12 @@
>
> subpool = svn_pool_create (dir_baton->pool);
>
> - for (hi = apr_hash_first (dir_baton->pool, entries); hi;
> - hi = apr_hash_next (hi))
> + sorted = svn_sort__hash (entries, sort_by_type_and_path, dir_baton->pool);
> +
> + for (i = 0; i < sorted->nelts; ++i)
> {
> + const svn_sort__item_t *item = &APR_ARRAY_IDX(sorted, i,
> + const svn_sort__item_t);
> const void *key;
> void *val;
> const svn_wc_entry_t *entry;
Again, 'val' isn't used after your patch, so you could remove it.
And 'key' is also unused (except for comparing against
SVN_WC_ENTRY_THIS_DIR, below, where we could easily use 'name'), so I'd
get rid of that as well.
> @@ -702,9 +739,9 @@
>
> svn_pool_clear (subpool);
>
> - apr_hash_this (hi, &key, NULL, &val);
> - name = key;
> - entry = val;
> + name = item->key;
> + key = item->key;
> + entry = item->value;
>
> /* Skip entry for the directory itself. */
> if (strcmp (key, SVN_WC_ENTRY_THIS_DIR) == 0)
Regards,
Malcolm
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH]: Change sort order for 'svn diff'
Posted by Kean Johnston <jk...@sco.com>.
> For this particular patch, I need to ask: what kinds of diffs does it
> guarantee to be in order? From a quick look, I'd guess BASE-to-working
> diffs in a WC, and repos-to-repos diffs, but not repos-to-WC diffs. Do
> you think that is correct? (I haven't tried it.)
Yes and no. In clients/cmdline/svn, no matter what form of
diff you request, you end up calling one of two functions: either
svn_client_diff_peg3() or svn_client_diff3(). The former ends up
calling do_diff_peg() and the latter ends up calling do_diff().
Both of those will call one of diff_repos_repos(), diff_repos_wc(),
or diff_wc_wc().
diff_wc_wc() calls svn_wc_diff3(), which ends up calling
directory_elements_diff(), which was one of the functions I changed
to adjust the sort order.
diff_repos_repos() and diff_repos_wc() call svn_ra_do_diff2() or
svn_ra_do_diff(), which call the do_diff "virtual" function, which
I believe gets us back to the same place as described for do_diff()
above.
With my patches, and a small local repository I created just for
testing, here's what I did. In all cases the output was from the
`COMMAND | grep ^Index:`. The desired order is:
Index: parser.y
Index: uidgid.c
Index: xfuncs.c
Index: cdmt/finalpkg.c
Index: cdmt/finalprd.c
Index: pkgadd/finalpkg.c
Index: pkgadd/finalprd.c
Index: pkgadd/produce.c
COMMAND: svn diff (Base-to-wc diff case)
Pre-patch output:
Index: uidgid.c
Index: pkgadd/finalprd.c
Index: pkgadd/produce.c
Index: pkgadd/finalpkg.c
Index: parser.y
Index: xfuncs.c
Index: cdmt/finalprd.c
Index: cdmt/finalpkg.c
Post-patch output:
(As expected above)
COMMAND: svn diff file:///u1/repo/trunk@2 file:///u1/repo/trunk@3
(repos-to-repos case):
Pre-patch output:
Index: uidgid.c
Index: pkgadd/finalprd.c
Index: pkgadd/produce.c
Index: pkgadd/finalpkg.c
Index: parser.y
Index: cdmt/finalprd.c
Index: cdmt/finalpkg.c
Index: xfuncs.c
Post-patch output:
(As expected above)
COMMAND: svn diff -r 2 (repos-to-wc diff case)
Pre-patch output:
Index: cdmt/finalprd.c
Index: cdmt/finalpkg.c
Index: uidgid.c
Index: pkgadd/finalprd.c
Index: pkgadd/produce.c
Index: pkgadd/finalpkg.c
Index: parser.y
Index: xfuncs.c
Post-patch output:
Index: cdmt/finalpkg.c
Index: cdmt/finalprd.c
Index: parser.y
Index: uidgid.c
Index: xfuncs.c
Index: pkgadd/finalpkg.c
Index: pkgadd/finalprd.c
Index: pkgadd/produce.c
So the repos-to-wc case is clearly being sorted, but it is not
sorting based on node type first. The levels of indirection
make my head hurt, so I dont know why this is the case. I will
start looking in to it but if someone could offer some insight
that will save me time, I would dearly appreciate it.
Kean
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH]: Change sort order for 'svn diff'
Posted by Julian Foad <ju...@btopenworld.com>.
Kean Johnston wrote:
>
> Attached is my first patch for Subversion, so go easy with the
> flames :) This is to change the order in which files are shown
> by and 'svn diff', such that all files in a directory are shown
> first, alphabetically, then all directories, also alphabetically.
Well, jolly good to get this ball rolling. Before actually reviewing the
patch, it will be interesting to see what people think of the idea.
My point of view is that output in some sort of deterministic order is very
valuable. Also that we don't have to make all of Subversion's operations
happen in deterministic order in a single release; we can do them as and when.
Each one will benefit some people.
One slightly unsettling aspect is that whatever order we choose will, once it
has been released, be the one and only order, because for some operations the
order is determined by the server and, for others, by the client, and, for
operations like "status" and "update", by both. Well, I suppose we could use
protocol options to negotiate an order, to some extent. I don't think this
should put us off.
For this particular patch, I need to ask: what kinds of diffs does it guarantee
to be in order? From a quick look, I'd guess BASE-to-working diffs in a WC,
and repos-to-repos diffs, but not repos-to-WC diffs. Do you think that is
correct? (I haven't tried it.)
> I put the function svn_repos_sort_by_type_and_path() in
> libsvn_repos, since that is the only consumer of the function.
Just to address that point, without getting into reviewing the patch: yes,
that's a good place for it, but its name should have a double underscore to
indicate it is private, and that function doesn't actually sort, it just
compares its arguments, so either rename it or (I'd prefer) make it take a hash
and return a sorted array.
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org