Git development
 help / color / mirror / Atom feed
* Re: [PATCH] Don't fflush(stdout) when it's not helpful
From: Linus Torvalds @ 2007-06-29 23:43 UTC (permalink / raw)
  To: Theodore Tso
  Cc: Junio C Hamano, Jeff King, Frank Lichtenheld, Jim Meyering, git
In-Reply-To: <20070629174046.GC16268@thunk.org>



On Fri, 29 Jun 2007, Theodore Tso wrote:
> 
> Comments?

Looks ok to me. 

This should probably be paired up with the change to git.c (in "next") to 
do the "fflush()" before the "ferror()" too, in case the error is pending.

		Linus

^ permalink raw reply

* Re: [PATCH] Don't fflush(stdout) when it's not helpful
From: Theodore Tso @ 2007-06-29 17:40 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Junio C Hamano, Jeff King, Frank Lichtenheld, Jim Meyering, git
In-Reply-To: <alpine.LFD.0.98.0706290851480.8675@woody.linux-foundation.org>

On Fri, Jun 29, 2007 at 09:06:22AM -0700, Linus Torvalds wrote:
> I think that patch looks fine, but I also think that there is a more 
> fundamental problem with this approach:
> 
>  - all these patches basically break the whole _point_ of Jim's original 
>    reason for wanting this!

Yeah, I pointed that out in my first patch.  It had seemed that
interactivity over a pipe was considered more important, though when
we started talking about things.  :-)

It looks like from my reading of the standard that ferror(f) should
not change the state of the file handle f.  So the following patch I
think should work; it checks ferror(f), and if it indicates that there
is an error, we try a flush to get the error message.  I've tested
under Linux and it gives the correct error message in the "git log >
/mnt/full-filesystem" case, and I believe it should DTRT on other
systems.

Comments?

						- Ted

commit 93a96f94028106687412acbb771bb18ee7ec5560
Author: Theodore Ts'o <tytso@mit.edu>
Date:   Thu Jun 28 14:10:58 2007 -0400

    Don't fflush(stdout) when it's not helpful
    
    This patch arose from a discussion started by Jim Meyering's patch
    whose intention was to provide better diagnostics for failed writes.
    Linus proposed a better way to do things, which also had the added
    benefit that adding a fflush() to git-log-* operations and incremental
    git-blame operations could improve interactive respose time feel, at
    the cost of making things a bit slower when we aren't piping the
    output to a downstream program.
    
    This patch skips the fflush() calls when stdout is a regular file, or
    if the environment variable GIT_FLUSH is set to "0".  This latter can
    speed up a command such as:
    
    GIT_FLUSH=0 strace -c -f -e write time git-rev-list HEAD | wc -l
    
    a tiny amount.
    
    Cc: Linus Torvalds <torvalds@linux-foundation.org>
    Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>

diff --git a/Documentation/git.txt b/Documentation/git.txt
index 20b5b7b..8269148 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -396,6 +396,16 @@ other
 'GIT_PAGER'::
 	This environment variable overrides `$PAGER`.
 
+'GIT_FLUSH'::
+	If this environment variable is set to "1", then commands such
+	as git-blame (in incremental mode), git-rev-list, git-log,
+	git-whatchanged, etc., will force a flush of the output stream
+	after each commit-oriented record have been flushed.   If this
+	variable is set to "0", the output of these commands will be done
+	using completely buffered I/O.   If this environment variable is
+	not set, git will choose buffered or record-oriented flushing
+	based on whether stdout appears to be redirected to a file or not.
+
 'GIT_TRACE'::
 	If this variable is set to "1", "2" or "true" (comparison
 	is case insensitive), git will print `trace:` messages on
diff --git a/builtin-blame.c b/builtin-blame.c
index f7e2c13..da23a6f 100644
--- a/builtin-blame.c
+++ b/builtin-blame.c
@@ -1459,6 +1459,7 @@ static void found_guilty_entry(struct blame_entry *ent)
 				printf("boundary\n");
 		}
 		write_filename_info(suspect->path);
+		maybe_flush_or_die(stdout, "stdout");
 	}
 }
 
diff --git a/builtin-rev-list.c b/builtin-rev-list.c
index 813aadf..86db8b0 100644
--- a/builtin-rev-list.c
+++ b/builtin-rev-list.c
@@ -100,7 +100,7 @@ static void show_commit(struct commit *commit)
 		printf("%s%c", buf, hdr_termination);
 		free(buf);
 	}
-	fflush(stdout);
+	maybe_flush_or_die(stdout, "stdout");
 	if (commit->parents) {
 		free_commit_list(commit->parents);
 		commit->parents = NULL;
diff --git a/cache.h b/cache.h
index ed83d92..0525c4e 100644
--- a/cache.h
+++ b/cache.h
@@ -532,6 +532,8 @@ extern char git_default_name[MAX_GITNAME];
 extern const char *git_commit_encoding;
 extern const char *git_log_output_encoding;
 
+/* IO helper functions */
+extern void maybe_flush_or_die(FILE *, const char *);
 extern int copy_fd(int ifd, int ofd);
 extern int read_in_full(int fd, void *buf, size_t count);
 extern int write_in_full(int fd, const void *buf, size_t count);
diff --git a/log-tree.c b/log-tree.c
index 0cf21bc..ced3f33 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -408,5 +408,6 @@ int log_tree_commit(struct rev_info *opt, struct commit *commit)
 		shown = 1;
 	}
 	opt->loginfo = NULL;
+	maybe_flush_or_die(stdout, "stdout");
 	return shown;
 }
diff --git a/write_or_die.c b/write_or_die.c
index 5c4bc85..e125e11 100644
--- a/write_or_die.c
+++ b/write_or_die.c
@@ -1,5 +1,45 @@
 #include "cache.h"
 
+/*
+ * Some cases use stdio, but want to flush after the write
+ * to get error handling (and to get better interactive
+ * behaviour - not buffering excessively).
+ *
+ * Of course, if the flush happened within the write itself,
+ * we've already lost the error code, and cannot report it any
+ * more. So we just ignore that case instead (and hope we get
+ * the right error code on the flush).
+ *
+ * If the file handle is stdout, and stdout is a file, then skip the
+ * flush entirely since it's not needed.
+ */
+void maybe_flush_or_die(FILE *f, const char *desc)
+{
+	static int skip_stdout_flush = -1;
+	struct stat st;
+	char *cp;
+
+	if (f == stdout) {
+		if (skip_stdout_flush < 0) {
+			cp = getenv("GIT_FLUSH");
+			if (cp)
+				skip_stdout_flush = (atoi(cp) == 0);
+			else if ((fstat(fileno(stdout), &st) == 0) &&
+				 S_ISREG(st.st_mode))
+				skip_stdout_flush = 1;
+			else
+				skip_stdout_flush = 0;
+		}
+		if (skip_stdout_flush && !ferror(f))
+			return;
+	}
+	if (fflush(f)) {
+		if (errno == EPIPE)
+			exit(0);
+		die("write failure on %s: %s", desc, strerror(errno));
+	}
+}
+
 int read_in_full(int fd, void *buf, size_t count)
 {
 	char *p = buf;

^ permalink raw reply related

* [PATCH] git add: respect core.filemode with unmerged entries
From: Johannes Schindelin @ 2007-06-29 17:32 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Sixt, git
In-Reply-To: <7v3b0bf4ea.fsf@assigned-by-dhcp.pobox.com>


When a merge left unmerged entries, git add failed to pick up the
file mode from the index, when core.filemode == 0. If more than one
unmerged entry is there, the order of stage preference is 2, 1, 3.

Noticed by Johannes Sixt.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	On Fri, 29 Jun 2007, Junio C Hamano wrote:

	> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
	> 
	> > On Fri, 29 Jun 2007, Johannes Sixt wrote:
	> >
	> >> However, if only two stages are present, the file mode is 
	> >> still taken from the file instead of from the index. As that 
	> >> easy to solve (at least for the unambiguous case)?
	> >
	> > It might be related to the bug Junio found, i.e. that I assumed stage 1 to 
	> > be "ours".
	> 
	> Actually it is because (-1-pos) and (1-pos) are two apart.

	I congratulate myself. My first off-by-two bug.

	> I am all for refactoring the funny "pick up an existing entry at
	> any stage, but favor 0 then 2 then 1 and finally 3" into a
	> separate function.  It makes sense, although I do not offhand
	> know of a place that we can immediately reuse it (logically,
	> diff-files ought to do that, but I haven't checked).

	Me neither. But at least I coded it (hopefully) correctly this 
	time, even accompied it by a test verifying that I got it right.

 read-cache.c   |   30 +++++++++++++++++++++++++++++-
 t/t3700-add.sh |   26 ++++++++++++++++++++++++++
 2 files changed, 55 insertions(+), 1 deletions(-)

diff --git a/read-cache.c b/read-cache.c
index 4362b11..a363f31 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -350,6 +350,34 @@ int remove_file_from_index(struct index_state *istate, const char *path)
 	return 0;
 }
 
+static int compare_name(struct cache_entry *ce, const char *path, int namelen)
+{
+	return namelen != ce_namelen(ce) || memcmp(path, ce->name, namelen);
+}
+
+static int index_name_pos_also_unmerged(struct index_state *istate,
+	const char *path, int namelen)
+{
+	int pos = index_name_pos(istate, path, namelen);
+	struct cache_entry *ce;
+
+	if (pos >= 0)
+		return pos;
+
+	/* maybe unmerged? */
+	pos = -1 - pos;
+	if (pos >= istate->cache_nr ||
+			compare_name((ce = istate->cache[pos]), path, namelen))
+		return -1;
+
+	/* order of preference: stage 2, 1, 3 */
+	if (ce_stage(ce) == 1 && pos + 1 < istate->cache_nr &&
+			ce_stage((ce = istate->cache[pos + 1])) == 2 &&
+			!compare_name(ce, path, namelen))
+		pos++;
+	return pos;
+}
+
 int add_file_to_index(struct index_state *istate, const char *path, int verbose)
 {
 	int size, namelen;
@@ -380,7 +408,7 @@ int add_file_to_index(struct index_state *istate, const char *path, int verbose)
 		 * from it, otherwise assume unexecutable regular file.
 		 */
 		struct cache_entry *ent;
-		int pos = index_name_pos(istate, path, namelen);
+		int pos = index_name_pos_also_unmerged(istate, path, namelen);
 
 		ent = (0 <= pos) ? istate->cache[pos] : NULL;
 		ce->ce_mode = ce_mode_from_stat(ent, st.st_mode);
diff --git a/t/t3700-add.sh b/t/t3700-add.sh
index ad8cc7d..0d80c6a 100755
--- a/t/t3700-add.sh
+++ b/t/t3700-add.sh
@@ -110,4 +110,30 @@ test_expect_success 'check correct prefix detection' '
 	git add 1/2/a 1/3/b 1/2/c
 '
 
+test_expect_success 'git add and filemode=0 with unmerged entries' '
+	echo 1 > stage1 &&
+	echo 2 > stage2 &&
+	echo 3 > stage3 &&
+	for s in 1 2 3
+	do
+		echo "100755 $(git hash-object -w stage$s) $s	file"
+	done | git update-index --index-info &&
+	git config core.filemode 0 &&
+	echo new > file &&
+	git add file &&
+	git ls-files --stage | grep "^100755 .* 0	file$"
+'
+
+test_expect_success 'git add and filemode=0 prefers stage 2 over stage 1' '
+	git rm --cached -f file &&
+	(
+		echo "100644 $(git hash-object -w stage1) 1	file"
+		echo "100755 $(git hash-object -w stage2) 2	file"
+	) | git update-index --index-info &&
+	git config core.filemode 0 &&
+	echo new > file &&
+	git add file &&
+	git ls-files --stage | grep "^100755 .* 0	file$"
+'
+
 test_done
-- 
1.5.2.2.3228.g16a27

^ permalink raw reply related

* Re: How do I label conflict blocks in merge-recursive output?
From: Johannes Schindelin @ 2007-06-29 17:16 UTC (permalink / raw)
  To: しらいしななこ; +Cc: GIT
In-Reply-To: <20070629232849.6117@nanako3.bluebottle.com>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1083 bytes --]

Hi,

On Fri, 29 Jun 2007, しらいしななこ wrote:

> A trouble I am seeing is that there are large hexadecimal string after
> these markers, like this:
> 
>    <<<<<<< 13bd5d46b4a5d4ef44c53fab11e74801c18b16d0:A
>    side
>    revision
>    =======
>    local
>    modification
>    >>>>>>> 9aa4ad6a3bdf5340bed969f6e14abb4e07e794f7:A
> 
> I do not know what object these are, and I do not think it is useful to 
> show them to the user.  I want them to say something more useful.
>
> [...]
>
> How do I tell merge-recursive to do that?  I tried to read the 
> documentation for merge-recursive but there is no manual page.

Yes, it is unfortunately undocumented. To override the name for a given 
SHA-1 (which is the long hex string, and which you can obtain by 
"git-rev-parse <commit>"), set the environment variable

	GITHEAD_<sha1>="This is a much nicer message"

IOW If you would have done this (_before_ calling merge-recursive):

	export GITHEAD_9aa4ad6a3bdf5340bed969f6e14abb4e07e794f7=nanako

then the last conflict marker would have read

	>>>>>>> nanako:A

Hth,
Dscho

^ permalink raw reply

* Re: How to share the same commits offline?
From: Jakub Narebski @ 2007-06-29 16:09 UTC (permalink / raw)
  To: git
In-Reply-To: <46852503.8090407@cosmocode.de>

[Cc: git@vger.kernel.org]

Tobias Sarnowski wrote:

> I am very happy with git and I am using it now on every project I
> develop, thanks Linus! My question is, how can I share objects with
> another repository without a direct connection?

Try git-bundle, since git version 1.5.1.

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

^ permalink raw reply

* Re: [PATCH] Don't fflush(stdout) when it's not helpful
From: Linus Torvalds @ 2007-06-29 16:06 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Jeff King, Theodore Tso, Frank Lichtenheld, Jim Meyering, git
In-Reply-To: <7vmyyjgrxk.fsf@assigned-by-dhcp.pobox.com>



On Fri, 29 Jun 2007, Junio C Hamano wrote:
> 
> Thanks for bringing it up, as I had the same "Huh?" moment.
> I would probably call that simply "do_not_flush".  Or name the
> variable "flush_stdout" and swap all the logic.

I think that patch looks fine, but I also think that there is a more 
fundamental problem with this approach:

 - all these patches basically break the whole _point_ of Jim's original 
   reason for wanting this!

So remember, we had two different reasons for flushing:

 - interactivity over a pipe. 

   Tools like "gitk" and "git gui blame" all run waiting for input, and we 
   want to give them the commit data as soon as possible.

 - error checking on a filesystem.

   Yes, "ferror()" _after_ the write shows an error happened, but doesn't 
   show what error it was. Doing a fflush() is a lot more likely to 
   actually give the right errors.

So non-files want flushing due to latency issues, and files want flushing 
due to error handling. And the patch under discussion basically broke the 
error handling.

(It turns out that it doesn't break it for the trivial "/dev/full" 
testcase, since that doesn't show up as a file, but it would break it for 
the *real* case of a filesystem that becomes full).

One option is to change the git.c thing to do

	if (fflush(stdout))
		die("write failure on standard output: %s", strerror(errno));
	if (ferror(stdout))
		die("unknown write failure on standard output");
	if (fclose(stdout))
		die("close failure on standard output: %s", strerror(errno));

where the "fflush()" is done first (regardless of ferror), just in the 
(probably futile) hope of getting the right errno.

		Linus

^ permalink raw reply

* How to share the same commits offline?
From: Tobias Sarnowski @ 2007-06-29 15:28 UTC (permalink / raw)
  To: git

Hi,

I am very happy with git and I am using it now on every project I
develop, thanks Linus! My question is, how can I share objects with
another repository without a direct connection?

If I create patches with git-format-patch I am loosing the commit id
informations, so the copied tags loose their target too. I want to be
able to distribute changes by e-mail without loosing the sha1 ids. (Just
dump copying the .git/object files and updateing the .git/refs only
cause errors - I did not grab the source codes enough to understand the
connections)

Thanks,
Tobias Sarnowski

^ permalink raw reply

* How do I label conflict blocks in merge-recursive output?
From: しらいしななこ @ 2007-06-29 14:28 UTC (permalink / raw)
  To: GIT

Hi, I am updating my 'git-save' script and have a question.  

When I play back the state previously recorded, I run:

   $ git-merge-recursive $base -- HEAD $stash

This is done after getting the $base (the state the stash was made from),
and $stash (the state of the interrupted work recorded in the stash) in
these variables.  When there are conflicts, I see '<<<<<<<' and '>>>>>>>'
markers in the merged file.

A trouble I am seeing is that there are large hexadecimal string after
these markers, like this:

   <<<<<<< 13bd5d46b4a5d4ef44c53fab11e74801c18b16d0:A
   side
   revision
   =======
   local
   modification
   >>>>>>> 9aa4ad6a3bdf5340bed969f6e14abb4e07e794f7:A

I do not know what object these are, and I do not think it is useful to
show them to the user.  I want them to say something more useful.

In a real conflict after a 'git-pull', I think I saw nicer labels that are
not hexadecimal strings after these markers.

How do I tell merge-recursive to do that?  I tried to read the
documentation for merge-recursive but there is no manual page.

TIA

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

----------------------------------------------------------------------
Find out how you can get spam free email.
http://www.bluebottle.com

^ permalink raw reply

* [PATCH] minor error in user-manual.txt
From: William Pursell @ 2007-06-29 13:41 UTC (permalink / raw)
  To: git

From: William Pursell <bill.pursell@gmail.com>
Date: Fri, 29 Jun 2007 14:08:29 +0100
Subject: [PATCH] minor error in user-manual.txt

---
 Documentation/user-manual.txt |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt
index 0bfa21b..b0b49c8 100644
--- a/Documentation/user-manual.txt
+++ b/Documentation/user-manual.txt
@@ -890,7 +890,7 @@ $ git archive --format=tar --prefix=project/ HEAD | gzip >latest.tar.gz
 -------------------------------------------------
 
 will use HEAD to produce a tar archive in which each filename is
-preceded by "prefix/".
+preceded by "project/".
 
 If you're releasing a new version of a software project, you may want
 to simultaneously make a changelog to include in the release
-- 
1.4.4.4

^ permalink raw reply related

* Re: Problem with Linus's git repository?
From: Andreas Ericsson @ 2007-06-29 13:00 UTC (permalink / raw)
  To: walt; +Cc: Linus Torvalds, git
In-Reply-To: <Pine.LNX.4.64.0706290537130.7055@x2.ybpnyarg>

walt wrote:
> 
> On Thu, 28 Jun 2007, Linus Torvalds wrote:
> 
>> On Thu, 28 Jun 2007, walt wrote:
>>> No, every morning I pull from you and Junio and Petr Baudis using cg-update,
> 
> 
>> Oh, don't use "cg-update", or just fix it to do "git pull".
> 
> I'm happy to stick with using git.  From reading the latest Docs, it seems
> that git-pull -v will do what I need (I think).
> 
> That's what I did just now to update from you and Junio, and the output
> seemed exactly appropriate -- ended with a fast-forward and no errors
> for either one.
> 
> However, a git-fsck turned up four dangling commits for Junio, and 42
> danglers for you, including a mix of blobs, trees, and commits.
> 

You will see dangling commits, blobs and trees from Junio because he rewinds
his 'pu' branch.

I'm not sure why you see any from Linus, as I'm not aware of any rewinding
in the official kernel tree (unless you're talking about some other repo,
in which case I'm clueless).

> Will a simple git-prune bring me correctly up to date, or am I missing
> some steps?
> 

You're already up to date. The dangling commits do not hurt your repo
in any way. Nor do they hurt the performance of any of the tools you
normally use. fsck suffers a bit from them, because it has to care
about them, but the normal DAG-traversing tools will never even
see them.

git prune would remove them and save you a tiny bit of diskspace.
Next time you pull from Junio, you'll most likely get orphaned commits
again though, meaning blobs, trees and commits will end up dangling once
more. It's perfectly normal and nothing to worry about.

For repo maintenance, I run "git gc" once a month on my local copies.
More to let my laptop do something while I'm fiddling with receipts and
stuff than for any real need (the idea of idle computers offend me for
some reason).

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

^ permalink raw reply

* Re: Problem with Linus's git repository?
From: walt @ 2007-06-29 12:47 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <alpine.LFD.0.98.0706280840570.8675@woody.linux-foundation.org>



On Thu, 28 Jun 2007, Linus Torvalds wrote:


> On Thu, 28 Jun 2007, walt wrote:
> >
> > No, every morning I pull from you and Junio and Petr Baudis using cg-update,


> Oh, don't use "cg-update", or just fix it to do "git pull".

I'm happy to stick with using git.  From reading the latest Docs, it seems
that git-pull -v will do what I need (I think).

That's what I did just now to update from you and Junio, and the output
seemed exactly appropriate -- ended with a fast-forward and no errors
for either one.

However, a git-fsck turned up four dangling commits for Junio, and 42
danglers for you, including a mix of blobs, trees, and commits.

Will a simple git-prune bring me correctly up to date, or am I missing
some steps?

Thanks!


--- Scanned by M+ Guardian Messaging Firewall ---

^ permalink raw reply

* Re: Bug: segfault during git-prune
From: Andy Parkins @ 2007-06-29 12:39 UTC (permalink / raw)
  To: git; +Cc: Linus Torvalds
In-Reply-To: <alpine.LFD.0.98.0706281525460.8675@woody.linux-foundation.org>

On Thursday 2007, June 28, Linus Torvalds wrote:
> On Thu, 28 Jun 2007, Andy Parkins wrote:
> > I had hoped that git-prune wouldn't be a risk because I have:
> >
> >  * -- * -- * -- * -- * (ffmpeg-svn)
> >
> >       * -- * -- * -- * (libswscale-svn)
>
> Ok. If all subproject branches are also visible in the superproject as
> refs, then "git prune" should work fine, and you can apply my patch and
> it should just work very naturally: the reachability analysis will find
> the subprojects not through the superproject link, but simply through the
> direct refs to the subproject.

Excellent.  That's what I'd hoped.

I'm actually really impressed that git is robust enough that my symbolic 
linking of all the .git subdirectories works so well.  It's actually turned 
out to be a really natural way of using a repository for strongly connected 
submodules.

> This is not unlike just having two different repositories sharing the
> same object directory: as long as the two different repositories both
> have the appropriate refs, pruning is fine. In other words, you can see
> them as just independent branches in the same repo.

Absolutely.  With my poor-mans submodule script that I was using before git 
had it's own submodule support, I had the script make a refs/superrefs 
directory, and every time I committed to the supermodule the script would 
write $subhash to $submodule/.git/refs/heads/superrefs/$subhash.  The it 
was safe to git-prune the submodules as well.

> And in fact, subprojects are obviously very much *designed* to work that
> way: a subproject is basically a "different repository". So the basic
> rule is that if it would work with totally independent repos, it works
> with subprojects.

I certainly agree, it's an extremely elegant way of working.  I can't 
imagine that any other VCS is capable of this sort of manipulation.

> Anyway, if that patch works for you, I'd suggest you just pass it on to
> Junio (and feel free to add my "Signed-off-by:" on it - but conditional
> on you having actually tested it).

Will do.  I'll certainly test it, and am happy to forward it on if that test 
is successful.


Andy
-- 
Dr Andy Parkins, M Eng (hons), MIET
andyparkins@gmail.com

^ permalink raw reply

* [PATCH] git-gui: properly popup error if gitk should be started but is not installed
From: Gerrit Pape @ 2007-06-29 11:32 UTC (permalink / raw)
  To: Shawn O. Pearce, git

On 'Visualize ...', a gitk process is started.  Since it is run in the
background, catching a possible startup error doesn't work, and the error
output goes to the console git-gui is started from.  The most probable
startup error is that gitk is not installed; so before trying to start,
check for the existence of the gitk program, and popup an error message
unless it's found.

This was noticed and reported by Paul Wise through
 http://bugs.debian.org/429810

Signed-off-by: Gerrit Pape <pape@smarden.org>
---
 git-gui.sh |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/git-gui.sh b/git-gui.sh
index 9df2e47..1b0691c 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -1070,10 +1070,10 @@ proc do_gitk {revs} {
 		append cmd { }
 		append cmd $revs
 	}
-
-	if {[catch {eval exec $cmd &} err]} {
-		error_popup "Failed to start gitk:\n\n$err"
+	if {! [file exists [gitexec gitk]]} {
+		error_popup "Unable to start gitk:\n\nFile does not exist"
 	} else {
+		eval exec $cmd &
 		set ui_status_value $starting_gitk_msg
 		after 10000 {
 			if {$ui_status_value eq $starting_gitk_msg} {
-- 
1.5.2.1

^ permalink raw reply related

* Re: [PATCH] Ignore end-of-line style when computing similarity score for rename detection
From: Johannes Schindelin @ 2007-06-29 10:19 UTC (permalink / raw)
  To: Steven Grimm; +Cc: Junio C Hamano, git
In-Reply-To: <4683FB25.3080204@midwinter.com>

Hi,

On Thu, 28 Jun 2007, Steven Grimm wrote:

> Johannes Schindelin wrote:
> > Somehow I think that this should be triggered by "--ignore-space-at-eol",
> > _and_ be accompanied by a test case.
> >   
> 
> Should --ignore-space-at-eol be an option to git-merge? Merges are where 
> this functionality matters; for simple diffs, --ignore-space-at-eol 
> actually already covers it.

Good point. However, I fail to see how the similarity detection should be 
so decoupled from the application. IOW what good is it if two files are 
rated similar if the merge cannot handle the CRLF/LF differences properly?

So two points here: since the merges are what you target, you definitely 
should mention that in the commit message. And you should make sure that 
all this trickery only kicks in when the merge has a chance to succeed.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] git add: respect core.filemode even with unmerged entriesin  the index
From: Junio C Hamano @ 2007-06-29 10:20 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Johannes Sixt, git
In-Reply-To: <Pine.LNX.4.64.0706291106190.4438@racer.site>

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

> On Fri, 29 Jun 2007, Johannes Sixt wrote:
>
>> However, if only two stages are present, the file mode is still taken 
>> from the file instead of from the index. As that easy to solve (at least 
>> for the unambiguous case)?
>
> It might be related to the bug Junio found, i.e. that I assumed stage 1 to 
> be "ours".

Actually it is because (-1-pos) and (1-pos) are two apart.  By
starting to scan from (1-pos), you are jumping directly to
either the next path (in which case you do not pick it up), or
"their" version (i.e. stage #3) iff there are three stages for
the path in question.

I am all for refactoring the funny "pick up an existing entry at
any stage, but favor 0 then 2 then 1 and finally 3" into a
separate function.  It makes sense, although I do not offhand
know of a place that we can immediately reuse it (logically,
diff-files ought to do that, but I haven't checked).

^ permalink raw reply

* Re: [PATCH] git add: respect core.filemode even with unmerged entriesin  the index
From: Johannes Schindelin @ 2007-06-29 10:07 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: git
In-Reply-To: <4684AD41.9868C32F@eudaptics.com>

Hi,

On Fri, 29 Jun 2007, Johannes Sixt wrote:

> However, if only two stages are present, the file mode is still taken 
> from the file instead of from the index. As that easy to solve (at least 
> for the unambiguous case)?

It might be related to the bug Junio found, i.e. that I assumed stage 1 to 
be "ours".

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] git add: respect core.filemode even with unmerged entries in the index
From: Johannes Schindelin @ 2007-06-29 10:06 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Sixt, git
In-Reply-To: <7vir97f98o.fsf@assigned-by-dhcp.pobox.com>

Hi,

On Fri, 29 Jun 2007, Junio C Hamano wrote:

> Hmph.
> 
>  * Are you sure about (1-pos)?
> 
>  * I _think_ we would want to favor our stage (i.e. #2)

Hmph back.

I _thought_ that our stage is #1, not even bothering to check the 
documentation. But it seems that you are right.

So your code looks fine to me, except that I would move the whole 
get_index_entry_for_file_even_if_the_entry_is_unmerged(name, namelen) 
code into its own function.

Want me to prepare a patch? If so, I'll do it tonight, since I will be 
occupied the rest of the (working) day.

Ciao,
Dscho

^ permalink raw reply

* Re: Alternative git logo and favicon
From: Julian Phillips @ 2007-06-29 10:08 UTC (permalink / raw)
  To: Henrik Nyh; +Cc: git
In-Reply-To: <e8965c600706290054w43f896f3jaba176974938752d@mail.gmail.com>

On Fri, 29 Jun 2007, Henrik Nyh wrote:

> I came up with an alternative logo/favicon to use with my gitweb:
> http://henrik.nyh.se/2007/06/alternative-git-logo-and-favicon.
>
> Thought I'd sent it to the list in case someone else likes them.

Shouldn't the "+" be green, and the "-" red, as in the original?  I 
thought the idea was to match the colours used by diff etc.?

-- 
Julian

  ---
Riches cover a multitude of woes.
 		-- Menander

^ permalink raw reply

* Re: [PATCH 4/4] diffcore-delta.c: Ignore CR in CRLF for text files
From: Junio C Hamano @ 2007-06-29  8:51 UTC (permalink / raw)
  To: しらいしななこ; +Cc: GIT
In-Reply-To: <200706290813.l5T8DJ6w024507@mi1.bluebottle.com>

しらいしななこ  <nanako3@bluebottle.com> writes:

> Quoting Junio C Hamano <gitster@pobox.com>:
>
>> This ignores CR byte in CRLF sequence in text file when
>> computing similarity of two blobs.
>> ...
>> +test_expect_success 'diff -M' '
>> +
>> +	git diff-tree -M -r --name-status HEAD^ HEAD |
>> +	sed -e "s/R[0-9]*/RNUM/" >actual &&
>> +	echo "RNUM	sample	elpmas" >expect &&
>> +	diff -u expect actual
>> +
>> +'
>
> I tried this test but it does not give R100.  The new file is unchanged except for
> LF -> CRLF.  Could you explain why?

Heh, I hate when people nitpick me ;-)

The similarity code counts "bytes copied from the source
material to the destination" and "bytes added to the source to
create the destination".

Rename detection uses only the former value.  The amount of data
copied from the source is divided by the larger of the size of
source or destination.

In the case of our test script, the source material is about 560
bytes, while the destination material is about 590 bytes, after 
adding CR to the end of every line.  We find that 560 bytes have
been copied from the source material, and floor(560 * 100 / 590)
is 94%, which is what you would see as the result.

We could adjust max_size variable inside diffcore_count_changes,
but I am not sure if it is worth the trouble.

^ permalink raw reply

* Re: Alternative git logo and favicon
From: Jakub Narebski @ 2007-06-29  8:49 UTC (permalink / raw)
  To: git
In-Reply-To: <e8965c600706290054w43f896f3jaba176974938752d@mail.gmail.com>

[Cc: git@vger.kernel.org]

Henrik Nyh wrote:

> I came up with an alternative logo/favicon to use with my gitweb:
> http://henrik.nyh.se/2007/06/alternative-git-logo-and-favicon.
> 
> Thought I'd sent it to the list in case someone else likes them.

Added to http://git.or.cz/gitwiki/GitRelatedLogos

But for me logo is too big and wrong proportions for gitweb (although quite
nice for git pages, I guess), and as favicon it is a bit unreadable.

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

^ permalink raw reply

* Re: [PATCH] git add: respect core.filemode even with unmerged entries in the index
From: Junio C Hamano @ 2007-06-29  8:36 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Johannes Sixt, git
In-Reply-To: <Pine.LNX.4.64.0706281653260.4438@racer.site>

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

> diff --git a/read-cache.c b/read-cache.c
> index 4362b11..a74e4a7 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -382,6 +382,12 @@ int add_file_to_index(struct index_state *istate, const char *path, int verbose)
>  		struct cache_entry *ent;
>  		int pos = index_name_pos(istate, path, namelen);
>  
> +		/* might be unmerged */
> +		if (pos < 0 && 1-pos < istate->cache_nr &&
> +				namelen == ce_namelen(istate->cache[1-pos]) &&
> +				!memcmp(path, istate->cache[1-pos]->name,
> +					namelen))
> +			pos = 1-pos;
>  		ent = (0 <= pos) ? istate->cache[pos] : NULL;
>  		ce->ce_mode = ce_mode_from_stat(ent, st.st_mode);
>  	}

Hmph.

 * Are you sure about (1-pos)?

 * I _think_ we would want to favor our stage (i.e. #2) when
   unmerged, over other stages.  Perhaps like this:

	if (pos < 0) {
        	int a_pos = -1, our_pos = -1;
		pos = -pos-1;
		while ((pos < istate->cache_nr) &&
			namelen == ce_namelen(istate->cache[pos]) &&
			!memcmp(path, istate->cache[pos]->name, namelen))
			if (a_pos < 0)
                        	a_pos = pos;
                        if (ce_stage(istate->cache[pos]) == 2)
                        	our_pos = pos;
			pos++;
                }
		if (our_pos >= 0)
                	pos = our_pos;
		else if (a_pos >= 0)
                	pos = a_pos;
		else
                	pos = -1;
	}
	ent = (0 <= pos) ? istate->cache[pos] : NULL;
	ce->ce_mode = ce_mode_from_stat(ent, st.st_mode);

^ permalink raw reply

* [PATCH] git-clone: fetch possibly detached HEAD over dumb http
From: Sven Verdoolaege @ 2007-06-29  8:31 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Louis-Noel Pouchet
In-Reply-To: <7vsl8bmxv9.fsf@assigned-by-dhcp.pobox.com>

git-clone supports cloning from a repo with detached HEAD,
but if this HEAD is not behind any branch tip then it
would not have been fetched over dumb http, resulting in a

	fatal: Not a valid object name HEAD

Since 928c210a, this would also happen on a http repo
with a HEAD that is a symbolic link where someone has
forgotton to run update-server-info.

Signed-off-by: Sven Verdoolaege <skimo@liacs.nl>
---
On Thu, Jun 28, 2007 at 05:02:18PM -0700, Junio C Hamano wrote:
> Ok.  But I think the change regresses when the remote side is
> actually on a particular branch, and is using symref to
> represent $GIT_DIR/HEAD.

Updated patch tested on both symbolic links and symrefs.

skimo

 git-clone.sh |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/git-clone.sh b/git-clone.sh
index bd44ce1..4cbf60f 100755
--- a/git-clone.sh
+++ b/git-clone.sh
@@ -72,6 +72,17 @@ Perhaps git-update-server-info needs to be run there?"
 	rm -fr "$clone_tmp"
 	http_fetch "$1/HEAD" "$GIT_DIR/REMOTE_HEAD" ||
 	rm -f "$GIT_DIR/REMOTE_HEAD"
+	if test -f "$GIT_DIR/REMOTE_HEAD"; then
+		head_sha1=`cat "$GIT_DIR/REMOTE_HEAD"`
+		case "$head_sha1" in
+		'ref: refs/'*)
+			;;
+		*)
+			git-http-fetch $v -a "$head_sha1" "$1" ||
+			rm -f "$GIT_DIR/REMOTE_HEAD"
+			;;
+		esac
+	fi
 }
 
 quiet=
-- 
1.5.2.2.585.g9cc0-dirty

^ permalink raw reply related

* Re: [PATCH 4/4] diffcore-delta.c: Ignore CR in CRLF for text files
From: しらいしななこ @ 2007-06-29  8:14 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: GIT
In-Reply-To: <1183098962312-git-send-email-gitster@pobox.com>

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

> This ignores CR byte in CRLF sequence in text file when
> computing similarity of two blobs.
> ...
> +test_expect_success 'diff -M' '
> +
> +	git diff-tree -M -r --name-status HEAD^ HEAD |
> +	sed -e "s/R[0-9]*/RNUM/" >actual &&
> +	echo "RNUM	sample	elpmas" >expect &&
> +	diff -u expect actual
> +
> +'

I tried this test but it does not give R100.  The new file is unchanged except for
LF -> CRLF.  Could you explain why?

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

----------------------------------------------------------------------
Get a free email address with REAL anti-spam protection.
http://www.bluebottle.com

^ permalink raw reply

* Re: [PATCH] git-clone: fetch possibly detached HEAD over dumb http
From: Sven Verdoolaege @ 2007-06-29  8:11 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Louis-Noel Pouchet
In-Reply-To: <7vsl8bmxv9.fsf@assigned-by-dhcp.pobox.com>

On Thu, Jun 28, 2007 at 05:02:18PM -0700, Junio C Hamano wrote:
> You would want to do this extra fetch only in case (1).
> I think the additional fetch would fail in case (2), and result
> in removal of $GIT_DIR/REMOTE_HEAD.

You're right.  It looks like I only tested it on symbolic link HEADs.
Sorry about that.  Will send a corrected patch later.

skimo

^ permalink raw reply

* Alternative git logo and favicon
From: Henrik Nyh @ 2007-06-29  7:54 UTC (permalink / raw)
  To: git

I came up with an alternative logo/favicon to use with my gitweb:
http://henrik.nyh.se/2007/06/alternative-git-logo-and-favicon.

Thought I'd sent it to the list in case someone else likes them.

^ 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