* Re: [PATCH v2 3/3] send-pack: assign remote errors to each ref
From: Junio C Hamano @ 2007-11-14 1:41 UTC (permalink / raw)
To: Jeff King; +Cc: git, Pierre Habouzit, Daniel Barkalow, Alex Riesen
In-Reply-To: <20071113113710.GC15880@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> This lets us show remote errors (e.g., a denied hook) along
> with the usual push output. ...
Yay!
> ... There are two drawbacks to this
> change:
>
> 1. cross-referencing the incoming status with the ref list
> is worst case O(n^2) (where n = number of refs); this
> can be fixed with a smarter implementation
Sure.
> 2. the status parsing is not foolproof. We get a line like
>
> ng refs/heads/master arbitrary msg
>
> which cannot be parsed unambiguously in the face of
> refnames with spaces. We do a prefix-match so that
> you will only run into problems if you have two refs,
> one of which is a prefix match of the other, and the
> longer having a space right after the prefix.
Is it really "arbitrary msg", or just a fixed set of strings?
Also I think we can rely on the order report-status extension
reports the per-ref result. It gives back the information the
same order send-pack side supplies the head information, no?
^ permalink raw reply
* Re: [PATCH v2 3/3] send-pack: assign remote errors to each ref
From: Junio C Hamano @ 2007-11-14 1:56 UTC (permalink / raw)
To: Jeff King; +Cc: git, Pierre Habouzit, Daniel Barkalow, Alex Riesen
In-Reply-To: <7vbq9xpprg.fsf@gitster.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
>> 2. the status parsing is not foolproof. We get a line like
>>
>> ng refs/heads/master arbitrary msg
>>
>> which cannot be parsed unambiguously...
>
> Is it really "arbitrary msg", or just a fixed set of strings?
This was a wrong thing to say. We may add new error message in
later versions of receive-pack and we want to make it compatible
with today's send-pack with minimum hassle. So that is a
problem. But...
> Also I think we can rely on the order report-status extension
> reports the per-ref result. It gives back the information the
> same order send-pack side supplies the head information, no?
I was hoping that this can solve both of the problems you
cited...
^ permalink raw reply
* Re: [PATCH v2 3/3] send-pack: assign remote errors to each ref
From: Johannes Schindelin @ 2007-11-14 2:01 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jeff King, git, Pierre Habouzit, Daniel Barkalow, Alex Riesen
In-Reply-To: <7vbq9xpprg.fsf@gitster.siamese.dyndns.org>
Hi,
On Tue, 13 Nov 2007, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > 2. the status parsing is not foolproof. We get a line like
> >
> > ng refs/heads/master arbitrary msg
> >
> > which cannot be parsed unambiguously in the face of
> > refnames with spaces.
Since when can refnames contain spaces? In my copy of git, bad_ref_char()
in refs.c returns 1 if ch <= ' '. It's the first error path, even.
Ciao,
Dscho
^ permalink raw reply
* [PATCH] Bisect visualize: use "for-each-ref" to list all good refs.
From: Christian Couder @ 2007-11-14 4:50 UTC (permalink / raw)
To: Junio Hamano; +Cc: git, Paul Mackerras
In bisect_visualize, "cd $GIT_DIR/refs && echo bisect/good-*" was
still used instead of "git for-each-ref". This patch fix it.
We now pass "refs/bisect/bad" and "--not refs/bisect/good-<rev>"
instead of "bisect/bad" and "--not bisect/good-<rev>" to gitk,
but it seems to work.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
git-bisect.sh | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/git-bisect.sh b/git-bisect.sh
index 1ed44e5..8d75d91 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -326,8 +326,8 @@ bisect_next() {
bisect_visualize() {
bisect_next_check fail
- not=`cd "$GIT_DIR/refs" && echo bisect/good-*`
- eval gitk bisect/bad --not $not -- $(cat "$GIT_DIR/BISECT_NAMES")
+ not=$(git for-each-ref --format='%(refname)' "refs/bisect/good-*")
+ eval gitk refs/bisect/bad --not $not -- $(cat "$GIT_DIR/BISECT_NAMES")
}
bisect_reset() {
--
1.5.3.5.629.g807b3
^ permalink raw reply related
* [PATCH] Don't allow fast-import tree delta chains to exceed maximum depth
From: Shawn O. Pearce @ 2007-11-14 4:48 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Brian Downing noticed fast-import can produce tree depths of up
to 6,035 objects and even deeper. Long delta chains can create
very small packfiles but cause problems during repacking as git
needs to unpack each tree to count the reachable blobs.
What's happening here is the active branch cache isn't big enough.
We're swapping out the branch and thus recycling the tree information
(struct tree_content) back into the free pool. When we later reload
the tree we set the delta_depth to 0 but we kept the tree we just
reloaded as a delta base.
So if the tree we reloaded was already at the maximum depth we
wouldn't know it and make the new tree a delta. Multiply the
number of times the branch cache has to swap out the tree times
max_depth (10) and you get the maximum delta depth of a tree created
by fast-import. In Brian's case above the active branch cache had
to swap the branch out 603/604 times during this import to produce
a tree with a delta depth of 6035.
Acked-by: Brian Downing <bdowning@lavos.net>
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
Junio, this patch is against maint. It will apply cleanly to maint
but is also crafted to ensure it should apply to next with git-am -3.
Its a real bug that's lasted a long time in fast-import. I think
it is maint material.
My fast-import tree is quite behind yours so I would appreciate
it if you could apply this to your own tree instead of trying to
fetch things from me. Thanks.
fast-import.c | 17 ++++++++++++-----
1 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/fast-import.c b/fast-import.c
index c07e3d8..7544949 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -154,13 +154,16 @@ Format of STDIN stream:
#define PACK_ID_BITS 16
#define MAX_PACK_ID ((1<<PACK_ID_BITS)-1)
+#define DEPTH_BITS 13
+#define MAX_DEPTH ((1<<DEPTH_BITS)-1)
struct object_entry
{
struct object_entry *next;
uint32_t offset;
- unsigned type : TYPE_BITS;
- unsigned pack_id : PACK_ID_BITS;
+ uint32_t type : TYPE_BITS,
+ pack_id : PACK_ID_BITS,
+ depth : DEPTH_BITS;
unsigned char sha1[20];
};
@@ -1105,7 +1108,7 @@ static int store_object(
unsigned pos = sizeof(hdr) - 1;
delta_count_by_type[type]++;
- last->depth++;
+ e->depth = ++last->depth++;
hdrlen = encode_header(OBJ_OFS_DELTA, deltalen, hdr);
write_or_die(pack_data->pack_fd, hdr, hdrlen);
@@ -1117,6 +1120,7 @@ static int store_object(
write_or_die(pack_data->pack_fd, hdr + pos, sizeof(hdr) - pos);
pack_size += sizeof(hdr) - pos;
} else {
+ e->depth = 0;
if (last)
last->depth = 0;
hdrlen = encode_header(type, datlen, hdr);
@@ -1181,7 +1185,7 @@ static void load_tree(struct tree_entry *root)
if (myoe && myoe->pack_id != MAX_PACK_ID) {
if (myoe->type != OBJ_TREE)
die("Not a tree: %s", sha1_to_hex(sha1));
- t->delta_depth = 0;
+ t->delta_depth = myoe->depth;
buf = gfi_unpack_entry(myoe, &size);
} else {
enum object_type type;
@@ -2347,8 +2351,11 @@ int main(int argc, const char **argv)
}
else if (!prefixcmp(a, "--max-pack-size="))
max_packsize = strtoumax(a + 16, NULL, 0) * 1024 * 1024;
- else if (!prefixcmp(a, "--depth="))
+ else if (!prefixcmp(a, "--depth=")) {
max_depth = strtoul(a + 8, NULL, 0);
+ if (max_depth > MAX_DEPTH)
+ die("--depth cannot exceed %u", MAX_DEPTH);
+ }
else if (!prefixcmp(a, "--active-branches="))
max_active_branches = strtoul(a + 18, NULL, 0);
else if (!prefixcmp(a, "--import-marks="))
--
1.5.3.5.1728.g34b3e
^ permalink raw reply related
* Re: [PATCH] Don't allow fast-import tree delta chains to exceed maximum depth
From: Brian Downing @ 2007-11-14 5:45 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Junio C Hamano, git
In-Reply-To: <20071114044842.GA6876@spearce.org>
On Tue, Nov 13, 2007 at 11:48:42PM -0500, Shawn O. Pearce wrote:
> @@ -1105,7 +1108,7 @@ static int store_object(
> unsigned pos = sizeof(hdr) - 1;
>
> delta_count_by_type[type]++;
> - last->depth++;
> + e->depth = ++last->depth++;
>
> hdrlen = encode_header(OBJ_OFS_DELTA, deltalen, hdr);
> write_or_die(pack_data->pack_fd, hdr, hdrlen);
I'm not sure, but I think that's too many ++'s.
The earlier patch had:
@@ -1084,6 +1087,7 @@ static int store_object(
delta_count_by_type[type]++;
last->depth++;
+ e->depth = last->depth;
hdrlen = encode_header(OBJ_OFS_DELTA, deltalen, hdr);
write_or_die(pack_data->pack_fd, hdr, hdrlen);
Which of course would be the equivalent of:
> + e->depth = ++last->depth;
Maybe there's some cleverness here I'm missing, though.
-bcd
^ permalink raw reply
* Re: [PATCH] Don't allow fast-import tree delta chains to exceed maximum depth
From: Junio C Hamano @ 2007-11-14 5:46 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git
In-Reply-To: <20071114044842.GA6876@spearce.org>
"Shawn O. Pearce" <spearce@spearce.org> writes:
> Junio, this patch is against maint. It will apply cleanly to maint
> but is also crafted to ensure it should apply to next with git-am -3.
> Its a real bug that's lasted a long time in fast-import. I think
> it is maint material.
Thanks.
> diff --git a/fast-import.c b/fast-import.c
> index c07e3d8..7544949 100644
> --- a/fast-import.c
> +++ b/fast-import.c
> @@ -154,13 +154,16 @@ Format of STDIN stream:
>
> #define PACK_ID_BITS 16
> #define MAX_PACK_ID ((1<<PACK_ID_BITS)-1)
> +#define DEPTH_BITS 13
> +#define MAX_DEPTH ((1<<DEPTH_BITS)-1)
>
> struct object_entry
> {
> struct object_entry *next;
> uint32_t offset;
> - unsigned type : TYPE_BITS;
> - unsigned pack_id : PACK_ID_BITS;
> + uint32_t type : TYPE_BITS,
> + pack_id : PACK_ID_BITS,
> + depth : DEPTH_BITS;
> unsigned char sha1[20];
> };
uint32_t with bit-width specifiers look somewhat funny here...
>
> @@ -1105,7 +1108,7 @@ static int store_object(
> unsigned pos = sizeof(hdr) - 1;
>
> delta_count_by_type[type]++;
> - last->depth++;
> + e->depth = ++last->depth++;
"lvalue required as increment operand"?
Wouldn't it be easier to read like this?
diff --git a/fast-import.c b/fast-import.c
index 7544949..d32c412 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -161,7 +161,7 @@ struct object_entry
{
struct object_entry *next;
uint32_t offset;
- uint32_t type : TYPE_BITS,
+ unsigned type : TYPE_BITS,
pack_id : PACK_ID_BITS,
depth : DEPTH_BITS;
unsigned char sha1[20];
@@ -1108,7 +1108,7 @@ static int store_object(
unsigned pos = sizeof(hdr) - 1;
delta_count_by_type[type]++;
- e->depth = ++last->depth++;
+ e->depth = last->depth + 1;
hdrlen = encode_header(OBJ_OFS_DELTA, deltalen, hdr);
write_or_die(pack_data->pack_fd, hdr, hdrlen);
@@ -1121,8 +1121,6 @@ static int store_object(
pack_size += sizeof(hdr) - pos;
} else {
e->depth = 0;
- if (last)
- last->depth = 0;
hdrlen = encode_header(type, datlen, hdr);
write_or_die(pack_data->pack_fd, hdr, hdrlen);
pack_size += hdrlen;
@@ -1138,6 +1136,7 @@ static int store_object(
free(last->data);
last->data = dat;
last->offset = e->offset;
+ last->depth = e->depth;
last->len = datlen;
}
return 0;
^ permalink raw reply related
* Re: [Newbie] How to *actually* get rid of remote tracking branch?
From: Steffen Prohaska @ 2007-11-14 5:48 UTC (permalink / raw)
To: Andreas Ericsson; +Cc: Junio C Hamano, Jakub Narebski, Sergei Organov, git
In-Reply-To: <473A493C.4070902@op5.se>
On Nov 14, 2007, at 2:02 AM, Andreas Ericsson wrote:
>> BTW, when you have this data-flow (probably typical in a shared
>> repository workflow):
>> Remote repository Your repository
>> refs/heads/foo -------(A)------> refs/remotes/origin/foo
>> ^ |
>> | (B)
>> | |
>> | V
>> `-------------(C)---------- refs/heads/foo
>> (A) "git fetch" with remote.origin.fetch configuration
>> set to +refs/heads/*:refs/remotes/origin/* keeps
>> your refs/remotes/origin/foo up-to-date with their
>> refs/heads/foo
>> (B) "git merge origin/foo" while on "foo" branch (there
>> are few shorthands like "git pull origin foo" while
>> on "foo" branch. If you say "git branch --track
>> foo origin/foo", then running "git pull" without
>> parameter while on "foo" branch).
>> (C) "git push origin foo".
>> * Everybody seems to agree that "refs/remotes/origin/foo" is
>> called a "remote tracking branch";
>
> I'd like to insert the significant dash there. remote-tracking vs
> remote tracking. It solves the one ambiguity with it, and would
> finally make it clear and consistent almost however it's used.
I recently tried to explain it in the following way:
"refs/remotes/origin/foo" mirrors the original branch (from the
remote repository) in your local repository. Because it is an
identical copy of the branch in the remote repository it is also
called a "remote-tracking branch" or sometimes just a "remote
branch", although it is stored locally in your repository.
>> * Your refs/heads/foo follows and builds on top of 'foo' branch
>> at the remote. Some people errorneously call it "tracking"
>> but that is because we do not have a good term for it;
>
> "private tracking branch"? Nah...
"refs/heads/foo" is your local branch that you work
on. Typically, it's simply called branch foo. However you
can say "my local branch" foo to emphasis that it's your
branch. This branch is set up in a way to automatically
merge changes from the remote branch it was created
from. It is sometimes said to "track" a remote branch,
however it must not be confused with a "remote-tracking
branch". The terminology used in the documentation is sometimes
confusing. A clear statement is: "My local branch foo
is configured to automatically merge the remote (tracking)
branch origin/foo, with origin configured to point to
git://server/path/to/repo.git."
>> * There is no good name for "refs/heads/foo at the remote". We
>> always say "the remote branch you follow" (or "track").
>
> "remote-repo branch" perhaps? The universe doesn't exactly toll its
> bells in approval at that, but at least it's not ambiguous.
Typically, the original branch in the remote repository and
its local remote-tracking branch point to the same commit;
or you can easy update the local copy with fetch. So, often
they are not distinguished. But you can say "branch foo
from the remote repository git://server/path/to/repo.git"
is mirrored locally as "remote-tracking branch origin/foo" if
you need to be precise.
If you say "I merged origin/foo" it really does not matter
if you mean the branch in the remote repository or its local
remote-tracking branch. In both cases the reader needs context
to understand what you exactly mean. If you want to be precise
here, you can say "I merged commit <sha1>".
A technical note: The '--track' option sets up the local
branch to track the original branch in the remote repository.
The setup is not referring to a local remote-tracking branch
at all. config.<branch>.merge contains the branch as it is
named in the remote repository. git is automatically merging
from the branch in the remote repository. The remote-tracking
branch would not be needed to do this. So I think it's
not a problem that "remote branch foo" sometimes means the
branch in the remote repository and sometimes means its local
remote-tracking branch. What you really mean in both cases
is the original branch in the remote repository. If you want
to explicitly point to a specific commit you need to name it
by other means (sha1) anyway.
> I've seen it written as "remote tracking branch" on irc a few times.
> It causes 10 minutes of confusion where no-one's sure what anyone
> else means. With the dash in there, there's no room for ambiguity.
I believe dash is a good idea.
Steffen
^ permalink raw reply
* Re: [PATCH] Don't allow fast-import tree delta chains to exceed maximum depth
From: Shawn O. Pearce @ 2007-11-14 5:54 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vhcjpnzvq.fsf@gitster.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> wrote:
> "Shawn O. Pearce" <spearce@spearce.org> writes:
> > + uint32_t type : TYPE_BITS,
> > + pack_id : PACK_ID_BITS,
> > + depth : DEPTH_BITS;
>
> uint32_t with bit-width specifiers look somewhat funny here...
Sure. But the prior item is also uint32_t and before that a pointer
so it aligns out nice. And we're using exactly 32 bits in this field.
So it just sort of made sense to me to declare it was a uint32_t.
> > @@ -1105,7 +1108,7 @@ static int store_object(
> > unsigned pos = sizeof(hdr) - 1;
> >
> > delta_count_by_type[type]++;
> > - last->depth++;
> > + e->depth = ++last->depth++;
>
> "lvalue required as increment operand"?
>
> Wouldn't it be easier to read like this?
Yes. Good call. :-)
> diff --git a/fast-import.c b/fast-import.c
> index 7544949..d32c412 100644
> --- a/fast-import.c
> +++ b/fast-import.c
> @@ -161,7 +161,7 @@ struct object_entry
> {
> struct object_entry *next;
> uint32_t offset;
> - uint32_t type : TYPE_BITS,
> + unsigned type : TYPE_BITS,
> pack_id : PACK_ID_BITS,
> depth : DEPTH_BITS;
> unsigned char sha1[20];
> @@ -1108,7 +1108,7 @@ static int store_object(
> unsigned pos = sizeof(hdr) - 1;
>
> delta_count_by_type[type]++;
> - e->depth = ++last->depth++;
> + e->depth = last->depth + 1;
>
> hdrlen = encode_header(OBJ_OFS_DELTA, deltalen, hdr);
> write_or_die(pack_data->pack_fd, hdr, hdrlen);
> @@ -1121,8 +1121,6 @@ static int store_object(
> pack_size += sizeof(hdr) - pos;
> } else {
> e->depth = 0;
> - if (last)
> - last->depth = 0;
> hdrlen = encode_header(type, datlen, hdr);
> write_or_die(pack_data->pack_fd, hdr, hdrlen);
> pack_size += hdrlen;
> @@ -1138,6 +1136,7 @@ static int store_object(
> free(last->data);
> last->data = dat;
> last->offset = e->offset;
> + last->depth = e->depth;
> last->len = datlen;
> }
> return 0;
--
Shawn.
^ permalink raw reply
* Re: Git and Scmbug integration
From: Martin Langhoff @ 2007-11-14 6:08 UTC (permalink / raw)
To: Kristis Makris; +Cc: David Symonds, jnareb, git, scmbug-users
In-Reply-To: <1194998142.4106.24.camel@localhost>
On Nov 14, 2007 12:55 PM, Kristis Makris <kristis.makris@asu.edu> wrote:
> It isn't a centralized bug-tracking system necessarily. Because
> different developers may used different, custom bug-tracking systems,
> with custom hooks in their own local Git repositories that integrate
> with only their own bug-tracking systems. And perhaps we can add the
> support in the Scmbug Git frontend to integrate with a centralized
> bug-tracker only on push operations if desired.
I disagree somewhat here. In git, local commits are extremely
lightweight, and as a developer I don't want anything remarkable to
happen on those, even locally. It's pushing (which is actually
publishing!) that makes those commits relevant.
Even if I have a local or distributed bugtracker, any purely local
commit is "draft".
And this is regardless of centralised or distributed -- that's a
matter of policy around the repo I'm pushing to. The distinction that
matters is local vs published. Local commits get removed, rebased,
redone, discarded a whole lot.
> But we can't explore any of these issues, discussed in the thread below
> too, unless we can extract what's needed from the hooks.
I concur with the chorus that chants "HEAD"... try with `git show
HEAD` for starters...
cheers,
martin
^ permalink raw reply
* Re: [PATCH v2 3/3] send-pack: assign remote errors to each ref
From: Junio C Hamano @ 2007-11-14 6:12 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Jeff King, git, Pierre Habouzit, Daniel Barkalow, Alex Riesen
In-Reply-To: <Pine.LNX.4.64.0711140159550.4362@racer.site>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> > 2. the status parsing is not foolproof. We get a line like
>> >
>> > ng refs/heads/master arbitrary msg
>> >
>> > which cannot be parsed unambiguously in the face of
>> > refnames with spaces.
>
> Since when can refnames contain spaces? In my copy of git, bad_ref_char()
> in refs.c returns 1 if ch <= ' '. It's the first error path, even.
Bah. You are right.
^ permalink raw reply
* Re: [PATCH] Improved and extended t5404
From: Alex Riesen @ 2007-11-14 7:19 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jeff King
In-Reply-To: <7vmythr8xf.fsf@gitster.siamese.dyndns.org>
Junio C Hamano, Wed, Nov 14, 2007 01:02:20 +0100:
> Alex Riesen <raa.lkml@gmail.com> writes:
>
> > Ignore exit code of git push in t5404, as it is not relevant for the
> > test
>
> This proposed log message solicits a "Huh? -- Since when
> ignoring exit code is an improvement?" reaction. If this push
> is expected to error out, then wouldn't you want to make sure it
> errors out as expected? If the problem is that the exit status
> is unreliable, maybe we need to make it reliable instead?
Well, it is kind of undefined. git push just updated some remote
references and failed on the others. It has had some failures, so it
returns non-0. And as I said, it really is not about the operation,
but about if the tracking and remote branches are set as we want them.
^ permalink raw reply
* Re: [PATCH] user-manual.txt: fix a few mistakes
From: Junio C Hamano @ 2007-11-14 7:19 UTC (permalink / raw)
To: Sergei Organov; +Cc: git, J Bruce Fields
In-Reply-To: <87bq9x7w4d.fsf@osv.gnss.ru>
Sergei Organov <osv@javad.com> writes:
> Signed-off-by: Sergei Organov <osv@javad.com>
> ---
> Documentation/user-manual.txt | 8 ++++----
> 1 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt
> index d99adc6..a169ef0 100644
> --- a/Documentation/user-manual.txt
> +++ b/Documentation/user-manual.txt
> @@ -475,7 +475,7 @@ Bisecting: 3537 revisions left to test after this
> If you run "git branch" at this point, you'll see that git has
> temporarily moved you to a new branch named "bisect". This branch
> points to a commit (with commit id 65934...) that is reachable from
> -v2.6.19 but not from v2.6.18. Compile and test it, and see whether
> +"master" but not from v2.6.18. Compile and test it, and see whether
> it crashes. Assume it does crash. Then:
>
> -------------------------------------------------
Thanks.
This hunk and the last hunk I do not have any problem with.
> @@ -1367,7 +1367,7 @@ If you make a commit that you later wish you hadn't, there are two
> fundamentally different ways to fix the problem:
>
> 1. You can create a new commit that undoes whatever was done
> - by the previous commit. This is the correct thing if your
> + by the old commit. This is the correct thing if your
> mistake has already been made public.
>
> 2. You can go back and modify the old commit. You should
But is this an improvement or just a churn?
> @@ -1567,8 +1567,8 @@ old history using, for example,
> $ git log master@{1}
> -------------------------------------------------
>
> -This lists the commits reachable from the previous version of the head.
> -This syntax can be used to with any git command that accepts a commit,
> +This lists the commits reachable from the previous version of the branch.
> +This syntax can be used with any git command that accepts a commit,
> not just with git log. Some other examples:
>
> -------------------------------------------------
^ permalink raw reply
* [PATCH] git-clean: consider core.excludesfile
From: shunichi fuji @ 2007-11-14 7:40 UTC (permalink / raw)
To: git
git-clean used "git ls-files" and "git ls-files" don't consider
core.excludesfile.
i add few lines.
--- /usr/bin/git-clean 2007-11-14 08:26:20.000000000 +0900
+++ git-clean 2007-11-14 09:43:03.000000000 +0900
@@ -81,9 +81,14 @@
if [ "$ignoredonly" ]; then
excl="$excl --ignored"
fi
+ core_excl="`git-config core.excludesfile`"
+ if [ -f "$core_excl" ]; then
+ core_excl_info="--exclude-from=$core_excl"
+ fi
fi
-git ls-files --others --directory $excl ${excl_info:+"$excl_info"} -- "$@" |
+git ls-files --others --directory $excl ${excl_info:+"$excl_info"} \
+${core_excl_info:+"$core_excl_info"} -- "$@" |
while read -r file; do
if [ -d "$file" -a ! -L "$file" ]; then
if [ -z "$cleandir" ]; then
^ permalink raw reply
* [PATCH] Grammar fixes for gitattributes documentation
From: Wincent Colaiuta @ 2007-11-14 7:51 UTC (permalink / raw)
To: Git Mailing List; +Cc: Junio Hamano
Tweak the "filter" section of the gitattributes documentation to add
some
missing articles and improve some word choices without changing the
semantics of the section.
Signed-off-by: Wincent Colaiuta <win@wincent.com>
---
Documentation/gitattributes.txt | 17 +++++++++--------
1 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/Documentation/gitattributes.txt b/Documentation/
gitattributes.txt
index 20cf8ff..b01786b 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -148,22 +148,23 @@ with `$Id$` upon check-in.
`filter`
^^^^^^^^
-A `filter` attribute can be set to a string value. This names
+A `filter` attribute can be set to a string value which names a
filter driver specified in the configuration.
-A filter driver consists of `clean` command and `smudge`
+A filter driver consists of a `clean` command and a `smudge`
command, either of which can be left unspecified. Upon
-checkout, when `smudge` command is specified, the command is fed
-the blob object from its standard input, and its standard output
-is used to update the worktree file. Similarly, `clean` command
-is used to convert the contents of worktree file upon checkin.
+checkout, when the `smudge` command is specified, the command is
+fed the blob object from its standard input, and its standard
+output is used to update the worktree file. Similarly, the
+`clean` command is used to convert the contents of worktree file
+upon checkin.
-Missing filter driver definition in the config is not an error
+A missing filter driver definition in the config is not an error
but makes the filter a no-op passthru.
The content filtering is done to massage the content into a
shape that is more convenient for the platform, filesystem, and
-the user to use. The keyword here is "more convenient" and not
+the user to use. The key phrase here is "more convenient" and not
"turning something unusable into usable". In other words, the
intent is that if someone unsets the filter driver definition,
or does not have the appropriate filter program, the project
--
1.5.3.5
^ permalink raw reply related
* Re: git-clean won't read global ignore
From: Junio C Hamano @ 2007-11-14 8:05 UTC (permalink / raw)
To: Pierre Habouzit; +Cc: shunichi fuji, git
In-Reply-To: <20071113225057.GB22836@artemis.corp>
I think the problem is core.excludesfile is too new to be
noticed by anything other than git-add and git-status.
* git-add and git-status know about it because they call
add_excludes_from_file() directly with their own notion of
which standard set of ignore files to use. This is just a
stupid duplication of code that need to be updated every time
the definition of the standard set of ignore files is
changed.
* git-ls-files does not notice any of the "ignore" files by
default, as it predates the standardized set of ignore
files. The calling scripts established the convention to use
.git/info/exclude, .gitignore in each file, and later
core.excludesfile.
* git-read-tree takes --exclude-per-directory=<gitignore>,
not because the flexibility was needed. Again, this was
because the option predates the standardization of the ignore
files.
* git-merge-recursive uses hardcoded per-directory .gitignore
and nothing else. git-clean (scripted version) does not
honor core.* because its call to underlying ls-files does not
know about it. git-clean in C (parked in 'pu') doesn't either.
We probably could change git-ls-files to use the standard set
when no excludes are specified from the command line, or
something like that, but this will be a change in semantics that
would affect the scripts in a subtle way. I am somewhat
reluctant to make such a change.
On the other hand, I think it makes perfect sense to fix
git-read-tree, git-merge-recursive and git-clean to follow the
same rule as other commands. I do not think of a valid use case
to give an exclude-per-directory that is nonstandard to
read-tree command, outside a "negative" test in the t1004 test
script.
To untangle this mess, I think the first step would be something
like this (this is against 'maint', as I was in the middle of
something else that is based on 'maint' when I started reading
this thread).
The next step would be to teach read-tree, merge-recursive and
clean (in C) to use setup_standard_excludes().
---
builtin-add.c | 22 ++--------------------
cache.h | 1 +
config.c | 7 +++++++
dir.c | 12 ++++++++++++
dir.h | 1 +
environment.c | 1 +
wt-status.c | 15 +--------------
7 files changed, 25 insertions(+), 34 deletions(-)
diff --git a/builtin-add.c b/builtin-add.c
index 373f87f..850e1c2 100644
--- a/builtin-add.c
+++ b/builtin-add.c
@@ -17,7 +17,6 @@ static const char builtin_add_usage[] =
"git-add [-n] [-v] [-f] [--interactive | -i] [-u] [--refresh] [--] <filepattern>...";
static int take_worktree_changes;
-static const char *excludes_file;
static void prune_directory(struct dir_struct *dir, const char **pathspec, int prefix)
{
@@ -57,12 +56,7 @@ static void fill_directory(struct dir_struct *dir, const char **pathspec,
memset(dir, 0, sizeof(*dir));
if (!ignored_too) {
dir->collect_ignored = 1;
- dir->exclude_per_dir = ".gitignore";
- path = git_path("info/exclude");
- if (!access(path, R_OK))
- add_excludes_from_file(dir, path);
- if (excludes_file != NULL && !access(excludes_file, R_OK))
- add_excludes_from_file(dir, excludes_file);
+ setup_standard_excludes(dir);
}
/*
@@ -144,18 +138,6 @@ static void refresh(int verbose, const char **pathspec)
free(seen);
}
-static int git_add_config(const char *var, const char *value)
-{
- if (!strcmp(var, "core.excludesfile")) {
- if (!value)
- die("core.excludesfile without value");
- excludes_file = xstrdup(value);
- return 0;
- }
-
- return git_default_config(var, value);
-}
-
static struct lock_file lock_file;
static const char ignore_error[] =
@@ -183,7 +165,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
exit(1);
}
- git_config(git_add_config);
+ git_config(git_default_config);
newfd = hold_locked_index(&lock_file, 1);
diff --git a/cache.h b/cache.h
index fc195bc..ecd809d 100644
--- a/cache.h
+++ b/cache.h
@@ -571,6 +571,7 @@ extern int pager_in_use;
extern int pager_use_color;
extern char *editor_program;
+extern char *excludes_file;
/* base85 */
int decode_85(char *dst, const char *line, int linelen);
diff --git a/config.c b/config.c
index dc3148d..56e99fc 100644
--- a/config.c
+++ b/config.c
@@ -431,6 +431,13 @@ int git_default_config(const char *var, const char *value)
return 0;
}
+ if (!strcmp(var, "core.excludesfile")) {
+ if (!value)
+ die("core.excludesfile without value");
+ excludes_file = xstrdup(value);
+ return 0;
+ }
+
/* Add other config variables here and to Documentation/config.txt. */
return 0;
}
diff --git a/dir.c b/dir.c
index f843c4d..73a39ed 100644
--- a/dir.c
+++ b/dir.c
@@ -709,3 +709,15 @@ int is_inside_dir(const char *dir)
char buffer[PATH_MAX];
return get_relative_cwd(buffer, sizeof(buffer), dir) != NULL;
}
+
+void setup_standard_excludes(struct dir_struct *dir)
+{
+ const char *path;
+
+ dir->exclude_per_dir = ".gitignore";
+ path = git_path("info/exclude");
+ if (!access(path, R_OK))
+ add_excludes_from_file(dir, path);
+ if (excludes_file && !access(excludes_file, R_OK))
+ add_excludes_from_file(dir, excludes_file);
+}
diff --git a/dir.h b/dir.h
index f55a87b..a5f4237 100644
--- a/dir.h
+++ b/dir.h
@@ -63,5 +63,6 @@ extern struct dir_entry *dir_add_name(struct dir_struct *dir, const char *pathna
extern char *get_relative_cwd(char *buffer, int size, const char *dir);
extern int is_inside_dir(const char *dir);
+extern void setup_standard_excludes(struct dir_struct *dir);
#endif
diff --git a/environment.c b/environment.c
index b5a6c69..1dab72e 100644
--- a/environment.c
+++ b/environment.c
@@ -34,6 +34,7 @@ char *pager_program;
int pager_in_use;
int pager_use_color = 1;
char *editor_program;
+char *excludes_file;
int auto_crlf = 0; /* 1: both ways, -1: only when adding git objects */
/* This is set by setup_git_dir_gently() and/or git_default_config() */
diff --git a/wt-status.c b/wt-status.c
index 10ce6ee..58dd716 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -22,7 +22,6 @@ static const char use_add_rm_msg[] =
"use \"git add/rm <file>...\" to update what will be committed";
static const char use_add_to_include_msg[] =
"use \"git add <file>...\" to include in what will be committed";
-static const char *excludes_file;
static int parse_status_slot(const char *var, int offset)
{
@@ -247,22 +246,16 @@ static void wt_status_print_changed(struct wt_status *s)
static void wt_status_print_untracked(struct wt_status *s)
{
struct dir_struct dir;
- const char *x;
int i;
int shown_header = 0;
memset(&dir, 0, sizeof(dir));
- dir.exclude_per_dir = ".gitignore";
if (!s->untracked) {
dir.show_other_directories = 1;
dir.hide_empty_directories = 1;
}
- x = git_path("info/exclude");
- if (file_exists(x))
- add_excludes_from_file(&dir, x);
- if (excludes_file && file_exists(excludes_file))
- add_excludes_from_file(&dir, excludes_file);
+ setup_standard_excludes(&dir);
read_directory(&dir, ".", "", 0, NULL);
for(i = 0; i < dir.nr; i++) {
@@ -360,11 +353,5 @@ int git_status_config(const char *k, const char *v)
int slot = parse_status_slot(k, 13);
color_parse(v, k, wt_status_colors[slot]);
}
- if (!strcmp(k, "core.excludesfile")) {
- if (!v)
- die("core.excludesfile without value");
- excludes_file = xstrdup(v);
- return 0;
- }
return git_default_config(k, v);
}
^ permalink raw reply related
* Re: [PATCH] Bisect visualize: use "for-each-ref" to list all good refs.
From: Junio C Hamano @ 2007-11-14 8:11 UTC (permalink / raw)
To: Christian Couder; +Cc: git
In-Reply-To: <20071114055036.2c5ae793.chriscool@tuxfamily.org>
Christian Couder <chriscool@tuxfamily.org> writes:
> In bisect_visualize, "cd $GIT_DIR/refs && echo bisect/good-*" was
> still used instead of "git for-each-ref". This patch fix it.
Heh, it still uses
echo "$rev" >"$GIT_DIR/refs/bisect/$tag"
So we are still _very_ much tied to refs on the filesystem not
in packed-refs.
Even worse is that bisect_clean_state uses
rm -fr "$GIT_DIR/refs/bisect"
so if you ever pack your refs in the middle of bisection and
then clean the state, you will still carry the old good set of
commits that are somewhat hard to cull.
^ permalink raw reply
* Re: git-clean won't read global ignore
From: Andreas Ericsson @ 2007-11-14 8:42 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Pierre Habouzit, shunichi fuji, git
In-Reply-To: <7vsl39l0b7.fsf@gitster.siamese.dyndns.org>
Junio C Hamano wrote:
> (this is against 'maint', as I was in the middle of
> something else that is based on 'maint' when I started reading
> this thread).
>
That's probably not a bad idea, as it really is a bug, and one
that can cause data-loss at that.
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
^ permalink raw reply
* Re: [PATCH] Improved and extended t5404
From: Junio C Hamano @ 2007-11-14 8:47 UTC (permalink / raw)
To: Alex Riesen; +Cc: git, Jeff King
In-Reply-To: <20071114071929.GA2942@steel.home>
Alex Riesen <raa.lkml@gmail.com> writes:
> Junio C Hamano, Wed, Nov 14, 2007 01:02:20 +0100:
>> Alex Riesen <raa.lkml@gmail.com> writes:
>>
>> > Ignore exit code of git push in t5404, as it is not relevant for the
>> > test
>>
>> This proposed log message solicits a "Huh? -- Since when
>> ignoring exit code is an improvement?" reaction. If this push
>> is expected to error out, then wouldn't you want to make sure it
>> errors out as expected? If the problem is that the exit status
>> is unreliable, maybe we need to make it reliable instead?
>
> Well, it is kind of undefined. git push just updated some remote
> references and failed on the others. It has had some failures, so it
> returns non-0. And as I said, it really is not about the operation,
> but about if the tracking and remote branches are set as we want them.
Ok, I guess the proposed log message was not clear enough, at
least for me.
Will apply.
^ permalink raw reply
* Re: [PATCH] Fix dependencies of parse-options test program
From: Pierre Habouzit @ 2007-11-14 8:48 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Alex Riesen, git, Junio C Hamano
In-Reply-To: <Pine.LNX.4.64.0711140012210.4362@racer.site>
[-- Attachment #1: Type: text/plain, Size: 1521 bytes --]
On Wed, Nov 14, 2007 at 12:12:49AM +0000, Johannes Schindelin wrote:
> Hi,
>
> On Wed, 14 Nov 2007, Pierre Habouzit wrote:
>
> > On Tue, Nov 13, 2007 at 11:46:20PM +0000, Johannes Schindelin wrote:
> >
> > > apparently I forgot to send this patch, which I thought was only
> > > relevant in the builtin-commit branch of mine:
> >
> > Btw is there a reason why git doesn't use the gcc -M* options to have
> > proper dependency informations ?
>
> I suspect the reason is that we do not want to depend on gcc...
right, but like said, other compilers can generate them, only with a
different CLI switch, and non withstanding this problem, there are tools
to generate such kind of information easily.
Though for those who care about this issue, this is hackable easily
using config.mak, here is how:
CFLAGS += -MMD -MF $(@D)/.$(@F:.o=.dep)
-include $(wildcard .*.dep */.*.dep) GIT-VERSION-FILE
It's a quick hack, but allow gcc + gnu make (I believe $(@D) is GNU)
users not to be bitten hard by those dependency issues.
The reason why I include GIT-VERSION-FILE again is that many make
complain if -include gets an empty like as an argument, and passing
/dev/null on certains occasions triggers warnings about /dev/null having
a date in the future on some machines for a reason I never found.
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: [PATCH] Grammar fixes for gitattributes documentation
From: Junio C Hamano @ 2007-11-14 8:55 UTC (permalink / raw)
To: Wincent Colaiuta; +Cc: Git Mailing List
In-Reply-To: <33080F0B-80C0-4860-9A74-C6878EE3B2CD@wincent.com>
Wincent Colaiuta <win@wincent.com> writes:
> Tweak the "filter" section of the gitattributes documentation to add
> some missing articles and improve some word choices without changing
> the semantics of the section.
>
> Signed-off-by: Wincent Colaiuta <win@wincent.com>
Thanks.
> diff --git a/Documentation/gitattributes.txt b/Documentation/
> gitattributes.txt
> index 20cf8ff..b01786b 100644
> --- a/Documentation/gitattributes.txt
> +++ b/Documentation/gitattributes.txt
> @@ -148,22 +148,23 @@ with `$Id$` upon check-in.
> `filter`
> ^^^^^^^^
>
> -A `filter` attribute can be set to a string value. This names
> +A `filter` attribute can be set to a string value which names a
> filter driver specified in the configuration.
Will we get the canned "which vs that" discussion on this change?
> -A filter driver consists of `clean` command and `smudge`
> +A filter driver consists of a `clean` command and a `smudge`
> command, either of which can be left unspecified. Upon
> -checkout, when `smudge` command is specified, the command is fed
> +checkout, when the `smudge` command is specified, the command is fed
> -the blob object from its standard input, and its standard output
> +the blob object from its standard input, and its standard output
> -is used to update the worktree file. Similarly, `clean` command
> +is used to update the worktree file. Similarly, the `clean` command
> -is used to convert the contents of worktree file upon checkin.
> +is used to convert the contents of worktree file upon checkin.
I do appreciate the grammar fixes, but I needed to re-wrap and
swap lines to see the real change. Especially, after this
re-wrapping, the updated lines with missing "the" fixed still
fit nicely below 70 columns and the right edge is not too ragged
to be distractive even for people who read unformatted text.
Could you please avoid this kind of unnecessary re-wrapping in
the future patches?
^ permalink raw reply
* Re: [PATCH] RESUBMIT: replace reference to git-rm with git-reset in git-commit doc
From: Junio C Hamano @ 2007-11-14 8:56 UTC (permalink / raw)
To: Jing Xue; +Cc: git
In-Reply-To: <20071112044300.GB7595@fawkes>
Jing Xue <jingxue@digizenstudio.com> writes:
> On top of that, I somehow still want to make it relevant to that
> git-reset instead of git-rm should be used to revert git-add. So how
> about this?
Thanks. I'll fix up the log message and with a bit of
rewording.
^ permalink raw reply
* Re: [PATCH] git-clean: consider core.excludesfile
From: Junio C Hamano @ 2007-11-14 8:58 UTC (permalink / raw)
To: shunichi fuji; +Cc: git
In-Reply-To: <30046e3b0711132340y2c503dd4laea24b9b1c79a160@mail.gmail.com>
Thanks. Sign-off please?
^ permalink raw reply
* Re: git-clean won't read global ignore
From: Junio C Hamano @ 2007-11-14 9:03 UTC (permalink / raw)
To: Andreas Ericsson; +Cc: Pierre Habouzit, shunichi fuji, git
In-Reply-To: <473AB508.4090109@op5.se>
Andreas Ericsson <ae@op5.se> writes:
> Junio C Hamano wrote:
>> (this is against 'maint', as I was in the middle of
>> something else that is based on 'maint' when I started reading
>> this thread).
>>
>
> That's probably not a bad idea, as it really is a bug, and one
> that can cause data-loss at that.
I agree that "git clean" needs to be fixed in 'maint', but the
thing is, setup_standard_excludes() approach would not apply to
anything in 'maint', 'master', nor 'next', and it is more of the
longer term thing to go together with the git-clean in C.
The scripted version needs to be fixed independently in a way
with lessor impact, like the patch Shunichi just posted.
^ permalink raw reply
* Re: [PATCH] Grammar fixes for gitattributes documentation
From: Wincent Colaiuta @ 2007-11-14 9:04 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <7vfxz9kxz3.fsf@gitster.siamese.dyndns.org>
El 14/11/2007, a las 9:55, Junio C Hamano escribió:
> Wincent Colaiuta <win@wincent.com> writes:
>
>> -A `filter` attribute can be set to a string value. This names
>> +A `filter` attribute can be set to a string value which names a
>> filter driver specified in the configuration.
>
> Will we get the canned "which vs that" discussion on this change?
Perhaps. Neither would be incorrect, although technically "that" is a
tighter match.
> I do appreciate the grammar fixes, but I needed to re-wrap and
> swap lines to see the real change. Especially, after this
> re-wrapping, the updated lines with missing "the" fixed still
> fit nicely below 70 columns and the right edge is not too ragged
> to be distractive even for people who read unformatted text.
>
> Could you please avoid this kind of unnecessary re-wrapping in
> the future patches?
Ok, sorry about that. I wasn't sure of the maximum allowed length in
the doc files, and the longest line I could find in that file was 67
chars, so I made sure that nothing exceeded that. Will make a note
that the official limit is 70. Perhaps this could be added to the
style document? Will whip up a patch for that.
Cheers,
Wincent
^ 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