Git development
 help / color / mirror / Atom feed
* Re: Considering teaching plumbing to users harmful
From: Andreas Ericsson @ 2008-07-18  8:19 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git
In-Reply-To: <alpine.DEB.1.00.0807161804400.8950@racer>

Johannes Schindelin wrote:
> Hi,
> 
> there have been a number of occasions where I came across people trying to 
> be helpful and teaching Git newbies a few tricks.
> 
> However, in quite a number of cases, which seem to surge over the last 
> weeks, I see people suggesting the use of rev-parse, ls-tree, rev-list 
> etc.
> 
> Their rationale is invariably "but I found it useful", and they seem to be 
> unable to recognize the puzzlement in the faces of the people they are 
> trying to help.
> 
> Instead they insist that they did nothing wrong.
> 
> I had the pleasure of introducing Git to a few users in the last months 
> and in my opinion, restricting myself to teaching them these commands 
> first helped tremendously:
> 
> - clone, pull, status, add, commit, push, log
> 
> All of these were presented without options, to keep things simple.
> 
> In particular, I refrained from giving them the "-a" option to commit.  
> That seemed to help incredibly with their embracing the index as a natural 
> concept (which it is).
> 
> Often I presented the "pull" and "push" commands _only_ with "origin 
> master" ("origin is where the repository came from, and master is the 
> branch; you will want to use other parameters here after you used Git for 
> a while").
> 
> _After_ they grew comfortable with Git, I taught them a few options here 
> and there, not hiding, but also not promoting the full range of options.
> 
> So the next tricks were
> 
> - log -p, rm, diff, diff --cached, show
> 
> The last one is "show", and with that command, I taught the 
> "<commit>:" and "<commit>:<file>" syntax, too (which some Git old-timers 
> did not know about ;-)
> 

Thanks for the excellent write-up. I wish I'd had this when I did the
introductory courses at my dayjob. With those simple commands, 90%
of the users get access to 90% of the usefulness of git, imo. And,
more importantly, it's enough to get them started right away.


> The pace needed to be adjusted to the users, in my experience, but not the 
> order.
> 
> Now, it makes me really, really sad that Git has a reputation of being 
> complicated, but I regularly hear from _my_ users that they do not 
> understand how that came about.
> 
> Am I the only one who deems teaching plumbing to users ("I like it raw!  
> So I teach it the same way!") harmful?
> 

I wholeheartedly agree. Telling people about "git rev-list" on day one
is probably the single greatest mistake I've ever done wrt git. To the
non-gitizen, it takes some mumbo-jumbo arguments and spits out a long
list of mumbo-jumbo output. Had I started with "git log" instead, it
would have been infinitely easier to explain how each commit has a
totally unique name.

In addition, I'd recommend setting
color.branch=auto
color.diff=auto
color.pager=true
color.status=true
before starting the "course". It makes the learning experience a whole
lot nicer.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

^ permalink raw reply

* Re: stgit: bug with refresh and -p
From: Karl Hasselström @ 2008-07-18  8:41 UTC (permalink / raw)
  To: Jon Smirl; +Cc: Git Mailing List, Catalin Marinas
In-Reply-To: <9e4733910807171829q6abdcfc2m90c40a70dbc8fef5@mail.gmail.com>

On 2008-07-17 21:29:01 -0400, Jon Smirl wrote:

> A refresh with -p is not picking up newly added files.
>
> jonsmirl@terra:~/fs$ stg status
> ? include/linux/i2c/max9485.h
> jonsmirl@terra:~/fs$ git add include/linux/i2c/max9485.h
> jonsmirl@terra:~/fs$ stg refresh -p max9485
> Checking for changes in the working directory ... done
> Popping patches "jds-audio" - "jds-psc" ... done
> Refreshing patch "max9485" ... done
> Fast-forwarded patches "jds-psc" - "jds-audio"
> jonsmirl@terra:~/fs$ stg status
> ? include/linux/i2c/max9485.h
> jonsmirl@terra:~/fs$ stg --version
> Stacked GIT 0.14.3.163.g06f9

Hmm, I don't immediately see why this happens. I've posted a bug about
it: https://gna.org/bugs/index.php?12038

It's fixed by the refresh rewrite in my experimental branch, but
there's no test for it, so I had to check by hand.

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle

^ permalink raw reply

* Re: [PATCH] Teach git submodule update to use distributed repositories
From: Nigel Magnay @ 2008-07-18  8:11 UTC (permalink / raw)
  To: Petr Baudis; +Cc: Johannes Schindelin, Git Mailing List
In-Reply-To: <20080717182253.GZ32184@machine.or.cz>

On Thu, Jul 17, 2008 at 7:22 PM, Petr Baudis <pasky@suse.cz> wrote:
> On Thu, Jul 17, 2008 at 04:07:11PM +0100, Nigel Magnay wrote:
>> And it works, but
>>
>> $ git pull fred
>> $ git submodule update
>>
>> Can leave you with problems, because if a submodule wasn't pushed to
>> origin, you won't have it available. This is because the commands are
>> equivalent to
>>
>> $ git pull fred
>> for each submodule()
>>   cd submodule
>>   git fetch origin
>>   git checkout <sha1>
>
> Oh! So, only after replying to most of your mail, I have realized what
> are you talking about all the time - _just_ this particular failure
> mode:
>
>        "Someone pushed out a repository repointing submodules to
>        invalid commits, and instead of waiting for the person to fix
>        this breakage, we want to do a one-off fetch of all submodules
>        from a different repository."
>
> There's nothing else you're trying to solve by this, right?
>

No.
"Someone says 'please review the state of my tree, _before_ I push it
out to a (central) repository"

Fred is a person (and != origin). His tree(s) are entirely correct and
consistent, and he doesn't yet wish to push to origin (and perhaps he
cannot, because he does not have permission to do so).

All the tutorials give credit to the fact that in git you don't need a
central server - you can pull directly from people. Except in the case
where you're using submodules, where you're basically forced to
hand-modify .git/config (in this instance, to point to where 'fred' is
storing his submodule trees) before doing a submodule update. This
makes git complicated for users.

I'm trying to improve the UI for projects using submodules to make it
mostly transparent; the best way I can come up with is to pick on
individual usecases and show that they're a particular pain and that
perhaps they don't need to be.

