Git development
 help / color / mirror / Atom feed
* Re: [PATCH 1/4] gitweb: notes feature
From: Jakub Narebski @ 2010-02-04 23:20 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Giuseppe Bilotta, git, Johannes Schindelin, Johan Herland
In-Reply-To: <7vwrys7lrm.fsf@alter.siamese.dyndns.org>

Dnia czwartek 4. lutego 2010 22:07, Junio C Hamano napisał:
> Jakub Narebski <jnareb@gmail.com> writes:
> 
>>> IOW, the code should be reading output from:
>>> 
>>>     GIT_NOTES_REF=$note_ref git show -s --format=%N $co{'id'}
>>> 
>>> as the notes tree may not be storing notes in a flat one-level namespace
>>> like you are assuming.
>>
>> First, for some mechanism of deployment (IIRC Apache's mod_perl) changes
>> to environment variables from CGI script are not passed to invoked
>> commands (I guess for security reasons).
> 
> I do not believe you are unable to spawn
> 
> 	open $fd, '-|' 'sh', '-c', "GIT_NOTES_REF=$note_ref git show ..." 
> 
> and read from it ;-).

You meant here

	my $git_command = quote_command(git_cmd(), 'show', ...);
	open my $fd, '-|', 'sh', '-c', "GIT_NOTES_REF=$note_ref $git_command" 

So you need to take care to quote parameters ($note_ref fortunately 
doesn't need to be quoted)... and you have one more process (shell)
spawned.


Therefore I think it would be nice to have --notes-ref option to git 
wrapper,... especially that it should be easy to set it up in such way
that it would be possible to pass --notes-ref multiple times, e.g.:

	git --notes-ref=commits --notes-ref=git-svn show ...

-- 
Jakub Narebski
Poland

^ permalink raw reply

* gitk: not all colours are configurable (so grey on black scheme is not possible)
From: Jonathan Nieder @ 2010-02-04 23:32 UTC (permalink / raw)
  To: git, 568470-forwarded; +Cc: Matthijs Kooijman, Paul Mackerras

Hi gitsters,

The following request was submitted against the Debian gitk package.
Sounds sensible to me; anyone interested?

From: Matthijs Kooijman <matthijs@stdin.nl>
Package: gitk
Version: 1:1.6.6-1
Severity: wishlist

gitk allows configuration of the colours used in the interface, by using
the edit -> preferences dialogs. I've used this feature to set the
background colour to black and the foreground colour to grey. However,
some colours in the interface are not configurable, which makes this
scheme very uncomfortable to work with.

In particular, I'm having the following issues:
 * The foreground text color of the interface. The setting "interface
   colour" allows me to set the background of most interface parts.
   (except for the areas in the interface showing the list, diff, etc.
   which are controllable with the "background colour" setting).
   However, I can't set the "interface colour" to a dark tone, since I
   can't control the text.

   This should be solved by adding an "interface text colour" setting.
 * The background text of some of the interface elements (in particular
   some textboxes in the new view / edit view dialogs) are controlled by
   the "background colour" setting instead of the "interface colour".
   However, the text color is not controlled by the "foregrond colour"
   (and, as noted in the previous point, is not configurable anywhere),
   so I get unreadable text if I set "background colour" to black.

   This should be solved by making all text boxes listen to the
   "interface colour" setting, or by making the text in these text boxes
   listen to the "foreground colour" setting.
 * In the diff view, the two header lines of every file (containing the
   filename and git hashes) has a fixed grey background. The text
   colour, however, is controlled by the "foreground colour" setting, so
   setting that to grey leaves you with unreadable headers.
   
   This should be solved by making a "Diff headers background colour"
   setting, or something similar.

Gr.

Matthijs
-- System Information:
Debian Release: squeeze/sid
  APT prefers unstable
  APT policy: (500, 'unstable'), (500, 'stable'), (1, 'experimental')
Architecture: amd64 (x86_64)

Kernel: Linux 2.6.32.7 (PREEMPT)
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/bash

Versions of packages gitk depends on:
ii  git-core                      1:1.6.6-1  fast, scalable, distributed revisi
ii  tk                            8.4.16-2   The Tk toolkit for Tcl and X11 (de

^ permalink raw reply

* Re: [PATCH 1/4] gitweb: notes feature
From: Giuseppe Bilotta @ 2010-02-04 23:38 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jakub Narebski, git, Johannes Schindelin, Johan Herland
In-Reply-To: <7vaavo90ic.fsf@alter.siamese.dyndns.org>

On Thu, Feb 4, 2010 at 10:03 PM, Junio C Hamano <gitster@pobox.com> wrote:
>
> AFAIU, the note code on the core side already creates a fan-out structure
> when notes tree gets large (see recent "What's cooking"; the series is
> parked in 'pu' but that is primarily because we are in feature freeze); it
> is not just "inherently not robust" but is much closer to "broken from day
> one" ;-).  Otherwise I wouldn't have wasted time to point it out.

Ouch, I hadn't considered that, indeed.

> Your code is a very good proof-of-concept, though.

Thanks. I guess at this point a proper implementation can wait for the
necessary core support functions.

> Regarding support of multiple notes hierarchies, listing, etc.
>
> See for example:
>
>  http://thread.gmane.org/gmane.comp.version-control.git/138079/focus=138128
>
> I expect more ideas from needs by end-user would come, as we gain
> experience with using notes in real projects.  You will certainly find
> some other needs of your own, like the "not an environment but a command
> line option" which Jakub mentioned, and "multiple hierarchies" like both
> you and I found need for.  Share them and let us together make the notes
> mechanism nicer to use.

Collecting those ideas together would also help define some sort of
roadmap, or at least have a clear idea of what's needed, to help drive
the design of the features themselves. Maybe we could start a TODO
page on the wiki collecting these ideas?

-- 
Giuseppe "Oblomov" Bilotta

^ permalink raw reply

* Re: [PATCH] grep: simple test for operation in a bare repository
From: René Scharfe @ 2010-02-05  0:24 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Junio C Hamano
In-Reply-To: <4B6A0BDE.2050908@lsrfire.ath.cx>

Am 04.02.2010 00:50, schrieb René Scharfe:
> Am 03.02.2010 19:16, schrieb René Scharfe:
>> Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
>> ---
>>  t/t7002-grep.sh |   17 +++++++++++++++++
>>  1 files changed, 17 insertions(+), 0 deletions(-)
> 
> Err, no, that won't do.  Sorry.
> 
> The test script fails to demonstrate the issue I've run into.  It runs
> successfully, but running git grep manually fails:
> 
> 	$ cd t/trash\ directory.t7002-grep/.git/bare_test_repo/
> 	$ git grep bla HEAD
> 	fatal: This operation must be run in a work tree
> 
> I have to dig a bit deeper and try to come back with a better test script.

OK, I have to admit defeat: I can't come up with a test script.  But
the issue is reproducible: git grep in a bare repository fails when
run with a pager.

	$ mkdir /tmp/a
	$ cd /tmp/a
	$ git init
	Initialized empty Git repository in /tmp/a/.git/
	$ echo a >a
	$ git add a
	$ git commit -m.
	[master (root-commit) e11f955] .
	 1 files changed, 1 insertions(+), 0 deletions(-)
	 create mode 100644 a

	$ git clone --bare . ../b
	Initialized empty Git repository in /tmp/b/
	$ cd /tmp/b

	$ git grep a HEAD
	fatal: This operation must be run in a work tree
	$ git grep a HEAD | cat
	HEAD:a:a
	$ git --no-pager grep a HEAD
	HEAD:a:a

Reverting 7e622650 (grep: prepare to run outside of a work tree), or
rather just setting the flag RUN_SETUP for grep in git.c again, makes
the first git grep call succeed, too.

As does the following patch, but I don't know why.  The call chain is
quite deep.  It seems that without the patch the static variable
git_dir in environment.c isn't updated when git finds out that it runs
in a bare repo -- but only if a pager is used.

There are five more sites in git.c, path.c and setup.c where $GIT_DIR
is set directly with setenv().  I wonder if they should better call
set_git_dir() instead, too.


diff --git a/setup.c b/setup.c
index 710e2f3..5fb9b25 100644
--- a/setup.c
+++ b/setup.c
@@ -406,7 +406,7 @@ const char *setup_git_directory_gently(int *nongit_ok)
 				cwd[offset] = '\0';
 				setenv(GIT_DIR_ENVIRONMENT, cwd, 1);
 			} else
-				setenv(GIT_DIR_ENVIRONMENT, ".", 1);
+				set_git_dir(".");
 			check_repository_format_gently(nongit_ok);
 			return NULL;
 		}

