Git development
 help / color / mirror / Atom feed
* Re: [PATCH 0/5] add pack index v2 reading capability to git v1.4.4.4
From: Junio C Hamano @ 2008-07-16 17:08 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Nicolas Pitre, git
In-Reply-To: <alpine.LFD.1.10.0807160924340.2835@woody.linux-foundation.org>

Linus Torvalds <torvalds@linux-foundation.org> writes:

> Actually, it fixes a crash. Didn't 1.4.4 SIGSEGV without this on pack-v2?

I do not think it should SEGV.  The pack-idx signature was chosen rather
carefully to allow older ones to die gracefully.

    error: non-monotonic index
    error: Could not read 4a588075c54cd5902e5f4d43b9d6b0c31d0f9769

But as I said in the other message, it's tagged and pushed out already.

^ permalink raw reply

* Re: [PATCH 0/5] add pack index v2 reading capability to git v1.4.4.4
From: Junio C Hamano @ 2008-07-16 17:04 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Nicolas Pitre, git, Gerrit Pape
In-Reply-To: <alpine.LFD.1.10.0807160924340.2835@woody.linux-foundation.org>

Linus Torvalds <torvalds@linux-foundation.org> writes:

> On Wed, 16 Jul 2008, Nicolas Pitre wrote:
>> 
>> Junio: if you could just apply them, tag the result as v1.4.4.5 and
>> push it out then at that point it simply will be up to Debian to make
>> it available as a "major usability fix".
>
> Actually, it fixes a crash. Didn't 1.4.4 SIGSEGV without this on pack-v2?
>
> So you don't even have to sell it as a usability issue, but literally as a 
> bugfix.

It's tagged and pushed out to both kernel.org and repo.or.cz.  Thanks, Nico.

I won't be doing my usual full release engineering on this one, but I did:

 - repack a copy of git.git to use pack idx version #2 for trial.

	Hint.  A handy way to tell which version of packfile you have is
	to run this:

	$ od -c .git/objects/pack/pack-$your_pack_id.idx | head -n 1
        
        If the output begins with "377 t 0 c", you have pack idx version
        #2.

 - check "git log" from v1.4.4.4 (fails) and v1.4.4.5 (reads ok)

 - repack this copy with "git repack" from 1.4.4.5.  This produced a
   repository usable by v1.4.4.4.

I also updated /pub/scm/git/git.git/config at kernel.org to use

	[pack]
        	indexVersion = 1

for now.  The repository is also repacked with pack idx version #1.

^ permalink raw reply

* Re: [PATCH 0/5] add pack index v2 reading capability to git v1.4.4.4
From: Linus Torvalds @ 2008-07-16 16:25 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: Junio C Hamano, git
In-Reply-To: <1216189899-14279-1-git-send-email-nico@cam.org>



On Wed, 16 Jul 2008, Nicolas Pitre wrote:
> 
> Junio: if you could just apply them, tag the result as v1.4.4.5 and
> push it out then at that point it simply will be up to Debian to make
> it available as a "major usability fix".

Actually, it fixes a crash. Didn't 1.4.4 SIGSEGV without this on pack-v2?

So you don't even have to sell it as a usability issue, but literally as a 
bugfix.

		Linus

^ permalink raw reply

* Re: [PATCH] Use SHELL_PATH
From: Junio C Hamano @ 2008-07-16 16:05 UTC (permalink / raw)
  To: sverre; +Cc: namsh, git
In-Reply-To: <7vr69tu91e.fsf@gitster.siamese.dyndns.org>

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

> It wants to make sure that the shell specified from the toplevel Makefile
> (or from make command line) is used to run the aggregation script.  It is
> often necessary when platform /bin/sh is not quite POSIX (e.g. the script
> in question uses arithmetic expansion "$(( $var1 + $var2 ))").
>
> Just saying "Use specified shell to run shell scripts" on the title line
> would be sufficient, but I wonder if this is the only remaining place...

People on funny platforms might find this one useful, which is consistent
with the way the patch under discussion called the shell.  This lets you
to say

	SHELL_PATH = /Program Files/GNU/Bash

or something silly like that ;-)

 Makefile |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/Makefile b/Makefile
index 9b52071..a2b2627 100644
--- a/Makefile
+++ b/Makefile
@@ -153,7 +153,7 @@ all::
 # broken, or spawning external process is slower than built-in grep git has).
 
 GIT-VERSION-FILE: .FORCE-GIT-VERSION-FILE
-	@$(SHELL_PATH) ./GIT-VERSION-GEN
+	@'$(SHELL_PATH_SQ)' ./GIT-VERSION-GEN
 -include GIT-VERSION-FILE
 
 uname_S := $(shell sh -c 'uname -s 2>/dev/null || echo not')

^ permalink raw reply related

* Re: [PATCH] Documentation/git-cherry-pick.txt et al.: Fix misleading -n description
From: Junio C Hamano @ 2008-07-16 15:49 UTC (permalink / raw)
  To: Petr Baudis; +Cc: git
In-Reply-To: <20080716123344.5875.88693.stgit@localhost>

Petr Baudis <pasky@suse.cz> writes:

> The manual page of git-cherry-pick and git-revert asserts that -n works
> primarily on the working tree, while in fact the primary object it operates
> on is the index, and the changes only "accidentally" propagate to the
> working tree. This e.g. leads innocent #git IRC folks to believe that you
> can use -n to prepare changes for git-add -i staging.

True, and false ;-)  Just "unstage" first in your "git add -i" session and
then add only the necessary pieces back to the index (alternatively you
can "reset" before entering "git add -i" session).

The change is good; will apply.

^ permalink raw reply

* Re: [PATCH] Use SHELL_PATH
From: Junio C Hamano @ 2008-07-16 15:46 UTC (permalink / raw)
  To: sverre; +Cc: namsh, git
In-Reply-To: <bd6139dc0807160347n2ebcd64fy7a15fe94bfabe580@mail.gmail.com>

"Sverre Rabbelier" <alturin@gmail.com> writes:

