* [PATCH v3 2/2] Reduce parse-options.o dependencies
From: Dmitry Ivankov @ 2011-08-11 9:15 UTC (permalink / raw)
To: git; +Cc: Jonathan Nieder, David Barr, Dmitry Ivankov
In-Reply-To: <1313054138-30885-1-git-send-email-divanorama@gmail.com>
Currently parse-options.o pulls quite a big bunch of dependencies.
his complicates it's usage in contrib/ because it pulls external
dependencies and it also increases executables size.
Split off less generic and more internal to git part of
parse-options.c to parse-options-cb.c.
Move prefix_filename function from setup.c to abspath.c. abspath.o
and wrapper.o pull each other, so it's unlikely to increase the
dependencies. It was a dependency of parse-options.o that pulled
many others.
Now parse-options.o pulls just abspath.o, ctype.o, strbuf.o, usage.o,
wrapper.o, libc directly and strlcpy.o indirectly.
Signed-off-by: Dmitry Ivankov <divanorama@gmail.com>
---
Makefile | 3 +-
abspath.c | 28 ++++++++++++
parse-options-cb.c | 125 ++++++++++++++++++++++++++++++++++++++++++++++++++++
parse-options.c | 121 --------------------------------------------------
setup.c | 28 ------------
5 files changed, 155 insertions(+), 150 deletions(-)
create mode 100644 parse-options-cb.c
diff --git a/Makefile b/Makefile
index 62ad0c2..7d47bdb 100644
--- a/Makefile
+++ b/Makefile
@@ -642,6 +642,7 @@ LIB_OBJS += pack-revindex.o
LIB_OBJS += pack-write.o
LIB_OBJS += pager.o
LIB_OBJS += parse-options.o
+LIB_OBJS += parse-options-cb.o
LIB_OBJS += patch-delta.o
LIB_OBJS += patch-ids.o
LIB_OBJS += path.o
@@ -2204,7 +2205,7 @@ test-delta$X: diff-delta.o patch-delta.o
test-line-buffer$X: vcs-svn/lib.a
-test-parse-options$X: parse-options.o
+test-parse-options$X: parse-options.o parse-options-cb.o
test-string-pool$X: vcs-svn/lib.a
diff --git a/abspath.c b/abspath.c
index 37287f8..f04ac18 100644
--- a/abspath.c
+++ b/abspath.c
@@ -139,3 +139,31 @@ const char *absolute_path(const char *path)
}
return buf;
}
+
+/*
+ * Unlike prefix_path, this should be used if the named file does
+ * not have to interact with index entry; i.e. name of a random file
+ * on the filesystem.
+ */
+const char *prefix_filename(const char *pfx, int pfx_len, const char *arg)
+{
+ static char path[PATH_MAX];
+#ifndef WIN32
+ if (!pfx_len || is_absolute_path(arg))
+ return arg;
+ memcpy(path, pfx, pfx_len);
+ strcpy(path + pfx_len, arg);
+#else
+ char *p;
+ /* don't add prefix to absolute paths, but still replace '\' by '/' */
+ if (is_absolute_path(arg))
+ pfx_len = 0;
+ else if (pfx_len)
+ memcpy(path, pfx, pfx_len);
+ strcpy(path + pfx_len, arg);
+ for (p = path + pfx_len; *p; p++)
+ if (*p == '\\')
+ *p = '/';
+#endif
+ return path;
+}
diff --git a/parse-options-cb.c b/parse-options-cb.c
new file mode 100644
index 0000000..c248f66
--- /dev/null
+++ b/parse-options-cb.c
@@ -0,0 +1,125 @@
+#include "git-compat-util.h"
+#include "parse-options.h"
+#include "cache.h"
+#include "commit.h"
+#include "color.h"
+#include "string-list.h"
+
+/*----- some often used options -----*/
+
+int parse_opt_abbrev_cb(const struct option *opt, const char *arg, int unset)
+{
+ int v;
+
+ if (!arg) {
+ v = unset ? 0 : DEFAULT_ABBREV;
+ } else {
+ v = strtol(arg, (char **)&arg, 10);
+ if (*arg)
+ return opterror(opt, "expects a numerical value", 0);
+ if (v && v < MINIMUM_ABBREV)
+ v = MINIMUM_ABBREV;
+ else if (v > 40)
+ v = 40;
+ }
+ *(int *)(opt->value) = v;
+ return 0;
+}
+
+int parse_opt_approxidate_cb(const struct option *opt, const char *arg,
+ int unset)
+{
+ *(unsigned long *)(opt->value) = approxidate(arg);
+ return 0;
+}
+
+int parse_opt_color_flag_cb(const struct option *opt, const char *arg,
+ int unset)
+{
+ int value;
+
+ if (!arg)
+ arg = unset ? "never" : (const char *)opt->defval;
+ value = git_config_colorbool(NULL, arg, -1);
+ if (value < 0)
+ return opterror(opt,
+ "expects \"always\", \"auto\", or \"never\"", 0);
+ *(int *)opt->value = value;
+ return 0;
+}
+
+int parse_opt_verbosity_cb(const struct option *opt, const char *arg,
+ int unset)
+{
+ int *target = opt->value;
+
+ if (unset)
+ /* --no-quiet, --no-verbose */
+ *target = 0;
+ else if (opt->short_name == 'v') {
+ if (*target >= 0)
+ (*target)++;
+ else
+ *target = 1;
+ } else {
+ if (*target <= 0)
+ (*target)--;
+ else
+ *target = -1;
+ }
+ return 0;
+}
+
+int parse_opt_with_commit(const struct option *opt, const char *arg, int unset)
+{
+ unsigned char sha1[20];
+ struct commit *commit;
+
+ if (!arg)
+ return -1;
+ if (get_sha1(arg, sha1))
+ return error("malformed object name %s", arg);
+ commit = lookup_commit_reference(sha1);
+ if (!commit)
+ return error("no such commit %s", arg);
+ commit_list_insert(commit, opt->value);
+ return 0;
+}
+
+int parse_opt_tertiary(const struct option *opt, const char *arg, int unset)
+{
+ int *target = opt->value;
+ *target = unset ? 2 : 1;
+ return 0;
+}
+
+int parse_options_concat(struct option *dst, size_t dst_size, struct option *src)
+{
+ int i, j;
+
+ for (i = 0; i < dst_size; i++)
+ if (dst[i].type == OPTION_END)
+ break;
+ for (j = 0; i < dst_size; i++, j++) {
+ dst[i] = src[j];
+ if (src[j].type == OPTION_END)
+ return 0;
+ }
+ return -1;
+}
+
+int parse_opt_string_list(const struct option *opt, const char *arg, int unset)
+{
+ struct string_list *v = opt->value;
+
+ if (unset) {
+ string_list_clear(v, 0);
+ return 0;
+ }
+
+ if (!arg)
+ return -1;
+
+ string_list_append(v, xstrdup(arg));
+ return 0;
+}
diff --git a/parse-options.c b/parse-options.c
index 7b061af..503ab5d 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -3,7 +3,6 @@
#include "cache.h"
#include "commit.h"
#include "color.h"
-#include "string-list.h"
static int parse_options_usage(struct parse_opt_ctx_t *ctx,
const char * const *usagestr,
@@ -584,123 +583,3 @@ static int parse_options_usage(struct parse_opt_ctx_t *ctx,
return usage_with_options_internal(ctx, usagestr, opts, 0, err);
}
-
-/*----- some often used options -----*/
-#include "cache.h"
-
-int parse_opt_abbrev_cb(const struct option *opt, const char *arg, int unset)
-{
- int v;
-
- if (!arg) {
- v = unset ? 0 : DEFAULT_ABBREV;
- } else {
- v = strtol(arg, (char **)&arg, 10);
- if (*arg)
- return opterror(opt, "expects a numerical value", 0);
- if (v && v < MINIMUM_ABBREV)
- v = MINIMUM_ABBREV;
- else if (v > 40)
- v = 40;
- }
- *(int *)(opt->value) = v;
- return 0;
-}
-
-int parse_opt_approxidate_cb(const struct option *opt, const char *arg,
- int unset)
-{
- *(unsigned long *)(opt->value) = approxidate(arg);
- return 0;
-}
-
-int parse_opt_color_flag_cb(const struct option *opt, const char *arg,
- int unset)
-{
- int value;
-
- if (!arg)
- arg = unset ? "never" : (const char *)opt->defval;
- value = git_config_colorbool(NULL, arg, -1);
- if (value < 0)
- return opterror(opt,
- "expects \"always\", \"auto\", or \"never\"", 0);
- *(int *)opt->value = value;
- return 0;
-}
-
-int parse_opt_verbosity_cb(const struct option *opt, const char *arg,
- int unset)
-{
- int *target = opt->value;
-
- if (unset)
- /* --no-quiet, --no-verbose */
- *target = 0;
- else if (opt->short_name == 'v') {
- if (*target >= 0)
- (*target)++;
- else
- *target = 1;
- } else {
- if (*target <= 0)
- (*target)--;
- else
- *target = -1;
- }
- return 0;
-}
-
-int parse_opt_with_commit(const struct option *opt, const char *arg, int unset)
-{
- unsigned char sha1[20];
- struct commit *commit;
-
- if (!arg)
- return -1;
- if (get_sha1(arg, sha1))
- return error("malformed object name %s", arg);
- commit = lookup_commit_reference(sha1);
- if (!commit)
- return error("no such commit %s", arg);
- commit_list_insert(commit, opt->value);
- return 0;
-}
-
-int parse_opt_tertiary(const struct option *opt, const char *arg, int unset)
-{
- int *target = opt->value;
- *target = unset ? 2 : 1;
- return 0;
-}
-
-int parse_options_concat(struct option *dst, size_t dst_size, struct option *src)
-{
- int i, j;
-
- for (i = 0; i < dst_size; i++)
- if (dst[i].type == OPTION_END)
- break;
- for (j = 0; i < dst_size; i++, j++) {
- dst[i] = src[j];
- if (src[j].type == OPTION_END)
- return 0;
- }
- return -1;
-}
-
-int parse_opt_string_list(const struct option *opt, const char *arg, int unset)
-{
- struct string_list *v = opt->value;
-
- if (unset) {
- string_list_clear(v, 0);
- return 0;
- }
-
- if (!arg)
- return -1;
-
- string_list_append(v, xstrdup(arg));
- return 0;
-}
diff --git a/setup.c b/setup.c
index 5ea5502..3463819 100644
--- a/setup.c
+++ b/setup.c
@@ -40,34 +40,6 @@ char *prefix_path(const char *prefix, int len, const char *path)
return sanitized;
}
-/*
- * Unlike prefix_path, this should be used if the named file does
- * not have to interact with index entry; i.e. name of a random file
- * on the filesystem.
- */
-const char *prefix_filename(const char *pfx, int pfx_len, const char *arg)
-{
- static char path[PATH_MAX];
-#ifndef WIN32
- if (!pfx_len || is_absolute_path(arg))
- return arg;
- memcpy(path, pfx, pfx_len);
- strcpy(path + pfx_len, arg);
-#else
- char *p;
- /* don't add prefix to absolute paths, but still replace '\' by '/' */
- if (is_absolute_path(arg))
- pfx_len = 0;
- else if (pfx_len)
- memcpy(path, pfx, pfx_len);
- strcpy(path + pfx_len, arg);
- for (p = path + pfx_len; *p; p++)
- if (*p == '\\')
- *p = '/';
-#endif
- return path;
-}
-
int check_filename(const char *prefix, const char *arg)
{
const char *name;
--
1.7.3.4
^ permalink raw reply related
* Re: git-mergetool: wrap tools with 3 files only to use the BASE file instead of MERGED
From: David Aguilar @ 2011-08-11 9:37 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Tanguy Ortolo, git, Sebastian Schuberth, Charles Bailey
In-Reply-To: <20110811084955.GA31233@elie.gateway.2wire.net>
On Thu, Aug 11, 2011 at 03:49:55AM -0500, Jonathan Nieder wrote:
> David Aguilar wrote:
>
> > I think it sounds like a good thing for certain tools.
> > Sebastian mentioned it being fine in ecmerge and bc3.
> > xxdiff also lets you specify the output file, so it
> > probably wouldn't need it either, I think.
>
> At the risk of taking away the itch for a good feature: meld joined
> the crowd of tools with -o to specify an output file in v1.5.0.
>
> http://thread.gmane.org/gmane.comp.gnome.meld.general/1270
What's the best way to use this and not break existing users?
meld v1.5.0 was released in December.
We could parse `meld --version` and use the new --output flag on
newer setups, leaving old setups alone. That's a lot of
code to carry around but it's nicest to users.
Here's another idea..
mergetool--lib has vimdiff and vimdiff2. Maybe we can add a new
meld3 tool that uses the --output flag? Users with older setups
are unaffected. Documentation is changed to mention meld v1.5.0
and the meld3 tool. New users have to configure :-/
Alternatively, rename the existing meld to meld2 and let the
new style call take over the current meld tool.
Older setups with new git that use meld can adjust their
config.
(and the final option: refactor mergetool--lib into separate
files. Yes, we should do that too! ;-))
what do you think?
--
David
^ permalink raw reply
* Re: git-mergetool: wrap tools with 3 files only to use the BASE file instead of MERGED
From: Sebastian Schuberth @ 2011-08-11 9:48 UTC (permalink / raw)
To: David Aguilar; +Cc: Jonathan Nieder, Tanguy Ortolo, git, Charles Bailey
In-Reply-To: <20110811093731.GB29507@gmail.com>
On Thu, Aug 11, 2011 at 11:37, David Aguilar <davvid@gmail.com> wrote:
> What's the best way to use this and not break existing users?
>
> meld v1.5.0 was released in December.
> We could parse `meld --version` and use the new --output flag on
> newer setups, leaving old setups alone. That's a lot of
> code to carry around but it's nicest to users.
I'd definitely prefer this solution, as it's the most convenient for users.
--
Sebastian Schuberth
^ permalink raw reply
* Re: [PATCH 3/5] revert: Allow mixed pick and revert instructions
From: Ramkumar Ramachandra @ 2011-08-11 9:50 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Git List, Junio C Hamano, Christian Couder, Daniel Barkalow,
Jeff King
In-Reply-To: <20110810151527.GC31315@elie.gateway.2wire.net>
Hi Jonathan,
I fixed everything; just a few comments/ doubts:
Jonathan Nieder writes:
>> } else
>> - return NULL;
>> + return -1;
>
> Unrelated to this patch: maybe
>
> return error("unrecognized action in sequencer file: %s", start);
>
> to be easier to debug.
I'd initially refrained from doing this so that errors don't become
overtly verbose, but I suppose it's alright considering the fact that
we're going to make the instruction sheet editable sometime in the
future. I tweaked the error strings a little so that we get something
like:
error: Unrecognized action: part
error: Could not parse line 3.
fatal: Unusable instruction sheet: .git/sequencer
In essence, I didn't want to be redundant and mention the sequencer in
every line. I like the above.
>> q = strchr(p, ' ');
>> if (!q)
>> - return NULL;
>> + return -1;
>
> So we reject "pick a87c8989"? That's a shame.
Good point. Fixing this will have to be in another patch, where I'll
advertise the fact that I'm changing the instruction sheet format.
>> q++;
>>
>> strlcpy(sha1_abbrev, p, q - p);
>
> memcpy would be clearer. Can't this overflow the sha1_abbrev buffer?
Good point. I'm tempted to check (q - p < 40); is there a better way
to do this by not hardcoding "40" perhaps?
> Acked-by: Jonathan Nieder <jrnieder@gmail.com>
Thanks.
-- Ram
^ permalink raw reply
* Re: [PATCH 3/5] revert: Allow mixed pick and revert instructions
From: Jonathan Nieder @ 2011-08-11 10:08 UTC (permalink / raw)
To: Ramkumar Ramachandra
Cc: Git List, Junio C Hamano, Christian Couder, Daniel Barkalow,
Jeff King
In-Reply-To: <CALkWK0mMVgnngU5JB4O+7crCjGCdRnVrbm4jjsgkFFUMeBS9_A@mail.gmail.com>
Ramkumar Ramachandra wrote:
> Good point. I'm tempted to check (q - p < 40); is there a better way
> to do this by not hardcoding "40" perhaps?
Something like the following seems idiomatic.
if (q - p + 1 > sizeof(buf))
return error(...);
memcpy(buf, p, q - p);
buf[q - p] = '\0';
^ permalink raw reply
* [PATCH v2 0/5] fixes for committer/author parsing/check
From: Dmitry Ivankov @ 2011-08-11 10:21 UTC (permalink / raw)
To: git; +Cc: SASAKI Suguru, Junio C Hamano, Dmitry Ivankov
This is a second version of [1]. It features one more test for a
fsck message and reworked commit messages.
Aside from fixing a parsing bug in fast-import and improving error
messages in git-fsck it makes them both to deny "name> <email>"
identities.
[1] http://thread.gmane.org/gmane.comp.version-control.git/178035
Dmitry Ivankov (5):
fast-import: add input format tests
fast-import: don't fail on omitted committer name
fast-import: check committer name more strictly
fsck: add a few committer name tests
fsck: improve committer/author check
Documentation/git-fast-import.txt | 4 +-
fast-import.c | 33 ++++++++-----
fsck.c | 10 ++--
t/t1450-fsck.sh | 36 +++++++++++++
t/t9300-fast-import.sh | 99 +++++++++++++++++++++++++++++++++++++
5 files changed, 164 insertions(+), 18 deletions(-)
--
1.7.3.4
^ permalink raw reply
* [PATCH v2 1/5] fast-import: add input format tests
From: Dmitry Ivankov @ 2011-08-11 10:21 UTC (permalink / raw)
To: git; +Cc: SASAKI Suguru, Junio C Hamano, Dmitry Ivankov
In-Reply-To: <1313058070-4774-1-git-send-email-divanorama@gmail.com>
Documentation/git-fast-import.txt says that git-fast-import is strict
about it's input format. But committer/author field parsing is a bit
loose. Invalid values can be unnoticed and written out to the commit,
either with format-conforming input or with non-format-conforming one.
Add one passing and one failing test for empty/absent committer name
with well-formed input. And a failed test with unnoticed ill-formed
input.
Reported-by: SASAKI Suguru <sss.sonik@gmail.com>
Signed-off-by: Dmitry Ivankov <divanorama@gmail.com>
---
t/t9300-fast-import.sh | 99 ++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 99 insertions(+), 0 deletions(-)
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index f256475..0844e9d 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -324,6 +324,105 @@ test_expect_success \
test `git rev-parse master` = `git rev-parse TEMP_TAG^`'
rm -f .git/TEMP_TAG
+git gc 2>/dev/null >/dev/null
+git prune 2>/dev/null >/dev/null
+
+cat >input <<INPUT_END
+commit refs/heads/empty-committer-1
+committer <> $GIT_COMMITTER_DATE
+data <<COMMIT
+empty commit
+COMMIT
+INPUT_END
+test_expect_success 'B: accept empty committer' '
+ git fast-import <input &&
+ out=$(git fsck) &&
+ echo "$out" &&
+ test -z "$out"
+'
+git update-ref -d refs/heads/empty-committer-1 || true
+
+git gc 2>/dev/null >/dev/null
+git prune 2>/dev/null >/dev/null
+
+cat >input <<INPUT_END
+commit refs/heads/empty-committer-2
+committer <a@b.com> $GIT_COMMITTER_DATE
+data <<COMMIT
+empty commit
+COMMIT
+INPUT_END
+test_expect_failure 'B: accept and fixup committer with no name' '
+ git fast-import <input &&
+ out=$(git fsck) &&
+ echo "$out" &&
+ test -z "$out"
+'
+git update-ref -d refs/heads/empty-committer-2 || true
+
+git gc 2>/dev/null >/dev/null
+git prune 2>/dev/null >/dev/null
+
+cat >input <<INPUT_END
+commit refs/heads/invalid-committer
+committer Name email> $GIT_COMMITTER_DATE
+data <<COMMIT
+empty commit
+COMMIT
+INPUT_END
+test_expect_failure 'B: fail on invalid committer (1)' '
+ test_must_fail git fast-import <input
+'
+git update-ref -d refs/heads/invalid-committer || true
+
+cat >input <<INPUT_END
+commit refs/heads/invalid-committer
+committer Name <e<mail> $GIT_COMMITTER_DATE
+data <<COMMIT
+empty commit
+COMMIT
+INPUT_END
+test_expect_failure 'B: fail on invalid committer (2)' '
+ test_must_fail git fast-import <input
+'
+git update-ref -d refs/heads/invalid-committer || true
+
+cat >input <<INPUT_END
+commit refs/heads/invalid-committer
+committer Name <email>> $GIT_COMMITTER_DATE
+data <<COMMIT
+empty commit
+COMMIT
+INPUT_END
+test_expect_failure 'B: fail on invalid committer (3)' '
+ test_must_fail git fast-import <input
+'
+git update-ref -d refs/heads/invalid-committer || true
+
+cat >input <<INPUT_END
+commit refs/heads/invalid-committer
+committer Name <email $GIT_COMMITTER_DATE
+data <<COMMIT
+empty commit
+COMMIT
+INPUT_END
+test_expect_failure 'B: fail on invalid committer (4)' '
+ test_must_fail git fast-import <input
+'
+git update-ref -d refs/heads/invalid-committer || true
+
+cat >input <<INPUT_END
+commit refs/heads/invalid-committer
+committer Name<email> $GIT_COMMITTER_DATE
+data <<COMMIT
+empty commit
+COMMIT
+INPUT_END
+test_expect_failure 'B: fail on invalid committer (5)' '
+ test_must_fail git fast-import <input
+'
+git update-ref -d refs/heads/invalid-committer || true
+
###
### series C
###
--
1.7.3.4
^ permalink raw reply related
* [PATCH v2 2/5] fast-import: don't fail on omitted committer name
From: Dmitry Ivankov @ 2011-08-11 10:21 UTC (permalink / raw)
To: git; +Cc: SASAKI Suguru, Junio C Hamano, Dmitry Ivankov
In-Reply-To: <1313058070-4774-1-git-send-email-divanorama@gmail.com>
fast-import format declares 'committer_name SP' to be optional in
'committer_name SP LT email GT'. But for a (commit) object SP is
obligatory while zero length committer_name is ok. git-fsck checks
that SP is present, so fast-import must prepend it if the name SP
part is omitted. It doesn't do so and thus for "LT email GT" ident
it writes a bad object.
Name cannot contain LT or GT, ident always comes after SP in fast-import.
So if ident starts with LT reuse the SP as if a valid 'SP LT email GT'
ident was passed.
This fixes a ident parsing bug for a well-formed fast-import input.
Though the parsing is still loose and can accept a ill-formed input.
Signed-off-by: Dmitry Ivankov <divanorama@gmail.com>
---
fast-import.c | 4 ++++
t/t9300-fast-import.sh | 2 +-
2 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/fast-import.c b/fast-import.c
index 7cc2262..ed1f7c9 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1973,6 +1973,10 @@ static char *parse_ident(const char *buf)
size_t name_len;
char *ident;
+ /* ensure there is a space delimiter even if there is no name */
+ if (*buf == '<')
+ --buf;
+
gt = strrchr(buf, '>');
if (!gt)
die("Missing > in ident string: %s", buf);
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index 0844e9d..8f3938c 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -352,7 +352,7 @@ data <<COMMIT
empty commit
COMMIT
INPUT_END
-test_expect_failure 'B: accept and fixup committer with no name' '
+test_expect_success 'B: accept and fixup committer with no name' '
git fast-import <input &&
out=$(git fsck) &&
echo "$out" &&
--
1.7.3.4
^ permalink raw reply related
* [PATCH v2 5/5] fsck: improve committer/author check
From: Dmitry Ivankov @ 2011-08-11 10:21 UTC (permalink / raw)
To: git; +Cc: SASAKI Suguru, Junio C Hamano, Dmitry Ivankov
In-Reply-To: <1313058070-4774-1-git-send-email-divanorama@gmail.com>
fsck allows a name with > character in it like "name> <email>". Also for
"name email>" fsck says "missing space before email".
More precisely, it seeks for a first '<', checks that ' ' preceeds it.
Then seeks to '<' or '>' and checks that it is the '>'. Missing space is
reported if either '<' is not found or it's not preceeded with ' '.
Change it to following. Seek to '<' or '>', check that it is '<' and is
preceeded with ' '. Seek to '<' or '>' and check that it is '>'. So now
"name> <email>" is rejected as "bad name". More strict name check is the
only change in what is accepted.
Report 'missing space' only if '<' is found and is not preceeded with a
space.
Signed-off-by: Dmitry Ivankov <divanorama@gmail.com>
---
fsck.c | 10 ++++++----
t/t1450-fsck.sh | 6 +++---
2 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/fsck.c b/fsck.c
index 60bd4bb..6c855f8 100644
--- a/fsck.c
+++ b/fsck.c
@@ -224,13 +224,15 @@ static int fsck_tree(struct tree *item, int strict, fsck_error error_func)
static int fsck_ident(char **ident, struct object *obj, fsck_error error_func)
{
- if (**ident == '<' || **ident == '\n')
- return error_func(obj, FSCK_ERROR, "invalid author/committer line - missing space before email");
- *ident += strcspn(*ident, "<\n");
- if ((*ident)[-1] != ' ')
+ if (**ident == '<')
return error_func(obj, FSCK_ERROR, "invalid author/committer line - missing space before email");
+ *ident += strcspn(*ident, "<>\n");
+ if (**ident == '>')
+ return error_func(obj, FSCK_ERROR, "invalid author/committer line - bad name");
if (**ident != '<')
return error_func(obj, FSCK_ERROR, "invalid author/committer line - missing email");
+ if ((*ident)[-1] != ' ')
+ return error_func(obj, FSCK_ERROR, "invalid author/committer line - missing space before email");
(*ident)++;
*ident += strcspn(*ident, "<>\n");
if (**ident != '>')
diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index 01ccefd..523ce9c 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -110,7 +110,7 @@ test_expect_success 'email with embedded > is not okay' '
grep "error in commit $new" out
'
-test_expect_failure 'missing < email delimiter is reported nicely' '
+test_expect_success 'missing < email delimiter is reported nicely' '
git cat-file commit HEAD >basis &&
sed "s/<//" basis >bad-email-2 &&
new=$(git hash-object -t commit -w --stdin <bad-email-2) &&
@@ -122,7 +122,7 @@ test_expect_failure 'missing < email delimiter is reported nicely' '
grep "error in commit $new.* - bad name" out
'
-test_expect_failure 'missing email is reported nicely' '
+test_expect_success 'missing email is reported nicely' '
git cat-file commit HEAD >basis &&
sed "s/[a-z]* <[^>]*>//" basis >bad-email-3 &&
new=$(git hash-object -t commit -w --stdin <bad-email-3) &&
@@ -134,7 +134,7 @@ test_expect_failure 'missing email is reported nicely' '
grep "error in commit $new.* - missing email" out
'
-test_expect_failure '> in name is reported' '
+test_expect_success '> in name is reported' '
git cat-file commit HEAD >basis &&
sed "s/ </> </" basis >bad-email-4 &&
new=$(git hash-object -t commit -w --stdin <bad-email-4) &&
--
1.7.3.4
^ permalink raw reply related
* [PATCH v2 3/5] fast-import: check committer name more strictly
From: Dmitry Ivankov @ 2011-08-11 10:21 UTC (permalink / raw)
To: git; +Cc: SASAKI Suguru, Junio C Hamano, Dmitry Ivankov
In-Reply-To: <1313058070-4774-1-git-send-email-divanorama@gmail.com>
The documentation declares following identity format:
(<name> SP)? LT <email> GT
where name is any string without LF and LT characters.
But fast-import just accepts any string up to first GT
instead of checking the whole format, and moreover just
writes it as is to the commit object.
git-fsck checks for [^<\n]* <[^<>\n]*> format. Note that the
space is mandatory. And the space quirk is already handled via
extending the string to the left when needed.
Modify fast-import input identity format to a slightly stricter
one - deny LF, LT and GT in both <name> and <email>. And check
for it.
This is stricter then git-fsck as fsck accepts "Name> <email>"
currently, but soon fsck check will be adjusted likewise.
Signed-off-by: Dmitry Ivankov <divanorama@gmail.com>
---
Documentation/git-fast-import.txt | 4 ++--
fast-import.c | 29 +++++++++++++++++------------
t/t9300-fast-import.sh | 10 +++++-----
3 files changed, 24 insertions(+), 19 deletions(-)
diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt
index 2969388..ba16889 100644
--- a/Documentation/git-fast-import.txt
+++ b/Documentation/git-fast-import.txt
@@ -425,8 +425,8 @@ Here `<name>` is the person's display name (for example
(``cm@example.com''). `LT` and `GT` are the literal less-than (\x3c)
and greater-than (\x3e) symbols. These are required to delimit
the email address from the other fields in the line. Note that
-`<name>` is free-form and may contain any sequence of bytes, except
-`LT` and `LF`. It is typically UTF-8 encoded.
+`<name>` and `<email>` are free-form and may contain any sequence
+of bytes, except `LT`, `GT` and `LF`. `<name>` is typically UTF-8 encoded.
The time of the change is specified by `<when>` using the date format
that was selected by the \--date-format=<fmt> command line option.
diff --git a/fast-import.c b/fast-import.c
index ed1f7c9..6d491b9 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1969,7 +1969,7 @@ static int validate_raw_date(const char *src, char *result, int maxlen)
static char *parse_ident(const char *buf)
{
- const char *gt;
+ const char *ltgt;
size_t name_len;
char *ident;
@@ -1977,28 +1977,33 @@ static char *parse_ident(const char *buf)
if (*buf == '<')
--buf;
- gt = strrchr(buf, '>');
- if (!gt)
+ ltgt = buf + strcspn(buf, "<>");
+ if (*ltgt != '<')
+ die("Missing < in ident string: %s", buf);
+ if (ltgt != buf && ltgt[-1] != ' ')
+ die("Missing space before < in ident string: %s", buf);
+ ltgt = ltgt + 1 + strcspn(ltgt + 1, "<>");
+ if (*ltgt != '>')
die("Missing > in ident string: %s", buf);
- gt++;
- if (*gt != ' ')
+ ltgt++;
+ if (*ltgt != ' ')
die("Missing space after > in ident string: %s", buf);
- gt++;
- name_len = gt - buf;
+ ltgt++;
+ name_len = ltgt - buf;
ident = xmalloc(name_len + 24);
strncpy(ident, buf, name_len);
switch (whenspec) {
case WHENSPEC_RAW:
- if (validate_raw_date(gt, ident + name_len, 24) < 0)
- die("Invalid raw date \"%s\" in ident: %s", gt, buf);
+ if (validate_raw_date(ltgt, ident + name_len, 24) < 0)
+ die("Invalid raw date \"%s\" in ident: %s", ltgt, buf);
break;
case WHENSPEC_RFC2822:
- if (parse_date(gt, ident + name_len, 24) < 0)
- die("Invalid rfc2822 date \"%s\" in ident: %s", gt, buf);
+ if (parse_date(ltgt, ident + name_len, 24) < 0)
+ die("Invalid rfc2822 date \"%s\" in ident: %s", ltgt, buf);
break;
case WHENSPEC_NOW:
- if (strcmp("now", gt))
+ if (strcmp("now", ltgt))
die("Date in ident must be 'now': %s", buf);
datestamp(ident + name_len, 24);
break;
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index 8f3938c..e53ca90 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -370,7 +370,7 @@ data <<COMMIT
empty commit
COMMIT
INPUT_END
-test_expect_failure 'B: fail on invalid committer (1)' '
+test_expect_success 'B: fail on invalid committer (1)' '
test_must_fail git fast-import <input
'
git update-ref -d refs/heads/invalid-committer || true
@@ -382,7 +382,7 @@ data <<COMMIT
empty commit
COMMIT
INPUT_END
-test_expect_failure 'B: fail on invalid committer (2)' '
+test_expect_success 'B: fail on invalid committer (2)' '
test_must_fail git fast-import <input
'
git update-ref -d refs/heads/invalid-committer || true
@@ -394,7 +394,7 @@ data <<COMMIT
empty commit
COMMIT
INPUT_END
-test_expect_failure 'B: fail on invalid committer (3)' '
+test_expect_success 'B: fail on invalid committer (3)' '
test_must_fail git fast-import <input
'
git update-ref -d refs/heads/invalid-committer || true
@@ -406,7 +406,7 @@ data <<COMMIT
empty commit
COMMIT
INPUT_END
-test_expect_failure 'B: fail on invalid committer (4)' '
+test_expect_success 'B: fail on invalid committer (4)' '
test_must_fail git fast-import <input
'
git update-ref -d refs/heads/invalid-committer || true
@@ -418,7 +418,7 @@ data <<COMMIT
empty commit
COMMIT
INPUT_END
-test_expect_failure 'B: fail on invalid committer (5)' '
+test_expect_success 'B: fail on invalid committer (5)' '
test_must_fail git fast-import <input
'
git update-ref -d refs/heads/invalid-committer || true
--
1.7.3.4
^ permalink raw reply related
* [PATCH v2 4/5] fsck: add a few committer name tests
From: Dmitry Ivankov @ 2011-08-11 10:21 UTC (permalink / raw)
To: git; +Cc: SASAKI Suguru, Junio C Hamano, Dmitry Ivankov
In-Reply-To: <1313058070-4774-1-git-send-email-divanorama@gmail.com>
fsck reports "missing space before <email>" for committer string equal
to "name email>" or to "". It'd be nicer to say "missing email" for
the second string and "name is bad" (has > in it) for the first one.
Add a failing test for these messages.
For "name> <email>" no error is reported. Looks like a bug, so add
such a failing test."
Signed-off-by: Dmitry Ivankov <divanorama@gmail.com>
---
t/t1450-fsck.sh | 36 ++++++++++++++++++++++++++++++++++++
1 files changed, 36 insertions(+), 0 deletions(-)
diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index bb01d5a..01ccefd 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -110,6 +110,42 @@ test_expect_success 'email with embedded > is not okay' '
grep "error in commit $new" out
'
+test_expect_failure 'missing < email delimiter is reported nicely' '
+ git cat-file commit HEAD >basis &&
+ sed "s/<//" basis >bad-email-2 &&
+ new=$(git hash-object -t commit -w --stdin <bad-email-2) &&
+ test_when_finished "remove_object $new" &&
+ git update-ref refs/heads/bogus "$new" &&
+ test_when_finished "git update-ref -d refs/heads/bogus" &&
+ git fsck 2>out &&
+ cat out &&
+ grep "error in commit $new.* - bad name" out
+'
+
+test_expect_failure 'missing email is reported nicely' '
+ git cat-file commit HEAD >basis &&
+ sed "s/[a-z]* <[^>]*>//" basis >bad-email-3 &&
+ new=$(git hash-object -t commit -w --stdin <bad-email-3) &&
+ test_when_finished "remove_object $new" &&
+ git update-ref refs/heads/bogus "$new" &&
+ test_when_finished "git update-ref -d refs/heads/bogus" &&
+ git fsck 2>out &&
+ cat out &&
+ grep "error in commit $new.* - missing email" out
+'
+
+test_expect_failure '> in name is reported' '
+ git cat-file commit HEAD >basis &&
+ sed "s/ </> </" basis >bad-email-4 &&
+ new=$(git hash-object -t commit -w --stdin <bad-email-4) &&
+ test_when_finished "remove_object $new" &&
+ git update-ref refs/heads/bogus "$new" &&
+ test_when_finished "git update-ref -d refs/heads/bogus" &&
+ git fsck 2>out &&
+ cat out &&
+ grep "error in commit $new" out
+'
+
test_expect_success 'tag pointing to nonexistent' '
cat >invalid-tag <<-\EOF &&
object ffffffffffffffffffffffffffffffffffffffff
--
1.7.3.4
^ permalink raw reply related
* Re: [PATCH v3 1/2] parse-options: export opterr, optbug
From: Jonathan Nieder @ 2011-08-11 10:42 UTC (permalink / raw)
To: Dmitry Ivankov; +Cc: git, David Barr, Stephen Boyd
In-Reply-To: <1313054138-30885-2-git-send-email-divanorama@gmail.com>
(+cc: Stephen)
Dmitry Ivankov wrote:
> opterror and optbug functions are used by some of parsing routines
> in parse-options.c to report errors and bugs respectively.
>
> Export these functions to allow more custom parsing routines to use
> them in a uniform way.
In other words, exposing opterror() allows custom option types to
behave more like the built-in ones when producing messages like
"option `<opt>' expects a numerical value". What should they pass
in the "flags" argument? Does this deserve a mention in the
"Option Callbacks" section of
Documentation/technical/api-parse-options.txt?
Would opterror() be enough? I don't see any current users of optbug
outside of parse_options_check() (which is part of low-level
machinery).
Aside from that, seems sensible.
[quoting in full for Stephen's convenience. One quick comment
below.]
>
> Signed-off-by: Dmitry Ivankov <divanorama@gmail.com>
> ---
> parse-options.c | 4 ++--
> parse-options.h | 2 ++
> 2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/parse-options.c b/parse-options.c
> index 879ea82..7b061af 100644
> --- a/parse-options.c
> +++ b/parse-options.c
> @@ -12,14 +12,14 @@ static int parse_options_usage(struct parse_opt_ctx_t *ctx,
> #define OPT_SHORT 1
> #define OPT_UNSET 2
>
> -static int optbug(const struct option *opt, const char *reason)
> +int optbug(const struct option *opt, const char *reason)
> {
> if (opt->long_name)
> return error("BUG: option '%s' %s", opt->long_name, reason);
> return error("BUG: switch '%c' %s", opt->short_name, reason);
> }
>
> -static int opterror(const struct option *opt, const char *reason, int flags)
> +int opterror(const struct option *opt, const char *reason, int flags)
> {
> if (flags & OPT_SHORT)
> return error("switch `%c' %s", opt->short_name, reason);
> diff --git a/parse-options.h b/parse-options.h
> index 05eb09b..59e0b52 100644
> --- a/parse-options.h
> +++ b/parse-options.h
> @@ -165,6 +165,8 @@ extern NORETURN void usage_msg_opt(const char *msg,
> const char * const *usagestr,
> const struct option *options);
>
> +extern int optbug(const struct option *opt, const char *reason);
> +extern int opterror(const struct option *opt, const char *reason, int flags);
> /*----- incremental advanced APIs -----*/
A blank line above the comment would make this more readable.
>
> enum {
> --
Except as noted above,
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Some (though not all) of the possible changes noted above implemented
below, plus an example caller (which made reading the patch a little
easier). What do you think?
builtin/read-tree.c | 2 +-
parse-options.c | 2 +-
parse-options.h | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git c/builtin/read-tree.c i/builtin/read-tree.c
index df6c4c88..6013090c 100644
--- c/builtin/read-tree.c
+++ i/builtin/read-tree.c
@@ -53,7 +53,7 @@ static int exclude_per_directory_cb(const struct option *opt, const char *arg,
opts = (struct unpack_trees_options *)opt->value;
if (opts->dir)
- die("more than one --exclude-per-directory given.");
+ return opterror(opt, "can only be supplied once", 0);
dir = xcalloc(1, sizeof(*opts->dir));
dir->flags |= DIR_SHOW_IGNORED;
diff --git c/parse-options.c i/parse-options.c
index 7b061afc..777611b1 100644
--- c/parse-options.c
+++ i/parse-options.c
@@ -12,7 +12,7 @@ static int parse_options_usage(struct parse_opt_ctx_t *ctx,
#define OPT_SHORT 1
#define OPT_UNSET 2
-int optbug(const struct option *opt, const char *reason)
+static int optbug(const struct option *opt, const char *reason)
{
if (opt->long_name)
return error("BUG: option '%s' %s", opt->long_name, reason);
diff --git c/parse-options.h i/parse-options.h
index 59e0b524..6d31ad3a 100644
--- c/parse-options.h
+++ i/parse-options.h
@@ -165,8 +165,8 @@ extern NORETURN void usage_msg_opt(const char *msg,
const char * const *usagestr,
const struct option *options);
-extern int optbug(const struct option *opt, const char *reason);
extern int opterror(const struct option *opt, const char *reason, int flags);
+
/*----- incremental advanced APIs -----*/
enum {
^ permalink raw reply related
* Re: [PATCH v3 2/2] Reduce parse-options.o dependencies
From: Jonathan Nieder @ 2011-08-11 11:04 UTC (permalink / raw)
To: Dmitry Ivankov; +Cc: git, David Barr, Stephen Boyd
In-Reply-To: <1313054138-30885-3-git-send-email-divanorama@gmail.com>
(+cc: Stephen)
Dmitry Ivankov wrote:
> Currently parse-options.o pulls quite a big bunch of dependencies.
> his complicates it's usage in contrib/ because it pulls external
> dependencies and it also increases executables size.
>
> Split off less generic and more internal to git part of
> parse-options.c to parse-options-cb.c.
>
> Move prefix_filename function from setup.c to abspath.c. abspath.o
> and wrapper.o pull each other, so it's unlikely to increase the
> dependencies. It was a dependency of parse-options.o that pulled
> many others.
>
> Now parse-options.o pulls just abspath.o, ctype.o, strbuf.o, usage.o,
> wrapper.o, libc directly and strlcpy.o indirectly.
So, in other words, currently linking to parse-options involves
linking to git's object access machinery and diff machinery, hence
libz, libssl for SHA-1, libpcre, etc. That is a waste of space,
startup time, and build complexity for simple programs that do not
need access to git's object db.
This patch does two things to address that:
- option callbacks which freely use the git object db and other
facilities move to a separate parse-options-cb.o translation
unit, so simple programs can avoid them;
- prefix_filename, which is used to support OPTION_FILENAME,
moves from setup.c to abspath.c, so it can be used without
pulling in unrelated git machinery.
The result is, as you say, that use of parse-options.o only pulls
in abspath, ctype, strbuf, usage, wrapper, and -lc --- which is to
say, just utility functions --- and you can build a program using
parse-options with a simple commandline like
cc -o test-program -Wall -W -O2 test-program.c libgit.a
Okay, on to the patch itself (comments sparsely scattered within).
>
> Signed-off-by: Dmitry Ivankov <divanorama@gmail.com>
> ---
> Makefile | 3 +-
> abspath.c | 28 ++++++++++++
> parse-options-cb.c | 125 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> parse-options.c | 121 --------------------------------------------------
> setup.c | 28 ------------
> 5 files changed, 155 insertions(+), 150 deletions(-)
> create mode 100644 parse-options-cb.c
>
> diff --git a/Makefile b/Makefile
> index 62ad0c2..7d47bdb 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -642,6 +642,7 @@ LIB_OBJS += pack-revindex.o
> LIB_OBJS += pack-write.o
> LIB_OBJS += pager.o
> LIB_OBJS += parse-options.o
> +LIB_OBJS += parse-options-cb.o
> LIB_OBJS += patch-delta.o
> LIB_OBJS += patch-ids.o
> LIB_OBJS += path.o
> @@ -2204,7 +2205,7 @@ test-delta$X: diff-delta.o patch-delta.o
>
> test-line-buffer$X: vcs-svn/lib.a
>
> -test-parse-options$X: parse-options.o
> +test-parse-options$X: parse-options.o parse-options-cb.o
>
> test-string-pool$X: vcs-svn/lib.a
>
> diff --git a/abspath.c b/abspath.c
> index 37287f8..f04ac18 100644
> --- a/abspath.c
> +++ b/abspath.c
> @@ -139,3 +139,31 @@ const char *absolute_path(const char *path)
> }
> return buf;
> }
> +
> +/*
> + * Unlike prefix_path, this should be used if the named file does
> + * not have to interact with index entry; i.e. name of a random file
> + * on the filesystem.
> + */
> +const char *prefix_filename(const char *pfx, int pfx_len, const char *arg)
> +{
> + static char path[PATH_MAX];
> +#ifndef WIN32
> + if (!pfx_len || is_absolute_path(arg))
> + return arg;
> + memcpy(path, pfx, pfx_len);
> + strcpy(path + pfx_len, arg);
> +#else
> + char *p;
> + /* don't add prefix to absolute paths, but still replace '\' by '/' */
> + if (is_absolute_path(arg))
> + pfx_len = 0;
> + else if (pfx_len)
> + memcpy(path, pfx, pfx_len);
> + strcpy(path + pfx_len, arg);
> + for (p = path + pfx_len; *p; p++)
> + if (*p == '\\')
> + *p = '/';
> +#endif
> + return path;
> +}
> diff --git a/parse-options-cb.c b/parse-options-cb.c
> new file mode 100644
> index 0000000..c248f66
> --- /dev/null
> +++ b/parse-options-cb.c
> @@ -0,0 +1,125 @@
> +#include "git-compat-util.h"
> +#include "parse-options.h"
> +#include "cache.h"
Style: Files in git tend to use only one of "git-compat-util.h",
"cache.h", or "builtin.h" and put it at the top. So in this case, it
should probably use just "cache.h".
> +#include "commit.h"
> +#include "color.h"
> +#include "string-list.h"
> +
> +/*----- some often used options -----*/
> +
> +int parse_opt_abbrev_cb(const struct option *opt, const char *arg, int unset)
> +{
> + int v;
> +
> + if (!arg) {
> + v = unset ? 0 : DEFAULT_ABBREV;
> + } else {
> + v = strtol(arg, (char **)&arg, 10);
> + if (*arg)
> + return opterror(opt, "expects a numerical value", 0);
> + if (v && v < MINIMUM_ABBREV)
> + v = MINIMUM_ABBREV;
> + else if (v > 40)
> + v = 40;
> + }
> + *(int *)(opt->value) = v;
> + return 0;
> +}
> +
> +int parse_opt_approxidate_cb(const struct option *opt, const char *arg,
> + int unset)
> +{
> + *(unsigned long *)(opt->value) = approxidate(arg);
> + return 0;
> +}
> +
> +int parse_opt_color_flag_cb(const struct option *opt, const char *arg,
> + int unset)
> +{
> + int value;
> +
> + if (!arg)
> + arg = unset ? "never" : (const char *)opt->defval;
> + value = git_config_colorbool(NULL, arg, -1);
> + if (value < 0)
> + return opterror(opt,
> + "expects \"always\", \"auto\", or \"never\"", 0);
> + *(int *)opt->value = value;
> + return 0;
> +}
> +
> +int parse_opt_verbosity_cb(const struct option *opt, const char *arg,
> + int unset)
> +{
> + int *target = opt->value;
> +
> + if (unset)
> + /* --no-quiet, --no-verbose */
> + *target = 0;
> + else if (opt->short_name == 'v') {
> + if (*target >= 0)
> + (*target)++;
> + else
> + *target = 1;
> + } else {
> + if (*target <= 0)
> + (*target)--;
> + else
> + *target = -1;
> + }
> + return 0;
> +}
> +
> +int parse_opt_with_commit(const struct option *opt, const char *arg, int unset)
> +{
> + unsigned char sha1[20];
> + struct commit *commit;
> +
> + if (!arg)
> + return -1;
> + if (get_sha1(arg, sha1))
> + return error("malformed object name %s", arg);
> + commit = lookup_commit_reference(sha1);
> + if (!commit)
> + return error("no such commit %s", arg);
> + commit_list_insert(commit, opt->value);
> + return 0;
> +}
> +
> +int parse_opt_tertiary(const struct option *opt, const char *arg, int unset)
> +{
> + int *target = opt->value;
> + *target = unset ? 2 : 1;
> + return 0;
> +}
> +
> +int parse_options_concat(struct option *dst, size_t dst_size, struct option *src)
> +{
> + int i, j;
> +
> + for (i = 0; i < dst_size; i++)
> + if (dst[i].type == OPTION_END)
> + break;
> + for (j = 0; i < dst_size; i++, j++) {
> + dst[i] = src[j];
> + if (src[j].type == OPTION_END)
> + return 0;
> + }
> + return -1;
> +}
> +
> +int parse_opt_string_list(const struct option *opt, const char *arg, int unset)
> +{
> + struct string_list *v = opt->value;
> +
> + if (unset) {
> + string_list_clear(v, 0);
> + return 0;
> + }
> +
> + if (!arg)
> + return -1;
> +
> + string_list_append(v, xstrdup(arg));
> + return 0;
> +}
> diff --git a/parse-options.c b/parse-options.c
> index 7b061af..503ab5d 100644
> --- a/parse-options.c
> +++ b/parse-options.c
> @@ -3,7 +3,6 @@
> #include "cache.h"
> #include "commit.h"
> #include "color.h"
> -#include "string-list.h"
>
> static int parse_options_usage(struct parse_opt_ctx_t *ctx,
> const char * const *usagestr,
> @@ -584,123 +583,3 @@ static int parse_options_usage(struct parse_opt_ctx_t *ctx,
> return usage_with_options_internal(ctx, usagestr, opts, 0, err);
> }
>
> -
> -/*----- some often used options -----*/
> -#include "cache.h"
> -
> -int parse_opt_abbrev_cb(const struct option *opt, const char *arg, int unset)
[... snip code moved verbatim ...]
> -}
> diff --git a/setup.c b/setup.c
> index 5ea5502..3463819 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -40,34 +40,6 @@ char *prefix_path(const char *prefix, int len, const char *path)
> return sanitized;
> }
>
> -/*
> - * Unlike prefix_path, this should be used if the named file does
> - * not have to interact with index entry; i.e. name of a random file
> - * on the filesystem.
> - */
> -const char *prefix_filename(const char *pfx, int pfx_len, const char *arg)
> -{
> - static char path[PATH_MAX];
> -#ifndef WIN32
> - if (!pfx_len || is_absolute_path(arg))
> - return arg;
> - memcpy(path, pfx, pfx_len);
> - strcpy(path + pfx_len, arg);
> -#else
> - char *p;
> - /* don't add prefix to absolute paths, but still replace '\' by '/' */
> - if (is_absolute_path(arg))
> - pfx_len = 0;
> - else if (pfx_len)
> - memcpy(path, pfx, pfx_len);
> - strcpy(path + pfx_len, arg);
> - for (p = path + pfx_len; *p; p++)
> - if (*p == '\\')
> - *p = '/';
> -#endif
> - return path;
> -}
> -
> int check_filename(const char *prefix, const char *arg)
> {
> const char *name;
> --
> 1.7.3.4
>
Except for the commit message, and with or without the #include tweak
mentioned above,
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Thanks. This seems to be in pretty good shape (and as mentioned before,
gets closer to fulfillment of a longstanding wish :)).
^ permalink raw reply
* Re: About git-diff
From: Luiz Ramos @ 2011-08-11 11:17 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Andreas Schwab, git
In-Reply-To: <7vr54sr1qi.fsf@alter.siamese.dyndns.org>
--- Em qui, 11/8/11, Junio C Hamano <gitster@pobox.com> escreveu:
> Luiz Ramos <luizzramos@yahoo.com.br>
> writes:
>
> > Given this, I'd suggest to change the inline
> documentation of git-diff (git help diff). In the version of
> my machine (1.7.4.4), it's like that:
> >
> > (snip)
> > ...
> > git diff [--options]
> <commit> [--] [<path>...]
> > This form is
> to view the changes you have in your working tree
> > relative to
> the named <commit>. You can use HEAD to compare it
> with
> > the latest
> commit, or a branch name to compare with the tip of a
> > different
> branch.
> > ...
>
> Strictly speaking, "the changes you have in your working
> tree" may be what
> is confusing. Your working tree does _not_ have "changes";
> it only has
> "contents". Changes are perceived only if you compare it
> with something
> else, as their _difference_.
>
> This operation compares "the contents of tracked files in
> your working
> tree" with "the contents recorded in the named
> <commit>"---the result of
> this comparison comparison matches what humans perceive as
> "changes".
>
> So perhaps updating the first sentence with:
>
> Compare the contents of tracked files in
> your working tree with
> what is recorded in the named
> <commit>.
>
Yes, agreed. This sentence seems to embed the right information in a more concise manner.
> would be all that is necessary. I didn't bother to look but
> I suspect we
> have a simlar description for "git diff [--options] [--]
> [<path>...]"
> form, and it should be updated in a similar way (the only
> difference is
> that it compares "with what is recorded in the index").
>
In my version (1.7.4.4):
(snip)
...
git diff [--options] [--] [<path>...]
This form is to view the changes you made relative to the index (staging area for the next commit). In other words, the differences are what you could tell git to further add to
the index but you still haven't. You can stage these changes by using git-add(1).
...
(snip)
And yes, if an untracked new file is in the working directory, it is not reported. Again, the words "changes" and "differences" seem to be confusing in this case as new files may be seen as "changes" or "differences" for an operator not used to git.
Luiz
^ permalink raw reply
* cherry-pick cannot merge binary files? (also strategy does not work?)
From: Piotr Krukowiecki @ 2011-08-11 12:58 UTC (permalink / raw)
To: Git Mailing List
Hi,
It seems cherry-pick can't merge binary files. I suppose this can be
considered a bug or a feature. But I expected I could use "theirs"
merge strategy to overcome (auto-resolve) this - but it seems it's
either not working, or I'm doing something wrong. Any hints?
[t]$ git init
Initialized empty Git repository in /tmp/t/.git/
[t (master)]$ echo a > a.txt && gzip a.txt && git add a.txt.gz && git
commit -m base
[master (root-commit) 64587708] base
1 files changed, 0 insertions(+), 0 deletions(-)
create mode 100644 a.txt.gz
[t (master)]$ mv a.txt.gz a.txt && gzip a.txt && git add a.txt.gz &&
git commit -m 1
[master 7b73fea9] 1
1 files changed, 0 insertions(+), 0 deletions(-)
[t (master)]$ git checkout -b two HEAD^
Switched to a new branch 'two'
[t (two)]$ mv a.txt.gz a.txt && echo b > a.txt && gzip a.txt && git
add a.txt.gz && git commit -m 2
[two 57723852] 2
1 files changed, 0 insertions(+), 0 deletions(-)
[t (two)]$ git cherry-pick master
warning: Cannot merge binary files: a.txt.gz (HEAD vs. 7b73fea9... 1)
error: could not apply 7b73fea9... 1
hint: after resolving the conflicts, mark the corrected paths
hint: with 'git add <paths>' or 'git rm <paths>'
hint: and commit the result with 'git commit'
[t (two|CHERRY-PICKING)]$ git reset
[t (two)]$ git cherry-pick --strategy=recursive --strategy-option=theirs master
warning: Cannot merge binary files: a.txt.gz (HEAD vs. 7b73fea9... 1)
error: could not apply 7b73fea9... 1
hint: after resolving the conflicts, mark the corrected paths
hint: with 'git add <paths>' or 'git rm <paths>'
hint: and commit the result with 'git commit'
Thanks,
--
Piotr Krukowiecki
^ permalink raw reply
* [PATCH] post-receive-email: Fix handling of hooks.emailmaxlines configuration option
From: Mike Crowe @ 2011-08-11 17:13 UTC (permalink / raw)
To: git; +Cc: mac
Make the hooks.emailmaxlines configuration option work correctly
again. It looks like this was broken by 53cad691.
Signed-off-by: Mike Crowe <mac@mcrowe.com>
---
contrib/hooks/post-receive-email | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/contrib/hooks/post-receive-email b/contrib/hooks/post-receive-email
index fa6d41a..ab6d298 100755
--- a/contrib/hooks/post-receive-email
+++ b/contrib/hooks/post-receive-email
@@ -85,7 +85,6 @@ prep_for_email()
oldrev=$(git rev-parse $1)
newrev=$(git rev-parse $2)
refname="$3"
- maxlines=$4
# --- Interpret
# 0000->1234 (create)
@@ -198,6 +197,8 @@ prep_for_email()
generate_email()
{
# Email parameters
+ maxlines=$1
+
# The email subject will contain the best description of the ref
# that we can build from the parameters
describe=$(git describe $rev 2>/dev/null)
--
1.7.2.5
^ permalink raw reply related
* [PATCH 0/2] Add an update=none option for 'loose' submodules
From: Heiko Voigt @ 2011-08-11 17:51 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jens Lehmann
If a submodule is used to seperate some bigger parts of a project into
an optional directory it is helpful to not clone/update them by default.
This series implements a new value 'none' for submodule.<name>.update.
If this option is set a submodule will not be updated or cloned by
default. If the user wants to work with the submodule he either needs
to explicitely configure the update option to 'checkout' or pass
--checkout as an option to the submodules. I chose this name to be
consistent with the existing --merge/--rebase options.
We have been talking about loose submodules for some time:
RFC patch for this series
http://thread.gmane.org/gmane.comp.version-control.git/175165
Using submodule groups/dependencies:
http://thread.gmane.org/gmane.comp.version-control.git/130928/focus=131050
http://thread.gmane.org/gmane.comp.version-control.git/176347/focus=178614
This lays the foundations for grouping of submodules. Once submodule
grouping will be implemented the value of submodule.$name.update
provides the default value when the user specifies no group. A group
specification could then be a layer on top which provides a shortcut to
choose other submodule.$name.update values to be registered in
.git/config.
Heiko Voigt (2):
submodule: move update configuration variable further up
add update 'none' flag to disable update of submodule by default
Documentation/git-submodule.txt | 8 ++++-
git-submodule.sh | 22 ++++++++++----
t/t7406-submodule-update.sh | 62 +++++++++++++++++++++++++++++++++++++++
3 files changed, 85 insertions(+), 7 deletions(-)
--
1.7.6.435.g741d34
^ permalink raw reply
* [PATCH 1/2] submodule: move update configuration variable further up
From: Heiko Voigt @ 2011-08-11 17:51 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jens Lehmann
In-Reply-To: <cover.1312923673.git.hvoigt@hvoigt.net>
Lets always initialize the 'update_module' variable with the final
value. This way we allow code which wants to check this configuration
early to do so right in the beginning of cmd_update().
Signed-off-by: Heiko Voigt <hvoigt@hvoigt.net>
---
git-submodule.sh | 13 +++++++------
1 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/git-submodule.sh b/git-submodule.sh
index f46862f..e544dbc 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -461,7 +461,13 @@ cmd_update()
fi
name=$(module_name "$path") || exit
url=$(git config submodule."$name".url)
- update_module=$(git config submodule."$name".update)
+ if ! test -z "$update"
+ then
+ update_module=$update
+ else
+ update_module=$(git config submodule."$name".update)
+ fi
+
if test -z "$url"
then
# Only mention uninitialized submodules when its
@@ -483,11 +489,6 @@ Maybe you want to use 'update --init'?")"
die "$(eval_gettext "Unable to find current revision in submodule path '\$path'")"
fi
- if ! test -z "$update"
- then
- update_module=$update
- fi
-
if test "$subsha1" != "$sha1"
then
subforce=$force
--
1.7.6.435.g741d34
^ permalink raw reply related
* [PATCH 2/2] add update 'none' flag to disable update of submodule by default
From: Heiko Voigt @ 2011-08-11 17:51 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jens Lehmann
In-Reply-To: <cover.1312923673.git.hvoigt@hvoigt.net>
This is useful to mark a submodule as unneeded by default. When this
option is set and the user wants to work with such a submodule he
needs to configure 'submodule.<name>.update=checkout' or pass the
--checkout option. Then the submodule can be handled like a normal
submodule.
Signed-off-by: Heiko Voigt <hvoigt@hvoigt.net>
---
Documentation/git-submodule.txt | 8 ++++-
git-submodule.sh | 9 +++++
t/t7406-submodule-update.sh | 62 +++++++++++++++++++++++++++++++++++++++
3 files changed, 78 insertions(+), 1 deletions(-)
diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 67cf5f0..6ec3fef 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -120,6 +120,8 @@ too (and can also report changes to a submodule's work tree).
init::
Initialize the submodules, i.e. register each submodule name
and url found in .gitmodules into .git/config.
+ It will also copy the value of `submodule.$name.update` into
+ .git/config.
The key used in .git/config is `submodule.$name.url`.
This command does not alter existing information in .git/config.
You can then customize the submodule clone URLs in .git/config
@@ -133,7 +135,7 @@ update::
checkout the commit specified in the index of the containing repository.
This will make the submodules HEAD be detached unless `--rebase` or
`--merge` is specified or the key `submodule.$name.update` is set to
- `rebase` or `merge`.
+ `rebase`, `merge` or `none`.
+
If the submodule is not yet initialized, and you just want to use the
setting as stored in .gitmodules, you can automatically initialize the
@@ -141,6 +143,10 @@ submodule with the `--init` option.
+
If `--recursive` is specified, this command will recurse into the
registered submodules, and update any nested submodules within.
++
+If the configuration key `submodule.$name.update` is set to `none` the
+submodule with name `$name` will not be updated by default. This can be
+overriden by adding `--checkout` to the command.
summary::
Show commit summary between the given commit (defaults to HEAD) and
diff --git a/git-submodule.sh b/git-submodule.sh
index e544dbc..34d2be6 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -429,6 +429,9 @@ cmd_update()
--recursive)
recursive=1
;;
+ --checkout)
+ update="checkout"
+ ;;
--)
shift
break
@@ -468,6 +471,12 @@ cmd_update()
update_module=$(git config submodule."$name".update)
fi
+ if test "$update_module" = "none"
+ then
+ echo "Skipping submodule '$path'"
+ continue
+ fi
+
if test -z "$url"
then
# Only mention uninitialized submodules when its
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index c679f36..58877d3 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -30,6 +30,7 @@ test_expect_success 'setup a submodule tree' '
git clone super submodule &&
git clone super rebasing &&
git clone super merging &&
+ git clone super none &&
(cd super &&
git submodule add ../submodule submodule &&
test_tick &&
@@ -58,6 +59,11 @@ test_expect_success 'setup a submodule tree' '
test_tick &&
git commit -m "rebasing"
)
+ (cd super &&
+ git submodule add ../none none &&
+ test_tick &&
+ git commit -m "none"
+ )
'
test_expect_success 'submodule update detaching the HEAD ' '
@@ -298,6 +304,62 @@ test_expect_success 'submodule update ignores update=rebase config for new submo
)
'
+test_expect_success 'submodule init picks up update=none' '
+ (cd super &&
+ git config -f .gitmodules submodule.none.update none &&
+ git submodule init none &&
+ test "none" = "$(git config submodule.none.update)"
+ )
+'
+
+test_expect_success 'submodule update - update=none in .git/config' '
+ (cd super &&
+ git config submodule.submodule.update none &&
+ (cd submodule &&
+ git checkout master &&
+ compare_head
+ ) &&
+ git diff --raw | grep " submodule" &&
+ git submodule update &&
+ git diff --raw | grep " submodule" &&
+ (cd submodule &&
+ compare_head
+ ) &&
+ git config --unset submodule.submodule.update &&
+ git submodule update submodule
+ )
+'
+
+test_expect_success 'submodule update - update=none in .git/config but --checkout given' '
+ (cd super &&
+ git config submodule.submodule.update none &&
+ (cd submodule &&
+ git checkout master &&
+ compare_head
+ ) &&
+ git diff --raw | grep " submodule" &&
+ git submodule update --checkout &&
+ test_must_fail git diff --raw \| grep " submodule" &&
+ (cd submodule &&
+ test_must_fail compare_head
+ ) &&
+ git config --unset submodule.submodule.update
+ )
+'
+
+test_expect_success 'submodule update --init skips submodule with update=none' '
+ (cd super &&
+ git add .gitmodules &&
+ git commit -m ".gitmodules"
+ ) &&
+ git clone super cloned &&
+ (cd cloned &&
+ git submodule update --init &&
+ test -e submodule/.git &&
+ test_must_fail test -e none/.git
+ )
+'
+
test_expect_success 'submodule update continues after checkout error' '
(cd super &&
git reset --hard HEAD &&
--
1.7.6.435.g741d34
^ permalink raw reply related
* [PATCH 00/11] Micro-optimizing lookup_object()
From: Junio C Hamano @ 2011-08-11 17:53 UTC (permalink / raw)
To: git
I noticed that a typical "git repack -a -d -f" spends about 50% of the
time during the "Counting objects" phase inside lookup_object(). The
look-up is implemented as a hashtable with linear probing that limits the
maximum load-factor to 50%, and during repacking the Linux kernel
repository, we count 2,139,209 objects, and the worst case we probe 50
hash entries to find a single object in the hash table due to slot
collisions. The lookup_object() function is called 88,603,392 times.
After cleaning up the existing code a bit, this series adds "--count-only"
option to "pack-objects" to tell it to stop after the "Counting objects"
phase for benchmarking purposes. To emulate the "Counting objects" phase
of a full repack, we can run this (perhaps under "/usr/bin/time"):
git pack-objects --count-only --keep-true-parents --honor-pack-keep \
--non-empty --all --reflog --no-reuse-delta \
--delta-base-offset --stdout </dev/null >/dev/null
On my development box, the slightly cleaned-up existing linear probing
code gives (best of three runs with hot cache):
31.89user 2.16system 0:34.16elapsed 99%CPU (0avgtext+0avgdata 2965264maxresident)k
0inputs+0outputs (0major+225336minor)pagefaults 0swaps
Then the series replaces lookup_object(), insert_obj_hash() and
grow_object_hash() with a naive implementation of cuckoo hashing that uses
two hash functions. The performance is not very impressive, and this wastes
too much memory, due to its rather poor strategy to re-grow the table size:
32.04user 3.43system 0:35.58elapsed 99%CPU (0avgtext+0avgdata 8178672maxresident)k
0inputs+0outputs (0major+1206546minor)pagefaults 0swaps
As we have 20-byte object names as the hash key material, we could easily
extend this to use 5 hash functions instead of 2. This reduces the memory
usage by improving the load factor, but we spend extra cycles in lookup:
31.66user 2.14system 0:33.91elapsed 99%CPU (0avgtext+0avgdata 2874176maxresident)k
0inputs+0outputs (0major+225342minor)pagefaults 0swaps
At this step with 5-way cuckoo, we are slightly better than the original
linear probing. By using smaller number of hash functions, we can reduce
the cycles we need in lookup, while we lose on the load factor.
Here is what we get from the code with 4 hashes:
30.88user 2.32system 0:33.31elapsed 99%CPU (0avgtext+0avgdata 3135984maxresident)k
0inputs+0outputs (0major+290857minor)pagefaults 0swaps
And here is with 3 hashes:
30.68user 2.26system 0:33.05elapsed 99%CPU (0avgtext+0avgdata 3660832maxresident)k
0inputs+0outputs (0major+421963minor)pagefaults 0swaps
The best balance is somewhere between 3-hash and 4-hash, it seems. We are
getting ~4% runtime performance improvements (31.89 vs 30.68).
Just as a sanity check, the final patch in the series reduces the number
of hashes back to 2, which yields a similar performance characteristics
from the original "naive" cuckoo implementation:
31.06user 3.22system 0:34.39elapsed 99%CPU (0avgtext+0avgdata 8176512maxresident)k
0inputs+0outputs (0major+1206542minor)pagefaults 0swaps
The real optimization opportunity _may_ be to reduce the calls we make to
the function---we are calling lookup() 40+ times on one object. But that
is outside the scope of this series.
This series is not for application but primarily is to serve as the
supplimental data for the above numbers. A re-rolled series that consists
of the earlier clean-ups and then a patch to replace the linear probing
with 4-way cuckoo will be queued instead in 'pu'.
Junio C Hamano (11):
object.c: code movement for readability
object.c: remove duplicated code for object hashing
pack-objects --count-only
object: next_size() helper for readability
object hash: we know the table size is a power of two
object: growing the hash-table more aggressively does not help much
object: try naive cuckoo hashing
object: try 5-way cuckoo -- use all 20-bytes of SHA-1
object: try 4-way cuckoo
object: try 3-way cuckoo
object: try 2-way cuckoo again
builtin/pack-objects.c | 7 +++
object.c | 138 +++++++++++++++++++++++++++++-------------------
2 files changed, 91 insertions(+), 54 deletions(-)
--
1.7.6.433.g1421f
^ permalink raw reply
* [PATCH 01/11] object.c: code movement for readability
From: Junio C Hamano @ 2011-08-11 17:53 UTC (permalink / raw)
To: git
In-Reply-To: <1313085196-13249-1-git-send-email-gitster@pobox.com>
This only moves the lines around a bit to gather the definitions of code
and data that manage the hash function and the object hash table into one
place (examine "blame -M" output to see nothing new is introduced).
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
object.c | 40 ++++++++++++++++++++--------------------
1 files changed, 20 insertions(+), 20 deletions(-)
diff --git a/object.c b/object.c
index 31976b5..35ff130 100644
--- a/object.c
+++ b/object.c
@@ -5,19 +5,6 @@
#include "commit.h"
#include "tag.h"
-static struct object **obj_hash;
-static int nr_objs, obj_hash_size;
-
-unsigned int get_max_object_index(void)
-{
- return obj_hash_size;
-}
-
-struct object *get_indexed_object(unsigned int idx)
-{
- return obj_hash[idx];
-}
-
static const char *object_type_strings[] = {
NULL, /* OBJ_NONE = 0 */
"commit", /* OBJ_COMMIT = 1 */
@@ -43,6 +30,19 @@ int type_from_string(const char *str)
die("invalid object type \"%s\"", str);
}
+static struct object **obj_hash;
+static int nr_objs, obj_hash_size;
+
+unsigned int get_max_object_index(void)
+{
+ return obj_hash_size;
+}
+
+struct object *get_indexed_object(unsigned int idx)
+{
+ return obj_hash[idx];
+}
+
static unsigned int hash_obj(struct object *obj, unsigned int n)
{
unsigned int hash;
@@ -50,6 +50,13 @@ static unsigned int hash_obj(struct object *obj, unsigned int n)
return hash % n;
}
+static unsigned int hashtable_index(const unsigned char *sha1)
+{
+ unsigned int i;
+ memcpy(&i, sha1, sizeof(unsigned int));
+ return i % obj_hash_size;
+}
+
static void insert_obj_hash(struct object *obj, struct object **hash, unsigned int size)
{
unsigned int j = hash_obj(obj, size);
@@ -62,13 +69,6 @@ static void insert_obj_hash(struct object *obj, struct object **hash, unsigned i
hash[j] = obj;
}
-static unsigned int hashtable_index(const unsigned char *sha1)
-{
- unsigned int i;
- memcpy(&i, sha1, sizeof(unsigned int));
- return i % obj_hash_size;
-}
-
struct object *lookup_object(const unsigned char *sha1)
{
unsigned int i;
--
1.7.6.433.g1421f
^ permalink raw reply related
* [PATCH 02/11] object.c: remove duplicated code for object hashing
From: Junio C Hamano @ 2011-08-11 17:53 UTC (permalink / raw)
To: git
In-Reply-To: <1313085196-13249-1-git-send-email-gitster@pobox.com>
The index into object hash was computed in two different places,
risking them to diverge. Implement a single helper function and
use it in these two locations.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
object.c | 17 +++++------------
1 files changed, 5 insertions(+), 12 deletions(-)
diff --git a/object.c b/object.c
index 35ff130..4ff2d7d 100644
--- a/object.c
+++ b/object.c
@@ -43,23 +43,16 @@ struct object *get_indexed_object(unsigned int idx)
return obj_hash[idx];
}
-static unsigned int hash_obj(struct object *obj, unsigned int n)
+static unsigned int hash_val(const unsigned char *sha1)
{
unsigned int hash;
- memcpy(&hash, obj->sha1, sizeof(unsigned int));
- return hash % n;
-}
-
-static unsigned int hashtable_index(const unsigned char *sha1)
-{
- unsigned int i;
- memcpy(&i, sha1, sizeof(unsigned int));
- return i % obj_hash_size;
+ memcpy(&hash, sha1, sizeof(unsigned int));
+ return hash;
}
static void insert_obj_hash(struct object *obj, struct object **hash, unsigned int size)
{
- unsigned int j = hash_obj(obj, size);
+ unsigned int j = hash_val(obj->sha1) % size;
while (hash[j]) {
j++;
@@ -77,7 +70,7 @@ struct object *lookup_object(const unsigned char *sha1)
if (!obj_hash)
return NULL;
- i = hashtable_index(sha1);
+ i = hash_val(sha1) % obj_hash_size;
while ((obj = obj_hash[i]) != NULL) {
if (!hashcmp(sha1, obj->sha1))
break;
--
1.7.6.433.g1421f
^ permalink raw reply related
* [PATCH 04/11] object: next_size() helper for readability
From: Junio C Hamano @ 2011-08-11 17:53 UTC (permalink / raw)
To: git
In-Reply-To: <1313085196-13249-1-git-send-email-gitster@pobox.com>
Move the heuristics to grow the table size into a separate
helper function to make the caller more readable.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
object.c | 7 ++++++-
1 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/object.c b/object.c
index 4ff2d7d..259a67e 100644
--- a/object.c
+++ b/object.c
@@ -81,10 +81,15 @@ struct object *lookup_object(const unsigned char *sha1)
return obj;
}
+static int next_size(int sz)
+{
+ return sz < 32 ? 32 : 2 * sz;
+}
+
static void grow_object_hash(void)
{
int i;
- int new_hash_size = obj_hash_size < 32 ? 32 : 2 * obj_hash_size;
+ int new_hash_size = next_size(obj_hash_size);
struct object **new_hash;
new_hash = xcalloc(new_hash_size, sizeof(struct object *));
--
1.7.6.433.g1421f
^ permalink raw reply related
* [PATCH 05/11] object hash: we know the table size is a power of two
From: Junio C Hamano @ 2011-08-11 17:53 UTC (permalink / raw)
To: git
In-Reply-To: <1313085196-13249-1-git-send-email-gitster@pobox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
object.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/object.c b/object.c
index 259a67e..d95d7a6 100644
--- a/object.c
+++ b/object.c
@@ -52,7 +52,7 @@ static unsigned int hash_val(const unsigned char *sha1)
static void insert_obj_hash(struct object *obj, struct object **hash, unsigned int size)
{
- unsigned int j = hash_val(obj->sha1) % size;
+ unsigned int j = hash_val(obj->sha1) & (size-1);
while (hash[j]) {
j++;
@@ -70,7 +70,7 @@ struct object *lookup_object(const unsigned char *sha1)
if (!obj_hash)
return NULL;
- i = hash_val(sha1) % obj_hash_size;
+ i = hash_val(sha1) & (obj_hash_size-1);
while ((obj = obj_hash[i]) != NULL) {
if (!hashcmp(sha1, obj->sha1))
break;
--
1.7.6.433.g1421f
^ permalink raw reply related
* [PATCH 03/11] pack-objects --count-only
From: Junio C Hamano @ 2011-08-11 17:53 UTC (permalink / raw)
To: git
In-Reply-To: <1313085196-13249-1-git-send-email-gitster@pobox.com>
This is merely to help debugging the bottleneck of "Counting objects"
phase of the pack-objects process. Use it like this:
git pack-objects --count-only --keep-true-parents --honor-pack-keep \
--non-empty --all --reflog --no-reuse-delta \
--delta-base-offset --stdout </dev/null >/dev/null
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/pack-objects.c | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index f402a84..6a208a9 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -2121,6 +2121,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
int use_internal_rev_list = 0;
int thin = 0;
int all_progress_implied = 0;
+ int count_only = 0;
uint32_t i;
const char **rp_av;
int rp_ac_alloc = 64;
@@ -2291,6 +2292,10 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
grafts_replace_parents = 0;
continue;
}
+ if (!strcmp("--count-only", arg)) {
+ count_only = 1;
+ continue;
+ }
usage(pack_usage);
}
@@ -2348,6 +2353,8 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
if (non_empty && !nr_result)
return 0;
+ if (count_only)
+ return 0;
if (nr_result)
prepare_pack(window, depth);
write_pack_file();
--
1.7.6.433.g1421f
^ 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