^ permalink raw reply related

* [PATCH] fix an error message in git-push so it goes to stderr
From: Larry D'Anna @ 2010-02-05  0:41 UTC (permalink / raw)
  To: git

Having it go to standard output interferes with git-push --porcelain.
---
 builtin-push.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/builtin-push.c b/builtin-push.c
index 5633f0a..0a27072 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -124,9 +124,9 @@ static int push_with_options(struct transport *transport, int flags)
 		return 0;
 
 	if (nonfastforward && advice_push_nonfastforward) {
-		printf("To prevent you from losing history, non-fast-forward updates were rejected\n"
-		       "Merge the remote changes before pushing again.  See the 'Note about\n"
-		       "fast-forwards' section of 'git push --help' for details.\n");
+		fprintf(stderr, "To prevent you from losing history, non-fast-forward updates were rejected\n"
+				"Merge the remote changes before pushing again.  See the 'Note about\n"
+				"fast-forwards' section of 'git push --help' for details.\n");
 	}
 
 	return 1;
-- 
1.7.0.rc1.33.g07cf0f.dirty

^ permalink raw reply related

* Re: [PATCH 1/4] gitweb: notes feature
From: Jakub Narebski @ 2010-02-05  0:44 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Giuseppe Bilotta, git, Johannes Schindelin, Johan Herland
In-Reply-To: <201002041821.22864.jnareb@gmail.com>

Jakub Narebski wrote:
> On Thu, 4 Feb 2010, Junio C Hamano wrote:
>> Junio C Hamano <gitster@pobox.com> writes:
>> 
>>> Giuseppe Bilotta <giuseppe.bilotta@gmail.com> writes:
>>>
>>>> +		my %notes = () ;
>>>> +		foreach my $note_ref (@note_refs) {
>>>> +			my $obj = "$note_ref:$co{'id'}";
[...]
 
> Second, parse_commit / parse_commits use
> 
>   git rev-list -z --parents --header --max-count-X
> 
> If this command automatically shows notes (or it can be modified to
> automatically show notes) after unindented "Notes:" line (as per
> git-notes documentation), then the only thing that needs to be
> changed to fill %commit{'notes'} is parse_commit_text subroutine.

This command automatically shows notes, even in absence of GIT_NOTES_REF
environment variable or core.notesRef (if core.notesRef is not unset),
so unless we want gitweb to display notes flattened into commit message
(which I think was intended behavior - design decision for notes 
display), we would need to modify parse_commit_text to gather notes 
into %commin{'notes'} (or something).

 
> There would be no need for extra subroutine (and extra command, or
> even up to two extra commands per note) to get notes data.

You are right that we would need it if we want to display notes from
non-default namespace.

Still, 1 or 2 git commands per commit is a bit too much (with shortlog
displaying 100 commits per page): that is what "git cat-file --batch"
was invented ;-)

-- 
Jakub Narebski
Poland

^ permalink raw reply

* Re: rebase vs rebase -i
From: Junio C Hamano @ 2010-02-05  0:45 UTC (permalink / raw)
  To: Jay Soffian; +Cc: Johannes Schindelin, git
In-Reply-To: <76718491002041157o37de139brb7995076274375aa@mail.gmail.com>

Jay Soffian <jaysoffian@gmail.com> writes:

> On Thu, Feb 4, 2010 at 1:46 PM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
>> Both "rebase -i" and "rebase -m" are really a cherry-pick in a loop.
>
> Well then I'm still confused. I see where pick_one() in
> git-rebase--interactive.sh is using cherry-pick.
>
> But call_merge() in git-rebase.sh is using git-merge-recursive (absent
> specifying another strategy).
>
> ?

I think Dscho's point is that cherry-pick internally runs the same
merge-recursive.

When you have a change C based on its parent C^ and want to replay that
effect on a (possibly unrelated) commit A, you would run three-way merge,
merging C into A as if C^ is the common ancestor.  The rebase script
cherry-pick, and revert all work with the same principle (for revert
obviously you would swap C and C^---you are applying the effect of going
from C to C^ in that case).

And no, "format-patch --stdout | am -3" pipe in the normal rebase codepath
will stay unless you can produce a benchmark that says the performance of
merge machinery is good enough these days.  Back when "rebase -m" was
introduced, it wasn't.

^ permalink raw reply

* Re: [PATCH 1/4] gitweb: notes feature
From: Junio C Hamano @ 2010-02-05  0:55 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Giuseppe Bilotta, git, Johannes Schindelin, Johan Herland
In-Reply-To: <201002050144.50538.jnareb@gmail.com>

Jakub Narebski <jnareb@gmail.com> writes:

> Still, 1 or 2 git commands per commit is a bit too much (with shortlog
> displaying 100 commits per page)

But who said you need to display notes for all commits by default?

It could be a thing that you would get _on demand_, just like patch text
or diffstat is given only on demand by going to the commitdiff page.

^ permalink raw reply

* Re: rebase vs rebase -i
From: Junio C Hamano @ 2010-02-05  1:18 UTC (permalink / raw)
  To: Jay Soffian; +Cc: Johannes Schindelin, git
In-Reply-To: <7vmxzose72.fsf@alter.siamese.dyndns.org>

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

> And no, "format-patch --stdout | am -3" pipe in the normal rebase codepath
> will stay unless you can produce a benchmark that says the performance of
> merge machinery is good enough these days.  Back when "rebase -m" was
> introduced, it wasn't.

Just for fun, I pulled from the kernel tree.  Its tip is at fc76be4 (Merge
master.kernel.org:/home/rmk/linux-2.6-arm, 2010-02-04), which is a merge
of a subsystem tree into the mainline.

I tried to rebase HEAD^2 (i.e. subsystem change) on top of HEAD^1 (the
state Linus merged that subsystem change into).  Either way, the
experiment is to linearlize a side branch that has 16 patches:

The experiment is to take a history of this shape:

  --A-...--N--O--P master^2 = test (subsystem tip)
                  \
             ---X--Y master
          master^1

and turn it into a history of this shape:


  --A-...--N--O--P master^2 = test (subsystem tip)
                  \
             ---X--Y master
                 \
                  A'-B'-...O'-P' (rebased tip)

The tree at rebased tip P' must match the merge Y, of course.

First, to prepare:

    $ git checkout -b test master^2 ;# at the tip of subsystem
    $ git checkout HEAD^0 ;# detach so that I can easily repeat

Best of 5 runs on my box are:

    $ git reset --hard test && time git rebase master^1
    real    0m3.060s
    user    0m1.976s
    sys     0m0.812s

vs

    $ git reset --hard test && time git -m rebase master^1
    real    0m19.060s
    user    0m15.025s
    sys     0m3.564s

The numbers are understandable; the series touches only 12 paths among
31.5k paths, so applying patches has to be faster.

I have a plan to make a merge go faster by using a yet another merge
strategy, but that has been backburnered for now.

^ permalink raw reply

* Re: rebase vs rebase -i
From: Jay Soffian @ 2010-02-05  1:20 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin, git
In-Reply-To: <7vmxzose72.fsf@alter.siamese.dyndns.org>

On Thu, Feb 4, 2010 at 7:45 PM, Junio C Hamano <gitster@pobox.com> wrote:
> I think Dscho's point is that cherry-pick internally runs the same
> merge-recursive.

Ah hah, that's what I was missing. Thank you.

> When you have a change C based on its parent C^ and want to replay that
> effect on a (possibly unrelated) commit A, you would run three-way merge,
> merging C into A as if C^ is the common ancestor.  The rebase script
> cherry-pick, and revert all work with the same principle (for revert
> obviously you would swap C and C^---you are applying the effect of going
> from C to C^ in that case).
>
> And no, "format-patch --stdout | am -3" pipe in the normal rebase codepath
> will stay unless you can produce a benchmark that says the performance of
> merge machinery is good enough these days.  Back when "rebase -m" was
> introduced, it wasn't.

Indeed, the difference is painful on a largish tree (910M after gc
--aggressive, 39k files). Best of 3 runs for each of these:

$ time git rebase --onto HEAD~11 HEAD~10
First, rewinding head to replay your work on top of it...
[...]
real	0m11.164s
user	0m2.671s
sys	0m4.836s

$ time git rebase -m --onto HEAD~11 HEAD~10
[...]
real	0m40.507s
user	0m17.848s
sys	0m16.052s

$ time GIT_EDITOR="sed -i -e 1d" git rebase -i HEAD~11
[...]
real	0m27.758s
user	0m12.615s
sys	0m13.134s

It looks like there's room for improvement to rebase -m. (2.53 Ghz
Core 2 Duo Macbook Pro, 4GB memory.)

j.

^ permalink raw reply

* Showing whitespace on minus lines of diff ouput
From: Jay Soffian @ 2010-02-05  1:47 UTC (permalink / raw)
  To: git

The colored diff output is quite helpful to show introductions of whitespace.

However, if whitespace has been removed, it's impossible to see in the
diff output since whitespace is colorized on '+' lines, but not '-'
lines.

I'm looking at diff.c, but wow. Can someone more familiar with this
file point me toward how I'd make a patch to colorize whitespace on
'-' lines as well?

Thanks,

j.

^ permalink raw reply

* Re: Showing whitespace on minus lines of diff ouput
From: Jay Soffian @ 2010-02-05  2:08 UTC (permalink / raw)
  To: git
In-Reply-To: <76718491002041747t327bf2f5l85e095244f6ee1ed@mail.gmail.com>

On Thu, Feb 4, 2010 at 8:47 PM, Jay Soffian <jaysoffian@gmail.com> wrote:
> I'm looking at diff.c, but wow. Can someone more familiar with this
> file point me toward how I'd make a patch to colorize whitespace on
> '-' lines as well?

Ah, got it. I think. Patch shortly. :-)

j.

^ permalink raw reply

* Re: [PATCH] grep: simple test for operation in a bare repository
From: Nguyen Thai Ngoc Duy @ 2010-02-05  2:40 UTC (permalink / raw)
  To: René Scharfe; +Cc: Git Mailing List, Junio C Hamano
In-Reply-To: <4B6B653A.8060909@lsrfire.ath.cx>

On Fri, Feb 5, 2010 at 7:24 AM, René Scharfe
<rene.scharfe@lsrfire.ath.cx> wrote:
> OK, I have to admit defeat: I can't come up with a test script.  But
> the issue is reproducible: git grep in a bare repository fails when
> run with a pager.
>
>        $ mkdir /tmp/a
>        $ cd /tmp/a
>        $ git init
>        Initialized empty Git repository in /tmp/a/.git/
>        $ echo a >a
>        $ git add a
>        $ git commit -m.
>        [master (root-commit) e11f955] .
>         1 files changed, 1 insertions(+), 0 deletions(-)
>         create mode 100644 a
>
>        $ git clone --bare . ../b
>        Initialized empty Git repository in /tmp/b/
>        $ cd /tmp/b
>
>        $ git grep a HEAD
>        fatal: This operation must be run in a work tree
>        $ git grep a HEAD | cat
>        HEAD:a:a
>        $ git --no-pager grep a HEAD
>        HEAD:a:a
>
> Reverting 7e622650 (grep: prepare to run outside of a work tree), or
> rather just setting the flag RUN_SETUP for grep in git.c again, makes
> the first git grep call succeed, too.
>
> As does the following patch, but I don't know why.  The call chain is
> quite deep.  It seems that without the patch the static variable
> git_dir in environment.c isn't updated when git finds out that it runs
> in a bare repo -- but only if a pager is used.

setup_pager() calls git_config(), which indirectly calls get_git_dir()
and sets git_dir in stone. Changing GIT_DIR environment variable alone
won't work, as you have seen.

When RUN_SETUP is set, setup_git_directory() would be called before
setup_pager() can kick in, so everything is properly set.

> There are five more sites in git.c, path.c and setup.c where $GIT_DIR
> is set directly with setenv().  I wonder if they should better call
> set_git_dir() instead, too.

Yes, they should.
-- 
Duy

^ permalink raw reply

* Re: Showing whitespace on minus lines of diff ouput
From: Jay Soffian @ 2010-02-05  2:54 UTC (permalink / raw)
  To: git
In-Reply-To: <76718491002041808r6fcbf88egd1e9ce4927eb36c0@mail.gmail.com>

On Thu, Feb 4, 2010 at 9:08 PM, Jay Soffian <jaysoffian@gmail.com> wrote:
> Ah, got it. I think. Patch shortly. :-)

Bah, still need help. Here's what I tried (apologizes for the gmail
munging of long lines):

diff --git a/diff.c b/diff.c
index 381cc8d..17133cd 100644
--- a/diff.c
+++ b/diff.c
@@ -331,6 +331,23 @@ static int new_blank_line_at_eof(struct
emit_callback *ecbdata, const char *line
 	return ws_blank_line(line, len, ecbdata->ws_rule);
 }

+static void emit_sub_line(const char *reset,
+			  struct emit_callback *ecbdata,
+			  const char *line, int len)
+{
+	const char *ws = diff_get_color(ecbdata->color_diff, DIFF_WHITESPACE);
+	const char *set = diff_get_color(ecbdata->color_diff, DIFF_FILE_OLD);
+
+	if (!*ws)
+		emit_line_0(ecbdata->file, set, reset, '-', line, len);
+	else {
+		/* Emit just the prefix, then the rest. */
+		emit_line_0(ecbdata->file, set, reset, '-', "", 0);
+		ws_check_emit(line, len, ecbdata->ws_rule,
+			      ecbdata->file, set, reset, ws);
+	}
+}
+
 static void emit_add_line(const char *reset,
 			  struct emit_callback *ecbdata,
 			  const char *line, int len)
