Git development
 help / color / mirror / Atom feed
* Re: [PATCH][v2] http authentication via prompts (with correct line lengths)
From: Mike Gaffney @ 2009-03-10 15:33 UTC (permalink / raw)
  To: Johannes Schindelin, git
In-Reply-To: <alpine.DEB.1.00.0903101132360.14295@intel-tinevez-2-302>

Johannes,
	Your points make sense, thank you for clarifying, it helps 
me understand what the underlying concerns are. The attitude doesn't really 
help.
	Does Junio's counter solution where git would prompt for the
password if the username contained a url solve your concerns with unsecure
config variables? The patch would be just a bugfix to current functionality.
	Also, libcurl does not warn you that you have an insecure netrc file.
Just tried it with 7.19.4 on cygwin and FC9.

Thanks for the feedback,
	Mike

^ permalink raw reply

* Re: BUG: git can't handle certain kinds of renames in merges
From: Pieter de Bie @ 2009-03-10 16:02 UTC (permalink / raw)
  To: Caleb Cushing; +Cc: git
In-Reply-To: <81bfc67a0903100745m3a425337h3f4f7cdbde6b5cfe@mail.gmail.com>


On Mar 10, 2009, at 2:45 PM, Caleb Cushing wrote:

> git://github.com/xenoterracide/git-test-case.git
>
> clone that. checkout branch 1. then git merge master to see my end  
> failure.
>
> steps to create
>
> add a file in a branch with a line (or more).
> create a new branch based on this branch and check it out.
> in the new branch mv the file into a directory with the same name as
> the file was. add -u and add the file so git sees the rename.
> checkout the original branch add some lines. checkout the new branch
> merge. the merge will go fine.
> remove a line from the new branches file.
> checkout master. add another line to that file.
> checkout new branch and attempt to merge.
>
> you should now see the point that may test case is at.

Yes, this is because automatic renaming detection fails
with this kind of toy examples. Git can't infer the file
was renamed because almost nothing is similar enough. Take
a look at the attached script and run it with 'sh test.sh'
and 'sh test.sh real_test', and look at the difference.

- Pieter

#!/usr/bin/bish
if test x$1 = x
then
	EXTRA_LINES=""
else
	EXTRA_LINES="line2\nline3\nline4\nline5"
fi
FILE1="a\n$EXTRA_LINES"
FILE2="a\n$EXTRA_LINES\nb"
FILE3="$EXTRA_LINES\nb"
FILE4="a\n$EXTRA_LINES\nb\nc"

echo -e $FILE2
rm -rf test_dir
mkdir test_dir
cd test_dir
git init

echo -e $FILE1 > file
git add file
git commit -am "Initial"

git checkout -b branch
git mv file a
mkdir file
git mv a file/file
git commit -m "Move"

git checkout master
echo -e $FILE2 > file
git commit -am "Add a line"

git checkout branch
git merge master

echo -e $FILE3 > file/file
git commit -am "Remove line"

git checkout master
echo -e $FILE4 > file
git commit -am "Add another line"

git checkout branch
git merge master

^ permalink raw reply

* Re: [RFC/PATCH] git push usability improvements and default change
From: Jay Soffian @ 2009-03-10 16:20 UTC (permalink / raw)
  To: Finn Arne Gangstad; +Cc: Junio C Hamano, git
In-Reply-To: <20090310100400.GC11448@pvv.org>

On Tue, Mar 10, 2009 at 6:04 AM, Finn Arne Gangstad <finnag@pvv.org> wrote:
> You are thinking of something like this in .gitconfig?
> [remote "*"]
>        push = __something__
>
> Previously you indicated that there is no way to specify the current
> matching rule in a remote push line I think?

No, you can. "push = :" works fine.

However, there's no way to specify "push = nothing" currently.

FWIW, I don't care for using remote.*.push for this use case. I think
push.default is clearer here.

j.

^ permalink raw reply

* Re: [PATCH/RFC] recv_sideband: Band #2 always goes to stderr
From: Jay Soffian @ 2009-03-10 16:35 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Johannes Sixt, Shawn O. Pearce, git, gitster, Peter Harris,
	Sebastian Schuberth, Nicolas Pitre
In-Reply-To: <alpine.DEB.1.00.0903101605460.14295@intel-tinevez-2-302>

On Tue, Mar 10, 2009 at 11:07 AM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
>> I deliberatly avoided "%.*s" in the former hunk (1) because of the posts
>> that we had yesterday about potentially misbehaved fprintf in the case
>> where the precision is 0;
>
> Didn't that turn out to be a false alarm?

Yes. A party who will remain nameless was reading the wrong man page
section (printf(1) vs printf(3)). :-)

j.

^ permalink raw reply

* Re: [PATCH] builtin-tag.c: remove global variable to use the callback data of git-config.
From: Junio C Hamano @ 2009-03-10 16:36 UTC (permalink / raw)
  To: Carlos Rica; +Cc: git, johannes.schindelin
In-Reply-To: <1236690219.20402.28.camel@luis-desktop>

Carlos Rica <jasampler@gmail.com> writes:

> Signed-off-by: Carlos Rica <jasampler@yahoo.es>
> ---
>
> This way the data flow is much clearer.

Good.  I think the Subject is backwards, though.

^ permalink raw reply

* Re: [PATCH 0/5] grep: color search patterns
From: René Scharfe @ 2009-03-10 16:38 UTC (permalink / raw)
  To: Nguyen Thai Ngoc Duy; +Cc: Git Mailing List, Junio C Hamano, Thiago Alves
In-Reply-To: <fcaeb9bf0903092301i7bc6322dtbd37f662fe4b224b@mail.gmail.com>

