Mailing-List: contact dev-help@subversion.apache.org; run by ezmlm
Precedence: bulk
Received-SPF: pass (nike.apache.org: domain of kamesh@collab.net designates
 64.125.134.138 as permitted sender)
Message-ID: <4D245598.8010001@collab.net>
Date: Wed, 05 Jan 2011 16:57:20 +0530
From: Kamesh Jayachandran <kamesh@collab.net>
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US;
 rv:1.9.2.13) Gecko/20101209 Fedora/3.1.7-0.35.b3pre.fc14 Thunderbird/3.1.7
MIME-Version: 1.0
To: dev@subversion.apache.org
Subject: Re: [PATCH] make diff against copy-source by default
References: <1293721664.22966.42.camel@prabhugnanasundar>
In-Reply-To: <1293721664.22966.42.camel@prabhugnanasundar>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit

Prabhu,

I am fine with this patch.

I would wait till next Monday(10th Jan) to get community's opinion on 
this before committing this.

With regards
Kamesh Jayachandran


On 12/30/2010 08:37 PM, Prabhu Gnana Sundar wrote:
> Hi all,
>
> This patch is a follow up to the patch that I gave last month
> (November).
> Here is the link to the mail archive...
> http://mail-archives.apache.org/mod_mbox/subversion-dev/201011.mbox/%
> 3C1291110994.4021.66.camel@prabhugnanasundar%3E
>
>
> After a few discussions about the inconsistent behaviour of 'svn diff'
> in different scenarios, Kamesh suggested that let 'svn diff' be done
> against the copy-source by default, making use of the copyfrom info.
>
> Now this patch would do 'diff' against the copy-source by default,
> whereas the prevailing default behaviour(of showing a *modified copy* as
> all adds) can be achieved by using the '--show-copies-as-adds'
> switch(that already exists in the code-base!).
>
> I was quickly in to it and after spending some time in the process I
> came to know that several existing 'test cases' *failed* due to the
> change in the behaviour of the 'svn diff'. Later found that the 'wc
> editor' needs to be taught to handle this behaviour (handling the
> copyfrom info and retrieving it from the repository as against working
> copy text-base).
>
> This took quite some amount of time...
>
> See [1] below of the mailer.py usecase which tries to get the same
> output as my patch do.
>
> May be that's the behaviour of the 'diff' that is prefered ? :)
>
> I have attached the patch and the log message with this mail. Please
> review and respond.
>
>
> [1]
> I found the following usecase which is already doing the same as what my
> patch intends to do.
>
> The mailer.py script has the same behaviour of 'diff'ing just as this
> patch does.
> http://mail-archives.apache.org/mod_mbox/subversion-commits/201012.mbox/%3C20101228204459.1E42D238890A@eris.apache.org%3E
>
> <snip from the above commit email>
> Copied:
> subversion/branches/1.6.x-svn_fs_commit_txn/subversion/include/private/svn_repos_private.h
> (from r1051763,
> subversion/trunk/subversion/include/private/svn_repos_private.h)
> URL:
> http://svn.apache.org/viewvc/subversion/branches/1.6.x-svn_fs_commit_txn/subversion/include/private/svn_repos_private.h?p2=subversion/branches/1.6.x-svn_fs_commit_txn/subversion/include/private/svn_repos_private.h&p1=subversion/trunk/subversion/include/private/svn_repos_private.h&r1=1051763&r2=1053428&rev=1053428&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/include/private/svn_repos_private.h
> (original)
> +++
> subversion/branches/1.6.x-svn_fs_commit_txn/subversion/include/private/svn_repos_private.h
> Tue Dec 28 20:44:58 2010
> @@ -38,53 +38,6 @@ extern "C" {
>
>
> /**
> - * Permanently delete @a path at revision @a revision in @a fs.
> - *
> - * Do not change the content of any other node in the repository, even
> other
> - * nodes that were copied from this one. The only other change in the
> - * repository is to "copied from" pointers that were pointing to the
> - * now-deleted node. These are removed or made to point to a previous
> - * version of the now-deleted node.
> - * (### TODO: details.)
> - *
> - * @a path is relative to the repository root and must start with "/".
> - *
> - * If administratively forbidden, return @c SVN_ERR_RA_NOT_AUTHORIZED.
> If not
> - * implemented by the RA layer or by the server, return
> - * @c SVN_ERR_RA_NOT_IMPLEMENTED.
> - *
> - * @note This functionality is not implemented in pre-1.7 servers and
> may not
> - * be implemented in all 1.7 and later servers.
> - *
> - * @note TODO: Maybe create svn_repos_fs_begin_obliteration_txn() and
> - * svn_repos_fs_commit_obliteration_txn() to enable an obliteration txn
> to be
> - * constructed at a higher level.
> - *
> - * @since New in 1.7.
> - */
> -svn_error_t *
> -svn_repos__obliterate_path_rev(svn_repos_t *repos,
> - const char *username,
> - svn_revnum_t revision,
> - const char *path,
> - apr_pool_t *pool);
> -
> -
> -/** Validate that property @a name is valid for use in a Subversion
> - * repository; return @c SVN_ERR_REPOS_BAD_ARGS if it isn't. For some
> - * "svn:" properties, also validate the @a value, and return
> - * @c SVN_ERR_BAD_PROPERTY_VALUE if it is not valid.
> - *
> - * Use @a pool for temporary allocations.
> - *
> - * @since New in 1.7.
> - */
> -svn_error_t *
> -svn_repos__validate_prop(const char *name,
> - const svn_string_t *value,
> - apr_pool_t *pool);
> -
> -/**
> * Given the error @a err from svn_repos_fs_commit_txn(), return an
> * string containing either or both of the svn_fs_commit_txn() error
> * and the SVN_ERR_REPOS_POST_COMMIT_HOOK_FAILED wrapped error from
> </snip>
>
>
>
> Thanks and regards
> Prabhu
>