@@ -434,7 +451,6 @@ static void emit_rewrite_lines(struct emit_callback *ecb,
 {
 	const char *endp = NULL;
 	static const char *nneof = " No newline at end of file\n";
-	const char *old = diff_get_color(ecb->color_diff, DIFF_FILE_OLD);
 	const char *reset = diff_get_color(ecb->color_diff, DIFF_RESET);

 	while (0 < size) {
@@ -444,8 +460,7 @@ static void emit_rewrite_lines(struct emit_callback *ecb,
 		len = endp ? (endp - data + 1) : size;
 		if (prefix != '+') {
 			ecb->lno_in_preimage++;
-			emit_line_0(ecb->file, old, reset, '-',
-				    data, len);
+			emit_sub_line(reset, ecb, data, len);
 		} else {
 			ecb->lno_in_postimage++;
 			emit_add_line(reset, ecb, data, len);
@@ -862,9 +877,12 @@ static void fn_out_consume(void *priv, char
*line, unsigned long len)
 			diff_get_color(ecbdata->color_diff,
 				       line[0] == '-' ? DIFF_FILE_OLD : DIFF_PLAIN);
 		ecbdata->lno_in_preimage++;
-		if (line[0] == ' ')
+		if (line[0] == ' ') {
 			ecbdata->lno_in_postimage++;
-		emit_line(ecbdata->file, color, reset, line, len);
+			emit_line(ecbdata->file, color, reset, line, len);
+		} else {
+			emit_sub_line(reset, ecbdata, line + 1, len - 1);
+		}
 	} else {
 		ecbdata->lno_in_postimage++;
 		emit_add_line(reset, ecbdata, line + 1, len - 1);


But for reasons I don't understand this screws up some of the diff
test cases. It's emitting "index" lines where it shouldn't, and "- No
newline at end of file" instead of "\ No newline at end of file".

I guess something is post-processing the output of the emit functions
and doesn't like the change?

Help appreciated. :-)

j.

^ permalink raw reply related

* Re: Showing whitespace on minus lines of diff ouput
From: Junio C Hamano @ 2010-02-05  3:03 UTC (permalink / raw)
  To: Jay Soffian; +Cc: git
In-Reply-To: <76718491002041747t327bf2f5l85e095244f6ee1ed@mail.gmail.com>

Jay Soffian <jaysoffian@gmail.com> writes:

> The colored diff output is quite helpful to show introductions of whitespace.
>
> However, if whitespace has been removed, it's impossible to see in the
> diff output since whitespace is colorized on '+' lines, but not '-'
> lines.
>
> I'm looking at diff.c, but wow. Can someone more familiar with this
> file point me toward how I'd make a patch to colorize whitespace on
> '-' lines as well?

Totally uninterested.  "diff -R" would be enough, wouldn't it?

^ permalink raw reply

* creating a new branch without an ancestor
From: Michael Wookey @ 2010-02-05  3:19 UTC (permalink / raw)
  To: Git Mailing List

Maybe I'm missing something from reading the docs, but I couldn't see
how to create a new branch in an existing repo that has no ancestor. I
would like to do something like what git.git does with some of the
other ancillary branches like "man", "html", and "todo".

I was hoping to do something like "git branch --no-ancestor
new-branch-name" but didn't see anything in the documentation that
describes the necessary branch options.

Or, am I looking in the wrong place?

^ permalink raw reply

* Re: creating a new branch without an ancestor
From: Larry D'Anna @ 2010-02-05  3:24 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Michael Wookey
In-Reply-To: <d2e97e801002041919r2043d05cjfb114a466605d9a1@mail.gmail.com>

* Michael Wookey (michaelwookey@gmail.com) [100204 22:20]:
> Maybe I'm missing something from reading the docs, but I couldn't see
> how to create a new branch in an existing repo that has no ancestor. I
> would like to do something like what git.git does with some of the
> other ancillary branches like "man", "html", and "todo".
> 
> I was hoping to do something like "git branch --no-ancestor
> new-branch-name" but didn't see anything in the documentation that
> describes the necessary branch options.
> 
> Or, am I looking in the wrong place?

git symbolic-ref HEAD refs/heads/whatever

That'll leave your index and working tree alone of course, so if you did a
commit right after that it would match the content of your current branch but
not the history.

    --larry

^ permalink raw reply

* Re: Showing whitespace on minus lines of diff ouput
From: Jay Soffian @ 2010-02-05  3:28 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7veil0o02z.fsf@alter.siamese.dyndns.org>

On Thu, Feb 4, 2010 at 10:03 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Totally uninterested.  "diff -R" would be enough, wouldn't it?

Duh, I hadn't thought of that.

j.

^ permalink raw reply

* Re: creating a new branch without an ancestor
From: Jay Soffian @ 2010-02-05  3:32 UTC (permalink / raw)
  To: Larry D'Anna; +Cc: Git Mailing List, Michael Wookey
In-Reply-To: <20100205032429.GA23285@cthulhu>

On Thu, Feb 4, 2010 at 10:24 PM, Larry D'Anna <larry@elder-gods.org> wrote:
> * Michael Wookey (michaelwookey@gmail.com) [100204 22:20]:
>> Maybe I'm missing something from reading the docs, but I couldn't see
>> how to create a new branch in an existing repo that has no ancestor. I
>> would like to do something like what git.git does with some of the
>> other ancillary branches like "man", "html", and "todo".
>>
>> I was hoping to do something like "git branch --no-ancestor
>> new-branch-name" but didn't see anything in the documentation that
>> describes the necessary branch options.
>>
>> Or, am I looking in the wrong place?
>
> git symbolic-ref HEAD refs/heads/whatever
>
> That'll leave your index and working tree alone of course, so if you did a
> commit right after that it would match the content of your current branch but
> not the history.

You can also create the branch in a new repo, then just fetch that
into your existing repo.

j.

^ permalink raw reply

* [PATCH] push: Use sideband channel for hook messages
From: Shawn O. Pearce @ 2010-02-05  3:37 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

Rather than sending hook messages over stderr, and losing them
entirely on git:// and smart HTTP transports, receive-pack now
puts them onto a multiplexed sideband channel if the send-pack
client asks for the side-band-64k capablity.  This ensures that
hooks from the server can report their detailed error messages,
if any, no matter what git-aware transport is being used.

When the side band channel is being used the push client will wind up
prefixing all server messages with "remote: ", just like fetch does.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---

 We should have done this back when we added smart HTTP, because
 otherwise the server can't send its hook messages back to the
 client.  I'd argue that is a bug, and so this should go in maint,
 but I can also see how some might consider this a new feature... so
 whatever.  :-)

 builtin-receive-pack.c  |  111 +++++++++++++++++++++++++++++++++++++----------
 builtin-send-pack.c     |   67 +++++++++++++++++++++-------
 run-command.c           |   24 ++++++++--
 run-command.h           |    1 +
 t/t5401-update-hooks.sh |   22 +++++-----
 5 files changed, 170 insertions(+), 55 deletions(-)

diff --git a/builtin-receive-pack.c b/builtin-receive-pack.c
index 78c0e69..e7bdd71 100644
--- a/builtin-receive-pack.c
+++ b/builtin-receive-pack.c
@@ -2,6 +2,7 @@
 #include "pack.h"
 #include "refs.h"
 #include "pkt-line.h"
+#include "sideband.h"
 #include "run-command.h"
 #include "exec_cmd.h"
 #include "commit.h"
@@ -27,11 +28,12 @@ static int receive_unpack_limit = -1;
 static int transfer_unpack_limit = -1;
 static int unpack_limit = 100;
 static int report_status;
+static int use_sideband;
 static int prefer_ofs_delta = 1;
 static int auto_update_server_info;
 static int auto_gc = 1;
 static const char *head_name;
-static char *capabilities_to_send;
+static int send_capabilities = 1;
 
 static enum deny_action parse_deny_action(const char *var, const char *value)
 {
@@ -105,19 +107,21 @@ static int receive_pack_config(const char *var, const char *value, void *cb)
 
 static int show_ref(const char *path, const unsigned char *sha1, int flag, void *cb_data)
 {
-	if (!capabilities_to_send)
+	if (!send_capabilities)
 		packet_write(1, "%s %s\n", sha1_to_hex(sha1), path);
 	else
-		packet_write(1, "%s %s%c%s\n",
-			     sha1_to_hex(sha1), path, 0, capabilities_to_send);
-	capabilities_to_send = NULL;
+		packet_write(1, "%s %s%c%s%s\n",
+			     sha1_to_hex(sha1), path, 0,
+			     " report-status delete-refs side-band-64k",
+			     prefer_ofs_delta ? " ofs-delta" : "");
+	send_capabilities = 0;
 	return 0;
 }
 
 static void write_head_info(void)
 {
 	for_each_ref(show_ref, NULL);
-	if (capabilities_to_send)
+	if (send_capabilities)
 		show_ref("capabilities^{}", null_sha1, 0, NULL);
 
 }
@@ -135,11 +139,25 @@ static struct command *commands;
 static const char pre_receive_hook[] = "hooks/pre-receive";
 static const char post_receive_hook[] = "hooks/post-receive";
 
+static int copy_to_sideband(int in, void *arg)
+{
+	char data[128];
+	while (1) {
+		ssize_t sz = xread(in, data, sizeof(data));
+		if (sz <= 0)
+			break;
+		send_sideband(1, 2, data, sz, use_sideband);
+	}
+	close(in);
+	return 0;
+}
+
 static int run_receive_hook(const char *hook_name)
 {
 	static char buf[sizeof(commands->old_sha1) * 2 + PATH_MAX + 4];
 	struct command *cmd;
 	struct child_process proc;
+	struct async sideband_mux;
 	const char *argv[2];
 	int have_input = 0, code;
 
@@ -159,9 +177,23 @@ static int run_receive_hook(const char *hook_name)
 	proc.in = -1;
 	proc.stdout_to_stderr = 1;
 
+	if (use_sideband) {
+		memset(&sideband_mux, 0, sizeof(sideband_mux));
+		sideband_mux.proc = copy_to_sideband;
+		sideband_mux.is_reader = 1;
+		code = start_async(&sideband_mux);
+		if (code)
+			return code;
+		proc.err = sideband_mux.out;
+	}
+
 	code = start_command(&proc);
-	if (code)
+	if (code) {
+		if (use_sideband)
+			finish_async(&sideband_mux);
 		return code;
+	}
+
 	for (cmd = commands; cmd; cmd = cmd->next) {
 		if (!cmd->error_string) {
 			size_t n = snprintf(buf, sizeof(buf), "%s %s %s\n",
@@ -173,6 +205,8 @@ static int run_receive_hook(const char *hook_name)
 		}
 	}
 	close(proc.in);
+	if (use_sideband)
+		finish_async(&sideband_mux);
 	return finish_command(&proc);
 }
 
@@ -180,6 +214,8 @@ static int run_update_hook(struct command *cmd)
 {
 	static const char update_hook[] = "hooks/update";
 	const char *argv[5];
+	struct child_process proc;
+	int code;
 
 	if (access(update_hook, X_OK) < 0)
 		return 0;
@@ -190,8 +226,18 @@ static int run_update_hook(struct command *cmd)
 	argv[3] = sha1_to_hex(cmd->new_sha1);
 	argv[4] = NULL;
 
-	return run_command_v_opt(argv, RUN_COMMAND_NO_STDIN |
-					RUN_COMMAND_STDOUT_TO_STDERR);
+	memset(&proc, 0, sizeof(proc));
+	proc.no_stdin = 1;
+	proc.stdout_to_stderr = 1;
+	proc.err = use_sideband ? -1 : 0;
+	proc.argv = argv;
+
+	code = start_command(&proc);
+	if (code)
+		return code;
+	if (use_sideband)
+		copy_to_sideband(proc.err, NULL);
+	return finish_command(&proc);
 }
 
 static int is_ref_checked_out(const char *ref)
@@ -380,8 +426,9 @@ static char update_post_hook[] = "hooks/post-update";
 static void run_update_post_hook(struct command *cmd)
 {
 	struct command *cmd_p;
-	int argc, status;
+	int argc;
 	const char **argv;
+	struct child_process proc;
 
 	for (argc = 0, cmd_p = cmd; cmd_p; cmd_p = cmd_p->next) {
 		if (cmd_p->error_string)
@@ -403,8 +450,18 @@ static void run_update_post_hook(struct command *cmd)
 		argc++;
 	}
 	argv[argc] = NULL;
-	status = run_command_v_opt(argv, RUN_COMMAND_NO_STDIN
-			| RUN_COMMAND_STDOUT_TO_STDERR);
+
+	memset(&proc, 0, sizeof(proc));
+	proc.no_stdin = 1;
+	proc.stdout_to_stderr = 1;
+	proc.err = use_sideband ? -1 : 0;
+	proc.argv = argv;
+
+	if (!start_command(&proc)) {
+		if (use_sideband)
+			copy_to_sideband(proc.err, NULL);
+		finish_command(&proc);
+	}
 }
 
 static void execute_commands(const char *unpacker_error)
@@ -464,6 +521,8 @@ static void read_head_info(void)
 		if (reflen + 82 < len) {
 			if (strstr(refname + reflen + 1, "report-status"))
 				report_status = 1;
+			if (strstr(refname + reflen + 1, "side-band-64k"))
+				use_sideband = LARGE_PACKET_MAX;
 		}
 		cmd = xmalloc(sizeof(struct command) + len - 80);
 		hashcpy(cmd->old_sha1, old_sha1);
@@ -563,17 +622,25 @@ static const char *unpack(void)
 static void report(const char *unpack_status)
 {
 	struct command *cmd;
-	packet_write(1, "unpack %s\n",
-		     unpack_status ? unpack_status : "ok");
+	struct strbuf buf = STRBUF_INIT;
+
+	packet_buf_write(&buf, "unpack %s\n",
+			 unpack_status ? unpack_status : "ok");
 	for (cmd = commands; cmd; cmd = cmd->next) {
 		if (!cmd->error_string)
-			packet_write(1, "ok %s\n",
-				     cmd->ref_name);
+			packet_buf_write(&buf, "ok %s\n",
+					 cmd->ref_name);
 		else
-			packet_write(1, "ng %s %s\n",
-				     cmd->ref_name, cmd->error_string);
+			packet_buf_write(&buf, "ng %s %s\n",
+					 cmd->ref_name, cmd->error_string);
 	}
-	packet_flush(1);
+	packet_buf_flush(&buf);
+
+	if (use_sideband)
+		send_sideband(1, 1, buf.buf, buf.len, use_sideband);
+	else
+		safe_write(1, buf.buf, buf.len);
+	strbuf_release(&buf);
 }
 
 static int delete_only(struct command *cmd)
@@ -670,10 +737,6 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
 	else if (0 <= receive_unpack_limit)
 		unpack_limit = receive_unpack_limit;
 
-	capabilities_to_send = (prefer_ofs_delta) ?
-		" report-status delete-refs ofs-delta " :
-		" report-status delete-refs ";
-
 	if (advertise_refs || !stateless_rpc) {
 		add_alternate_refs();
 		write_head_info();
@@ -707,5 +770,7 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
 		if (auto_update_server_info)
 			update_server_info(0);
 	}
+	if (use_sideband)
+		packet_flush(1);
 	return 0;
 }
diff --git a/builtin-send-pack.c b/builtin-send-pack.c
index 8fffdbf..4bbc39f 100644
--- a/builtin-send-pack.c
+++ b/builtin-send-pack.c
@@ -372,6 +372,15 @@ static void print_helper_status(struct ref *ref)
 	strbuf_release(&buf);
 }
 
+static int sideband_decoder_callback(int out, void *data)
+{
+	int *fd = data;
+	int in = fd[0];
+	int ret = recv_sideband("send-pack", in, out);
+	close(out);
+	return ret;
+}
+
 int send_pack(struct send_pack_args *args,
 	      int fd[], struct child_process *conn,
 	      struct ref *remote_refs,
@@ -382,18 +391,22 @@ int send_pack(struct send_pack_args *args,
 	struct strbuf req_buf = STRBUF_INIT;
 	struct ref *ref;
 	int new_refs;
-	int ask_for_status_report = 0;
 	int allow_deleting_refs = 0;
-	int expect_status_report = 0;
+	int status_report = 0;
+	int use_sideband = 0;
+	unsigned cmds_sent = 0;
 	int ret;
+	struct async sideband_decoder;
 
 	/* Does the other end support the reporting? */
 	if (server_supports("report-status"))
-		ask_for_status_report = 1;
+		status_report = 1;
 	if (server_supports("delete-refs"))
 		allow_deleting_refs = 1;
 	if (server_supports("ofs-delta"))
 		args->use_ofs_delta = 1;
+	if (server_supports("side-band-64k"))
+		use_sideband = 1;
 
 	if (!remote_refs) {
 		fprintf(stderr, "No refs in common and none specified; doing nothing.\n"
@@ -456,28 +469,30 @@ int send_pack(struct send_pack_args *args,
 		if (!ref->deletion)
 			new_refs++;
 
-		if (!args->dry_run) {
+		if (args->dry_run) {
+			ref->status = REF_STATUS_OK;
+		} else {
 			char *old_hex = sha1_to_hex(ref->old_sha1);
 			char *new_hex = sha1_to_hex(ref->new_sha1);
 
-			if (ask_for_status_report) {
-				packet_buf_write(&req_buf, "%s %s %s%c%s",
+			if (!cmds_sent && (status_report || use_sideband)) {
+				packet_buf_write(&req_buf, "%s %s %s%c%s%s",
 					old_hex, new_hex, ref->name, 0,
-					"report-status");
-				ask_for_status_report = 0;
-				expect_status_report = 1;
+					status_report ? " report-status" : "",
+					use_sideband ? " side-band-64k" : "");
 			}
 			else
 				packet_buf_write(&req_buf, "%s %s %s",
 					old_hex, new_hex, ref->name);
+			ref->status = status_report ?
+				REF_STATUS_EXPECTING_REPORT :
+				REF_STATUS_OK;
+			cmds_sent++;
 		}
-		ref->status = expect_status_report ?
-			REF_STATUS_EXPECTING_REPORT :
-			REF_STATUS_OK;
 	}
 
 	if (args->stateless_rpc) {
-		if (!args->dry_run) {
+		if (!args->dry_run && cmds_sent) {
 			packet_buf_flush(&req_buf);
 			send_sideband(out, -1, req_buf.buf, req_buf.len, LARGE_PACKET_MAX);
 		}
@@ -487,23 +502,43 @@ int send_pack(struct send_pack_args *args,
 	}
 	strbuf_release(&req_buf);
 
-	if (new_refs && !args->dry_run) {
+	if (use_sideband && cmds_sent) {
+		memset(&sideband_decoder, 0, sizeof(sideband_decoder));
+		sideband_decoder.proc = sideband_decoder_callback;
+		sideband_decoder.data = fd;
+		if (start_async(&sideband_decoder))
+			die("cannot start sideband decoder");
+		in = sideband_decoder.out;
+	}
+
+	if (new_refs && cmds_sent) {
 		if (pack_objects(out, remote_refs, extra_have, args) < 0) {
 			for (ref = remote_refs; ref; ref = ref->next)
 				ref->status = REF_STATUS_NONE;
+			if (use_sideband)
+				finish_async(&sideband_decoder);
 			return -1;
 		}
 	}
-	if (args->stateless_rpc && !args->dry_run)
+	if (args->stateless_rpc && cmds_sent)
 		packet_flush(out);
 
-	if (expect_status_report)
+	if (status_report && cmds_sent)
 		ret = receive_status(in, remote_refs);
 	else
 		ret = 0;
 	if (args->stateless_rpc)
 		packet_flush(out);
 
+	if (use_sideband && cmds_sent) {
+		int err = finish_async(&sideband_decoder);
+		if (err) {
+			error("sideband decoder failed %d", err);
+			ret = -1;
+		}
+		close(sideband_decoder.out);
+	}
+
 	if (ret < 0)
 		return ret;
 	for (ref = remote_refs; ref; ref = ref->next) {
diff --git a/run-command.c b/run-command.c
index cf2d8f7..7d1fd88 100644
--- a/run-command.c
+++ b/run-command.c
@@ -94,6 +94,9 @@ fail_pipe:
 		else if (need_err) {
 			dup2(fderr[1], 2);
 			close_pair(fderr);
+		} else if (cmd->err > 1) {
+			dup2(cmd->err, 2);
+			close(cmd->err);
 		}
 
 		if (cmd->no_stdout)
@@ -228,6 +231,8 @@ fail_pipe:
 
 	if (need_err)
 		close(fderr[1]);
+	else if (cmd->err)
+		close(cmd->err);
 
 	return 0;
 }
@@ -326,10 +331,19 @@ static unsigned __stdcall run_thread(void *data)
 int start_async(struct async *async)
 {
 	int pipe_out[2];
+	int proc_fd, call_fd;
 
 	if (pipe(pipe_out) < 0)
 		return error("cannot create pipe: %s", strerror(errno));
-	async->out = pipe_out[0];
+
+	if (async->is_reader) {
+		proc_fd = pipe_out[0];
+		call_fd = pipe_out[1];
+	} else {
+		call_fd = pipe_out[0];
+		proc_fd = pipe_out[1];
+	}
+	async->out = call_fd;
 
 #ifndef WIN32
 	/* Flush stdio before fork() to avoid cloning buffers */
@@ -342,12 +356,12 @@ int start_async(struct async *async)
 		return -1;
 	}
 	if (!async->pid) {
-		close(pipe_out[0]);
-		exit(!!async->proc(pipe_out[1], async->data));
+		close(call_fd);
+		exit(!!async->proc(proc_fd, async->data));
 	}
-	close(pipe_out[1]);
+	close(proc_fd);
 #else
-	async->fd_for_proc = pipe_out[1];
+	async->fd_for_proc = proc_fd;
 	async->tid = (HANDLE) _beginthreadex(NULL, 0, run_thread, async, 0, NULL);
 	if (!async->tid) {
 		error("cannot create thread: %s", strerror(errno));
diff --git a/run-command.h b/run-command.h
index fb34209..8aed83f 100644
--- a/run-command.h
+++ b/run-command.h
@@ -70,6 +70,7 @@ struct async {
 	int (*proc)(int fd, void *data);
 	void *data;
 	int out;	/* caller reads from here and closes it */
+	unsigned is_reader:1;
 #ifndef WIN32
 	pid_t pid;
 #else
diff --git a/t/t5401-update-hooks.sh b/t/t5401-update-hooks.sh
index 64f66c9..c3cf397 100755
--- a/t/t5401-update-hooks.sh
+++ b/t/t5401-update-hooks.sh
@@ -118,19 +118,19 @@ test_expect_success 'send-pack produced no output' '
 '
 
 cat <<EOF >expect
-STDOUT pre-receive
-STDERR pre-receive
-STDOUT update refs/heads/master
-STDERR update refs/heads/master
-STDOUT update refs/heads/tofail
-STDERR update refs/heads/tofail
-STDOUT post-receive
-STDERR post-receive
-STDOUT post-update
-STDERR post-update
+remote: STDOUT pre-receive
+remote: STDERR pre-receive
+remote: STDOUT update refs/heads/master
+remote: STDERR update refs/heads/master
+remote: STDOUT update refs/heads/tofail
+remote: STDERR update refs/heads/tofail
+remote: STDOUT post-receive
+remote: STDERR post-receive
+remote: STDOUT post-update
+remote: STDERR post-update
 EOF
 test_expect_success 'send-pack stderr contains hook messages' '
-	grep ^STD send.err >actual &&
+	grep ^remote: send.err | sed "s/ *\$//" >actual &&
 	test_cmp - actual <expect
 '
 
-- 
1.7.0.rc1.199.g9253ab

-- 
Shawn.

^ permalink raw reply related

* Re: [PATCH] push: Use sideband channel for hook messages
From: Junio C Hamano @ 2010-02-05  5:53 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git
In-Reply-To: <20100205033748.GA19255@spearce.org>

"Shawn O. Pearce" <spearce@spearce.org> writes:

> Rather than sending hook messages over stderr, and losing them
> entirely on git:// and smart HTTP transports, receive-pack now
> puts them onto a multiplexed sideband channel if the send-pack
> client asks for the side-band-64k capablity.  This ensures that
> hooks from the server can report their detailed error messages,
> if any, no matter what git-aware transport is being used.
>
> When the side band channel is being used the push client will wind up
> prefixing all server messages with "remote: ", just like fetch does.
>
> Signed-off-by: Shawn O. Pearce <spearce@spearce.org>

This feels to me a topic that has multiple unrelated changes mixed
together, making it unnecessarily confusing to review.

 - capabilities_to_send -> send_capabilities;

 - use of sideband in communication from send-pack to receive-pack;

 - adding communication in the opposite direction to async infrastructure;

 - use_sideband that is a boolean but is also used to hold
   LARGE_PACKET_MAX (on the receiving end).

^ permalink raw reply

* Re: Showing whitespace on minus lines of diff ouput
From: Junio C Hamano @ 2010-02-05  6:02 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jay Soffian, git
In-Reply-To: <7veil0o02z.fsf@alter.siamese.dyndns.org>

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

> Jay Soffian <jaysoffian@gmail.com> writes:
>
>> The colored diff output is quite helpful to show introductions of whitespace.
>>
>> However, if whitespace has been removed, it's impossible to see in the
>> diff output since whitespace is colorized on '+' lines, but not '-'
>> lines.
>>
>> I'm looking at diff.c, but wow. Can someone more familiar with this
>> file point me toward how I'd make a patch to colorize whitespace on
>> '-' lines as well?
>
> Totally uninterested.  "diff -R" would be enough, wouldn't it?

Actually the reason I don't like the idea to highlight corrected errors
was not just because "'diff -R' is enough".

The coloring of whitespace errors are like compiler warnings, at least to
me, and giving similar highlighting to corrected ones defeats the purpose
by making the output more distracting..

^ permalink raw reply

* Re: Showing whitespace on minus lines of diff ouput
From: Jay Soffian @ 2010-02-05  6:17 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vr5p0kyp0.fsf@alter.siamese.dyndns.org>

On Fri, Feb 5, 2010 at 1:02 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Actually the reason I don't like the idea to highlight corrected errors
> was not just because "'diff -R' is enough".
>
> The coloring of whitespace errors are like compiler warnings, at least to
> me, and giving similar highlighting to corrected ones defeats the purpose
> by making the output more distracting..

That makes perfect sense. I didn't think it through very well. :-)

j.

^ permalink raw reply

* reverting vs resetting
From: Mihamina Rakotomandimby @ 2010-02-05  6:12 UTC (permalink / raw)
  To: Git Mailing List

Manao ahoana, Hello, Bonjour,

We put a gateway iptables script (SNAT, DNAT,...) under GIT.

Someone (Bob) made a mistake on it, but he did not know. That broke the
gateway.

Another one (Andy), did not try to undo Bob's mistake but tried (and
tried, and tried... always committing) to solve the problem. Andy made
about 50 commits.

When we decided to giveup, we decided to get back to the configuration
before Bob broke it. But it would be interesting to see Andy's history.

Reverting or resetting?

As far as I know: 
- Reverting Bob's commit will only cancel Bob's mistake but not Andy's
  tries to solve it.
- Resetting will forget the history.

Any suggestion?

Misaotra, Thanks, Merci.

-- 
       Architecte Informatique chez Blueline/Gulfsat:
    Administration Systeme, Recherche & Developpement
                +261 34 29 155 34 / +261 33 11 207 36

^ permalink raw reply

* Re: reverting vs resetting
From: Avery Pennarun @ 2010-02-05  6:48 UTC (permalink / raw)
  To: Mihamina Rakotomandimby; +Cc: Git Mailing List
In-Reply-To: <20100205091223.6b4cffb1@pbmiha.malagasy.com>

On Fri, Feb 5, 2010 at 1:12 AM, Mihamina Rakotomandimby
<mihamina@gulfsat.mg> wrote:
> When we decided to giveup, we decided to get back to the configuration
> before Bob broke it. But it would be interesting to see Andy's history.
>
> Reverting or resetting?

Try this:

    git checkout HEAD~5 .
    git commit -m "reverted back to the version before this mess"

Replace HEAD~5 with the revision you want it to look like.  But don't
forget the "." in the checkout line!  All the magic is in there.  When
you specify a path ("."), git won't change the revision HEAD points
to, but it will change all the files and the index to the ones from
the specified version.  Then you can commit these files, producing
effectively a single commit that undoes a bunch of previous ones.

Have fun,

Avery

^ 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