You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Arfrever Frehtes Taifersar Arahesis <Ar...@GMail.Com> on 2009/04/16 10:55:30 UTC
Re: svn commit: r37294 - in trunk/subversion: include libsvn_client
libsvn_wc tests/cmdline tests/cmdline/svntest
2009-04-16 03:25 Stefan Sperling <st...@elego.de> napisał(a):
> Author: stsp
> Date: Wed Apr 15 18:25:31 2009
> New Revision: 37294
>
> Log:
> Remove dependency on GNU patch, and use the new work-in-progress internal
> implementation instead.
>
> This commit was first saved with svn diff, reverted, re-applied
> with the internal svn patch, and diffed again, and the 2 diffs
> were compared with an external diff, and there was no difference.
>
> * subversion/tests/cmdline/svntest/main.py
> (has_patch): Remove.
>
> * subversion/tests/cmdline/patch_tests.py
> (gnupatch_garbage_re): Remove.
> (patch_basic): Track removal of above.
> (patch_unidiff, patch_copy_and_move): Expect output of internal patch only.
>
> * subversion/include/svn_wc.h
> (svn_wc_apply_unidiff): Undeclare.
>
> * subversion/libsvn_wc/patch.c
> (svn_wc_apply_unidiff): Remove.
>
> * subversion/libsvn_client/patch.c
> (svn_client_patch): Track removal of svn_wc_apply_unidiff, and lose the
> env var runtime check.
>
> Suggested by: rhuijben
> (and gstein said "rip out the spawn? yes")
>
> Modified:
> trunk/subversion/include/svn_wc.h
> trunk/subversion/libsvn_client/patch.c
> trunk/subversion/libsvn_wc/patch.c
> trunk/subversion/tests/cmdline/patch_tests.py
> trunk/subversion/tests/cmdline/svntest/main.py
>
> Modified: trunk/subversion/include/svn_wc.h
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/include/svn_wc.h?pathrev=37294&r1=37293&r2=37294
> ==============================================================================
> --- trunk/subversion/include/svn_wc.h Wed Apr 15 18:00:28 2009 (r37293)
> +++ trunk/subversion/include/svn_wc.h Wed Apr 15 18:25:31 2009 (r37294)
> @@ -5870,28 +5870,6 @@ svn_wc_apply_svnpatch(apr_file_t *decode
> void *diff_edit_baton,
> apr_pool_t *pool);
>
> -/**
> - * Run an external patch program against @a patch_path patch file. @a
> - * outfile and @a errfile are respectively connected to the external
> - * program's stdout and stderr pipes when executed. @a config is looked
> - * up for the SVN_CONFIG_OPTION_PATCH_CMD entry to use as the patch
> - * program. If missing or @a config is @c NULL, the function tries to
> - * execute 'patch' literally, which should work on most *NIX systems at
> - * least. This involves searching into $PATH. The external program is
> - * given the patch file via its stdin pipe.
> - *
> - * The program is passed the '--force' argument when @a force is set.
> - *
> - * @since New in 1.7
> - */
> -svn_error_t *
> -svn_wc_apply_unidiff(const char *patch_path,
> - svn_boolean_t force,
> - apr_file_t *outfile,
> - apr_file_t *errfile,
> - apr_hash_t *config,
> - apr_pool_t *pool);
> -
Public svn_wc_apply_svnpatch() function still exists. Would it make
sense to have a public function which applies unidiff parts of
patches?
> Modified: trunk/subversion/libsvn_wc/patch.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_wc/patch.c?pathrev=37294&r1=37293&r2=37294
> ==============================================================================
> --- trunk/subversion/libsvn_wc/patch.c Wed Apr 15 18:00:28 2009 (r37293)
> +++ trunk/subversion/libsvn_wc/patch.c Wed Apr 15 18:25:31 2009 (r37294)
> @@ -437,105 +437,3 @@ svn_wc_apply_svnpatch(apr_file_t *decode
> apr_pool_destroy(subpool);
> return SVN_NO_ERROR;
> }
> -
> -svn_error_t *
> -svn_wc_apply_unidiff(const char *patch_path,
> - svn_boolean_t force,
> - apr_file_t *outfile,
> - apr_file_t *errfile,
> - apr_hash_t *config,
> - apr_pool_t *pool)
> -{
> - const char *patch_cmd = NULL;
> - const char **patch_cmd_args;
> - const char *patch_cmd_tmp = NULL;
> - int exitcode = 0;
> - apr_exit_why_e exitwhy = 0;
> - svn_boolean_t patch_bin_guess = TRUE;
> - apr_file_t *patchfile;
> - int nargs = 3; /* the command, the prefix arg and NULL at least */
> - int i = 0;
> - apr_pool_t *subpool = svn_pool_create(pool);
> - svn_boolean_t dry_run = FALSE; /* disable dry_run for now */
> -
> - if (force)
> - ++nargs;
> -
> - if (dry_run)
> - ++nargs;
> -
> - patch_cmd_args = apr_palloc(subpool, nargs * sizeof(char *));
> -
> - if (config)
> - {
> - svn_config_t *cfg = apr_hash_get(config,
> - SVN_CONFIG_CATEGORY_CONFIG,
> - APR_HASH_KEY_STRING);
> - svn_config_get(cfg, &patch_cmd_tmp, SVN_CONFIG_SECTION_HELPERS,
> - SVN_CONFIG_OPTION_PATCH_CMD, NULL);
SVN_CONFIG_OPTION_PATCH_CMD should be removed from svn_config.h.
> - }
> -
> - if (patch_cmd_tmp)
> - {
> - patch_bin_guess = FALSE;
> - SVN_ERR(svn_path_cstring_to_utf8(&patch_cmd, patch_cmd_tmp,
> - subpool));
> - }
> - else
> - patch_cmd = "patch";
> -
> - patch_cmd_args[i++] = patch_cmd;
> - patch_cmd_args[i++] = "-p0"; /* TODO: make it smarter in detecting CWD */
> -
> - if (force)
> - patch_cmd_args[i++] = "--force";
> -
> - if (dry_run)
> - patch_cmd_args[i++] = "--dry-run";
> -
> - patch_cmd_args[i++] = NULL;
> -
> - assert(i == nargs);
> -
> - /* We want to feed the external program's stdin with the patch itself. */
> - SVN_ERR(svn_io_file_open(&patchfile, patch_path,
> - APR_READ, APR_OS_DEFAULT, subpool));
> -
> - /* Now run the external program. The parent process should close
> - * opened pipes/files. */
> - SVN_ERR(svn_io_run_cmd(".", patch_cmd, patch_cmd_args,
> - &exitcode, &exitwhy, TRUE, patchfile, outfile,
> - errfile, subpool));
> -
> - /* This is where we have to make the assumption that if the exitcode
> - * isn't 0 nor 1 then the external program got into trouble or wasn't
> - * even executed--command not found (see below). Basically we're
> - * trying to stick with patch(1) behaviour as stated in the man page:
> - * "patch's exit status is 0 if all hunks are applied successfully, 1
> - * if some hunks cannot be applied, and 2 if there is more serious
> - * trouble." */
> - if (exitcode != 0 && exitcode != 1)
> - {
> - /* This is the case when we're trying to execute 'patch' and got
> - * some weird exitcode.
> - * XXX: I haven't figured out how to check against the 'command
> - * not found' error, which returns exitcode == 255. Is there a
> - * macro somewhere to compare with? Let's use > 2 for now. */
> - if (patch_bin_guess && exitcode > 2)
> - return svn_error_create
> - (SVN_ERR_EXTERNAL_PROGRAM_MISSING, NULL, NULL);
SVN_ERR_EXTERNAL_PROGRAM_MISSING should be removed from svn_error_codes.h.
> Modified: trunk/subversion/tests/cmdline/patch_tests.py
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/tests/cmdline/patch_tests.py?pathrev=37294&r1=37293&r2=37294
> ==============================================================================
> --- trunk/subversion/tests/cmdline/patch_tests.py Wed Apr 15 18:00:28 2009 (r37293)
> +++ trunk/subversion/tests/cmdline/patch_tests.py Wed Apr 15 18:25:31 2009 (r37294)
> ...
> @@ -393,12 +372,9 @@ def patch_copy_and_move(sbox):
>
> # list all tests here, starting with None:
> test_list = [ None,
> - Wimp('Broken on platforms with non-GNU patch or non-\\n newlines',
> - SkipUnless(patch_basic, svntest.main.has_patch)),
> - Wimp('Broken on platforms with non-GNU patch or non-\\n newlines',
> - SkipUnless(patch_unidiff, svntest.main.has_patch)),
> - Wimp('Broken on platforms with non-GNU patch or non-\\n newlines',
> - SkipUnless(patch_copy_and_move, svntest.main.has_patch)),
> + patch_basic,
> + patch_unidiff,
> + patch_copy_and_move,
Definition of Wimp() at the beginning of this file should be removed.
--
Arfrever Frehtes Taifersar Arahesis
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1745104
Re: svn commit: r37294 - in trunk/subversion: include libsvn_client
libsvn_wc tests/cmdline tests/cmdline/svntest
Posted by Edmund Wong <ed...@kdtc.net>.
Greg Stein wrote:
>> So whoever created this mailing list software, aren't aware of these
>> issues? It would do havok to those using PGP-signed messages,
>> no? (Not that I noticed a lot of these here).
>
> Oh, they (CollabNet) are aware. They have some fixes for the various
> issues, but they aren't going to be fixed until the next release
> (whenever that may be).
It's from CollabNet? Interesting.
It's a pretty interesting ML software they have. They can take
an e-mail and hold it on a mod queue and then have the mods
sending a response (allowing comments too?). Then sending
the approved message to both the mailing list and the forum.
That's a pretty cool.
With the mailer.py, if Collabnet had a nntp server, it'd be
a cool bonus. Of course, posting on the nntp server might
complicate their processes since they'd have to take the
nntp post and save it to the mailing list as well. And plus
the fact moderating nntp newsgroups is a whole different
ball game. (Read only nntp groups? Would do well for
svn and issues lists.)
>
> And yah, it would probably throw off signed messages. But as you note,
> it is very rare for somebody in an Open Source community to sign a
> message as protection against tampering. Not that useful in an open
> environment :-P
Point well taken. :)
Edmund
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1765030
Re: svn commit: r37294 - in trunk/subversion: include libsvn_client
libsvn_wc tests/cmdline tests/cmdline/svntest
Posted by Greg Stein <gs...@gmail.com>.
On Fri, Apr 17, 2009 at 09:38, Edmund Wong <ed...@kdtc.net> wrote:
> Greg Stein wrote:
>
>>> Just a matter of curiosity and possibly a stupid question;
>>> but, why isn't there a message link at the end of your
>>> message, while there are in others?
>>
>> Who the hell knows? We have some crappy non-standard mailing list
>> software running this list. It does a lot of bizarre things that
>> mailing list software should not do. First and foremost, it should
>> pass messages intact (and maybe add some headers), but no... it
>> rewrites message content.
>>
>
> So whoever created this mailing list software, aren't aware of these
> issues? It would do havok to those using PGP-signed messages,
> no? (Not that I noticed a lot of these here).
Oh, they (CollabNet) are aware. They have some fixes for the various
issues, but they aren't going to be fixed until the next release
(whenever that may be).
And yah, it would probably throw off signed messages. But as you note,
it is very rare for somebody in an Open Source community to sign a
message as protection against tampering. Not that useful in an open
environment :-P
Cheers,
-g
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1764694
Re: svn commit: r37294 - in trunk/subversion: include libsvn_client
libsvn_wc tests/cmdline tests/cmdline/svntest
Posted by Edmund Wong <ed...@kdtc.net>.
Greg Stein wrote:
>> Just a matter of curiosity and possibly a stupid question;
>> but, why isn't there a message link at the end of your
>> message, while there are in others?
>
> Who the hell knows? We have some crappy non-standard mailing list
> software running this list. It does a lot of bizarre things that
> mailing list software should not do. First and foremost, it should
> pass messages intact (and maybe add some headers), but no... it
> rewrites message content.
>
So whoever created this mailing list software, aren't aware of these
issues? It would do havok to those using PGP-signed messages,
no? (Not that I noticed a lot of these here).
Edmund
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1763128
Re: svn commit: r37294 - in trunk/subversion: include libsvn_client
libsvn_wc tests/cmdline tests/cmdline/svntest
Posted by Greg Stein <gs...@gmail.com>.
On Thu, Apr 16, 2009 at 14:25, Edmund Wong <ed...@kdtc.net> wrote:
> Stefan Sperling wrote:
>> On Thu, Apr 16, 2009 at 12:31:42PM +0100, Stefan Sperling wrote:
>>> On Thu, Apr 16, 2009 at 12:55:30PM +0200, Arfrever Frehtes Taifersar Arahesis wrote:
>>>> Public svn_wc_apply_svnpatch() function still exists. Would it make
>>>> sense to have a public function which applies unidiff parts of
>>>> patches?
>>> Quite possibly, yes. We can make a new one, or promote the
>>> apply_textdiffs() function in libsvn_client/patch.c to public.
>>
>> On second thought, one public function should be enough.
>> It should accept both svnpatches or unidiffs, and do the right thing
>> for each.
>>
>> Stefan
>>
>
> Hi,
>
> Just a matter of curiosity and possibly a stupid question;
> but, why isn't there a message link at the end of your
> message, while there are in others?
Who the hell knows? We have some crappy non-standard mailing list
software running this list. It does a lot of bizarre things that
mailing list software should not do. First and foremost, it should
pass messages intact (and maybe add some headers), but no... it
rewrites message content.
Sigh.
-g
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1746565
Re: svn commit: r37294 - in trunk/subversion: include libsvn_client
libsvn_wc tests/cmdline tests/cmdline/svntest
Posted by Edmund Wong <ed...@kdtc.net>.
Stefan Sperling wrote:
> On Thu, Apr 16, 2009 at 12:31:42PM +0100, Stefan Sperling wrote:
>> On Thu, Apr 16, 2009 at 12:55:30PM +0200, Arfrever Frehtes Taifersar Arahesis wrote:
>>> Public svn_wc_apply_svnpatch() function still exists. Would it make
>>> sense to have a public function which applies unidiff parts of
>>> patches?
>> Quite possibly, yes. We can make a new one, or promote the
>> apply_textdiffs() function in libsvn_client/patch.c to public.
>
> On second thought, one public function should be enough.
> It should accept both svnpatches or unidiffs, and do the right thing
> for each.
>
> Stefan
>
Hi,
Just a matter of curiosity and possibly a stupid question;
but, why isn't there a message link at the end of your
message, while there are in others?
Edmund
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1746415
Re: svn commit: r37294 - in trunk/subversion: include libsvn_client
libsvn_wc tests/cmdline tests/cmdline/svntest
Posted by Greg Stein <gs...@gmail.com>.
On Thu, Apr 16, 2009 at 13:53, Arfrever Frehtes Taifersar Arahesis
<Ar...@gmail.com> wrote:
> 2009-04-16 13:33 Stefan Sperling <st...@elego.de> napisał(a):
>> On Thu, Apr 16, 2009 at 12:31:42PM +0100, Stefan Sperling wrote:
>>> On Thu, Apr 16, 2009 at 12:55:30PM +0200, Arfrever Frehtes Taifersar Arahesis wrote:
>>> > Public svn_wc_apply_svnpatch() function still exists. Would it make
>>> > sense to have a public function which applies unidiff parts of
>>> > patches?
>>>
>>> Quite possibly, yes. We can make a new one, or promote the
>>> apply_textdiffs() function in libsvn_client/patch.c to public.
>
> So the code which applies unidiff parts of patches is in libsvn_client,
> while the code which applies SVNPATCH1 blocks is in libsvn_wc?
> If yes, is this discrepancy intentional?
Certainly not. It is crappy code organization left over from the merge.
Cheers,
-g
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1746110
Re: svn commit: r37294 - in trunk/subversion: include libsvn_client
libsvn_wc tests/cmdline tests/cmdline/svntest
Posted by Arfrever Frehtes Taifersar Arahesis <Ar...@GMail.Com>.
2009-04-16 13:33 Stefan Sperling <st...@elego.de> napisał(a):
> On Thu, Apr 16, 2009 at 12:31:42PM +0100, Stefan Sperling wrote:
>> On Thu, Apr 16, 2009 at 12:55:30PM +0200, Arfrever Frehtes Taifersar Arahesis wrote:
>> > Public svn_wc_apply_svnpatch() function still exists. Would it make
>> > sense to have a public function which applies unidiff parts of
>> > patches?
>>
>> Quite possibly, yes. We can make a new one, or promote the
>> apply_textdiffs() function in libsvn_client/patch.c to public.
So the code which applies unidiff parts of patches is in libsvn_client,
while the code which applies SVNPATCH1 blocks is in libsvn_wc?
If yes, is this discrepancy intentional?
> On second thought, one public function should be enough.
> It should accept both svnpatches or unidiffs, and do the right thing
> for each.
+1.
--
Arfrever Frehtes Taifersar Arahesis
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1745936
Re: svn commit: r37294 - in trunk/subversion: include libsvn_client
libsvn_wc tests/cmdline tests/cmdline/svntest
Posted by Stefan Sperling <st...@elego.de>.
On Thu, Apr 16, 2009 at 12:31:42PM +0100, Stefan Sperling wrote:
> On Thu, Apr 16, 2009 at 12:55:30PM +0200, Arfrever Frehtes Taifersar Arahesis wrote:
> > Public svn_wc_apply_svnpatch() function still exists. Would it make
> > sense to have a public function which applies unidiff parts of
> > patches?
>
> Quite possibly, yes. We can make a new one, or promote the
> apply_textdiffs() function in libsvn_client/patch.c to public.
On second thought, one public function should be enough.
It should accept both svnpatches or unidiffs, and do the right thing
for each.
Stefan
Re: svn commit: r37294 - in trunk/subversion: include libsvn_client
libsvn_wc tests/cmdline tests/cmdline/svntest
Posted by Stefan Sperling <st...@elego.de>.
On Thu, Apr 16, 2009 at 12:55:30PM +0200, Arfrever Frehtes Taifersar Arahesis wrote:
> Public svn_wc_apply_svnpatch() function still exists. Would it make
> sense to have a public function which applies unidiff parts of
> patches?
Quite possibly, yes. We can make a new one, or promote the
apply_textdiffs() function in libsvn_client/patch.c to public.
> SVN_CONFIG_OPTION_PATCH_CMD should be removed from svn_config.h.
> SVN_ERR_EXTERNAL_PROGRAM_MISSING should be removed from svn_error_codes.h.
> Definition of Wimp() at the beginning of this file should be removed.
r37301.
Thanks,
Stefan