You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Stefan Sperling <st...@elego.de> on 2010/11/12 00:07:53 UTC

Re: svn commit: r1034139 - in /subversion/trunk/subversion: libsvn_client/patch.c tests/cmdline/patch_tests.py

On Thu, Nov 11, 2010 at 09:21:26PM -0000, dannas@apache.org wrote:
> Author: dannas
> Date: Thu Nov 11 21:21:16 2010
> New Revision: 1034139
> 
> URL: http://svn.apache.org/viewvc?rev=1034139&view=rev
> Log:
> Add support for handling symlinks in 'svn patch'.

Nice :)

If you haven't done so, can you close the corresponding issue, too?
Thanks!

One question below:

> 
> * subversion/libsvn_client_patch.c
>   (patch_target_t): Add 'is_special' field.
>   (apply_one_patch): Record the possible presence of a svn:special 
>     property.
>   (install_patched_target): Install symlinks.
> 
> * subversion/tests/cmdline/patch_tests.py
>   (patch_add_symlink): New.
> 
> Modified:
>     subversion/trunk/subversion/libsvn_client/patch.c
>     subversion/trunk/subversion/tests/cmdline/patch_tests.py
> 
> Modified: subversion/trunk/subversion/libsvn_client/patch.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/patch.c?rev=1034139&r1=1034138&r2=1034139&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_client/patch.c (original)
> +++ subversion/trunk/subversion/libsvn_client/patch.c Thu Nov 11 21:21:16 2010
> @@ -203,6 +203,9 @@ typedef struct patch_target_t {
>    /* True if the patch changed any of the properties of the target. */
>    svn_boolean_t has_prop_changes;
>  
> +  /* True if the patch contained a svn:special property. */
> +  svn_boolean_t is_special;
> +
>    /* All the information that is specific to the content of the target. */
>    target_content_info_t *content_info;
>  
> @@ -1736,6 +1739,9 @@ apply_one_patch(patch_target_t **patch_t
>        prop_name = svn__apr_hash_index_key(hash_index);
>        prop_patch = svn__apr_hash_index_val(hash_index);
>  
> +      if (! strcmp(prop_name, SVN_PROP_SPECIAL))
> +        target->is_special = TRUE;
> +
>        /* We'll store matched hunks in prop_content_info. */
>        prop_target = apr_hash_get(target->prop_targets, prop_name, 
>                                   APR_HASH_KEY_STRING);
> @@ -2105,9 +2111,33 @@ install_patched_target(patch_target_t *t
>  
>        if (! dry_run && ! target->skipped)
>          {
> -          /* Copy the patched file on top of the target file. */
> -          SVN_ERR(svn_io_copy_file(target->patched_path,
> -                                   target->local_abspath, FALSE, pool));
> +          if (target->is_special)
> +            {
> +              svn_stream_t *stream;
> +              svn_stream_t *patched_stream;
> +              apr_file_t *file;
> +
> +              SVN_ERR(svn_io_file_open(&file, target->patched_path,
> +                                       APR_READ | APR_BINARY, APR_OS_DEFAULT,
> +                                       pool));
> +
> +              patched_stream = svn_stream_from_aprfile2(file, FALSE /* disown */,
> +                                                pool);
> +              SVN_ERR(svn_subst_create_specialfile(&stream, 
> +                                                   target->local_abspath,
> +                                                   pool, pool));
> +              SVN_ERR(svn_stream_copy3(patched_stream, stream, 
> +                                       NULL, /* cancel_func */ 
> +                                       NULL, /* cancel_baton */
> +                                       pool));
> +            }

What's the effect of the copy? Isn't svn_subst_create_specialfile() enough?

> +          else
> +            {
> +              /* Copy the patched file on top of the target file. */
> +              SVN_ERR(svn_io_copy_file(target->patched_path,
> +                                       target->local_abspath, FALSE, pool));
> +            }
> +

Re: svn commit: r1034139 - in /subversion/trunk/subversion: libsvn_client/patch.c tests/cmdline/patch_tests.py

Posted by Daniel Näslund <da...@longitudo.com>.
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 -0000, dannas@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: svn commit: r1034139 - in /subversion/trunk/subversion: libsvn_client/patch.c tests/cmdline/patch_tests.py

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Stefan Sperling wrote on Fri, Nov 12, 2010 at 01:07:53 +0100:
> On Thu, Nov 11, 2010 at 09:21:26PM -0000, dannas@apache.org wrote:
> > Author: dannas
> > Date: Thu Nov 11 21:21:16 2010
> > New Revision: 1034139
> > 
> > URL: http://svn.apache.org/viewvc?rev=1034139&view=rev
> > Log:
> > Add support for handling symlinks in 'svn patch'.
> 
> Nice :)
> 
> If you haven't done so, can you close the corresponding issue, too?
> Thanks!
> 

Issue #3697.

> One question below:
> 
> > +          if (target->is_special)
> > +            {
> > +              svn_stream_t *stream;
> > +              svn_stream_t *patched_stream;
> > +              apr_file_t *file;
> > +
> > +              SVN_ERR(svn_io_file_open(&file, target->patched_path,
> > +                                       APR_READ | APR_BINARY, APR_OS_DEFAULT,
> > +                                       pool));
> > +
> > +              patched_stream = svn_stream_from_aprfile2(file, FALSE /* disown */,
> > +                                                pool);
> > +              SVN_ERR(svn_subst_create_specialfile(&stream, 
> > +                                                   target->local_abspath,
> > +                                                   pool, pool));
> > +              SVN_ERR(svn_stream_copy3(patched_stream, stream, 
> > +                                       NULL, /* cancel_func */ 
> > +                                       NULL, /* cancel_baton */
> > +                                       pool));
> > +            }
> 
> What's the effect of the copy? Isn't svn_subst_create_specialfile() enough?
> 

No.  svn_subst_create_specialfile() returns a stream, and to create the
special file one should write its repository-normal form to that stream.

That's exactly what the code is doing.

> > +          else
> > +            {
> > +              /* Copy the patched file on top of the target file. */
> > +              SVN_ERR(svn_io_copy_file(target->patched_path,
> > +                                       target->local_abspath, FALSE, pool));
> > +            }
> > +