* [PATCHv2 1/3] read-cache: let refresh_cache_ent pass up changed flags
From: Jeff King @ 2011-11-18 11:11 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Carlos Martín Nieto, git
In-Reply-To: <20111118110938.GA5940@sigill.intra.peff.net>
This will enable refresh_cache to differentiate more cases
of modification (such as typechange) when telling the user
what isn't fresh.
Signed-off-by: Jeff King <peff@peff.net>
---
Same as 3/4 from my v1 series.
read-cache.c | 9 ++++++---
1 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/read-cache.c b/read-cache.c
index 5790a91..8e69ea3 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1001,7 +1001,8 @@ int add_index_entry(struct index_state *istate, struct cache_entry *ce, int opti
*/
static struct cache_entry *refresh_cache_ent(struct index_state *istate,
struct cache_entry *ce,
- unsigned int options, int *err)
+ unsigned int options, int *err,
+ int *changed_ret)
{
struct stat st;
struct cache_entry *updated;
@@ -1033,6 +1034,8 @@ int add_index_entry(struct index_state *istate, struct cache_entry *ce, int opti
}
changed = ie_match_stat(istate, ce, &st, options);
+ if (changed_ret)
+ *changed_ret = changed;
if (!changed) {
/*
* The path is unchanged. If we were told to ignore
@@ -1130,7 +1133,7 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char **p
if (pathspec && !match_pathspec(pathspec, ce->name, strlen(ce->name), 0, seen))
continue;
- new = refresh_cache_ent(istate, ce, options, &cache_errno);
+ new = refresh_cache_ent(istate, ce, options, &cache_errno, NULL);
if (new == ce)
continue;
if (!new) {
@@ -1157,7 +1160,7 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char **p
static struct cache_entry *refresh_cache_entry(struct cache_entry *ce, int really)
{
- return refresh_cache_ent(&the_index, ce, really, NULL);
+ return refresh_cache_ent(&the_index, ce, really, NULL, NULL);
}
static int verify_hdr(struct cache_header *hdr, unsigned long size)
--
1.7.7.3.8.g38efa
^ permalink raw reply related
* Re: [PATCH 4/4] refresh_index: notice typechanges in output
From: Jeff King @ 2011-11-18 11:09 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Carlos Martín Nieto, git
In-Reply-To: <20111115020506.GA6305@sigill.intra.peff.net>
On Mon, Nov 14, 2011 at 09:05:06PM -0500, Jeff King wrote:
> Do you want to add your patch on top, or do you want me to re-roll with
> this squashed in? I can also hold the re-roll until post-release if you
> want.
You mentioned squashing in the "what's cooking" message. Rather than
squashing just the typechange bits, how about this re-roll, which I
think is a little easier to follow:
[1/3]: read-cache: let refresh_cache_ent pass up changed flags
[2/3]: refresh_index: rename format variables
[3/3]: refresh_index: make porcelain output more specific
-Peff
^ permalink raw reply
* Fixing a broken GIT repo
From: Bart van den Burg @ 2011-11-18 10:54 UTC (permalink / raw)
To: git
Hi
I somehow managed to break my GIT repo. Whenever I try to clone or fetch
from a clean local repo, I get an error.
I'm able to go back on the server, to the very last commit where
everything works, but as soon as I make a change locally and push it, it
breaks again. I've been trying to figure out what's wrong here for about
two days now, but I'm completely lost.
Here's the steps I take to reproduce the problem.
On the server, I can reset the ref to the last working commit:
git@server:~/shifter_rai.git$ echo
"cc5693a275c25003edc77b59f5a5c603a857f711" > refs/heads/master
Then, on my local computer, I can clone fine, but after making a commit,
it breaks again:
bbu@SIT-WST-05 /d/workspace9
$ git clone git@git.samson-it.nl:/home/git/shifter_rai
Cloning into shifter_rai...
remote: Counting objects: 9557, done.
remote: Compressing objects: 100% (1887/1887), done.
remote: Total 9557 (delta 7107), reused 9397 (delta 7019)
Receiving objects: 100% (9557/9557), 2.85 MiB | 1.06 MiB/s, done.
Resolving deltas: 100% (7107/7107), done.
bbu@SIT-WST-05 /d/workspace9
$ cd shifter_rai/
bbu@SIT-WST-05 /d/workspace9/shifter_rai (master)
$ echo "test" > test
bbu@SIT-WST-05 /d/workspace9/shifter_rai (master)
$ git add test
warning: LF will be replaced by CRLF in test.
The file will have its original line endings in your working directory.
bbu@SIT-WST-05 /d/workspace9/shifter_rai (master)
$ git commit
[master 85d1ee9] test
warning: LF will be replaced by CRLF in test.
The file will have its original line endings in your working directory.
1 files changed, 1 insertions(+), 0 deletions(-)
create mode 100644 test
bbu@SIT-WST-05 /d/workspace9/shifter_rai (master)
$ git push
Counting objects: 4, done.
Delta compression using up to 4 threads.
Compressing objects: 100% (2/2), done.
Writing objects: 100% (3/3), 271 bytes, done.
Total 3 (delta 1), reused 0 (delta 0)
To git@git.samson-it.nl:/home/git/shifter_rai
cc5693a..85d1ee9 master -> master
bbu@SIT-WST-05 /d/workspace9/shifter_rai (master)
$ cd ..
bbu@SIT-WST-05 /d/workspace9
$ rm -rf shifter_rai/
bbu@SIT-WST-05 /d/workspace9
$ git clone git@git.samson-it.nl:/home/git/shifter_rai
Cloning into shifter_rai...
remote: Counting objects: 9557, done.
remote: Compressing objects: 100% (1887/1887), done.
remote: Total 9557 (delta 7107), reused 9397 (delta 7019)
Receiving objects: 100% (9557/9557), 2.85 MiB | 1.16 MiB/s, done.
Resolving deltas: 100% (7107/7107), done.
error: refs/remotes/origin/master does not point to a valid object!
error: Trying to write ref refs/heads/master with nonexistant object
85d1ee957c65485ed9c937a4f1bfdd44fda4ea35
fatal: Cannot update the ref 'HEAD'.
Needless to say, the mentioned object in fact does exist on the server:
git@server:~/shifter_rai.git$ ls -la
objects/85/d1ee957c65485ed9c937a4f1bfdd44fda4ea35
-r--r--r-- 1 git git 153 Nov 18 11:39
objects/85/d1ee957c65485ed9c937a4f1bfdd44fda4ea35
Can anyone tell me what is happening here, and how I can fix it?
^ permalink raw reply
* Re: [PATCH] receive-pack, fetch-pack: reject bogus pack that records objects twice
From: Jeff King @ 2011-11-18 10:33 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Shawn O. Pearce
In-Reply-To: <7v7h2znv36.fsf@alter.siamese.dyndns.org>
On Wed, Nov 16, 2011 at 10:04:13PM -0800, Junio C Hamano wrote:
> When receive-pack & fetch-pack are run and store the pack obtained over
> the wire to a local repository, they internally run the index-pack command
> with the --strict option. Make sure that we reject incoming packfile that
> records objects twice to avoid spreading such a damage.
If we are fixing a thin pack (which should be the case most of the
time), we are rewriting the packfile anyway. Shouldn't we just omit
the duplicate?
I guess I'm a little confused about what is generating these duplicates.
A buggy git? A malicious server? Bad luck?
-Peff
^ permalink raw reply
* Re: [PATCH v2] Makefile: add option to disable automatic dependency generation
From: Nguyen Thai Ngoc Duy @ 2011-11-18 10:07 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Junio C Hamano, Git Mailing List, Fredrik Kuivinen
In-Reply-To: <20111118095820.GF25145@elie.hsd1.il.comcast.net>
On Fri, Nov 18, 2011 at 4:58 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Now that the COMPUTE_HEADER_DEPENDENCIES feature is turned on
> automatically for compilers that support it (see v1.7.8-rc0~142^2~1,
> 2011-08-18), there is no easy way to force it off. For example,
> setting COMPUTE_HEADER_DEPENDENCIES to the empty string in config.mak
> just tells the makefile to treat it as undefined and run a test
> command to see if the -MMD option is supported.
>
> So allow setting COMPUTE_HEADER_DEPENDENCIES=no to explicitly force
> the feature off. The new semantics:
>
> - "yes" means to explicitly enable the feature
> - "no" means to disable it
> - "auto" means to autodetect
>
> The default is still "auto". Any value other than these three will
> cause the build to error out with a descriptive message so typos and
> stale settings in config.mak don't result in mysterious behavior.
>
> Makefile:1278: *** please set COMPUTE_HEADER_DEPENDENCIES to
> yes, no, or auto (not "1"). Stop.
>
> So now when someone using a compiler without -MMD support reports
> trouble building git, you can reproduce it by running "make
> COMPUTE_HEADER_DEPENDENCIES=no".
>
> Suggested-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> Improved-by: Junio C Hamano <gitster@pobox.com>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Tested-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
--
Duy
^ permalink raw reply
* [PATCH] Makefile: add missing header file dependencies
From: Jonathan Nieder @ 2011-11-18 10:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nguyen Thai Ngoc Duy, Git Mailing List, Fredrik Kuivinen
In-Reply-To: <20111118095820.GF25145@elie.hsd1.il.comcast.net>
When the streaming filter API was introduced in v1.7.7-rc0~60^2~7
(2011-05-20), we forgot to add its header to LIB_H. Most translation
units depend on streaming.h via cache.h.
v1.7.5-rc0~48 (Fix sparse warnings, 2011-03-22) introduced undeclared
dependencies by url.o on url.h and thread-utils.o on thread-utils.h.
Noticed by make CHECK_HEADER_DEPENDENCIES=1.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Some makefile buglets found while testing.
Makefile | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/Makefile b/Makefile
index ee34eab8..34ac7957 100644
--- a/Makefile
+++ b/Makefile
@@ -518,6 +518,7 @@ LIB_H += compat/win32/syslog.h
LIB_H += compat/win32/poll.h
LIB_H += compat/win32/dirent.h
LIB_H += connected.h
+LIB_H += convert.h
LIB_H += csum-file.h
LIB_H += decorate.h
LIB_H += delta.h
@@ -2009,13 +2010,13 @@ builtin/branch.o builtin/checkout.o builtin/clone.o builtin/reset.o branch.o tra
builtin/bundle.o bundle.o transport.o: bundle.h
builtin/bisect--helper.o builtin/rev-list.o bisect.o: bisect.h
builtin/clone.o builtin/fetch-pack.o transport.o: fetch-pack.h
-builtin/grep.o builtin/pack-objects.o transport-helper.o: thread-utils.h
+builtin/grep.o builtin/pack-objects.o transport-helper.o thread-utils.o: thread-utils.h
builtin/send-pack.o transport.o: send-pack.h
builtin/log.o builtin/shortlog.o: shortlog.h
builtin/prune.o builtin/reflog.o reachable.o: reachable.h
builtin/commit.o builtin/revert.o wt-status.o: wt-status.h
builtin/tar-tree.o archive-tar.o: tar.h
-connect.o transport.o http-backend.o: url.h
+connect.o transport.o url.o http-backend.o: url.h
http-fetch.o http-walker.o remote-curl.o transport.o walker.o: walker.h
http.o http-walker.o http-push.o http-fetch.o remote-curl.o: http.h url.h
--
1.7.8.rc2
^ permalink raw reply related
* [PATCH v2] Makefile: add option to disable automatic dependency generation
From: Jonathan Nieder @ 2011-11-18 9:58 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nguyen Thai Ngoc Duy, Git Mailing List, Fredrik Kuivinen
In-Reply-To: <7vty62klg9.fsf@alter.siamese.dyndns.org>
Now that the COMPUTE_HEADER_DEPENDENCIES feature is turned on
automatically for compilers that support it (see v1.7.8-rc0~142^2~1,
2011-08-18), there is no easy way to force it off. For example,
setting COMPUTE_HEADER_DEPENDENCIES to the empty string in config.mak
just tells the makefile to treat it as undefined and run a test
command to see if the -MMD option is supported.
So allow setting COMPUTE_HEADER_DEPENDENCIES=no to explicitly force
the feature off. The new semantics:
- "yes" means to explicitly enable the feature
- "no" means to disable it
- "auto" means to autodetect
The default is still "auto". Any value other than these three will
cause the build to error out with a descriptive message so typos and
stale settings in config.mak don't result in mysterious behavior.
Makefile:1278: *** please set COMPUTE_HEADER_DEPENDENCIES to
yes, no, or auto (not "1"). Stop.
So now when someone using a compiler without -MMD support reports
trouble building git, you can reproduce it by running "make
COMPUTE_HEADER_DEPENDENCIES=no".
Suggested-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Improved-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Junio C Hamano wrote:
> Eek. At least at the end user UI level, couldn't we do this as a tristate?
Nice idea. Here it is.
Makefile | 31 ++++++++++++++++++++++++-------
1 files changed, 24 insertions(+), 7 deletions(-)
diff --git a/Makefile b/Makefile
index 34ac7957..b1c80a67 100644
--- a/Makefile
+++ b/Makefile
@@ -250,6 +250,12 @@ all::
# DEFAULT_EDITOR='$GIT_FALLBACK_EDITOR',
# DEFAULT_EDITOR='"C:\Program Files\Vim\gvim.exe" --nofork'
#
+# Define COMPUTE_HEADER_DEPENDENCIES to "yes" if you want dependencies on
+# header files to be automatically computed, to avoid rebuilding objects when
+# an unrelated header file changes. Define it to "no" to use the hard-coded
+# dependency rules. The default is "auto", which means to use computed header
+# dependencies if your compiler is detected to support it.
+#
# Define CHECK_HEADER_DEPENDENCIES to check for problems in the hard-coded
# dependency rules.
#
@@ -1246,21 +1252,32 @@ endif
endif
ifdef CHECK_HEADER_DEPENDENCIES
-COMPUTE_HEADER_DEPENDENCIES =
+COMPUTE_HEADER_DEPENDENCIES = no
USE_COMPUTED_HEADER_DEPENDENCIES =
-else
+endif
+
ifndef COMPUTE_HEADER_DEPENDENCIES
+COMPUTE_HEADER_DEPENDENCIES = auto
+endif
+
+ifeq ($(COMPUTE_HEADER_DEPENDENCIES),auto)
dep_check = $(shell $(CC) $(ALL_CFLAGS) \
-c -MF /dev/null -MMD -MP -x c /dev/null -o /dev/null 2>&1; \
echo $$?)
ifeq ($(dep_check),0)
-COMPUTE_HEADER_DEPENDENCIES=YesPlease
-endif
+override COMPUTE_HEADER_DEPENDENCIES = yes
+else
+override COMPUTE_HEADER_DEPENDENCIES = no
endif
endif
-ifdef COMPUTE_HEADER_DEPENDENCIES
+ifeq ($(COMPUTE_HEADER_DEPENDENCIES),yes)
USE_COMPUTED_HEADER_DEPENDENCIES = YesPlease
+else
+ifneq ($(COMPUTE_HEADER_DEPENDENCIES),no)
+$(error please set COMPUTE_HEADER_DEPENDENCIES to yes, no, or auto \
+(not "$(COMPUTE_HEADER_DEPENDENCIES)"))
+endif
endif
ifdef SANE_TOOL_PATH
@@ -1907,7 +1924,7 @@ OBJECTS := $(GIT_OBJS) $(XDIFF_OBJS) $(VCSSVN_OBJS)
dep_files := $(foreach f,$(OBJECTS),$(dir $f).depend/$(notdir $f).d)
dep_dirs := $(addsuffix .depend,$(sort $(dir $(OBJECTS))))
-ifdef COMPUTE_HEADER_DEPENDENCIES
+ifeq ($(COMPUTE_HEADER_DEPENDENCIES),yes)
$(dep_dirs):
@mkdir -p $@
@@ -1920,7 +1937,7 @@ Please unset CHECK_HEADER_DEPENDENCIES and try again)
endif
endif
-ifndef COMPUTE_HEADER_DEPENDENCIES
+ifneq ($(COMPUTE_HEADER_DEPENDENCIES),yes)
ifndef CHECK_HEADER_DEPENDENCIES
dep_dirs =
missing_dep_dirs =
--
1.7.8.rc2
^ permalink raw reply related
* Re: [PATCH 8/8] Enable GIT_DEBUG_MEMCHECK on git_pathname()
From: Junio C Hamano @ 2011-11-18 7:35 UTC (permalink / raw)
To: Johan Herland
Cc: Jonathan Nieder, Nguyen Thai Ngoc Duy, Jeff King,
Ramkumar Ramachandra, git, Junio C Hamano, Michael Haggerty
In-Reply-To: <CALKQrgfTKmSd8se3n3xq89SXRmNPm3qz3Ckv2mUghot8kStKxA@mail.gmail.com>
Thanks, both. Will queue.
^ permalink raw reply
* [PATCH 3/3] bulk-checkin: do not write an object that already exists
From: Junio C Hamano @ 2011-11-18 7:11 UTC (permalink / raw)
To: git
In-Reply-To: <1321600317-30546-1-git-send-email-gitster@pobox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
bulk-checkin.c | 12 +++++++++++-
t/t1050-large.sh | 18 +++++++++++++++++-
2 files changed, 28 insertions(+), 2 deletions(-)
diff --git a/bulk-checkin.c b/bulk-checkin.c
index 82166ba..60178ef 100644
--- a/bulk-checkin.c
+++ b/bulk-checkin.c
@@ -29,7 +29,11 @@ static void finish_bulk_checkin(struct bulk_checkin_state *state)
if (!state->f)
return;
- if (state->nr_written == 1) {
+ if (state->nr_written == 0) {
+ close(state->f->fd);
+ unlink(state->pack_tmp_name);
+ goto clear_exit;
+ } else if (state->nr_written == 1) {
sha1close(state->f, sha1, CSUM_FSYNC);
} else {
int fd = sha1close(state->f, sha1, 0);
@@ -45,6 +49,8 @@ static void finish_bulk_checkin(struct bulk_checkin_state *state)
&state->pack_idx_opts, sha1);
for (i = 0; i < state->nr_written; i++)
free(state->written[i]);
+
+clear_exit:
free(state->written);
memset(state, 0, sizeof(*state));
@@ -56,6 +62,10 @@ static int already_written(struct bulk_checkin_state *state, unsigned char sha1[
{
int i;
+ /* The object may already exist in the repository */
+ if (has_sha1_file(sha1))
+ return 1;
+
/* Might want to keep the list sorted */
for (i = 0; i < state->nr_written; i++)
if (!hashcmp(state->written[i]->sha1, sha1))
diff --git a/t/t1050-large.sh b/t/t1050-large.sh
index fbd5ced..0726472 100755
--- a/t/t1050-large.sh
+++ b/t/t1050-large.sh
@@ -9,6 +9,7 @@ test_expect_success setup '
git config core.bigfilethreshold 200k &&
echo X | dd of=large1 bs=1k seek=2000 &&
echo X | dd of=large2 bs=1k seek=2000 &&
+ echo X | dd of=large3 bs=1k seek=2000 &&
echo Y | dd of=huge bs=1k seek=2500
'
@@ -34,7 +35,22 @@ test_expect_success 'add a large file or two' '
test -f "$l" || continue
bad=t
done &&
- test -z "$bad"
+ test -z "$bad" &&
+
+ # attempt to add another copy of the same
+ git add large3 &&
+ bad= count=0 &&
+ for p in .git/objects/pack/pack-*.pack
+ do
+ count=$(( $count + 1 ))
+ if test -f "$p" && idx=${p%.pack}.idx && test -f "$idx"
+ then
+ continue
+ fi
+ bad=t
+ done &&
+ test -z "$bad" &&
+ test $count = 1
'
test_expect_success 'checkout a large file' '
--
1.7.8.rc3.111.g7d421
^ permalink raw reply related
* [PATCH 1/3] csum-file: introduce sha1file_checkpoint
From: Junio C Hamano @ 2011-11-18 7:11 UTC (permalink / raw)
To: git
In-Reply-To: <1321600317-30546-1-git-send-email-gitster@pobox.com>
It is useful to be able to rewind a check-summed file to a certain
previous state after writing data into it using sha1write() API. The
fast-import command does this after streaming a blob data to the packfile
being generated and then noticing that the same blob has already been
written, and it does this with a private code truncate_pack() that is
commented as "Yes, this is a layering violation".
Introduce two API functions, sha1file_checkpoint(), that allows the caller
to save a state of a sha1file, and then later revert it to the saved state.
Use it to reimplement truncate_pack().
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
csum-file.c | 20 ++++++++++++++++++++
csum-file.h | 9 +++++++++
fast-import.c | 25 ++++++++-----------------
3 files changed, 37 insertions(+), 17 deletions(-)
diff --git a/csum-file.c b/csum-file.c
index fc97d6e..53f5375 100644
--- a/csum-file.c
+++ b/csum-file.c
@@ -158,6 +158,26 @@ struct sha1file *sha1fd_throughput(int fd, const char *name, struct progress *tp
return f;
}
+void sha1file_checkpoint(struct sha1file *f, struct sha1file_checkpoint *checkpoint)
+{
+ sha1flush(f);
+ checkpoint->offset = f->total;
+ checkpoint->ctx = f->ctx;
+}
+
+int sha1file_truncate(struct sha1file *f, struct sha1file_checkpoint *checkpoint)
+{
+ off_t offset = checkpoint->offset;
+
+ if (ftruncate(f->fd, offset) ||
+ lseek(f->fd, offset, SEEK_SET) != offset)
+ return -1;
+ f->total = offset;
+ f->ctx = checkpoint->ctx;
+ f->offset = 0; /* sha1flush() was called in checkpoint */
+ return 0;
+}
+
void crc32_begin(struct sha1file *f)
{
f->crc32 = crc32(0, NULL, 0);
diff --git a/csum-file.h b/csum-file.h
index 6a7967c..3b540bd 100644
--- a/csum-file.h
+++ b/csum-file.h
@@ -17,6 +17,15 @@ struct sha1file {
unsigned char buffer[8192];
};
+/* Checkpoint */
+struct sha1file_checkpoint {
+ off_t offset;
+ git_SHA_CTX ctx;
+};
+
+extern void sha1file_checkpoint(struct sha1file *, struct sha1file_checkpoint *);
+extern int sha1file_truncate(struct sha1file *, struct sha1file_checkpoint *);
+
/* sha1close flags */
#define CSUM_CLOSE 1
#define CSUM_FSYNC 2
diff --git a/fast-import.c b/fast-import.c
index 8d8ea3c..a8db41b 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1143,17 +1143,11 @@ static int store_object(
return 0;
}
-static void truncate_pack(off_t to, git_SHA_CTX *ctx)
+static void truncate_pack(struct sha1file_checkpoint *checkpoint)
{
- if (ftruncate(pack_data->pack_fd, to)
- || lseek(pack_data->pack_fd, to, SEEK_SET) != to)
+ if (sha1file_truncate(pack_file, checkpoint))
die_errno("cannot truncate pack to skip duplicate");
- pack_size = to;
-
- /* yes this is a layering violation */
- pack_file->total = to;
- pack_file->offset = 0;
- pack_file->ctx = *ctx;
+ pack_size = checkpoint->offset;
}
static void stream_blob(uintmax_t len, unsigned char *sha1out, uintmax_t mark)
@@ -1166,8 +1160,8 @@ static void stream_blob(uintmax_t len, unsigned char *sha1out, uintmax_t mark)
unsigned long hdrlen;
off_t offset;
git_SHA_CTX c;
- git_SHA_CTX pack_file_ctx;
git_zstream s;
+ struct sha1file_checkpoint checkpoint;
int status = Z_OK;
/* Determine if we should auto-checkpoint. */
@@ -1175,11 +1169,8 @@ static void stream_blob(uintmax_t len, unsigned char *sha1out, uintmax_t mark)
|| (pack_size + 60 + len) < pack_size)
cycle_packfile();
- offset = pack_size;
-
- /* preserve the pack_file SHA1 ctx in case we have to truncate later */
- sha1flush(pack_file);
- pack_file_ctx = pack_file->ctx;
+ sha1file_checkpoint(pack_file, &checkpoint);
+ offset = checkpoint.offset;
hdrlen = snprintf((char *)out_buf, out_sz, "blob %" PRIuMAX, len) + 1;
if (out_sz <= hdrlen)
@@ -1245,14 +1236,14 @@ static void stream_blob(uintmax_t len, unsigned char *sha1out, uintmax_t mark)
if (e->idx.offset) {
duplicate_count_by_type[OBJ_BLOB]++;
- truncate_pack(offset, &pack_file_ctx);
+ truncate_pack(&checkpoint);
} else if (find_sha1_pack(sha1, packed_git)) {
e->type = OBJ_BLOB;
e->pack_id = MAX_PACK_ID;
e->idx.offset = 1; /* just not zero! */
duplicate_count_by_type[OBJ_BLOB]++;
- truncate_pack(offset, &pack_file_ctx);
+ truncate_pack(&checkpoint);
} else {
e->depth = 0;
--
1.7.8.rc3.111.g7d421
^ permalink raw reply related
* [PATCH 2/3] bulk-checkin: do not write the same object twice
From: Junio C Hamano @ 2011-11-18 7:11 UTC (permalink / raw)
To: git
In-Reply-To: <1321600317-30546-1-git-send-email-gitster@pobox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
bulk-checkin.c | 28 ++++++++++++++++++++++++----
t/t1050-large.sh | 20 +++++++++++++-------
2 files changed, 37 insertions(+), 11 deletions(-)
diff --git a/bulk-checkin.c b/bulk-checkin.c
index c7e693e..82166ba 100644
--- a/bulk-checkin.c
+++ b/bulk-checkin.c
@@ -52,6 +52,17 @@ static void finish_bulk_checkin(struct bulk_checkin_state *state)
reprepare_packed_git();
}
+static int already_written(struct bulk_checkin_state *state, unsigned char sha1[])
+{
+ int i;
+
+ /* Might want to keep the list sorted */
+ for (i = 0; i < state->nr_written; i++)
+ if (!hashcmp(state->written[i]->sha1, sha1))
+ return 1;
+ return 0;
+}
+
static void deflate_to_pack(struct bulk_checkin_state *state,
unsigned char sha1[],
int fd, size_t size, enum object_type type,
@@ -64,6 +75,7 @@ static void deflate_to_pack(struct bulk_checkin_state *state,
int write_object = (flags & HASH_WRITE_OBJECT);
int status = Z_OK;
struct pack_idx_entry *idx = NULL;
+ struct sha1file_checkpoint checkpoint;
hdrlen = sprintf((char *)obuf, "%s %" PRIuMAX,
typename(type), (uintmax_t)size) + 1;
@@ -73,6 +85,7 @@ static void deflate_to_pack(struct bulk_checkin_state *state,
if (write_object) {
idx = xcalloc(1, sizeof(*idx));
idx->offset = state->offset;
+ sha1file_checkpoint(state->f, &checkpoint);
crc32_begin(state->f);
}
memset(&s, 0, sizeof(s));
@@ -121,10 +134,17 @@ static void deflate_to_pack(struct bulk_checkin_state *state,
git_SHA1_Final(sha1, &ctx);
if (write_object) {
idx->crc32 = crc32_end(state->f);
- hashcpy(idx->sha1, sha1);
- ALLOC_GROW(state->written,
- state->nr_written + 1, state->alloc_written);
- state->written[state->nr_written++] = idx;
+
+ if (already_written(state, sha1)) {
+ sha1file_truncate(state->f, &checkpoint);
+ state->offset = checkpoint.offset;
+ free(idx);
+ } else {
+ hashcpy(idx->sha1, sha1);
+ ALLOC_GROW(state->written,
+ state->nr_written + 1, state->alloc_written);
+ state->written[state->nr_written++] = idx;
+ }
}
}
diff --git a/t/t1050-large.sh b/t/t1050-large.sh
index 36def25..fbd5ced 100755
--- a/t/t1050-large.sh
+++ b/t/t1050-large.sh
@@ -7,22 +7,28 @@ test_description='adding and checking out large blobs'
test_expect_success setup '
git config core.bigfilethreshold 200k &&
- echo X | dd of=large bs=1k seek=2000 &&
+ echo X | dd of=large1 bs=1k seek=2000 &&
+ echo X | dd of=large2 bs=1k seek=2000 &&
echo Y | dd of=huge bs=1k seek=2500
'
test_expect_success 'add a large file or two' '
- git add large huge &&
+ git add large1 huge large2 &&
# make sure we got a single packfile and no loose objects
- bad= count=0 &&
+ bad= count=0 idx= &&
for p in .git/objects/pack/pack-*.pack
do
count=$(( $count + 1 ))
- test -f "$p" && continue
+ if test -f "$p" && idx=${p%.pack}.idx && test -f "$idx"
+ then
+ continue
+ fi
bad=t
done &&
test -z "$bad" &&
test $count = 1 &&
+ cnt=$(git show-index <"$idx" | wc -l) &&
+ test $cnt = 2 &&
for l in .git/objects/??/??????????????????????????????????????
do
test -f "$l" || continue
@@ -32,10 +38,10 @@ test_expect_success 'add a large file or two' '
'
test_expect_success 'checkout a large file' '
- large=$(git rev-parse :large) &&
- git update-index --add --cacheinfo 100644 $large another &&
+ large1=$(git rev-parse :large1) &&
+ git update-index --add --cacheinfo 100644 $large1 another &&
git checkout another &&
- cmp large another ;# this must not be test_cmp
+ cmp large1 another ;# this must not be test_cmp
'
test_done
--
1.7.8.rc3.111.g7d421
^ permalink raw reply related
* [PATCH 0/3] bulk-checkin continued
From: Junio C Hamano @ 2011-11-18 7:11 UTC (permalink / raw)
To: git
In-Reply-To: <1319846051-462-1-git-send-email-gitster@pobox.com>
This updates the earlier bulk-checkin series ($gmane/184440) to further
enhance the "large file" topic from 1.7.6 cycle.
The first one adds two API functions to allow truncating a checksummed
file that is being written. This is the same patch as the one I sent
earlier today.
The second one prevents the bulk-checkin code from writing the same object
twice to the stream, and the third one further makes it notice that the
object it has just written already exists in the repository. In either
case, the packfile is rewound using the new sha1file_checkpoint/truncate
API, and the packfile itself is removed if truncation results in an empty
output.
The next step is to add the "split-blob" entry in the packfile, but that
is a much larger task and will take longer.
Junio C Hamano (3):
csum-file: introduce sha1file_checkpoint
bulk-checkin: do not write the same object twice
bulk-checkin: do not write an object that already exists
bulk-checkin.c | 40 +++++++++++++++++++++++++++++++++++-----
csum-file.c | 20 ++++++++++++++++++++
csum-file.h | 9 +++++++++
fast-import.c | 25 ++++++++-----------------
t/t1050-large.sh | 38 ++++++++++++++++++++++++++++++--------
5 files changed, 102 insertions(+), 30 deletions(-)
--
1.7.8.rc3.111.g7d421
^ permalink raw reply
* Re: [PATCH 8/8] Enable GIT_DEBUG_MEMCHECK on git_pathname()
From: Jonathan Nieder @ 2011-11-18 6:52 UTC (permalink / raw)
To: Johan Herland
Cc: Nguyen Thai Ngoc Duy, Jeff King, Ramkumar Ramachandra, git,
Junio C Hamano, Michael Haggerty
In-Reply-To: <CALKQrgfTKmSd8se3n3xq89SXRmNPm3qz3Ckv2mUghot8kStKxA@mail.gmail.com>
Johan Herland wrote:
> Acked-by: Johan Herland <johan@herland.net>
Thanks for looking it over.
^ permalink raw reply
* Re: [PATCH] Makefile: add option to disable automatic dependency generation
From: Jonathan Nieder @ 2011-11-18 6:21 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nguyen Thai Ngoc Duy, Git Mailing List, Fredrik Kuivinen
In-Reply-To: <7vty62klg9.fsf@alter.siamese.dyndns.org>
Junio C Hamano wrote:
> Eek. At least at the end user UI level, couldn't we do this as a tristate?
> E.g. "YesPlease" (or anything that begins with Y if you are ambitious) to
> explicitly enable, empty (or "auto") to autodetect, and anything else to
> decline?
Ah, I didn't mind the UI so much. Handling
COMPUTE_HEADER_DEPENDENCIES_FORCE = (yes | no | auto)
should be doable. I'd suggest making any other value error out, so
typos don't result in mysterious behavior.
> Even better, couldn't we either (1) rearrange .dep/ files somehow, so that
> compiler difference does not matter
Yes, I'm working on an incantation all the compilers like (it
shouldn't be hard --- adding an -MQ option should be enough, but I
want to understand the bug first). But even with such a fix, I think
it will be important to have a way to turn the feature off. When
someone using a compiler without -MMD support reports a bug, wouldn't
it be nice to be able to reproduce it?
^ permalink raw reply
* Re: [PATCH 8/8] Enable GIT_DEBUG_MEMCHECK on git_pathname()
From: Johan Herland @ 2011-11-18 6:16 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Nguyen Thai Ngoc Duy, Jeff King, Ramkumar Ramachandra, git,
Junio C Hamano, Michael Haggerty
In-Reply-To: <20111118012746.GA22485@elie.hsd1.il.comcast.net>
On Fri, Nov 18, 2011 at 02:27, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Subject: notes merge: eliminate OUTPUT macro
>
> The macro is variadic, which breaks support for pre-C99 compilers,
> and it hides an "if", which can make code hard to understand on
> first reading if some arguments have side-effects.
>
> The OUTPUT macro seems to have been inspired by the "output" function
> from merge-recursive. But that function in merge-recursive exists to
> indent output based on the level of recursion and there is no similar
> justification for such a function in "notes merge".
>
> Noticed with 'make CC="gcc -std=c89 -pedantic"':
>
> notes-merge.c:24:22: warning: anonymous variadic macros were introduced in C99 [-Wvariadic-macros]
>
> Encouraged-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Acked-by: Johan Herland <johan@herland.net>
--
Johan Herland, <johan@herland.net>
www.herland.net
^ permalink raw reply
* Re: [PATCH] Makefile: add option to disable automatic dependency generation
From: Junio C Hamano @ 2011-11-18 6:12 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Nguyen Thai Ngoc Duy, Git Mailing List, Fredrik Kuivinen
In-Reply-To: <20111118045742.GA25145@elie.hsd1.il.comcast.net>
Jonathan Nieder <jrnieder@gmail.com> writes:
> Duy noticed that now that the COMPUTE_HEADER_DEPENDENCIES feature is
> turned on automatically for compilers that support it (see
> v1.7.8-rc0~142^2~1, 2011-08-18), there is no easy way to force it off.
> For example, setting COMPUTE_HEADER_DEPENDENCIES to the empty string
> in config.mak just tells the makefile to treat it as undefined and
> run a test command to see if the -MMD option is supported.
>
> Introduce a new NO_COMPUTE_HEADER_DEPENDENCIES variable that forces
> the feature off.
Eek. At least at the end user UI level, couldn't we do this as a tristate?
E.g. "YesPlease" (or anything that begins with Y if you are ambitious) to
explicitly enable, empty (or "auto") to autodetect, and anything else to
decline?
Even better, couldn't we either (1) rearrange .dep/ files somehow, so that
compiler difference does not matter, or (2) have dep_check to perform a
trial run to detect versions of compilers that produce the output that we
cannot use?
^ permalink raw reply
* Re: t/t1304: avoid -d option to setfacl (db82657)
From: Brandon Casey @ 2011-11-18 5:37 UTC (permalink / raw)
To: wsp; +Cc: git
In-Reply-To: <gitster/git/commit/db826571e4099066fe44233d95642591016c831b/729354@github.com>
[cc git@vger.kernel.org since that is the appropriate place for this discusion]
On Thu, Nov 17, 2011 at 8:59 PM, wsp
<reply+c-729354-3460aca0fa61e627f9d1a271cf70a99d5c1e7e4e-921167@reply.github.com>
wrote:
> Could this test case be reviewed again? It fails on FreeBSD where the appropriate way to specify default ACL's is with the "-d" option. The "d[efault]:" syntax is invalid on FreeBSD.
Well, I'm not sure there is a right answer here.
Linux accepts -d or "d[efault]"
Solaris only accepts "d[efault]"
FreeBSD only accepts -d
a quick search shows z/OS only accepts "d[efault]"
other?
I think most everything else rolls their own implementation into chmod
or chacl or something like that.
The abandoned POSIX draft does actually specify the FreeBSD behavior.
So I think it's kind of a toss-up. Which option we choose should
probably depend on whether we get more test coverage by using the
"d[efault]" notation or by using the -d option. That depends on
whether there are more Solaris users compiling git or whether there
are more FreeBSD users. I don't know the answer to that either. I
tend to think there are very few of either.
-Brandon
^ permalink raw reply
* test t/t1304-default-acl.sh
From: Scott Parrish @ 2011-11-18 5:10 UTC (permalink / raw)
To: Git Mailing List
Hi folks,
This test case fails on FreeBSD 8.2. Specifically, the use of the "d[efault]:" syntax in the ACL tag is invalid on FreeBSD. The manpage indicates that default ACL's should be set via the "-d" command line option to setfacl. See http://www.FreeBSD.org/cgi/man.cgi?query=setfacl&apropos=0&sektion=0&manpath=FreeBSD+8.2-RELEASE&arch=default&format=html for details.
Note that the test used "-d" at one time but appears to have been changed to use "d:" to accommodate Solaris where "-d" has the effect of deleting ACL entries. The change was made on this commit: https://github.com/gitster/git/commit/db826571e4099066fe44233d95642591016c831b
So I don't think it is as straight-forward as reverting to the use of "-d".
Thoughts?
Cheers,
-Scott
^ permalink raw reply
* Re: [PATCH] Makefile: add option to disable automatic dependency generation
From: Jonathan Nieder @ 2011-11-18 5:00 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: Git Mailing List, Fredrik Kuivinen
In-Reply-To: <20111118045742.GA25145@elie.hsd1.il.comcast.net>
Jonathan Nieder wrote:
> - If COMPUTE_HEADER_DEPENDENCIES is empty and NO_COMPUTE_... is
> nonempty, the build uses gcc's on-the-fly dependency generation
> feature.
Erm. This should say:
"- If COMPUTE_HEADER_DEPENDENCIES is nonempty and NO_COMPUTE_... is
empty,"
Sorry for the noise.
^ permalink raw reply
* [PATCH] Makefile: add option to disable automatic dependency generation
From: Jonathan Nieder @ 2011-11-18 4:57 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: Git Mailing List, Fredrik Kuivinen
In-Reply-To: <CACsJy8A44PFtYrm8NQU+48sVkOe8mjJyO9opO5-TwRtAd-TKsQ@mail.gmail.com>
Duy noticed that now that the COMPUTE_HEADER_DEPENDENCIES feature is
turned on automatically for compilers that support it (see
v1.7.8-rc0~142^2~1, 2011-08-18), there is no easy way to force it off.
For example, setting COMPUTE_HEADER_DEPENDENCIES to the empty string
in config.mak just tells the makefile to treat it as undefined and
run a test command to see if the -MMD option is supported.
Introduce a new NO_COMPUTE_HEADER_DEPENDENCIES variable that forces
the feature off. The new semantics:
- If NO_COMPUTE_HEADER_DEPENDENCIES is set to a nonempty string,
the -MMD option will not be used. The build relies on hard-coded
dependencies in the "ifndef USE_COMPUTED_HEADER_DEPENDENCIES"
section of the Makefile instead.
- If COMPUTE_HEADER_DEPENDENCIES is empty and NO_COMPUTE_... is
nonempty, the build uses gcc's on-the-fly dependency generation
feature.
- If neither is nonempty, the makefile runs a quick test command
to decide whether the compiler supports the -MMD option and
whether to enable this feature.
Inspired-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Nguyen Thai Ngoc Duy wrote:
> Hmm.. I guess it's my compiler's fault then. Next question, how can I
> disable this feature?
"make COMPUTE_HEADER_DEPENDENCIES=" works. But there's no way aside
from "override COMPUTE_HEADER_DEPENDENCIES=" to do that in config.mak.
And yuck.
By the way, I'm not convinced it's your compiler's fault. It might be
my compiler's fault. More reading to do...
Makefile | 15 ++++++++++++++-
1 files changed, 14 insertions(+), 1 deletions(-)
diff --git a/Makefile b/Makefile
index ee34eab8..e4a658f6 100644
--- a/Makefile
+++ b/Makefile
@@ -250,6 +250,13 @@ all::
# DEFAULT_EDITOR='$GIT_FALLBACK_EDITOR',
# DEFAULT_EDITOR='"C:\Program Files\Vim\gvim.exe" --nofork'
#
+# Define COMPUTE_HEADER_DEPENDENCIES if your compiler supports the -MMD option
+# and you want to avoid rebuilding objects when an unrelated header file
+# changes.
+#
+# Define NO_COMPUTE_HEADER_DEPENDENCIES if you want to disable automatic
+# dependency generation even though your compiler is detected to support it.
+#
# Define CHECK_HEADER_DEPENDENCIES to check for problems in the hard-coded
# dependency rules.
#
@@ -1245,10 +1252,16 @@ endif
endif
ifdef CHECK_HEADER_DEPENDENCIES
+NO_COMPUTE_HEADER_DEPENDENCIES = YesPlease
+endif
+
+ifdef NO_COMPUTE_HEADER_DEPENDENCIES
COMPUTE_HEADER_DEPENDENCIES =
USE_COMPUTED_HEADER_DEPENDENCIES =
-else
+endif
+
ifndef COMPUTE_HEADER_DEPENDENCIES
+ifndef NO_COMPUTE_HEADER_DEPENDENCIES
dep_check = $(shell $(CC) $(ALL_CFLAGS) \
-c -MF /dev/null -MMD -MP -x c /dev/null -o /dev/null 2>&1; \
echo $$?)
--
1.7.8.rc2
^ permalink raw reply related
* Re: A flaw in dep generation with gcc -MMD?
From: Miles Bader @ 2011-11-18 4:49 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: Jonathan Nieder, Git Mailing List
In-Reply-To: <CACsJy8A44PFtYrm8NQU+48sVkOe8mjJyO9opO5-TwRtAd-TKsQ@mail.gmail.com>
Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
>> Interesting. What compiler do you use?
>
> $ gcc --version
> gcc (Gentoo 4.4.4-r2 p1.2, pie-0.4.5) 4.4.4
FWIW, gcc 4.4.6 on debian does the correct thing too...
-Miles
--
Yo mama's so fat when she gets on an elevator it HAS to go down.
^ permalink raw reply
* Re: Git Gems
From: Ramkumar Ramachandra @ 2011-11-18 4:38 UTC (permalink / raw)
To: Hilco Wijbenga; +Cc: Git List
In-Reply-To: <CAE1pOi1gyshz_502NQvLNAByfwiYXW2fzA+EnGKz8tuFrCpkxg@mail.gmail.com>
Hi Hilco,
Hilco Wijbenga wrote:
> [CC: Git Users <git@vger.kernel.org>]
For the record, this is the official Git list: both "developers" and
"users" hang out here.
> [...]
> As an example, 'git rebase' didn't really seem useful until I
> understood (well, more or less) what it did. Until then, I just
> naively assumed that merge commits and non-linear history were
> something you simply had to live with. I'm guessing that, like me, a
> lot of people come to Git with quite a few assumptions and
> preconceived notions due to their exposure to other SCM tools. :-(
I'm not sure how a listing is going to help; nevertheless, here are a
few of the lesser-known features of the top of my head (in no
particular order): rerere, attributes, replace refs, filter-branch,
blame's -C and -M switches, log's -S switch, custom diff drivers,
bundle, submodule, stash, notes, and reflog.
[You can find more by digging through the sources]
Which brings us to an interesting aside: unlike many other SCMs which
have a definite and finite set of features, git is really just a
toolkit that grows everyday- various people use various subsets and
write up their own custom scripts to help them automate tasks. The
"git rev-list/ git rev-parse/ git cat-file" is an awesome trio to
start writing shell scripts with :)
Cheers.
-- Ram
^ permalink raw reply
* Re: A flaw in dep generation with gcc -MMD?
From: Nguyen Thai Ngoc Duy @ 2011-11-18 3:56 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Git Mailing List
In-Reply-To: <20111118034142.GA25228@elie.hsd1.il.comcast.net>
On Fri, Nov 18, 2011 at 10:41 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Nguyen Thai Ngoc Duy wrote:
>
>> My builtin/.depend/add.o.d says
>>
>> add.o: .... cache.h ...
>
> Interesting. What compiler do you use?
$ gcc --version
gcc (Gentoo 4.4.4-r2 p1.2, pie-0.4.5) 4.4.4
> $ head -1 builtin/.depend/add.o.d
> builtin/add.o: builtin/add.c cache.h git-compat-util.h compat/bswap.h \
> $ gcc --version | head -1
> gcc (Debian 4.6.2-4) 4.6.2
Hmm.. I guess it's my compiler's fault then. Next question, how can I
disable this feature?
--
Duy
^ permalink raw reply
* Re: A flaw in dep generation with gcc -MMD?
From: Jonathan Nieder @ 2011-11-18 3:41 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: Git Mailing List
In-Reply-To: <CACsJy8BZMDyf4MCiKxPJ5Z+XS+C-MC82SpMFyWgiXmb9xCnScw@mail.gmail.com>
Nguyen Thai Ngoc Duy wrote:
> My builtin/.depend/add.o.d says
>
> add.o: .... cache.h ...
Interesting. What compiler do you use?
Thanks for finding it,
Jonathan
$ head -1 builtin/.depend/add.o.d
builtin/add.o: builtin/add.c cache.h git-compat-util.h compat/bswap.h \
$ gcc --version | head -1
gcc (Debian 4.6.2-4) 4.6.2
^ permalink raw reply
* Re: [PATCH 0/3] avoiding unintended consequences of git_path() usage
From: Nguyen Thai Ngoc Duy @ 2011-11-18 3:33 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Ramkumar Ramachandra, Junio C Hamano, Git List
In-Reply-To: <20111116075955.GB13706@elie.hsd1.il.comcast.net>
On Wed, Nov 16, 2011 at 01:59:55AM -0600, Jonathan Nieder wrote:
> Ramkumar Ramachandra wrote:
> > Junio C Hamano wrote:
>
> >> Or perhaps http://thread.gmane.org/gmane.comp.version-control.git/184963/focus=185436
> >
> > I noticed that sha1_to_hex() also operates like this. The
> > resolve_ref() function is really important, but using the same
> > technique for these tiny functions is probably an overkill
>
> I don't follow. Do you mean that not being confusing is overkill,
> because the function is small that no one will bother to look up the
> right semantics? Wait, that sentence didn't come out the way I
> wanted. ;-)
>
> Jokes aside, here's a rough series to do the git_path ->
> git_path_unsafe renaming. While writing it, I noticed a couple of
> bugs, hence the two patches before the last one. Patch 2 is the more
> interesting one.
Or perhaps we can use per-file buffer rings instead of a global one.
This means git_path() can only interfere another one in the same file,
making the interaction simpler and hopefully simple enough for reviewers
to catch 90% bugs, therefore safe enough to avoid the _unsafe suffix.
Adding static variable declaration in cache.h is ugly, but that could be
moved to a separate header file.
diff --git a/cache.h b/cache.h
index 2e6ad36..437bc3a 100644
--- a/cache.h
+++ b/cache.h
@@ -660,9 +660,13 @@ extern char *git_snpath(char *buf, size_t n, const char *fmt, ...)
extern char *git_pathdup(const char *fmt, ...)
__attribute__((format (printf, 1, 2)));
+#define git_path(...) git_path_1(pathname_array[3 & ++pathname_index], __VA_ARGS__)
+static char pathname_array[4][PATH_MAX];
+static int pathname_index;
+
/* Return a statically allocated filename matching the sha1 signature */
extern char *mkpath(const char *fmt, ...) __attribute__((format (printf, 1, 2)));
-extern char *git_path(const char *fmt, ...) __attribute__((format (printf, 1, 2)));
+extern char *git_path_1(char *pathname, const char *fmt, ...) __attribute__((format (printf, 2, 3)));
extern char *git_path_submodule(const char *path, const char *fmt, ...)
__attribute__((format (printf, 2, 3)));
diff --git a/path.c b/path.c
index b6f71d1..3c95db1 100644
--- a/path.c
+++ b/path.c
@@ -101,10 +101,9 @@ char *mkpath(const char *fmt, ...)
return cleanup_path(pathname);
}
-char *git_path(const char *fmt, ...)
+char *git_path_1(char *pathname, const char *fmt, ...)
{
const char *git_dir = get_git_dir();
- char *pathname = get_pathname();
va_list args;
unsigned len;
^ permalink raw reply related
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