Nguyen Thai Ngoc Duy schrieb:
> On 3/7/09, René Scharfe <rene.scharfe@lsrfire.ath.cx> wrote:
>> Match coloring is a major missing feature of git grep.  It makes reading
>>  the output much easier and brings grep onto the same visual level as the
>>  other colorized git commands.
> 
> "git --color test" did not colorize the result for me. "git --color
> --no-ext-grep test" did. Maybe you should ignore external grep unless
> it is explicitly requested, like in the last patch. I have very
> limited net access these days. Let's see if I can work out something
> for tomorrow, unless you beat me to it.

I assume with "last patch" you mean your last patch, right?  It
automatically switched to internal grep if coloring was turned on.

Patch 5 of my series adds support for coloring external greps, but you
have to explicitly specify the config option grep.color.external.  It
can be set by distributors in the system config file -- they should know
which grep option is needed.

But I can see that a half-colored git grep can be confusing.

René

^ permalink raw reply

* Re: [PATCH] git-instaweb: fix lighttpd configuration on cygwin
From: Pascal Obry @ 2009-03-10 17:34 UTC (permalink / raw)
  To: Ramsay Jones; +Cc: Junio C Hamano, GIT Mailing-list
In-Reply-To: <49B5609B.5070705@ramsay1.demon.co.uk>

Ramsay Jones a écrit :
> Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
> ---
> 
> A recent email from Pascal reminded me that I had this patch
> kicking around. I haven't tested this on Linux, since I don't
> have lighttpd installed there (I already have Apache), but I
> don't expect any problems; famous last words!
> 
> The essence of the change is the addition of the "mod_setenv"
> module, along with the PATH environment variable assignment.
> (otherwise gitweb can't find a shell and, therefore, can't
> execute any git programs)
> 
> Also, I had to add a default mime-type assignment of text/plain
> ("" => "text/plain").  If my memory serves me, the other additions
> to the mime-type mapping was just cosmetic; and why not?
> 
> Hmmm, I just noticed that the first hunk, which is needed to
> suppress a "lighttpd: not found" error message, could perhaps be
> seen as an un-related fix. Dunno.

Tested on my side using Cygwin 1.5.25 and httpd. Working fine. Thanks.

  Tested-by: Pascal Obry <pascal@obry.net>

or

  Signed-off-by: Pascal Obry <pascal@obry.net>

Not sure as I have never seen Tested-by!

Pascal.

-- 

--|------------------------------------------------------
--| Pascal Obry                           Team-Ada Member
--| 45, rue Gabriel Peri - 78114 Magny Les Hameaux FRANCE
--|------------------------------------------------------
--|    http://www.obry.net  -  http://v2p.fr.eu.org
--| "The best way to travel is by means of imagination"
--|
--| gpg --keyserver keys.gnupg.net --recv-key F949BD3B

^ permalink raw reply

* Re: [PATCH/RFC] recv_sideband: Band #2 always goes to stderr
From: Nicolas Pitre @ 2009-03-10 17:35 UTC (permalink / raw)
  To: Johannes Sixt
  Cc: Johannes Schindelin, Shawn O. Pearce, git, gitster, Peter Harris,
	Sebastian Schuberth
In-Reply-To: <49B683BC.3030104@viscovery.net>

On Tue, 10 Mar 2009, Johannes Sixt wrote:

> Johannes Schindelin schrieb:
> > I think, indeed, that you can avoid the memcpy() by using %.*s.  The 
> > private buffer is only used to make sure that the text is written in one 
> > go anyway (i.e. that two sidebands messages are not written to the same 
> > line because they use multiple calls to fprintf()/fwrite() per line), 
> > right?
> 
> It must be something like that, yes. ;)

It is a bit more complex than that.

Messages received over sideband #2 may have two kinds of line break 
characters: \r or \n.  We also have a string suffix which is used to 
clear the remaining of the screen line in those cases where a previous 
line is overwritten and the previous line was longer.  That may happen 
if the previous line was terminated with \r.  In that case, the string 
suffix _must_ be inserted before the line break character for obvious 
reasons.


Nicolas

^ permalink raw reply

* Re: [PATCH/RFC] recv_sideband: Band #2 always goes to stderr
From: Nicolas Pitre @ 2009-03-10 17:37 UTC (permalink / raw)
  To: Johannes Sixt
  Cc: Johannes Schindelin, git, gitster, Peter Harris,
	Sebastian Schuberth
In-Reply-To: <49B61703.8030602@viscovery.net>

On Tue, 10 Mar 2009, Johannes Sixt wrote:

> From: Johannes Sixt <j6t@kdbg.org>
> 
> This removes the last parameter of recv_sideband, by which the callers
> told which channel band #2 data should be written to. Since both callers
> of the function passed 2 for the parameter, we hereby remove the
> parameter and send band #2 to stderr explicitly using fprintf.
> 
> This has the nice side-effect that the band #2 data (most importantly
> progress reports during a fetch operation) passes through our ANSI
> emulation layer on Windows.

You appear to modify band #3 as well.  Better mention it in the commit 
log.

> Signed-off-by: Johannes Sixt <j6t@kdbg.org>

Acked-by: Nicolas Pitre <nico@cam.org>


