Git development
 help / color / mirror / Atom feed
* [PATCH 2/8] t1402: Ignore a few cases that must fail due to DOS path expansion
From: Pat Thoyts @ 2011-10-14 22:53 UTC (permalink / raw)
  To: Git; +Cc: Junio C Hamano, msysGit, Johannes Schindelin
In-Reply-To: <1318632815-29945-1-git-send-email-patthoyts@users.sourceforge.net>

From: Johannes Schindelin <johannes.schindelin@gmx.de>

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/t1402-check-ref-format.sh |   15 +++++++++------
 1 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/t/t1402-check-ref-format.sh b/t/t1402-check-ref-format.sh
index 710fcca..1a5e343 100755
--- a/t/t1402-check-ref-format.sh
+++ b/t/t1402-check-ref-format.sh
@@ -36,7 +36,7 @@ invalid_ref 'refs///heads/foo'
 valid_ref 'refs///heads/foo' --normalize
 invalid_ref 'heads/foo/'
 invalid_ref '/heads/foo'
-valid_ref '/heads/foo' --normalize
+test_have_prereq MINGW || valid_ref '/heads/foo' --normalize
 invalid_ref '///heads/foo'
 valid_ref '///heads/foo' --normalize
 invalid_ref './foo'
@@ -120,9 +120,12 @@ invalid_ref "$ref" --allow-onelevel
 invalid_ref "$ref" --refspec-pattern
 invalid_ref "$ref" '--refspec-pattern --allow-onelevel'
 invalid_ref "$ref" --normalize
-valid_ref "$ref" '--allow-onelevel --normalize'
-invalid_ref "$ref" '--refspec-pattern --normalize'
-valid_ref "$ref" '--refspec-pattern --allow-onelevel --normalize'
+if test_have_prereq NOT_MINGW
+then
+	valid_ref "$ref" '--allow-onelevel --normalize'
+	invalid_ref "$ref" '--refspec-pattern --normalize'
+	valid_ref "$ref" '--refspec-pattern --allow-onelevel --normalize'
+fi
 
 test_expect_success "check-ref-format --branch @{-1}" '
 	T=$(git write-tree) &&
@@ -166,10 +169,10 @@ invalid_ref_normalized() {
 
 valid_ref_normalized 'heads/foo' 'heads/foo'
 valid_ref_normalized 'refs///heads/foo' 'refs/heads/foo'
-valid_ref_normalized '/heads/foo' 'heads/foo'
+test_have_prereq MINGW || valid_ref_normalized '/heads/foo' 'heads/foo'
 valid_ref_normalized '///heads/foo' 'heads/foo'
 invalid_ref_normalized 'foo'
-invalid_ref_normalized '/foo'
+test_have_prereq MINGW || invalid_ref_normalized '/foo'
 invalid_ref_normalized 'heads/foo/../bar'
 invalid_ref_normalized 'heads/./foo'
 invalid_ref_normalized 'heads\foo'
-- 
1.7.7.1.gbba15

^ permalink raw reply related

* [PATCH 4/8] git-svn: On MSYS, escape and quote SVN_SSH also if set by the user
From: Pat Thoyts @ 2011-10-14 22:53 UTC (permalink / raw)
  To: Git; +Cc: Junio C Hamano, msysGit, Sebastian Schuberth
In-Reply-To: <1318632815-29945-1-git-send-email-patthoyts@users.sourceforge.net>

From: Sebastian Schuberth <sschuberth@gmail.com>

While GIT_SSH does not require any escaping / quoting (e.g. for paths
containing spaces), SVN_SSH requires it due to its use in a Perl script.

Previously, SVN_SSH has only been escaped and quoted automatically if it
was unset and thus derived from GIT_SSH. For user convenience, do the
escaping and quoting also for a SVN_SSH set by the user. This way, the
user is able to use the same unescaped and unquoted syntax for GIT_SSH
and SVN_SSH.

Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com>
---
 git-svn.perl |   15 +++++++--------
 1 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/git-svn.perl b/git-svn.perl
index a0410f0..3b33379 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -22,14 +22,13 @@ $Git::SVN::default_ref_id = $ENV{GIT_SVN_ID} || 'git-svn';
 $Git::SVN::Ra::_log_window_size = 100;
 $Git::SVN::_minimize_url = 'unset';
 
-if (! exists $ENV{SVN_SSH}) {
-	if (exists $ENV{GIT_SSH}) {
-		$ENV{SVN_SSH} = $ENV{GIT_SSH};
-		if ($^O eq 'msys') {
-			$ENV{SVN_SSH} =~ s/\\/\\\\/g;
-			$ENV{SVN_SSH} =~ s/(.*)/"$1"/;
-		}
-	}
+if (! exists $ENV{SVN_SSH} && exists $ENV{GIT_SSH}) {
+	$ENV{SVN_SSH} = $ENV{GIT_SSH};
+}
+
+if (exists $ENV{SVN_SSH} && $^O eq 'msys') {
+	$ENV{SVN_SSH} =~ s/\\/\\\\/g;
+	$ENV{SVN_SSH} =~ s/(.*)/"$1"/;
 }
 
 $Git::SVN::Log::TZ = $ENV{TZ};
-- 
1.7.7.1.gbba15

^ permalink raw reply related

* [PATCH 8/8] mingw: ensure sockets are initialized before calling gethostname
From: Pat Thoyts @ 2011-10-14 22:53 UTC (permalink / raw)
  To: Git; +Cc: Junio C Hamano, msysGit, Pat Thoyts
In-Reply-To: <1318632815-29945-1-git-send-email-patthoyts@users.sourceforge.net>

If the Windows sockets subsystem has not been initialized yet then an
attempt to get the hostname returns an error and prints a warning to the
console. This solves this issue for msysGit as seen with 'git fetch'.

Signed-off-by: Pat Thoyts <patthoyts@users.sourceforge.net>
---
 compat/mingw.c |    7 +++++++
 compat/mingw.h |    3 +++
 2 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/compat/mingw.c b/compat/mingw.c
index 8947418..efdc703 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -1321,6 +1321,13 @@ static void ensure_socket_initialization(void)
 	initialized = 1;
 }
 
+#undef gethostname
+int mingw_gethostname(char *name, int namelen)
+{
+    ensure_socket_initialization();
+    return gethostname(name, namelen);
+}
+
 #undef gethostbyname
 struct hostent *mingw_gethostbyname(const char *host)
 {
diff --git a/compat/mingw.h b/compat/mingw.h
index ce9dd98..fecf0d0 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -190,6 +190,9 @@ char *mingw_getcwd(char *pointer, int len);
 char *mingw_getenv(const char *name);
 #define getenv mingw_getenv
 
+int mingw_gethostname(char *host, int namelen);
+#define gethostname mingw_gethostname
+
 struct hostent *mingw_gethostbyname(const char *host);
 #define gethostbyname mingw_gethostbyname
 
-- 
1.7.7.1.gbba15

^ permalink raw reply related

* [PATCH 5/8] t9901: fix line-ending dependency on windows
From: Pat Thoyts @ 2011-10-14 22:53 UTC (permalink / raw)
  To: Git; +Cc: Junio C Hamano, msysGit, Pat Thoyts
In-Reply-To: <1318632815-29945-1-git-send-email-patthoyts@users.sourceforge.net>

Signed-off-by: Pat Thoyts <patthoyts@users.sourceforge.net>
---
 t/t9901-git-web--browse.sh |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/t/t9901-git-web--browse.sh b/t/t9901-git-web--browse.sh
index 7906e5d..1185b42 100755
--- a/t/t9901-git-web--browse.sh
+++ b/t/t9901-git-web--browse.sh
@@ -12,7 +12,7 @@ test_expect_success \
 	echo http://example.com/foo\&bar >expect &&
 	git config browser.custom.cmd echo &&
 	git web--browse --browser=custom \
-		http://example.com/foo\&bar >actual &&
+		http://example.com/foo\&bar | tr -d "\r" >actual &&
 	test_cmp expect actual
 '
 
@@ -21,7 +21,7 @@ test_expect_success \
 	echo http://example.com/foo\;bar >expect &&
 	git config browser.custom.cmd echo &&
 	git web--browse --browser=custom \
-		http://example.com/foo\;bar >actual &&
+		http://example.com/foo\;bar | tr -d "\r" >actual &&
 	test_cmp expect actual
 '
 
@@ -30,7 +30,7 @@ test_expect_success \
 	echo http://example.com/foo#bar >expect &&
 	git config browser.custom.cmd echo &&
 	git web--browse --browser=custom \
-		http://example.com/foo#bar >actual &&
+		http://example.com/foo#bar | tr -d "\r" >actual &&
 	test_cmp expect actual
 '
 
@@ -44,7 +44,7 @@ test_expect_success \
 	chmod +x "fake browser" &&
 	git config browser.w3m.path "`pwd`/fake browser" &&
 	git web--browse --browser=w3m \
-		http://example.com/foo >actual &&
+		http://example.com/foo | tr -d "\r" >actual &&
 	test_cmp expect actual
 '
 
@@ -59,7 +59,7 @@ test_expect_success \
 		}
 		f" &&
 	git web--browse --browser=custom \
-		http://example.com/foo >actual &&
+		http://example.com/foo | tr -d "\r" >actual &&
 	test_cmp expect actual
 '
 
-- 
1.7.7.1.gbba15

^ permalink raw reply related

* [PATCH 0/8] Some patches from msysGit
From: Pat Thoyts @ 2011-10-14 22:53 UTC (permalink / raw)
  To: Git; +Cc: Junio C Hamano, msysGit, Pat Thoyts

This series collects some recent patches required for msysGit
applied onto 'next' for upstream application.

Johannes Schindelin (4):
  t1020: disable the pwd test on MinGW
  t1402: Ignore a few cases that must fail due to DOS path expansion
  t9001: do not fail only due to CR/LF issues
  t9300: do not run --cat-blob-fd related tests on MinGW

Pat Thoyts (3):
  t9901: fix line-ending dependency on windows
  mergetools: use the correct tool for Beyond Compare 3 on Windows
  mingw: ensure sockets are initialized before calling gethostname

Sebastian Schuberth (1):
  git-svn: On MSYS, escape and quote SVN_SSH also if set by the user

 compat/mingw.c              |    7 +++++++
 compat/mingw.h              |    3 +++
 git-svn.perl                |   15 +++++++--------
 mergetools/bc3              |    9 ++++++++-
 t/t1020-subdirectory.sh     |    2 +-
 t/t1402-check-ref-format.sh |   15 +++++++++------
 t/t9001-send-email.sh       |    1 +
 t/t9300-fast-import.sh      |    8 ++++----
 t/t9901-git-web--browse.sh  |   10 +++++-----
 9 files changed, 45 insertions(+), 25 deletions(-)

-- 
1.7.7.1.gbba15

^ permalink raw reply

* Re: [PATCH] Git-p4: git-p4.changeOnSubmit to do 'change' instead of 'submit'.
From: Andrei Warkentin @ 2011-10-14 22:55 UTC (permalink / raw)
  To: Tor Arvid Lund; +Cc: git, gitster, Andrei Warkentin
In-Reply-To: <CA+DMoH-HqA0DCyUSttO-iYO0rUHq1nLqM9W0imAOjHC5H1r_9w@mail.gmail.com>

Hi Tor,

Thanks for the review!

----- Original Message -----
> Just out of curiosity... what is 'sd'?
> 

SourceDepot, a p4 fork that is used elsewhere, not by me though ;).

