* Re: git hang with corrupted .pack
From: Junio C Hamano @ 2009-10-22 6:06 UTC (permalink / raw)
To: Nicolas Pitre, Shawn O. Pearce; +Cc: Andy Isaacson, git, Alex Riesen
In-Reply-To: <7vaazln61u.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Nicolas Pitre <nico@fluxnic.net> writes:
>
>> I didn't spend the time needed to think about this issue and your
>> proposed fix yet.
I looked at the issue again, and came to the conclusion that we need quite
different fix for the two inflate callsites, as they do quite different
things.
-- >8 --
Subject: Fix incorrect error check while reading deflated pack data
The loop in get_size_from_delta() feeds a deflated delta data from the
pack stream _until_ we get inflated result of 20 bytes[*] or we reach the
end of stream.
Side note. This magic number 20 does not have anything to do with the
size of the hash we use, but comes from 1a3b55c (reduce delta head
inflated size, 2006-10-18).
The loop reads like this:
do {
in = use_pack();
stream.next_in = in;
st = git_inflate(&stream, Z_FINISH);
curpos += stream.next_in - in;
} while ((st == Z_OK || st == Z_BUF_ERROR) &&
stream.total_out < sizeof(delta_head));
This git_inflate() can return:
- Z_STREAM_END, if use_pack() fed it enough input and the delta itself
was smaller than 20 bytes;
- Z_OK, when some progress has been made;
- Z_BUF_ERROR, if no progress is possible, because we either ran out of
input (due to corrupt pack), or we ran out of output before we saw the
end of the stream.
The fix b3118bd (sha1_file: Fix infinite loop when pack is corrupted,
2009-10-14) attempted was against a corruption that appears to be a valid
stream that produces a result larger than the output buffer, but we are
not even trying to read the stream to the end in this loop. If avail_out
becomes zero, total_out will be the same as sizeof(delta_head) so the loop
will terminate without the "fix". There is no fix from b3118bd needed for
this loop, in other words.
The loop in unpack_compressed_entry() is quite a different story. It
feeds a deflated stream (either delta or base) and allows the stream to
produce output up to what we expect but no more.
do {
in = use_pack();
stream.next_in = in;
st = git_inflate(&stream, Z_FINISH);
curpos += stream.next_in - in;
} while (st == Z_OK || st == Z_BUF_ERROR)
This _does_ risk falling into an endless interation, as we can exhaust
avail_out if the length we expect is smaller than what the stream wants to
produce (due to pack corruption). In such a case, avail_out will become
zero and inflate() will return Z_BUF_ERROR, while avail_in may (or may
not) be zero.
But this is not a right fix:
do {
in = use_pack();
stream.next_in = in;
st = git_inflate(&stream, Z_FINISH);
+ if (st == Z_BUF_ERROR && (stream.avail_in || !stream.avail_out)
+ break; /* wants more input??? */
curpos += stream.next_in - in;
} while (st == Z_OK || st == Z_BUF_ERROR)
as Z_BUF_ERROR from inflate() may be telling us that avail_in has also run
out before reading the end of stream marker. In such a case, both avail_in
and avail_out would be zero, and the loop should iterate to allow the end
of stream marker to be seen by inflate from the input stream.
The right fix for this loop is likely to be to increment the initial
avail_out by one (we allocate one extra byte to terminate it with NUL
anyway, so there is no risk to overrun the buffer), and break out if we
see that avail_out has become zero, in order to detect that the stream
wants to produce more than what we expect. After the loop, we have a
check that exactly tests this condition:
if ((st != Z_STREAM_END) || stream.total_out != size) {
free(buffer);
return NULL;
}
So here is a patch (without my previous botched attempts) to fix this
issue. The first hunk reverts the corresponding hunk from b3118bd, and
the second hunk is the same fix proposed earlier.
---
sha1_file.c | 8 +++-----
1 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/sha1_file.c b/sha1_file.c
index 4cc8939..63981fb 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1357,8 +1357,6 @@ unsigned long get_size_from_delta(struct packed_git *p,
in = use_pack(p, w_curs, curpos, &stream.avail_in);
stream.next_in = in;
st = git_inflate(&stream, Z_FINISH);
- if (st == Z_BUF_ERROR && (stream.avail_in || !stream.avail_out))
- break;
curpos += stream.next_in - in;
} while ((st == Z_OK || st == Z_BUF_ERROR) &&
stream.total_out < sizeof(delta_head));
@@ -1589,15 +1587,15 @@ static void *unpack_compressed_entry(struct packed_git *p,
buffer[size] = 0;
memset(&stream, 0, sizeof(stream));
stream.next_out = buffer;
- stream.avail_out = size;
+ stream.avail_out = size + 1;
git_inflate_init(&stream);
do {
in = use_pack(p, w_curs, curpos, &stream.avail_in);
stream.next_in = in;
st = git_inflate(&stream, Z_FINISH);
- if (st == Z_BUF_ERROR && (stream.avail_in || !stream.avail_out))
- break;
+ if (!stream.avail_out)
+ break; /* the payload is larger than it should be */
curpos += stream.next_in - in;
} while (st == Z_OK || st == Z_BUF_ERROR);
git_inflate_end(&stream);
^ permalink raw reply related
* Re: Deciding between Git/Mercurial
From: Dilip M @ 2009-10-22 2:38 UTC (permalink / raw)
To: newsgroups; +Cc: git
In-Reply-To: <h9nlhj$heq$1@ger.gmane.org>
Hi Anteru,
On Sun, Sep 27, 2009 at 5:54 PM, Anteru wrote:
..snip..
> So far, my key arguments are that git is more robust (more projects using
> it, larger developer base), of course git's excellent performance and the
> much better support for SVN, which is important for us as we can slowly
> migrate from SVN->Git, while hgmercurial is still in the making (and
> Python's SVN->Hg switch is for instance waiting for it).
So finally which you choosed? Just curious?
--
Dilip
^ permalink raw reply
* It's urgent!
From: Mr.Patrick K.W Chan @ 2009-10-22 1:30 UTC (permalink / raw)
It's urgent!
I am Mr. Patrick K.W Chan , happily married with children, I am the
Executive Director & Chief financial Officer of Hang Seng Bank Ltd. I have
an obscured business suggestion for you, Please get back to me asap so
that we can share in the ratio to be agreed by both of us. You can reach
me on my private email: patrickchankw53@yahoo.com.hk.
--
This mail was scanned by BitDefender
For more information please visit http://www.bitdefender.com
^ permalink raw reply
* Re: [PATCH] blame: make sure that the last line ends in an LF
From: Sverre Rabbelier @ 2009-10-22 1:33 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git List, Johannes Schindelin
In-Reply-To: <7viqe9n72w.fsf@alter.siamese.dyndns.org>
Heya,
On Tue, Oct 20, 2009 at 15:28, Junio C Hamano <gitster@pobox.com> wrote:
> For both styles of output, adding an extra LF after "no newline" would be
> necessary to make the output legible (for human) and parsable (for
> scripts).
You mean like this right?
```
+ printf("\n\\ No newline at end of file\n");
```
Or does it need _another_ newline, like this
```
+ printf("\n\\ No newline at end of file\n\n");
```
> In addition, it would help Porcelains to re-construct the final text if
> you added a boolean "incomplete-line" (put it on its own line, immediately
> after "filename test" line). Then they will know that LF after "second
> line, no newline" was not there in the original and was added for
> parsability.
What do we do in the case that the last few lines are attributed to
the same commit? Do we just signify 'incomplete line' to mean that the
last one of those is incomplete?
> I am not sure what we want to do for non-porcelain output (other than
> adding the extra LF at the end). Assuming that they are meant to be read
> by humans (and casual scripts that do not bother reading --porcelain
> format), it might be best not to add any extra marking.
Fine by me :).
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* Re: [PATCH 3/3] git checkout --nodwim
From: Johannes Schindelin @ 2009-10-22 0:27 UTC (permalink / raw)
To: Nanako Shiraishi
Cc: Avery Pennarun, Junio C Hamano, Alex Riesen, git, Jay Soffian
In-Reply-To: <20091022062145.6117@nanako3.lavabit.com>
Hi,
On Thu, 22 Oct 2009, Nanako Shiraishi wrote:
> Quoting Avery Pennarun <apenwarr@gmail.com>
>
> > On Sun, Oct 18, 2009 at 3:53 PM, Junio C Hamano <gitster@pobox.com> wrote:
> >> Helping hands in polishing it up is very welcome.
> >
> > I find the idea of an option for "don't do what I mean" to be pretty
> > entertaining. Or maybe just misleading :)
> >
> > Have fun,
> >
> > Avery
>
> As Junio asked for helping hands, let's try to be helpful and constructive.
>
> Maybe "don't second-guess" explains it better?
My take on it:
1) --no-porcelain
2) we all are bike-shedding, not being constructive at all
Ciao,
Dscho
^ permalink raw reply
* Re: [Foundation-l] Wikipedia meets git
From: David Gerard @ 2009-10-21 23:36 UTC (permalink / raw)
To: Wikimedia Foundation Mailing List; +Cc: Bernie Innocenti, git
In-Reply-To: <ee9cc730910211308u5a0284d6he483b0904ddb6068@mail.gmail.com>
2009/10/21 jamesmikedupont@googlemail.com <jamesmikedupont@googlemail.com>:
> most people are working on very small subsets of the data. Very few
> people will want to have all the data, think about getting all the
> versions from all the git repos, it would be the same.
> My idea is for smaller chapters who want to get started easily, or
> towns, regions to host their own branches of relevant data.
> Given a world full of such servers, the sum would be great but the
> individual branches needed at one time would be small.
A distributed backend is a nice idea anyway - imagine a meteor hitting
the Florida data centres ...
And there are third-party users who could benefit from a highly
distributed backend, such as Wikileaks.
This thread should probably move to mediawiki-l ...
- d.
^ permalink raw reply
* Re: ident hash usage question
From: Eugene Sajine @ 2009-10-21 23:32 UTC (permalink / raw)
To: Alex Riesen, Junio C Hamano, Daniel Barkalow; +Cc: git
In-Reply-To: <81b0412b0910202325i7a2f440bxcc9ce3ab9b684b2c@mail.gmail.com>
Many thanks to everybody for your time and comments!
I have plenty of info to think about now...;)
With best regards,
Eugene
^ permalink raw reply
* [PATCH] document delta attribute
From: Joey Hess @ 2009-10-21 23:20 UTC (permalink / raw)
To: git
I always wanted a way to turn off delta compression. Turns out git had one,
but it wasn't documented. Added documentation based on
the commit message of a74db82e15cd8a2c53a4a83e9a36dc7bf7a4c750.
Skipped the .jpg example though because I don't want to over-encourage use
of the attribute. (Also because jpeg file changes might delta-compress
well, if only metadata is changed.)
Signed-off-by: Joey Hess <joey@kitenet.net>
---
Documentation/gitattributes.txt | 12 ++++++++++++
1 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index 1195e83..e3e3d2c 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -577,6 +577,18 @@ If this attribute is not set or has an invalid value, the value of the
(See linkgit:git-config[1]).
+Packing objects
+~~~~~~~~~~~~~~~
+
+`delta`
+^^^^^^^
+
+This attribute controls whether linkgit:git-pack-objects[1] will try
+to delta compress files. The attribute is set by default. You may benefit
+from unsetting it for certain large binary files for which attempting delta
+compression will only use CPU cycles and memory.
+
+
USING ATTRIBUTE MACROS
----------------------
--
1.6.4.3
^ permalink raw reply related
* Re: [PATCH] git checkout --no-guess
From: Avery Pennarun @ 2009-10-21 22:51 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Nanako Shiraishi, Alex Riesen, Johannes Schindelin,
Jay Soffian
In-Reply-To: <7vtyxsxtmp.fsf_-_@alter.siamese.dyndns.org>
On Wed, Oct 21, 2009 at 6:35 PM, Junio C Hamano <gitster@pobox.com> wrote:
> As this is strictly script-only option, do not even bother to document it,
> and do bother to hide it from "git checkout -h".
Is it a standard git policy to not document script-only options? As a
person who writes scripts that use git, we will need to discover these
options somehow...
Just curious. (And now wondering how many other wonderful options are
in there but undocumented...)
Thanks,
Avery
^ permalink raw reply
* Re: [PATCH] Document GNU_ROFF in Makefile
From: Anders Kaseorg @ 2009-10-21 22:51 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Miklos Vajna, Thomas Rast, git, bill lam
In-Reply-To: <7vk4yoz8sf.fsf@alter.siamese.dyndns.org>
On Wed, 21 Oct 2009, Junio C Hamano wrote:
> > +# Define GNU_ROFF if you have GNU roff and you don't want to have pretty
> > +# apostrophe so that cut&pasting examples to the shell will work.
>
> This makes it sound as if groff is the only roff implementation that has
> this problem---iow, if we use non-GNU roff then the documentation comes
> out just fine. Is that the case?
Yes:
built without GNU_ROFF built with GNU_ROFF
viewed with non-GNU roff correct (') wrong (no output!)
viewed with GNU groff wrong (´) correct (')
In order to build a manpage that can be viewed correctly on both
platforms, the conditional logic should live in the manpage itself (as per
the bug comments I linked to and Thomas quoted from).
Anders
^ permalink raw reply
* Re: confusion with git diff-tree output
From: Jan Krüger @ 2009-10-21 22:42 UTC (permalink / raw)
To: David Roundy; +Cc: git
In-Reply-To: <117f2cc80910211523m5c1399aej594398fb6597e5de@mail.gmail.com>
> David Roundy <roundyd@physics.oregonstate.edu> wrote:
> You're right. I figured I must be overlooking something obvious, and
> that was it. What surprised me was that -p implies -r, which is not
> documented. Since the -p output was recursive, I incorrectly presumed
> that this was the default.
I suppose that's because you can't really display the diff for a
directory in any meaningful way other than recursing.
Jan
^ permalink raw reply
* [PATCH] git checkout --no-guess
From: Junio C Hamano @ 2009-10-21 22:35 UTC (permalink / raw)
To: git
Cc: Nanako Shiraishi, Avery Pennarun, Alex Riesen,
Johannes Schindelin, Jay Soffian
In-Reply-To: <7vskdcz973.fsf@alter.siamese.dyndns.org>
Porcelains may want to make sure their calls to "git checkout" will
reliably fail regardless of the presense of random remote tracking
branches by the new DWIMmery introduced.
Luckily all existing in-tree callers have extra checks to make sure they
feed local branch name when they want to switch, or they explicitly ask to
detach HEAD at the given commit, so there is no need to add this option
for them.
As this is strictly script-only option, do not even bother to document it,
and do bother to hide it from "git checkout -h".
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Junio C Hamano <gitster@pobox.com> writes:
> Nanako Shiraishi <nanako3@lavabit.com> writes:
>
>> As Junio asked for helping hands, let's try to be helpful and constructive.
>>
>> Maybe "don't second-guess" explains it better?
>
> Perhaps --no-guess, as --no-second-guess is rather hard to read even in scripts.
builtin-checkout.c | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/builtin-checkout.c b/builtin-checkout.c
index fb7e68a..da04eed 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -616,6 +616,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
struct tree *source_tree = NULL;
char *conflict_style = NULL;
int patch_mode = 0;
+ int dwim_new_local_branch = 1;
struct option options[] = {
OPT__QUIET(&opts.quiet),
OPT_STRING('b', NULL, &opts.new_branch, "new branch", "branch"),
@@ -631,6 +632,9 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
OPT_STRING(0, "conflict", &conflict_style, "style",
"conflict style (merge or diff3)"),
OPT_BOOLEAN('p', "patch", &patch_mode, "select hunks interactively"),
+ { OPTION_BOOLEAN, 0, "guess", &dwim_new_local_branch, NULL,
+ "second guess 'git checkout no-such-branch'",
+ PARSE_OPT_NOARG | PARSE_OPT_HIDDEN },
OPT_END(),
};
int has_dash_dash;
@@ -715,6 +719,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
if (has_dash_dash) /* case (1) */
die("invalid reference: %s", arg);
if (!patch_mode &&
+ dwim_new_local_branch &&
opts.track == BRANCH_TRACK_UNSPECIFIED &&
!opts.new_branch &&
!check_filename(NULL, arg) &&
--
1.6.5.1.107.gba912
^ permalink raw reply related
* [PATCH 2/2] receive-pack: run "gc --auto --quiet " and optionally "update-server-info"
From: Junio C Hamano @ 2009-10-21 22:32 UTC (permalink / raw)
To: Johannes Sixt
Cc: Junio C Hamano, Nicolas Pitre, Johan Herland, git, H. Peter Anvin,
ftpadmin
In-Reply-To: <4ADEC0DB.4000104@viscovery.net>
Introduce two new configuration variables, receive.autogc (defaults to
true) and receive.updateserverinfo (defaults to false). When these are
set, receive-pack runs "gc --auto --quiet" and "update-server-info"
respectively after it finishes receiving data from "git push" and updating
refs.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Acked-by: Nicolas Pitre <nico@fluxnic.net>
---
* third round; autogc is default (since v2) and runs "gc --auto"
with "--quiet" (change in this round).
Documentation/config.txt | 9 +++++++++
builtin-receive-pack.c | 20 ++++++++++++++++++++
2 files changed, 29 insertions(+), 0 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index cd17814..ba6ed10 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1320,6 +1320,11 @@ rebase.stat::
Whether to show a diffstat of what changed upstream since the last
rebase. False by default.
+receive.autogc::
+ By default, git-receive-pack will run "git-gc --auto" after
+ receiving data from git-push and updating refs. You can stop
+ it by setting this variable to false.
+
receive.fsckObjects::
If it is set to true, git-receive-pack will check all received
objects. It will abort in the case of a malformed object or a
@@ -1355,6 +1360,10 @@ receive.denyNonFastForwards::
even if that push is forced. This configuration variable is
set when initializing a shared repository.
+receive.updateserverinfo::
+ If set to true, git-receive-pack will run git-update-server-info
+ after receiving data from git-push and updating refs.
+
remote.<name>.url::
The URL of a remote repository. See linkgit:git-fetch[1] or
linkgit:git-push[1].
diff --git a/builtin-receive-pack.c b/builtin-receive-pack.c
index b771fe9..e8bde02 100644
--- a/builtin-receive-pack.c
+++ b/builtin-receive-pack.c
@@ -28,6 +28,8 @@ static int transfer_unpack_limit = -1;
static int unpack_limit = 100;
static int report_status;
static int prefer_ofs_delta = 1;
+static int auto_update_server_info;
+static int auto_gc = 1;
static const char *head_name;
static char *capabilities_to_send;
@@ -88,6 +90,16 @@ static int receive_pack_config(const char *var, const char *value, void *cb)
return 0;
}
+ if (strcmp(var, "receive.updateserverinfo") == 0) {
+ auto_update_server_info = git_config_bool(var, value);
+ return 0;
+ }
+
+ if (strcmp(var, "receive.autogc") == 0) {
+ auto_gc = git_config_bool(var, value);
+ return 0;
+ }
+
return git_default_config(var, value, cb);
}
@@ -672,6 +684,14 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
report(unpack_status);
run_receive_hook(post_receive_hook);
run_update_post_hook(commands);
+ if (auto_gc) {
+ const char *argv_gc_auto[] = {
+ "gc", "--auto", "--quiet", NULL,
+ };
+ run_command_v_opt(argv_gc_auto, RUN_GIT_CMD);
+ }
+ if (auto_update_server_info)
+ update_server_info(0);
}
return 0;
}
--
1.6.5.1.107.gba912
^ permalink raw reply related
* [PATCH 1/2] gc --auto --quiet: make the notice a bit less verboase
From: Junio C Hamano @ 2009-10-21 22:30 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Nicolas Pitre, Johan Herland, git, H. Peter Anvin, ftpadmin
In-Reply-To: <4ADEC0DB.4000104@viscovery.net>
When "gc --auto --quiet" decides there is something to do, it tells the
user what it is doing, as it is going to make the user wait for a bit.
But the message was a bit too long.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Johannes Sixt <j.sixt@viscovery.net> writes:
> I'm slightly in favor of adding "--quiet", because even with this option
> we see some output:
>
> $ git gc --auto --quiet
> Auto packing your repository for optimum performance. You may also
> run "git gc" manually. See "git help gc" for more information.
>
> A compromise would be to reduce this message to the first sentence if
> --quiet was given.
builtin-gc.c | 11 +++++++----
1 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/builtin-gc.c b/builtin-gc.c
index 7d3e9cc..093517e 100644
--- a/builtin-gc.c
+++ b/builtin-gc.c
@@ -216,10 +216,13 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
*/
if (!need_to_gc())
return 0;
- fprintf(stderr, "Auto packing your repository for optimum "
- "performance. You may also\n"
- "run \"git gc\" manually. See "
- "\"git help gc\" for more information.\n");
+ fprintf(stderr,
+ "Auto packing the repository for optimum performance.%s\n",
+ quiet
+ ? ""
+ : (" You may also\n"
+ "run \"git gc\" manually. See "
+ "\"git help gc\" for more information."));
} else
append_option(argv_repack,
prune_expire && !strcmp(prune_expire, "now")
--
1.6.5.1.107.gba912
^ permalink raw reply related
* Re: [PATCH] Document GNU_ROFF in Makefile
From: Junio C Hamano @ 2009-10-21 22:22 UTC (permalink / raw)
To: Miklos Vajna; +Cc: Thomas Rast, git, Anders Kaseorg, Junio C Hamano, bill lam
In-Reply-To: <20091021213149.GX6115@genesis.frugalware.org>
Miklos Vajna <vmiklos@frugalware.org> writes:
> Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
> ---
>
> On Wed, Oct 21, 2009 at 08:57:27PM +0200, Thomas Rast <trast@student.ethz.ch> wrote:
>> Unfortunately, as Anders Kaseorg kindly pointed out, this is not
>> portable beyond groff, so we add an extra Makefile variable GNU_ROFF
>> which you need to enable to get the new quoting.
>
> You forgot to document this in Makefile. Maybe this could be just
> squashed in, Junio will decide.
>
> Makefile | 3 +++
> 1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index fea237b..40da590 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -159,6 +159,9 @@ all::
> # Define ASCIIDOC_NO_ROFF if your DocBook XSL escapes raw roff directives
> # (versions 1.72 and later and 1.68.1 and earlier).
> #
> +# Define GNU_ROFF if you have GNU roff and you don't want to have pretty
> +# apostrophe so that cut&pasting examples to the shell will work.
> +#
This makes it sound as if groff is the only roff implementation that has
this problem---iow, if we use non-GNU roff then the documentation comes
out just fine. Is that the case?
Or is the situation more like "we know how to fix this for groff, and the
fix is enabled if this variable is defined---on other roff implementations
the output is still broken"?
> # Define NO_PERL_MAKEMAKER if you cannot use Makefiles generated by perl's
> # MakeMaker (e.g. using ActiveState under Cygwin).
> #
> --
> 1.6.5
^ permalink raw reply
* Re: confusion with git diff-tree output
From: David Roundy @ 2009-10-21 22:23 UTC (permalink / raw)
To: git
In-Reply-To: <20091021195103.01cef9c4@perceptron>
You're right. I figured I must be overlooking something obvious, and
that was it. What surprised me was that -p implies -r, which is not
documented. Since the -p output was recursive, I incorrectly presumed
that this was the default.
David
On Wed, Oct 21, 2009 at 1:51 PM, Jan Krüger <jk@jk.gs> wrote:
> Tree objects are recursively nested, i.e.
>
>> 66b67ea1763799c0b2ac01f6803177ca870f6544 M Iolaus
>
> is a reference to another tree object... and since a file in that
> subtree changed, a new tree object that contains a different file
> record is now referenced as "Iolaus".
>
> By default git diff-tree doesn't recurse, but you can use -r for that.
> Which is documented, I might add. ;)
^ permalink raw reply
* Re: [PATCH 3/3] git checkout --nodwim
From: Junio C Hamano @ 2009-10-21 22:14 UTC (permalink / raw)
To: Nanako Shiraishi
Cc: Avery Pennarun, Junio C Hamano, Alex Riesen, git,
Johannes Schindelin, Jay Soffian
In-Reply-To: <20091022062145.6117@nanako3.lavabit.com>
Nanako Shiraishi <nanako3@lavabit.com> writes:
> As Junio asked for helping hands, let's try to be helpful and constructive.
>
> Maybe "don't second-guess" explains it better?
Perhaps --no-guess, as --no-second-guess is rather hard to read even in scripts.
^ permalink raw reply
* Re: [PATCH v3] new --dirty option for git describe
From: Junio C Hamano @ 2009-10-21 22:07 UTC (permalink / raw)
To: Jean Privat; +Cc: git, Junio C Hamano, Shawn O. Pearce
In-Reply-To: <1256132122-12509-1-git-send-email-jean@pryen.org>
Thanks. I'll queue it to 'pu' after minor reformatting and typofixes to
the log message.
> 3 Use --dirty as in this patch
> ...
> Pro: has an easy fallback to alternative 1 if the world become suddenly
> ideal, or at least allows some kind of future transition plan if
> people *really* bother:
> 1- ask that scripts use either "git describe HEAD" or
> "git describe --dirty" (no more "git describe")
> 2- change default
> Once the transition is done, the role of the --dirty option will
> just be the way to specify an alternative mark (and a noop if alone)
Also I'd drop this as a Pro specific to #3. It is clear that the first
one is incompatible from the beginning and there is no "smooth migration
path" possible; also I think the second one can be migrated equally well
in exactly the same way, so this is not an advantage specific to #3.
> diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh
> index 8c7e081..e061177 100755
> --- a/t/t6120-describe.sh
> +++ b/t/t6120-describe.sh
> @@ -123,6 +123,20 @@ test_expect_success 'rename tag Q back to A' '
> test_expect_success 'pack tag refs' 'git pack-refs'
> check_describe A-* HEAD
>
> +check_describe "A-*[0-9a-f]" --dirty
> +
> +test_expect_success 'set-up dirty working tree' '
> + echo >>file
> +'
> +
> +check_describe "A-*[0-9a-f]-dirty" --dirty
> +
> +check_describe "A-*[0-9a-f].mod" --dirty=.mod
> +
> +test_expect_success 'describe --dirty HEAD' '
> + test_must_fail git describe --dirty HEAD
> +'
"clean work tree gets no marks; dirty work tree gets expected marks with
or without the mark string; the option is incompatible with an explicit
rev". Nicely covers all cases. Good job.
^ permalink raw reply
* [PATCH] git-merge: imply --no-ff when --no-commit is given
From: Junio C Hamano @ 2009-10-21 21:46 UTC (permalink / raw)
To: git; +Cc: Clemens Buchacher, Björn Steinbrink, Daniel Barkalow,
Thomas Rast
In-Reply-To: <7v3a5c2zrr.fsf@alter.siamese.dyndns.org>
Traditionally "git merge --no-commit" meant just that: do not create a new
commit even when a merge succeeds. But this leads to confusion when the
merged commit is a descendant of the current commit, in which case we
succeed the merge by fast-forwarding and without creating a new commit.
Make --no-commit imply --no-ff; --no-commit is a request by the user to
tweak the resulting merge and it is clear indication that the user wants
to have a merge, even if it is an extra one, to futz with.
There is a test that relies on --no-commit silently fast forwarding; that
is obviously broken by this change.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
* This is another possibility, which I think is worse than the other one
in practice but the justification sounds more respectable.
Unlike the other one, this will still make --no-commit a no-op when you
are already up-to-date. As I do not think --no-ff makes much sense in
my own workflow (either here or dayjob) it is not exactly my itch, but
I suspect that people who wanted to have --no-ff may want to create an
extra commit even in such a case; it may be a bug to allow up-to-date
when --no-ff is given. Yes, it would make the --no-ff even more insane
than it already is, but I suspect it would be more consistent with the
original reasoning of wanting to have the option in the first place,
namely, to leave the trace of the fact that a "merge" was done at that
point in the history.
builtin-merge.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/builtin-merge.c b/builtin-merge.c
index b6b8428..fa86799 100644
--- a/builtin-merge.c
+++ b/builtin-merge.c
@@ -874,6 +874,9 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
option_commit = 0;
}
+ if (!option_commit)
+ allow_fast_forward = 0;
+
if (!argc)
usage_with_options(builtin_merge_usage,
builtin_merge_options);
--
1.6.5.1.107.gba912
^ permalink raw reply related
* Re: [PATCH v2] Quote ' as \(aq in manpages
From: Anders Kaseorg @ 2009-10-21 21:42 UTC (permalink / raw)
To: Thomas Rast; +Cc: git, Miklos Vajna, Junio C Hamano, bill lam
In-Reply-To: <7a3e6c8c5a11e14c19bc1a27608dcc78171c9feb.1256151199.git.trast@student.ethz.ch>
On Wed, 21 Oct 2009, Thomas Rast wrote:
> Unfortunately, as Anders Kaseorg kindly pointed out, this is not
> portable beyond groff, so we add an extra Makefile variable GNU_ROFF
> which you need to enable to get the new quoting.
Note that GNU_ROFF is a property of the target system on which the
manpages will be read, unlike the existing variables (ASCIIDOC8,
DOCBOOK_XSL_172, ASCIIDOC_NO_ROFF) which are properties of the host system
on which they are built.
> To save you the effort of clicking the links, the header definitions
> would be
>
> .ie \n(.g .ds Aq \(aq
> .el .ds Aq '
>
> and you then have to change the template to quote to \(Aq instead.
If someone knows how to get this definition into the header, that would be
preferable, because then you could read the same manpage on both GNU and
non-GNU systems instead of building separately for each.
It would be even better if someone would work with the Docbook developers
to get this fixed upstream. (Unfortunately, there has been no reply to my
comment in their bug tracker.)
Anders
^ permalink raw reply
* [RFC/PATCH] git-merge: forbid fast-forward and up-to-date when --no-commit is given
From: Junio C Hamano @ 2009-10-21 21:41 UTC (permalink / raw)
To: git; +Cc: Clemens Buchacher, Björn Steinbrink, Daniel Barkalow,
Thomas Rast
In-Reply-To: <7v3a5c2zrr.fsf@alter.siamese.dyndns.org>
Traditionally "git merge --no-commit" meant just that: do not create a new
commit even when a merge succeeds. But this leads to confusion when the
merged commit is a descendant of the current commit, in which case we
succeed the merge by fast-forwarding and without creating a new commit.
Also when the merged commit is already a part of the history, we succeeded
without doing anything.
Error out when --no-commit is given but the merge would result in a
fast-forward or an up-to-date.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
* This is the first alternative. I think it makes more sense than the
other one, but I am unsure, as I obviously do not get confused when
--no-commit becomes no-op due to a fast-forward nor an up-to-date and
am rather happy with the current behaviour.
builtin-merge.c | 11 +++++++++++
1 files changed, 11 insertions(+), 0 deletions(-)
diff --git a/builtin-merge.c b/builtin-merge.c
index b6b8428..4cfdf75 100644
--- a/builtin-merge.c
+++ b/builtin-merge.c
@@ -829,6 +829,12 @@ static int evaluate_result(void)
return cnt;
}
+static void check_no_commit(const char *msg)
+{
+ if (!option_commit)
+ die("The merge will %s but --no-commit was given.", msg);
+}
+
int cmd_merge(int argc, const char **argv, const char *prefix)
{
unsigned char result_tree[20];
@@ -996,6 +1002,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
* If head can reach all the merge then we are up to date.
* but first the most common case of merging one remote.
*/
+ check_no_commit("be a no-op because you are up-to-date");
finish_up_to_date("Already up-to-date.");
return 0;
} else if (allow_fast_forward && !remoteheads->next &&
@@ -1006,6 +1013,9 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
struct object *o;
char hex[41];
+ if (allow_fast_forward)
+ check_no_commit("fast forward");
+
strcpy(hex, find_unique_abbrev(head, DEFAULT_ABBREV));
if (verbosity >= 0)
@@ -1074,6 +1084,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
}
}
if (up_to_date) {
+ check_no_commit("fast forward");
finish_up_to_date("Already up-to-date. Yeeah!");
return 0;
}
--
1.6.5.1.107.gba912
^ permalink raw reply related
* Re: [PATCH] modernize fetch/merge/pull examples
From: Junio C Hamano @ 2009-10-21 21:38 UTC (permalink / raw)
To: Clemens Buchacher
Cc: Björn Steinbrink, Daniel Barkalow, Thomas Rast, git
In-Reply-To: <20091021172123.GB27495@localhost>
Clemens Buchacher <drizzd@aon.at> writes:
> The "git pull" documentation has examples which follow an outdated
> style. Update the examples to use "git merge" where appropriate and
> move the examples to the corresponding manpages.
Makes sense, although I did some minor rewording.
I noticed something related while reading this (the issue does not affect
the validity of this patch).
> +* Merge branch `maint` into the current branch, but do not make
> + a commit automatically:
> ++
> +------------------------------------------------
> +$ git merge --no-commit maint
> +------------------------------------------------
> ++
> +This can be used when you want to include further changes to the
> +merge, or want to write your own merge commit message.
When you are up to date with maint this would be a no-op, and when you are
strictly behind maint it will succeed without creating a new commit.
I have two possible approaches to fix this issue.
^ permalink raw reply
* [PATCH] Document GNU_ROFF in Makefile
From: Miklos Vajna @ 2009-10-21 21:31 UTC (permalink / raw)
To: Thomas Rast; +Cc: git, Anders Kaseorg, Junio C Hamano, bill lam
In-Reply-To: <7a3e6c8c5a11e14c19bc1a27608dcc78171c9feb.1256151199.git.trast@student.ethz.ch>
Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---
On Wed, Oct 21, 2009 at 08:57:27PM +0200, Thomas Rast <trast@student.ethz.ch> wrote:
> Unfortunately, as Anders Kaseorg kindly pointed out, this is not
> portable beyond groff, so we add an extra Makefile variable GNU_ROFF
> which you need to enable to get the new quoting.
You forgot to document this in Makefile. Maybe this could be just
squashed in, Junio will decide.
Makefile | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/Makefile b/Makefile
index fea237b..40da590 100644
--- a/Makefile
+++ b/Makefile
@@ -159,6 +159,9 @@ all::
# Define ASCIIDOC_NO_ROFF if your DocBook XSL escapes raw roff directives
# (versions 1.72 and later and 1.68.1 and earlier).
#
+# Define GNU_ROFF if you have GNU roff and you don't want to have pretty
+# apostrophe so that cut&pasting examples to the shell will work.
+#
# Define NO_PERL_MAKEMAKER if you cannot use Makefiles generated by perl's
# MakeMaker (e.g. using ActiveState under Cygwin).
#
--
1.6.5
^ permalink raw reply related
* [RFC PATCH 2/2] Make it possible to apply a range of changes at once
From: Jeff Epler @ 2009-10-21 21:20 UTC (permalink / raw)
To: git; +Cc: Jeff Epler
In-Reply-To: <1256160023-29629-1-git-send-email-jepler@unpythonic.net>
---
The diff looks bigger than it is because it changed the indentation
level of about 80 lines, and that made it necessary to reflow a lengthy
commit block as well.
git-gui.sh | 15 +++-
lib/diff.tcl | 225 ++++++++++++++++++++++++++++++++--------------------------
2 files changed, 135 insertions(+), 105 deletions(-)
diff --git a/git-gui.sh b/git-gui.sh
index 09b2720..c69d904 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -3194,7 +3194,7 @@ set ui_diff_applyhunk [$ctxm index last]
lappend diff_actions [list $ctxm entryconf $ui_diff_applyhunk -state]
$ctxm add command \
-label [mc "Apply/Reverse Line"] \
- -command {apply_line $cursorX $cursorY; do_rescan}
+ -command {apply_range_or_line $cursorX $cursorY; do_rescan}
set ui_diff_applyline [$ctxm index last]
lappend diff_actions [list $ctxm entryconf $ui_diff_applyline -state]
$ctxm add separator
@@ -3234,12 +3234,21 @@ proc popup_diff_menu {ctxm ctxmmg x y X Y} {
if {[string first {U} $state] >= 0} {
tk_popup $ctxmmg $X $Y
} else {
+ set has_range [expr {[$::ui_diff tag nextrange sel 0.0] != {}}]
if {$::ui_index eq $::current_diff_side} {
set l [mc "Unstage Hunk From Commit"]
- set t [mc "Unstage Line From Commit"]
+ if {$has_range} {
+ set t [mc "Unstage Lines From Commit"]
+ } else {
+ set t [mc "Unstage Line From Commit"]
+ }
} else {
set l [mc "Stage Hunk For Commit"]
- set t [mc "Stage Line For Commit"]
+ if {$has_range} {
+ set t [mc "Stage Lines For Commit"]
+ } else {
+ set t [mc "Stage Line For Commit"]
+ }
}
if {$::is_3way_diff || $::is_submodule_diff
|| $current_diff_path eq {}
diff --git a/lib/diff.tcl b/lib/diff.tcl
index 066755b..0fe3ec6 100644
--- a/lib/diff.tcl
+++ b/lib/diff.tcl
@@ -533,10 +533,23 @@ proc apply_hunk {x y} {
}
}
-proc apply_line {x y} {
+proc apply_range_or_line {x y} {
global current_diff_path current_diff_header current_diff_side
global ui_diff ui_index file_states
+ set selected [$ui_diff tag nextrange sel 0.0]
+
+ if {$selected == {}} {
+ set first [$ui_diff index "@$x,$y"]
+ set last $first
+ } else {
+ set first [lindex $selected 0]
+ set last [lindex $selected 1]
+ }
+
+ set first_l [$ui_diff index "$first linestart"]
+ set last_l [$ui_diff index "$last lineend"]
+
if {$current_diff_path eq {} || $current_diff_header eq {}} return
if {![lock_index apply_hunk]} return
@@ -559,120 +572,128 @@ proc apply_line {x y} {
}
}
- set the_l [$ui_diff index @$x,$y]
+ set wholepatch {}
- # operate only on change lines
- set c1 [$ui_diff get "$the_l linestart"]
- if {$c1 ne {+} && $c1 ne {-}} {
- unlock_index
- return
- }
- set sign $c1
-
- set i_l [$ui_diff search -backwards -regexp ^@@ $the_l 0.0]
- if {$i_l eq {}} {
- unlock_index
- return
- }
- # $i_l is now at the beginning of a line
+ while {$first_l < $last_l} {
+ set i_l [$ui_diff search -backwards -regexp ^@@ $first_l 0.0]
+ if {$i_l eq {}} {
+ # If there's not a @@ above, then the selected range
+ # must have come before the first_l @@
+ set i_l [$ui_diff search -regexp ^@@ $first_l $last_l]
+ }
+ if {$i_l eq {}} {
+ unlock_index
+ return
+ }
+ # $i_l is now at the beginning of a line
- # pick start line number from hunk header
- set hh [$ui_diff get $i_l "$i_l + 1 lines"]
- set hh [lindex [split $hh ,] 0]
- set hln [lindex [split $hh -] 1]
+ # pick start line number from hunk header
+ set hh [$ui_diff get $i_l "$i_l + 1 lines"]
+ set hh [lindex [split $hh ,] 0]
+ set hln [lindex [split $hh -] 1]
- # There is a special situation to take care of. Consider this hunk:
- #
- # @@ -10,4 +10,4 @@
- # context before
- # -old 1
- # -old 2
- # +new 1
- # +new 2
- # context after
- #
- # We used to keep the context lines in the order they appear in the
- # hunk. But then it is not possible to correctly stage only
- # "-old 1" and "+new 1" - it would result in this staged text:
- #
- # context before
- # old 2
- # new 1
- # context after
- #
- # (By symmetry it is not possible to *un*stage "old 2" and "new 2".)
- #
- # We resolve the problem by introducing an asymmetry, namely, when
- # a "+" line is *staged*, it is moved in front of the context lines
- # that are generated from the "-" lines that are immediately before
- # the "+" block. That is, we construct this patch:
- #
- # @@ -10,4 +10,5 @@
- # context before
- # +new 1
- # old 1
- # old 2
- # context after
- #
- # But we do *not* treat "-" lines that are *un*staged in a special
- # way.
- #
- # With this asymmetry it is possible to stage the change
- # "old 1" -> "new 1" directly, and to stage the change
- # "old 2" -> "new 2" by first staging the entire hunk and
- # then unstaging the change "old 1" -> "new 1".
-
- # This is non-empty if and only if we are _staging_ changes;
- # then it accumulates the consecutive "-" lines (after converting
- # them to context lines) in order to be moved after the "+" change
- # line.
- set pre_context {}
-
- set n 0
- set i_l [$ui_diff index "$i_l + 1 lines"]
- set patch {}
- while {[$ui_diff compare $i_l < "end - 1 chars"] &&
- [$ui_diff get $i_l "$i_l + 2 chars"] ne {@@}} {
- set next_l [$ui_diff index "$i_l + 1 lines"]
- set c1 [$ui_diff get $i_l]
- if {[$ui_diff compare $i_l <= $the_l] &&
- [$ui_diff compare $the_l < $next_l]} {
- # the line to stage/unstage
- set ln [$ui_diff get $i_l $next_l]
- if {$c1 eq {-}} {
- set n [expr $n+1]
+ # There is a special situation to take care of. Consider this
+ # hunk:
+ #
+ # @@ -10,4 +10,4 @@
+ # context before
+ # -old 1
+ # -old 2
+ # +new 1
+ # +new 2
+ # context after
+ #
+ # We used to keep the context lines in the order they appear in
+ # the hunk. But then it is not possible to correctly stage only
+ # "-old 1" and "+new 1" - it would result in this staged text:
+ #
+ # context before
+ # old 2
+ # new 1
+ # context after
+ #
+ # (By symmetry it is not possible to *un*stage "old 2" and "new
+ # 2".)
+ #
+ # We resolve the problem by introducing an asymmetry, namely,
+ # when a "+" line is *staged*, it is moved in front of the
+ # context lines that are generated from the "-" lines that are
+ # immediately before the "+" block. That is, we construct this
+ # patch:
+ #
+ # @@ -10,4 +10,5 @@
+ # context before
+ # +new 1
+ # old 1
+ # old 2
+ # context after
+ #
+ # But we do *not* treat "-" lines that are *un*staged in a
+ # special way.
+ #
+ # With this asymmetry it is possible to stage the change "old
+ # 1" -> "new 1" directly, and to stage the change "old 2" ->
+ # "new 2" by first staging the entire hunk and then unstaging
+ # the change "old 1" -> "new 1".
+
+ # This is non-empty if and only if we are _staging_ changes;
+ # then it accumulates the consecutive "-" lines (after
+ # converting them to context lines) in order to be moved after
+ # the "+" change line.
+ set pre_context {}
+
+ set n 0
+ set m 0
+ set i_l [$ui_diff index "$i_l + 1 lines"]
+ set patch {}
+ while {[$ui_diff compare $i_l < "end - 1 chars"] &&
+ [$ui_diff get $i_l "$i_l + 2 chars"] ne {@@}} {
+ set next_l [$ui_diff index "$i_l + 1 lines"]
+ set c1 [$ui_diff get $i_l]
+ if {[$ui_diff compare $first_l <= $i_l] &&
+ [$ui_diff compare $i_l < $last_l] &&
+ ($c1 eq {-} || $c1 eq {+})} {
+ # a line to stage/unstage
+ set ln [$ui_diff get $i_l $next_l]
+ if {$c1 eq {-}} {
+ set n [expr $n+1]
+ set patch "$patch$pre_context$ln"
+ } else {
+ set m [expr $m+1]
+ set patch "$patch$ln$pre_context"
+ }
+ set pre_context {}
+ } elseif {$c1 ne {-} && $c1 ne {+}} {
+ # context line
+ set ln [$ui_diff get $i_l $next_l]
set patch "$patch$pre_context$ln"
- } else {
- set patch "$patch$ln$pre_context"
- }
- set pre_context {}
- } elseif {$c1 ne {-} && $c1 ne {+}} {
- # context line
- set ln [$ui_diff get $i_l $next_l]
- set patch "$patch$pre_context$ln"
- set n [expr $n+1]
- set pre_context {}
- } elseif {$c1 eq $to_context} {
- # turn change line into context line
- set ln [$ui_diff get "$i_l + 1 chars" $next_l]
- if {$c1 eq {-}} {
- set pre_context "$pre_context $ln"
- } else {
- set patch "$patch $ln"
+ set n [expr $n+1]
+ set m [expr $m+1]
+ set pre_context {}
+ } elseif {$c1 eq $to_context} {
+ # turn change line into context line
+ set ln [$ui_diff get "$i_l + 1 chars" $next_l]
+ if {$c1 eq {-}} {
+ set pre_context "$pre_context $ln"
+ } else {
+ set patch "$patch $ln"
+ }
+ set n [expr $n+1]
+ set m [expr $m+1]
}
- set n [expr $n+1]
+ set i_l $next_l
}
- set i_l $next_l
+ set patch "$patch$pre_context"
+ set wholepatch "$wholepatch@@ -$hln,$n +$hln,$m @@\n$patch"
+ set first_l [$ui_diff index "$next_l + 1 lines"]
}
- set patch "$patch$pre_context"
- set patch "@@ -$hln,$n +$hln,[eval expr $n $sign 1] @@\n$patch"
if {[catch {
set enc [get_path_encoding $current_diff_path]
set p [eval git_write $apply_cmd]
fconfigure $p -translation binary -encoding $enc
puts -nonewline $p $current_diff_header
- puts -nonewline $p $patch
+ puts -nonewline $p $wholepatch
close $p} err]} {
error_popup [append $failed_msg "\n\n$err"]
}
--
1.6.5.rc1.49.ge970
^ permalink raw reply related
* [RFC PATCH v2 0/2] git-gui: (un)stage a range of changes at once
From: Jeff Epler @ 2009-10-21 21:20 UTC (permalink / raw)
To: git; +Cc: Jeff Epler
Compared to the first version, I fixed a bug concerning staging line(s)
when all following lines are deletions (a preexisting bug in git-gui).
This version is made based off the master branch of git-gui.git, rather
than the master branch of git.git.
The first change fixes a long-standing git-gui bug in an area that the
new feature is rewriting anyway. If there's interest in the new feature
then maybe the two should just be squashed (using the message from the
second). If not, it'd be nice to see the bugfix applied anyway.
Jeff Epler (2):
Fix applying a line when all following lines are deletions
Make it possible to apply a range of changes at once
git-gui.sh | 15 +++-
lib/diff.tcl | 224 ++++++++++++++++++++++++++++++++--------------------------
2 files changed, 135 insertions(+), 104 deletions(-)
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox