Re: Fix for issue 3620 - unlock command
On Mon, 2010-11-15, Noorul Islam K M wrote: Noorul Islam K M noo...@collab.net writes: Noorul Islam K M noo...@collab.net writes: Make 'svn unlock' verify that both working copy paths and URLs are not passed. * subversion/tests/cmdline/input_validation_tests.py (invalid_unlock_targets): New test, verifying that svn unlock copes well with invalid target combinations. * subversion/svn/unlock-cmd.c (svn_cl__unlock): For consistency with other sub-commands, raise the SVN_ERR_CL_ARG_PARSING_ERROR if both working copy paths and URLs are passed, and use the same error message also used elsewhere. [...] Hi Noorul. I committed this as r1035208. Thanks again for this series of patches. Once this patch goes, I think we can wrap this issue. I tested by hand and found three more :-) svn patch foo ^/ svn relocate ^/ ^/ ^/ svn switch ^/ ^/ I mentioned them in the issue: http://subversion.tigris.org/issues/show_bug.cgi?id=3620. If you'd like to address those too, that would be wonderful. You don't have to, of course. - Julian
Re: [PATCH] extend svn_subst_translate_string() to record whether re-encoding and/or line ending translation were performed (v. 3)
On Sat, 2010-11-13, Daniel Trebbien wrote: The only difference between translate_newline() and translate_newline_alt() is the lines of code from here to the end of the function. I think you could keep translate_newline() as is, and just move these checks elsewhere (to a new function or to the caller; I haven't thought about this in detail). In any case, please avoid the code duplication if possible. (Function pointers are nice, but sometimes just a way to shoot yourself in the foot.) If you're unsure what approach to take, do brainstorm on-list before sending a new iteration of the patch. (That saves time to you and to us) After thinking about this problem some more, I thought about using a macro solution. [...] Do you like this idea? I have attached C code that I have tested. In this sample, the macro that outputs the definitions of translate_newline() and translate_newline_alt() is called DEFINE_TRANSLATE_NEWLINE_FN. Hi Daniel T. Sorry to jump in here just to say this, but: no, please don't define the function inside a macro. I'm not sure how to concisely explain the reasons why, but I'd be happy to try if you need me to, although I'm sure you can basically see or feel why. And also let's not have one function overwrite a pointer to itself with a pointer to another function. I don't think the problem we are facing needs such a complex solution. Let's step back. Someone mentioned a concern about efficiency of checking something once per newline. What's the check? Just if ((newline_len != eol_str_len) || (*newline_buf != *eol_str))? If so, that's nothing to worry about. Or if there's a more expensive check, you could prefix it with if (! translated_eol). That's the most complexity it needs, I think! As for Daniel S's subpool question - my guess is that he's right in thinking that creating and destroying a subpool is not worthwhile. - Julian
Re: 1.6.14 tarballs rolling next week (11/17)
On Tue, Nov 9, 2010 at 11:52 AM, Hyrum K. Wright hyrum_wri...@mail.utexas.edu wrote: There's been a couple of high-profile bugs found and fixed, so I'd like to go ahead and roll 1.6.14 next week. Please nominate, review, vote, etc. Another reminder that 1.6.14 is rolling this Wednesday. -Hyrum
Translation help
To any current and/or interested translators: In an effort to make translation of Subversion easier, and more complete, I've installed an instance of Pootle, a web-based translation tool. You can access it here: http://translate.hyrumwright.org/projects/svn/ It currently allows anonymous submission of translation suggestions, but only users authorized for the project and language may submit changes. These changes will not be automatically committed to the repository upon submission, but will be, perhaps by a daily cron job. Please keep in mind that the various guidelines around localization in HACKING are still in effect: http://subversion.apache.org/docs/community-guide/l10n.html Anyway, I'm interesting whatever feedback translators (or even non-translators, such as myself) have about the use of such a tool for Subversion. I plan on forwarding this to us...@s.a.o and possibly commun...@a.o if/when people decide the above approach is a good idea. -Hyrum
Re: svn commit: r1034139 - in /subversion/trunk/subversion: libsvn_client/patch.c tests/cmdline/patch_tests.py
On Fri, Nov 12, 2010 at 03:14:13AM +0200, Daniel Shahaf wrote: Stefan Sperling wrote on Fri, Nov 12, 2010 at 01:07:53 +0100: On Thu, Nov 11, 2010 at 09:21:26PM -, dan...@apache.org wrote: Add support for handling symlinks in 'svn patch'. If you haven't done so, can you close the corresponding issue, too? Issue #3697. You beat me too it, sorry about beeing so slow. :) Daniel (dannas)
Re: [PATCH] Bug in svn_fs_paths_changed2() Python bindings?
Hi all, On Wednesday, August 11, 2010 01:09:50 pm C. Michael Pilato wrote: On 08/11/2010 03:10 PM, C. Michael Pilato wrote: On 08/10/2010 09:22 PM, Alexey Neyman wrote: Okay, try again: [[[ Fix the type of structures returned in bindings from svn_fs_paths_changed2(). * subversion/include/svn_fs.h (svn_fs_paths_changed2): Rename the argument from changed_paths_p to changed_paths_p2, so that it's different from argument to svn_fs_paths_changed(). Minor nit -- I think 'changed_paths2_p' would be a better name. To me, a number at the very end of a parameter name says differentiator (as in 'strcmp(str1, str2)') whereas having that '2' closer to the 'changed_paths' name says version iterator. But like I said, it's a minor nit. Otherwise, +1 on the patch. Committed with the aforementioned tweaks in r984565. Thanks, Alexey. Could this patch be picked up in 1.6.14? Regards, Alexey.
Re: [PATCH] make check-ctypes-python fails when executed from non src build direction
Hi Everyone, Ping. This thread has received no further comments. Gavin Beau Baumanis E: gav...@thespidernet.com On 01/11/2010, at 9:24 PM, Julian Foad wrote: On Mon, 2010-11-01, Noorul Islam K M wrote: I get the following error when I try executing $ make check-ctypes-python from build directory other then source directory. [...] ImportError: No module named functions make: *** [check-ctypes-python] Error 1 I also build in a separate directory, and have that same problem. But ... * build/run_ctypesgen.sh: Use source directory as target instead of build directory. (cat $abs_srcdir/$cp_relpath/csvn/core/functions.py.in; \ sed -e '/^FILE =/d' $output | \ perl -pe 's{(\s+\w+)\.restype = POINTER\(svn_error_t\)}{\1.restype = POINTER(svn_error_t)\n\1.errcheck = _svn_errcheck}' \ - ) $abs_builddir/$cp_relpath/csvn/core/functions.py + ) $abs_srcdir/$cp_relpath/csvn/core/functions.py I looked for other references to functions.py, to check whether anything else expects it to be in the build dir. I found that subversion/bindings/ctypes-python/setup.py:build_functions_py() also builds functions.py from functions.py.in. Why the duplication, and should we un-duplicate it? Here is the change that introduced this: [[[ r877501 | gstein | 2009-04-22 16:28:24 +0100 (Wed, 22 Apr 2009) | 12 lines First step in integrating ctypesgen tighter into the build system. This adds a little script to invoke ctypesgen rather than relying on setup.py and distutils (don't get me started on that package). * build/run_ctypesgen.sh: (): new script to invoke ctypesgen, given a bunch of configuration parameters as arguments. take particular care that we only write to the build tree, not the source tree. * Makefile.in: (ctypes-python): use new helper script Index: subversion/trunk/Makefile.in === --- subversion/trunk/Makefile.in (revision 877500) +++ subversion/trunk/Makefile.in (revision 877501) @@ -797,17 +797,13 @@ install-swig-rb-doc: # ctypes-python variables and make targets CTYPESGEN = @CTYPESGEN@ CTYPES_PYTHON_SRC_DIR = $(abs_srcdir)/subversion/bindings/ctypes-python ctypes-python: local-all - cd $(CTYPES_PYTHON_SRC_DIR); \ - $(LT_EXECUTE) $(PYTHON) setup.py build --subversion=$(prefix) \ - --apr=$(SVN_APR_PREFIX) --apr-util=$(SVN_APRUTIL_PREFIX) \ - --ctypesgen=$(CTYPESGEN) --svn-headers=$(abs_srcdir)/subversion/include \ - --cppflags=$(CPPFLAGS) --ldflags=$(EXTRA_CTYPES_LDFLAGS) + $(abs_srcdir)/build/run_ctypesgen.sh $(LT_EXECUTE) $(CPPFLAGS) $(EXTRA_CTYPES_LDFLAGS) $(PYTHON) $(CTYPESGEN) $(abs_srcdir) $(abs_builddir) $(prefix) $(SVN_APR_PREFIX) $(SVN_APRUTIL_PREFIX) install-ctypes-python: ctypes-python cd $(CTYPES_PYTHON_SRC_DIR); \ $(PYTHON) setup.py install --prefix=$(DESTDIR)$(prefix) check-ctypes-python: ctypes-python ]]] It looks like build_functions_py() expects to create functions.py in the *source* directory, and build/run_ctypesgen.sh intentionally creates it in the *build* directory. - Julian
Re: [PATCH] Use svn_fs_fs__id_unparse() to construct the noderev cache key
Ping. This has received no further comments. Gavin Beau Baumanis E: gav...@thespidernet.com On 31/10/2010, at 12:42 PM, Daniel Shahaf wrote: Is there a reason not to apply this let's not reinvent the wheel patch? [[[ Index: subversion/libsvn_fs_fs/fs_fs.c === --- subversion/libsvn_fs_fs/fs_fs.c (revision 1029231) +++ subversion/libsvn_fs_fs/fs_fs.c (working copy) @@ -2205,15 +2205,14 @@ read_rep_offsets(representation_t **rep_p, return SVN_NO_ERROR; } -/* Combine the revision and offset of the ID to a string that will - * be used as a cache key. Allocations will be made from POOL. +/* Return a string that uniquely identifies the noderev with the + * given ID, for use as a cache key. */ static const char * get_noderev_cache_key(const svn_fs_id_t *id, apr_pool_t *pool) { - return svn_fs_fs__combine_two_numbers(svn_fs_fs__id_rev(id), -svn_fs_fs__id_offset(id), -pool); + const svn_string_t *id_unparsed = svn_fs_fs__id_unparse(id, pool); + return id_unparsed-data; } /* Look up the NODEREV_P for ID in FS' node revsion cache. If noderev ]]] (It does not resolve my test failures.) Daniel
Re: [PATCH] Fix Perl bindings for svn_auth_get_platform_specific_client_providers
Ping. This patch submission has received no comments. Gavin Beau Baumanis E: gav...@thespidernet.com On 02/11/2010, at 1:42 AM, Matthijs Kooijman wrote: Hi folks, this is a resubmit of a patch that got lost in the list earlier. The patch is a trivial patch, which adds perl bindings for the apr_array_header_t **providers type, which fixes the svn_auth_get_platform_specific_client_providers function. The patch also includes a basic test case. Gr. Matthijs swig-provider-array.patch
Re: [PATCH] Indentation error in swig binding
Hi Phillip / Prabhu, Just checking if there is anything left to do for this thread. Not being a SVN developer - I can't tell if the initial patch is no longer required if the correct version of dependencies is used, or whether there is still some further discussion required. Gavin Beau Baumanis E: gav...@thespidernet.com On 01/11/2010, at 11:09 PM, Philip Martin wrote: prabhugnanasundar prabh...@collab.net writes: I am using Debian Lenny. I get swig 1.3.25 with it by default. When I According to http://packages.debian.org/search?keywords=swig you should get 1.3.36. That's what I have on my Debian/lenny box. try to install the swig-py, I get indentation error at core.py file (line 3178). Since it is an auto generated code from core.i file, modifying the core.i file solved the problem. My friend uses swig 1.3.40 which comes with Fedora core 13. In that version everything works fine even if the indentation is changed at will. Though in swig 1.3.25 it works iff the indentation is proper. Hence the problem can be solved by indenting the core.i file. With 1.3.36 the generated code looks like: def svn_auth_open(*args): svn_auth_open(svn_auth_baton_t auth_baton, apr_array_header_t providers, apr_pool_t pool) val = apply(_core.svn_auth_open, args) val.__dict__[_deps] = list(args[0]) return val With your patch it changes to: def svn_auth_open(*args): svn_auth_open(svn_auth_baton_t auth_baton, apr_array_header_t providers, apr_pool_t pool) val = apply(_core.svn_auth_open, args) val.__dict__[_deps] = list(args[0]) return val which doesn't work. Running make check-swig-py gives: File /home/pm/sw/subversion/obj/subversion/bindings/swig/python/libsvn/core.py, line 2919 val.__dict__[_deps] = list(args[0]) ^ IndentationError: unexpected indent make: *** [check-swig-py] Error 1 In r911480 rdonch has fixed an indentation error which he claims has solved the problem. But he did not mention the swig version. He has changed the indentation from two to three, but that does not work for From three to two. swig 1.3.25. Hope my patch solves everyone's problem. I have the log and the patch as follows... [[[ Clear the indentation problem in swig * subversion/bindings/swig/core.i (svn_auth_open): The default indentation of the file is found to be 2. subversion/bindings/swig/python/libsvn/core.py file generates indentation of 4. Patch by: Prabhu Gnana Sundar prabh...@collab.net ]]] Index: subversion/bindings/swig/core.i === --- subversion/bindings/swig/core.i (revision 1029575) +++ subversion/bindings/swig/core.i (working copy) @@ -751,7 +751,7 @@ # references to the providers are gone, they will still be alive, # keeping the baton valid. %feature(pythonappend) svn_auth_open %{ - val.__dict__[_deps] = list(args[0]) +val.__dict__[_deps] = list(args[0]) %} #endif -- Philip
Re: [PATCH] svn-dev.el - Update paths and links
Ping. This patch submission has received no comments. Gavin Beau Baumanis E: gav...@thespidernet.com On 08/11/2010, at 11:27 PM, Noorul Islam K M wrote: Log [[[ Update paths and links * tools/dev/svn-dev.el (svn-site-source-tree-top): Rename svn-source-tree-top as svn-site-source-tree-top as now the web site code resides in a separate location. Update doc string. (svn-faq-file, svn-faq-file, svn-url-base, svn-hacking-url): Update location ]]] Thanks and Regards Noorul Index: tools/dev/svn-dev.el === --- tools/dev/svn-dev.el (revision 1032461) +++ tools/dev/svn-dev.el (working copy) @@ -152,15 +152,18 @@ ;;; Net if you don't have a local copy, but it requires a very recent ;;; version of Emacs, so I didn't bother with it here. -kfogel) -(defvar svn-source-tree-top (expand-file-name ~/projects/svn/) - *Top directory of your Subversion source tree. You almost -certainly want to set this in your .emacs, to override the default; -use `(setq svn-source-tree-top \/path/to/the/tree\)'.) +(defvar svn-site-source-tree-top (expand-file-name ~/projects/svn/site/) + *Top directory of your Subversion site source tree of +repository \http://svn.apache.org/repos/asf/subversion/site\;. +You almost certainly want to set this in your .emacs, to override +the default; use `(setq svn-site-source-tree-top +\/path/to/the/site/tree\)'.) -(defvar svn-faq-file (concat svn-source-tree-top /www/faq.html) +(defvar svn-faq-file (concat svn-site-source-tree-top /publish/faq.html) *A local copy of the Subversion FAQ.) -(defvar svn-hacking-file (concat svn-source-tree-top /www/hacking.html) +(defvar svn-hacking-file (concat svn-site-source-tree-top + /docs/community-guide/community-guide.html) *A local copy of the Subversion hacking.html file.) ;; Helper for referring to issue numbers in a user-friendly way. @@ -188,11 +191,13 @@ (start (car bounds)) (end (cdr bounds))) (delete-region start end))) - (insert (format http://svn.collab.net/viewcvs/svn?rev=%sview=rev; rev))) + (insert (format http://svn.apache.org/viewcvs?view=revisionrevision=%s; + rev))) -(defconst svn-url-base http://subversion.tigris.org/;) +(defconst svn-url-base http://subversion.apache.org/;) (defconst svn-faq-url (concat svn-url-base faq.html)) -(defconst svn-hacking-url (concat svn-url-base hacking.html)) +(defconst svn-hacking-url (concat svn-url-base + docs/community-guide/community-guide.html)) (defun svn-html-get-targets (file) Build a list of targets for the Subversion web file FILE.
Re: [PATCH] error leak on performance branch
Hi Stefan, Just checking if there is anything remainig with this patch? I haven't noticed a committed reply or any further comments. Gavin Beau Baumanis E: gav...@thespidernet.com On 09/11/2010, at 3:33 AM, Stefan Sperling wrote: On Mon, Nov 08, 2010 at 06:56:16AM -0600, Hyrum K. Wright wrote: On Sun, Nov 7, 2010 at 11:58 AM, Stefan Sperling s...@elego.de wrote: [[[ * subversion/libsvn_fs_util/caching.c (svn_fs__get_global_membuffer_cache): Fix a possible error leak. ]]] Index: subversion/libsvn_fs_util/caching.c === --- subversion/libsvn_fs_util/caching.c (revision 1032308) +++ subversion/libsvn_fs_util/caching.c (working copy) @@ -62,6 +62,7 @@ svn_fs__get_global_membuffer_cache(void) /* auto-allocate cache*/ apr_allocator_t *allocator = NULL; apr_pool_t *pool = NULL; + svn_error_t *err; if (apr_allocator_create(allocator)) return NULL; @@ -75,12 +76,17 @@ svn_fs__get_global_membuffer_cache(void) apr_allocator_max_free_set(allocator, 1); pool = svn_pool_create_ex(NULL, allocator); - svn_cache__membuffer_cache_create - (cache, - (apr_size_t)cache_size, - (apr_size_t)(cache_size / 16), - ! svn_fs_get_cache_config()-single_threaded, - pool); If we're choosing to ignore the error above, you can just wrap the entire call in an svn_error_clear(). No need to introduce and check a temp variable. Ah, nice trick. Thanks. + err = svn_cache__membuffer_cache_create + (cache, + (apr_size_t)cache_size, + (apr_size_t)(cache_size / 16), + ! svn_fs_get_cache_config()-single_threaded, + pool); + if (err) +{ + svn_error_clear(err); + return NULL; +} Does this early return actually change functionality? If so, this patch is about more than just fixing an error leak... It doesn't change functionality. Cache is NULL in this case, and we return it next: } return cache; I'm not sure though if svn_cache__membuffer_cache_create() guarantees that cache remains NULL in case of error. Maybe this should be documented as such to allow callers to ignore errors this way. New patch below, also fixing a space-before-paren and another similar error leak. [[[ * subversion/libsvn_fs_util/caching.c (svn_fs__get_global_membuffer_cache, svn_fs__get_global_file_handle_cache): Fix a possible error leak. ]]] Index: subversion/libsvn_fs_util/caching.c === --- subversion/libsvn_fs_util/caching.c (revision 1032333) +++ subversion/libsvn_fs_util/caching.c (working copy) @@ -75,12 +75,12 @@ svn_fs__get_global_membuffer_cache(void) apr_allocator_max_free_set(allocator, 1); pool = svn_pool_create_ex(NULL, allocator); - svn_cache__membuffer_cache_create - (cache, - (apr_size_t)cache_size, - (apr_size_t)(cache_size / 16), - ! svn_fs_get_cache_config()-single_threaded, - pool); + svn_error_clear(svn_cache__membuffer_cache_create( +cache, +(apr_size_t)cache_size, +(apr_size_t)(cache_size / 16), +! svn_fs_get_cache_config()-single_threaded, +pool)); } return cache; @@ -96,10 +96,10 @@ svn_fs__get_global_file_handle_cache(void) static svn_file_handle_cache_t *cache = NULL; if (!cache) -svn_file_handle_cache__create_cache(cache, -cache_settings.file_handle_count, -!cache_settings.single_threaded, -svn_pool_create(NULL)); +svn_error_clear(svn_file_handle_cache__create_cache( + cache, cache_settings.file_handle_count, + !cache_settings.single_threaded, + svn_pool_create(NULL))); return cache; }
Re: Interrupting an update after change of externals causes corrupt working copy
Almost forgot about this. It's now filed in the issue tracker: http://subversion.tigris.org/issues/show_bug.cgi?id=3751 Cheers, Johan On Thu, Sep 30, 2010 at 10:03 PM, Johan Corveleyn jcor...@gmail.com wrote: Hi devs, As per Daniel Shahaf's suggestion, I'm continuing this discussion on the dev list. See the previous mails in this thread on the users list for some context (or http://svn.haxx.se/users/archive-2010-09/0406.shtml). I'll summarize below. This issue reproduces with 1.6.12 as well as with trunk. It would be interesting to know the WC-NG folks take on this. Note: the subject line may be slightly exaggerated, as there seems to be a good workaround to repair things (at least in case of intra-repository externals). Summary --- Steps to reproduce: 1) Change a directory external definition. 2) svn update 3) Interrupt (Ctrl-C) while it is processing the switch of the external. I did this after some files of the external were already Updated. Result: svn status now shows a lot of files and directories as S (switched), and some directories missing. Running svn info on the switched files/dirs shows that they still point to the old external location. Running svn update again fixes the missing dirs, but not the switched nodes. svn revert does nothing (which is logical: there are no local changes). svn cleanup does nothing. Workaround: Running svn switch $new_url $external_dir brings everything back to normal. So: - Wouldn't it be better if svn update could repair this (as in resuming the interrupted update from before)? - Should I file an issue for this? - I'm not sure how all this plays out with externals from remote repositories (haven't tested this). With or without a change of the remote location. Would an svn switch --relocate fix things in the same way then? Note: this can be easily reproduced with any large (public) repository (I think). Just define some externals in working copy only (no need to commit), update, change external definition (no need to commit), update, interrupt. Cheers, -- Johan