> > This new config option lets a 'p4 change -i' run instead of
> > the 'p4 submit -i'.
> 
> Well... I have to say that I'm not crazy about this patch... I don't
> think it is very elegant to have a config flag that says that "when
> the user says 'git p4 submit', then don't submit, but do something
> else instead".
> 
> I would much rather have made a patch to introduce some new command
> like 'git p4 change'.
> 

Agreed, how about something like this?

The commands dict maps command name to class and optional dict passed to cmd.run(). That way 'change'
can really mean P4Submit with an extra parameter not to submit but to do a changelist instead. The
reason why I initially made the config flag was because I didn't want to copy-paste P4Submit into P4Change.

commands = {
    "debug" : [ P4Debug, {} ]
    "submit" : [ P4Submit, { "doChange" : 0 } ]
    "commit" : [ P4Submit, { "doChange" : 0 } ]
    "change" : [ P4Submit, { "doChange" : 1 } ]
    "sync" : [ P4Sync, {} ],
    "rebase" : [ P4Rebase, {} ],
    "clone" : [ P4Clone, {} ],
    "rollback" : [ P4RollBack, {} ],
    "branches" : [ P4Branches, {} ]
}

A

^ permalink raw reply

* Re: Git shouldn't allow to push a new branch called HEAD
From: P Rouleau @ 2011-10-14 23:00 UTC (permalink / raw)
  To: git
In-Reply-To: <1318591877.2938.20.camel@mastroc3.mobc3.local>

Daniele Segato <daniele.segato <at> gmail.com> writes:

> 
> Hi all,
> 
> following from a discussion in IRC freenode #git between me, sitaram an
> shruggar
> 
> step to reproduce:
> 
> $ # time to create the remote HEAD branch
> $ cd buggenerator/
> $ git push origin HEAD:HEAD
> 
> But I think that git shouldn't allow the remote HEAD reference to be
> created in the first place

Maybe git should also refuse to create a local branch named HEAD. I made a
mistake recently where I used something likes this:

for B in $(git branch -a|grep "remotes/origin/"); do git co -t $B ; done

After that, git st was giving a warning about an ambiguous HEAD ref. Hopefully,
a simple "git branch -d HEAD" fixed it once I found the problem.

P.Rouleau

^ permalink raw reply

* What's cooking in git.git (Oct 2011, #05; Fri, 14)
From: Junio C Hamano @ 2011-10-14 23:23 UTC (permalink / raw)
  To: git

Here are the topics that have been cooking.  Commits prefixed with '-' are
only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'.

The second and third wave of topics have graduated to 'master'.

Here are the repositories that have my integration branches:

With maint, master, next, pu, todo, html and man:

	git://git.kernel.org/pub/scm/git/git.git
	git://repo.or.cz/alt-git.git
	https://code.google.com/p/git-core/
	https://github.com/git/git

With only maint, master, html and man:

	git://git.sourceforge.jp/gitroot/git-core/git.git
	git://git-core.git.sourceforge.net/gitroot/git-core/git-core

With all the topics and integration branches but not todo, html or man:

	https://github.com/gitster/git

--------------------------------------------------
[New Topics]

* jk/config-test-cleanup (2011-10-12) 2 commits
  (merged to 'next' on 2011-10-12 at 7c857dd)
 + t1300: test mixed-case variable retrieval
 + t1300: put git invocations inside test function

Will merge to 'master' in the fourth wave.

* jm/maint-apply-detects-corrupt-patch-header (2011-10-12) 1 commit
  (merged to 'next' on 2011-10-12 at 80d9503)
 + fix "git apply --index ..." not to deref NULL

Will merge to 'master' in the fifth wave.

* mh/ref-api (2011-10-12) 7 commits
 - clear_cached_refs(): inline function
 - write_ref_sha1(): only invalidate the loose ref cache
 - clear_cached_refs(): extract two new functions
 - clear_cached_refs(): rename parameter
 - invalidate_ref_cache(): expose this function in refs API
 - invalidate_ref_cache(): take the submodule as parameter
 - invalidate_ref_cache(): rename function from invalidate_cached_refs()

The first rename felt somewhat inconsistent in that it reworded the object
of one particular verb "invalidate" to "ref_cache" but otherwise that
entity is left as "cached_refs" throughout the codebase.

The updated series looked almost all trivial and sensible but they would
depend on this renaming, so I haven't queued them here yet. Personally I
think we should either not rename just the function name (i.e. drop the
first patch in this series) or rename both the function and what it
operates on (i.e. enhance the first patch in this series).

* bk/submodule-in-recursive-merge (2011-10-13) 2 commits
 - submodule: Search for merges only at end of recursive merge
 - submodule: Demonstrate known breakage during recursive merge

Brad helped resurrecting good bits earlier tangled in the stalled topic
hv/submodule-merge-search by mistake.
Will merge to 'next'.

* jk/pull-rebase-with-work-tree (2011-10-13) 1 commit
 - pull,rebase: handle GIT_WORK_TREE better

Looked reasonable.
Will merge to 'next'.

* sg/complete-refs (2011-10-12) 10 commits
 - completion: remove broken dead code from __git_heads() and __git_tags()
 - completion: fast initial completion for config 'remote.*.fetch' value
 - completion: improve ls-remote output filtering in __git_refs_remotes()
 - completion: query only refs/heads/ in __git_refs_remotes()
 - completion: support full refs from remote repositories
 - completion: improve ls-remote output filtering in __git_refs()
 - completion: make refs completion consistent for local and remote repos
 - completion: optimize refs completion
 - completion: document __gitcomp()
 - Merge branches 'tm/completion-push-set-upstream', 'tm/completion-commit-fixup-squash' and 'sg/completion' into HEAD
 (this branch uses sg/completion, tm/completion-commit-fixup-squash and tm/completion-push-set-upstream.)

Re-roll expected.

* jc/unseekable-bundle (2011-10-13) 2 commits
 - bundle: add parse_bundle_header() helper function
 - bundle: allowing to read from an unseekable fd

I am not entirely happy with the first patch but it is not so bad either.

* js/check-ref-format-test-mingw (2011-10-13) 1 commit
 - t1402-check-ref-format: skip tests of refs beginning with slash on Windows

Will merge to 'next'.

* jk/daemon-msgs (2011-10-14) 1 commit
 - daemon: give friendlier error messages to clients

Will merge to 'next'.

* jk/http-auth (2011-10-14) 6 commits
 - http_init: accept separate URL parameter
 - http: use hostname in credential description
 - http: retry authentication failures for all http requests
 - remote-curl: don't retry auth failures with dumb protocol
 - improve httpd auth tests
 - url: decode buffers that are not NUL-terminated
 (this branch is tangled with jk/http-auth-keyring and js/cred-macos-x-keychain-2.)

Michael helped resurrecting uncontentious bits from the credential series.
Will merge to 'next'.

* jk/maint-pack-objects-compete-with-delete (2011-10-14) 2 commits
 - downgrade "packfile cannot be accessed" errors to warnings
 - pack-objects: protect against disappearing packs

Will merge to 'next'.

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

* bw/grep-no-index-no-exclude (2011-09-15) 2 commits
  (merged to 'next' on 2011-10-06 at 325270b)
 + grep --no-index: don't use git standard exclusions
 + grep: do not use --index in the short usage output
 (this branch is used by jc/grep-untracked-exclude and jc/maint-grep-untracked-exclude.)

Originally merged to 'next' on 2011-09-26.

* cb/do-not-pretend-to-hijack-long-help (2011-10-05) 1 commit
  (merged to 'next' on 2011-10-06 at 46851fe)
 + use -h for synopsis and --help for manpage consistently

* cp/git-web-browse-browsers (2011-10-03) 1 commit
  (merged to 'next' on 2011-10-06 at da42ad0)
 + git-web--browse: avoid the use of eval

* cs/perl-config-path-send-email (2011-09-30) 2 commits
  (merged to 'next' on 2011-10-06 at 93c00f0)
 + use new Git::config_path() for aliasesfile
 + Add Git::config_path()

Originally merged to 'next' on 2011-10-05.

* di/fast-import-empty-tag-note-fix (2011-09-22) 2 commits
  (merged to 'next' on 2011-10-06 at 3a01ef1)
 + fast-import: don't allow to note on empty branch
 + fast-import: don't allow to tag empty branch

Originally merged to 'next' on 2011-10-05.

* il/archive-err-signal (2011-10-05) 1 commit
  (merged to 'next' on 2011-10-06 at 7e3083f)
 + Support ERR in remote archive like in fetch/push

* jc/apply-blank-at-eof-fix (2011-09-26) 1 commit
  (merged to 'next' on 2011-10-06 at a9dfd8f)
 + apply --whitespace=error: correctly report new blank lines at end

Originally merged to 'next' on 2011-10-05.

* jc/grep-untracked-exclude (2011-10-04) 1 commit
  (merged to 'next' on 2011-10-06 at b16cffe)
 + Merge branch 'jc/maint-grep-untracked-exclude' into jc/grep-untracked-exclude
 (this branch uses bw/grep-no-index-no-exclude and jc/maint-grep-untracked-exclude.)

* jc/is-url-simplify (2011-10-03) 1 commit
  (merged to 'next' on 2011-10-06 at d6c6741)
 + url.c: simplify is_url()

* jc/maint-grep-untracked-exclude (2011-10-04) 1 commit
 + grep: teach --untracked and --exclude-standard options
 (this branch is used by jc/grep-untracked-exclude; uses bw/grep-no-index-no-exclude.)

* jc/parse-options-boolean (2011-09-28) 5 commits
  (merged to 'next' on 2011-10-06 at dd4936c)
 + apply: use OPT_NOOP_NOARG
 + revert: use OPT_NOOP_NOARG
 + parseopt: add OPT_NOOP_NOARG
 + archive.c: use OPT_BOOL()
 + parse-options: deprecate OPT_BOOLEAN

* jn/ident-from-etc-mailname (2011-10-06) 2 commits
  (merged to 'next' on 2011-10-06 at a68770d)
 + ident: do not retrieve default ident when unnecessary
 + ident: check /etc/mailname if email is unknown

* jn/no-g-plus-s-on-bsd (2011-10-03) 1 commit
  (merged to 'next' on 2011-10-06 at 3d85674)
 + Makefile: do not set setgid bit on directories on GNU/kFreeBSD

* js/maint-merge-one-file-osx-expr (2011-10-06) 1 commit
  (merged to 'next' on 2011-10-07 at fbb28a2)
 + merge-one-file: fix "expr: non-numeric argument"

* nd/daemon-log-sock-errors (2011-10-03) 1 commit
  (merged to 'next' on 2011-10-06 at 5f3630f)
 + daemon: log errors if we could not use some sockets

* nd/document-err-packet (2011-10-03) 1 commit
  (merged to 'next' on 2011-10-06 at 0c5f5d0)
 + pack-protocol: document "ERR" line

* nd/git-daemon-error-msgs (2011-10-03) 1 commit
  (merged to 'next' on 2011-10-06 at 209126d)
 + daemon: return "access denied" if a service is not allowed

* nd/maint-autofix-tag-in-head (2011-09-18) 4 commits
  (merged to 'next' on 2011-10-06 at c083e69)
 + Accept tags in HEAD or MERGE_HEAD
 + merge: remove global variable head[]
 + merge: use return value of resolve_ref() to determine if HEAD is invalid
 + merge: keep stash[] a local variable

Originally merged to 'next' on 2011-09-27.

* nd/maint-sparse-errors (2011-09-22) 2 commits
  (merged to 'next' on 2011-10-06 at e3cbb90)
 + Add explanation why we do not allow to sparse checkout to empty working tree
 + sparse checkout: show error messages when worktree shaping fails

Originally merged to 'next' on 2011-09-22.

* rs/diff-cleanup-records-fix (2011-10-03) 2 commits
  (merged to 'next' on 2011-10-06 at 91f035f)
 + diff: resurrect XDF_NEED_MINIMAL with --minimal
 + Revert removal of multi-match discard heuristic in 27af01

* rs/name-rev-usage (2011-10-03) 1 commit
  (merged to 'next' on 2011-10-06 at e51878e)
 + name-rev: split usage string

Originally merged to 'next' on 2011-10-05.

* rs/pending (2011-10-03) 8 commits
  (merged to 'next' on 2011-10-06 at 998462b)
 + commit: factor out clear_commit_marks_for_object_array
 + checkout: use leak_pending flag
 + bundle: use leak_pending flag
 + bisect: use leak_pending flag
 + revision: add leak_pending flag
 + checkout: use add_pending_{object,sha1} in orphan check
 + revision: factor out add_pending_sha1
 + checkout: check for "Previous HEAD" notice in t2020

* rs/test-ctype (2011-10-03) 2 commits
  (merged to 'next' on 2011-10-06 at b8c26d2)
 + test-ctype: add test for is_pathspec_magic
 + test-ctype: macrofy

Originally merged to 'next' on 2011-10-05.

* sp/smart-http-failure (2011-10-04) 1 commit
  (merged to 'next' on 2011-10-06 at 02f9982)
 + remote-curl: Fix warning after HTTP failure

* zj/send-email-authen-sasl (2011-09-29) 1 commit
  (merged to 'next' on 2011-10-06 at 78b31cd)
 + send-email: auth plain/login fix

Originally merged to 'next' on 2011-10-05.

--------------------------------------------------
[Stalled]

* hv/submodule-merge-search (2011-10-13) 4 commits
 - submodule.c: make two functions static
 - allow multiple calls to submodule merge search for the same path
 - push: Don't push a repository with unpushed submodules
 - push: teach --recurse-submodules the on-demand option

What the topic aims to achieve may make sense, but the implementation
looked somewhat suboptimal.

The fix-up at the tip queued on fg/submodule-auto-push topic has been
moved to this topic.

* sr/transport-helper-fix-rfc (2011-07-19) 2 commits
 - t5800: point out that deleting branches does not work
 - t5800: document inability to push new branch with old content

Perhaps 281eee4 (revision: keep track of the end-user input from the
command line, 2011-08-25) would help.

* cn/fetch-prune (2011-10-07) 4 commits
 - fetch: treat --tags like refs/tags/*:refs/tags/* when pruning
 - fetch: honor the user-provided refspecs when pruning refs
 - t5510: add tests for fetch --prune
 - fetch: free all the additional refspecs

Probably needs a little bit more polish to reduce code duplication between
existing remote_find_tracking() and new find_in_refs().

* jn/gitweb-manpages (2011-10-10) 6 commits
 . gitweb: Add gitweb manpages to 'gitweb' package in git.spec
 . Documentation: Add gitweb config variables to git-config(1)
 . Documentation: Link to gitweb(1) and gitweb.conf(5) in other manpages
 . gitweb: Add manpage for gitweb
 . gitweb: Add manpage for gitweb configuration files
 . Documentation: Preparation for gitweb manpages

A re-roll already being discussed.

* rr/revert-cherry-pick (2011-10-12) 7 commits
 - revert: further simplify parsing of a line in insn sheet
 - revert: Simplify passing command-line arguments around
 - revert: Allow mixed pick and revert instructions
 - revert: Make commit descriptions in insn sheet optional
 - revert: Fix buffer overflow in insn sheet parser
 - revert: Simplify getting commit subject
 - revert: Free memory after get_message call

Probably needs a little bit more polish, e.g. squashing the tip fixup into
an earlier one in the series.

* jc/signed-commit (2011-10-05) 4 commits
 - commit: teach --gpg-sign option
 - Split GPG interface into its own helper library
 - rename "match_refs()" to "match_push_refs()"
 - send-pack: typofix error message

This is to replace the earlier "signed push" experiments. "verify-tag"
equivalent needs to be written before this can proceed. I suspect that
teaching "verify-tag" to notice and also handle signed commits would be
the easiest, but "git tag --verify $commit" might look slightly funny
from the UI POV. I dunno.

* jc/lookup-object-hash (2011-08-11) 6 commits
 - object hash: replace linear probing with 4-way cuckoo hashing
 - object hash: we know the table size is a power of two
 - object hash: next_size() helper for readability
 - pack-objects --count-only
 - object.c: remove duplicated code for object hashing
 - object.c: code movement for readability

I do not think there is anything fundamentally wrong with this series, but
the risk of breakage far outweighs observed performance gain in one
particular workload.

--------------------------------------------------
[Cooking]

* ef/mingw-syslog (2011-10-07) 1 commit
  (merged to 'next' on 2011-10-11 at d5d6945)
 + mingw: avoid using strbuf in syslog

Will merge to 'master' in the fourth wave.

* jk/name-hash-dirent (2011-10-07) 1 commit
  (merged to 'next' on 2011-10-11 at e2ea68b)
 + fix phantom untracked files when core.ignorecase is set

Will merge to 'master' in the fourth wave.

* js/merge-edit-option (2011-10-12) 1 commit
  (merged to 'next' on 2011-10-12 at db28da3)
 + Teach merge the '[-e|--edit]' option

Will merge to 'master' in the fifth wave.

* mm/maint-config-explicit-bool-display (2011-10-10) 1 commit
  (merged to 'next' on 2011-10-11 at 795939f)
 + config: display key_delim for config --bool --get-regexp

Will merge to 'master' in the fourth wave.

* rs/diff-whole-function (2011-10-10) 2 commits
  (merged to 'next' on 2011-10-11 at 6196752)
 + diff: add option to show whole functions as context
 + xdiff: factor out get_func_line()

Will merge to 'master' in the fifth wave.

* rs/pickaxe (2011-10-07) 7 commits
  (merged to 'next' on 2011-10-11 at 27d02b2)
 + pickaxe: factor out pickaxe
 + pickaxe: give diff_grep the same signature as has_changes
 + pickaxe: pass diff_options to contains and has_changes
 + pickaxe: factor out has_changes
 + pickaxe: plug regex/kws leak
 + pickaxe: plug regex leak
 + pickaxe: plug diff filespec leak with empty needle

Will merge to 'master' in the fifth wave.

* sc/difftool-skip (2011-10-14) 2 commits
  (merged to 'next' on 2011-10-14 at b91c581)
 + t7800: avoid arithmetic expansion notation
  (merged to 'next' on 2011-10-11 at 38d7e84)
 + git-difftool: allow skipping file by typing 'n' at prompt

Will merge to 'master' in the fifth wave.

* sg/completion (2011-10-10) 2 commits
  (merged to 'next' on 2011-10-11 at 4724640)
 + completion: unite --format and --pretty for 'log' and 'show'
 + completion: unite --reuse-message and --reedit-message for 'notes'
 (this branch is used by sg/complete-refs; uses tm/completion-commit-fixup-squash.)

Will merge to 'master' in the fourth wave.

* tc/fetch-leak (2011-10-07) 1 commit
  (merged to 'next' on 2011-10-11 at d867153)
 + fetch: plug two leaks on error exit in store_updated_refs

Will merge to 'master' in the fourth wave.

* jc/check-ref-format-fixup (2011-10-12) 2 commits
 - Restrict ref-like names immediately below $GIT_DIR
 - refs.c: move dwim_ref()/dwim_log() from sha1_name.c

An attempt to fix-up unfortunate side effect of mh/check-ref-format-3
topic. "git show -s config" is never meant to refer to $GIT_DIR/config
and treat it as a file that records an object name.

Will merge to 'next'.

* jc/maint-remove-renamed-ref (2011-10-12) 1 commit
  (merged to 'next' on 2011-10-12 at 819c3e4)
 + branch -m/-M: remove undocumented RENAMED-REF

Will merge to 'master' in the fifth wave.

* tm/completion-commit-fixup-squash (2011-10-06) 2 commits
  (merged to 'next' on 2011-10-11 at 6bb192e)
 + completion: commit --fixup and --squash
 + completion: unite --reuse-message and --reedit-message handling
 (this branch is used by sg/complete-refs and sg/completion.)

Will merge to 'master' in the fourth wave.

* tm/completion-push-set-upstream (2011-10-06) 1 commit
  (merged to 'next' on 2011-10-11 at 85544e5)
 + completion: push --set-upstream
 (this branch is used by sg/complete-refs.)

Will merge to 'master' in the fourth wave.

* js/no-cherry-pick-head-after-punted (2011-10-06) 1 commit
  (merged to 'next' on 2011-10-10 at acb29ee)
 + Merge branch 'js/maint-no-cherry-pick-head-after-punted' into js/no-cherry-pick-head-after-punted
 (this branch uses js/maint-no-cherry-pick-head-after-punted.)

Will merge to 'master' in the fifth wave.

* js/maint-no-cherry-pick-head-after-punted (2011-10-06) 2 commits
 + cherry-pick: do not give irrelevant advice when cherry-pick punted
 + revert.c: defer writing CHERRY_PICK_HEAD till it is safe to do so
 (this branch is used by js/no-cherry-pick-head-after-punted.)

Will merge to 'maint' later.

* js/log-show-children (2011-10-04) 1 commit
  (merged to 'next' on 2011-10-06 at de8f6f2)
 + log --children

Will merge to 'master' in the fifth wave.

* ph/transport-with-gitfile (2011-10-11) 5 commits
  (merged to 'next' on 2011-10-12 at 6d58417)
 + Fix is_gitfile() for files too small or larger than PATH_MAX to be a gitfile
  (merged to 'next' on 2011-10-06 at 891b8b6)
 + Add test showing git-fetch groks gitfiles
 + Teach transport about the gitfile mechanism
 + Learn to handle gitfiles in enter_repo
 + enter_repo: do not modify input

Will merge to 'master' in the fifth wave.

* jc/checkout-from-tree-keep-local-changes (2011-09-30) 1 commit
  (merged to 'next' on 2011-10-06 at 64061aa)
 + checkout $tree $path: do not clobber local changes in $path not in $tree

Originally merged to 'next' on 2011-10-05.
Will merge to 'master' in the fourth wave.

* ph/push-to-delete-nothing (2011-09-30) 1 commit
  (merged to 'next' on 2011-10-06 at 33ac777)
 + receive-pack: don't pass non-existent refs to post-{receive,update} hooks

Will merge to 'master' in the fourth wave.

* js/bisect-no-checkout (2011-09-21) 1 commit
  (merged to 'next' on 2011-10-06 at 0354e94)
 + bisect: fix exiting when checkout failed in bisect_start()

Originally merged to 'next' on 2011-09-21.
Will merge to 'master' in the fourth wave.

* jc/request-pull-show-head-4 (2011-10-09) 10 commits
  (merged to 'next' on 2011-10-10 at 092175e)
 + environment.c: Fix an sparse "symbol not declared" warning
 + builtin/log.c: Fix an "Using plain integer as NULL pointer" warning
  (merged to 'next' on 2011-10-07 at fcaeca0)
 + fmt-merge-msg: use branch.$name.description
  (merged to 'next' on 2011-10-06 at fa5e0fe)
 + request-pull: use the branch description
 + request-pull: state what commit to expect
 + request-pull: modernize style
 + branch: teach --edit-description option
 + format-patch: use branch description in cover letter
 + branch: add read_branch_desc() helper function
 + Merge branch 'bk/ancestry-path' into jc/branch-desc

Will merge to 'master' in the fifth wave.

* bc/attr-ignore-case (2011-10-11) 5 commits
  (merged to 'next' on 2011-10-11 at daa6b51)
 + attr.c: respect core.ignorecase when matching attribute patterns
 + attr: read core.attributesfile from git_default_core_config
 + builtin/mv.c: plug miniscule memory leak
 + cleanup: use internal memory allocation wrapper functions everywhere
 + attr.c: avoid inappropriate access to strbuf "buf" member

Will merge to 'master' in the fourth wave.

--------------------------------------------------
[Discarded]

* jk/http-auth-keyring (2011-10-12) 23 commits
 . http_init: accept separate URL parameter
 . credential-cache: don't cache items without context
 . check_expirations: don't copy over same element
 . t0300: add missing EOF terminator for <<
 . credential-store: use a better storage format
 . t0300: make alternate username tests more robust
 . t0300: make askpass tests a little more robust
 . credential-cache: fix expiration calculation corner cases
 . docs: minor tweaks to credentials API
 . credentials: make credential_fill_gently() static
 . credentials: add "getpass" helper
 . credentials: add "store" helper
 . credentials: add "cache" helper
 . docs: end-user documentation for the credential subsystem
 . http: use hostname in credential description
 . allow the user to configure credential helpers
 . look for credentials in config before prompting
 . http: use credential API to get passwords
 . introduce credentials API
 - http: retry authentication failures for all http requests
 - remote-curl: don't retry auth failures with dumb protocol
 - improve httpd auth tests
 - url: decode buffers that are not NUL-terminated
 (this branch is used by js/cred-macos-x-keychain-2; is tangled with jk/http-auth.)

Discarded without prejudice to allow design level discussions to continue.
Expecting a re-roll based on jk/http-auth

* js/cred-macos-x-keychain-2 (2011-10-12) 1 commit
 . contrib: add a pair of credential helpers for Mac OS X's keychain
 (this branch uses jk/http-auth-keyring; is tangled with jk/http-auth.)

Discarded without prejudice to allow design level discussions to continue.
Expecting a re-roll based on jk/http-auth

^ permalink raw reply

* Re: [PATCH] send-email: Fix %config_path_settings handling
From: Junio C Hamano @ 2011-10-14 23:23 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Michael J Gruber, Git Mailing List, Cord Seele, Cord Seele
In-Reply-To: <201110142253.32695.jnareb@gmail.com>

Jakub Narebski <jnareb@gmail.com> writes:

> From: Cord Seele <cowose@gmail.com>
> ...
> Signed-off-by: Cord Seele <cowose@gmail.com>
> Tested-by: Michael J Gruber <git@drmicha.warpmail.net>
> Signed-off-by: Jakub Narebski <jnareb@gmail.com>

Thanks.

> diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
> index 579ddb7..87b4acc 100755
> --- a/t/t9001-send-email.sh
> +++ b/t/t9001-send-email.sh
> @@ -1168,4 +1168,32 @@ test_expect_success $PREREQ '--force sends cover letter template anyway' '
>  	test -n "$(ls msgtxt*)"
>  '
>  
> +test_expect_success $PREREQ 'sendemail.aliasfiletype=mailrc' '
> +	clean_fake_sendmail &&
> +	echo "alias sbd  somebody@example.org" >.mailrc &&
> +	git config --replace-all sendemail.aliasesfile "$(pwd)/.mailrc" &&
> +	git config sendemail.aliasfiletype mailrc &&
> +	git send-email \
> +	  --from="Example <nobody@example.com>" \
> +	  --to=sbd \
> +	  --smtp-server="$(pwd)/fake.sendmail" \
> +	  outdir/0001-*.patch \
> +	  2>errors >out &&
> +	grep "^!somebody@example\.org!$" commandline1
> +'
> +
> +test_expect_success $PREREQ 'sendemail.aliasfile=~/.mailrc' '
> +	clean_fake_sendmail &&
> +	echo "alias sbd  someone@example.org" >~/.mailrc &&
> +	git config --replace-all sendemail.aliasesfile "~/.mailrc" &&
> +	git config sendemail.aliasfiletype mailrc &&
> +	git send-email \
> +	  --from="Example <nobody@example.com>" \
> +	  --to=sbd \
> +	  --smtp-server="$(pwd)/fake.sendmail" \
> +	  outdir/0001-*.patch \
> +	  2>errors >out &&
> +	grep "^!someone@example\.org!$" commandline1
> +'
> +
>  test_done

^ permalink raw reply

* Re: [PATCHv3] daemon: give friendlier error messages to clients
From: Sitaram Chamarty @ 2011-10-14 23:39 UTC (permalink / raw)
  To: Jeff King
  Cc: Jonathan Nieder, Junio C Hamano, Nguyen Thai Ngoc Duy, git,
	Ilari Liusvaara, Johannes Sixt
In-Reply-To: <20111014211921.GB16429@sigill.intra.peff.net>

On Sat, Oct 15, 2011 at 2:49 AM, Jeff King <peff@peff.net> wrote:
> When the git-daemon is asked about an inaccessible
> repository, it simply hangs up the connection without saying
> anything further. This makes it hard to distinguish between
> a repository we cannot access (e.g., due to typo), and a
> service or network outage.
>
> Instead, let's print an "ERR" line, which git clients
> understand since v1.6.1 (2008-12-24).
>
> Because there is a risk of leaking information about
> non-exported repositories, by default all errors simply say
> "access denied". Sites which don't have hidden repositories,

I suggest that even the "secure" version of the message say something
like "access denied or repository not exported".  You're still not
leaking anything, but it reduces confusion to the user, who otherwise
may not realise it *could be* the latter.

regards

sitaram

> or don't care, can pass a flag to turn on more specific
> messages.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> Minor tweaks to the documentation and code style to make Jonathan happy.
> :)
>
> Note: I labeled this "v3", as it is the third one posted, but the prior
> ones were not labeled with versions at all.
>
>  Documentation/git-daemon.txt |   10 ++++++++++
>  daemon.c                     |   25 +++++++++++++++++++++----
>  2 files changed, 31 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/git-daemon.txt b/Documentation/git-daemon.txt
> index 69a1e4a..31b28fc 100644
> --- a/Documentation/git-daemon.txt
> +++ b/Documentation/git-daemon.txt
> @@ -161,6 +161,16 @@ the facility of inet daemon to achieve the same before spawning
>        repository configuration.  By default, all the services
>        are overridable.
>
> +--informative-errors::
> +--no-informative-errors::
> +       When informative errors are turned on, git-daemon will report
> +       more verbose errors to the client, differentiating conditions
> +       like "no such repository" from "repository not exported". This
> +       is more convenient for clients, but may leak information about
> +       the existence of unexported repositories.  When informative
> +       errors are not enabled, all errors report "access denied" to the
> +       client. The default is --no-informative-errors.
> +
>  <directory>::
>        A directory to add to the whitelist of allowed directories. Unless
>        --strict-paths is specified this will also include subdirectories
> diff --git a/daemon.c b/daemon.c
> index 4c8346d..6f111af 100644
> --- a/daemon.c
> +++ b/daemon.c
> @@ -20,6 +20,7 @@
>  static int log_syslog;
>  static int verbose;
>  static int reuseaddr;
> +static int informative_errors;
>
>  static const char daemon_usage[] =
>  "git daemon [--verbose] [--syslog] [--export-all]\n"
> @@ -247,6 +248,14 @@ static int git_daemon_config(const char *var, const char *value, void *cb)
>        return 0;
>  }
>
> +static int daemon_error(const char *dir, const char *msg)
> +{
> +       if (!informative_errors)
> +               msg = "access denied";
> +       packet_write(1, "ERR %s: %s", msg, dir);
> +       return -1;
> +}
> +
>  static int run_service(char *dir, struct daemon_service *service)
>  {
>        const char *path;
> @@ -257,11 +266,11 @@ static int run_service(char *dir, struct daemon_service *service)
>        if (!enabled && !service->overridable) {
>                logerror("'%s': service not enabled.", service->name);
>                errno = EACCES;
> -               return -1;
> +               return daemon_error(dir, "service not enabled");
>        }
>
>        if (!(path = path_ok(dir)))
> -               return -1;
> +               return daemon_error(dir, "no such repository");
>
>        /*
>         * Security on the cheap.
> @@ -277,7 +286,7 @@ static int run_service(char *dir, struct daemon_service *service)
>        if (!export_all_trees && access("git-daemon-export-ok", F_OK)) {
>                logerror("'%s': repository not exported.", path);
>                errno = EACCES;
> -               return -1;
> +               return daemon_error(dir, "repository not exported");
>        }
>
>        if (service->overridable) {
> @@ -291,7 +300,7 @@ static int run_service(char *dir, struct daemon_service *service)
>                logerror("'%s': service not enabled for '%s'",
>                         service->name, path);
>                errno = EACCES;
> -               return -1;
> +               return daemon_error(dir, "service not enabled");
>        }
>
>        /*
> @@ -1167,6 +1176,14 @@ int main(int argc, char **argv)
>                        make_service_overridable(arg + 18, 0);
>                        continue;
>                }
> +               if (!prefixcmp(arg, "--informative-errors")) {
> +                       informative_errors = 1;
> +                       continue;
> +               }
> +               if (!prefixcmp(arg, "--no-informative-errors")) {
> +                       informative_errors = 0;
> +                       continue;
> +               }
>                if (!strcmp(arg, "--")) {
>                        ok_paths = &argv[i+1];
>                        break;
> --
> 1.7.6.4.37.g43b58b
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>



-- 
Sitaram

^ permalink raw reply

* Re: [PATCHv3] daemon: give friendlier error messages to clients
From: Nguyen Thai Ngoc Duy @ 2011-10-15  0:51 UTC (permalink / raw)
  To: Jeff King
  Cc: Jonathan Nieder, Junio C Hamano, git, Ilari Liusvaara,
	Johannes Sixt
In-Reply-To: <20111014211921.GB16429@sigill.intra.peff.net>

On Sat, Oct 15, 2011 at 8:19 AM, Jeff King <peff@peff.net> wrote:
> @@ -257,11 +266,11 @@ static int run_service(char *dir, struct daemon_service *service)
>        if (!enabled && !service->overridable) {
>                logerror("'%s': service not enabled.", service->name);
>                errno = EACCES;
> -               return -1;
> +               return daemon_error(dir, "service not enabled");
>        }

Nit picking. In this case the service is disabled entirely regardless
dir and it uses the same message with the case where service is
disabled per repo later on. Maybe we could reword it a little bit to
differentiate the two cases? Say the first case "service disabled",
and the second one "service not enabled"?
-- 
Duy

^ permalink raw reply

* [PATCHv4 0/5] Be more careful when prunning
From: Carlos Martín Nieto @ 2011-10-15  5:04 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Junio C Hamano, mathstuf

Hello,

This version covers Junio's concerns about code and functionality
duplication with the third patch. The order of arguments for
get_stale_heads is not changed anymore in the fourth patch.

The first patch is not that big a deal, but it's better if we're
freeing the refspecs, we might as well free all of them.

The second patch introduces expected failures for the features that
this series fixes.

The third patch is new and moves the logic out of remote_find_tracking
into a query_refspecs function. remote_find_tracking and
apply_refspecs are both changed to be wrappers around this
function. This way, the logic is in one function instead of three
different ones.

The fourth patch changes prune_refs and get_stale_heads so the caller
has to decide which refspecs are the appropriate ones to use. For
example, running

    git fetch --prune origin refs/heads/master:refs/heads/master

doesn't remove the other branches anymore. For a more interesting (and
believable) example, let's take

    git fetch --prune origin refs/heads/b/*:refs/heads/b/*

because you want to prune the refs inside the b/ namespace
only. Currently git will delete all the refs that aren't under that
namespace. With the second patch applied, git won't remove any refs
outside the b/ namespace.

What is probably the most usual case is covered by the fifth patch,
which pretends that a "refs/tags/*:refs/tags/*" refspec was given on
the command-line. That fixes the

    git fetch --prune --tags origin

case. The non-tag refs are kept now.

Cheers,
   cmn

Carlos Martín Nieto (5):
  fetch: free all the additional refspecs
  t5510: add tests for fetch --prune
  remote: separate out the remote_find_tracking logic into
    query_refspecs
  fetch: honor the user-provided refspecs when pruning refs
  fetch: treat --tags like refs/tags/*:refs/tags/* when pruning

 builtin/fetch.c  |   33 ++++++++++++++---
 builtin/remote.c |    3 +-
 remote.c         |  106 +++++++++++++++++++++++++++++------------------------
 remote.h         |    2 +-
 t/t5510-fetch.sh |   50 +++++++++++++++++++++++++
 5 files changed, 139 insertions(+), 55 deletions(-)

-- 
1.7.5.2.354.g349bf

^ permalink raw reply

* [PATCH 4/5] fetch: honor the user-provided refspecs when pruning refs
From: Carlos Martín Nieto @ 2011-10-15  5:04 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Junio C Hamano, mathstuf
In-Reply-To: <1318655066-29001-1-git-send-email-cmn@elego.de>

If the user gave us refspecs on the command line, we should use those
when deciding whether to prune a ref instead of relying on the
refspecs in the config.

Previously, running

    git fetch --prune origin refs/heads/master:refs/remotes/origin/master

would delete every other ref under the origin namespace because we
were using the refspec to filter the available refs but using the
configured refspec to figure out if a ref had been deleted on the
remote. This is clearly the wrong thing to do.

Change prune_refs and get_stale_heads to simply accept a list of
references and a list of refspecs. The caller of either function needs
to decide what refspecs should be used to decide whether a ref is
stale.

Signed-off-by: Carlos Martín Nieto <cmn@elego.de>
---
 builtin/fetch.c  |   12 ++++++++----
 builtin/remote.c |    3 ++-
 remote.c         |   36 ++++++++++++++++++++++++------------
 remote.h         |    2 +-
 t/t5510-fetch.sh |    4 ++--
 5 files changed, 37 insertions(+), 20 deletions(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 605d1bf..e295d97 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -540,10 +540,10 @@ static int fetch_refs(struct transport *transport, struct ref *ref_map)
 	return ret;
 }
 
-static int prune_refs(struct transport *transport, struct ref *ref_map)
+static int prune_refs(struct refspec *refs, int ref_count, struct ref *ref_map)
 {
 	int result = 0;
-	struct ref *ref, *stale_refs = get_stale_heads(transport->remote, ref_map);
+	struct ref *ref, *stale_refs = get_stale_heads(refs, ref_count, ref_map);
 	const char *dangling_msg = dry_run
 		? _("   (%s will become dangling)\n")
 		: _("   (%s has become dangling)\n");
@@ -734,8 +734,12 @@ static int do_fetch(struct transport *transport,
 		free_refs(ref_map);
 		return 1;
 	}
-	if (prune)
-		prune_refs(transport, ref_map);
+	if (prune) {
+		if (ref_count)
+			prune_refs(refs, ref_count, ref_map);
+		else
+			prune_refs(transport->remote->fetch, transport->remote->fetch_refspec_nr, ref_map);
+	}
 	free_refs(ref_map);
 
 	/* if neither --no-tags nor --tags was specified, do automated tag
diff --git a/builtin/remote.c b/builtin/remote.c
index f2a9c26..de52367 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -349,7 +349,8 @@ static int get_ref_states(const struct ref *remote_refs, struct ref_states *stat
 		else
 			string_list_append(&states->tracked, abbrev_branch(ref->name));
 	}
-	stale_refs = get_stale_heads(states->remote, fetch_map);
+	stale_refs = get_stale_heads(states->remote->fetch,
+				     states->remote->fetch_refspec_nr, fetch_map);
 	for (ref = stale_refs; ref; ref = ref->next) {
 		struct string_list_item *item =
 			string_list_append(&states->stale, abbrev_branch(ref->name));
diff --git a/remote.c b/remote.c
index e94c6d2..069b1ff 100644
--- a/remote.c
+++ b/remote.c
@@ -1679,36 +1679,48 @@ struct ref *guess_remote_head(const struct ref *head,
 }
 
 struct stale_heads_info {
-	struct remote *remote;
 	struct string_list *ref_names;
 	struct ref **stale_refs_tail;
+	struct refspec *refs;
+	int ref_count;
 };
 
 static int get_stale_heads_cb(const char *refname,
 	const unsigned char *sha1, int flags, void *cb_data)
 {
 	struct stale_heads_info *info = cb_data;
-	struct refspec refspec;
-	memset(&refspec, 0, sizeof(refspec));
-	refspec.dst = (char *)refname;
-	if (!remote_find_tracking(info->remote, &refspec)) {
-		if (!((flags & REF_ISSYMREF) ||
-		    string_list_has_string(info->ref_names, refspec.src))) {
-			struct ref *ref = make_linked_ref(refname, &info->stale_refs_tail);
-			hashcpy(ref->new_sha1, sha1);
-		}
+	struct refspec query;
+	memset(&query, 0, sizeof(struct refspec));
+	query.dst = (char *)refname;
+
+	if (query_refspecs(info->refs, info->ref_count, &query))
+	    return 0; /* No matches */
+
+
+	/*
+	 * If we did find a suitable refspec and it's not a symref and
+	 * it's not in the list of refs that currently exist in that
+	 * remote we consider it to be stale.
+	 */
+	if (!((flags & REF_ISSYMREF) ||
+	      string_list_has_string(info->ref_names, query.src))) {
+		struct ref *ref = make_linked_ref(refname, &info->stale_refs_tail);
+		hashcpy(ref->new_sha1, sha1);
 	}
+
+	free(query.src);
 	return 0;
 }
 
-struct ref *get_stale_heads(struct remote *remote, struct ref *fetch_map)
+struct ref *get_stale_heads(struct refspec *refs, int ref_count, struct ref *fetch_map)
 {
 	struct ref *ref, *stale_refs = NULL;
 	struct string_list ref_names = STRING_LIST_INIT_NODUP;
 	struct stale_heads_info info;
-	info.remote = remote;
 	info.ref_names = &ref_names;
 	info.stale_refs_tail = &stale_refs;
+	info.refs = refs;
+	info.ref_count = ref_count;
 	for (ref = fetch_map; ref; ref = ref->next)
 		string_list_append(&ref_names, ref->name);
 	sort_string_list(&ref_names);
diff --git a/remote.h b/remote.h
index 9a30a9d..f2541b5 100644
--- a/remote.h
+++ b/remote.h
@@ -164,6 +164,6 @@ struct ref *guess_remote_head(const struct ref *head,
 			      int all);
 
 /* Return refs which no longer exist on remote */
-struct ref *get_stale_heads(struct remote *remote, struct ref *fetch_map);
+struct ref *get_stale_heads(struct refspec *refs, int ref_count, struct ref *fetch_map);
 
 #endif
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 8b5e925..581049b 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -86,7 +86,7 @@ test_expect_success 'fetch --prune on its own works as expected' '
 	test_must_fail git rev-parse origin/extrabranch
 '
 
-test_expect_failure 'fetch --prune with a branch name keeps branches' '
+test_expect_success 'fetch --prune with a branch name keeps branches' '
 	cd "$D" &&
 	git clone . prune-branch &&
 	cd prune-branch &&
@@ -96,7 +96,7 @@ test_expect_failure 'fetch --prune with a branch name keeps branches' '
 	git rev-parse origin/extrabranch
 '
 
-test_expect_failure 'fetch --prune with a namespace keeps other namespaces' '
+test_expect_success 'fetch --prune with a namespace keeps other namespaces' '
 	cd "$D" &&
 	git clone . prune-namespace &&
 	cd prune-namespace &&
-- 
1.7.5.2.354.g349bf

^ permalink raw reply related

* [PATCH 5/5] fetch: treat --tags like refs/tags/*:refs/tags/* when pruning
From: Carlos Martín Nieto @ 2011-10-15  5:04 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Junio C Hamano, mathstuf
In-Reply-To: <1318655066-29001-1-git-send-email-cmn@elego.de>

If --tags is specified, add that refspec to the list given to
prune_refs so it knows to treat it as a filter on what refs to
should consider for prunning. This way

    git fetch --prune --tags origin

only prunes tags and doesn't delete the branch refs.

Signed-off-by: Carlos Martín Nieto <cmn@elego.de>
---
 builtin/fetch.c  |   23 +++++++++++++++++++++--
 t/t5510-fetch.sh |    4 ++--
 2 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index e295d97..9d481f8 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -735,10 +735,29 @@ static int do_fetch(struct transport *transport,
 		return 1;
 	}
 	if (prune) {
-		if (ref_count)
+		/* If --tags was specified, pretend the user gave us the canonical tags refspec */
+		if (tags == TAGS_SET) {
+			const char *tags_str = "refs/tags/*:refs/tags/*";
+			struct refspec *tags_refspec, *refspec;
+
+			/* Copy the refspec and add the tags to it */
+			refspec = xcalloc(ref_count + 1, sizeof(struct refspec));
+			tags_refspec = parse_fetch_refspec(1, &tags_str);
+			memcpy(refspec, refs, ref_count * sizeof(struct refspec));
+			memcpy(&refspec[ref_count], tags_refspec, sizeof(struct refspec));
+			ref_count++;
+
+			prune_refs(refspec, ref_count, ref_map);
+
+			ref_count--;
+			/* The rest of the strings belong to fetch_one */
+			free_refspec(1, tags_refspec);
+			free(refspec);
+		} else if (ref_count) {
 			prune_refs(refs, ref_count, ref_map);
-		else
+		} else {
 			prune_refs(transport->remote->fetch, transport->remote->fetch_refspec_nr, ref_map);
+		}
 	}
 	free_refs(ref_map);
 
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 581049b..e0af4c4 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -105,7 +105,7 @@ test_expect_success 'fetch --prune with a namespace keeps other namespaces' '
 	git rev-parse origin/master
 '
 
-test_expect_failure 'fetch --prune --tags does not delete the remote-tracking branches' '
+test_expect_success 'fetch --prune --tags does not delete the remote-tracking branches' '
 	cd "$D" &&
 	git clone . prune-tags &&
 	cd prune-tags &&
@@ -116,7 +116,7 @@ test_expect_failure 'fetch --prune --tags does not delete the remote-tracking br
 	test_must_fail git rev-parse somebranch
 '
 
-test_expect_failure 'fetch --prune --tags with branch does not delete other remote-tracking branches' '
+test_expect_success 'fetch --prune --tags with branch does not delete other remote-tracking branches' '
 	cd "$D" &&
 	git clone . prune-tags-branch &&
 	cd prune-tags-branch &&
-- 
1.7.5.2.354.g349bf

^ permalink raw reply related

* [PATCH 3/5] remote: separate out the remote_find_tracking logic into query_refspecs
From: Carlos Martín Nieto @ 2011-10-15  5:04 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Junio C Hamano, mathstuf
In-Reply-To: <1318655066-29001-1-git-send-email-cmn@elego.de>

Move the body of remote_find_tracking to a new function query_refspecs
which does the same (find a refspec that matches and apply the
transformation) but explicitely wants the list of refspecs.

Make remote_find_tracking and apply_refspecs use query_refspecs.

Signed-off-by: Carlos Martín Nieto <cmn@elego.de>
---
 remote.c |   70 ++++++++++++++++++++++++++++++-------------------------------
 1 files changed, 34 insertions(+), 36 deletions(-)

diff --git a/remote.c b/remote.c
index b8ecfa5..e94c6d2 100644
--- a/remote.c
+++ b/remote.c
@@ -828,59 +828,57 @@ static int match_name_with_pattern(const char *key, const char *name,
 	return ret;
 }
 
-char *apply_refspecs(struct refspec *refspecs, int nr_refspec,
-		     const char *name)
+static int query_refspecs(struct refspec *refs, int ref_count, struct refspec *query)
 {
 	int i;
-	char *ret = NULL;
-	for (i = 0; i < nr_refspec; i++) {
-		struct refspec *refspec = refspecs + i;
-		if (refspec->pattern) {
-			if (match_name_with_pattern(refspec->src, name,
-						    refspec->dst, &ret))
-				return ret;
-		} else if (!strcmp(refspec->src, name))
-			return strdup(refspec->dst);
-	}
-	return NULL;
-}
+	int find_src = query->src == NULL;
 
-int remote_find_tracking(struct remote *remote, struct refspec *refspec)
-{
-	int find_src = refspec->src == NULL;
-	char *needle, **result;
-	int i;
+	if (find_src && !query->dst)
+		return error("query_refspecs: need either src or dst");
 
-	if (find_src) {
-		if (!refspec->dst)
-			return error("find_tracking: need either src or dst");
-		needle = refspec->dst;
-		result = &refspec->src;
-	} else {
-		needle = refspec->src;
-		result = &refspec->dst;
-	}
+	for (i = 0; i < ref_count; i++) {
+		struct refspec *refspec = &refs[i];
+		const char *key = find_src ? refspec->dst : refspec->src;
+		const char *value = find_src ? refspec->src : refspec->dst;
+		const char *needle = find_src ? query->dst : query->src;
+		char **result = find_src ? &query->src : &query->dst;
 
-	for (i = 0; i < remote->fetch_refspec_nr; i++) {
-		struct refspec *fetch = &remote->fetch[i];
-		const char *key = find_src ? fetch->dst : fetch->src;
-		const char *value = find_src ? fetch->src : fetch->dst;
-		if (!fetch->dst)
+		if (!refspec->dst)
 			continue;
-		if (fetch->pattern) {
+		if (refspec->pattern) {
 			if (match_name_with_pattern(key, needle, value, result)) {
-				refspec->force = fetch->force;
+				query->force = refspec->force;
 				return 0;
 			}
 		} else if (!strcmp(needle, key)) {
 			*result = xstrdup(value);
-			refspec->force = fetch->force;
+			query->force = refspec->force;
 			return 0;
 		}
 	}
+
 	return -1;
 }
 
+char *apply_refspecs(struct refspec *refspecs, int nr_refspec,
+		     const char *name)
+{
+	struct refspec query;
+
+	memset(&query, 0x0, sizeof(struct refspec));
+	query.src = (char *) name;
+
+	if (query_refspecs(refspecs, nr_refspec, &query))
+		return NULL;
+
+	return query.dst;
+}
+
+int remote_find_tracking(struct remote *remote, struct refspec *refspec)
+{
+	return query_refspecs(remote->fetch, remote->fetch_refspec_nr, refspec);
+}
+
 static struct ref *alloc_ref_with_prefix(const char *prefix, size_t prefixlen,
 		const char *name)
 {
-- 
1.7.5.2.354.g349bf

^ permalink raw reply related

* [PATCH 1/5] fetch: free all the additional refspecs
From: Carlos Martín Nieto @ 2011-10-15  5:04 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Junio C Hamano, mathstuf
In-Reply-To: <1318655066-29001-1-git-send-email-cmn@elego.de>

Signed-off-by: Carlos Martín Nieto <cmn@elego.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/fetch.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index e422ced..605d1bf 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -918,7 +918,7 @@ static int fetch_one(struct remote *remote, int argc, const char **argv)
 	atexit(unlock_pack);
 	refspec = parse_fetch_refspec(ref_nr, refs);
 	exit_code = do_fetch(transport, refspec, ref_nr);
-	free(refspec);
+	free_refspec(ref_nr, refspec);
 	transport_disconnect(transport);
 	transport = NULL;
 	return exit_code;
-- 
1.7.5.2.354.g349bf

^ permalink raw reply related

* [PATCH 2/5] t5510: add tests for fetch --prune
From: Carlos Martín Nieto @ 2011-10-15  5:04 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Junio C Hamano, mathstuf
In-Reply-To: <1318655066-29001-1-git-send-email-cmn@elego.de>

The failures will be fixed in later commits.

Signed-off-by: Carlos Martín Nieto <cmn@elego.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t5510-fetch.sh |   50 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 50 insertions(+), 0 deletions(-)

diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 7e433b1..8b5e925 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -76,6 +76,56 @@ test_expect_success "fetch test for-merge" '
 	cut -f -2 .git/FETCH_HEAD >actual &&
 	test_cmp expected actual'
 
+test_expect_success 'fetch --prune on its own works as expected' '
+	cd "$D" &&
+	git clone . prune &&
+	cd prune &&
+	git fetch origin refs/heads/master:refs/remotes/origin/extrabranch &&
+
+	git fetch --prune origin &&
+	test_must_fail git rev-parse origin/extrabranch
+'
+
+test_expect_failure 'fetch --prune with a branch name keeps branches' '
+	cd "$D" &&
+	git clone . prune-branch &&
+	cd prune-branch &&
+	git fetch origin refs/heads/master:refs/remotes/origin/extrabranch &&
+
+	git fetch --prune origin master &&
+	git rev-parse origin/extrabranch
+'
+
+test_expect_failure 'fetch --prune with a namespace keeps other namespaces' '
+	cd "$D" &&
+	git clone . prune-namespace &&
+	cd prune-namespace &&
+
+	git fetch --prune origin refs/heads/a/*:refs/remotes/origin/a/* &&
+	git rev-parse origin/master
+'
+
+test_expect_failure 'fetch --prune --tags does not delete the remote-tracking branches' '
+	cd "$D" &&
+	git clone . prune-tags &&
+	cd prune-tags &&
+	git fetch origin refs/heads/master:refs/tags/sometag &&
+
+	git fetch --prune --tags origin &&
+	git rev-parse origin/master &&
+	test_must_fail git rev-parse somebranch
+'
+
+test_expect_failure 'fetch --prune --tags with branch does not delete other remote-tracking branches' '
+	cd "$D" &&
+	git clone . prune-tags-branch &&
+	cd prune-tags-branch &&
+	git fetch origin refs/heads/master:refs/remotes/origin/extrabranch &&
+
+	git fetch --prune --tags origin master &&
+	git rev-parse origin/extrabranch
+'
+
 test_expect_success 'fetch tags when there is no tags' '
 
     cd "$D" &&
-- 
1.7.5.2.354.g349bf

^ permalink raw reply related

* Re: [PATCH 2/8] t1402: Ignore a few cases that must fail due to DOS path expansion
From: Junio C Hamano @ 2011-10-15  5:36 UTC (permalink / raw)
  To: Pat Thoyts; +Cc: Git, msysGit, Johannes Schindelin
In-Reply-To: <1318632815-29945-3-git-send-email-patthoyts@users.sourceforge.net>

Pat Thoyts <patthoyts@users.sourceforge.net> writes:

>  t/t1402-check-ref-format.sh |   15 +++++++++------

Didn't we see a different patch that attempts to address the same issue
recently on the list from J6t, or is this a fix for a different problem?

>  1 files changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/t/t1402-check-ref-format.sh b/t/t1402-check-ref-format.sh
> index 710fcca..1a5e343 100755
> --- a/t/t1402-check-ref-format.sh
> +++ b/t/t1402-check-ref-format.sh
> @@ -36,7 +36,7 @@ invalid_ref 'refs///heads/foo'
>  valid_ref 'refs///heads/foo' --normalize
>  invalid_ref 'heads/foo/'
>  invalid_ref '/heads/foo'
> -valid_ref '/heads/foo' --normalize
> +test_have_prereq MINGW || valid_ref '/heads/foo' --normalize
>  invalid_ref '///heads/foo'
>  valid_ref '///heads/foo' --normalize
>  invalid_ref './foo'
> @@ -120,9 +120,12 @@ invalid_ref "$ref" --allow-onelevel
>  invalid_ref "$ref" --refspec-pattern
>  invalid_ref "$ref" '--refspec-pattern --allow-onelevel'
>  invalid_ref "$ref" --normalize
> -valid_ref "$ref" '--allow-onelevel --normalize'
> -invalid_ref "$ref" '--refspec-pattern --normalize'
> -valid_ref "$ref" '--refspec-pattern --allow-onelevel --normalize'
> +if test_have_prereq NOT_MINGW
> +then
> +	valid_ref "$ref" '--allow-onelevel --normalize'
> +	invalid_ref "$ref" '--refspec-pattern --normalize'
> +	valid_ref "$ref" '--refspec-pattern --allow-onelevel --normalize'
> +fi
>  
>  test_expect_success "check-ref-format --branch @{-1}" '
>  	T=$(git write-tree) &&
> @@ -166,10 +169,10 @@ invalid_ref_normalized() {
>  
>  valid_ref_normalized 'heads/foo' 'heads/foo'
>  valid_ref_normalized 'refs///heads/foo' 'refs/heads/foo'
> -valid_ref_normalized '/heads/foo' 'heads/foo'
> +test_have_prereq MINGW || valid_ref_normalized '/heads/foo' 'heads/foo'
>  valid_ref_normalized '///heads/foo' 'heads/foo'
>  invalid_ref_normalized 'foo'
> -invalid_ref_normalized '/foo'
> +test_have_prereq MINGW || invalid_ref_normalized '/foo'
>  invalid_ref_normalized 'heads/foo/../bar'
>  invalid_ref_normalized 'heads/./foo'
>  invalid_ref_normalized 'heads\foo'

^ permalink raw reply

* Re: [PATCH 5/8] t9901: fix line-ending dependency on windows
From: Junio C Hamano @ 2011-10-15  5:44 UTC (permalink / raw)
  To: Pat Thoyts; +Cc: Git, msysGit
In-Reply-To: <1318632815-29945-6-git-send-email-patthoyts@users.sourceforge.net>

Pat Thoyts <patthoyts@users.sourceforge.net> writes:

> Signed-off-by: Pat Thoyts <patthoyts@users.sourceforge.net>
> ---
>  t/t9901-git-web--browse.sh |   10 +++++-----
>  1 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/t/t9901-git-web--browse.sh b/t/t9901-git-web--browse.sh
> index 7906e5d..1185b42 100755
> --- a/t/t9901-git-web--browse.sh
> +++ b/t/t9901-git-web--browse.sh
> @@ -12,7 +12,7 @@ test_expect_success \
>  	echo http://example.com/foo\&bar >expect &&
>  	git config browser.custom.cmd echo &&
>  	git web--browse --browser=custom \
> -		http://example.com/foo\&bar >actual &&
> +		http://example.com/foo\&bar | tr -d "\r" >actual &&
>  	test_cmp expect actual
>  '

This is unnice for two reasons. We have web--browse five times in this
test script, and you add tr exactly the same way to each and every one of
them. And you are losing the error condition from each and every one of
these web--browse invocations.

Having to do the same change to all invocations of the same command
suggests that perhaps you can and should wrap that pattern into a single
helper, perhaps like:

test_web_browse () {
	# browser=$1 url=$2
	git web--browse --browser="$1" "$2" >actual &&
        tr -d '\015' <actual >text &&
        test_cmp expect text
}

or something?

^ permalink raw reply

* Re: A note from the maintainer
From: Martin von Zweigbergk @ 2011-10-15  5:47 UTC (permalink / raw)
  To: Junio C Hamano, Paul Mackerras; +Cc: git
In-Reply-To: <7vr52s9ny5.fsf@alter.siamese.dyndns.org>

On Tue, Oct 4, 2011 at 7:22 PM, Junio C Hamano <gitster@pobox.com> wrote:
>  - gitk-git/ comes from Paul Mackerras's gitk project:
>
>        git://git.kernel.org/pub/scm/gitk/gitk.git

I don't seem to be able to fetch from there. Is it still supposed to be there?

Martin

^ permalink raw reply

* Re: [PATCH 7/8] mergetools: use the correct tool for Beyond Compare 3 on Windows
From: Junio C Hamano @ 2011-10-15  5:50 UTC (permalink / raw)
  To: Pat Thoyts; +Cc: Git, msysGit
In-Reply-To: <1318632815-29945-8-git-send-email-patthoyts@users.sourceforge.net>

Pat Thoyts <patthoyts@users.sourceforge.net> writes:

> On Windows the bcompare tool launches a graphical program and does
> not wait for it to terminate. A separate 'bcomp' tool is provided which
> will wait for the view to exit so we use this instead.

Hmm, does this only apply to Windows, or are there other platforms on
which BC3 supplies bcomp for the exact same reason? What I am trying to
get at is that it might be nicer if we do not have to check uname, e.g.

	if type bcomp >/dev/null 2>/dev/null
        then
        	echo bcomp
	else
        	echo bcompare
	fi

> Reported-by: Werner BEROUX <werner@beroux.com>
> Signed-off-by: Pat Thoyts <patthoyts@users.sourceforge.net>
> ---
>  mergetools/bc3 |    9 ++++++++-
>  1 files changed, 8 insertions(+), 1 deletions(-)
>
> diff --git a/mergetools/bc3 b/mergetools/bc3
> index 27b3dd4..b642bf2 100644
> --- a/mergetools/bc3
> +++ b/mergetools/bc3
> @@ -16,5 +16,12 @@ merge_cmd () {
>  }
>  
>  translate_merge_tool_path() {
> -	echo bcompare
> +	case $(uname -s) in
> +	*MINGW*)
> +		echo bcomp
> +		;;
> +	*)
> +		echo bcompare
> +		;;
> +	esac
>  }

^ permalink raw reply

* Re: [PATCHv3] daemon: give friendlier error messages to clients
From: Junio C Hamano @ 2011-10-15  5:55 UTC (permalink / raw)
  To: Sitaram Chamarty
  Cc: Jeff King, Jonathan Nieder, Nguyen Thai Ngoc Duy, git,
	Ilari Liusvaara, Johannes Sixt
In-Reply-To: <CAMK1S_g0aKUa=+ndAm7rqeoPAobjVb6oJ1Z4DqSeNrdauXNH3w@mail.gmail.com>

Sitaram Chamarty <sitaramc@gmail.com> writes:

>> Because there is a risk of leaking information about
>> non-exported repositories, by default all errors simply say
>> "access denied". Sites which don't have hidden repositories,
>
> I suggest that even the "secure" version of the message say something
> like "access denied or repository not exported".  You're still not
> leaking anything, but it reduces confusion to the user, who otherwise
> may not realise it *could be* the latter.

I kind of like the suggestion, but I am afraid that "access denied,
repository nonexistent or not exported" can soon easily get long enough to
be unmanageable.

^ permalink raw reply

* Re: [PATCHv3] daemon: give friendlier error messages to clients
From: Sitaram Chamarty @ 2011-10-15  7:09 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Jeff King, Jonathan Nieder, Nguyen Thai Ngoc Duy, git,
	Ilari Liusvaara, Johannes Sixt
In-Reply-To: <7vk486x0hq.fsf@alter.siamese.dyndns.org>

On Sat, Oct 15, 2011 at 11:25 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Sitaram Chamarty <sitaramc@gmail.com> writes:
>
>>> Because there is a risk of leaking information about
>>> non-exported repositories, by default all errors simply say
>>> "access denied". Sites which don't have hidden repositories,
>>
>> I suggest that even the "secure" version of the message say something
>> like "access denied or repository not exported".  You're still not
>> leaking anything, but it reduces confusion to the user, who otherwise
>> may not realise it *could be* the latter.
>
> I kind of like the suggestion, but I am afraid that "access denied,
> repository nonexistent or not exported" can soon easily get long enough to
> be unmanageable.

When someone who *does* have access makes a typo, "access denied"
makes it harder to realise it, because it subtly implies the repo
*does* exist and it's an ACL issue.  I've seen lots of frustrating
back-and-forth between admin and user before someone eventually
noticed the typo.

"Access denied or no such repo" is much better.  (The "not exported"
nuance is not relevant in this context; you can safely ignore it.)

regards

-- 
Sitaram

^ permalink raw reply

* Re: [PATCHv3] daemon: give friendlier error messages to clients
From: Jakub Narebski @ 2011-10-15  8:16 UTC (permalink / raw)
  To: Sitaram Chamarty
  Cc: Junio C Hamano, Jeff King, Jonathan Nieder, Nguyen Thai Ngoc Duy,
	git, Ilari Liusvaara, Johannes Sixt
In-Reply-To: <CAMK1S_gkB49qhnt8U=3G3UPnjo2vzFx5mL4cOM1Ubu68ySJrDA@mail.gmail.com>

Sitaram Chamarty <sitaramc@gmail.com> writes:
> On Sat, Oct 15, 2011 at 11:25 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Sitaram Chamarty <sitaramc@gmail.com> writes:
>>
>>>> Because there is a risk of leaking information about
>>>> non-exported repositories, by default all errors simply say
>>>> "access denied". Sites which don't have hidden repositories,
>>>
>>> I suggest that even the "secure" version of the message say something
>>> like "access denied or repository not exported".  You're still not
>>> leaking anything, but it reduces confusion to the user, who otherwise
>>> may not realise it *could be* the latter.
>>
>> I kind of like the suggestion, but I am afraid that "access denied,
>> repository nonexistent or not exported" can soon easily get long enough to
>> be unmanageable.
> 
> When someone who *does* have access makes a typo, "access denied"
> makes it harder to realise it, because it subtly implies the repo
> *does* exist and it's an ACL issue.  I've seen lots of frustrating
> back-and-forth between admin and user before someone eventually
> noticed the typo.
> 
> "Access denied or no such repo" is much better.  (The "not exported"
> nuance is not relevant in this context; you can safely ignore it.)

To join this bike-shedding:

  "Access denied or repository not available"

or just

  "Repository not available"

-- 
Jakub Narębski

^ permalink raw reply

* Re: [PATCHv3] daemon: give friendlier error messages to clients
From: Jonathan Nieder @ 2011-10-15  8:26 UTC (permalink / raw)
  To: Jakub Narebski
  Cc: Sitaram Chamarty, Junio C Hamano, Jeff King, Nguyen Thai Ngoc Duy,
	git, Ilari Liusvaara, Johannes Sixt
In-Reply-To: <m3r52e7js7.fsf@localhost.localdomain>

Jakub Narebski wrote:
> Sitaram Chamarty <sitaramc@gmail.com> writes:

>> "Access denied or no such repo" is much better.  (The "not exported"
>> nuance is not relevant in this context; you can safely ignore it.)
>
> To join this bike-shedding:
>
>   "Access denied or repository not available"
>
> or just
>
>   "Repository not available"

If such details about the message matter, then I have to say that
Sitaram's "access denied or no such repository" is as close to perfect
as I can imagine.  The admin who is eventually forwarded this message
will be reminded to check two things:

 - that access is not denied, neither globally, using a whitelist,
   using filesystem permissions, nor by leaving out
   git-daemon-export-ok, and

 - that such a repo exists at all, and there was not a typo in the
   address.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox