Re: The --password and clumsy users issue
On Fri, Jul 4, 2014 at 8:46 AM, Branko Čibej wrote: > On 04.07.2014 02:14, Gabriela Gibson wrote: > > I also looked at the C90 standard because I thought maybe they defined > argv as immutable (since it should not complain about being const with this > type of main declaration I think) and this is what is says: > > "The parameters argc and argv and the strings pointed to by the argv > array shall > be modifiable by the program, and retain their last-stored values > between program > startup and program termination." > > This seems a bit ambiguous --- so it's changeable, but between start up > and termination they retain their value? > > > Read your quote again. "they retain the *last-stored* value". > > > There is a comma (after the info that they are changeable), and then you get the info chunk that "and retain their last-stored values between program startup and program termination" which sets a start and end point and a condition in the middle. Since they are not changing at random (or we hope not!) and since we can be sure that they exist during execution(so the two conditions don't need to be mentioned especially, since they are a givens), it's reasonable (and probably wrong) to infer that the authors meant to convey that internally they can be modified, but externally, they are treated as immutable. TIL that the C90 standard contains a Rohrschachtest %-) Gabriela -- Visit my Coding Diary: http://gabriela-gibson.blogspot.com/
Re: The --password and clumsy users issue
What if we leave the current --password as is (for convenience) and just add an optional, secondary password mechanism for those admins who want to be doubly sure? On Fri, Jul 4, 2014 at 7:00 AM, Ben Reser wrote: > On 7/3/14 9:10 PM, Martin Furter wrote: > > 3) Allow the password to be supplied over stdin using the special value > "-". > > > > Nobody will see the password. The only leak is that a password has been > > supplied using stdin. An attacker will have to convince the calling > application > > to run something different than svn which logs the password to a file. > > A lot of tools have options to provide passwords via a file. That's > certainly > a better option for people who want to do automation and are concerned > with the > leakage via the command line. > > Just adding "-" as a special value for the existing --password option > doesn't > necessarily help people doing automation since you're going to have to do > something to send that password over stdin, which might end up leaked via > the > same mechanism (though usually things like echo are shell internals and > don't > spawn processes). > > I'd rather add a --password-file option and that can take the "-" special > value > for STDIN if people want and that other values just read the password form > the > path provided. > > The question then becomes is it better to just leave --password with > documentation and maybe some sort of redaction or should we just remove it > entirely. > > Yes it would take some minor effort for anyone upgrading that depends on > --password now, including our own test suite, but it ought to be rather > trivial > to write the password to a file with proper permissions and just start > passing > the name of the file on the command line. > > To that end how about: > > "--password-file ARG : specify a password stored in file ARG (secure > provided > proper permissions are set on the file, interactive users may prefer the > interactive password prompts when credentials are needed)" > > Deprecate --password in 1.9 but leave it with the following help text: > "(deprecated, insecure use --password-file instead)" > > Remove --password support entirely in 1.10 and emit an error telling users > to > use --password-file. > -- Visit my Coding Diary: http://gabriela-gibson.blogspot.com/
Re: The --password and clumsy users issue
I also looked at the C90 standard because I thought maybe they defined argv
as immutable (since it should not complain about being const with this type
of main declaration I think) and this is what is says:
"The parameters argc and argv and the strings pointed to by the argv array
shall
be modifiable by the program, and retain their last-stored values between
program
startup and program termination."
This seems a bit ambiguous --- so it's changeable, but between start up and
termination they retain their value?
So, that is maybe why the kernel's info isn't changing but the args can be
modified?
G
On Fri, Jul 4, 2014 at 12:50 AM, Gabriela Gibson
wrote:
> Oh, I was playing about with this earlier, but didn't get all that far:
>
> I go this far in svn.c (has breakpoint marker in it)
>
> [[[
> int
>
> main(int argc, char **argv)
>
> {
>
> apr_pool_t *pool;
>
> int exit_code = EXIT_SUCCESS;
>
> svn_error_t *err;
>
> const char **argv_org;
>
>
>
> /* Initialize the app. */
>
> if (svn_cmdline_init("svn", stderr) != EXIT_SUCCESS)
>
> return EXIT_FAILURE;
>
>
>
> /* Create our top-level pool. Use a separate mutexless allocator,
>
> * given this application is single threaded.
>
> */
>
> pool = apr_allocator_owner_get(svn_pool_create_allocator(FALSE));
>
>
>
> {
> int i;
>
>
>
> argv_org = apr_palloc(pool, sizeof(argv));
>
>
>
> B for(i = 0; i < argc; i++)
>
> {
>
> argv_org[i] = apr_pstrdup(pool, argv[i]);
>
> if (i > 2 && !strcmp(argv[i-1],"--password"))
>
> {
>
> argv[i-1] = "#\0";
>
> }
>
> if (i > 2 && !strcmp(argv[i-1],"--username"))
>
> {
>
> argv[i-1] = "#\0";
>
> }
>
>
>
> }
>
> }
> ]]]
>
>
> it compiles and runs and in gdb the vars do change, but the compiler isn't
> happy:
> subversion/svn/svn.c: In function 'main':
>
> subversion/svn/svn.c:3048:23: warning: assignment discards 'const'
> qualifier from pointer target type [enable\
> d by default]
>
> argv[i-1] = "#\0";
>
>^
>
> subversion/svn/svn.c:3052:23: warning: assignment discards 'const'
> qualifier from pointer target type [enable\
> d by default]
>
> argv[i-1] = "#\0";
>
> At breakpoint B, gdb gives me:
>
> (gdb) p argv
>
> $1 = (char **) 0x7fffe468
>
> (gdb) p argv[1]
>
> $2 = 0x7fffe81e "svn"
>
> (gdb) p argv[2]
>
> $3 = 0x7fffe822 "help"
>
> (gdb) p argv[3]
>
> $4 = 0x4438b3 "#"
>
> (gdb)
>
> So it def. does change it.
>
>
> On Fri, Jul 4, 2014 at 12:40 AM, Ben Reser wrote:
>
>> On 7/3/14 4:23 PM, Gabriela Gibson wrote:
>> > Could that be because of the libtool svn-lt script that sits in the
>> middle?
>> > Because in gdb it does change, but not in ps.
>>
>> No because I wasn't even doing anything with Subversion yet let alone
>> libtool.
>> It was just a very basic C program with nothing more than a main. It's
>> probably because the argv array belongs to the process itself and then it
>> points to memory that the kernel owns (or at least watches). I'm not
>> sure on
>> all the details of how the kernel starts the binary and passes in the
>> arguments.
>>
>> I've attached the example I was working with that didn't make it to this
>> list
>> already.
>>
>>
>
>
> --
> Visit my Coding Diary: http://gabriela-gibson.blogspot.com/
>
--
Visit my Coding Diary: http://gabriela-gibson.blogspot.com/
Re: The --password and clumsy users issue
Oh, I was playing about with this earlier, but didn't get all that far:
I go this far in svn.c (has breakpoint marker in it)
[[[
int
main(int argc, char **argv)
{
apr_pool_t *pool;
int exit_code = EXIT_SUCCESS;
svn_error_t *err;
const char **argv_org;
/* Initialize the app. */
if (svn_cmdline_init("svn", stderr) != EXIT_SUCCESS)
return EXIT_FAILURE;
/* Create our top-level pool. Use a separate mutexless allocator,
* given this application is single threaded.
*/
pool = apr_allocator_owner_get(svn_pool_create_allocator(FALSE));
{
int i;
argv_org = apr_palloc(pool, sizeof(argv));
B for(i = 0; i < argc; i++)
{
argv_org[i] = apr_pstrdup(pool, argv[i]);
if (i > 2 && !strcmp(argv[i-1],"--password"))
{
argv[i-1] = "#\0";
}
if (i > 2 && !strcmp(argv[i-1],"--username"))
{
argv[i-1] = "#\0";
}
}
}
]]]
it compiles and runs and in gdb the vars do change, but the compiler isn't
happy:
subversion/svn/svn.c: In function 'main':
subversion/svn/svn.c:3048:23: warning: assignment discards 'const'
qualifier from pointer target type [enable\
d by default]
argv[i-1] = "#\0";
^
subversion/svn/svn.c:3052:23: warning: assignment discards 'const'
qualifier from pointer target type [enable\
d by default]
argv[i-1] = "#\0";
At breakpoint B, gdb gives me:
(gdb) p argv
$1 = (char **) 0x7fffe468
(gdb) p argv[1]
$2 = 0x7fffe81e "svn"
(gdb) p argv[2]
$3 = 0x7fffe822 "help"
(gdb) p argv[3]
$4 = 0x4438b3 "#"
(gdb)
So it def. does change it.
On Fri, Jul 4, 2014 at 12:40 AM, Ben Reser wrote:
> On 7/3/14 4:23 PM, Gabriela Gibson wrote:
> > Could that be because of the libtool svn-lt script that sits in the
> middle?
> > Because in gdb it does change, but not in ps.
>
> No because I wasn't even doing anything with Subversion yet let alone
> libtool.
> It was just a very basic C program with nothing more than a main. It's
> probably because the argv array belongs to the process itself and then it
> points to memory that the kernel owns (or at least watches). I'm not sure
> on
> all the details of how the kernel starts the binary and passes in the
> arguments.
>
> I've attached the example I was working with that didn't make it to this
> list
> already.
>
>
--
Visit my Coding Diary: http://gabriela-gibson.blogspot.com/
Re: The --password and clumsy users issue
Ben wrote: > Rewriting the array isn't seen by the kernel. At least when I initially wrote > that example I tried just setting argv[++i] = "" and the ps output > didn't change. Could that be because of the libtool svn-lt script that sits in the middle? Because in gdb it does change, but not in ps. G On Fri, Jul 4, 2014 at 12:17 AM, Gabriela Gibson wrote: > This discussion kicked off on the wrong list, so since I started > it, I've concatenated the posts. > > Intro: > -- > > I used the --password option in a commit and then found that the > process with my password in full view hung around for an entire week > and could be seen by anyone with access to the same machine by using > ps ef. (no passwords were divulged in the process, but some blushes > occurred) > > So, the questions I had were: > > * can and should we make svn's --password option a little more secure > against clumsy users than it currently is? > > * Is there anything we could do to let svn admins compel users to not > use --password? > > * What do we gain by not removing the option of having > passwords in the clear? > > Ben suggested that we maybe should add a sentence warning users that > the --password switch is not secure, since I felt that the current > info about it makes it look too 'legit'. ie.: > > "(this usually is not secure)" > > - > Julian Foad wrote: > > Gabriela does make some good points. I think it *is* a good idea to > annotate the --password option as Ben suggested, as the first and > simplest response, but we should *also* consider doing more. Merely > providing an option prominently is implicitly promoting it. We should > reconsider how we expose and document it, and even whether we should > still be providing this option at all -- do we know of better best > practices now? > > -- > > This is a summary of Ben's reply: > > Ben Reser wrote on Thu, Jul 03, 2014 at 12:54:58 -0700: > > There are only two options to resolve this in my opinion (ignoring the > simple > > documentation solution): > > > 1) Remove the option. > > > I don't think this is particularly desirable. We use it in our test > suite in > > order to test without having to drive the password prompting.I've used > it in > > examples when showing people how to setup authentication when I wanted > which > > user I was using to always be explicit but didn't want to type a > password all > > the time. Other people are surely using it for automation in places > where they > > aren't concerned about the password leaking via the command line and > it's more > > convenient than seeding the auth cache or driving the password prompts > with > > something like expect. Removing the option certainly creates problems > for us > > as a project with testing and almost certainly causes problems for our > users. > > > 2) Redact the password in the argv after starting up and finding the > bits to > > redact. > > > I have mixed feelings on this one. I'm not entirely sure that all > kernels > > respect this, I know it works on Linux and OS X. But even if we presume > that > > it works on every system where this is an issue, then all it really does > is > > narrow the window in which the password is exposed.If a user is > routinely > > using svn with --password and someone sees the redacted password all > they have > > to do is search for newly made svn processes and race our command line > parser > > to retrieve the password before it gets redacted. So in the end we add > some > > more complexity to our command line tools and users are slightly safer > but > > they're still not really safe. On one hand this helps users in > situations > > where they are not actively being attacked but the password is just > > accidentally observed by someone. But it does nothing to help users who > are > > being actively attacked trying to recover their password.In fact I'd say > it > > creates a false perception of security because the risk would be > obscured from > > the user. > > --- > > Danielsh wrote: > > Ben Reser wrote on Thu, Jul 03, 2014 at 12:54:58 -0700: > > 2) Redact the password in the argv after starting up and finding the > bits to > > redact. > > > > I have mixed feelings on this one. I'm not entirely sure that all > kernels > > respect this, I know it works on Linux and OS X. B
The --password and clumsy users issue
This discussion kicked off on the wrong list, so since I started it, I've concatenated the posts. Intro: -- I used the --password option in a commit and then found that the process with my password in full view hung around for an entire week and could be seen by anyone with access to the same machine by using ps ef. (no passwords were divulged in the process, but some blushes occurred) So, the questions I had were: * can and should we make svn's --password option a little more secure against clumsy users than it currently is? * Is there anything we could do to let svn admins compel users to not use --password? * What do we gain by not removing the option of having passwords in the clear? Ben suggested that we maybe should add a sentence warning users that the --password switch is not secure, since I felt that the current info about it makes it look too 'legit'. ie.: "(this usually is not secure)" - Julian Foad wrote: Gabriela does make some good points. I think it *is* a good idea to annotate the --password option as Ben suggested, as the first and simplest response, but we should *also* consider doing more. Merely providing an option prominently is implicitly promoting it. We should reconsider how we expose and document it, and even whether we should still be providing this option at all -- do we know of better best practices now? -- This is a summary of Ben's reply: Ben Reser wrote on Thu, Jul 03, 2014 at 12:54:58 -0700: > There are only two options to resolve this in my opinion (ignoring the simple > documentation solution): > 1) Remove the option. > I don't think this is particularly desirable. We use it in our test suite in > order to test without having to drive the password prompting.I've used it in > examples when showing people how to setup authentication when I wanted which > user I was using to always be explicit but didn't want to type a password all > the time. Other people are surely using it for automation in places where they > aren't concerned about the password leaking via the command line and it's more > convenient than seeding the auth cache or driving the password prompts with > something like expect. Removing the option certainly creates problems for us > as a project with testing and almost certainly causes problems for our users. > 2) Redact the password in the argv after starting up and finding the bits to > redact. > I have mixed feelings on this one. I'm not entirely sure that all kernels > respect this, I know it works on Linux and OS X. But even if we presume that > it works on every system where this is an issue, then all it really does is > narrow the window in which the password is exposed.If a user is routinely > using svn with --password and someone sees the redacted password all they have > to do is search for newly made svn processes and race our command line parser > to retrieve the password before it gets redacted. So in the end we add some > more complexity to our command line tools and users are slightly safer but > they're still not really safe. On one hand this helps users in situations > where they are not actively being attacked but the password is just > accidentally observed by someone. But it does nothing to help users who are > being actively attacked trying to recover their password.In fact I'd say it > creates a false perception of security because the risk would be obscured from > the user. --- Danielsh wrote: Ben Reser wrote on Thu, Jul 03, 2014 at 12:54:58 -0700: > 2) Redact the password in the argv after starting up and finding the bits to > redact. > > I have mixed feelings on this one. I'm not entirely sure that all kernels > respect this, I know it works on Linux and OS X. But even if we presume that > it works on every system where this is an issue, then all it really does is It doesn't need to work on "every" system.It just needs to work on _some_ systems; it'd be fine for it to be a noop on other systems. > narrow the window in which the password is exposed. If a user is routinely > using svn with --password and someone sees the redacted password all they have > to do is search for newly made svn processes and race our command line parser > to retrieve the password before it gets redacted. So in the end we add some > more complexity to our command line tools and users are slightly safer but > they're still not really safe. On one hand this helps users in situations > where they are not actively being attacked but the password is just > accidentally observed by someone. But it does nothing to help users who are > being actively attacked trying to recover their password. In fact I'd say it > creates a false perception of security because the risk would be obscured from > the user. Note there is a middle way here: we could partially redact the password, e.g., replace `pa
New help section 'Global Options'?
Currently, the global options are displayed on many svn help topics, and that's an extra 16 lines at the end, which makes the actual help info that is sought scroll off the screen, and it's not information that always needed. What do you think about moving the global options to their own help section and leaving just one line with a pointer to them? Proposed: Global options: Type svn help global for details Current: Global options: --username ARG : specify a username ARG --password ARG : specify a password ARG --no-auth-cache : do not cache authentication tokens --non-interactive: do no interactive prompting (default is to prompt only if standard input is a terminal device) --force-interactive : do interactive prompting even if standard input is not a terminal device --trust-server-cert : accept SSL server certificates from unknown certificate authorities without prompting (but only with '--non-interactive') --config-dir ARG : read user configuration files from directory ARG --config-option ARG : set user configuration option in the format: FILE:SECTION:OPTION=[VALUE] For example: servers:global:http-library=serf Gabriela -- Visit my Coding Diary: http://gabriela-gibson.blogspot.com/
[PATCH] New --diff-cmd (just for discussion)
Hi, After looking at Stefan and Ivan's very helpful critique, I had one of those 'may be good or may be terrible' ideas :) --invoke-diff-cmd is now absorbed into --diff-cmd (whilst preserving the old functionality and look), and most issues that Ivan and Stefan flagged up have been resolved thus. I like this better because there is only one --diff-cmd now, and it's a much simpler structure (at least this is the theory). I've adjusted the original diff_external_diffcmd_test and added 4 more, and all bar one XFAIL pass. Because I'm not a great parser coder, and I'm not quite sure that the current behavior is what we actually want, I've left the library function style in svn_io__create_custom_diff_cmd() in place for now, until we have a precise idea of what is actually required, since I know exactly what this does, which isn't a givens for C strings ;-) As suggested by danielsh, I attempted to add support for program names with a space in them and allowed the user to type 'my\ diff foo bar baz' but unfortunately this fails at shell level, and I don't know enough about this subject to find a good solution here. The failed test diff_tests.py 52 shows up like this: (I renamed the diff_script to 'diff script' at generation time, test 52 is a copy of test 51, bar the diff script name change.) [[[ g@campion:~/test_trunk/subversion/tests/cmdline$ ./diff_tests.py 52 W: exec of './diff\ script' failed: No such file or directorysubversion/svn/diff-cmd.c:427, W: subversion/libsvn_client/diff.c:2403, W: subversion/libsvn_client/diff.c:2211, W: subversion/libsvn_client/diff.c:1676, W: subversion/libsvn_wc/diff_local.c:500, W: subversion/libsvn_wc/status.c:2717, W: subversion/libsvn_wc/status.c:1460, W: subversion/libsvn_wc/status.c:1115, W: subversion/libsvn_wc/status.c:834, W: subversion/libsvn_wc/diff_local.c:369, W: subversion/libsvn_wc/diff_editor.c:555, W: subversion/libsvn_client/diff.c:950, W: subversion/libsvn_client/diff.c:828, W: subversion/libsvn_subr/io.c:3235: (apr_err=SVN_ERR_EXTERNAL_PROGRAM) W: svn: E200012: './diff\ script -L "X Y Z" -L "A B C D" %svn_old +%svn_new+' was expanded to './diff\ script "X Y Z" -L "A B C D" /home/g/test_trunk/subversion/tests/cmdline/svn-test-work/working_copies/diff_tests-52/.svn/pristine/2c/2c0aa9014a0cd07f01795a333d82485ef6d083e2.svn-base +/home/g/test_trunk/subversion/tests/cmdline/svn-test-work/working_copies/diff_tests-52/iota+ ' and returned 255 W: CWD: /home/g/test_trunk/subversion/tests/cmdline/svn-test-work/working_copies/diff_tests-52 W: EXCEPTION: Failure: Command failed: "/home/g/test_trunk/subversion/svn/svn diff --diff-cmd=./diff\ script -L "X Y Z" -L "A B C D" %svn_old +%svn_new+ iota ..."; exit code 1 Traceback (most recent call last): File "/home/g/test_trunk/subversion/tests/cmdline/svntest/main.py", line 1621, in run rc = self.pred.run(sandbox) File "/home/g/test_trunk/subversion/tests/cmdline/svntest/testcase.py", line 114, in run return self._delegate.run(sandbox) File "/home/g/test_trunk/subversion/tests/cmdline/svntest/testcase.py", line 176, in run return self.func(sandbox) File "./diff_tests.py", line 3292, in diff_external_diffcmd_4 iota_path) File "/home/g/test_trunk/subversion/tests/cmdline/svntest/actions.py", line 284, in run_and_verify_svn expected_exit, *varargs) File "/home/g/test_trunk/subversion/tests/cmdline/svntest/actions.py", line 322, in run_and_verify_svn2 exit_code, out, err = main.run_svn(want_err, *varargs) File "/home/g/test_trunk/subversion/tests/cmdline/svntest/main.py", line 702, in run_svn *(_with_auth(_with_config_dir(varargs File "/home/g/test_trunk/subversion/tests/cmdline/svntest/main.py", line 368, in run_command None, *varargs) File "/home/g/test_trunk/subversion/tests/cmdline/svntest/main.py", line 560, in run_command_stdin '"; exit code ' + str(exit_code)) Failure: Command failed: "/home/g/test_trunk/subversion/svn/svn diff --diff-cmd=./diff\ script -L "X Y Z" -L "A B C D" %svn_old +%svn_new+ iota ..."; exit code 1 XFAIL: diff_tests.py 52: svn diff --diff-cmd w. spaces in diff program name ]]] thanks for looking, Gabriela [[[ Patch for discussion: Add new syntax facility to --diff-cmd and it's config counterpart, whilst preserving the old behaviour. The new syntax allows the use of 4 keywords: %svn_new_label, %svn_old_label, %svn_old %svn_new So constructs like: $svn diff --diff-cmd= "diff --ignore-file-name-case -L "One Two" -L %svn_new_label %svn_old %svn_new" are possible. --extensions is not used if the code does not detect an old style diff-command command. Characters immediately adjacent to a keyword are also supported (as requested by Julian Foad). The tests for this patch may be run with ./diff_tests.py 48 through 52 * subversion/include/private/svn_io_private.h (svn_io__create_custom_diff_cmd): Declare new function and document it. * subversion/libsvn_subr/io.c (svn_io__create
[PATCH] --invoke-diff-cmd
Hi, I'm not sure how to update my branch because the underlying base has changed(and doesn't merge all that well) and thus, I restarted with just a patch and a copy of the current trunk. So, just to keep things simple, I just post it as a patch for now. Gabriela Ps.: I'll take a look at the --invoke-diff3-cmd part this week sometime. == Introduction to --invoke-diff-cmd == --invoke-diff-cmd allows command line selection of an external diff program and will be extended to cover the existing diff3 option with a similar --invoke-diff3-cmd option. Currently this capability is provided by user written shell scripts which are passed as the diff program via the svn config file. --invoke-diff-cmd is currently implemented for 'diff', 'log', 'svnlook' and the config file. See: http://subversion.tigris.org/issues/show_bug.cgi?id=2044 for the original motivation for this project. What --invoke-diff-cmd provides === Users can type 'free-style' command lines for their selected diff/merge program, and optionally select a diff command file that applies stored commands to selected files. If the file diffed is not in the list, the given command will be used instead. from 'svn help diff': --invoke-diff-cmd ARG: use ARG as format string for external diff command invocation. Substitutions: %svn_new new file %svn_old old file %svn_label_new label of the new file %svn_label_old label of the old file Examples: --invoke-diff-cmd='diff -y %svn_new %svn_old' --invoke-diff-cmd="kdiff3 -auto -o /home/u/log \ %svn_new %svn_old --L1 %svn_new_label \ --L2 "Custom Label" ' Substitution variables may be embedded in strings: +%svn_new, %svn_new- and file=%svn_label_new+ Structure of the feature: = API components -- ./subversion/libsvn_subr/io.c __create_custom_diff_cmd() transforms the user input 'invoke-diff-cmd' into a command line call by substitution the labels and file names(where defined), whilst leaving everything else untouched. ./subversion/libsvn_subr/io.c svn_io_run_external_diff() calls __create_custom_diff_cmd() and does all the error checking required, before routing the result to the actual call to the APR routine that makes it. UI components - --invoke-diff-cmd and its user interface components for the command line have been installed everywhere where --diff-cmd is available, in svnlook.c, svn.c, svnlog.c. Tests = The test for the 'invoke-diff-cmd' feature is /subversion/tests/cmdline/diff_tests.py diff_invoke_external_diffcmd [[[ * subversion/include/private/svn_io_private.h (svn_io__create_custom_diff_cmd): New function declaration. * subversion/include/svn_config.h (SVN_CONFIG_OPTION_INVOKE_DIFF_CMD): New definition. * subversion/include/svn_error_codes.h (SVN_CLIENT_DIFF_CMD): New macro. * subversion/include/svn_io.h (svn_io_run_external_diff): New function. * subversion/libsvn_client/diff.c (diff_writer_info_t): New member: 'invoke_diff_cmd'. (create_diff_writer_info): Add routine to read invoke_diff_cmd either from the commandline (preferential) or from the config file, after it is established that the diff-cmd is not defined on the commandline or in the .subversion/config file. Readjust the flow of the function to ensure that the internal diff routine is called if neither diff_cmd or invoke_diff_cmd are called. (diff_content_changed): Raise an error if both diff_cmd and invoke-diff-cmd are set. Add invoke-diff-cmd to if condition. Add comment explaining the presence of non-canonical path in function call. Call svn_io_run_external_diff if --invoke-diff-cmd option was specified, otherwise retain previous behaviour. * subversion/libsvn_subr/config_file.c (svn_config_ensure,"invoke-diff-cmd"): New entry: invoke-diff-cmd. Add help info. * subversion/libsvn_subr/io.c (svn_io__create_custom_diff_cmd): New function. (svn_io_run_external_diff): New function. * subversion/svn/cl.h (struct svn_cl__opt_state_t.diff): New member: 'invoke_diff_cmd'. * subversion/svn/log-cmd.c (log_receiver_baton): New struct member invoke_diff_cmd. (svn_cl__log): Ensure mutual exclusions between invoke_diff_cmd and diff-cmd. Require 'diff' option to be set when requesting invoke_diff option. Populate log_receiver_baton member invoke_diff_cmd. * subversion/svnlook/svnlook.c (enum): New variable svnlook__invoke_diff_cmd. (options_table[]): New entry 'invoke-diff-cmd'. (cmd_tablcmd[]): Add svnlook__invoke_diff_cmd to diff cmd table entry. (svnlook_opt_state): New member v
[PATCH] change of variable name: 'diff_cmd_baton' to 'dwi' in subversion/libsvn_client/diff.c
Whilst rooting around, I saw that the local variable 'diff_cmd_baton'
was changed to the type diff_writer_into_t and in most other placed had
the name 'dwi'.
I changed all the local instances in subversion/libsvn_client/diff.c to
match the new naming scheme.
Gabriela
[[[
Change all occurrences of 'diff_cmd_baton' to 'dwi'.
* subversion/libsvn_client/diff.c
(just_paths_for_diff_labels),
(diff_props_changed),
(svn_client_diff6),
(svn_client_diff_peg6): changed local variable diff_cmd_baton to
dwi throughout.
]]]
Index: subversion/libsvn_client/diff.c
===
--- subversion/libsvn_client/diff.c (revision 1602013)
+++ subversion/libsvn_client/diff.c (working copy)
@@ -164,7 +164,7 @@
svn_boolean_t is_url1;
svn_boolean_t is_url2;
/* ### Holy cow. Due to anchor/target weirdness, we can't
- simply join diff_cmd_baton->orig_path_1 with path, ditto for
+ simply join dwi->orig_path_1 with path, ditto for
orig_path_2. That will work when they're directory URLs, but
not for file URLs. Nor can we just use anchor1 and anchor2
from do_diff(), at least not without some more logic here.
@@ -606,13 +606,13 @@
const apr_array_header_t *propchanges,
apr_hash_t *original_props,
svn_boolean_t show_diff_header,
- diff_writer_info_t *diff_cmd_baton,
+ diff_writer_info_t *dwi,
apr_pool_t *scratch_pool)
{
apr_array_header_t *props;
/* If property differences are ignored, there's nothing to do. */
- if (diff_cmd_baton->ignore_properties)
+ if (dwi->ignore_properties)
return SVN_NO_ERROR;
SVN_ERR(svn_categorize_props(propchanges, NULL, NULL, &props,
@@ -620,23 +620,23 @@
if (props->nelts > 0)
{
- /* We're using the revnums from the diff_cmd_baton since there's
+ /* We're using the revnums from the dwi since there's
* no revision argument to the svn_wc_diff_callback_t
* dir_props_changed(). */
SVN_ERR(display_prop_diffs(props, original_props,
diff_relpath,
- diff_cmd_baton->ddi.anchor,
- diff_cmd_baton->ddi.orig_path_1,
- diff_cmd_baton->ddi.orig_path_2,
+ dwi->ddi.anchor,
+ dwi->ddi.orig_path_1,
+ dwi->ddi.orig_path_2,
rev1,
rev2,
- diff_cmd_baton->header_encoding,
- diff_cmd_baton->outstream,
- diff_cmd_baton->relative_to_dir,
+ dwi->header_encoding,
+ dwi->outstream,
+ dwi->relative_to_dir,
show_diff_header,
- diff_cmd_baton->use_git_diff_format,
- diff_cmd_baton->ddi.session_relpath,
- diff_cmd_baton->wc_ctx,
+ dwi->use_git_diff_format,
+ dwi->ddi.session_relpath,
+ dwi->wc_ctx,
scratch_pool));
}
@@ -2339,7 +2339,7 @@
svn_client_ctx_t *ctx,
apr_pool_t *pool)
{
- diff_writer_info_t diff_cmd_baton = { 0 };
+ diff_writer_info_t dwi = { 0 };
svn_opt_revision_t peg_revision;
const svn_diff_tree_processor_t *diff_processor;
svn_diff_tree_processor_t *processor;
@@ -2353,33 +2353,33 @@
peg_revision.kind = svn_opt_revision_unspecified;
/* setup callback and baton */
- diff_cmd_baton.ddi.orig_path_1 = path_or_url1;
- diff_cmd_baton.ddi.orig_path_2 = path_or_url2;
+ dwi.ddi.orig_path_1 = path_or_url1;
+ dwi.ddi.orig_path_2 = path_or_url2;
- SVN_ERR(create_diff_writer_info(&diff_cmd_baton, options,
+ SVN_ERR(create_diff_writer_info(&dwi, options,
ctx->config, pool));
- diff_cmd_baton.pool = pool;
- diff_cmd_baton.outstream = outstream;
- diff_cmd_baton.errstream = errstream;
- diff_cmd_baton.header_encoding = header_encoding;
+ dwi.pool = pool;
+ dwi.outstream = outstream;
+ dwi.errstream = errstream;
+ dwi.header_encoding = header_encoding;
- diff_cmd_baton.force_binary = ignore_content_type;
- diff_cmd_baton.ignore_properties = ignore_properties;
- diff_cmd_baton.properties_only = properties_only;
- diff_cmd_baton.relative_to_dir = relative_to_dir;
- diff_cmd_baton.use_git_diff_format = use_git_diff_format;
- diff_cmd_baton.no_diff_added = no_diff_added;
- diff_cmd_baton.no_diff_deleted = no_diff_deleted;
- diff_cmd_baton.show_copies_as_adds = show_copies_as_adds;
+ dwi.f
Re: [PATCH] Redirect from old HACKING GUIDE that shows up as Nr.2 in Google search, to current Community Guide
On Tue, Jun 10, 2014 at 11:23 AM, Neels Hofmeyr wrote: > ... but please don't be put off by Ben's formal tone :) > > No, not in the least :-) Fact is, I _did_ forget the 'on the xxx branch' bit(and I'm somewhat rusty rgds log messages, and knowing what branch is meant is useful), and I was surprised that Google localises that kind of search. Shopping stuff being local I can understand, but localising tech info seems strange to me. I agree that it is a good thing to help people find the location of the > hacking guide, and I believe it was a bad idea to rename this document, > the well known one-stop place for new committers. > > What I see in the UK as the first entry is the svn collabnet pages that redirects to the current document. Maybe we should delete just the beef of the branch and keep the redirect, and so hold on to that 2nd place in the referrals which comes up for Bert? People will still be looking for the 'Hacking Guide' because it had this name for years, so it would be good to make us easy to find. Regards renaming, maybe we could add the word 'Guide' to the current page after 'Hacking', in the box after the main title, so instead of 'Subversion Community Guide (aka "Hacking")' we'd have 'Subversion Community Guide (aka "Hacking Guide")'. Another thing we could do is add the term 'Hacking Guide' as a meta tag. Gabriela -- Visit my Coding Diary: http://gabriela-gibson.blogspot.com/
[PATCH] Redirect from old HACKING GUIDE that shows up as Nr.2 in Google search, to current Community Guide
The current Community Guide doesn't turn up in Google searches for 'svn hacking guide' because the document is now called 'Apache Subversion Community Guide (aka "HACKING")', whereas before it was called 'Hacking Guide'. https://www.google.co.uk/search?q=svn+hacking+guide&oq=svn+hacking+guide&aqs=chrome..69i57.5847j0j7&sourceid=chrome&es_sm=93&ie=UTF-8&qscrl=1 The first link is correctly redirected but the second link points to an outdated version. [[[ Add redirection to point to the current version of the Apache Subversion Community Guide to old HACKING GUIDE branch because it turns up in position 2 when searching Google for 'svn Hacking Guide'. * subversion/branches/take2/www/hacking.html (meta): Redirect to current Community Guide. ]]] Gabriela Index: www/hacking.html === --- www/hacking.html (revision 1598969) +++ www/hacking.html (working copy) @@ -2,6 +2,21 @@ "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd";> http://www.w3.org/1999/xhtml";> + + +http://subversion.apache.org/docs/community-guide/";> + + + window.location.href = "http://subversion.apache.org/docs/community-guide/"; + + +Page Redirection + +If you are not redirected automatically, follow the + +link to the Apache Subversion Community Guide (aka 'HACKING') +
SVN password encryption
Hi, Could we make the svn password encrypted by default by setting the ./subversion/servers entry 'store-plaintext-passwords' to 'no'? Currently, setting up password encryption requires digging through the docs, and it's tempting, especially for casual users, to avoid that effort by storing the password in clear text. Whilst people shouldn't do that, there is just so much software and so little time, and all too often, 'I'll do that later' never happens. Even if the user sets up password encryption, the previously created clear text password will sit around until they realise this problem and find and delete that file. I think that making passwords encrypted by default and requiring work to be 'unsafe' is a good solution here. Or maybe, the ability to store clear text passwords ought to be removed all together. Also, it might be an idea that once the password for a particular user is changed from clear text to encrypted, that the corresponding clear text file is automatically removed; and that people who upgrade their svn and still use a clear text passwords are prompted with the offer of an automatic fix that encrypts their current clear text passwords, and then removes the old clear text files, but gives them a chance of making a note in case they long forgotten their passwords. What do you think? regards, Gabriela -- Visit my Coding Diary: http://gabriela-gibson.blogspot.com/
Re: prop edit: lost user edit bug
On Tue, Dec 24, 2013 at 4:02 PM, Bert Huijben wrote: > We have a baton in the callback, so no reason for bottle magic. > > Hi Bert, Thanks for the reply and the hint, I've looked again and I just cannot see how to work this. (It's probably too obvious :-) Is there an example I could study? Gabriela -- Visit my Coding Diary: http://gabriela-gibson.blogspot.com/
Re: prop edit: lost user edit bug
On 23/12/13 19:13, Gabriela Gibson wrote: Hi, (snip) Also, if the commit fails, the user would still not be informed where to find their work so they can resubmit after fixing their network issues. We could write to console to inform the user and just live with one extra file in /tmp. I'm wondering if we could set up a 'message in a bottle' mechanism in the error code. In the current problem, we need to send the user a message if the process fails for some reason later down the line, but if everything works ok, they don't need that information. Gabriela
prop edit: lost user edit bug
Hi,
I found the other day that if my network fails for some reason
whilst editing a prop, the entire edit is lost.
I took a look at the propedit code and found the following:
in subversion/svn/propedit-cmd.c in line 145 and 278 we call:
SVN_ERR(svn_cmdline__edit_string_externally(
&propval, NULL, .
which lets the user write the prop but removes the file.
This function is defined in
subversion/include/private/svn_cmdline_private.h:
and the code is located in subversion/svn/propedit-cmd.c:
svn_error_t *
svn_cmdline__edit_string_externally(svn_string_t **edited_contents,
const char **tmpfile_left,
...
In the function body, the variable tmpfile_left is actually never used
as a data conduit, but as a boolean flag in order to set remove_file
and then reassigned internally if it's detected inside this function ie:
if (tmpfile_left)
{
*tmpfile_left = svn_dirent_join(base_dir, tmpfile_name, pool);
remove_file = FALSE;
}
However, this assigned variable isn't used anywhere either, only
the remove_file flag is utilised.
Callers of this function are located in
./svnmucc/svnmucc.c:767,
./svn/propedit-cmd.c:143:
./svn/propedit-cmd.c:278:
where in every case, tmpfile_left is seeded as NULL,
and
./svn/util.c:431: where it's called with an value that's carried via
struct log_msg_baton *lmb = log_msg_baton;
like so:
err = svn_cmdline__edit_string_externally(&msg_string,
lmb->tmpfile_left, ...
I could change the calls in subversion/svn/propedit-cmd.c to give
this a value and prevent the removal of the tmp file, but that would
leave the file sitting around even if the commit of the prop is
successful.
Also, if the commit fails, the user would still not be informed where
to find their work so they can resubmit after fixing their network
issues. We could write to console to inform the user and just live
with one extra file in /tmp.
I think the tmpfile_left could be changed to svn_boolean_t, and if
lmb->tmpfile_left needs updating it probably is clearer if that happens
in the calling code rather than in a service function, since it's just
built from the parameters that were passed in.
I'm not sure what to do next, would you have some advice for me
please?
Gabriela
[PROPOSAL] Apache Labs: PhraseBook (Subversion inspired project)
Hi, I proposed the following project called PhraseBook to Apache Labs: http://mail-archives.apache.org/mod_mbox/labs-labs/201312.mbox/%3CCALfVaASwHH_AePjkAm%3D_UyoTyTZ2rYH_mhg-PNCErGuZbv0uSA%40mail.gmail.com%3E together this this follow-up (that contains more information): http://mail-archives.apache.org/mod_mbox/labs-labs/201312.mbox/%3CCALfVaAS%3Dw%3D-DY2n4DQemx7NKF9jurwg8gwDcoPv2YtCE%2Bm5wbQ%40mail.gmail.com%3E Originally I had the idea to build a collection of usage tips for Subversion, but as it turned out, the idea that transpired in the process is probably general enough to merit it's own project. Please take a look, I hope you will be entertained! thanks, Gabriela Ps.: I have one +1 vote and 48 hours left so far, need 2 more :) -- Visit my Coding Diary: http://gabriela-gibson.blogspot.com/
SVN Book diff idea for the HTML version
It would be nice to have SVN book versions on the website that have a differing portions of the text in blue for changes, and green for new additions and red for now obsolete parts. It would make re-reading the book for a new version much easier for old hands, you could immediately spot everything you need to reconsider. So, if I upgrade from 1.6 to 1.7, I would like to have the normal 1.7 book, the 1.7/1.6 'blue/green' version and the 1.6/1.7 'red' version. (blue & green might not be possible to automate, but even just the blue version would be very useful, or perhaps it's even possible to have this all in one version.) regards, Gabriela -- Visit my Coding Diary: http://gabriela-gibson.blogspot.com/
The FLOSS 2013 survey
If you have contributed to a FLOSS project (such as Subversion), please take a moment to fill out this survey: http://floss2013.libresoft.es/ thanks, Gabriela
Re: svn commit: r1542741 [1/3] - in /subversion/branches/invoke-diff-cmd-feature: BRANCH-README subversion/include/svn_io.h subversion/libsvn_client/diff.c subversion/svn/svn.c subversion/svnlook/svnl
Thanks Brane, no :) plain fact is that I didn't think that changing code I messed up within the branch and then fixed as significant. I sort of see the branch as a semi-private space, where I do stuff that you can look at, but that doesn't have much meaning (hence me terming N changes in M files as 'trivial' -- it's jst a bit of of umm ascii) until it's good and ready for me to invite scrutiny. Privacy for patzers :) Maybe what is needed is a personal commit space where I can reign supreme. (fact is, my computer is ancient (but nice and familiar to use) and I don't trust it.) My other problem is that Apache code carries a copyright, so I'm reluctant to use newfangled things like clouds and their fancy (lng) usage legalese that could get me (and Apache) into trouble. (some ppl just like to see the world burn and the lawyers earn). I also hope not to give ppl too much useless work, hence me leaving off certain log messages off b/c they really are a waste of human time, it's just where I goofed and not worth anyone's time. That's the main concept here, I'm trying to save your time, probably in the wrong way :) Gabriela On Sun, Nov 17, 2013 at 8:19 PM, Branko Čibej wrote: > On 17.11.2013 21:08, Gabriela Gibson wrote: > > Thank you Ivan, > > I was blithely assuming that because I was removing trivial changes I > introduced to the branch, that no-one would track them :) Also I was > trying to avoid writing trivial things for people to read. > > The log message is fixed now. > > Now I'm curious -- what tools do people use to revise patches that allow > them to instantly spot (seemingly) minor additions? > > > Do you mean "revise patches" or "review commits"? If it's the latter, the > fact that the list of affected files in the log message and the > automatically generated list of changes in the commit mail are different is > an obvious hint that something was either left out of the log message, or > inadvertently committed. > > -- Brane > > > -- > Branko Čibej | Director of Subversion > WANdisco // Non-Stop Data > e. br...@wandisco.com >
Re: svn commit: r1542741 [1/3] - in /subversion/branches/invoke-diff-cmd-feature: BRANCH-README subversion/include/svn_io.h subversion/libsvn_client/diff.c subversion/svn/svn.c subversion/svnlook/svnl
Thank you Ivan, I was blithely assuming that because I was removing trivial changes I introduced to the branch, that no-one would track them :) Also I was trying to avoid writing trivial things for people to read. The log message is fixed now. Now I'm curious -- what tools do people use to revise patches that allow them to instantly spot (seemingly) minor additions? Many thanks, Gabriela On Sun, Nov 17, 2013 at 3:02 PM, Ivan Zhakov wrote: > On 17 November 2013 18:56, wrote: > > Author: gbg > > Date: Sun Nov 17 14:56:28 2013 > > New Revision: 1542741 > > > > URL: http://svn.apache.org/r1542741 > > Log: > > On the invoke-diff-cmd-feature branch: Update BRANCH-README file. > > Trivial white space changes to assorted files. > > > > * BRANCH-README: Update general shape and rework all log messages. > > Add new diff file. > > > > Modified: > > subversion/branches/invoke-diff-cmd-feature/BRANCH-README > > > subversion/branches/invoke-diff-cmd-feature/subversion/include/svn_io.h > > > subversion/branches/invoke-diff-cmd-feature/subversion/libsvn_client/diff.c > > subversion/branches/invoke-diff-cmd-feature/subversion/svn/svn.c > > > subversion/branches/invoke-diff-cmd-feature/subversion/svnlook/svnlook.c > > > Dear Gabriel, > > Your log message doesn't match real changes again. Many people > reviewing all commits and such commits are distracting. > > > -- > Ivan Zhakov >
--invoke-diff3-cmd ready for review
Hi, The --invoke-diff3-cmd branch can be found here: http://svn.apache.org/viewvc/subversion/branches/invoke-diff3-feature/BRANCH-README?view=markup&pathrev=1542514 thanks for looking! Gabriela
Re: invoke-diff-cmd branch is ready for trunk inclusion
On 10/11/13 09:56, Ivan Zhakov wrote: On 4 November 2013 22:41, Gabriela Gibson wrote: Hi, the latest invoke-diff-cmd branch: http://svn.apache.org/viewvc?view=revision&revision=1538071 is ready to be merged into the trunk, iff everyone agrees that the current substitution syntax is just right and the documentation and the code itself is of acceptable standard. Hi Gabriella, Could you please write full log message for current changes sitting in branch. I.e. present it in the same way as regular patches are submitted. Reviewing such long branch by following all commits on branch is not convenient. Given that some changes on totally incorrect and fixed latter. Some of them missing log messages and etc. Hi Ivan, I've updated the BRANCH-README file and hope this is more useful now. If not, holler :) http://svn.apache.org/viewvc/subversion/branches/invoke-diff-cmd-feature/BRANCH-README?view=markup&pathrev=1540536 thanks, Gabriela
Re: [PATCH] svnlook.py: Make it usable as a library
*sigh* googlemail ninja's me once again... On Sat, Nov 9, 2013 at 9:03 AM, anatoly techtonik wrote: > ping > > Please, CC. > > On Mon, May 28, 2012 at 7:26 PM, anatoly techtonik > wrote: > > [[[ > > * tools/examples/svnlook.py: Make it usable as a library > > ]]] > > > > Of course it would be better to use iterator interface instead of > > copying lists in memory, but I don't know if Subversion bindings have > > one. Not all commands are implemented, as I don't have time to > > rewrite everything, but at least it is clearly documented what is left, > > so it should easy do to if anyone will need it for their hook scripts. > > > > This is the same as: > > https://github.com/apache/subversion/pull/1/files > > > > Please, CC. > Ok I bite :) (note I'm the app around here, so... :) I looked at it (looks good code-wise) but I'm not sure what it does! Could you please explain what it is and how and why I would use it? thanks, Gabriela
Re: invoke-diff-cmd branch is ready for trunk inclusion
On Wed, Nov 6, 2013 at 3:12 PM, Julian Foad wrote: Gabriela Gibson wrote: Hi Julian and everyone, thank you for the thorough review, the revision with the fixes can be seen here: http://svn.apache.org/r1539448 > the latest invoke-diff-cmd branch: > > http://svn.apache.org/viewvc?view=revision&revision=1538071 > > is ready to be merged into the trunk, iff everyone agrees that the > current substitution syntax is just right and the documentation and the > code itself is of acceptable standard. Hi Gabriela. It looks to me like this work is near complete. I have at last got around to reviewing it and I think there's only one significant thing to do before merging to trunk. Your BRANCH-README file <http://svn.apache.org/repos/asf/subversion/branches/invoke-diff-cmd-feature/BRANCH-README> is a fantastic window into the branch, much more detailed and helpful than the rest of us ever write. I can easily see exactly what the whole feature does, how it's structured, and what your plans are, without having been paying close attention. I'm mentioning this mainly so anybody else contemplating taking a look at this knows this is a good place to start reading. And I'm pleased to see you have taken care to adhere to our coding style. Thank you very much for the compliments! === Interpolation/substitution Syntax === I said at the beginning of this exercise that we should leave the syntax issue till later, and now it is later. :) I also tried command strings involving "%%", "%%%", "%svn_old_label%svn_new_label%" and other variations to see if I could get it to output a plain '%' character when I wanted it and if it would substitute correctly in all combinations and so on. Basically I'm looking for the substitution syntax to (1) be regular, easy to predict the result, with a small number of easy-to-remember rules; (2) be possible to programatically 'escape' any arbitrary string X (even if it contains sequences like '%%' or '%old_label%' or anything that would otherwise have special meaning) to produce a result Y in such a way that putting Y through the substitution will recreate X exactly; and (3) be exactly the same syntax that we already use somewhere else in Subversion. I've taken the entire interpolation mechanism out, completely. Here is why: 1) the apr function called is set to not be a shell, but passes the arguments directly to the called program. 2) we not longer use commonly used variable name, such as 'f1' 'l1' etc, which users may want to preserve for their own purposes, which was the original reason for wanting the interpolation ability. 3) After some thinking about this, I figured that whilst interpolation is fine on the command line, if we were to add individual --invoke-diff-cmd entries to the props (before I discarded that idea in favor of the --cfg-file idea) the interpolation would end up being an implicit magic number in form of '%%%'s. Since we cannot know who uses this repository, I was wondering what happens if two+ teams in maybe different companies use the same repository, and when someone changes the prop command, a lot of unintended breakage down the chain may ensue, or it ends up locking users into a pattern one person decided that may not be useful for others. Hence the idea for the files and I then realized that we can keep it simple and do not need to interpolate at all. We now simply reserve 10 keyword that start with '%svn_' and parse those out when we see them, and users will just have to be creative in their variable name choice. That may not be a good approach but it sure is simple. I unfortunately omitted to document this properly, but have done so now. The reserved keywords are: Diff(4): %svn_old %svn_label_old %svn_new %svn_label_new Merge(6): %svn_mine %svn_label_mine %svn_yours %svn_label_yours %svn_base %svn_label_base. 'label' is placed before the descriptor because this way we can avoid needing a closing delimiter, otherwise, %svn_old_label will get misparsed as /file/foo.c_label. 4) Users can use interpolation with any of their private variables, ie, %f1 or f1 is all no problem, but we do not touch those strings at all, but simply copy them 'as is'. I can however add code to eat the first % of such constructs, but it will be just be 'for show' :-) Users might find this behavior less confusing, but there is also point 3) to consider. Whatever you like as your favorite diff program may not be the next man's choice, which is why I was lobbying to have the option of arbitrary diff-config files instead of using ~/.subversion/config or per-file props that apply globally to every user. That said, %%svn_old will curren
invoke-diff-cmd branch is ready for trunk inclusion
Hi, the latest invoke-diff-cmd branch: http://svn.apache.org/viewvc?view=revision&revision=1538071 is ready to be merged into the trunk, iff everyone agrees that the current substitution syntax is just right and the documentation and the code itself is of acceptable standard. If at all possible, it would be convenient if the invoke-diff-cmd branch is not deleted quite yet, because I'm using a particular merge revision in that branch for testing the forthcoming invoke-merge-cmd feature. Gabriela
ASFBot log message display question
Hi everyone, I asked Humbedoh of the Apache Infrastructure team to remove any extraneous spaces in the ASFBot message (and to leave 2 spaces after a full stop) to make the IRC reports easier to read. It occurred to me that only the actual precis of the log message is important, and that the less people have to read, the easier it is to take in, and also, partial information can be unsatisfying to scan. Some log message precis' will have more than one paragraph, but if it fits I think ASFBot should display this. The question is, how does everyone feel about not displaying the * path/to/file (wibble): Woo. detailed portion of the log message? Gabriela
Configure script error message for ruby problem
Hi,
Whilst looking at the configure output flying by, the following
caught my eye:
configure: WARNING: The detected Ruby is too old for Subversion to use
configure: WARNING: A Ruby which has rb_hash_foreach is required to use the
configure: WARNING: Subversion Ruby bindings
configure: WARNING: Upgrade to the official 1.8.2 release, or later
I have never used ruby, but was piqued to check:
$ ruby --version
ruby 1.9.3p194 (2012-04-20 revision 35410) [i686-linux]
I asked on IRC and breser took a look and thought that the error
message (and the check routine) could be improved, and that the problem
may be that I haven't got ruby-dev installed.
[23:36] ruby -r mkmf -e 'exit(have_func("rb_hash_foreach") ? 0
: 1)'; echo $?
[23:39] cinnamon:
https://www.opencsw.org/mantis/print_bug_page.php?bug_id=3445
[23:39] cinnamon: That might be why, realize that's not
directly related but it looks like the mkmf test fails if ruby
static library is missing.
[23:40] breser:
/usr/lib/ruby/1.9.1/rubygems/custom_require.rb:36:in `require':
cannot load such file -- mkmf (LoadError) from
/usr/lib/ruby/1.9.1/rubygems/custom_require.rb:36:in `require'
I duly installed ruby-dev, however even after removing it with
apt-get --purge, configure was still (nearly) happy:
[[[
checking rb_hash_foreach... yes
checking for rdoc... /usr/bin/rdoc
checking for Ruby major version... 1
checking for Ruby minor version... 9
checking for Ruby teeny version... 3
configure: WARNING: WARNING: The detected Ruby is 1.9.3
configure: WARNING: WARNING: Only 1.8.x releases are fully supported,
1.9.3 support is new
checking for swig... none
configure: Configuring python swig binding
checking for Python includes... -I/usr/include/python2.7
checking for compiling Python extensions... i686-linux-gnu-gcc -pthread
-fPIC
checking for linking Python extensions... i686-linux-gnu-gcc -pthread
-shared -Wl,-O1 -Wl,-Bsymbolic-functions -Wl,-Bsymbolic-
functions -Wl,-z,relro -fno-strict-aliasing -DNDEBUG -g -fwrapv -O2
-Wall -Wstrict-prototypes -D_FORTIFY_SOURCE=2 -g -fstack-p
rotector --param=ssp-buffer-size=4 -Wformat -Werror=format-security
checking for linking Python libraries... -Wl,-O1
-Wl,-Bsymbolic-functions -Wl,-Bsymbolic-functions -Wl,-z,relro
checking for apr_int64_t Python/C API format string... L
checking perl version... 5014002
configure: Configuring Ruby SWIG binding
checking for Ruby include path... -I. -I/usr/include/ruby-1.9.1
-I/usr/include/ruby-1.9.1/ruby -I/usr/include/ruby-1.9.1/ruby/
backward -I/usr/include/ruby-1.9.1/i686-linux checking how to compile
Ruby extensions... gcc -g3 -fno-omit-frame-pointer -fno-inline -Wall
-Wmissing-prototypes -Wstrict-prototypes -Wmissing-declarations -pthread
-DSVN_DEBUG -DAP_DEBUG
checking how to link Ruby extensions... gcc -shared -shrext .so checking
how to link Ruby libraries... -lruby-1.9.1 -lpthread -lrt -ldl -lcrypt -lm
checking for rb_errinfo... yeschecking where to install Ruby scripts...
/usr/local/lib/site_ruby/1.9.1
checking where to install Ruby extensions...
/usr/local/lib/site_ruby/1.9.1/i686-linux
checking how to use output level for Ruby bindings tests... normal
checking for ctypesgen.py... none
]]]
Note that configure is finding the headers for 1.9.1 here and not
1.9.3 as I would have expected, and it looks like apt-get set a
path correctly which got it to work, but I'm not sure how to
figure out what it did. Also, I cannot find a way of restoring
my system to the state that produced the original problem, it
could well be that the apt-get removal was not as comprehensive
as it should have been.
When I looked at the SWIG site, I found this advice:
quote:
32.1.2 Getting the right header files
In order to compile the wrapper code, the compiler needs the
ruby.h header file. This file is usually contained in a directory
such as
/usr/lib/ruby/1.8/x86_64-linux-gnu/ruby.h
/usr/local/lib/ruby/1.6/i686-linux/ruby.h
The exact location may vary on your machine, but the above
location is typical. If you are not entirely sure where Ruby is
installed, you can run Ruby to find out. For example:
$ ruby -e 'puts $:.join("\n")'
/usr/local/lib/ruby/site_ruby/1.6
/usr/local/lib/ruby/site_ruby/1.6/i686-linux
/usr/local/lib/ruby/site_ruby /usr/local/lib/ruby/1.6
/usr/local/lib/ruby/1.6/i686-linux .
--/quote
This command executed on my Ubuntu system gives me:
$ ruby -e 'puts $:.join("\n")'
/usr/local/lib/site_ruby/1.9.1
/usr/local/lib/site_ruby/1.9.1/i686-linux
/usr/local/lib/site_ruby
/usr/lib/ruby/vendor_ruby/1.9.1
/usr/lib/ruby/vendor_ruby/1.9.1/i686-linux
/usr/lib/ruby/vendor_ruby
/usr/lib/ruby/1.9.1
/usr/lib/ruby/1.9.1/i686-linux
breser thought that we'd need some kind of test for mkmf that
doesn't fail, unless mkmf is broken.
I don't know enough to fix this problem, perhaps someone on the
list knows what is going on here?
thanks,
Gabriela
Re: New invoke-diff-cmd feature: --svn_cfg-file and --svn-cfg-file-query
On 30/10/13 17:21, Julian Foad wrote: > This new config-file feature. I think the basic idea is totally > needed: to be able to configure that different diff > programs (and/or with different arguments) should be invoked for > different kinds of files. For example: use oodiff[1] for Open > Document Text docs, ImageMagick "compare"[2] for images, > and "kdiff3" for plain text files and everything else. Also, adding the internal svn diff/merge as an option would also be useful -- for example, when I merged my branch initially, to my horror, kdiff3 showed me 30 conflicts, but the internal svn merge only had 2 conflicts (for which I was very grateful!) So being able to select a visual tool only when you really need it instead of getting prompted 30 times is preferable, likewise, as a bonus service, if svn could the leg work for the user and select the best merge tool to use that would be great. If kdiff3 finds 30 conflicts and svn merge only produces 2, it would save a lot of time if svn should inform me of that fact and, make the optimal choice for me, maybe even interactively if I ask it to. (Say, lets' call this feature 'opti-merge') Not saying this kind of thing is part of this feature, but looking ahead I can see something like that building on the diff-config file concept. As an aside (because it's handy to have) here is the merge of my branch that produced this situation: svn co -r r1526439 https://svn.apache.org/repos/asf/subversion/trunk/ trunk svn co -r r1502389 https://svn.apache.org/repos/asf/subversion/branches/invoke-diff-cmd-feature branch $ kdiff3 --version Qt: 4.8.4 KDE Development Platform: 4.10.5 kdiff3: 0.9.97 (32 bit) >> There are now two optional, internal switches to the invoke-diff-cmd command: >> >> --svn_cfg-file and --svn-cfg-file-query. > Maybe you did it that way because it meant less code churn or an > easier starting point for you? I coded this pretzel because it kept the the patch small and made changing svn.c unnecessary, and so makes it easier to review given that it's just a proof on concept demo. > At the very least a filename pattern with wildcards is > needed (for precedent, see the auto-props configuration). > More than that, I think it would be nice to be able to match on > the value of svn:mime-type and/or other conditions to make it a > bit more powerful than just filenames, but just filename matching > would be powerful enough initially. *nod* > You mentioned to me that you have specific ideas about why you > want a separate config file rather than embedding the > configuration in the '~/.subversion/config' file like we do with > autoprops configuration. Basically it's because diff preferences > can vary per project, which I agree with. Could you elaborate a > bit, here? * It's safer and easier to just tell svn which diff-config file to employ, at the point of use. Every time someone edits or swaps out the ~/.subversion/config file, they run the risk of mistyping or miscopying. Also, even if it only takes 2 minutes to set up, it this occurs 1000 times, that's 33 man hours wasted on a boring task. * ~/.subversion/* isn't versioned since it's not part of a trunk but the personal workspace set-up. There may be situations where different setups are required -- users may work on more than one trunk revision or also on different projects that use svn. * The current design is easily managable and also scriptable, moreover it's versionable if that is desired. * Individual users may have different ideas as to what constitutes a decent diff program, so this accommodates everyone. At this point, it's probably best if we split the branch up here, and just have the invoke-diff-cmd branch for the basic diff part, ready to go into trunk once everyone is happy with it, and make two new branches: invoke-merge-cmd-feature and diff-config-feature. Gabriela
Re: syntax for --invoke-diff-cmd (was: Branch 'invoke-diff-cmd-feature' is ready for half-way review)
On 28/10/13 23:15, Johan Corveleyn wrote: Hi Johan and everyone, substitution variables no longer needs to be escaped, since we set aside 10 keywords that may not be used for anything else. The escape mechanism was a hack because I didn't want to monopolize common variable names(like f1, l1 etc), but Roderich's timely reminder to use the correct svn syntax of 'old' and 'new' in the help file made the natural solution obvious: just use the pattern: svn_[old|new|mine|yours|base]...* I also like this approach because it removes the need for the user to remember whether f1 token was the old or the new, or perhaps mine, yours or base. Currently the extra % at the end is there because we strcmp and then just substitute, even when it's inside a string to allow for things like + and - in front and at the end of file names, some diff programs require this syntax and it accommodates input like foo=%svn_old. Currently, the % at the end ensures are %svn_old% and %svn_old_label% are unique tokens and if we remove the end %, %svn_old_label would end up expanded as /tmp/file_label. So to make this new %svn-* pattern work, we could change it thus: %svn_old %svn_label_old Ie, just put 'label' in second position, I think this is an acceptable looking, clear syntax and keeps things simple. "BTW, thanks for continuing your work, and for hanging in there even though it all takes a while (and sorry for giving the syntax feedback so late). " Thanks for everyone's patience in that matter and for putting up with my unusual syntax experiments :) Picking syntax *is* important because such decisions shape many hours of user time, and changing things later on is always unpopular and usually impossible. So taking the extra time and care is well worth the effort, if people are not 100% happy with a choice, there is always a good reason for it. It's very easy to change the delimiter patterns in the code, so the question can be considered right up to the moment where we release, it doesn't hold anything up at all if someone can think of a better scheme in a few week's time. Gabriela Ps,: I'm sure we can get it completed for 1.9 (do we have an ETA here?) and if time is getting short and I'm still confuzzled, I'll start asking for help. What takes the time is me learning things and getting lost on silly issues like this: http://gabriela-gibson.blogspot.co.uk/2013/10/gdb-and-shell-game.html
New invoke-diff-cmd feature: --svn_cfg-file and --svn-cfg-file-query
Hi, I added an internal mechanism to the --invoke-diff-cmd which allows the user to automatically or interactively select individual files and their respective diff cmds, configurable via an arbitrary config file. There are now two optional, internal switches to the invoke-diff-cmd command: --svn_cfg-file and --svn-cfg-file-query. The usage in each case is: --invoke-diff-cmd='(--switch) (path/to/config/file) (default-diff-cmd)' Those two switches do the following: 1) --svn-cfg-file causes svn to check the given diff_cmds_config file and apply the rules therein automatically on a per-file basis. If a file is not specifically mentioned in the svn-cfg-file, then the given --invoke-diff-cmd is applied. 2) --svn-cfg-file-query works like --svn-cfg-file, but it will prompt the user for every file in the given config-file whether they want to use the instruction in the config file, the default invoke-diff-cmd or input something completely different. Note: --svn-cfg-file-query is not implemented yet. The config file itself looks like so: # Note the test1 label subversion/svn/svn.c = diff -L "test1" %svn_old% %svn_new% # Testing with more than one word in the label subversion/libsvn_subr/io.c = diff -L "test 2" %svn_old% %svn_new% The code itself has a few efficiency issues, but is sufficient for a proof-of-concept demonstration. The revision is located here: http://svn.apache.org/viewvc?view=revision&revision=r1536020 I sent an old copy of the BRANCH-README, the up-to-date version that describes the new feature is here: http://svn.apache.org/viewvc?view=revision&revision=r1536037 Thanks for looking! Gabriela
Re: Branch 'invoke-diff-cmd-feature' is ready for half-way review
Hi,
thank you very much for your time!
1.) new vs original issue: Fixed, thank you for spotting this.
2.) The delimiter:
There appears to be some team miscommunication here -- afaik, the last
status was that danielsh asked me to implement the semi-colon, so this
is what you got^Whad :)
Now I had a long think and realised there are other issues with
expanding that have the potential to be traps, and so, I removed the
ability to escape the delimiter and changed the syntax to look like
so:
[[[
struct replace_tokens_tab
{
const char *delimiter;
const char *replace;
} tokens_tab[] = { /* Diff terminology */
{ "%svn_new_label%", label1 },
{ "%svn_old_label%", label2 },
{ "%svn_base_label%", label3 },
{ "%svn_old%", from },
{ "%svn_new%", to },
{ "%svn_base%", base },
{ NULL, NULL }
};
if (label3) /* Merge terminology */
{
tokens_tab[0].delimiter = "%svn_to_label%";
tokens_tab[1].delimiter = "%svn_from_label%";
tokens_tab[3].delimiter = "%svn_to%";
tokens_tab[4].delimiter = "%svn_from%";
}
]]]
Rationale:
this new syntax frees the commonly used 'fN' and 'lN' variable names and
is completely unambiguous, and also fairly unique. It's more to type,
but much less to worry about. Moreover it matches the
%custom_keyword% syntax for the props, which is new in SVN 1.8:
http://subversion.apache.org/docs/release-notes/1.8#custom-keywords
The issue is that, if we allow escaping (which we need to do if we use
%f1 %f1% or ;f1 etc) it only is necessary because we're appropriating
common variable names or a shell character as part of the delimiter.
But what letting users add escapes does (as a side effect) is that it
creates a magic global number(in form of n escapes), and whilst this
is not an issue with immediate use that only occurs on the command line
or the config file, once/if we implement issue 2447 and allow file props
to carry individual invoke-diff-cmds, this ability can get problematic:
http://subversion.tigris.org/issues/show_bug.cgi?id=2447
3.) The pool longevity:
**result[] is allocated to the pool that is passed into
svn_io_run_external_diff(), and so persists past the life of
__create_custom_diff_cmd. Reassigning word->data has no effect on
what has been put into **result[]. (I checked with GDB)
4.) The word->elt_size query:
"
result = apr_palloc(pool,
(words->nelts+1) * words->elt_size * sizeof(char *) )
Why is words->elt_size needed here - result is an array of char*?"
words->elt_size give us the size of the largest substitution we're
making, word->nelts+1 is the max number of entires possible in
**result[].
5.) The quoting issue:
Weak quotes do not need to be escaped since they are always inside
strong quotes. The apr procedure apr_proc_create() that does the work
here has been primed in this instance to call the desired program
directly and not through a shell, so no additional interpretation
happens on the way, what you type is what you get! If the user adds
escapes it makes no difference at all, it works either way, at least
for GNU diff.
Gabriela
Branch 'invoke-diff-cmd-feature' is ready for half-way review
Hi, my branch is ready for a half-way review as the 'invoke-diff-cmd' part is complete and adding the 'invoke-diff3-cmd' part would only duplicate current errors. The BRANCH-README file is here: https://svn.apache.org/repos/asf/subversion/branches/invoke-diff-cmd-feature/BRANCH-README thanks for looking! Gabriela
Re: Presenting net code changes for a branch
On 13/10/13 15:15, Lieven Govaerts wrote: Hi Lieven, The last time you updated the invoke-diff-cmd branch from trunk was in rr1526487, you updated from trunk r1526439. Sorry, I had merged locally but not committed. (done now) So if you compare your branch to trunk@1526439 you will get a diff file with only your changes. No? $ svn diff -x -pwb ^/subversion/trunk@1526439 ^/subversion/branches/invoke-diff-cmd-feature Previously I got all the merge changes, without seeing any of mine at all. And now that it's been re-merged (I reverted and started over) & committed, I get nothing at all. But: diff -r ~/trunk/subversion/ ~/branches/invoke-diff-cmd-feature/subversion/ shows me all my changes as expected. Yet, this: svn diff -x -pwb /home/g/trunk/ /home/g/branches/invoke-diff-cmd-feature/ shows me nothing at all (before it showed me what the merge added from trunk, but none of my changes). For some reason, the ^ expansion does not work on my system so I use a full path here. thanks, Gabriela ps.: I attached a file with my shell session, just in case I'm not seeing the obvious (as per usual!) pps.: this by now it probably a svn-user discussion, and so I apologise to the dev list. I'm not sure, but should we continue this thread on svn-user? $ cd ~/trunk ~/trunk> $ svn diff -x -pwb /home/g/trunk/ /home/g/branches/invoke-diff-cmd-feature/ ~/trunk> $ cd ~/branches/invoke-diff-cmd-feature ~/branches/invoke-diff-cmd-feature> $ svn diff -x -pwb /home/g/trunk/ /home/g/branches/invoke-diff-cmd-feature/ ~/branches/invoke-diff-cmd-feature> $ diff -r --brief ~/trunk/subversion/ ~/branches/invoke-diff-cmd-feature/subversion/ | grep -v Only Files /home/g/trunk/subversion/include/svn_client.h and /home/g/branches/invoke-diff-cmd-feature/subversion/include/svn_client.h differ Files /home/g/trunk/subversion/include/svn_config.h and /home/g/branches/invoke-diff-cmd-feature/subversion/include/svn_config.h differ Files /home/g/trunk/subversion/include/svn_error_codes.h and /home/g/branches/invoke-diff-cmd-feature/subversion/include/svn_error_codes.h differ Files /home/g/trunk/subversion/include/svn_io.h and /home/g/branches/invoke-diff-cmd-feature/subversion/include/svn_io.h differ Files /home/g/trunk/subversion/libsvn_client/deprecated.c and /home/g/branches/invoke-diff-cmd-feature/subversion/libsvn_client/deprecated.c differ Files /home/g/trunk/subversion/libsvn_client/diff.c and /home/g/branches/invoke-diff-cmd-feature/subversion/libsvn_client/diff.c differ Files /home/g/trunk/subversion/libsvn_subr/config_file.c and /home/g/branches/invoke-diff-cmd-feature/subversion/libsvn_subr/config_file.c differ Files /home/g/trunk/subversion/libsvn_subr/io.c and /home/g/branches/invoke-diff-cmd-feature/subversion/libsvn_subr/io.c differ Files /home/g/trunk/subversion/libsvn_subr/io.c~ and /home/g/branches/invoke-diff-cmd-feature/subversion/libsvn_subr/io.c~ differ Files /home/g/trunk/subversion/libsvn_wc/wc-queries.h and /home/g/branches/invoke-diff-cmd-feature/subversion/libsvn_wc/wc-queries.h differ Files /home/g/trunk/subversion/svn/cl.h and /home/g/branches/invoke-diff-cmd-feature/subversion/svn/cl.h differ Files /home/g/trunk/subversion/svn/diff-cmd.c and /home/g/branches/invoke-diff-cmd-feature/subversion/svn/diff-cmd.c differ Files /home/g/trunk/subversion/svn/log-cmd.c and /home/g/branches/invoke-diff-cmd-feature/subversion/svn/log-cmd.c differ Files /home/g/trunk/subversion/svn/svn.c and /home/g/branches/invoke-diff-cmd-feature/subversion/svn/svn.c differ Files /home/g/trunk/subversion/svnlook/svnlook.c and /home/g/branches/invoke-diff-cmd-feature/subversion/svnlook/svnlook.c differ Files /home/g/trunk/subversion/tests/cmdline/diff_tests.py and /home/g/branches/invoke-diff-cmd-feature/subversion/tests/cmdline/diff_tests.py differ Files /home/g/trunk/subversion/tests/libsvn_fs_fs/fs-pack-test and /home/g/branches/invoke-diff-cmd-feature/subversion/tests/libsvn_fs_fs/fs-pack-test differ ~/branches/invoke-diff-cmd-feature> $ svn info Path: . Working Copy Root Path: /home/g/branches/invoke-diff-cmd-feature URL: https://svn.apache.org/repos/asf/subversion/branches/invoke-diff-cmd-feature Relative URL: ^/subversion/branches/invoke-diff-cmd-feature Repository Root: https://svn.apache.org/repos/asf Repository UUID: 13f79535-47bb-0310-9956-ffa450edef68 Revision: 1531702 Node Kind: directory Schedule: normal Last Changed Author: gbg Last Changed Rev: 1531702 Last Changed Date: 2013-10-13 17:10:59 +0100 (Sun, 13 Oct 2013) ~/branches/invoke-diff-cmd-feature> $ cd - /home/g/trunk ~/trunk> $ svn info Path: . Working Copy Root Path: /home/g/trunk URL: https://svn.apache.org/repos/asf/subversion/trunk Relative URL: ^/subversion/trunk Repository Root: https://svn.apache.org/repos/asf Repository UUID: 13f79535-47bb-0310-9956-ffa450edef68 Revision: 1531712 Node Kind: directory Schedule: normal Last Changed Author: brane Last Changed Rev: 1531612 Last Changed Date: 2013-10-13 02:45:
Re: Presenting net code changes for a branch
On 13/10/13 13:34, Lieven Govaerts wrote: Is this any different than running: svn diff -x -pwb ^/subversion/trunk ^/subversion/branches/invoke-diff-cmd-feature ? At this moment there is a large diff between trunk and your branch, but that's because you haven't brought your branch up to date with trunk yet. But once you do that, the diff between branches should represent your additions on the branch. Hi Lieven, thank you for the tip, but it's not working for me currently. The trunk has been freshly merged into the branch, and your command shows me all the changes that have been merged from the trunk to the branch, but, interestingly so, none of the changes I've made to the branch. The original diff file in the other post is my actual code (44k, oh dear) which is why I'm looking for a way of taming this into something that is presentable. Gabriela ps.: i made a mistake in the original bash code, it should read diff -p --context=0 "$HOME/trunk/$line" "$PWD/$line"; fi;\
Presenting net code changes for a branch
Hi,
my branch has grown into a veritable forest, and so, I thought that
it would be convenient to present the net code changes in a file called
'entire-branch-code' (attached).
This is generated like so:
1) Merge trunk to branch
2) run this bash command:
svn log --stop-on-copy | grep '^*\ subversion' |\
awk '{gsub(/^ +| +$/,"")} {print $0}' | cut -d '*' -f 2 |\
sort --unique | while read line; do echo $line | if [ -a $line ];\
then echo ""; echo \
"";\
echo "";\
diff -p --context=0 -b -B -w "$HOME/trunk/$line" "$PWD/$line"; fi;\
done > entire-branch-code
Is this useful? Can this be improved?
thanks,
Gabriela
*** /home/g/trunk/subversion/include/svn_client.h 2013-09-24
21:40:40.306770699 +0100
--- /home/g/branches/invoke-diff-cmd-feature/subversion/include/svn_client.h
2013-09-26 13:50:43.967821533 +0100
*** svn_client_blame(const char *path_or_url
*** 3023
--- 3024,3028
+ * @a invoke_diff_cmd is used to call an external diff program but may
+ * not be @c NULL. The command line invocation will override the
+ * invoke-diff-cmd invocation entry(if any) in the Subversion
+ * configuration file.
+ *
*** svn_client_blame(const char *path_or_url
*** 3053
--- 3059,3087
+ * @since New in 1.9.
+ */
+ svn_error_t *
+ svn_client_diff7(const apr_array_header_t *options,
+ const char *path_or_url1,
+ const svn_opt_revision_t *revision1,
+ const char *path_or_url2,
+ const svn_opt_revision_t *revision2,
+ const char *relative_to_dir,
+ svn_depth_t depth,
+ svn_boolean_t ignore_ancestry,
+ svn_boolean_t no_diff_added,
+ svn_boolean_t no_diff_deleted,
+ svn_boolean_t show_copies_as_adds,
+ svn_boolean_t ignore_content_type,
+ svn_boolean_t ignore_properties,
+ svn_boolean_t properties_only,
+ svn_boolean_t use_git_diff_format,
+ const char *header_encoding,
+ svn_stream_t *outstream,
+ svn_stream_t *errstream,
+ const apr_array_header_t *changelists,
+ const char *invoke_diff_cmd,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *pool);
+
+ /** Similar to svn_client_diff7(), but with @a invoke_diff_cmd.
+ *
+ * @deprecated Provided for backward compatibility with the 1.8 API.
*** svn_client_blame(const char *path_or_url
*** 3055
--- 3090
+ SVN_DEPRECATED
*** svn_client_diff(const apr_array_header_t
*** 3213
! * identically to svn_client_diff6(), using @a path_or_url for both of that
--- 3248
! * identically to svn_client_diff7(), using @a path_or_url for both of that
*** svn_client_diff(const apr_array_header_t
*** 3216
! * All other options are handled identically to svn_client_diff6().
--- 3251
! * All other options are handled identically to svn_client_diff7().
*** svn_client_diff(const apr_array_header_t
*** 3217
--- 3253,3285
+ * @since New in 1.9.
+ */
+ svn_error_t *
+ svn_client_diff_peg7(const apr_array_header_t *diff_options,
+ const char *path_or_url,
+ const svn_opt_revision_t *peg_revision,
+ const svn_opt_revision_t *start_revision,
+ const svn_opt_revision_t *end_revision,
+ const char *relative_to_dir,
+ svn_depth_t depth,
+ svn_boolean_t ignore_ancestry,
+ svn_boolean_t no_diff_added,
+ svn_boolean_t no_diff_deleted,
+ svn_boolean_t show_copies_as_adds,
+ svn_boolean_t ignore_content_type,
+ svn_boolean_t ignore_properties,
+ svn_boolean_t properties_only,
+ svn_boolean_t use_git_diff_format,
+ const char *header_encoding,
+ svn_stream_t *outstream,
+ svn_stream_t *errstream,
+ const apr_array_header_t *changelists,
+ const char *invoke_diff_cmd,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *pool);
+
+
+ /**
+ * Similar to svn_client_peg7(), but with @a no_diff_added set to
+ * FALSE, @a ignore_properties set to FALSE and @a properties_only
+ * set to FALSE.
+ *
+ * @deprecated Provided for backward compatibility with the 1.7 API.
*** svn_client_diff(const apr_array_header_t
*** 3219
--- 3288
+ SVN_DEPRECATED
*** svn_client_diff_peg6(const apr_array_hea
*** 3
Re: Feature Request -- svn commit --unimportant
I can see Eitan's point -- if we went ahead and changed all the obsolete tags on the Subversion website, it would remove a lot of useful blame info. Is a two stage operation possible? say, you type: 'svn commit --blame-revert ...' Part one would be a regular commit, part two would be a second commit that restores all the original authors that the first commit modified; so one commit would produce two consecutive revisions. To be (relatively) safe, this could be a repository admin action only. Gabriela On Sun, Oct 6, 2013 at 8:45 PM, Bert Huijben wrote: > > > > -Original Message- > > From: Gabriela Gibson [mailto:gabriela.gib...@gmail.com] > > Sent: zaterdag 5 oktober 2013 17:48 > > To: Subversion Development; li...@eitanadler.com > > Subject: Feature Request -- svn commit --unimportant > > > > Hi, > > > > The following conversation took place on the svn-dev IRC yesterday > > evening, with Eitan Adler: > > > > [22:34] feature request: "svn commit --unimportant" which does > > not destroy "svn blame" unless you write "svn blame --all" > > [22:44] Eitan: what were you trying to do that made you think > > of this? > > [22:45] cinnamon: fix up whitespace errors > > [22:45] cinnamon: the idea is similar to wikipedia's "trivial > edit" > > [22:46] cinnamon: I'd love to go en mass through our code and > > fix up internal whitespace errors but that would destroy "svn blame" > > of who wrote what > > [22:46] nod, that would defeat the purpose of blame somewhat > > [22:46] you can of course use the ignore white space option for > blame > > [22:47] lgo: in some cases it isn't just whitespace > > [22:47] for example, in our documentation we use "&os;" instead > > of "FreeBSD" > > [22:47] and I'd love to be able to change all the latter to the > > former > > [22:47] etc. etc. etc. > > [22:48] cinnamon: that is why I propose "blame" and "blame > > --include-everything/" > > > > I think this would be a very useful addition for many users and so > > that it does not get lost in the IRC chat, I thought I post it here > > for discussion. > > I assume you do know about the -x argument to blame where you can ask blame > to ignore whitespace only changes? > (E.g. add "-x -b", see 'svn help blame') > > > In theory something like this could be implemented by adding some revision > property to specify which commits to ignore. But it might be hard to keep > things working if there are revisions where this property is supplied, but > major changes are committed. In that case it would be very hard to show old > revisions for some lines. > > Bert > > > > Gabriela > >
Feature Request -- svn commit --unimportant
Hi, The following conversation took place on the svn-dev IRC yesterday evening, with Eitan Adler: [22:34] feature request: "svn commit --unimportant" which does not destroy "svn blame" unless you write "svn blame --all" [22:44] Eitan: what were you trying to do that made you think of this? [22:45] cinnamon: fix up whitespace errors [22:45] cinnamon: the idea is similar to wikipedia's "trivial edit" [22:46] cinnamon: I'd love to go en mass through our code and fix up internal whitespace errors but that would destroy "svn blame" of who wrote what [22:46] nod, that would defeat the purpose of blame somewhat [22:46] you can of course use the ignore white space option for blame [22:47] lgo: in some cases it isn't just whitespace [22:47] for example, in our documentation we use "&os;" instead of "FreeBSD" [22:47] and I'd love to be able to change all the latter to the former [22:47] etc. etc. etc. [22:48] cinnamon: that is why I propose "blame" and "blame --include-everything/" I think this would be a very useful addition for many users and so that it does not get lost in the IRC chat, I thought I post it here for discussion. Gabriela
Re: Sqlite compiler spam delenda est.
Many thanks Julian and Alan, for the bashism heads-up and the grep -f tip. I'll think that this trick might be a nice addition to the build tips on the wiki? Will this work with the various Windows Bash clones? G On Tue, Oct 1, 2013 at 2:16 PM, Julian Foad wrote: > Gabriela Gibson wrote: > > > Because sqlite produces ~5 extra letters of bumpf per compile that > often > > obscures actually important compiler messages, I've not been as diligent > as > > I should have been about reading compiler messages. A lot the time I > get lucky > > and it doesn't matter, but I also got caught out by that. > > > > Here is how to get rid of all the noise and make your compiler output > useful > > once again: > > > > First we take a snapshot of the 'native' compiler messages that come > > with the trunk: > > > > make 1>stdout.report 2>stderr.constant; \ > > sort --unique stderr.constant > stderr.unique | grep -v sqlite > > > > This removes the sqlite warnings and shows you only the current warning > messages > > that are actually ~/trunk related. > > Cool. I like tricks like this. > > At the end of my build script I print a slightly condensed summary of all > the warnings that were produced, by piping the (stderr) log file content > through the following command: > > grep -B1 'subversion/.*: warning:' | > sed -e 's,/home/julianfoad/src/subversion[^/]*/,,' \ > -e "s/[‘’]/'/g" \ > -e 's/ warning: / /' > > (The second sed expression replaces the curly quotes that my UK-localized > GCC outputs with plain ASCII quote marks to make it less likely to display > wrongly on screen.) > > > Any subsequent compiles that are started with the line below will use the > > generated stderr.unique file to filter output and remove every compiler > message > > that is 'native' to the trunk, leaving just the messages that pertain to > > your code: > > > > make > stdout.report 2> >(tee stderr.report | while read line; do echo > > $line | grep -F -- $line" stderr.unique 2> /dev/null; if [ $? = 1 ]; > > then echo "$line" >&2; fi; done) > > I expect you were wondering if grep could do this by itself. It can: > > make > stdout.report 2> >(tee stderr.report | grep -v -F -f stderr.unique > >&2) > > - Julian > > > > You can also type this line when you invoke the emacs compile buffer and > your > > C-x ` will continue to work. >
Sqlite compiler spam delenda est.
Because sqlite produces ~5 extra letters of bumpf per compile that often obscures actually important compiler messages, I've not been as diligent as I should have been about reading compiler messages. A lot the time I get lucky and it doesn't matter, but I also got caught out by that. Here is how to get rid of all the noise and make your compiler output useful once again: First we take a snapshot of the 'native' compiler messages that come with the trunk: make 1>stdout.report 2>stderr.constant; \ sort --unique stderr.constant > stderr.unique | grep -v sqlite This removes the sqlite warnings and shows you only the current warning messages that are actually ~/trunk related. Any subsequent compiles that are started with the line below will use the generated stderr.unique file to filter output and remove every compiler message that is 'native' to the trunk, leaving just the messages that pertain to your code: make > stdout.report 2> >(tee stderr.report | while read line; do echo $line | grep -F -- $line" stderr.unique 2> /dev/null; if [ $? = 1 ]; then echo "$line" >&2; fi; done) You can also type this line when you invoke the emacs compile buffer and your C-x ` will continue to work.
Managing my patch collection and svn sessions
I'm looking at my hoard of patches and log messages in my ~/patches directory, and it's large and not really all that useful. Still, it's not really something I'd want to delete, even failed attempts and odd snippets are often useful to keep around. In a way it's a bit ironic to contribute code to a VCS but to hoard patches, notes and log messages like it's 1995, I'm sure I must be doing it wrong. How do you keep your local patch collection organised? Or did you find that old patches are not really useful to you? I did try keeping various trunks, but this brought other problems, so I don't anymore, what I do if I need to switch is to take a patch and revert the 'working' trunk, and apply the patch for the other project. I'd really like a program that takes care of managing my svn work sessions with different projects. So, say I want to swap my current session of project A with session N of the project B I worked on last week: * stash the current workspace of my session on project A (patch, history, notes, etc) * fetch the correct trunk/branch revision pertaining to loaded session of project B * apply the patch of the selected session of project B. * restore my shell history of that session (or entire project) * show me the notes I kept for that session, or if I ask, all the notes of sessions of project B Is there such a tool? Maybe I just need a better way of keeping house?
[PATCH] Deprecated function called in subversion/libsvn_wc/info.c svn_wc_info_dup
[[[
Update call to deprecated function.
* subversion/libsvn_wc/info.c:
(svn_wc_info_dup): Update call to deprecated function and type to
current versions.
]]]
:
Index: subversion/libsvn_wc/info.c
===
--- subversion/libsvn_wc/info.c (revision 1516201)
+++ subversion/libsvn_wc/info.c (working copy)
@@ -51,10 +51,10 @@ svn_wc_info_dup(const svn_wc_info_t *info,
= apr_array_make(pool, info->conflicts->nelts, info->conflicts->elt_size);
for (i = 0; i < info->conflicts->nelts; i++)
{
- APR_ARRAY_PUSH(new_conflicts, svn_wc_conflict_description2_t *)
-= svn_wc__conflict_description2_dup(
+ APR_ARRAY_PUSH(new_conflicts, svn_wc_conflict_description3_t *)
+= svn_wc__conflict_description3_dup(
APR_ARRAY_IDX(info->conflicts, i,
- const svn_wc_conflict_description2_t *),
+ const svn_wc_conflict_description3_t *),
pool);
}
new_info->conflicts = new_conflicts;
svn quick-start idea
'Quick-starts' is an idea for setting up a info-style help system that offers users 'quick-starts' on the command line. Something like that is often used on MUDS too, for players and wizards. Quick-starts are short 'cooking recipes' that help users get from A to B in the shortest way, but may also contain hints and links to the svn book or other web resources. Here is how it all would (probably) look: $ svn --quick-start help => --qs topicslist the available topics => --qs help help how to use quick-start => --qs help locallist local svn help entries => ... $ svn --qs topics => Repository, Propset, Diff, Log, Merge, ..., Tricks, . => => To view the list of quick-starts pertaining to a topic, type => $svn --qs . => The quick-starts are listed by number, to view it type => $svn --qs => To view all the available quick-starts pertaining to 'foo' that => also contain '-bar' type $ svn --qs --grep "foo" "bar" $ svn --qs Repository 1. make a local repository with local branches for one user 2. make a public repository 3. upgrade a repository ... $ svn --quick-start Repository 1 => Make a local repository with local branches for one user => => => $ mkdir myRepository; svnadmin create myRepository; ls -al nwRepo/ => => ... more instruction steps ... => => Also see the Subversion book, page n $ ... == Disadvantages: * it's OS dependent. * It adds documentation chores. * it's a group project for devs and users. Advantages: * it's within svn, where you need it, if you do. If you have to switch away to the web or pick up the book, there goes part of your concentration. * it helps devs to quickly understand how parts of svn are used which they are not familiar with. * it's searchable. If you want to see in what constructs and combinations command 'foo' is used, you can get a comprehensive list, so it also functions as an 'svn command dictionary'. * it allows implementation of an elective daily tip feature (I find them quite fun) * it can be built over time, the starter kit is done if we have quick-starts for the most basic tasks. * devs can populate it as they go along, and users can contribute. For fun add a small token gift for a lottery user contribs can win -- mugs, tshirts or other small souveniers. * Users could add their own site-specific section(s) for their local installation. * it would act as the canonical source for quick-starts. The web has a ton of svn cooking recipes, not all of which work(anymore), and as was pointed out, copying text from a webpage can be unhealthy for your computer and privacy. have a great weekend, Gabriela
Re: svn commit: r1500884 - /subversion/branches/invoke-diff-cmd-feature/subversion/svnlook/svnlook.c
On Mon, Jul 8, 2013 at 8:23 PM, Johan Corveleyn wrote: > On Mon, Jul 8, 2013 at 9:03 PM, wrote: > > Author: gbg > > I didn't realize you intended to deprecate --diff-cmd in favor of > --invoke-diff-cmd. Are you sure? I can see that they're related and > mutually exclusive, but not sure if they are direct "successors". > > I think it depends on the perspective. Technically, diff-cmd is a subset of invoke-diff-cmd, but in practice, people will probably think of invoke-diff-cmd as an extension. > You made this change only for 'svnlook diff', but not for 'svn diff'. > Is that intentional, or just Work In Progress? > > diff-cmd itself has been rerouted through the code for invoke-diff-cmd and the code it used to run on has been deprecated in r1500647. Does --invoke-diff-cmd support the "simple invocation style of > --diff-cmd" as well? I.e. can I do 'svnlook diff -r3 $REPOS > --invoke-diff-cmd /usr/bin/diff' and get the same result as with > --diff-cmd? > > No, -- invoke-diff-cmd just takes whatever you give it, substitutes ;f1 ... ;l3 and passes that expansion on to your diff command. I changed the help string in svnlook.c with the idea that it will move people towards using invoke-diff-cmd (which does not have diff-cmd's restrictions), but as you rightly point out, one man's restrictions are another man's labour saving device :-) So I don't know what to do here, I can just change the help string back and we keep diff-cmd around(and it keeps life simple for everyone) or, I can made all the other help strings match the deprecation notice. Gabriela
[PATCH] HACKING GUIDE: IRC address and freenode webchat client addition
[[[ Add information about svn-dev IRC channel. * community-guide/general.part.html: (participating): Add IRC address and freenode webchat client. ]]] Notes: -Go to http://subversion.apache.org/"; ->http://subversion.apache.org/ and was removed with the reasoning that: a) they are already here (and where we'd like them to be) b) we want them to look at the links in this section and not wander off to get lost. I've set the user name to 'visitorNNN' so we know they came through the HACKING GUIDE. Gabriela Index: community-guide/general.part.html === --- community-guide/general.part.html (revision 1479254) +++ community-guide/general.part.html (working copy) @@ -19,13 +19,16 @@ A number of Subversion's developers are paid by th improve Subversion, while many others are simply excellent volunteers who are interested in building a better version control system. -The community exists mainly through mailing lists and a Subversion -repository. To participate: +The community exists mainly through IRC, mailing lists and a +Subversion repository. To participate: -Go to http://subversion.apache.org/"; ->http://subversion.apache.org/ and + + +Join us on irc.freenode.net #svn-dev or via the +http://webchat.freenode.net/?nick=visitor&channels=svn-dev"; +>freenode IRC webchat interface. + - Join the "dev", "commits", and "announce" mailing lists. The dev list, dev@subversion.apache.org, is where almost all discussion takes place. All development questions should go
[PATCH] HACKING GUIDE: update and relocate advice on how to write log message for branches.
[[[ Rework branch and log message documentation. Tidy HTML. * community-guide/conventions.part.html (log-messages): Rework relocated branch log message documentation moved from community-guide/general.part.html#lightweight-branches. Remove mention of 'CIA' and substitute with ASFBot and update link. Adjust paragraph containing link to 'partial-commit-access' to columns limit. * community-guide/general.part.html (branch-creation-and-management): Add link to log message section. (lightweight-branches): Relocate documentation pertaining to log messages to community-guide/conventions.part.html#log-messages. ]]] Index: community-guide/conventions.part.html === --- community-guide/conventions.part.html (revision 1479254) +++ community-guide/conventions.part.html (working copy) @@ -849,18 +849,33 @@ surprising frequency, too: for example, it might b original commit and now the change is being ported to a maintenance branch. -The log message is the introduction to the change. Start it off -with one line indicating the general nature of the change, and follow -that with a descriptive paragraph if necessary. This not only helps -put developers in the right frame of mind for reading the rest of the -log message, but also plays well with the "CIA" bot that echoes the -first line of each commit to realtime forums like IRC. (For details, -see http://cia.vc/";>http://cia.vc/.) However, if the -commit is just one simple change to one file, then you can dispense -with the general description and simply go straight to the detailed -description, in the standard filename-then-symbol format shown -below. +The log message is the introduction to the change. + +If you are working on a branch, prefix your log message with: + + On the 'name-of-branch' branch: (Start of your log message) + + + +Start your log message with one line indicating the general nature +of the change, and follow that with a descriptive paragraph if +necessary. + + + + This not only helps put developers in the right frame of mind for +reading the rest of the log message, but also plays well with the +"ASFBot" bot that echoes the first line of each commit to realtime +forums like IRC. (For details, see +http://wilderness.apache.org/";>http://wilderness.apache.org/ +) + + If the commit is just one simple change to one file, then you can +dispense with the general description and simply go straight to the +detailed description, in the standard filename-then-symbol format +shown below. + Throughout the log message, use full sentences, not sentence fragments. Fragments are more often ambiguous, and it takes only a few more seconds to write out what you mean. Certain fragments like @@ -973,9 +988,9 @@ of changes that accomplishes a single goal, and ea start with a sentence or two summarizing the change. Truly independent changes should be made in separate commits, of course. -See #crediting">Crediting for how to give credit to -someone else if you are committing their patch, or committing a change -they suggested. +See #crediting"> +Crediting for how to give credit to someone else if you are +committing their patch, or committing a change they suggested. One should never need the log entries to understand the current code. If you find yourself writing a significant explanation in the @@ -1032,6 +1047,21 @@ Malagasy translation." Please write your log mess everybody involved in the project can understand the changes you made. +If you're using a branch to "checkpoint" your code, and don't feel +it's ready for review, please put some sort of notice at the top of +the log message, after the 'On the 'xxx' branch notice', such as: + + + *** checkpoint commit -- please don't waste your time reviewing it *** + + +And if a later commit on that branch should be reviewed, +then please supply, in the log message, the appropriate 'svn diff' +command, since the diff would likely involve two non-adjacent commits +on that branch, and reviewers shouldn't have to spend time figuring +out which ones they are. + + Index: community-guide/general.part.html === --- community-guide/general.part.html (revision 1479254) +++ community-guide/general.part.html (working copy) @@ -432,6 +432,11 @@ in this way, so making good use of that feature (b 1.5 or newer clients, and by performing all merges to and from the roots of branches) is highly encouraged. +For our policy on log messages for your branch, please note the +section on +#log-messages"> +writing log messages. + @@ -450,27 +455,14 @@ applies to committers of other http://www projects, but please talk to us (on dev@)—introduce yourself and the problem you plan to work on. -If you're just using the branch to "checkpoint" your code, and -don't feel it's ready for review, please put some sort of notice at -the top of the
Re: Review of invoke-diff-cmd-feature branch
On 6/11/13, Daniel Shahaf wrote:
> Gabriela Gibson wrote on Mon, Jun 10, 2013 at 23:39:45 +0100:
>> On 6/10/13, Daniel Shahaf wrote:
>>
>> * @a invoke_diff_cmd takes an argument which is used to call an
>> * external diff program. When invoked, the argument may not be @c
>> * NULL or the empty string "". The command line invocation will
>> * override the invoke-diff-cmd invocation entry (if any) in the
>> * Subversion configuration file.
>>
>> Is that passable?
>>
>
> No, because the argument _may_ be NULL (since the deprecated code path
> (and people updating calls to deprecated functions in their code to call
> the non-deprecated equivalents) needs it), and the docstring says it may
> not be. Note, even people who updated their code to the 1.9 API might
> still want to pass NULL because they don't want to override the
> invoke-diff-cmd in the config (or because they want to use the builtin
> diff implementation).
>
> Does this make sense?
I had a look how TortoiseSVN approaches this. From what I can
see, no matter what we do, they have to adjust their code, because
they turn off the config file options explicitly when they run
their custom code, so adding invoke-diff-cmd to the config file
will scupper that plan[1]
So in that sense, behavior *has* changed and our addition is not
backward compatible with older clients, even if they use ...diff6().
I think I should probably write a short intro to the new code for
client devs to make updating their code easier. Another thing to
consider is whether we should offer a 'switch service' in to make
external surgery of the set config file values unnecessary.
On the bright side, I've finally understood why it's OK to pass
NULL safely because the code path is selected on that value. What
I should have said in the doxygen comment to
svn_io_create_custom_diff_cmd is that thou shall not pass NULL.
New comment for svn_client_diff7 is now:
* @a invoke_diff_cmd takes an argument which is used to call an
* external diff program when not NULL or "".
(Will also update the svn_io_create_custom_diff_cmd doxygen comment.)
==
About the delimiter to select:
I don't have the expertise to make an informed decisions here, or
even to hold a strong opinion. But I happily code whatever the
team chooses :) I could also rewrite (or try to!) the
svn_io_create_custom_diff_cmd in C printf style if the current
code shape is the wrong approach for what we want to do (it
probably is)
I still think the ---f1 pattern is the easiest to use because
it's guaranteed not to interfere with anything, nothing needs to
be escaped either and everything users want to do can be done
much simpler.
The strongest argument against ---f1 was it's novel and a bit
ugly, but, in the scheme of things, I think this is a smaller
problem than overlaying user's interpolation schemes and
painstakingly counting and escaping %'s or $'s.
Also it visually separates the user's interpolation scheme nicely
from ours.
It's no problem to allow users the choice of two interpolating
syntaxes either, maybe the breadths of possibilities is just to
broad to make a decent swiss army knife here that covers
everyone's needs?
(other stuff has been noted and is on the to-do list)
Gabriela
[1]
[[[
bool SVN::CreatePatch(const CTSVNPath& path1, const SVNRev& revision1,
const CTSVNPath& path2, const SVNRev& revision2,
const CTSVNPath& relativeToDir, svn_depth_t depth,
bool ignoreancestry, bool nodiffadded, bool
nodiffdeleted, bool showCopiesAsAdds, bool ignorecontenttype,
bool useGitFormat, bool ignoreproperties, bool
propertiesonly, const CString& options, bool bAppend, const CTSVNPath&
outputfile)
{
// to create a patch, we need to remove any custom diff tools
which might be set in the config file
svn_config_t * cfg = (svn_config_t *)apr_hash_get (m_pctx->config,
SVN_CONFIG_CATEGORY_CONFIG, APR_HASH_KEY_STRING);
CStringA diffCmd;
CStringA diff3Cmd;
if (cfg)
{
const char * value;
svn_config_get(cfg, &value, SVN_CONFIG_SECTION_HELPERS,
SVN_CONFIG_OPTION_DIFF_CMD, NULL);
diffCmd = CStringA(value);
svn_config_get(cfg, &value, SVN_CONFIG_SECTION_HELPERS,
SVN_CONFIG_OPTION_DIFF3_CMD, NULL);
diff3Cmd = CStringA(value);
svn_config_set(cfg, SVN_CONFIG_SECTION_HELPERS,
SVN_CONFIG_OPTION_DIFF_CMD, NULL);
svn_config_set(cfg, SVN_CONFIG_SECTION_HELPERS,
SVN_CONFIG_OPTION_DIFF3_CMD, NULL);
}
bool bRet = Diff(path1, revision1, path2, revision2,
relativeToDir, depth, ignoreancestry, nodiffadded, nodiffdeleted,
showCopiesAsAdds, ignorecontenttype, useGitFormat, ign
Re: Review of invoke-diff-cmd-feature branch
On 6/10/13, Daniel Shahaf wrote: >> Index: subversion/include/svn_client.h >> === >> --- subversion/include/svn_client.h (revision 1484305) >> +++ subversion/include/svn_client.h (working copy) >> @@ -2988,8 +2988,8 @@ svn_client_blame(const char *path_or_url, >> * >> * @a invoke_diff_cmd is used to call an external diff program but may >> * not be @c NULL. The command line invocation will override the >> - * invoke-diff-cmd invocation entry(if any) in the Subversion >> - * configuration file. >> + * invoke-diff-cmd invocation entry(if any) in @c ctx->config. >> + * ### "May not be NULL" !? log-cmd.c and deprecated.c pass NULL for it. >> >> Hmm, what I was trying to communicate was that if a NULL(or "") is >> passed, >> this is an error that will be caught. I'm not quite sure what to write >> here now. >> >> The deprecated function is guaranteed to not have an execution path to >> invoke-diff-cmd but the log-cmd.c has been fixed. >> > > Can you explain that, please? I've added --invoke-diff-cmd to log-cmd.c so it's available in the same way as --diff-cmd. Example: svn log --diff --invoke-diff-cmd="diff -y %f1% %f2%" -r 1484733 > Existing API users call svn_client_diff6(). API compatibility rules > provide that if they run their code against libsvn_client compiled from > yoru branch, it will continue to work as it has. Does your branch > behave that way? I think it does. Because if you still use the deprecated function, you can't use the invoke-diff-cmd because it does not yet exist in your world. > > If yes, your docstring is wrong. If not, you have a bug. I see your point. I'll change the text in a new patch to the following: * @a invoke_diff_cmd takes an argument which is used to call an * external diff program. When invoked, the argument may not be @c * NULL or the empty string "". The command line invocation will * override the invoke-diff-cmd invocation entry (if any) in the * Subversion configuration file. Is that passable? > >> -- >> >> --- subversion/libsvn_subr/config_file.c (revision 1484305) >> +++ subversion/libsvn_subr/config_file.c (working copy) >> @@ -1086,6 +1086,9 @@ svn_config_ensure(const char *config_dir, apr_pool >> "# diff3-has-program-arg = [yes | no]" NL >> "### Set invoke-diff-cmd to the absolute path of your 'diff'" NL >> "### program." NL >> + /* ### Document how the setting may contain argv too, >> + not only argv[0] */ >> "### This will override the compile-time default, which is to >> use" NL >> "### Subversion's internal diff implementation." NL >> "# invoke-diff-cmd = \"diff -y --label %l1% %f1% --label %l2% >> %f2%\""NL >> Index: subversion/libsvn_subr/io.c >> + /* ### Document how the setting may contain argv too, >> + not only argv[0] */ >> >> Not sure what you mean here? >> > > When a program receives a parameter which is executed as a command, > there are three common ways to parse that parameter: (a) as a shell > command to be passed to system(); (b) as the command name (the first > parameter to execv(), either absolute or in PATH; (c) as a list of > strings, a la execv(). I was requesting that you document which how the > argument is used. > How about:(in config_file.c) "### Set invoke-diff-cmd to the absolute path of your 'diff'"NL "### program and provide a command string with substitutions, which" NL "### is passed in execv() style." NL ^^ "### This will override the compile-time default, which is to use" NL "### Subversion's internal diff implementation." NL "### Substitutions: %f1% original file"NL "###%f2% changed file" NL "###%l1% label of the original file" NL "###%l2% label of the changed file" NL "### Examples: --invoke-diff-cmd=\"diff -y %f1% %f2%\""NL "### --invoke-diff-cmd=\"kdiff3 -auto -o /home/u/log \\" NL "### +%f1% %f2% --L1 %l1% --L2 \"Custom Label\" \"" NL "### The switch symbol '%' can be escaped in the usual way" NL "### using the '%' character: %%f1% will be passed as %f1%." NL > (I think in this case the answer is "it is split to words using a > hand-rolled implementating of splitting". The fact that that answer is > effectively "it is parsed using some locally-rolled parsing rules" is a > separate problem which I commented about below.) > >> -- >> >> + /* This reimplements "split a string into argv". Is there an existing >> + svn/apr function that does that can be reused here? (We might gain >> + an "escape spaces" functionality this way.) */ >>tmp = svn_cstring_split(cmd," ",TRUE, subpool); >> >> I didn't find one, but p
Re: Review of invoke-diff-cmd-feature branch
Hi, I hope I've resolved most issues, here are ones I need to ask about: Index: subversion/include/svn_client.h === --- subversion/include/svn_client.h (revision 1484305) +++ subversion/include/svn_client.h (working copy) @@ -2988,8 +2988,8 @@ svn_client_blame(const char *path_or_url, * * @a invoke_diff_cmd is used to call an external diff program but may * not be @c NULL. The command line invocation will override the - * invoke-diff-cmd invocation entry(if any) in the Subversion - * configuration file. + * invoke-diff-cmd invocation entry(if any) in @c ctx->config. + * ### "May not be NULL" !? log-cmd.c and deprecated.c pass NULL for it. Hmm, what I was trying to communicate was that if a NULL(or "") is passed, this is an error that will be caught. I'm not quite sure what to write here now. The deprecated function is guaranteed to not have an execution path to invoke-diff-cmd but the log-cmd.c has been fixed. -- --- subversion/libsvn_subr/config_file.c (revision 1484305) +++ subversion/libsvn_subr/config_file.c (working copy) @@ -1086,6 +1086,9 @@ svn_config_ensure(const char *config_dir, apr_pool "# diff3-has-program-arg = [yes | no]" NL "### Set invoke-diff-cmd to the absolute path of your 'diff'" NL "### program." NL + /* ### Document the replaceables */ (done) + /* ### Document how the setting may contain argv too, + not only argv[0] */ "### This will override the compile-time default, which is to use" NL "### Subversion's internal diff implementation." NL "# invoke-diff-cmd = \"diff -y --label %l1% %f1% --label %l2% %f2%\""NL Index: subversion/libsvn_subr/io.c + /* ### Document how the setting may contain argv too, + not only argv[0] */ Not sure what you mean here? -- + /* This reimplements "split a string into argv". Is there an existing + svn/apr function that does that can be reused here? (We might gain + an "escape spaces" functionality this way.) */ tmp = svn_cstring_split(cmd," ",TRUE, subpool); I didn't find one, but perhaps I missed it? -- + How does this parse "%%%f1%"? Is "%%f1%%" an error? %%%f1% becomes %%f1% and %%f1%% becomes %f1%%, neither is an error. However, %f1%% is parsed out as sub%, also, +%f1% ends up as +sub. What you cannot do is: %%f1% to get %sub, it will render as %f1%. If this is a show stopper, we could go back to the triple dash model and not deal with escaping %'s, or choose another delimiter. + (The answers to both of these questions should have been + decided prior to coding, and I recall a list thread but I + don't recall that thread reached consensus on a specific + escaping UI.) Has consensus on the UI implemented here + been reached? */ I'm not sure, what do others think? -- @@ -3025,6 +3040,7 @@ svn_io_run_external_diff(const char *dir, if (pexitcode == NULL) pexitcode = &exitcode; + /* if invoke_diff_cmd is "", cmd[0] would be NULL? */ SVN_ERR(svn_io_run_cmd(dir, cmd[0], cmd, pexitcode, NULL, TRUE, NULL, outfile, errfile, pool)); I check for this condition before it reaches there. Neither "" nor NULL value for invoke_diff_cmd is accepted, but an error is raised before it reaches svn_io_create_custom_diff_cmd().
Re: Review of invoke-diff-cmd-feature branch
Because there is no option object that we can query from within
io.c, diff.c and other places, we have to add extra courier
parameters for every command line option we wish to add, in a
number of places.
We have 4 (potential) diff commands -- diff-cmd, diff3-cmd,
invoke-diff-cmd and invoke-diff3-cmd.
That's a lot of courier parameters, and all of them are not
neccessary to correct execution until the very end of the
process.
So, it would make sense to keep the process ignorant of what kind
of diff is required, until we need to know which one in the very
last stage.
Idea 1:
Make a command line options object we can query from anywhere
where it's needed, so we don't need any courier variables.
Idea 2:
Add a marker in front of a generic 'diff' option on the command line
which is passed as the only courier parameter that tells the inner
workings what exactly to work on.
ie: diff-cmd=diff would turn up in (say) svn_io_pick_diff_style() as
'diff-cmd-svn-optiondiff'.
In svn_io_pick_diff_style(), we check whether the marker string is
'diff-cmd-svn-optiondiff' or
'diff3-cmd-svn-optiondiff3' or
'invoke-diff-cmd-svn-optiondiffdiff %f1% ' or,
'invoke-diff3-cmd-svn-optiondiff %f1%'...'
snip the marker bit ('...svn-option') from the payload, and use that
info to redirect the flow towards the correct diff functions, or, even
collect all the diff functions we have into one that does the entire
thing in one place.
Not pretty, but, it saves us a lot of deprecation and extra code
which only adds to the overall complexity of the code base
without actually being all that functional.
Re: svn commit: r1484260 - /subversion/branches/invoke-diff-cmd-feature/subversion/libsvn_client/diff.c
On 19/05/13 17:48, Daniel Shahaf wrote:
On Sun, May 19, 2013 at 10:15:55AM -, g...@apache.org wrote:
Author: gbg
Date: Sun May 19 10:15:55 2013
New Revision: 1484260
URL: http://svn.apache.org/r1484260
Log:
Seperate variable declaration from assigment.
* subversion/libsvn_client/diff.c
(set_up_diff_cmd_and_options): Seperate variable declaration from assigment.
Modified:
subversion/branches/invoke-diff-cmd-feature/subversion/libsvn_client/diff.c
Modified:
subversion/branches/invoke-diff-cmd-feature/subversion/libsvn_client/diff.c
URL:
http://svn.apache.org/viewvc/subversion/branches/invoke-diff-cmd-feature/subversion/libsvn_client/diff.c?rev=1484260&r1=1484259&r2=1484260&view=diff
==
--- subversion/branches/invoke-diff-cmd-feature/subversion/libsvn_client/diff.c
(original)
+++ subversion/branches/invoke-diff-cmd-feature/subversion/libsvn_client/diff.c
Sun May 19 10:15:55 2013
@@ -2465,7 +2465,9 @@ set_up_diff_cmd_and_options(struct diff_
/* old style diff_cmd has precedence in config file */
if (config)
{
- svn_config_t *cfg = svn_hash_gets(config, SVN_CONFIG_CATEGORY_CONFIG);
+ svn_config_t *cfg;
+
+ cfg = svn_hash_gets(config, SVN_CONFIG_CATEGORY_CONFIG);
Why? This doesn't seem to serve any useful purpose (in fact, I think it makes
the code harder to read).
Because (if I understood this correctly!) the compiler builds more
efficient code and the startup time of the application is improved.
"To summarize, it is always preferable to add variables
as uninitialized or initialized with zero as opposed to as
initialized with a value other than zero."
See Page 16 here:
http://software.intel.com/sites/default/files/m/a/1/e/dsohowto.pdf
Re: Diff Project --invoke-diff-cmd part
Hi, I made a feature branch for the project here: https://svn.apache.org/repos/asf/subversion/branches/invoke-diff-cmd-feature/ Gabriela
Re: Diff Project --invoke-diff-cmd part
On 13/05/13 22:53, Gabriela Gibson wrote: Hi, thanks for all the comments and help, here is the next attempt. Gabriela Sorry I had one mistake in the patch that has now been fixed. Please ignore the patch in the previous post, the attached patch is the working one. Gabriela Index: subversion/include/svn_client.h === --- subversion/include/svn_client.h (revision 1480974) +++ subversion/include/svn_client.h (working copy) @@ -2986,6 +2986,11 @@ svn_client_blame(const char *path_or_url, * The above two options are mutually exclusive. It is an error to set * both to TRUE. * + * @a invoke_diff_cmd is used to call an external diff program but may + * not be @c NULL. The command line invocation will override the + * invoke-diff-cmd invocation entry(if any) in the Subversion + * configuration file. + * * Generated headers are encoded using @a header_encoding. * * Diff output will not be generated for binary files, unless @a @@ -3016,8 +3021,38 @@ svn_client_blame(const char *path_or_url, * @note @a relative_to_dir doesn't affect the path index generated by * external diff programs. * + * @since New in 1.9. + */ +svn_error_t * +svn_client_diff7(const apr_array_header_t *options, + const char *path_or_url1, + const svn_opt_revision_t *revision1, + const char *path_or_url2, + const svn_opt_revision_t *revision2, + const char *relative_to_dir, + svn_depth_t depth, + svn_boolean_t ignore_ancestry, + svn_boolean_t no_diff_added, + svn_boolean_t no_diff_deleted, + svn_boolean_t show_copies_as_adds, + svn_boolean_t ignore_content_type, + svn_boolean_t ignore_properties, + svn_boolean_t properties_only, + svn_boolean_t use_git_diff_format, + const char *header_encoding, + svn_stream_t *outstream, + svn_stream_t *errstream, + const apr_array_header_t *changelists, + const char *invoke_diff_cmd, + svn_client_ctx_t *ctx, + apr_pool_t *pool); + +/** Similar to svn_client_diff7(), but with @a invoke_diff_cmd. + * + * @deprecated Provided for backward compatibility with the 1.8 API. * @since New in 1.8. */ +SVN_DEPRECATED svn_error_t * svn_client_diff6(const apr_array_header_t *diff_options, const char *path_or_url1, @@ -3175,14 +3210,47 @@ svn_client_diff(const apr_array_header_t *diff_opt * be either a working-copy path or URL. * * If @a peg_revision is #svn_opt_revision_unspecified, behave - * identically to svn_client_diff6(), using @a path_or_url for both of that + * identically to svn_client_diff7(), using @a path_or_url for both of that * function's @a path_or_url1 and @a path_or_url2 arguments. * - * All other options are handled identically to svn_client_diff6(). + * All other options are handled identically to svn_client_diff7(). * - * @since New in 1.8. + * @since New in 1.9. */ svn_error_t * +svn_client_diff_peg7(const apr_array_header_t *diff_options, + const char *path_or_url, + const svn_opt_revision_t *peg_revision, + const svn_opt_revision_t *start_revision, + const svn_opt_revision_t *end_revision, + const char *relative_to_dir, + svn_depth_t depth, + svn_boolean_t ignore_ancestry, + svn_boolean_t no_diff_added, + svn_boolean_t no_diff_deleted, + svn_boolean_t show_copies_as_adds, + svn_boolean_t ignore_content_type, + svn_boolean_t ignore_properties, + svn_boolean_t properties_only, + svn_boolean_t use_git_diff_format, + const char *header_encoding, + svn_stream_t *outstream, + svn_stream_t *errstream, + const apr_array_header_t *changelists, + const char *invoke_diff_cmd, + svn_client_ctx_t *ctx, + apr_pool_t *pool); + + +/** Similar to svn_client_peg5(), but with @a no_diff_added set to + * FALSE, @a ignore_properties set to FALSE and @a properties_only + * set to false. + * + * @deprecated Provided for backward compatibility with the 1.7 API. + * @since New in 1.9. + */ +SVN_DEPRECATED +svn_error_t * svn_client_diff_peg6(const apr_array_header_t *diff_options, const char *path_or_url, const svn_opt_revision_t *peg_revision, Index: subversion/include/svn_config.h === --- subversion/include/svn_config.h
Re: Diff Project --invoke-diff-cmd part
Hi, thanks for all the comments and help, here is the next attempt. Gabriela [[[ Add new diff option "--invoke-diff-cmd" which allows the user to define a custom command line or config file entry for an external diff program. * subversion/include/svn_client.h (svn_client_diff7, svn_client_diff_peg7): Declare the new API. Like svn_client_diff[_peg]6 but with invoke_diff_cmd parameter. (svn_client_diff6, svn_client_diff_peg_6): Deprecate. * subversion/include/svn_config.h (SVN_CONFIG_OPTION_INVOKE_DIFF_CMD): New definition. * subversion/include/svn_io.h (svn_io_create_custom_diff_cmd): New function. (svn_io_run_external_diff): New function. * subversion/libsvn_client/deprecated.c (svn_client_diff6, svn_client_diff_peg6): New deprecation wrappers. * subversion/libsvn_client/diff.c (struct diff_cmd_baton): New member: 'invoke_diff_cmd'. (diff_content_changed): Call svn_io_run_external_diff if --invoke-diff-cmd option was specified, otherwise retain previous behaviour. (set_up_diff_cmd_and_options): Apply invoke-diff-cmd option preferentially. Old behavior unchanged. (svn_client_diff_peg_7): Rename and update from svn_client_diff_peg_6. Add new parameter: invoke_diff_cmd. (svn_client_diff7): Rename and update from svn_client_diff6, add new parameter 'invoke_diff_cmd'. (): Update all comments mentioning 'svn_client_diff6' to 'svn_client_diff7'. * subversion/libsvn_subr/config_file.c (svn_config_ensure): New entry: invoke-diff-cmd. * subversion/libsvn_subr/io.c (svn_io_create_custom_diff_cmd): New function. (svn_io_run_external_diff): New function. * subversion/svn/cl.h (struct svn_cl__opt_state_t.diff): New member: 'invoke_diff_cmd'. * subversion/svn/diff-cmd.c (svn_cl__diff): Update call to svn_client_diff6 to svn_client_diff7. * subversion/svn/svn.c (svn_cl__options[]): Add help info and new variable: 'opt_invoke_diff_cmd'. (svn_cl__cmd_table[]): New option: 'invoke-diff-cmd'. (sub_main): Prohibit simultaneous usage of --invoke-diff-cmd and --internal-diff. Add new opt_state.diff.invoke-diff-cmd option to the option selector. Add call to svn_config_set. * subversion/tests/cmdline/diff_tests.py (diff_invoke_external_diffcmd): New function. (test_list): Add new entry 'diff_invoke_external_diffcmd'. * tools/hook-scripts/argv_dump.pl (New File): Perl script that enumerates output by svn to assist with testing. ]]] Index: subversion/include/svn_client.h === --- subversion/include/svn_client.h (revision 1480974) +++ subversion/include/svn_client.h (working copy) @@ -2986,6 +2986,11 @@ svn_client_blame(const char *path_or_url, * The above two options are mutually exclusive. It is an error to set * both to TRUE. * + * @a invoke_diff_cmd is used to call an external diff program but may + * not be @c NULL. The command line invocation will override the + * invoke-diff-cmd invocation entry(if any) in the Subversion + * configuration file. + * * Generated headers are encoded using @a header_encoding. * * Diff output will not be generated for binary files, unless @a @@ -3016,8 +3021,38 @@ svn_client_blame(const char *path_or_url, * @note @a relative_to_dir doesn't affect the path index generated by * external diff programs. * + * @since New in 1.9. + */ +svn_error_t * +svn_client_diff7(const apr_array_header_t *options, + const char *path_or_url1, + const svn_opt_revision_t *revision1, + const char *path_or_url2, + const svn_opt_revision_t *revision2, + const char *relative_to_dir, + svn_depth_t depth, + svn_boolean_t ignore_ancestry, + svn_boolean_t no_diff_added, + svn_boolean_t no_diff_deleted, + svn_boolean_t show_copies_as_adds, + svn_boolean_t ignore_content_type, + svn_boolean_t ignore_properties, + svn_boolean_t properties_only, + svn_boolean_t use_git_diff_format, + const char *header_encoding, + svn_stream_t *outstream, + svn_stream_t *errstream, + const apr_array_header_t *changelists, + const char *invoke_diff_cmd, + svn_client_ctx_t *ctx, + apr_pool_t *pool); + +/** Similar to svn_client_diff7(), but with @a invoke_diff_cmd. + * + * @deprecated Provided for backward compatibility with the 1.8 API. * @since New in 1.8. */ +SVN_DEPRECATED svn_error_t * svn_client_diff6(const apr_array_header_t *diff_options, const char *path_or_url1, @@ -3175,14 +3210,47 @@ svn_client_diff(const apr_array_header_t *diff_opt * be either a working-copy path or URL. * * If @a peg_revision is #svn_opt_revision_unspecified, behave - * identically to
--invoke-diff-cmd test suite woes
Hi,
All my manual tests for the current --invoke-diff-cmd work fine, but I
cannot get the test suite to work.
I added the following new test (nr 49) to diff_tests.py:
[[[
# Check the order of the arguments for an external diff tool
def diff_invoke_external_diffcmd(sbox):
"svn diff --diff-invoke-cmd has correct arguments"
sbox.build(read_only = True)
os.chdir(sbox.wc_dir)
iota_path = 'iota'
svntest.main.file_append(iota_path, "new text in iota")
expected_output = svntest.verify.ExpectedOutput([
"Index: iota\n",
"===\n",
"--- l1\n",
"+++ l2\n",
"@@ -1 +1,2 @@\n",
"This is the file 'iota'.\n",
"+new text in iota\n",
"\ No newline at end of file\n",
# os.path.abspath(svntest.wc.text_base_path("iota")) + "\n",
#os.path.abspath("iota") + "\n
])
# Check that the output of diff corresponds with the expected arguments,
# in the correct order.
svntest.actions.run_and_verify_svn(None, expected_output, [],
'diff', '--invoke-diff-cmd="diff
-u --label l1 %f1% --label l2 %f2%', "",
iota_path)
]]]
If I now run this test, svn diff correctly expands the command (as
expected, see the line starting with "W: svn: E200012:"), but svn
crashes (the test fails too, but that is I think a separate problem)
g@musashi:~/trunk_ziff/subversion/tests/cmdline$ ./diff_tests.py 49
W: subversion/svn/diff-cmd.c:412,
W: subversion/libsvn_client/diff.c:2130,
W: subversion/libsvn_client/diff.c:1654,
W: subversion/libsvn_wc/diff_local.c:500,
W: subversion/libsvn_wc/status.c:2758,
W: subversion/libsvn_wc/status.c:1460,
W: subversion/libsvn_wc/status.c:1212,
W: subversion/libsvn_wc/status.c:931,
W: subversion/libsvn_wc/diff_local.c:364,
W: subversion/libsvn_wc/diff_editor.c:568,
W: subversion/libsvn_diff/diff_tree.c:1208,
W: subversion/libsvn_wc/diff_editor.c:2708,
W: subversion/libsvn_client/diff.c:975,
W: subversion/libsvn_client/diff.c:839,
W: subversion/libsvn_subr/io.c:3040: (apr_err=SVN_ERR_EXTERNAL_PROGRAM)
W: svn: E200012: '"diff -u --label l1 %f1% --label l2 %f2%' was expanded
to "diff -u --label l1
/home/g/trunk_ziff/subversion/tests/cmdline/svn-test-work/working_copies/diff_tests-49/.svn/pristine/2c/2c0aa9014a0cd07f01795a333d82485ef6d083e2.svn-base
--label l2
/home/g/trunk_ziff/subversion/tests/cmdline/svn-test-work/working_copies/diff_tests-49/iota
and returned 255
W: CWD:
/home/g/trunk_ziff/subversion/tests/cmdline/svn-test-work/working_copies/diff_tests-49
W: EXCEPTION: Failure: Command failed:
"/home/g/trunk_ziff/subversion/svn/svn diff --invoke-diff-cmd="diff -u
--label l1 %f1% --label l2 %f2% ..."; exit code 1
Traceback (most recent call last):
File "/home/g/trunk_ziff/subversion/tests/cmdline/svntest/main.py",
line 1550, in run
rc = self.pred.run(sandbox)
File
"/home/g/trunk_ziff/subversion/tests/cmdline/svntest/testcase.py", line
176, in run
return self.func(sandbox)
File "./diff_tests.py", line 3304, in diff_invoke_external_diffcmd
iota_path)
File
"/home/g/trunk_ziff/subversion/tests/cmdline/svntest/actions.py", line
282, in run_and_verify_svn
expected_exit, *varargs)
File
"/home/g/trunk_ziff/subversion/tests/cmdline/svntest/actions.py", line
320, in run_and_verify_svn2
exit_code, out, err = main.run_svn(want_err, *varargs)
File "/home/g/trunk_ziff/subversion/tests/cmdline/svntest/main.py",
line 682, in run_svn
*(_with_auth(_with_config_dir(varargs
File "/home/g/trunk_ziff/subversion/tests/cmdline/svntest/main.py",
line 365, in run_command
None, *varargs)
File "/home/g/trunk_ziff/subversion/tests/cmdline/svntest/main.py",
line 557, in run_command_stdin
'"; exit code ' + str(exit_code))
Failure: Command failed: "/home/g/trunk_ziff/subversion/svn/svn diff
--invoke-diff-cmd="diff -u --label l1 %f1% --label l2 %f2% ..."; exit
code 1
FAIL: diff_tests.py 49: svn diff --diff-invoke-cmd has correct arguments
g@musashi:~/trunk_ziff/subversion/tests/cmdline$
Running the rejected output without svn works:
g@musashi:~/trunk_ziff/subversion/tests/cmdline$ diff -u --label l1
/home/g/trunk_ziff/subversion/tests/cmdline/svn-test-work/working_copies/diff_tests-49/.svn/pristine/2c/2c0aa9014a0cd07f01795a333d82485ef6d083e2.svn-base
--label l2
/home/g/trunk_ziff/subversion/tests/cmdline/svn-test-work/working_copies/diff_tests-49/iota
--- l1
+++ l2
@@ -1 +1,2 @@
This is the file 'iota'.
+new text in iota
\ No newline at end of file
g@musashi:~/trunk_ziff/subversion/tests/cmdline$
So does running svn with the rejected output without the test suite:
g@musashi:~/trunk_ziff/subversion/tests/cmdline$
/home/g/trunk_ziff/
[PATCH] HACKING GUIDE entry for SVN_DBG usage in the Debugging Subversion section
Hi,
I created a short SVN_DBG overview for the debugging page in the
HACKING GUIDE.
Please let me know if this can be improved.
Gabriela
[[[
Add section describing usage of the SVN_DBG macro to the Community Guide
page 'Debugging Subversion'.
* publish/docs/community-guide/debugging.part.html
(svn_dbg): Provide overview, links, hints and sample patches.
* publish/docs/community-guide/debugging.toc.html
(): Add link to section svn_dbg.
Suggested by: danielsh
]]]
(Just the raw text for ease of reading)
---
Debugging with SVN_DBG
The SVN_DBG debugging tool is a C Preprocessor macro that sends
debugging output to stdout (by default) or stderr whilst not
interfering with the SVN test suite.
It provides an alternative to a debugger such as gdb, or
alternatively, extra information to assist in the use of a
debugger. It might be especially useful in situations where a
debugger cannot be used.
The svn_debug module contains two debug aid macros that print the
file:line of the call and printf-like arguments to the
#SVN_DBG_OUTPUT stdio stream (#stdout by default):
SVN_DBG( ( const char *fmt, ...) ) /* double braces are neccessary */
and
SVN_DBG_PROPS( ( apr_hash_t *props, const char *header_fmt, ...) )
Controlling SVN_DBG output:
* SVN_DBG is enabled whenever svn is configured with
--enable-maintainer-mode.
* The SVN test suite turns off SVN_DBG output automatically, to
suppress the output manually, set the SVN_DBG_QUIET variable
to 1 in your shell environment.
* When you are done, please be sure to remove any instances of
the SVN_DBG and SVN_DBG_PROPS macros from any code you are
committing and from any patch that you send to the
list. (AKA: Do not forget a scalpel in the patient!)
The SVN_DBG macro definitions and code are located in:
* subversion/include/private/svn_debug.h
* subversion/libsvn_subr/debug.c
Sample patch showing usage of the SVN_DBG macro:
Index: subversion/libsvn_fs_fs/fs_fs.c
===
--- subversion/libsvn_fs_fs/fs_fs.c (revision 1476635)
+++ subversion/libsvn_fs_fs/fs_fs.c (working copy)
@@ -2303,6 +2303,9 @@ get_node_revision_body(node_revision_t **noderev_p
/* First, try a cache lookup. If that succeeds, we are done here. */
SVN_ERR(get_cached_node_revision_body(noderev_p, fs, id, &is_cached,
pool));
+ SVN_DBG(("Getting %s from: %s\n",
+ svn_fs_fs__id_unparse(id),
+ is_cached ? "cache" : "disk"));
if (is_cached)
return SVN_NO_ERROR;
Sample patch showing usage of the SVN_DBG_PROPS macro:
Index: subversion/svn/proplist-cmd.c
===
--- subversion/svn/proplist-cmd.c(revision 1475745)
+++ subversion/svn/proplist-cmd.c(working copy)
@@ -221,6 +221,7 @@ svn_cl__proplist(apr_getopt_t *os,
URL, &(opt_state->start_revision),
&rev, ctx, scratch_pool));
+ /* this can be called with svn proplist --revprop -r */
+ SVN_DBG_PROPS((proplist,"The variable apr_hash_t *proplist
contains: "));
if (opt_state->xml)
{
svn_stringbuf_t *sb = NULL;
Add section describing usage of the SVN_DBG macro to the Community Guide page
'Debugging SVN'.
* publish/docs/community-guide/debugging.part.html
(svn_dbg): Provide overview, links, hints and sample patches.
* publish/docs/community-guide/debugging.toc.html
(): Add link to section svn_dbg.
Suggested by: danielshIndex: publish/docs/community-guide/debugging.part.html
===
--- publish/docs/community-guide/debugging.part.html (revision 1479254)
+++ publish/docs/community-guide/debugging.part.html (working copy)
@@ -6,8 +6,95 @@
-TODO: document SVN_DBG
+
+Debugging with SVN_DBG
+ #debugging-svn_debug"
+title="Link to this section">¶
+
+The SVN_DBG debugging tool is a C Preprocessor macro that sends
+debugging output to stdout (by default) or stderr whilst not interfering
+with the SVN test suite.
+
+It provides an alternative to a debugger such as gdb, or
+alternatively, extra information to assist in the use of a debugger. It
+might be especially useful in situations where a debugger cannot be
+used.
+
+The svn_debug module contains two debug aid macros that print the
+file:line of the call and printf-like arguments to
+the #SVN_DBG_OUTPUT stdio stream (#stdout by
+default):
+
+
+SVN_DBG( ( const char *fmt, ...) ) /* double braces are neccessary */
+
+and
+
+SVN_DBG_PROPS( ( apr_hash_t *props, const char *header_fmt, ...) )
+
+
+Controlling SVN_DBG output:
+
+SVN_DBG is enabled whenever svn is configured
+with --enable-maintainer-mode.
+
+The SVN test suite turns off SVN_DBG output automatically, to
+suppress the output manually, set the SVN_DBG_QUIET
+variable to 1 in your shell enviro
Re: [PATCH] get-deps.sh zlib version number and file type changed
On 30/04/13 18:27, Ben Reser wrote: 1) Need to avoid using GNU tar options on this. Just like we don't use the j option to tar to deal with bzip2 we can't use the z option to deal with gzip. Should this note be a comment in get-deps.sh?
[PATCH] get-deps.sh zlib version number and file type changed
[[[
Update the zlib HTTP link to fetch http://zlib.net/zlib-1.2.8.tar.gz
* subversion/build/get-deps.sh:
(): Update zlib version number.
(get_zlib): Modify tar command and file extension.
]]]
Index: get-deps.sh
===
--- get-deps.sh (revision 1477638)
+++ get-deps.sh (working copy)
@@ -26,7 +26,7 @@
APR=apr-1.4.6
APR_UTIL=apr-util-1.5.1
SERF=serf-1.2.0
-ZLIB=zlib-1.2.7
+ZLIB=zlib-1.2.8
SQLITE_VERSION=3.7.15.1
SQLITE_VERSION_LIST=`echo $SQLITE_VERSION | sed -e 's/\./ /g'`
SQLITE=sqlite-amalgamation-`printf %d%02d%02d%02d $SQLITE_VERSION_LIST`
@@ -86,10 +86,10 @@ get_zlib() {
test -d $BASEDIR/zlib && return
cd $TEMPDIR
-$HTTP_FETCH http://www.zlib.net/$ZLIB.tar.bz2
+$HTTP_FETCH http://www.zlib.net/$ZLIB.tar.gz
cd $BASEDIR
-bzip2 -dc $TEMPDIR/$ZLIB.tar.bz2 | tar -xf -
+tar xfz $TEMPDIR/$ZLIB.tar.gz
mv $ZLIB zlib
}
Re: Diff Project --invoke-diff-cmd part
Sorry I forgot to explain the changed syntax. Here is the help section: --invoke-diff-cmd ARG: use ARG as format string for external diff command invocation. Substitutions: %f1 %f2 files to compare %l1 %l2 user defined labels Examples: --invoke-diff-cmd="diff -y %f1 %f2 --invoke-diff-cmd="kdiff3 -auto -o /home/u/log \ %f1 %f2 --L1 %l1 --L2 "Custom Label" " The switch symbol '%' can be modified by defining an alternative symbol string, starting with '#','$' or '-' Example: --invoke-diff-cmd="--- diff -y ---f1 ---f2"
Re: Diff Project --invoke-diff-cmd part
Take two. Thanks for looking! Gabriela [[[ Add new diff option "--invoke-diff-cmd" which allows the user to define a custom command line or config file entry for an external diff program. * subversion/include/svn_client.h (svn_client_diff6): Deprecate. Add new Doxygen comment. (svn_client_diff7): Add new Doxygen comment. (svn_client_diff7): New function. (svn_client_diff_peg_6): Deprecate. Refresh Doxygen comment. (svn_client_diff_peg_7): New function. * subversion/include/svn_config.h (SVN_CONFIG_OPTION_INVOKE_DIFF_CMD): New definition. * subversion/include/svn_io.h (svn_io_create_custom_diff_cmd): New function. (svn_io_run_external_diff): New function. * subversion/libsvn_client/deprecated.c (svn_client_diff6): Deprecate. (svn_client_diff_peg6): Deprecate. * subversion/libsvn_client/diff.c (struct diff_cmd_baton): New variable. (diff_content_changed): Call svn_io_run_external_diff if --invoke-diff-cmd option was specied, otherwise retain previous behaviour. (set_up_diff_cmd_and_options): Apply invoke-diff-cmd option preferentially. Old behavior unchanged. (svn_client_diff_peg_7): New function. (svn_client_diff6): Deprecate. (svn_client_diff7): New function. * subversion/libsvn_subr/config_file.c (svn_config_ensure): Add help text. * subversion/libsvn_subr/io.c (svn_io_create_custom_diff_cmd): New function. (svn_io_run_external_diff): New function. * subversion/svn/cl.h (struct svn_cl__opt_state_t): New struct member. (struct svn_cl__opt_state_t.diff): New struct member. * subversion/svn/diff-cmd.c (svn_cl__diff): Modify call to deprecated function. * subversion/svn/svn.c (svn_cl__options[]): New variable and help info. (svn_cl__cmd_table[]): New variable. (sub_main): Add exclusiveness condition. Expand if condition. ]]] Index: subversion/include/svn_client.h === --- subversion/include/svn_client.h (revision 1475745) +++ subversion/include/svn_client.h (working copy) @@ -2986,6 +2986,9 @@ svn_client_blame(const char *path_or_url, * The above two options are mutually exclusive. It is an error to set * both to TRUE. * + * If @a invoke_diff_cmd is non-null, invoke external diff command + * with the string it contains. + * * Generated headers are encoded using @a header_encoding. * * Diff output will not be generated for binary files, unless @a @@ -3015,9 +3018,39 @@ svn_client_blame(const char *path_or_url, * * @note @a relative_to_dir doesn't affect the path index generated by * external diff programs. + * + * @since New in 1.8. + */ +svn_error_t * +svn_client_diff7(const apr_array_header_t *options, + const char *path_or_url1, + const svn_opt_revision_t *revision1, + const char *path_or_url2, + const svn_opt_revision_t *revision2, + const char *relative_to_dir, + svn_depth_t depth, + svn_boolean_t ignore_ancestry, + svn_boolean_t no_diff_added, + svn_boolean_t no_diff_deleted, + svn_boolean_t show_copies_as_adds, + svn_boolean_t ignore_content_type, + svn_boolean_t ignore_properties, + svn_boolean_t properties_only, + svn_boolean_t use_git_diff_format, + const char *header_encoding, + svn_stream_t *outstream, + svn_stream_t *errstream, + const apr_array_header_t *changelists, + svn_client_ctx_t *ctx, + apr_pool_t *pool, + const char *invoke_diff_cmd); + +/** Similar to svn_client_diff7(), but with @a invoke_diff_cmd. * + * @deprecated Provided for backward compatibility with the 1.8 API. * @since New in 1.8. */ +SVN_DEPRECATED svn_error_t * svn_client_diff6(const apr_array_header_t *diff_options, const char *path_or_url1, @@ -3175,14 +3208,46 @@ svn_client_diff(const apr_array_header_t *diff_opt * be either a working-copy path or URL. * * If @a peg_revision is #svn_opt_revision_unspecified, behave - * identically to svn_client_diff6(), using @a path_or_url for both of that + * identically to svn_client_diff7(), using @a path_or_url for both of that * function's @a path_or_url1 and @a path_or_url2 arguments. * - * All other options are handled identically to svn_client_diff6(). + * All other options are handled identically to svn_client_diff7(). * * @since New in 1.8. */ svn_error_t * +svn_client_diff_peg7(const apr_array_header_t *diff_options, + const char *path_or_url, + const svn_opt_revision_t *peg_revision, + const svn_opt_revision_t *start_revision, + const svn_opt_revision_t *end_revision, + const char *relative_to_dir, +
Re: Diff Project --invoke-diff-cmd part
On 28/04/13 10:24, Daniel Shahaf wrote:
Alan Barrett wrote on Wed, Apr 24, 2013 at 09:44:11 +0200:
On Tue, 23 Apr 2013, Gabriela Gibson wrote:
Also, a minor design nit (sorry, no code review): The "---f1"
construct is something I've never seen before.
That's why I picked it --- I checked extensively, and no-one uses a
triple dash, so it does exactly what we hope it will: never interfere
with anything that people might do. Also, I think it looks quite
'unixy' and it's easy to read. I expect fewer problems on windows.
Speaking as somebody who might use this feature, I would much prefer a
more familiar notation like "%(f1)". To my eyes, "---f1" does not look
unixy or easy to read; familiar constructs are easier to read than
unfamiliar constructs.
In addition to the familiarity issue, there's an issue with escapes: you
need a way of representing a literal "---f1" sequence that does not
expand to anything. With notation like "%(f1)" there's already a
widespread convention of using "%%" to represent a "%" character that
does not introduce an expnsion.
True. However, both cmd.exe and the Subversion config file parser use
'%' as a metacharacter, and each of them escapes it differently, so the
way to generate a single, literal '%' character would be:
in ~/.subversion/config;
%% in the value of --config-option=... on unix;
^%^%in the value of --config-option=... on windows;
% not guaranteed to, but works in practice when not followed by either
/[(]/ or /\w+[%]/.
That's going to be challenging to document clearly.
What about $(f1)? That's also familiar (makefile syntax) but might be
a little saner to escape in various contexts.
Daniel
P.S. It appears our config files' %-interpolation feature doesn't kick
in for --config-option's argument. I'm not sure whether that's a good
thing or not.
Hi,
I meant to send the patch tonight and then, I realised I forgot to add
a feature that Julian Foad suggested.
However, I have a cunning plan to address the issue that Alan raised,
he has a good point that for plain usage, uniformity and conformity is
desirable.
I hope this idea will fix the issues danielsh raised too -- might need
some polishing, then again, it might not be as useful as I hope it is.
Here is the current svn_io_create_custom_diff_cmd: (the guts will
change tomorrow, but it will do the same thing will stuff like +---f1
working added on.)
Current svn help diff output (for unix) is: (adjusted for email fit)
--invoke-diff-cmd ARG:
use ARG as format string for external diff command invocation.
Substitutions: %f1 %f2 files to compare
%l1 %l2 user defined labels
Examples: --invoke-diff-cmd="diff -y %f1 %f2
--invoke-diff-cmd="kdiff3 -auto -o /home/u/log \
%f1 %f2 --L1 %l1 --L2 "Custom Label"
The switch symbol '%' can be modified by defining an
alternative symbol string, starting with a non-alpha
numeric character. Example:
--invoke-diff-cmd="--- diff -y ---f1 ---f2
[[[
const char **
svn_io_create_custom_diff_cmd(const char *label1,
const char *label2,
const char *label3,
const char *tmpfile1,
const char *tmpfile2,
const char *base,
const char *cmd,
apr_pool_t *pool)
{
apr_array_header_t * external_diff_cmd, *delimiters;
const char ** ret;
char * delimiter_prefix;
char * default_delimiter_prefix;
int has_custom_delimiter, len, i, j;
apr_pool_t *subpool;
subpool = svn_pool_create(pool);
#ifdef WIN32
default_delimiter_prefix = apr_pstrdup(subpool, "---");
else
default_delimiter_prefix = apr_pstrdup(subpool, "%");
#endif
external_diff_cmd = svn_cstring_split(cmd, " ", FALSE, subpool);
if (cmd[0] > 33 && cmd[0] < 43) /* user requests custom delimiter
prefix */
{
delimiter_prefix = APR_ARRAY_IDX(external_diff_cmd,0, char *);
has_custom_delimiter = 1;
}
else
{
delimiter_prefix = apr_pstrdup(subpool, default_delimiter_prefix);
has_custom_delimiter = 0;
}
delimiters = svn_cstring_split("f1 f2 f3 l1 l2 l3", " ", TRUE, subpool);
{ /* add the selected prefix to the delimiters */
char *s;
s = apr_pcalloc(subpool, (1+
strlen(delimiter_prefix)+
delimiters->nelts)*sizeof(char *));
for (i = 0; i < delimiters->nelts; i++)
{
strcat(s, delimiter_prefix);
strcat(s, APR_ARRAY_IDX(delimiters, i, char *));
strcat(s," ");
}
delimiters = svn_cstring_split(s, " ", FALSE, subpool);
}
[PATCH] Doxygen documentation change for svn_cstring_split() in svn_string.h
Danielsh suggested on IRC that I clarify the API documentation for the
function svn_cstring_split().
[[[
Clarify the doxygen documentation for the semantics of the @a sep_chars
parameter.
* subversion/include/svn_string.h
(svn_cstring_split): Update doxygen comment.
Suggested by: Danielsh
]]]
Index: subversion/include/svn_string.h
===
--- subversion/include/svn_string.h (revision 1471133)
+++ subversion/include/svn_string.h (working copy)
@@ -407,15 +407,17 @@ svn_string_compare_stringbuf(const svn_string_t *s
* @{
*/
-/** Divide @a input into substrings along @a sep_chars boundaries, return an
- * array of copies of those substrings (plain const char*), allocating both
- * the array and the copies in @a pool.
+/** Divide @a input into substrings, interpreting any char from @a sep
+ * as a token separator.
*
- * None of the elements added to the array contain any of the
- * characters in @a sep_chars, and none of the new elements are empty
- * (thus, it is possible that the returned array will have length
- * zero).
+ * Return an array of copies of those substrings (plain const char*),
+ * allocating both the array and the copies in @a pool.
*
+ * Because none of the elements added to the resulting array contain
+ * any of the characters in @a sep_chars, and none of the new elements
+ * are empty, it is possible that the returned array will have length
+ * zero.
+ *
* If @a chop_whitespace is TRUE, then remove leading and trailing
* whitespace from the returned strings.
*/
[PATCH] Fix for --internal-diff cmd coredump problem
[[[
Fix --internal-diff coredump due to NULL pointer in string routine.
* subversion/libsvn_subr/config.c
(make_string_from_option): Add test to ensure that the string passed
to strcmp is not a NULL value.
]]]
Index: subversion/libsvn_subr/config.c
===
--- subversion/libsvn_subr/config.c (revision 1465268)
+++ subversion/libsvn_subr/config.c (working copy)
@@ -464,7 +464,7 @@ make_string_from_option(const char **valuep, svn_c
/* before attempting to expand an option, check for the placeholder.
* If none is there, there is no point in calling expand_option_value.
*/
- if (strchr(opt->value, '%'))
+ if (opt->value && strchr(opt->value, '%'))
{
apr_pool_t *tmp_pool = (x_pool ? x_pool : svn_pool_create(cfg->x_pool));
Re: Diff Project --invoke-diff-cmd part
Many thanks for the inspiring feedback! Gabriela Julian Foad wrote: >> * This patch breaks the override --internal-diff for now, because >> this part has to be revised anyway when the invoke-diff3-cmd part >> gets added. > OK, we'll have to decide what should override what. I have not formed > any opinion on that yet. Personally I feel that if you define an option on the command line, it should always override any choice made in the config file for convenience and flexibility. Also, I think the --internal-diff-or-merge should turn off whatever diff/merge arrangements the config file specifies(let's leave the old command --internal-diff untouched), especially if we add the following feature: Paul Watson mentioned on the user list: http://mail-archives.apache.org/mod_mbox/subversion-users/201304.mbox/%3CDB6B7C8F18A6884EB20D085DA247FBC51779663A%40EVS1.phs.org%3E that he would like to use mime-types to define what diff program to use for certain files. (note M. Pilatos' RFC in the thread) Should the config file store pair values, mime-type and the invoke-diff(n)-cmd to be used? New command line design example: invoke-diff-cmd="---MIME-TYPE=custom_mime_type diffProg ---f1 ---MIME-TYPE=default=diff " or, if you just want the diff program you select to be applied to any mime-type you define: invoke-diff-cmd="diff ---f1 ..." The corresponding config file entries would look like: INVOKE_DIFF_MIME_TYPE_1=myCustomDiffType INVOKE_DIFF_MIME_TYPE_1_CMD=myDiffProg ---f1 ... INVOKE_DIFF_MIME_TYPE_2=default INVOKE_DIFF_MIME_TYPE_2_CMD=myFavoriteDiff ---f1 ... ... INVOKE_MERGE_MIME_TYPE_1=myCustomMergeType INVOKE_MERGE_MIME_TYPE_1_CMD=myMergeProg ---f1 ... I wasn't sure if this kind of dev talk belongs on the user list, so I CC'ed Paul Watson for now. Let me know if this should be posted to the existing thread in users as well. > I think it would be desirable to be able to create arguments > that contain the substituted file name or label as well as some other > text, such as "--file1=foo" or "+foo" (which is a form accepted by > kdiff3). But although forms such as these are accepted by some of the > diff programs I tried, it wasn't necessary for any of them. I didn't > come across any other problems. Just to clarify, would you like "+---f1" expanding to +diff_file_name or ---f1+foo to diff_file_name+foo respectively? - Danielsh wrote: > Yes. Also, please document the return value (NULL-terminated argv array?). I thought for quite a while that apr_array_header_t should be used instead of the argv array in svn_io_create_custom_diff_cmd(..) but then it occurred to me that this is API level and that would force folks to use APR to access this. Question 1: does/should svn_io_create_custom_diff_cmd live in the API, i.e. svn_io.c? It's where it is currently because of scope issues, but not because I considered it API material, and I think that API users probably should not have access to this --- I may be wrong, but how and why would people want to use this function, if they would? Question 2: Given the additional issue of the diff_cmd_baton data which is passed piecemeal at times because (I guess) of scope issues, is there a case for a private diff.h file that covers the diff universe? - M. Compilato wrote: > I like the idea! I note that you don't draw attention to any existing > issues, but you might want to review issue #2044 and see if your work > resolves the request there: > http://subversion.tigris.org/issues/show_bug.cgi?id=2044 Yes, the new command fixes this. > Also, a minor design nit (sorry, no code review): The "---f1" > construct is something I've never seen before. That's why I picked it --- I checked extensively, and no-one uses a triple dash, so it does exactly what we hope it will: never interfere with anything that people might do. Also, I think it looks quite 'unixy' and it's easy to read. I expect fewer problems on windows.
Subversion InstantPlayground patch
Here is a little dev toy for casual submitters and people who just
want to have a quick play with the svn source, or perhaps use svn as a
framework to explore one of the API's that svn uses.
The patch sets up a new option for the svn diff command called
'InstantPlayground' and puts a function called
InstantPlaygroundFunction into subversion/libsvn_client/diff.c
together with all the parameters needed for playing with the svn
diff code.
It also demonstrates how to set up a command line option, so
people who want something different should easily be able to use
this as a guide, even if the patch eventually goes 'out of date'.
It's not meant for committing, but I thought that other people
might find this little tool as useful as I do, so here it is :)
To use it, apply the patch, then open diff.c and find the
InstantPlaygroundFunction, where your code will go.
[[[
This is a demonstration patch that shows how to add a new sub command
to the svn command line structure for the diff command.
This is aimed at casual submitters or anyone who wants a quick
'ready-to-play' set-up to explore svn or to use svn as a framework
to to play with the API's svn uses.
To call your code from the command line, do:
svn diff --InstantPlayground="your input here"
* subversion/include/svn_client.h (svn_client_diff6):
Add the InstantPlayground char* parameter that contains the input
string to the parameter list.
* subversion/libsvn_client/deprecated.c
(svn_client_diff5): Add the InstantPlayground char* parameter to
the call this deprecated function makes to svn_client_diff6.
(InstantPlaygroundFunction): Your code goes here. The parameter
'diff_cmd_baton' contains everything that would be normally passed
into the svn diff domain.
* subversion/libsvn_client/diff.c (diff_cmd_baton):
Define the char* InstantPlayground variable within the
diff_cmd_baton structure.
(svn_client_diff6): Add the InstantPlayground char* parameter that
contains the input string to the parameter list, assign it to
diff_cmd_baton and reroute the call to the InstantPlaygroundFunction
instead of do_diff).
* subversion/svn/cl.h (svn_cl__opt_state_t):
Add the char* InstantPlayground field to the diff command structure
contained within the svn diff command options.
* subversion/svn/diff-cmd.c
(svn_cl__diff): Call svnclient_diff6 with the new parameter char *
InstantPlayground added.
* subversion/svn/svn.c
(svn_cl__longopt_t): Define the opt_InstantPlayground parameter to the
long options list that do not have a short option.
(svn_cl__options): This is the main list of svn command line options
and we're adding the opt_InstantPlayground as a sub option to the
diff group within.
(svn_cl__cmd_table): Add opt_InstantPlayground to the list of diff
sub options that can be called.
(sub_main): Assign the input string passed on the command line to the
opt_InstantPlayground variable.
]]]
Index: subversion/include/svn_client.h
===
--- subversion/include/svn_client.h (revision 1466727)
+++ subversion/include/svn_client.h (working copy)
@@ -3031,8 +3031,9 @@ svn_client_diff6(const apr_array_header_t *diff_op
svn_stream_t *errstream,
const apr_array_header_t *changelists,
svn_client_ctx_t *ctx,
- apr_pool_t *pool);
-
+ apr_pool_t *pool,
+ char *InstantPlayground);
+
/** Similar to svn_client_diff6(), but with @a outfile and @a errfile,
* instead of @a outstream and @a errstream, and with @a
* no_diff_added, @a ignore_properties, and @a properties_only always
Index: subversion/libsvn_client/deprecated.c
===
--- subversion/libsvn_client/deprecated.c (revision 1466727)
+++ subversion/libsvn_client/deprecated.c (working copy)
@@ -943,7 +943,9 @@ svn_client_diff5(const apr_array_header_t *diff_op
ignore_content_type, FALSE /* ignore_properties */,
FALSE /* properties_only */, use_git_diff_format,
header_encoding,
- outstream, errstream, changelists, ctx, pool);
+ outstream, errstream, changelists, ctx, pool,
+/* remove the last parameter to remove the InstantPlayground! */
+ NULL);
}
svn_error_t *
Index: subversion/libsvn_client/diff.c
===
--- subversion/libsvn_client/diff.c (revision 1466727)
+++ subversion/libsvn_client/diff.c (working copy)
@@ -609,6 +609,7 @@ struct diff_cmd_baton {
/* The anchor to prefix before wc paths */
const char *anchor;
+ const char *InstantPlayground;
/* Whether the local diff target of a repos->wc diff is a copy. */
svn_boolean_t repos_wc_diff_target_is_copy;
};
@@ -2500,6 +2501,15 @@ set_up_diff_cmd_and_options(struct diff_
Diff Project --invoke-diff-cmd part
This patch plugs in a new option --invoke-diff-cmd into the existing diff command structure, but does leave the existing "diff-cmd" option untouched. This addition allows the user to define a complex diff command, to be applied in place of the internal diff, for example: svn diff --invoke-diff-cmd="kdiff3 -auto -o /home/g/log ---f1 ---f2 --L1 ---l1 --L2 "MyLabel"" either on the command line(for example): svn diff --invoke-diff-cmd="diff -y ---f1 ---f2" which expands to "diff -y *from *to", or, in the config file by adding invoke-diff-cmd= diff -y ---f1 ---f2 where ---f1 ---f2 are file 1 and 2. and ---l1 ---l2 are labels. What's missing: -- * The merge part, --invoke-diff3-cmd. * Tests. I will write them, but I have to spent some time reading into the test suite first. * This patch breaks the override --internal-diff for now, because this part has to be revised anyway when the invoke-diff3-cmd part gets added. * I added the help blurb to subversion/svn/svn.c:340 but svn help is not showing it. I will hunt for th reason over the weekend, I didn't want to delay the patch any longer. Thanks for looking! Gabriela [[[ Add new diff option "--invoke-diff-cmd" which allows the user to define a custom command line or config file entry for an external diff program. * subversion/include/svn_client.h (svn_client_diff7): Add new function. (svn_client_diff6): Remove deprecated function. * subversion/include/svn_config.h (): Add new definition. * subversion/include/svn_io.h (svn_io_create_custom_diff_cmd): Add new function. (svn_io_run_external_diff): Add new function. * subversion/libsvn_client/deprecated.c (svn_client_diff6): Add deprecated function. * subversion/libsvn_client/diff.c (struct diff_cmd_baton): Add new variable. (diff_content_changed): Add new conditional function call. (set_up_diff_cmd_and_options): Add new condition. * subversion/libsvn_subr/config_file.c (svn_config_ensure): Add help text. * subversion/libsvn_subr/io.c (svn_io_create_custom_diff_cmd): Add new function. (svn_io_run_external_diff): Add new function. * subversion/svn/cl.h (struct svn_cl__opt_state_t): Add new variable. (struct svn_cl__opt_state_t.diff): Add new variable. * subversion/svn/diff-cmd.c (svn_cl__diff): Modify call to deprecated function. * subversion/svn/svn.c (svn_cl__options[]): Add new variable and help info. ]]] Index: subversion/include/svn_client.h === --- subversion/include/svn_client.h (revision 1466727) +++ subversion/include/svn_client.h (working copy) @@ -2978,6 +2978,9 @@ svn_client_blame(const char *path_or_url, * The above two options are mutually exclusive. It is an error to set * both to TRUE. * + * If @a invoke_diff_cmd is non-null, invoke extermal diff command + * with the string it contains. + * * Generated headers are encoded using @a header_encoding. * * Diff output will not be generated for binary files, unless @a @@ -3007,9 +3010,39 @@ svn_client_blame(const char *path_or_url, * * @note @a relative_to_dir doesn't affect the path index generated by * external diff programs. + * + * @since New in 1.8. + */ +svn_error_t * +svn_client_diff7(const apr_array_header_t *options, + const char *path_or_url1, + const svn_opt_revision_t *revision1, + const char *path_or_url2, + const svn_opt_revision_t *revision2, + const char *relative_to_dir, + svn_depth_t depth, + svn_boolean_t ignore_ancestry, + svn_boolean_t no_diff_added, + svn_boolean_t no_diff_deleted, + svn_boolean_t show_copies_as_adds, + svn_boolean_t ignore_content_type, + svn_boolean_t ignore_properties, + svn_boolean_t properties_only, + svn_boolean_t use_git_diff_format, + const char *header_encoding, + svn_stream_t *outstream, + svn_stream_t *errstream, + const apr_array_header_t *changelists, + svn_client_ctx_t *ctx, + apr_pool_t *pool, + const char *invoke_diff_cmd); + +/** Similar to svn_client_diff7(), but with @a invoke_diff_cmd. * + * @deprecated Provided for backward compatibility with the 1.8 API. * @since New in 1.8. */ +SVN_DEPRECATED svn_error_t * svn_client_diff6(const apr_array_header_t *diff_options, const char *path_or_url1, Index: subversion/include/svn_config.h === --- subversion/include/svn_config.h (revision 1466727) +++ subversion/include/svn_config.h (working copy) @@ -112,6 +112,7 @@ typedef struct svn_config_t svn_config_t; #define SVN_CONFIG_OPTION_DIFF_EXTENSIONS "diff-extensions" #define SVN_CONFIG_OPTION_DIFF3_CMD
Re: [PATCH] run_test.py appearance changes
On 4/2/13, Daniel Shahaf wrote:
I'm not sure how to interpret a return value of (0,0) that's not
accompanied by an error flag (C errno!=0, or a Python exception). Is
that normal behaviour, a bug we should be working around in our code, or
an indication of a bug in our logic in the preceding lines?
I took a look around a few distros[1], and in no case does this function
return an error, to cater for dumb terminals if I understand this correctly.
More info: http://en.wikipedia.org/wiki/POSIX_terminal_interface
i.e., if (0,0) means "_get_term_width() was going 35mph in a school zone
at the time of the call to fcntl.ioctl()", we should fix that.
_get_term_width()'s 'sin' is not checking for a sane minimum size, which
is I think is a reasonable omission, because setting up a TERM with a
claimed size of (0,0) isn't easily anticipated and we're only crashing
svn and not aeroplanes :>
But if (0,0) is just a terminal emulator bug, or expected behaviour, then +1 to
commit.
I think since emacs is mainly affected here, it's definitely worthwhile
to fix that because the output is hard to read, and whilst the 'success'
message is in green, someone who is red-green color blind won't be
able to read it easily without at least one space.
Also, the bug eats the progress meter, which is a pity.
This change will actually change the output if you resize the terminal
window while running tests interactively (at least in terminal emulators
that support the ioctl_GWINSZ approach).
Oops, I hadn't thought that someone might adjust their window mid-test.
However, resizing your window results in quite a visual mess in either
case. -- best avoided :)
I'm not objected to it, but it's not clear to me what it gains either (shaves
100 syscalls
from a 'make check' run?).
I agree, it's a a bit very picky to fix this, now that I think about it
-- coding on muds gets you this reflexive habit lest you summon the lag
monster. LPC MUDS are single threaded and Objective C is interpreted,
and every object has a computational limit per heartbeat and the driver
removes objects that time out before completing from the execution list
as irretrievably broken.
So, if you're doing anything 'fancy', you often end up staggering
executions over a number of game turns, and every even so small
inefficiency counts because it all adds up and the entire game is affected.
Gabriela
[1]
The linux kernel (linux-3.8.5) end of the syscall:
case TIOCSWINSZ:
return tiocswinsz(real_tty, p);
static int tiocgwinsz(struct tty_struct *tty, struct winsize __user *arg)
{
int err;
mutex_lock(&tty->termios_mutex);
err = copy_to_user(arg, &tty->winsize, sizeof(*arg));
mutex_unlock(&tty->termios_mutex);
return err ? -EFAULT: 0;
}
The freebsd 9.1 kernel code:
case TIOCGWINSZ:
/* Obtain window size. */
*(struct winsize*)data = tp->t_winsize;
return (0);
4.4BSD-Lite2:
int
ttioctl(tp, cmd, data, flag)
register struct tty *tp;
u_long cmd;
void *data;
int flag;
{
/* */
case TIOCGWINSZ:/* get window size */
*(struct winsize *)data = tp->t_winsize;
break;
/* */
return (0);
}
Re: [PATCH] run_test.py appearance changes
On 01/04/13 01:31, Daniel Shahaf wrote:
I reworked hunk 2 because I found a bug.
The actual problem I saw was caused because the script didn't handle the
case where the terminal dimension returns as (0,0). I've fixed
this in the new patch, and cleaned up inefficient code.
> I did note, though, that you missed adding the SVN_DEPRECATED
> marker to the being-deprecated function.
Thanks :) I'll fix that once I have some feedback on the rest.
[[[
Disable ANSI color for dumb terminals, fix logic bug where
dimensions of (0,0) was treated as a positive result,
re-factored _run_tests to remove repeated calculation of
line_length.
* build/run_tests.py
(_get_term_width): Add test condition.
(TestHarness): Add new variable.
(_run_test): Add new parameter, remove function call.
]]]
Index: build/run_tests.py
===
--- build/run_tests.py (revision 1463012)
+++ build/run_tests.py (working copy)
@@ -100,6 +100,11 @@ def _get_term_width():
os.close(fd)
except:
pass
+ ## Some terminals (eg emacs *shell*, emacs *compilation*)
+ ## seem to return (0,0) from ioctl_GWINSZ, so let's deal
+ ## with that case...
+ if cr == (0, 0):
+cr = None
if not cr:
try:
cr = (os.environ['LINES'], os.environ['COLUMNS'])
@@ -178,7 +183,8 @@ class TestHarness:
self.log = None
self.ssl_cert = ssl_cert
self.http_proxy = http_proxy
-if not sys.stdout.isatty() or sys.platform == 'win32':
+if not sys.stdout.isatty() or sys.platform == 'win32' or \
+ os.getenv("TERM") == "dumb":
TextColors.disable()
def run(self, list):
@@ -186,8 +192,9 @@ class TestHarness:
there is a log file. Return zero iff all test programs passed.'''
self._open_log('w')
failed = 0
+line_length = _get_term_width()
for cnt, prog in enumerate(list):
- failed = self._run_test(prog, cnt, len(list)) or failed
+ failed = self._run_test(prog, cnt, len(list), line_length) or failed
if self.log is None:
return failed
@@ -550,7 +557,7 @@ class TestHarness:
return failed
- def _run_test(self, prog, test_nr, total_tests):
+ def _run_test(self, prog, test_nr, total_tests, line_length):
"Run a single test. Return the test's exit code."
if self.log:
@@ -587,7 +594,6 @@ class TestHarness:
progabs = os.path.abspath(os.path.join(self.srcdir, prog))
old_cwd = os.getcwd()
-line_length = _get_term_width()
dots_needed = line_length \
- len(test_info) \
- len('success')
Re: [PATCH] Change label strings in svn up to match svn diff
On 31/03/13 00:39, Daniel Shahaf wrote:
Gabriela Gibson wrote on Sat, Mar 30, 2013 at 21:31:31 +:
Code inspection tells me that oldrev_str can become the @a suffix
parameter to the svn_io_open_uniquely_named() call in
preserve_pre_merge_files(), in which case [\t ()] are all inappropriate
characters.
I assume the "label" you refer to is the text following the
<<<<<>>>>> conflict markers?
I was trying to change the label for the working copy in a 3-way merge
to show the relative file path.
Take 2 below:
[[[
Change "mine_label" passed to external diff3-cmd to list relative
path of file.
* subversion/include/svn_io.h
(svn_io_run_diff3_4): Fix comment, re-version function.
(svn_io_run_diff3_3): Change comment to reflect deprecation.
* subversion/libsvn_subr/deprecated.c
(svn_io_run_diff3_3): Move function from io.c.
* subversion/libsvn_subr/io.c
(svn_io_run_diff3_3): Move function to deprecated.c.
(svn_io_run_diff3_4): Add parameter for use in "mine_label".
* subversion/libsvn_wc/merge.c
(do_text_merge_external): Add new parameter.
]]]
Index: subversion/include/svn_io.h
===
--- subversion/include/svn_io.h (revision 1463012)
+++ subversion/include/svn_io.h (working copy)
@@ -1819,15 +1819,16 @@ svn_io_run_diff(const char *dir,
const char *diff_cmd,
apr_pool_t *pool);
-
-
/** Invoke the configured @c diff3 program, in utf8-encoded @a dir
* like this:
*
- * diff3 -E -m @a mine @a older @a yours > @a merged
+ * diff3 -E -m -L @a relpath + @a mine_label -L @a older_label \
+ * -L @a yours_label @a mine @a older @a yours > @a merged
*
* (See the diff3 documentation for details.)
*
+ * If @a relpath is NULL, then use @a mine_label alone.
+ *
* If @a user_args is non-NULL, replace "-E" with the const char*
* elements that @a user_args contains.
*
@@ -1853,6 +1854,27 @@ svn_io_run_diff(const char *dir,
*
* Do all allocation in @a pool.
*
+ * @since New in 1.8.
+ */
+svn_error_t *
+svn_io_run_diff3_4(int *exitcode,
+ const char *dir,
+ const char *mine,
+ const char *older,
+ const char *yours,
+ const char *mine_label,
+ const char *older_label,
+ const char *yours_label,
+ apr_file_t *merged,
+ const char *diff3_cmd,
+ const char *relpath,
+ const apr_array_header_t *user_args,
+ apr_pool_t *pool);
+
+/** Similar to svn_io_run_diff3_4(), but with @a relpath not provided
+ * encoded in internal encoding used by APR.
+ *
+ * @deprecated Provided for backwards compatibility with the 1.5 API.
* @since New in 1.4.
*/
svn_error_t *
Index: subversion/libsvn_subr/deprecated.c
===
--- subversion/libsvn_subr/deprecated.c (revision 1463012)
+++ subversion/libsvn_subr/deprecated.c (working copy)
@@ -713,6 +713,30 @@ svn_io_run_diff(const char *dir,
}
svn_error_t *
+svn_io_run_diff3_3(int *exitcode,
+ const char *dir,
+ const char *mine,
+ const char *older,
+ const char *yours,
+ const char *mine_label,
+ const char *older_label,
+ const char *yours_label,
+ apr_file_t *merged,
+ const char *diff3_cmd,
+ const apr_array_header_t *user_args,
+ apr_pool_t *pool)
+{
+ SVN_ERR(svn_path_cstring_to_utf8(&diff3_cmd, diff3_cmd, pool));
+
+ return svn_error_trace(svn_io_run_diff3_4(exitcode, dir,
+mine, older, yours,
+mine_label, older_label,
+yours_label, merged,
+diff3_cmd, NULL,
+user_args, pool));
+}
+
+svn_error_t *
svn_io_run_diff3_2(int *exitcode,
const char *dir,
const char *mine,
Index: subversion/libsvn_subr/io.c
===
--- subversion/libsvn_subr/io.c (revision 1463012)
+++ subversion/libsvn_subr/io.c (working copy)
@@ -2921,9 +2921,8 @@ svn_io_run_diff2(const char *dir,
return SVN_NO_ERROR;
}
-
svn_error_t *
-svn_io_run_diff3_3(int *exitcode,
+svn_io_run_diff3_4(int *exitcode,
const char *dir,
const char *mine,
const char *older,
@@ -2933,6 +2932,7 @@ svn_error_t *
const char *yours_label,
apr_file_t *merged,
const char *diff3_cmd,
+
[PATCH] run_test.py appearance changes
[[[
Disable ANSI color for dumb terminals, format terminal test output.
* build/run_tests.py
(TestHarness): Add test condition, format terminal output.
]]]
I disabled color conditionally for dumb terminals because the control
characters are displayed in the raw in those.
Index: build/run_tests.py
===
--- build/run_tests.py (revision 1463012)
+++ build/run_tests.py (working copy)
@@ -178,7 +178,8 @@ class TestHarness:
self.log = None
self.ssl_cert = ssl_cert
self.http_proxy = http_proxy
-if not sys.stdout.isatty() or sys.platform == 'win32':
+if not sys.stdout.isatty() or sys.platform == 'win32' or \
+ os.getenv("TERM") == "dumb":
TextColors.disable()
def run(self, list):
@@ -565,7 +566,8 @@ class TestHarness:
progdir, progbase = os.path.split(prog)
if self.log:
# Using write here because we don't want even a trailing space
- test_info = '[%s/%d] %s' % (str(test_nr + 1).zfill(len(str(total_tests))),
+ test_info = '[%s/%d] %-35s' % \
+ (str(test_nr + 1).zfill(len(str(total_tests))),
total_tests, progbase)
if self.list_tests:
sys.stdout.write('Listing tests in %s' % (test_info, ))
[PATCH] Change label strings in svn up to match svn diff
[[[ Change the label strings for "svn merge" and "svn update" when diff3-cmd is used to match the label strings for "svn diff" when diff-cmd is used. * subversion/libsvn_wc/update_editor.c (svn_wc__perform_file_merge): Alter label to include filename. ]]] Index: subversion/libsvn_wc/update_editor.c === --- subversion/libsvn_wc/update_editor.c (revision 1460218) +++ subversion/libsvn_wc/update_editor.c (working copy) @@ -3768,7 +3768,9 @@ svn_wc__perform_file_merge(svn_skel_t **work_items const char *new_text_base_tmp_abspath; enum svn_wc_merge_outcome_t merge_outcome = svn_wc_merge_unchanged; svn_skel_t *work_item; + const char *child_relpath = svn_dirent_skip_ancestor(wri_abspath, + local_abspath); *work_items = NULL; SVN_ERR(svn_wc__db_pristine_get_path(&new_text_base_tmp_abspath, @@ -3792,16 +3795,20 @@ svn_wc__perform_file_merge(svn_skel_t **work_items if (!SVN_IS_VALID_REVNUM(old_revision)) old_revision = 0; - oldrev_str = apr_psprintf(scratch_pool, ".r%ld%s%s", + + oldrev_str = apr_psprintf(scratch_pool, "%s\t(revision %ld) %s%s", + child_relpath, old_revision, *path_ext ? "." : "", *path_ext ? path_ext : ""); - newrev_str = apr_psprintf(scratch_pool, ".r%ld%s%s", + newrev_str = apr_psprintf(scratch_pool, "%s\t(revision %ld) %s%s", + child_relpath, target_revision, *path_ext ? "." : "", *path_ext ? path_ext : ""); - mine_str = apr_psprintf(scratch_pool, ".mine%s%s", + mine_str = apr_psprintf(scratch_pool, "%s\t(working copy) %s%s", + child_relpath, *path_ext ? "." : "", *path_ext ? path_ext : "");
Re: [PATCH] Code tidying for subversion/include/config.h
On 25/03/13 13:40, C. Michael Pilato wrote:
Note that if there's a way to make the additional comment *not*
show up in our doxygen docs, that's preferred -- I don't suspect the
indentation-dependent layout of that header will survive the transformation
to that output format.
After trying out @cond and \internal with limited success as they both
remove the selected text in the documentation and html source, I've
found that if you use one "*" in the comment like so:
/* code-only comment */
Doxygen doesn't pick this up, since it's not a Doxygen comment like
/** or /*! and so, the comment is omitted from the documentation, but
displayed in the source code html.
I could not find a way to preserve the comment placement in the
documentation, however, given the layout of the html, it would not be as
visually useful as it is in the code.
Index: subversion/include/svn_config.h
===
--- subversion/include/svn_config.h (revision 1460775)
+++ subversion/include/svn_config.h (working copy)
@@ -62,6 +62,10 @@ typedef struct svn_config_t svn_config_t;
* client configuration files.
* @{
*/
+
+ /* This list of #defines is intentionally presented as a nested list
+that matches the in-config hierarchy. */
+
#define SVN_CONFIG_CATEGORY_SERVERS"servers"
#define SVN_CONFIG_SECTION_GROUPS "groups"
#define SVN_CONFIG_SECTION_GLOBAL "global"
Re: [PATCH] Code tidying for subversion/include/config.h
On 23/03/13 15:58, Daniel Shahaf wrote: Gabriela Gibson wrote on Sat, Mar 23, 2013 at 15:27:31 +: [[[ Align variables for easier reading. *subversion/include/config.h(): Align variables. ]]] -0. The indentation is intention and reflects the nesting (e.g., --config-option=servers:global:http-proxy-username=danielsh). It's actually easier to read the way it is now. A recent log message by cmpilato actually called out that fact explicitly, didn't you notice that? Oh -- I hadn't noticed that at all -- I've never seen this trick before and I think that the log message that explains it should be in the code file, because some folks might not realize what is happening here and miss out, I certainly did :) Gabriela
[PATCH] Code tidying for subversion/include/config.h
[[[
Align variables for easier reading.
*subversion/include/config.h(): Align variables.
]]]
Index: subversion/include/svn_config.h
===
--- subversion/include/svn_config.h (revision 1460153)
+++ subversion/include/svn_config.h (working copy)
@@ -62,9 +62,9 @@ typedef struct svn_config_t svn_config_t;
* client configuration files.
* @{
*/
-#define SVN_CONFIG_CATEGORY_SERVERS"servers"
-#define SVN_CONFIG_SECTION_GROUPS "groups"
-#define SVN_CONFIG_SECTION_GLOBAL "global"
+#define SVN_CONFIG_CATEGORY_SERVERS "servers"
+#define SVN_CONFIG_SECTION_GROUPS "groups"
+#define SVN_CONFIG_SECTION_GLOBAL "global"
#define SVN_CONFIG_OPTION_HTTP_PROXY_HOST "http-proxy-host"
#define SVN_CONFIG_OPTION_HTTP_PROXY_PORT "http-proxy-port"
#define SVN_CONFIG_OPTION_HTTP_PROXY_USERNAME "http-proxy-username"
@@ -92,43 +92,45 @@ typedef struct svn_config_t svn_config_t;
/** @since New in 1.8. */
#define SVN_CONFIG_OPTION_HTTP_MAX_CONNECTIONS "http-max-connections"
-#define SVN_CONFIG_CATEGORY_CONFIG "config"
-#define SVN_CONFIG_SECTION_AUTH "auth"
+#define SVN_CONFIG_CATEGORY_CONFIG "config"
+#define SVN_CONFIG_SECTION_AUTH "auth"
#define SVN_CONFIG_OPTION_PASSWORD_STORES "password-stores"
#define SVN_CONFIG_OPTION_KWALLET_WALLET"kwallet-wallet"
-#define SVN_CONFIG_OPTION_KWALLET_SVN_APPLICATION_NAME_WITH_PID "kwallet-svn-application-name-with-pid"
+#define SVN_CONFIG_OPTION_KWALLET_SVN_APPLICATION_NAME_WITH_PID \
+"kwallet-svn-application-name-with-pid"
/** @since New in 1.8. */
-#define SVN_CONFIG_OPTION_SSL_CLIENT_CERT_FILE_PROMPT "ssl-client-cert-file-prompt"
+#define SVN_CONFIG_OPTION_SSL_CLIENT_CERT_FILE_PROMPT \
+ "ssl-client-cert-file-prompt"
/* The majority of options of the "auth" section
* has been moved to SVN_CONFIG_CATEGORY_SERVERS. */
-#define SVN_CONFIG_SECTION_HELPERS "helpers"
-#define SVN_CONFIG_OPTION_EDITOR_CMD"editor-cmd"
-#define SVN_CONFIG_OPTION_DIFF_CMD "diff-cmd"
+#define SVN_CONFIG_SECTION_HELPERS"helpers"
+#define SVN_CONFIG_OPTION_EDITOR_CMD "editor-cmd"
+#define SVN_CONFIG_OPTION_DIFF_CMD"diff-cmd"
/** @since New in 1.7. */
-#define SVN_CONFIG_OPTION_DIFF_EXTENSIONS "diff-extensions"
-#define SVN_CONFIG_OPTION_DIFF3_CMD "diff3-cmd"
-#define SVN_CONFIG_OPTION_DIFF3_HAS_PROGRAM_ARG "diff3-has-program-arg"
-#define SVN_CONFIG_OPTION_MERGE_TOOL_CMD"merge-tool-cmd"
-#define SVN_CONFIG_SECTION_MISCELLANY "miscellany"
-#define SVN_CONFIG_OPTION_GLOBAL_IGNORES"global-ignores"
-#define SVN_CONFIG_OPTION_LOG_ENCODING "log-encoding"
-#define SVN_CONFIG_OPTION_USE_COMMIT_TIMES "use-commit-times"
+#define SVN_CONFIG_OPTION_DIFF_EXTENSIONS "diff-extensions"
+#define SVN_CONFIG_OPTION_DIFF3_CMD "diff3-cmd"
+#define SVN_CONFIG_OPTION_DIFF3_HAS_PROGRAM_ARG "diff3-has-program-arg"
+#define SVN_CONFIG_OPTION_MERGE_TOOL_CMD "merge-tool-cmd"
+#define SVN_CONFIG_SECTION_MISCELLANY "miscellany"
+#define SVN_CONFIG_OPTION_GLOBAL_IGNORES "global-ignores"
+#define SVN_CONFIG_OPTION_LOG_ENCODING"log-encoding"
+#define SVN_CONFIG_OPTION_USE_COMMIT_TIMES"use-commit-times"
/** @deprecated Not used by Subversion since 2003/r847039 (well before 1.0) */
-#define SVN_CONFIG_OPTION_TEMPLATE_ROOT "template-root"
-#define SVN_CONFIG_OPTION_ENABLE_AUTO_PROPS "enable-auto-props"
-#define SVN_CONFIG_OPTION_NO_UNLOCK "no-unlock"
-#define SVN_CONFIG_OPTION_MIMETYPES_FILE"mime-types-file"
-#define SVN_CONFIG_OPTION_PRESERVED_CF_EXTS "preserved-conflict-file-exts"
-#define SVN_CONFIG_OPTION_INTERACTIVE_CONFLICTS "interactive-conflicts"
-#define SVN_CONFIG_OPTION_MEMORY_CACHE_SIZE "memory-cache-size"
-#define SVN_CONFIG_SECTION_TUNNELS "tunnels"
-#define SVN_CONFIG_SECTION_AUTO_PROPS "auto-props"
+#define SVN_CONFIG_OPTION_TEMPLATE_ROOT "template-root"
+#define SVN_CONFIG_OPTION_ENABLE_AUTO_PROPS "enable-auto-props"
+#define SVN_CONFIG_OPTION_NO_UNLOCK "no-unlock"
+#define SVN_CONFIG_OPTION_MIMETYPES_FILE "mime-types-file"
+#define SVN_CONFIG_OPTION_PRESERVED_CF_EXTS "preserved-conflict-file-exts"
+#define SVN_CONFIG_OPTION_INTERACTIVE_CONFLICTS "interactive-conflicts"
+#define SVN_CONFIG_OPTION_MEMORY_CACHE_SIZE "memory-cache-size"
+#define SVN_CONFIG_SECTION_TUNNELS"tunnels"
+#define
Re: Issue #2044 - Fully customizable external diff invocations
On 21/03/13 15:05, Julian Foad wrote: > - Does Subversion provide good labels? I have been using > 'diff3-cmd' configured to run kdiff3, and the labels Subversion passes > to it are like '.mine', '.r1459015' and '.r1459080' -- they don't > include the file name at all, which makes it very hard to see what > file I'm being asked to merge. (Maybe the diff3-cmd option was > never designed to run a GUI diff tool? But I do it.) And for > 'merge-tool-cmd' it doesn't appear to pass any labels at the moment. > What a lot of inconsistency to sort out. But if the labels passed > to the diff-cmd are always good, you don't worry about this yet. Thank you for the well-thought-out response. It cleared up some misconceptions, and yes, I did think of merge there. :) I have to think about your letter in detail a little more before I can say anymore. Gabriela ps.: I think that the issue with kdiff3 is with the label handling. The section below suggests to me that kdiff3 is dropping some info from the label it has been passed: g_at_musashi:~/tmp/test-wc$ $SVN diff --diff-cmd=/home/g/programming/perlfiles/dump_diff.pl Index: testfile === Dumping @ARGV... Arg 0 is >-u< Arg 1 is >-L< Arg 2 is >testfile (revision 1)< Arg 3 is >-L< Arg 4 is >testfile (working copy)< Arg 5 is >/home/g/tmp/test-wc/.svn/pristine/91/91b7b0b1e27bfbf7bc646946f35fa972c47c2d32.svn-base< Arg 6 is >/home/g/tmp/test-wc/testfile< g_at_musashi:~/tmp/test-wc$
Re: svn diff fix for bug 2044
Thanks for all the feedback, advice and ideas so far :) IIRC Subversion needs to communicate the following file names to the user's diff program: mine, yours, base and output It then takes the user input (--diff-cmd) from either the command line or a script, and produces a command to run the external diff in order to harvest that output. All we need to do is to render the command line *exactly* as typed within the delimiters of (say) %% at the end and start, with the exception that %mine %yours, %base and %output will be string substituted by subversion accordingly (a bit like printf does) A simple case: -diff-cmd=%%/usr/fancy_diff_cmd %mine %yours %base %output%% or more complex: -diff-cmd=%%/usr/fancy_diff_cmd -x "" -L ' ' %mine -Q %yours -Z %base -q "xyzzy" %output%% The reason for the un-unix-like "%" syntax is that we don't want to restrict user's freedom to use any kind of switches or whatever syntax they want to use. Also, the %% delimiter keeps it visually simple for the user as well and already matches the token pattern we're looking for, plus it makes the quoting issues go away. What do you think?
[PATCH] subversion/svn/cl.h comment tidying
[[[
Align comments for easier reading.
*subversion/svn/cl.h:
(svn_cl__opt_state_t): align comments.
Patch by: Gabriela Gibson
]]]
(this has been catching my eye every time I saw the file ;-)
Index: subversion/svn/cl.h
===
--- subversion/svn/cl.h (revision 1458566)
+++ subversion/svn/cl.h (working copy)
@@ -180,17 +180,17 @@ typedef struct svn_cl__opt_state_t
svn_boolean_t no_auth_cache; /* do not cache authentication information */
struct
{
- const char *diff_cmd; /* the external diff command to use */
- svn_boolean_t internal_diff;/* override diff_cmd in config file */
- svn_boolean_t no_diff_added; /* do not show diffs for deleted files */
- svn_boolean_t no_diff_deleted; /* do not show diffs for deleted files */
+ const char *diff_cmd; /* the external diff command to use */
+ svn_boolean_t internal_diff; /* override diff_cmd in config file */
+ svn_boolean_t no_diff_added; /* do not show diffs for deleted files */
+ svn_boolean_t no_diff_deleted; /* do not show diffs for deleted files */
svn_boolean_t show_copies_as_adds; /* do not diff copies with their source */
- svn_boolean_t notice_ancestry; /* notice ancestry for diff-y operations */
- svn_boolean_t summarize; /* create a summary of a diff */
+ svn_boolean_t notice_ancestry; /* notice ancestry for diff-y operations */
+ svn_boolean_t summarize; /* create a summary of a diff */
svn_boolean_t use_git_diff_format; /* Use git's extended diff format */
- svn_boolean_t ignore_properties; /* ignore properties */
- svn_boolean_t properties_only; /* Show properties only */
- svn_boolean_t patch_compatible; /* Output compatible with GNU patch */
+ svn_boolean_t ignore_properties; /* ignore properties */
+ svn_boolean_t properties_only; /* Show properties only */
+ svn_boolean_t patch_compatible;/* Output compatible with GNU patch */
} diff;
svn_boolean_t ignore_ancestry; /* ignore ancestry for merge-y operations */
svn_boolean_t ignore_externals;/* ignore externals definitions */
@@ -219,21 +219,21 @@ typedef struct svn_cl__opt_state_t
apr_hash_t *revprop_table; /* table of revision properties to get/set */
svn_boolean_t parents; /* create intermediate directories */
svn_boolean_t use_merge_history; /* use/display extra merge information */
- svn_cl__accept_t accept_which; /* how to handle conflicts */
- svn_cl__show_revs_t show_revs; /* mergeinfo flavor */
- svn_depth_t set_depth; /* new sticky ambient depth value */
- svn_boolean_t reintegrate; /* use "reintegrate" merge-source heuristic */
+ svn_cl__accept_t accept_which; /* how to handle conflicts */
+ svn_cl__show_revs_t show_revs; /* mergeinfo flavor */
+ svn_depth_t set_depth; /* new sticky ambient depth value */
+ svn_boolean_t reintegrate; /* use "reintegrate" merge-source heuristic */
svn_boolean_t trust_server_cert; /* trust server SSL certs that would
otherwise be rejected as "untrusted" */
int strip; /* number of leading path components to strip */
- svn_boolean_t ignore_keywords; /* do not expand keywords */
- svn_boolean_t reverse_diff; /* reverse a diff (e.g. when patching) */
+ svn_boolean_t ignore_keywords; /* do not expand keywords */
+ svn_boolean_t reverse_diff; /* reverse a diff (e.g. when patching) */
svn_boolean_t ignore_whitespace; /* don't account for whitespace when
patching */
- svn_boolean_t show_diff;/* produce diff output (maps to --diff) */
- svn_boolean_t allow_mixed_rev; /* Allow operation on mixed-revision WC */
+ svn_boolean_t show_diff; /* produce diff output (maps to --diff) */
+ svn_boolean_t allow_mixed_rev; /* Allow operation on mixed-revision WC */
svn_boolean_t include_externals; /* Recurses (in)to file & dir externals */
- svn_boolean_t show_inherited_props; /* get inherited properties */
+ svn_boolean_t show_inherited_props; /* get inherited properties */
apr_array_header_t* search_patterns; /* pattern arguments for --search */
} svn_cl__opt_state_t;
Re: svn diff fix for bug 2044
On 19/03/13 13:09, Julian Foad wrote:
Julian Foad wrote:
> For the record, the summary line of issue #2044 is 'Fully customizable
> external diff invocations'. (I like to mention the summary alongside
> the number as I am not good at memorizing issue numbers.) I'm curious
> about your patch because I am interested in issue #2044 and would like
> to see how this particular change would fit in.
>
> Please could you tell me more precisely what your patch does and why?
> Of course I could read carefully through your patch to discover the
> 'what', but not the 'why'.
Hi Julian,
It's not really a patch as such, not yet anyway :> Also, this strictly
speaking is issue 2074, which was marked as a duplicate of 2044 since it
partially solves 2074.
Given the following perl script posing as my diff command:
cat << EOF > dump_diff.pl
#!/usr/bin/perl -w
print "Dumping \@ARGV...\n";
for (my $i = 0; $i < @ARGV; $i++) {
print "Arg $i is >$ARGV[$i]<\n";
}
exit 0;
EOF
The output of this 'diff-command' looks like on a test repository with
a single change:
g@musashi:~/tmp/test-wc$ $SVN diff
--diff-cmd=/home/g/programming/perlfiles/dump_diff.pl
Index: testfile
===
Dumping @ARGV...
Arg 0 is >-u<
Arg 1 is >-L<
Arg 2 is >testfile (revision 1)<
Arg 3 is >-L<
Arg 4 is >testfile (working copy)<
Arg 5 is
>/home/g/tmp/test-wc/.svn/pristine/91/91b7b0b1e27bfbf7bc646946f35fa972c47c2d32.svn-base<
Arg 6 is >/home/g/tmp/test-wc/testfile<
g@musashi:~/tmp/test-wc$
The goal of my patch is to provide two facilities:
1. Allow the complete removal of the "-u" switch.
2. Allow the replacement of the "-L" switch
(One question out of 2 above is, should we allow the complete removal
of the -L as the patch does for the "-u". Currently, there is an
empty element in argv, the alternative is to add another flag, (say)
--no-diff-label)
The patch adds a 'char *user_label_string' and a 'svn_boolean_t
ext_string_present' parameter to the internal structures.
This part of the patch (once it's completed) allows you to do:
g@musashi:~/trunk_diff4$ ~/trunk_diff4/subversion/svn/svn diff -x ""
--diff-label="" --diff-cmd=/home/g/programming/perlfiles/dump_diff.pl
Index: subversion/include/svn_client.h
===
Dumping @ARGV...
Arg 0 is ><
Arg 1 is >subversion/include/svn_client.h(revision 1458417)<
Arg 2 is ><
Arg 3 is >subversion/include/svn_client.h(working copy)<
Arg 4 is
>/home/g/trunk_diff4/.svn/pristine/90/908abcdec38f17df77baf339075101ba4471a4e4.svn-base<
Arg 5 is >/tmp/svn-40JzGW<
or also:
g@musashi:~/trunk_diff4$ ~/trunk_diff4/subversion/svn/svn diff -x
"Hansel" --diff-label="Gretel" --diff-cmd=/home/g/programming/perlfiles
/dump_diff.pl
Index: subversion/include/svn_client.h
===
Dumping @ARGV...
Arg 0 is >Hansel<
Arg 1 is >Gretel<
Arg 2 is >subversion/include/svn_client.h(revision 1458417)<
Arg 3 is >Gretel<
Arg 4 is >subversion/include/svn_client.h(working copy)<
Arg 5 is
>/home/g/trunk_diff4/.svn/pristine/90/908abcdec38f17df77baf339075101ba4471a4e4.svn-base<
Arg 6 is >/tmp/svn-BhDgjc<
otherwise, the behavior is exactly as it was before.
regards,
Gabriela
svn diff fix for bug 2044
Hi,
I've made some changes to meet some feature requests regarding svn diff.
Could you please take a look and let me know if I'm on the right track?
thanks,
Gabriela
[[[
Change "svn diff" to allow removal of "-u" and use of arbitrary strings
in place of current hard-coded "-L" switch. This partially resolves.
http://subversion.tigris.org/issues/show_bug.cgi?id=2044
* subversion/svn/cl.h
(svn_cl__opt_state_t) Add two new fields.
* subversion/svn/svn.c
(svn_cl__longopt_t) Add new field.
(svn_cl__options) Add new field and help information.
(svn_cl__cmd_table) Add two new parameters.
(sub_main) Initialize new field.
(sub_main) Add new case.
(sub_main) Add conditional call to svn_config_set.
* subversion/svn/diff-cmd.c
(svn_cl__diff) Add braces and minor indentation issue?
(svn_cl__diff) Change call to svn_client_diff6 to svn_client_diff7.
* subversion/include/svn_config.h
() Add new declarations.
* subversion/include/svn_io.h
(svn_io_run_diff2_2) Add new function.
* subversion/include/svn_client.h
(svn_client_blame) Add new function. svn_client_diff7.
* subversion/libsvn_client/diff.c
(diff_cmd_baton) Add new field.
(diff_content_changed) Replace call to svn_io_run_diff2 with
svn_io_run_diff2_2.
(svn_client_diff7) Add new function.
* subversion/libsvn_subr/io.c
(svn_io_run_diff2_2) Add new function.
]]]
Index: subversion/libsvn_subr/io.c
===
--- subversion/libsvn_subr/io.c (revision 1458203)
+++ subversion/libsvn_subr/io.c (working copy)
@@ -2831,7 +2831,105 @@ svn_io_run_cmd(const char *path,
return svn_io_wait_for_cmd(&cmd_proc, cmd, exitcode, exitwhy, pool);
}
+svn_error_t *
+svn_io_run_diff2_2(const char *dir,
+ const char *const *user_args,
+ int num_user_args,
+ const char *label1,
+ const char *label2,
+ const char *user_label_string,
+ const char *from,
+ const char *to,
+ int *pexitcode,
+ apr_file_t *outfile,
+ apr_file_t *errfile,
+ const char *diff_cmd,
+ svn_boolean_t ext_string_present,
+ apr_pool_t *pool)
+{
+ const char **args;
+ int i;
+ int exitcode;
+ int nargs = 4; /* the diff command itself, two paths, plus a trailing NULL */
+ apr_pool_t *subpool = svn_pool_create(pool);
+ if (pexitcode == NULL)
+pexitcode = &exitcode;
+
+ if (user_args != NULL)
+nargs += num_user_args;
+ else
+/* Handling the case where '-u ""' is provided, to avoid adding -u */
+if (! ext_string_present)
+ nargs += 1; /* -u */
+
+ if (label1 != NULL)
+nargs += 2; /* the -L and the label itself */
+ if (label2 != NULL)
+nargs += 2; /* the -L and the label itself */
+
+ args = apr_palloc(subpool, nargs * sizeof(char *));
+
+ i = 0;
+ args[i++] = diff_cmd;
+
+ if (user_args != NULL)
+{
+ int j;
+ for (j = 0; j < num_user_args; ++j)
+args[i++] = user_args[j];
+}
+ else
+if (! ext_string_present)
+ args[i++] = "-u"; /* assume -u if the user didn't give us any args */
+
+ if (label1 != NULL)
+{
+ if (! user_label_string)
+args[i++] = "-L";
+ else
+args[i++] = user_label_string;
+ args[i++] = label1;
+}
+ if (label2 != NULL)
+{
+ if (! user_label_string)
+args[i++] = "-L";
+ else
+args[i++] = user_label_string;
+ args[i++] = label2;
+}
+
+ args[i++] = svn_dirent_local_style(from, subpool);
+ args[i++] = svn_dirent_local_style(to, subpool);
+ args[i++] = NULL;
+
+ SVN_ERR_ASSERT(i == nargs);
+
+ SVN_ERR(svn_io_run_cmd(dir, diff_cmd, args, pexitcode, NULL, TRUE,
+ NULL, outfile, errfile, subpool));
+
+ /* The man page for (GNU) diff describes the return value as:
+
+ "An exit status of 0 means no differences were found, 1 means
+some differences were found, and 2 means trouble."
+
+ A return value of 2 typically occurs when diff cannot read its input
+ or write to its output, but in any case we probably ought to return an
+ error for anything other than 0 or 1 as the output is likely to be
+ corrupt.
+ */
+ if (*pexitcode != 0 && *pexitcode != 1)
+return svn_error_createf(SVN_ERR_EXTERNAL_PROGRAM, NULL,
+ _("'%s' returned %d"),
+ svn_dirent_local_style(diff_cmd, pool),
+ *pexitcode);
+
+ svn_pool_destroy(subpool);
+
+ return SVN_NO_ERROR;
+}
+
svn_error_t *
svn_io_run_diff2(const char *dir,
const char *const *user_args,
Index: subversion/svn/svn.c
===
--- subversion/svn/svn.c (revision 1458203)
+++ subversion/svn/svn.c (working copy)
@@ -74,6 +74,7 @@ typedef enum svn_cl__longopt_t {
opt_config_options,
[RFC] get-deps.sh make-over
I would like to make get-deps.sh a little more informative. 1) every time it downloads something, it should tell you what it got, where it is and what it's needed for and what you're expected to do with it, if anything. For example: - downloading apt (required library) from ... - done -- -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=- - downloading Serf (optional library) from ... - renaming directory -- done -- 2) if it doesn't download something it should tell you what there is, if possible, e.g. -- Serf 1.1.1 already present in trunk/serf/ 3) A more informative --help output that introduces get-deps.sh, with subcategories that display the help for the downloads, e.g. ./get-deps.sh help serf --- Comments? Gabriela
Re: OT: $SVN envvars Re: svn 1.8 debugging-compilation-serf-configure problem because of 1.6 Ubuntu install
On 04/03/13 23:17, Daniel Shahaf wrote: Gabriela Gibson wrote on Mon, Mar 04, 2013 at 23:16:21 +: On 04/03/13 22:57, Daniel Shahaf wrote: Gabriela Gibson wrote on Mon, Mar 04, 2013 at 22:33:23 +: What I've also found during my experiments at the weekend is that apr-related entries in LD_FLAGS are edited while configure runs. So: LD_FLAGS="-L/usr/local/serf/lib -L/usr/local/apr/lib" ./configure \ --enable-maintainer-mode --with-serf=/usr/local/serf failed. Examining config.log showed that the failure appeared to occur in the link test, and that the entry for "-L/usr/local/apr/lib" has been removed. Which link test was it? It's hard to make any comment about the situation without seeing the code and the reason it strips -L flags. (Is that our code, or something in autoconf's m4 library, that strips the -L flags?) Since there is interest in the config.log: LD_FLAGS="-L/usr/local/apr -L/foo/bar/baz" ./configure LDFLAGS shouldn't have an underscore? Oh. No wonder that it got dropped! %-) (Thanks, good catch. I guess I made one initial typo and then perpetuated it with M-/ -- oops!)
Re: OT: $SVN envvars Re: svn 1.8 debugging-compilation-serf-configure problem because of 1.6 Ubuntu install
On 04/03/13 22:57, Daniel Shahaf wrote: Gabriela Gibson wrote on Mon, Mar 04, 2013 at 22:33:23 +: What I've also found during my experiments at the weekend is that apr-related entries in LD_FLAGS are edited while configure runs. So: LD_FLAGS="-L/usr/local/serf/lib -L/usr/local/apr/lib" ./configure \ --enable-maintainer-mode --with-serf=/usr/local/serf failed. Examining config.log showed that the failure appeared to occur in the link test, and that the entry for "-L/usr/local/apr/lib" has been removed. Which link test was it? It's hard to make any comment about the situation without seeing the code and the reason it strips -L flags. (Is that our code, or something in autoconf's m4 library, that strips the -L flags?) Since there is interest in the config.log: LD_FLAGS="-L/usr/local/apr -L/foo/bar/baz" ./configure --with-serf=/usr/local/serf --enable-maintainer-mode configure:5535: gcc -o conftest -g -O2 -g -O2 -Wall -Wmissing-prototypes -Wstrict-prototypes -Wmissing-declarations -pthread -\ D_REENTRANT -D_GNU_SOURCE -D_LARGEFILE64_SOURCE -I/home/g/trunk3/apr/include -I/home/g/trunk3/apr-util/include -I/usr/local/s\ erf/include/serf-1-L/usr/local/serf/lib conftest.c -lserf-1 -L/home/g/trunk3/apr-util -laprutil-1 -L/home/g/trunk3/apr -lapr-\ 1 -lz >&5 conftest.c:28:1: warning: function declaration isn't a prototype [-Wstrict-prototypes] conftest.c:30:1: warning: function declaration isn't a prototype [-Wstrict-prototypes] /usr/bin/ld: cannot find -laprutil-1 /usr/bin/ld: cannot find -lapr-1 collect2: ld returned 1 exit status
Re: OT: $SVN envvars Re: svn 1.8 debugging-compilation-serf-configure problem because of 1.6 Ubuntu install
On 04/03/13 22:57, Daniel Shahaf wrote: Gabriela Gibson wrote on Mon, Mar 04, 2013 at 22:33:23 +: What I've also found during my experiments at the weekend is that apr-related entries in LD_FLAGS are edited while configure runs. So: LD_FLAGS="-L/usr/local/serf/lib -L/usr/local/apr/lib" ./configure \ --enable-maintainer-mode --with-serf=/usr/local/serf failed. Examining config.log showed that the failure appeared to occur in the link test, and that the entry for "-L/usr/local/apr/lib" has been removed. Which link test was it? It's hard to make any comment about the situation without seeing the code and the reason it strips -L flags. (Is that our code, or something in autoconf's m4 library, that strips the -L flags?) I think there is only check for serf.h, and one check for compiling. The compile test has an extra "-L/usr/local/serf/lib" flag, so I've mentally referred to it as "the link test". I've provided the entire serf section of the config.log file in a later email, but the section I was referring to is: configure:5510: checking for serf_context_create in -lserf-1 configure:5535: gcc -o conftest -g -O2 -g -O2 -Wall -Wmissing-prototypes -Wstrict-prototypes -Wmissing-declarations -pthread -D_REENTRANT -D_GNU_SOURCE -D_LARGEFILE64_SOURCE -I/home/g/trunk3/apr/include -I/home/g/trunk3/apr-util/include -I/usr/local/serf/include/serf-1-L/usr/local/serf/lib conftest.c -lserf-1 -L/home/g/trunk3/apr-util -laprutil-1 -L/home/g/trunk3/apr -lapr-1 -lz >&5 conftest.c:28:1: warning: function declaration isn't a prototype [-Wstrict-prototypes] conftest.c:30:1: warning: function declaration isn't a prototype [-Wstrict-prototypes] /usr/bin/ld: cannot find -laprutil-1 /usr/bin/ld: cannot find -lapr-1 collect2: ld returned 1 exit status The only way I could make the serf link test pass (and also add that "-L/usr/local/apr/lib" flag) was to supply "--with-apr --with-apr-util" and that was what my patch documented. That's.. odd. Adding "--with-apr" (without argument) should actually be a no-op, since APR is a mandatory dependency. Ah, bad comms on my part. I've always used --with-apr=/usr/local/apr --with apr-util=/usr/local/apr, rather than naked arguments. I didn't realise that was misleading.
Re: OT: $SVN envvars Re: svn 1.8 debugging-compilation-serf-configure problem because of 1.6 Ubuntu install
On 04/03/13 22:48, Ben Reser wrote:
Can you provide the output of the serf sections in config.log?
./configure --enable-maintainer-mode --with-serf=/usr/local/serf
configure:5459: result: yes
configure:5491: serf library configuration via prefix
configure:5501: checking serf.h usability
configure:5501: gcc -c -g -O2 -g -O2 -Wall -Wmissing-prototypes
-Wstrict-prototypes -Wmissing-declarations -pthread -D_REENTRANT
-D_GNU_SOURCE -D_LARGEFILE64_SOURCE -I/home/g/trunk3/apr/include
-I/home/g/trunk3/apr-util/include -I/usr/local/serf/include/serf-1
conftest.c >&5
configure:5501: $? = 0
configure:5501: result: yes
configure:5501: checking serf.h presence
configure:5501: gcc -E -D_REENTRANT -D_GNU_SOURCE
-D_LARGEFILE64_SOURCE -I/home/g/trunk3/apr/include
-I/home/g/trunk3/apr-util/include -I/usr/local/serf/include/serf-1
conftest.c
configure:5501: $? = 0
configure:5501: result: yes
configure:5501: checking for serf.h
configure:5501: result: yes
configure:5510: checking for serf_context_create in -lserf-1
configure:5535: gcc -o conftest -g -O2 -g -O2 -Wall
-Wmissing-prototypes -Wstrict-prototypes -Wmissing-declarations -pthread
-D_REENTRANT -D_GNU_SOURCE -D_LARGEFILE64_SOURCE
-I/home/g/trunk3/apr/include -I/home/g/trunk3/apr-util/include
-I/usr/local/serf/include/serf-1-L/usr/local/serf/lib conftest.c
-lserf-1 -L/home/g/trunk3/apr-util -laprutil-1 -L/home/g/trunk3/apr
-lapr-1 -lz >&5
conftest.c:28:1: warning: function declaration isn't a prototype
[-Wstrict-prototypes]
conftest.c:30:1: warning: function declaration isn't a prototype
[-Wstrict-prototypes]
/usr/bin/ld: cannot find -laprutil-1
/usr/bin/ld: cannot find -lapr-1
collect2: ld returned 1 exit status
configure:5535: $? = 1
configure: failed program was:
| /* confdefs.h */
| #define PACKAGE_NAME "subversion"
| #define PACKAGE_TARNAME "subversion"
| #define PACKAGE_VERSION "1.8.0"
| #define PACKAGE_STRING "subversion 1.8.0"
| #define PACKAGE_BUGREPORT "http://subversion.apache.org/";
| #define PACKAGE_URL ""
| #define SVN_SOVERSION 0
| #define STDC_HEADERS 1
| #define HAVE_SYS_TYPES_H 1
| #define HAVE_SYS_STAT_H 1
| #define HAVE_STDLIB_H 1
| #define HAVE_STRING_H 1
| #define HAVE_MEMORY_H 1
| #define HAVE_STRINGS_H 1
| #define HAVE_INTTYPES_H 1
| #define HAVE_STDINT_H 1
| #define HAVE_UNISTD_H 1
| #define HAVE_SERF_H 1
| /* end confdefs.h. */
|
| /* Override any GCC internal prototype to avoid an error.
|Use char because int might match the return type of a GCC
|builtin and then its argument prototype would still apply. */
| #ifdef __cplusplus
| extern "C"
| #endif
| char serf_context_create ();
| int
| main ()
| {
| return serf_context_create ();
| ;
| return 0;
| }
configure:5545: result: no
configure:5604: serf library configuration via pkg-config
configure:5608: checking for serf-2 library
configure:5634: result: no
configure:5608: checking for serf-1 library
configure:5634: result: no
configure:5642: checking was serf enabled
configure:5648: result: no
configure:5658: error: Serf was explicitly enabled but an appropriate
version was not found.
Re: OT: $SVN envvars Re: svn 1.8 debugging-compilation-serf-configure problem because of 1.6 Ubuntu install
On 04/03/13 19:44, Daniel Shahaf wrote: > Do you use the system's apr (e.g., /usr/lib/libapr-1.so) or compile your > own? If the former you should install the "apr-dev" (or similar) > package, if the latter it appears you need to pass --with-apr to > serf/serfmake (or serf/configure if you use that). I guess I should tell you how my setup looks. I'm possibly repeating stuff from earlier emails, because I'm trying to put all the info in one place. I started with an installed package of svn for Ubuntu 12.04 Desktop on a clean operating system install: g@musashi:~$ dpkg -l | grep subversion ii subversion 1.6.17dfsg-3ubuntu3 Advanced version control system Beyond this, I've been working with a trunk and the dependencies as downloaded by get-deps.sh - pretty much the classic minimal dev env. I chose to do this, because I thought one way to assist the project was to look through the eyes of someone coming new to Subversion dev as much as possible. This weekend, in order to tackle the compilation of Serf, I did "sudo make install" in the apr and apr-util directories, and compiled Serf with the following commands: ## Serf compile command ./configure make sudo make install OTOH, when I tried to use your suggestion with ./serfmake, I get a failure g@musashi:~/trunk3/serf$ sudo ./serfmake --prefix=/usr/local/serf \ build check install ERROR: exception: ERROR: A configuration script was not found: apr-1-config serfmake [cmd] [options] Commands: build Builds (default) check Runs test cases install Installs serf into PREFIX clean Cleans Options: --with-apr=PATH prefix for installed APR and APR-util (needs apr-1-config and apu-1-config; will look in PATH) --with-gssapi=PATH build serf with GSSAPI support (needs krb5-config; will look in PATH/bin) --prefix=PATH install serf into PATH (default: /usr/local) Quick guide: serfmake --prefix=/usr/local/serf --with-apr=/usr/local/apr install g@musashi:~/trunk3/serf$ Providing --with-apr: g@musashi:~/trunk3/serf$ sudo ./serfmake --prefix=/usr/local/serf \ --with-apr=/usr/local/apr build check install works. So that's the behaviour of the serf compile. Sorry for the wordy-ness, but I'm trying to get everything into one place! What I've also found during my experiments at the weekend is that apr-related entries in LD_FLAGS are edited while configure runs. So: LD_FLAGS="-L/usr/local/serf/lib -L/usr/local/apr/lib" ./configure \ --enable-maintainer-mode --with-serf=/usr/local/serf failed. Examining config.log showed that the failure appeared to occur in the link test, and that the entry for "-L/usr/local/apr/lib" has been removed. The only way I could make the serf link test pass (and also add that "-L/usr/local/apr/lib" flag) was to supply "--with-apr --with-apr-util" and that was what my patch documented. Whether this is "undocumented behaviour", "a bug in the build system", "there is a need to document a requirement for apr-dev", or "my retarded/invalid build environment" I don't feel sufficiently experienced to judge. I just thought - this isn't right, so submit a patch. It might not be the correct fix :) Ben wrote > In your case if you've installed serf in /usr/local/serf, I'd try > building without any --with-serf option or --with-serf=yes. Failing > that try the same build but with the following environment variable > set: > export PKG_CONFIG_PATH=/usr/local/serf/lib/pkgconfig I'm not familiar with pkgconfig, so I need to educate myself. I tried yesterday without any option, and serf isn't tested for. I've also tried the --with-serf=yes suggestion: g@musashi:~/trunk3$ ./configure --with-serf=yes --enable-maintainer-mode This failed (as in configure warned that serf hadn't been found). I've further tried with the variable: export PKG_CONFIG_PATH=/usr/local/serf/lib/pkgconfig ./configure --with-serf=yes --enable-maintainer-mode and this succeeds. G
Re: OT: $SVN envvars Re: svn 1.8 debugging-compilation-serf-configure problem because of 1.6 Ubuntu install
On 04/03/13 19:02, Daniel Shahaf wrote: ./serfmake --prefix=/usr/local/serf build check install Sorry, I should have mentioned that properly -- serf is installed at this point. But I double checked, and for me, it is still broken. Btw, those instructions no longer work for the new serf, look: g@musashi:~/trunk6$ cd serf g@musashi:~/trunk6/serf$ ./serfmake --prefix=/usr/local/serf build check install ERROR: exception: ERROR: A configuration script was not found: apr-1-config serfmake [cmd] [options] Commands: build Builds (default) check Runs test cases install Installs serf into PREFIX clean Cleans Options: --with-apr=PATH prefix for installed APR and APR-util (needs apr-1-config and apu-1-config; will look in PATH) --with-gssapi=PATH build serf with GSSAPI support (needs krb5-config; will look in PATH/bin) --prefix=PATH install serf into PATH (default: /usr/local) Quick guide: serfmake --prefix=/usr/local/serf --with-apr=/usr/local/apr install
Re: OT: $SVN envvars Re: svn 1.8 debugging-compilation-serf-configure problem because of 1.6 Ubuntu install
On 04/03/13 18:09, Daniel Shahaf wrote: Gabriela Gibson wrote on Mon, Mar 04, 2013 at 17:51:15 +: +However, to compile serf with Subversion, the following configure +flags are also required: + + --with-apr=/path/to/apr/install + --with-apr-util=/path/to/apr-util/install + First of all, are these options to svn's configure or to serf's configure? Serf requires them, but in this case, the Subversion build is meant. In either case, I don't understand why they would be required. apr* are mandatory dependencies, and the --with-apr argument should only serve to tell configure where the APR installation it should use use, i.e., to skip looking for apr in the standard locations (/usr, /usr/local, etc). Can you explain? That would be overambitious! :) :) :) I simply thought that because serf needs those switches in it's own compile, I try that, and I got lucky. However, here is what happens for me: Steps to reproduce: svn co https://svn.apache.org/repos/asf/subversion/trunk trunk3 cd trunk3 ./get-deps.sh cd apr ./buildconf cd ../apr-util/ ./buildconf cd .. ./autogen.sh ./configure --enable-maintainer-mode --with-serf=/usr/local/serf Error message: checking for inttypes.h... yes checking for stdint.h... yes checking for unistd.h... yes configure: serf library configuration via prefix checking serf.h usability... yes checking serf.h presence... yes checking for serf.h... yes checking for serf_context_create in -lserf-1... no configure: serf library configuration via pkg-config checking for serf-2 library... no checking for serf-1 library... no checking was serf enabled... no An appropriate version of serf could not be found, so libsvn_ra_serf will not be built. If you want to build libsvn_ra_serf, please install serf 1.2.0 or newer. configure: error: Serf was explicitly enabled but an appropriate version was not found. g@musashi:~/trunk3$
Re: OT: $SVN envvars Re: svn 1.8 debugging-compilation-serf-configure problem because of 1.6 Ubuntu install
On 01/03/13 22:53, Daniel Shahaf wrote:
BTW, I do 'alias -g \$svn=\$SVN' so that the envvar is called "SVN"
(scripts look for that) but interactively I can type '$svn' in
lowercase.
_create_aliases() {
thank you for that :)
The instructions on building serf were a little outdated, but I got it
all to work in the end.
(see attached patch and log)
Fix the serf install information available throughout the build
system, specifically mentioning that the additional ./configure flags
--with-apr and --with-apr-util flags are needed at the same time as
--with-serf.
* trunk/get-deps.sh (serf): add echo statements to avoid user
pitfalls.
* trunk/build/ac-macros/serf.m4 (AC_ARG_WITH): Remove outdated
statement that the serf compile will occur automatically if serf is
present and improve ./configure flag warning regarding --with-serf
failure.
* trunk/INSTALL (Dependencies in Detail): Modify ./configure flag
advice regarding Serf.
* subversion/libsvn_ra_serf/README: Modifiy./configure flag
advice regarding Serf.
Index: get-deps.sh
===
--- get-deps.sh (revision 1452302)
+++ get-deps.sh (working copy)
@@ -80,6 +80,8 @@ get_serf() {
bzip2 -dc $TEMPDIR/$SERF.tar.bz2 | tar -xf -
mv $SERF serf
+
+echo "Serf has been downloaded, but must be manually compiled and installed."
}
get_zlib() {
Index: subversion/libsvn_ra_serf/README
===
--- subversion/libsvn_ra_serf/README (revision 1452302)
+++ subversion/libsvn_ra_serf/README (working copy)
@@ -12,14 +12,15 @@ The latest serf releases can be fetched at:
The latest serf sources can be fetched via SVN at:
http://serf.googlecode.com/svn/trunk/
-ra_serf can be enabled with the following configure flags:
+ra_serf can be enabled with the following configure flag:
"--with-serf=/path/to/serf/install"
-As Neon is currently Subversion's default RA DAV layer, you also need
-to add "http-library = serf" to your ~/.subversion/servers file to
-choose ra_serf at runtime. Alternately, you can build with only
-support for ra_serf:
- "--without-neon --with-serf=/path/to/serf/install"
+However, to compile serf with Subversion, the following configure
+flags are also required:
+
+ --with-apr=/path/to/apr/install
+ --with-apr-util=/path/to/apr-util/install
+
For more about how ra_serf/ra_neon talk WebDAV, consult notes/webdav-protocol.
Working copies are interchangable between ra_serf and ra_neon. (They both use
Index: build/ac-macros/serf.m4
===
--- build/ac-macros/serf.m4 (revision 1452302)
+++ build/ac-macros/serf.m4 (working copy)
@@ -51,7 +51,7 @@ AC_DEFUN(SVN_LIB_SERF,
serf_check_version="$1.$2.$3"
AC_ARG_WITH(serf,AS_HELP_STRING([--with-serf=PREFIX],
- [Serf HTTP client library (enabled by default if found)]),
+ [Serf HTTP client library, (req. --with-apr & --with-apr-util flags)]),
[
if test "$withval" = "yes" ; then
serf_required=yes
Index: INSTALL
===
--- INSTALL (revision 1452302)
+++ INSTALL (working copy)
@@ -314,13 +314,12 @@ I.INTRODUCTION
serf. Though optional, we strongly recommend this.
In order to use ra_serf, you must install serf, and run Subversion's
- ./configure with the argument --with-serf. If serf is installed in a
- non-standard place, you should use
+ ./configure with all of the arguments:
- --with-serf=/path/to/serf/install
+ --with-serf=/path/to/serf/install
+ --with-apr=/path/to/apr/install
+ --with-apr-util=/path/to/apr-util/install
- instead.
-
For more information on serf and Subversion's ra_serf, see the file
subversion/libsvn_ra_serf/README.
[RFC] gtest branch posted
Here is the patch for the branch I added earlier on: https://svn.apache.org/repos/asf/subversion/branches/gtest_addition/ Please let me know if there is anything I can do to improve upon this. Thanks :) Gabriela Index: INSTALL === --- INSTALL (revision 1452117) +++ INSTALL (working copy) @@ -153,6 +153,12 @@ I.INTRODUCTION configured via auto-props or the mime-types-file option take precedence. + * Googletest aka Gtest (OPTIONAL) + + This optional package is used by the tests for Subversions' + C++ bindings. + + C. Dependencies in Detail Subversion depends on a number of third party tools and libraries. @@ -566,6 +572,13 @@ I.INTRODUCTION --with-libmagic + 22. Googletest (OPTIONAL) + + Googletest can be installed and built in-tree by invoking + + $ ./get-dep.sh gtest + $ ./configure --with-gtest + D. Documentation The primary documentation for Subversion is the free book Index: Makefile.in === --- Makefile.in (revision 1452117) +++ Makefile.in (working copy) @@ -134,6 +134,8 @@ APACHE_INCLUDES = @APACHE_INCLUDES@ APACHE_LIBEXECDIR = $(DESTDIR)@APACHE_LIBEXECDIR@ APACHE_LDFLAGS = @APACHE_LDFLAGS@ +GTEST_INCLUDES = -Ilibgtest -Ilibgtest/include/ -Ilibgtest/include/gtest/internal -Ilibgtest/include/gtest + SWIG = @SWIG@ SWIG_PY_INCLUDES = @SWIG_PY_INCLUDES@ -I$(SWIG_SRC_DIR)/python/libsvn_swig_py SWIG_PY_COMPILE = @SWIG_PY_COMPILE@ @@ -182,9 +184,10 @@ SWIG_LDFLAGS = @SWIG_LDFLAGS@ $(EXTRA_SWIG_LDFLAGS COMPILE = $(CC) $(CMODEFLAGS) $(CPPFLAGS) $(CMAINTAINERFLAGS) $(CFLAGS) $(INCLUDES) COMPILE_CXX = $(CXX) $(CXXMODEFLAGS) $(CPPFLAGS) $(CXXMAINTAINERFLAGS) $(CXXFLAGS) $(INCLUDES) +COMPILE_GTEST_CXX = $(COMPILE_CXX) $(GTEST_INCLUDES) -o $@ -c LT_COMPILE = $(LIBTOOL) $(LTFLAGS) --mode=compile $(COMPILE) $(LT_CFLAGS) LT_COMPILE_CXX = $(LIBTOOL) $(LTCXXFLAGS) --mode=compile $(COMPILE_CXX) $(LT_CFLAGS) - +LT_COMPILE_GTEST_CXX = $(LIBTOOL) $(LTCXXFLAGS) --mode=compile $(COMPILE_CXX) $(LT_FLAGS) $(GTEST_INCLUDES) -o $@ -c # Execute a command that loads libraries from the build dir LT_EXECUTE = $(LIBTOOL) $(LTFLAGS) --mode=execute `for f in $(abs_builddir)/subversion/*/*.la; do echo -dlopen $$f; done` @@ -208,6 +211,8 @@ LINK = $(LIBTOOL) $(LTFLAGS) --mode=link $(CC) $(L LINK_LIB = $(LINK) $(LT_SO_VERSION) LINK_CXX = $(LIBTOOL) $(LTCXXFLAGS) --mode=link $(CXX) $(LT_LDFLAGS) $(CXXFLAGS) $(LDFLAGS) -rpath $(libdir) LINK_CXX_LIB = $(LINK_CXX) $(LT_SO_VERSION) +## LINK_GTEST_CXX = ar -rv libgtest.a src/gtest-all.lo +LINK_GTEST_CXX = $(LIBTOOL) $(LTCXXFLAGS) --mode=link $(CXX) $(LT_LDFLAGS) $(CXXFLAGS) $(LDFLAGS) -rpath $(libdir) # special link rule for mod_dav_svn LINK_APACHE_MOD = $(LIBTOOL) $(LTFLAGS) --mode=link $(CC) $(LT_LDFLAGS) $(CFLAGS) $(LDFLAGS) -rpath $(APACHE_LIBEXECDIR) -avoid-version -module $(APACHE_LDFLAGS) Index: build/generator/gen_base.py === --- build/generator/gen_base.py (revision 1452117) +++ build/generator/gen_base.py (working copy) @@ -431,6 +431,8 @@ class TargetLinked(Target): if glob.glob(src): if src[-2:] == '.c': objname = src[:-2] + self.objext + elif src[-3:] == '.cc': +objname = src[:-3] + self.objext elif src[-4:] == '.cpp': objname = src[:-4] + self.objext else: Index: build.conf === --- build.conf (revision 1452117) +++ build.conf (working copy) @@ -409,7 +409,6 @@ type = sql-header path = subversion/libsvn_subr sources = internal_statements.sql - # # # TARGETS FOR I18N SUPPORT @@ -657,8 +656,26 @@ install = tests compile-cmd = $(COMPILE_CXXHL_CXX) link-cmd = $(LINK_CXX) + # # +# Gtest targets +# + +# renamed from gtest to libgtest because libtool couldn't output +# a library that didn't have the prefix 'lib' +[libgtest] +description = Gtest Test Suite +type = lib +path = libgtest +headers = include/gtest +sources = src/gtest-all.cc +install = libgtest-install +compile-cmd = $(LT_COMPILE_GTEST_CXX) +link-cmd = $(LINK_CXX) + +# +# # TESTING TARGETS # Index: configure.ac === --- configure.ac (revision 1452117) +++ configure.ac (working copy) @@ -625,7 +625,23 @@ fi AC_SUBST(SVN_GNOME_KEYRING_INCLUDES) AC_SUBST(SVN_GNOME_KEYRING_LIBS) +dnl gtest - +AC_ARG_ENABLE([gtest], + [AS_HELP_STRING([--enable-gtest], + [Enable tests using the Google C++ Testing Framework. +
svn 1.8 debugging-compilation-serf-configure problem because of 1.6 Ubuntu install
I have an old 1.6 Ubuntu installed svn, and it's blocking me from debugging because the client format is to old. Since I need a working svn to do anything, I cannot just get rid of it. This is what happens when I try to debug anything: in emacs24: M-x libtool --mode=execute gdb -i=mi --epoch /home/g/trunk/subversion/svn/svn cd ~/trunk run log but alas: "subversion/libsvn_wc/wc_db.c:8898: (apr_err=155036) subversion/libsvn_wc/wc_db_wcroot.c:737: (apr_err=155036) subversion/libsvn_wc/wc_db_wcroot.c:316: (apr_err=155036) svn: E155036: The working copy at '/home/g/trunk' is too old (format 10) to work with client version '1.8.0-dev (under development)' (expects format 31). You need to upgrade the working copy first. Process gdb-inferior killed " and the reason I was trying to get serf going is: g@musashi:~/svn/log$ SVN=~/trunk/subversion/svn/svn g@musashi:~/svn/log$ rm -rf trunk g@musashi:~/svn/log$ $SVN co https://svn.apache.org/repos/asf/subversion/trunk subversion/svn/checkout-cmd.c:168: (apr_err=17) subversion/libsvn_client/checkout.c:103: (apr_err=17) subversion/libsvn_client/ra.c:498: (apr_err=17) subversion/libsvn_client/ra.c:364: (apr_err=17) subversion/libsvn_ra/ra_loader.c:467: (apr_err=17) svn: E17: Unrecognised URL scheme for 'https://svn.apache.org/repos/asf/subversion/trunk' g@musashi:~/svn/log$ (which I assume to be a serf problem) To cut a very long story short: nothing works for me :) And btw, the configure script still claims that: -- configure: WARNING: unrecognized options: --enable-maintainer-mode, --with-serf, --with-prefix -- And serf still isn't compiling for me: ./configure --enable-maintainer-mode --with-serf=/home/g/trunk/ --with-prefix=/usr/svntest/ (also tried /home/h/trunk/serf just to be sure) -- configure: serf library configuration via pkg-config checking for serf-2 library... no checking for serf-1 library... no checking was serf enabled... no An appropriate version of serf could not be found, so libsvn_ra_serf will not be built. If you want to build libsvn_ra_serf, please install serf 1.0.0 or newer. configure: error: Serf was explicitly enabled but an appropriate version was not found. - (it clearly is present, get-deps.sh fetched and unzipped it) Also, I note the ./configure --help tells me that: --with-serf=PREFIXSerf HTTP client library (enabled by default if found) which makes me think that serf should build without me telling it to, since, it's present, and it I guess the problem is that it's not found? Phillip said that he doesn't see that 'configure warning' happening for him, maybe it's an Ubuntu specific problem and a hint why things are not working out? many thanks for any advice, Gabriela
Re: [PATCH] htaccess file and permissions
On 28/02/13 14:42, C. Michael Pilato wrote: IMO, your log message should explain the reason for and value of such a sweeping change. The diff already tells us what you've done -- developers coming a few years after us will want to know *why* it was done. How about: [[[ Update web server configuration file and change all the file permissions to remove the executable bit. "AddOutputFilter INCLUDES .html" is provided by httpd.conf and this makes the "XBitHack On" setting unnecessary. Similarly, the file permissions of the *.html files that are set to 'executable' are not necessary. See: http://svn.haxx.se/dev/archive-2013-02/0274.shtml for the discussion. * site/publish/.htaccess(): remove "XBitHack On". * site/* remove svn:executable property from all files. Patch by: Gabriela Gibson Suggested by: danielsh ]]]
[PATCH] htaccess file and permissions
[[[ Update web server configuration file and change file permissions to remove the executable bit. * site/publish/.htaccess(): remove "XBitHack On". * site/* remove svn:executable property from all files, with the exception of 'download/download.cgi'. Patch by: Gabriela Gibson Suggested by: danielsh ]]] Property changes on: publish/faq.ja.html ___ Deleted: svn:executable - * Property changes on: publish/security/index.html ___ Deleted: svn:executable - * Property changes on: publish/download/download.html ___ Deleted: svn:executable - * Property changes on: publish/download/index.html ___ Deleted: svn:executable - * Property changes on: publish/opw.html ___ Deleted: svn:executable - * Property changes on: publish/contributing.html ___ Deleted: svn:executable - * Property changes on: publish/faq.html ___ Deleted: svn:executable - * Property changes on: publish/index.html ___ Deleted: svn:executable - * Property changes on: publish/source-code.html ___ Deleted: svn:executable - * Index: publish/.htaccess === --- publish/.htaccess (revision 1450368) +++ publish/.htaccess (working copy) @@ -1,7 +1,6 @@ # duplicated in httpd.conf in r795618 Options +Includes -XBitHack On RedirectMatch ^/buildbot(.*) http://ci.apache.org/waterfall?\ \&show=bb-openbsd\ \&show=svn-slik-w2k3-x64-local\ Property changes on: publish/TEMPLATE ___ Deleted: svn:executable - * Property changes on: publish/dev/index.html ___ Deleted: svn:executable - * Property changes on: publish/mailing-lists.html ___ Deleted: svn:executable - * Property changes on: publish/news.html ___ Deleted: svn:executable - * Property changes on: publish/features.html ___ Deleted: svn:executable - * Property changes on: publish/packages.html ___ Deleted: svn:executable - * Property changes on: publish/reporting-issues.html ___ Deleted: svn:executable - * Property changes on: publish/roadmap.html ___ Deleted: svn:executable - * Property changes on: publish/pronunciation/index.html ___ Deleted: svn:executable - * Property changes on: publish/docs/api/1.6/installdox ___ Deleted: svn:executable - * Property changes on: publish/docs/api/1.7/installdox ___ Deleted: svn:executable - * Property changes on: publish/docs/index.html ___ Deleted: svn:executable - * Property changes on: publish/docs/community-guide/issues.html ___ Deleted: svn:executable - * Property changes on: publish/docs/community-guide/guide-nav.html ___ Deleted: svn:executable - * Property changes on: publish/docs/community-guide/debugging.toc.html ___ Deleted: svn:executable - * Property changes on: publish/docs/community-guide/community-guide.html ___ Deleted: svn:executable - * Property changes on: publish/docs/community-guide/building.toc.html ___ Deleted: svn:executable - * Property changes on: publish/docs/community-guide/general.part.html ___ Deleted: svn:executable - * Property changes on: publish/docs/community-guide/releasing.toc.html ___ Deleted: svn:executable - * Property changes on:
Re: [RFC] Build System Documentation
On 24/02/13 17:57, Branko Čibej wrote: On 24.02.2013 18:21, Gabriela Gibson wrote: "svn diff" on which target? :) Well, "svn diff -c 858288" works great, so I assumed that it's legit :> (now I have to wonder _why_ this works, heh) If I run your command on the root of a trunk checkout, the result is empty, as expected, since there are no trunk changes in there. However, if I say, Hmm, why should svn care about current changes and where I am when I ask it to show me the changes of revision n?
Re: [RFC] Build System Documentation
On 22/02/13 00:45, Daniel Shahaf wrote: How about just linking to a revnum wherein a standalone application was added? That's less likely to get out of date. For example % svn log -qv -l1 -r 1:HEAD subversion/tests/cmdline/atomic-ra-revprop-change.c r965054 | danielsh | 2010-07-17 14:23:38 +0300 (Sat, 17 Jul 2010) Changed paths: M /subversion/branches/atomic-revprop/build.conf M /subversion/branches/atomic-revprop/subversion/tests/cmdline A /subversion/branches/atomic-revprop/subversion/tests/cmdline/atomic-ra-revprop-change.c ... when I then try: svn diff -c 965054 I get a silent failure. However, svn blame subversion/tests/cmdline/atomic-ra-revprop-change.c clearly shows that the revision 965054 has contents. What is going on?
Re: [RFC] Build System Documentation
On 22/02/13 20:09, Daniel Shahaf wrote: Gabriela Gibson wrote on Fri, Feb 22, 2013 at 17:13:08 +: On 20/02/13 21:06, Ben Reser wrote: You skipped the primary function of gen-make.py which is caused by this call "generator.write()". There are two gen-make.py files -- I was describing /trunk/gen-make.py. It may be worthwhile to rename the /trunk/gen-make.py if that is possible. There's just one. build/generator/gen_make.py has an underscore. I think that there is too little difference between '_' and '-'. That said, does the '-' have a different meaning than '_'? (In the same spirit that anything starting '__' is not for public consumption.)
Re: [RFC] Build System Documentation
On 20/02/13 21:06, Ben Reser wrote: On Mon, Feb 18, 2013 at 4:10 AM, Gabriela Gibson wrote: Thank you for the corrections and ideas, they have been worked into the document. Also typo of recognized/recognised (depending on preference US vs UK spelling). What is the preferred spelling for Subversion projects? It's nicer to stick to one or the other. You can easily see the difference by running `./autogen.sh` saving the build-outputs.mk and then running `./autogen.sh -s` and comparing the results. Should there be a section on how and when to run the various parts of the build system? You skipped the primary function of gen-make.py which is caused by this call "generator.write()". There are two gen-make.py files -- I was describing /trunk/gen-make.py. It may be worthwhile to rename the /trunk/gen-make.py if that is possible. I'm working on a new section to briefly describe the Python suite. [libsvn_fs_base] implementation of the BDB backend. [libsvn_fs_fs] implementation of the fsfs backend. [libsvn_ra_local] Accessing repositories via direct libsvn_fs More accurately: Accessing repositories via direct libsvn_repos. Primarily this is implemented on top of libsvn_repos but some things end up using functions in libsvn_fs. So I wouldn't mention libsvn_fs. PATCH idea: I took the examples from the comments in build.conf, and I could update build.conf comments to incorporate your corrections for the comment less entries.
Re: [RFC] Build System Documentation
On 22/02/13 00:45, Daniel Shahaf wrote: % svn log -qv -l1 -r 1:HEAD subversion/tests/cmdline/atomic-ra-revprop-change.c r965054 | danielsh | 2010-07-17 14:23:38 +0300 (Sat, 17 Jul 2010) Changed paths: M /subversion/branches/atomic-revprop/build.conf M /subversion/branches/atomic-revprop/subversion/tests/cmdline A /subversion/branches/atomic-revprop/subversion/tests/cmdline/atomic-ra-revprop-change.c ... Thanks, I exchanged examples. I will add gtest in the same way as the external example, once it is ready. (which already shows that you forgot to show the svn:ignore property mods) I'm not sure what you mean here? More accurately, "configuration files for the hudson buildslaves we once tried to configure". They're not part of the build system. Ok, I removed it from the doc. (should those files be removed from trunk/build ?)
Re: svn commit: r1448847 - in /subversion/site: ./ publish/docs/community-guide/
On 21/02/13 23:18, Daniel Shahaf wrote: On Thu, Feb 21, 2013 at 11:12:27PM -, g...@apache.org wrote: Patch by: Gabriela Gibson Also, please add an "Approved by: danielsh" tag, and add yourself to trunk/COMMITTERS. Almost all done(rest happens tomorrow), thank you very much for all the help! :)
Build System: autogen.sh header warnings when compiling with gtest
Ben Reser asked me in the "[RFC] Build System Documentation thread" about the gtest example: " > -mv $GTEST gtest > +mv $GTEST libgtest > +echo "Gtest has been installed, please note:" > +echo "autogen.sh will issue spurious header warnings." > +echo "./configure --enable-gtest will issue repeated spurious warnings > that" > +echo "the option --enable-gtest is not recognsed." Why is that? I haven't really been following the gtest work that much but the spurious warnings issue seems like an issue in what you did rather than something you should be adding documentation around." The header issue: autogen.sh complains with this error message unless the headers get added to the private includes in build.conf at line 37, where gen-make.py picks it up, but if the optional gtest package is not around, it crashes the compile. ./configure warnings: Bug report has been filed now.
Re: [PATCH] Procedure for making web page changes
On 20/02/13 18:54, Daniel Shahaf wrote: Gabriela Gibson wrote on Wed, Feb 20, 2013 at 16:22:36 +: On 20/02/13 15:28, Gabriela Gibson wrote: +++ publish/.htaccess (working copy) @@ -1,7 +1,6 @@ # duplicated in httpd.conf in r795618 Options +Includes -XBitHack On RedirectMatch ^/buildbot(.*) http://ci.apache.org/waterfall?\ \&show=bb-openbsd\ \&show=svn-slik-w2k3-x64-local\ You might want to commit this part separately? It isn't as much related to the other changes as something we found while working on them. True. Removed and queued. +++ publish/docs/community-guide/community-guide.html (working copy) @@ -73,6 +77,7 @@ first. You seem to have added "web" before "debugging" here, while it's after "debugging" everywhere else. Oops. Thanks, and well spotted! +++ publish/docs/community-guide/web.part.html (revision 0) A couple of minor comments about this file: +Putting it all together, an example VirtualHost configuration is: + +<VirtualHost *:8080> +ServerAdmin webmaster@localhost Consider putting this example in a separate *.conf file, to make it easier to reuse? I've left it on for now, but I'm wondering if you find it too spammy this way or it's just the convenience of the *.conf file? We can do both, but indeed, there is a lot to be said for brevity. 'diff -u ~/projects/svn/site/**/svnsite.conf /etc/apache2/svnsite.conf' +then upload the resulting file to a HTML validatory, for "validatory"? Yes :) It's an unusual but technically correct term. I've left it in for now, because one thing about documentation is that it's a little boring to read, and interesting wording helps people remember things easier. Addition of section to Community Guide describing procedure for Use the imperative: "Add a section" Fixed. You didn't include README in the patch. Oops. Fixed. Forgot to 'svn add'. Bottom line: once the points about README and .htaccess are addressed, Done. +1 to commit. (Everything else can be addressed post-commit if you disagree with the review.) Thanks or the patch! To sum up: 1) .htaccess and permission changes will go into a separate patch. 2) With view to a further patch, the term 'Validatory' is still being discussed, and we need to decide is the conf stays on the page and/or is moved to it's own file, linked from this spot. [[[ Add a section to Community Guide describing procedure for obtaining the source for the Subversion web site. Addition of navigation links to new page within community guide. Add three html files creating one extra page on web site accessible at /docs/community-guide/web.html. Also add README in top level directory providing link to new web page. * publish/docs/community-guide/web.html (New page): Provide the top-level html page for the new page. * publish/docs/community-guide/web.toc.html (New page): Provide table-of-contents for new page. * publish/docs/community-guide/web.part.html (New page): Provide body text for new page (Introduction): Provide brief overview of structure of web pages on subversion site. (web_sources): Provide instructions for accessing web sources via https or svn. (web_mirror): Provide instructions for creating mirror of Subversion site with Apache. (web_validation): Provide instructions for validating changes. (web_patch_creation): Describe format of commit log message for web changes. * README (New file): Provide link to new /docs/community-guide/web.html page. * publish/docs/community-guide/releasing.html (site-content): Add SSI variable pointing to web.html. * publish/docs/community-guide/issues.html (site-content): Add SSI variable pointing to web.html. * publish/docs/community-guide/guide-nav.html (topmenu): Add SSI variable pointing to web.html. * publish/docs/community-guide/community-guide.html (site-content): Add SSI variable pointing to web.html. (Table of Contents): Add link to web.toc.html. * publish/docs/community-guide/conventions.html (site-content): Add SSI variable pointing to web.html. * publish/docs/community-guide/index.html (Table of Contents): Add link to web.html. * publish/docs/community-guide/l10n.html (site-content): Add SSI variable pointing to web.html. * publish/docs/community-guide/mailing-lists.html (site-content): Add SSI variable pointing to web.html. * publish/docs/community-guide/general.html (site-content): Add SSI variable pointing to web.html. * publish/docs/community-guide/roles.html (site-content): Add SSI variable pointing to web.html. * publish/docs/community-guide/debugging.html (site-content): Add SSI variable pointing to web.html. * publish/docs/community-guide/building.html (site-content): Add SSI variable pointing to web.html. Patch by: Gabriela Gibson ]]] Index: publish/docs/co
