[PATCH v4 0/3] git config cache special querying api utilizing the cache
Hi, [PATCH v4]: Introduced `config_set` construct which points to a ordered set of config-files cached as hashmaps. Added relevant API functions. For more details see the documentation. Rewrote the git_config_get* family to use `config_set` internally. Added tests for both config_set API and git_config_get family. Added type specific API functions which parses the found value and converts it into a specific type. Most of the changes implemented are the result of discussion in [6]. Thanks to Eric, Ramsay, Junio, Matthieu Karsten for their suggestions and review. [PATCH v3]: Added flag for NULL values that were causing segfaults in some cases. Added test-config for usage examples. Minor changes and corrections. Refer to discussion thread[5] for more details. Thanks to Matthieu, Jeff and Junio for their valuable suggestions. [PATCH v2]:Changed the string_list to a struct instead of pointer to a struct. Added string-list initilization functions. Minor mistakes corrected acoording to review comments[4]. Thanks to Eric and Matthieu for their review. [PATCH V1]:Most of the invaluable suggestions by Eric Sunshine, Torsten Bogershausen and Jeff King has been implemented[1]. Complete rewrite of config_cache*() family using git_config() as hook as suggested by Jeff. Thanks for the review. [RFC V2]: Improved according to the suggestions by Eric Sunshine and Torsten Bogershausen. Added cache invalidation when config file is changed.[2] I am using git_config_set_multivar_in_file() as an update hook. This is patch is for this year's GSoC. My project is Git Config API improvements. The link of my proposal is appended below [3]. The aim of this patch series is to generate a cache for querying values from the config files in a non-callback manner as the current method reads and parses the config files every time a value is queried for. The cache is generated from hooking the update_cache function to the current parsing and callback mechanism in config.c. It is implemented as an hashmap using the hashmap-api with variables and its corresponding values list as its members. The values in the list are sorted in order of increasing priority. The cache is initialised the first time when any of the new query functions is called. It is invalidated by using git_config_set_multivar_in_file() as an update hook. We add two new functions to the config-api, git_config_get_string() and git_config_get_string_multi() for querying in a non callback manner from the cache. [1] http://marc.info/?t=14017206626r=1w=2 [2] http://git.661346.n2.nabble.com/RFC-PATCH-0-2-Git-config-cache-amp-special-querying-api-utilizing-the-cache-td7611691.html [3] https://drive.google.com/file/d/0B4suZ-aHqDcnSUZJRXVTTnZUN1E/edit?usp=sharing [4] http://thread.gmane.org/gmane.comp.version-control.git/251073/focus=251369 [5] http://thread.gmane.org/gmane.comp.version-control.git/251704/ [6] http://thread.gmane.org/gmane.comp.version-control.git/252329/ Tanay Abhra (2): config-hash.c test-config .gitignore | 1 + Documentation/technical/api-config.txt | 144 Makefile | 2 + cache.h| 41 config-hash.c | 405 + config.c | 3 + t/t1308-config-hash.sh | 163 + test-config.c | 129 +++ 8 files changed, 888 insertions(+) create mode 100644 config-hash.c create mode 100755 t/t1308-config-hash.sh create mode 100644 test-config.c -- 1.9.0.GIT -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 1/2] add `config_set` API for caching config files
Currently `git_config()` uses a callback mechanism and file rereads for config values. Due to this approach, it is not uncommon for the config files to be parsed several times during the run of a git program, with different callbacks picking out different variables useful to themselves. Add a `config_set` construct that points to an ordered set of config files specified by the user, each of which represents what was read and cached in core as hashmaps. Add two external functions `git_configset_get_value` and `git_configset_get_value_multi` for querying from the config sets, which follow `last one wins` semantic(i.e. if there are multiple matches for the queried key in the files of the configset the value returned will the last value in the last matching file of the configset) Add type specific query functions like `git_configset_get_bool` and similar. Add a default `config_set`, `the_config_set` to cache all key-value pairs read from usual config files(repo specific .git/config, user wide ~/.gitconfig and the global /etc/gitconfig). `the_config_set` uses a single hashmap populated using git_config(). The reason for doing so is twofold, one is to honour include directives, another is to guarantee O(1) lookups for usual config values, as values are queried for hundred of times during a run of a git program. Add two external functions `git_config_get_value` and `git_config_get_value_multi` for querying in a non-callback manner from `the_config_set`. Also, add type specific query functions that are implemented as a thin wrapper around the `config_set` API. Signed-off-by: Tanay Abhra tanay...@gmail.com --- Documentation/technical/api-config.txt | 144 Makefile | 1 + cache.h| 41 config-hash.c | 405 + config.c | 3 + 5 files changed, 594 insertions(+) create mode 100644 config-hash.c diff --git a/Documentation/technical/api-config.txt b/Documentation/technical/api-config.txt index 230b3a0..2c02fee 100644 --- a/Documentation/technical/api-config.txt +++ b/Documentation/technical/api-config.txt @@ -77,6 +77,75 @@ To read a specific file in git-config format, use `git_config_from_file`. This takes the same callback and data parameters as `git_config`. +Querying For Specific Variables +--- + +For programs wanting to query for specific variables in a non-callback +manner, the config API provides two functions `git_config_get_value` +and `git_config_get_value_multi`.They both read values from an internal +cache generated previously from reading the config files. + +`git_config_get_value` takes two parameters, + +- a key string in canonical flat form for which the corresponding value + with the highest priority (i.e. value in the repo config will be + preferred over value in user wide config for the same variable) will + be retrieved. + +- a pointer to a string which will point to the retrieved value. The caller + should not free or modify the value returned as it is owned by the cache. + +`git_config_get_value` returns 0 on success, or -1 for no value found. + +`git_config_get_value_multi` allocates and returns a `string_list` +containing all the values for the key passed as parameter, sorted in order +of increasing priority (Note: caller has to call `string_list_clear` on +the returned pointer and then free it). + +`git_config_get_value_multi` returns NULL for no value found. + +`git_config_clear` is provided for resetting and invalidating the cache. + +`git_config_iter` gives a way to iterate over the keys in cache. Existing +`git_config` callback function signature is used for iterating. + +The config API also provides type specific API functions which do conversion +as well as retrieval for the queried variable, including: + +`git_config_get_int`:: +Parse the retrieved value to an integer, including unit factors. Dies on +error; otherwise, allocates and copies the integer into the dest parameter. +Returns 0 on success, or 1 if no value was found. + +`git_config_get_ulong`:: +Identical to `git_config_get_int` but for unsigned longs. + +`git_config_bool`:: +Parse the retrieved value into a boolean value, respecting keywords like +true and false. Integer values are converted into true/false values +(when they are non-zero or zero, respectively). Other values cause a die(). +If parsing is successful, allocates and copies the result into the dest +parameter. Returns 0 on success, or 1 if no value is found. + +`git_config_get_bool_or_int`:: +Same as `git_config_get_bool`, except that integers are copied as-is, and +an `is_bool` flag is unset. + +`git_config_get_maybe_bool`:: +Same as `git_config_get_bool`, except that it returns -1 on error rather +than dying. + +`git_config_get_string`:: +Allocates and copies the retrieved value string into the `dest` parameter; +if NULL string is given, prints an error
[PATCH 2/2] test-config: Add tests for the config_set API
Expose the `config_set` C API as a set of simple commands in order to facilitate testing. Add tests for the `config_set` API as well as for `git_config_get_*()` family for the usual config files. Signed-off-by: Tanay Abhra tanay...@gmail.com --- .gitignore | 1 + Makefile | 1 + t/t1308-config-hash.sh | 163 + test-config.c | 129 ++ 4 files changed, 294 insertions(+) create mode 100755 t/t1308-config-hash.sh create mode 100644 test-config.c diff --git a/.gitignore b/.gitignore index 42294e5..eeb66e2 100644 --- a/.gitignore +++ b/.gitignore @@ -176,6 +176,7 @@ /gitweb/static/gitweb.js /gitweb/static/gitweb.min.* /test-chmtime +/test-config /test-ctype /test-date /test-delta diff --git a/Makefile b/Makefile index d503f78..e875070 100644 --- a/Makefile +++ b/Makefile @@ -548,6 +548,7 @@ X = PROGRAMS += $(patsubst %.o,git-%$X,$(PROGRAM_OBJS)) TEST_PROGRAMS_NEED_X += test-chmtime +TEST_PROGRAMS_NEED_X += test-config TEST_PROGRAMS_NEED_X += test-ctype TEST_PROGRAMS_NEED_X += test-date TEST_PROGRAMS_NEED_X += test-delta diff --git a/t/t1308-config-hash.sh b/t/t1308-config-hash.sh new file mode 100755 index 000..6c059c5 --- /dev/null +++ b/t/t1308-config-hash.sh @@ -0,0 +1,163 @@ +#!/bin/sh + +test_description='Test git config-hash API in different settings' + +. ./test-lib.sh + +test_expect_success 'clear default config' ' + rm -f .git/config +' + +cat .git/config EOF +[core] + penguin = very blue + Movie = BadPhysics + UPPERCASE = true + MixedCase = true + my = + foo + baz = sam +[Cores] + WhatEver = Second +[my Foo bAr] + hi = hello +[core] + baz = bat + baz = hask +[lamb] + chop = 65 + head = none +[goat] + legs = 4 + head = true + skin = false + nose = 1 + horns +EOF + +test_expect_success 'get value for a simple key' ' + echo very blue expect + test-config get_value core.penguin actual + test_cmp expect actual +' + +test_expect_success 'get value for a key with value as an empty string' ' + echo expect + test-config get_value core.my actual + test_cmp expect actual +' + +test_expect_success 'get value for a key with value as NULL' ' + echo (NULL) expect + test-config get_value core.foo actual + test_cmp expect actual +' +test_expect_success 'upper case key' ' + echo true expect + test-config get_value core.UPPERCASE actual + test_cmp expect actual +' +test_expect_success 'mixed case key' ' + echo true expect + test-config get_value core.MixedCase actual + test_cmp expect actual +' +test_expect_success 'key and value with mixed case' ' + echo BadPhysics expect + test-config get_value core.Movie actual + test_cmp expect actual +' + +test_expect_success 'key with case sensitive subsection' ' + echo hello expect + test-config get_value my.Foo bAr.hiactual + test_cmp expect actual +' + +test_expect_success 'find value with misspelled key' ' + echo Value not found for \my.fOo Bar.hi\ expect + test_must_fail test-config get_value my.fOo Bar.hiactual + test_cmp expect actual +' + +test_expect_success 'find value with the highest priority' ' + echo hask expect + test-config get_value core.bazactual + test_cmp expect actual +' + +test_expect_success 'find integer value for a key' ' + echo 65 expect + test-config get_int lamb.chop actual + test_cmp expect actual +' + +test_expect_success 'find integer if value is non parse-able' ' + echo 65 expect + test_must_fail test-config get_int lamb.head actual + test_must_fail test_cmp expect actual +' + +cat expect EOF +1 +0 +1 +1 +1 +EOF + +test_expect_success 'find bool value for the entered key' ' + test-config get_bool goat.head actual + test-config get_bool goat.skin actual + test-config get_bool goat.nose actual + test-config get_bool goat.horns actual + test-config get_bool goat.legs actual + test_cmp expect actual +' + +cat expect EOF +sam +bat +hask +EOF + +test_expect_success 'find multiple values' ' + test-config get_value_multi core.bazactual + test_cmp expect actual +' + +cat config2 EOF +[core] + baz = lama +[my] + new = silk +[core] + baz = ball +EOF + +test_expect_success 'find value from a configset' ' + echo silk expect + test-config configset_get_value 2 config2 .git/config my.new actual + test_cmp expect actual +' + +test_expect_success 'find value with highest priority from a configset' ' + echo hask expect + test-config configset_get_value 2 config2 .git/config core.baz actual + test_cmp expect actual +' + +cat except EOF +sam +bat +hask +lama +ball +EOF +
[PATCH] git-am: add option to extract email Message-Id: tag into commit log
Some workflows prefer to track exactly which email message was used to
generate a commit. This can be used, for example, to generate an
automated acknowledgement when a patch is committed as a response to
the patch email, or as a reference to the thread which introduced the
patch.
Support this by adding a --message-id option (abbreviated as -m) to
git-am, which will then extract the message ID and append it to the
email commit log.
Signed-off-by: Avi Kivity a...@cloudius-systems.com
---
Documentation/git-am.txt | 6 ++
builtin/mailinfo.c | 2 +-
git-am.sh| 10 +-
3 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt
index 9adce37..8a251a1 100644
--- a/Documentation/git-am.txt
+++ b/Documentation/git-am.txt
@@ -15,6 +15,7 @@ SYNOPSIS
[--whitespace=option] [-Cn] [-pn] [--directory=dir]
[--exclude=path] [--include=path] [--reject] [-q | --quiet]
[--[no-]scissors] [-S[keyid]] [--patch-format=format]
+[--message-id]
[(mbox | Maildir)...]
'git am' (--continue | --skip | --abort)
@@ -121,6 +122,11 @@ default. You can use `--no-utf8` to override this.
user to lie about the author date by using the same
value as the committer date.
+-m::
+--message-id::
+ Extract the Message-Id: header from the e-mail and
+ append it to the commit message's tag stanza.
+
--skip::
Skip the current patch. This is only meaningful when
restarting an aborted patch.
diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index cf11c8d..f1e1fed 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -278,7 +278,7 @@ static void cleanup_space(struct strbuf *sb)
static void decode_header(struct strbuf *line);
static const char *header[MAX_HDR_PARSED] = {
- From,Subject,Date,
+ From,Subject,Date,Message-Id
};
static inline int cmp_header(const struct strbuf *line, const char *hdr)
diff --git a/git-am.sh b/git-am.sh
index ee61a77..22799ff 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -39,6 +39,7 @@ committer-date-is-author-datelie about committer date
ignore-date use current timestamp for author date
rerere-autoupdate update the index with reused conflict resolution if possible
S,gpg-sign? GPG-sign commits
+m,message-idcopy the Message-Id: header to the commit's tag stanza
rebasing* (internal use for git-rebase)
. git-sh-setup
@@ -371,7 +372,7 @@ split_patches () {
prec=4
dotest=$GIT_DIR/rebase-apply
sign= utf8=t keep= keepcr= skip= interactive= resolved= rebasing= abort=
-resolvemsg= resume= scissors= no_inbody_headers=
+resolvemsg= resume= scissors= no_inbody_headers= message_id=
git_apply_opt=
committer_date_is_author_date=
ignore_date=
@@ -442,6 +443,8 @@ it will be removed. Please do not use it anymore.
gpg_sign_opt=-S ;;
--gpg-sign=*)
gpg_sign_opt=-S${1#--gpg-sign=} ;;
+ -m|--message-id)
+ message_id=t ;;
--)
shift; break ;;
*)
@@ -565,6 +568,7 @@ Use \git am --abort\ to remove it.)
echo $git_apply_opt $dotest/apply-opt
echo $threeway $dotest/threeway
echo $sign $dotest/sign
+ echo $message_id $dotest/message-id
echo $utf8 $dotest/utf8
echo $keep $dotest/keep
echo $scissors $dotest/scissors
@@ -757,6 +761,10 @@ To restore the original branch and stop patching run
\\$cmdline --abort\.
then
cat $dotest/msg-clean
fi
+ if test 't' == $message_id
+ then
+ grep ^Message-Id: $dotest/info
+ fi
if test '' != $ADD_SIGNOFF
then
echo $ADD_SIGNOFF
--
1.9.3
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] git-am: add option to extract email Message-Id: tag into commit log
Some workflows prefer to track exactly which email message was used to
generate a commit. This can be used, for example, to generate an
automated acknowledgement when a patch is committed as a response to
the patch email, or as a reference to the thread which introduced the
patch.
Support this by adding a --message-id option (abbreviated as -m) to
git-am, which will then extract the message ID and append it to the
email commit log.
Signed-off-by: Avi Kivity a...@cloudius-systems.com
---
v2: adjust to pass test suite (t5100)
Documentation/git-am.txt | 6 ++
builtin/mailinfo.c | 2 +-
git-am.sh| 10 +-
t/t5100/info0004 | 1 +
t/t5100/info0005 | 1 +
t/t5100/info0012 | 1 +
6 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt
index 9adce37..8a251a1 100644
--- a/Documentation/git-am.txt
+++ b/Documentation/git-am.txt
@@ -15,6 +15,7 @@ SYNOPSIS
[--whitespace=option] [-Cn] [-pn] [--directory=dir]
[--exclude=path] [--include=path] [--reject] [-q | --quiet]
[--[no-]scissors] [-S[keyid]] [--patch-format=format]
+[--message-id]
[(mbox | Maildir)...]
'git am' (--continue | --skip | --abort)
@@ -121,6 +122,11 @@ default. You can use `--no-utf8` to override this.
user to lie about the author date by using the same
value as the committer date.
+-m::
+--message-id::
+ Extract the Message-Id: header from the e-mail and
+ append it to the commit message's tag stanza.
+
--skip::
Skip the current patch. This is only meaningful when
restarting an aborted patch.
diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index cf11c8d..f1e1fed 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -278,7 +278,7 @@ static void cleanup_space(struct strbuf *sb)
static void decode_header(struct strbuf *line);
static const char *header[MAX_HDR_PARSED] = {
- From,Subject,Date,
+ From,Subject,Date,Message-Id
};
static inline int cmp_header(const struct strbuf *line, const char *hdr)
diff --git a/git-am.sh b/git-am.sh
index ee61a77..c0e7bdd 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -39,6 +39,7 @@ committer-date-is-author-datelie about committer date
ignore-date use current timestamp for author date
rerere-autoupdate update the index with reused conflict resolution if possible
S,gpg-sign? GPG-sign commits
+m,message-idcopy the Message-Id: header to the commit's tag stanza
rebasing* (internal use for git-rebase)
. git-sh-setup
@@ -371,7 +372,7 @@ split_patches () {
prec=4
dotest=$GIT_DIR/rebase-apply
sign= utf8=t keep= keepcr= skip= interactive= resolved= rebasing= abort=
-resolvemsg= resume= scissors= no_inbody_headers=
+resolvemsg= resume= scissors= no_inbody_headers= message_id=
git_apply_opt=
committer_date_is_author_date=
ignore_date=
@@ -442,6 +443,8 @@ it will be removed. Please do not use it anymore.
gpg_sign_opt=-S ;;
--gpg-sign=*)
gpg_sign_opt=-S${1#--gpg-sign=} ;;
+ -m|--message-id)
+ message_id=t ;;
--)
shift; break ;;
*)
@@ -565,6 +568,7 @@ Use \git am --abort\ to remove it.)
echo $git_apply_opt $dotest/apply-opt
echo $threeway $dotest/threeway
echo $sign $dotest/sign
+ echo $message_id $dotest/message-id
echo $utf8 $dotest/utf8
echo $keep $dotest/keep
echo $scissors $dotest/scissors
@@ -757,6 +761,10 @@ To restore the original branch and stop patching run
\\$cmdline --abort\.
then
cat $dotest/msg-clean
fi
+ if test 't' == $message_id
+ then
+ grep ^Message-Id: $dotest/info || true
+ fi
if test '' != $ADD_SIGNOFF
then
echo $ADD_SIGNOFF
diff --git a/t/t5100/info0004 b/t/t5100/info0004
index 616c309..f7e2983 100644
--- a/t/t5100/info0004
+++ b/t/t5100/info0004
@@ -2,4 +2,5 @@ Author: YOSHIFUJI Hideaki / 吉藤英明
Email: yoshf...@linux-ipv6.org
Subject: GIT: Try all addresses for given remote name
Date: Thu, 21 Jul 2005 09:10:36 -0400 (EDT)
+Message-Id: 20050721.091036.01119516.yoshf...@linux-ipv6.org
diff --git a/t/t5100/info0005 b/t/t5100/info0005
index 46a46fc..592388f 100644
--- a/t/t5100/info0005
+++ b/t/t5100/info0005
@@ -2,4 +2,5 @@ Author: David Kågedal
Email: dav...@lysator.liu.se
Subject: Fixed two bugs in git-cvsimport-script.
Date: Mon, 15 Aug 2005 20:18:25 +0200
+Message-Id: u5tacjjdpxq@lysator.liu.se
diff --git a/t/t5100/info0012 b/t/t5100/info0012
index ac1216f..b5d89a1 100644
--- a/t/t5100/info0012
+++ b/t/t5100/info0012
@@ -2,4 +2,5 @@ Author: Dmitriy Blinov
Email: b...@mnsspb.ru
Subject: Изменён список пакетов необходимых для сборки
Date: Wed, 12 Nov 2008 17:54:41 +0300
+Message-Id:
Re: [PATCH v4 1/2] add `config_set` API for caching config files
Tanay Abhra tanay...@gmail.com writes:
Add a `config_set` construct that points to an ordered set of config files
specified by the user, each of which represents what was read and cached
in core as hashmaps. Add two external functions `git_configset_get_value`
and `git_configset_get_value_multi` for querying from the config sets,
which follow `last one wins` semantic(i.e. if there are multiple matches
Space before (
for the queried key in the files of the configset the value returned will
the last value in the last matching file of the configset) Add type
will _be_ ?
Does this remark also apply to git_configset_get_value_multi? My
understanding was that last one wins applied only to
git_configset_get_value.
Add a default `config_set`, `the_config_set` to cache all key-value pairs
I don't like the name the_config_set. It's not the only one. Perhaps
repo_config_set? (not totally satisfactory as it does not contain only
the repo, but the repo+user+system)
What do others think?
read from usual config files(repo specific .git/config, user wide
(You always forget space before '('...)
~/.gitconfig and the global /etc/gitconfig). `the_config_set` uses a
single hashmap populated using git_config(). The reason for doing so is
twofold, one is to honour include directives, another is to guarantee O(1)
lookups for usual config values, as values are queried for hundred of
times during a run of a git program.
What is the reason to deal with `the_config_set` and other config sets
differently? You're giving arguments to store `the_config_set` as a
single hashmap, but what is the reason to store others as multiple
hashmaps?
And actually, I don't completely buy your arguments: having 3 or 4
hashmaps (.git/config, ~/.gitconfig, ~/.config/git/config and
/etc/gitconfig) would be a O(4) lookup, which is still O(1), and you
could deal with include directives by having .git/config and included
files in a hashmap, ~/.gitconfig and included files in a second
hashmap, ...
My guess is that the real argument is git_config already does what I
want and I'm too lazy to change it. And I do consider lazyness as a
quality for a computer-scientist ;-).
I would personally find it much simpler to have a single hashmap. We'd
lose the ability to invalidate the cache for only a single file, but I'm
not sure it's worth the code complexity.
+Querying For Specific Variables
+---
+
+For programs wanting to query for specific variables in a non-callback
+manner, the config API provides two functions `git_config_get_value`
+and `git_config_get_value_multi`.They both read values from an internal
Space after .
+`git_config_iter` gives a way to iterate over the keys in cache. Existing
+`git_config` callback function signature is used for iterating.
Existing refers to the old state. It's OK in a commit message, but
won't make sense in the future, when your non-callback API and the old
callback API will live side by side.
+The config API also provides type specific API functions which do conversion
+as well as retrieval for the queried variable, including:
+
+`git_config_get_int`::
+Parse the retrieved value to an integer, including unit factors. Dies on
+error; otherwise, allocates and copies the integer into the dest parameter.
+Returns 0 on success, or 1 if no value was found.
It seems strange to me to return 1 here, and -1 in git_config_get_value
to mean the same thing.
+`git_config_bool`::
Isn't it git_config_get_bool?
+/* config-hash.c */
You may point to the documentation in the comment too.
+#define CONFIG_SET_INIT { NULL, 0, 0 }
+
+struct config_set {
+ struct config_set_item *item;
+ unsigned int nr, alloc;
+};
+
+struct config_set_item {
+ struct hashmap config_hash;
+ char *filename;
Perhaps const char *?
+static struct hashmap *add_config_hash(const char *filename, struct
config_set *cs)
+{
+ int ret = 0;
+ struct config_set_item *ct;
+ struct config_set_cb cb;
+ ALLOC_GROW(cs-item, cs-nr + 1, cs-alloc);
+ ct = cs-item[cs-nr++];
+ ct-filename = xstrdup(filename);
+ config_hash_init(ct-config_hash);
+ cb.cs = cs;
+ cb.ct = ct;
+ /*
+ * When filename is default, hashmap is constructed from the usual
set of
+ * config files (i.e repo specific .git/config, user wide ~/.gitconfig
and the
+ * global /etc/gitconfig), used in `the_config_set`
+ */
+ if (!strcmp(filename, default))
How does one read a file actually called default with this API?
More generally, why do you need a special-case here? I think it would be
much better to leave this part unaware of the default, and have a
separate function like create_repo_config_hash (or
create_the_config_hash) that would call git_config(...). There actually
isn't much shared code between the default case and the others in your
function.
+static struct hashmap *get_config_hash(const char *filename, struct
Re: [PATCH 2/2] test-config: Add tests for the config_set API
Tanay Abhra tanay...@gmail.com writes:
+test_expect_success 'clear default config' '
+ rm -f .git/config
+'
+
+cat .git/config EOF
t/README says:
- Put all code inside test_expect_success and other assertions.
Even code that isn't a test per se, but merely some setup code
should be inside a test assertion.
Even these cat would better be in a test_expect_success 'initialize
config'.
(Not applied everywhere in Git's code essentially because some tests
were written before the guideline was set and never updated).
+[core]
+ penguin = very blue
+ Movie = BadPhysics
+ UPPERCASE = true
+ MixedCase = true
+ my =
+ foo
+ baz = sam
+[Cores]
+ WhatEver = Second
+[my Foo bAr]
+ hi = hello
To really stress the case sensitive middle part case, you should also
have other sections like
[my foo bar]
hi = lower-case
[my FOO BAR]
hi = upper-case
and check that you get the right value for my.*.hi
Similarly, I'd add a [CORE] and a [CoRe] section to check that their
content is actually merged with [core].
+test_expect_success 'get value for a key with value as an empty string' '
+ echo expect
+ test-config get_value core.my actual
+ test_cmp expect actual
+'
+
+test_expect_success 'get value for a key with value as NULL' '
+ echo (NULL) expect
+ test-config get_value core.foo actual
+ test_cmp expect actual
+'
+test_expect_success 'upper case key' '
Keep the style consistent, if you separate tests with a single blank
line, do it everywhere.
+cat expect EOF
See above, should be in test_expect_success.
Also, expect, not expect.
There are other instances.
+1
+0
+1
+1
+1
+EOF
+
+test_expect_success 'find bool value for the entered key' '
+ test-config get_bool goat.head actual
The first one should be a single , or you should clear actual before
the test.
+int main(int argc, char **argv)
+{
+ int i, no_of_files;
+ int ret = 0;
+ const char *v;
+ int val;
+ const struct string_list *strptr;
+ struct config_set cs = CONFIG_SET_INIT;
+ if (argc == 3 !strcmp(argv[1], get_value)) {
+ if (!git_config_get_value(argv[2], v)) {
+ if (!v)
+ printf((NULL)\n);
+ else
+ printf(%s\n, v);
+ return 0;
+ } else {
+ printf(Value not found for \%s\\n, argv[2]);
+ return -1;
+ }
+ } else if (argc == 3 !strcmp(argv[1], get_value_multi)) {
+ strptr = git_config_get_value_multi(argv[2]);
+ if (strptr) {
+ for (i = 0; i strptr-nr; i++) {
+ v = strptr-items[i].string;
+ if (!v)
+ printf((NULL)\n);
+ else
+ printf(%s\n, v);
+ }
+ return 0;
+ } else {
+ printf(Value not found for \%s\\n, argv[2]);
+ return -1;
+ }
+ } else if (argc == 3 !strcmp(argv[1], get_int)) {
+ if (!git_config_get_int(argv[2], val)) {
+ printf(%d\n, val);
+ return 0;
+ } else {
+ printf(Value not found for \%s\\n, argv[2]);
+ return -1;
+ }
+ } else if (argc == 3 !strcmp(argv[1], get_bool)) {
+ if (!git_config_get_bool(argv[2], val)) {
+ printf(%d\n, val);
+ return 0;
+ } else {
+ printf(Value not found for \%s\\n, argv[2]);
+ return -1;
+ }
+ } else if (!strcmp(argv[1], configset_get_value)) {
+ no_of_files = git_config_int(unused, argv[2]);
Why ask the user to give a number of files on the command-line. With a
syntax like
test-config configset_get_value key files...
you could just use argc to iterate over argv. Here, you trust the user
to provide the right value, and most likely segfault otherwise (and this
is not really documented). I know this is only test code, but why not do
it right anyway ;-).
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] git-am: add option to extract email Message-Id: tag into commit log
--- a/git-am.sh
+++ b/git-am.sh
@@ -39,6 +39,7 @@ committer-date-is-author-datelie about committer date
ignore-date use current timestamp for author date
rerere-autoupdate update the index with reused conflict resolution if
possible
S,gpg-sign? GPG-sign commits
+m,message-idcopy the Message-Id: header to the commit's tag stanza
rebasing* (internal use for git-rebase)
. git-sh-setup
@@ -371,7 +372,7 @@ split_patches () {
prec=4
dotest=$GIT_DIR/rebase-apply
sign= utf8=t keep= keepcr= skip= interactive= resolved= rebasing= abort=
-resolvemsg= resume= scissors= no_inbody_headers=
+resolvemsg= resume= scissors= no_inbody_headers= message_id=
git_apply_opt=
committer_date_is_author_date=
ignore_date=
@@ -442,6 +443,8 @@ it will be removed. Please do not use it anymore.
gpg_sign_opt=-S ;;
--gpg-sign=*)
gpg_sign_opt=-S${1#--gpg-sign=} ;;
+ -m|--message-id)
+ message_id=t ;;
--)
shift; break ;;
*)
@@ -565,6 +568,7 @@ Use \git am --abort\ to remove it.)
echo $git_apply_opt $dotest/apply-opt
echo $threeway $dotest/threeway
echo $sign $dotest/sign
+ echo $message_id $dotest/message-id
echo $utf8 $dotest/utf8
echo $keep $dotest/keep
echo $scissors $dotest/scissors
@@ -757,6 +761,10 @@ To restore the original branch and stop patching run
\\$cmdline --abort\.
then
cat $dotest/msg-clean
fi
+ if test 't' == $message_id
The == is bash special, please use = instead
And the 't' can be written as t, so that the whole line looks like this:
if test t = $message_id
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/2] add `config_set` API for caching config files
On 7/2/2014 2:44 PM, Matthieu Moy wrote:
Tanay Abhra tanay...@gmail.com writes:
Add a `config_set` construct that points to an ordered set of config files
specified by the user, each of which represents what was read and cached
in core as hashmaps. Add two external functions `git_configset_get_value`
and `git_configset_get_value_multi` for querying from the config sets,
which follow `last one wins` semantic(i.e. if there are multiple matches
Space before (
Noted. Sorry about that.
for the queried key in the files of the configset the value returned will
the last value in the last matching file of the configset) Add type
Does this remark also apply to git_configset_get_value_multi? My
understanding was that last one wins applied only to
git_configset_get_value.
Maybe a reworded sentence may work,
`git_configset_get_value_multi` returns a list of values sorted in order of
increasing priority (i.e. last match will be at the end of the list.)
Add a default `config_set`, `the_config_set` to cache all key-value pairs
I don't like the name the_config_set. It's not the only one. Perhaps
repo_config_set? (not totally satisfactory as it does not contain only
the repo, but the repo+user+system)
What do others think?
read from usual config files(repo specific .git/config, user wide
~/.gitconfig and the global /etc/gitconfig). `the_config_set` uses a
single hashmap populated using git_config(). The reason for doing so is
twofold, one is to honour include directives, another is to guarantee O(1)
lookups for usual config values, as values are queried for hundred of
times during a run of a git program.
What is the reason to deal with `the_config_set` and other config sets
differently? You're giving arguments to store `the_config_set` as a
single hashmap, but what is the reason to store others as multiple
hashmaps?
And actually, I don't completely buy your arguments: having 3 or 4
hashmaps (.git/config, ~/.gitconfig, ~/.config/git/config and
/etc/gitconfig) would be a O(4) lookup, which is still O(1), and you
could deal with include directives by having .git/config and included
files in a hashmap, ~/.gitconfig and included files in a second
hashmap, ...
My guess is that the real argument is git_config already does what I
want and I'm too lazy to change it. And I do consider lazyness as a
quality for a computer-scientist ;-).
I would personally find it much simpler to have a single hashmap. We'd
lose the ability to invalidate the cache for only a single file, but I'm
not sure it's worth the code complexity.
Point noted. I can take the multiple hashmap route for `the_config_set`.
Still, since it will be the most used config set in the code and for each
query it would have to look through n hashmaps hampering performance. I
can change it if you want but like you, I don't think it is worth the code
complexity.
+Querying For Specific Variables
+---
+
+For programs wanting to query for specific variables in a non-callback
+manner, the config API provides two functions `git_config_get_value`
+and `git_config_get_value_multi`. They both read values from an internal
+`git_config_iter` gives a way to iterate over the keys in cache. Existing
+`git_config` callback function signature is used for iterating.
Existing refers to the old state. It's OK in a commit message, but
won't make sense in the future, when your non-callback API and the old
callback API will live side by side.
Noted.
+The config API also provides type specific API functions which do conversion
+as well as retrieval for the queried variable, including:
+
+`git_config_get_int`::
+Parse the retrieved value to an integer, including unit factors. Dies on
+error; otherwise, allocates and copies the integer into the dest parameter.
+Returns 0 on success, or 1 if no value was found.
It seems strange to me to return 1 here, and -1 in git_config_get_value
to mean the same thing.
Noted. Some of the type specific function return -1 on wrong parsing, I will
put return value 1 for no value found in all of the cases.
+`git_config_bool`::
+/* config-hash.c */
+#define CONFIG_SET_INIT { NULL, 0, 0 }
+
+struct config_set {
+struct config_set_item *item;
+unsigned int nr, alloc;
+};
+
+struct config_set_item {
+struct hashmap config_hash;
+char *filename;
+static struct hashmap *add_config_hash(const char *filename, struct
config_set *cs)
+{
+int ret = 0;
+struct config_set_item *ct;
+struct config_set_cb cb;
+ALLOC_GROW(cs-item, cs-nr + 1, cs-alloc);
+ct = cs-item[cs-nr++];
+ct-filename = xstrdup(filename);
+config_hash_init(ct-config_hash);
+cb.cs = cs;
+cb.ct = ct;
+/*
+ * When filename is default, hashmap is constructed from the usual
set of
+ * config files (i.e repo specific .git/config, user wide ~/.gitconfig
and the
+ * global /etc/gitconfig), used
Re: [PATCH 2/2] test-config: Add tests for the config_set API
On 7/2/2014 2:59 PM, Matthieu Moy wrote:
Tanay Abhra tanay...@gmail.com writes:
+test_expect_success 'clear default config' '
+rm -f .git/config
+'
+
+cat .git/config EOF
t/README says:
- Put all code inside test_expect_success and other assertions.
Even code that isn't a test per se, but merely some setup code
should be inside a test assertion.
Even these cat would better be in a test_expect_success 'initialize
config'.
(Not applied everywhere in Git's code essentially because some tests
were written before the guideline was set and never updated).
Sorry about that. I followed t1300-repo-config.sh which has these mistakes
also.
+[core]
+penguin = very blue
+Movie = BadPhysics
+UPPERCASE = true
+MixedCase = true
+my =
+foo
+baz = sam
+[Cores]
+WhatEver = Second
+[my Foo bAr]
+hi = hello
To really stress the case sensitive middle part case, you should also
have other sections like
[my foo bar]
hi = lower-case
[my FOO BAR]
hi = upper-case
and check that you get the right value for my.*.hi
Similarly, I'd add a [CORE] and a [CoRe] section to check that their
content is actually merged with [core].
Noted.
+test_expect_success 'get value for a key with value as an empty string' '
+echo expect
+test-config get_value core.my actual
+test_cmp expect actual
+'
+
+test_expect_success 'get value for a key with value as NULL' '
+echo (NULL) expect
+test-config get_value core.foo actual
+test_cmp expect actual
+'
+test_expect_success 'upper case key' '
Keep the style consistent, if you separate tests with a single blank
line, do it everywhere.
+cat expect EOF
See above, should be in test_expect_success.
Also, expect, not expect.
There are other instances.
Noted. Again copied t1300-repo-config.sh style for cat.
+1
+0
+1
+1
+1
+EOF
+
+test_expect_success 'find bool value for the entered key' '
+test-config get_bool goat.head actual
The first one should be a single , or you should clear actual before
the test.
Noted.
+int main(int argc, char **argv)
+{
+int i, no_of_files;
+int ret = 0;
+const char *v;
+int val;
+const struct string_list *strptr;
+struct config_set cs = CONFIG_SET_INIT;
+if (argc == 3 !strcmp(argv[1], get_value)) {
+if (!git_config_get_value(argv[2], v)) {
+if (!v)
+printf((NULL)\n);
+else
+printf(%s\n, v);
+return 0;
+} else {
+printf(Value not found for \%s\\n, argv[2]);
+return -1;
+}
+} else if (argc == 3 !strcmp(argv[1], get_value_multi)) {
+strptr = git_config_get_value_multi(argv[2]);
+if (strptr) {
+for (i = 0; i strptr-nr; i++) {
+v = strptr-items[i].string;
+if (!v)
+printf((NULL)\n);
+else
+printf(%s\n, v);
+}
+return 0;
+} else {
+printf(Value not found for \%s\\n, argv[2]);
+return -1;
+}
+} else if (argc == 3 !strcmp(argv[1], get_int)) {
+if (!git_config_get_int(argv[2], val)) {
+printf(%d\n, val);
+return 0;
+} else {
+printf(Value not found for \%s\\n, argv[2]);
+return -1;
+}
+} else if (argc == 3 !strcmp(argv[1], get_bool)) {
+if (!git_config_get_bool(argv[2], val)) {
+printf(%d\n, val);
+return 0;
+} else {
+printf(Value not found for \%s\\n, argv[2]);
+return -1;
+}
+} else if (!strcmp(argv[1], configset_get_value)) {
+no_of_files = git_config_int(unused, argv[2]);
Why ask the user to give a number of files on the command-line. With a
syntax like
test-config configset_get_value key files...
you could just use argc to iterate over argv. Here, you trust the user
to provide the right value, and most likely segfault otherwise (and this
is not really documented). I know this is only test code, but why not do
it right anyway ;-).
Yup, your way is much better, thanks for the review.
Tanay.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3] git-am: add option to extract email Message-Id: tag into commit log
Some workflows prefer to track exactly which email message was used to
generate a commit. This can be used, for example, to generate an
automated acknowledgement when a patch is committed as a response to
the patch email, or as a reference to the thread which introduced the
patch.
Support this by adding a --message-id option (abbreviated as -m) to
git-am, which will then extract the message ID and append it to the
email commit log.
Signed-off-by: Avi Kivity a...@cloudius-systems.com
---
v3: remove bashism and unneeded quoting
v2: adjust to pass test suite (t5100)
Documentation/git-am.txt | 6 ++
builtin/mailinfo.c | 2 +-
git-am.sh| 10 +-
t/t5100/info0004 | 1 +
t/t5100/info0005 | 1 +
t/t5100/info0012 | 1 +
6 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt
index 9adce37..8a251a1 100644
--- a/Documentation/git-am.txt
+++ b/Documentation/git-am.txt
@@ -15,6 +15,7 @@ SYNOPSIS
[--whitespace=option] [-Cn] [-pn] [--directory=dir]
[--exclude=path] [--include=path] [--reject] [-q | --quiet]
[--[no-]scissors] [-S[keyid]] [--patch-format=format]
+[--message-id]
[(mbox | Maildir)...]
'git am' (--continue | --skip | --abort)
@@ -121,6 +122,11 @@ default. You can use `--no-utf8` to override this.
user to lie about the author date by using the same
value as the committer date.
+-m::
+--message-id::
+ Extract the Message-Id: header from the e-mail and
+ append it to the commit message's tag stanza.
+
--skip::
Skip the current patch. This is only meaningful when
restarting an aborted patch.
diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index cf11c8d..f1e1fed 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -278,7 +278,7 @@ static void cleanup_space(struct strbuf *sb)
static void decode_header(struct strbuf *line);
static const char *header[MAX_HDR_PARSED] = {
- From,Subject,Date,
+ From,Subject,Date,Message-Id
};
static inline int cmp_header(const struct strbuf *line, const char *hdr)
diff --git a/git-am.sh b/git-am.sh
index ee61a77..9fae315 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -39,6 +39,7 @@ committer-date-is-author-datelie about committer date
ignore-date use current timestamp for author date
rerere-autoupdate update the index with reused conflict resolution if possible
S,gpg-sign? GPG-sign commits
+m,message-idcopy the Message-Id: header to the commit's tag stanza
rebasing* (internal use for git-rebase)
. git-sh-setup
@@ -371,7 +372,7 @@ split_patches () {
prec=4
dotest=$GIT_DIR/rebase-apply
sign= utf8=t keep= keepcr= skip= interactive= resolved= rebasing= abort=
-resolvemsg= resume= scissors= no_inbody_headers=
+resolvemsg= resume= scissors= no_inbody_headers= message_id=
git_apply_opt=
committer_date_is_author_date=
ignore_date=
@@ -442,6 +443,8 @@ it will be removed. Please do not use it anymore.
gpg_sign_opt=-S ;;
--gpg-sign=*)
gpg_sign_opt=-S${1#--gpg-sign=} ;;
+ -m|--message-id)
+ message_id=t ;;
--)
shift; break ;;
*)
@@ -565,6 +568,7 @@ Use \git am --abort\ to remove it.)
echo $git_apply_opt $dotest/apply-opt
echo $threeway $dotest/threeway
echo $sign $dotest/sign
+ echo $message_id $dotest/message-id
echo $utf8 $dotest/utf8
echo $keep $dotest/keep
echo $scissors $dotest/scissors
@@ -757,6 +761,10 @@ To restore the original branch and stop patching run
\\$cmdline --abort\.
then
cat $dotest/msg-clean
fi
+ if test t = $message_id
+ then
+ grep ^Message-Id: $dotest/info || true
+ fi
if test '' != $ADD_SIGNOFF
then
echo $ADD_SIGNOFF
diff --git a/t/t5100/info0004 b/t/t5100/info0004
index 616c309..f7e2983 100644
--- a/t/t5100/info0004
+++ b/t/t5100/info0004
@@ -2,4 +2,5 @@ Author: YOSHIFUJI Hideaki / 吉藤英明
Email: yoshf...@linux-ipv6.org
Subject: GIT: Try all addresses for given remote name
Date: Thu, 21 Jul 2005 09:10:36 -0400 (EDT)
+Message-Id: 20050721.091036.01119516.yoshf...@linux-ipv6.org
diff --git a/t/t5100/info0005 b/t/t5100/info0005
index 46a46fc..592388f 100644
--- a/t/t5100/info0005
+++ b/t/t5100/info0005
@@ -2,4 +2,5 @@ Author: David Kågedal
Email: dav...@lysator.liu.se
Subject: Fixed two bugs in git-cvsimport-script.
Date: Mon, 15 Aug 2005 20:18:25 +0200
+Message-Id: u5tacjjdpxq@lysator.liu.se
diff --git a/t/t5100/info0012 b/t/t5100/info0012
index ac1216f..b5d89a1 100644
--- a/t/t5100/info0012
+++ b/t/t5100/info0012
@@ -2,4 +2,5 @@ Author: Dmitriy Blinov
Email: b...@mnsspb.ru
Subject: Изменён список пакетов необходимых для сборки
Date: Wed, 12 Nov 2008 17:54:41
Re: [PATCH v2] git-am: add option to extract email Message-Id: tag into commit log
On 07/02/2014 12:58 PM, Torsten Bögershausen wrote: @@ -757,6 +761,10 @@ To restore the original branch and stop patching run \\$cmdline --abort\. then cat $dotest/msg-clean fi + if test 't' == $message_id The == is bash special, please use = instead And the 't' can be written as t, so that the whole line looks like this: if test t = $message_id Thanks. v3 posted with these changes. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] git-merge-file: do not add LF at EOF while applying unrelated change
Hi Max, On Wed, 2 Jul 2014, Max Kirillov wrote: On Mon, Jun 30, 2014 at 04:55:10PM +0200, Johannes Schindelin wrote: I just wish the tests were a little easier to understand... What could be improved with them? Oh, I would name the files more appropriately, for example. That is, instead of test1.txt I would call it mixed-endings.txt or lf-only.txt or some such. And instead of the Latin version of Psalm 23, I would put lines into the files that describe their own role in the test, i.e. unchanged ends with a carriage return ends with a line feed unchanged or similar. Please keep in mind that this critique is most likely on my *own* work, for all I know *I* introduced those files. By the way, for \r\n eol it did even worse, adding just \n. And I guess it still adds just \n for union merge. Should file merge consider the core.eol? I think it should, and for the conflict markers also, it looks ugly when whole file has \r\n but the conflict markers have \n. But then git-merge-file could not be used outside of repository, I guess. Oh, why not? It could read the configuration if it's inside a working directory, and just read /etc/gitconfig and $HOME/.gitconfig when outside... In general, I wish file merging (and diffing) were more tolerant of the line endings in input. Because in windows environment, when people have different core.autocrlf, it becomes quite frustrating to always get conflicts and changes. Amen! Ciao, Dscho -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] git-am: add option to extract email Message-Id: tag into commit log
Hi Avi,
On 07/02/2014 10:51 AM, Avi Kivity wrote:
Some workflows prefer to track exactly which email message was used to
generate a commit. This can be used, for example, to generate an
automated acknowledgement when a patch is committed as a response to
the patch email, or as a reference to the thread which introduced the
patch.
Support this by adding a --message-id option (abbreviated as -m) to
git-am, which will then extract the message ID and append it to the
email commit log.
Signed-off-by: Avi Kivity a...@cloudius-systems.com
---
v2: adjust to pass test suite (t5100)
Documentation/git-am.txt | 6 ++
builtin/mailinfo.c | 2 +-
git-am.sh| 10 +-
t/t5100/info0004 | 1 +
t/t5100/info0005 | 1 +
t/t5100/info0012 | 1 +
6 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt
index 9adce37..8a251a1 100644
--- a/Documentation/git-am.txt
+++ b/Documentation/git-am.txt
@@ -15,6 +15,7 @@ SYNOPSIS
[--whitespace=option] [-Cn] [-pn] [--directory=dir]
[--exclude=path] [--include=path] [--reject] [-q | --quiet]
[--[no-]scissors] [-S[keyid]] [--patch-format=format]
+ [--message-id]
[(mbox | Maildir)...]
'git am' (--continue | --skip | --abort)
@@ -121,6 +122,11 @@ default. You can use `--no-utf8` to override this.
user to lie about the author date by using the same
value as the committer date.
+-m::
+--message-id::
+ Extract the Message-Id: header from the e-mail and
+ append it to the commit message's tag stanza.
+
--skip::
Skip the current patch. This is only meaningful when
restarting an aborted patch.
diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index cf11c8d..f1e1fed 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -278,7 +278,7 @@ static void cleanup_space(struct strbuf *sb)
static void decode_header(struct strbuf *line);
static const char *header[MAX_HDR_PARSED] = {
- From,Subject,Date,
+ From,Subject,Date,Message-Id
};
static inline int cmp_header(const struct strbuf *line, const char *hdr)
diff --git a/git-am.sh b/git-am.sh
index ee61a77..c0e7bdd 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -39,6 +39,7 @@ committer-date-is-author-datelie about committer date
ignore-date use current timestamp for author date
rerere-autoupdate update the index with reused conflict resolution if
possible
S,gpg-sign? GPG-sign commits
+m,message-idcopy the Message-Id: header to the commit's tag stanza
rebasing* (internal use for git-rebase)
. git-sh-setup
@@ -371,7 +372,7 @@ split_patches () {
prec=4
dotest=$GIT_DIR/rebase-apply
sign= utf8=t keep= keepcr= skip= interactive= resolved= rebasing= abort=
-resolvemsg= resume= scissors= no_inbody_headers=
+resolvemsg= resume= scissors= no_inbody_headers= message_id=
git_apply_opt=
committer_date_is_author_date=
ignore_date=
@@ -442,6 +443,8 @@ it will be removed. Please do not use it anymore.
gpg_sign_opt=-S ;;
--gpg-sign=*)
gpg_sign_opt=-S${1#--gpg-sign=} ;;
+ -m|--message-id)
+ message_id=t ;;
Doesn't the message-id line in OPTIONS_SPEC make the negated long
option --no-message-id available as well? If that's the case, then
the corresponding case arm is missing from here.
--)
shift; break ;;
*)
@@ -565,6 +568,7 @@ Use \git am --abort\ to remove it.)
echo $git_apply_opt $dotest/apply-opt
echo $threeway $dotest/threeway
echo $sign $dotest/sign
+ echo $message_id $dotest/message-id
To match the local style conventions, the space character after the
redirection operator should be removed.
Also, isn't the patch missing the bits where the state of message-id
is read? Like so:
if test $(cat $dotest/message-id) = t
then
message_id=t
fi
echo $utf8 $dotest/utf8
echo $keep $dotest/keep
echo $scissors $dotest/scissors
@@ -757,6 +761,10 @@ To restore the original branch and stop patching run
\\$cmdline --abort\.
then
cat $dotest/msg-clean
fi
+ if test 't' == $message_id
+ then
+ grep ^Message-Id: $dotest/info || true
Why is the true guard needed here? The exit status of grep seems to
never be checked.
Although I cannot come up with an example where this would matter,
you might want to consider using the grep wrapper sane_grep from
git-sh-setup.sh instead. It resets the environment variable
GREP_OPTIONS before calling grep so that no unexpected user options
come into play.
+ fi
if test '' != $ADD_SIGNOFF
then
echo $ADD_SIGNOFF
[..]
Fabian
--
To unsubscribe from this list: send the line unsubscribe git in
the body of
Re: [PATCH v4 1/2] add `config_set` API for caching config files
Tanay Abhra tanay...@gmail.com writes:
On 7/2/2014 2:44 PM, Matthieu Moy wrote:
Tanay Abhra tanay...@gmail.com writes:
Maybe a reworded sentence may work,
`git_configset_get_value_multi` returns a list of values sorted in order of
increasing priority (i.e. last match will be at the end of the list.)
OK.
read from usual config files(repo specific .git/config, user wide
~/.gitconfig and the global /etc/gitconfig). `the_config_set` uses a
single hashmap populated using git_config(). The reason for doing so is
twofold, one is to honour include directives, another is to guarantee O(1)
lookups for usual config values, as values are queried for hundred of
times during a run of a git program.
What is the reason to deal with `the_config_set` and other config sets
differently? You're giving arguments to store `the_config_set` as a
single hashmap, but what is the reason to store others as multiple
hashmaps?
And actually, I don't completely buy your arguments: having 3 or 4
hashmaps (.git/config, ~/.gitconfig, ~/.config/git/config and
/etc/gitconfig) would be a O(4) lookup, which is still O(1), and you
could deal with include directives by having .git/config and included
files in a hashmap, ~/.gitconfig and included files in a second
hashmap, ...
My guess is that the real argument is git_config already does what I
want and I'm too lazy to change it. And I do consider lazyness as a
quality for a computer-scientist ;-).
I would personally find it much simpler to have a single hashmap. We'd
lose the ability to invalidate the cache for only a single file, but I'm
not sure it's worth the code complexity.
Point noted. I can take the multiple hashmap route for `the_config_set`.
Still, since it will be the most used config set in the code and for each
query it would have to look through n hashmaps hampering performance. I
can change it if you want but like you, I don't think it is worth the code
complexity.
That's why my suggestion is to use a single hashmap everywhere.
I don't have strong opinion either way, but whichever way you go, you
should justify the choice better in the commit message.
+The config API also provides type specific API functions which do
conversion
+as well as retrieval for the queried variable, including:
+
+`git_config_get_int`::
+Parse the retrieved value to an integer, including unit factors. Dies on
+error; otherwise, allocates and copies the integer into the dest parameter.
+Returns 0 on success, or 1 if no value was found.
It seems strange to me to return 1 here, and -1 in git_config_get_value
to mean the same thing.
Noted. Some of the type specific function return -1 on wrong parsing, I will
put return value 1 for no value found in all of the cases.
I'm not sure I fully get the existing convention. My understanding is
that when the extracted value is returned, -1 is used as a special value
to mean no value (e.g. git_config_maybe_bool can return 1, 0 or -1),
but when the extracted value is written to a by-address parameter, then
the return value is 1 or 0.
+static struct hashmap *get_config_hash(const char *filename, struct
config_set *cs)
+{
+ int i;
+ for(i = 0; i cs-nr; i++) {
+ if (!strcmp(cs-item[i].filename, filename))
+ return cs-item[i].config_hash;
+ }
+ return add_config_hash(filename, cs);
+}
+
+static struct config_hash_entry *config_hash_find_entry(const char *key,
const char* filename,
+ struct config_set *cs)
I don't get the logic here.
Either the caller explicitly manages a config_set variable like
config_set my_set = git_configset_init();
git_configset_add_file(my_set, my-file);
git_configset_get_value(my_set, some.variable.name);
Or there's an implicit singleton mapping files to hashmaps to allow the
user to say
git_configset_get_value(my-file, some.variable.name);
without asking the user to explicitly manipulate config_set variables.
It seems to me that your solution is a bad compromise between both
solutions: you do require the user to manipulate config_set variables,
but you also require a filename to look for the right entry in the list.
Did you miss the end of Junio's message:
http://thread.gmane.org/gmane.comp.version-control.git/252567
?
This is an internal function which is used to iterate over every `config_set`
item which contains a hashmap and filename as an identifier.
The exposed API which I documented above doesn't require the user to pass the
filename when querying for a value.
The API works like this, suppose there are two files,
A.config B.config
[foo] [foo]
a = b a = d
a = c a = e
config_set *my_set = git_configset_init();
git_configset_add_file(my_set, A.config);
git_configset_add_file(my_set, B.config);
git_configset_get_value(my_set, foo.a);
Here get_value calls config_hash_find_entry once for each
Re: [PATCH v2] git-am: add option to extract email Message-Id: tag into commit log
On 07/02/2014 05:18 PM, Fabian Ruch wrote:
Hi Avi,
On 07/02/2014 10:51 AM, Avi Kivity wrote:
Some workflows prefer to track exactly which email message was used to
generate a commit. This can be used, for example, to generate an
automated acknowledgement when a patch is committed as a response to
the patch email, or as a reference to the thread which introduced the
patch.
Support this by adding a --message-id option (abbreviated as -m) to
git-am, which will then extract the message ID and append it to the
email commit log.
Signed-off-by: Avi Kivity a...@cloudius-systems.com
---
v2: adjust to pass test suite (t5100)
Documentation/git-am.txt | 6 ++
builtin/mailinfo.c | 2 +-
git-am.sh| 10 +-
t/t5100/info0004 | 1 +
t/t5100/info0005 | 1 +
t/t5100/info0012 | 1 +
6 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt
index 9adce37..8a251a1 100644
--- a/Documentation/git-am.txt
+++ b/Documentation/git-am.txt
@@ -15,6 +15,7 @@ SYNOPSIS
[--whitespace=option] [-Cn] [-pn] [--directory=dir]
[--exclude=path] [--include=path] [--reject] [-q | --quiet]
[--[no-]scissors] [-S[keyid]] [--patch-format=format]
+[--message-id]
[(mbox | Maildir)...]
'git am' (--continue | --skip | --abort)
@@ -121,6 +122,11 @@ default. You can use `--no-utf8` to override this.
user to lie about the author date by using the same
value as the committer date.
+-m::
+--message-id::
+ Extract the Message-Id: header from the e-mail and
+ append it to the commit message's tag stanza.
+
--skip::
Skip the current patch. This is only meaningful when
restarting an aborted patch.
diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index cf11c8d..f1e1fed 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -278,7 +278,7 @@ static void cleanup_space(struct strbuf *sb)
static void decode_header(struct strbuf *line);
static const char *header[MAX_HDR_PARSED] = {
- From,Subject,Date,
+ From,Subject,Date,Message-Id
};
static inline int cmp_header(const struct strbuf *line, const char *hdr)
diff --git a/git-am.sh b/git-am.sh
index ee61a77..c0e7bdd 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -39,6 +39,7 @@ committer-date-is-author-datelie about committer date
ignore-date use current timestamp for author date
rerere-autoupdate update the index with reused conflict resolution if possible
S,gpg-sign? GPG-sign commits
+m,message-idcopy the Message-Id: header to the commit's tag stanza
rebasing* (internal use for git-rebase)
. git-sh-setup
@@ -371,7 +372,7 @@ split_patches () {
prec=4
dotest=$GIT_DIR/rebase-apply
sign= utf8=t keep= keepcr= skip= interactive= resolved= rebasing= abort=
-resolvemsg= resume= scissors= no_inbody_headers=
+resolvemsg= resume= scissors= no_inbody_headers= message_id=
git_apply_opt=
committer_date_is_author_date=
ignore_date=
@@ -442,6 +443,8 @@ it will be removed. Please do not use it anymore.
gpg_sign_opt=-S ;;
--gpg-sign=*)
gpg_sign_opt=-S${1#--gpg-sign=} ;;
+ -m|--message-id)
+ message_id=t ;;
Doesn't the message-id line in OPTIONS_SPEC make the negated long
option --no-message-id available as well? If that's the case, then
the corresponding case arm is missing from here.
I don't know, but some other booleans don't supply negations, for
example --reject.
--)
shift; break ;;
*)
@@ -565,6 +568,7 @@ Use \git am --abort\ to remove it.)
echo $git_apply_opt $dotest/apply-opt
echo $threeway $dotest/threeway
echo $sign $dotest/sign
+ echo $message_id $dotest/message-id
To match the local style conventions, the space character after the
redirection operator should be removed.
Sure.
Also, isn't the patch missing the bits where the state of message-id
is read? Like so:
if test $(cat $dotest/message-id) = t
then
message_id=t
fi
Good catch, I guess this fixes a restarted am.
echo $utf8 $dotest/utf8
echo $keep $dotest/keep
echo $scissors $dotest/scissors
@@ -757,6 +761,10 @@ To restore the original branch and stop patching run \\$cmdline
--abort\.
then
cat $dotest/msg-clean
fi
+ if test 't' == $message_id
+ then
+ grep ^Message-Id: $dotest/info || true
Why is the true guard needed here? The exit status of grep seems to
never be checked.
I usually code scripts with -e, but here it's unneeded.
Although I cannot come up with an example where this would matter,
you might want to consider using the grep wrapper sane_grep from
git-sh-setup.sh instead. It resets the environment variable
GREP_OPTIONS before calling grep so that no
[PATCH v4] git-am: add option to extract email Message-Id: tag into commit log
Some workflows prefer to track exactly which email message was used to
generate a commit. This can be used, for example, to generate an
automated acknowledgement when a patch is committed as a response to
the patch email, or as a reference to the thread which introduced the
patch.
Support this by adding a --message-id option (abbreviated as -m) to
git-am, which will then extract the message ID and append it to the
email commit log.
Signed-off-by: Avi Kivity a...@cloudius-systems.com
---
v4: adjust coding style
recover message_id variable after a resumed git-am
use sane_grep
drop unneeded grep error handling
v3: remove bashism and unneeded quoting
v2: adjust to pass test suite (t5100)
Documentation/git-am.txt | 6 ++
builtin/mailinfo.c | 2 +-
git-am.sh| 14 +-
t/t5100/info0004 | 1 +
t/t5100/info0005 | 1 +
t/t5100/info0012 | 1 +
6 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt
index 9adce37..8a251a1 100644
--- a/Documentation/git-am.txt
+++ b/Documentation/git-am.txt
@@ -15,6 +15,7 @@ SYNOPSIS
[--whitespace=option] [-Cn] [-pn] [--directory=dir]
[--exclude=path] [--include=path] [--reject] [-q | --quiet]
[--[no-]scissors] [-S[keyid]] [--patch-format=format]
+[--message-id]
[(mbox | Maildir)...]
'git am' (--continue | --skip | --abort)
@@ -121,6 +122,11 @@ default. You can use `--no-utf8` to override this.
user to lie about the author date by using the same
value as the committer date.
+-m::
+--message-id::
+ Extract the Message-Id: header from the e-mail and
+ append it to the commit message's tag stanza.
+
--skip::
Skip the current patch. This is only meaningful when
restarting an aborted patch.
diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index cf11c8d..f1e1fed 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -278,7 +278,7 @@ static void cleanup_space(struct strbuf *sb)
static void decode_header(struct strbuf *line);
static const char *header[MAX_HDR_PARSED] = {
- From,Subject,Date,
+ From,Subject,Date,Message-Id
};
static inline int cmp_header(const struct strbuf *line, const char *hdr)
diff --git a/git-am.sh b/git-am.sh
index ee61a77..fd0181f 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -39,6 +39,7 @@ committer-date-is-author-datelie about committer date
ignore-date use current timestamp for author date
rerere-autoupdate update the index with reused conflict resolution if possible
S,gpg-sign? GPG-sign commits
+m,message-idcopy the Message-Id: header to the commit's tag stanza
rebasing* (internal use for git-rebase)
. git-sh-setup
@@ -371,7 +372,7 @@ split_patches () {
prec=4
dotest=$GIT_DIR/rebase-apply
sign= utf8=t keep= keepcr= skip= interactive= resolved= rebasing= abort=
-resolvemsg= resume= scissors= no_inbody_headers=
+resolvemsg= resume= scissors= no_inbody_headers= message_id=
git_apply_opt=
committer_date_is_author_date=
ignore_date=
@@ -442,6 +443,8 @@ it will be removed. Please do not use it anymore.
gpg_sign_opt=-S ;;
--gpg-sign=*)
gpg_sign_opt=-S${1#--gpg-sign=} ;;
+ -m|--message-id)
+ message_id=t ;;
--)
shift; break ;;
*)
@@ -565,6 +568,7 @@ Use \git am --abort\ to remove it.)
echo $git_apply_opt $dotest/apply-opt
echo $threeway $dotest/threeway
echo $sign $dotest/sign
+ echo $message_id $dotest/message-id
echo $utf8 $dotest/utf8
echo $keep $dotest/keep
echo $scissors $dotest/scissors
@@ -651,6 +655,10 @@ then
else
SIGNOFF=
fi
+if test $(cat $dotest/message-id) = t
+then
+ message_id=t
+fi
last=$(cat $dotest/last)
this=$(cat $dotest/next)
@@ -757,6 +765,10 @@ To restore the original branch and stop patching run
\\$cmdline --abort\.
then
cat $dotest/msg-clean
fi
+ if test t = $message_id
+ then
+ sane_grep ^Message-Id: $dotest/info
+ fi
if test '' != $ADD_SIGNOFF
then
echo $ADD_SIGNOFF
diff --git a/t/t5100/info0004 b/t/t5100/info0004
index 616c309..f7e2983 100644
--- a/t/t5100/info0004
+++ b/t/t5100/info0004
@@ -2,4 +2,5 @@ Author: YOSHIFUJI Hideaki / 吉藤英明
Email: yoshf...@linux-ipv6.org
Subject: GIT: Try all addresses for given remote name
Date: Thu, 21 Jul 2005 09:10:36 -0400 (EDT)
+Message-Id: 20050721.091036.01119516.yoshf...@linux-ipv6.org
diff --git a/t/t5100/info0005 b/t/t5100/info0005
index 46a46fc..592388f 100644
--- a/t/t5100/info0005
+++ b/t/t5100/info0005
@@ -2,4 +2,5 @@ Author: David Kågedal
Email: dav...@lysator.liu.se
Subject: Fixed two bugs in git-cvsimport-script.
Date: Mon, 15 Aug 2005 20:18:25 +0200
Re: [PATCH 00/14] Add submodule test harness
(Not sure if this is the right thread) (I haven't checked if this is fixed in your latest version) On what I have on pu 7a0da7902cbbc9a876b90c9, Tue Jul 1 14:51:53 2014 -0700 Many submodule tests are broken. One problem is here: lib-submodule-update.sh:264: possible problem: echo -n is not portable (please use printf): echo -n sub1 lib-submodule-update.sh:507: possible problem: echo -n is not portable (please use printf): echo -n sub1 You can remove the empty echo -n to create an empty file: sub1 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Show containing branches in log?
I know that with the `git branch` command I can determine which branches contain a commit. Is there a way to represent this graphically with `git log`? Sometimes I just have a commit, and I need to find out what branch contains that commit. The reason why `git branch --contains` doesn't solve this problem for me is that it names almost all branches because of merge commits. Too much ancestry has been built since this commit, so there is no way to find the closest branch that contains that commit. Is there a way to graphically see what is the nearest named ref to the specified commit in the logs? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] git-am: add option to extract email Message-Id: tag into commit log
diff --git a/git-am.sh b/git-am.sh
index ee61a77..fd0181f 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -39,6 +39,7 @@ committer-date-is-author-datelie about committer date
ignore-date use current timestamp for author date
rerere-autoupdate update the index with reused conflict resolution if
possible
S,gpg-sign? GPG-sign commits
+m,message-idcopy the Message-Id: header to the commit's tag stanza
rebasing* (internal use for git-rebase)
. git-sh-setup
@@ -371,7 +372,7 @@ split_patches () {
prec=4
dotest=$GIT_DIR/rebase-apply
sign= utf8=t keep= keepcr= skip= interactive= resolved= rebasing= abort=
-resolvemsg= resume= scissors= no_inbody_headers=
+resolvemsg= resume= scissors= no_inbody_headers= message_id=
git_apply_opt=
committer_date_is_author_date=
ignore_date=
@@ -442,6 +443,8 @@ it will be removed. Please do not use it anymore.
gpg_sign_opt=-S ;;
--gpg-sign=*)
gpg_sign_opt=-S${1#--gpg-sign=} ;;
+ -m|--message-id)
+ message_id=t ;;
--)
shift; break ;;
*)
@@ -565,6 +568,7 @@ Use \git am --abort\ to remove it.)
echo $git_apply_opt $dotest/apply-opt
echo $threeway $dotest/threeway
echo $sign $dotest/sign
+ echo $message_id $dotest/message-id
echo $utf8 $dotest/utf8
echo $keep $dotest/keep
echo $scissors $dotest/scissors
@@ -651,6 +655,10 @@ then
else
SIGNOFF=
fi
+if test $(cat $dotest/message-id) = t
Does the usage of '' inside of '' look good, or can we write like this:
if test $(cat $dotest/message-id) = t
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/9] add strip_suffix function
Jeff King p...@peff.net writes:
For that reason, the mem form puts its length parameter
next to the buffer (since they are a pair), and the string
form puts it at the end (since it is an out-parameter). The
compiler can notice when you get the order wrong, which
should help prevent writing one when you meant the other.
Very sensible consideration. I like commits that careful thinking
behind them shows through them.
Signed-off-by: Jeff King p...@peff.net
---
I hope the word strip is OK, as it does not actually NUL-terminate
(doing so would make it unusable for many cases). Between the comment
below and the const in the parameter, I think it should be pretty
clear that it does not touch the string. And I could not think of a
better word.
All other words I can think of offhand, trim, chomp, etc., hint
shortening of the input string, and by definition shortening of a
string implies NUL-termination.
The mem variant deals with a counted string, however, so its
shortening implies NUL-termination a lot less [*1*] and should be
fine.
If we want to avoid implying NUL-termination, the only way to do so
would be to use wording that does not hint shortening. At least for
the C-string variant, which is measuring the length of the basename
part (i.e. `basename $str $suffix`) without touching anything else,
e.g. basename_length(hello.c, .c, len), but at the same time
you want to make it a boolean to signal if the string ends with the
suffix, so perhaps has_suffix(hello.c, .c, len)?
[Footnote]
*1* ... but not entirely, because we often NUL-terminate even
counted strings (the buffer returned from read_sha1_file() and
the payload of strbuf are two examples).
git-compat-util.h | 27 +++
1 file changed, 27 insertions(+)
diff --git a/git-compat-util.h b/git-compat-util.h
index b6f03b3..d044c42 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -358,6 +358,33 @@ static inline const char *skip_prefix(const char *str,
const char *prefix)
return NULL;
}
+/*
+ * If buf ends with suffix, return 1 and subtract the length of the suffix
+ * from *len. Otherwise, return 0 and leave *len untouched.
+ */
+static inline int strip_suffix_mem(const char *buf, size_t *len,
+const char *suffix)
+{
+ size_t suflen = strlen(suffix);
+ if (*len suflen || memcmp(buf + (*len - suflen), suffix, suflen))
+ return 0;
+ *len -= suflen;
+ return 1;
+}
+
+/*
+ * If str ends with suffix, return 1 and set *len to the size of the string
+ * without the suffix. Otherwise, return 0 and set *len to the size of the
+ * string.
+ *
+ * Note that we do _not_ NUL-terminate str to the new length.
+ */
+static inline int strip_suffix(const char *str, const char *suffix, size_t
*len)
+{
+ *len = strlen(str);
+ return strip_suffix_mem(str, len, suffix);
+}
+
#if defined(NO_MMAP) || defined(USE_WIN32_MMAP)
#ifndef PROT_READ
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Compile Error on Git 2.0.1 on Redhat 5.9 with Fix
When compiling Git 2.0.1 on RedHat 5.9 as a non-root user I get the following error: BUILD ERROR ``` make prefix=/home/eldon/local all doc info ... CC zlib.o CC unix-socket.o CC thread-utils.o CC compat/strlcpy.o AR libgit.a /bin/sh: gar: command not found make: *** [libgit.a] Error 127 ``` My fix was to make a symlink below: SYMLINK ``` gar - /usr/bin/ar ``` LINUX VERSION ``` lsb_release -i -r Distributor ID: RedHatEnterpriseClient Release:5.9 ``` I think the fix is to allow the use of ar if gar does not exist. I don't know if this exists for every Redhat install but gar is not available in /usr/bin but ar does. ``` /usr/bin/ar --version GNU ar 2.17.50.0.6-20.el5_8.3 20061020 Copyright 2005 Free Software Foundation, Inc. ``` It could just be my corporate install of RedHat that is weird, but thought this might be at least a point to describe a fix. ``` gcc --version gcc (GCC) 4.1.2 20080704 (Red Hat 4.1.2-54) Copyright (C) 2006 Free Software Foundation, Inc. ``` Eldon Nelson eldon_nel...@ieee.org Cell: 952-393-3481 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] git-am: add option to extract email Message-Id: tag into commit log
On 07/02/2014 06:03 PM, Torsten Bögershausen wrote: @@ -565,6 +568,7 @@ Use \git am --abort\ to remove it.) echo $git_apply_opt $dotest/apply-opt echo $threeway $dotest/threeway echo $sign $dotest/sign + echo $message_id $dotest/message-id echo $utf8 $dotest/utf8 echo $keep $dotest/keep echo $scissors $dotest/scissors @@ -651,6 +655,10 @@ then else SIGNOFF= fi +if test $(cat $dotest/message-id) = t Does the usage of '' inside of '' look good, or can we write like this: if test $(cat $dotest/message-id) = t With your change, it will fail if the file is missing or empty. Complex shell scripts cannot be made to look good. If that's a requirement then the script should be rewritten in a reasonable language. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Compile Error on Git 2.0.1 on Redhat 5.9 with Fix
Eldon Nelson eldon_nel...@ieee.org writes: make prefix=/home/eldon/local all doc info ... CC zlib.o CC unix-socket.o CC thread-utils.o CC compat/strlcpy.o AR libgit.a /bin/sh: gar: command not found [...] I think the fix is to allow the use of ar if gar does not exist. It is already the case. The Makefile defaults to ar and does not even contain any mention of gar. The configure script checks for both: configure.ac:AC_CHECK_TOOLS(AR, [gar ar], :) My guess is that you ran the configure script on a machine where gar is present, and then used the result on a machine where it isn't. Normally, at configure time you should see this: checking for ar... ar You actually don't need to run the configure script, the Makefile runs fine without it. Delete config.mak.autogen to cancel the effect of ./configure. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Compile Error on Git 2.0.1 on Redhat 5.9 with Fix
On Wed, Jul 02, 2014 at 10:56:25AM -0500, Eldon Nelson wrote: When compiling Git 2.0.1 on RedHat 5.9 as a non-root user I get the following error: BUILD ERROR ``` make prefix=/home/eldon/local all doc info ... CC zlib.o CC unix-socket.o CC thread-utils.o CC compat/strlcpy.o AR libgit.a /bin/sh: gar: command not found make: *** [libgit.a] Error 127 ``` The Makefile defines AR as: $ git grep ^AR Makefile Makefile:AR = ar so it should already do what you want by default. But that make variable can be overridden. Can you show us the contents of your config.mak and config.mak.autogen files (if either exists)? It's also possible that you have AR set in your environment (show us echo $AR), but that would usually not override the Makefile unless make -e is used (and that is not in your command-line above, but it's possible that make is a shell alias or something). My fix was to make a symlink below: SYMLINK ``` gar - /usr/bin/ar ``` A simpler workaround is probably: make AR=ar ... but probably the right solution is to eliminate whatever is providing the bogus override in the first place. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Show containing branches in log?
On Wed, Jul 02, 2014 at 09:50:57AM -0500, Robert Dailey wrote: I know that with the `git branch` command I can determine which branches contain a commit. Is there a way to represent this graphically with `git log`? Sometimes I just have a commit, and I need to find out what branch contains that commit. The reason why `git branch --contains` doesn't solve this problem for me is that it names almost all branches because of merge commits. Too much ancestry has been built since this commit, so there is no way to find the closest branch that contains that commit. Is there a way to graphically see what is the nearest named ref to the specified commit in the logs? Have you tried git describe --contains --all commit? To some degree, I fear your question isn't something git can answer. If the branch containing the commit has been merged into other branches, then they all contain the commit. There is not really any reason to prefer one over the other (describe --contains will try to find the closest branch, but that is based on heuristics and is not necessarily well-defined). -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/9] add strip_suffix function
On Wed, Jul 02, 2014 at 08:54:44AM -0700, Junio C Hamano wrote: Jeff King p...@peff.net writes: For that reason, the mem form puts its length parameter next to the buffer (since they are a pair), and the string form puts it at the end (since it is an out-parameter). The compiler can notice when you get the order wrong, which should help prevent writing one when you meant the other. Very sensible consideration. I like commits that careful thinking behind them shows through them. I would like to take credit for advanced thinking, but I actually did what felt natural, and only noticed the compiler will tell you when you are wrong effect when I got it wrong while writing a later patch in the series. :) If we want to avoid implying NUL-termination, the only way to do so would be to use wording that does not hint shortening. At least for the C-string variant, which is measuring the length of the basename part (i.e. `basename $str $suffix`) without touching anything else, e.g. basename_length(hello.c, .c, len), but at the same time you want to make it a boolean to signal if the string ends with the suffix, so perhaps has_suffix(hello.c, .c, len)? I think that invites some confusion with ends_with, which is the same thing (but just does not take the len parameter). We could just add this feature to ends_with, and ask callers who do not care to pass NULL, but that makes those call sites uglier. Having had a day to mull it over, and having read your email, I think I still prefer strip_suffix. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Show containing branches in log?
On Wed, Jul 2, 2014 at 11:34 AM, Jeff King p...@peff.net wrote: Have you tried git describe --contains --all commit? To some degree, I fear your question isn't something git can answer. If the branch containing the commit has been merged into other branches, then they all contain the commit. There is not really any reason to prefer one over the other (describe --contains will try to find the closest branch, but that is based on heuristics and is not necessarily well-defined). I have not tried that command. Note I mentioned named refs, so nameless branches I'm not worried about. Even if I merge branch A into branch B, branch A is still closest in terms of number of steps to get to the commit, because to get to the commit through B you have to cross over a merge commit. Basically the priority should be directness and distance. The more direct a branch is (i.e. the lesser number of merge commits it goes through to get to the commit) the more relevant it is. As a second condition, distance would be used in cases where the directness of it is the same. Sorting this in the log graph and seeing it visually (I could even use --simplify-by-decoration) would help me understand the topography of git's history relative to the commit(s) I specify. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: Show containing branches in log?
-Original Message- From: Jeff King Sent: Wednesday, July 02, 2014 12:35 On Wed, Jul 02, 2014 at 09:50:57AM -0500, Robert Dailey wrote: I know that with the `git branch` command I can determine which branches contain a commit. Is there a way to represent this graphically with `git log`? Sometimes I just have a commit, and I need to find out what branch contains that commit. The reason why `git branch --contains` doesn't solve this problem for me is that it names almost all branches because of merge commits. Too much ancestry has been built since this commit, so there is no way to find the closest branch that contains that commit. Is there a way to graphically see what is the nearest named ref to the specified commit in the logs? Have you tried git describe --contains --all commit? To some degree, I fear your question isn't something git can answer. If the branch containing the commit has been merged into other branches, then they all contain the commit. There is not really any reason to prefer one over the other (describe --contains will try to find the closest branch, but that is based on heuristics and is not necessarily well-defined). Another way I answer this question is git log --oneline --graph --all and then search for the commit and follow the lines. -- -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=- - - - Jason Pyeron PD Inc. http://www.pdinc.us - - Principal Consultant 10 West 24th Street #100- - +1 (443) 269-1555 x333Baltimore, Maryland 21218 - - - -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=- This message is copyright PD Inc, subject to license 20080407P00. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Show containing branches in log?
On Wed, Jul 2, 2014 at 11:52 AM, Jason Pyeron jpye...@pdinc.us wrote: -Original Message- From: Jeff King Sent: Wednesday, July 02, 2014 12:35 On Wed, Jul 02, 2014 at 09:50:57AM -0500, Robert Dailey wrote: I know that with the `git branch` command I can determine which branches contain a commit. Is there a way to represent this graphically with `git log`? Sometimes I just have a commit, and I need to find out what branch contains that commit. The reason why `git branch --contains` doesn't solve this problem for me is that it names almost all branches because of merge commits. Too much ancestry has been built since this commit, so there is no way to find the closest branch that contains that commit. Is there a way to graphically see what is the nearest named ref to the specified commit in the logs? Have you tried git describe --contains --all commit? To some degree, I fear your question isn't something git can answer. If the branch containing the commit has been merged into other branches, then they all contain the commit. There is not really any reason to prefer one over the other (describe --contains will try to find the closest branch, but that is based on heuristics and is not necessarily well-defined). Another way I answer this question is git log --oneline --graph --all and then search for the commit and follow the lines. If that were a practical solution I wouldn't be here asking this question. Unfortunately, in a repository with multiple parallel release branches, it is impossible to just eye-ball the graph and find what you're looking for. Especially when the last 4 weeks worth of commits consumes over 10 pages of log graph. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/2] add `config_set` API for caching config files
Tanay Abhra tanay...@gmail.com writes:
diff --git a/Documentation/technical/api-config.txt
b/Documentation/technical/api-config.txt
index 230b3a0..2c02fee 100644
--- a/Documentation/technical/api-config.txt
+++ b/Documentation/technical/api-config.txt
@@ -77,6 +77,75 @@ To read a specific file in git-config format, use
`git_config_from_file`. This takes the same callback and data parameters
as `git_config`.
+Querying For Specific Variables
+---
+
+For programs wanting to query for specific variables in a non-callback
+manner, the config API provides two functions `git_config_get_value`
+and `git_config_get_value_multi`.They both read values from an internal
+cache generated previously from reading the config files.
+
+`git_config_get_value` takes two parameters,
+
+- a key string in canonical flat form for which the corresponding value
What is canonical flat form?
+ with the highest priority (i.e. value in the repo config will be
+ preferred over value in user wide config for the same variable) will
+ be retrieved.
+
+- a pointer to a string which will point to the retrieved value. The caller
+ should not free or modify the value returned as it is owned by the cache.
+
+`git_config_get_value` returns 0 on success, or -1 for no value found.
+
+`git_config_get_value_multi` allocates and returns a `string_list`
+containing all the values for the key passed as parameter, sorted in order
+of increasing priority (Note: caller has to call `string_list_clear` on
+the returned pointer and then free it).
+
+`git_config_get_value_multi` returns NULL for no value found.
+
+`git_config_clear` is provided for resetting and invalidating the cache.
+
+`git_config_iter` gives a way to iterate over the keys in cache. Existing
+`git_config` callback function signature is used for iterating.
Instead of doing prose above and then enumeration below,
consistently using the enumeration-style would make the descriptions
of API functions easier to find and read. Also show what parameters
each function takes. E.g.
`git_config_get_value(const char *key, const char **value)`::
Find the highest-priority value for the
configuration variable `key`, store the pointer to
it in `value` and return 0. When the configuration
variable `key` is not found, return -1 without
touching `value`.
A few random thoughts.
- Are a value for the variable is found and no value for the
variable is found the only possible outcome? Should somebody
(not necessarily the calling code) be notified of an error---for
example, if you opened a config file successfully but later found
that the file could not be parsed due to a syntax error or an I/O
error, is it possible the caller of this function to tell, or are
these just some special cases of variable not found?
- The same goes for the multi variant but it is a bit more grave,
as a function that returns an int can later be updated to
return different negative values to signal different kinds of
errors, but a function that returns a pointer to string-list can
only return one kind of NULL ;-)
- For the purpose of git_config_get_value(), what is the value
returned for core.filemode when the configuration file says
[core] filemode\n, i.e. when git_config() callback would get a
NULL for value to signal a boolean true?
- Is there a reason why you need a separate and new config_iter?
What does it do differently from the good-old git_config() and
how? It does not belong to Querying for Specific Variables
anyway.
+The config API also provides type specific API functions which do conversion
+as well as retrieval for the queried variable, including:
+
+`git_config_get_int`::
+Parse the retrieved value to an integer, including unit factors. Dies on
+error; otherwise, allocates and copies the integer into the dest parameter.
+Returns 0 on success, or 1 if no value was found.
allocates and copies Is a caller forced to do something like
this?
int *val;
if (!git_config_get_int(int.one, val)) {
use(*val);
free(val);
}
I'd expect it to be more like:
int val;
if (!git_config_get_int(int.one, val))
use(val);
Also, is there a reason why the not found signal is 1 (as
opposed to -1 for the lower-level get_value() API)?
+`git_config_get_ulong`::
+Identical to `git_config_get_int` but for unsigned longs.
Obviously this is not identical but merely Similar to.
+`git_config_bool`::
git_config_get_bool, perhaps?
+Custom Configsets
+-
+
+A `config_set` points at an ordered set of config files, each of
+which represents what was read and cached in-core from a file.
This over-specifies the implementation. Judging from the list of
API functions below, an
Re: [PATCH v4 1/2] add `config_set` API for caching config files
Matthieu Moy matthieu@grenoble-inp.fr writes: I don't like the name the_config_set. It's not the only one. Perhaps repo_config_set? (not totally satisfactory as it does not contain only the repo, but the repo+user+system) What do others think? I actually do like the_configset, which goes nicely parallel to the_index. Neither is the only one, but most of the time our code operates on the primary one and the_frotz refers to it (and convenience wrappers that does not specify which set defaults to it). What is the reason to deal with `the_config_set` and other config sets differently? You're giving arguments to store `the_config_set` as a single hashmap, but what is the reason to store others as multiple hashmaps? Confusion, probably. the_configset should be just a singleton instance used to store the values we use for the default config system, but its shape should be exactly the same as the other ones users can use to read .gitmodules and friends with. And actually, I don't completely buy your arguments: having 3 or 4 hashmaps (.git/config, ~/.gitconfig, ~/.config/git/config and /etc/gitconfig) would be a O(4) lookup, which is still O(1), and you could deal with include directives by having .git/config and included files in a hashmap, ~/.gitconfig and included files in a second hashmap, ... OK. I would personally find it much simpler to have a single hashmap. We'd lose the ability to invalidate the cache for only a single file, but I'm not sure it's worth the code complexity. OK, too. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Experimental TDB support for GIT REFS
On Fri, Jun 27, 2014 at 5:37 PM, Shawn Pearce spea...@spearce.org wrote: On Fri, Jun 27, 2014 at 2:30 PM, Ronnie Sahlberg sahlb...@google.com wrote: List, One of my ref transaction aims is to make define a stable public API for accessing refs. Once this is done I want to make it possible to replace the current .git/refs/* model with a different type of backend. In my case I want to add support to store all refs and reflogs in a TDB database but once we have a pluggable backend framework for refs, if someone wants to store the refs in a SQL database, that should be fairly trivial too. There are a few series queued before this becomes possible, but is anyone wants to test or play with my git can use TDB database you can find an implementation of this at https://github.com/rsahlberg/git/tree/backend-struct-tdb Interesting! But the link 404s :( Yeah, sorry but there were issues :-( Issues solved now though and refactoring done. Please see https://github.com/rsahlberg/git/tree/backend-struct-db This branch adds a refs backend that communicates via a domain socket to a refs daemon. The refs daemon in turn interfaces with the actual database. My branch contains one hack refs daemon that uses a TDB database for the refs storage. This daemon is only about 600 lines of code, most of which is marshalling and socket handling and only a small amount of TDB specific code. This small daemon should make it easy for folks to add arbitrary database interfaces easily. Taking refsd-tdb.c and modifying it to instead attach to a SQL database should only take a few hundred lines of changes or so. To build the daemon and start it : $ gcc refsd-tdb.c -o refsd-tdb -ltdb create /var/lib/git and /var/log/git $ ./refsd-tdb /var/lib/git/refs.socket /var/lib/git /var/log/git/refsd.log Then you can inspect the database with tdbdump /var/lib/git/refs.tdb You can then create a repository that uses this database : $ git init --db-repo-name=ROCC --db-socket=/var/lib/git/refs.socket . Then further commands will operate on the refs tdb. See this as a prototype to experiment with and see the direction of the refs transaction and pluggable backend support. There is a lot additional work and cleanup that needs to be done before this will become production code. (yes I know we should not do hand marshalling and unmarshalling for the PDUs on the domain socket. We should use some lightweight encoding library like XDR and rpcgen or similar.) Please test, comment and have fun. regards ronnie sahlberg -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/2] add `config_set` API for caching config files
Junio C Hamano gits...@pobox.com writes: Tanay Abhra tanay...@gmail.com writes: diff --git a/Documentation/technical/api-config.txt b/Documentation/technical/api-config.txt index 230b3a0..2c02fee 100644 --- a/Documentation/technical/api-config.txt +++ b/Documentation/technical/api-config.txt @@ -77,6 +77,75 @@ To read a specific file in git-config format, use `git_config_from_file`. This takes the same callback and data parameters as `git_config`. +Querying For Specific Variables +--- + +For programs wanting to query for specific variables in a non-callback +manner, the config API provides two functions `git_config_get_value` +and `git_config_get_value_multi`.They both read values from an internal +cache generated previously from reading the config files. + +`git_config_get_value` takes two parameters, + +- a key string in canonical flat form for which the corresponding value What is canonical flat form? Actually, it's defined above in the same file (was here before the patch): - the name of the parsed variable. This is in canonical flat form: the section, subsection, and variable segments will be separated by dots, and the section and variable segments will be all lowercase. E.g., But it might make sense to reword the doc, e.g. introduce a glossary section at the beginning. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] git-am: add option to extract email Message-Id: tag into commit log
Avi Kivity a...@cloudius-systems.com writes: + if test 't' == $message_id + then + grep ^Message-Id: $dotest/info || true + fi if test '' != $ADD_SIGNOFF then echo $ADD_SIGNOFF Seeing how existing code carefully makes sure that ADD_SIGNOFF has an empty line before it when and only when necessary to ensure that there is a blank after the existing log message, I would suspect that this patch that blindly inserts a line is doubly wrong. The output from grep may be appended without adding a blank when necessary, and appending of ADD_SIGNOFF may end up adding an extra blank after Message-Id. Am I reading the patch wrong? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/9] add strip_suffix function
Jeff King p...@peff.net writes: Having had a day to mull it over, and having read your email, I think I still prefer strip_suffix. That's OK. I was only trying to help you explore different avenues, without having strong preference myself either way. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH RFC v2 00/19] Enable options --signoff, --reset-author for pick, reword
Hi, this reroll applies the comments from Eric, Junio and Michael. In particular, * It turned out that `pick_one` does not need option handling at all and the option-like argument `-n` determines whether `pick_one` or `do_pick` creates the replay commit. The latter happens if the task wants to rewrite the commit being picked (f.i., for the purpose of editing the log message or resetting the authorship). `do_pick` still seems to require a flexible parsing of options, i.e. a relatively expensive loop, since it receives the whitelisted user-supplied options. Unsupported and unknown options are treated as an unknown command error. * The `do_pick` options are documented in the same order they are listed in the function signature. Moreover, it is mentioned which options rewrite commits being picked. * The test cases output differing actual values as changes to the expected values and not the other way around. Moreover, the failed rebases are not cleaned up until the respective test succeeds. Two stages (and two sub-stages) can be identified in the rerolled patch series: 1. Route primary to-do list commands through `do_pick` a. Implement reword in terms of do_pick (5/19) b. Implement squash in terms of do_pick (14/19) 2. Add user options to main commands Enable options --signoff, --reset-author for pick, reword (19/19) The last stage was added in this reroll. It enables the parsing of line options for to-do list commands, which is still restricted to options without arguments because it is unclear how spaces can be parsed as characters rather than separators where needed. For instance, if we were to support pick --author=A U Thor fa1afe1 Some changes then read(1) would hand us the tokens `--author=A`, `U` and `Thor` instead of `--author=A U Thor`, which we would want to relay to `do_pick`. Interpreting the shell quoting would help. However, eval(1) seems to disqualify itself as an interpreter because the to-do list entry could potentially contain any shell command line. This could be both a security and a usability issue. For this reason, the patch series still hasn't graduated from being RFC. Fabian Fabian Ruch (19): rebase -i: Failed reword prints redundant error message rebase -i: reword complains about empty commit despite --keep-empty rebase -i: reword executes pre-commit hook on interim commit rebase -i: Teach do_pick the option --edit rebase -i: Implement reword in terms of do_pick rebase -i: Stop on root commits with empty log messages rebase -i: The replay of root commits is not shown with --verbose rebase -i: Root commits are replayed with an unnecessary option rebase -i: Commit only once when rewriting picks rebase -i: Do not die in do_pick rebase -i: Teach do_pick the option --amend rebase -i: Teach do_pick the option --file rebase -i: Prepare for squash in terms of do_pick --amend rebase -i: Implement squash in terms of do_pick rebase -i: Explicitly distinguish replay commands and exec tasks rebase -i: Parse to-do list command line options rebase -i: Teach do_pick the option --reset-author rebase -i: Teach do_pick the option --signoff rebase -i: Enable options --signoff, --reset-author for pick, reword git-rebase--interactive.sh| 277 ++ t/t3404-rebase-interactive.sh | 8 ++ t/t3412-rebase-root.sh| 39 ++ 3 files changed, 273 insertions(+), 51 deletions(-) -- 2.0.0 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH RFC v2 03/19] rebase -i: reword executes pre-commit hook on interim commit
The to-do list command `reword` replays a commit like `pick` but lets
the user also edit the commit's log message. This happens in two
steps. Firstly, the named commit is cherry-picked. Secondly, the
commit created in the first step is amended using an unchanged index
to edit the log message. The pre-commit hook is meant to verify the
changes introduced by a commit (for instance, catching inserted
trailing white space). Since the committed tree is not changed
between the two steps, do not execute the pre-commit hook in the
second step.
Specify the git-commit option `--no-verify` to disable the pre-commit
hook when editing the log message. Because `--no-verify` also skips
the commit-msg hook, execute the hook from within
git-rebase--interactive after the commit is created. Fortunately, the
commit message is still available in `$GIT_DIR/COMMIT_EDITMSG` after
git-commit terminates. Caveat: In case the commit-msg hook finds the
new log message ill-formatted, the user is only notified of the
failed commit-msg hook but the log message is used for the commit
anyway. git-commit ought to offer more fine-grained control over
which hooks are executed.
Signed-off-by: Fabian Ruch baf...@gmail.com
---
git-rebase--interactive.sh | 17 +
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 689400e..b50770d 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -504,10 +504,19 @@ do_next () {
mark_action_done
do_pick $sha1 $rest
- git commit --allow-empty --amend --no-post-rewrite
${gpg_sign_opt:+$gpg_sign_opt} || {
- warn Could not amend commit after successfully picking
$sha1... $rest
- exit_with_patch $sha1 1
- }
+ # TODO: Work around the fact that git-commit lets us
+ # disable either both the pre-commit and the commit-msg
+ # hook or none. Disable the pre-commit hook because the
+ # tree is left unchanged but run the commit-msg hook
+ # from here because the log message is altered.
+ git commit --allow-empty --amend --no-post-rewrite -n
${gpg_sign_opt:+$gpg_sign_opt}
+ if test -x $GIT_DIR/hooks/commit-msg
+ then
+ $GIT_DIR/hooks/commit-msg
$GIT_DIR/COMMIT_EDITMSG
+ fi || {
+ warn Could not amend commit after successfully
picking $sha1... $rest
+ exit_with_patch $sha1 1
+ }
record_in_rewritten $sha1
;;
edit|e)
--
2.0.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH RFC v2 01/19] rebase -i: Failed reword prints redundant error message
The to-do list command `reword` replays a commit like `pick` but lets
the user also edit the commit's log message. If the edited log
message is empty or is found ill-formatted by one of the commit
hooks, git-rebase--interactive prints three error messages to the
console.
1. The git-commit output, which contains all the output from hook
scripts.
2. A rebase diagnosis saying at which task on the to-do list it
got stuck.
3. Generic presumptions about what could have triggered the
error.
The third message contains redundant information and does not add any
enlightenment either, which makes the output unnecessarily longish
and different from the other command's output. For instance, this is
what the output looks like if the log message is empty (contains
duplicate Signed-off-by lines).
(1.) Aborting commit due to empty commit message. (Duplicate Signed-off-by
lines.)
(2.) Could not amend commit after successfully picking fa1afe1... Some
change
(3.) This is most likely due to an empty commit message, or the pre-commit
hook
failed. If the pre-commit hook failed, you may need to resolve the
issue before
you are able to reword the commit.
Discard the third message.
It is true that a failed hook script might not output any diagnosis
but then the generic message is not of much help either. Since this
lack of information affects the built-in git commands for commit,
merge and cherry-pick first, the solution would be to keep track of
the failed hooks in their output so that the user knows which of her
hooks require improvement.
Signed-off-by: Fabian Ruch baf...@gmail.com
---
git-rebase--interactive.sh | 3 ---
1 file changed, 3 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 7e1eda0..e733d7f 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -506,9 +506,6 @@ do_next () {
do_pick $sha1 $rest
git commit --amend --no-post-rewrite
${gpg_sign_opt:+$gpg_sign_opt} || {
warn Could not amend commit after successfully picking
$sha1... $rest
- warn This is most likely due to an empty commit
message, or the pre-commit hook
- warn failed. If the pre-commit hook failed, you may
need to resolve the issue before
- warn you are able to reword the commit.
exit_with_patch $sha1 1
}
record_in_rewritten $sha1
--
2.0.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH RFC v2 02/19] rebase -i: reword complains about empty commit despite --keep-empty
The to-do list command `reword` replays a commit like `pick` but lets
the user also edit the commit's log message. If `--keep-empty` is
passed as option to git-rebase--interactive, empty commits ought to
be replayed without complaints. However, if the users chooses to
reword an empty commit by changing the respective to-do list entry
from
pick fa1afe1 Empty commit
to
reword fa1afe1 Empty commit
, then git-rebase--interactive suddenly fails to replay the empty
commit. This is especially counterintuitive because `reword` is
thought of as a `pick` that alters the log message in some way but
nothing more and the unchanged to-do list entry would not fail.
Handle `reword` by cherry-picking the named commit and editing the
log message using
git commit --allow-empty --amend
instead of
git commit --amend.
Add test.
Signed-off-by: Fabian Ruch baf...@gmail.com
---
git-rebase--interactive.sh| 2 +-
t/t3404-rebase-interactive.sh | 8
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index e733d7f..689400e 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -504,7 +504,7 @@ do_next () {
mark_action_done
do_pick $sha1 $rest
- git commit --amend --no-post-rewrite
${gpg_sign_opt:+$gpg_sign_opt} || {
+ git commit --allow-empty --amend --no-post-rewrite
${gpg_sign_opt:+$gpg_sign_opt} || {
warn Could not amend commit after successfully picking
$sha1... $rest
exit_with_patch $sha1 1
}
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 8197ed2..9931143 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -75,6 +75,14 @@ test_expect_success 'rebase --keep-empty' '
test_line_count = 6 actual
'
+test_expect_success 'rebase --keep-empty' '
+ git checkout emptybranch
+ set_fake_editor
+ FAKE_LINES=1 reword 2 git rebase --keep-empty -i HEAD~2
+ git log --oneline actual
+ test_line_count = 6 actual
+'
+
test_expect_success 'rebase -i with the exec command' '
git checkout master
(
--
2.0.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH RFC v2 05/19] rebase -i: Implement reword in terms of do_pick
The to-do list command `reword` replays a commit like `pick` but lets
the user also edit the commit's log message. If one thinks of `pick`
entries as scheduled `cherry-pick` command lines, then `reword`
becomes an alias for the command line `cherry-pick --edit`. The
porcelain `rebase--interactive` defines a function `do_pick` for
processing the `pick` entries on to-do lists. Reimplement `reword` in
terms of `do_pick --edit`.
If the user picks a commit using the to-do list line
reword fa1afe1 Some change
execute the command `do_pick --edit fa1afe1 Some change` which
carries out exactly the same steps as the case arm for `reword` in
`do_next` so far.
Signed-off-by: Fabian Ruch baf...@gmail.com
---
git-rebase--interactive.sh | 15 +--
1 file changed, 1 insertion(+), 14 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index e06d9b6..4c875d5 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -555,20 +555,7 @@ do_next () {
comment_for_reflog reword
mark_action_done
- do_pick $sha1 $rest
- # TODO: Work around the fact that git-commit lets us
- # disable either both the pre-commit and the commit-msg
- # hook or none. Disable the pre-commit hook because the
- # tree is left unchanged but run the commit-msg hook
- # from here because the log message is altered.
- git commit --allow-empty --amend --no-post-rewrite -n
${gpg_sign_opt:+$gpg_sign_opt}
- if test -x $GIT_DIR/hooks/commit-msg
- then
- $GIT_DIR/hooks/commit-msg
$GIT_DIR/COMMIT_EDITMSG
- fi || {
- warn Could not amend commit after successfully
picking $sha1... $rest
- exit_with_patch $sha1 1
- }
+ do_pick --edit $sha1 $rest
record_in_rewritten $sha1
;;
edit|e)
--
2.0.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH RFC v2 10/19] rebase -i: Do not die in do_pick
Since `do_pick` might be executed in a sub-shell (a modified author
environment for instance), calling `die` in `do_pick` has no effect
but exiting the sub-shell with a failure exit status. The
git-rebase--interactive script is not terminated. Moreover, if
`do_pick` is called while a squash or fixup is in effect,
`die_with_patch` will discard `$squash_msg` as commit message.
Lastly, after a `die` in `do_pick` `do_next` has no chance to
reschedule tasks that failed before changes could be applied.
Indicate an error in `do_pick` using return statements and properly
kill the script at the call sites. Although possible in principle,
the issued error messages are no more indicating whether `do_pick`
failed while applying or while committing the changes. This reduces
code complexity at the call site and does not matter from a user's
point of view because a glance at the index reveals whether there are
conflicts or not and in-depth troubleshooting is still possible using
the `--verbose` option.
Remove the commit message title argument from `do_pick`'s interface,
which has become unused.
Signed-off-by: Fabian Ruch baf...@gmail.com
---
git-rebase--interactive.sh | 37 -
1 file changed, 20 insertions(+), 17 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 46b2db1..0070b3e 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -464,7 +464,7 @@ record_in_rewritten() {
# Apply the changes introduced by the given commit to the current head.
#
-# do_pick [--edit] commit title
+# do_pick [--edit] commit
#
# Wrapper around git-cherry-pick.
#
@@ -476,9 +476,11 @@ record_in_rewritten() {
# commit
# The commit to cherry-pick.
#
-# title
-# The commit message title of commit. Used for information
-# purposes only.
+# The return value is 1 if applying the changes resulted in a conflict
+# and 2 if the specified arguments were incorrect. If the changes could
+# be applied successfully but creating the commit failed, a value
+# greater than 2 is returned. No commit is created in either case and
+# the index is left with the (conflicting) changes in place.
do_pick () {
rewrite=
rewrite_amend=
@@ -491,7 +493,8 @@ do_pick () {
rewrite_edit=y
;;
-*)
- die do_pick: unrecognized option -- $1
+ warn do_pick: unrecognized option -- $1
+ return 2
;;
*)
break
@@ -499,7 +502,11 @@ do_pick () {
esac
shift
done
- test $# -ne 2 die do_pick: wrong number of arguments
+ if test $# -ne 1
+ then
+ warn do_pick: wrong number of arguments
+ return 2
+ fi
if test $(git rev-parse HEAD) = $squash_onto
then
@@ -517,11 +524,9 @@ do_pick () {
# rebase --continue.
git commit --allow-empty --allow-empty-message --amend \
--no-post-rewrite -n -q -C $1
- pick_one -n $1 ||
- die_with_patch $1 Could not apply $1... $2
+ pick_one -n $1 || return 1
else
- pick_one ${rewrite:+-n} $1 ||
- die_with_patch $1 Could not apply $1... $2
+ pick_one ${rewrite:+-n} $1 || return 1
fi
if test -n $rewrite
@@ -529,8 +534,7 @@ do_pick () {
git commit --allow-empty --no-post-rewrite -n --no-edit \
${rewrite_amend:+--amend} \
${rewrite_edit:+--edit} \
- ${gpg_sign_opt:+$gpg_sign_opt} ||
- die_with_patch $1 Could not rewrite commit after
successfully picking $1... $2
+ ${gpg_sign_opt:+$gpg_sign_opt} || return 3
fi
# TODO: Work around the fact that git-commit lets us
@@ -543,8 +547,7 @@ do_pick () {
if test -x $GIT_DIR/hooks/commit-msg
then
$GIT_DIR/hooks/commit-msg $GIT_DIR/COMMIT_EDITMSG
- fi ||
- die_with_patch $1 Could not rewrite commit after
successfully picking $1... $2
+ fi || return 3
fi
}
@@ -559,21 +562,21 @@ do_next () {
comment_for_reflog pick
mark_action_done
- do_pick $sha1 $rest
+ do_pick $sha1 || die_with_patch $sha1 Could not apply $sha1...
$rest
record_in_rewritten $sha1
;;
reword|r)
comment_for_reflog reword
mark_action_done
- do_pick --edit $sha1 $rest
+ do_pick --edit $sha1 || die_with_patch $sha1 Could not apply
$sha1... $rest
record_in_rewritten $sha1
;;
[PATCH RFC v2 04/19] rebase -i: Teach do_pick the option --edit
`do_pick` is the git-cherry-pick wrapper in git-rebase--interactive
that is used to implement the to-do list command `pick`. To cater for
the different pick behaviours (like `reword`), `do_pick` accepts
several options not only from the git-cherry-pick but also the
git-commit interface. Add the common option `--edit` to let the user
edit the log message of the named commit.
Loop over `$@` to parse the `do_pick` arguments. Assign the local
variable `edit` if one of the options is `--edit` so that the
remainder of `do_pick` can easily check whether the client code asked
to edit the commit message. If one of the options is unknown, mention
it on the console and `die`. Break the loop on the first non-option
and do some sanity checking to ensure that there exactly two
non-options, which are interpreted by the remainder as `commit` and
`title` like before.
`do_pick` ought to act as a wrapper around `cherry-pick`.
Unfortunately, it cannot just forward `--edit` to the `cherry-pick`
command line. The assembled command line is executed within a command
substitution for controlling the verbosity of `rebase--interactive`.
Passing `--edit` would either hang the terminal or clutter the
substituted command output with control sequences. Execute the
`reword` code from `do_next` instead if the option `--edit` is
specified.
Signed-off-by: Fabian Ruch baf...@gmail.com
---
git-rebase--interactive.sh | 52 ++
1 file changed, 52 insertions(+)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index b50770d..e06d9b6 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -461,7 +461,42 @@ record_in_rewritten() {
esac
}
+# Apply the changes introduced by the given commit to the current head.
+#
+# do_pick [--edit] commit title
+#
+# Wrapper around git-cherry-pick.
+#
+# -e, --edit
+# After picking commit, open an editor and let the user edit the
+# commit message. The editor contents becomes the commit message of
+# the new head. This creates a fresh commit.
+#
+# commit
+# The commit to cherry-pick.
+#
+# title
+# The commit message title of commit. Used for information
+# purposes only.
do_pick () {
+ edit=
+ while test $# -gt 0
+ do
+ case $1 in
+ -e|--edit)
+ edit=y
+ ;;
+ -*)
+ die do_pick: unrecognized option -- $1
+ ;;
+ *)
+ break
+ ;;
+ esac
+ shift
+ done
+ test $# -ne 2 die do_pick: wrong number of arguments
+
if test $(git rev-parse HEAD) = $squash_onto
then
# Set the correct commit message and author info on the
@@ -483,6 +518,23 @@ do_pick () {
pick_one $1 ||
die_with_patch $1 Could not apply $1... $2
fi
+
+ if test -n $edit
+ then
+ # TODO: Work around the fact that git-commit lets us
+ # disable either both the pre-commit and the commit-msg
+ # hook or none. Disable the pre-commit hook because the
+ # tree is left unchanged but run the commit-msg hook
+ # from here because the log message is altered.
+ git commit --allow-empty --amend --no-post-rewrite -n
${gpg_sign_opt:+$gpg_sign_opt}
+ if test -x $GIT_DIR/hooks/commit-msg
+ then
+ $GIT_DIR/hooks/commit-msg
$GIT_DIR/COMMIT_EDITMSG
+ fi || {
+ warn Could not amend commit after successfully
picking $1... $2
+ exit_with_patch $1 1
+ }
+ fi
}
do_next () {
--
2.0.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH RFC v2 15/19] rebase -i: Explicitly distinguish replay commands and exec tasks
There are two kinds of to-do list commands available. One kind
replays a commit (`pick`, `reword`, `edit`, `squash` and `fixup` that
is) and the other executes a shell command (`exec`). We will call the
first kind replay commands.
The two kinds of tasks are scheduled using different line formats.
Replay commands expect a commit hash argument following the command
name and exec concatenates all arguments to assemble a command line.
Adhere to the distinction of formats by not trying to parse the
`sha1` field unless we are dealing with a replay command. Move the
replay command handling code to a new function `do_replay` which
assumes the first argument to be a commit hash and make no more such
assumptions in `do_next`.
Signed-off-by: Fabian Ruch baf...@gmail.com
---
git-rebase--interactive.sh | 42 --
1 file changed, 28 insertions(+), 14 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 008f3a0..9de7441 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -585,13 +585,12 @@ do_pick () {
fi
}
-do_next () {
- rm -f $msg $author_script $amend || exit
- read -r command sha1 rest $todo
+do_replay () {
+ command=$1
+ sha1=$2
+ rest=$3
+
case $command in
- $comment_char*|''|noop)
- mark_action_done
- ;;
pick|p)
comment_for_reflog pick
@@ -656,6 +655,28 @@ do_next () {
esac
record_in_rewritten $sha1
;;
+ *)
+ read -r command $todo
+ warn Unknown command: $command
+ fixtodo=Please fix this using 'git rebase --edit-todo'.
+ if git rev-parse --verify -q $sha1 /dev/null
+ then
+ die_with_patch $sha1 $fixtodo
+ else
+ die $fixtodo
+ fi
+ ;;
+ esac
+}
+
+do_next () {
+ rm -f $msg $author_script $amend || exit
+ read -r command sha1 rest $todo
+
+ case $command in
+ $comment_char*|''|noop)
+ mark_action_done
+ ;;
x|exec)
read -r command rest $todo
mark_action_done
@@ -695,14 +716,7 @@ do_next () {
fi
;;
*)
- warn Unknown command: $command $sha1 $rest
- fixtodo=Please fix this using 'git rebase --edit-todo'.
- if git rev-parse --verify -q $sha1 /dev/null
- then
- die_with_patch $sha1 $fixtodo
- else
- die $fixtodo
- fi
+ do_replay $command $sha1 $rest
;;
esac
test -s $todo return
--
2.0.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH RFC v2 09/19] rebase -i: Commit only once when rewriting picks
The options passed to `do_pick` determine whether the picked commit
will be rewritten or not. If the commit gets rewritten, because the
user requested to edit the commit message for instance, let
`pick_one` merely apply the changes introduced by the commit and do
not commit the resulting tree yet. If the commit is replayed as is,
leave it to `pick_one` to recreate the commit (possibly by
fast-forwarding the head). This makes it easier to combine git-commit
options like `--edit` and `--amend` in `do_pick` because
git-cherry-pick does not support `--amend`.
In the case of `--edit`, do not `exit_with_patch` but assign
`rewrite` to pick the changes with `-n`. If the pick conflicts, no
commit is created which we would have to amend when continuing the
rebase. To complete the pick after the conflicts are resolved the
user just resumes with `git rebase --continue`.
git-commit lets the user edit the commit log message by default. We
do not want that for the rewriting git-commit command line because
the default behaviour of git-cherry-pick is exactly the opposite.
Pass `--no-edit` when rewriting a picked commit. An explicit `--edit`
passed to `do_pick` (for instance, when reword is executed) enables
the editor launch again.
If `rebase--interactive` is used to rebase a complete branch onto
some head, `rebase` creates a sentinel commit that requires special
treatment by `do_pick`. Do not finalize the pick here either because
its commit message can be altered as for any other pick. Since the
orphaned root commit gets a temporary parent, it is always rewritten.
Safely use the rewrite infrastructure of `do_pick` to create the
final commit.
Signed-off-by: Fabian Ruch baf...@gmail.com
---
git-rebase--interactive.sh | 55 +++---
1 file changed, 33 insertions(+), 22 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 17836d5..46b2db1 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -63,7 +63,8 @@ msgnum=$state_dir/msgnum
author_script=$state_dir/author-script
# When an edit rebase command is being processed, the SHA1 of the
-# commit to be edited is recorded in this file. When git rebase
+# commit to be edited is recorded in this file. The same happens when
+# rewriting a commit fails, for instance reword. When git rebase
# --continue is executed, if there are any staged changes then they
# will be amended to the HEAD commit, but only provided the HEAD
# commit is still the commit to be edited. When any other rebase
@@ -479,12 +480,15 @@ record_in_rewritten() {
# The commit message title of commit. Used for information
# purposes only.
do_pick () {
- edit=
+ rewrite=
+ rewrite_amend=
+ rewrite_edit=
while test $# -gt 0
do
case $1 in
-e|--edit)
- edit=y
+ rewrite=y
+ rewrite_edit=y
;;
-*)
die do_pick: unrecognized option -- $1
@@ -499,6 +503,10 @@ do_pick () {
if test $(git rev-parse HEAD) = $squash_onto
then
+ rewrite=y
+ rewrite_amend=y
+ git rev-parse --verify HEAD $amend
+
# Set the correct commit message and author info on the
# sentinel root before cherry-picking the original changes
# without committing (-n). Finally, update the sentinel again
@@ -509,31 +517,34 @@ do_pick () {
# rebase --continue.
git commit --allow-empty --allow-empty-message --amend \
--no-post-rewrite -n -q -C $1
- pick_one -n $1
- git commit --allow-empty \
- --amend --no-post-rewrite -n --no-edit \
- ${gpg_sign_opt:+$gpg_sign_opt} ||
+ pick_one -n $1 ||
die_with_patch $1 Could not apply $1... $2
else
- pick_one $1 ||
+ pick_one ${rewrite:+-n} $1 ||
die_with_patch $1 Could not apply $1... $2
fi
- if test -n $edit
+ if test -n $rewrite
then
- # TODO: Work around the fact that git-commit lets us
- # disable either both the pre-commit and the commit-msg
- # hook or none. Disable the pre-commit hook because the
- # tree is left unchanged but run the commit-msg hook
- # from here because the log message is altered.
- git commit --allow-empty --amend --no-post-rewrite -n
${gpg_sign_opt:+$gpg_sign_opt}
- if test -x $GIT_DIR/hooks/commit-msg
- then
- $GIT_DIR/hooks/commit-msg
$GIT_DIR/COMMIT_EDITMSG
- fi || {
-
[PATCH RFC v2 19/19] rebase -i: Enable options --signoff, --reset-author for pick, reword
pick and reword are atomic to-do list commands in the sense that they
open a new task which is closed after the respective command is
completed. squash and fixup are not atomic. They create a new task
which is not completed until the last squash or fixup is processed.
Lift the general unknown option blockade for the pick and reword
commands. If `do_cmd` comes across one of the options `--signoff` and
`--reset-author` while parsing a to-do entry and the scheduled
command is either `pick` or `reword`, relay the option to `do_pick`.
The `do_pick` options `--gpg-sign` and `--file` are not yet supported
because `do_cmd` cannot handle option arguments and options with
spaces at the moment. It is true that edit is one of the atomic
commands but it displays hash information when the rebase is stopped
and some options rewrite the picked commit which alters that
information. squash and fixup still do not accept user options as the
interplay of `--reset-author` and the author script are yet to be
determined.
Signed-off-by: Fabian Ruch baf...@gmail.com
---
git-rebase--interactive.sh | 19 ---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index bb258bb..c34a9a7 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -624,6 +624,16 @@ do_replay () {
while test $# -gt 0
do
case $1 in
+ --signoff|--reset-author)
+ case $command in
+ pick|reword)
+ ;;
+ *)
+ warn Unsupported option: $1
+ command=unknown
+ ;;
+ esac
+ ;;
-*)
warn Unknown option: $1
command=unknown
@@ -644,21 +654,24 @@ do_replay () {
comment_for_reflog pick
mark_action_done
- do_pick $sha1 || die_with_patch $sha1 Could not apply $sha1...
$rest
+ eval do_pick $opts $sha1 \
+ || die_with_patch $sha1 Could not apply $sha1... $rest
record_in_rewritten $sha1
;;
reword|r)
comment_for_reflog reword
mark_action_done
- do_pick --edit $sha1 || die_with_patch $sha1 Could not apply
$sha1... $rest
+ eval do_pick --edit $opts $sha1 \
+ || die_with_patch $sha1 Could not apply $sha1... $rest
record_in_rewritten $sha1
;;
edit|e)
comment_for_reflog edit
mark_action_done
- do_pick $sha1 || die_with_patch $sha1 Could not apply $sha1...
$rest
+ eval do_pick $opts $sha1 \
+ || die_with_patch $sha1 Could not apply $sha1... $rest
warn Stopped at $sha1... $rest
exit_with_patch $sha1 0
;;
--
2.0.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH RFC v2 07/19] rebase -i: The replay of root commits is not shown with --verbose
The command line used to recreate root commits specifies the
erroneous option `-q` which suppresses the commit summary message.
However, git-rebase--interactive tends to tell the user about the
commits it creates, if she wishes (cf. command line option
`--verbose`). The code parts handling non-root commits or squash
commits all output commit summary messages. Do not make the replay of
root commits an exception. Remove the option.
It is OK to suppress the commit summary when git-commit is used to
initialize the authorship of the sentinel commit because the
existence of this additional commit is a detail of
git-rebase--interactive's implementation. The option `-q` was
probably introduced as a copy-and-paste error stemming from that part
of the root commit handling code.
Signed-off-by: Fabian Ruch baf...@gmail.com
---
git-rebase--interactive.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 0af96f2..ff04d5d 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -511,7 +511,7 @@ do_pick () {
--no-post-rewrite -n -q -C $1
pick_one -n $1
git commit --allow-empty \
- --amend --no-post-rewrite -n -q -C $1 \
+ --amend --no-post-rewrite -n -C $1 \
${gpg_sign_opt:+$gpg_sign_opt} ||
die_with_patch $1 Could not apply $1... $2
else
--
2.0.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH RFC v2 17/19] rebase -i: Teach do_pick the option --reset-author
`do_pick` is the git-cherry-pick wrapper in git-rebase--interactive
that is used to implement many of the to-do list commands.
Eventually, the complete `do_pick` interface will be exposed to the
user in some form or another and those commands will become simple
aliases for the `do_pick` options now used to implement them.
Add the git-commit option `--reset-author` to the options pool of
`do_pick`. It rewrites the author date and name of the picked commit
to match the committer date and name.
If `--reset-author` is passed to `do_pick`, set the `rewrite` flag
and relay the option to the git-commit command line which creates the
final commit. If `--amend` is not passed as well, the fresh
authorship effect is achieved by the mere fact that we are creating a
new commit.
Signed-off-by: Fabian Ruch baf...@gmail.com
---
git-rebase--interactive.sh | 23 ++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 2119d00..a9fcb76 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -464,10 +464,18 @@ record_in_rewritten() {
# Apply the changes introduced by the given commit to the current head.
#
-# do_pick [--amend] [--file file] [--edit] commit
+# do_pick [--reset-author] [--amend] [--file file] [--edit] commit
#
# Wrapper around git-cherry-pick.
#
+# --reset-author
+# Pretend the changes were made for the first time. Declare that the
+# authorship of the resulting commit now belongs to the committer.
+# This also renews the author timestamp. This creates a fresh
+# commit.
+#
+# _This is not a git-cherry-pick option._
+#
# --amend
# After picking commit, replace the current head commit with a new
# commit that also introduces the changes of commit.
@@ -501,6 +509,10 @@ do_pick () {
while test $# -gt 0
do
case $1 in
+ --reset-author)
+ rewrite=y
+ rewrite_author=y
+ ;;
--amend)
if test $(git rev-parse HEAD) = $squash_onto || !
git rev-parse --verify HEAD
then
@@ -562,12 +574,21 @@ do_pick () {
pick_one ${rewrite:+-n} $1 || return 1
fi
+ if test -n $rewrite_author test -z $rewrite_amend
+ then
+ # keep rewrite flag to create a new commit, rewrite
+ # without --reset-author though because it can only be
+ # used with -C, -c or --amend
+ rewrite_author=
+ fi
+
if test -n $rewrite
then
git commit --allow-empty --no-post-rewrite -n --no-edit \
${rewrite_amend:+--amend} \
${rewrite_edit:+--edit} \
${rewrite_message:+--file $rewrite_message} \
+ ${rewrite_author:+--reset-author} \
${gpg_sign_opt:+$gpg_sign_opt} || return 3
fi
--
2.0.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH RFC v2 14/19] rebase -i: Implement squash in terms of do_pick
The to-do list command `squash` and its close relative `fixup` replay
the changes of a commit like `pick` but do not recreate the commit.
Instead they replace the previous commit with a new commit that also
introduces the changes of the squashed commit. This is roughly like
cherry-picking without committing and using git-commit to amend the
previous commit.
The to-do list
pick a Some changes
squash b Some more changes
gets translated into the sequence of git commands
git cherry-pick a
git cherry-pick -n b
git commit --amend
and if git-cherry-pick supported `--amend` this would look even more
like the to-do list it is based on
git cherry-pick a
git cherry-pick --amend b.
Since `do_pick` takes care of `pick` entries and the above suggests
`squash` as an alias for `pick --amend`, reimplement `squash` in
terms of `do_pick --amend`. Introduce `$squash_msg` as the commit
message via the `--file` option. When the last commit of a squash
series is processed, the user is asked to review the log message.
Pass `--edit` as additional option in this case. The only difference
in the options passed to git-commit and `do_pick` is the omitted
`--no-verify`. However, `do_pick` does not execute the verification
hooks anyway because it solely replays commits and assumes that they
have been verified before.
Signed-off-by: Fabian Ruch baf...@gmail.com
---
git-rebase--interactive.sh | 30 ++
1 file changed, 6 insertions(+), 24 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 37800be..008f3a0 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -637,37 +637,19 @@ do_next () {
squash|s|fixup|f)
# This is an intermediate commit; its message will only
be
# used in case of trouble. So use the long version:
- if ! pick_one -n $sha1
- then
- git rev-parse --verify HEAD $amend
- die_failed_squash $sha1 Could not apply
$sha1... $rest
- fi
- do_with_author output git commit --amend --no-verify -F
$squash_msg \
- ${gpg_sign_opt:+$gpg_sign_opt} ||
- die_failed_squash $sha1 $rest
+ do_with_author do_pick --amend -F $squash_msg $sha1 \
+ || die_failed_squash $sha1 Could not apply
$sha1... $rest
;;
*)
# This is the final command of this squash/fixup group
if test -f $fixup_msg
then
- if ! pick_one -n $sha1
- then
- git rev-parse --verify HEAD $amend
- die_failed_squash $sha1 Could not
apply $sha1... $rest
- fi
- do_with_author git commit --amend --no-verify
-F $fixup_msg \
- ${gpg_sign_opt:+$gpg_sign_opt} ||
- die_failed_squash $sha1 $rest
+ do_with_author do_pick --amend -F $fixup_msg
$sha1 \
+ || die_failed_squash $sha1 Could not
apply $sha1... $rest
else
cp $squash_msg $GIT_DIR/SQUASH_MSG || exit
- if ! pick_one -n $sha1
- then
- git rev-parse --verify HEAD $amend
- die_failed_squash $sha1 Could not
apply $sha1... $rest
- fi
- do_with_author git commit --amend --no-verify
-F $GIT_DIR/SQUASH_MSG -e \
- ${gpg_sign_opt:+$gpg_sign_opt} ||
- die_failed_squash $sha1 $rest
+ do_with_author do_pick --amend -F
$GIT_DIR/SQUASH_MSG -e $sha1 \
+ || die_failed_squash $sha1 Could not
apply $sha1... $rest
fi
rm -f $squash_msg $fixup_msg
;;
--
2.0.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH RFC v2 18/19] rebase -i: Teach do_pick the option --signoff
`do_pick` is the git-cherry-pick wrapper in git-rebase--interactive
that is currently used to implement most of the to-do list commands
and offers additional options that will eventually find their way
onto to-do lists.
To extend the repertoire of available options, add the git-commit and
git-cherry-pick option `--signoff` to the `do_pick` interface. It
appends a Signed-off-by: line using the committer identity to the log
message of the picked commit.
Signed-off-by: Fabian Ruch baf...@gmail.com
---
git-rebase--interactive.sh | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index a9fcb76..bb258bb 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -464,10 +464,15 @@ record_in_rewritten() {
# Apply the changes introduced by the given commit to the current head.
#
-# do_pick [--reset-author] [--amend] [--file file] [--edit] commit
+# do_pick [--signoff] [--reset-author] [--amend] [--file file]
+# [--edit] commit
#
# Wrapper around git-cherry-pick.
#
+# -s, --signoff
+# Insert a Signed-off-by: line using the committer identity at the
+# end of the commit log message. This creates a fresh commit.
+#
# --reset-author
# Pretend the changes were made for the first time. Declare that the
# authorship of the resulting commit now belongs to the committer.
@@ -509,6 +514,10 @@ do_pick () {
while test $# -gt 0
do
case $1 in
+ -s|--signoff)
+ rewrite=y
+ rewrite_signoff=y
+ ;;
--reset-author)
rewrite=y
rewrite_author=y
@@ -585,6 +594,7 @@ do_pick () {
if test -n $rewrite
then
git commit --allow-empty --no-post-rewrite -n --no-edit \
+ ${rewrite_signoff:+--signoff} \
${rewrite_amend:+--amend} \
${rewrite_edit:+--edit} \
${rewrite_message:+--file $rewrite_message} \
--
2.0.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH RFC v2 12/19] rebase -i: Teach do_pick the option --file
`do_pick` is the git-cherry-pick wrapper in git-rebase--interactive
that is used to implement the to-do list command `pick`, `reword` and
`edit`. To cater for the different pick behaviours (like `squash`),
`do_pick` accepts several options not only from the git-cherry-pick
but also the git-commit interface.
Add the option `--file` from the git-commit interface to the options
pool of `do_pick`. It expects an argument itself which is interpreted
as a file path and takes the commit message from the given file. If
`--file` is passed to `do_pick`, assign the given file path to the
local variable `rewrite_message` and relay the option
--file $rewrite_message
to the git-commit command line which creates the commit.
Signed-off-by: Fabian Ruch baf...@gmail.com
---
git-rebase--interactive.sh | 20 +++-
1 file changed, 19 insertions(+), 1 deletion(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 046d358..47e3edf 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -464,7 +464,7 @@ record_in_rewritten() {
# Apply the changes introduced by the given commit to the current head.
#
-# do_pick [--amend] [--edit] commit
+# do_pick [--amend] [--file file] [--edit] commit
#
# Wrapper around git-cherry-pick.
#
@@ -474,6 +474,12 @@ record_in_rewritten() {
#
# _This is not a git-cherry-pick option._
#
+# -F file, --file file
+# Take the commit message from the given file. This creates a fresh
+# commit.
+#
+# _This is not a git-cherry-pick option._
+#
# -e, --edit
# After picking commit, open an editor and let the user edit the
# commit message. The editor contents becomes the commit message of
@@ -491,6 +497,7 @@ do_pick () {
rewrite=
rewrite_amend=
rewrite_edit=
+ rewrite_message=
while test $# -gt 0
do
case $1 in
@@ -504,6 +511,16 @@ do_pick () {
rewrite_amend=y
git rev-parse --verify HEAD $amend
;;
+ -F|--file)
+ if test $# -eq 0
+ then
+ warn do_pick: option --file specified but no
file given
+ return 2
+ fi
+ rewrite=y
+ rewrite_message=$2
+ shift
+ ;;
-e|--edit)
rewrite=y
rewrite_edit=y
@@ -550,6 +567,7 @@ do_pick () {
git commit --allow-empty --no-post-rewrite -n --no-edit \
${rewrite_amend:+--amend} \
${rewrite_edit:+--edit} \
+ ${rewrite_message:+--file $rewrite_message} \
${gpg_sign_opt:+$gpg_sign_opt} || return 3
fi
--
2.0.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH RFC v2 11/19] rebase -i: Teach do_pick the option --amend
`do_pick` is the git-cherry-pick wrapper in git-rebase--interactive
that is used to implement the to-do list commands `pick`, `reword`
and `edit`. To cater for the different pick behaviours (like
`squash`), `do_pick` accepts several options not only from the
git-cherry-pick but also the git-commit interface.
Add the option `--amend` from the git-commit interface to the options
pool of `do_pick`. It creates a new commit for the changes introduced
by the picked commit and the previous one. The previous commit is
then replaced with the new commit. If no other options are specified,
the log message of the previous commit is used.
Be careful when `--amend` is used to pick a root commit because HEAD
might point to the sentinel commit but there is still nothing to
amend. Be sure to initialize `amend` so that commits are squashed
even when git-rebase--interactive is interrupted for resolving
conflicts. It is not a mistake to do the initialization regardless of
any conflicts because `amend` is always cleared before the next to-do
item is processed.
Signed-off-by: Fabian Ruch baf...@gmail.com
---
git-rebase--interactive.sh | 18 +-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 0070b3e..046d358 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -464,10 +464,16 @@ record_in_rewritten() {
# Apply the changes introduced by the given commit to the current head.
#
-# do_pick [--edit] commit
+# do_pick [--amend] [--edit] commit
#
# Wrapper around git-cherry-pick.
#
+# --amend
+# After picking commit, replace the current head commit with a new
+# commit that also introduces the changes of commit.
+#
+# _This is not a git-cherry-pick option._
+#
# -e, --edit
# After picking commit, open an editor and let the user edit the
# commit message. The editor contents becomes the commit message of
@@ -488,6 +494,16 @@ do_pick () {
while test $# -gt 0
do
case $1 in
+ --amend)
+ if test $(git rev-parse HEAD) = $squash_onto || !
git rev-parse --verify HEAD
+ then
+ warn do_pick: nothing to amend
+ return 2
+ fi
+ rewrite=y
+ rewrite_amend=y
+ git rev-parse --verify HEAD $amend
+ ;;
-e|--edit)
rewrite=y
rewrite_edit=y
--
2.0.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH RFC v2 16/19] rebase -i: Parse to-do list command line options
Read in to-do list lines as
command args
instead of
command sha1 rest
so that to-do list command lines can specify additional arguments
apart from the commit hash and the log message title, which become
the non-options in `args`. Loop over `args`, put all options (an
argument beginning with a dash) in `opts`, stop the loop on the first
non-option and assign it to `sha1`. The loop does not know the
options it parses so that options that take an argument themselves
are not supported at the moment. Neither are options that contain
spaces because the shell expansion of `args` in `do_next` interprets
white space characters as argument separator, that is a command line
like
pick --author A U Thor fa1afe1 Some change
is parsed as the pick command
pick --author
and the commit hash
A
which obviously results in an unknown revision error. For the sake of
completeness, in the example above the message title variable `rest`
is assigned the string 'U Thor fa1afe1 Some change' (without the
single quotes).
Print an error message for unknown or unsupported command line
options, which means an error for all specified options at the
moment. Cleanly break the `do_next` loop by assigning the special
value 'unknown' to the local variable `command`, which triggers the
unknown command case in `do_cmd`.
The to-do list is also parsed when the commit hashes are translated
between long and short format before and after the to-do list is
edited. Apply the same procedure as in `do_cmd` with the exception
that we only care about where the options stop and the commit hash
begins. Do not reject any options when transforming the commit
hashes.
Signed-off-by: Fabian Ruch baf...@gmail.com
---
git-rebase--interactive.sh | 49 ++
1 file changed, 41 insertions(+), 8 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 9de7441..2119d00 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -587,8 +587,26 @@ do_pick () {
do_replay () {
command=$1
- sha1=$2
- rest=$3
+ shift
+
+ opts=
+ while test $# -gt 0
+ do
+ case $1 in
+ -*)
+ warn Unknown option: $1
+ command=unknown
+ ;;
+ *)
+ break
+ ;;
+ esac
+ opts=$opts $(git rev-parse --sq-quote $1)
+ shift
+ done
+ sha1=$1
+ shift
+ rest=$*
case $command in
pick|p)
@@ -671,7 +689,7 @@ do_replay () {
do_next () {
rm -f $msg $author_script $amend || exit
- read -r command sha1 rest $todo
+ read -r command args $todo
case $command in
$comment_char*|''|noop)
@@ -716,7 +734,7 @@ do_next () {
fi
;;
*)
- do_replay $command $sha1 $rest
+ do_replay $command $args
;;
esac
test -s $todo return
@@ -796,19 +814,34 @@ skip_unnecessary_picks () {
}
transform_todo_ids () {
- while read -r command rest
+ while read -r command args
do
case $command in
$comment_char* | exec)
# Be careful for oddball commands like 'exec'
# that do not have a SHA-1 at the beginning of $rest.
+ newargs=\ $args
;;
*)
- sha1=$(git rev-parse --verify --quiet $@ ${rest%% *})
- rest=$sha1 ${rest#* }
+ newargs=
+ sha1=
+ for arg in $args
+ do
+ case $arg in
+ -*)
+ newargs=$newargs $arg
+ ;;
+ *)
+ test -z $sha1
+ sha1=$(git rev-parse --verify
--quiet $@ $arg)
+ arg=$sha1
+ newargs=$newargs $arg
+ ;;
+ esac
+ done
;;
esac
- printf '%s\n' $command${rest:+ }$rest
+ printf '%s\n' $command$newargs
done $todo $todo.new
mv -f $todo.new $todo
}
--
2.0.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH RFC v2 13/19] rebase -i: Prepare for squash in terms of do_pick --amend
Rewrite `squash` and `fixup` handling in `do_next` using the sequence
pick_one
commit
in order to test the correctness of a single `do_squash` or
parameterised `do_pick` and make the subsequent patch reimplementing
`squash` in terms of such a single function more readable.
Do not call `rm -f $GIT_DIR/MERGE_MSG` since it has no effect on
the state after git-rebase--interactive terminates. The option `-F`
makes git-commit ignore `MERGE_MSG` for the log message. If
git-commit succeeds, `MERGE_MSG` is removed, and if it fails,
`MERGE_MSG` is overwritten by the error sequence `die_failed_squash`.
In summary, removing `MERGE_MSG` neither influences the squash commit
message nor the file state after git-commit returns.
Moreover, `pick_one` ignores `$GIT_DIR/SQUASH_MSG` and does not touch
`$squash_msg` so that it is correct to execute `pick_one` immediately
before git-commit.
Might be squashed into the subsequent commit.
Signed-off-by: Fabian Ruch baf...@gmail.com
---
git-rebase--interactive.sh | 21 +++--
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 47e3edf..37800be 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -633,15 +633,15 @@ do_next () {
author_script_content=$(get_author_ident_from_commit HEAD)
echo $author_script_content $author_script
eval $author_script_content
- if ! pick_one -n $sha1
- then
- git rev-parse --verify HEAD $amend
- die_failed_squash $sha1 $rest
- fi
case $(peek_next_command) in
squash|s|fixup|f)
# This is an intermediate commit; its message will only
be
# used in case of trouble. So use the long version:
+ if ! pick_one -n $sha1
+ then
+ git rev-parse --verify HEAD $amend
+ die_failed_squash $sha1 Could not apply
$sha1... $rest
+ fi
do_with_author output git commit --amend --no-verify -F
$squash_msg \
${gpg_sign_opt:+$gpg_sign_opt} ||
die_failed_squash $sha1 $rest
@@ -650,12 +650,21 @@ do_next () {
# This is the final command of this squash/fixup group
if test -f $fixup_msg
then
+ if ! pick_one -n $sha1
+ then
+ git rev-parse --verify HEAD $amend
+ die_failed_squash $sha1 Could not
apply $sha1... $rest
+ fi
do_with_author git commit --amend --no-verify
-F $fixup_msg \
${gpg_sign_opt:+$gpg_sign_opt} ||
die_failed_squash $sha1 $rest
else
cp $squash_msg $GIT_DIR/SQUASH_MSG || exit
- rm -f $GIT_DIR/MERGE_MSG
+ if ! pick_one -n $sha1
+ then
+ git rev-parse --verify HEAD $amend
+ die_failed_squash $sha1 Could not
apply $sha1... $rest
+ fi
do_with_author git commit --amend --no-verify
-F $GIT_DIR/SQUASH_MSG -e \
${gpg_sign_opt:+$gpg_sign_opt} ||
die_failed_squash $sha1 $rest
--
2.0.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 04/16] trace: improve trace performance
Karsten Blees karsten.bl...@gmail.com writes:
Replace the 'const char *key' parameter in the API with a pointer to a
'struct trace_key' that bundles the environment variable name with
additional, trace-internal state. Change the call sites of these APIs to
use a static 'struct trace_key' instead of a string constant.
Nice. Very nice ;-).
In trace.c::get_trace_fd(), save and reuse the file descriptor in 'struct
trace_key'.
Add a 'trace_disable()' API, so that packet_trace() can cleanly disable
tracing when it encounters packed data (instead of using unsetenv()).
Again, very nice.
Signed-off-by: Karsten Blees bl...@dcon.de
---
builtin/receive-pack.c | 2 +-
commit.h | 1 +
pkt-line.c | 8 ++--
shallow.c | 10 ++---
trace.c| 100
++---
trace.h| 16 ++--
6 files changed, 78 insertions(+), 59 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index c323081..1451050 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -438,7 +438,7 @@ static int update_shallow_ref(struct command *cmd, struct
shallow_info *si)
uint32_t mask = 1 (cmd-index % 32);
int i;
- trace_printf_key(GIT_TRACE_SHALLOW,
+ trace_printf_key(trace_shallow,
shallow: update_shallow_ref %s\n, cmd-ref_name);
for (i = 0; i si-shallow-nr; i++)
if (si-used_shallow[i]
diff --git a/commit.h b/commit.h
index a9f177b..08ef643 100644
--- a/commit.h
+++ b/commit.h
@@ -235,6 +235,7 @@ extern void assign_shallow_commits_to_refs(struct
shallow_info *info,
int *ref_status);
extern int delayed_reachability_test(struct shallow_info *si, int c);
extern void prune_shallow(int show_only);
+extern struct trace_key trace_shallow;
int is_descendant_of(struct commit *, struct commit_list *);
int in_merge_bases(struct commit *, struct commit *);
diff --git a/pkt-line.c b/pkt-line.c
index bc63b3b..8bc89b1 100644
--- a/pkt-line.c
+++ b/pkt-line.c
@@ -3,7 +3,7 @@
char packet_buffer[LARGE_PACKET_MAX];
static const char *packet_trace_prefix = git;
-static const char trace_key[] = GIT_TRACE_PACKET;
+static struct trace_key trace_packet = TRACE_KEY_INIT(PACKET);
void packet_trace_identity(const char *prog)
{
@@ -15,7 +15,7 @@ static void packet_trace(const char *buf, unsigned int len,
int write)
int i;
struct strbuf out;
- if (!trace_want(trace_key))
+ if (!trace_want(trace_packet))
return;
/* +32 is just a guess for header + quoting */
@@ -27,7 +27,7 @@ static void packet_trace(const char *buf, unsigned int len,
int write)
if ((len = 4 starts_with(buf, PACK)) ||
(len = 5 starts_with(buf+1, PACK))) {
strbuf_addstr(out, PACK ...);
- unsetenv(trace_key);
+ trace_disable(trace_packet);
}
else {
/* XXX we should really handle printable utf8 */
@@ -43,7 +43,7 @@ static void packet_trace(const char *buf, unsigned int len,
int write)
}
strbuf_addch(out, '\n');
- trace_strbuf(trace_key, out);
+ trace_strbuf(trace_packet, out);
strbuf_release(out);
}
diff --git a/shallow.c b/shallow.c
index 0b267b6..de07709 100644
--- a/shallow.c
+++ b/shallow.c
@@ -325,7 +325,7 @@ void prune_shallow(int show_only)
strbuf_release(sb);
}
-#define TRACE_KEY GIT_TRACE_SHALLOW
+struct trace_key trace_shallow = TRACE_KEY_INIT(SHALLOW);
/*
* Step 1, split sender shallow commits into ours and theirs
@@ -334,7 +334,7 @@ void prune_shallow(int show_only)
void prepare_shallow_info(struct shallow_info *info, struct sha1_array *sa)
{
int i;
- trace_printf_key(TRACE_KEY, shallow: prepare_shallow_info\n);
+ trace_printf_key(trace_shallow, shallow: prepare_shallow_info\n);
memset(info, 0, sizeof(*info));
info-shallow = sa;
if (!sa)
@@ -365,7 +365,7 @@ void remove_nonexistent_theirs_shallow(struct
shallow_info *info)
{
unsigned char (*sha1)[20] = info-shallow-sha1;
int i, dst;
- trace_printf_key(TRACE_KEY, shallow:
remove_nonexistent_theirs_shallow\n);
+ trace_printf_key(trace_shallow, shallow:
remove_nonexistent_theirs_shallow\n);
for (i = dst = 0; i info-nr_theirs; i++) {
if (i != dst)
info-theirs[dst] = info-theirs[i];
@@ -516,7 +516,7 @@ void assign_shallow_commits_to_refs(struct shallow_info
*info,
int *shallow, nr_shallow = 0;
struct paint_info pi;
- trace_printf_key(TRACE_KEY, shallow:
assign_shallow_commits_to_refs\n);
+ trace_printf_key(trace_shallow, shallow:
assign_shallow_commits_to_refs\n);
shallow = xmalloc(sizeof(*shallow) * (info-nr_ours + info-nr_theirs));
for (i = 0; i
[PATCH] enums: remove trailing ',' after last item in enum
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
builtin/clean.c | 2 +-
builtin/tag.c | 2 +-
pretty.c| 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/builtin/clean.c b/builtin/clean.c
index 9a91515..27701d2 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -48,7 +48,7 @@ enum color_clean {
CLEAN_COLOR_PROMPT = 2,
CLEAN_COLOR_HEADER = 3,
CLEAN_COLOR_HELP = 4,
- CLEAN_COLOR_ERROR = 5,
+ CLEAN_COLOR_ERROR = 5
};
#define MENU_OPTS_SINGLETON01
diff --git a/builtin/tag.c b/builtin/tag.c
index c6e8a71..ef76556 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -83,7 +83,7 @@ static int in_commit_list(const struct commit_list *want,
struct commit *c)
enum contains_result {
CONTAINS_UNKNOWN = -1,
CONTAINS_NO = 0,
- CONTAINS_YES = 1,
+ CONTAINS_YES = 1
};
/*
diff --git a/pretty.c b/pretty.c
index 4f51287..924bc61 100644
--- a/pretty.c
+++ b/pretty.c
@@ -274,7 +274,7 @@ static void add_rfc822_quoted(struct strbuf *out, const
char *s, int len)
enum rfc2047_type {
RFC2047_SUBJECT,
- RFC2047_ADDRESS,
+ RFC2047_ADDRESS
};
static int is_rfc2047_special(char ch, enum rfc2047_type type)
--
2.0.1.442.g7fe6834.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Re* [PATCH v3 04/14] refs.c: add a new update_type field to ref_update
On Wed, Jun 18, 2014 at 2:10 PM, Junio C Hamano gits...@pobox.com wrote:
Junio C Hamano gits...@pobox.com writes:
Ronnie Sahlberg sahlb...@google.com writes:
Add a field that describes what type of update this refers to. For now
the only type is UPDATE_SHA1 but we will soon add more types.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c | 25 +
1 file changed, 21 insertions(+), 4 deletions(-)
diff --git a/refs.c b/refs.c
index 4e3d4c3..4129de6 100644
--- a/refs.c
+++ b/refs.c
@@ -3374,6 +3374,10 @@ int for_each_reflog(each_ref_fn fn, void *cb_data)
return retval;
}
+enum transaction_update_type {
+ UPDATE_SHA1 = 0,
indent with SP?
Unlike an array initialisation, e.g.
int foo[] = { 1,2,3,4,5, };
some compilers we support complain if enum definition ends with a
trailing comma.
I do recall we had fixes to drop the comma after the last element in
enum definition in the past, in response real compilation breakages
on some platforms.
You are right. I think I recall this too on old c compilers on legacy
unix boxens.
I have fixed this and will resend the series.
I have also checked additional enums in the sources and found 3 more places.
I sent this as a single patch to the list with the subject :
Subject: [PATCH] enums: remove trailing ',' after last item in enum
But there is a curious thing:
git grep -Asomenumber 'enum ' master -- \*.c
tells me that builtin/clean.c would fail to compile for those folks.
Most likely. It might be that there are less and less people on
really old c-compilers these days.
Here is an off-topic fix that may no longer be needed.
builtin/clean.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/clean.c b/builtin/clean.c
index 9a91515..27701d2 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -48,7 +48,7 @@ enum color_clean {
CLEAN_COLOR_PROMPT = 2,
CLEAN_COLOR_HEADER = 3,
CLEAN_COLOR_HELP = 4,
- CLEAN_COLOR_ERROR = 5,
+ CLEAN_COLOR_ERROR = 5
};
#define MENU_OPTS_SINGLETON01
Thanks
ronnie sahlberg
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] t7300: repair filesystem permissions with test_when_finished
We create a directory that cannot be removed, confirm that it cannot be removed, and then fix it like: chmod 0 foo test_must_fail git clean -d -f chmod 755 foo If the middle step fails but leaves the directory (e.g., the bug is that clean does not notice the failure), this pollutes the test repo with an unremovable directory. Not only does this cause further tests to fail, but it means that rm -rf fails on the whole trash directory, and the user has to intervene manually to even re-run the test script. We can bump the chmod 755 recovery to a test_when_finished block to be sure that it always runs. Signed-off-by: Jeff King p...@peff.net --- t/t7300-clean.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh index 74de814..04118ad 100755 --- a/t/t7300-clean.sh +++ b/t/t7300-clean.sh @@ -426,10 +426,10 @@ test_expect_success SANITY 'removal failure' ' mkdir foo touch foo/bar + test_when_finished chmod 755 foo (exec foo/bar chmod 0 foo -test_must_fail git clean -f -d -chmod 755 foo) +test_must_fail git clean -f -d) ' test_expect_success 'nested git work tree' ' -- 2.0.0.566.gfe3e6b2 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 11/16] trace: add 'file:line' to all trace output
Karsten Blees karsten.bl...@gmail.com writes: +#else + +/* + * Macros to add file:line - see above for C-style declarations of how these + * should be used. + * + * TRACE_CONTEXT may be set to __FUNCTION__ if the compiler supports it. The + * default is __FILE__, as it is consistent with assert(), and static function + * names are not necessarily unique. + */ +#define TRACE_CONTEXT __FILE__ Hmph, seeing may be set to forces me to wonder how. Perhaps #ifndef/#endif around it? Also, can it be set to something like __FILE__ : __FUNCTION__ which may alleviate the alleged problem of not necessarily unique perhaps? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t7300: repair filesystem permissions with test_when_finished
Jeff King wrote: Signed-off-by: Jeff King p...@peff.net --- t/t7300-clean.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) Does the later git clean -d with an unreadable empty directory test need the same treatment? Thanks, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t7300: repair filesystem permissions with test_when_finished
On Wed, Jul 02, 2014 at 12:01:59PM -0700, Jonathan Nieder wrote: Jeff King wrote: Signed-off-by: Jeff King p...@peff.net --- t/t7300-clean.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) Does the later git clean -d with an unreadable empty directory test need the same treatment? I don't think so, because it is also failing during the experiments I'm doing and it is not causing the same problem. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFH] git clean deletes excluded files in untracked directories
If you have an untracked directory that contains excluded files, like
this:
mkdir foo
echo content foo/one
echo content foo/two
echo foo/one .gitignore
then git clean -d will notice that foo is untracked and recursively
delete it and its contents, including the ignored foo/one.
Our stance has always been that ignored files are not precious, so in
that sense it is not a big loss. But git clean does provide a -x
option, and takes care to avoid deleting ignored files when it is not
given. So I'd argue that we should delete foo/two but retain
foo/one, unless -x is given.
I'm not sure of the best way to go about that, though. The patch below
is my first attempt. It stops using DIR_SHOW_OTHER_DIRECTORIES when we
find the list of files to delete, which means we get the full set of
paths (so instead of foo/, we see foo/one and foo/two after
calling fill_directory). But then we have to notice that foo should go
away if all of its contents do. For that, I use remove_path, which just
walks up the pathname, removing leading directories until we hit a
non-empty one.
But there are a few problems:
1. Is it possible that we want to remove foo/bar but not foo? The
obvious case would be that foo is excluded but foo/bar is not. I
guess you could get that with negative excludes, like:
foo/
!foo/bar
2. The output is different (we report each file we are removing,
rather than just the root directory).
3. The error handling is different. If we have an unreadable untracked
directory now, fill_directory does not recurse into it; we get the
directory name and complain when we cannot remove it. But with this
patch, fill_directory tries to enter it, can't, and simply fails to
report any entries inside it (this causes test failures in t7300).
So this is probably not the right way to go about it. I'm not sure of
the best way.
One option is to actually teach clean's recursive remove_dirs function
to actually check excludes itself for each file. That feels hacky,
though, since we should be checking them already in fill_directory.
Another is to add a flag to fill_directory to indicate that it should
recurse into directories to find excluded files, but not otherwise (we
usually avoid it for reasons of efficiency, but since we would be
immediately entering them to delete anyway, I don't think that's a
concern here). I think this makes the fill_directory logic quite
complicated, though.
Suggestions?
---
diff --git a/builtin/clean.c b/builtin/clean.c
index 9a91515..f159eed 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -914,7 +914,8 @@ int cmd_clean(int argc, const char **argv, const char
*prefix)
if (force 1)
rm_flags = 0;
- dir.flags |= DIR_SHOW_OTHER_DIRECTORIES;
+ if (!remove_directories)
+ dir.flags |= DIR_SHOW_OTHER_DIRECTORIES;
if (read_cache() 0)
die(_(index file corrupt));
@@ -985,7 +986,7 @@ int cmd_clean(int argc, const char **argv, const char
*prefix)
printf(dry_run ? _(msg_would_remove) :
_(msg_remove), qname);
}
} else {
- res = dry_run ? 0 : unlink(abs_path.buf);
+ res = dry_run ? 0 : remove_path(abs_path.buf);
if (res) {
qname = quote_path_relative(item-string, NULL,
buf);
warning(_(msg_warn_remove_failed), qname);
diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh
index 04118ad..647844f 100755
--- a/t/t7300-clean.sh
+++ b/t/t7300-clean.sh
@@ -527,4 +527,12 @@ test_expect_success 'git clean -d respects pathspecs
(pathspec is prefix of dir)
test_path_is_dir foobar
'
+test_expect_success 'clean -d does not clean ignored files' '
+ mkdir -p foo
+ echo content foo/file
+ echo foo/* .gitignore
+ git clean -df /dev/tty
+ test_path_is_file foo/file
+'
+
test_done
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 11/16] trace: add 'file:line' to all trace output
Karsten Blees karsten.bl...@gmail.com writes: Also, can it be set to something like __FILE__ : __FUNCTION__ which may alleviate the alleged problem of not necessarily unique perhaps? Should work with MSVC. With GCC, however, __FUNCTION__ is a string constant supplied by the compiler, so string literal concatenation doesn't work. Shucks, but not too big a deal, I guess ;-) Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 11/16] trace: add 'file:line' to all trace output
Am 02.07.2014 20:57, schrieb Junio C Hamano: Karsten Blees karsten.bl...@gmail.com writes: +#else + +/* + * Macros to add file:line - see above for C-style declarations of how these + * should be used. + * + * TRACE_CONTEXT may be set to __FUNCTION__ if the compiler supports it. The + * default is __FILE__, as it is consistent with assert(), and static function + * names are not necessarily unique. + */ +#define TRACE_CONTEXT __FILE__ Hmph, seeing may be set to forces me to wonder how. Perhaps #ifndef/#endif around it? Right, shame on me. I didn't think it would be important enough to warrant a Makefile option, but #ifndef sure wouldn't hurt. Also, can it be set to something like __FILE__ : __FUNCTION__ which may alleviate the alleged problem of not necessarily unique perhaps? Should work with MSVC. With GCC, however, __FUNCTION__ is a string constant supplied by the compiler, so string literal concatenation doesn't work. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/14] Add submodule test harness
Am 02.07.2014 16:54, schrieb Torsten Bögershausen:
(Not sure if this is the right thread)
(I haven't checked if this is fixed in your latest version)
Yes, this is the right thread and no, it isn't fixed yet.
On what I have on pu 7a0da7902cbbc9a876b90c9, Tue Jul 1 14:51:53 2014 -0700
Many submodule tests are broken.
One problem is here:
lib-submodule-update.sh:264: possible problem: echo -n is not portable
(please use printf): echo -n sub1
lib-submodule-update.sh:507: possible problem: echo -n is not portable
(please use printf): echo -n sub1
You can remove the empty echo -n to create an empty file:
sub1
Thanks for spotting and diagnosing this. Running make lint in the
test directory only feeds the tests to check-non-portable-shell.pl,
but not the *lib*.sh helper scripts, which made me miss this one.
The following diff should fix it for you. Am I understanding you
correctly that you are experiencing other failures too? I see no
other incompatibilities when running ./check-non-portable-shell.pl
on all the shell scripts in the repo.
-8
diff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh
index 24c9fd7..3584755 100755
--- a/t/lib-submodule-update.sh
+++ b/t/lib-submodule-update.sh
@@ -304,7 +304,7 @@ test_submodule_switch () {
(
cd submodule_update
git branch -t add_sub1 origin/add_sub1
- echo -n sub1
+ sub1
test_must_fail $command add_sub1
test_superproject_content origin/no_submodule
test_must_be_empty sub1
@@ -547,7 +547,7 @@ test_submodule_forced_switch () {
(
cd submodule_update
git branch -t add_sub1 origin/add_sub1
- echo -n sub1
+ sub1
$command add_sub1
test_superproject_content origin/add_sub1
test_dir_is_empty sub1
--
2.0.1.458.gf680257.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] enums: remove trailing ',' after last item in enum
Ronnie Sahlberg wrote: Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- builtin/clean.c | 2 +- builtin/tag.c | 2 +- pretty.c| 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) Is there some gcc option or other tool that can automatically detect this kind of problem so the regress/fix cycle doesn't have to repeat too many times? Looks like v1.7.2-rc0~32^2~16 (2010-03-14) and v1.7.4.2~34 (2011-03-16) tried to fix this in the past. Using the test from v1.7.4.2~34 also finds enums with trailing comma in grep.h log-tree.c Thanks, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t7300: repair filesystem permissions with test_when_finished
Jeff King p...@peff.net writes: On Wed, Jul 02, 2014 at 12:01:59PM -0700, Jonathan Nieder wrote: Jeff King wrote: Signed-off-by: Jeff King p...@peff.net --- t/t7300-clean.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) Does the later git clean -d with an unreadable empty directory test need the same treatment? I don't think so, because it is also failing during the experiments I'm doing and it is not causing the same problem. It tests that clean -d is happy if a blind rmdir(2) removes the directory. If it fails for whatever reason (e.g. add exit(0) at the beginning of cmd_clean(), for example) to remove the directory, we do leave an empty unreadable directory behind. But as long as that directory is empty, that will not cause us to remove the trash directory at the end, so we should be OK. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] enums: remove trailing ',' after last item in enum
GCC can check/error for this with --pedantic -Werror On Wed, Jul 2, 2014 at 12:58 PM, Jonathan Nieder jrnie...@gmail.com wrote: Ronnie Sahlberg wrote: Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- builtin/clean.c | 2 +- builtin/tag.c | 2 +- pretty.c| 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) Is there some gcc option or other tool that can automatically detect this kind of problem so the regress/fix cycle doesn't have to repeat too many times? Looks like v1.7.2-rc0~32^2~16 (2010-03-14) and v1.7.4.2~34 (2011-03-16) tried to fix this in the past. Using the test from v1.7.4.2~34 also finds enums with trailing comma in grep.h log-tree.c Thanks, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 2/7] replace: add test for --graft
Christian Couder chrisc...@tuxfamily.org writes:
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
Signed-off-by: Junio C Hamano gits...@pobox.com
---
t/t6050-replace.sh | 12
1 file changed, 12 insertions(+)
diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index 68b3cb2..ca45a84 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -351,4 +351,16 @@ test_expect_success 'replace ref cleanup' '
test -z $(git replace)
'
+test_expect_success '--graft with and without already replaced object' '
+ test $(git log --oneline | wc -l) = 7
+ git replace --graft $HASH5
+ test $(git log --oneline | wc -l) = 3
+ git cat-file -p $HASH5 | test_must_fail grep parent
Please do not run non-git command under test_must_fail.
+ test_must_fail git replace --graft $HASH5 $HASH4 $HASH3
+ git replace --force -g $HASH5 $HASH4 $HASH3
+ git cat-file -p $HASH5 | grep parent $HASH4
+ git cat-file -p $HASH5 | grep parent $HASH3
+ git replace -d $HASH5
+'
+
test_done
For all these git cat-file -p $commit | grep ..., I would think
you should add commit_has_parents helper function to allow a bit
more careful testing. As written, the test will mistake a commit
with string parent $HASHx anywhere, not in the header part, and
the test does not ensure that $HASH{3,4} is the {first,second}
parent.
commit_has_parents $HASH5 $HASH4 $HASH3
would then may be implemented like:
commit_has_parents () {
git cat-file commit $1 payload
sed -n -e '/^$/q' -e 's/^parent //p' payload actual
shift
printf 'parent %s\n' $@ expect
test_cmp expect actual
}
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 5/7] replace: refactor replacing parents
Christian Couder chrisc...@tuxfamily.org writes:
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
builtin/replace.c | 42 +-
1 file changed, 25 insertions(+), 17 deletions(-)
diff --git a/builtin/replace.c b/builtin/replace.c
index 3515979..ad47237 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -295,27 +295,14 @@ static int edit_and_replace(const char *object_ref, int
force)
return replace_object_sha1(object_ref, old, replacement, new, force);
}
-static int create_graft(int argc, const char **argv, int force)
+static void replace_parents(struct strbuf *buf, int argc, const char **argv)
It is somewhat strange to see that a new function introduced earlier
in the series is rewritten with a refactoring. Shouldn't the new
function have been done right from the beginning instead?
{
- unsigned char old[20], new[20];
- const char *old_ref = argv[0];
- struct commit *commit;
- struct strbuf buf = STRBUF_INIT;
struct strbuf new_parents = STRBUF_INIT;
const char *parent_start, *parent_end;
- const char *buffer;
- unsigned long size;
int i;
- if (get_sha1(old_ref, old) 0)
- die(_(Not a valid object name: '%s'), old_ref);
- commit = lookup_commit_or_die(old, old_ref);
-
/* find existing parents */
- buffer = get_commit_buffer(commit, size);
- strbuf_add(buf, buffer, size);
- unuse_commit_buffer(commit, buffer);
- parent_start = buf.buf;
+ parent_start = buf-buf;
parent_start += 46; /* tree + hex sha1 + \n */
parent_end = parent_start;
@@ -332,13 +319,34 @@ static int create_graft(int argc, const char **argv,
int force)
}
/* replace existing parents with new ones */
- strbuf_splice(buf, parent_start - buf.buf, parent_end - parent_start,
+ strbuf_splice(buf, parent_start - buf-buf, parent_end - parent_start,
new_parents.buf, new_parents.len);
+ strbuf_release(new_parents);
+}
+
+static int create_graft(int argc, const char **argv, int force)
+{
+ unsigned char old[20], new[20];
+ const char *old_ref = argv[0];
+ struct commit *commit;
+ struct strbuf buf = STRBUF_INIT;
+ const char *buffer;
+ unsigned long size;
+
+ if (get_sha1(old_ref, old) 0)
+ die(_(Not a valid object name: '%s'), old_ref);
+ commit = lookup_commit_or_die(old, old_ref);
+
+ buffer = get_commit_buffer(commit, size);
+ strbuf_add(buf, buffer, size);
+ unuse_commit_buffer(commit, buffer);
+
+ replace_parents(buf, argc, argv);
+
if (write_sha1_file(buf.buf, buf.len, commit_type, new))
die(_(could not write replacement commit for: '%s'), old_ref);
- strbuf_release(new_parents);
strbuf_release(buf);
if (!hashcmp(old, new))
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 6/7] replace: remove signature when using --graft
Christian Couder chrisc...@tuxfamily.org writes:
It could be misleading to keep a signature in a
replacement commit, so let's remove it.
Note that there should probably be a way to sign
the replacement commit created when using --graft,
but this can be dealt with in another commit or
patch series.
Both paragraphs read very sensibly.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
builtin/replace.c | 5 +
commit.c | 34 ++
commit.h | 2 ++
3 files changed, 41 insertions(+)
diff --git a/builtin/replace.c b/builtin/replace.c
index ad47237..000db65 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -344,6 +344,11 @@ static int create_graft(int argc, const char **argv, int
force)
replace_parents(buf, argc, argv);
+ if (remove_signature(buf))
+ warning(_(the original commit '%s' has a gpg signature.\n
+ It will be removed in the replacement commit!),
Hmmm... does the second line of this message start with the usual
warning: prefix?
diff --git a/commit.c b/commit.c
index fb7897c..54e157d 100644
--- a/commit.c
+++ b/commit.c
@@ -1177,6 +1177,40 @@ int parse_signed_commit(const struct commit *commit,
return saw_signature;
}
+int remove_signature(struct strbuf *buf)
+{
+ const char *line = buf-buf;
+ const char *tail = buf-buf + buf-len;
+ int in_signature = 0;
+ const char *sig_start = NULL;
+ const char *sig_end = NULL;
+
+ while (line tail) {
+ const char *next = memchr(line, '\n', tail - line);
+ next = next ? next + 1 : tail;
This almost makes me wonder if we want something similar to
strchrnul() we use for NUL-terminated strings, and I suspect that
you would find more instances by running git grep -A2 memchr.
I don't know what such a helper function should be named, though.
Certainly not memchrnul().
+
+ if (in_signature line[0] == ' ')
+ sig_end = next;
+ else if (starts_with(line, gpg_sig_header)
+ line[gpg_sig_header_len] == ' ') {
+ sig_start = line;
+ sig_end = next;
+ in_signature = 1;
+ } else {
+ if (*line == '\n')
+ /* dump the whole remainder of the buffer */
+ next = tail;
+ in_signature = 0;
+ }
+ line = next;
+ }
+
+ if (sig_start)
+ strbuf_remove(buf, sig_start - buf-buf, sig_end - sig_start);
If there are two instances of gpg_sig, this will remove only the
last one, but there is no chance both signatures of such a commit
can validate OK, and we won't be losing something in between anyway,
so it should be fine.
+ return sig_start != NULL;
+}
+
static void handle_signed_tag(struct commit *parent, struct
commit_extra_header ***tail)
{
struct merge_remote_desc *desc;
diff --git a/commit.h b/commit.h
index 2e1492a..4234dae 100644
--- a/commit.h
+++ b/commit.h
@@ -327,6 +327,8 @@ struct commit *get_merge_parent(const char *name);
extern int parse_signed_commit(const struct commit *commit,
struct strbuf *message, struct strbuf
*signature);
+extern int remove_signature(struct strbuf *buf);
+
extern void print_commit_list(struct commit_list *list,
const char *format_cur,
const char *format_last);
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Race condition in git push --mirror can cause silent ref rewinding
Heya, We recently ran into a particularly troubling race condition, discovered in git 2.0.0. The setup for it is as follows: The repository is a bare repository, which developers push to via ssh; it mirrors its changes out onto github. In its config: [remote github] url = g...@github.com:bestpractical/rt.git fetch = +refs/*:refs/* mirror = yes It has a post-receive hook which does: sudo -u git -H /usr/bin/git push github We recently saw a situation where a push of a new branch caused a simultaneous update of a different branch (by a different user) to be rewound. From the reflog of the created branch (4.2/html-gumbo-loading): 1aefd600fcbb5ded14376f77d77a14758668fb39 Wallace Reis wr...@bestpractical.com 1404326443 -0400 push And the updated branch (4.2-trunk), which was rewound: 44dc8ad0e4603e3f674b7c00deacc122ca52707a 1e743b6225d502ad1a265929fb873f4c0bf4f8a5 Kevin Falcone falc...@bestpractical.com 1404326446 -0400push 1e743b6225d502ad1a265929fb873f4c0bf4f8a5 44dc8ad0e4603e3f674b7c00deacc122ca52707a git g...@bestpractical.com 1404326446 -0400update by push It is my belief that this comes because the --mirror argument causes the local refs to be treated as tracking refs -- and thus updates all of them during the push. I believe the race condition is thus: 1. User A starts a push --mirror; git records the values of the refs 2. User B updates a ref, commit mail goes out, etc 3. User A's push completes, updates tracking branch to value at (1). Needless to say, silently losing commits which appeared for all purposes to be pushed successfully (neither User A nor User B sees anything out of the ordinary) is extremely troubling. - Alex -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v1 0/4] hashmap improvements
Here are a few small hashmap improvements, partly resulting from recent discussion of the config-cache topic. Karsten Blees (4): hashmap: factor out getting an int hash code from a SHA1 hashmap: improve struct hashmap member documentation hashmap: add simplified hashmap_get_from_hash() API hashmap: add string interning API Documentation/technical/api-hashmap.txt | 54 ++--- builtin/describe.c | 13 ++-- decorate.c | 5 +-- diffcore-rename.c | 11 +++ hashmap.c | 38 +++ hashmap.h | 27 + khash.h | 11 ++- name-hash.c | 5 ++- object.c| 13 +--- pack-objects.c | 5 ++- t/t0011-hashmap.sh | 13 test-hashmap.c | 25 ++- 12 files changed, 159 insertions(+), 61 deletions(-) -- 1.9.4.msysgit.0.dirty -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Race condition in git push --mirror can cause silent ref rewinding
Alex Vandiver a...@chmrr.net writes: [remote github] url = g...@github.com:bestpractical/rt.git fetch = +refs/*:refs/* mirror = yes git push github master^:master must stay a usable way to update the published repository to an arbitrary commit, so if set to mirror, do not pretend that a fetch in reverse has happened during 'git push' will not be a solution to this issue. Perhaps removing remote.github.fetch would be one sane way forward. Otherwise, even if your git push does not pretend to immediately fetch from there (i.e. even if the reported behaviour was a bug, without doing anything to trigger it) somebody running git fetch in this repository can destroy what other person pushes into this repository at the same time exactly the same way, I would think. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v1 1/4] hashmap: factor out getting an int hash code from a, SHA1
Copying the first bytes of a SHA1 is duplicated in six places, however,
the implications (wrong byte order on little-endian systems) is documented
only once.
Add a properly documented API for this.
Signed-off-by: Karsten Blees bl...@dcon.de
---
Documentation/technical/api-hashmap.txt | 9 +
builtin/describe.c | 11 ++-
decorate.c | 5 +
diffcore-rename.c | 4 +---
hashmap.h | 11 +++
khash.h | 11 ++-
object.c| 13 +
pack-objects.c | 5 ++---
8 files changed, 29 insertions(+), 40 deletions(-)
diff --git a/Documentation/technical/api-hashmap.txt
b/Documentation/technical/api-hashmap.txt
index b977ae8..4689968 100644
--- a/Documentation/technical/api-hashmap.txt
+++ b/Documentation/technical/api-hashmap.txt
@@ -58,6 +58,15 @@ Functions
+
`strihash` and `memihash` are case insensitive versions.
+`unsigned int sha1hash(const unsigned char *sha1)`::
+
+ Converts a cryptographic hash (e.g. SHA-1) into an int-sized hash code
+ for use in hash tables. Cryptographic hashes are supposed to have
+ uniform distribution, so in contrast to `memhash()`, this just copies
+ the first `sizeof(int)` bytes without shuffling any bits. Note that
+ the results will be different on big-endian and little-endian
+ platforms, so they should not be stored or transferred over the net!
+
`void hashmap_init(struct hashmap *map, hashmap_cmp_fn equals_function, size_t
initial_size)`::
Initializes a hashmap structure.
diff --git a/builtin/describe.c b/builtin/describe.c
index 24d740c..57e84c8 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -56,17 +56,10 @@ static int commit_name_cmp(const struct commit_name *cn1,
return hashcmp(cn1-peeled, peeled ? peeled : cn2-peeled);
}
-static inline unsigned int hash_sha1(const unsigned char *sha1)
-{
- unsigned int hash;
- memcpy(hash, sha1, sizeof(hash));
- return hash;
-}
-
static inline struct commit_name *find_commit_name(const unsigned char *peeled)
{
struct commit_name key;
- hashmap_entry_init(key, hash_sha1(peeled));
+ hashmap_entry_init(key, sha1hash(peeled));
return hashmap_get(names, key, peeled);
}
@@ -114,7 +107,7 @@ static void add_to_known_names(const char *path,
if (!e) {
e = xmalloc(sizeof(struct commit_name));
hashcpy(e-peeled, peeled);
- hashmap_entry_init(e, hash_sha1(peeled));
+ hashmap_entry_init(e, sha1hash(peeled));
hashmap_add(names, e);
e-path = NULL;
}
diff --git a/decorate.c b/decorate.c
index 7cb5d29..b2aac90 100644
--- a/decorate.c
+++ b/decorate.c
@@ -8,10 +8,7 @@
static unsigned int hash_obj(const struct object *obj, unsigned int n)
{
- unsigned int hash;
-
- memcpy(hash, obj-sha1, sizeof(unsigned int));
- return hash % n;
+ return sha1hash(obj-sha1) % n;
}
static void *insert_decoration(struct decoration *n, const struct object
*base, void *decoration)
diff --git a/diffcore-rename.c b/diffcore-rename.c
index 749a35d..6fa97d4 100644
--- a/diffcore-rename.c
+++ b/diffcore-rename.c
@@ -242,14 +242,12 @@ struct file_similarity {
static unsigned int hash_filespec(struct diff_filespec *filespec)
{
- unsigned int hash;
if (!filespec-sha1_valid) {
if (diff_populate_filespec(filespec, 0))
return 0;
hash_sha1_file(filespec-data, filespec-size, blob,
filespec-sha1);
}
- memcpy(hash, filespec-sha1, sizeof(hash));
- return hash;
+ return sha1hash(filespec-sha1);
}
static int find_identical_files(struct hashmap *srcs,
diff --git a/hashmap.h b/hashmap.h
index a816ad4..ed5425a 100644
--- a/hashmap.h
+++ b/hashmap.h
@@ -13,6 +13,17 @@ extern unsigned int strihash(const char *buf);
extern unsigned int memhash(const void *buf, size_t len);
extern unsigned int memihash(const void *buf, size_t len);
+static inline unsigned int sha1hash(const unsigned char *sha1)
+{
+ /*
+* Equivalent to 'return *(int *)sha1;', but safe on platforms that
+* don't support unaligned reads.
+*/
+ unsigned int hash;
+ memcpy(hash, sha1, sizeof(hash));
+ return hash;
+}
+
/* data structures */
struct hashmap_entry {
diff --git a/khash.h b/khash.h
index 57ff603..06c7906 100644
--- a/khash.h
+++ b/khash.h
@@ -320,19 +320,12 @@ static const double __ac_HASH_UPPER = 0.77;
code;
\
} }
-static inline khint_t __kh_oid_hash(const unsigned char *oid)
-{
- khint_t hash;
-
[PATCH v1 2/4] hashmap: improve struct hashmap member documentation
Signed-off-by: Karsten Blees bl...@dcon.de --- Documentation/technical/api-hashmap.txt | 16 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/Documentation/technical/api-hashmap.txt b/Documentation/technical/api-hashmap.txt index 4689968..dc21a7c 100644 --- a/Documentation/technical/api-hashmap.txt +++ b/Documentation/technical/api-hashmap.txt @@ -8,11 +8,19 @@ Data Structures `struct hashmap`:: - The hash table structure. + The hash table structure. Members can be used as follows, but should + not be modified directly: + -The `size` member keeps track of the total number of entries. The `cmpfn` -member is a function used to compare two entries for equality. The `table` and -`tablesize` members store the hash table and its size, respectively. +The `size` member keeps track of the total number of entries (0 means the +hashmap is empty). ++ +`tablesize` is the allocated size of the hash table. A non-0 value indicates +that the hashmap is initialized. It may also be useful for statistical purposes +(i.e. `size / tablesize` is the current load factor). ++ +`cmpfn` stores the comparison function specified in `hashmap_init()`. In +advanced scenarios, it may be useful to change this, e.g. to switch between +case-sensitive and case-insensitive lookup. `struct hashmap_entry`:: -- 1.9.4.msysgit.0.dirty -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v1 4/4] hashmap: add string interning API
Interning short strings with high probability of duplicates can reduce the
memory footprint and speed up comparisons.
Add strintern() and memintern() APIs that use a hashmap to manage the pool
of unique, interned strings.
Note: strintern(getenv()) could be used to sanitize git's use of getenv(),
in case we ever encounter a platform where a call to getenv() invalidates
previous getenv() results (which is allowed by POSIX).
Signed-off-by: Karsten Blees bl...@dcon.de
---
Documentation/technical/api-hashmap.txt | 15 +
hashmap.c | 38 +
hashmap.h | 8 +++
t/t0011-hashmap.sh | 13 +++
test-hashmap.c | 14
5 files changed, 88 insertions(+)
diff --git a/Documentation/technical/api-hashmap.txt
b/Documentation/technical/api-hashmap.txt
index f9215d6..00c4c29 100644
--- a/Documentation/technical/api-hashmap.txt
+++ b/Documentation/technical/api-hashmap.txt
@@ -193,6 +193,21 @@ more entries.
`hashmap_iter_first` is a combination of both (i.e. initializes the iterator
and returns the first entry, if any).
+`const char *strintern(const char *string)`::
+`const void *memintern(const void *data, size_t len)`::
+
+ Returns the unique, interned version of the specified string or data,
+ similar to the `String.intern` API in Java and .NET, respectively.
+ Interned strings remain valid for the entire lifetime of the process.
++
+Can be used as `[x]strdup()` or `xmemdupz` replacement, except that interned
+strings / data must not be modified or freed.
++
+Interned strings are best used for short strings with high probability of
+duplicates.
++
+Uses a hashmap to store the pool of interned strings.
+
Usage example
-
diff --git a/hashmap.c b/hashmap.c
index d1b8056..f693839 100644
--- a/hashmap.c
+++ b/hashmap.c
@@ -226,3 +226,41 @@ void *hashmap_iter_next(struct hashmap_iter *iter)
current = iter-map-table[iter-tablepos++];
}
}
+
+struct pool_entry {
+ struct hashmap_entry ent;
+ size_t len;
+ unsigned char data[FLEX_ARRAY];
+};
+
+static int pool_entry_cmp(const struct pool_entry *e1,
+ const struct pool_entry *e2,
+ const unsigned char *keydata)
+{
+ return e1-data != keydata
+ (e1-len != e2-len || memcmp(e1-data, keydata, e1-len));
+}
+
+const void *memintern(const void *data, size_t len)
+{
+ static struct hashmap map;
+ struct pool_entry key, *e;
+
+ /* initialize string pool hashmap */
+ if (!map.tablesize)
+ hashmap_init(map, (hashmap_cmp_fn) pool_entry_cmp, 0);
+
+ /* lookup interned string in pool */
+ hashmap_entry_init(key, memhash(data, len));
+ key.len = len;
+ e = hashmap_get(map, key, data);
+ if (!e) {
+ /* not found: create it */
+ e = xmallocz(sizeof(struct pool_entry) + len);
+ hashmap_entry_init(e, key.ent.hash);
+ e-len = len;
+ memcpy(e-data, data, len);
+ hashmap_add(map, e);
+ }
+ return e-data;
+}
diff --git a/hashmap.h b/hashmap.h
index 12f0668..507884b 100644
--- a/hashmap.h
+++ b/hashmap.h
@@ -87,4 +87,12 @@ static inline void *hashmap_iter_first(struct hashmap *map,
return hashmap_iter_next(iter);
}
+/* string interning */
+
+extern const void *memintern(const void *data, size_t len);
+static inline const char *strintern(const char *string)
+{
+ return memintern(string, strlen(string));
+}
+
#endif
diff --git a/t/t0011-hashmap.sh b/t/t0011-hashmap.sh
index 391e2b6..f97c805 100755
--- a/t/t0011-hashmap.sh
+++ b/t/t0011-hashmap.sh
@@ -237,4 +237,17 @@ test_expect_success 'grow / shrink' '
'
+test_expect_success 'string interning' '
+
+test_hashmap intern value1
+intern Value1
+intern value2
+intern value2
+ value1
+Value1
+value2
+value2
+
+'
+
test_done
diff --git a/test-hashmap.c b/test-hashmap.c
index 3c9f67b..07aa7ec 100644
--- a/test-hashmap.c
+++ b/test-hashmap.c
@@ -234,6 +234,20 @@ int main(int argc, char *argv[])
/* print table sizes */
printf(%u %u\n, map.tablesize, map.size);
+ } else if (!strcmp(intern, cmd) l1) {
+
+ /* test that strintern works */
+ const char *i1 = strintern(p1);
+ const char *i2 = strintern(p1);
+ if (strcmp(i1, p1))
+ printf(strintern(%s) returns %s\n, p1, i1);
+ else if (i1 == p1)
+ printf(strintern(%s) returns input pointer\n,
p1);
+ else if (i1 != i2)
+ printf(strintern(%s) != strintern(%s), i1,
i2);
+ else
+
[PATCH v1 3/4] hashmap: add simplified hashmap_get_from_hash() API
Hashmap entries are typically looked up by just a key. The hashmap_get()
API expects an initialized entry structure instead, to support compound
keys. This flexibility is currently only needed by find_dir_entry() in
name-hash.c (and compat/win32/fscache.c in the msysgit fork). All other
(currently five) call sites of hashmap_get() have to set up a near emtpy
entry structure, resulting in duplicate code like this:
struct hashmap_entry keyentry;
hashmap_entry_init(keyentry, hash(key));
return hashmap_get(map, keyentry, key);
Add a hashmap_get_from_hash() API that allows hashmap lookups by just
specifying the key and its hash code, i.e.:
return hashmap_get_from_hash(map, hash(key), key);
Signed-off-by: Karsten Blees bl...@dcon.de
---
Documentation/technical/api-hashmap.txt | 14 ++
builtin/describe.c | 4 +---
diffcore-rename.c | 7 +++
hashmap.h | 8
name-hash.c | 5 ++---
test-hashmap.c | 11 +++
6 files changed, 31 insertions(+), 18 deletions(-)
diff --git a/Documentation/technical/api-hashmap.txt
b/Documentation/technical/api-hashmap.txt
index dc21a7c..f9215d6 100644
--- a/Documentation/technical/api-hashmap.txt
+++ b/Documentation/technical/api-hashmap.txt
@@ -118,6 +118,20 @@ hashmap_entry) that has at least been initialized with the
proper hash code
If an entry with matching hash code is found, `key` and `keydata` are passed
to `hashmap_cmp_fn` to decide whether the entry matches the key.
+`void *hashmap_get_from_hash(const struct hashmap *map, unsigned int hash,
const void *keydata)`::
+
+ Returns the hashmap entry for the specified hash code and key data,
+ or NULL if not found.
++
+`map` is the hashmap structure.
++
+`hash` is the hash code of the entry to look up.
++
+If an entry with matching hash code is found, `keydata` is passed to
+`hashmap_cmp_fn` to decide whether the entry matches the key. The
+`entry_or_key` parameter points to a bogus hashmap_entry structure that
+should not be used in the comparison.
+
`void *hashmap_get_next(const struct hashmap *map, const void *entry)`::
Returns the next equal hashmap entry, or NULL if not found. This can be
diff --git a/builtin/describe.c b/builtin/describe.c
index 57e84c8..ee6a3b9 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -58,9 +58,7 @@ static int commit_name_cmp(const struct commit_name *cn1,
static inline struct commit_name *find_commit_name(const unsigned char *peeled)
{
- struct commit_name key;
- hashmap_entry_init(key, sha1hash(peeled));
- return hashmap_get(names, key, peeled);
+ return hashmap_get_from_hash(names, sha1hash(peeled), peeled);
}
static int replace_name(struct commit_name *e,
diff --git a/diffcore-rename.c b/diffcore-rename.c
index 6fa97d4..2e44a37 100644
--- a/diffcore-rename.c
+++ b/diffcore-rename.c
@@ -257,15 +257,14 @@ static int find_identical_files(struct hashmap *srcs,
int renames = 0;
struct diff_filespec *target = rename_dst[dst_index].two;
- struct file_similarity *p, *best, dst;
+ struct file_similarity *p, *best = NULL;
int i = 100, best_score = -1;
/*
* Find the best source match for specified destination.
*/
- best = NULL;
- hashmap_entry_init(dst, hash_filespec(target));
- for (p = hashmap_get(srcs, dst, NULL); p; p = hashmap_get_next(srcs,
p)) {
+ p = hashmap_get_from_hash(srcs, hash_filespec(target), NULL);
+ for (; p; p = hashmap_get_next(srcs, p)) {
int score;
struct diff_filespec *source = p-filespec;
diff --git a/hashmap.h b/hashmap.h
index ed5425a..12f0668 100644
--- a/hashmap.h
+++ b/hashmap.h
@@ -68,6 +68,14 @@ extern void *hashmap_put(struct hashmap *map, void *entry);
extern void *hashmap_remove(struct hashmap *map, const void *key,
const void *keydata);
+static inline void *hashmap_get_from_hash(const struct hashmap *map,
+ unsigned int hash, const void *keydata)
+{
+ struct hashmap_entry key;
+ hashmap_entry_init(key, hash);
+ return hashmap_get(map, key, keydata);
+}
+
/* hashmap_iter functions */
extern void hashmap_iter_init(struct hashmap *map, struct hashmap_iter *iter);
diff --git a/name-hash.c b/name-hash.c
index 49fd508..702cd05 100644
--- a/name-hash.c
+++ b/name-hash.c
@@ -213,12 +213,11 @@ struct cache_entry *index_dir_exists(struct index_state
*istate, const char *nam
struct cache_entry *index_file_exists(struct index_state *istate, const char
*name, int namelen, int icase)
{
struct cache_entry *ce;
- struct hashmap_entry key;
lazy_init_name_hash(istate);
- hashmap_entry_init(key, memihash(name, namelen));
- ce = hashmap_get(istate-name_hash, key, NULL);
+ ce = hashmap_get_from_hash(istate-name_hash,
+
Re: Experimental TDB support for GIT REFS
On Wed, Jul 2, 2014 at 10:11 AM, Ronnie Sahlberg sahlb...@google.com wrote: On Fri, Jun 27, 2014 at 5:37 PM, Shawn Pearce spea...@spearce.org wrote: On Fri, Jun 27, 2014 at 2:30 PM, Ronnie Sahlberg sahlb...@google.com wrote: List, One of my ref transaction aims is to make define a stable public API for accessing refs. Once this is done I want to make it possible to replace the current .git/refs/* model with a different type of backend. In my case I want to add support to store all refs and reflogs in a TDB database but once we have a pluggable backend framework for refs, if someone wants to store the refs in a SQL database, that should be fairly trivial too. There are a few series queued before this becomes possible, but is anyone wants to test or play with my git can use TDB database you can find an implementation of this at https://github.com/rsahlberg/git/tree/backend-struct-tdb Interesting! But the link 404s :( Yeah, sorry but there were issues :-( Issues solved now though and refactoring done. Please see https://github.com/rsahlberg/git/tree/backend-struct-db This branch adds a refs backend that communicates via a domain socket to a refs daemon. The refs daemon in turn interfaces with the actual database. My branch contains one hack refs daemon that uses a TDB database for the refs storage. This daemon is only about 600 lines of code, most of which is marshalling and socket handling and only a small amount of TDB specific code. This small daemon should make it easy for folks to add arbitrary database interfaces easily. Taking refsd-tdb.c and modifying it to instead attach to a SQL database should only take a few hundred lines of changes or so. To build the daemon and start it : $ gcc refsd-tdb.c -o refsd-tdb -ltdb create /var/lib/git and /var/log/git $ ./refsd-tdb /var/lib/git/refs.socket /var/lib/git /var/log/git/refsd.log Then you can inspect the database with tdbdump /var/lib/git/refs.tdb You can then create a repository that uses this database : $ git init --db-repo-name=ROCC --db-socket=/var/lib/git/refs.socket . Then further commands will operate on the refs tdb. See this as a prototype to experiment with and see the direction of the refs transaction and pluggable backend support. There is a lot additional work and cleanup that needs to be done before this will become production code. (yes I know we should not do hand marshalling and unmarshalling for the PDUs on the domain socket. We should use some lightweight encoding library like XDR and rpcgen or similar.) Please test, comment and have fun. One enhancement to this could be to make it easier to use for trivial users that do not want to share one database across multiple repos. We could have something like $ git init --db=tdb . core.db=tdb could then mean that instead of connecting to a domain socket for a shared daemon instead it would just popen(refsd-core.db ..., rw) and automaticallt set the asguments ro refsd-tdb to point to the local .git directory. This would make the setup simpler for the trivial usecase since it would avoid having to manuallt initializing a dedicated daemon before the git commands will start working. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Race condition in git push --mirror can cause silent ref rewinding
On 07/02/2014 06:20 PM, Junio C Hamano wrote: Alex Vandiver a...@chmrr.net writes: [remote github] url = g...@github.com:bestpractical/rt.git fetch = +refs/*:refs/* mirror = yes git push github master^:master must stay a usable way to update the published repository to an arbitrary commit, so if set to mirror, do not pretend that a fetch in reverse has happened during 'git push' will not be a solution to this issue. Hm? I'm confused, as mirror isn't compatible with refspecs: $ git push github master^:master error: --mirror can't be combined with refspecs Perhaps removing remote.github.fetch would be one sane way forward. Ahh -- I see. The repository predates a9f5a355, which split `git remote add --mirror` into `--mirror=push` and `--mirror=fetch`, because of more or less this exact problem. Of course, there is nothing much that can be done for existing repositories in this situation as it's a legitimate combination. - Alex -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Support for EBCDIC
Is Git supposed to be usable in an environment where the execution character set is EBCDIC? I ask because, in browsing the source code (version 2.0.0), I stumbled across three functions that won't work as presumably intended in an EBCDIC environment (strihash(), memihash(), and git_user_agent_sanitized()). I can report them as bugs, but if EBCDIC is considered out of scope, then they aren't bugs. These three functions can be readily fixed to make them portable across character sets. There may be other spots that are harder to fix. I have done a lot of grepping and Googling, but I haven't found a clear, authoritative answer to this question. From searching this mailing list, it appears that nobody is interested in supporting EBCDIC. However I found one wiki page describing how to run Git on an IBM i, which is an EBCDIC-based successor to the AS/400 series. See: http://wsip-174-79-32-155.ph.ph.cox.net/wiki/index.php/PASE/Git That installation was reportedly running version 1.7.9.4, which I believe predates the introduction of strihash() and memihash(); I don't know about git_user_agent_sanitized(). Mind you, I'm not advocating for EBCDIC. I escaped from the EBCDIC world about fifteen years ago, and have no desire to return. I just want to know if character set issues are worth reporting. The same issues may arise for other, more obscure character sets. Scott McKellar -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] git-merge-file: do not add LF at EOF while applying unrelated change
On Wed, Jul 02, 2014 at 04:08:28PM +0200, Johannes Schindelin wrote: What could be improved with them? Oh, I would name the files more appropriately, for example. That is, instead of test1.txt I would call it mixed-endings.txt or lf-only.txt or some such. And instead of the Latin version of Psalm 23, I would put lines into the files that describe their own role in the test, i.e. unchanged ends with a carriage return ends with a line feed unchanged or similar. Please keep in mind that this critique is most likely on my *own* work, for all I know *I* introduced those files. I asked to have something in mind if I return to this. Thanks for the notes. -- Max -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/14] Add submodule test harness
On 07/02/2014 09:57 PM, Jens Lehmann wrote: Am 02.07.2014 16:54, schrieb Torsten Bögershausen: (Not sure if this is the right thread) (I haven't checked if this is fixed in your latest version) Yes, this is the right thread and no, it isn't fixed yet. On what I have on pu 7a0da7902cbbc9a876b90c9, Tue Jul 1 14:51:53 2014 -0700 Many submodule tests are broken. One problem is here: lib-submodule-update.sh:264: possible problem: echo -n is not portable (please use printf): echo -n sub1 lib-submodule-update.sh:507: possible problem: echo -n is not portable (please use printf): echo -n sub1 You can remove the empty echo -n to create an empty file: sub1 Thanks for spotting and diagnosing this. Running make lint in the test directory only feeds the tests to check-non-portable-shell.pl, but not the *lib*.sh helper scripts, which made me miss this one. The following diff should fix it for you. Am I understanding you correctly that you are experiencing other failures too? I see no other incompatibilities when running ./check-non-portable-shell.pl on all the shell scripts in the repo. The longer story is that I run the test suite once a week or so. Most often under Mac OS, sometimes cygwin or Linux. Whenever there is a breakage under Mac OS which I can not debug within some minutes, I run it under Linux to see if there is the same breakage. The ./check-non-portable-shell.pl can sometimes give an indication why some test fail. You can run it from command line: ./check-non-portable-shell.pl *.sh and it will find the echo -n which I reported. On the longer run it could probably check all *.sh files, not only the ones under t/ I do not have the time to test the snipped patch below, but I can check pu when the next round of your patch is in and give you some more info. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
