Git development
 help / color / mirror / Atom feed
* Re: A flaw in dep generation with gcc -MMD?
From: Jonathan Nieder @ 2011-11-18 21:16 UTC (permalink / raw)
  To: Samuel Bronson; +Cc: git, Nguyen Thai Ngoc Duy, Miles Bader
In-Reply-To: <loom.20111118T191851-152@post.gmane.org>

Hi,

(restoring cc list.  Unfortunately gmane's web interface doesn't
 provide a built-in way to reply-to-all.  [1] has a hackish
 workaround.)
Samuel Bronson wrote:
> Nguyen Thai Ngoc Duy writes:

>> "gcc -MF depfile -MMD -MP -c -o path/to/file.o" will produce "depfile"
>> with target given by "-o". When ccache runs, it executes "gcc -MF
>> depfile -MMD -MP -E" instead to get the final content for hashing.
>> Notice that "-c -o" combination is replaced by "-E". The latter
>> produces target without leading path.
[...]
> I'm pretty sure you should report this against ccache; GCC seems to be
> behaving as documented.

Seems to have been fixed by v3.0pre0~187:

 commit e8354384
 Author: Andrea Bittau <a.bittau@cs.ucl.ac.uk>
 Date:   Sun Nov 1 19:39:58 2009 +0100

     Fix handling of the -MD and -MDD options

     From <http://lists.samba.org/archive/ccache/2007q2/000272.html>:

     The -MD and -MDD options automatically determine where the dependency file
     should land and what the target should look like based on the -o option.
     However, ccache drops -o and things mess up. The original patch was posted by
     Kaz Kylheku but I reworked it to make it work properly. Here is his post:

     http://lists.samba.org/archive/ccache/2006q4/000249.html

Workaround incoming in a few moments.

Thanks,
Jonathan

^ permalink raw reply

* Re: .git ignored regardless of --git-dir value
From: Junio C Hamano @ 2011-11-18 21:26 UTC (permalink / raw)
  To: Shawn Ferris; +Cc: git
In-Reply-To: <CAC2kKA_PZNDg_dPjWXKeFU4ZVpMas3PubZfSgTnfCfVPuNPdsA@mail.gmail.com>

Shawn Ferris <shawn.ferris@gmail.com> writes:

> Is it expected behavior to have the .git directory ignored, even after
> specifying an alternate location with --git-dir? For example:
>
> $ git --git-dir=.foo init
> Initialized empty Git repository in /home/sferris/work/t/.foo/

GIT_DIR and --git-dir are meant to refer to a different .git dir (or a
bare-looking repository) located elsewhere, and not for a random pathname
like ".foo". No matter what, ".git/" anywhere is ignored from very early
days of Git, as Linus himself writes in the source, e.g. 8695c8b (Add
"show-files" command to show the list of managed (or non-managed) files.,
2005-04-11):

 ...
 * Also, we currently ignore all names starting with a dot.
 * That likely will not change. 
 ...

^ permalink raw reply

* [ANNOUNCE] Git 1.7.7.4
From: Junio C Hamano @ 2011-11-18 22:38 UTC (permalink / raw)
  To: git; +Cc: Linux Kernel

The latest maintenance release Git 1.7.7.4 is available.

The release tarballs are found at:

    http://code.google.com/p/git-core/downloads/list

and their SHA-1 checksums are:

5b6920989480a37ec65977e756b24961578795dd  git-1.7.7.4.tar.gz
6012cb017a04ded85c48ca5510f741e98c02f671  git-htmldocs-1.7.7.4.tar.gz
cb21e55ae793865453c165a0e666348f2db8c740  git-manpages-1.7.7.4.tar.gz

Also the following public repositories all have a copy of the v1.7.7.4
tag and the maint branch that the tag points at:

  url = git://repo.or.cz/alt-git.git
  url = https://code.google.com/p/git-core/
  url = git://git.sourceforge.jp/gitroot/git-core/git.git
  url = git://git-core.git.sourceforge.net/gitroot/git-core/git-core
  url = https://github.com/gitster/git


Git v1.7.7.4 Release Notes
==========================

Fixes since v1.7.7.3
--------------------

 * A few header dependencies were missing from the Makefile.

 * Some newer parts of the code used C99 __VA_ARGS__ while we still
   try to cater to older compilers.

 * "git name-rev --all" tried to name all _objects_, naturally failing to
   describe many blobs and trees, instead of showing only commits as
   advertised in its documentation.

----------------------------------------------------------------

Changes since v1.7.7.3 are as follows:

Jonathan Nieder (2):
      notes merge: eliminate OUTPUT macro
      Makefile: add missing header file dependencies

Junio C Hamano (2):
      name-rev --all: do not even attempt to describe non-commit object
      Git 1.7.7.4

Marc-André Lureau (1):
      mailmap: xcalloc mailmap_info

^ permalink raw reply

* Re: .git ignored regardless of --git-dir value
From: Junio C Hamano @ 2011-11-18 22:47 UTC (permalink / raw)
  To: git; +Cc: Shawn Ferris
In-Reply-To: <7vsjlljf57.fsf@alter.siamese.dyndns.org>

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

> Shawn Ferris <shawn.ferris@gmail.com> writes:
>
>> Is it expected behavior to have the .git directory ignored, even after
>> specifying an alternate location with --git-dir? For example:
>>
>> $ git --git-dir=.foo init
>> Initialized empty Git repository in /home/sferris/work/t/.foo/
>
> GIT_DIR and --git-dir are meant to refer to a different .git dir (or a
> bare-looking repository) located elsewhere, and not for a random pathname
> like ".foo". No matter what, ".git/" anywhere is ignored from very early
> days of Git, as Linus himself writes in the source, e.g. 8695c8b (Add
> "show-files" command to show the list of managed (or non-managed) files.,
> 2005-04-11):
>
>  ...
>  * Also, we currently ignore all names starting with a dot.
>  * That likely will not change. 
>  ...

Sorry, but the above quote is wrong.  The correct one is from 453ec4b
(libify git-ls-files directory traversal, 2006-05-16), and survives to
this day in dir.c:

 ...
 * Read a directory tree. We currently ignore anything but
 * directories, regular files and symlinks. That's because git
 * doesn't handle them at all yet. Maybe that will change some
 * day.
 *
 * Also, we ignore the name ".git" (even if it is not a directory).
 * That likely will not change.
 ...

In other words, originally we ignored all names starting with a dot and
declared that likely will not change, but then loosened the rule to let
people manage their .bashrc and friends. But ".git" is still special, and
that likely will not change.

^ permalink raw reply

* [PATCH] Makefile: explicitly set target name for autogenerated dependencies
From: Jonathan Nieder @ 2011-11-18 23:23 UTC (permalink / raw)
  To: Nguyen Thai Ngoc Duy; +Cc: Git Mailing List, Fredrik Kuivinen, Junio C Hamano
In-Reply-To: <CACsJy8BZMDyf4MCiKxPJ5Z+XS+C-MC82SpMFyWgiXmb9xCnScw@mail.gmail.com>

"gcc -MF depfile -MMD -MP -c -o path/to/file.o" produces a makefile
snippet named "depfile" describing what files are needed to build the
target given by "-o".  When ccache versions before v3.0pre0~187 (Fix
handling of the -MD and -MDD options, 2009-11-01) run, they execute

	gcc -MF depfile -MMD -MP -E

instead to get the final content for hashing.  Notice that the "-c -o"
combination is replaced by "-E".  The result is a target name without
a leading path.

Thus when building git with such versions of ccache with
COMPUTE_HEADER_DEPENDENCIES enabled, the generated makefile snippets
define dependencies for the wrong target:

	$ make builtin/add.o
	GIT_VERSION = 1.7.8.rc3
	    * new build flags or prefix
	    CC builtin/add.o
	$ head -1 builtin/.depend/add.o.d
	add.o: builtin/add.c cache.h git-compat-util.h compat/bswap.h strbuf.h \

