* Re: [PATCH 2/7] Simplify strbuf uses in archive-tar.c using the proper functions.
From: Pierre Habouzit @ 2007-09-06 18:08 UTC (permalink / raw)
To: Kristian Høgsberg; +Cc: git
In-Reply-To: <1189101569.3423.17.camel@hinata.boston.redhat.com>
[-- Attachment #1: Type: text/plain, Size: 2139 bytes --]
On Thu, Sep 06, 2007 at 05:59:29PM +0000, Kristian Høgsberg wrote:
> On Thu, 2007-09-06 at 13:20 +0200, Pierre Habouzit wrote:
> > + strbuf_grow(sb, len);
> > + strbuf_addf(sb, "%u %s=", len, keyword);
> > + strbuf_add(sb, value, valuelen);
> > + strbuf_addch(sb, '\n');
> > }
>
> This entire function can be collapsed to just:
>
> strbuf_addf(sb, "%u %s=%.*s\n", len, keyword, valuelen, value);
yes, but it's less efficient, because %.*s says that sprintf must copy
at most valuelen bytes from value, but it still has to stop if it finds
a \0 before. And the strbuf_grow has sense because the extend policy at
snprintf time is optimistic: we try to write, and if it didn't fit, we
try again. So there is a huge benefit if we have a clue of the final
size.
I would not change a thing.
> > + strbuf_init(&ext_header);
>
> Just use your STRBUF_INIT macro?
Many people dilike it, I'm not sure it's a good idea either, and the
performance hit should be negligible, if it's not, then we can still
make the _init() function an inline.
> > if (ext_header.len > 0) {
> > write_entry(sha1, NULL, 0, ext_header.buf, ext_header.len);
> > - free(ext_header.buf);
> > }
>
> Remove excess braces?
bah, I don't like to strip braces so I won't do that, else you end up
with stupidities like:
if (foo)
// bar();
do_some_very_important_stuff();
Call me paranoid but well, it saved me so many times ...
> > - memcpy(path.buf, base, baselen);
> > - memcpy(path.buf + baselen, filename, filenamelen);
> > - path.len = baselen + filenamelen;
> > - path.buf[path.len] = '\0';
> > + strbuf_grow(&path, MAX(PATH_MAX, baselen + filenamelen + 1));
> > + strbuf_reset(&path);
>
> Does strbuf_reset() do anything here?
>
> > + strbuf_add(&path, base, baselen);
Yes _reset() sets length to 0. so the add here will write at the start
of the buffer again. It definitely is important !
--
·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 2/7] Simplify strbuf uses in archive-tar.c using the proper functions.
From: Kristian Høgsberg @ 2007-09-06 17:59 UTC (permalink / raw)
To: Pierre Habouzit; +Cc: git
In-Reply-To: <11890776111843-git-send-email-madcoder@debian.org>
On Thu, 2007-09-06 at 13:20 +0200, Pierre Habouzit wrote:
> This is just cleaner way to deal with strbufs, using its API rather than
> reinventing it in the module (e.g. strbuf_append_string is just the plain
> strbuf_addstr function, and it was used to perform what strbuf_addch does
> anyways).
> ---
> archive-tar.c | 65 ++++++++++++++-------------------------------------------
> 1 files changed, 16 insertions(+), 49 deletions(-)
>
> diff --git a/archive-tar.c b/archive-tar.c
> index a0763c5..c84d7c0 100644
> --- a/archive-tar.c
> +++ b/archive-tar.c
> @@ -78,19 +78,6 @@ static void write_trailer(void)
> }
> }
>
> -static void strbuf_append_string(struct strbuf *sb, const char *s)
> -{
> - int slen = strlen(s);
> - int total = sb->len + slen;
> - if (total + 1 > sb->alloc) {
> - sb->buf = xrealloc(sb->buf, total + 1);
> - sb->alloc = total + 1;
> - }
> - memcpy(sb->buf + sb->len, s, slen);
> - sb->len = total;
> - sb->buf[total] = '\0';
> -}
> -
> /*
> * pax extended header records have the format "%u %s=%s\n". %u contains
> * the size of the whole string (including the %u), the first %s is the
> @@ -100,26 +87,17 @@ static void strbuf_append_string(struct strbuf *sb, const char *s)
> static void strbuf_append_ext_header(struct strbuf *sb, const char *keyword,
> const char *value, unsigned int valuelen)
> {
> - char *p;
> - int len, total, tmp;
> + int len, tmp;
>
> /* "%u %s=%s\n" */
> len = 1 + 1 + strlen(keyword) + 1 + valuelen + 1;
> for (tmp = len; tmp > 9; tmp /= 10)
> len++;
>
> - total = sb->len + len;
> - if (total > sb->alloc) {
> - sb->buf = xrealloc(sb->buf, total);
> - sb->alloc = total;
> - }
> -
> - p = sb->buf;
> - p += sprintf(p, "%u %s=", len, keyword);
> - memcpy(p, value, valuelen);
> - p += valuelen;
> - *p = '\n';
> - sb->len = total;
> + strbuf_grow(sb, len);
> + strbuf_addf(sb, "%u %s=", len, keyword);
> + strbuf_add(sb, value, valuelen);
> + strbuf_addch(sb, '\n');
> }
This entire function can be collapsed to just:
strbuf_addf(sb, "%u %s=%.*s\n", len, keyword, valuelen, value);
>
> static unsigned int ustar_header_chksum(const struct ustar_header *header)
> @@ -153,8 +131,7 @@ static void write_entry(const unsigned char *sha1, struct strbuf *path,
> struct strbuf ext_header;
>
> memset(&header, 0, sizeof(header));
> - ext_header.buf = NULL;
> - ext_header.len = ext_header.alloc = 0;
> + strbuf_init(&ext_header);
Just use your STRBUF_INIT macro?
> if (!sha1) {
> *header.typeflag = TYPEFLAG_GLOBAL_HEADER;
> @@ -225,8 +202,8 @@ static void write_entry(const unsigned char *sha1, struct strbuf *path,
>
> if (ext_header.len > 0) {
> write_entry(sha1, NULL, 0, ext_header.buf, ext_header.len);
> - free(ext_header.buf);
> }
Remove excess braces?
> + strbuf_release(&ext_header);
> write_blocked(&header, sizeof(header));
> if (S_ISREG(mode) && buffer && size > 0)
> write_blocked(buffer, size);
> @@ -235,11 +212,11 @@ static void write_entry(const unsigned char *sha1, struct strbuf *path,
> static void write_global_extended_header(const unsigned char *sha1)
> {
> struct strbuf ext_header;
> - ext_header.buf = NULL;
> - ext_header.len = ext_header.alloc = 0;
> +
> + strbuf_init(&ext_header);
STRBUF_INIT macro?
> strbuf_append_ext_header(&ext_header, "comment", sha1_to_hex(sha1), 40);
> write_entry(NULL, NULL, 0, ext_header.buf, ext_header.len);
> - free(ext_header.buf);
> + strbuf_release(&ext_header);
> }
>
> static int git_tar_config(const char *var, const char *value)
> @@ -260,28 +237,18 @@ static int write_tar_entry(const unsigned char *sha1,
> const char *base, int baselen,
> const char *filename, unsigned mode, int stage)
> {
> - static struct strbuf path;
> + static struct strbuf path = STRBUF_INIT;
> int filenamelen = strlen(filename);
> void *buffer;
> enum object_type type;
> unsigned long size;
>
> - if (!path.alloc) {
> - path.buf = xmalloc(PATH_MAX);
> - path.alloc = PATH_MAX;
> - path.len = path.eof = 0;
> - }
> - if (path.alloc < baselen + filenamelen + 1) {
> - free(path.buf);
> - path.buf = xmalloc(baselen + filenamelen + 1);
> - path.alloc = baselen + filenamelen + 1;
> - }
> - memcpy(path.buf, base, baselen);
> - memcpy(path.buf + baselen, filename, filenamelen);
> - path.len = baselen + filenamelen;
> - path.buf[path.len] = '\0';
> + strbuf_grow(&path, MAX(PATH_MAX, baselen + filenamelen + 1));
> + strbuf_reset(&path);
Does strbuf_reset() do anything here?
> + strbuf_add(&path, base, baselen);
> + strbuf_add(&path, filename, filenamelen);
> if (S_ISDIR(mode) || S_ISGITLINK(mode)) {
> - strbuf_append_string(&path, "/");
> + strbuf_addch(&path, '/');
> buffer = NULL;
> size = 0;
> } else {
^ permalink raw reply
* Re: [RFC] Convert builin-mailinfo.c to use The Better String Library.
From: Linus Torvalds @ 2007-09-06 17:50 UTC (permalink / raw)
To: Dmitry Kakurin; +Cc: Matthieu Moy, Git
In-Reply-To: <4AFD7EAD1AAC4E54A416BA3F6E6A9E52@ntdev.corp.microsoft.com>
On Wed, 5 Sep 2007, Dmitry Kakurin wrote:
>
> When I first looked at Git source code two things struck me as odd:
> 1. Pure C as opposed to C++. No idea why. Please don't talk about portability,
> it's BS.
*YOU* are full of bullshit.
C++ is a horrible language. It's made more horrible by the fact that a lot
of substandard programmers use it, to the point where it's much much
easier to generate total and utter crap with it. Quite frankly, even if
the choice of C were to do *nothing* but keep the C++ programmers out,
that in itself would be a huge reason to use C.
In other words: the choice of C is the only sane choice. I know Miles
Bader jokingly said "to piss you off", but it's actually true. I've come
to the conclusion that any programmer that would prefer the project to be
in C++ over C is likely a programmer that I really *would* prefer to piss
off, so that he doesn't come and screw up any project I'm involved with.
C++ leads to really really bad design choices. You invariably start using
the "nice" library features of the language like STL and Boost and other
total and utter crap, that may "help" you program, but causes:
- infinite amounts of pain when they don't work (and anybody who tells me
that STL and especially Boost are stable and portable is just so full
of BS that it's not even funny)
- inefficient abstracted programming models where two years down the road
you notice that some abstraction wasn't very efficient, but now all
your code depends on all the nice object models around it, and you
cannot fix it without rewriting your app.
In other words, the only way to do good, efficient, and system-level and
portable C++ ends up to limit yourself to all the things that are
basically available in C. And limiting your project to C means that people
don't screw that up, and also means that you get a lot of programmers that
do actually understand low-level issues and don't screw things up with any
idiotic "object model" crap.
So I'm sorry, but for something like git, where efficiency was a primary
objective, the "advantages" of C++ is just a huge mistake. The fact that
we also piss off people who cannot see that is just a big additional
advantage.
If you want a VCS that is written in C++, go play with Monotone. Really.
They use a "real database". They use "nice object-oriented libraries".
They use "nice C++ abstractions". And quite frankly, as a result of all
these design decisions that sound so appealing to some CS people, the end
result is a horrible and unmaintainable mess.
But I'm sure you'd like it more than git.
Linus
^ permalink raw reply
* Re: [PATCH] git-svn: remove --first-parent, add --upstream
From: Steven Grimm @ 2007-09-06 17:49 UTC (permalink / raw)
To: Lars Hjemli; +Cc: Junio C Hamano, git, Eric Wong
In-Reply-To: <1189096669534-git-send-email-hjemli@gmail.com>
Lars Hjemli wrote:
> This makes git-svn always issue the --first-parent option to git-log when
> trying to establish the "base" subversion branch, so the --first-parent
> option to git-svn is no longer needed. Instead a new option, --upstream
> <revspec>, is introduced. When this is specified the search for embedded
> git-svn-id lines in commit messages starts at the specified revision, if
> not specified the search starts at HEAD.
>
This is a much better solution -- I can't personally imagine a scenario
where I'd want anything other than the --first-parent behavior, so
making it the default is much more convenient. Thanks.
-Steve
^ permalink raw reply
* Re: [PATCH 1/7] Rework strbuf API and semantics.
From: Kristian Høgsberg @ 2007-09-06 17:49 UTC (permalink / raw)
To: Pierre Habouzit; +Cc: git
In-Reply-To: <118907761140-git-send-email-madcoder@debian.org>
On Thu, 2007-09-06 at 13:20 +0200, Pierre Habouzit wrote:
> The gory details are explained in strbuf.h. The change of semantics this
> patch enforces is that the embeded buffer has always a '\0' character after
> its last byte, to always make it a C-string. The offs-by-one changes are all
> related to that very change.
>
> A strbuf can be used to store byte arrays, or as an extended string
> library. The `buf' member can be passed to any C legacy string function,
> because strbuf operations always ensure there is a terminating \0 at the end
> of the buffer, not accounted in the `len' field of the structure.
>
> A strbuf can be used to generate a string/buffer whose final size is not
> really known, and then "strbuf_detach" can be used to get the built buffer,
> and keep the wrapping "strbuf" structure usable for further work again.
>
> Other interesting feature: strbuf_grow(sb, size) ensure that there is
> enough allocated space in `sb' to put `size' new octets of data in the
> buffer. It helps avoiding reallocating data for nothing when the problem the
> strbuf helps to solve has a known typical size.
This looks good, should be very useful.
Kristian
^ permalink raw reply
* Re: People unaware of the importance of "git gc"?
From: Junio C Hamano @ 2007-09-06 17:49 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Nicolas Pitre, Nix, Steven Grimm, Linus Torvalds,
Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0709061651550.28586@racer.site>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> On Wed, 5 Sep 2007, Junio C Hamano wrote:
>
>> @@ -20,6 +20,7 @@ static const char builtin_gc_usage[] = "git-gc [--prune] [--aggressive]";
>>
>> static int pack_refs = 1;
>> static int aggressive_window = -1;
>> +static int gc_auto_threshold = 6700;
>
> Please don't do that.
>
> When you share objects with another git directory, git-gc --auto can get
> rid of the objects when some objects go away in the referenced repository.
I thought the whole point of "gc --auto" was to have something
that does not lose/prune any objects, even the ones that do not
seem to be referenced from anywhere. That is why invocations of
"git gc --auto" do not say --prune as you saw the second patch,
and the repack command "gc --auto" runs is "repack -d -l"
instead of "repack -a -d -l", which means that it does run
git-prune-packed after repacking but not git-prune.
Maybe I am missing something...
^ permalink raw reply
* Re: [PATCH] Include a git-push example for creating a remote branch
From: Junio C Hamano @ 2007-09-06 17:42 UTC (permalink / raw)
To: Carl Worth; +Cc: Miles Bader, Shawn O. Pearce, git
In-Reply-To: <874pi7dcba.wl%cworth@cworth.org>
Carl Worth <cworth@cworth.org> writes:
> On Wed, 05 Sep 2007 23:30:31 -0700, Junio C Hamano wrote:
>> It is just nobody felt strong enough reason to sugarcoat the
>> normalized syntax with something like:
>>
>> git push --create remote foo v1.2.0
>
> Couldn't we just use an initial + to indicate this as well?
Unfortunately, no, because + has already been taken to mean
something completely different.
^ permalink raw reply
* Re: strbuf new API, take 2 for inclusion
From: Pierre Habouzit @ 2007-09-06 17:19 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20070906171621.GA5305@coredump.intra.peff.net>
[-- Attachment #1: Type: text/plain, Size: 748 bytes --]
On Thu, Sep 06, 2007 at 05:16:21PM +0000, Jeff King wrote:
> On Thu, Sep 06, 2007 at 07:15:02PM +0200, Pierre Habouzit wrote:
>
> > Yes, Junio already did that remark. The reason is that it's forward
> > compatible: if we ever change strbuf's intitial value for some reason,
> > we would just have to rebuild the code. As junio disliked it (and I'm
> > not sure I love it either) I've used it where using the _init() function
> > was impractical.
>
> OK, I missed that discussion. Thanks for the explanation.
That was on IRC, maybe that's the why :)
--
·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 6/7] Eradicate yet-another-buffer implementation in buitin-rerere.c
From: Pierre Habouzit @ 2007-09-06 17:17 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0709061504521.28586@racer.site>
[-- Attachment #1: Type: text/plain, Size: 990 bytes --]
On Thu, Sep 06, 2007 at 02:05:36PM +0000, Johannes Schindelin wrote:
> Hi,
>
> On Thu, 6 Sep 2007, Pierre Habouzit wrote:
>
> > Signed-off-by: Pierre Habouzit <madcoder@debian.org>
> > ---
> > builtin-rerere.c | 56 +++++++++++++++++------------------------------------
> > 1 files changed, 18 insertions(+), 38 deletions(-)
>
> I like that one very much, but ...
>
> > FILE *f = fopen(path, "r");
> > FILE *out;
> >
> > + strbuf_init(&minus);
> > + strbuf_init(&plus);
> > +
>
> You used spaces instead of tabs here.
crap, and I did that in the 5th patch as well. well, I'll maybe send
privately a "fixed" version of the patch to junio then, to avoid
flooding the list with spacing issues.
And I'll also set my vim to use tabs when I'm hacking on git.
--
·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: strbuf new API, take 2 for inclusion
From: Jeff King @ 2007-09-06 17:16 UTC (permalink / raw)
To: git
In-Reply-To: <20070906171502.GF8451@artemis.corp>
On Thu, Sep 06, 2007 at 07:15:02PM +0200, Pierre Habouzit wrote:
> Yes, Junio already did that remark. The reason is that it's forward
> compatible: if we ever change strbuf's intitial value for some reason,
> we would just have to rebuild the code. As junio disliked it (and I'm
> not sure I love it either) I've used it where using the _init() function
> was impractical.
OK, I missed that discussion. Thanks for the explanation.
-Peff
^ permalink raw reply
* Re: strbuf new API, take 2 for inclusion
From: Pierre Habouzit @ 2007-09-06 17:15 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20070906125811.GA32400@coredump.intra.peff.net>
[-- Attachment #1: Type: text/plain, Size: 1214 bytes --]
On Thu, Sep 06, 2007 at 12:58:11PM +0000, Jeff King wrote:
> On Thu, Sep 06, 2007 at 01:20:04PM +0200, Pierre Habouzit wrote:
>
> > I've also stripped as many STRBUF_INIT uses as possible, some people
> > didn't liked it. I've kept its use for "static" strbufs where it's way
> > more convenient that a function call.
>
> The STRBUF_INIT initializer just sets everything to '0' or NULL. Static
> objects already have this done automagically by the compiler, so there's
> no need to use STRBUF_INIT at all there.
Yes, Junio already did that remark. The reason is that it's forward
compatible: if we ever change strbuf's intitial value for some reason,
we would just have to rebuild the code. As junio disliked it (and I'm
not sure I love it either) I've used it where using the _init() function
was impractical.
And yes { 0 } would have worked the same as per C standard. It's just
that I setup my compiler to flag missing C89 initializers (because it
often detects real errors).
Here are the whys'
--
·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 5/3] archive: rename attribute specfile to export-subst
From: Johannes Schindelin @ 2007-09-06 17:13 UTC (permalink / raw)
To: René Scharfe
Cc: Junio C Hamano, Andreas Ericsson, Git Mailing List,
Michael Gernoth, Thomas Glanzmann
In-Reply-To: <46E02FFF.8090902@lsrfire.ath.cx>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 572 bytes --]
Hi,
On Thu, 6 Sep 2007, René Scharfe wrote:
> As suggested by Junio and Johannes, change the name of the former
> attribute specfile to export-subst to indicate its function rather
> than purpose and to make clear that it is not applied to working tree
> files.
>
> Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
ACK!
(Even if I did not really suggest "export-subst", which I like very
much...)
The bigger question is now if these two patches should be folded back into
your original patch series, or stand alone as commits of their own...
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 4/3] archive: specfile syntax change: "$Format:%PLCHLDR$" instead of just "%PLCHLDR"
From: Johannes Schindelin @ 2007-09-06 17:11 UTC (permalink / raw)
To: René Scharfe
Cc: Junio C Hamano, Andreas Ericsson, Git Mailing List,
Michael Gernoth, Thomas Glanzmann
In-Reply-To: <46E028B9.2090908@lsrfire.ath.cx>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 1351 bytes --]
Hi,
On Thu, 6 Sep 2007, René Scharfe wrote:
> As suggested by Johannes, --pretty=format: placeholders in specfiles
> need to be wrapped in $Format:...$ now.
Thanks.
> diff --git a/builtin-archive.c b/builtin-archive.c
> index faccce3..a8a0f01 100644
> --- a/builtin-archive.c
> +++ b/builtin-archive.c
> @@ -81,14 +81,58 @@ static int run_remote_archiver(const char *remote, int argc,
> return !!rv;
> }
>
> +static void *format_specfile(const struct commit *commit, const char *format,
> + unsigned long *sizep)
Should this not be "char *buffer" instead of "const char *format"? Or
even better: a "struct strbuf *"?
> +{
> + unsigned long len = *sizep, result_len = 0;
> + const char *a = format;
> + char *result = NULL;
> +
> + for (;;) {
> + const char *b, *c;
> + char *fmt, *formatted = NULL;
> + unsigned long a_len, fmt_len, formatted_len, allocated = 0;
Maybe initialise formatted_len, just to be on the safe side?
> +
> + b = memchr(a, '$', len);
> + if (!b || a + len < b + 9 || memcmp(b + 1, "Format:", 7))
> + break;
Wouldn't memmem(buffer, len, "$Format:", 8) be better here?
A general comment: since you plan to output the result into a file anyway,
it should be even easier to avoid realloc(), and do a
print_formatted_specfile() instead of a format_specfile(), no?
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 9/9] Implement git commit as a builtin command.
From: Johannes Schindelin @ 2007-09-06 16:59 UTC (permalink / raw)
To: Kristian Høgsberg; +Cc: git
In-Reply-To: <11890382271931-git-send-email-krh@redhat.com>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 3976 bytes --]
Hi,
On Wed, 5 Sep 2007, Kristian Høgsberg wrote:
> contrib/examples/git-commit.sh | 665 +++++++++++++++++++++++++++++++++++
> git-commit.sh | 665 -----------------------------------
You might want to use "git format-patch -M" next time ;-)
> @@ -357,7 +358,6 @@ BUILTIN_OBJS = \
> builtin-rev-parse.o \
> builtin-revert.o \
> builtin-rm.o \
> - builtin-runstatus.o \
Better keep it; some people's scripts could depend on it.
> +struct option {
> + enum option_type type;
> + const char *long_name;
> + char short_name;
> + void *value;
> +};
> +
> +static int scan_options(const char ***argv, struct option *options)
> +{
I would not (no longer, anyway) be opposed to replacing the option parsing
in git with getopt(); I hear that it is small enough to keep a copy in
compat/getopt.c.
But let's go forward with builtin-commit; getopt() can come later.
> +static char *
> +prepare_index(const char **files, const char *prefix)
> +{
> + int fd;
> + struct tree *tree;
> + struct lock_file *next_index_lock;
> +
> + fd = hold_locked_index(&lock_file, 1);
> + if (read_cache() < 0)
> + die("index file corrupt");
> +
> + if (all) {
> + add_files_to_cache(fd, files, NULL);
> + return lock_file.filename;
> + } else if (also) {
> + add_files_to_cache(fd, files, prefix);
> + return lock_file.filename;
> + }
> +
> + if (interactive)
> + interactive_add();
> +
> + if (*files == NULL) {
> + /* Commit index as-is. */
> + rollback_lock_file(&lock_file);
> + return get_index_file();
> + }
> +
> + /*
> + * FIXME: Warn on unknown files. Shell script does
> + *
> + * commit_only=`git-ls-files --error-unmatch -- "$@"`
> + */
> +
> + /*
> + * FIXME: shell script does
> + *
> + * git-read-tree --index-output="$TMP_INDEX" -i -m HEAD
> + *
> + * which warns about unmerged files in the index.
> + */
> +
> + /* update the user index file */
> + add_files_to_cache(fd, files, prefix);
I suspect this, or ...
> +
> + if (!initial_commit) {
> + tree = parse_tree_indirect(head_sha1);
> + if (!tree)
> + die("failed to unpack HEAD tree object");
> + if (read_tree(tree, 0, NULL))
> + die("failed to read HEAD tree object");
> + }
> +
> + /* Uh oh, abusing lock_file to create a garbage collected file */
> + next_index_lock = xmalloc(sizeof(*next_index_lock));
> + fd = hold_lock_file_for_update(next_index_lock,
> + git_path("next-index-%d", getpid()), 1);
> + add_files_to_cache(fd, files, prefix);
... this, but not both.
> +/* Find out if the message starting at position 'start' in the strbuf
> + * contains only whitespace and Signed-off-by lines. */
> +static int message_is_empty(struct strbuf *sb, int start)
> +{
> + static const char signed_off_by[] = "Signed-off-by: ";
I think you already defined that globally earlier.
In the function message_is_empty() you write:
> + /* See if the template is just a prefix of the message. */
> + strbuf_init(&tmpl);
> + if (template_file && strbuf_read_path(&tmpl, template_file) > 0) {
> + stripspace(&tmpl, 1);
> + if (start + tmpl.len <= sb->len &&
> + memcmp(tmpl.buf, sb->buf + start, tmpl.len) == 0)
> + start += tmpl.len;
Could we not bail out here, if there is no match? In that case, the
message is clearly not empty...
> + /* Check if the rest is just whitespace and Signed-of-by's. */
> + for (i = start; i < sb->len; i++) {
> + nl = memchr(sb->buf + i, '\n', sb->len - i);
> + if (nl)
> + eol = nl - sb->buf;
> + else
> + eol = sb->len;
Why not just "if (isspace(sb->buf[i]) || sb->buf[i] == '\n') continue;"?
This would also catch the cases where people indent their S-O-Bs.
> +
> + if (strlen(signed_off_by) <= eol - i &&
> + !prefixcmp(sb->buf + i, signed_off_by)) {
> + i = eol;
> + continue;
> + }
> + while (i < eol)
> + if (!isspace(sb->buf[i++]))
> + return 0;
> + }
> +
> + return 1;
> +}
I did not review the rest of the code closely yet...
All in all: well done!
Ciao,
Dscho
^ permalink raw reply
* [PATCH 5/3] archive: rename attribute specfile to export-subst
From: René Scharfe @ 2007-09-06 16:51 UTC (permalink / raw)
To: Junio C Hamano
Cc: Johannes Schindelin, Andreas Ericsson, Git Mailing List,
Michael Gernoth, Thomas Glanzmann
In-Reply-To: <7vzm02klip.fsf@gitster.siamese.dyndns.org>
Junio C Hamano schrieb:
> René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
>
>>> Maybe we should not so much name it by purpose, but by function. How
>>> about "substformat" for the attribute name, and replacing any
>>> $Format:blablub$ inside those files with something a la
>>> --pretty=format:blablub?
>> I like the $Format:...$ notation. How about naming the attribute
>> "template", as that's what a thus marked file is?
>
> Sounds good, although I suspect "template" might confuse newbies
> that checkout may apply the substitution as well. How about
> something with "export" in it? export-subst, perhaps?
Well, including "export" in the name makes sense, yes. I can't come up
with a better name, let's take this.
--- snip! ---
As suggested by Junio and Johannes, change the name of the former
attribute specfile to export-subst to indicate its function rather
than purpose and to make clear that it is not applied to working tree
files.
Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
Documentation/gitattributes.txt | 6 +++---
builtin-archive.c | 14 +++++++-------
t/t5000-tar-tree.sh | 18 +++++++++---------
3 files changed, 19 insertions(+), 19 deletions(-)
diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index 37b3be8..d0e951e 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -424,10 +424,10 @@ frotz unspecified
Creating an archive
~~~~~~~~~~~~~~~~~~~
-`specfile`
-^^^^^^^^^^
+`export-subst`
+^^^^^^^^^^^^^^
-If the attribute `specfile` is set for a file then git will expand
+If the attribute `export-subst` is set for a file then git will expand
several placeholders when adding this file to an archive. The
expansion depends on the availability of a commit ID, i.e. if
gitlink:git-archive[1] has been given a tree instead of a commit or a
diff --git a/builtin-archive.c b/builtin-archive.c
index a8a0f01..af14837 100644
--- a/builtin-archive.c
+++ b/builtin-archive.c
@@ -81,8 +81,8 @@ static int run_remote_archiver(const char *remote, int argc,
return !!rv;
}
-static void *format_specfile(const struct commit *commit, const char *format,
- unsigned long *sizep)
+static void *format_subst(const struct commit *commit, const char *format,
+ unsigned long *sizep)
{
unsigned long len = *sizep, result_len = 0;
const char *a = format;
@@ -131,22 +131,22 @@ static void *convert_to_archive(const char *path,
const void *src, unsigned long *sizep,
const struct commit *commit)
{
- static struct git_attr *attr_specfile;
+ static struct git_attr *attr_export_subst;
struct git_attr_check check[1];
if (!commit)
return NULL;
- if (!attr_specfile)
- attr_specfile = git_attr("specfile", 8);
+ if (!attr_export_subst)
+ attr_export_subst = git_attr("export-subst", 12);
- check[0].attr = attr_specfile;
+ check[0].attr = attr_export_subst;
if (git_checkattr(path, ARRAY_SIZE(check), check))
return NULL;
if (!ATTR_TRUE(check[0].value))
return NULL;
- return format_specfile(commit, src, sizep);
+ return format_subst(commit, src, sizep);
}
void *sha1_file_to_archive(const char *path, const unsigned char *sha1,
diff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh
index 6e89e07..42e28ab 100755
--- a/t/t5000-tar-tree.sh
+++ b/t/t5000-tar-tree.sh
@@ -28,7 +28,7 @@ commit id embedding:
TAR=${TAR:-tar}
UNZIP=${UNZIP:-unzip}
-SPECFILEFORMAT=%H%n
+SUBSTFORMAT=%H%n
test_expect_success \
'populate workdir' \
@@ -36,7 +36,7 @@ test_expect_success \
echo simple textfile >a/a &&
mkdir a/bin &&
cp /bin/sh a/bin &&
- printf "A\$Format:%s\$O" "$SPECFILEFORMAT" >a/specfile &&
+ printf "A\$Format:%s\$O" "$SUBSTFORMAT" >a/substfile &&
ln -s a a/l1 &&
(p=long_path_to_a_file && cd a &&
for depth in 1 2 3 4 5; do mkdir $p && cd $p; done &&
@@ -108,20 +108,20 @@ test_expect_success \
'diff -r a c/prefix/a'
test_expect_success \
- 'create an archive with a specfile' \
- 'echo specfile specfile >a/.gitattributes &&
+ 'create an archive with a substfile' \
+ 'echo substfile export-subst >a/.gitattributes &&
git archive HEAD >f.tar &&
rm a/.gitattributes'
test_expect_success \
- 'extract specfile' \
+ 'extract substfile' \
'(mkdir f && cd f && $TAR xf -) <f.tar'
test_expect_success \
- 'validate specfile contents' \
- 'git log --max-count=1 "--pretty=format:A${SPECFILEFORMAT}O" HEAD \
- >f/a/specfile.expected &&
- diff f/a/specfile.expected f/a/specfile'
+ 'validate substfile contents' \
+ 'git log --max-count=1 "--pretty=format:A${SUBSTFORMAT}O" HEAD \
+ >f/a/substfile.expected &&
+ diff f/a/substfile.expected f/a/substfile'
test_expect_success \
'git archive --format=zip' \
^ permalink raw reply related
* Re: [PATCH 7/9] Add strbuf_read_path().
From: Johannes Schindelin @ 2007-09-06 16:40 UTC (permalink / raw)
To: Kristian Høgsberg; +Cc: git
In-Reply-To: <11890382262161-git-send-email-krh@redhat.com>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 288 bytes --]
Hi,
On Wed, 5 Sep 2007, Kristian Høgsberg wrote:
> +extern int strbuf_read_path(struct strbuf *sb, const char *path);
May I suggest renaming this to "strbuf_read_file()"? I kind of expected a
function which reads in the absolute path of a file, judging by the
name...
Ciao,
Dscho
^ permalink raw reply
* [PATCH] git-svn: remove --first-parent, add --upstream
From: Lars Hjemli @ 2007-09-06 16:37 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Eric Wong
In-Reply-To: <20070906075104.GA10192@hand.yhbt.net>
This makes git-svn always issue the --first-parent option to git-log when
trying to establish the "base" subversion branch, so the --first-parent
option to git-svn is no longer needed. Instead a new option, --upstream
<revspec>, is introduced. When this is specified the search for embedded
git-svn-id lines in commit messages starts at the specified revision, if
not specified the search starts at HEAD.
Signed-off-by: Lars Hjemli <hjemli@gmail.com>
---
Documentation/git-svn.txt | 10 +++++-----
git-svn.perl | 18 +++++++++---------
2 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
index 42d7b82..2903777 100644
--- a/Documentation/git-svn.txt
+++ b/Documentation/git-svn.txt
@@ -317,15 +317,15 @@ This is only used with the 'dcommit' command.
Print out the series of git arguments that would show
which diffs would be committed to SVN.
---first-parent::
+--upstream=<revspec>::
This is only used with the 'dcommit', 'rebase', 'log', 'find-rev' and
'show-ignore' commands.
-These commands tries to detect the upstream subversion branch by means of
-the embedded 'git-svn-id' line in commit messages. When --first-parent is
-specified, git-svn only follows the first parent of each commit, effectively
-ignoring commits brought into the current branch through merge-operations.
+These commands tries to detect the upstream subversion branch by traversing
+the first parent of each commit (starting at HEAD), looking for an embedded
+'git-svn-id' line in the commit messages. When --upstream is specified,
+git-svn starts the traversal at the specified commit instead of HEAD.
--
diff --git a/git-svn.perl b/git-svn.perl
index d21eb7f..947a944 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -59,7 +59,7 @@ my ($_stdin, $_help, $_edit,
$_template, $_shared,
$_version, $_fetch_all, $_no_rebase,
$_merge, $_strategy, $_dry_run, $_local,
- $_prefix, $_no_checkout, $_verbose, $_first_parent);
+ $_prefix, $_no_checkout, $_verbose, $_upstream);
$Git::SVN::_follow_parent = 1;
my %remote_opts = ( 'username=s' => \$Git::SVN::Prompt::_username,
'config-dir=s' => \$Git::SVN::Ra::config_dir,
@@ -119,14 +119,14 @@ my %cmd = (
'dry-run|n' => \$_dry_run,
'fetch-all|all' => \$_fetch_all,
'no-rebase' => \$_no_rebase,
- 'first-parent' => \$_first_parent,
+ 'upstream=s' => \$_upstream,
%cmt_opts, %fc_opts } ],
'set-tree' => [ \&cmd_set_tree,
"Set an SVN repository to a git tree-ish",
{ 'stdin|' => \$_stdin, %cmt_opts, %fc_opts, } ],
'show-ignore' => [ \&cmd_show_ignore, "Show svn:ignore listings",
{ 'revision|r=i' => \$_revision,
- 'first-parent' => \$_first_parent
+ 'upstream=s' => \$_upstream
} ],
'multi-fetch' => [ \&cmd_multi_fetch,
"Deprecated alias for $0 fetch --all",
@@ -148,11 +148,11 @@ my %cmd = (
'authors-file|A=s' => \$_authors,
'color' => \$Git::SVN::Log::color,
'pager=s' => \$Git::SVN::Log::pager,
- 'first-parent' => \$_first_parent
+ 'upstream=s' => \$_upstream
} ],
'find-rev' => [ \&cmd_find_rev, "Translate between SVN revision numbers and tree-ish",
{
- 'first-parent' => \$_first_parent
+ 'upstream=s' => \$_upstream
} ],
'rebase' => [ \&cmd_rebase, "Fetch and rebase your working directory",
{ 'merge|m|M' => \$_merge,
@@ -160,7 +160,7 @@ my %cmd = (
'strategy|s=s' => \$_strategy,
'local|l' => \$_local,
'fetch-all|all' => \$_fetch_all,
- 'first-parent' => \$_first_parent,
+ 'upstream=s' => \$_upstream,
%fc_opts } ],
'commit-diff' => [ \&cmd_commit_diff,
'Commit a diff between two trees',
@@ -818,9 +818,9 @@ sub cmt_metadata {
sub working_head_info {
my ($head, $refs) = @_;
- my @args = ('log', '--no-color');
- push @args, '--first-parent' if $_first_parent;
- my ($fh, $ctx) = command_output_pipe(@args, $head);
+ my @args = ('log', '--no-color', '--first-parent');
+ push @args, ($_upstream ? $_upstream : $head);
+ my ($fh, $ctx) = command_output_pipe(@args);
my $hash;
my %max;
while (<$fh>) {
--
1.5.3.1.g0e33-dirty
^ permalink raw reply related
* Re: [PATCH 6/9] Rewrite launch_editor, create_tag and stripspace to use strbufs.
From: Johannes Schindelin @ 2007-09-06 16:38 UTC (permalink / raw)
To: Kristian Høgsberg; +Cc: git
In-Reply-To: <1189038225525-git-send-email-krh@redhat.com>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 1594 bytes --]
Hi,
On Wed, 5 Sep 2007, Kristian Høgsberg wrote:
> diff --git a/strbuf.c b/strbuf.c
> index fcfc05e..ed2afea 100644
> --- a/strbuf.c
> +++ b/strbuf.c
> @@ -73,43 +74,15 @@ void strbuf_printf(struct strbuf *sb, const char *fmt, ...)
> {
> char buffer[2048];
> va_list args;
> - int len, size = 2 * sizeof buffer;
> + int len;
>
> va_start(args, fmt);
> len = vsnprintf(buffer, sizeof(buffer), fmt, args);
> va_end(args);
>
> - if (len > sizeof(buffer)) {
> - /*
> - * Didn't fit in the buffer, but this vsnprintf at
> - * least gives us the required length back. Grow the
> - * buffer acccordingly and try again.
> - */
> - strbuf_grow(sb, len);
> - va_start(args, fmt);
> - len = vsnprintf(sb->buf + sb->len,
> - sb->alloc - sb->len, fmt, args);
> - va_end(args);
> - } else if (len >= 0) {
> - /*
> - * The initial vsnprintf fit in the temp buffer, just
> - * copy it to the strbuf.
> - */
> - strbuf_add(sb, buffer, len);
> - } else {
> - /*
> - * This vnsprintf sucks and just returns -1 when the
> - * buffer is too small. Keep doubling the size until
> - * it fits.
> - */
> - while (len < 0) {
> - strbuf_grow(sb, size);
> - va_start(args, fmt);
> - len = vsnprintf(sb->buf + sb->len,
> - sb->alloc - sb->len, fmt, args);
> - va_end(args);
> - size *= 2;
> - }
> - }
> + if (len > sizeof(buffer) || len < 0)
> + die("out of buffer space\n");
> +
> + strbuf_add(sb, buffer, len);
> }
Really?
(If you find the time, it would be really nice to rebase that patch series
on top of Pierre's strbuf work...)
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] Add a new lstat and fstat implementation based on Win32 API
From: Marius Storm-Olsen @ 2007-09-06 16:34 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Johannes Schindelin, Johannes Sixt, Git Mailing List
In-Reply-To: <46E0285E.1000603@eudaptics.com>
[-- Attachment #1: Type: text/plain, Size: 942 bytes --]
Johannes Sixt wrote:
> Johannes Sixt schrieb:
>> Marius Storm-Olsen schrieb:
>>> Johannes Schindelin wrote:
>>>> To make it easier on others, I just uploaded it into the
>>>> "teststat" branch on 4msysgit.git (subject to removal in a few
>>>> days).
>>>
>>> Ok, I've updated the patch in the 4msysgit.git repo, 'teststat'
>>> branch. RFC, and please test.
>>
>> Thanks a lot! I've pushed it out in mingw.git's master.
>>
>> The reason that t4200-rerere.sh fails is that we now store UTC in
>> st_mtime. However, for the garbage-collection we compare this entry
>> to a local time stamp.
>
> This analysis is incorrect, I think. The reason we fail seems to be
> that t4200 uses test-chmtime, which uses utime(). Likely, we need a
> wrapper for that one?
Ok, could you make a quick patch to add a git_utime() (and probably
git_time()) and see if the tests pass without the UTC to Local time patch?
--
.marius
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 187 bytes --]
^ permalink raw reply
* Re: [PATCH] Add a new lstat implementation based on Win32 API, and make stat use that implementation too.
From: David Kastrup @ 2007-09-06 16:33 UTC (permalink / raw)
To: git
In-Reply-To: <20070906162657.GF2329@genesis.frugalware.org>
Miklos Vajna <vmiklos@frugalware.org> writes:
> On Wed, Sep 05, 2007 at 09:01:46PM +0200, David Kastrup <dak@gnu.org> wrote:
>> > the situation what triggers the 'no such file' problem is:
>> >
>> > ----
>> > $ touch foo/Makefile
>> > $ mkdir bar
>> > $ ln -s foo/Makefile bar
>> > $ cd bar
>> > $ cat Makefile
>> > cat: Makefile: No such file or directory
>> > ----
>>
>> This is under Vista? It would be the same under Unix.
>
> no, i never stated that this is Vista ;-) this is Linux.
Ok, so now we have established that we have not actually established
anything with regard to relative links under Vista short of what
Microsoft claims in its developer information (which has its fair
share of misleading and wrong information).
If anybody is as fortunate as to actually have Vista available, it
would be nice if he corroborated that relative links under Vista are
indeed (as Microsoft appears to claim) relative with regard to the
current work directory rather than the directory containing the link.
--
David Kastrup, Kriemhildstr. 15, 44793 Bochum
^ permalink raw reply
* Re: [PATCH 4/9] Introduce entry point for launching add--interactive.
From: Johannes Schindelin @ 2007-09-06 16:31 UTC (permalink / raw)
To: Kristian Høgsberg; +Cc: git
In-Reply-To: <11890382253220-git-send-email-krh@redhat.com>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 1310 bytes --]
Hi,
On Wed, 5 Sep 2007, Kristian Høgsberg wrote:
> diff --git a/builtin-add.c b/builtin-add.c
> index 3dd4ded..e79e8f7 100644
> --- a/builtin-add.c
> +++ b/builtin-add.c
> @@ -153,6 +154,13 @@ static int git_add_config(const char *var, const char *value)
> return git_default_config(var, value);
> }
>
> +int interactive_add(void)
> +{
> + const char *argv[2] = { "add--interactive", NULL };
> +
> + return run_command_v_opt(argv, RUN_GIT_CMD);
> +}
I'd rather have this in builtin-commit.c, since it is quite funny if
builtin-add.c has code to fork() and exec() itself (eventually, that
is) ;-)
> diff --git a/commit.h b/commit.h
> index 467872e..64e1d4b 100644
> --- a/commit.h
> +++ b/commit.h
> @@ -122,4 +122,13 @@ extern struct commit_list *get_shallow_commits(struct object_array *heads,
> int depth, int shallow_flag, int not_shallow_flag);
>
> int in_merge_bases(struct commit *, struct commit **, int);
> +
> +extern const unsigned char *
> +create_commit(const unsigned char *tree_sha1,
> + unsigned char parent_sha1[][20], int parents,
> + const char *author_info, const char *committer_info,
> + const char *message, int length);
> +
> +extern int interactive_add(void);
> +
Just a guess: you did not want create_commit() to creep in here, right?
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 1/9] Enable wt-status output to a given FILE pointer.
From: Johannes Schindelin @ 2007-09-06 16:27 UTC (permalink / raw)
To: Kristian Høgsberg; +Cc: git
In-Reply-To: <11890382183913-git-send-email-krh@redhat.com>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 228 bytes --]
Hi,
On Wed, 5 Sep 2007, Kristian Høgsberg wrote:
> Still defaults to stdout, but you can now override wt_status.fp after
> calling wt_status_prepare().
Would it not be easier to freopen(filename, "a", stdout)?
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] Add a new lstat implementation based on Win32 API, and make stat use that implementation too.
From: Miklos Vajna @ 2007-09-06 16:26 UTC (permalink / raw)
To: David Kastrup; +Cc: git
In-Reply-To: <85abs1hr6t.fsf@lola.goethe.zz>
[-- Attachment #1: Type: text/plain, Size: 455 bytes --]
On Wed, Sep 05, 2007 at 09:01:46PM +0200, David Kastrup <dak@gnu.org> wrote:
> > the situation what triggers the 'no such file' problem is:
> >
> > ----
> > $ touch foo/Makefile
> > $ mkdir bar
> > $ ln -s foo/Makefile bar
> > $ cd bar
> > $ cat Makefile
> > cat: Makefile: No such file or directory
> > ----
>
> This is under Vista? It would be the same under Unix.
no, i never stated that this is Vista ;-) this is Linux.
- VMiklos
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* [PATCH 4/3] archive: specfile syntax change: "$Format:%PLCHLDR$" instead of just "%PLCHLDR"
From: René Scharfe @ 2007-09-06 16:20 UTC (permalink / raw)
To: Junio C Hamano
Cc: Johannes Schindelin, Andreas Ericsson, Git Mailing List,
Michael Gernoth, Thomas Glanzmann
In-Reply-To: <7vzm02klip.fsf@gitster.siamese.dyndns.org>
As suggested by Johannes, --pretty=format: placeholders in specfiles
need to be wrapped in $Format:...$ now. This syntax change restricts
the expansion of placeholders and makes it easier to use with files
that contain non-placeholder percent signs.
Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
Documentation/gitattributes.txt | 5 +++-
builtin-archive.c | 52 +++++++++++++++++++++++++++++++++++---
t/t5000-tar-tree.sh | 4 +-
3 files changed, 53 insertions(+), 8 deletions(-)
diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index 47a621b..37b3be8 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -432,7 +432,10 @@ several placeholders when adding this file to an archive. The
expansion depends on the availability of a commit ID, i.e. if
gitlink:git-archive[1] has been given a tree instead of a commit or a
tag then no replacement will be done. The placeholders are the same
-as those for the option `--pretty=format:` of gitlink:git-log[1].
+as those for the option `--pretty=format:` of gitlink:git-log[1],
+except that they need to be wrapped like this: `$Format:PLACEHOLDERS$`
+in the file. E.g. the string `$Format:%H$` will be replaced by the
+commit hash.
GIT
diff --git a/builtin-archive.c b/builtin-archive.c
index faccce3..a8a0f01 100644
--- a/builtin-archive.c
+++ b/builtin-archive.c
@@ -81,14 +81,58 @@ static int run_remote_archiver(const char *remote, int argc,
return !!rv;
}
+static void *format_specfile(const struct commit *commit, const char *format,
+ unsigned long *sizep)
+{
+ unsigned long len = *sizep, result_len = 0;
+ const char *a = format;
+ char *result = NULL;
+
+ for (;;) {
+ const char *b, *c;
+ char *fmt, *formatted = NULL;
+ unsigned long a_len, fmt_len, formatted_len, allocated = 0;
+
+ b = memchr(a, '$', len);
+ if (!b || a + len < b + 9 || memcmp(b + 1, "Format:", 7))
+ break;
+ c = memchr(b + 8, '$', len - 8);
+ if (!c)
+ break;
+
+ a_len = b - a;
+ fmt_len = c - b - 8;
+ fmt = xmalloc(fmt_len + 1);
+ memcpy(fmt, b + 8, fmt_len);
+ fmt[fmt_len] = '\0';
+
+ formatted_len = format_commit_message(commit, fmt, &formatted,
+ &allocated);
+ result = xrealloc(result, result_len + a_len + formatted_len);
+ memcpy(result + result_len, a, a_len);
+ memcpy(result + result_len + a_len, formatted, formatted_len);
+ result_len += a_len + formatted_len;
+ len -= c + 1 - a;
+ a = c + 1;
+ }
+
+ if (result && len) {
+ result = xrealloc(result, result_len + len);
+ memcpy(result + result_len, a, len);
+ result_len += len;
+ }
+
+ *sizep = result_len;
+
+ return result;
+}
+
static void *convert_to_archive(const char *path,
const void *src, unsigned long *sizep,
const struct commit *commit)
{
static struct git_attr *attr_specfile;
struct git_attr_check check[1];
- char *interpolated = NULL;
- unsigned long allocated = 0;
if (!commit)
return NULL;
@@ -102,9 +146,7 @@ static void *convert_to_archive(const char *path,
if (!ATTR_TRUE(check[0].value))
return NULL;
- *sizep = format_commit_message(commit, src, &interpolated, &allocated);
-
- return interpolated;
+ return format_specfile(commit, src, sizep);
}
void *sha1_file_to_archive(const char *path, const unsigned char *sha1,
diff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh
index 3d5d01b..6e89e07 100755
--- a/t/t5000-tar-tree.sh
+++ b/t/t5000-tar-tree.sh
@@ -36,7 +36,7 @@ test_expect_success \
echo simple textfile >a/a &&
mkdir a/bin &&
cp /bin/sh a/bin &&
- printf "%s" "$SPECFILEFORMAT" >a/specfile &&
+ printf "A\$Format:%s\$O" "$SPECFILEFORMAT" >a/specfile &&
ln -s a a/l1 &&
(p=long_path_to_a_file && cd a &&
for depth in 1 2 3 4 5; do mkdir $p && cd $p; done &&
@@ -119,7 +119,7 @@ test_expect_success \
test_expect_success \
'validate specfile contents' \
- 'git log --max-count=1 "--pretty=format:$SPECFILEFORMAT" HEAD \
+ 'git log --max-count=1 "--pretty=format:A${SPECFILEFORMAT}O" HEAD \
>f/a/specfile.expected &&
diff f/a/specfile.expected f/a/specfile'
--
1.5.3
^ permalink raw reply related
* Re: [PATCH] Add a new lstat and fstat implementation based on Win32 API
From: Johannes Sixt @ 2007-09-06 16:18 UTC (permalink / raw)
To: Marius Storm-Olsen; +Cc: Johannes Schindelin, Johannes Sixt, Git Mailing List
In-Reply-To: <46DD0C16.70101@eudaptics.com>
Johannes Sixt schrieb:
> Marius Storm-Olsen schrieb:
>> Johannes Schindelin wrote:
>>> To make it easier on others, I just uploaded it into the "teststat"
>>> branch on 4msysgit.git (subject to removal in a few days).
>>
>> Ok, I've updated the patch in the 4msysgit.git repo, 'teststat' branch.
>> RFC, and please test.
>
> Thanks a lot! I've pushed it out in mingw.git's master.
>
> The reason that t4200-rerere.sh fails is that we now store UTC in
> st_mtime. However, for the garbage-collection we compare this entry to a
> local time stamp.
This analysis is incorrect, I think. The reason we fail seems to be that
t4200 uses test-chmtime, which uses utime(). Likely, we need a wrapper for
that one?
-- Hannes
^ 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