> On Wed, Jul 16, 2008 at 3:31 AM, SungHyun Nam <goweol@gmail.com> wrote:
>>
>> Signed-off-by: SungHyun Nam <goweol@gmail.com>
>> ---
>>  t/Makefile |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/t/Makefile b/t/Makefile
>> index a778865..0d65ced 100644
>> --- a/t/Makefile
>> +++ b/t/Makefile
>> @@ -26,7 +26,7 @@ clean:
>>        $(RM) -r 'trash directory' test-results
>>
>>  aggregate-results:
>> -       ./aggregate-results.sh test-results/t*-*
>> +       '$(SHELL_PATH_SQ)' ./aggregate-results.sh test-results/t*-*
>>
>>  # we can test NO_OPTIMIZE_COMMITS independently of LC_ALL
>>  full-svn-test:
>> --
>> 1.5.6.3.350.g6c11a
>
> It is not clear to me what this patch does, there is no justification
> in the commit msg either. Instead you say what is being done, which we
> can see from the commit diff. Please clarify?

It wants to make sure that the shell specified from the toplevel Makefile
(or from make command line) is used to run the aggregation script.  It is
often necessary when platform /bin/sh is not quite POSIX (e.g. the script
in question uses arithmetic expansion "$(( $var1 + $var2 ))").

Just saying "Use specified shell to run shell scripts" on the title line
would be sufficient, but I wonder if this is the only remaining place...

^ permalink raw reply

* Re: git submodules and commit
From: Avery Pennarun @ 2008-07-16 15:43 UTC (permalink / raw)
  To: Nigel Magnay; +Cc: Git Mailing List
In-Reply-To: <320075ff0807160332k5e49c256tb4191de628ecf41c@mail.gmail.com>

On 7/16/08, Nigel Magnay <nigel.magnay@gmail.com> wrote:
> I wonder if this is a fairly common pattern. We tend to have modules
>  as git repositories, and projects that tie together those git
>  repositories as submodules. [and submodules are necessary because they're
>  shared between multiple supermodules].

I have exactly the same problem as you, and have been working on
improving my own workflow so that someday I can offer patches that
might be generally applicable.

In the meantime, my solution is... some shell scripts checked in at
the top level of my project. :)

In one of my applications, I have a /wv submodule, which provides a
cross-platform build environment.  That environment respectively
contains a /wv/wvstreams submodule, which is a library that we use.

When I make a change to wvstreams that's needed for my application, I
need to check into wvstreams, then check that link into wv, then check
that link into the application.  Then, when I push, I have to make
sure to always push wvstreams first, then wv, then application, or
else other users can end up with "commit id xxxxxx not found" type
errors.

So basically, committing is always harmless, since I can do anything I
want in my own repo (and I want to be able to update wvstreams
*without* always updating wv, and so on).  The tricky part is pushing.
 Here's the script I wrote to make sure I don't screw up when pushing:


~/src/vx-lin $ cat push-git-modules
#!/bin/sh -x
set -e
test -e wv/wvstreams/Makefile
(cd wv/wvstreams && git push origin HEAD:master) &&
(cd wv && git push origin HEAD:master) &&
git push origin HEAD:master ||
echo "Failed!"


Now, this script is pretty flawed.  Notably, it always pushes to the
'master' branch, which is stupid.  However, it works in our particular
workflow, because wvstreams isn't being modified by too many
developers and it's okay if we all commit to master.  This is also
aided by the fact that people are trained to push only after they've
made all the unit tests pass, etc.  And further, individual apps don't
have to update their wvstreams to the latest anyway unless they really
need the latest changes, which is a wonderful feature of git
submodules.

Now, sometimes the above push script will fail.  In my experience,
this is only when someone else has pushed in something before you,
which means a fast-forward is not possible on at least one of the
repos.  When that happens, you have to pull first, using this script:

~/src/vx-lin $ cat newest-git-modules
#!/bin/sh -x
set -e
test -e wv/wvstreams/Makefile
git pull origin master &&
(cd wv && git pull origin master) &&
(cd wv/wvstreams && git pull origin master) ||
echo "Failed!"

This pulls in the latest version of application, wv, and wvstreams, in
that order, and stops in case of any merge conflicts so that you can
resolve them by hand.  It's safe to run the above script more than
once in case you're not sure if it's done or not.

After pulling the new modules, you may need to make new commits to
update to the latest submodule commits - if that's indeed what you
want.  And then you can run push-git-modules, and be reasonably
assured that it will work (unless someone made another push while you
were fixing conflicts).

Finally, I have another script that retrieves the *currently linked*
version of the git modules.  I wish git-checkout would do this
automatically, but it doesn't, for apparently-difficult-to-resolve
safety reasons.  Anyway, note that this script uses the existence of
submodule/Makefile as "proof" that the submodule was checked out
correctly.


~/src/vx-lin $ cat get-git-modules
#!/bin/sh -x
set -e
git submodule init
git submodule update
test -e wv/Makefile
(cd wv && git submodule init && git submodule update)
test -e wv/wvstreams/Makefile


>  I guess it probably gets sticky when there are merge conflicts. Is
>  anyone working on this kind of thing; I might be able to give some
>  time to help work on it?

So as you can see, my scripts are crappy.  However, they have already
drastically reduced the number of mistakes made by developers in my
group (especially commits lost due to 'git submodule update' at the
wrong time, and pushes of the supermodule before the submodule).

If you want to work with me on my new submodule workflow (and I'd
certainly appreciate it!) then I'd suggest one or more of the
following starting points:

- Take the recursive push, pull, and update operations described
above, make them general (ie. not referring to my submodules by name
:)), and add them as commands in the real git-submodule script.  The
trickiest part here will be figuring out which remote branch to
push/pull.

- Perhaps add a "recursive commit" operation that recursively
auto-commits submodule refs, for use after running the
newest-git-modules script.  The commit message could be auto-generated
using something like "git-whatchanged" on the submodule.

- See what can be done about making git-checkout automatically
git-submodule-update *if and only if* the currently checked-out commit
of the submodule exactly matches the one that was checked out last
time, *and* the desired commit is already available in the submodule
repo (which is not necessarily the case, if you haven't fetched it
yet).  That is, as with any file in git, if it hasn't changed from the
one in the repo, you know you won't lose any information if you just
auto-replace it with the new version.

- Fix git-submodule-update to not just switch submodule branches if
you've made checkins in that submodule.  Right now, commits to a
submodule by default don't go to any branch, so if you subsequently
run git-submodule-update, your commits are lost (except for the
reflog).  This is very un-git-like in general, and
git-submodule-update should be much more polite.

Note that git-submodule is only about 800 lines of shell.  It's
remarkably straightforward to make it do whatever you want.  The hard
part is figuring out what you want, and making sure you don't stomp on
*other* people's workflows while you're there.

Also note that even if you don't contribute any of the above, I'm
planning to someday make time to do it myself :)  But don't hold your
breath.  I've been busy.

Have fun,

Avery

^ permalink raw reply

* [PATCH v2] Fix buffer overflow in prepare_attr_stack
From: Dmitry Potapov @ 2008-07-16 15:39 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: git, Johannes Schindelin
In-Reply-To: <487E11F7.60601@viscovery.net>

If PATH_MAX on your system is smaller than a path stored in the git repo,
it may cause the buffer overflow in prepare_attr_stack.

Signed-off-by: Dmitry Potapov <dpotapov@gmail.com>
---

On Wed, Jul 16, 2008 at 05:21:27PM +0200, Johannes Sixt wrote:
> Dmitry Potapov schrieb:
> > +			pathbuf.len = 0;
> 
> +			strbuf_reset(&pathbuf);
> 
> -- Hannes

 attr.c |   15 +++++++++------
 1 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/attr.c b/attr.c