After a change in a header file, object files in a subdirectory are
not automatically rebuilt by "make":

	$ touch cache.h
	$ make builtin/add.o
	$

Luckily we can prevent trouble by explicitly supplying the name of the
target to ccache and gcc, using the -MQ option.  Do so.

Reported-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Hi,

Nguyen Thai Ngoc Duy wrote:

> My builtin/.depend/add.o.d says
>
> add.o: .... cache.h ...
>
> Shouldn't it be "builtin/add.o: ... cache.h ..."?

The following seems to do the trick for me.  Thanks again for catching
it.

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

diff --git a/Makefile b/Makefile
index ee34eab8..71ad4b26 100644
--- a/Makefile
+++ b/Makefile
@@ -1250,7 +1250,8 @@ USE_COMPUTED_HEADER_DEPENDENCIES =
 else
 ifndef COMPUTE_HEADER_DEPENDENCIES
 dep_check = $(shell $(CC) $(ALL_CFLAGS) \
-	-c -MF /dev/null -MMD -MP -x c /dev/null -o /dev/null 2>&1; \
+	-c -MF /dev/null -MQ /dev/null -MMD -MP \
+	-x c /dev/null -o /dev/null 2>&1; \
 	echo $$?)
 ifeq ($(dep_check),0)
 COMPUTE_HEADER_DEPENDENCIES=YesPlease
@@ -1912,7 +1913,7 @@ $(dep_dirs):
 
 missing_dep_dirs := $(filter-out $(wildcard $(dep_dirs)),$(dep_dirs))
 dep_file = $(dir $@).depend/$(notdir $@).d
-dep_args = -MF $(dep_file) -MMD -MP
+dep_args = -MF $(dep_file) -MQ $@ -MMD -MP
 ifdef CHECK_HEADER_DEPENDENCIES
 $(error cannot compute header dependencies outside a normal build. \
 Please unset CHECK_HEADER_DEPENDENCIES and try again)
-- 
1.7.8.rc3

^ permalink raw reply related

* Re: .git ignored regardless of --git-dir value
From: Shawn Ferris @ 2011-11-18 23:44 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v7h2xjbf4.fsf@alter.siamese.dyndns.org>

On Fri, Nov 18, 2011 at 3:47 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Junio C Hamano <gitster@pobox.com> writes:

>>
>> GIT_DIR and --git-dir are meant to refer to a different .git dir (or a
>> bare-looking repository) located elsewhere, and not for a random pathname
>> like ".foo". No matter what, ".git/" anywhere is ignored from very early

I'm extremely green to git, so please forgive if this is obvious, but,
could I indulge and ask what a sample use case of that would be?
(since it's not intended for what I assumed it was) And, if the
behavior I expected was possible, simply, with perhaps a separate
option, what is the likelihood a patch would be accepted? (Or am I
just talking nonsense and should just go away) ;D

Thanks Again!

Shawn

^ permalink raw reply

* Re: [PATCH] receive-pack, fetch-pack: reject bogus pack that records objects twice
From: Shawn Pearce @ 2011-11-18 23:50 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20111118103355.GA4854@sigill.intra.peff.net>

On Fri, Nov 18, 2011 at 02:33, Jeff King <peff@peff.net> wrote:
> On Wed, Nov 16, 2011 at 10:04:13PM -0800, Junio C Hamano wrote:
>
>> When receive-pack & fetch-pack are run and store the pack obtained over
>> the wire to a local repository, they internally run the index-pack command
>> with the --strict option. Make sure that we reject incoming packfile that
>> records objects twice to avoid spreading such a damage.
>
> If we are fixing a thin pack (which should be the case most of the
> time), we are rewriting the packfile anyway. Shouldn't we just omit
> the duplicate?
>
> I guess I'm a little confused about what is generating these duplicates.
> A buggy git? A malicious server? Bad luck?

A buggy Git. We found a case where JGit could generate duplicate
objects in the pack stream during a clone. The resulting client
worked... until it tried to do `git gc` or really any sort of `git
pack-objects`.

In my opinion, a pack should never contain duplicate objects. Its a
buggy remote that sends them. What I like about this patch is it stops
and tells the user the remote is broken, which it is.

^ permalink raw reply

* Re: A flaw in dep generation with gcc -MMD?
From: Nguyen Thai Ngoc Duy @ 2011-11-19  4:53 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: Miles Bader, Jonathan Nieder, Git Mailing List
In-Reply-To: <m21ut5dyei.fsf@igel.home>

On Sat, Nov 19, 2011 at 2:27 AM, Andreas Schwab <schwab@linux-m68k.org> wrote:
> Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
>
>> OK it's not gcc problem. I upgraded to 4.5.3 and still had the same
>> problem. I used ccache though. Without ccache, gcc produced correct
>> .o.d files.
>
> I'm also using ccache (version 3.1.3) and get correct dependencies.

I use ccache-2.4. Jonathan pointed out 3.x has been fixed elsewhere in
this thread . I'm going to ask Gentoo to stablize one of 3.x version.
-- 
Duy

^ permalink raw reply

* Re: [PATCH] Makefile: explicitly set target name for autogenerated dependencies
From: Nguyen Thai Ngoc Duy @ 2011-11-19  5:13 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Git Mailing List, Fredrik Kuivinen, Junio C Hamano
In-Reply-To: <20111118232324.GA8746@elie.hsd1.il.comcast.net>

On Sat, Nov 19, 2011 at 6:23 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Luckily we can prevent trouble by explicitly supplying the name of the
> target to ccache and gcc, using the -MQ option.  Do so.
>
> Reported-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
> ---
> Hi,
>
> Nguyen Thai Ngoc Duy wrote:
>
>> My builtin/.depend/add.o.d says
>>
>> add.o: .... cache.h ...
>>
>> Shouldn't it be "builtin/add.o: ... cache.h ..."?
>
> The following seems to do the trick for me.  Thanks again for catching
> it.

Works for me too.
-- 
Duy

^ permalink raw reply

* clean bug on ignored subdirectories with no tracked files?
From: Jay Soffian @ 2011-11-19  5:38 UTC (permalink / raw)
  To: git

git init test_repo &&
cd test_repo &&
mkdir -p foo/bar &&
echo baz > foo/bar/baz &&
echo /foo/bar > .gitignore &&
git add .gitignore &&
git clean -n -d

Initialized empty Git repository in .../test_repo/.git/
Would remove foo/

Seems surprising. The work-around is either using '/foo' in the
top-level .gitignore or adding a tracked file to the 'foo' directory.

j.

^ permalink raw reply

* [PATCH] upload-archive: use start_command instead of fork
From: Jeff King @ 2011-11-19  7:40 UTC (permalink / raw)
  To: Erik Faye-Lund; +Cc: git

The POSIX-function fork is not supported on Windows. Use our
start_command API instead, respawning ourselves in a special
"writer" mode to follow the alternate code path.

Remove the NOT_MINGW-prereq for t5000, as git-archive --remote
now works.

Signed-off-by: Jeff King <peff@peff.net>
---
Now that Junio has reverted the original implementation, I took a closer
look at how this ought to be done.

I looked briefly into doing the writing in-process, or using
start_async.  But the code relies too heavily on being able to die(),
and having the parent process grab stderr and the exit code to send to
the client. So going that direction would involve a lot of changes
either to the async code, or to the archive code.

We really do want a separate process, as it fits in with our usual die()
mentality for errors.  But fortunately looking into the async stuff made
me realize another issue with the original patch. Originally we forked
as the first thing. Thus any error messages or die()s (e.g., because we
couldn't enter_repo, or got bogus input from the client) would be in the
child process, and the parent sideband muxer would relay the errors
properly.  But in your original patch, we do quite a bit of processing
before spawning; those errors would go to git-daemon's stderr, and the
client would just see an unexpected hangup.

So we really do want to just switch the "fork immediately" to "spawn
immediately". Which fortunately means the change ends up being even
smaller, and I'm way more confident that there won't be side effects;
it's almost nothing but converting pipe/fork code into start_command
code.

And as an added bonus, the diffstat hopefully speaks for itself. :)

 builtin.h                |    1 +
 builtin/upload-archive.c |   43 ++++++++++++-------------------------------
 git.c                    |    1 +
 t/t5000-tar-tree.sh      |   10 +++++-----
 4 files changed, 19 insertions(+), 36 deletions(-)

diff --git a/builtin.h b/builtin.h
index 0e9da90..f2357a9 100644
--- a/builtin.h
+++ b/builtin.h
@@ -133,6 +133,7 @@ int copy_note_for_rewrite(struct notes_rewrite_cfg *c,
 extern int cmd_update_ref(int argc, const char **argv, const char *prefix);
 extern int cmd_update_server_info(int argc, const char **argv, const char *prefix);
 extern int cmd_upload_archive(int argc, const char **argv, const char *prefix);
+extern int cmd_upload_archive_writer(int argc, const char **argv, const char *prefix);
 extern int cmd_upload_tar(int argc, const char **argv, const char *prefix);
 extern int cmd_var(int argc, const char **argv, const char *prefix);
 extern int cmd_verify_tag(int argc, const char **argv, const char *prefix);
diff --git a/builtin/upload-archive.c b/builtin/upload-archive.c
index 2d0b383..b928beb 100644
--- a/builtin/upload-archive.c
+++ b/builtin/upload-archive.c
@@ -6,6 +6,7 @@
 #include "archive.h"
 #include "pkt-line.h"
 #include "sideband.h"
+#include "run-command.h"
 
 static const char upload_archive_usage[] =
 	"git upload-archive <repo>";
@@ -13,12 +14,9 @@
 static const char deadchild[] =
 "git upload-archive: archiver died with error";
 
-static const char lostchild[] =
-"git upload-archive: archiver process was lost";
-
 #define MAX_ARGS (64)
 
-static int run_upload_archive(int argc, const char **argv, const char *prefix)
+int cmd_upload_archive_writer(int argc, const char **argv, const char *prefix)
 {
 	const char *sent_argv[MAX_ARGS];
 	const char *arg_cmd = "argument ";
@@ -96,8 +94,8 @@ static ssize_t process_input(int child_fd, int band)
 
 int cmd_upload_archive(int argc, const char **argv, const char *prefix)
 {
-	pid_t writer;
-	int fd1[2], fd2[2];
+	struct child_process writer = { argv };
+
 	/*
 	 * Set up sideband subprocess.
 	 *
@@ -105,39 +103,24 @@ int cmd_upload_archive(int argc, const char **argv, const char *prefix)
 	 * multiplexed out to our fd#1.  If the child dies, we tell the other
 	 * end over channel #3.
 	 */
-	if (pipe(fd1) < 0 || pipe(fd2) < 0) {
-		int err = errno;
-		packet_write(1, "NACK pipe failed on the remote side\n");
-		die("upload-archive: %s", strerror(err));
-	}
-	writer = fork();
-	if (writer < 0) {
+	argv[0] = "upload-archive--writer";
+	writer.out = writer.err = -1;
+	writer.git_cmd = 1;
+	if (start_command(&writer)) {
 		int err = errno;
-		packet_write(1, "NACK fork failed on the remote side\n");
+		packet_write(1, "NACK unable to spawn subprocess\n");
 		die("upload-archive: %s", strerror(err));
 	}
-	if (!writer) {
-		/* child - connect fd#1 and fd#2 to the pipe */
-		dup2(fd1[1], 1);
-		dup2(fd2[1], 2);
-		close(fd1[1]); close(fd2[1]);
-		close(fd1[0]); close(fd2[0]); /* we do not read from pipe */
-
-		exit(run_upload_archive(argc, argv, prefix));
-	}
 
-	/* parent - read from child, multiplex and send out to fd#1 */
-	close(fd1[1]); close(fd2[1]); /* we do not write to pipe */
 	packet_write(1, "ACK\n");
 	packet_flush(1);
 
 	while (1) {
 		struct pollfd pfd[2];
-		int status;
 
-		pfd[0].fd = fd1[0];
+		pfd[0].fd = writer.out;
 		pfd[0].events = POLLIN;
-		pfd[1].fd = fd2[0];
+		pfd[1].fd = writer.err;
 		pfd[1].events = POLLIN;
 		if (poll(pfd, 2, -1) < 0) {
 			if (errno != EINTR) {
@@ -156,9 +139,7 @@ int cmd_upload_archive(int argc, const char **argv, const char *prefix)
 			if (process_input(pfd[0].fd, 1))
 				continue;
 
-		if (waitpid(writer, &status, 0) < 0)
-			error_clnt("%s", lostchild);
-		else if (!WIFEXITED(status) || WEXITSTATUS(status) > 0)
+		if (finish_command(&writer))
 			error_clnt("%s", deadchild);
 		packet_flush(1);
 		break;
diff --git a/git.c b/git.c
index 8e34903..250f448 100644
--- a/git.c
+++ b/git.c
@@ -434,6 +434,7 @@ static void handle_internal_command(int argc, const char **argv)
 		{ "update-ref", cmd_update_ref, RUN_SETUP },
 		{ "update-server-info", cmd_update_server_info, RUN_SETUP },
 		{ "upload-archive", cmd_upload_archive },
+		{ "upload-archive--writer", cmd_upload_archive_writer },
 		{ "var", cmd_var, RUN_SETUP_GENTLY },
 		{ "verify-pack", cmd_verify_pack },
 		{ "verify-tag", cmd_verify_tag, RUN_SETUP },
diff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh
index d906898..889842e 100755
--- a/t/t5000-tar-tree.sh
+++ b/t/t5000-tar-tree.sh
@@ -96,7 +96,7 @@ test_expect_success 'git archive with --output' \
     'git archive --output=b4.tar HEAD &&
     test_cmp b.tar b4.tar'
 
-test_expect_success NOT_MINGW 'git archive --remote' \
+test_expect_success 'git archive --remote' \
     'git archive --remote=. HEAD >b5.tar &&
     test_cmp b.tar b5.tar'
 
@@ -266,7 +266,7 @@ test_expect_success 'archive --list mentions user filter' '
 	grep "^bar\$" output
 '
 
-test_expect_success NOT_MINGW 'archive --list shows only enabled remote filters' '
+test_expect_success 'archive --list shows only enabled remote filters' '
 	git archive --list --remote=. >output &&
 	! grep "^tar\.foo\$" output &&
 	grep "^bar\$" output
@@ -298,7 +298,7 @@ test_expect_success 'extension matching requires dot' '
 	test_cmp b.tar config-implicittar.foo
 '
 
-test_expect_success NOT_MINGW 'only enabled filters are available remotely' '
+test_expect_success 'only enabled filters are available remotely' '
 	test_must_fail git archive --remote=. --format=tar.foo HEAD \
 		>remote.tar.foo &&
 	git archive --remote=. --format=bar >remote.bar HEAD &&
@@ -341,12 +341,12 @@ test_expect_success GZIP,GUNZIP 'extract tgz file' '
 	test_cmp b.tar j.tar
 '
 
-test_expect_success GZIP,NOT_MINGW 'remote tar.gz is allowed by default' '
+test_expect_success GZIP 'remote tar.gz is allowed by default' '
 	git archive --remote=. --format=tar.gz HEAD >remote.tar.gz &&
 	test_cmp j.tgz remote.tar.gz
 '
 
-test_expect_success GZIP,NOT_MINGW 'remote tar.gz can be disabled' '
+test_expect_success GZIP 'remote tar.gz can be disabled' '
 	git config tar.tar.gz.remote false &&
 	test_must_fail git archive --remote=. --format=tar.gz HEAD \
 		>remote.tar.gz
-- 
1.7.7.3.8.g38efa

^ permalink raw reply related

* [PATCH 1/2] MSVC: Do not close stdout to prevent a crash
From: Vincent van Ravesteijn @ 2011-11-19 13:45 UTC (permalink / raw)
  To: git
  Cc: msysgit, gitster, kusmabite, Johannes.Schindelin,
	Vincent van Ravesteijn

When compiled with MSVC, git crashes on Windows when calling
fstat(stdout) when stdout is closed. fstat is being called at the end of
run_builtin and this will thus be a problem for builtin command that close
stdout. This happens for 'format-patch' which closes stdout after a call to
freopen which directs stdout to the format patch file.

To prevent the crash and to prevent git from writing cruft into the patch
file, we do not close stdout, but redirect it to "nul" instead.

Signed-off-by: Vincent van Ravesteijn <vfr@lyx.org>
---
 compat/mingw.c |    8 ++++++++
 compat/mingw.h |    3 +++
 2 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/compat/mingw.c b/compat/mingw.c
index efdc703..8943df5 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -319,6 +319,14 @@ ssize_t mingw_write(int fd, const void *buf, size_t count)
 	return write(fd, buf, min(count, 31 * 1024 * 1024));
 }
 
+#undef fclose
+int mingw_fclose (FILE *stream)
+{
+	if (fileno(stream) == 1 && freopen("nul", "w", stream))
+		return 0;
+	return fclose(stream);
+}
+
 #undef fopen
 FILE *mingw_fopen (const char *filename, const char *otype)
 {
diff --git a/compat/mingw.h b/compat/mingw.h
index ff18401..80a6015 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -179,6 +179,9 @@ int mingw_open (const char *filename, int oflags, ...);
 ssize_t mingw_write(int fd, const void *buf, size_t count);
 #define write mingw_write
 
+int mingw_fclose(FILE *stream);
+#define fclose mingw_fclose
+
 FILE *mingw_fopen (const char *filename, const char *otype);
 #define fopen mingw_fopen
 
-- 
1.7.4.1

^ permalink raw reply related

* [PATCH 2/2] MSVC: Use _putenv instead of putenv to prevent a crash
From: Vincent van Ravesteijn @ 2011-11-19 13:52 UTC (permalink / raw)
  To: git
  Cc: msysgit, gitster, kusmabite, Johannes.Schindelin,
	Vincent van Ravesteijn
In-Reply-To: <1321710345-2299-1-git-send-email-vfr@lyx.org>

Git crashes when trying to clear a nonexistent environment variable using
the putenv function. The crash occurs when freeing the option string. In
debug mode the assert "CrtIsValidHeapPointer" fails.

Using _putenv instead of putenv makes the crash and assert disappear.

Signed-off-by: Vincent van Ravesteijn <vfr@lyx.org>
---
The strange thing is that all over the internet people are claiming
that there is no difference between putenv and _putenv. Still, using
_putenv fixes the crash for me. 

If there is someone around who is more knowledgeable in this area,
please comment.

 compat/setenv.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/compat/setenv.c b/compat/setenv.c
index 3a22ea7..b23937d 100644
--- a/compat/setenv.c
+++ b/compat/setenv.c
@@ -23,7 +23,7 @@ int gitsetenv(const char *name, const char *value, int replace)
 	memcpy(envstr + namelen + 1, value, valuelen);
 	envstr[namelen + valuelen + 1] = 0;
 
-	out = putenv(envstr);
+	out = _putenv(envstr);
 	/* putenv(3) makes the argument string part of the environment,
 	 * and changing that string modifies the environment --- which
 	 * means we do not own that storage anymore.  Do not free
-- 
1.7.4.1

^ permalink raw reply related

* Re: [PATCH 1/2] MSVC: Do not close stdout to prevent a crash
From: Andreas Schwab @ 2011-11-19 14:41 UTC (permalink / raw)
  To: Vincent van Ravesteijn
  Cc: git, msysgit, gitster, kusmabite, Johannes.Schindelin
In-Reply-To: <1321710345-2299-1-git-send-email-vfr@lyx.org>

Vincent van Ravesteijn <vfr@lyx.org> writes:

> When compiled with MSVC, git crashes on Windows when calling
> fstat(stdout) when stdout is closed. fstat is being called at the end of

ITYM fileno(stdout).

> run_builtin and this will thus be a problem for builtin command that close
> stdout. This happens for 'format-patch' which closes stdout after a call to
> freopen which directs stdout to the format patch file.

It shouldn't do that in the first place.  This is an error on any
platform.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply

* Fwd: I: [1] new comment on LinkedIn - in search of a git guru from paypal
From: Elia Pinto @ 2011-11-19 16:30 UTC (permalink / raw)
  To: Git

[-- Attachment #1: Type: text/plain, Size: 1433 bytes --]

 Sorry for the OT. But perhaps someone here could be interested.  best regards.

---------- Forwarded message ----------
From: "pinto.elia@gmail.com" <pinto.elia@gmail.com>
Date: Sat, 19 Nov 2011 15:48:26 +0000
Subject: I: [1] new comment on LinkedIn
To: "gitter.spiros@gmail.com" <gitter.spiros@gmail.com>


----Messaggio originale----
Da: Git Version Control System Group Members
Inviato:  19/11/2011, 13:36
A: Elia Pinto
Oggetto: [1] new comment on LinkedIn


Git Version Control System
  Today's new discussions from Git Version Control System group
members.  Change the frequency of this digest:
  http://www.linkedin.com/e/-b9sv29-gv6lq696-2k/ahs/1742047/EMLt_anet_settings/?hs=false&tok=1CjwcIloCk8501






Send me an email for each new discussion &#187;
http://www.linkedin.com/e/-b9sv29-gv6lq696-2k/snp/1742047/true/grp_email_subscribe_new_posts/?hs=false&tok=2_AZzjcLek8501


Active Discussion of the day

 * Marie  Godeloson started a discussion on a news article:
  Seeking a Git Guru - will work closely with architects and other key
engineering teams to help build and deploy our next generation
applications for PayPal. Email resume to mgodeloson@paypal.com (1)

  > Is this vacancy applicable for India based locations ?



 View discussion &raquo;
http://www.linkedin.com/e/-b9sv29-gv6lq696-2k/vai/1742047/80991981/member/EMLt_anet_act_disc/?hs=false&tok=35hWQM5KSk8501






-- 
Inviato dal mio dispositivo mobile

[-- Attachment #2: INBOX__184637_1.txt --]
[-- Type: application/octet-stream, Size: 1013 bytes --]

[-- Attachment #3: INBOX__184637_2.txt --]
[-- Type: application/octet-stream, Size: 1359 bytes --]

^ permalink raw reply

* Switch from svn to git and modify repo completely
From: Matthias Fechner @ 2011-11-19 17:11 UTC (permalink / raw)
  To: Git

Dear Git List,

I just started to migrate a svn repo to a git repo and was successfully
with the "git svn" command and waiting a long time :)

I have now some confidential documents in the repository I must remove
completely (including the complete history). These are single files
spread over the complete repository. Is this possible with git?

The next step I would like to do is separate the repo and smaller ones.
The problem here is it is not only take this directory and move it out,
it is a complete mess and the files are spread over the complete repo
and over all the revisions (files were moved around in the old
subversion repo from folder to folder). Is there a possibility to say
take file1, file2, fileN and directory1, directoryN and move it to
another repo or remove all not matching files/directories completely?

Thanks a lot for some hints which commands I could use here or if it is
possible.

Bye
Matthias

^ permalink raw reply

* Re: Switch from svn to git and modify repo completely
From: Alexey Shumkin @ 2011-11-19 18:50 UTC (permalink / raw)
  To: Matthias Fechner; +Cc: git
In-Reply-To: <4EC7E32A.9040903@fechner.net>

> Dear Git List,
> 
> I just started to migrate a svn repo to a git repo and was
> successfully with the "git svn" command and waiting a long time :)
> 
> I have now some confidential documents in the repository I must remove
> completely (including the complete history). These are single files
> spread over the complete repository. Is this possible with git?

take a look at "git filter-branch"

^ permalink raw reply

* Re: [PATCH 1/2] MSVC: Do not close stdout to prevent a crash
From: Junio C Hamano @ 2011-11-19 19:11 UTC (permalink / raw)
  To: Andreas Schwab
  Cc: Vincent van Ravesteijn, git, msysgit, gitster, kusmabite,
	Johannes.Schindelin
In-Reply-To: <m2sjlkcgyl.fsf@igel.home>

Andreas Schwab <schwab@linux-m68k.org> writes:

> Vincent van Ravesteijn <vfr@lyx.org> writes:
>
>> When compiled with MSVC, git crashes on Windows when calling
>> fstat(stdout) when stdout is closed. fstat is being called at the end of
>
> ITYM fileno(stdout).
> 
>> run_builtin and this will thus be a problem for builtin command that close
>> stdout. This happens for 'format-patch' which closes stdout after a call to
>> freopen which directs stdout to the format patch file.
>
> It shouldn't do that in the first place.  This is an error on any
> platform.

Correct. The clean-up codepath is for built-in command implementations
that write out their result and return 0 to signal success. If we let the
crt0 to run its usual clean-ups like closing the standard output stream,
we wouldn't be able to catch errors from there.

For built-ins that perform their own clean-ups, it is their responsibility
to be careful, hence we skip this part of the code.

We have relied on fstat(-1, &st) to correctly error out, and if MSVC build
crashes, it is a bug in its fstat() emulation, I would think.

We could do something like the following patch to be extra defensive,
though.

 git.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/git.c b/git.c
index 8e34903..64c28e4 100644
--- a/git.c
+++ b/git.c
@@ -309,8 +309,8 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv)
 	if (status)
 		return status;
 
-	/* Somebody closed stdout? */
-	if (fstat(fileno(stdout), &st))
+	if (fileno(stdout) < 0 || /* Somebody closed stdout? */
+	    fstat(fileno(stdout), &st))
 		return 0;
 	/* Ignore write errors for pipes and sockets.. */
 	if (S_ISFIFO(st.st_mode) || S_ISSOCK(st.st_mode))

^ permalink raw reply related

* Re: clean bug on ignored subdirectories with no tracked files?
From: Junio C Hamano @ 2011-11-19 19:23 UTC (permalink / raw)
  To: Jay Soffian; +Cc: git
In-Reply-To: <CAG+J_Dxw00e_cr7i3R9DAbTrqZvJHYk2yeUa=xGKh+Zqqmp-SA@mail.gmail.com>

Jay Soffian <jaysoffian@gmail.com> writes:

> git init test_repo &&
> cd test_repo &&
> mkdir -p foo/bar &&
> echo baz > foo/bar/baz &&
> echo /foo/bar > .gitignore &&
> git add .gitignore &&
> git clean -n -d
>
> Initialized empty Git repository in .../test_repo/.git/
> Would remove foo/
>
> Seems surprising.

You said "everythingthing in foo/bar is uninteresting and can be cleaned",
you have one untracked file in "foo/bar" hierarchy, and you have nothing
else in "foo/" hierarchy.

Removing the uninteresting cruft as your .gitignore instructs Git makes
the entire "foo/" hierarchy devoid of any contents. I would *expect* Git
to clean "foo" in this case.

I've seen some "surprising" behaviour in "git clean" (which I do not use
myself, I do not consider part of "my code", and I am not surprised if it
has many bugs), but I fail to see what is surprising in your transcript.

It would be a different issue if you had ">foo/other" before your "clean".
Then "foo/" has "foo/clean" that is not declared to be uninteresting.

^ permalink raw reply

* Re: [PATCH 0/3] avoiding unintended consequences of git_path() usage
From: Ramsay Jones @ 2011-11-19 19:25 UTC (permalink / raw)
  To: Nguyen Thai Ngoc Duy
  Cc: Jonathan Nieder, Ramkumar Ramachandra, Junio C Hamano, Git List
In-Reply-To: <CACsJy8CYj_s92zG-LnBKtHxV2uaG8-rq-VNJiQYwNJXGKbFeDw@mail.gmail.com>

Nguyen Thai Ngoc Duy wrote:
> On Wed, Nov 16, 2011 at 3:59 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
>> Nguyen Thai Ngoc Duy wrote:
>>
>>> Or perhaps
>> [...]
>>>  - git_path(const char *path) maintains a small hash table to keep
>>> track of all returned strings based with "path" as key.
>>>
>>> Out of 142 git_path() calls in my tree, 97 of them are in form
>>> git_path("some static string").
>> The main bit I dislike about patch 3/3 is that constructs like
>> 'unlink(git_path("MERGE_HEAD"));' are not actually unsafe
> 
> Well, we can create wrappers (e.g. repo_unlink(const char *) that
> calls git_path internally). According to grep/sed these functions are
> used in form xxx(git_path(xxx))
> 
>      16 unlink
>       8 file_exists
>       7 stat
>       6 fopen
>       5 rename
>       5 open
>       4 unlink_or_warn
>       3 safe_create_dir
>       3 adjust_shared_perm
>       3 access
>       2 xstrdup
>       2 safe_create_leading_directories

This one at least, maybe others, is unsafe on cygwin. Indeed it causes
a test failure in t3200-branch.sh; patch is on it's way ...

ATB,
Ramsay Jones

^ permalink raw reply

* [PATCH 0/3] test failures on v1.7.8-rc2
From: Ramsay Jones @ 2011-11-19 19:37 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: GIT Mailing-list

Hi Junio,

The test-suite on cygwin has a number of failures. These patches fix
up most of the problems. I know v1.7.8-rc3 is out already, but it takes
2 hours 15 minutes to run the tests on cygwin (and that's without the svn
tests!) so I didn't want to rebase and re-test the patches tonight ... ;-)

    [PATCH 1/3] t5501-*.sh: Fix url passed to clone in setup test
    [PATCH 2/3] config.c: Fix a static buffer overwrite bug by avoiding mkpath()
    [PATCH 3/3] git-submodule.sh: Don't use $path variable in eval_gettext string

Note that patch #3 is an RFC; see patch for more.

[with these patches applied, I have GIT_SKIP_TEST='t0061.3 t0070.3']

ATB,
Ramsay Jones

^ permalink raw reply

* [PATCH 1/3] t5501-*.sh: Fix url passed to clone in setup test
From: Ramsay Jones @ 2011-11-19 19:38 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: GIT Mailing-list


In particular, the url passed to git-clone has an extra '/' given
after the 'file://' schema prefix, thus:

    git clone --reference=original "file:///$(pwd)/original one

Once the prefix is removed, the remainder of the url looks something
like "//home/ramsay/git/t/...", which is then interpreted as an
network path. This then results in a "Permission denied" error, like
so:

    ramsay $ ls //home
    ls: cannot access //home: No such host or network path
    ramsay $ ls //home/ramsay
    ls: cannot access //home/ramsay: Permission denied
    ramsay $

In order to fix the problem, we simply remove the extraneous '/'
character from the url.

Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---
 t/t5501-fetch-push-alternates.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/t/t5501-fetch-push-alternates.sh b/t/t5501-fetch-push-alternates.sh
index b5ced84..1bc57ac 100755
--- a/t/t5501-fetch-push-alternates.sh
+++ b/t/t5501-fetch-push-alternates.sh
@@ -28,7 +28,7 @@ test_expect_success setup '
 		done
 	) &&
 	(
-		git clone --reference=original "file:///$(pwd)/original" one &&
+		git clone --reference=original "file://$(pwd)/original" one &&
 		cd one &&
 		echo Z >count &&
 		git add count &&
-- 
1.7.7

^ permalink raw reply related

* [PATCH 2/3] config.c: Fix a static buffer overwrite bug by avoiding mkpath()
From: Ramsay Jones @ 2011-11-19 19:42 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: GIT Mailing-list


On cygwin, test number 21 of t3200-branch.sh (git branch -m q q2
without config should succeed) fails. The failure involves the
functions from path.c which parcel out internal static buffers
from the git_path() and mkpath() functions.

In particular, the rename_ref() function calls safe_create_leading\
_directories() with a filename returned by git_path("logs/%s", ref).
safe_create_leading_directories(), in turn, calls stat() on each
element of the path it is given. On cygwin, this leads to a call
to git_config() for each component of the path, since this test
explicitly removes the config file. git_config() calls mkpath(), so
on the fourth component of the path, the original buffer passed
into the function is overwritten with the config filename.

Note that this bug is specific to cygwin and it's schizophrenic
stat() functions (see commits adbc0b6, 7faee6b and 7974843). The
lack of a config file and a path with at least four elements is
also important to trigger the bug.

In order to fix the problem, we replace the call to mkpath() with
a call to mksnpath() and provide our own buffer.

Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---
 config.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/config.c b/config.c
index edf9914..5a9ca84 100644
--- a/config.c
+++ b/config.c
@@ -851,6 +851,7 @@ int git_config_system(void)
 
 int git_config_early(config_fn_t fn, void *data, const char *repo_config)
 {
+	char buf[4096];
 	int ret = 0, found = 0;
 	const char *home = NULL;
 
@@ -865,12 +866,11 @@ int git_config_early(config_fn_t fn, void *data, const char *repo_config)
 
 	home = getenv("HOME");
 	if (home) {
-		char *user_config = xstrdup(mkpath("%s/.gitconfig", home));
+		char *user_config = mksnpath(buf, sizeof(buf), "%s/.gitconfig", home);
 		if (!access(user_config, R_OK)) {
 			ret += git_config_from_file(fn, user_config, data);
 			found += 1;
 		}
-		free(user_config);
 	}
 
 	if (repo_config && !access(repo_config, R_OK)) {
-- 
1.7.7

^ permalink raw reply related

* [PATCH/RFC 3/3] git-submodule.sh: Don't use $path variable in eval_gettext string
From: Ramsay Jones @ 2011-11-19 19:59 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: GIT Mailing-list, avarab, Jens.Lehmann, Johannes Sixt


The eval_gettext (and eval_gettextln) i18n shell functions call
git-sh-i18n--envsubst to process the variable references in the
string parameter. Unfortunately, environment variables are case
insensitive on windows, which leads to failure when eval_gettext
exports $path.

Commit df599e9 (Windows: teach getenv to do a case-sensitive search,
06-06-2011) solved this problem on MinGW by overriding the system
getenv() function to allow git-sh-i18n--envsubst to read $path
rather than $PATH from the environment.

As an alternative, we finesse the problem by renaming the $path
variable to $sm_path (submodule path). We note that the foreach
subcommand provides $path to user scripts, so we can't simply
rename it to $sm_path.

Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---

Note that, on cygwin, this fixes t7400-*.sh, t7407-*.sh and t7407-*.sh
(and maybe t7408-*.sh; I didn't get that far ;-).

Note that the first hunk has an independent fix which could be split
out into a separate patch.

This is marked RFC because the approach taken by commit df599e9 may
be the preferred solution; I just wanted to try a different approach
since t7400-submodue-basic.sh fails for me on MinGW (in a way that
seems to imply that $PATH has been corrupted; *but* I haven't spent
any time investigating it).

ATB,
Ramsay Jones

 git-submodule.sh |  161 +++++++++++++++++++++++++++--------------------------
 1 files changed, 82 insertions(+), 79 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index 3adab93..1acd626 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -101,11 +101,12 @@ module_list()
 module_name()
 {
 	# Do we have "submodule.<something>.path = $1" defined in .gitmodules file?
+	sm_path="$1"
 	re=$(printf '%s\n' "$1" | sed -e 's/[].[^$\\*]/\\&/g')
 	name=$( git config -f .gitmodules --get-regexp '^submodule\..*\.path$' |
 		sed -n -e 's|^submodule\.\(.*\)\.path '"$re"'$|\1|p' )
 	test -z "$name" &&
-	die "$(eval_gettext "No submodule mapping found in .gitmodules for path '\$path'")"
+	die "$(eval_gettext "No submodule mapping found in .gitmodules for path '\$sm_path'")"
 	echo "$name"
 }
 
@@ -119,7 +120,7 @@ module_name()
 #
 module_clone()
 {
-	path=$1
+	sm_path=$1
 	url=$2
 	reference="$3"
 	quiet=
@@ -130,12 +131,12 @@ module_clone()
 
 	gitdir=
 	gitdir_base=
-	name=$(module_name "$path" 2>/dev/null)
-	base_path=$(dirname "$path")
+	name=$(module_name "$sm_path" 2>/dev/null)
+	base_path=$(dirname "$sm_path")
 
 	gitdir=$(git rev-parse --git-dir)
 	gitdir_base="$gitdir/modules/$base_path"
-	gitdir="$gitdir/modules/$path"
+	gitdir="$gitdir/modules/$sm_path"
 
 	case $gitdir in
 	/*)
@@ -158,18 +159,18 @@ module_clone()
 
 	if test -d "$gitdir"
 	then
-		mkdir -p "$path"
-		echo "gitdir: $rel_gitdir" >"$path/.git"
+		mkdir -p "$sm_path"
+		echo "gitdir: $rel_gitdir" >"$sm_path/.git"
 		rm -f "$gitdir/index"
 	else
 		mkdir -p "$gitdir_base"
 		if test -n "$reference"
 		then
-			git-clone $quiet "$reference" -n "$url" "$path" --separate-git-dir "$gitdir"
+			git-clone $quiet "$reference" -n "$url" "$sm_path" --separate-git-dir "$gitdir"
 		else
-			git-clone $quiet -n "$url" "$path" --separate-git-dir "$gitdir"
+			git-clone $quiet -n "$url" "$sm_path" --separate-git-dir "$gitdir"
 		fi ||
-		die "$(eval_gettext "Clone of '\$url' into submodule path '\$path' failed")"
+		die "$(eval_gettext "Clone of '\$url' into submodule path '\$sm_path' failed")"
 	fi
 }
 
@@ -221,14 +222,14 @@ cmd_add()
 	done
 
 	repo=$1
-	path=$2
+	sm_path=$2
 
-	if test -z "$path"; then
-		path=$(echo "$repo" |
+	if test -z "$sm_path"; then
+		sm_path=$(echo "$repo" |
 			sed -e 's|/$||' -e 's|:*/*\.git$||' -e 's|.*[/:]||g')
 	fi
 
-	if test -z "$repo" -o -z "$path"; then
+	if test -z "$repo" -o -z "$sm_path"; then
 		usage
 	fi
 
@@ -249,7 +250,7 @@ cmd_add()
 
 	# normalize path:
 	# multiple //; leading ./; /./; /../; trailing /
-	path=$(printf '%s/\n' "$path" |
+	sm_path=$(printf '%s/\n' "$sm_path" |
 		sed -e '
 			s|//*|/|g
 			s|^\(\./\)*||
@@ -259,49 +260,49 @@ cmd_add()
 			tstart
 			s|/*$||
 		')
-	git ls-files --error-unmatch "$path" > /dev/null 2>&1 &&
-	die "$(eval_gettext "'\$path' already exists in the index")"
+	git ls-files --error-unmatch "$sm_path" > /dev/null 2>&1 &&
+	die "$(eval_gettext "'\$sm_path' already exists in the index")"
 
-	if test -z "$force" && ! git add --dry-run --ignore-missing "$path" > /dev/null 2>&1
+	if test -z "$force" && ! git add --dry-run --ignore-missing "$sm_path" > /dev/null 2>&1
 	then
 		eval_gettextln "The following path is ignored by one of your .gitignore files:
-\$path
+\$sm_path
 Use -f if you really want to add it." >&2
 		exit 1
 	fi
 
 	# perhaps the path exists and is already a git repo, else clone it
-	if test -e "$path"
+	if test -e "$sm_path"
 	then
-		if test -d "$path"/.git -o -f "$path"/.git
+		if test -d "$sm_path"/.git -o -f "$sm_path"/.git
 		then
-			eval_gettextln "Adding existing repo at '\$path' to the index"
+			eval_gettextln "Adding existing repo at '\$sm_path' to the index"
 		else
-			die "$(eval_gettext "'\$path' already exists and is not a valid git repo")"
+			die "$(eval_gettext "'\$sm_path' already exists and is not a valid git repo")"
 		fi
 
 	else
 
-		module_clone "$path" "$realrepo" "$reference" || exit
+		module_clone "$sm_path" "$realrepo" "$reference" || exit
 		(
 			clear_local_git_env
-			cd "$path" &&
+			cd "$sm_path" &&
 			# ash fails to wordsplit ${branch:+-b "$branch"...}
 			case "$branch" in
 			'') git checkout -f -q ;;
 			?*) git checkout -f -q -B "$branch" "origin/$branch" ;;
 			esac
-		) || die "$(eval_gettext "Unable to checkout submodule '\$path'")"
+		) || die "$(eval_gettext "Unable to checkout submodule '\$sm_path'")"
 	fi
-	git config submodule."$path".url "$realrepo"
+	git config submodule."$sm_path".url "$realrepo"
 
-	git add $force "$path" ||
-	die "$(eval_gettext "Failed to add submodule '\$path'")"
+	git add $force "$sm_path" ||
+	die "$(eval_gettext "Failed to add submodule '\$sm_path'")"
 
-	git config -f .gitmodules submodule."$path".path "$path" &&
-	git config -f .gitmodules submodule."$path".url "$repo" &&
+	git config -f .gitmodules submodule."$sm_path".path "$sm_path" &&
+	git config -f .gitmodules submodule."$sm_path".url "$repo" &&
 	git add --force .gitmodules ||
-	die "$(eval_gettext "Failed to register submodule '\$path'")"
+	die "$(eval_gettext "Failed to register submodule '\$sm_path'")"
 }
 
 #
@@ -339,23 +340,25 @@ cmd_foreach()
 	exec 3<&0
 
 	module_list |
-	while read mode sha1 stage path
+	while read mode sha1 stage sm_path
 	do
-		if test -e "$path"/.git
+		if test -e "$sm_path"/.git
 		then
-			say "$(eval_gettext "Entering '\$prefix\$path'")"
-			name=$(module_name "$path")
+			say "$(eval_gettext "Entering '\$prefix\$sm_path'")"
+			name=$(module_name "$sm_path")
 			(
-				prefix="$prefix$path/"
+				prefix="$prefix$sm_path/"
 				clear_local_git_env
-				cd "$path" &&
+				# we make $path available to scripts ...
+				path=$sm_path
+				cd "$sm_path" &&
 				eval "$@" &&
 				if test -n "$recursive"
 				then
 					cmd_foreach "--recursive" "$@"
 				fi
 			) <&3 3<&- ||
-			die "$(eval_gettext "Stopping at '\$path'; script returned non-zero status.")"
+			die "$(eval_gettext "Stopping at '\$sm_path'; script returned non-zero status.")"
 		fi
 	done
 }
@@ -389,15 +392,15 @@ cmd_init()
 	done
 
 	module_list "$@" |
-	while read mode sha1 stage path
+	while read mode sha1 stage sm_path
 	do
 		# Skip already registered paths
-		name=$(module_name "$path") || exit
+		name=$(module_name "$sm_path") || exit
 		if test -z "$(git config "submodule.$name.url")"
 		then
 			url=$(git config -f .gitmodules submodule."$name".url)
 			test -z "$url" &&
-			die "$(eval_gettext "No url found for submodule path '\$path' in .gitmodules")"
+			die "$(eval_gettext "No url found for submodule path '\$sm_path' in .gitmodules")"
 
 			# Possibly a url relative to parent
 			case "$url" in
@@ -406,7 +409,7 @@ cmd_init()
 				;;
 			esac
 			git config submodule."$name".url "$url" ||
-			die "$(eval_gettext "Failed to register url for submodule path '\$path'")"
+			die "$(eval_gettext "Failed to register url for submodule path '\$sm_path'")"
 		fi
 
 		# Copy "update" setting when it is not set yet
@@ -414,9 +417,9 @@ cmd_init()
 		test -z "$upd" ||
 		test -n "$(git config submodule."$name".update)" ||
 		git config submodule."$name".update "$upd" ||
-		die "$(eval_gettext "Failed to register update mode for submodule path '\$path'")"
+		die "$(eval_gettext "Failed to register update mode for submodule path '\$sm_path'")"
 
-		say "$(eval_gettext "Submodule '\$name' (\$url) registered for path '\$path'")"
+		say "$(eval_gettext "Submodule '\$name' (\$url) registered for path '\$sm_path'")"
 	done
 }
 
@@ -488,14 +491,14 @@ cmd_update()
 	cloned_modules=
 	module_list "$@" | {
 	err=
-	while read mode sha1 stage path
+	while read mode sha1 stage sm_path
 	do
 		if test "$stage" = U
 		then
-			echo >&2 "Skipping unmerged submodule $path"
+			echo >&2 "Skipping unmerged submodule $sm_path"
 			continue
 		fi
-		name=$(module_name "$path") || exit
+		name=$(module_name "$sm_path") || exit
 		url=$(git config submodule."$name".url)
 		if ! test -z "$update"
 		then
@@ -506,7 +509,7 @@ cmd_update()
 
 		if test "$update_module" = "none"
 		then
-			echo "Skipping submodule '$path'"
+			echo "Skipping submodule '$sm_path'"
 			continue
 		fi
 
@@ -515,20 +518,20 @@ cmd_update()
 			# Only mention uninitialized submodules when its
 			# path have been specified
 			test "$#" != "0" &&
-			say "$(eval_gettext "Submodule path '\$path' not initialized
+			say "$(eval_gettext "Submodule path '\$sm_path' not initialized
 Maybe you want to use 'update --init'?")"
 			continue
 		fi
 
-		if ! test -d "$path"/.git -o -f "$path"/.git
+		if ! test -d "$sm_path"/.git -o -f "$sm_path"/.git
 		then
-			module_clone "$path" "$url" "$reference"|| exit
+			module_clone "$sm_path" "$url" "$reference"|| exit
 			cloned_modules="$cloned_modules;$name"
 			subsha1=
 		else
-			subsha1=$(clear_local_git_env; cd "$path" &&
+			subsha1=$(clear_local_git_env; cd "$sm_path" &&
 				git rev-parse --verify HEAD) ||
-			die "$(eval_gettext "Unable to find current revision in submodule path '\$path'")"
+			die "$(eval_gettext "Unable to find current revision in submodule path '\$sm_path'")"
 		fi
 
 		if test "$subsha1" != "$sha1"
@@ -544,10 +547,10 @@ Maybe you want to use 'update --init'?")"
 			then
 				# Run fetch only if $sha1 isn't present or it
 				# is not reachable from a ref.
-				(clear_local_git_env; cd "$path" &&
+				(clear_local_git_env; cd "$sm_path" &&
 					( (rev=$(git rev-list -n 1 $sha1 --not --all 2>/dev/null) &&
 					 test -z "$rev") || git-fetch)) ||
-				die "$(eval_gettext "Unable to fetch in submodule path '\$path'")"
+				die "$(eval_gettext "Unable to fetch in submodule path '\$sm_path'")"
 			fi
 
 			# Is this something we just cloned?
@@ -561,24 +564,24 @@ Maybe you want to use 'update --init'?")"
 			case "$update_module" in
 			rebase)
 				command="git rebase"
-				die_msg="$(eval_gettext "Unable to rebase '\$sha1' in submodule path '\$path'")"
-				say_msg="$(eval_gettext "Submodule path '\$path': rebased into '\$sha1'")"
+				die_msg="$(eval_gettext "Unable to rebase '\$sha1' in submodule path '\$sm_path'")"
+				say_msg="$(eval_gettext "Submodule path '\$sm_path': rebased into '\$sha1'")"
 				must_die_on_failure=yes
 				;;
 			merge)
 				command="git merge"
-				die_msg="$(eval_gettext "Unable to merge '\$sha1' in submodule path '\$path'")"
-				say_msg="$(eval_gettext "Submodule path '\$path': merged in '\$sha1'")"
+				die_msg="$(eval_gettext "Unable to merge '\$sha1' in submodule path '\$sm_path'")"
+				say_msg="$(eval_gettext "Submodule path '\$sm_path': merged in '\$sha1'")"
 				must_die_on_failure=yes
 				;;
 			*)
 				command="git checkout $subforce -q"
-				die_msg="$(eval_gettext "Unable to checkout '\$sha1' in submodule path '\$path'")"
-				say_msg="$(eval_gettext "Submodule path '\$path': checked out '\$sha1'")"
+				die_msg="$(eval_gettext "Unable to checkout '\$sha1' in submodule path '\$sm_path'")"
+				say_msg="$(eval_gettext "Submodule path '\$sm_path': checked out '\$sha1'")"
 				;;
 			esac
 
-			if (clear_local_git_env; cd "$path" && $command "$sha1")
+			if (clear_local_git_env; cd "$sm_path" && $command "$sha1")
 			then
 				say "$say_msg"
 			elif test -n "$must_die_on_failure"
@@ -592,11 +595,11 @@ Maybe you want to use 'update --init'?")"
 
 		if test -n "$recursive"
 		then
-			(clear_local_git_env; cd "$path" && eval cmd_update "$orig_flags")
+			(clear_local_git_env; cd "$sm_path" && eval cmd_update "$orig_flags")
 			res=$?
 			if test $res -gt 0
 			then
-				die_msg="$(eval_gettext "Failed to recurse into submodule path '\$path'")"
+				die_msg="$(eval_gettext "Failed to recurse into submodule path '\$sm_path'")"
 				if test $res -eq 1
 				then
 					err="${err};$die_msg"
@@ -883,30 +886,30 @@ cmd_status()
 	done
 
 	module_list "$@" |
-	while read mode sha1 stage path
+	while read mode sha1 stage sm_path
 	do
-		name=$(module_name "$path") || exit
+		name=$(module_name "$sm_path") || exit
 		url=$(git config submodule."$name".url)
-		displaypath="$prefix$path"
+		displaypath="$prefix$sm_path"
 		if test "$stage" = U
 		then
 			say "U$sha1 $displaypath"
 			continue
 		fi
-		if test -z "$url" || ! test -d "$path"/.git -o -f "$path"/.git
+		if test -z "$url" || ! test -d "$sm_path"/.git -o -f "$sm_path"/.git
 		then
 			say "-$sha1 $displaypath"
 			continue;
 		fi
-		set_name_rev "$path" "$sha1"
-		if git diff-files --ignore-submodules=dirty --quiet -- "$path"
+		set_name_rev "$sm_path" "$sha1"
+		if git diff-files --ignore-submodules=dirty --quiet -- "$sm_path"
 		then
 			say " $sha1 $displaypath$revname"
 		else
 			if test -z "$cached"
 			then
-				sha1=$(clear_local_git_env; cd "$path" && git rev-parse --verify HEAD)
-				set_name_rev "$path" "$sha1"
+				sha1=$(clear_local_git_env; cd "$sm_path" && git rev-parse --verify HEAD)
+				set_name_rev "$sm_path" "$sha1"
 			fi
 			say "+$sha1 $displaypath$revname"
 		fi
@@ -916,10 +919,10 @@ cmd_status()
 			(
 				prefix="$displaypath/"
 				clear_local_git_env
-				cd "$path" &&
+				cd "$sm_path" &&
 				eval cmd_status "$orig_args"
 			) ||
-			die "$(eval_gettext "Failed to recurse into submodule path '\$path'")"
+			die "$(eval_gettext "Failed to recurse into submodule path '\$sm_path'")"
 		fi
 	done
 }
@@ -951,9 +954,9 @@ cmd_sync()
 	done
 	cd_to_toplevel
 	module_list "$@" |
-	while read mode sha1 stage path
+	while read mode sha1 stage sm_path
 	do
-		name=$(module_name "$path")
+		name=$(module_name "$sm_path")
 		url=$(git config -f .gitmodules --get submodule."$name".url)
 
 		# Possibly a url relative to parent
@@ -968,11 +971,11 @@ cmd_sync()
 			say "$(eval_gettext "Synchronizing submodule url for '\$name'")"
 			git config submodule."$name".url "$url"
 
-			if test -e "$path"/.git
+			if test -e "$sm_path"/.git
 			then
 			(
 				clear_local_git_env
-				cd "$path"
+				cd "$sm_path"
 				remote=$(get_default_remote)
 				git config remote."$remote".url "$url"
 			)
-- 
1.7.7

^ permalink raw reply related

* Re: [msysGit] [PATCH 1/2] MSVC: Do not close stdout to prevent a crash
From: Johannes Sixt @ 2011-11-19 20:11 UTC (permalink / raw)
  To: Vincent van Ravesteijn
  Cc: git, msysgit, gitster, kusmabite, Johannes.Schindelin
In-Reply-To: <1321710345-2299-1-git-send-email-vfr@lyx.org>

Am 19.11.2011 14:45, schrieb Vincent van Ravesteijn:
> When compiled with MSVC, git crashes on Windows when calling
> fstat(stdout) when stdout is closed. fstat is being called at the end of
> run_builtin and this will thus be a problem for builtin command that close
> stdout. This happens for 'format-patch' which closes stdout after a call to
> freopen which directs stdout to the format patch file.

This crash happens because of the some drainbramage in the MS's newer C
runtime: Its functions never return EINVAL (like fstat should in this
case), but it is assumed that cases where EINVAL should be returned are
programming errors, and the application is redirected, via an "invalid
agument handler" to abort() instead. (It is advertized as a security
feature.)

> To prevent the crash and to prevent git from writing cruft into the patch
> file, we do not close stdout, but redirect it to "nul" instead.

A more robust solution is to add invalidcontinue.obj to the linker
command line. This installs an invalid argument handler that does not
abort, and restores a sane behavior.

-- Hannes

^ 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