[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] Implement '--include-externals' option to 'svn list'

From: vijay <vijay_at_collab.net>
Date: Thu, 08 Nov 2012 17:04:35 +0530

On Wednesday 07 November 2012 08:57 PM, Stefan Fuhrmann wrote:
> On Wed, Nov 7, 2012 at 3:02 PM, vijay <vijay_at_collab.net
> <mailto:vijay_at_collab.net>> wrote:
>
> On Tuesday 06 November 2012 08:09 PM, Stefan Fuhrmann wrote:
>
> On Mon, Nov 5, 2012 at 5:24 PM, vijay <vijay_at_collab.net
> <mailto:vijay_at_collab.net>
> <mailto:vijay_at_collab.net <mailto:vijay_at_collab.net>>> wrote:
>
> Hi,
>
> This patch implements '--include-externals' option to 'svn
> list'
> [Issue #4225] [1].
>
> All tests pass with 'make check' & 'make davautocheck'.
>
> Attached the patch and log message.
>
> Please review this patch and share your thoughts.
>
> Thanks in advance for your time.
>
> Thanks & Regards,
> Vijayaguru
>
> [1]
> http://subversion.tigris.org/____issues/show_bug.cgi?id=4225
> <http://subversion.tigris.org/__issues/show_bug.cgi?id=4225>
>
> <http://subversion.tigris.org/__issues/show_bug.cgi?id=4225
> <http://subversion.tigris.org/issues/show_bug.cgi?id=4225>>
>
>
> Hi Vijay,
>
> Not sure whether these points have already been
> discussed in previous threads, but the following
> two points caught my attention:
>
> +typedef svn_error_t *(*svn_client_list_func2_t)(
> + void *baton,
> + const char *path,
> + const svn_dirent_t *dirent,
> + const svn_lock_t *lock,
> + const char *abs_path,
>
> o.k.
>
> + svn_boolean_t notify_external_start,
> + svn_boolean_t notify_external_end,
> + const char *external_parent_url,
> + const char *external_target,
>
> Maybe, this should be modeled to create a more "seamless"
> appearance. Only keep the external_parent_url member.
> If it is NULL, this entry has not been pulled in here by
> some external. Otherwise it contains the parent URL as
> defined by your patch.
>
> I don't see the see the need to expose more information.
> Why would you need to group externals? The external_target
> member should be given implicitly by path / dirent.
> Am I missing something here?
>
>
>
> The external_target member will not be given by path / dirent.
> We will get it by parsing the externals description.
>
> Suppose that path1 in repo1 has svn:externals set to path2 in repo2.
>
> When we list path1 with externals included,
>
> 1. First, list path1.
> 2. Then, process all the externals defined under path1. Parse
> through the individual external description and populate
> external_target.
>
> For example,
>
> external description under path1:
> external_desc = exdir http://<url-of-path2-in-repo2>
>
> external_target = exdir
> external_parent_url = http://<url-of-path1-in-repo1>
>
> url of external = http://<url-of-path2-in-repo2>
>
> 3. List the entries by reaching 'url of external'.
>
>
> OK, I now see what you are trying to do here -
> I had read to much into the code.
>
> However, in this form, the added functionality is
> of limited use, IMHO. I understand that what you
> list is basically which paths you would get for a
> "svn co $url".
>
> While this is fine, I see two issues with it:
>
> * trees don't get overlayed. Example:
> $>svn ls $URL -R
> sub1
> sub1/fileA
> sub1/fileB
> sub2
> $>svn propget "svn:externals" $URL
> http://somewhere/ sub1/subsub
> $>svn ls $URL -R --include-externals
> sub1
> sub1/fileA
> sub1/fileB
> sub2
> sub1/subsub [external].
>
> Desired output
> sub1
> sub1/fileA
> sub1/fileB
> sub1/subsub [external @$URL].
> sub2

I was referring externals related code base in checkout/update and
export while implementing this option.

 From subversion/libsvn_client/update.c
<snip>
/* We handle externals after the update is complete, so that
      handling external items (and any errors therefrom) doesn't delay
      the primary operation. */
</snip>

So I preferred the same for 'list' also. But there is a difference in
externals processing between the commands checkout/update and list. The
former processes the externals by default. The latter processes
externals only if we specifically ask it. What should we do here?

>
> * result of ls on a sub-folder results in less output
> $>svn ls $URL/sub1 -R --include-externals
> fileA
> fileB
>
> Desired output:
> fileA
> fileB
> subsub [external @$URL].

Bert raised the same question and C-Mike answered here [1].

The current implementation uses svn_ra_get_dir() to fetch all properties
and filters "svn:externals" under current list target. I don't know how
to extend it for externals defined higher up in the tree.

Thanks & Regards,
Vijayaguru

[1] http://svn.haxx.se/dev/archive-2012-10/0353.shtml

>
> So, it is hard to build e.g. explorer-like GUIs based
> on this information. The GUI would still need to collect,
> remember and "splice" the externals manually. That
> would render your extension almost useless.
>
> -- Stefan^2.
>
> --
> Certified & Supported Apache Subversion Downloads:
> /
>
> http://www.wandisco.com/subversion/download
>
> /
Received on 2012-11-08 12:35:12 CET

This is an archived mail posted to the Subversion Dev mailing list.