index 0fb47d3..17f6a4d 100644
--- a/attr.c
+++ b/attr.c
@@ -459,7 +459,9 @@ static void prepare_attr_stack(const char *path, int dirlen)
 {
 	struct attr_stack *elem, *info;
 	int len;
-	char pathbuf[PATH_MAX];
+	struct strbuf pathbuf;
+
+	strbuf_init(&pathbuf, dirlen+2+strlen(GITATTRIBUTES_FILE));
 
 	/*
 	 * At the bottom of the attribute stack is the built-in
@@ -510,13 +512,14 @@ static void prepare_attr_stack(const char *path, int dirlen)
 			len = strlen(attr_stack->origin);
 			if (dirlen <= len)
 				break;
-			memcpy(pathbuf, path, dirlen);
-			memcpy(pathbuf + dirlen, "/", 2);
-			cp = strchr(pathbuf + len + 1, '/');
+			strbuf_reset(&pathbuf);
+			strbuf_add(&pathbuf, path, dirlen);
+			strbuf_addch(&pathbuf, '/');
+			cp = strchr(pathbuf.buf + len + 1, '/');
 			strcpy(cp + 1, GITATTRIBUTES_FILE);
-			elem = read_attr(pathbuf, 0);
+			elem = read_attr(pathbuf.buf, 0);
 			*cp = '\0';
-			elem->origin = strdup(pathbuf);
+			elem->origin = strdup(pathbuf.buf);
 			elem->prev = attr_stack;
 			attr_stack = elem;
 			debug_push(elem);
-- 
1.5.6.3.3.geccd

^ permalink raw reply related

* [PATCH v2] Fix buffer overflow in git-grep
From: Dmitry Potapov @ 2008-07-16 15:33 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin, Dmitry Potapov
In-Reply-To: <alpine.DEB.1.00.0807161232110.8503@eeepc-johanness>

If PATH_MAX on your system is smaller than any path stored in the git
repository, that can cause memory corruption inside of the grep_tree
function used by git-grep.

Signed-off-by: Dmitry Potapov <dpotapov@gmail.com>
---
I have converted grep_tree code to use path_buf.

 builtin-grep.c |   26 ++++++++++++++------------
 1 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/builtin-grep.c b/builtin-grep.c
index ef29910..507bb95 100644
--- a/builtin-grep.c
+++ b/builtin-grep.c
@@ -427,33 +427,35 @@ static int grep_tree(struct grep_opt *opt, const char **paths,
 	struct name_entry entry;
 	char *down;
 	int tn_len = strlen(tree_name);
-	char *path_buf = xmalloc(PATH_MAX + tn_len + 100);
+	struct strbuf pathbuf;
+
+	strbuf_init(&pathbuf, PATH_MAX + tn_len);
 
 	if (tn_len) {
-		tn_len = sprintf(path_buf, "%s:", tree_name);
-		down = path_buf + tn_len;
-		strcat(down, base);
-	}
-	else {
-		down = path_buf;
-		strcpy(down, base);
+		strbuf_add(&pathbuf, tree_name, tn_len);
+		strbuf_addch(&pathbuf, ':');
+		tn_len = pathbuf.len;
 	}
-	len = strlen(path_buf);
+	strbuf_addstr(&pathbuf, base);
+	len = pathbuf.len;
 
 	while (tree_entry(tree, &entry)) {
-		strcpy(path_buf + len, entry.path);
+		int te_len = tree_entry_len(entry.path, entry.sha1);
+		pathbuf.len = len;
+		strbuf_add(&pathbuf, entry.path, te_len);
 
 		if (S_ISDIR(entry.mode))
 			/* Match "abc/" against pathspec to
 			 * decide if we want to descend into "abc"
 			 * directory.
 			 */
-			strcpy(path_buf + len + tree_entry_len(entry.path, entry.sha1), "/");
+			strbuf_addch(&pathbuf, '/');
 
+		down = pathbuf.buf + tn_len;
 		if (!pathspec_matches(paths, down))
 			;
 		else if (S_ISREG(entry.mode))
-			hit |= grep_sha1(opt, entry.sha1, path_buf, tn_len);
+			hit |= grep_sha1(opt, entry.sha1, pathbuf.buf, tn_len);
 		else if (S_ISDIR(entry.mode)) {
 			enum object_type type;
 			struct tree_desc sub;
-- 
1.5.6.3.1.g4e6bb

^ permalink raw reply related

* Re: [PATCH] Fix buffer overflow in prepare_attr_stack
From: Johannes Sixt @ 2008-07-16 15:21 UTC (permalink / raw)
  To: Dmitry Potapov; +Cc: git, Johannes Schindelin
In-Reply-To: <1216220043-27678-2-git-send-email-dpotapov@gmail.com>

Dmitry Potapov schrieb:
> +			pathbuf.len = 0;

+			strbuf_reset(&pathbuf);

-- Hannes

^ permalink raw reply

* Re: MSysGit Stability
From: Pascal Obry @ 2008-07-16 14:59 UTC (permalink / raw)
  To: Joe Fiorini; +Cc: git
In-Reply-To: <73fd69b50807151458u22a383a3l343779e47f4161fa@mail.gmail.com>

Joe,

> I'm going to be working with a small dev shop in the near future and
> we would like to use Git for our project.  Two of us are on Macs, but
> the other is on Windows.  I know the options are MSysGit or Git
> through cygwin.  I'm curious which is better to use and if MSysGit is
> even stable yet.  Does anyone have experience running Git on Windows?

Yes, all the time on some quite large projects and since a year now. No 
problem, quite stable. I'm using Git on Cygwin that I build myself 
almost every day. I'm also working with git-svn as many projects I'm 
working on are based on Subversion. At least Git Cygwin is a perfect 
solution on Windows.

> Any experiences you can share?  Is MSysGit ready yet or should we
> wait?

I have never used MSysGit, so I won't comment.

Pascal.

-- 

--|------------------------------------------------------
--| Pascal Obry                           Team-Ada Member
--| 45, rue Gabriel Peri - 78114 Magny Les Hameaux FRANCE
--|------------------------------------------------------
--|              http://www.obry.net
--| "The best way to travel is by means of imagination"
--|
--| gpg --keyserver wwwkeys.pgp.net --recv-key C1082595

^ permalink raw reply

* [PATCH] Fix buffer overflow in prepare_attr_stack
From: Dmitry Potapov @ 2008-07-16 14:54 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin, Dmitry Potapov
In-Reply-To: <1216220043-27678-1-git-send-email-dpotapov@gmail.com>

If PATH_MAX on your system is smaller than a path stored in the git repo,
it may cause the buffer overflow in prepare_attr_stack.

Signed-off-by: Dmitry Potapov <dpotapov@gmail.com>
---
 attr.c |   15 +++++++++------
 1 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/attr.c b/attr.c
index 0fb47d3..73b6d6d 100644
--- a/attr.c
+++ b/attr.c
@@ -459,7 +459,9 @@ static void prepare_attr_stack(const char *path, int dirlen)
 {
 	struct attr_stack *elem, *info;
 	int len;
-	char pathbuf[PATH_MAX];
+	struct strbuf pathbuf;
+
+	strbuf_init(&pathbuf, dirlen+2+strlen(GITATTRIBUTES_FILE));
 
 	/*
 	 * At the bottom of the attribute stack is the built-in
@@ -510,13 +512,14 @@ static void prepare_attr_stack(const char *path, int dirlen)
 			len = strlen(attr_stack->origin);
 			if (dirlen <= len)
 				break;
-			memcpy(pathbuf, path, dirlen);
-			memcpy(pathbuf + dirlen, "/", 2);
-			cp = strchr(pathbuf + len + 1, '/');
+			pathbuf.len = 0;
+			strbuf_add(&pathbuf, path, dirlen);
+			strbuf_addch(&pathbuf, '/');
+			cp = strchr(pathbuf.buf + len + 1, '/');
 			strcpy(cp + 1, GITATTRIBUTES_FILE);
-			elem = read_attr(pathbuf, 0);
+			elem = read_attr(pathbuf.buf, 0);
 			*cp = '\0';
-			elem->origin = strdup(pathbuf);
+			elem->origin = strdup(pathbuf.buf);
 			elem->prev = attr_stack;
 			attr_stack = elem;
 			debug_push(elem);
-- 
1.5.6.3.3.gfcafb

^ permalink raw reply related

* [PATCH] Fix buffer overflow in git diff
From: Dmitry Potapov @ 2008-07-16 14:54 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin, Dmitry Potapov
In-Reply-To: <37fcd2780807160733o156e49c6m51314ff18764ccd5@mail.gmail.com>

If PATH_MAX on your system is smaller than a path stored, it may cause
buffer overflow and stack corruption in diff_addremove() and diff_change()
functions when running git-diff

Signed-off-by: Dmitry Potapov <dpotapov@gmail.com>
---
 diff-lib.c  |    8 ++++----
 diff.c      |   11 ++---------
 diff.h      |    9 ++++-----
 revision.c  |    4 ++--
 tree-diff.c |   26 ++++++++++++++++++++++----
 5 files changed, 34 insertions(+), 24 deletions(-)

diff --git a/diff-lib.c b/diff-lib.c
index b17722d..e7eaff9 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -171,7 +171,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
 			if (silent_on_removed)
 				continue;
 			diff_addremove(&revs->diffopt, '-', ce->ce_mode,
-				       ce->sha1, ce->name, NULL);
+				       ce->sha1, ce->name);
 			continue;
 		}
 		changed = ce_match_stat(ce, &st, ce_option);
@@ -184,7 +184,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
 		newmode = ce_mode_from_stat(ce, st.st_mode);
 		diff_change(&revs->diffopt, oldmode, newmode,
 			    ce->sha1, (changed ? null_sha1 : ce->sha1),
-			    ce->name, NULL);
+			    ce->name);
 
 	}
 	diffcore_std(&revs->diffopt);
@@ -208,7 +208,7 @@ static void diff_index_show_file(struct rev_info *revs,
 				 const unsigned char *sha1, unsigned int mode)
 {
 	diff_addremove(&revs->diffopt, prefix[0], mode,
-		       sha1, ce->name, NULL);
+		       sha1, ce->name);
 }
 
 static int get_stat_data(struct cache_entry *ce,
@@ -312,7 +312,7 @@ static int show_modified(struct oneway_unpack_data *cbdata,
 		return 0;
 
 	diff_change(&revs->diffopt, oldmode, mode,
-		    old->sha1, sha1, old->name, NULL);
+		    old->sha1, sha1, old->name);
 	return 0;
 }
 
diff --git a/diff.c b/diff.c
index 78c4d3a..386de82 100644
--- a/diff.c
+++ b/diff.c
@@ -3356,9 +3356,8 @@ int diff_result_code(struct diff_options *opt, int status)
 void diff_addremove(struct diff_options *options,
 		    int addremove, unsigned mode,
 		    const unsigned char *sha1,
-		    const char *base, const char *path)
+		    const char *concatpath)
 {
-	char concatpath[PATH_MAX];
 	struct diff_filespec *one, *two;
 
 	if (DIFF_OPT_TST(options, IGNORE_SUBMODULES) && S_ISGITLINK(mode))
@@ -3380,9 +3379,6 @@ void diff_addremove(struct diff_options *options,
 		addremove = (addremove == '+' ? '-' :
 			     addremove == '-' ? '+' : addremove);
 
-	if (!path) path = "";
-	sprintf(concatpath, "%s%s", base, path);
-
 	if (options->prefix &&
 	    strncmp(concatpath, options->prefix, options->prefix_length))
 		return;
@@ -3403,9 +3399,8 @@ void diff_change(struct diff_options *options,
 		 unsigned old_mode, unsigned new_mode,
 		 const unsigned char *old_sha1,
 		 const unsigned char *new_sha1,
-		 const char *base, const char *path)
+		 const char *concatpath)
 {
-	char concatpath[PATH_MAX];
 	struct diff_filespec *one, *two;
 
 	if (DIFF_OPT_TST(options, IGNORE_SUBMODULES) && S_ISGITLINK(old_mode)
@@ -3418,8 +3413,6 @@ void diff_change(struct diff_options *options,
 		tmp = old_mode; old_mode = new_mode; new_mode = tmp;
 		tmp_c = old_sha1; old_sha1 = new_sha1; new_sha1 = tmp_c;
 	}
-	if (!path) path = "";
-	sprintf(concatpath, "%s%s", base, path);
 
 	if (options->prefix &&
 	    strncmp(concatpath, options->prefix, options->prefix_length))
diff --git a/diff.h b/diff.h
index 5dc0cb5..50fb5dd 100644
--- a/diff.h
+++ b/diff.h
@@ -14,12 +14,12 @@ typedef void (*change_fn_t)(struct diff_options *options,
 		 unsigned old_mode, unsigned new_mode,
 		 const unsigned char *old_sha1,
 		 const unsigned char *new_sha1,
-		 const char *base, const char *path);
+		 const char *fullpath);
 
 typedef void (*add_remove_fn_t)(struct diff_options *options,
 		    int addremove, unsigned mode,
 		    const unsigned char *sha1,
-		    const char *base, const char *path);
+		    const char *fullpath);
 
 typedef void (*diff_format_fn_t)(struct diff_queue_struct *q,
 		struct diff_options *options, void *data);
@@ -164,14 +164,13 @@ extern void diff_addremove(struct diff_options *,
 			   int addremove,
 			   unsigned mode,
 			   const unsigned char *sha1,
-			   const char *base,
-			   const char *path);
+			   const char *fullpath);
 
 extern void diff_change(struct diff_options *,
 			unsigned mode1, unsigned mode2,
 			const unsigned char *sha1,
 			const unsigned char *sha2,
-			const char *base, const char *path);
+			const char *fullpath);
 
 extern void diff_unmerge(struct diff_options *,
 			 const char *path,
diff --git a/revision.c b/revision.c
index fc66755..8dc3ca7 100644
--- a/revision.c
+++ b/revision.c
@@ -259,7 +259,7 @@ static int tree_difference = REV_TREE_SAME;
 static void file_add_remove(struct diff_options *options,
 		    int addremove, unsigned mode,
 		    const unsigned char *sha1,
-		    const char *base, const char *path)
+		    const char *fullpath)
 {
 	int diff = REV_TREE_DIFFERENT;
 
@@ -285,7 +285,7 @@ static void file_change(struct diff_options *options,
 		 unsigned old_mode, unsigned new_mode,
 		 const unsigned char *old_sha1,
 		 const unsigned char *new_sha1,
-		 const char *base, const char *path)
+		 const char *fullpath)
 {
 	tree_difference = REV_TREE_DIFFERENT;
 	DIFF_OPT_SET(options, HAS_CHANGES);
diff --git a/tree-diff.c b/tree-diff.c
index e1e2e6c..4c67ea5 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -15,6 +15,15 @@ static char *malloc_base(const char *base, int baselen, const char *path, int pa
 	return newbase;
 }
 
+static char *malloc_fullname(const char *base, int baselen, const char *path, int pathlen)
+{
+	char *fullname = xmalloc(baselen + pathlen + 1);
+	memcpy(fullname, base, baselen);
+	memcpy(fullname + baselen, path, pathlen);
+	fullname[baselen + pathlen] = 0;
+	return fullname;
+}
+
 static void show_entry(struct diff_options *opt, const char *prefix, struct tree_desc *desc,
 		       const char *base, int baselen);
 
@@ -24,6 +33,7 @@ static int compare_tree_entry(struct tree_desc *t1, struct tree_desc *t2, const
 	const char *path1, *path2;
 	const unsigned char *sha1, *sha2;
 	int cmp, pathlen1, pathlen2;
+	char *fullname;
 
 	sha1 = tree_entry_extract(t1, &path1, &mode1);
 	sha2 = tree_entry_extract(t2, &path2, &mode2);
@@ -56,14 +66,20 @@ static int compare_tree_entry(struct tree_desc *t1, struct tree_desc *t2, const
 		int retval;
 		char *newbase = malloc_base(base, baselen, path1, pathlen1);
 		if (DIFF_OPT_TST(opt, TREE_IN_RECURSIVE))
+		{
+			newbase[baselen + pathlen1] = 0;
 			opt->change(opt, mode1, mode2,
-				    sha1, sha2, base, path1);
+				    sha1, sha2, newbase);
+			newbase[baselen + pathlen1] = '/';
+		}
 		retval = diff_tree_sha1(sha1, sha2, newbase, opt);
 		free(newbase);
 		return retval;
 	}
 
-	opt->change(opt, mode1, mode2, sha1, sha2, base, path1);
+	fullname = malloc_fullname(base, baselen, path1, pathlen1);
+	opt->change(opt, mode1, mode2, sha1, sha2, fullname);
+	free(fullname);
 	return 0;
 }
 
@@ -205,10 +221,10 @@ static void show_entry(struct diff_options *opt, const char *prefix, struct tree
 	unsigned mode;
 	const char *path;
 	const unsigned char *sha1 = tree_entry_extract(desc, &path, &mode);
+	int pathlen = tree_entry_len(path, sha1);
 
 	if (DIFF_OPT_TST(opt, RECURSIVE) && S_ISDIR(mode)) {
 		enum object_type type;
-		int pathlen = tree_entry_len(path, sha1);
 		char *newbase = malloc_base(base, baselen, path, pathlen);
 		struct tree_desc inner;
 		void *tree;
@@ -224,7 +240,9 @@ static void show_entry(struct diff_options *opt, const char *prefix, struct tree
 		free(tree);
 		free(newbase);
 	} else {
-		opt->add_remove(opt, prefix[0], mode, sha1, base, path);
+		char *fullname = malloc_fullname(base, baselen, path, pathlen);
+		opt->add_remove(opt, prefix[0], mode, sha1, fullname);
+		free(fullname);
 	}
 }
 
-- 
1.5.6.3.3.gfcafb

^ permalink raw reply related

* Re: [PATCH] Fix buffer overflow in git-grep
From: Johannes Schindelin @ 2008-07-16 14:47 UTC (permalink / raw)
  To: Dmitry Potapov; +Cc: git
In-Reply-To: <37fcd2780807160733o156e49c6m51314ff18764ccd5@mail.gmail.com>

Hi,

On Wed, 16 Jul 2008, Dmitry Potapov wrote:

> On Wed, Jul 16, 2008 at 3:54 PM, Dmitry Potapov <dpotapov@gmail.com> wrote:
> >
> > For instance, git show will show you nothing at all and exit with 0.
> > The same problem with git whatchanged. The whole history mysteriously
> > disappeared at that commit, and git whatchanged exited with 0 without
> > any error or warning... Though git log will show you all history, but
> > if you run it with -p then it will also exit with zero at this commit
> > silently like previously history do not exist at all.
> 
> It turned out that git actually crashed while doing diff, but because it
> was a forked process, the parent exited normally with code 0. I wonder
> should not the parent process to exit with a non-zero code if the child
> died by SIGSEG or another signal?

We'll probably need to use the MinGW pager handling for Unix, too, and 
check for a died child.

Not overly trivial, though.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] Fix buffer overflow in git-grep
From: Dmitry Potapov @ 2008-07-16 14:33 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git
In-Reply-To: <20080716115420.GD2925@dpotapov.dyndns.org>

On Wed, Jul 16, 2008 at 3:54 PM, Dmitry Potapov <dpotapov@gmail.com> wrote:
>
> For instance, git show will show you nothing at all and exit with 0.
> The same problem with git whatchanged. The whole history mysteriously
> disappeared at that commit, and git whatchanged exited with 0 without
> any error or warning... Though git log will show you all history, but
> if you run it with -p then it will also exit with zero at this commit
> silently like previously history do not exist at all.

It turned out that git actually crashed while doing diff, but because it
was a forked process, the parent exited normally with code 0. I wonder
should not the parent process to exit with a non-zero code if the child
died by SIGSEG or another signal?

Dmitry

^ permalink raw reply

* Re: git submodules and commit
From: Nigel Magnay @ 2008-07-16 14:31 UTC (permalink / raw)
  To: Petr Baudis; +Cc: Johannes Sixt, Git Mailing List
In-Reply-To: <20080716141738.GN32184@machine.or.cz>

> On Wed, Jul 16, 2008 at 03:03:41PM +0100, Nigel Magnay wrote:
>> - All the technical capability is there, it's just the porcelain
>> that's causing the friction.
>> then
>>  would this not seem to be an area that could be improved? Even if it
>> were an optional mode of working?
>
> So, were there already any patches posted to add such a functionality
> that were rejected? If not, apparently noone cared _enough_, yet. ;-)
> You may be the first!
>
> I don't know if there are any _present_ "free developers" willing to
> pick up this task now.  For many (most?) Git developers, submodules
> simply aren't a priority.  For me, they actually currently are, but I
> probably won't want to use them in your way either (even though I can
> agree that your sentiments are valid), so I will personally invest my
> time in doing other things than figuring out the precise semantics
> these operations should have etc.
>

That's cool. I was guessing it might be the case (or alternatively
that someone might say 'yeah, but it's 25% of the way there'); my
original query was also one of an offer of help ;-) My guess though is
that the core-devs have much more connected neural pathways at
thinking about the problems around the edge cases to be able to give
warnings of  'there be dragons'!


Nigel

^ permalink raw reply

* Re: git submodules and commit
From: Petr Baudis @ 2008-07-16 14:17 UTC (permalink / raw)
  To: Nigel Magnay; +Cc: Johannes Sixt, Git Mailing List
In-Reply-To: <320075ff0807160703v3f16ff5bue722b760ad66488e@mail.gmail.com>

On Wed, Jul 16, 2008 at 03:03:41PM +0100, Nigel Magnay wrote:
> - All the technical capability is there, it's just the porcelain
> that's causing the friction.
> then
>  would this not seem to be an area that could be improved? Even if it
> were an optional mode of working?

So, were there already any patches posted to add such a functionality
that were rejected? If not, apparently noone cared _enough_, yet. ;-)
You may be the first!

I don't know if there are any _present_ "free developers" willing to
pick up this task now.  For many (most?) Git developers, submodules
simply aren't a priority.  For me, they actually currently are, but I
probably won't want to use them in your way either (even though I can
agree that your sentiments are valid), so I will personally invest my
time in doing other things than figuring out the precise semantics
these operations should have etc.

-- 
				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 submodules and commit
From: Nigel Magnay @ 2008-07-16 14:03 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Git Mailing List
In-Reply-To: <487DF9BB.10107@viscovery.net>

On Wed, Jul 16, 2008 at 2:38 PM, Johannes Sixt <j.sixt@viscovery.net> wrote:
> Nigel Magnay schrieb:
>> P and S aren't distant projects, they're closely coupled.
>
> And I'm saying that submodules are designed for *loosely* coupled projects.
>
> It's no wonder that this tool is awkward to use in your workflow.
>

Ok in a sense. I don't think it's particularly clear from the
documentation that this is a limitation of submodules though.

Given that
- The only way in git to separate out re-usable modules is by the use
of submodules
and
- It's a pretty common usecase for these submodules to be interrelated
and
- Looking over the list archives, it seems this is quite common complaint

"I really like the git submodule implementation, I just don't like how
hard it is to work with"

 "The current behaviour strongly encourages me to avoid submodules
when I would otherwise like to use them, just to keep the rest of my
team members (who are not git experts) from going insane."

 "For my use case, I passionately dislike the fact that a submodule is
not updated automatically.  There's never a time when I don't want to
update the submodule.  The submodule is a very important piece of our
project and the super-project depends on it being at the right
version."

and
- All the technical capability is there, it's just the porcelain
that's causing the friction.
then
 would this not seem to be an area that could be improved? Even if it
were an optional mode of working?

^ permalink raw reply

* Re: Breakage caused by 2fe403e7 git-svn.perl: workaround assertions in svn library 1.5.0
From: SZEDER Gábor @ 2008-07-16 13:45 UTC (permalink / raw)
  To: Kevin Ballard; +Cc: Git Mailing List, pape, Eric Wong
In-Reply-To: <6583867F-ED86-4E3D-AE8B-39FB974C43D7@sb.org>

Hi,

On Tue, Jul 08, 2008 at 05:41:48PM -0700, Kevin Ballard wrote:
> Commit 2fe403e7 causes `git svn info` and `git svn info .` to break.
>
>   Use of uninitialized value in localtime at /usr/local/libexec/git- 
> core/git-svn line 4277.
>   No such file or directory at /usr/local/libexec/git-core/git-svn line 
> 897.
>
> If it makes a difference, I have svn 1.4.4 installed, not svn 1.5.
the same problem here, except that I have svn v1.4.6.

The patch in question adds the following line to git-svn.perl's
canonicalize_path():

+       $path =~ s#^\.$##;

which makes an empty string from the path ".".  This is especially
unfortunate for 'git svn info', cmd_info() begins with

        my $path = canonicalize_path(shift or ".");

and from that point everything is ruined, but it boils out only when
it tries to actually open() the empty path.

Removing the mentioned line from canonicalize_path() does resolve this
issue on svn v1.4.6, but I'm afraid it will brake again on svn v1.5.

Gábor

^ permalink raw reply

* Re: git submodules and commit
From: Johannes Sixt @ 2008-07-16 13:38 UTC (permalink / raw)
  To: Nigel Magnay; +Cc: Git Mailing List
In-Reply-To: <320075ff0807160548qae5d702jafe3df63363c512c@mail.gmail.com>

Nigel Magnay schrieb:
> P and S aren't distant projects, they're closely coupled.

And I'm saying that submodules are designed for *loosely* coupled projects.

It's no wonder that this tool is awkward to use in your workflow.

-- Hannes

^ permalink raw reply

* Re: MSysGit Stability
From: Johannes Schindelin @ 2008-07-16 12:58 UTC (permalink / raw)
  To: Peter Harris; +Cc: Joe Fiorini, git
In-Reply-To: <eaa105840807160532t73e6b851h5b78c0d8ad4c1c1a@mail.gmail.com>

Hi,

On Wed, 16 Jul 2008, Peter Harris wrote:

> On Tue, Jul 15, 2008 at 5:58 PM, Joe Fiorini wrote:
> >
> >   Does anyone have experience running Git on Windows?
> 
> I use msysgit regularly. The core git is quite stable, although
> occasionally peripherals have a bit of trouble. For example, git-svn
> was broken recently.

Note that git-svn will be removed in the next alpha release of msysGit, 
due to lack of people interested in fixing the thing.  No, complaining and 
asking others to do all the work does not account for "interest".

Ciao,
Dscho

^ permalink raw reply

* Re: git submodules and commit
From: Nigel Magnay @ 2008-07-16 12:48 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Git Mailing List
In-Reply-To: <487DDCFC.9020007@viscovery.net>

> Ah, is this your actual scenario? Just to make sure we are talking about
> the same thing:
>
> - You own superproject P.
> - $Maintainer owns submodule S.
> - You use S in P.
> - You make changes to S that you would like $Maintainer to include in the
> next release.
> x You use in P your changes to S while $Maintainer has not yet released a
> new version of S with your changes.
> - Finally your changes arrive via the new release of S.
>
> That *is* the intended use-case for submodules. But you have to play the
> game by the rules:
>

Yes, that is the situation - with the proviso that it's not always
clear in company environments who $Maintainer actually is. For
example, if the only changes occurring in S come from me, then chances
are come release cycle, $Maintainer == me.

P and S aren't distant projects, they're closely coupled.

> - $Maintainer defines the official states of S.
>
Yes - there is one branch ('master') which the changes eventually
should be merged to, and releases will be performed on

> - You must never commit an unofficial state of S in P.
>

If by that you mean that the only person to move the branch 'master'
is $Maintainer, then I agree.
If by that you mean that you can't commit at all to the S tree (and
the S submodule pointer) then I don't agree, and I think that's a
serious limitation in productivity.

> The critical step in above list I marked with x:
>
> - During the period where only *you* have the new changes to S, you must
> *not* commit your submodule state to P. Instead, you write P in such a way
> that it can work with both the old version of S and the upcoming release
> that will have your changes[*]. This way you make sure that your consumers
> of P always have a working version regardless of which version of S they use.
>

Just to be clear - there's more than just 'me' working on P - there's
a whole team of people working on it. And there's Q R S and T teams
also working on projects that also have S.

Changes that happen to S are, often, new features or bug fixes. We
can't just stop because there isn't an 'official' version of S yet
(and the official version might end up simply being a FF anyway), so
saying 'don't commit your submodule state to P' is unrealistic.

And that should be the big advantage of git. If we suddenly find we
need some additional functionality in S, we just add it to our
P-branch-of-S. The $Maintainer (if he exists) can review these
upcoming changes in the tree, and merge them to master as appropriate
(or work with the projects to iron out cross-branch
incompatibilities). The best example is that S is a "product", and (by
management decree), the only product changes that happen will occur
because of *projects* (like P). And we can do this (and it's
infinitely better than svn, where 'ooh, branches too hard, everyone in
[P-T] just commit to trunk'. But the UI is an ache.


> - After you have received the new release of S from $Maintainer, you
> commit the new state of S in P. And if you are nice to your consumers of
> P, then you *do not* remove the workaround from P just yet, so that you
> don't force them to upgrade S. You will remove it later only if it becomes
> a maintainance burden.
>
Maintaining backwards compatibility isn't an issue at all for us.

> [*] If it is not possible to make P work with old and new versions, then
> you have to work closely with the $Maintainer so that you never need
> commit an unofficial state of S into P.
>
> -- Hannes
>
>

^ permalink raw reply

* [PATCH] Documentation/git-cherry-pick.txt et al.: Fix misleading -n description
From: Petr Baudis @ 2008-07-16 12:35 UTC (permalink / raw)
  To: git; +Cc: gitster

The manual page of git-cherry-pick and git-revert asserts that -n works
primarily on the working tree, while in fact the primary object it operates
on is the index, and the changes only "accidentally" propagate to the
working tree. This e.g. leads innocent #git IRC folks to believe that you
can use -n to prepare changes for git-add -i staging.

Signed-off-by: Petr Baudis <pasky@suse.cz>
---

  I'm actually not *entirely* sure about the exact working tree vs. index
semantics of the -n parameter, this is a guess based on the code.
Unfortunately, I'm currently too busy with other things to check
thoroughly, and I'm sure other people on the list will know off-hand.

 Documentation/git-cherry-pick.txt |   10 +++++-----
 Documentation/git-revert.txt      |   16 ++++++++--------
 2 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt
index a691173..50fb3d5 100644
--- a/Documentation/git-cherry-pick.txt
+++ b/Documentation/git-cherry-pick.txt
@@ -58,14 +58,14 @@ OPTIONS
 	Usually the command automatically creates a commit with
 	a commit log message stating which commit was
 	cherry-picked.  This flag applies the change necessary
-	to cherry-pick the named commit to your working tree,
-	but does not make the commit.  In addition, when this
-	option is used, your working tree does not have to match
+	to cherry-pick the named commit to your working tree
+	and the index, but does not make the commit.  In addition,
+	when this option is used, your index does not have to match
 	the HEAD commit.  The cherry-pick is done against the
-	beginning state of your working tree.
+	beginning state of your index.
 +
 This is useful when cherry-picking more than one commits'
-effect to your working tree in a row.
+effect to your index in a row.
 
 -s::
 --signoff::
diff --git a/Documentation/git-revert.txt b/Documentation/git-revert.txt
index 5411edc..271850f 100644
--- a/Documentation/git-revert.txt
+++ b/Documentation/git-revert.txt
@@ -43,16 +43,16 @@ OPTIONS
 -n::
 --no-commit::
 	Usually the command automatically creates a commit with
-	a commit log message stating which commit was reverted.
-	This flag applies the change necessary to revert the
-	named commit to your working tree, but does not make the
-	commit.  In addition, when this option is used, your
-	working tree does not have to match the HEAD commit.
-	The revert is done against the beginning state of your
-	working tree.
+	a commit log message stating which commit was
+	reverted.  This flag applies the change necessary
+	to revert the named commit to your working tree
+	and the index, but does not make the commit.  In addition,
+	when this option is used, your index does not have to match
+	the HEAD commit.  The revert is done against the
+	beginning state of your index.
 +
 This is useful when reverting more than one commits'
-effect to your working tree in a row.
+effect to your index in a row.
 
 -s::
 --signoff::

^ permalink raw reply related

* Re: MSysGit Stability
From: Peter Harris @ 2008-07-16 12:33 UTC (permalink / raw)
  To: Joe Fiorini; +Cc: git
In-Reply-To: <73fd69b50807151458u22a383a3l343779e47f4161fa@mail.gmail.com>

On Tue, Jul 15, 2008 at 5:58 PM, Joe Fiorini wrote:
>
>   Does anyone have experience running Git on Windows?

I use msysgit regularly. The core git is quite stable, although
occasionally peripherals have a bit of trouble. For example, git-svn
was broken recently.

Overall, I would recommend msysgit for anyone stuck on Windows.

Peter Harris

^ permalink raw reply

* Re: git submodules and commit
From: Petr Baudis @ 2008-07-16 12:11 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Nigel Magnay, Git Mailing List
In-Reply-To: <487DDCFC.9020007@viscovery.net>

On Wed, Jul 16, 2008 at 01:35:24PM +0200, Johannes Sixt wrote:
> Ah, is this your actual scenario? Just to make sure we are talking about
> the same thing:
> 
> - You own superproject P.
> - $Maintainer owns submodule S.
> - You use S in P.
> - You make changes to S that you would like $Maintainer to include in the
> next release.
> x You use in P your changes to S while $Maintainer has not yet released a
> new version of S with your changes.
> - Finally your changes arrive via the new release of S.
> 
> That *is* the intended use-case for submodules. But you have to play the
> game by the rules:
> 
> - $Maintainer defines the official states of S.
> 
> - You must never commit an unofficial state of S in P.

I think the issue here is that $Maintainer = him (or Maintainers(P) =
Maintainers(S), in general); the workflow you described still works, but
is overly complicated and that is the original complaint.

				Petr "Pasky" Baudis

^ 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