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

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

[Junio C Hamano]

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.

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

[Shawn O. Pearce]

Junio C Hamano <gitster@pobox.com> wrote:
> * 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.

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.
 
-- 
Shawn.

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

[Sverre Rabbelier]

Heya,

On Wed, Aug 26, 2009 at 16:49, Shawn O. Pearce<spearce@spearce.org> wrote:
> In that thread we have decided to slightly
> change this grammar and this series needs to be respun.

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?

-- 
Cheers,

Sverre Rabbelier

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

[Shawn O. Pearce]

Sverre Rabbelier <srabbelier@gmail.com> wrote:
> On Wed, Aug 26, 2009 at 16:49, Shawn O. Pearce<spearce@spearce.org> wrote:
> > In that thread we have decided to slightly
> > change this grammar and this series needs to be respun.
> 
> 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?

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.

-- 
Shawn.

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

[Brandon Casey]

Junio C Hamano wrote:

> [Stalled]
> 
> * 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.

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(-)

diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 960d7d8..641d0c3 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -505,6 +505,14 @@ test_expect_success '--suppress-cc=cc' '
 	test_suppression cc
 '
 
+yes () {
+	test -n "$*" && y="$*" || y='y'
+	while echo "$y"
+	do
+		:
+	done
+}
+
 test_confirm () {
 	yes | \
 		GIT_SEND_EMAIL_NOTTY=1 \
-- 
1.6.4

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

[Junio C Hamano]

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

>> This seems to break t9001.  Near the tip of 'pu' I have a iffy
>> workaround.
>
> Can you squash this into your 'iffy' workaround to help platforms
> (Solaris 7, IRIX 6.5) without the 'yes' utility?

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.

> +yes () {
> +	test -n "$*" && y="$*" || y='y'

Shouldn't it be

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

so that

	yes ""

would give runs of empty lines?        

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

[Brandon Casey]

Junio C Hamano wrote:
> Brandon Casey <brandon.casey.ctr@nrlssc.navy.mil> writes:
> 
>>> This seems to break t9001.  Near the tip of 'pu' I have a iffy
>>> workaround.
>> Can you squash this into your 'iffy' workaround to help platforms
>> (Solaris 7, IRIX 6.5) without the 'yes' utility?
> 
> 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.

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

-brandon



>> +yes () {
>> +	test -n "$*" && y="$*" || y='y'
> 
> Shouldn't it be
> 
> 	if test $# = 0
>         then
>         	y=y
> 	else
>         	y="$*"
> 	fi
> 
> so that
> 
> 	yes ""
> 
> would give runs of empty lines?        

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

[Sverre Rabbelier]

Heya,

On Wed, Aug 26, 2009 at 17:12, Shawn O. Pearce<spearce@spearce.org> 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.

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

-- 
Cheers,

Sverre Rabbelier

[PATCH master] t/test-lib.sh: provide a shell implementation of the 'yes' utility

[Brandon Casey]

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(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index a5b8d03..f2ca536 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -685,6 +685,21 @@ do
 	esac
 done
 
+# Provide an implementation of the 'yes' utility
+yes () {
+	if test $# = 0
+	then
+		y=y
+	else
+		y="$*"
+	fi
+
+	while echo "$y"
+	do
+		:
+	done
+}
+
 # Fix some commands on Windows
 case $(uname -s) in
 *MINGW*)
-- 
1.6.4

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

[Daniel Barkalow]

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

> * 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.

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*

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

[Junio C Hamano]

Daniel Barkalow <barkalow@iabervon.org> writes:

>> There was a discussion that suggests that the use of colon ':' before vcs
>> helper name needs to be corrected.  Nothing happened since.
>
> 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.

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".

>  - 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.

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.

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

[Junio C Hamano]

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

> Daniel Barkalow <barkalow@iabervon.org> writes:
>
>>> There was a discussion that suggests that the use of colon ':' before vcs
>>> helper name needs to be corrected.  Nothing happened since.
>>
>> 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.
>
> That is different from what I recall.
> ...
> And from my point of view, this is what is blocking the series...

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.

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

[Daniel Barkalow]

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

> Daniel Barkalow <barkalow@iabervon.org> writes:
> 
> >> There was a discussion that suggests that the use of colon ':' before vcs
> >> helper name needs to be corrected.  Nothing happened since.
> >
> > 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.
> 
> 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:

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

> 
> 	<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.

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).

> >  - 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.
> 
> 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.

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*

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

[Junio C Hamano]

Daniel Barkalow <barkalow@iabervon.org> writes:

> On Sat, 29 Aug 2009, Junio C Hamano wrote:
>
>> ..., if only to avoid confusion with our own earlier misdesigned
>> syntax git+ssh://), so the canonical syntax would be:
>
> (with the syntax <helper>+, "git+ssh://" would specify the helper "git", 
> which is as good an explicit identifier of the internal handling as any)

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.

> 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)

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.

>> 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.
>
> 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".

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.

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

[Daniel Barkalow]

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

> Daniel Barkalow <barkalow@iabervon.org> writes:
> 
> > On Sat, 29 Aug 2009, Junio C Hamano wrote:
> >
> >> ..., if only to avoid confusion with our own earlier misdesigned
> >> syntax git+ssh://), so the canonical syntax would be:
> >
> > (with the syntax <helper>+, "git+ssh://" would specify the helper "git", 
> > which is as good an explicit identifier of the internal handling as any)
> 
> Sure but how would you explain "ssh+git://" then ;-)?

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.

> 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.

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.

> > 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)
> 
> 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.

Fair enough.

> >> 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.
> >
> > 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".
> 
> 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.

Agreed.

> 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.

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*