> ---
> Johannes Sixt schrieb:
> > Johannes Schindelin schrieb:
> >> To make use of it during a fetch, write() needs to be overridden, too.
> > 
> > No, that's not necessary with the patch that I'm about to send in a
> > moment. To replace write() for ANSI emulation really goes too far.
> 
> Here it is. The patch is still RFC because I didn't have a chance, yet,
> to test it in practice. It passes the test suite.
> 
> -- Hannes
> 
>  builtin-archive.c    |    2 +-
>  builtin-fetch-pack.c |    2 +-
>  sideband.c           |   20 +++++++++-----------
>  sideband.h           |    2 +-
>  4 files changed, 12 insertions(+), 14 deletions(-)
> 
> diff --git a/builtin-archive.c b/builtin-archive.c
> index 60adef9..ab50ceb 100644
> --- a/builtin-archive.c
> +++ b/builtin-archive.c
> @@ -52,7 +52,7 @@ static int run_remote_archiver(int argc, const char **argv,
>  		die("git archive: expected a flush");
> 
>  	/* Now, start reading from fd[0] and spit it out to stdout */
> -	rv = recv_sideband("archive", fd[0], 1, 2);
> +	rv = recv_sideband("archive", fd[0], 1);
>  	close(fd[0]);
>  	close(fd[1]);
>  	rv |= finish_connect(conn);
> diff --git a/builtin-fetch-pack.c b/builtin-fetch-pack.c
> index c2e5adc..2b36099 100644
> --- a/builtin-fetch-pack.c
> +++ b/builtin-fetch-pack.c
> @@ -482,7 +482,7 @@ static int sideband_demux(int fd, void *data)
>  {
>  	int *xd = data;
> 
> -	return recv_sideband("fetch-pack", xd[0], fd, 2);
> +	return recv_sideband("fetch-pack", xd[0], fd);
>  }
> 
>  static int get_pack(int xd[2], char **pack_lockfile)
> diff --git a/sideband.c b/sideband.c
> index cca3360..a706ac8 100644
> --- a/sideband.c
> +++ b/sideband.c
> @@ -19,7 +19,7 @@
> 
>  #define FIX_SIZE 10  /* large enough for any of the above */
> 
> -int recv_sideband(const char *me, int in_stream, int out, int err)
> +int recv_sideband(const char *me, int in_stream, int out)
>  {
>  	unsigned pf = strlen(PREFIX);
>  	unsigned sf;
> @@ -41,8 +41,7 @@ int recv_sideband(const char *me, int in_stream, int out, int err)
>  		if (len == 0)
>  			break;
>  		if (len < 1) {
> -			len = sprintf(buf, "%s: protocol error: no band designator\n", me);
> -			safe_write(err, buf, len);
> +			fprintf(stderr, "%s: protocol error: no band designator\n", me);
>  			return SIDEBAND_PROTOCOL_ERROR;
>  		}
>  		band = buf[pf] & 0xff;
> @@ -50,8 +49,8 @@ int recv_sideband(const char *me, int in_stream, int out, int err)
>  		switch (band) {
>  		case 3:
>  			buf[pf] = ' ';
> -			buf[pf+1+len] = '\n';
> -			safe_write(err, buf, pf+1+len+1);
> +			buf[pf+1+len] = '\0';
> +			fprintf(stderr, "%s\n", buf);
>  			return SIDEBAND_REMOTE_ERROR;
>  		case 2:
>  			buf[pf] = ' ';
> @@ -95,12 +94,13 @@ int recv_sideband(const char *me, int in_stream, int out, int err)
>  					memcpy(save, b + brk, sf);
>  					b[brk + sf - 1] = b[brk - 1];
>  					memcpy(b + brk - 1, suffix, sf);
> -					safe_write(err, b, brk + sf);
> +					fprintf(stderr, "%.*s", brk + sf, b);
>  					memcpy(b + brk, save, sf);
>  					len -= brk;
>  				} else {
>  					int l = brk ? brk : len;
> -					safe_write(err, b, l);
> +					if (l > 0)
> +						fprintf(stderr, "%.*s", l, b);
>  					len -= l;
>  				}
> 
> @@ -112,10 +112,8 @@ int recv_sideband(const char *me, int in_stream, int out, int err)
>  			safe_write(out, buf + pf+1, len);
>  			continue;
>  		default:
> -			len = sprintf(buf,
> -				      "%s: protocol error: bad band #%d\n",
> -				      me, band);
> -			safe_write(err, buf, len);
> +			fprintf(stderr, "%s: protocol error: bad band #%d\n",
> +				me, band);
>  			return SIDEBAND_PROTOCOL_ERROR;
>  		}
>  	}
> diff --git a/sideband.h b/sideband.h
> index a84b691..d72db35 100644
> --- a/sideband.h
> +++ b/sideband.h
> @@ -7,7 +7,7 @@
>  #define DEFAULT_PACKET_MAX 1000
>  #define LARGE_PACKET_MAX 65520
> 
> -int recv_sideband(const char *me, int in_stream, int out, int err);
> +int recv_sideband(const char *me, int in_stream, int out);
>  ssize_t send_sideband(int fd, int band, const char *data, ssize_t sz, int packet_max);
> 
>  #endif
> -- 
> 1.6.2.987.g90c1d
> 
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply

* Re: [RFC/PATCH] git push usability improvements and default change
From: Jeff King @ 2009-03-10 17:52 UTC (permalink / raw)
  To: Finn Arne Gangstad; +Cc: git
In-Reply-To: <1236638151-6465-1-git-send-email-finnag@pvv.org>

On Mon, Mar 09, 2009 at 11:35:44PM +0100, Finn Arne Gangstad wrote:

> git push has learned two new command line options --matching and
> --current, which override any configuration.  'matching' pushes all
> branches that already exist at the remote, while 'current' pushes the
> current branch to whatever it is tracking

I have not been following this topic too closely, so can you please
explain (or point me to an explanation about) something? How do these
options interact with refspecs given on the command line? That is, why
would I choose to use:

  git push --current

