* Re: [PATCH 3/3] git-archive: add support for --submodules
From: Johannes Schindelin @ 2009-01-18 15:51 UTC (permalink / raw)
To: Lars Hjemli; +Cc: Junio C Hamano, git
In-Reply-To: <1232275999-14852-4-git-send-email-hjemli@gmail.com>
Hi,
On Sun, 18 Jan 2009, Lars Hjemli wrote:
> @@ -320,6 +323,7 @@ static int parse_archive_args(int argc, const char **argv,
> args->base = base;
> args->baselen = strlen(base);
>
> + set_traverse_gitlinks(submodules);
As I said, this is a per-call thing. So you need to add that option to
the archiver_args struct and use it in write_archive_entries().
Ciao,
Dscho
^ permalink raw reply
* Re: easy way to make tracking branches?
From: Stephan Beyer @ 2009-01-18 15:53 UTC (permalink / raw)
To: Sitaram Chamarty; +Cc: git
In-Reply-To: <slrngn6j4t.9he.sitaramc@sitaramc.homelinux.net>
Hi,
Sitaram Chamarty wrote:
> > Now I want to make "foo" a tracking branch for "bar".
> > I do:
> >
> > git config branch.foo.remote srv
> > git config branch.foo.merge refs/heads/bar
>
> I just do a "git pull srv" when the tracking is *not* setup,
> and git reminds me what commands to use.
Oh, this is great! (Just tested.)
Usually (in my use case) there is no need to pull before I set up
a tracking branch, so I never tried this.
And git only reminds you if you have no tracking branch set for
srv yet. But that's no problem, because if one is set I can
copy&paste the lines in .git/config.
Altogether, this is a solution I can live with.
> > And to get a comfortable git-push, I do:
> >
> > git config --add remote.srv.push foo:bar
>
> This one you'll just have to remember, I guess :-)
This is not too hard. :-)
> > git checkout -b foo2 srv/bar
> > git branch -d foo
> > git branch -m foo
> >
> > which is suboptimal because deleting foo can remove some
> > other settings for the branch, e.g. mergeoptions.
>
> it also doesn't seem to set remote.srv.push, as far as I can
> tell.
Right.
Thanks,
Stephan
--
Stephan Beyer <s-beyer@gmx.net>, PGP 0x6EDDD207FCC5040F
^ permalink raw reply
* [PATCH] sha1_file: add function to insert alternate object db
From: Lars Hjemli @ 2009-01-18 15:55 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.DEB.1.00.0901181629590.3586@pacific.mpi-cbg.de>
This function will be used when implementing traversal into submodules.
Signed-off-by: Lars Hjemli <hjemli@gmail.com>
---
On Sun, Jan 18, 2009 at 16:32, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
>
> On Sun, 18 Jan 2009, Lars Hjemli wrote:
>> +int add_alt_odb(const char *path)
>> +{
>> + return link_alt_odb_entry(path, strlen(path), NULL, 0);
>
> This function can return the error message "object directory %s does not
> exist; check .git/objects/info/alternates." Maybe you want to change
> that, even if the user you are introducing might not hit that code path.
Something like this, maybe?
cache.h | 1 +
sha1_file.c | 17 ++++++++++++-----
2 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/cache.h b/cache.h
index 8e1af26..0dbe2a6 100644
--- a/cache.h
+++ b/cache.h
@@ -724,6 +724,7 @@ extern struct alternate_object_database {
char base[FLEX_ARRAY]; /* more */
} *alt_odb_list;
extern void prepare_alt_odb(void);
+extern int add_alt_odb(const char *path, int quiet);
extern void add_to_alternates_file(const char *reference);
typedef int alt_odb_fn(struct alternate_object_database *, void *);
extern void foreach_alt_odb(alt_odb_fn, void*);
diff --git a/sha1_file.c b/sha1_file.c
index f08493f..4b7e691 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -252,7 +252,8 @@ static void read_info_alternates(const char * alternates, int depth);
* SHA1, an extra slash for the first level indirection, and the
* terminating NUL.
*/
-static int link_alt_odb_entry(const char * entry, int len, const char * relative_base, int depth)
+static int link_alt_odb_entry(const char * entry, int len,
+ const char * relative_base, int depth, int quiet)
{
const char *objdir = get_object_directory();
struct alternate_object_database *ent;
@@ -285,9 +286,10 @@ static int link_alt_odb_entry(const char * entry, int len, const char * relative
/* Detect cases where alternate disappeared */
if (!is_directory(ent->base)) {
- error("object directory %s does not exist; "
- "check .git/objects/info/alternates.",
- ent->base);
+ if (!quiet)
+ error("object directory %s does not exist; "
+ "check .git/objects/info/alternates.",
+ ent->base);
free(ent);
return -1;
}
@@ -347,7 +349,7 @@ static void link_alt_odb_entries(const char *alt, const char *ep, int sep,
relative_base, last);
} else {
link_alt_odb_entry(last, cp - last,
- relative_base, depth);
+ relative_base, depth, 0);
}
}
while (cp < ep && *cp == sep)
@@ -356,6 +358,11 @@ static void link_alt_odb_entries(const char *alt, const char *ep, int sep,
}
}
+int add_alt_odb(const char *path, int quiet)
+{
+ return link_alt_odb_entry(path, strlen(path), NULL, 0, quiet);
+}
+
static void read_info_alternates(const char * relative_base, int depth)
{
char *map;
--
1.6.1.150.g5e733b
^ permalink raw reply related
* meaning of --8<--
From: Markus Heidelberg @ 2009-01-18 15:56 UTC (permalink / raw)
To: git
Hi,
I've seen lines like "--8<--" several times on this list, but have no
clue what it is about. OK, seems like it's used to insert diffs in the
middle of a mail message.
But is this a common convention or git specific and handled by git-am?
Is it documented anywhere?
Markus
^ permalink raw reply
* Re: meaning of --8<--
From: Peter Harris @ 2009-01-18 16:04 UTC (permalink / raw)
To: markus.heidelberg; +Cc: git
In-Reply-To: <200901181656.37813.markus.heidelberg@web.de>
On Sun, Jan 18, 2009 at 10:56 AM, Markus Heidelberg wrote:
> I've seen lines like "--8<--" several times on this list, but have no
> clue what it is about. OK, seems like it's used to insert diffs in the
> middle of a mail message.
> But is this a common convention or git specific and handled by git-am?
> Is it documented anywhere?
Common convention, meaning 'cut here'.
Perforated line ------
Scissors 8<
Also used in print (although usually with picture of scissors instead
of the ASCII-art).
Peter Harris
^ permalink raw reply
* Re: [PATCH 2/3] Teach read_tree_recursive() how to traverse into submodules
From: René Scharfe @ 2009-01-18 16:13 UTC (permalink / raw)
To: Lars Hjemli; +Cc: Junio C Hamano, git
In-Reply-To: <1232275999-14852-3-git-send-email-hjemli@gmail.com>
Lars Hjemli schrieb:
> The traversal of submodules is only triggered if the current submodule
> HEAD commit object is accessible. To this end, read_tree_recursive()
> will try to insert the submodule odb as an alternate odb but the lack
> of such an odb is not treated as an error since it is then assumed that
> the user is not interested in the submodule content. However, if the
> submodule odb is found it is treated as an error if the HEAD commit
> object is missing.
Callers of read_tree_recursive() specify a tree to traverse.
Unconditionally using the HEAD of submodules feels a bit restrictive,
but I don't use submodules, so I have no idea what I'm actually talking
about here. :)
> int read_tree_recursive(struct tree *tree,
> const char *base, int baselen,
> int stage, const char **match,
> @@ -132,6 +188,30 @@ int read_tree_recursive(struct tree *tree,
> return -1;
> continue;
> }
> + if (S_ISGITLINK(entry.mode) && get_traverse_gitlinks()) {
> + int retval;
> + char *newbase;
> + struct tree *subtree;
> + unsigned int pathlen = tree_entry_len(entry.path, entry.sha1);
> +
> + newbase = xmalloc(baselen + 1 + pathlen);
> + memcpy(newbase, base, baselen);
> + memcpy(newbase + baselen, entry.path, pathlen);
> + newbase[baselen + pathlen] = 0;
> + if (!traverse_gitlink(newbase, entry.sha1, &subtree)) {
> + free(newbase);
> + continue;
> + }
> + newbase[baselen + pathlen] = '/';
> + retval = read_tree_recursive(subtree,
> + newbase,
> + baselen + pathlen + 1,
> + stage, match, fn, context);
> + free(newbase);
> + if (retval)
> + return -1;
> + continue;
> + }
> }
> return 0;
> }
You don't need to call get_traverse_gitlinks() in the if statement above
if you make all read_tree_recursive() callback functions return 0 for
gitlinks that they don't want to follow and READ_TREE_RECURSIVE for
those they do. It's cleaner without the static variable and its
accessors and more flexible, too: the callbacks might decide to traverse
only certain submodules.
René
^ permalink raw reply
* Re: meaning of --8<--
From: Jay Soffian @ 2009-01-18 16:13 UTC (permalink / raw)
To: markus.heidelberg; +Cc: git
In-Reply-To: <200901181656.37813.markus.heidelberg@web.de>
On Sun, Jan 18, 2009 at 10:56 AM, Markus Heidelberg
<markus.heidelberg@web.de> wrote:
> Hi,
>
> I've seen lines like "--8<--" several times on this list, but have no
> clue what it is about.
It is supposed to represent a pair of scissors. Sometimes it is the
other direction: >8. And I've seen folks botch it as: <8 or 8>. This
list is the first (and maybe only?) place I've seen it. Typically you
might see:
--snip--
or
--cut--
> But is this a common convention or git specific and handled by git-am?
Seems to be a convention of this list, and git-am does nothing with it afaik.
j.
^ permalink raw reply
* Re: [PATCH/RFC] git-am: Make it easier to see which patch failed
From: Johannes Schindelin @ 2009-01-18 16:17 UTC (permalink / raw)
To: Jeff King; +Cc: Stephan Beyer, Junio C Hamano, Jonas Flodén, git
In-Reply-To: <20090118153928.GA16664@coredump.intra.peff.net>
Hi,
On Sun, 18 Jan 2009, Jeff King wrote:
> On Sun, Jan 18, 2009 at 10:41:13AM +0100, Stephan Beyer wrote:
>
> > > Looks sane except that I do not think you need printf nor the leading
> > > blank line, i.e.
> > >
> > > echo "Patch failed at $msgnum ($FIRSTLINE)"
> >
> > Hmm, IIRC if $FIRSTLINE contains \n or something like that, it will
> > interpret this as newline in some shell/echo implementations.
> >
> > So printf "...%s..." "$FOO" is always sane for user input.
>
> Yes, I'm surprised Junio doesn't remember the mass conversions we
> already had to do (4b7cc26a and 293623ed). But looking at the date, I
> guess it _has_ been a year and a half. :)
Hey, be nice to Junio. Have you seen the amount of mails on this list
recently? I think Junio's the only one really reading all of them; even
if you were right, he would be entitled to a nicer reminder.
But you are wrong. And Stephan is wrong, too.
The name "FIRSTLINE" suggests that it is indeed a first line, and
consequently cannot contain a newline.
And indeed, it is defined as
FIRSTLINE=$(sed 1q "$dotest/final-commit")
Just do the following in any of your favorite shells:
$ FIRSTLINE=$(sed 1q README)
$ echo "$FIRSTLINE."
You'll find that the "." is not in a new line.
And I know that we relied on that behavior for an eternity.
So there is certainly no need for a printf here.
'nuff said,
Dscho
^ permalink raw reply
* Re: [TOY PATCH] git-resurrect: find traces of a branch name and resurrect it
From: Johannes Schindelin @ 2009-01-18 16:19 UTC (permalink / raw)
To: Thomas Rast; +Cc: git, Junio C Hamano
In-Reply-To: <1232242703-19086-1-git-send-email-trast@student.ethz.ch>
Hi,
On Sun, 18 Jan 2009, Thomas Rast wrote:
> Makefile | 1 +
> git-resurrect.sh | 109 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
Maybe have it in contrib/ instead?
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 2/3] Teach read_tree_recursive() how to traverse into submodules
From: Lars Hjemli @ 2009-01-18 16:37 UTC (permalink / raw)
To: René Scharfe; +Cc: Junio C Hamano, git
In-Reply-To: <49735530.4090901@lsrfire.ath.cx>
On Sun, Jan 18, 2009 at 17:13, René Scharfe <rene.scharfe@lsrfire.ath.cx> wrote:
> Lars Hjemli schrieb:
>> The traversal of submodules is only triggered if the current submodule
>> HEAD commit object is accessible. To this end, read_tree_recursive()
>> will try to insert the submodule odb as an alternate odb but the lack
>> of such an odb is not treated as an error since it is then assumed that
>> the user is not interested in the submodule content. However, if the
>> submodule odb is found it is treated as an error if the HEAD commit
>> object is missing.
>
> Callers of read_tree_recursive() specify a tree to traverse.
> Unconditionally using the HEAD of submodules feels a bit restrictive,
> but I don't use submodules, so I have no idea what I'm actually talking
> about here. :)
For bare repositories (where the submodule repo is added to
objects/info/alternates), following the tree of the linked commit is
the only option. And for non-bare repositories with the submodule
checked out, I think we should honor the users choice of checked out
HEAD in the submodule (especially since we don't have any other way to
specify which submodule commit to follow).
>
>> int read_tree_recursive(struct tree *tree,
>> const char *base, int baselen,
>> int stage, const char **match,
>> @@ -132,6 +188,30 @@ int read_tree_recursive(struct tree *tree,
>> return -1;
>> continue;
>> }
>> + if (S_ISGITLINK(entry.mode) && get_traverse_gitlinks()) {
>> + int retval;
>> + char *newbase;
>> + struct tree *subtree;
>> + unsigned int pathlen = tree_entry_len(entry.path, entry.sha1);
>> +
>> + newbase = xmalloc(baselen + 1 + pathlen);
>> + memcpy(newbase, base, baselen);
>> + memcpy(newbase + baselen, entry.path, pathlen);
>> + newbase[baselen + pathlen] = 0;
>> + if (!traverse_gitlink(newbase, entry.sha1, &subtree)) {
>> + free(newbase);
>> + continue;
>> + }
>> + newbase[baselen + pathlen] = '/';
>> + retval = read_tree_recursive(subtree,
>> + newbase,
>> + baselen + pathlen + 1,
>> + stage, match, fn, context);
>> + free(newbase);
>> + if (retval)
>> + return -1;
>> + continue;
>> + }
>> }
>> return 0;
>> }
>
> You don't need to call get_traverse_gitlinks() in the if statement above
> if you make all read_tree_recursive() callback functions return 0 for
> gitlinks that they don't want to follow and READ_TREE_RECURSIVE for
> those they do. It's cleaner without the static variable and its
> accessors and more flexible, too: the callbacks might decide to traverse
> only certain submodules.
I like the idea, but it will require thorough review of all
read_tree_recursive() consumers. So now we've got three different
approaches:
* me: global setting
* dscho: parameter to read_tree_recursive()
* you: accept the return value from the callback function
Junio, what would you prefer?
--
larsh
^ permalink raw reply
* Re: [PATCH/RFC] git-am: Make it easier to see which patch failed
From: Thomas Rast @ 2009-01-18 16:49 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Jeff King, Stephan Beyer, Junio C Hamano, Jonas Flodén, git
In-Reply-To: <alpine.DEB.1.00.0901181711090.3586@pacific.mpi-cbg.de>
[-- Attachment #1: Type: text/plain, Size: 995 bytes --]
Johannes Schindelin wrote:
> On Sun, 18 Jan 2009, Jeff King wrote:
> > On Sun, Jan 18, 2009 at 10:41:13AM +0100, Stephan Beyer wrote:
> > > Hmm, IIRC if $FIRSTLINE contains \n or something like that, it will
> > > interpret this as newline in some shell/echo implementations.
> > >
> > > So printf "...%s..." "$FOO" is always sane for user input.
>
> But you are wrong. And Stephan is wrong, too.
>
> The name "FIRSTLINE" suggests that it is indeed a first line, and
> consequently cannot contain a newline.
I think the point was that $FIRSTLINE can contain a backslash sequence
such as (literally) \n or \r. Indeed 'man 1p echo' on my system says
_string_ A string to be written to standard output. If the first
operand is -n, or if any of the operands contain a
backslash ( '\' ) character, the results are
implementation- defined.
(Those POSIX manpages are really useful!)
--
Thomas Rast
trast@{inf,student}.ethz.ch
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: [PATCH/RFC] git-am: Make it easier to see which patch failed
From: Stephan Beyer @ 2009-01-18 16:53 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Jeff King, Junio C Hamano, Jonas Flodén, git
In-Reply-To: <alpine.DEB.1.00.0901181711090.3586@pacific.mpi-cbg.de>
Hi,
Johannes Schindelin wrote:
> On Sun, 18 Jan 2009, Jeff King wrote:
>
> > On Sun, Jan 18, 2009 at 10:41:13AM +0100, Stephan Beyer wrote:
> >
> > > > Looks sane except that I do not think you need printf nor the leading
> > > > blank line, i.e.
> > > >
> > > > echo "Patch failed at $msgnum ($FIRSTLINE)"
> > >
> > > Hmm, IIRC if $FIRSTLINE contains \n or something like that, it will
> > > interpret this as newline in some shell/echo implementations.
> > >
> > > So printf "...%s..." "$FOO" is always sane for user input.
> >
> > Yes, I'm surprised Junio doesn't remember the mass conversions we
> > already had to do (4b7cc26a and 293623ed). But looking at the date, I
> > guess it _has_ been a year and a half. :)
>
> Hey, be nice to Junio. Have you seen the amount of mails on this list
> recently? I think Junio's the only one really reading all of them; even
> if you were right, he would be entitled to a nicer reminder.
I had almost written the same text but then I thought Jeff did not mean
it bad, he was just surprised.
> But you are wrong. And Stephan is wrong, too.
>
> The name "FIRSTLINE" suggests that it is indeed a first line, and
> consequently cannot contain a newline.
>
> And indeed, it is defined as
>
> FIRSTLINE=$(sed 1q "$dotest/final-commit")
>
> Just do the following in any of your favorite shells:
>
> $ FIRSTLINE=$(sed 1q README)
> $ echo "$FIRSTLINE."
>
> You'll find that the "." is not in a new line.
I have to disagree:
$ cat newline
foo\nbar
$ FIRSTLINE=$(sed 1q newline)
$ echo "$FIRSTLINE."
foo
bar.
$ exit
> And I know that we relied on that behavior for an eternity.
Where? We should perhaps fix it then.
Regards,
Stephan
--
Stephan Beyer <s-beyer@gmx.net>, PGP 0x6EDDD207FCC5040F
^ permalink raw reply
* Re: fatal: git grep: cannot generate relative filenames containing '..'
From: George Spelvin @ 2009-01-18 16:58 UTC (permalink / raw)
To: gitster, azure; +Cc: linux, git
In-Reply-To: <83vdsefz9j.fsf@kalahari.s2.org>
Hannu Koivisto <azure@iki.fi> wrote:
> It turns out that this "entire git tree" is practically my only use
> case when using normal (find and) grep and when I first tried git
> grep, I actually expected it to do just that with no path specified.
>
> Why? Because of the way at least git diff and git log work. I
> thought that just like with them, "git grep foo" would search the
> entire git tree and I would have to say "git grep foo ." to limit to
> the current directory and its subdirectories. git-grep(1)'s "Look
> for specified patterns in the working tree files..." at least
> didn't seem to disagree with my expectation so I was a bit puzzled.
>
> So I'd rather see git grep behave in a way consistent with git log
> and git diff (I realize that would change current behaviour instead
> of extending it).
D'oh. You're completely right. Space-dot is trivial to type if you
want the current directory only, and that is more consistent.
Could that be considered for 1.7?
^ permalink raw reply
* Re: [PATCH/RFC] git-am: Make it easier to see which patch failed
From: Jeff King @ 2009-01-18 17:07 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Stephan Beyer, Junio C Hamano, Jonas Flodén, git
In-Reply-To: <alpine.DEB.1.00.0901181711090.3586@pacific.mpi-cbg.de>
On Sun, Jan 18, 2009 at 05:17:43PM +0100, Johannes Schindelin wrote:
> > Yes, I'm surprised Junio doesn't remember the mass conversions we
> > already had to do (4b7cc26a and 293623ed). But looking at the date, I
> > guess it _has_ been a year and a half. :)
>
> Hey, be nice to Junio. Have you seen the amount of mails on this list
> recently? I think Junio's the only one really reading all of them; even
> if you were right, he would be entitled to a nicer reminder.
I didn't mean to be mean. On the contrary, I was surprised because _he_
usually is the one reminding _me_ about such fixes. I guess Junio is
human, after all. :)
> But you are wrong. And Stephan is wrong, too.
>
> The name "FIRSTLINE" suggests that it is indeed a first line, and
> consequently cannot contain a newline.
It is not "this is a problem because it might contain a newline" but "this
is a problem because it might contain an escape sequence, _an example_
of which is a \n newline." So the question is whether you can guarantee
that $FIRSTLINE does not contain a backslash. Which I don't think is the
case here.
-Peff
^ permalink raw reply
* Re: [PATCH 1/2] color: make it easier for non-config to parse color specs
From: René Scharfe @ 2009-01-18 17:10 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Markus Heidelberg, git
In-Reply-To: <20090117153229.GA27071@coredump.intra.peff.net>
Jeff King schrieb:
> We have very featureful color-parsing routines which are
> used for color.diff.* and other options. Let's make it
> easier to use those routines from other parts of the code.
>
> This patch adds a color_parse_mem() helper function which
> takes a length-bounded string instead of a NUL-terminated
> one. While the helper is only a few lines long, it is nice
> to abstract this out so that:
>
> - callers don't forget to free() the temporary buffer
>
> - right now, it is implemented in terms of color_parse().
> But it would be more efficient to reverse this and
> implement color_parse in terms of color_parse_mem.
Thusly?
diff --git a/color.c b/color.c
index fc0b72a..14eac93 100644
--- a/color.c
+++ b/color.c
@@ -41,6 +41,11 @@ static int parse_attr(const char *name, int len)
void color_parse(const char *value, const char *var, char *dst)
{
+ color_parse_mem(value, strlen(value), var, dst);
+}
+
+void color_parse_mem(const char *value, int len, const char *var, char *dst)
+{
const char *ptr = value;
int attr = -1;
int fg = -2;
@@ -52,18 +57,22 @@ void color_parse(const char *value, const char *var, char *dst)
}
/* [fg [bg]] [attr] */
- while (*ptr) {
+ while (len > 0) {
const char *word = ptr;
- int val, len = 0;
+ int val, wordlen = 0;
- while (word[len] && !isspace(word[len]))
- len++;
+ while (len > 0 && !isspace(word[wordlen])) {
+ wordlen++;
+ len--;
+ }
- ptr = word + len;
- while (*ptr && isspace(*ptr))
+ ptr = word + wordlen;
+ while (len > 0 && isspace(*ptr)) {
ptr++;
+ len--;
+ }
- val = parse_color(word, len);
+ val = parse_color(word, wordlen);
if (val >= -1) {
if (fg == -2) {
fg = val;
@@ -75,7 +84,7 @@ void color_parse(const char *value, const char *var, char *dst)
}
goto bad;
}
- val = parse_attr(word, len);
+ val = parse_attr(word, wordlen);
if (val < 0 || attr != -1)
goto bad;
attr = val;
^ permalink raw reply related
* Re: [PATCH 2/2] expand --pretty=format color options
From: René Scharfe @ 2009-01-18 17:13 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Markus Heidelberg, git
In-Reply-To: <20090117153846.GB27071@coredump.intra.peff.net>
Jeff King schrieb:
> Currently, the only colors available to --pretty=format
> users are red, green, and blue. Rather than expand it with a
> few new colors, this patch makes the usual config color
> syntax available, including more colors, backgrounds, and
> attributes.
>
> Because colors are no longer bounded to a single word (e.g.,
> %Cred), this uses a more advanced syntax that features a
> beginning and end delimiter (but the old syntax still
> works). So you can now do:
>
> git log --pretty=tformat:'%C(yellow)%h%C(reset) %s'
>
> to emulate --pretty=oneline, or even
>
> git log --pretty=tformat:'%C(cyan magenta bold)%s%C(reset)'
>
> if you want to relive the awesomeness of 4-color CGA.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> René: I saw you mention an end goal of implementing the other --pretty
> formats in terms of --pretty=format:. So maybe this will help just a
> little.
Nice!
Another step would be for --pretty=format to respect the color config,
i.e. it shouldn't print any colour codes if colouring is turned off or
if set to auto while writing to file or pipe.
René
^ permalink raw reply
* [PATCH] builtin-fsck: fix off by one head count
From: Christian Couder @ 2009-01-18 17:27 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
According to the man page, if "git fsck" is passed one or more
heads, it should verify connectivity and validity of only objects
reachable from the heads it is passed.
But before this patch, when it was passed only one head, then it
behaved as if no heads were passed (that is it used the index file,
all references in $GIT_DIR/refs/* and all reflogs). In fact it just
ignored the first head it was passed.
It looks like this bug was introduced when "builtin-fsck.c" was
changed to use "parse_options" (commit
5ac0a2063e8f824f6e8ffb4d18de74c55aae7131).
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin-fsck.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
Sorry if you receive this patch more than once, but it
seems that something did not work when I tried to send
it to the mailing list before.
diff --git a/builtin-fsck.c b/builtin-fsck.c
index 5a08ec1..fb35554 100644
--- a/builtin-fsck.c
+++ b/builtin-fsck.c
@@ -629,7 +629,7 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)
}
heads = 0;
- for (i = 1; i < argc; i++) {
+ for (i = 0; i < argc; i++) {
const char *arg = argv[i];
if (!get_sha1(arg, head_sha1)) {
struct object *obj = lookup_object(head_sha1);
--
1.6.1.85.g8ffc
^ permalink raw reply related
* Re: [PATCH 1/2] color: make it easier for non-config to parse color specs
From: Jeff King @ 2009-01-18 17:28 UTC (permalink / raw)
To: René Scharfe; +Cc: Junio C Hamano, Markus Heidelberg, git
In-Reply-To: <4973628C.8080501@lsrfire.ath.cx>
On Sun, Jan 18, 2009 at 06:10:36PM +0100, René Scharfe wrote:
> > - right now, it is implemented in terms of color_parse().
> > But it would be more efficient to reverse this and
> > implement color_parse in terms of color_parse_mem.
>
> Thusly?
Yes, except for the bugs you introduced. :)
> +void color_parse_mem(const char *value, int len, const char *var, char *dst)
> +{
> const char *ptr = value;
> int attr = -1;
> int fg = -2;
What's missing in the context here (because it wasn't changed) is:
> if (!strcasecmp(value, "reset")) {
> strcpy(dst, "\033[m");
> return;
> }
which doesn't work, since our string is actually something like
"reset)\0" or even "reset)some totally unrelated string". So we would
need a "memcasecmp" here.
And then in the error case, we call:
> die("bad color value '%s' for variable '%s'", value, var);
which is also bogus.
I don't know if this is really even worth it. The timing difference is
pretty minimal:
$ time ./git log --pretty=tformat:'%Credfoo%Creset' >/dev/null
real 0m0.673s
user 0m0.652s
sys 0m0.016s
$ time ./git log --pretty=tformat:'%C(red)foo%C(reset)' >/dev/null
real 0m0.692s
user 0m0.660s
sys 0m0.032s
That's about 1 microsecond per commit.
-Peff
^ permalink raw reply
* Re: [PATCH 1/2] color: make it easier for non-config to parse color specs
From: Jeff King @ 2009-01-18 17:36 UTC (permalink / raw)
To: René Scharfe; +Cc: Junio C Hamano, Markus Heidelberg, git
In-Reply-To: <20090118172802.GA17434@coredump.intra.peff.net>
On Sun, Jan 18, 2009 at 12:28:02PM -0500, Jeff King wrote:
> I don't know if this is really even worth it. The timing difference is
> pretty minimal:
BTW, if we really care about every inch of performance in
--pretty=format:, it would probably make sense to pre-parse the string
into a mini-bytecode. So any complex parsing or lookup that is not
dependent on the commit itself can be done once, instead of per-commit.
I don't think it would make a huge performance difference now, but there
has been talk of more complex substitution syntax (and this %C() is an
example), which would probably benefit.
-Peff
^ permalink raw reply
* Re: [PATCH 2/2] expand --pretty=format color options
From: Jeff King @ 2009-01-18 17:37 UTC (permalink / raw)
To: René Scharfe; +Cc: Junio C Hamano, Markus Heidelberg, git
In-Reply-To: <49736331.8010003@lsrfire.ath.cx>
On Sun, Jan 18, 2009 at 06:13:21PM +0100, René Scharfe wrote:
> Another step would be for --pretty=format to respect the color config,
> i.e. it shouldn't print any colour codes if colouring is turned off or
> if set to auto while writing to file or pipe.
That makes sense to me. In theory, we could offer an "always use this
color" and a "conditionally use this color" substitution. But I don't
really see why anyone would want the "always use this color" one (they
could just say --color, then).
-Peff
^ permalink raw reply
* Re: [PATCH/RFC] git-am: Make it easier to see which patch failed
From: Johannes Schindelin @ 2009-01-18 17:44 UTC (permalink / raw)
To: Jeff King; +Cc: Stephan Beyer, Junio C Hamano, Jonas Flodén, git
In-Reply-To: <20090118170711.GA17055@coredump.intra.peff.net>
Hi.
On Sun, 18 Jan 2009, Jeff King wrote:
> On Sun, Jan 18, 2009 at 05:17:43PM +0100, Johannes Schindelin wrote:
>
> > The name "FIRSTLINE" suggests that it is indeed a first line, and
> > consequently cannot contain a newline.
>
> It is not "this is a problem because it might contain a newline" but
> "this is a problem because it might contain an escape sequence, _an
> example_ of which is a \n newline." So the question is whether you can
> guarantee that $FIRSTLINE does not contain a backslash. Which I don't
> think is the case here.
Oh. Okay, so I was wrong. But only because dash's echo behaves in a
strange way: it makes "-e" a noop?
Ciao,
Dscho
^ permalink raw reply
* Re: meaning of --8<--
From: Jeff King @ 2009-01-18 17:44 UTC (permalink / raw)
To: Markus Heidelberg; +Cc: git
In-Reply-To: <200901181656.37813.markus.heidelberg@web.de>
On Sun, Jan 18, 2009 at 04:56:37PM +0100, Markus Heidelberg wrote:
> I've seen lines like "--8<--" several times on this list, but have no
> clue what it is about. OK, seems like it's used to insert diffs in the
> middle of a mail message.
> But is this a common convention or git specific and handled by git-am?
> Is it documented anywhere?
As others have explained, it is supposed to be scissors, and it is not
handled by git-am. But the real point of it is to help the person
applying (e.g., Junio) manually separate cover letter from the commit
message. So one way of sending cover material is putting it after the
"---" and before the diff:
Subject: [PATCH] first line of commit message
more commit message
---
Here is cover letter material that doesn't go in the commit message.
It is stripped automatically by git-am.
diff --git ...
but that is often unnatural to read, because the cover letter material
often introduces the commit, especially when you are replying in the
middle of a thread. So then you end up with:
Subject: Re: whatever thread you're in
Somebody else said:
> blah blah blah
I disagree. You should do it like this instead:
-- >8 --
first line of commit message
more commit message
---
diff --git ...
git-am will put everything down to the "---" into the commit message,
but it is simple enough to amend away everything else.
So in that sense, it doesn't really matter _what_ the separator is, but
it should be visibly obvious to a human so that they can fix up the
commit message. The only exception is that it should _not_ be "---",
because that is treated specially by git-am.
-Peff
^ permalink raw reply
* Re: [PATCH 1/2] color: make it easier for non-config to parse color specs
From: René Scharfe @ 2009-01-18 17:45 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Markus Heidelberg, git
In-Reply-To: <20090118172802.GA17434@coredump.intra.peff.net>
Jeff King schrieb:
> On Sun, Jan 18, 2009 at 06:10:36PM +0100, René Scharfe wrote:
>
>>> - right now, it is implemented in terms of color_parse().
>>> But it would be more efficient to reverse this and
>>> implement color_parse in terms of color_parse_mem.
>> Thusly?
>
> Yes, except for the bugs you introduced. :)
Eeek! :)
>> +void color_parse_mem(const char *value, int len, const char *var, char *dst)
>> +{
>> const char *ptr = value;
>> int attr = -1;
>> int fg = -2;
>
> What's missing in the context here (because it wasn't changed) is:
>
>> if (!strcasecmp(value, "reset")) {
>> strcpy(dst, "\033[m");
>> return;
>> }
>
> which doesn't work, since our string is actually something like
> "reset)\0" or even "reset)some totally unrelated string". So we would
> need a "memcasecmp" here.
if (!strncasecmp(value, "reset", len)) {
> And then in the error case, we call:
>
>> die("bad color value '%s' for variable '%s'", value, var);
>
> which is also bogus.
die("bad color value '%.*s' for variable '%s', len, value, var);
> I don't know if this is really even worth it. The timing difference is
> pretty minimal:
>
> $ time ./git log --pretty=tformat:'%Credfoo%Creset' >/dev/null
> real 0m0.673s
> user 0m0.652s
> sys 0m0.016s
> $ time ./git log --pretty=tformat:'%C(red)foo%C(reset)' >/dev/null
> real 0m0.692s
> user 0m0.660s
> sys 0m0.032s
>
> That's about 1 microsecond per commit.
Hmm, not too much overhead, agreed, but it would still be nice to avoid it.
René
^ permalink raw reply
* Re: [PATCH 2/3] Teach read_tree_recursive() how to traverse into submodules
From: Lars Hjemli @ 2009-01-18 17:45 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.DEB.1.00.0901181635290.3586@pacific.mpi-cbg.de>
On Sun, Jan 18, 2009 at 16:48, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Hi,
>
> On Sun, 18 Jan 2009, Lars Hjemli wrote:
>
>> diff --git a/environment.c b/environment.c
>> @@ -159,3 +161,13 @@ int set_git_dir(const char *path)
>> setup_git_env();
>> return 0;
>> }
>> +
>> +int get_traverse_gitlinks()
>> +{
>> + return traverse_gitlinks;
>> +}
>> +
>> +void set_traverse_gitlinks(int traverse)
>> +{
>> + traverse_gitlinks = traverse;
>> +}
>
> If you have full accessors anyway, it is much easier and cleaner to make
> this a global variable to begin with.
>
> However, environment.c is reserved for things that come from the config
> and can be overridden by the user. That is certainly not the case for
> traverse_gitlinks.
>
> But let's think about it again: should traverse_gitlinks be a global
> varible at all? I think not. It should be a per-call decision.
Yes, it might be a cleaner solution to add an extra parameter to
read_tree_recursive(), and since it currently has only 7 callsites the
patch wouldn't be too big either. Junio, what do you think?
>> +/* Try to add the objectdb of a submodule */
>> +int add_gitlink_odb(char *relpath)
>
> This wants to be static.
Thanks
>
>> +{
>> + const char *odbpath;
>> + struct stat st;
>> +
>> + odbpath = read_gitfile_gently(mkpath("%s/.git", relpath));
>> + if (!odbpath)
>> + odbpath = mkpath("%s/.git/objects", relpath);
>> +
>> + if (stat(odbpath, &st))
>> + return 1;
>> +
>> + return add_alt_odb(odbpath);
>> +}
>> +
>> +/* Check if we should recurse into the specified submodule */
>> +int traverse_gitlink(char *path, const unsigned char *commit_sha1,
>
> This, too.
Thanks again ;-)
>
>> + struct tree **subtree)
>> +{
>> + unsigned char sha1[20];
>> + int linked_odb = 0;
>> + struct commit *commit;
>> + void *buffer;
>> + enum object_type type;
>> + unsigned long size;
>> +
>> + hashcpy(sha1, commit_sha1);
>> + if (!add_gitlink_odb(path)) {
>> + linked_odb = 1;
>> + if (resolve_gitlink_ref(path, "HEAD", sha1))
>> + die("Unable to lookup HEAD in %s", path);
>> + }
>
> Why would you want to continue if add_gitlink_odb() did not find a checked
> out submodule?
>
> Seems you want to fall back to look in the superproject's object database.
> But I think that is wrong, as I have a superproject with many platform
> dependent submodules, only one of which is checked out, and for
> convenience, the submodules all live in the superproject's repository.
Actually, I want this to work for bare repositories by specifying the
submodule odbs in the alternates file. So if the current submodule odb
wasn't found my plan was to check if the commit object was accessible
anyways but don't die() if it wasn't.
>> + commit = lookup_commit(sha1);
>> + if (!commit)
>> + die("traverse_gitlink(): internal error");
>
> s/internal error/could not access commit '%s' of submodule '%s'",
> sha1_to_hex(sha1), path);/
Ok (I belive this codepath is virtually impossible to hit, hence the
"internal error", but I could of course be mistaken).
>> @@ -132,6 +188,30 @@ int read_tree_recursive(struct tree *tree,
>> return -1;
>> continue;
>> }
>> + if (S_ISGITLINK(entry.mode) && get_traverse_gitlinks()) {
>
> Like I said, traverse_gitlinks should be a flag to read_tree_recursive.
> So preferably, you should add a parameter 'flags' and make that option an
> enum.
>
>> + int retval;
>> + char *newbase;
>> + struct tree *subtree;
>> + unsigned int pathlen = tree_entry_len(entry.path, entry.sha1);
>
> Nit: Long line.
Will fix
>
>> +
>> + newbase = xmalloc(baselen + 1 + pathlen);
>> + memcpy(newbase, base, baselen);
>> + memcpy(newbase + baselen, entry.path, pathlen);
>> + newbase[baselen + pathlen] = 0;
>
> We have strbufs for that.
>
>> + if (!traverse_gitlink(newbase, entry.sha1, &subtree)) {
>> + free(newbase);
>> + continue;
>> + }
>> + newbase[baselen + pathlen] = '/';
>
> ... to avoid this off-by-one.
Actually, I don't think this is off-by-one, since (baselen + pathlen +
1) is passed along to read_tree_recursive() as the new baselen. But
using strbufs might be cleaner anyways.
Thanks for the review.
--
larsh
^ permalink raw reply
* Re: [PATCH/RFC] git-am: Make it easier to see which patch failed
From: Jeff King @ 2009-01-18 17:49 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Stephan Beyer, Junio C Hamano, Jonas Flodén, git
In-Reply-To: <alpine.DEB.1.00.0901181843230.3586@pacific.mpi-cbg.de>
On Sun, Jan 18, 2009 at 06:44:01PM +0100, Johannes Schindelin wrote:
> Oh. Okay, so I was wrong. But only because dash's echo behaves in a
> strange way: it makes "-e" a noop?
Right. "-e" isn't in POSIX at all, and it is a SysV-ism to allow escapes
in any argument (I don't know if "-e" was introduced by GNU people, or
came from elsewhere).
See here for the gory details:
http://www.opengroup.org/onlinepubs/009695399/utilities/echo.html
-Peff
^ 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