Git development
 help / color / mirror / Atom feed
* Re: [PATCH] Show submodules as modified when they contain a dirty work tree
From: Jens Lehmann @ 2010-01-14 21:38 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Git Mailing List, Johannes Schindelin, Shawn O. Pearce,
	Heiko Voigt, Lars Hjemli
In-Reply-To: <7v6375lkpj.fsf@alter.siamese.dyndns.org>

Am 13.01.2010 23:10, schrieb Junio C Hamano:
> And a patch to add:
> 
>>> * It doesn't give detailed output when doing a "git diff* -p" with or
>>>   without the --submodule option. It should show something like
>>>
>>>     diff --git a/sub b/sub
>>>     index 5431f52..3f35670 160000
>>>     --- a/sub
>>>     +++ b/sub
>>>     @@ -1 +1 @@
>>>     -Subproject commit 5431f529197f3831cdfbba1354a819a79f948f6f
>>>     +Subproject commit 3f356705649b5d566d97ff843cf193359229a453-dirty
>>>
> 
> would look like the attached.

Your patch did not show submodules as dirty when the refs were identical.
The following patch fixes that and extends the test to catch that too.

-- >8 --
Subject: Show a modified submodule directory as dirty even if the refs match

When the submodules HEAD and the ref committed in the HEAD of the
superproject were the same, "git diff[-index] HEAD" did not show the
submodule as dirty when it should.

Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---
 diff-lib.c                |    3 ++-
 t/t4027-diff-submodule.sh |   35 +++++++++++++++++++++++++++++++++++
 2 files changed, 37 insertions(+), 1 deletions(-)

diff --git a/diff-lib.c b/diff-lib.c
index 5ce226b..9cdf6da 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -233,7 +233,8 @@ static int get_stat_data(struct cache_entry *ce,
 			return -1;
 		}
 		changed = ce_match_stat(ce, &st, 0);