over

  git push - HEAD

(assuming your earlier patch is applied, or "git push HEAD" if Junio's
suggested DWIMmery is implemented). And what does it mean to say

  git push --matching - HEAD

? Those are conflicting instructions. Is one followed and one discarded?
Are they merged?

-Peff

^ permalink raw reply

* Re: [PATCH 2/3] config: set help text for --bool-or-int
From: Jeff King @ 2009-03-10 17:59 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: git, Junio C Hamano
In-Reply-To: <94a0d4530903091450gdcad625g95cd9550cfb724fa@mail.gmail.com>

On Mon, Mar 09, 2009 at 11:50:09PM +0200, Felipe Contreras wrote:

> Yeah, I meant usage_with_help. I don't know what should be done, but I
> think two things should be achieved:
> 
> a) don't crash
> b) encourage the options to always have a description

I don't know of a good way to check at compile time that the struct
member is non-NULL. So I think we are stuck with parse_options dying, or
doing something to cover it up, like:

> Perhaps not showing the option at all, or perhaps showing "**EMPTY**".

But the problem there is that if you _did_ mean for it to be shown, such
a bug can easily escape notice. Tests will generally still pass, so
unless you are scanning the output manually, nothing will tell you what
happened.

I think our best bet is to simply try to catch such things in code
review. This one slipped through, but it was still caught in 'next',
most because it _did_ crash on a non-glibc platform.

> Minor nitpick: "value is interpreted either as bool or int"
> 
> The value is what it is, the --boo-or-int option doesn't change the
> value, just how it is interpreted.

Fair enough. My patch is is in 'next' now, so please go ahead and submit
a new patch with your suggested change. You might want to change --bool
and --int at the same time, though, as they use the same wording.

-Peff

^ permalink raw reply

* Re: import files w/ history
From: Jeff King @ 2009-03-10 18:03 UTC (permalink / raw)
  To: Csaba Henk; +Cc: git
In-Reply-To: <slrngr99ei.1t4t.csaba-ml@beastie.creo.hu>

On Mon, Mar 09, 2009 at 05:15:16AM +0000, Csaba Henk wrote:

> But I still had a hard time with it... Finally I realized that if I do
> filtering this way, I have to start filtering from the topmost commit
> which affects the given file.
> 
> If I just start from origin/HEAD (assuming that it's on a commit which
> does not affect the file), then it won't be found as a key of the mapping
> created by git-filter-branch (as it's ignored because rev-listing was
> narrowed down to the file), and therefore filter-branch finally punts
> with "WARNING: Ref '<sha1>' is unchanged". I don't know if it's an
> intended behaviour, or something which could/should be improved, or at
> least documented... seems to be some sort of POLS violation to me (at
> least I was surprised :) ).

I think passing path limiters to filter-branch is just something that
nobody ever really tried before. I think the solutions are, in order of
decreasing easiness and increasing difficulty:

  1. document the problem in Documentation/git-filter-branch.txt

  2. create a failing test for it in the test suite

  3. fix the failing test. ;)

Do you want to try a patch for one (or more!) of those?

-Peff

^ permalink raw reply

* Re: [PATCH/RFD] builtin-revert.c: release index lock when cherry-picking an empty commit
From: Jeff King @ 2009-03-10 18:17 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Chris Johnsen, Miklos Vajna
In-Reply-To: <7v8wnflrws.fsf@gitster.siamese.dyndns.org>

On Sun, Mar 08, 2009 at 12:45:55PM -0700, Junio C Hamano wrote:

> If this part from your analysis is true for a shell:
> 
> > eval 'false
> >
> > '
> > echo status is $?
> >
> > generates:
> > ...
> >   status is 0
> 
> I would be very tempted to declare that shell is unfit for any serious
> use, not just for test suite.  Removing the empty line at the end of a
> scriptlet that such a broken shell misinterprets as an empty command
> that is equivalent to ":" (or "true") might hide breakages in the test
> suite, but
> 
>  (1) eval "$string" is used outside of test suite, most notably "am" and
>      "bisect".  I think "am"'s use is safe, but I wouldn't be surprised if
>      the scriptlet "bisect" internally creates has empty lines if only for
>      debuggability; and more importantly
> 
>  (2) who knows what _other_ things may be broken in such a shell?

OK, good points. I was just hoping not to cause people on FreeBSD undue
pain. What is the best way to make such a declaration? I can think of:

  1. A mention in the release notes.

  2. A test in the Makefile similar to the $(:) test.

  3. Getting in touch with the freebsd ports maintainer for git and
     suggesting a dependency on bash (and/or seeing if he wants to push
     through a fix for /bin/sh).

     I don't know if the same problem exists on other BSD-influenced systems,
     or how closely they share the ports collection (it's been quite a
     while since I've really admin'd a freebsd box). For that matter, I
     wonder if this is also a problem on OS X. Can somebody with an OS X
     box try:

       $ /bin/sh
       $ eval 'false

         '
       $ echo $?

     It should print '1'; if it prints '0', the shell is broken.

-Peff

^ permalink raw reply

* Re: [PATCH 2/2] ls-files: fix broken --no-empty-directory
From: Jeff King @ 2009-03-10 19:11 UTC (permalink / raw)
  To: Miklos Vajna; +Cc: Junio C Hamano, git
In-Reply-To: <20090308211312.GE4371@genesis.frugalware.org>

On Sun, Mar 08, 2009 at 10:13:12PM +0100, Miklos Vajna wrote:

> > +		OPT_BIT(0, "no-empty-directory", &dir.flags,
> > +			"don't show empty directories",
> >  			DIR_HIDE_EMPTY_DIRECTORIES),
> >  		OPT_BOOLEAN('u', "unmerged", &show_unmerged,
> >  			"show unmerged files in the output"),
> 
> Thanks for catching this. But then why not using PARSE_OPT_NONEG?
> 
> That would avoid --no-no-empty-directory.

I think either we don't care about negation, in which case it is not
hurting anybody to support --no-no-empty-directory, or we do, in which
case you actually want to do the negation properly. Which would be
something like:

diff --git a/builtin-ls-files.c b/builtin-ls-files.c
index 437c366..2c5f7a5 100644
--- a/builtin-ls-files.c
+++ b/builtin-ls-files.c
@@ -427,6 +427,7 @@ static int option_parse_exclude_standard(const struct option *opt,
 int cmd_ls_files(int argc, const char **argv, const char *prefix)
 {
 	int require_work_tree = 0, show_tag = 0;
+	int show_empty_directories = 1;
 	struct dir_struct dir;
 	struct option builtin_ls_files_options[] = {
 		{ OPTION_CALLBACK, 'z', NULL, NULL, NULL,
@@ -454,9 +455,8 @@ int cmd_ls_files(int argc, const char **argv, const char *prefix)
 		OPT_BIT(0, "directory", &dir.flags,
 			"show 'other' directories' name only",
 			DIR_SHOW_OTHER_DIRECTORIES),
-		OPT_BIT(0, "no-empty-directory", &dir.flags,
-			"don't show empty directories",
-			DIR_HIDE_EMPTY_DIRECTORIES),
+		OPT_BOOLEAN(0, "empty-directory", &show_empty_directories,
+			"show empty directories (on by default)"),
 		OPT_BOOLEAN('u', "unmerged", &show_unmerged,
 			"show unmerged files in the output"),
 		{ OPTION_CALLBACK, 'x', "exclude", &dir.exclude_list[EXC_CMDL], "pattern",
@@ -506,6 +506,8 @@ int cmd_ls_files(int argc, const char **argv, const char *prefix)
 		show_stage = 1;
 	if (dir.exclude_per_dir)
 		exc_given = 1;
+	if (!show_empty_directories)
+		dir.flags |= DIR_HIDE_EMPTY_DIRECTORIES;
 
 	if (require_work_tree && !is_inside_work_tree())
 		setup_work_tree();


which is even still a little confusing, as you get "--empty-directory"
in the usage message. But you would almost never want to use that, as it
is already the default.

-Peff

^ permalink raw reply related

* [BUG] - git-read-tree segfaults
From: Jiri Olsa @ 2009-03-10 19:34 UTC (permalink / raw)
  To: git

Hi,

I was following the git core tutorial and git-read-tree got me segfault.
I accidentaly executed the git-read-tree without "-m -u" options and
got segfault.

I can reproduce with the latest git using following script.

-----------------------------------------------
#!/bin/sh

GIT=$1

rm -rf crash; mkdir -p crash; cd crash
$GIT init
echo "xxx" > xxx
$GIT add xxx
$GIT commit -m "xxx"
$GIT checkout -b yyy
echo "yyy" > yyy
$GIT add yyy
$GIT commit -m "yyy"
echo "yyy1" >> yyy
$GIT commit -a -m "yyy1"
$GIT checkout master
echo "xxx1" >> xxx
$GIT commit -a -m "xxx1"
mb=$($GIT merge-base HEAD yyy)
$GIT read-tree $mb HEAD yyy
-----------------------------------------------

regards,
jirka

^ permalink raw reply

* Confusing `stash apply` behavior
From: Tim Visher @ 2009-03-10 19:46 UTC (permalink / raw)
  To: git

Hello Everyone,

I was just trying to do some experimentation with `stash` and I've run
into a problem.

I was working on my `master` branch and decided that I wanted to
create a `dev` branch.  I did `git stash` and then `git checkout -b
dev`.  Then I did `git stash apply` and everything worked as expected.
 I continued working but then realized that I wanted a `refactoring`
branch.  In the process of continuing to work I had also cleared the
stash stack with `git stash clear`, although this had no visible
effect other than to remove any entries from `git stash list`.  As
before, I did `git stash` and then `git checkout -b refactoring` and
here lies my problem.

When I do `git stash apply`, it deletes the file I'm working with.

    $ ls
    featureList.txt*  keycontrol.mdb*

    $ git show stash@{0}
    commit b3c0f4b9b3c3ef7741a03fb27174f5838abc939d
    Merge: 9fb9886 112bba9
    Author: Tim Visher <timothy.visher@fms.treas.gov>
    Date:   Tue Mar 10 15:25:04 2009 -0400

    WIP on dev: 9fb9886 Added DB Lock file to .gitignore. EOM

    diff --cc keycontrol.mdb
    index 68a9bac,68a9bac..0000000
    --- a/keycontrol.mdb
    +++ b/keycontrol.mdb

    $ git stash apply
    Removing keycontrol.mdb
    # On branch refactoring
    # Changed but not updated:
    #   (use "git add/rm <file>..." to update what will be committed)
    #   (use "git checkout -- <file>..." to discard changes in working
directory)
    #
    #       deleted:    keycontrol.mdb
    #
    no changes added to commit (use "git add" and/or "git commit -a")

Considering the output of `git show` I would expect that the contents
of the stash are, well, what I expect them to be: a new version of
keycontrol.mdb.

I'm sure I'm missing something completely juvenile but I could really
use some help because that stash represents about an hours worth of
work.  Not something to totally loose sleep over but something that
would be nicer to not have to do over.

Thanks in advance for your help!

-- 

In Christ,

Timmy V.

http://burningones.com/
http://five.sentenc.es/ - Spend less time on e-mail

^ permalink raw reply

* Re: BUG: git can't handle certain kinds of renames in merges
From: Caleb Cushing @ 2009-03-10 19:51 UTC (permalink / raw)
  To: Pieter de Bie; +Cc: git
In-Reply-To: <252A6411-2658-4DC6-A7F4-29CA3981F8FB@frim.nl>

On Tue, Mar 10, 2009 at 12:02 PM, Pieter de Bie <pieter@frim.nl> wrote:
> ind of toy examples. Git can't infer the file
> was renamed because almost nothing is similar enough. Take
> a look at the attached script and run it with 'sh test.sh'
> and 'sh test.sh real_test', and look at the difference.

I've got a much bigger example(hundreds of lines, and dozens change at
a time)  which resolves in the same problem or worse with
git-tree-write errors. I'm trying to figure out now how to work around
this issue.

-- 
Caleb Cushing

http://xenoterracide.blogspot.com

^ permalink raw reply

* Re: [PATCH/RFD] builtin-revert.c: release index lock when cherry-picking an empty commit
From: Tomas Carnecky @ 2009-03-10 18:25 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, git, Chris Johnsen, Miklos Vajna
In-Reply-To: <20090310181730.GD26351@sigill.intra.peff.net>


On Mar 10, 2009, at 7:17 PM, Jeff King wrote:

> On Sun, Mar 08, 2009 at 12:45:55PM -0700, Junio C Hamano wrote:
>
>> If this part from your analysis is true for a shell:
>>
>>> eval 'false
>>>
>>> '
>>> echo status is $?
>>>
>>> generates:
>>> ...
>>>  status is 0
>>
>> I would be very tempted to declare that shell is unfit for any  
>> serious
>> use, not just for test suite.  Removing the empty line at the end  
>> of a
>> scriptlet that such a broken shell misinterprets as an empty command
>> that is equivalent to ":" (or "true") might hide breakages in the  
>> test
>> suite, but
>>
>> (1) eval "$string" is used outside of test suite, most notably "am"  
>> and
>>     "bisect".  I think "am"'s use is safe, but I wouldn't be  
>> surprised if
>>     the scriptlet "bisect" internally creates has empty lines if  
>> only for
>>     debuggability; and more importantly
>>
>> (2) who knows what _other_ things may be broken in such a shell?
>
> OK, good points. I was just hoping not to cause people on FreeBSD  
> undue
> pain. What is the best way to make such a declaration? I can think of:
>
>  1. A mention in the release notes.
>
>  2. A test in the Makefile similar to the $(:) test.
>
>  3. Getting in touch with the freebsd ports maintainer for git and
>     suggesting a dependency on bash (and/or seeing if he wants to push
>     through a fix for /bin/sh).
>
>     I don't know if the same problem exists on other BSD-influenced  
> systems,
>     or how closely they share the ports collection (it's been quite a
>     while since I've really admin'd a freebsd box). For that matter, I
>     wonder if this is also a problem on OS X. Can somebody with an  
> OS X
>     box try:
>
>       $ /bin/sh
>       $ eval 'false
>
>         '
>       $ echo $?
>
>     It should print '1'; if it prints '0', the shell is broken.

prints '1' here (10.5.6)

tom

^ permalink raw reply

* Re: [PATCH/RFD] builtin-revert.c: release index lock when cherry-picking an empty commit
From: Tomas Carnecky @ 2009-03-10 19:33 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, git, Chris Johnsen, Miklos Vajna
In-Reply-To: <20090310181730.GD26351@sigill.intra.peff.net>


On Mar 10, 2009, at 7:17 PM, Jeff King wrote:

> On Sun, Mar 08, 2009 at 12:45:55PM -0700, Junio C Hamano wrote:
>
>> If this part from your analysis is true for a shell:
>>
>>> eval 'false
>>>
>>> '
>>> echo status is $?
>>>
>>> generates:
>>> ...
>>> status is 0
>>
>> I would be very tempted to declare that shell is unfit for any  
>> serious
>> use, not just for test suite.  Removing the empty line at the end  
>> of a
>> scriptlet that such a broken shell misinterprets as an empty command
>> that is equivalent to ":" (or "true") might hide breakages in the  
>> test
>> suite, but
>>
>> (1) eval "$string" is used outside of test suite, most notably "am"  
>> and
>>    "bisect".  I think "am"'s use is safe, but I wouldn't be  
>> surprised if
>>    the scriptlet "bisect" internally creates has empty lines if  
>> only for
>>    debuggability; and more importantly
>>
>> (2) who knows what _other_ things may be broken in such a shell?
>
> OK, good points. I was just hoping not to cause people on FreeBSD  
> undue
> pain. What is the best way to make such a declaration? I can think of:
>
> 1. A mention in the release notes.
>
> 2. A test in the Makefile similar to the $(:) test.
>
> 3. Getting in touch with the freebsd ports maintainer for git and
>    suggesting a dependency on bash (and/or seeing if he wants to push
>    through a fix for /bin/sh).
>
>    I don't know if the same problem exists on other BSD-influenced  
> systems,
>    or how closely they share the ports collection (it's been quite a
>    while since I've really admin'd a freebsd box). For that matter, I
>    wonder if this is also a problem on OS X. Can somebody with an OS X
>    box try:
>
>      $ /bin/sh
>      $ eval 'false
>
>        '
>      $ echo $?
>
>    It should print '1'; if it prints '0', the shell is broken.

prints '1' here (10.5.6)

tom

^ permalink raw reply

* Re: [BUG] - git-read-tree segfaults
From: Johannes Schindelin @ 2009-03-10 19:50 UTC (permalink / raw)
  To: Jiri Olsa; +Cc: git
In-Reply-To: <35476bd20903101234q71bc520fh18828d7170a4a2f5@mail.gmail.com>

Hi,

On Tue, 10 Mar 2009, Jiri Olsa wrote:

> Hi,
> 
> I was following the git core tutorial and git-read-tree got me segfault.
> I accidentaly executed the git-read-tree without "-m -u" options and
> got segfault.
> 
> I can reproduce with the latest git using following script.

I can reproduce with the script, too...

Working on it,
Dscho

^ permalink raw reply

* Re: [BUG] - git-read-tree segfaults
From: Johannes Schindelin @ 2009-03-10 20:21 UTC (permalink / raw)
  To: Jiri Olsa; +Cc: git
In-Reply-To: <35476bd20903101234q71bc520fh18828d7170a4a2f5@mail.gmail.com>

Hi,

On Tue, 10 Mar 2009, Jiri Olsa wrote:

> mb=$($GIT merge-base HEAD yyy)
> $GIT read-tree $mb HEAD yyy

While I agree that it is a bad thing for Git to segfault, I think this 
here is a pilot error.  You try to read 3 trees at the same time, but not 
perform a merge.  IMHO you want to add -m at least.

Ciao,
Dscho

^ permalink raw reply

* Re: setting up tracking on push
From: Marc Branchaud @ 2009-03-10 20:26 UTC (permalink / raw)
  To: Miles Bader; +Cc: git
In-Reply-To: <87d4cuobrc.fsf@catnip.gol.com>

Miles Bader wrote:
> Jay Soffian <jaysoffian@gmail.com> writes:
>> I don't think we need a new command, but it would probably fit into
>> git remote
>>
>> $ git remote track <remote> [<branch>]
> 
> That seems a bit backwards -- it's more of a branch operation than a
> remote operation...

Agreed.

We just ran into this situation today with pull: Someone wanted one of their local branches to start tracking a remote.

It would be good if the branch command allowed modification of a branch's properties.  At the very least, branch-creation commands like "git branch --track foobranch origin/master" could offer to modify if the branch already exists, instead of just quitting.

BTW, I think the following (untested) incantation adds tracking to a branch:

	git config branch.foobranch.remote origin
	git config branch.foobranch.merge refs/heads/master

(Substitute "origin" and "master" to suit your needs.)

		M.

^ permalink raw reply

* Help understanding "rebase"
From: John M. Dlugosz @ 2009-03-10 21:07 UTC (permalink / raw)
  To: git

Here is the situation:  An old topic branch containing 3 commits.  A dev branch that has 
recently been merged.  To catch up the topic's work before adding it to dev, I expected 
that rebase would do what I ended up doing manually, detailed below.

Instead, it crunched away for a long time and gave errors applying patches.

So I did it manually by checking out dev, then cherry-picking each of the three commits. 
Actually, this left it on top of dev, but suppose I had created a new branch at dev, 
cherry-picked the stuff from the old topic branch, and then deleted the old topic branch. 
  Now I have a new topic branch with the rebased changes, albeit with a different branch 
name.  Point is, there were no conflicts and the changes were simple, so cherry-picking 
each node was clean.

So, what did the rebase command try to do?  I think it may have something to do with 
finding a common root between the topic and dev, which, due to the merge, was a long way 
back.  Something like this:

	  o--o--   ...  --o
	 /                 \
	A--...--B--   ... --C--D <== dev
	         \
                   q--r--s  <== topic


I was able to cherry-pick q,r,s on top of D without any issues.  So why did rebase get in 
such a tizzy?

--John

^ permalink raw reply

* Re: Help understanding "rebase"
From: Brandon Casey @ 2009-03-10 21:30 UTC (permalink / raw)
  To: John M. Dlugosz; +Cc: git
In-Reply-To: <gp6kqj$tkb$1@ger.gmane.org>


John M. Dlugosz wrote:
> Here is the situation:  An old topic branch containing 3 commits.  A dev
> branch that has recently been merged.  To catch up the topic's work
> before adding it to dev, I expected that rebase would do what I ended up
> doing manually, detailed below.
> 
> Instead, it crunched away for a long time and gave errors applying patches.
> 
> So I did it manually by checking out dev, then cherry-picking each of
> the three commits. Actually, this left it on top of dev, but suppose I
> had created a new branch at dev, cherry-picked the stuff from the old
> topic branch, and then deleted the old topic branch.  Now I have a new
> topic branch with the rebased changes, albeit with a different branch
> name.  Point is, there were no conflicts and the changes were simple, so
> cherry-picking each node was clean.
> 
> So, what did the rebase command try to do?  I think it may have
> something to do with finding a common root between the topic and dev,
> which, due to the merge, was a long way back.  Something like this:
> 
>       o--o--   ...  --o
>      /                 \
>     A--...--B--   ... --C--D <== dev
>              \
>                   q--r--s  <== topic
> 
> 
> I was able to cherry-pick q,r,s on top of D without any issues.  So why
> did rebase get in such a tizzy?

It may help those who know the internals of git-rebase if you supplied the
commands you used and your git version.

So, you're saying you did

   git checkout topic
   git rebase dev

or the equivalent

   git rebase dev topic

?  Are you sure you didn't get the arguments to rebase reversed?

-brandon

^ permalink raw reply

* [PATCH 1/2] recv_sideband: Bands #2 and #3 always go to stderr
From: Johannes Sixt @ 2009-03-10 21:54 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: git, gitster, Peter Harris, Sebastian Schuberth, Nicolas Pitre
In-Reply-To: <49B61703.8030602@viscovery.net>

This removes the last parameter of recv_sideband, by which the callers
told which channel bands #2 and #3 should be written to.

Sayeth Shawn Pearce:

   The definition of the streams in the current sideband protocol
   are rather well defined for the one protocol that uses it,
   fetch-pack/receive-pack:

     stream #1:  pack data
     stream #2:  stderr messages, progress, meant for tty
     stream #3:  abort message, remote is dead, goodbye!

Since both callers of the function passed 2 for the parameter, we hereby
remove it and send bands #2 and #3 to stderr explicitly using fprintf.

This has the nice side-effect that these two streams pass through our
ANSI emulation layer on Windows.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
Acked-by: Nicolas Pitre <nico@cam.org>
---
 This patch has an updated commit message and this small change:

  				} else {
  					int l = brk ? brk : len;
 -					if (l > 0)
 -						fprintf(stderr, "%.*s", l, b);
 +					fprintf(stderr, "%.*s", l, b);
  					len -= l;
  				}

 I also has undergone some practical tests in addition to the
 test suite.

 -- Hannes

 builtin-archive.c    |    2 +-
 builtin-fetch-pack.c |    2 +-
 sideband.c           |   19 ++++++++-----------
 sideband.h           |    2 +-
 4 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/builtin-archive.c b/builtin-archive.c
index 60adef9..ab50ceb 100644
--- a/builtin-archive.c
+++ b/builtin-archive.c
@@ -52,7 +52,7 @@ static int run_remote_archiver(int argc, const char **argv,
 		die("git archive: expected a flush");
 
 	/* Now, start reading from fd[0] and spit it out to stdout */
-	rv = recv_sideband("archive", fd[0], 1, 2);
+	rv = recv_sideband("archive", fd[0], 1);
 	close(fd[0]);
 	close(fd[1]);
 	rv |= finish_connect(conn);
diff --git a/builtin-fetch-pack.c b/builtin-fetch-pack.c
index c2e5adc..2b36099 100644
--- a/builtin-fetch-pack.c
+++ b/builtin-fetch-pack.c
@@ -482,7 +482,7 @@ static int sideband_demux(int fd, void *data)
 {
 	int *xd = data;
 
-	return recv_sideband("fetch-pack", xd[0], fd, 2);
+	return recv_sideband("fetch-pack", xd[0], fd);
 }
 
 static int get_pack(int xd[2], char **pack_lockfile)
diff --git a/sideband.c b/sideband.c
index cca3360..899b1ff 100644
--- a/sideband.c
+++ b/sideband.c
@@ -19,7 +19,7 @@
 
 #define FIX_SIZE 10  /* large enough for any of the above */
 
-int recv_sideband(const char *me, int in_stream, int out, int err)
+int recv_sideband(const char *me, int in_stream, int out)
 {
 	unsigned pf = strlen(PREFIX);
 	unsigned sf;
@@ -41,8 +41,7 @@ int recv_sideband(const char *me, int in_stream, int out, int err)
 		if (len == 0)
 			break;
 		if (len < 1) {
-			len = sprintf(buf, "%s: protocol error: no band designator\n", me);
-			safe_write(err, buf, len);
+			fprintf(stderr, "%s: protocol error: no band designator\n", me);
 			return SIDEBAND_PROTOCOL_ERROR;
 		}
 		band = buf[pf] & 0xff;
@@ -50,8 +49,8 @@ int recv_sideband(const char *me, int in_stream, int out, int err)
 		switch (band) {
 		case 3:
 			buf[pf] = ' ';
-			buf[pf+1+len] = '\n';
-			safe_write(err, buf, pf+1+len+1);
+			buf[pf+1+len] = '\0';
+			fprintf(stderr, "%s\n", buf);
 			return SIDEBAND_REMOTE_ERROR;
 		case 2:
 			buf[pf] = ' ';
@@ -95,12 +94,12 @@ int recv_sideband(const char *me, int in_stream, int out, int err)
 					memcpy(save, b + brk, sf);
 					b[brk + sf - 1] = b[brk - 1];
 					memcpy(b + brk - 1, suffix, sf);
-					safe_write(err, b, brk + sf);
+					fprintf(stderr, "%.*s", brk + sf, b);
 					memcpy(b + brk, save, sf);
 					len -= brk;
 				} else {
 					int l = brk ? brk : len;
-					safe_write(err, b, l);
+					fprintf(stderr, "%.*s", l, b);
 					len -= l;
 				}
 
@@ -112,10 +111,8 @@ int recv_sideband(const char *me, int in_stream, int out, int err)
 			safe_write(out, buf + pf+1, len);
 			continue;
 		default:
-			len = sprintf(buf,
-				      "%s: protocol error: bad band #%d\n",
-				      me, band);
-			safe_write(err, buf, len);
+			fprintf(stderr, "%s: protocol error: bad band #%d\n",
+				me, band);
 			return SIDEBAND_PROTOCOL_ERROR;
 		}
 	}
diff --git a/sideband.h b/sideband.h
index a84b691..d72db35 100644
--- a/sideband.h
+++ b/sideband.h
@@ -7,7 +7,7 @@
 #define DEFAULT_PACKET_MAX 1000
 #define LARGE_PACKET_MAX 65520
 
-int recv_sideband(const char *me, int in_stream, int out, int err);
+int recv_sideband(const char *me, int in_stream, int out);
 ssize_t send_sideband(int fd, int band, const char *data, ssize_t sz, int packet_max);
 
 #endif
-- 
1.6.2.103.gb3eac.dirty

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox