What's cooking in git.git (Aug 2009, #05; Wed, 26)

Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' while commits prefixed with '+' are in 'next'. The ones marked with '.' do not appear in any of the integration branches, but I am still holding onto them.

After the 1.6.5 cycle, the next release will be 1.7.0, and we will push out the planned "push safety" change. 1.7.0 would be a good time to introduce "justifiable" changes that are not strictly backward compatible.

During 1.6.5 cycle, 'next' will hold topics meant for 1.6.5 and 1.7.0.

-------------------------------------------------- [Graduated to "master"]

* aj/fix-read-tree-from-scratch (2009-08-17) 1 commit
  (merged to 'next' on 2009-08-20 at 7a04133)
 + read-tree: Fix regression with creation of a new index file.

* jc/maint-checkout-index-to-prefix (2009-08-16) 1 commit
  (merged to 'next' on 2009-08-20 at 2f6aea2)
 + check_path(): allow symlinked directories to checkout-index --prefix

* jl/submodule-summary-diff-files (2009-08-15) 2 commits
  (merged to 'next' on 2009-08-15 at 165bd8e)
 + Documentaqtion/git-submodule.txt: Typofix
  (merged to 'next' on 2009-08-14 at a702e78)
 + git submodule summary: add --files option

* lh/short-decorate (2009-08-15) 1 commit
  (merged to 'next' on 2009-08-18 at b8c1d96)
 + git-log: allow --decorate[=short|full]

* oa/stash-na (2009-08-11) 1 commit
  (merged to 'next' on 2009-08-14 at 12c2e2b)
 + git stash: Give friendlier errors when there is nothing to apply
--------------------------------------------------
[New Topics]

* np/maint-1.6.3-deepen (2009-08-24) 1 commit.
  (merged to 'next' on 2009-08-25 at 8e383d4)
 + fix simple deepening of a repo

* jk/maint-1.6.3-checkout-unborn (2009-08-24) 1 commit.
  (merged to 'next' on 2009-08-25 at 5f29625)
 + checkout: do not imply "-f" on unborn branches

* mr/gitweb-snapshot (2009-08-25) 3 commits
 - gitweb: add t9501 tests for checking HTTP status codes
 - gitweb: split test suite into library and tests
 - gitweb: improve snapshot error handling
--------------------------------------------------
[Stalled]

* js/stash-dwim (2009-07-27) 1 commit.
  (merged to 'next' on 2009-08-16 at 67896c4)
 + Make 'git stash -k' a short form for 'git stash save --keep-index'
 (this branch is used by tr/reset-checkout-patch.)

* tr/reset-checkout-patch (2009-08-18) 8 commits.
  (merged to 'next' on 2009-08-18 at e465bb3)
 + tests: disable interactive hunk selection tests if perl is not available
  (merged to 'next' on 2009-08-16 at 67896c4)
 + DWIM 'git stash save -p' for 'git stash -p'
 + Implement 'git stash save --patch'
 + Implement 'git checkout --patch'
 + Implement 'git reset --patch'
 + builtin-add: refactor the meat of interactive_add()
 + Add a small patch-mode testing library
 + git-apply--interactive: Refactor patch mode code
 (this branch uses js/stash-dwim.)

There was a discussion on better DWIMmery for the above two topics to (1) forbid "git stash save --anything-with-dash" and (2) redirect with any option "git stash --opt" to "git stash save --opt", to keep it flexible and safe at the same time. I think it is a sane thing to do, but nothing has happened lately.

* jn/gitweb-blame (2009-08-06) 3 commits
 - gitweb: Create links leading to 'blame_incremental' using JavaScript
 - gitweb: Incremental blame (WIP)
 - gitweb: Add optional "time to generate page" info in footer

Ajax-y blame WIP