-		if (changed) {
+		if (changed
+		    || (S_ISGITLINK(ce->ce_mode) && is_submodule_modified(ce->name))) {
 			mode = ce_mode_from_stat(ce, st.st_mode);
 			sha1 = null_sha1;
 		}
diff --git a/t/t4027-diff-submodule.sh b/t/t4027-diff-submodule.sh
index bf8c980..83c1914 100755
--- a/t/t4027-diff-submodule.sh
+++ b/t/t4027-diff-submodule.sh
@@ -97,6 +97,41 @@ test_expect_success 'git diff HEAD with dirty submodule (untracked)' '
 	test_cmp expect.body actual.body
 '

+test_expect_success 'git diff HEAD with dirty submodule (work tree, refs match)' '
+	git commit -m "x" sub &&
+	echo >>sub/world &&
+	git diff HEAD >actual &&
+	sed -e "1,/^@@/d" actual >actual.body &&
+	expect_from_to >expect.body $subprev $subprev-dirty &&
+	test_cmp expect.body actual.body
+'
+
+test_expect_success 'git diff HEAD with dirty submodule (index, refs match)' '
+	(
+		cd sub &&
+		git reset --hard &&
+		echo >>world &&
+		git add world
+	) &&
+	git diff HEAD >actual &&
+	sed -e "1,/^@@/d" actual >actual.body &&
+	expect_from_to >expect.body $subprev $subprev-dirty &&
+	test_cmp expect.body actual.body
+'
+
+test_expect_success 'git diff HEAD with dirty submodule (untracked, refs match)' '
+	(
+		cd sub &&
+		git reset --hard &&
+		git clean -qfdx &&
+		>cruft
+	) &&
+	git diff HEAD >actual &&
+	sed -e "1,/^@@/d" actual >actual.body &&
+	expect_from_to >expect.body $subprev $subprev-dirty &&
+	test_cmp expect.body actual.body
+'
+
 test_expect_success 'git diff (empty submodule dir)' '
 	: >empty &&
 	rm -rf sub/* sub/.git &&
-- 
1.6.6.294.g1f7f2.dirty

^ permalink raw reply related

* git checkout -f: What am I missing?
From: Soham Mehta @ 2010-01-14 21:16 UTC (permalink / raw)
  To: git

Hi,

I have a situation with git that I'm trying to understand:

Description:
1) GIT_DIR is set to /path/to/repo/.git
2) Repository is /not /a bare repo, and all files are nicely checked-out 
in /path/to/repo/
3) Somebody pushes to that repo using ssh (any branch, checked-out or not)
4) Default post-receive hook runs (it is the only one +x) which sends 
out an email 
(http://repo.or.cz/w/git.git/blob/HEAD:/contrib/hooks/post-receive-email)
5) After it is done sending the email, I put "git checkout -f", at the 
end in the same file, in case someone pushes to a checked-out branch

Problem:
It runs "checkout -f" as if inside .git directory, instead of on the 
parent. i.e. it gets all files from the parent and writes them inside 
.git. Parent is left untouched.

Some more info:
0) We don't have GIT_DIR set in the environment. The hook does a 
rev-parse to find it.
1) echo of $GIT_DIR right before the checkout -f line gives a "." .
2) It works as expected if I do this:  cd /path/to/repo && git 
--git-dir=/path/to/repo/.git/ checkout -f

What I do know:
1) Pushing to a checked-out branch is not a git best-practice, and some 
git behavior is undefined in that case. We already have plans to go away 
from that.
2) Git tends to like full path names instead of relative ones

Can someone help me understand this behavior?

Thanks.
-Soham

^ permalink raw reply

* Re: [PATCH 3/4] start_command: detect execvp failures early
From: Johannes Sixt @ 2010-01-14 21:53 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Ilari Liusvaara, git
In-Reply-To: <7viqb49xwb.fsf@alter.siamese.dyndns.org>

On Donnerstag, 14. Januar 2010, Junio C Hamano wrote:
> Johannes Sixt <j6t@kdbg.org> writes:
> > Previously, failures during execvp could be detected only by
> > finish_command. However, in some situations it is beneficial for the
> > parent process to know earlier that the child process will not run.
> >
> > The idea to use a pipe to signal failures to the parent process and
> > the test case were lifted from patches by Ilari Liusvaara.
>
> I wonder if we can do this without pipe, perhaps using "vfork, exec, then
> update a variable"....

Except that some systems implement vfork(2) in terms of fork(2), I heard. 
Moreover, I think that we do way too many things before the exec; isn't that 
dangerous?

-- Hannes

^ permalink raw reply

* Re: [PATCH] git push --track
From: Nanako Shiraishi @ 2010-01-14 22:27 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Rudolf Polzer, git
In-Reply-To: <alpine.DEB.1.00.1001141130210.4985@pacific.mpi-cbg.de>

Quoting Johannes Schindelin <Johannes.Schindelin@gmx.de>

> On Thu, 14 Jan 2010, Nanako Shiraishi wrote:
>
>> Quoting Rudolf Polzer <divVerent@alientrap.org>
>> 
>> > I'd like a feature to automatically "transform" a non-tracking local
>> > branch into a tracking branch on push. A patch to do that is attached.
>> 
>> How well does this take earlier discussions on the same topic into account? For example, did you study the design discussion in
>>   http://thread.gmane.org/gmane.comp.version-control.git/135325/focus=135390
>
> Thank you for looking up that reference.
>
> Do you remember what the outcome was?

I summarized it when I reminded Junio on this topic last time and it is in the same discussion thread: http://thread.gmane.org/gmane.comp.version-control.git/135325/focus=136216

Here is an extended version.

'git branch --will-track origin/topic topic origin/master' was proposed as a way to fork a topic branch at origin's master. Later 'git pull' will merge topic from origin to topic.

This is bad in two ways. It will force users to decide when the branch is created. It will not allow users to configure branch.topic.rebase variable.

'git push --track' was suggested as a way to let users delay that decision.

'git branch --configure' to update the same information for an existing branch was suggested as an alternative UI. An added benefit is that this approach will allow the same option to be used when creating a branch.

'git pull --remember' that remembers the options used from the command line was suggested as a solution in addition to 'git branch --reconfigure'. Users can postpone the decision even more than 'git push --track', and it naturally supports setting branch.topic.rebase with 'git pull --rebase --remember'.  It also has two additional benefits. 'push --track' configures what happens when you 'pull' (counter-intuitive), but 'pull --remember' makes 'pull' to change the setting used by 'pull' (much more natural). Also it does not add the confusing word 'track' to the interface (for a more detailed discussion on 'track', see http://article.gmane.org/gmane.comp.version-control.git/136785).

-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

^ permalink raw reply

* Re: [PATCH 3/4] start_command: detect execvp failures early
From: Junio C Hamano @ 2010-01-14 22:40 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Ilari Liusvaara, git
In-Reply-To: <201001142253.19595.j6t@kdbg.org>

Johannes Sixt <j6t@kdbg.org> writes:

> On Donnerstag, 14. Januar 2010, Junio C Hamano wrote:
>> Johannes Sixt <j6t@kdbg.org> writes:
>> > Previously, failures during execvp could be detected only by
>> > finish_command. However, in some situations it is beneficial for the
>> > parent process to know earlier that the child process will not run.
>> >
>> > The idea to use a pipe to signal failures to the parent process and
>> > the test case were lifted from patches by Ilari Liusvaara.
>>
>> I wonder if we can do this without pipe, perhaps using "vfork, exec, then
>> update a variable"....
>
> Except that some systems implement vfork(2) in terms of fork(2), I heard. 

As long as they implement it correctly, we don't care if it is implemented
in terms of fork or knife or chopstick.  The only thing we care about in
this crazy-idea was the shared address space the child can write into
until it exec or _exit while the parent is in a frozen state.

> Moreover, I think that we do way too many things before the exec; isn't that 
> dangerous?

Yeah, we do seem to do quite a lot more than I thought.  Scratch that
crazy idea, then.

^ permalink raw reply

* [PATCH RESEND2] git gui: Use git diff --submodule when available
From: Jens Lehmann @ 2010-01-14 22:41 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Git Mailing List, Junio C Hamano

Changed the use of submodule summary to diff --submodule because the
implementation in C is much faster than the submodule script. Also a test
has been added to make sure that the underlying git supports the diff
option --submodule (which was introduced in 1.6.6).

Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---

Did not hear anything about my last resend on December 28th, so i try
again.

To explain what i mean by "much faster", here are the numbers (best of
three) for a small repo with a changed submodule (git checkout HEAD^)
under Linux:


time git submodule summary
<snip>

real	0m0.219s
user	0m0.050s
sys	0m0.111s


time git diff --submodule
<snip>

real	0m0.012s
user	0m0.003s
sys	0m0.009s


In my experience this patch changes the user experience from
"a noticeable delay" to "instantaneous".


 git-gui/lib/diff.tcl |   21 +++++++++------------
 1 files changed, 9 insertions(+), 12 deletions(-)

diff --git a/git-gui/lib/diff.tcl b/git-gui/lib/diff.tcl
index bd5d189..0623e3e 100644
--- a/git-gui/lib/diff.tcl
+++ b/git-gui/lib/diff.tcl
@@ -281,6 +281,14 @@ proc start_show_diff {cont_info {add_opts {}}} {
 		}
 	}

+	if {[git-version >= "1.6.6"]} {
+		if {[string match {160000 *} [lindex $s 2]]
+		    || [string match {160000 *} [lindex $s 3]]} {
+			set is_submodule_diff 1
+			lappend cmd --submodule
+		}
+	}
+
 	lappend cmd -p
 	lappend cmd --no-color
 	if {$repo_config(gui.diffcontext) >= 1} {
@@ -296,16 +304,6 @@ proc start_show_diff {cont_info {add_opts {}}} {
 		lappend cmd $path
 	}

-	if {[string match {160000 *} [lindex $s 2]]
-        || [string match {160000 *} [lindex $s 3]]} {
-		set is_submodule_diff 1
-		if {$w eq $ui_index} {
-			set cmd [list submodule summary --cached -- $path]
-		} else {
-			set cmd [list submodule summary --files -- $path]
-		}
-	}
-
 	if {[catch {set fd [eval git_read --nice $cmd]} err]} {
 		set diff_active 0
 		unlock_index
@@ -387,8 +385,7 @@ proc read_diff {fd cont_info} {
 			}
 		} elseif {$is_submodule_diff} {
 			if {$line == ""} continue
-			if {[regexp {^\* } $line]} {
-				set line [string replace $line 0 1 {Submodule }]
+			if {[regexp {^Submodule } $line]} {
 				set tags d_@
 			} else {
 				set op [string range $line 0 2]
-- 
1.6.6.rc2.274.g2cee7

^ permalink raw reply related

* Re: [RFC 0/2] Git-over-TLS (gits://) client side support
From: Ilari Liusvaara @ 2010-01-14 23:08 UTC (permalink / raw)
  To: Avery Pennarun; +Cc: Andreas Krey, Nguyen Thai Ngoc Duy, git
In-Reply-To: <32541b131001141246o1f5ce816gc4a26b81343aaa2d@mail.gmail.com>

On Thu, Jan 14, 2010 at 03:46:56PM -0500, Avery Pennarun wrote:
> On Thu, Jan 14, 2010 at 3:51 AM, Ilari Liusvaara
> <ilari.liusvaara@elisanet.fi> wrote:

> > The client tries only one auth method instead of potentially trying
> > multiple. Witness the 'use verbose mode and check if it uses the key'
> > type stuff.
> 
> I believe this is a limitation of the client, not of the protocol.  So
> a patch to the ssh client could fix this.

This is also about interfaces to user. It effectively can't be patched.

<forking OpenSSH>

Get real.

> > And if you host the repo system too, you would get second key anyway
> > (and SSH is not too good at handling multiple keys).
> 
> I'm not really sure about this.  ssh-add seems pretty easy.
 
Anyway, two SSH keys in interactive use means which to use has to be selected,
one doesn't.

-Ilari

^ permalink raw reply

* Re: [PATCH] Show submodules as modified when they contain a dirty work tree
From: Junio C Hamano @ 2010-01-14 23:13 UTC (permalink / raw)
  To: Jens Lehmann
  Cc: Git Mailing List, Johannes Schindelin, Shawn O. Pearce,
	Heiko Voigt, Lars Hjemli
In-Reply-To: <4B4F8EF1.3080709@web.de>

Jens Lehmann <Jens.Lehmann@web.de> writes:

> Subject: Show a modified submodule directory as dirty even if the refs match
>
> When the submodules HEAD and the ref committed in the HEAD of the
> superproject were the same, "git diff[-index] HEAD" did not show the
> submodule as dirty when it should.
>
> Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
> ---
>  diff-lib.c                |    3 ++-
>  t/t4027-diff-submodule.sh |   35 +++++++++++++++++++++++++++++++++++
>  2 files changed, 37 insertions(+), 1 deletions(-)
>
> diff --git a/diff-lib.c b/diff-lib.c
> index 5ce226b..9cdf6da 100644
> --- a/diff-lib.c
> +++ b/diff-lib.c
> @@ -233,7 +233,8 @@ static int get_stat_data(struct cache_entry *ce,
>  			return -1;
>  		}
>  		changed = ce_match_stat(ce, &st, 0);
> -		if (changed) {
> +		if (changed
> +		    || (S_ISGITLINK(ce->ce_mode) && is_submodule_modified(ce->name))) {

You had a check in your previous patch that decides to call or skip
diff_change() based on is_submodule_modified() for diff-files, but forgot
to have the same for diff-index, which this patch does.  Perhaps we want
to squash this into 4519d9c (Show submodules as modified when they contain
a dirty work tree, 2010-01-13).

The existing code is a bit unfortunate; by the time we come to the output
routine, the information we found from is_submodule_modified() is lost;
that is why my "would look like this" patch calls is_submodule_modified().

We may want to add one parameter to diff_change() and diff_addremove(), to
tell them if the work-tree side (if we are comparing something with the
work tree) is a modified submodule, and add one bit to the diff_filespec
structure to record that in diff_change() and diff_addremove() (obviously
only when adding).  That way, my "would looks like this" patch needs to
check the result of is_submodule_modified() the front-ends left in the
filespec, instead of running it again.

^ permalink raw reply

* Re: [PATCH] git push --track
From: Junio C Hamano @ 2010-01-14 23:13 UTC (permalink / raw)
  To: Rudolf Polzer
  Cc: Tay Ray Chuan, Ilari Liusvaara, Matthieu Moy, Miles Bader,
	Johannes Schindelin, git
In-Reply-To: <20100114070000.GA1528@rm.endoftheinternet.org>

Rudolf Polzer <divVerent@alientrap.org> writes:

>> Each patch should be sent out in its own mail. (As Matthieu has
>> recommended, you should check out Documentation/SubmittingPatches.)
>
> So, using a newsreader is not accepted practice? Why is the mailing list on a
> newsgroup then?

I do read this list in a newsreader.  When I say "reply" or "follow-up",
the response is sent out to the list via e-mail.  Your newsreader should
be configurable in a similar way, I think.

^ permalink raw reply

* Re: Problem creating commits/trees with commit-tree/mktree
From: Gavin Beatty @ 2010-01-14 23:18 UTC (permalink / raw)
  To: Avery Pennarun; +Cc: git
In-Reply-To: <32541b131001111038m3dacaf72sc12f24aba8c60e@mail.gmail.com>

On Mon, Jan 11, 2010 at 19:38, Avery Pennarun <apenwarr@gmail.com> wrote:
> When I'm doing similar things, I often prefer just using a temporary
> git index file to keep track of my intermediate trees.  Just set
> GIT_INDEX_FILE to point at a temporary file; then you can use
> git-read-tree to read in an old tree, and git-update-index
> (particularly with the --stdin flag) to update it.  Then you can use
> git-write-tree to convert the temporary index into a real tree object.

Your suggestion works well.  git update-index --info-only -z
--index-info takes ls-tree -r -z style format and pre-existing
objects: just as I wanted.

Thanks, git-fast-import was too much for the quite simple git-related
code I need.

-- 
Gavin Beatty

SEMPER UBI SUB UBI

^ permalink raw reply

* [PATCH 1/2] Fix uninitialized variable in get_refs_via_rsync().
From: Richard Weinberger @ 2010-01-14 23:28 UTC (permalink / raw)
  To: git; +Cc: gitster

This fixes a crash when cloning via rsync://.

Signed-off-by: Richard Weinberger <richard@nod.at>
---
 transport.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/transport.c b/transport.c
index b5332c0..3b489b3 100644
--- a/transport.c
+++ b/transport.c
@@ -143,7 +143,7 @@ static const char *rsync_url(const char *url)
 static struct ref *get_refs_via_rsync(struct transport *transport, int 
for_push)
 {
 	struct strbuf buf = STRBUF_INIT, temp_dir = STRBUF_INIT;
-	struct ref dummy, *tail = &dummy;
+	struct ref dummy = {0}, *tail = &dummy;
 	struct child_process rsync;
 	const char *args[5];
 	int temp_dir_len;
-- 
1.6.5.7

^ permalink raw reply related

* [PATCH 2/2] Fix variable initialization in insert_packed_refs().
From: Richard Weinberger @ 2010-01-14 23:29 UTC (permalink / raw)
  To: git; +Cc: gitster

Signed-off-by: Richard Weinberger <richard@nod.at>
---
 transport.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/transport.c b/transport.c
index 3b489b3..ecc20a1 100644
--- a/transport.c
+++ b/transport.c
@@ -102,7 +102,7 @@ static void insert_packed_refs(const char *packed_refs, 
struct ref **list)
 		return;
 
 	for (;;) {
-		int cmp = cmp, len;
+		int cmp, len;
 
 		if (!fgets(buffer, sizeof(buffer), f)) {
 			fclose(f);
-- 
1.6.5.7

^ permalink raw reply related

* Re: [PATCH] git push --track
From: Junio C Hamano @ 2010-01-14 23:46 UTC (permalink / raw)
  To: Rudolf Polzer; +Cc: Tay Ray Chuan, git
In-Reply-To: <20100114070316.GC1528@rm.endoftheinternet.org>

Rudolf Polzer <divVerent@alientrap.org> writes:

> On Thu, Jan 14, 2010 at 09:27:26AM +0800, Tay Ray Chuan wrote:
>> before I put up my comments on the patch, I wonder if git-push is the
>> best place to add this feature, as git-push usually deals with
>> "pushing" data to another repo.
>> 
>> I think git-branch would be a better place to do this.
>
> I think git-branch can already do this: after pushing, you can do git
> branch -f --track origin/mybranch.
>
> But the goal of this is to postponing the decision to track to the push time,
> and adding as little as possible extra commands/options to do this.

Thinking about this again (when was the last time we discussed it?), I
like the "git branch -f" suggestion (modulo one small nit).  

Yes, "push --track" lets you postpone the decision; branching, working on
it, pushing it out _and_ _then_ using your "branch -f" trick will let you
postpone the decision even further.  And it doesn't add --track to the UI.

The small nit is that "branch -f --track me origin/me" will happily
overwrite "me", even when your "me" is not up to date with "origin/me",
losing commits.

Perhaps we could teach "branch --track me origin/me" (i.e. no "-f") not to
barf even when "me" exists, as long as "me" is a subset of "origin/me",
and treat it as a request to re-configure the upstream information for the
existing branch "me" and at the same time fast-forward it to "origin/me"?

^ permalink raw reply

* Re: [PATCH] git push --track
From: Junio C Hamano @ 2010-01-14 23:50 UTC (permalink / raw)
  To: Nanako Shiraishi; +Cc: Johannes Schindelin, Rudolf Polzer, git
In-Reply-To: <20100115072741.6117@nanako3.lavabit.com>

Nanako Shiraishi <nanako3@lavabit.com> writes:

> I summarized it when I reminded Junio on this topic last time and it is in the same discussion thread: http://thread.gmane.org/gmane.comp.version-control.git/135325/focus=136216

Please wrap your lines for readability.

> 'git pull --remember' that remembers...

Although I admit I was the one who suggested it, and I think that it is
the most natural way to do this from the end user's point of view, from
the implementation point of view, it is the most difficult one in its
current form.  "git pull" does not interpret refspecs and delegates all
the hard work to "git fetch".

^ permalink raw reply

* git clone against firewall
From: Sebastian Pipping @ 2010-01-14 23:45 UTC (permalink / raw)
  To: git

hello!


with a firewall blocking outgoing connections to port 9418 a

  git clone git://...

of git 1.6.6 seems to never return, i.e. loop forever.  in my rather
automated environment (gentoo's tool layman calling git) this behavior
is rather troublesome - i need some kind of abort-and-error instead:
what i'm trying to do is loop over a number of clone URL alternatives of
the same repository like ..

  git://git.overlays.gentoo.org/dev/dberkholz.git
  http://git.overlays.gentoo.org/gitroot/dev/dberkholz.git
  git+ssh://git@git.overlays.gentoo.org/dev/dberkholz.git

.. and stop at the first clone that returns without error.

are there means to make git fail in such a case or to apply a timout?
if not please consider adding a related commandline option to git-clone.

thank you.



sebastian

^ permalink raw reply

* Re: [spf:guess] Re: Bug? git-svn clone dies with "fatal: ambiguous argument '...': unknown revision or path not in the working tree."
From: Sam Vilain @ 2010-01-14 23:57 UTC (permalink / raw)
  To: Eric Wong; +Cc: Eric Hanchrow, git, Andrew Myrick
In-Reply-To: <20100113052958.GA23182@dcvr.yhbt.net>

Eric Wong wrote:
>> It chugged along happily for a while, but then died like this:
>>
>> fatal: ambiguous argument
>> '2d2df13977551168a54ffa9b706484242a58736a^..d038748d49a0de5802fe3c13f46d0e080d064290':
>>     
> [...]
> As we see below, d038748d49a0de5802fe3c13f46d0e080d064290 is a merge
> commit.  So I'll Cc Sam and Andrew on this since they know their way
> around the mergeinfo stuff far better than I do and will hopefully have
> some insight into things.
>
> Since it's probably related to the new mergeinfo handling, reverting to
> a version without it (1.6.5.7) might be the best way to go for now.
>   

I'm at a loss. I can't get my rev-list to say "ambiguous argument" when
I pass it a similar range (eg d4e1b47a9225^..a24a32dd on git.git). Why
does it matter that the d038748 commit is a merge commit?

Eric H, is this repository available publicly for me to test? I guess
it's possible that argument is not being passed to rev-list but to some
other command ... would be nice to be able to reproduce it.

Sam

^ permalink raw reply

* Re: [PATCH] Show submodules as modified when they contain a dirty work tree
From: Jens Lehmann @ 2010-01-15  0:24 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Git Mailing List, Johannes Schindelin, Shawn O. Pearce,
	Heiko Voigt, Lars Hjemli
In-Reply-To: <7v3a288em2.fsf@alter.siamese.dyndns.org>

Am 15.01.2010 00:13, schrieb Junio C Hamano:
> Jens Lehmann <Jens.Lehmann@web.de> writes:
> 
>> Subject: Show a modified submodule directory as dirty even if the refs match
>>
>> When the submodules HEAD and the ref committed in the HEAD of the
>> superproject were the same, "git diff[-index] HEAD" did not show the
>> submodule as dirty when it should.
>>
>> Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
>> ---
>>  diff-lib.c                |    3 ++-
>>  t/t4027-diff-submodule.sh |   35 +++++++++++++++++++++++++++++++++++
>>  2 files changed, 37 insertions(+), 1 deletions(-)
>>
>> diff --git a/diff-lib.c b/diff-lib.c
>> index 5ce226b..9cdf6da 100644
>> --- a/diff-lib.c
>> +++ b/diff-lib.c
>> @@ -233,7 +233,8 @@ static int get_stat_data(struct cache_entry *ce,
>>  			return -1;
>>  		}
>>  		changed = ce_match_stat(ce, &st, 0);
>> -		if (changed) {
>> +		if (changed
>> +		    || (S_ISGITLINK(ce->ce_mode) && is_submodule_modified(ce->name))) {
> 
> You had a check in your previous patch that decides to call or skip
> diff_change() based on is_submodule_modified() for diff-files, but forgot
> to have the same for diff-index, which this patch does.  Perhaps we want
> to squash this into 4519d9c (Show submodules as modified when they contain
> a dirty work tree, 2010-01-13).

Of course you are right, the change you quoted should have been in my
patch in the first place. So squashing it seems to be the right thing to
do (but AFAICS the tests i added might be a problem, as they use
expect_from_to() which your intermediate patch added. Maybe squash these
tests into your patch and the diff you quoted above into mine?).


> The existing code is a bit unfortunate; by the time we come to the output
> routine, the information we found from is_submodule_modified() is lost;
> that is why my "would look like this" patch calls is_submodule_modified().
> 
> We may want to add one parameter to diff_change() and diff_addremove(), to
> tell them if the work-tree side (if we are comparing something with the
> work tree) is a modified submodule, and add one bit to the diff_filespec
> structure to record that in diff_change() and diff_addremove() (obviously
> only when adding).  That way, my "would looks like this" patch needs to
> check the result of is_submodule_modified() the front-ends left in the
> filespec, instead of running it again.

Good idea, i've been already exploring this line of thought too and came
to the same conclusion (i noticed that when calling plain "git diff" in a
repo with submodules, is_submodule_modified() gets called *three* times
for each submodule, which is not /that/ good for performance ;-). But i
intended to do this optimization in a subsequent patch (and in preparation
for "git diff --submodule" being able to print /how/ the submodule is
dirty without having to scan it again).

^ permalink raw reply

* Re: [PATCH] git push --track
From: Miles Bader @ 2010-01-15  0:30 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Rudolf Polzer, Tay Ray Chuan, git
In-Reply-To: <7vr5ps5jx1.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:
> Yes, "push --track" lets you postpone the decision; branching, working on
> it, pushing it out _and_ _then_ using your "branch -f" trick will let you
> postpone the decision even further.

Nonetheless, "push --track" is by far the most natural UI for the most
common case, I think.

> And it doesn't add --track to the UI.

That's not a positive...

None of this is _necessary_, it's to make git more convenient.

Halfway-measures seem like "oh the user can just use this branch
command" seem like, well, halfway measures.  Sure, it would be nice to
have a branch command _too_, just to make it easier for branch
maintenance, but it's not what's really wanted.

-Miles

-- 
Faith, n. Belief without evidence in what is told by one who speaks without
knowledge, of things without parallel.

^ permalink raw reply

* Re: [PATCH v2 3/3] commit: show interesting ident information in  summary
From: Felipe Contreras @ 2010-01-15  1:21 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Thomas Rast, Jeff King, Adam Megacz, git
In-Reply-To: <7vy6k0bheg.fsf@alter.siamese.dyndns.org>

On Thu, Jan 14, 2010 at 9:44 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Users are lazy.
>
> And the ones that suffer from the issue discussed in this thread will not
> read the manual your patch touches.  When you make changes to the manual,
> you should not be targetting them, as they won't read it anyway.  Instead,
> the description of the manual should aim to help people who _read_ it.

The world is not clear-cut between users who read, and users don't.
Most probably user laziness follows a Pareto distribution:
http://en.wikipedia.org/wiki/File:Pareto_distributionPDF.png

The long tail of users who don't read much is so big that you will
find *a lot* that don't read anything at all, therefore you would also
find many that read a bit, and as a consequence a tiny amount that
actually would read the whole user manual.

Clearly, Thomas' comment implies that some people might need to adjust
their mental model to reflect reality.

-- 
Felipe Contreras

^ permalink raw reply

* Re: [PATCH 0/9] Gitweb caching v5
From: Jakub Narebski @ 2010-01-15  1:40 UTC (permalink / raw)
  To: John 'Warthog9' Hawley, John 'Warthog9' Hawley; +Cc: git
In-Reply-To: <1263432185-21334-1-git-send-email-warthog9@eaglescrag.net>

"John 'Warthog9' Hawley" <warthog9@eaglescrag.net> writes:

> Afternoon everyone,
>  
> This is the latest incarnation of gitweb w/ caching.  This is
> finally at the point where it should probably start either being
> considered for inclusion or mainline, or I need to accept that this
> will never get in and more perminantely fork (as is the case with
> Fedora where this is going in as gitweb-caching as a parrallel rpm
> package).
> 
> That said this brings the base up to mainline (again),

Could you tell us which commit is the base of this series (like in
git-request-pull output), i.e. which commit this series is rebased
against?

> it updates a
> number of elements in the caching engine, and this is a much cleaner
> break-out of the tree vs. what I am currently developing against.

Is caching engine part changed since v2?

> v5:
> 	- Missed a couple of things that were in my local tree, and
> 	  added them back in.

That doesn't tell us much.

> 	- Split up the die_error and the version matching patch
> 	- Set version matching to be on by default - otherwise this
> 	  really is code that will never get checked, or at best
> 	  enabled by default by distributions
> 	- Added a minor code cleanup with respect to $site_header
> 	  that was already in my tree
> 	- Applied against a more recent git tree vs. 1.6.6-rc2
> 	- Removed breakout patch for now (did that in v4 actually)
> 	  and will deal with that separately 
> 
> 	http://git.kernel.org/?p=git/warthog9/gitweb.git;a=shortlog;h=refs/heads/gitweb-ml-v5
 
----
Short comments about patches in this series; I will be sending
detailed comments for each patch individually.

> John 'Warthog9' Hawley (9):
>   gitweb: Load checking
Looks good.

>   gitweb: change die_error to take "extra" argument for extended die
>     information
Commit message could be better (summary should really be shorter), and
I think there is some indent typo, but otherwise looks good.

>   gitweb: Add option to force version match
For me it needs to be disabled in gitweb test suite (t/gitweb-lib.sh),
if it is enabled by default.  I really like that I can test current
gitweb without need to recompile git.

Also it should have tests that it works as intended (both for matching
and non-matching versions) in t/t9501-gitweb-standalone-http-status.sh

>   gitweb: Makefile improvements
Does it differ from my proposal (i.e. gitweb/Makefile doing the work),
based on your idea ("make gitweb" for Makefile and gitweb/Makefile)?

>   gitweb: add a get function to compliment print_local_time
>   gitweb: add a get function to compliment print_sort_th
Those two looks O.K. from what I seen.

>   gitweb: cleanup error message produced by undefined $site_header
Shouldn't there be such protection for other such variables, like
$site_footer and $home_text (and a bit diferent protection against
undefined $projects_list)?  By the way, how did you arrived at
undefined $site_header: deafult build configuration leaves it empty,
but defined. 

>   gitweb: Convert output to using indirect file handle
I have alternate solution, using shorter filehandle name (just $out)
in

  git://repo.or.cz/git/jnareb-git.git gitweb/cache-kernel
  http://repo.or.cz/w/git/jnareb-git.git/shortlog/refs/heads/gitweb/cache-kernel

I would have to think a bit about separate handle for binary files;
I am not sure if it is really required.

>   gitweb: File based caching layer (from git.kernel.org)
I am working (time permitting) in spliting this large code drop into
smaller commits, namely:

 - href(..., -path_info => 0)          (for cache key)
 - simple file based caching + tests
 - global expire time + tests
 - output caching in gitweb            (WIP)
 - adaptive expiration time            (planned)
 - tee output / cache write            (planned)
 - expire time variation from CHI      (planned)
 - locking for single writer           (planned)
 - server-side generating info         (planned)
 - AJAX-y generating info              (wishlist)

while ensuring that it pass all existing gitweb tests, and adding new
tests for new features.

-- 
Jakub Narebski
Poland
ShadeHawk on #git

^ permalink raw reply

* [PATCH] git status: display current branch name in color
From: Michael Wookey @ 2010-01-15  2:23 UTC (permalink / raw)
  To: Git Mailing List

There is an existing highlight when the user is not on any branch.
Enhance this functionality to always provide the name of the current
branch in color.

Signed-off-by: Michael Wookey <michaelwookey@gmail.com>
---
 wt-status.c |   10 ++++++----
 wt-status.h |    3 ++-
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/wt-status.c b/wt-status.c
index 5d56988..bdaa98b 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -17,6 +17,7 @@ static char default_wt_status_colors[][COLOR_MAXLEN] = {
 	GIT_COLOR_RED,    /* WT_STATUS_UNTRACKED */
 	GIT_COLOR_RED,    /* WT_STATUS_NOBRANCH */
 	GIT_COLOR_RED,    /* WT_STATUS_UNMERGED */
+	GIT_COLOR_GREEN,  /* WT_STATUS_BRANCH */
 };

 static const char *color(int slot, struct wt_status *s)
@@ -553,7 +554,7 @@ static void wt_status_print_tracking(struct wt_status *s)

 void wt_status_print(struct wt_status *s)
 {
-	const char *branch_color = color(WT_STATUS_HEADER, s);
+	const char *branch_color = color(WT_STATUS_BRANCH, s);

 	if (s->branch) {
 		const char *on_what = "On branch ";
@@ -561,12 +562,13 @@ void wt_status_print(struct wt_status *s)
 		if (!prefixcmp(branch_name, "refs/heads/"))
 			branch_name += 11;
 		else if (!strcmp(branch_name, "HEAD")) {
-			branch_name = "";
+			branch_name = "Not currently on any branch.";
 			branch_color = color(WT_STATUS_NOBRANCH, s);
-			on_what = "Not currently on any branch.";
+			on_what = "";
 		}
 		color_fprintf(s->fp, color(WT_STATUS_HEADER, s), "# ");
-		color_fprintf_ln(s->fp, branch_color, "%s%s", on_what, branch_name);
+		color_fprintf(s->fp, color(WT_STATUS_HEADER, s), "%s", on_what);
+		color_fprintf_ln(s->fp, branch_color, "%s", branch_name);
 		if (!s->is_initial)
 			wt_status_print_tracking(s);
 	}
diff --git a/wt-status.h b/wt-status.h
index c60f40a..b0cf235 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -12,6 +12,7 @@ enum color_wt_status {
 	WT_STATUS_UNTRACKED,
 	WT_STATUS_NOBRANCH,
 	WT_STATUS_UNMERGED,
+	WT_STATUS_BRANCH,
 };

 enum untracked_status_type {
@@ -40,7 +41,7 @@ struct wt_status {
 	int relative_paths;
 	int submodule_summary;
 	enum untracked_status_type show_untracked_files;
-	char color_palette[WT_STATUS_UNMERGED+1][COLOR_MAXLEN];
+	char color_palette[WT_STATUS_BRANCH+1][COLOR_MAXLEN];

 	/* These are computed during processing of the individual sections */
 	int commitable;
-- 
1.6.6.197.gfd7f6

^ permalink raw reply related

* Re: [PATCH 0/9] Gitweb caching v5
From: J.H. @ 2010-01-15  4:29 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: John 'Warthog9' Hawley, git
In-Reply-To: <m37hrkdu4k.fsf@localhost.localdomain>

On 01/14/2010 05:40 PM, Jakub Narebski wrote:
> "John 'Warthog9' Hawley" <warthog9@eaglescrag.net> writes:
> 
>> Afternoon everyone,
>>  
>> This is the latest incarnation of gitweb w/ caching.  This is
>> finally at the point where it should probably start either being
>> considered for inclusion or mainline, or I need to accept that this
>> will never get in and more perminantely fork (as is the case with
>> Fedora where this is going in as gitweb-caching as a parrallel rpm
>> package).
>>
>> That said this brings the base up to mainline (again),
> 
> Could you tell us which commit is the base of this series (like in
> git-request-pull output), i.e. which commit this series is rebased
> against?

This series was based on
git://git.kernel.org/pub/scm/git/git.git
054d2fa05cf0bc55fe1556c9e87d58d67a144f44

http://git.kernel.org/?p=git/warthog9/gitweb.git;a=shortlog;h=refs/heads/gitweb-ml-v5

> 
>> it updates a
>> number of elements in the caching engine, and this is a much cleaner
>> break-out of the tree vs. what I am currently developing against.
> 
> Is caching engine part changed since v2?

Slightly, not dramatically.  Only changes were to modify the caching
engine to deal with the change in output mechanism (I.E. my $output ->
print {$fh})

<snip>
>> John 'Warthog9' Hawley (9):
>>   gitweb: Load checking
> Looks good.
> 
>>   gitweb: change die_error to take "extra" argument for extended die
>>     information
> Commit message could be better (summary should really be shorter), and
> I think there is some indent typo, but otherwise looks good.
> 
>>   gitweb: Add option to force version match
> For me it needs to be disabled in gitweb test suite (t/gitweb-lib.sh),
> if it is enabled by default.  I really like that I can test current
> gitweb without need to recompile git.
> 
> Also it should have tests that it works as intended (both for matching
> and non-matching versions) in t/t9501-gitweb-standalone-http-status.sh

I'll get t9501 cleaned up and make sure that the tests default to
turning it off, and I'll add a test to confirm that this works.

>>   gitweb: Makefile improvements
> Does it differ from my proposal (i.e. gitweb/Makefile doing the work),
> based on your idea ("make gitweb" for Makefile and gitweb/Makefile)?

I think this is taken straight from the version you had, I don't think
I've modified it.

> 
>>   gitweb: add a get function to compliment print_local_time
>>   gitweb: add a get function to compliment print_sort_th
> Those two looks O.K. from what I seen.
> 
>>   gitweb: cleanup error message produced by undefined $site_header
> Shouldn't there be such protection for other such variables, like
> $site_footer and $home_text (and a bit diferent protection against
> undefined $projects_list)?  By the way, how did you arrived at
> undefined $site_header: deafult build configuration leaves it empty,
> but defined.

I would have to go back and figure it out, but it's something I hit
years ago and added that check to keep it from spewing all over my logs.
 Could easily add it to the others mentioned.

>>   gitweb: Convert output to using indirect file handle
> I have alternate solution, using shorter filehandle name (just $out)
> in
> 
>   git://repo.or.cz/git/jnareb-git.git gitweb/cache-kernel
>   http://repo.or.cz/w/git/jnareb-git.git/shortlog/refs/heads/gitweb/cache-kernel
> 
> I would have to think a bit about separate handle for binary files;
> I am not sure if it is really required.

For caching you have to have it.  When your outputing the data back from
the cache you need to switch the output mode for the browser to receive
the data properly.  Otherwise the resulting output from the caching
engine is going to be garbage.  The caching engine explicitly stores the
binary files separate from the rest of the response.

>>   gitweb: File based caching layer (from git.kernel.org)
> I am working (time permitting) in spliting this large code drop into
> smaller commits, namely:

first up, is there a reason not to take the caching layer as it stands
while you work on these?  I'm fine with adding test cases for what's
there now if you want, but I guess I'm confused about explicitly wanting
to break these into smaller commits.

>  - href(..., -path_info => 0)          (for cache key)

You actually *really* want to have the full url vs. just the path_info.
 While I accept that this means that you will end up with multiple
copies of data being stored it helps dramatically if you have multiple
sites pointing into the same caching space.  If you happen to have two
distinct trees

http://git.public.com/?p=test.git;a=summary
http://git.private.com/?p=test.git;a=summary

That respectively point to:

/group/public/git/test.git
/group/private/git/test.git

you'll end up squashing the cache files needlessly and erroneously as
what's in the cache file will depend on what last site was hit that
generated the file.

>  - simple file based caching + tests
>  - global expire time + tests
>  - output caching in gitweb            (WIP)
>  - adaptive expiration time            (planned)
>  - tee output / cache write            (planned)

You sadly can't 'tee' the output as this would re-introduce the
stampeding heard problem which is one of the reasons the caching layer
came about in the first place.  Suppose you could give one person the
output but make everyone else wait for the cache to finish writing out,
or have the waiting client processes tail the file while it's generated
but those both seem a little excessive vs. just waiting.

>  - expire time variation from CHI      (planned)
>  - locking for single writer           (planned)
>  - server-side generating info         (planned)
>  - AJAX-y generating info              (wishlist)

If it's helpful I can genuinely devote several more days to this to get
these cleaned up.  Which would save you a fair amount of time in
breaking this up.

- John 'Warthog9' Hawley

^ permalink raw reply

* Re: [PATCH] git push --track
From: Junio C Hamano @ 2010-01-15  5:47 UTC (permalink / raw)
  To: Rudolf Polzer; +Cc: git, Ilari Liusvaara
In-Reply-To: <op.u6haiiiog402ra@nb-04>

"Rudolf Polzer" <divVerent@alientrap.org> writes:

> On Wed, 13 Jan 2010 16:43:10 +0100, Ilari Liusvaara
> <ilari.liusvaara@elisanet.fi> wrote:
>
>> - Some lines look way too long (~160 chars, should be max 80 unles
>> it would linebreak error message).
>
> Yes, also I got told that I used the wrong braces style... well, fixed
> that.

You didn't, although you tried to "hide" one level, which is even worse.

If you see overlong lines in patch text, it often is that the added
codepath is too deeply nested, and it often becomes much easier to
understand if you split it into a separate smaller helper function.

For example, if you have

	if (A) {
        	if (B) {
                	do something #1
                        if (C) {
                        	do something #2
                                while (D) {
                                	if (E) {
                                        	do something #3
					}
				}
			}
		}
	}

it often is much easier to read if you did:

	if (A)
		helper(...)

and wrote a helper that is

	helper()
        {
        	if (!B)
                	return
		do something #1
                if (!C)
                	return
		do something #2
		while (D) {
                	if (!E)
                        	continue
			do something #3
		}
	}

>> - Should the tracking be set up even if only part of ref update suceeded
>> (for those that succeeded), not requiring all to succeed?

I think giving configuration to the ones that succeeded, while not doing
so for the ones that failed, would be the best.

>> - Is --track the best name for this?

Most probably not.  "git branch --track" was already a mistake, whose
damage can be seen in the first message in this thread.  I originally read
"this converts a local branch to a tracking branch", and went "Huh??? ---
Is this patch running 'mv refs/heads/frotz refs/remotes/origin/frotz'?
What's fun about it???"

> @@ -115,6 +116,36 @@ static int push_with_options(struct transport
> *transport, int flags)
>  		fprintf(stderr, "Pushing to %s\n", transport->url);
>  	err = transport_push(transport, refspec_nr, refspec, flags,
>  			     &nonfastforward);
> +	if (err == 0 && flags & TRANSPORT_PUSH_TRACK) {

Style:

 - Have SP between syntactic keyword and open parenthesis.

 - Never place an opening brace, except the one that begins a function
   body, on its own line;

Also the overlong line is merely a symptom that you are putting too much
stuff in this function.  The whole addition should probably be a helper
function.

> @@ -115,6 +116,33 @@ static int push_with_options(struct transport *transport, int flags)
>  		fprintf(stderr, "Pushing to %s\n", transport->url);
>  	err = transport_push(transport, refspec_nr, refspec, flags,
>  			     &nonfastforward);
> +	if (err == 0 && flags & TRANSPORT_PUSH_TRACK)
> +	{
> +		struct ref *remote_refs =
> +			transport->get_refs_list(transport, 1);

You have already pushed by calling transport_push() before you got "err"
back.  Do you need to make a second, separate call to ls-remote here and
if so why?

I have a feeling that it is more appropriate to have the additional code
in transport_push(), which gets ls-remote information, runs match_refs()
and finally calls transport->push_refs().  I think the extra branch
configuration would fit better inside the if block immediately after all
that happens, i.e.

	if (!(flags & TRANSPORT_PUSH_DRY_RUN)) {
		struct ref *ref;
		for (ref = remote_refs; ref; ref = ref->next)
			update_tracking_ref(transport->remote, ref, verbose);
+		if (flags & TRANSPORT_PUSH_RECONFIGURE_FORK)
+			configure_forked_branch(...);
	}

in transport.c

> +		if(!(flags & TRANSPORT_PUSH_DRY_RUN))
> +		if(!match_refs(local_refs, &remote_refs, refspec_nr, refspec, match_flags))

Yuck; hiding the fact that you have an over-nested logic is not a way to
fix it.

^ permalink raw reply

* Re: Removal of post-upload-hook
From: Arun Raghavan @ 2010-01-15  6:12 UTC (permalink / raw)
  To: Jeff King; +Cc: Shawn O. Pearce, git
In-Reply-To: <20100114204305.GC26883@coredump.intra.peff.net>

2010/1/15 Jeff King <peff@peff.net>:
> On Thu, Jan 14, 2010 at 11:41:07AM -0800, Shawn O. Pearce wrote:
>
>> > Because receive-pack runs as the user who is pushing, not as the
>> > repository owner. So by convincing you to push to my repository in a
>> > multi-user environment, I convince you to run some arbitrary code of
>> > mine.
>>
>> Uhhh, this was in fetch/upload-pack Peff, not push/receive-pack.
>>
>> Same issue though.
>
> Errr...yeah. Sorry for the confusion. But yes, it's the same mechanism,
> except that it is even easier to get people to pull from you (to get
> them to push, you first have to get them to write a worthwhile code
> contribution. ;) ).

:)

Another thought - would it be acceptable to have a config option to
enable/disable these types of hooks, so that people who are not
affected by the problem or explicitly don't care can use them? Perhaps
a core.allowInsecureHooks ?

Cheers,
-- 
Arun Raghavan
http://arunraghavan.net/
(Ford_Prefect | Gentoo) & (arunsr | GNOME)

^ permalink raw reply

* [PATCH v2] Test t5560: Fix test when run with dash
From: Tarmigan Casebolt @ 2010-01-15  6:44 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Michael Haggerty, git, Shawn O. Pearce, Tarmigan Casebolt
In-Reply-To: <7vfx69hyd5.fsf@alter.siamese.dyndns.org>

The dash shell is more finicky than some others.

In particular, it does not seem to like the pattern of setting an
environment variable on the same line as you call a shell function
like this:

        REQUEST_METHOD="GET" some_shell_function

as you might use to set a variable only for one command if that
command were an executable or a shell builtin.

Reported-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Tarmigan Casebolt <tarmigan+git@gmail.com>
---
Junio, that description matches my understanding of the problem.
I can't tell from my reading of the POSIX spec whether my usage was
wrong or if dash is wrong, which is why I shied away from an
explanation.  As a practical matter though, this patch does fix the
issue.

This version takes a slighty different approach that I think leaves
things clearer and doesn't pass in tons of arguements to the shell
function.  If you prefer the old approach, I can send a patch that way
instead.
---
 t/t5560-http-backend-noserver.sh |   15 ++++++++++-----
 1 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/t/t5560-http-backend-noserver.sh b/t/t5560-http-backend-noserver.sh
index 5f8c88e..44885b8 100755
--- a/t/t5560-http-backend-noserver.sh
+++ b/t/t5560-http-backend-noserver.sh
@@ -14,8 +14,9 @@ run_backend() {
 }
 
 GET() {
-	REQUEST_METHOD="GET" \
+	export REQUEST_METHOD="GET" &&
 	run_backend "/repo.git/$1" &&
+	unset REQUEST_METHOD &&
 	if ! grep "Status" act.out >act
 	then
 		printf "Status: 200 OK\r\n" >act
@@ -25,9 +26,11 @@ GET() {
 }
 
 POST() {
-	REQUEST_METHOD="POST" \
-	CONTENT_TYPE="application/x-$1-request" \
+	export REQUEST_METHOD="POST" &&
+	export CONTENT_TYPE="application/x-$1-request" &&
 	run_backend "/repo.git/$1" "$2" &&
+	unset REQUEST_METHOD &&
+	unset CONTENT_TYPE &&
 	if ! grep "Status" act.out >act
 	then
 		printf "Status: 200 OK\r\n" >act
@@ -43,13 +46,15 @@ log_div() {
 . "$TEST_DIRECTORY"/t556x_common
 
 expect_aliased() {
+	export REQUEST_METHOD="GET" &&
 	if test $1 = 0; then
-		REQUEST_METHOD=GET run_backend "$2"
+		run_backend "$2"
 	else
-		REQUEST_METHOD=GET run_backend "$2" &&
+		run_backend "$2" &&
 		echo "fatal: '$2': aliased" >exp.err &&
 		test_cmp exp.err act.err
 	fi
+	unset REQUEST_METHOD
 }
 
 test_expect_success 'http-backend blocks bad PATH_INFO' '
-- 
1.6.6

^ permalink raw reply related


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