>
> Now, I think that this is a completely wrong problem to solve. Your
> gitweb is going to be broken, everyone has to jump through hoops because
> of this, and that all just because of a single mistake. It shouldn't
> have _happenned_ in the first place.
>
> So the proper solution for this should be to make an update hook that
> will simply not _let_ you push out a tree that's broken like this.
> Something like this (completely untested):
>
> die() { echo "$@"; exit 1; }
> git rev-list ^$2 $3 | while read commit; do
>        git show $commit:.gitmodules >/tmp/gm$$
>        git config -f /tmp/gm$$ --get-regexp 'submodule\..*\.path' |
>                cut -d ' ' -f 1 |
>                sed 's/^.*\.//; s/\..*$//;' |
>                while read submodule; do
>                        path=$(git config -f /tmp/gm$$ "submodule.$submodule.path")
>                        url=$(git config -f /tmp/gm$$ "submodule.$submodule.url")
>                        entry=$(git ls-tree $commit "$path")
>                        [ -n "$entry" ] || die "submodule $submodule points at a non-existing path"
>                        [ "$(echo "$entry" | cut -d ' ' -f 1)" = "160000" ] || die "submodule $submodule does not point to a gitlink entry"
>
>                        subcommit="$(echo "$entry" | cut -d ' ' -f 2)"
>                        urlhash="$(echo "$url" | sha1sum | cut -d ' ' -f 1)"
>                        # We keep local copies of submodule repositories
>                        # for commit existence checking
>                        echo "Please wait, updating $url cache..."
>                        if [ -d /tmp/ucache/$urlhash ]; then
>                                (cd /tmp/ucache/$urlhash && git fetch)
>                        else
>                                git clone --bare "$url" /tmp/ucache/$urlhash
>                        fi
>                        [ "$(git --git-dir=/tmp/ucache/$urlhash cat-file -t "$subcommit" 2>/dev/null)" = "commit" ] || die "submodule $submodule does not point at an existing commit"
>                done
>        done
>
> Comments? If it seems good, it might be worth including in
> contrib/hooks/. Maybe even in the default update hook, controlled by
> a config option.
>
> All the troubles here stem from the fact that normally, Git will not let
> you push any invalid state to the server. This is not completely true in
> this case, but we should prevent this behaviour instead of inventing
> hacks to work it around.
>
>> Unless each submodule had a [remote] specified for "fred", you'd be
>> stuffed. But what you could do is either by passing the right URL, or
>> looking at the superproject [remote] for "fred" - i.e: If in the
>> superproject you have
>>
>> [remote "fred"]
>>         url = ssh://git@fred.local/pub/scm/git/workspace/thing/.git
>> [submodule "module"]
>>         url = ssh://git@repo/pub/scm/git/module.git
>>
>> Then the submodule "module" on fred, if it's a working-copy, can be calculated
>>        ssh://git@fred.local/pub/scm/git/workspace/thing/module/.git
>>
>> If it isn't a WC then you'd have to have a [remote "fred"] in that
>> submodule, but I'm thinking that'd be a rare case.
>
> This is ultra-evil. I think assuming things like this is extremely dirty
> and not reasonable for a universal code, _unless_ we explicitly decide
> that this is a new convention you want to introduce as a recommendation.
> But you should've been very clear about this upfront.
>
> _If_ you still insist on the one-off fetches for some reason, I think
> it's reasonable to provide your own simple script for your users that
> will autogenerate these URLs appropriately for your particular setup.
> I don't think there is any real need for a more generic solution.
>
>> I'd assumed (possibly wrongly?) that there was resistance to putting
>> any of the submodule logic in things other than git-submodules.
>
> Are you following the thread about submodule support for git mv, git rm?
>
> --
>                                Petr "Pasky" Baudis
> GNU, n. An animal of South Africa, which in its domesticated state
> resembles a horse, a buffalo and a stag. In its wild condition it is
> something like a thunderbolt, an earthquake and a cyclone. -- A. Pierce
>

^ permalink raw reply

* Re: Git bash - is this MSYS bash or something else?
From: Lennart Borgman (gmail) @ 2008-07-18  8:06 UTC (permalink / raw)
  To: Steffen Prohaska; +Cc: git
In-Reply-To: <288BA074-4995-47FB-BD22-75D36BF77A44@zib.de>

Steffen Prohaska wrote:
> 
> On Jul 18, 2008, at 2:47 AM, Lennart Borgman (gmail) wrote:
> 
>> I wonder if the bash.exe that comes with GIT is MSYS bash or a rewrite?
> 
> It is the MSYS bash.

Thanks Steffen.

>> Does it follow the usual MSYS bash rules?
> 
> What does this mean?

I just wondered if something was changed regarding starting of programs 
from MSYS bash.

^ permalink raw reply

* Re: GSoC podcast 18: Git
From: Matthieu Moy @ 2008-07-18  7:56 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, spearce
In-Reply-To: <alpine.DEB.1.00.0807170133150.4318@eeepc-johanness>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Hi list,
>
> you might be interested in listening to Leslie Hawthorn interviewing Git's 
> GSoC administrator (Shawn) and his lazy bum^Wbackup administrator (yours 
> truly).
>
> I haven't seen it being mentioned on the official blog, but after waiting 
> for over an hour, I figured I could just send the direct link:
>
> http://google-developer-podcast.googlecode.com/files/gsocpodcast018.ogg

Interesting.

Didn't listen the whole thing yet, but there's a sentence from Shawn
that summarizes really well the point behind private Vs public stuff:

 "Make it look like you are a better programmer than you really are."

-- 
Matthieu

^ permalink raw reply

* Re: Considering teaching plumbing to users harmful
From: Dmitry Potapov @ 2008-07-18  7:41 UTC (permalink / raw)
  To: David Kastrup; +Cc: git
In-Reply-To: <86k5fk1ooq.fsf@lola.quinscape.zz>

On Thu, Jul 17, 2008 at 06:05:25PM +0200, David Kastrup wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
> 
> > Distinguishing between branch part of directory name by _convention_
> > is design mistake; the fact that the tool doesn't help to ensure that
> > (a) tags lie on branch (b) tags _doesn't change_ is an example of this
> > stupidity.
> 
> How much have you worked with Subversion so far?  I am doing quite a bit
> of work with it, and the do-everything-via-copying paradigm does not get
> in my hair.  It actually means that I have to remember fewer commands.
> And it is pretty easy to understand.

Staying on trunk, try to compare your files foo and bar with version
1.0. With Git, it is simple
git v1.0 -- foo bar
but I don't think you can do that so easily with SVN.

But the real problem with the do-everything-via-copying paradigm is
that now revisions do not identify anything meaningful without some
path. This leads to problems if you try to implement merge. In Git,
it is simple -- merge is a commit with more than one parent. You cannot
do like that in SVN. Instead, you have to track merges per file. This
is not just waste of resources, but this merges is very difficult if
not impossible to visulize in any useful way. But if you cannot see
something, you cannot control it well. So, not accidently, in systems
with inter-file branching, creating feature branches is discouraged.

Besides having fewer commands doesn't necessary mean easier to learn
or to use. If you have one command that conflates different concepts,
it is usually more difficult than having a devote command per concept.

Can you remove the concept of branches and merges and have your VCS
still useful? Well, you can say "we don't use branches, every developer
commits directly on trunk". The consequence of that choice is that a lot
of work-in-progress code is pushed to trunk. But no one has the perfect
foresight. Some ideas will turn out to be not so good. Some developers
will not finish their work and will be reassigned to more urgent tasks.
As result, just as release time approaches, you have a lot of unfinished
crap in your trunk. Of course, if you don't care about quality of your
software, it may be easier for you to work in this way.

However, if you do care about quality, you have to use feature branches
for WIP, and for this workflow to work, you need that merging is really
easy.  Git makes that for you, yet you may need to learn new concepts --
branching and merging.  Thus comparision of what is easier or difficult
is meaningless without defining goals. It is always easy to be sloppy
and don't care...

Dmitry

^ permalink raw reply

* [PATCH] Add ANSI control code emulation for the Windows console
From: Johannes Sixt @ 2008-07-18  7:34 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Peter Harris, Johannes Sixt
In-Reply-To: <1216366485-12201-3-git-send-email-johannes.sixt@telecom.at>

From: Peter Harris <git@peter.is-a-geek.org>

This adds only the minimum necessary to keep git pull/merge's diffstat from
wrapping. Notably absent is support for the K (erase) operation, and support
for POSIX write.

Signed-off-by: Peter Harris <git@peter.is-a-geek.org>
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
 Makefile         |    2 +-
 compat/mingw.h   |   11 ++
 compat/winansi.c |  345 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 357 insertions(+), 1 deletions(-)
 create mode 100644 compat/winansi.c

diff --git a/Makefile b/Makefile
index 78e08d3..2c6cc9d 100644
--- a/Makefile
+++ b/Makefile
@@ -740,7 +740,7 @@ ifneq (,$(findstring MINGW,$(uname_S)))
 	COMPAT_CFLAGS += -D__USE_MINGW_ACCESS -DNOGDI -Icompat
 	COMPAT_CFLAGS += -DSNPRINTF_SIZE_CORR=1
 	COMPAT_CFLAGS += -DSTRIP_EXTENSION=\".exe\"
-	COMPAT_OBJS += compat/mingw.o compat/fnmatch.o compat/regex.o
+	COMPAT_OBJS += compat/mingw.o compat/fnmatch.o compat/regex.o compat/winansi.o
 	EXTLIBS += -lws2_32
 	X = .exe
 	template_dir = ../share/git-core/templates/
diff --git a/compat/mingw.h b/compat/mingw.h
index 6bc049a..5f11114 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -194,6 +194,17 @@ sig_handler_t mingw_signal(int sig, sig_handler_t handler);
 #define signal mingw_signal
 
 /*
+ * ANSI emulation wrappers
+ */
+
+int winansi_fputs(const char *str, FILE *stream);
+int winansi_printf(const char *format, ...) __attribute__((format (printf, 1, 2)));
+int winansi_fprintf(FILE *stream, const char *format, ...) __attribute__((format (printf, 2, 3)));
+#define fputs winansi_fputs
+#define printf(...) winansi_printf(__VA_ARGS__)
+#define fprintf(...) winansi_fprintf(__VA_ARGS__)
+
+/*
  * git specific compatibility
  */
 
diff --git a/compat/winansi.c b/compat/winansi.c
new file mode 100644
index 0000000..e2d96df
--- /dev/null
+++ b/compat/winansi.c
@@ -0,0 +1,345 @@
+/*
+ * Copyright 2008 Peter Harris <git@peter.is-a-geek.org>
+ */
+
+#include <windows.h>
+#include "../git-compat-util.h"
+
+/*
+ Functions to be wrapped:
+*/
+#undef printf
+#undef fprintf
+#undef fputs
+/* TODO: write */
+
+/*
+ ANSI codes used by git: m, K
+
+ This file is git-specific. Therefore, this file does not attempt
+ to implement any codes that are not used by git.
+
+ TODO: K
+*/
+
+static HANDLE console;
+static WORD plain_attr;
+static WORD attr;
+static int negative;
+
+static void init(void)
+{
+	CONSOLE_SCREEN_BUFFER_INFO sbi;
+
+	static int initialized = 0;
+	if (initialized)
+		return;
+
+	console = GetStdHandle(STD_OUTPUT_HANDLE);
+	if (console == INVALID_HANDLE_VALUE)
+		console = NULL;
+
+	if (!console)
+		return;
+
+	GetConsoleScreenBufferInfo(console, &sbi);
+	attr = plain_attr = sbi.wAttributes;
+	negative = 0;
+
+	initialized = 1;
+}
+
+
+#define FOREGROUND_ALL (FOREGROUND_RED | FOREGROUND_GREEN | FOREGROUND_BLUE)
+#define BACKGROUND_ALL (BACKGROUND_RED | BACKGROUND_GREEN | BACKGROUND_BLUE)
+
+static void set_console_attr(void)
+{
+	WORD attributes = attr;
+	if (negative) {
+		attributes &= ~FOREGROUND_ALL;
+		attributes &= ~BACKGROUND_ALL;
+
+		/* This could probably use a bitmask
+		   instead of a series of ifs */
+		if (attr & FOREGROUND_RED)
+			attributes |= BACKGROUND_RED;
+		if (attr & FOREGROUND_GREEN)
+			attributes |= BACKGROUND_GREEN;
+		if (attr & FOREGROUND_BLUE)
+			attributes |= BACKGROUND_BLUE;
+
+		if (attr & BACKGROUND_RED)
+			attributes |= FOREGROUND_RED;
+		if (attr & BACKGROUND_GREEN)
+			attributes |= FOREGROUND_GREEN;
+		if (attr & BACKGROUND_BLUE)
+			attributes |= FOREGROUND_BLUE;
+	}
+	SetConsoleTextAttribute(console, attributes);
+}
+
+static const char *set_attr(const char *str)
+{
+	const char *func;
+	size_t len = strspn(str, "0123456789;");
+	func = str + len;
+
+	switch (*func) {
+	case 'm':
+		do {
+			long val = strtol(str, (char **)&str, 10);
+			switch (val) {
+			case 0: /* reset */
+				attr = plain_attr;
+				negative = 0;
+				break;
+			case 1: /* bold */
+				attr |= FOREGROUND_INTENSITY;
+				break;
+			case 2:  /* faint */
+			case 22: /* normal */
+				attr &= ~FOREGROUND_INTENSITY;
+				break;
+			case 3:  /* italic */
+				/* Unsupported */
+				break;
+			case 4:  /* underline */
+			case 21: /* double underline */
+				/* Wikipedia says this flag does nothing */
+				/* Furthermore, mingw doesn't define this flag
+				attr |= COMMON_LVB_UNDERSCORE; */
+				break;
+			case 24: /* no underline */
+				/* attr &= ~COMMON_LVB_UNDERSCORE; */
+				break;
+			case 5:  /* slow blink */
+			case 6:  /* fast blink */
+				/* We don't have blink, but we do have
+				   background intensity */
+				attr |= BACKGROUND_INTENSITY;
+				break;
+			case 25: /* no blink */
+				attr &= ~BACKGROUND_INTENSITY;
+				break;
+			case 7:  /* negative */
+				negative = 1;
+				break;
+			case 27: /* positive */
+				negative = 0;
+				break;
+			case 8:  /* conceal */
+			case 28: /* reveal */
+				/* Unsupported */
+				break;
+			case 30: /* Black */
+				attr &= ~FOREGROUND_ALL;
+				break;
+			case 31: /* Red */
+				attr &= ~FOREGROUND_ALL;
+				attr |= FOREGROUND_RED;
+				break;
+			case 32: /* Green */
+				attr &= ~FOREGROUND_ALL;
+				attr |= FOREGROUND_GREEN;
+				break;
+			case 33: /* Yellow */
+				attr &= ~FOREGROUND_ALL;
+				attr |= FOREGROUND_RED | FOREGROUND_GREEN;
+				break;
+			case 34: /* Blue */
+				attr &= ~FOREGROUND_ALL;
+				attr |= FOREGROUND_BLUE;
+				break;
+			case 35: /* Magenta */
+				attr &= ~FOREGROUND_ALL;
+				attr |= FOREGROUND_RED | FOREGROUND_BLUE;
+				break;
+			case 36: /* Cyan */
+				attr &= ~FOREGROUND_ALL;
+				attr |= FOREGROUND_GREEN | FOREGROUND_BLUE;
+				break;
+			case 37: /* White */
+				attr |= FOREGROUND_RED |
+					FOREGROUND_GREEN |
+					FOREGROUND_BLUE;
+				break;
+			case 38: /* Unknown */
+				break;
+			case 39: /* reset */
+				attr &= ~FOREGROUND_ALL;
+				attr |= (plain_attr & FOREGROUND_ALL);
+				break;
+			case 40: /* Black */
+				attr &= ~BACKGROUND_ALL;
+				break;
+			case 41: /* Red */
+				attr &= ~BACKGROUND_ALL;
+				attr |= BACKGROUND_RED;
+				break;
+			case 42: /* Green */
+				attr &= ~BACKGROUND_ALL;
+				attr |= BACKGROUND_GREEN;
+				break;
+			case 43: /* Yellow */
+				attr &= ~BACKGROUND_ALL;
+				attr |= BACKGROUND_RED | BACKGROUND_GREEN;
+				break;
+			case 44: /* Blue */
+				attr &= ~BACKGROUND_ALL;
+				attr |= BACKGROUND_BLUE;
+				break;
+			case 45: /* Magenta */
+				attr &= ~BACKGROUND_ALL;
+				attr |= BACKGROUND_RED | BACKGROUND_BLUE;
+				break;
+			case 46: /* Cyan */
+				attr &= ~BACKGROUND_ALL;
+				attr |= BACKGROUND_GREEN | BACKGROUND_BLUE;
+				break;
+			case 47: /* White */
+				attr |= BACKGROUND_RED |
+					BACKGROUND_GREEN |
+					BACKGROUND_BLUE;
+				break;
+			case 48: /* Unknown */
+				break;
+			case 49: /* reset */
+				attr &= ~BACKGROUND_ALL;
+				attr |= (plain_attr & BACKGROUND_ALL);
+				break;
+			default:
+				/* Unsupported code */
+				break;
+			}
+			str++;
+		} while (*(str-1) == ';');
+
+		set_console_attr();
+		break;
+	case 'K':
+		/* TODO */
+		break;
+	default:
+		/* Unsupported code */
+		break;
+	}
+
+	return func + 1;
+}
+
+static int ansi_emulate(const char *str, FILE *stream)
+{
+	int rv = 0;
+	const char *pos = str;
+
+	while (*pos) {
+		pos = strstr(str, "\033[");
+		if (pos) {
+			size_t len = pos - str;
+
+			if (len) {
+				size_t out_len = fwrite(str, 1, len, stream);
+				rv += out_len;
+				if (out_len < len)
+					return rv;
+			}
+
+			str = pos + 2;
+			rv += 2;
+
+			fflush(stream);
+
+			pos = set_attr(str);
+			rv += pos - str;
+			str = pos;
+		} else {
+			rv += strlen(str);
+			fputs(str, stream);
+			return rv;
+		}
+	}
+	return rv;
+}
+
+int winansi_fputs(const char *str, FILE *stream)
+{
+	int rv;
+
+	if (!isatty(fileno(stream)))
+		return fputs(str, stream);
+
+	init();
+
+	if (!console)
+		return fputs(str, stream);
+
+	rv = ansi_emulate(str, stream);
+
+	if (rv >= 0)
+		return 0;
+	else
+		return EOF;
+}
+
+static int winansi_vfprintf(FILE *stream, const char *format, va_list list)
+{
+	int len, rv;
+	char small_buf[256];
+	char *buf = small_buf;
+	va_list cp;
+
+	if (!isatty(fileno(stream)))
+		goto abort;
+
+	init();
+
+	if (!console)
+		goto abort;
+
+	va_copy(cp, list);
+	len = vsnprintf(small_buf, sizeof(small_buf), format, cp);
+	va_end(cp);
+
+	if (len > sizeof(small_buf) - 1) {
+		buf = malloc(len + 1);
+		if (!buf)
+			goto abort;
+
+		len = vsnprintf(buf, len + 1, format, list);
+	}
+
+	rv = ansi_emulate(buf, stream);
+
+	if (buf != small_buf)
+		free(buf);
+	return rv;
+
+abort:
+	rv = vfprintf(stream, format, list);
+	return rv;
+}
+
+int winansi_fprintf(FILE *stream, const char *format, ...)
+{
+	va_list list;
+	int rv;
+
+	va_start(list, format);
+	rv = winansi_vfprintf(stream, format, list);
+	va_end(list);
+
+	return rv;
+}
+
+int winansi_printf(const char *format, ...)
+{
+	va_list list;
+	int rv;
+
+	va_start(list, format);
+	rv = winansi_vfprintf(stdout, format, list);
+	va_end(list);
+
+	return rv;
+}
-- 
1.5.6.1.275.g0a3e0f

^ permalink raw reply related

* [PATCH] Teach lookup_prog not to select directories
From: Johannes Sixt @ 2008-07-18  7:34 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Eric Raible, Johannes Sixt
In-Reply-To: <1216366485-12201-1-git-send-email-johannes.sixt@telecom.at>

From: Eric Raible <raible@gmail.com>

Without this simple fix "git gui" in the git source directory
finds the git-gui directory instead of the tcl script in /usr/bin.

Signed-off-by: Eric Raible <raible@gmail.com>
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
 compat/mingw.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/compat/mingw.c b/compat/mingw.c
index 3a05fe7..bb33c8d 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -536,7 +536,8 @@ static char *lookup_prog(const char *dir, const char *cmd, int isexe, int exe_on
 		return xstrdup(path);
 	path[strlen(path)-4] = '\0';
 	if ((!exe_only || isexe) && access(path, F_OK) == 0)
-		return xstrdup(path);
+		if (!(GetFileAttributes(path) & FILE_ATTRIBUTE_DIRECTORY))
+			return xstrdup(path);
 	return NULL;
 }
 
-- 
1.5.6.1.275.g0a3e0f

^ permalink raw reply related

* [PATCH] builtin-clone: Use is_dir_sep() instead of '/'
From: Johannes Sixt @ 2008-07-18  7:34 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Johannes Sixt
In-Reply-To: <1216366485-12201-2-git-send-email-johannes.sixt@telecom.at>

Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
 builtin-clone.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/builtin-clone.c b/builtin-clone.c
index 643c7d4..fddf47f 100644
--- a/builtin-clone.c
+++ b/builtin-clone.c
@@ -115,7 +115,7 @@ static char *guess_dir_name(const char *repo, int is_bundle)
 			if (!after_slash_or_colon)
 				end = p;
 			p += 7;
-		} else if (*p == '/' || *p == ':') {
+		} else if (is_dir_sep(*p) || *p == ':') {
 			if (end == limit)
 				end = p;
 			after_slash_or_colon = 1;
-- 
1.5.6.1.275.g0a3e0f

^ permalink raw reply related

* [PATCH] Windows related patches
From: Johannes Sixt @ 2008-07-18  7:34 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Johannes Sixt

Eric Raible (1):
  Teach lookup_prog not to select directories

Johannes Sixt (2):
  builtin-clone: Use is_dir_sep() instead of '/'
  Windows: set gitexecdir = $(bindir)

Peter Harris (1):
  Add ANSI control code emulation for the Windows console

^ permalink raw reply

* [PATCH] Windows: set gitexecdir = $(bindir)
From: Johannes Sixt @ 2008-07-18  7:34 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Johannes Sixt
In-Reply-To: <1216366485-12201-4-git-send-email-johannes.sixt@telecom.at>

The "dash-less" change aims to remove git commands from $PATH. It does so
by defining a GIT_EXEC_PATH that is different from $(bindir). On Windows
we want a relocatable installation of the git tool, so we cannot use an
absolute GIT_EXEC_PATH.  Therefore, the implementation of
builtin_exec_path() on Windows derives the exec-path from the command
invocation, and disregards GIT_EXEC_PATH. But this broke when
$(gitexecdir) became different from $(bindir), so we restore the earlier
behavior here.

This counteracts the aims of the "dash-less" change on Windows, but better
this way than having no working git at all.

Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
 Makefile |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/Makefile b/Makefile
index 36339d3..7d466b3 100644
--- a/Makefile
+++ b/Makefile
@@ -742,6 +742,7 @@ ifneq (,$(findstring MINGW,$(uname_S)))
 	COMPAT_OBJS += compat/mingw.o compat/fnmatch.o compat/regex.o compat/winansi.o
 	EXTLIBS += -lws2_32
 	X = .exe
+	gitexecdir = $(bindir)
 	template_dir = ../share/git-core/templates/
 	ETC_GITCONFIG = ../etc/gitconfig
 endif
-- 
1.5.6.1.275.g0a3e0f

^ permalink raw reply related

* Re: Re* git-remote SEGV on t5505 test.
From: Junio C Hamano @ 2008-07-18  6:26 UTC (permalink / raw)
  To: SungHyun Nam; +Cc: git
In-Reply-To: <7vsku7d8ak.fsf_-_@gitster.siamese.dyndns.org>

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

> SungHyun Nam <namsh@posdata.co.kr> writes:
>
>>     Is it possible that we can use 'SHELL_PATH' here?
>
> It is not just possible but we really should.  There are other test
> scripts that use hardcoded /bin/sh, but by setting SHELL_PATH the user is
> already telling us that what the vendor has in /bin/sh isn't adequately
> POSIX enough, and we really should try to honor that.
>
> "git grep -n /bin/sh t/t*sh | grep -v ':1:#!'" would tell you which ones
> are suspect.

SungHyun, I did not test this patch myself (all my shells grok $() command
substitutions), so I won't be committing this until/unless I see a "tested
on system X and works fine".

^ permalink raw reply

* Re: [PATCH 2/2 v2] git-gui: "Stage Line": Treat independent changes in adjacent lines better
From: Junio C Hamano @ 2008-07-18  6:21 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Shawn O. Pearce, git
In-Reply-To: <1216300911-5170-1-git-send-email-johannes.sixt@telecom.at>

Johannes Sixt <johannes.sixt@telecom.at> writes:

> Assume that we want to commit these states:
>
>   Old state == HEAD    Intermediate state   New state
>   --------------------------------------------------------
>   context before       context before       context before
>   old 1                new 1                new 1
>   old 2                old 2                new 2
>   context after        context after        context after

Much easier to understand.  Thanks.

^ permalink raw reply

* Re* git-remote SEGV on t5505 test.
From: Junio C Hamano @ 2008-07-18  6:18 UTC (permalink / raw)
  To: SungHyun Nam; +Cc: git
In-Reply-To: <48802DCD.2090704@posdata.co.kr>

SungHyun Nam <namsh@posdata.co.kr> writes:

>     Is it possible that we can use 'SHELL_PATH' here?

It is not just possible but we really should.  There are other test
scripts that use hardcoded /bin/sh, but by setting SHELL_PATH the user is
already telling us that what the vendor has in /bin/sh isn't adequately
POSIX enough, and we really should try to honor that.

"git grep -n /bin/sh t/t*sh | grep -v ':1:#!'" would tell you which ones
are suspect.

---

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

diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index de5b980..1c857cf 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -13,7 +13,7 @@ test_expect_success \
 
 test_expect_success \
     'Setup helper tool' \
-    '(echo "#!/bin/sh"
+    '(echo "#!$SHELL_PATH"
       echo shift
       echo output=1
       echo "while test -f commandline\$output; do output=\$((\$output+1)); done"
@@ -138,7 +138,7 @@ test_expect_success 'Valid In-Reply-To when prompting' '
 '
 
 test_expect_success 'setup fake editor' '
-	(echo "#!/bin/sh" &&
+	(echo "#!$SHELL_PATH" &&
 	 echo "echo fake edit >>\"\$1\""
 	) >fake-editor &&
 	chmod +x fake-editor
@@ -235,7 +235,7 @@ test_expect_success 'sendemail.cc unset' '
 
 test_expect_success '--compose adds MIME for utf8 body' '
 	clean_fake_sendmail &&
-	(echo "#!/bin/sh" &&
+	(echo "#!$SHELL_PATH" &&
 	 echo "echo utf8 body: àéìöú >>\"\$1\""
 	) >fake-editor-utf8 &&
 	chmod +x fake-editor-utf8 &&
@@ -254,7 +254,7 @@ test_expect_success '--compose adds MIME for utf8 body' '
 
 test_expect_success '--compose respects user mime type' '
 	clean_fake_sendmail &&
-	(echo "#!/bin/sh" &&
+	(echo "#!$SHELL_PATH" &&
 	 echo "(echo MIME-Version: 1.0"
 	 echo " echo Content-Type: text/plain\\; charset=iso-8859-1"
 	 echo " echo Content-Transfer-Encoding: 8bit"

^ permalink raw reply related

* Re: [PATCH] Enable git rev-list to parse --quiet
From: Junio C Hamano @ 2008-07-18  6:10 UTC (permalink / raw)
  To: Nick Andrew; +Cc: git
In-Reply-To: <7v8wvzeojm.fsf@gitster.siamese.dyndns.org>

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

> Nick Andrew <nick@nick-andrew.net> writes:
> ...
>> After fix:
>
> Thanks for noticing, but this replaces one breakage with another.
>
> Your new behaviour is a new "tell me if it is an empty set" option, and it
> means quite different thing from what --quiet does.

And here is how I would do it if I were interested in such a feature.

-- >8 --
rev-list --check-empty

This new option squelches the output entirely and signals if the specified
set is empty by its exit status.  E.g.

    $ git rev-list --check-empty HEAD..HEAD

will exit with a non-zero status, and

    $ git rev-list --check-empty HEAD^..HEAD

will exit with zero status.

---
 builtin-rev-list.c |   15 ++++++++++++++-
 1 files changed, 14 insertions(+), 1 deletions(-)

diff --git a/builtin-rev-list.c b/builtin-rev-list.c
index 507201e..4f9cce9 100644
--- a/builtin-rev-list.c
+++ b/builtin-rev-list.c
@@ -52,6 +52,7 @@ static const char rev_list_usage[] =
 
 static struct rev_info revs;
 
+static int check_empty;
 static int bisect_list;
 static int show_timestamp;
 static int hdr_termination;
@@ -161,6 +162,8 @@ static void show_commit(struct commit *commit)
 
 static void finish_commit(struct commit *commit)
 {
+	if (check_empty)
+		exit(0);
 	if (commit->parents) {
 		free_commit_list(commit->parents);
 		commit->parents = NULL;
@@ -171,6 +174,8 @@ static void finish_commit(struct commit *commit)
 
 static void finish_object(struct object_array_entry *p)
 {
+	if (check_empty)
+		exit(0);
 	if (p->item->type == OBJ_BLOB && !has_sha1_file(p->item->sha1))
 		die("missing blob object '%s'", sha1_to_hex(p->item->sha1));
 }
@@ -593,6 +598,10 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
 	for (i = 1 ; i < argc; i++) {
 		const char *arg = argv[i];
 
+		if (!strcmp(arg, "--check-empty")) {
+			check_empty = 1;
+			continue;
+		}
 		if (!strcmp(arg, "--header")) {
 			revs.verbose_header = 1;
 			continue;
@@ -650,6 +659,9 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
 
 	if (prepare_revision_walk(&revs))
 		die("revision walk setup failed");
+	if (check_empty)
+		quiet = 1;
+
 	if (revs.tree_objects)
 		mark_edges_uninteresting(revs.commits, &revs, show_edge);
 
@@ -699,6 +711,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
 	traverse_commit_list(&revs,
 		quiet ? finish_commit : show_commit,
 		quiet ? finish_object : show_object);
-
+	if (check_empty)
+		exit(1);
 	return 0;
 }

^ permalink raw reply related

* Re: [PATCH] Remove function stubs in shell.c
From: Dmitry Potapov @ 2008-07-18  6:06 UTC (permalink / raw)
  To: Stephan Beyer; +Cc: Shawn O. Pearce, Junio C Hamano, git
In-Reply-To: <1216343197-20290-1-git-send-email-s-beyer@gmx.net>

On Fri, Jul 18, 2008 at 03:06:37AM +0200, Stephan Beyer wrote:
> These function stubs may be useful, but because they redefine
> functions, they provoke a linker error.
> 
> Signed-off-by: Stephan Beyer <s-beyer@gmx.net>
> ---
> Or would you rather like this one? ;)

Unfortunately, this one will increase the size of git-shell
considerably:

   text    data     bss     dec     hex filename
  18182     824    8232   27238    6a66 git-shell
===
   text    data     bss     dec     hex filename
 146738    1376   93164  241278   3ae7e git-shell

So, personally, I don't like it.

Dmitry

^ permalink raw reply

* Re: [PATCH] Link git-shell only to a subset of libgit.a
From: Dmitry Potapov @ 2008-07-18  6:03 UTC (permalink / raw)
  To: Stephan Beyer; +Cc: Shawn O. Pearce, Junio C Hamano, git
In-Reply-To: <1216343070-20237-1-git-send-email-s-beyer@gmx.net>

On Fri, Jul 18, 2008 at 03:04:30AM +0200, Stephan Beyer wrote:
> Commit 5b8e6f85 introduced stubs for three functions that make no sense
> for git-shell. But those stubs defined libgit.a functions a second time
> so that a linker can complain.
> 
> Now git-shell is only linked to a subset of libgit.a.
> 
> Signed-off-by: Stephan Beyer <s-beyer@gmx.net>

I agree, it is probably better to specify explicitly what files to link
for git-shell if we want to keep it small and avoid the problem with
some linkers.

Dmitry

^ permalink raw reply

* Re: [PATCH] shrink git-shell by avoiding redundant dependencies
From: Dmitry Potapov @ 2008-07-18  5:59 UTC (permalink / raw)
  To: Stephan Beyer; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <20080718002620.GE8421@leksak.fem-net>

On Fri, Jul 18, 2008 at 02:26:20AM +0200, Stephan Beyer wrote:
> 
> Dmitry Potapov wrote:
> > diff --git a/shell.c b/shell.c
> > index b27d01c..91ca7de 100644
> > --- a/shell.c
> > +++ b/shell.c
> > @@ -3,6 +3,14 @@
> >  #include "exec_cmd.h"
> >  #include "strbuf.h"
> >  
> > +/* Stubs for functions that make no sense for git-shell. These stubs
> > + * are provided here to avoid linking in external redundant modules.
> > + */
> > +void release_pack_memory(size_t need, int fd){}
> > +void trace_argv_printf(const char **argv, const char *fmt, ...){}
> > +void trace_printf(const char *fmt, ...){}
> > +
> > +
> 
> I don't really understand why this works.
> You redefine libgit.a functions here
> 
> So the linker should complain like that:
> 	libgit.a(sha1_file.o): In function `release_pack_memory':
> 	/home/sbeyer/src/git/sha1_file.c:624: multiple definition of `release_pack_memory'
> 	shell.o:/home/sbeyer/src/git/shell.c:9: first defined here
> 	collect2: ld returned 1 exit status
> 
> And, in fact, it does when I move a function from a builtin to a lib
> source file, for example launch_editor() from builtin-tag.c to strbuf.c,
> like the following one:

It works because these functions are defined in object files, so the
linker should not search for them in libraries. However, if the linker
is forced to link sha1_file.c for some other reason, you will get the
conflict.

Dmitry

^ permalink raw reply

* [PATCH (GITK) 3/3] gitk: On Windows use a Cygwin-specific flag for kill.
From: Alexander Gavrilov @ 2008-07-18  5:47 UTC (permalink / raw)
  To: git; +Cc: Paul Mackerras
In-Reply-To: <200807180946.43560.angavrilov@gmail.com>

MSysGit compiles git binaries as native Windows executables,
so they cannot be killed unless a special flag is specified.

This flag is implemented by the Cygwin version of kill,
which is also included in MSysGit.

Signed-off-by: Alexander Gavrilov <angavrilov@gmail.com>
---
 gitk |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/gitk b/gitk
index b523c98..d7fea26 100755
--- a/gitk
+++ b/gitk
@@ -388,7 +388,12 @@ proc stop_instance {inst} {
     set fd $commfd($inst)
     catch {
 	set pid [pid $fd]
-	exec kill $pid
+
+	if {$::tcl_platform(platform) eq {windows}} {
+	    exec kill -f $pid
+	} else {
+	    exec kill $pid
+	}
     }
     catch {close $fd}
     nukefile $fd
-- 
1.5.6.2.39.gd084

^ permalink raw reply related

* [PATCH (GITK) 2/3] gitk: Register diff-files & diff-index in commfd, to ensure kill.
From: Alexander Gavrilov @ 2008-07-18  5:46 UTC (permalink / raw)
  To: git; +Cc: Paul Mackerras
In-Reply-To: <200807180945.43504.angavrilov@gmail.com>

Local change analysis can take a noticeable amount of time on large
file sets, and produce no output if there are no changes. Register
the back-ends in commfd, so that they get properly killed on window
close.

Signed-off-by: Alexander Gavrilov <angavrilov@gmail.com>
---
 gitk |   39 +++++++++++++++++++++++----------------
 1 files changed, 23 insertions(+), 16 deletions(-)

diff --git a/gitk b/gitk
index 29d79d6..b523c98 100755
--- a/gitk
+++ b/gitk
@@ -90,6 +90,15 @@ proc dorunq {} {
     }
 }
 
+proc reg_instance {fd} {
+    global commfd leftover loginstance
+
+    set i [incr loginstance]
+    set commfd($i) $fd
+    set leftover($i) {}
+    return $i
+}
+
 proc unmerged_files {files} {
     global nr_unmerged
 
@@ -294,10 +303,10 @@ proc parseviewrevs {view revs} {
 # Start off a git log process and arrange to read its output
 proc start_rev_list {view} {
     global startmsecs commitidx viewcomplete curview
-    global commfd leftover tclencoding
+    global tclencoding
     global viewargs viewargscmd viewfiles vfilelimit
     global showlocalchanges commitinterest
-    global viewactive loginstance viewinstances vmergeonly
+    global viewactive viewinstances vmergeonly
     global pending_select mainheadid
     global vcanopt vflags vrevs vorigargs
 
@@ -354,10 +363,8 @@ proc start_rev_list {view} {
 	error_popup "[mc "Error executing git log:"] $err"
 	return 0
     }
-    set i [incr loginstance]
+    set i [reg_instance $fd]
     set viewinstances($view) [list $i]
-    set commfd($i) $fd
-    set leftover($i) {}
     if {$showlocalchanges && $mainheadid ne {}} {
 	lappend commitinterest($mainheadid) {dodiffindex}
     }
@@ -420,8 +427,8 @@ proc getcommits {} {
 
 proc updatecommits {} {
     global curview vcanopt vorigargs vfilelimit viewinstances
-    global viewactive viewcomplete loginstance tclencoding
-    global startmsecs commfd showneartags showlocalchanges leftover
+    global viewactive viewcomplete tclencoding
+    global startmsecs showneartags showlocalchanges
     global mainheadid pending_select
     global isworktree
     global varcid vposids vnegids vflags vrevs
@@ -482,10 +489,8 @@ proc updatecommits {} {
     if {$viewactive($view) == 0} {
 	set startmsecs [clock clicks -milliseconds]
     }
-    set i [incr loginstance]
+    set i [reg_instance $fd]
     lappend viewinstances($view) $i
-    set commfd($i) $fd
-    set leftover($i) {}
     fconfigure $fd -blocking 0 -translation lf -eofchar {}
     if {$tclencoding != {}} {
 	fconfigure $fd -encoding $tclencoding
@@ -4063,10 +4068,11 @@ proc dodiffindex {} {
     incr lserial
     set fd [open "|git diff-index --cached HEAD" r]
     fconfigure $fd -blocking 0
-    filerun $fd [list readdiffindex $fd $lserial]
+    set i [reg_instance $fd]
+    filerun $fd [list readdiffindex $fd $lserial $i]
 }
 
-proc readdiffindex {fd serial} {
+proc readdiffindex {fd serial inst} {
     global mainheadid nullid nullid2 curview commitinfo commitdata lserial
 
     set isdiff 1
@@ -4077,7 +4083,7 @@ proc readdiffindex {fd serial} {
 	set isdiff 0
     }
     # we only need to see one line and we don't really care what it says...
-    close $fd
+    stop_instance $inst
 
     if {$serial != $lserial} {
 	return 0
@@ -4086,7 +4092,8 @@ proc readdiffindex {fd serial} {
     # now see if there are any local changes not checked in to the index
     set fd [open "|git diff-files" r]
     fconfigure $fd -blocking 0
-    filerun $fd [list readdifffiles $fd $serial]
+    set i [reg_instance $fd]
+    filerun $fd [list readdifffiles $fd $serial $i]
 
     if {$isdiff && ![commitinview $nullid2 $curview]} {
 	# add the line for the changes in the index to the graph
@@ -4103,7 +4110,7 @@ proc readdiffindex {fd serial} {
     return 0
 }
 
-proc readdifffiles {fd serial} {
+proc readdifffiles {fd serial inst} {
     global mainheadid nullid nullid2 curview
     global commitinfo commitdata lserial
 
@@ -4115,7 +4122,7 @@ proc readdifffiles {fd serial} {
 	set isdiff 0
     }
     # we only need to see one line and we don't really care what it says...
-    close $fd
+    stop_instance $inst
 
     if {$serial != $lserial} {
 	return 0
-- 
1.5.6.2.39.gd084

^ permalink raw reply related

* [PATCH (GITK) 1/3] gitk: Kill back-end processes on window close.
From: Alexander Gavrilov @ 2008-07-18  5:45 UTC (permalink / raw)
  To: git; +Cc: Paul Mackerras
In-Reply-To: <200807180944.48570.angavrilov@gmail.com>

When collecting commits for a rarely changed, or recently
created file or directory, rev-list may work for a noticeable
period of time without producing any output. Such processes
don't receive SIGPIPE for a while after gitk is closed, thus
becoming runaway CPU hogs.

Signed-off-by: Alexander Gavrilov <angavrilov@gmail.com>
---
 gitk |   35 +++++++++++++++++++++++++----------
 1 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/gitk b/gitk
index fddcb45..29d79d6 100755
--- a/gitk
+++ b/gitk
@@ -375,19 +375,33 @@ proc start_rev_list {view} {
     return 1
 }
 
+proc stop_instance {inst} {
+    global commfd leftover
+
+    set fd $commfd($inst)
+    catch {
+	set pid [pid $fd]
+	exec kill $pid
+    }
+    catch {close $fd}
+    nukefile $fd
+    unset commfd($inst)
+    unset leftover($inst)
+}
+
+proc stop_backends {} {
+    global commfd
+
+    foreach inst [array names commfd] {
+	stop_instance $inst
+    }
+}
+
 proc stop_rev_list {view} {
-    global commfd viewinstances leftover
+    global viewinstances
 
     foreach inst $viewinstances($view) {
-	set fd $commfd($inst)
-	catch {
-	    set pid [pid $fd]
-	    exec kill $pid
-	}
-	catch {close $fd}
-	nukefile $fd
-	unset commfd($inst)
-	unset leftover($inst)
+	stop_instance $inst
     }
     set viewinstances($view) {}
 }
@@ -2103,6 +2117,7 @@ proc makewindow {} {
     bind . <$M1B-minus> {incrfont -1}
     bind . <$M1B-KP_Subtract> {incrfont -1}
     wm protocol . WM_DELETE_WINDOW doquit
+    bind . <Destroy> {stop_backends}
     bind . <Button-1> "click %W"
     bind $fstring <Key-Return> {dofind 1 1}
     bind $sha1entry <Key-Return> gotocommit
-- 
1.5.6.2.39.gd084

^ permalink raw reply related

* Re: git-remote SEGV on t5505 test.
From: SungHyun Nam @ 2008-07-18  5:44 UTC (permalink / raw)
  To: git
In-Reply-To: <7vsku7es3n.fsf@gitster.siamese.dyndns.org>

Junio C Hamano wrote:
> SungHyun Nam <namsh@posdata.co.kr> writes:
> 
>> And the 'skip_prefix()' returns NULL in this case.
>> (The old skip_prefix() never returns NULL).
> 
> Thanks.  Something like this?

With this patch, 'make test' passed all the tests successfully.
I ran test with GIT_SKIP_TESTS="t90?? t91?? t92?? t94??".

Regards,
namsh

P.S I skipped svn/cvs test because I don't need them (And no
     svn/cvs installed).  But, I skipped send-email test because it
     fails.  I thought I don't need to use send-email.

     And now I check what caused fail.  The problem was this test
     script generates 'fake.sendmail' which uses '/bin/sh'.

     Is it possible that we can use 'SHELL_PATH' here?  Or could
     you apply the patch below?

diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index de5b980..0320aa1 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -16,7 +16,7 @@ test_expect_success \
      '(echo "#!/bin/sh"
        echo shift
        echo output=1
-      echo "while test -f commandline\$output; do
output=\$((\$output+1)); done"
+      echo "while test -f commandline\$output; do output=\`expr
\$output + 1\`; done"
        echo for a
        echo do
        echo "  echo \"!\$a!\""

^ permalink raw reply related

* [PATCH (GITK) 0/3] Avoid runaway processes in gitk
From: Alexander Gavrilov @ 2008-07-18  5:44 UTC (permalink / raw)
  To: git; +Cc: Paul Mackerras

As in the 'git gui blame' case, gitk back-end processes can sometimes
run for a while without producing any output, e.g. diff-files on a slow
filesystem.

These patches make gitk explicitly kill its back-end processes.

Alexander Gavrilov (3):
      gitk: Kill back-end processes on window close.
      gitk: Register diff-files & diff-index in commfd, to ensure kill.
      gitk: On Windows use a Cygwin-specific flag for kill.

 gitk |   79 ++++++++++++++++++++++++++++++++++++++++++++---------------------
 1 files changed, 53 insertions(+), 26 deletions(-)

^ permalink raw reply

* Re: [PATCH] Enable git rev-list to parse --quiet
From: Junio C Hamano @ 2008-07-18  5:42 UTC (permalink / raw)
  To: Nick Andrew; +Cc: git
In-Reply-To: <20080718040459.13073.76896.stgit@marcab.local.tull.net>

Nick Andrew <nick@nick-andrew.net> writes:

> Enable git rev-list to parse --quiet
>
> git rev-list never sees the --quiet option because --quiet is
> also an option for diff-files.
>
> Example:
>
> $ ./git rev-list --quiet ^HEAD~2 HEAD
> 1e102bf7c83281944ffd9202a7d35c514e4a5644
> 3bf0dd1f4e75ee1591169b687ce04dff00ae2e3e
> $ echo $?
> 0
>
> The fix scans the argument list to detect --quiet before passing it
> to setup_revisions(). It also arranges to count the number of commits
> or objects (whether sent to STDOUT or not) so --quiet can return an
> appropriate exit code (1 if there were commits/objects, 0 otherwise).
>
> After fix:

Thanks for noticing, but this replaces one breakage with another.

Your new behaviour is a new "tell me if it is an empty set" option, and it
means quite different thing from what --quiet does.

The --quiet option is designed primarily for sanity checking after a
failed fetch by commit walkers.  Here is how it works (well, at least how
it is supposed to work).

Imagine you have this history:

	---o---o---X

and the other side has this history:

	---o---o---X---A---B---C

And you run fetch over a dumb protocol; the commit walker fetches C,
discovers you do not have its parent B and tries to fetch it, and you
somehow kill that process.  Your repository will have:

	---o---o---X           C

Now, we do not mark C with our refs, so we do not say "Ok we have
everything leading up to C" when you re-run the same commit walker.
Instead, we'll let the walker walk again starting from C.  So we will
never in corrupt state.

But you might want to see if your repository has this kind of failure.
For that, you can run rev-list starting from C and X --- it will fail
after it finds out that C's parent is B and tries to read it.  And you
will learn the failure with the exit code from the command.  --quiet was
about squelching the output of "I've seen C", "I've seen X", as the only
thing you care about in that mode of usage is if the history is well
connected which is reported by the exit code.

-- >8 --
Subject: [PATCH] rev-list: honor --quiet option

Nick Andrew noticed that rev-list lets the --quiet option to be parsed by
underlying diff_options parser but did not pick up the result.  This
resulted in --quiet option to become effectively a no-op.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin-rev-list.c |    6 +-----
 1 files changed, 1 insertions(+), 5 deletions(-)

diff --git a/builtin-rev-list.c b/builtin-rev-list.c
index 8e1720c..507201e 100644
--- a/builtin-rev-list.c
+++ b/builtin-rev-list.c
@@ -589,7 +589,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
 	revs.abbrev = 0;
 	revs.commit_format = CMIT_FMT_UNSPECIFIED;
 	argc = setup_revisions(argc, argv, &revs, NULL);
-
+	quiet = DIFF_OPT_TST(&revs.diffopt, QUIET);
 	for (i = 1 ; i < argc; i++) {
 		const char *arg = argv[i];
 
@@ -621,10 +621,6 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
 			read_revisions_from_stdin(&revs);
 			continue;
 		}
-		if (!strcmp(arg, "--quiet")) {
-			quiet = 1;
-			continue;
-		}
 		usage(rev_list_usage);
 
 	}
-- 
1.5.6.3.573.gd2d2

^ permalink raw reply related

* Re: [PATCH (GIT-GUI) 0/3] Improve gui blame usability for large repositories
From: Alexander Gavrilov @ 2008-07-18  5:39 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git
In-Reply-To: <20080717021623.GC16740@spearce.org>

Hello,

On Thursday 17 July 2008 06:16:23 Shawn O. Pearce wrote:
> Alexander Gavrilov <angavrilov@gmail.com> wrote:
> > Full copy detection can take quite some time on large repositories, which
> > substantially decreases perceived usability of the 'git gui blame' command.
> > This set of patches tries to overcome it by:
> > 
> > 1) Allowing the user to disable '-C -C' and/or set the detection threshold.
> > 
> > 2) Explicitly killing back-end processes, which don't produce any output
> >   during copy detection, and thus normally won't receive SIGPIPE until
> >   it is finished. Runaway processes are annoying.
> > 
> > 3) To compensate for (1), adding a context menu item to manually invoke
> >   blame -C -C -C on a group of lines.
> 
> This series is nicely done.  Works well on revision.c in git.git,
> where the blame can be costly to compute with full copy detection.
> And getting the incremental from the context menu also works well.

I also thought that it would be useful to be able to specify the line range explicitly,
but couldn't invent a good UI for it. Selecting lines with the mouse also causes a
commit to be selected and recolored in green, and popping up a dialog to request
line numbers seems too lame.

Alexander

^ 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