[PATCH] Fix Makefile.svn to build APR with threads
Hi, Here's a quick patch to fix Makefile.svn. I need a +1 to commmit this. Thanks. [[[ Makefile.svn: Fix build * tools/dev/unix-build/Makefile.svn: Configure APR to build with threads enabled to prevent breaking the Subversion build. ]]] Index: tools/dev/unix-build/Makefile.svn === --- tools/dev/unix-build/Makefile.svn (revision 1040428) +++ tools/dev/unix-build/Makefile.svn (working copy) @@ -238,8 +238,7 @@ cd $(APR_OBJDIR) \ env CFLAGS=-O0 -g $(APR_SRCDIR)/configure \ --prefix=$(PREFIX)/apr \ - --enable-maintainer-mode \ - --disable-threads + --enable-maintainer-mode touch $@ # compile apr
Re: 1.7.x bug - svn add no longer accepts wildcards?
On 30.11.2010 05:32, Daniel Becroft wrote: Hi, During testing of a 1.7.x build, I've noticed that 'svn add' on trunk no longer accepts wildcards: svn 1.6.x: D:\temp\svn_sandpit\workingcopy\trunk\Aecho 111 1alpha.txt D:\temp\svn_sandpit\workingcopy\trunk\Aecho 111 1beta.txt D:\temp\svn_sandpit\workingcopy\trunk\Asvn add *.txt A alpha.txt A beta.txt Whereas 1.7.x gives: D:\temp\svn_sandpit\workingcopy\trunk\Aecho 111 1alpha.txt D:\temp\svn_sandpit\workingcopy\trunk\Aecho 111 1beta.txt D:\temp\svn_sandpit\workingcopy\trunk\AD:\temp\svn_sandpit\svn7.exe add *.txt svn: warning: 'D:\temp\svn_sandpit\workingcopy\trunk\A\*.txt' not found I thought that wildcards were expanded by the OS/Shell, and then passed to SVN. Is this not the case? Not on windows, you have to link a magic library into your code to get that behaviour. -- Brane
Re: If RCS can stand it, why can ’t your system?
Hi Eric, Greg Stein writes: We have an import/export format that has existed since svn 1.0, and that is our dumpfile format. I do not think we are required or need to be shamed into supporting additional formats. And for what it's worth, a *git* GSoC student wrote a remote dumpfile generator to feed into git's fast-import. We gave the guy commit access to *our* trunk and let him build the tool. Yep, that's completely true. I am that GSoC student, and subversion/svnrdump is complete. The Subversion community was supportive of it. And yes, I wrote both dump and load functionality. So there is some factual inaccuracy in your statement: Subversion is typical in that it has a proof-of-concept third-party exporter that loses some metadata, and no importer We have also finished building a converter from dumpfilev3 - git fast-import stream, and are currently working on building the reverse. That way, there should be no bias and people should be able to move into Subversion as easily as they can move out of it. At most, you could criticize the fact that we weren't able to build it earlier/ faster. Yes, it was a very non-trivial task (atleast for me). Also: we have extended our diff generation to support git's diff markers. Indeed. Daniel did this as part of another GSoC project. `svn diff --git` works perfectly now. In short, there is NO WAY anybody can say we have something against git. If somebody came along with code to deal with git's streams, then we'd consider it, and the utility it may provide over our own dumpfile format. Since Subversion already has so much infrastructure to deal with deltified dumpfile, I'd vote to keep that as the native import/export format. -- Ram
[PATCH] enhance diff to make it behave uniformly
Hi,
In my previous patch [1] I suggested the addition of the
'--diff-copy-from' switch to 'svn diff' to solve the cases where 'svn
diff' would do the diff against the copyfrom source file when a file is
reinstated or diff target is *not* explicit file.
But since there is already a point that diff behaves differently in
different scenarios, making 'svn diff' work the same in all scenarios is
one good option. This patch would solve the problem of making 'svn diff'
work the same in all conditions by passing 'TRUE' for the
'diff_copy_from' option by default. Hence I removed the
'--diff-copy-from' switch in this patch. This is a more generic way to
bridge the gap.
I am attaching the patch and the log with this mail. Please review and
comment on the same.
I still retain this param on svn_client_diff5 and svn_ra_diff4 in case
somebody wants old behaviour, otherwise I can remove that too.
Regards
Prabhu
[1]http://mail-archives.apache.org/mod_mbox/subversion-dev/201011.mbox/%
3c1291018400.4021.3.ca...@prabhugnanasundar%3e
Index: subversion/libsvn_ra/deprecated.c
===
--- subversion/libsvn_ra/deprecated.c (revision 1040016)
+++ subversion/libsvn_ra/deprecated.c (working copy)
@@ -248,6 +248,31 @@
keep_locks, pool);
}
+svn_error_t *svn_ra_do_diff3(svn_ra_session_t *session,
+ const svn_ra_reporter3_t **reporter,
+ void **report_baton,
+ svn_revnum_t revision,
+ const char *diff_target,
+ svn_depth_t depth,
+ svn_boolean_t ignore_ancestry,
+ svn_boolean_t text_deltas,
+ const char *versus_url,
+ const svn_delta_editor_t *diff_editor,
+ void *diff_baton,
+ apr_pool_t *pool)
+{
+ SVN_ERR_ASSERT(svn_path_is_empty(diff_target)
+ || svn_path_is_single_path_component(diff_target));
+ return session-vtable-do_diff(session,
+ reporter, report_baton,
+ revision, diff_target,
+ depth, ignore_ancestry,
+ text_deltas, FALSE /* diff copy from */,
+ versus_url, diff_editor,
+ diff_baton, pool);
+}
+
+
svn_error_t *svn_ra_do_diff2(svn_ra_session_t *session,
const svn_ra_reporter2_t **reporter,
void **report_baton,
@@ -270,8 +295,8 @@
(b-reporter3), (b-reporter3_baton),
revision, diff_target,
SVN_DEPTH_INFINITY_OR_FILES(recurse),
- ignore_ancestry, text_deltas, versus_url,
- diff_editor, diff_baton, pool);
+ ignore_ancestry, text_deltas, FALSE,
+ versus_url, diff_editor, diff_baton, pool);
}
svn_error_t *svn_ra_do_diff(svn_ra_session_t *session,
Index: subversion/libsvn_ra/wrapper_template.h
===
--- subversion/libsvn_ra/wrapper_template.h (revision 1040016)
+++ subversion/libsvn_ra/wrapper_template.h (working copy)
@@ -361,7 +361,7 @@
svn_depth_t depth = SVN_DEPTH_INFINITY_OR_FILES(recurse);
SVN_ERR(VTBL.do_diff(session_baton, reporter3, baton3, revision,
- diff_target, depth, ignore_ancestry, TRUE,
+ diff_target, depth, ignore_ancestry, TRUE, FALSE,
versus_url, diff_editor, diff_baton, pool));
compat_wrap_reporter(reporter, report_baton, reporter3, baton3, pool);
Index: subversion/libsvn_ra/ra_loader.c
===
--- subversion/libsvn_ra/ra_loader.c (revision 1040016)
+++ subversion/libsvn_ra/ra_loader.c (working copy)
@@ -845,7 +845,7 @@
status_editor, status_baton, pool);
}
-svn_error_t *svn_ra_do_diff3(svn_ra_session_t *session,
+svn_error_t *svn_ra_do_diff4(svn_ra_session_t *session,
const svn_ra_reporter3_t **reporter,
void **report_baton,
svn_revnum_t revision,
@@ -853,6 +853,7 @@
svn_depth_t depth,
svn_boolean_t ignore_ancestry,
svn_boolean_t text_deltas,
+ svn_boolean_t diff_copy_from,
const char *versus_url,
const svn_delta_editor_t *diff_editor,
void *diff_baton,
@@ -864,7
Re: If RCS can stand it, why can ’t your system?
Hi Eric, Eric Raymond writes: Ramkumar Ramachandra artag...@gmail.com: Yep, that's completely true. I am that GSoC student, and subversion/svnrdump is complete. The Subversion community was supportive of it. And yes, I wrote both dump and load functionality. So there is some factual inaccuracy in your statement: Subversion is typical in that it has a proof-of-concept third-party exporter that loses some metadata, and no importer Thanks for the information. Where can I find documentation on this capability? I'm particularly interested in how it handles the two issues I mentioned - mapping between Subversion's server-loxcal commit identities and import=-stream's full addresses, and mapping between Subversion merge-tracking info and the input-stream's commit-ancestry fields. `svnrdump (dump|load)` behaves almost exactly like `svnadmin (dump|load)` over the network. The conversion to git fast-import is merged into git.git: the infrastructure is in ^/vcs-svn and a small end-user tool is available in ^/contrib/svn-fe -- usage information is in svn-fe.txt: http://git.kernel.org/?p=git/git.git;a=blob;f=contrib/svn-fe/svn-fe.txt It currently doesn't handle any of the issues you mentioned- we haven't built the mapper yet, and it's just a dumb converter at the moment. We are currently working on some plumbing to support incremental imports, and build a remote helper. After that, we will work on the mapper and the git fast-import - dumpfile v3 converter to feed `svnrdump load`. Also, there has been a lot of discussion about the mapper on the mailing list, and here's one particularly huge thread: http://thread.gmane.org/gmane.comp.version-control.git/158940/focus=159054 Hope that helps :) Since Subversion already has so much infrastructure to deal with deltified dumpfile, I'd vote to keep that as the native import/export format. That decision is orthogonal to what I'm interested in, and I have no position on it. Ok. -- Ram
Re: [PATCH] extend svn_subst_translate_string() to record whether re-encoding and/or line ending translation were performed (v. 2)
On Mon, 2010-11-29, Danny Trebbien wrote: [...] My conclusion from all of this is that regardless of the value of `repair`, my changes do not appear to decrease the performance of svn_subst_translate_string() as long as svn_subst_translate_string2() is called directly. Hi Danny. (I notice you changed your email From name to Danny.) Statistics was never my strength so I'll just look to your conclusion. It sounds like it doesn't need any optimization, certainly nothing major. Therefore we should definitely make the functional change first. I just looked back at the previous emails and had a chat with Daniel Sh., and he agrees. Would you like to re-post your patch, when you're ready, without any of this optimization but with any other changes that are still needed? Daniel Shahaf wrote: As I don't recall (m)any other issues with the patch, I think it's a short distance from resolving this issue to committing the patch. Yup, a short distance now. Thanks. - Julian
Re: If RCS can stand it, why can ’t your system?
Hi, Eric Raymond writes: Ramkumar Ramachandra artag...@gmail.com: `svnrdump (dump|load)` behaves almost exactly like `svnadmin (dump|load)` over the network. What do you mean 'over the network'? Can I get a exported stream on stdout from the dump mode? Can I feed a stream on stdin to the load mode? Yep, it does *exactly* that :) $ svnrdump help dump dump: usage: svnrdump dump URL [-r LOWER[:UPPER]] Dump revisions LOWER to UPPER of repository at remote URL to stdout in a 'dumpfile' portable format. If only LOWER is given, dump that one revision. $ svnrdump help load load: usage: svnrdump load URL Load a 'dumpfile' given on stdin to a repository at remote URL. http://git.kernel.org/?p=git/git.git;a=blob;f=contrib/svn-fe/svn-fe.txt Looks like svnadmin dump --incremental REPO | svn-fe the export command reposurgeon needs, but I'm not clear what the corresponding import command would be. Yes, that is correct. You might consider using svnrdump for local repositories as well though- just use 'file://path' in place of 'path'; it's faster than svnadmin in some bechmarks. You can import once we finish building the git fast-import stream - dumpfile converter :) -- Ram
Re: [PATCH] Fix Makefile.svn to build APR with threads
On Tue, Nov 30, 2010 at 02:09:10PM +0530, Ramkumar Ramachandra wrote: Hi, Here's a quick patch to fix Makefile.svn. I need a +1 to commmit this. If the build breaks because of threadless APR, that's a bug in Subversion, not in Makefile.svn. If you want to build with thread support, please make it conditional on a global Makefile flag, as in make THREADED=1. I'd like to keep the default non-threaded for two reasons. The first is that I use OpenBSD and OpenBSD's threading implementation is a pure userspace implementation. It has to set stdin and stdout to non-blocking -- otherwise, whenever a thread blocks for i/o in the kernel the userspace thread-scheduler would also be blocked and couldn't switch to another thread. However, non-blocking stdio makes it very inconvenient to do things like svn diff | less because less expects blocking reads. The lines get garbled when you start scrolling in less. (Kernel thread support is being worked on by OpenBSD developers.) The second reason is that virtually nobody else is compiling and running without thread support in APR. This sometimes causes thread-less cases to be overlooked. Most recently I found a bug in the performance branch due to this. So it's good to have test coverage for the thread-less case. Because of this I will likely continue compiling APR thread-less even when OpenBSD finally gets kernel thread support. Your patch is too short. It's missing at least one hunk. Because if threading is enabled for APR, you should also enable it for sqlite and any other dependencies affected. Stefan Thanks. [[[ Makefile.svn: Fix build * tools/dev/unix-build/Makefile.svn: Configure APR to build with threads enabled to prevent breaking the Subversion build. ]]] Index: tools/dev/unix-build/Makefile.svn === --- tools/dev/unix-build/Makefile.svn (revision 1040428) +++ tools/dev/unix-build/Makefile.svn (working copy) @@ -238,8 +238,7 @@ cd $(APR_OBJDIR) \ env CFLAGS=-O0 -g $(APR_SRCDIR)/configure \ --prefix=$(PREFIX)/apr \ - --enable-maintainer-mode \ - --disable-threads + --enable-maintainer-mode touch $@ # compile apr
Input validation observations
I tried some potentially invalid inputs to svn a week or two ago and made notes on what I found. Just posting here in case anyone wants to do something about one or more of them. Noorul, I'm including you in the To addresses because you said you were looking for more small tasks to do, so feel free to pick one of these if you want to. Where I end with a question mark, it means I'm not sure if we want this change, it's just a suggestion. * svn checkout ^/ ^/y - A asf/cxf, A asf/cxf/utils, (Don't try this without being ready on the Ctrl-C or Ctrl-\!) It seems to ignore ^/y and create ./(basename(^/)); should fail: '^/y' is not a WC path. * svn checkout ^/subversion/trunk/build ^/y - Checked out revision 1040465. URL 'https://svn.apache.org/repos/asf/y' doesn't exist. Bleach - that's just crazy. Should fail: '^/y' is not a WC path. * svn copy a ^/b c doesn't detect the mixed source types in cl, only in lib; should reject them at CLI level? * svn info ^/b - Not a valid URL; should be path '/b' not found in revision REV? * svn mkdir ^/ a - Illegal repository URL 'a'; should say can't mix URL and local targets? * svn mkdir a ^/ - Can't create directory 'https:/svn.apache.org/repos/asf'; should not print the URL as if it's a local path. * svn mv ^/ ^/ - Cannot move path 'https:/svn.apache.org/repos/asf' into itself; should not print the URL as if it's a local path. * svn update ^/a - Skipped 'https://svn.apache.org/repos/asf/a' ...; should fail: '^/a' is not a WC path? - Julian
[PATCH] Include a reference to Makefile.svn in HACKING
Hi, I didn't know what else to write, so it's fairly short :p Thanks. [[[ community-guide: Add new section referring to ^/tools/dev/unix-build * publish/docs/community-guide/building.part.html: Add a new quickstart section pointing users to Makefile.svn * publish/docs/community-guide/building.toc.html: Update the table of contents to include this new section. ]]] Index: publish/docs/community-guide/building.toc.html === --- publish/docs/community-guide/building.toc.html (revision 1040464) +++ publish/docs/community-guide/building.toc.html (working copy) @@ -1,5 +1,6 @@ !--#if expr='$DOCUMENT_NAME = building.html || $DOCUMENT_NAME = community-guide.html' -- ul + lia href=#quickstartQuickly setting up a build environment under unix/a/li lia href=#configuryThe configuration/build system under unix/a/li lia href=#automated-testsAutomated tests/a/li lia href=#build-farmBuild farm/a/li @@ -7,6 +8,7 @@ /ul !--#else -- ul + lia href=building.html#quickstartQuickly setting up a build environment under unix/a/li lia href=building.html#configuryThe configuration/build system under unix/a/li lia href=building.html#automated-testsAutomated tests/a/li lia href=building.html#build-farmBuild farm/a/li Index: publish/docs/community-guide/building.part.html === --- publish/docs/community-guide/building.part.html (revision 1040464) +++ publish/docs/community-guide/building.part.html (working copy) @@ -6,6 +6,27 @@ !--#include virtual=building.toc.html -- +div class=h2 id=quickstart +h2Quickly setting up a build environment under unix + a class=sectionlink href=!--#echo var=GUIDE_BUILDING_PAGE --#quickstart +title=Link to this sectionpara;/a +/h2 + +pStefan Sperling wrote a nice script that starts from scratch to set +up a nice build environment under unix; it fetches, configures and +compiles the latest versions of dependencies as required. To start +development from scratch, simply +download a +href=http://svn.apache.org/repos/asf/subversion/trunk/tools/dev/unix-build/Makefile.svn; +Makefile.svn/a, rename it to `Makefile` and invoke `make`. For more +information, read the a +href=http://svn.apache.org/repos/asf/subversion/trunk/tools/dev/unix-build/READNE; +documentation/a supplied along with it. +/p + +/div !-- quickstart -- + + div class=h2 id=configury h2The configuration/build system under unix a class=sectionlink href=!--#echo var=GUIDE_BUILDING_PAGE --#configury
Re: 1.7.x bug - svn add no longer accepts wildcards?
On Tue, Nov 30, 2010 at 7:18 PM, Branko Čibej br...@xbc.nu wrote: On 30.11.2010 05:32, Daniel Becroft wrote: Hi, During testing of a 1.7.x build, I've noticed that 'svn add' on trunk no longer accepts wildcards: svn 1.6.x: D:\temp\svn_sandpit\workingcopy\trunk\Aecho 111 1alpha.txt D:\temp\svn_sandpit\workingcopy\trunk\Aecho 111 1beta.txt D:\temp\svn_sandpit\workingcopy\trunk\Asvn add *.txt A alpha.txt A beta.txt Whereas 1.7.x gives: D:\temp\svn_sandpit\workingcopy\trunk\Aecho 111 1alpha.txt D:\temp\svn_sandpit\workingcopy\trunk\Aecho 111 1beta.txt D:\temp\svn_sandpit\workingcopy\trunk\AD:\temp\svn_sandpit\svn7.exe add *.txt svn: warning: 'D:\temp\svn_sandpit\workingcopy\trunk\A\*.txt' not found I thought that wildcards were expanded by the OS/Shell, and then passed to SVN. Is this not the case? Not on windows, you have to link a magic library into your code to get that behaviour. -- Brane Hmm ... that makes sense. My two separate EXEs were obtained from two separate sources, so obviously the 1.6.x version links this 'magic library'. I don't suppose you can tell me which library, so I can see if that's included in the 1.7.x build? Cheers, Daniel B.
Re: svn commit: r1037738 - Summary of updates
On Mon, Nov 29, 2010 at 02:17:13PM -0500, C. Michael Pilato wrote:
Not necessarily anything more. I mean, test expectations reflect, to some
degree, the expectations of Subversion users wrapping the command-line for
output parsing. I sense we're cool with some sane amount of change in that
output so as to make the tool better without completely breaking the world.
:-) ('Cause if we're cool with breaking the world, I have quite a few
changes in mind that I'd like to make to the output!)
I've said this before, but, yes, break the world.
I'd say just propose these changes. I think we can afford to break compat in
the CLI output. The CLI is for interactive use. People can script against
it, sure. But they should be using XML output where possible, or use the
bindings, if they rely on their scripts for production rather than quick
run-once-and-throw-away hacks.
Stefan
Re: SQLite and callbacks
Hyrum K. Wright hyrum_wri...@mail.utexas.edu writes: Stefan's patch to make a recursive proplist much more performant highlights the great benefit that our sqlite-backed storage can have. However, he reverted it due to concerns about the potential for database contention. The theory was that the callback might try and call additional wc functions to get more information, and such nested statements weren't healthy for sqlite. We talked about it for a bit in IRC this morning, and the picture raised by this issue was quite dire. It's not the nested statements that are the problem, it's extending the duration of the select. The callback is to the application so it could be updating a GUI, interacting with the user, etc. and so could take a much greater length of time than other selects. An SQLite read blocks SQlite writes by other threads/processes, so the long lived select may cause writes to fail that would otherwise succeed. (We have a busy timeout of 10 seconds so that's the sort of time that becomes a problem.) It's related to the problem of querying the working copy while it is locked. In 1.6 it was possible to run status, info, propget, etc. while an update was in progress. In 1.7 that's no longer possible, all the read operations fail. If we decide that this change in behaviour is acceptable then the long-lived select isn't really a problem, it simply stops the operations like update from starting. If we want the 1.6 behaviour then the long-lived select becomes more of a problem, as it makes update much more likely to fail part way through. The callback is not necessarily wrong, it depends what sort of behaviour we are trying to provide. -- Philip
Re: SQLite and callbacks
On Mon, Nov 29, 2010 at 07:43:40PM -0600, Hyrum K. Wright wrote:
We use callbacks extensively throughout our code as a means of
providing streamy feedback to callers. It's a pretty good paradigm,
and one that has served us well. We don't put many restrictions on
what the callbacks can do in terms of fetching more information or
calling other functions.
Enter wc-ng.
Stefan's patch to make a recursive proplist much more performant
highlights the great benefit that our sqlite-backed storage can have.
However, he reverted it due to concerns about the potential for
database contention. The theory was that the callback might try and
call additional wc functions to get more information, and such nested
statements weren't healthy for sqlite. We talked about it for a bit
in IRC this morning, and the picture raised by this issue was quite
dire.
In an attempt to find out what the consequences of these nested
queries are, I wrote a test program to attempt to demonstrate the
failure, only now I can't seem to do so. Attached is the test
program, but when I run it, I'm able to successfully execute multiple
prepared statements on the same set of rows simultaneously, which was
the concern we had about our callback mechanism in sqlite.
So is this a valid problem? If so, could somebody use the attached
test program to illustrate it for those of us who may not fully
understand the situation?
If you run this version of your test, it will run in an endless loop.
The callback inserts new values, the caller will see those and invoke
the callback again.
We need a way to prevent writes done within the callback from being
visible to the caller.
#include stdio.h
#include string.h
#include sqlite3.h
#define CHECK_ERR \
if (sqlite3_errcode(db) \
(sqlite3_errcode(db) != SQLITE_ROW) \
(sqlite3_errcode(db) != SQLITE_DONE)) \
fprintf(stderr, %d: %d: %s\n, __LINE__, sqlite3_errcode(db),
sqlite3_errmsg(db));
#define TEST_DATA \
create table foo (num int, message text); \
\
insert into foo values (1, 'A is for Allegator'); \
insert into foo values (2, 'B is for Bayou'); \
insert into foo values (3, 'C is for Cyprus Trees'); \
insert into foo values (4, 'D is for Dew'); \
insert into foo values (5, 'E is for Everything like'); \
insert into foo values (6, 'F Ferns or'); \
insert into foo values (7, 'G Grass that''s'); \
insert into foo values (8, 'H Home to you'); \
void callback(sqlite3 *db, int num)
{
const char *query = insert into foo values (?1,?2);;
sqlite3_stmt *stmt;
const unsigned char *msg = new message for you!;
printf(Got number: %d\n, num);
sqlite3_prepare_v2(db, query, -1, stmt, NULL);
CHECK_ERR;
sqlite3_bind_int(stmt, 1, 42);
sqlite3_bind_text(stmt, 2, msg, strlen(msg) + 1, SQLITE_STATIC);
CHECK_ERR;
sqlite3_step(stmt);
CHECK_ERR;
sqlite3_finalize(stmt);
CHECK_ERR;
}
void get_numbers(sqlite3 *db,
void (*callback)(sqlite3 *, int))
{
const char *query = select num from foo;;
sqlite3_stmt *stmt;
int code;
sqlite3_prepare_v2(db, query, -1, stmt, NULL);
CHECK_ERR;
code = sqlite3_step(stmt);
CHECK_ERR;
while (code == SQLITE_ROW)
{
int number = sqlite3_column_int(stmt, 0);
callback(db, number);
code = sqlite3_step(stmt);
CHECK_ERR;
}
sqlite3_finalize(stmt);
CHECK_ERR;
}
int
main(int argc, char *argv[])
{
sqlite3 *db;
remove(test.db);
sqlite3_open_v2(test.db, db, SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE,
NULL);
CHECK_ERR;
sqlite3_extended_result_codes(db, 1);
CHECK_ERR;
sqlite3_exec(db, TEST_DATA, NULL, NULL, NULL);
CHECK_ERR;
get_numbers(db, callback);
sqlite3_close(db);
return 0;
}
Re: [PATCH] extend svn_subst_translate_string() to record whether re-encoding and/or line ending translation were performed (v. 2)
Julian Foad wrote on Tue, Nov 30, 2010 at 10:40:59 +: On Mon, 2010-11-29, Danny Trebbien wrote: [...] My conclusion from all of this is that regardless of the value of `repair`, my changes do not appear to decrease the performance of svn_subst_translate_string() as long as svn_subst_translate_string2() is called directly. Hi Danny. (I notice you changed your email From name to Danny.) Statistics was never my strength so I'll just look to your conclusion. It sounds like it doesn't need any optimization, certainly nothing major. Therefore we should definitely make the functional change first. I just looked back at the previous emails and had a chat with Daniel Sh., and he agrees. It would be more accurate to say that I don't know yet whether I agree or not, since I haven't had the chance to digest dtrebbien's t-test results yet. Would you like to re-post your patch, when you're ready, without any of this optimization but with any other changes that are still needed? +1. (I don't need to be convinced myself of the statistics to be convinced that having the patch proceed without the optimization is the way to go here.) Daniel Shahaf wrote: As I don't recall (m)any other issues with the patch, I think it's a short distance from resolving this issue to committing the patch. Yup, a short distance now. But once there, Danny has another patch in the pipeline...
Re: svn commit: r1040482 - in /subversion/site/publish/docs/community-guide: building.part.html building.toc.html
artag...@apache.org wrote on Tue, Nov 30, 2010 at 12:19:34 -: Author: artagnon Date: Tue Nov 30 12:19:34 2010 New Revision: 1040482 URL: http://svn.apache.org/viewvc?rev=1040482view=rev Log: community-guide: Add new section referring to ^/tools/dev/unix-build * publish/docs/community-guide/building.part.html: Add a new quickstart section pointing users to Makefile.svn Perhaps add a reference to INSTALL too? * publish/docs/community-guide/building.toc.html: Update the table of contents to include this new section. Modified: subversion/site/publish/docs/community-guide/building.part.html subversion/site/publish/docs/community-guide/building.toc.html Modified: subversion/site/publish/docs/community-guide/building.part.html URL: http://svn.apache.org/viewvc/subversion/site/publish/docs/community-guide/building.part.html?rev=1040482r1=1040481r2=1040482view=diff == --- subversion/site/publish/docs/community-guide/building.part.html (original) +++ subversion/site/publish/docs/community-guide/building.part.html Tue Nov 30 12:19:34 2010 @@ -6,6 +6,26 @@ !--#include virtual=building.toc.html -- +div class=h2 id=quickstart +h2Quickly setting up a build environment under unix + a class=sectionlink href=!--#echo var=GUIDE_BUILDING_PAGE --#quickstart +title=Link to this sectionpara;/a +/h2 + +pStefan Sperling wrote a nice Makefile (for GNU make) that starts +from scratch to set up a nice build environment under unix; it +fetches, configures and compiles the latest versions of dependencies +as required. To start development from scratch, simply download +a href=http://svn.apache.org/repos/asf/subversion/trunk/tools/dev/unix-build/Makefile.svn; +Makefile.svn/a, create a symlink called `GNUmakefile` that points +to the file, and invoke `make`. For more information, read the +a href=http://svn.apache.org/repos/asf/subversion/trunk/tools/dev/unix-build/README; +documentation/a supplied along with it. +/p + +/div !-- quickstart -- + + div class=h2 id=configury h2The configuration/build system under unix a class=sectionlink href=!--#echo var=GUIDE_BUILDING_PAGE --#configury Modified: subversion/site/publish/docs/community-guide/building.toc.html URL: http://svn.apache.org/viewvc/subversion/site/publish/docs/community-guide/building.toc.html?rev=1040482r1=1040481r2=1040482view=diff == --- subversion/site/publish/docs/community-guide/building.toc.html (original) +++ subversion/site/publish/docs/community-guide/building.toc.html Tue Nov 30 12:19:34 2010 @@ -1,5 +1,6 @@ !--#if expr='$DOCUMENT_NAME = building.html || $DOCUMENT_NAME = community-guide.html' -- ul + lia href=#quickstartQuickly setting up a build environment under unix/a/li lia href=#configuryThe configuration/build system under unix/a/li lia href=#automated-testsAutomated tests/a/li lia href=#build-farmBuild farm/a/li @@ -7,6 +8,7 @@ /ul !--#else -- ul + lia href=building.html#quickstartQuickly setting up a build environment under unix/a/li lia href=building.html#configuryThe configuration/build system under unix/a/li lia href=building.html#automated-testsAutomated tests/a/li lia href=building.html#build-farmBuild farm/a/li
RE: svnsync failure when syncing with a repository that used ISO-8859-1 for log messages
I must be missing something here, is this patch applied to the current SVN
source?
I have downloaded SVN 1.6.15 source from Tigris.org, and cant find the changes
in the source. Also this patch is applied to a file : subversion/svnsync/sync.c
but in my downloaded source there is only a file main.c in the svnsync
directory.
We are for the moment stuck with this UTF-8 problem in one of our svn:ignore
properties, and cant proceed with seting up this sync, so any pointers are
welcome!
Geir
Note : All inquiries regarding Subversion, MKS and general Development servers
should be directed to EDB SourceControl System
-Original Message-
From: Daniel Trebbien [mailto:dtrebb...@gmail.com]
Sent: 9. september 2010 17:49
To: Daniel Shahaf
Cc: us...@subversion.apache.org; dev@subversion.apache.org
Subject: Re: svnsync failure when syncing with a repository that used
ISO-8859-1 for log messages
On Thu, Sep 9, 2010 at 3:48 AM, Daniel Shahaf d...@daniel.shahaf.name
wrote:
CC += dev@, and let me point you to our patch submission guidelines:
http://subversion.apache.org/docs/community-
guide/general.html#patches
Summary for dev@: allow svnsync to translate non-UTF-8 log messages
to UTF-8.
(more below)
Daniel Trebbien wrote on Wed, Sep 08, 2010 at 18:58:06 -0700:
On Wed, Sep 8, 2010 at 4:39 PM, Daniel Trebbien
dtrebb...@gmail.com wrote:
I think that a call to `svn_subst_translate_string`
(http://svn.collab.net/svn-doxygen/svn__subst_8h.html#a29) needs
to be
added in the `svnsync_normalize_revprops` function when `propname`
is
svn:log.
After applying the following patch to `subversion/svnsync/sync.c`, I
can confirm that the svnsync: Error while replaying commit error
disappears, allowing the sync to complete, and that the problem is
indeed a log message encoding issue:
diff --git a/subversion/svnsync/sync.c b/subversion/svnsync/sync.c
index 8f7be9e..c7a5e38 100644
--- a/subversion/svnsync/sync.c
+++ b/subversion/svnsync/sync.c
@@ -114,6 +114,14 @@ svnsync_normalize_revprops(apr_hash_t
*rev_props,
/* And count this. */
(*normalized_count)++;
}
+
+ if (strcmp(propname, svn:log) == 0)
+ {
Should this use svn_prop_needs_translation()?
Though it does not appear so, the added lines are within the check for
`svn_prop_needs_translation`.
+ svn_string_t *new_propval;
+ SVN_ERR(svn_subst_translate_string(new_propval,
propval, ISO-8859-1, pool));
+ apr_hash_set(rev_props, propname,
APR_HASH_KEY_STRING, propval = new_propval);
Please, please, please, no assignment inside a function call.
^
Fixed
+ (*normalized_count)++;
+ }
}
}
return SVN_NO_ERROR;
Here I hard-coded the encoding, but I think that the encoding to use
should be retrieved from the Subversion config file. Now the
questions
are:
1. What is the best way to pass the `log-encoding` option of the
[miscellany] section to the `svnsync_normalize_revprops` function?
What is 'log-encoding' normally used for? Only for recoding the
commit
message supplied by an editor, right? So I'm not sure we should use
it
here, perhaps a new command-line option will be better. (svnsync
$subcommand --source-encoding=%s)
I like the idea of adding a command line option, so I think that this
is the way to go.
Do other properties need to be re-encoded, potentially? I was only
thinking about `svn:log`, but perhaps other properties might need
re-encoding as well. If not, then I think that the command line option
should be more specific (e.g.: `--source-log-encoding=%s`).
And either way, the default behaviour should be to translate nothing.
(If you always translate from latin1, you break syncs for people who,
correctly, used UTF-8 log messages the entire time.)
I agree. The default behavior should definitely be to not re-encode
the log messages.
2. Should `svnsync` always store the log messages in UTF-8 even
though
the original repository log message encoding is different?
You have no choice on the matter: the RA API instructs you to supply
it
a UTF-8 log message. (IIRC, even the libsvn_client API expects an
already-UTF-8 message.)
3. Should there be separate configuration options for the log
message
encoding of the source repository and the log message encoding of
the
destination repository?
No, see (2).
Another question: which line of code raises the svnsync: Error while
replaying commit error message? I would like to try to make this more
helpful.
Re: [PATCH] extend svn_subst_translate_string() to record whether re-encoding and/or line ending translation were performed (v. 2)
Re-reading my original review comment, I think it sounds more requiring than I intended it to be. My fault. Also, this thread (and in particular Danny's last mail) nicely demonstrate another aspect of our patch submission process: namely, that patch reviews are nothing but the start of a discussion about the point they highlight, and it's fine and healthy to present a disagreeing opinion upon receiving a patch review. Daniel Julian Foad wrote on Tue, Nov 30, 2010 at 10:40:59 +: On Mon, 2010-11-29, Danny Trebbien wrote: [...] My conclusion from all of this is that regardless of the value of `repair`, my changes do not appear to decrease the performance of svn_subst_translate_string() as long as svn_subst_translate_string2() is called directly. Hi Danny. (I notice you changed your email From name to Danny.) Statistics was never my strength so I'll just look to your conclusion. It sounds like it doesn't need any optimization, certainly nothing major. Therefore we should definitely make the functional change first. I just looked back at the previous emails and had a chat with Daniel Sh., and he agrees. Would you like to re-post your patch, when you're ready, without any of this optimization but with any other changes that are still needed? Daniel Shahaf wrote: As I don't recall (m)any other issues with the patch, I think it's a short distance from resolving this issue to committing the patch. Yup, a short distance now. Thanks. - Julian
subversion.org web site needs a front page - any volunteers?
I think subversion.org needs a front page with some basic Point you to where you need to go links. I just tried browsing and found: * subversion.org and www.subversion.org redirect to svncorp.org. That's probably not what most people are looking for. * buildbot.subversion.org has a single link to what it calls the official Subversion buildbot, but is in fact the old buildbot master which appears to be running but with no slaves connected. Does anyone fancy improving this situation? How would a volunteer get access to the content and redirection config? - Julian
Re: Input validation observations
Julian Foad julian.f...@wandisco.com writes: I tried some potentially invalid inputs to svn a week or two ago and made notes on what I found. Just posting here in case anyone wants to do something about one or more of them. Noorul, I'm including you in the To addresses because you said you were looking for more small tasks to do, so feel free to pick one of these if you want to. Thank you! I will start working on these one by one. Where I end with a question mark, it means I'm not sure if we want this change, it's just a suggestion. * svn checkout ^/ ^/y - A asf/cxf, A asf/cxf/utils, (Don't try this without being ready on the Ctrl-C or Ctrl-\!) It seems to ignore ^/y and create ./(basename(^/)); should fail: '^/y' is not a WC path. * svn checkout ^/subversion/trunk/build ^/y - Checked out revision 1040465. URL 'https://svn.apache.org/repos/asf/y' doesn't exist. Bleach - that's just crazy. Should fail: '^/y' is not a WC path. * svn copy a ^/b c doesn't detect the mixed source types in cl, only in lib; should reject them at CLI level? * svn info ^/b - Not a valid URL; should be path '/b' not found in revision REV? * svn mkdir ^/ a - Illegal repository URL 'a'; should say can't mix URL and local targets? * svn mkdir a ^/ - Can't create directory 'https:/svn.apache.org/repos/asf'; should not print the URL as if it's a local path. * svn mv ^/ ^/ - Cannot move path 'https:/svn.apache.org/repos/asf' into itself; should not print the URL as if it's a local path. * svn update ^/a - Skipped 'https://svn.apache.org/repos/asf/a' ...; should fail: '^/a' is not a WC path? svn help update says that the argument should be a PATH. I think it is right to force the user to enter local PATH. Thanks and Regards Noorul
[PATCH] community-guide: Update buildbot section
Hi, I need a +1 to commit this. Also, please suggest a better link for the ASF Infra team (if there is one). Thanks. [[[ community-guide: Rewrite the Buildbot section * publish/docs/community-guide/building.part.html: Remove historical cruft and update with relevant information. ]]] Index: publish/docs/community-guide/building.part.html === --- publish/docs/community-guide/building.part.html (revision 1040482) +++ publish/docs/community-guide/building.part.html (working copy) @@ -278,32 +278,15 @@ title=Link to this sectionpara;/a /h2 -pLieven Govaerts has set up a -a href=http://buildbot.sourceforge.net/; BuildBot/a build/test -farm at a href=http://buildbot.subversion.org/buildbot/; -http://buildbot.subversion.org/buildbot//a, see this message:/p +pThe a href=http://apache.org/dev/#infrastructure;ASF Infra/a +team manages a a href=http://buildbot.sourceforge.net/;BuildBot/a +build/test farm. The Buildbot waterfall for the Subversion project is +located at a href=http://subversion.apache.org/buildbot/; +http://subversion.apache.org/buildbot//a. For more information +about build services, head over +to a href=http://ci.apache.org;ci.apache.org/a. +/p -pre - a href=http://subversion.tigris.org/servlets/ReadMsg?list=devamp;msgNo=114212;http://subversion.tigris.org/servlets/ReadMsg?list=devamp;msgNo=114212/a - (Thread URL: a href=http://subversion.tigris.org/servlets/BrowseList?list=devamp;by=threadamp;from=450110;http://subversion.tigris.org/servlets/BrowseList?list=devamp;by=threadamp;from=450110/a) - Message-ID: 20060326205918.f3c5070...@adicia.telenet-ops.be - From: Lieven Govaerts lt;l...@mobsol.begt; - To: lt;d...@subversion.tigris.orggt; - Subject: Update: Subversion build and test farm with Buildbot. - Date: Sun, 26 Mar 2006 22:56:11 +0200 -/pre - -pfor more details. (a href=http://buildbot.sourceforge.net/; -BuildBot/a is a system for centrally managing multiple automated -testing environments; it's especially useful for portability testing, -including of uncommitted changes.)/p - -p class=todoOnce the details of the build farm get finalized, -integrate those instructions here. Perhaps the information at -a href=https://svn.apache.org/repos/asf/subversion/trunk/www/build-farm.html?p=867958; -https://svn.apache.org/repos/asf/subversion/trunk/www/build-farm.html?p=867958/a -can serve as a good starting point?/p - /div !-- build-farm --
Re: SQLite and callbacks
On Tue, Nov 30, 2010 at 01:32:27PM +, Philip Martin wrote:
Stefan Sperling s...@elego.de writes:
If you run this version of your test, it will run in an endless loop.
The callback inserts new values, the caller will see those and invoke
the callback again.
Hmm, I expected the select to block the write causing it to return
SQLITE_BUSY. That is what happens if the write is done by a separate
process, but apparently not when it's the same process (or maybe it's
using the same database handle that makes it work).
With a different db handle, the callback cannot insert values into the db:
$ ./test
Got number: 1
45: 5: database is locked
48: 5: database is locked
Got number: 2
45: 5: database is locked
48: 5: database is locked
Got number: 3
45: 5: database is locked
48: 5: database is locked
Got number: 4
45: 5: database is locked
48: 5: database is locked
Got number: 5
45: 5: database is locked
48: 5: database is locked
Got number: 6
45: 5: database is locked
48: 5: database is locked
Got number: 7
45: 5: database is locked
48: 5: database is locked
Got number: 8
45: 5: database is locked
48: 5: database is locked
#include stdio.h
#include string.h
#include sqlite3.h
#define CHECK_ERR \
if (sqlite3_errcode(db) \
(sqlite3_errcode(db) != SQLITE_ROW) \
(sqlite3_errcode(db) != SQLITE_DONE)) \
fprintf(stderr, %d: %d: %s\n, __LINE__, sqlite3_errcode(db),
sqlite3_errmsg(db));
#define TEST_DATA \
create table foo (num int, message text); \
\
insert into foo values (1, 'A is for Allegator'); \
insert into foo values (2, 'B is for Bayou'); \
insert into foo values (3, 'C is for Cyprus Trees'); \
insert into foo values (4, 'D is for Dew'); \
insert into foo values (5, 'E is for Everything like'); \
insert into foo values (6, 'F Ferns or'); \
insert into foo values (7, 'G Grass that''s'); \
insert into foo values (8, 'H Home to you'); \
void callback(int num)
{
const char *query = insert into foo values (?1,?2);;
sqlite3_stmt *stmt;
const unsigned char *msg = new message for you!;
sqlite3 *db;
sqlite3_open_v2(test.db, db, SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE,
NULL);
printf(Got number: %d\n, num);
sqlite3_prepare_v2(db, query, -1, stmt, NULL);
CHECK_ERR;
sqlite3_bind_int(stmt, 1, 42);
sqlite3_bind_text(stmt, 2, msg, strlen(msg) + 1, SQLITE_STATIC);
CHECK_ERR;
sqlite3_step(stmt);
CHECK_ERR;
sqlite3_finalize(stmt);
CHECK_ERR;
sqlite3_close(db);
}
void get_numbers(sqlite3 *db,
void (*callback)(int))
{
const char *query = select num from foo;;
sqlite3_stmt *stmt;
int code;
sqlite3_prepare_v2(db, query, -1, stmt, NULL);
CHECK_ERR;
code = sqlite3_step(stmt);
CHECK_ERR;
while (code == SQLITE_ROW)
{
int number = sqlite3_column_int(stmt, 0);
callback(number);
code = sqlite3_step(stmt);
CHECK_ERR;
}
sqlite3_finalize(stmt);
CHECK_ERR;
}
int
main(int argc, char *argv[])
{
sqlite3 *db;
remove(test.db);
sqlite3_open_v2(test.db, db, SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE,
NULL);
CHECK_ERR;
sqlite3_extended_result_codes(db, 1);
CHECK_ERR;
sqlite3_exec(db, TEST_DATA, NULL, NULL, NULL);
CHECK_ERR;
get_numbers(db, callback);
sqlite3_close(db);
return 0;
}
Re: svn commit: r1039398 - in /subversion/trunk/subversion: include/svn_dirent_uri.h libsvn_delta/path_driver.c libsvn_subr/dirent_uri.c tests/libsvn_subr/dirent_uri-test.c
Daniel Shahaf wrote: Add some more svn_fspath__* APIs, with tests for some of them. These tests pass both with and without FSPATH_USE_URI defined. * subversion/include/svn_dirent_uri.h, subversion/libsvn_subr/dirent_uri.c (svn_fspath__dirname, svn_fspath__split, svn_fspath__skip_ancestor, svn_fspath__is_ancestor, svn_fspath__get_longest_ancestor): New functions. * subversion/libsvn_delta/path_driver.c (svn_fspath__get_longest_ancestor): Remove this temporary macro. * subversion/tests/libsvn_subr/dirent_uri-test.c (test_fspath_basename): Rename and extend to become ... (test_fspath_dirname_basename_split): ... this new test. (test_fspath_get_longest_ancestor): New tests. (test_funcs): Add the new tests. ### TODO: Tests. What is that ### comment? Is it the mailer saying you should have added a few more tests to this commit? Heh! I didn't intend to commit that, but it was me reminding myself that I ought to write tests for the remaining functions: skip_ancestor and is_ancestor. In the end I decided I was OK with committing it without (or before) writing tests for those. Tests are important ... but somehow those are way down my priority list. Thanks for noticing. I've edited the log message to clarify. - Julian
Re: 1.7.x bug - svn add no longer accepts wildcards?
On 11/30/2010 07:02 AM, Daniel Becroft wrote: On Tue, Nov 30, 2010 at 7:18 PM, Branko Čibej br...@xbc.nu wrote: Not on windows, you have to link a magic library into your code to get that behaviour. Hmm ... that makes sense. My two separate EXEs were obtained from two separate sources, so obviously the 1.6.x version links this 'magic library'. I don't suppose you can tell me which library, so I can see if that's included in the 1.7.x build? setargv.obj (see http://msdn.microsoft.com/en-us/library/8bch7bkk%28v=VS.80%29.aspx) -- C. Michael Pilato cmpil...@collab.net CollabNet www.collab.net Distributed Development On Demand
Re: [PATCH] Fix Makefile.svn to build APR with threads
Hi, Stefan Sperling writes: The first is that I use OpenBSD and OpenBSD's threading implementation is a pure userspace implementation. It has to set stdin and stdout to non-blocking -- otherwise, whenever a thread blocks for i/o in the kernel the userspace thread-scheduler would also be blocked and couldn't switch to another thread. However, non-blocking stdio makes it very inconvenient to do things like svn diff | less because less expects blocking reads. The lines get garbled when you start scrolling in less. (Kernel thread support is being worked on by OpenBSD developers.) The second reason is that virtually nobody else is compiling and running without thread support in APR. This sometimes causes thread-less cases to be overlooked. Most recently I found a bug in the performance branch due to this. So it's good to have test coverage for the thread-less case. Because of this I will likely continue compiling APR thread-less even when OpenBSD finally gets kernel thread support. Thanks for the explanation! Unfortunately, I'm not able to get the build to break with threading disabled anymore :| Have to run a few more experiments before I can conclusively say what went wrong back then. Your patch is too short. It's missing at least one hunk. Because if threading is enabled for APR, you should also enable it for sqlite and any other dependencies affected. Ok, got it. Here's my second try. [[[ Makefile.svn: Optionally allow building with threading support * tools/dev/unix-build/Makefile.svn: Add a new ENABLE_THREADING variable to control whether APR and sqlite should be built with threading support. ]]] Index: tools/dev/unix-build/Makefile.svn === --- tools/dev/unix-build/Makefile.svn (revision 1040658) +++ tools/dev/unix-build/Makefile.svn (working copy) @@ -3,6 +3,7 @@ # WARNING: This may or may not work on your system. This Makefile is # an example, rather than a ready-made universal solution. +ENABLE_THREADING ?= no # OpenBSD doesn't have kernel threads for example ENABLE_PYTHON_BINDINGS ?= yes ENABLE_RUBY_BINDINGS ?= yes ENABLE_PERL_BINDINGS ?= yes @@ -241,12 +242,21 @@ | sed -e '/^.*APR_ADDTO(CPPFLAGS, \[-D_POSIX_THREADS\]).*$$/d' \ $(APR_SRCDIR)/build/apr_hints.m4 cd $(APR_SRCDIR) ./buildconf - cd $(APR_OBJDIR) \ -env CFLAGS=-O0 -g $(PROFILE_CFLAGS) \ - $(APR_SRCDIR)/configure \ - --prefix=$(PREFIX)/apr \ - --enable-maintainer-mode \ - --disable-threads + if [ $(ENABLE_THREADING) = yes ]; then \ + cd $(APR_OBJDIR) \ +env CFLAGS=-O0 -g $(PROFILE_CFLAGS) \ + $(APR_SRCDIR)/configure \ + --prefix=$(PREFIX)/apr \ + --enable-maintainer-mode \ + --enable-threads; \ + else \ + cd $(APR_OBJDIR) \ +env CFLAGS=-O0 -g $(PROFILE_CFLAGS) \ + $(APR_SRCDIR)/configure \ + --prefix=$(PREFIX)/apr \ + --enable-maintainer-mode \ + --disable-threads; \ + fi; touch $@ # compile apr @@ -706,12 +716,21 @@ # configure sqlite $(SQLITE_OBJDIR)/.configured: $(SQLITE_OBJDIR)/.retrieved - cd $(SQLITE_OBJDIR) \ -env CFLAGS=-g $(PROFILE_CFLAGS) \ - $(SQLITE_SRCDIR)/configure \ - --prefix=$(PREFIX)/sqlite \ - --disable-tcl \ - --disable-threadsafe + if [ $(ENABLE_THREADING) = yes ]; then \ + cd $(SQLITE_OBJDIR) \ +env CFLAGS=-g $(PROFILE_CFLAGS) \ + $(SQLITE_SRCDIR)/configure \ + --prefix=$(PREFIX)/sqlite \ + --disable-tcl \ + --enable-threadsafe; \ + else \ + cd $(SQLITE_OBJDIR) \ +env CFLAGS=-g $(PROFILE_CFLAGS) \ + $(SQLITE_SRCDIR)/configure \ + --prefix=$(PREFIX)/sqlite \ + --disable-tcl \ + --disable-threadsafe; \ + fi; touch $@ # compile sqlite
Re: [PATCH] Fix Makefile.svn to build APR with threads
On Wed, Dec 01, 2010 at 12:18:08AM +0530, Ramkumar Ramachandra wrote: Your patch is too short. It's missing at least one hunk. Because if threading is enabled for APR, you should also enable it for sqlite and any other dependencies affected. Ok, got it. Here's my second try. [[[ Makefile.svn: Optionally allow building with threading support * tools/dev/unix-build/Makefile.svn: Add a new ENABLE_THREADING variable to control whether APR and sqlite should be built with threading support. ]]] Index: tools/dev/unix-build/Makefile.svn === --- tools/dev/unix-build/Makefile.svn (revision 1040658) +++ tools/dev/unix-build/Makefile.svn (working copy) @@ -3,6 +3,7 @@ # WARNING: This may or may not work on your system. This Makefile is # an example, rather than a ready-made universal solution. +ENABLE_THREADING ?= no # OpenBSD doesn't have kernel threads for example You can call this variable THREADING so it's shorter to type on the command line. ENABLE_PYTHON_BINDINGS ?= yes ENABLE_RUBY_BINDINGS ?= yes ENABLE_PERL_BINDINGS ?= yes @@ -241,12 +242,21 @@ | sed -e '/^.*APR_ADDTO(CPPFLAGS, \[-D_POSIX_THREADS\]).*$$/d' \ $(APR_SRCDIR)/build/apr_hints.m4 cd $(APR_SRCDIR) ./buildconf - cd $(APR_OBJDIR) \ - env CFLAGS=-O0 -g $(PROFILE_CFLAGS) \ - $(APR_SRCDIR)/configure \ - --prefix=$(PREFIX)/apr \ - --enable-maintainer-mode \ - --disable-threads + if [ $(ENABLE_THREADING) = yes ]; then \ + cd $(APR_OBJDIR) \ + env CFLAGS=-O0 -g $(PROFILE_CFLAGS) \ + $(APR_SRCDIR)/configure \ + --prefix=$(PREFIX)/apr \ + --enable-maintainer-mode \ + --enable-threads; \ + else \ + cd $(APR_OBJDIR) \ + env CFLAGS=-O0 -g $(PROFILE_CFLAGS) \ + $(APR_SRCDIR)/configure \ + --prefix=$(PREFIX)/apr \ + --enable-maintainer-mode \ + --disable-threads; \ + fi; touch $@ # compile apr I would define a make variable that contains threading or non-threading flags depending on THREADING=1, like this: ifdef THREADING THREADING_FLAG=--enable-threads else THREADING_FLAG=--enable-threads endif Then you can do this: cd $(APR_OBJDIR) \ env CFLAGS=-O0 -g $(PROFILE_CFLAGS) \ $(APR_SRCDIR)/configure \ --prefix=$(PREFIX)/apr \ --enable-maintainer-mode \ $(THREADING_FLAG); \ @@ -706,12 +716,21 @@ # configure sqlite $(SQLITE_OBJDIR)/.configured: $(SQLITE_OBJDIR)/.retrieved - cd $(SQLITE_OBJDIR) \ - env CFLAGS=-g $(PROFILE_CFLAGS) \ - $(SQLITE_SRCDIR)/configure \ - --prefix=$(PREFIX)/sqlite \ - --disable-tcl \ - --disable-threadsafe + if [ $(ENABLE_THREADING) = yes ]; then \ + cd $(SQLITE_OBJDIR) \ + env CFLAGS=-g $(PROFILE_CFLAGS) \ + $(SQLITE_SRCDIR)/configure \ + --prefix=$(PREFIX)/sqlite \ + --disable-tcl \ + --enable-threadsafe; \ + else \ + cd $(SQLITE_OBJDIR) \ + env CFLAGS=-g $(PROFILE_CFLAGS) \ + $(SQLITE_SRCDIR)/configure \ + --prefix=$(PREFIX)/sqlite \ + --disable-tcl \ + --disable-threadsafe; \ + fi; touch $@ # compile sqlite Same here. Stefan
Re: [PATCH] Fix Makefile.svn to build APR with threads
On Tue, Nov 30, 2010 at 07:52:36PM +0100, Stefan Sperling wrote: ifdef THREADING THREADING_FLAG=--enable-threads else THREADING_FLAG=--enable-threads endif Oops... s/enable/disable/ on the second line obviously :)
Re: 1.5.8 release, Dec. 1
*poke* (Just a reminder for folks who've not taken a glance at 1.5.x/STATUS yet.) On 11/23/2010 10:31 AM, Hyrum K. Wright wrote: I know folks are busy (hint, hint) testing and signing 1.6.15, but just a reminder that 1.5.8 will roll next week. In updating CHANGES for the release (see r1038163), there is actually some good stuff in there already. Please also review STATUS, so we can get any more low-hanging fixes in there. (I didn't see the 'blame -g' memory fix in CHANGES, but it should go into 1.5.8, imho) There is also the question of whether a vote for a change on the 1.6.x branch constitutes a vote on the 1.5.x, but I'll save that bikeshed for another day. :) -Hyrum On Tue, Nov 9, 2010 at 9:02 PM, Hyrum K. Wright hyrum_wri...@mail.utexas.edu wrote: In light of some of the recent activity on the 1.5.x branch, together with the policy that we still maintain that branch, I'm planning on rolling a 1.5.8 release. Next week will be 1.6.14, the week following is the Thanksgiving holidays in the States, so I'll push it to Dec. 1, if nobody has any objections. I'd invite folks to review 1.5.x/STATUS, and to think about revisions that we could perhaps merge to 1.5.x, including serious performance, security or correctness bug fixes. I don't think we need to fix every bug in the last 20 months, but if there is something that stands out, please consider nominating it. I can't guarantee that this won't be the last release from the 1.5.x line, but it is definitely a possibility. -Hyrum -- C. Michael Pilato cmpil...@collab.net CollabNet www.collab.net Distributed Development On Demand
diff-optimizations-tokens branch: I think I'm going to abandon it
Hi devs, As mentioned in [1], I've created two branches to try out two different approaches for the diff optimizations of prefix/suffix scanning. The first one, diff-optimizations-bytes, has a working implementation of the optimization. It still has some open todo items, but it basically works. The second one, diff-optimizations-tokens, takes a more high-level approach by working in the token handling layer. It takes the extracted lines as a whole, and compares them, to scan for identical prefix and suffix. I preferred this new approach, because it seemed more elegant (and works both for diff_file and diff_memory (property diffs)). However, although the token-based prefix scanning works adequately, I'm now stuck with the suffix scanning. I am now considering to abandon the tokens-approach, for the following reasons: 1) There is still a lot of work. Scanning for identical suffix is quite difficult, because we now have to extract tokens (lines) in reverse. I've put in place a stub for that function (datasource_get_previous_token), but that still needs to be implemented. And that's the hardest part, IMHO. Not only that, but I just realized that I'll also have to implement a reverse variant of util.c#svn_diff__normalize_buffer (which contains the encouraging comment It only took me forever to get this routine right,... (added by ehu in r866123)), and maybe also of token_compare (not sure). 2) I'm beginning to see that token-based suffix scanning will not be as fast as byte-based suffix scanning. Simply because, in the case of byte-based suffix scanning, we can completely ignore line structure. We never have to compare characters with \n or \r, we just keep reading bytes and comparing them. So there is an extra overhead for token-based suffix scanning. So, unless someone can convince me otherwise, I'm probably going to stop with the token approach. Because of 2), I don't think it's worth it to spend the effort needed for 1), especially because the byte-based approach already works. Any thoughts? Cheers, -- Johan [1] http://svn.haxx.se/dev/archive-2010-11/0416.shtml
Re: [PATCH] extend svn_subst_translate_string() to record whether re-encoding and/or line ending translation were performed (v. 4)
Attached is version 4 of the patch and corresponding log message that address Daniel Shahaf's feedback from November 9. Per a message from Julian (http://article.gmane.org/gmane.comp.version-control.subversion.devel/124073) and Daniel Shahaf (http://article.gmane.org/gmane.comp.version-control.subversion.devel/124082), I have removed the changes to optimize translate_newline() for now. There is value for a summary line --- just like the Conflicts summary in 'svn up' and friends (svn/notify.c): if someone runs 'svnsync sync' manually and doesn't pipe the output into a pager, they still know at the end something was wrong. Good point. I could do a summary at the end as well as more detailed messages for each revision that is affected. +/** Translate the data in @a value (assumed to be encoded in charset + * @a encoding) to UTF8 and LF line-endings. If @a encoding is @c NULL, + * then assume that @a value is in the system-default character encoding. You changed 'language encoding' to 'character encoding'. Changed back. Note that the docstring has been reworded since version 2 of the patch. I am now following the new wording and modified it as appropriate. I'm sorry: I wasn't clear. I wasn't actually asking you to revert that, but just tried to ask why you made that change (because it is departs from the existing wording, but I didn't recall seeing the change mentioned anywhere). Okay. Not related to your patch, but the above if/else block calls the UTF-8 translation routines on value-data. Since this is specifically the translate_string() API (and there is a separate translate_cstring() API), I think either we should fix this (the whole reason svn_string_t exists is to allow embedded NULs) or at least document this limitation in the API docs. value-data can be NULL? NUL != NULL. In general, the 'data' member of an svn_string_t is never a NULL pointer. However, the memory it points to --- value-data[0] through value-data[value-len-1] --- may contain NUL (aka ASCII 0, aka '\0') bytes. I understand now. Also: I forgot to say this earlier, but parts of this subst.c code have been refactored on the 'performance' branch. Some of those refactorings have been merged, but I'm not sure if all of them were. Could you have a look there and see if there are any unmerged changes there? And if so, how they relate to this patch? Attached is a diff of subversion/libsvn_subr/subst.c from tr...@1040648 to branches/performa...@1040648. The unmerged changes were to add static functions translated_stream_skip() and translated_stream_buffered() and use them in svn_subst_read_specialfile() when configuring the svn_stream_t S. My changes affect different parts of the file. (translation_baton): Added the TRANSLATED_EOL and TRANSLATE_NEWLINE_FN fields. (create_translation_baton): Added a new parameter TRANSLATED_EOL that is passed to the resulting translation_baton. Sets TRANSLATE_NEWLINE_FN to either translate_newline or translate_newline_alt as appropriate. (translate_chunk): Replaced the three calls to translate_newline() with calls to TRANSLATE_NEWLINE_FN. Present tense, so s/Added/Add/, etc. Fixed. * subversion/libsvn_subr/deprecated.c Received the implementation of the deprecated wrapper function svn_subst_translate_string(). Write in the standard file-symbol syntax: * subversion/libsvn_subr/deprecated.c (svn_subst_translate_string): ... Fixed. Index: subversion/include/svn_subst.h === --- subversion/include/svn_subst.h (revision 1032431) +++ subversion/include/svn_subst.h (working copy) @@ -592,19 +592,46 @@ svn_subst_stream_detranslated(svn_stream_t **strea /* EOL conversion and character encodings */ +/** @deprecated Provided for backward compatibility with the 1.6 API. Callers + * should use svn_subst_translate_string2(). + * + * Similar to svn_subst_translate_string2(), except that the information about + * whether re-encoding or line ending translation were performed is discarded. + */ Okay, except that the two paragraphs are in the wrong order, and the should use comment isn't needed in this case. Fixed. +SVN_DEPRECATED +svn_error_t *svn_subst_translate_string(svn_string_t **new_value, + const svn_string_t *value, + const char *encoding, + apr_pool_t *pool); + /** Translate the string @a value from character encoding @a encoding to * UTF8, and also from its current line-ending style to LF line-endings. If * @a encoding is @c NULL, translate from the system-default encoding. * + * If @a translated_to_utf8 is not @c NULL, then + * code*translated_to_utf8/code is set to @c TRUE if at least one + * character of @a value in the source character encoding was translated to + *
1.7.x - merge now accesses all files in WC?
Hi, Running a merge with a 1.7.x build, SVN now seems to access every file under a given working copy, whereas 1.6.x did not. The following observations have been made using Process Monitor. Under 1.6.x, the following file(s) are accessed merging a revision (modifying bar.txt only): - .svn\entries - .svn\prop-base\foo.c.svn-base (for files with existing properties) - bar.txt Under 1.7.x, the following file(s) are accessed (merging the same revision as above): - .svn\wc.db - Every versioned file in the working copy I can't see any reason why all these files would need to be accessed. I seem to recall some discussion about preventing/warning merging into modified working copies, could this be the cause? I've tried modifying files within the WC, but the merge still succeeds. --- Daniel Becroft
Re: Deprecate svn_fs_revision_prop() and svn_fs_txn_prop()?
Blair Zajac wrote on Tue, Nov 30, 2010 at 16:42:22 -0800: We allow binary data to be stored in revision properties, and given that svn_fs_revision_prop() and svn_fs_txn_prop() return const char * data, Is your svn_fs_revision_prop() different from mine? [[[ /** Set @a *value_p to the value of the property named @a propname on * transaction @a txn. If @a txn has no property by that name, set * @a *value_p to zero. Allocate the result in @a pool. */ svn_error_t * svn_fs_txn_prop(svn_string_t **value_p, svn_fs_txn_t *txn, const char *propname, apr_pool_t *pool); /** Set @a *value_p to the value of the property named @a propname on * revision @a rev in the filesystem @a fs. If @a rev has no property by * that name, set @a *value_p to zero. Allocate the result in @a pool. */ svn_error_t * svn_fs_revision_prop(svn_string_t **value_p, svn_fs_t *fs, svn_revnum_t rev, const char *propname, apr_pool_t *pool); ]]]
Re: diff-optimizations-tokens branch: I think I'm going to abandon it
Johan Corveleyn wrote on Wed, Dec 01, 2010 at 00:25:27 +0100: I am now considering to abandon the tokens-approach, for the following reasons: ... So, unless someone can convince me otherwise, I'm probably going to stop with the token approach. Because of 2), I don't think it's worth it to spend the effort needed for 1), especially because the byte-based approach already works. In other words, you're saying that the token-based approach: (b) won't be as fast as the bytes-based approach can be, and (a) requires much effort to be spent on implementing the reverse reading of tokens? (i.e., a backwards gets()) Any thoughts? -tokens/BRANCH-README mentions one of the advantages of the tokens approach being that the comparison is done only after whitespace and newlines have been canonicalized (if -x-w or -x--ignore-eol-style is in effect). IIRC, the -bytes approach doesn't currently take advantage of these -x flags? What is the practical effect of the fact the -bytes approach doesn't take advantage of these flags? If a file (with a moderately long history) has had all its newlines changed in rN, then I assume that your -bytes optimizations will speed up all the diffs that 'blame' performs on that file, except for the single diff between rN and its predecessor?
Re: svnsync failure when syncing with a repository that used ISO-8859-1 for log messages
Danny Trebbien wrote on Tue, Nov 30, 2010 at 07:35:38 -0800: I suspect that when all is said and done, the version of Subversion that will contain the patch will be in the 1.7.x series. Indeed. We don't add new features in patch releases (1.6.0-1.6.x), and I'm afraid that Danny's patches do qualify as new features in this context... (They add new API's and new --flags.) Daniel
Re: svn commit: r1040831 - in /subversion/trunk/subversion: include/svn_checksum.h libsvn_subr/checksum.c
stef...@apache.org wrote on Tue, Nov 30, 2010 at 23:56:41 -:
Author: stefan2
Date: Tue Nov 30 23:56:40 2010
New Revision: 1040831
URL: http://svn.apache.org/viewvc?rev=1040831view=rev
Log:
Fix 'svnadmin verify' for format 5 repositories. During the checking process,
they yield NULL for SHA1 checksums. The most robust way to fix that is to
allow NULL for checksum in svn_checksum_to_cstring. This seems justified
as NULL is already a valid return value instead of e.g. an empty string. So,
callers may receive NULL as result and could therefore assume that NULL is
a valid input, too
Can you explain how to trigger the bug you are fixing? Just running
'svnadmin verify' on my r1040058-created Greek repository doesn't.
* subversion/include/svn_checksum.h
(svn_checksum_to_cstring): extend doc string
* subversion/libsvn_subr/checksum.c
(svn_checksum_to_cstring): return NULL for NULL checksums as well
Modified:
subversion/trunk/subversion/include/svn_checksum.h
subversion/trunk/subversion/libsvn_subr/checksum.c
Modified: subversion/trunk/subversion/include/svn_checksum.h
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_checksum.h?rev=1040831r1=1040830r2=1040831view=diff
==
--- subversion/trunk/subversion/include/svn_checksum.h (original)
+++ subversion/trunk/subversion/include/svn_checksum.h Tue Nov 30 23:56:40
2010
@@ -121,7 +121,7 @@ svn_checksum_to_cstring_display(const sv
/** Return the hex representation of @a checksum, allocating the
* string in @a pool. If @a checksum-digest is all zeros (that is,
- * 0, not '0'), then return NULL.
+ * 0, not '0') or NULL, then return NULL.
*
First, this change doesn't match the code change: the docstring change
says CHECKSUM-DIGEST may be NULL, but the code change allows CHECKSUM
to be NULL.
Second, let's have the docstring say that NULL is only allowed by 1.7+.
(Passing NULL will segfault in 1.6.)
(doxygen has a @note directive.)
* @since New in 1.6.
*/
Modified: subversion/trunk/subversion/libsvn_subr/checksum.c
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/checksum.c?rev=1040831r1=1040830r2=1040831view=diff
==
--- subversion/trunk/subversion/libsvn_subr/checksum.c (original)
+++ subversion/trunk/subversion/libsvn_subr/checksum.c Tue Nov 30 23:56:40
2010
@@ -135,6 +135,9 @@ const char *
svn_checksum_to_cstring(const svn_checksum_t *checksum,
apr_pool_t *pool)
{
+ if (checksum == NULL)
+return NULL;
+
switch (checksum-kind)
{
case svn_checksum_md5:
Re: Deprecate svn_fs_revision_prop() and svn_fs_txn_prop()?
On 11/30/10 6:20 PM, Daniel Shahaf wrote: Blair Zajac wrote on Tue, Nov 30, 2010 at 16:42:22 -0800: We allow binary data to be stored in revision properties, and given that svn_fs_revision_prop() and svn_fs_txn_prop() return const char * data, Is your svn_fs_revision_prop() different from mine? Nevermind, I was reading it wrong. Too long at the puter :) Blair
Re: [PATCH] Fix Makefile.svn to build APR with threads
Hi Stefan, Thanks for the suggestions. How about this? [[[ Makefile.svn: Optionally allow building with threading support * tools/dev/unix-build/Makefile.svn: Add a new THREADING variable to control whether APR and sqlite should be built with threading support. Suggested by: stsp Review by: stsp ]]] Index: tools/dev/unix-build/Makefile.svn === --- tools/dev/unix-build/Makefile.svn (revision 1040690) +++ tools/dev/unix-build/Makefile.svn (working copy) @@ -233,6 +233,12 @@ fi touch $@ +ifdef THREADING +THREADS_FLAG=--enable-threads +else +THREADS_FLAG=--disable-threads +endif + # configure apr $(APR_OBJDIR)/.configured: $(APR_OBJDIR)/.retrieved cp $(APR_SRCDIR)/build/apr_hints.m4 \ @@ -246,7 +252,7 @@ $(APR_SRCDIR)/configure \ --prefix=$(PREFIX)/apr \ --enable-maintainer-mode \ - --disable-threads + $(THREADS_FLAG) touch $@ # compile apr @@ -704,6 +710,12 @@ tar -C $(SRCDIR) -zxf $(DISTDIR)/$(SQLITE_DIST) touch $@ +ifdef THREADING +THREADSAFE_FLAG=--enable-threadsafe +else +THREADSAFE_FLAG=--disable-threadsafe +endif + # configure sqlite $(SQLITE_OBJDIR)/.configured: $(SQLITE_OBJDIR)/.retrieved cd $(SQLITE_OBJDIR) \ @@ -711,7 +723,7 @@ $(SQLITE_SRCDIR)/configure \ --prefix=$(PREFIX)/sqlite \ --disable-tcl \ - --disable-threadsafe + $(THREADSAFE_FLAG) touch $@ # compile sqlite