* db/vcs-helper (2009-08-09) 17 commits
 - Allow helpers to request marks for fast-import
 - Allow helpers to report in "list" command that the ref is unchanged
 - Add support for "import" helper command
 - transport-helper_init(): fix a memory leak in error path
 - Add a config option for remotes to specify a foreign vcs
 - Allow programs to not depend on remotes having urls
 - Allow fetch to modify refs
 - Use a function to determine whether a remote is valid
 - Use a clearer style to issue commands to remote helpers
  (merged to 'next' on 2009-08-07 at f3533ba)
 + Makefile: install hardlinks for git-remote-<scheme> supported by libcurl if possible
 + Makefile: do not link three copies of git-remote-* programs
 + Makefile: git-http-fetch does not need expat
  (merged to 'next' on 2009-08-06 at 15da79d)
 + http-fetch: Fix Makefile dependancies
 + Add transport native helper executables to .gitignore
  (merged to 'next' on 2009-08-05 at 33d491e)
 + git-http-fetch: not a builtin
 + Use an external program to implement fetching with curl
 + Add support for external programs for handling native fetches
 (this branch is used by jh/cvs-helper.)

There was a discussion that suggests that the use of colon ':' before vcs helper name needs to be corrected. Nothing happened since.

* je/send-email-no-subject (2009-08-05) 1 commit
 - send-email: confirm on empty mail subjects

This seems to break t9001. Near the tip of 'pu' I have a iffy workaround. -------------------------------------------------- [Cooking]

* mm/reset-report (2009-08-21) 2 commits
  (merged to 'next' on 2009-08-25 at f2a4424)
 + reset: make the reminder output consistent with "checkout"
 + Rename REFRESH_SAY_CHANGED to REFRESH_IN_PORCELAIN.

* wl/insta-mongoose (2009-08-21) 1 commit
  (merged to 'next' on 2009-08-25 at da1d566)
 + Add support for the Mongoose web server.

* lt/approxidate (2009-08-22) 2 commits
  (merged to 'next' on 2009-08-26 at 62853f9)
 + Further 'approxidate' improvements
 + Improve on 'approxidate'

Fixes a few "reasonably formatted but thus-far misparsed" date strings. As Nico suggested, we would need a test to prevent regression to existing support for date strings that are "reasonably formatted".

* jc/mailinfo-scissors (2009-08-25) 2 commits
 - Documentation: describe the scissors mark support of "git am"
 - Teach mailinfo to ignore everything before -- >8 -- mark

* tf/diff-whitespace-incomplete-line (2009-08-23) 2 commits.
  (merged to 'next' on 2009-08-26 at 4fc7784)
 + xutils: Fix xdl_recmatch() on incomplete lines
 + xutils: Fix hashing an incomplete line with whitespaces at the end

* cc/sequencer-rebase-i (2009-08-21) 17 commits.
 - rebase -i: use "git sequencer--helper --cherry-pick"
 - sequencer: add "--cherry-pick" option to "git sequencer--helper"
 - sequencer: add "do_commit()" and related functions
 - pick: libify "pick_help_msg()"
 - revert: libify pick
 - rebase -i: use "git sequencer--helper --fast-forward"
 - sequencer: let "git sequencer--helper" callers set "allow_dirty"
 - sequencer: add "--fast-forward" option to "git sequencer--helper"
 - sequencer: add "do_fast_forward()" to perform a fast forward
 - rebase -i: use "git sequencer--helper --reset-hard"
 - sequencer: add "--reset-hard" option to "git sequencer--helper"
 - sequencer: add comments about reset_almost_hard()
 - sequencer: add "reset_almost_hard()" and related functions
 - rebase -i: use "git sequencer--helper --make-patch"
 - sequencer: free memory used in "make_patch" function
 - sequencer: add "make_patch" function to save a patch
 - sequencer: add "builtin-sequencer--helper.c"

Migrating "rebase -i" bit by bit to C. I am inclined to agree with Dscho that maybe this approach forces the migration to follow the structure of the shell script too much, and could force a suboptimal end result, but we'll see.

* as/maint-graph-interesting-fix (2009-08-21) 2 commits.
  (merged to 'next' on 2009-08-25 at 9d5e215)
 + Add tests for rev-list --graph with options that simplify history
 + graph API: fix bug in graph_is_interesting()

Looked sane.

* jc/shortstatus (2009-08-15) 11 commits
  (merged to 'next' on 2009-08-15 at 7e40766)
 + git commit --dry-run -v: show diff in color when asked
 + Documentation/git-commit.txt: describe --dry-run
  (merged to 'next' on 2009-08-12 at 53bda17)
 + wt-status: collect untracked files in a separate "collect" phase
 + Make git_status_config() file scope static to builtin-commit.c
 + wt-status: move wt_status_colors[] into wt_status structure
 + wt-status: move many global settings to wt_status structure
 + commit: --dry-run
  (merged to 'next' on 2009-08-06 at fe8cb94)
 + status: show worktree status of conflicted paths separately
 + wt-status.c: rework the way changes to the index and work tree are summarized
 + diff-index: keep the original index intact
 + diff-index: report unmerged new entries
 (this branch is used by jc/1.7.0-status.)

Will cook for a bit more and then merge.

* jc/maint-unpack-objects-strict (2009-08-13) 1 commit.
  (merged to 'next' on 2009-08-23 at 38eb750)
 + Fix "unpack-objects --strict"

Will merge.

* jh/submodule-foreach (2009-08-20) 9 commits
  (merged to 'next' on 2009-08-20 at 671bea4)
 + git clone: Add --recursive to automatically checkout (nested) submodules
 + t7407: Use 'rev-parse --short' rather than bash's substring expansion notation
  (merged to 'next' on 2009-08-18 at f4a881d)
 + git submodule status: Add --recursive to recurse into nested submodules
 + git submodule update: Introduce --recursive to update nested submodules
 + git submodule foreach: Add --recursive to recurse into nested submodules
 + git submodule foreach: test access to submodule name as '$name'
 + Add selftest for 'git submodule foreach'
 + git submodule: Cleanup usage string and add option parsing to cmd_foreach()
 + git submodule foreach: Provide access to submodule name, as '$name'

Will merge.

* jh/notes (2009-07-29) 8 commits.
 - t3302-notes-index-expensive: Speed up create_repo()
 - fast-import: Add support for importing commit notes
 - First draft of notes tree parser with support for fanout subtrees
 - Teach "-m <msg>" and "-F <file>" to "git notes edit"
 - Add an expensive test for git-notes
 - Speed up git notes lookup
 - Add a script to edit/inspect notes
 - Introduce commit notes

The cvs-helper series depends on this one. I have a recollection that some people were not happy about the fan-out of the notes tree layout, but has the issue been resolved to a concensus?

* ne/rev-cache (2009-08-21) 6 commits
 . support for path name caching in rev-cache
 . full integration of rev-cache into git, completed test suite
 . administrative functions for rev-cache, start of integration into git
 . support for non-commit object caching in rev-cache
 . basic revision cache system, no integration or features
 . man page and technical discussion for rev-cache

Updated but seems to break upload-pack tests when merged to 'pu'; given what this series touches, breakages in that area are expected. May discard if a working reroll comes, to give it a fresh start.

* jh/cvs-helper (2009-08-18) 7 commits
 - More fixes to the git-remote-cvs installation procedure
 - Fix the Makefile-generated path to the git_remote_cvs package in git-remote-cvs
 - Add simple selftests of git-remote-cvs functionality
 - git-remote-cvs: Remote helper program for CVS repositories
 - 2/2: Add Python support library for CVS remote helper
 - 1/2: Add Python support library for CVS remote helper
 - Basic build infrastructure for Python scripts
 (this branch uses db/vcs-helper.)

Builds on db/vcs-helper (which is stalled, so this cannot move).

The testing of Python part seemed to be still fragile even with the latest fix on one of my boches with an earlier round already installed, but I didn't look very deeply before removing the older installation.

* sr/gfi-options (2009-08-24) 4 commits
 - fast-import: test the new option command
 - fast-import: add option command
 - fast-import: put marks reading in it's own function
 - fast-import: put option parsing code in seperate functions

Will merge to 'next' shortly.

* lt/block-sha1 (2009-08-17) 4 commits
  (merged to 'next' on 2009-08-18 at 67a1ce8)
 + remove ARM and Mozilla SHA1 implementations
 + block-sha1: guard gcc extensions with __GNUC__
 + make sure byte swapping is optimal for git
 + block-sha1: make the size member first in the context struct

May merge soon; Solaris performance patches that was discussed earlier can happen on 'master', as the series is usable as-is.

* nd/sparse (2009-08-20) 20 commits
 - sparse checkout: inhibit empty worktree
 - Add tests for sparse checkout
 - read-tree: add --no-sparse-checkout to disable sparse checkout support
 - unpack-trees(): ignore worktree check outside checkout area
 - unpack_trees(): apply $GIT_DIR/info/sparse-checkout to the final index
 - unpack-trees(): "enable" sparse checkout and load $GIT_DIR/info/sparse-checkout
 - unpack-trees.c: generalize verify_* functions
 - unpack-trees(): add CE_WT_REMOVE to remove on worktree alone
 - Introduce "sparse checkout"
 - dir.c: export excluded_1() and add_excludes_from_file_1()
 - excluded_1(): support exclude files in index
 - unpack-trees(): carry skip-worktree bit over in merged_entry()
 - Read .gitignore from index if it is skip-worktree
 - Avoid writing to buffer in add_excludes_from_file_1()
 - Teach Git to respect skip-worktree bit (writing part)
 - Teach Git to respect skip-worktree bit (reading part)
 - Introduce "skip-worktree" bit in index, teach Git to get/set this bit
 - Add test-index-version
 - update-index: refactor mark_valid() in preparation for new options
  (merged to 'next' on 2009-08-20 at ea167d7)
 + Prevent diff machinery from examining assume-unchanged entries on worktree

The first one was an independent fix; the rest has been replaced with the "return of no-checkout" series. -------------------------------------------------- [For 1.7.0]

* jc/1.7.0-status (2009-08-15) 3 commits
  (merged to 'next' on 2009-08-22 at b3507bb)
 + git status: not "commit --dry-run" anymore
 + git stat -s: short status output
 + git stat: the beginning of "status that is not a dry-run of commit"
 (this branch uses jc/shortstatus.)

With this, "git status" is no longer "git commit --preview".

* jc/1.7.0-send-email-no-thread-default (2009-08-22) 1 commit
  (merged to 'next' on 2009-08-22 at 5106de8)
 + send-email: make --no-chain-reply-to the default

* jc/1.7.0-diff-whitespace-only-status (2009-05-23) 2 commits.
  (merged to 'next' on 2009-08-02 at 9c08420)
 + diff: Rename QUIET internal option to QUICK
 + diff: change semantics of "ignore whitespace" options

This changes exit code from "git diff --ignore-whitespace" and friends when there is no actual output. It is a backward incompatible change, but we could argue that it is a bugfix.

* jc/1.7.0-push-safety (2009-02-09) 2 commits
  (merged to 'next' on 2009-08-02 at 38b82fe)
 + Refuse deleting the current branch via push
 + Refuse updating the current branch in a non-bare repository via push
--------------------------------------------------
[I have been too busy to purge these]

* jc/log-tz (2009-03-03) 1 commit.
 - Allow --date=local --date=other-format to work as expected

Maybe some people care about this.  I dunno.

* jc/mailinfo-remove-brackets (2009-07-15) 1 commit.
 - mailinfo: -b option keeps [bracketed] strings that is not a [PATCH] marker

Maybe some people care about this.  I dunno.

* ar/maint-1.6.2-merge-recursive-d-f (2009-05-11) 2 commits.
 . Fix for a merge where a branch has an F->D transition
 . Add a reminder test case for a merge with F/D transition

* jc/merge-convert (2009-01-26) 1 commit.
 . git-merge-file: allow converting the results for the work tree

* lt/read-directory (2009-05-15) 3 commits.
 . Add initial support for pathname conversion to UTF-8
 . read_directory(): infrastructure for pathname character set conversion
 . Add 'fill_directory()' helper function for directory traversal

* ps/blame (2009-03-12) 1 commit.
 . blame.c: start libifying the blame infrastructure

* pb/tracking (2009-07-16) 7 commits.
 . branch.c: if remote is not config'd for branch, don't try delete push config
 . branch, checkout: introduce autosetuppush
 . move deletion of merge configuration to branch.c
 . remote: add per-remote autosetupmerge and autosetuprebase configuration
 . introduce a struct tracking_config
 . branch: install_branch_config and struct tracking refactoring
 . config: allow false and true values for branch.autosetuprebase

Has been ejected from 'pu' for some time, expecting a reroll.

Junio C Hamano <gitster@pobox.com> wrote:

Please don't.

There is an off-git ML thread running between the various DVCS tool developers who work on the fast-import/fast-export tools for the respective DVCSes. In that thread we have decided to slightly change this grammar and this series needs to be respun.

If you are itching to do something, eject it from pu and wait for
the respin.
 

Heya,

On Wed, Aug 26, 2009 at 16:49, Shawn O. Pearce<spearce@spearce.org> wrote:

Speaking of which, do you want me to add the feature command to the grammar and rebase this on top of that? I got the impression we were far enough discussing that for at least an RPC patch?

Sverre Rabbelier <srabbelier@gmail.com> wrote:

RFC patch. And yes, because I think we already agreed in that thread that the date-format option is actually a feature command, and not an option command. The other feature commands being kicked around can be held for another series.

Heya,

On Wed, Aug 26, 2009 at 17:12, Shawn O. Pearce<spearce@spearce.org> wrote:

Done, see v5a of the series (I messed up v5).

Junio C Hamano wrote:

Can you squash this into your 'iffy' workaround to help platforms (Solaris 7, IRIX 6.5) without the 'yes' utility?

---
 t/t9001-send-email.sh |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

Brandon Casey <brandon.casey.ctr@nrlssc.navy.mil> writes:

Not in this form, for two reasons ;-)

(1) t7610-mergetool.sh,also seems to use "yes".  Perhaps define something
    in test-lib.sh?

(2) The implementation is iffy.

Shouldn't it be

	if test $# = 0
        then
        	y=y
	else
        	y="$*"
	fi

so that

	yes ""

would give runs of empty lines?        

Junio C Hamano wrote:

Looks good, I'll rework it sometime if you don't beat me to it.

-brandon

From: Brandon Casey <drafnel@gmail.com>

Some platforms (IRIX 6.5, Solaris 7) do not provide the 'yes' utility. Currently, some tests, including t7610 and t9001, try to call this program. Due to the way the tests are structured, the tests still pass even though this program is missing. Rather than succeeding by chance, let's provide an implementation of the simple 'yes' utility in shell for all platforms to use.

Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
---
 t/test-lib.sh |   15 +++++++++++++++
 1 files changed, 15 insertions(+), 0 deletions(-)

On Wed, 26 Aug 2009, Junio C Hamano wrote:

I believe the outcome of that discussion was:

 - We want to keep supporting using regular location URLs that are URLs of 
   git repositories (e.g., http://git.savannah.gnu.org/cgit/xboard.git), 
   and we probably want to do it with a helper which runs when 
   run_command() is given "remote-<scheme>". I think installing hardlinks 
   in EXECPATH ended up being the best implementation here. This is 
   currently a special case, because these URLs have push handled 
   internally (or, rather, with internal code calling a different external 
   program), but I think we want to make it no longer special at all, so 
   that people can install the handling for access to native repos via 
   dumb filesystem-like protocols separately. This is in next.

 - We want to support a separate "vcs" option for cases where repositories 
   in the foreign system need to be addressed through the combination of a 
   bunch of options, which will be read from the configuration by the 
   helper. The helper which gets run is "remote-<value of vcs option>". 
   This is in pu.

 - We want to support URLs of some sort leading to using helpers 
   appropriate for foreign systems that use URLs and are most conveniently 
   located this way. We didn't come to any concensus on what this should 
   do, but we also haven't had any helpers proposed yet that would use it, 
   and the series doesn't include any code that would lead to running a 
   helper other than in one of the above two cases. (jh's cvs helper 
   clearly wants "cvsroot" and "cvsmodule" options, and my p4 helper wants 
   "port" and "codeline" options; SVN is the big use cases for URLs, but 
   nobody's tackled that as a helper)

So I think this issue is squarely in "future work" anyway, and the current part of the series should be able to move forward, unless I've missed some other issue.

Of course, there are a bunch of things that are beyond the present contents of the series, but I think there's nothing wrong in the series as it is, and it's worthwhile without any further patches.

	-Daniel
*This .sig left intentionally blank*

Daniel Barkalow <barkalow@iabervon.org> writes:

That is different from what I recall.

I think you said <scheme> in the above to mean that in the general URL syntax, <scheme> refers to the token before the colon, and you would be feeding the rest (i.e. after the colon, and for many <scheme>'s it typically begins with //) to the scheme.

A flaw with this that was pointed out was that this conflicts with the scp-like syntax. A remote.$name.url of foo:bar/baz could name $HOME/bar/baz on host foo (perhaps a nickname in .ssh/config), or the source "foo" helper recognizes with the name bar/baz.

If I recall correctly, suggestions made later in the discussion were to use either <helper>+ or <helper>:: as the prefix to avoid this issue, and use it to choose remote-<helper> (and I think I probably would vote for double-colon, if only to avoid confusion with our own earlier misdesigned syntax git+ssh://), so the canonical syntax would be:

	<helper>::<whatever is fed to the helper, typicall a URL>

while we would support obvious short-hands for transports we traditionally supported without explicit "<helper>::" prefix when we choose to eject it from "built-in" set of transports.

E.g. http://git.savannah.gnu.org/cgit/xboard.git would be handled by curl based walker, so if you spell it in the very canonical form, the url would be curl::http://git.savannah.gnu.org/cgit/xboard.git, but nobody has to use it. Instead, the transport dispatcher internally recognizes http:// and picks the curl based walker helper, which is remote-curl without any extra hardlinks.

And from my point of view, this is what is blocking the series; and there still is no -rc0 yet (I've been hoping to do a 1.6.5 mid September before I leave for Japan for about a week), because I think it is pointless to do a new release without "the ejection of curl from builtin".

After you explained this in the thread (I think for the second time), I see no problem with this, except that I think to support this we should notice and raise an error when we see a remote has both vcs and url, because the only reason we would want to say "vcs", if I recall your explanation correctly, is that such a transport does not have the concept of URL, i.e. a well defined single string that identifies the repository.

Junio C Hamano <gitster@pobox.com> writes:

Just to make my position clear.

I brought up the issue of "should we leave it as a single colon that confuses scp-like syntax and forces us to have three extra hardlinks?" in the message I am following up to, not because I firmly am on that side of the argument, but because I thought your version of "outcome" did not match my recollection around that particular issue. I am more or less neutral myself. Avoiding the confusion seems obviously the simple and right thing to do, and I cannot think of obvious downsides of such a change, but there probably are some downsides you have in mind, and I can well imagine the issue may become "perfect is the enemy of good".

And I said "blocking the series", not in the sense that _I_ demand to change it from colon to something else, but in the sense that I recall the issue hasn't been settled in the discussion.

On Sat, 29 Aug 2009, Junio C Hamano wrote:

(with the syntax <helper>+, "git+ssh://" would specify the helper "git", which is as good an explicit identifier of the internal handling as any)

If the policy is that we're going to have "traditionally supported" schemes, where the internal code knows what helper supports them, I can fix up the series so that the curl-based helper is named "curl", and we can say that the check for "http://", "ftp://", and "https://" is recognizing traditionally-supported schemes, and we can defer coming up with what the syntax for the explicit handler selection is. (For that matter, if there's a // after the colon, it's obviously not a handy ssh-style location, since the second slash would do nothing)

I think you're right that we decided that things we used to support internally are a special case, and there's no need to try to generalize them to be the general mechanism (even though I think we simultaneously worked out how to implement the design we were abandoning, which confused my memory).

A user who mostly uses Perforce as a foreign repositories but is using a SVN repo on occasion might want to use "vcs" regardless, but I agree with forcing the helper to use a different option for the case of a URL that git isn't going to look at. That is, you ought to be able to use:

[remote "origin"]
	vcs = svn
	(something) = http://svn.savannah.gnu.org/...

But "(something)" shouldn't be "url".

So my changes will be:

 - name the curl-based helper executable "git-remote-curl", and run it for 
   traditionally supported schemes by special-case.
 - prohibit using both "vcs" and "url" in a remote.

Agreed?

	-Daniel
*This .sig left intentionally blank*

Daniel Barkalow <barkalow@iabervon.org> writes:

Sure but how would you explain "ssh+git://" then ;-)?

Luckily neither is advertised in our documentation set as far as I can tell, so I do not think it is a huge deal between + vs ::, but as Peff says in

    http://thread.gmane.org/gmane.comp.version-control.git/125615

I think the latter is probably less problematic.

With plus, a helper that talks with a Subversion repository whose native URL is http://host/path would look like svn+http://host/path, which is reasonable. When talking the Subversion protocol over SSH, however, the native URL for the repository would be svn+ssh://host/path, so the URL with helper name on our side becomes svn+svn+ssh://host/path. We could recognize "svn+" part and implicitly pass the whole thing to svn helper upon seeing svn+ssh://host/path, but I do not think we would want to make the dispatcher too familiar with what the backends do. Using something other than plus sign would avoid this issue.

That sounds like a sane approach to first get the "eject curl from builtin" out the door. We might extend the dispatcher in the future by changing "traditionally supported" criterion to "commonly used", e.g. recognize "svn+ssh://" as something the svn helper would want to handle, but that is a future extension we do not have to address right now.

I actuallly do not have a strong opinion on this one either way. I said "I think" when I suggested it, but it was actually without thinking too deeply, hoping that you would come up with a good counter-argument.

For example, if we envision that for most of the helpers there will be one primary string that identifies the repository, but the primary string alone is not enough for the helper without some auxiliary information, it would be natural to use remote.$name.url for that primary string. I do not know if that would be the case, but I was hoping that you would have a better intuition[*1*], as you have thought this topic through a lot longer and deeper than I have. So I'd rather leave the decision on that "no vcs/url at the same time restriction" up to you. It is in general easier to start more strict and then loosen the restiction later, than the other way around, when we cannot decide, though.

Thanks.

[Footnote]

*1* What I mean by intuition is that you do not have to have the right answer backed by research _now_, but have a good guess as to what the right answer would be.

On Mon, 31 Aug 2009, Junio C Hamano wrote:

That's just weird, and looks wrong to me, like ssh tunnelled over the git native protocol. I think, if we support it, it would have to be another special case.

I was thinking that the dispatcher would always pass the whole thing, and the svn helper would have to deal with svn+http:// and svn+svn:// as well as svn+ssh://. The dispatcher shouldn't be familiar with what the backends do, but the backends could stand to be familiar with what might be there just to get to them. On the other hand, I don't have a good intuition for what makes sense to users of such systems, since I've only extensively used cvs, git, and perforce.

Fair enough.

Agreed.

I don't really have a particularly good guess about the case of vcs and a URL, because I haven't used systems that are not git and use URLs.

	-Daniel
*This .sig left intentionally blank*