* [PATCH] use oid_to_hex_r() for converting struct object_id hashes to hex strings
From: René Scharfe @ 2017-01-28 22:03 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, brian m. carlson
Patch generated by Coccinelle and contrib/coccinelle/object_id.cocci.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
builtin/blame.c | 4 ++--
builtin/merge-index.c | 2 +-
builtin/rev-list.c | 2 +-
diff.c | 2 +-
4 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/builtin/blame.c b/builtin/blame.c
index 126b8c9e5b..cffc626540 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1901,7 +1901,7 @@ static void emit_porcelain(struct scoreboard *sb, struct blame_entry *ent,
struct origin *suspect = ent->suspect;
char hex[GIT_SHA1_HEXSZ + 1];
- sha1_to_hex_r(hex, suspect->commit->object.oid.hash);
+ oid_to_hex_r(hex, &suspect->commit->object.oid);
printf("%s %d %d %d\n",
hex,
ent->s_lno + 1,
@@ -1941,7 +1941,7 @@ static void emit_other(struct scoreboard *sb, struct blame_entry *ent, int opt)
int show_raw_time = !!(opt & OUTPUT_RAW_TIMESTAMP);
get_commit_info(suspect->commit, &ci, 1);
- sha1_to_hex_r(hex, suspect->commit->object.oid.hash);
+ oid_to_hex_r(hex, &suspect->commit->object.oid);
cp = nth_line(sb, ent->lno);
for (cnt = 0; cnt < ent->num_lines; cnt++) {
diff --git a/builtin/merge-index.c b/builtin/merge-index.c
index ce356b1da1..2d1b6db6bd 100644
--- a/builtin/merge-index.c
+++ b/builtin/merge-index.c
@@ -22,7 +22,7 @@ static int merge_entry(int pos, const char *path)
if (strcmp(ce->name, path))
break;
found++;
- sha1_to_hex_r(hexbuf[stage], ce->oid.hash);
+ oid_to_hex_r(hexbuf[stage], &ce->oid);
xsnprintf(ownbuf[stage], sizeof(ownbuf[stage]), "%o", ce->ce_mode);
arguments[stage] = hexbuf[stage];
arguments[stage + 4] = ownbuf[stage];
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index c43decda70..0aa93d5891 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -237,7 +237,7 @@ static int show_bisect_vars(struct rev_list_info *info, int reaches, int all)
cnt = reaches;
if (revs->commits)
- sha1_to_hex_r(hex, revs->commits->item->object.oid.hash);
+ oid_to_hex_r(hex, &revs->commits->item->object.oid);
if (flags & BISECT_SHOW_ALL) {
traverse_commit_list(revs, show_commit, show_object, info);
diff --git a/diff.c b/diff.c
index f08cd8e033..d91a344908 100644
--- a/diff.c
+++ b/diff.c
@@ -3015,7 +3015,7 @@ static struct diff_tempfile *prepare_temp_file(const char *name,
if (!one->oid_valid)
sha1_to_hex_r(temp->hex, null_sha1);
else
- sha1_to_hex_r(temp->hex, one->oid.hash);
+ oid_to_hex_r(temp->hex, &one->oid);
/* Even though we may sometimes borrow the
* contents from the work tree, we always want
* one->mode. mode is trustworthy even when
--
2.11.0
^ permalink raw reply related
* Re: [PATCH 1/3] Documentation/stash: remove mention of git reset --hard
From: Jeff King @ 2017-01-28 23:54 UTC (permalink / raw)
To: Thomas Gummerer
Cc: Junio C Hamano, git, Stephan Beyer, Marc Strapetz,
Johannes Schindelin
In-Reply-To: <20170128192958.GB31189@hank>
On Sat, Jan 28, 2017 at 07:30:28PM +0000, Thomas Gummerer wrote:
> Thanks all who chimed in here. My new description is definitely not
> right. The reason I wanted to change it is part because it's an
> implementation detail, and part because it's going to be not quite
> right when the filename argument is introduced.
>
> How about the following:
>
> Save your local modifications to a new 'stash' and roll them back
> both in the working tree and in the index.
>
> As an added bonus this also matches what git stash save -p does.
IMHO that is both informative and accurate. You could add:
(unless --keep-index was used)
at the end of the sentence, though I am not sure it is necessary.
-Peff
^ permalink raw reply
* Re: [PATCH v3 16/27] attr: convert git_all_attrs() to use "struct attr_check"
From: Stefan Beller @ 2017-01-28 23:50 UTC (permalink / raw)
To: Brandon Williams; +Cc: git@vger.kernel.org, Junio C Hamano, Duy Nguyen
In-Reply-To: <20170128020207.179015-17-bmwill@google.com>
On Fri, Jan 27, 2017 at 6:01 PM, Brandon Williams <bmwill@google.com> wrote:
> From: Junio C Hamano <gitster@pobox.com>
>
> This updates the other two ways the attribute check is done via an
> array of "struct attr_check_item" elements. These two niches
> appear only in "git check-attr".
>
> * The caller does not know offhand what attributes it wants to ask
> about and cannot use attr_check_initl() to prepare the
> attr_check structure.
>
> * The caller may not know what attributes it wants to ask at all,
> and instead wants to learn everything that the given path has.
>
> Such a caller can call attr_check_alloc() to allocate an empty
> attr_check, and then call attr_check_append() to add attribute names
> one by one.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> Signed-off-by: Brandon Williams <bmwill@google.com>
> ---
> attr.c | 168 ++++++++++++++++++++++++---------------------------
> attr.h | 9 +--
> builtin/check-attr.c | 60 +++++++++---------
> 3 files changed, 112 insertions(+), 125 deletions(-)
>
> diff --git a/attr.c b/attr.c
> index de8bf35a3..40818246f 100644
> --- a/attr.c
> +++ b/attr.c
> @@ -132,75 +132,6 @@ struct git_attr *git_attr(const char *name)
> return git_attr_internal(name, strlen(name));
> }
>
> -struct attr_check *attr_check_alloc(void)
> -{
> - return xcalloc(1, sizeof(struct attr_check));
> -}
> -
> -struct attr_check *attr_check_initl(const char *one, ...)
> -{
> - struct attr_check *check;
> - int cnt;
> - va_list params;
> - const char *param;
> -
> - va_start(params, one);
> - for (cnt = 1; (param = va_arg(params, const char *)) != NULL; cnt++)
> - ;
> - va_end(params);
> -
> - check = attr_check_alloc();
> - check->nr = cnt;
> - check->alloc = cnt;
> - check->items = xcalloc(cnt, sizeof(struct attr_check_item));
> -
> - check->items[0].attr = git_attr(one);
> - va_start(params, one);
> - for (cnt = 1; cnt < check->nr; cnt++) {
> - const struct git_attr *attr;
> - param = va_arg(params, const char *);
> - if (!param)
> - die("BUG: counted %d != ended at %d",
> - check->nr, cnt);
> - attr = git_attr(param);
> - if (!attr)
> - die("BUG: %s: not a valid attribute name", param);
> - check->items[cnt].attr = attr;
> - }
> - va_end(params);
> - return check;
> -}
This being moved down to below (being review churn) sounds like a
rebase mistake. ;)
> -
> -struct attr_check_item *attr_check_append(struct attr_check *check,
> - const struct git_attr *attr)
> -{
> - struct attr_check_item *item;
> -
> - ALLOC_GROW(check->items, check->nr + 1, check->alloc);
> - item = &check->items[check->nr++];
> - item->attr = attr;
> - return item;
> -}
> -
> -void attr_check_reset(struct attr_check *check)
> -{
> - check->nr = 0;
> -}
> -
> -void attr_check_clear(struct attr_check *check)
> -{
> - free(check->items);
> - check->items = NULL;
> - check->alloc = 0;
> - check->nr = 0;
> -}
> -
> -void attr_check_free(struct attr_check *check)
> -{
> - attr_check_clear(check);
> - free(check);
> -}
> -
> /* What does a matched pattern decide? */
> struct attr_state {
> struct git_attr *attr;
> @@ -439,6 +370,75 @@ static void free_attr_elem(struct attr_stack *e)
> free(e);
> }
>
> +struct attr_check *attr_check_alloc(void)
> +{
> + return xcalloc(1, sizeof(struct attr_check));
> +}
> +
> +struct attr_check *attr_check_initl(const char *one, ...)
> +{
> + struct attr_check *check;
> + int cnt;
> + va_list params;
> + const char *param;
> +
> + va_start(params, one);
> + for (cnt = 1; (param = va_arg(params, const char *)) != NULL; cnt++)
> + ;
> + va_end(params);
> +
> + check = attr_check_alloc();
> + check->nr = cnt;
> + check->alloc = cnt;
> + check->items = xcalloc(cnt, sizeof(struct attr_check_item));
> +
> + check->items[0].attr = git_attr(one);
> + va_start(params, one);
> + for (cnt = 1; cnt < check->nr; cnt++) {
> + const struct git_attr *attr;
> + param = va_arg(params, const char *);
> + if (!param)
> + die("BUG: counted %d != ended at %d",
> + check->nr, cnt);
> + attr = git_attr(param);
> + if (!attr)
> + die("BUG: %s: not a valid attribute name", param);
> + check->items[cnt].attr = attr;
> + }
> + va_end(params);
> + return check;
> +}
> +
> +struct attr_check_item *attr_check_append(struct attr_check *check,
> + const struct git_attr *attr)
> +{
> + struct attr_check_item *item;
> +
> + ALLOC_GROW(check->items, check->nr + 1, check->alloc);
> + item = &check->items[check->nr++];
> + item->attr = attr;
> + return item;
> +}
> +
> +void attr_check_reset(struct attr_check *check)
> +{
> + check->nr = 0;
> +}
> +
> +void attr_check_clear(struct attr_check *check)
> +{
> + free(check->items);
> + check->items = NULL;
> + check->alloc = 0;
> + check->nr = 0;
> +}
> +
> +void attr_check_free(struct attr_check *check)
> +{
> + attr_check_clear(check);
> + free(check);
> +}
> +
> static const char *builtin_attr[] = {
> "[attr]binary -diff -merge -text",
> NULL,
> @@ -906,32 +906,22 @@ int git_check_attrs(const char *path, int num, struct attr_check_item *check)
> return 0;
> }
Below is where the actual change for this patch starts?
>
> -int git_all_attrs(const char *path, int *num, struct attr_check_item **check)
> +void git_all_attrs(const char *path, struct attr_check *check)
> {
> - int i, count, j;
> + int i;
>
> - collect_some_attrs(path, 0, NULL);
> + attr_check_reset(check);
> + collect_some_attrs(path, check->nr, check->items);
>
> - /* Count the number of attributes that are set. */
> - count = 0;
> - for (i = 0; i < attr_nr; i++) {
> - const char *value = check_all_attr[i].value;
> - if (value != ATTR__UNSET && value != ATTR__UNKNOWN)
> - ++count;
> - }
> - *num = count;
> - ALLOC_ARRAY(*check, count);
> - j = 0;
> for (i = 0; i < attr_nr; i++) {
> + const char *name = check_all_attr[i].attr->name;
> const char *value = check_all_attr[i].value;
> - if (value != ATTR__UNSET && value != ATTR__UNKNOWN) {
> - (*check)[j].attr = check_all_attr[i].attr;
> - (*check)[j].value = value;
> - ++j;
> - }
> + struct attr_check_item *item;
> + if (value == ATTR__UNSET || value == ATTR__UNKNOWN)
> + continue;
> + item = attr_check_append(check, git_attr(name));
> + item->value = value;
> }
> -
> - return 0;
> }
>
> int git_check_attr(const char *path, struct attr_check *check)
> diff --git a/attr.h b/attr.h
> index e611b139a..9f2729842 100644
> --- a/attr.h
> +++ b/attr.h
> @@ -56,13 +56,10 @@ int git_check_attrs(const char *path, int, struct attr_check_item *);
> extern int git_check_attr(const char *path, struct attr_check *check);
>
> /*
> - * Retrieve all attributes that apply to the specified path. *num
> - * will be set to the number of attributes on the path; **check will
> - * be set to point at a newly-allocated array of git_attr_check
> - * objects describing the attributes and their values. *check must be
> - * free()ed by the caller.
> + * Retrieve all attributes that apply to the specified path.
> + * check holds the attributes and their values.
> */
> -int git_all_attrs(const char *path, int *num, struct attr_check_item **check);
> +extern void git_all_attrs(const char *path, struct attr_check *check);
>
> enum git_attr_direction {
> GIT_ATTR_CHECKIN,
> diff --git a/builtin/check-attr.c b/builtin/check-attr.c
> index 889264a5b..40cdff13e 100644
> --- a/builtin/check-attr.c
> +++ b/builtin/check-attr.c
> @@ -24,12 +24,13 @@ static const struct option check_attr_options[] = {
> OPT_END()
> };
>
> -static void output_attr(int cnt, struct attr_check_item *check,
> - const char *file)
> +static void output_attr(struct attr_check *check, const char *file)
> {
> int j;
> + int cnt = check->nr;
> +
> for (j = 0; j < cnt; j++) {
> - const char *value = check[j].value;
> + const char *value = check->items[j].value;
>
> if (ATTR_TRUE(value))
> value = "set";
> @@ -42,36 +43,38 @@ static void output_attr(int cnt, struct attr_check_item *check,
> printf("%s%c" /* path */
> "%s%c" /* attrname */
> "%s%c" /* attrvalue */,
> - file, 0, git_attr_name(check[j].attr), 0, value, 0);
> + file, 0,
> + git_attr_name(check->items[j].attr), 0, value, 0);
> } else {
> quote_c_style(file, NULL, stdout, 0);
> - printf(": %s: %s\n", git_attr_name(check[j].attr), value);
> + printf(": %s: %s\n",
> + git_attr_name(check->items[j].attr), value);
> }
> -
> }
> }
>
> static void check_attr(const char *prefix,
> - int cnt, struct attr_check_item *check,
> + struct attr_check *check,
> + int collect_all,
> const char *file)
> {
> char *full_path =
> prefix_path(prefix, prefix ? strlen(prefix) : 0, file);
> - if (check != NULL) {
> - if (git_check_attrs(full_path, cnt, check))
> - die("git_check_attrs died");
> - output_attr(cnt, check, file);
> +
> + if (collect_all) {
> + git_all_attrs(full_path, check);
> } else {
> - if (git_all_attrs(full_path, &cnt, &check))
> - die("git_all_attrs died");
> - output_attr(cnt, check, file);
> - free(check);
> + if (git_check_attr(full_path, check))
> + die("git_check_attr died");
> }
> + output_attr(check, file);
> +
> free(full_path);
> }
>
> static void check_attr_stdin_paths(const char *prefix,
> - int cnt, struct attr_check_item *check)
> + struct attr_check *check,
> + int collect_all)
> {
> struct strbuf buf = STRBUF_INIT;
> struct strbuf unquoted = STRBUF_INIT;
> @@ -85,7 +88,7 @@ static void check_attr_stdin_paths(const char *prefix,
> die("line is badly quoted");
> strbuf_swap(&buf, &unquoted);
> }
> - check_attr(prefix, cnt, check, buf.buf);
> + check_attr(prefix, check, collect_all, buf.buf);
> maybe_flush_or_die(stdout, "attribute to stdout");
> }
> strbuf_release(&buf);
> @@ -100,7 +103,7 @@ static NORETURN void error_with_usage(const char *msg)
>
> int cmd_check_attr(int argc, const char **argv, const char *prefix)
> {
> - struct attr_check_item *check;
> + struct attr_check *check;
> int cnt, i, doubledash, filei;
>
> if (!is_bare_repository())
> @@ -160,28 +163,25 @@ int cmd_check_attr(int argc, const char **argv, const char *prefix)
> error_with_usage("No file specified");
> }
>
> - if (all_attrs) {
> - check = NULL;
> - } else {
> - check = xcalloc(cnt, sizeof(*check));
> + check = attr_check_alloc();
> + if (!all_attrs) {
> for (i = 0; i < cnt; i++) {
> - const char *name;
> - struct git_attr *a;
> - name = argv[i];
> - a = git_attr(name);
> + struct git_attr *a = git_attr(argv[i]);
> if (!a)
> return error("%s: not a valid attribute name",
> - name);
> - check[i].attr = a;
> + argv[i]);
> + attr_check_append(check, a);
> }
> }
>
> if (stdin_paths)
> - check_attr_stdin_paths(prefix, cnt, check);
> + check_attr_stdin_paths(prefix, check, all_attrs);
> else {
> for (i = filei; i < argc; i++)
> - check_attr(prefix, cnt, check, argv[i]);
> + check_attr(prefix, check, all_attrs, argv[i]);
> maybe_flush_or_die(stdout, "attribute to stdout");
> }
> +
> + attr_check_free(check);
> return 0;
> }
> --
> 2.11.0.483.g087da7b7c-goog
>
^ permalink raw reply
* git-daemon shallow checkout fail
From: Bob Proulx @ 2017-01-29 0:29 UTC (permalink / raw)
To: git
I am trying to understand a problem with shallow checkouts through the
git-daemon. The server side fails trying to create a shallow_XXXXXX
file in the repository. But of course it can't due to no permissions
from the git-daemon user.
However the problem driving me crazy is that this only fails this way
on one machine. Unfortunately failing on the machine I need to use.
If I try this same setup on any other machine I try then there is no
failure and it works okay. Therefore I conclude that in the failing
case it is trying to write a shallow_XXXXXX file in the repository but
in all of the passing cases it does not. I browsed through the
git-daemon source but couldn't deduce the flow yet.
Does anyone know why one system would try to create shallow_XXXXXX
files in the repository while another one would not?
Trying to be unambiguous here is my test case:
mkdir /run/git
chmod 755 /run/git
chown nobody:root /run/git
cd /run/git && env -i HOME=/run/git PATH=/usr/local/bin:/usr/bin:/bin su -s /bin/sh -c "git-daemon --export-all --base-path=/srv/git --verbose" nobody
[18340] Ready to rumble
That sets up the test case. Have any bare git repository in /srv/git
for use for cloning.
git clone --depth 1 git://localhost/test-project.git
Cloning into 'test-project'...
fatal: The remote end hung up unexpectedly
fatal: early EOF
fatal: index-pack failed
And the server side says:
[26071] Request upload-pack for '/test-project.git'
[26071] fatal: Unable to create temporary file '/srv/git/test-project.git/shallow_xKwnvZ': Permission denied
[26055] [26071] Disconnected (with error)
Of course git-daemon running as nobody can't create a temporary file
shallow_XXXXXX in the /srv/git/test-project.git because it has no
permissions by design. But why does this work on other systems and
not work on my target system?
git --version # from today's git clone build
git version 2.11.0.485.g4e59582
Thanks!
Bob
^ permalink raw reply
* Re: [PATCH v3 16/27] attr: convert git_all_attrs() to use "struct attr_check"
From: Brandon Williams @ 2017-01-29 2:44 UTC (permalink / raw)
To: Stefan Beller; +Cc: git@vger.kernel.org, Junio C Hamano, Duy Nguyen
In-Reply-To: <CAGZ79kax8Q6bD1Xm-kGAK=yTG6TYWcR3SCOjN9BeX9rzwkyaig@mail.gmail.com>
On 01/28, Stefan Beller wrote:
>
> This being moved down to below (being review churn) sounds like a
> rebase mistake. ;)
>
Yep, thanks for catching that. I'll need to fix that up.
--
Brandon Williams
^ permalink raw reply
* Re: [PATCH 2/3] stash: introduce push verb
From: Thomas Gummerer @ 2017-01-29 12:39 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Stephan Beyer, Marc Strapetz, Jeff King, Johannes Schindelin
In-Reply-To: <xmqqziihskbi.fsf@gitster.mtv.corp.google.com>
On 01/23, Junio C Hamano wrote:
> Thomas Gummerer <t.gummerer@gmail.com> writes:
>
> > + stash_msg="$*"
> > +
> > + if test -z stash_msg
>
> A dollar-sign is missing here, I think.
Yes, thanks.
> > + then
> > + push_stash $push_options
> > + else
> > + push_stash $push_options -m "$stash_msg"
> > + fi
> > +}
--
Thomas
^ permalink raw reply
* [PATCH] receive-pack: call string_list_clear() unconditionally
From: René Scharfe @ 2017-01-29 13:09 UTC (permalink / raw)
To: Git List; +Cc: Stefan Beller, Junio C Hamano
string_list_clear() handles empty lists just fine, so remove the
redundant check.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
builtin/receive-pack.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 6b97cbdbe..1dbb8a069 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -1942,8 +1942,7 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
run_receive_hook(commands, "post-receive", 1,
&push_options);
run_update_post_hook(commands);
- if (push_options.nr)
- string_list_clear(&push_options, 0);
+ string_list_clear(&push_options, 0);
if (auto_gc) {
const char *argv_gc_auto[] = {
"gc", "--auto", "--quiet", NULL,
--
2.11.0
^ permalink raw reply related
* Re: [PATCH 3/3] stash: support filename argument
From: Thomas Gummerer @ 2017-01-29 13:29 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Stephan Beyer, Marc Strapetz, Jeff King, Johannes Schindelin
In-Reply-To: <xmqqinp5sj98.fsf@gitster.mtv.corp.google.com>
On 01/23, Junio C Hamano wrote:
> Thomas Gummerer <t.gummerer@gmail.com> writes:
>
> > diff --git a/git-stash.sh b/git-stash.sh
> > index d6b4ae3290..7dcce629bd 100755
> > --- a/git-stash.sh
> > +++ b/git-stash.sh
> > @@ -41,7 +41,7 @@ no_changes () {
> > untracked_files () {
> > excl_opt=--exclude-standard
> > test "$untracked" = "all" && excl_opt=
> > - git ls-files -o -z $excl_opt
> > + git ls-files -o -z $excl_opt -- $1
>
> Does $1 need to be quoted to prevent it from split at $IFS?
Not sure, I'll check and add a test for the re-roll.
> > @@ -56,6 +56,23 @@ clear_stash () {
> > }
> >
> > create_stash () {
> > + files=
> > + while test $# != 0
> > + do
> > + case "$1" in
> > + --)
> > + shift
> > + break
> > + ;;
> > + --files)
> > + ;;
> > + *)
> > + files="$1 $files"
> > + ;;
>
> Hmph. What is this "no-op" option about? Did you mean to say
> something like this instead?
>
> case "$1" in
> ...
> --file)
> case $# in
> 1)
> die "--file needs a pathspec" ;;
> *)
> shift
> files="$files$1 " ;;
> esac
> ;;
>
Hmm that would require multiple --file arguments to create_stash,
which I wanted to avoid. But probably the correct solution is to
introduce a new format for create_stash, which allows a -m before the
message, and uses -- to disambiguate before the file name arguments.
This would be similar to what Johannes suggested in [1], deprecating
the old syntax in git stash create. While this didn't work in git
stash save, it would work in git stash create, as "--" isn't used to
disambiguate anything there yet.
> Another thing I noticed. We won't support filenames with embedded
> $IFS characters at all?
>
> I somehow had an impression that the script was carefully done
> (e.g. by using -z option where appropriate) to add such a
> limitation.
>
> Perhaps we have broken it over time and it no longer matters
> (i.e. there already may be existing breakages), but this troubles
> me somehow.
Good point, I didn't think about $IFS characters. Fill fix in the
next round.
> By the way, in addition to "push" thing that corrects the argument
> convention by requiring "-m" before the message, we need to correct
> create_stash that is used internally from "stash push" somehow?
Yeah, I think that would make sense, see above.
[1]: http://public-inbox.org/git/alpine.DEB.2.20.1701241148300.3469@virtualbox/
^ permalink raw reply
* Re: [PATCH v2] git-p4: Fix git-p4.mapUser on Windows
From: Luke Diamand @ 2017-01-29 17:10 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Lars Schneider, Git Users, George Vanburgh
In-Reply-To: <xmqqa8ac1k7o.fsf@gitster.mtv.corp.google.com>
On 27 January 2017 at 17:33, Junio C Hamano <gitster@pobox.com> wrote:
> George Vanburgh <george@vanburgh.me> writes:
>
>> From: George Vanburgh <gvanburgh@bloomberg.net>
>>
>> When running git-p4 on Windows, with multiple git-p4.mapUser entries in
>> git config - no user mappings are applied to the generated repository.
>> ...
>> Using splitlines solves this issue, by splitting config on all
>> typical delimiters ('\n', '\r\n' etc.)
>
> Luke, Lars, this version seems to be in line with the conclusion of
> your earlier reviews, e.g.
>
> <CAE5ih7_+Vc9oqKdjo8h2vgZPup4pto9wd=sBb=W6hCs4tuW2Jg@mail.gmail.com>
>
> Even though it looks OK to my eyes, I'll wait for Acks or further
> refinement suggestions from either of you two before acting on this
> patch.
It looks good to me. The tests all pass, and the change looks correct.
Ack.
Luke
>
> Thanks.
>
>> Signed-off-by: George Vanburgh <gvanburgh@bloomberg.net>
>> ---
>> git-p4.py | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/git-p4.py b/git-p4.py
>> index f427bf6..b66f68b 100755
>> --- a/git-p4.py
>> +++ b/git-p4.py
>> @@ -656,7 +656,7 @@ def gitConfigInt(key):
>> def gitConfigList(key):
>> if not _gitConfig.has_key(key):
>> s = read_pipe(["git", "config", "--get-all", key], ignore_error=True)
>> - _gitConfig[key] = s.strip().split(os.linesep)
>> + _gitConfig[key] = s.strip().splitlines()
>> if _gitConfig[key] == ['']:
>> _gitConfig[key] = []
>> return _gitConfig[key]
>>
>> --
>> https://github.com/git/git/pull/319
^ permalink raw reply
* [RFC] Proof of concept: Support multiple authors
From: Cornelius Schumacher @ 2017-01-29 18:06 UTC (permalink / raw)
To: git; +Cc: josh
This patch is a proof of concept implementation of support for
multiple authors. It adds an optional `authors` header to commits
which is set when there are authors configured in the git config.
A new command `git-authors` is used to manage the authors settings.
Authors are identified by initials and their names and emails are
set in a `.git_authors_map` file.
Signed-off-by: Cornelius Schumacher <schumacher@kde.org>
---
When doing pair programming we have to work around the limitation that
git can only have a single author in each commit. There are some tools
which help with that such as [git-duet] [1], but there are still some
limits, because the information about multiple authors is not reflected
in the native git data model.
Here is a proposal how to change that and implement native support for
multiple authors in git. It comes with a patch as a proof of concept.
The patch by no means is finished, it doesn't cover all cases and needs
more tests and error handling. It's meant as an illustration of the
concept.
The basic idea is to introduce a new optional `authors` header in
commits which contains a list of authors. The header is set in each new
commit when there is an entry `authors.current` in the git config listing
the current authors. When this config is not there the behavior falls
back to the current standard behavior.
When the header is there it is treated in the same way as the author
header. It's preserved on merges and similar operations, is displayed in
git show, and used to create a list of `From` addresses in `format-patch`.
Email supports multiple `From` addresses as specified in section 3.6.2 of
RFC 5322.
When multiple authors are configured, they still write the standard author
header to keep backwards compatibility. The first author is used as author
and committer. In the future it might be good to implement something like
automatic rotation of the order of authors to give credit in a fair way.
To make it easier to work with the authors there is a new command
`git-authors`. It sets the list of authors using initials as shortcut for
the full configuration with name and email. The mapping of initials to
names and email addresses is taken from a file `.git_authors_map` in the
home directory of the users. This way it's possible to quickly set a list
of authors by running a command such as `git authors ab cd`. This is
useful when doing pair programming because the people working together
usually switch quite frequently and using the command with the intials is
quicker and less error-prone than editing the configuration with full
names and emails.
The command also supports setting a single author, setting more than two
authors or clearing the configuration for multiple authors to go back to
the standard behavior without the new authors header.
The concept of the command and the mappings file is similar to what
git-duet does, so that it should be familiar to many people doing pair
programming. The behavior of git doesn't change when the new feature is
not used and when it's used it should be backwards compatible so that it
doesn't break existing functionality. This should make a smooth transition
for users who choose to make use of it.
Adding support for multiple authors would make the life of developers doing
pair programming easier. It would be useful in itself, but it would also
need support by other tools around git to use its full potential. This
might take a while, but I think it's worth the effort.
I'm willing to continue to work on this and create a patch which is suitable
for inclusion in git.
What do you think?
[1]: https://github.com/git-duet/git-duet
.gitignore | 1 +
Makefile | 3 +
authors.c | 205 ++++++++++++++++++++++++++++++++++++++++++++++++
authors.h | 29 +++++++
builtin.h | 1 +
builtin/am.c | 18 ++++-
builtin/authors.c | 82 +++++++++++++++++++
builtin/commit-tree.c | 2 +-
builtin/commit.c | 56 +++++++++++--
builtin/merge.c | 4 +-
cache.h | 1 +
commit.c | 17 ++--
commit.h | 6 +-
git.c | 1 +
ident.c | 2 +-
mailinfo.c | 17 ++++
mailinfo.h | 2 +
notes-cache.c | 2 +-
notes-utils.c | 2 +-
pretty.c | 79 ++++++++++++++++++-
t/helper/.gitignore | 1 +
t/helper/test-authors.c | 42 ++++++++++
t/t9904-authors.sh | 32 ++++++++
23 files changed, 581 insertions(+), 24 deletions(-)
create mode 100644 authors.c
create mode 100644 authors.h
create mode 100644 builtin/authors.c
create mode 100644 t/helper/test-authors.c
create mode 100755 t/t9904-authors.sh
diff --git a/.gitignore b/.gitignore
index 6722f78..1323907 100644
--- a/.gitignore
+++ b/.gitignore
@@ -16,6 +16,7 @@
/git-apply
/git-archimport
/git-archive
+/git-authors
/git-bisect
/git-bisect--helper
/git-blame
diff --git a/Makefile b/Makefile
index 27afd0f..22f4b28 100644
--- a/Makefile
+++ b/Makefile
@@ -597,6 +597,7 @@ X =
PROGRAMS += $(patsubst %.o,git-%$X,$(PROGRAM_OBJS))
+TEST_PROGRAMS_NEED_X += test-authors
TEST_PROGRAMS_NEED_X += test-chmtime
TEST_PROGRAMS_NEED_X += test-ctype
TEST_PROGRAMS_NEED_X += test-config
@@ -700,6 +701,7 @@ LIB_OBJS += archive-tar.o
LIB_OBJS += archive-zip.o
LIB_OBJS += argv-array.o
LIB_OBJS += attr.o
+LIB_OBJS += authors.o
LIB_OBJS += base85.o
LIB_OBJS += bisect.o
LIB_OBJS += blob.o
@@ -859,6 +861,7 @@ BUILTIN_OBJS += builtin/am.o
BUILTIN_OBJS += builtin/annotate.o
BUILTIN_OBJS += builtin/apply.o
BUILTIN_OBJS += builtin/archive.o
+BUILTIN_OBJS += builtin/authors.o
BUILTIN_OBJS += builtin/bisect--helper.o
BUILTIN_OBJS += builtin/blame.o
BUILTIN_OBJS += builtin/branch.o
diff --git a/authors.c b/authors.c
new file mode 100644
index 0000000..ba782a0
--- /dev/null
+++ b/authors.c
@@ -0,0 +1,205 @@
+#include "cache.h"
+#include "authors.h"
+#include "string.h"
+#include "strbuf.h"
+
+/*
+ * given an authors line, split the fields
+ * to allow the caller to parse it.
+ * Signal a success by returning 0.
+ */
+int split_authors_line(struct authors_split *split, const char *line, int len)
+{
+ const char *cp;
+
+ memset(split, 0, sizeof(*split));
+
+ split->begin = line;
+
+ for (cp = line + len - 1; *cp != '>'; cp--)
+ if (cp == line) return -1;
+
+ split->end = cp + 1;
+ return 0;
+}
+
+void read_authors_map_line(struct string_list *map, char *buffer)
+{
+ int len = strlen(buffer);
+
+ if (len && buffer[len - 1] == '\n')
+ buffer[--len] = 0;
+
+ string_list_insert(map, xstrdup(buffer));
+}
+
+void read_authors_map_file(struct string_list *map)
+{
+ char buffer[1024];
+ FILE *f;
+ const char *filename;
+ const char *home;
+
+ home = getenv("HOME");
+ if (!home)
+ die("HOME not set");
+
+ filename = mkpathdup("%s/.git_authors_map", home);
+
+ f = fopen(filename, "r");
+ if (!f) {
+ if (errno == ENOENT) {
+ warning("~/.git_authors_map does not exist");
+ return;
+ }
+ die_errno("unable to open authors map at %s", filename);
+ }
+
+ while (fgets(buffer, sizeof(buffer), f) != NULL)
+ read_authors_map_line(map, buffer);
+ fclose(f);
+}
+
+char *lookup_author(struct string_list *map, const char *author_abbr)
+{
+ struct string_list_item *author_item = NULL;
+ struct string_list_item *item;
+
+ for_each_string_list_item(item, map) {
+ if (strncmp(item->string, author_abbr, strlen(author_abbr)) == 0 &&
+ strlen(item->string) > strlen(author_abbr) &&
+ *(item->string + strlen(author_abbr)) == ' ') {
+ author_item = item;
+ break;
+ }
+ }
+
+ if (!author_item)
+ return NULL;
+
+ return xstrdup(author_item->string + strlen(author_abbr) + 1);
+}
+
+const char *expand_authors(struct string_list *map, const char *author_shorts)
+{
+ int i;
+ const char *author_start = author_shorts;
+ const char *author_end;
+ char *author_short, *expanded_author;
+ static struct strbuf expanded_authors = STRBUF_INIT;
+
+ strbuf_reset(&expanded_authors);
+
+ for (i = 0; i <= strlen(author_shorts); i++) {
+ author_end = author_shorts + i;
+ if (*author_end == ' ' || *author_end == '\0') {
+ author_short = xstrndup(author_start, author_end - author_start);
+ expanded_author = lookup_author(map, author_short);
+ if (!expanded_author)
+ die("Could not expand author '%s'. Add it to the file ~/.git_authors_map.", author_short);
+ else {
+ if (expanded_authors.len > 0)
+ strbuf_addch(&expanded_authors, ',');
+ strbuf_addstr(&expanded_authors, expanded_author);
+ free(expanded_author);
+ }
+ free(author_short);
+
+ author_start = author_end + 1;
+ }
+ }
+
+ return expanded_authors.buf;
+}
+
+const char *git_authors_info(void)
+{
+ static struct strbuf authors_info = STRBUF_INIT;
+ const char *authors_config = NULL;
+ const char *date_str = NULL;
+ struct string_list authors_map = STRING_LIST_INIT_NODUP;
+
+ if (git_config_get_string_const("authors.current", &authors_config))
+ return NULL;
+
+ read_authors_map_file(&authors_map);
+
+ strbuf_reset(&authors_info);
+ strbuf_addstr(&authors_info, expand_authors(&authors_map, authors_config));
+
+ strbuf_addch(&authors_info, ' ');
+ date_str = getenv("GIT_AUTHOR_DATE");
+ if (date_str && date_str[0]) {
+ if (parse_date(date_str, &authors_info) < 0)
+ die("invalid date format: %s", date_str);
+ }
+ else
+ strbuf_addstr(&authors_info, ident_default_date());
+
+ return authors_info.buf;
+}
+
+const char *git_authors_first_info(const char *authors)
+{
+ static struct strbuf authors_first_info = STRBUF_INIT;
+ struct authors_split split;
+ const char *cp;
+
+ if (split_authors_line(&split, authors, strlen(authors)) < 0)
+ die("invalid authors format: %s", authors);
+
+ for (cp = split.begin; cp < split.end; cp++)
+ if (*cp == ',')
+ break;
+ strbuf_add(&authors_first_info, split.begin, cp - split.begin);
+ strbuf_add(&authors_first_info, split.end, strlen(authors));
+
+ return authors_first_info.buf;
+}
+
+const char *authors_split_to_email_froms(const struct authors_split *authors)
+{
+ static struct strbuf email_froms = STRBUF_INIT;
+ const char *cp;
+
+ strbuf_reset(&email_froms);
+
+ strbuf_addstr(&email_froms, "From: ");
+ for(cp = authors->begin; cp < authors->end; cp++)
+ if (*cp == ',')
+ strbuf_addstr(&email_froms, "\nFrom: ");
+ else
+ strbuf_addch(&email_froms, *cp);
+ strbuf_addch(&email_froms, '\n');
+
+ return email_froms.buf;
+}
+
+int has_multiple_authors(const char *authors)
+{
+ const char *cp = authors;
+
+ while (*cp != '\0')
+ if (*cp++ == ',')
+ return 1;
+ return 0;
+}
+
+const char *fmt_authors(const char *authors, const char *date_str)
+{
+ static struct strbuf authors_info = STRBUF_INIT;
+
+ strbuf_reset(&authors_info);
+
+ strbuf_addstr(&authors_info, authors);
+
+ strbuf_addch(&authors_info, ' ');
+ if (date_str && date_str[0]) {
+ if (parse_date(date_str, &authors_info) < 0)
+ die("invalid date format: %s", date_str);
+ }
+ else
+ strbuf_addstr(&authors_info, ident_default_date());
+
+ return authors_info.buf;
+}
diff --git a/authors.h b/authors.h
new file mode 100644
index 0000000..aa96674
--- /dev/null
+++ b/authors.h
@@ -0,0 +1,29 @@
+#ifndef AUTHORS_H
+#define AUTHORS_H
+
+#include "string-list.h"
+
+struct authors_split {
+ const char *begin;
+ const char *end;
+};
+/*
+ * Signals an success with 0
+ */
+extern int split_authors_line(struct authors_split *, const char *, int);
+
+extern const char *git_authors_info(void);
+extern const char *git_authors_first_info(const char *);
+
+extern void read_authors_map_file(struct string_list *);
+
+extern char *lookup_author(struct string_list *, const char *);
+extern const char *expand_authors(struct string_list *, const char *);
+
+extern const char *authors_split_to_email_froms(const struct authors_split *);
+
+extern int has_multiple_authors(const char *);
+
+extern const char *fmt_authors(const char *, const char *);
+
+#endif /* AUTHORS_H */
diff --git a/builtin.h b/builtin.h
index b9122bc..fec370e 100644
--- a/builtin.h
+++ b/builtin.h
@@ -34,6 +34,7 @@ extern int cmd_am(int argc, const char **argv, const char *prefix);
extern int cmd_annotate(int argc, const char **argv, const char *prefix);
extern int cmd_apply(int argc, const char **argv, const char *prefix);
extern int cmd_archive(int argc, const char **argv, const char *prefix);
+extern int cmd_authors(int argc, const char **argv, const char *prefix);
extern int cmd_bisect__helper(int argc, const char **argv, const char *prefix);
extern int cmd_blame(int argc, const char **argv, const char *prefix);
extern int cmd_branch(int argc, const char **argv, const char *prefix);
diff --git a/builtin/am.c b/builtin/am.c
index 31fb605..cc131d4 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -30,6 +30,7 @@
#include "mailinfo.h"
#include "apply.h"
#include "string-list.h"
+#include "authors.h"
/**
* Returns 1 if the file is empty or does not exist, 0 otherwise.
@@ -106,6 +107,7 @@ struct am_state {
char *author_name;
char *author_email;
char *author_date;
+ char *authors;
char *msg;
size_t msg_len;
@@ -171,6 +173,7 @@ static void am_state_release(struct am_state *state)
free(state->author_name);
free(state->author_email);
free(state->author_date);
+ free(state->authors);
free(state->msg);
argv_array_clear(&state->git_apply_opts);
}
@@ -1237,6 +1240,7 @@ static int parse_mail(struct am_state *state, const char *mail)
struct strbuf author_name = STRBUF_INIT;
struct strbuf author_date = STRBUF_INIT;
struct strbuf author_email = STRBUF_INIT;
+ struct strbuf authors = STRBUF_INIT;
int ret = 0;
struct mailinfo mi;
@@ -1303,6 +1307,8 @@ static int parse_mail(struct am_state *state, const char *mail)
strbuf_addstr(&author_email, x);
else if (skip_prefix(sb.buf, "Date: ", &x))
strbuf_addstr(&author_date, x);
+ else if (skip_prefix(sb.buf, "Authors: ", &x))
+ strbuf_addstr(&authors, x);
}
fclose(fp);
@@ -1333,6 +1339,10 @@ static int parse_mail(struct am_state *state, const char *mail)
assert(!state->author_date);
state->author_date = strbuf_detach(&author_date, NULL);
+ assert(!state->authors);
+ if (authors.len > 0)
+ state->authors = strbuf_detach(&authors, NULL);
+
assert(!state->msg);
state->msg = strbuf_detach(&msg, &state->msg_len);
@@ -1341,6 +1351,7 @@ static int parse_mail(struct am_state *state, const char *mail)
strbuf_release(&author_date);
strbuf_release(&author_email);
strbuf_release(&author_name);
+ strbuf_release(&authors);
strbuf_release(&sb);
clear_mailinfo(&mi);
return ret;
@@ -1678,6 +1689,7 @@ static void do_commit(const struct am_state *state)
struct commit_list *parents = NULL;
const char *reflog_msg, *author;
struct strbuf sb = STRBUF_INIT;
+ const char *authors_info = NULL;
if (run_hook_le(NULL, "pre-applypatch", NULL))
exit(1);
@@ -1701,8 +1713,12 @@ static void do_commit(const struct am_state *state)
setenv("GIT_COMMITTER_DATE",
state->ignore_date ? "" : state->author_date, 1);
+ if (state->authors)
+ authors_info = fmt_authors(state->authors,
+ state->ignore_date ? NULL : state->author_date);
+
if (commit_tree(state->msg, state->msg_len, tree.hash, parents, commit.hash,
- author, state->sign_commit))
+ author, authors_info, state->sign_commit))
die(_("failed to write commit object"));
reflog_msg = getenv("GIT_REFLOG_ACTION");
diff --git a/builtin/authors.c b/builtin/authors.c
new file mode 100644
index 0000000..3342b91
--- /dev/null
+++ b/builtin/authors.c
@@ -0,0 +1,82 @@
+#include "builtin.h"
+#include "authors.h"
+#include "parse-options.h"
+
+static const char *const builtin_authors_usage[] = {
+ N_("git authors [<options>]"),
+ NULL
+};
+
+static int actions;
+
+#define ACTION_LIST (1<<0)
+#define ACTION_GET (1<<1)
+#define ACTION_SET (1<<2)
+#define ACTION_CLEAR (1<<3)
+
+static struct option builtin_authors_options[] = {
+ OPT_BIT('c', "clear", &actions, N_("clear current authors"), ACTION_CLEAR),
+ OPT_BIT('l', "list", &actions, N_("list all available authors"), ACTION_LIST)
+};
+
+static struct string_list authors_map = STRING_LIST_INIT_NODUP;
+
+int cmd_authors(int argc, const char **argv, const char *prefix)
+{
+ struct string_list_item *item;
+
+ argc = parse_options(argc, argv, prefix, builtin_authors_options,
+ builtin_authors_usage,
+ PARSE_OPT_STOP_AT_NON_OPTION);
+
+ if (actions == 0) {
+ if (argc == 0)
+ actions = ACTION_GET;
+ else
+ actions = ACTION_SET;
+ }
+
+ read_authors_map_file(&authors_map);
+
+ if (actions == ACTION_LIST)
+ for_each_string_list_item(item, &authors_map) {
+ printf("%s\n", item->string);
+ }
+ else if (actions == ACTION_GET) {
+ const char *authors_config = NULL;
+ const char *expanded_authors;
+
+ if (git_config_get_string_const("authors.current", &authors_config))
+ die("No current authors set. Use `git authors <initials> <initials> to set authors.");
+
+ printf("Short: %s\n", authors_config);
+
+ expanded_authors = expand_authors(&authors_map, authors_config);
+
+ printf("Expanded: %s\n", expanded_authors);
+ }
+ else if (actions == ACTION_SET) {
+ int i;
+ static struct strbuf authors_info = STRBUF_INIT;
+ int lookup_error = 0;
+
+ for (i = 0; i < argc; ++i) {
+ if (!lookup_author(&authors_map, argv[i])) {
+ lookup_error--;
+ error("Couldn't find author '%s'", argv[i]);
+ }
+ if (i > 0)
+ strbuf_addch(&authors_info, ' ');
+ strbuf_addstr(&authors_info, argv[i]);
+ }
+ if (lookup_error < 0)
+ die("Add missing authors to ~/.git_authors_map");
+
+ git_config_set("authors.current", authors_info.buf);
+ }
+ else if (actions == ACTION_CLEAR) {
+ git_config_set("authors.current", NULL);
+ }
+
+ return 0;
+}
diff --git a/builtin/commit-tree.c b/builtin/commit-tree.c
index 6050172..ecc2ce4 100644
--- a/builtin/commit-tree.c
+++ b/builtin/commit-tree.c
@@ -118,7 +118,7 @@ int cmd_commit_tree(int argc, const char **argv, const char *prefix)
}
if (commit_tree(buffer.buf, buffer.len, tree_oid.hash, parents,
- commit_oid.hash, NULL, sign_commit)) {
+ commit_oid.hash, NULL, NULL, sign_commit)) {
strbuf_release(&buffer);
return 1;
}
diff --git a/builtin/commit.c b/builtin/commit.c
index 711f96c..88038ce 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -33,6 +33,7 @@
#include "notes-utils.h"
#include "mailmap.h"
#include "sigchain.h"
+#include "authors.h"
static const char * const builtin_commit_usage[] = {
N_("git commit [<options>] [--] <pathspec>..."),
@@ -622,6 +623,29 @@ static void determine_author_info(struct strbuf *author_ident)
free(date);
}
+static void determine_authors_info(struct strbuf *authors_info)
+{
+ const char *current_authors_info;
+
+ if (author_message) {
+ struct authors_split authors;
+ size_t len;
+ const char *a;
+
+ a = find_commit_header(author_message_buffer, "authors", &len);
+ if (a) {
+ if (split_authors_line(&authors, a, len) < 0)
+ die(_("commit '%s' has malformed authors line"), author_message);
+
+ strbuf_add(authors_info, a, len);
+ }
+ } else {
+ current_authors_info = git_authors_info();
+ if (current_authors_info)
+ strbuf_addstr(authors_info, current_authors_info);
+ }
+}
+
static int author_date_is_interesting(void)
{
return author_message || force_date;
@@ -660,7 +684,8 @@ static void adjust_comment_line_char(const struct strbuf *sb)
static int prepare_to_commit(const char *index_file, const char *prefix,
struct commit *current_head,
struct wt_status *s,
- struct strbuf *author_ident)
+ struct strbuf *author_ident,
+ struct strbuf *authors_info)
{
struct stat statbuf;
struct strbuf committer_ident = STRBUF_INIT;
@@ -671,8 +696,15 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
int clean_message_contents = (cleanup_mode != CLEANUP_NONE);
int old_display_comment_prefix;
- /* This checks and barfs if author is badly specified */
- determine_author_info(author_ident);
+ determine_authors_info(authors_info);
+ if (authors_info->len > 0) {
+ strbuf_addstr(author_ident, git_authors_first_info(authors_info->buf));
+ strbuf_addstr(&committer_ident, git_authors_first_info(authors_info->buf));
+ } else {
+ /* This checks and barfs if author is badly specified */
+ determine_author_info(author_ident);
+ strbuf_addstr(&committer_ident, git_committer_info(IDENT_STRICT));
+ }
if (!no_verify && run_commit_hook(use_editor, index_file, "pre-commit", NULL))
return 0;
@@ -800,11 +832,11 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
strbuf_release(&sb);
/* This checks if committer ident is explicitly given */
- strbuf_addstr(&committer_ident, git_committer_info(IDENT_STRICT));
if (use_editor && include_status) {
int ident_shown = 0;
int saved_color_setting;
struct ident_split ci, ai;
+ struct authors_split authors_split;
if (whence != FROM_COMMIT) {
if (cleanup_mode == CLEANUP_SCISSORS)
@@ -854,6 +886,14 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
assert_split_ident(&ai, author_ident);
assert_split_ident(&ci, &committer_ident);
+ if (authors_info->len > 0) {
+ split_authors_line(&authors_split, authors_info->buf, authors_info->len);
+ status_printf_ln(s, GIT_COLOR_NORMAL,
+ _("%s"
+ "Authors: %.*s"),
+ ident_shown++ ? "" : "\n",
+ (int)(authors_split.end - authors_split.begin), authors_split.begin);
+ }
if (ident_cmp(&ai, &ci))
status_printf_ln(s, GIT_COLOR_NORMAL,
_("%s"
@@ -1637,6 +1677,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
struct strbuf sb = STRBUF_INIT;
struct strbuf author_ident = STRBUF_INIT;
+ struct strbuf authors_info = STRBUF_INIT;
const char *index_file, *reflog_msg;
char *nl;
unsigned char sha1[20];
@@ -1675,7 +1716,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
/* Set up everything for writing the commit object. This includes
running hooks, writing the trees, and interacting with the user. */
if (!prepare_to_commit(index_file, prefix,
- current_head, &s, &author_ident)) {
+ current_head, &s, &author_ident, &authors_info)) {
rollback_index_files();
return 1;
}
@@ -1762,11 +1803,14 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
}
if (commit_tree_extended(sb.buf, sb.len, active_cache_tree->sha1,
- parents, sha1, author_ident.buf, sign_commit, extra)) {
+ parents, sha1, author_ident.buf,
+ authors_info.len > 0 ? authors_info.buf : NULL,
+ sign_commit, extra)) {
rollback_index_files();
die(_("failed to write commit object"));
}
strbuf_release(&author_ident);
+ strbuf_release(&authors_info);
free_commit_extra_headers(extra);
nl = strchr(sb.buf, '\n');
diff --git a/builtin/merge.c b/builtin/merge.c
index a96d4fb..a23599c 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -798,7 +798,7 @@ static int merge_trivial(struct commit *head, struct commit_list *remoteheads)
pptr = commit_list_append(remoteheads->item, pptr);
prepare_to_commit(remoteheads);
if (commit_tree(merge_msg.buf, merge_msg.len, result_tree, parents,
- result_commit, NULL, sign_commit))
+ result_commit, NULL, NULL, sign_commit))
die(_("failed to write commit object"));
finish(head, remoteheads, result_commit, "In-index merge");
drop_save();
@@ -823,7 +823,7 @@ static int finish_automerge(struct commit *head,
strbuf_addch(&merge_msg, '\n');
prepare_to_commit(remoteheads);
if (commit_tree(merge_msg.buf, merge_msg.len, result_tree, parents,
- result_commit, NULL, sign_commit))
+ result_commit, NULL, NULL, sign_commit))
die(_("failed to write commit object"));
strbuf_addf(&buf, "Merge made by the '%s' strategy.", wt_strategy);
finish(head, remoteheads, result_commit, buf.buf);
diff --git a/cache.h b/cache.h
index 40f7ddd..3cb4a84 100644
--- a/cache.h
+++ b/cache.h
@@ -1327,6 +1327,7 @@ extern const char *fmt_ident(const char *name, const char *email, const char *da
extern const char *fmt_name(const char *name, const char *email);
extern const char *ident_default_name(void);
extern const char *ident_default_email(void);
+extern const char *ident_default_date(void);
extern const char *git_editor(void);
extern const char *git_pager(int stdout_is_tty);
extern int git_ident_config(const char *, const char *, void *);
diff --git a/commit.c b/commit.c
index 2cf8515..40eb1c4 100644
--- a/commit.c
+++ b/commit.c
@@ -11,6 +11,7 @@
#include "commit-slab.h"
#include "prio-queue.h"
#include "sha1-lookup.h"
+#include "authors.h"
static struct commit_extra_header *read_commit_extra_header_lines(const char *buf, size_t len, const char **);
@@ -1312,7 +1313,8 @@ static inline int standard_header_field(const char *field, size_t len)
(len == 6 && !memcmp(field, "parent ", 7)) ||
(len == 6 && !memcmp(field, "author ", 7)) ||
(len == 9 && !memcmp(field, "committer ", 10)) ||
- (len == 8 && !memcmp(field, "encoding ", 9)));
+ (len == 8 && !memcmp(field, "encoding ", 9)) ||
+ (len == 7 && !memcmp(field, "authors ", 8)));
}
static int excluded_header_field(const char *field, size_t len, const char **exclude)
@@ -1388,14 +1390,14 @@ void free_commit_extra_headers(struct commit_extra_header *extra)
int commit_tree(const char *msg, size_t msg_len,
const unsigned char *tree,
struct commit_list *parents, unsigned char *ret,
- const char *author, const char *sign_commit)
+ const char *author, const char *authors, const char *sign_commit)
{
struct commit_extra_header *extra = NULL, **tail = &extra;
int result;
append_merge_tag_headers(parents, &tail);
result = commit_tree_extended(msg, msg_len, tree, parents, ret,
- author, sign_commit, extra);
+ author, authors, sign_commit, extra);
free_commit_extra_headers(extra);
return result;
}
@@ -1518,7 +1520,8 @@ N_("Warning: commit message did not conform to UTF-8.\n"
int commit_tree_extended(const char *msg, size_t msg_len,
const unsigned char *tree,
struct commit_list *parents, unsigned char *ret,
- const char *author, const char *sign_commit,
+ const char *author, const char *authors,
+ const char *sign_commit,
struct commit_extra_header *extra)
{
int result;
@@ -1549,11 +1552,13 @@ int commit_tree_extended(const char *msg, size_t msg_len,
/* Person/date information */
if (!author)
- author = git_author_info(IDENT_STRICT);
+ author = authors ? git_authors_first_info(authors) : git_author_info(IDENT_STRICT);
strbuf_addf(&buffer, "author %s\n", author);
- strbuf_addf(&buffer, "committer %s\n", git_committer_info(IDENT_STRICT));
+ strbuf_addf(&buffer, "committer %s\n", authors ? git_authors_first_info(authors) : git_committer_info(IDENT_STRICT));
if (!encoding_is_utf8)
strbuf_addf(&buffer, "encoding %s\n", git_commit_encoding);
+ if (authors)
+ strbuf_addf(&buffer, "authors %s\n", authors);
while (extra) {
add_extra_header(&buffer, extra);
diff --git a/commit.h b/commit.h
index 9c12abb..4880460 100644
--- a/commit.h
+++ b/commit.h
@@ -331,12 +331,14 @@ extern void append_merge_tag_headers(struct commit_list *parents,
extern int commit_tree(const char *msg, size_t msg_len,
const unsigned char *tree,
struct commit_list *parents, unsigned char *ret,
- const char *author, const char *sign_commit);
+ const char *author, const char *authors,
+ const char *sign_commit);
extern int commit_tree_extended(const char *msg, size_t msg_len,
const unsigned char *tree,
struct commit_list *parents, unsigned char *ret,
- const char *author, const char *sign_commit,
+ const char *author, const char *authors,
+ const char *sign_commit,
struct commit_extra_header *);
extern struct commit_extra_header *read_commit_extra_headers(struct commit *, const char **);
diff --git a/git.c b/git.c
index b367cf6..b6fb852 100644
--- a/git.c
+++ b/git.c
@@ -397,6 +397,7 @@ static struct cmd_struct commands[] = {
{ "annotate", cmd_annotate, RUN_SETUP },
{ "apply", cmd_apply, RUN_SETUP_GENTLY },
{ "archive", cmd_archive, RUN_SETUP_GENTLY },
+ { "authors", cmd_authors, RUN_SETUP },
{ "bisect--helper", cmd_bisect__helper, RUN_SETUP },
{ "blame", cmd_blame, RUN_SETUP },
{ "branch", cmd_branch, RUN_SETUP },
diff --git a/ident.c b/ident.c
index ac4ae02..b538abc 100644
--- a/ident.c
+++ b/ident.c
@@ -177,7 +177,7 @@ const char *ident_default_email(void)
return git_default_email.buf;
}
-static const char *ident_default_date(void)
+const char *ident_default_date(void)
{
if (!git_default_date.len)
datestamp(&git_default_date);
diff --git a/mailinfo.c b/mailinfo.c
index a489d9d..43c85de 100644
--- a/mailinfo.c
+++ b/mailinfo.c
@@ -1,6 +1,7 @@
#include "cache.h"
#include "utf8.h"
#include "strbuf.h"
+#include "authors.h"
#include "mailinfo.h"
static void cleanup_space(struct strbuf *sb)
@@ -533,6 +534,18 @@ static int check_header(struct mailinfo *mi,
{
int i, ret = 0, len;
struct strbuf sb = STRBUF_INIT;
+ struct strbuf from = STRBUF_INIT;
+
+ if (overwrite && cmp_header(line, "From")) {
+ len = strlen("From: ");
+ strbuf_add(&from, line->buf + len, line->len - len);
+ decode_header(mi, &from);
+
+ if (mi->authors.len > 0)
+ strbuf_addch(&mi->authors,',');
+ strbuf_addstr(&mi->authors, from.buf);
+ }
+ strbuf_release(&from);
/* search for the interesting parts */
for (i = 0; header[i]; i++) {
@@ -1068,6 +1081,8 @@ static void handle_info(struct mailinfo *mi)
fprintf(mi->output, "%s: %s\n", header[i], hdr->buf);
}
}
+ if (has_multiple_authors(mi->authors.buf))
+ fprintf(mi->output, "Authors: %s\n", mi->authors.buf);
fprintf(mi->output, "\n");
}
@@ -1133,6 +1148,7 @@ void setup_mailinfo(struct mailinfo *mi)
strbuf_init(&mi->charset, 0);
strbuf_init(&mi->log_message, 0);
strbuf_init(&mi->inbody_header_accum, 0);
+ strbuf_init(&mi->authors, 0);
mi->header_stage = 1;
mi->use_inbody_headers = 1;
mi->content_top = mi->content;
@@ -1147,6 +1163,7 @@ void clear_mailinfo(struct mailinfo *mi)
strbuf_release(&mi->email);
strbuf_release(&mi->charset);
strbuf_release(&mi->inbody_header_accum);
+ strbuf_release(&mi->authors);
free(mi->message_id);
for (i = 0; mi->p_hdr_data[i]; i++)
diff --git a/mailinfo.h b/mailinfo.h
index 04a2535..d264d0d 100644
--- a/mailinfo.h
+++ b/mailinfo.h
@@ -33,6 +33,8 @@ struct mailinfo {
struct strbuf log_message;
int input_error;
+
+ struct strbuf authors;
};
extern void setup_mailinfo(struct mailinfo *);
diff --git a/notes-cache.c b/notes-cache.c
index 5dfc5cb..2b58add 100644
--- a/notes-cache.c
+++ b/notes-cache.c
@@ -58,7 +58,7 @@ int notes_cache_write(struct notes_cache *c)
if (write_notes_tree(&c->tree, tree_sha1))
return -1;
if (commit_tree(c->validity, strlen(c->validity), tree_sha1, NULL,
- commit_sha1, NULL, NULL) < 0)
+ commit_sha1, NULL, NULL, NULL) < 0)
return -1;
if (update_ref("update notes cache", c->tree.update_ref, commit_sha1,
NULL, 0, UPDATE_REFS_QUIET_ON_ERR) < 0)
diff --git a/notes-utils.c b/notes-utils.c
index 24a3361..8adf381 100644
--- a/notes-utils.c
+++ b/notes-utils.c
@@ -26,7 +26,7 @@ void create_notes_commit(struct notes_tree *t, struct commit_list *parents,
/* else: t->ref points to nothing, assume root/orphan commit */
}
- if (commit_tree(msg, msg_len, tree_sha1, parents, result_sha1, NULL, NULL))
+ if (commit_tree(msg, msg_len, tree_sha1, parents, result_sha1, NULL, NULL, NULL))
die("Failed to commit notes tree to database");
}
diff --git a/pretty.c b/pretty.c
index 5e68383..d505f2a 100644
--- a/pretty.c
+++ b/pretty.c
@@ -11,6 +11,7 @@
#include "reflog-walk.h"
#include "gpg-interface.h"
#include "trailer.h"
+#include "authors.h"
static char *user_format;
static struct cmt_fmt_map {
@@ -510,6 +511,55 @@ void pp_user_info(struct pretty_print_context *pp,
}
}
+void pp_authors_info(struct pretty_print_context *pp,
+ struct strbuf *sb,
+ const char *line, const char *encoding)
+{
+ struct ident_split ident;
+ char *line_end;
+ struct authors_split authors;
+
+ if (pp->fmt == CMIT_FMT_ONELINE)
+ return;
+
+ line_end = strchrnul(line, '\n');
+
+ if (split_authors_line(&authors, line, line_end - line))
+ return;
+
+ if (cmit_fmt_is_mail(pp->fmt)) {
+ strbuf_addstr(sb, authors_split_to_email_froms(&authors));
+ } else {
+ strbuf_addstr(sb, "Authors: ");
+ if (pp->fmt == CMIT_FMT_FULLER)
+ strbuf_addstr(sb, " ");
+ strbuf_add(sb, authors.begin, authors.end - authors.begin);
+ strbuf_addch(sb, '\n');
+ }
+
+ /* Use the ident split for parsing the date part */
+ if (split_ident_line(&ident, line, line_end - line))
+ return;
+
+ switch (pp->fmt) {
+ case CMIT_FMT_MEDIUM:
+ strbuf_addf(sb, "Date: %s\n",
+ show_ident_date(&ident, &pp->date_mode));
+ break;
+ case CMIT_FMT_EMAIL:
+ case CMIT_FMT_MBOXRD:
+ strbuf_addf(sb, "Date: %s\n",
+ show_ident_date(&ident, DATE_MODE(RFC2822)));
+ break;
+ case CMIT_FMT_FULLER:
+ strbuf_addf(sb, "AuthorsDate: %s\n",
+ show_ident_date(&ident, &pp->date_mode));
+ break;
+ default:
+ break;
+ }
+}
+
static int is_blank_line(const char *line, int *len_p)
{
int len = *len_p;
@@ -1544,6 +1594,23 @@ static void pp_header(struct pretty_print_context *pp,
struct strbuf *sb)
{
int parents_shown = 0;
+ int has_authors = 0;
+ const char *msg = *msg_p;
+
+ /* Check if header has `authors` header */
+ for(;;) {
+ const char *line = msg;
+ int linelen = get_one_line(msg);
+ if (!linelen)
+ break;
+ msg += linelen;
+ if (linelen == 1)
+ break;
+ if (starts_with(line, "authors ")) {
+ has_authors = 1;
+ break;
+ }
+ }
for (;;) {
const char *name, *line = *msg_p;
@@ -1582,13 +1649,19 @@ static void pp_header(struct pretty_print_context *pp,
* FULLER shows both authors and dates.
*/
if (skip_prefix(line, "author ", &name)) {
- strbuf_grow(sb, linelen + 80);
- pp_user_info(pp, "Author", sb, name, encoding);
+ if (!has_authors) {
+ strbuf_grow(sb, linelen + 80);
+ pp_user_info(pp, "Author", sb, name, encoding);
+ }
}
if (skip_prefix(line, "committer ", &name) &&
(pp->fmt == CMIT_FMT_FULL || pp->fmt == CMIT_FMT_FULLER)) {
strbuf_grow(sb, linelen + 80);
- pp_user_info(pp, "Commit", sb, name, encoding);
+ pp_user_info(pp, has_authors ? "Commit " : "Commit", sb, name, encoding);
+ }
+ if (skip_prefix(line, "authors ", &name)) {
+ strbuf_grow(sb, linelen + 80);
+ pp_authors_info(pp, sb, name, encoding);
}
}
}
diff --git a/t/helper/.gitignore b/t/helper/.gitignore
index d6e8b36..9ad4bbc 100644
--- a/t/helper/.gitignore
+++ b/t/helper/.gitignore
@@ -1,3 +1,4 @@
+/test-authors
/test-chmtime
/test-ctype
/test-config
diff --git a/t/helper/test-authors.c b/t/helper/test-authors.c
new file mode 100644
index 0000000..ab93f9f
--- /dev/null
+++ b/t/helper/test-authors.c
@@ -0,0 +1,42 @@
+#include "cache.h"
+#include "authors.h"
+
+static const char *usage_msg = "\n"
+" test-authors split [authors_info]\n"
+" test-authors has-multiple-authors [authors]\n";
+
+static void test_split_authors(const char **argv)
+{
+ struct authors_split split;
+ int result;
+ struct strbuf splitted = STRBUF_INIT;
+
+ printf("%s -> ",*argv);
+ result = split_authors_line(&split, *argv, strlen(*argv));
+ if (result)
+ printf("error");
+ else {
+ strbuf_add(&splitted, split.begin, split.end - split.begin);
+ printf(splitted.buf);
+ }
+ printf("\n");
+}
+
+static void test_has_multiple_authors(const char **argv)
+{
+ printf("%s -> %s\n", *argv, has_multiple_authors(*argv) ? "yes" : "no");
+}
+
+int cmd_main(int argc, const char **argv)
+{
+ argv++;
+ if (argc != 3)
+ usage(usage_msg);
+ if (!strcmp(*argv, "split"))
+ test_split_authors(argv+1);
+ else if (!strcmp(*argv, "has-multiple"))
+ test_has_multiple_authors(argv+1);
+ else
+ usage(usage_msg);
+ return 0;
+}
diff --git a/t/t9904-authors.sh b/t/t9904-authors.sh
new file mode 100755
index 0000000..eb7fffa
--- /dev/null
+++ b/t/t9904-authors.sh
@@ -0,0 +1,32 @@
+#!/bin/sh
+
+test_description='test authors'
+
+. ./test-lib.sh
+
+check_split() {
+ echo "$1 -> $2" >expect
+ test_expect_success "split '$1'" "
+ test-authors split '$1' >actual &&
+ test_cmp expect actual
+ "
+}
+
+check_split 'xxx' 'error'
+check_split 'Some Guy <sg@example.com> 1484387401 +0100' 'Some Guy <sg@example.com>'
+check_split 'Some Guy <sg@example.com>,Another Pal <ap@example.com> 1484387401 +0100' 'Some Guy <sg@example.com>,Another Pal <ap@example.com>'
+check_split 'Some Guy <sg@example.com>,Another Pal <ap@example.com>' 'Some Guy <sg@example.com>,Another Pal <ap@example.com>'
+
+check_has_multiple() {
+ echo "$1 -> $2" >expect
+ test_expect_success "has multiple authors '$1'" "
+ test-authors has-multiple '$1' >actual &&
+ test_cmp expect actual
+ "
+}
+
+check_has_multiple 'abc' 'no'
+check_has_multiple 'Some Guy <sg@example.com>' 'no'
+check_has_multiple 'Some Guy <sg@example.com>,Another Pal <ap@example.com>' 'yes'
+
+test_done
--
2.6.6
^ permalink raw reply related
* Re: show all merge conflicts
From: G. Sylvie Davies @ 2017-01-29 6:45 UTC (permalink / raw)
To: Jeff King; +Cc: G. Sylvie Davies, Michael Spiegel, git
In-Reply-To: <20170128142808.hefnv7r3h6zidobw@sigill.intra.peff.net>
On Sat, Jan 28, 2017 at 6:28 AM, Jeff King <peff@peff.net> wrote:
> On Fri, Jan 27, 2017 at 09:42:41PM -0800, G. Sylvie Davies wrote:
>
>> Aside from the usual "git log -cc", I think this should work (replace
>> HEAD with whichever commit you are analyzing):
>>
>> git diff --name-only HEAD^2...HEAD^1 > m1
>> git diff --name-only HEAD^1...HEAD^2 > b1
>> git diff --name-only HEAD^1..HEAD > m2
>> git diff --name-only HEAD^2..HEAD > b2
>>
>> If files listed between m1 and b2 differ, then the merge is dirty.
>> Similarly for m2 and b1.
>>
>> More information here:
>>
>> http://stackoverflow.com/questions/27683077/how-do-you-detect-an-evil-merge-in-git/41356308#41356308
>
> I don't think that can reliably find evil merges, since it looks at the
> file level. If you had one hunk resolved for "theirs" and one hunk for
> "ours" in a given file, then the file will be listed in each diff,
> whether it has evil hunks or not.
>
Well, you have to do both. Do "git show -c" to catch that one
("theirs" for one hunk, "ours" for the other, same file).
And then do that sequence of the 4 "git diff" commands to identify
dirty merges where "theirs" or "ours" was applied to entire files, and
thus not showing up in the "git show -c".
> I don't think this is just about evil merges, though. For instance,
> try:
>
> seq 1 10 >file
> git add file
> git commit -m base
>
> sed s/4/master/ <file >tmp && mv tmp file
> git commit -am master
>
> git checkout -b other HEAD^
> sed s/4/other/ <file >tmp && mv tmp file
> git commit -am other
>
> git merge master
> git checkout --ours file
> git commit -am merged
>
> merge=$(git rev-parse HEAD)
>
> The question is: were there conflicts in $merge, and how were they
> resolved?
>
> That isn't an evil merge, but there's still something interesting to
> show that "git log --cc" won't display.
>
> Replaying the merge like:
>
> git checkout $merge^1
> git merge $merge^2
> git diff -R $merge
>
> shows you the patch to go from the conflict state to the final one.
>
I know the stackoverflow question asks "how to detect evil merges",
and I go along with that in my answer. But honestly I prefer to call
them dirty rather than evil, and by "dirty" I just mean merges that
did not resolve cleanly via "git merge", and had some form of user
intervention, be it conflict resolution, or other strange things.
The trick I propose with the sequence of 4 "git diff" commands
identifies that merge from your example as dirty:
$ cat b1 m2
file
$ cat b2 m1
file
file
The trick doesn't really tell you much except that the merge is dirty.
If you notice that the "m2" file is empty, I think that's one way to
realize that master's edit was dropped, and therefore "other" won.
Maybe it even merged cleanly but someone did a "git commit --amend" to
make it the merge dirty after the fact.
I do like your approach, it's very simple and reliable. But in my
situation I'm writing pre-receive hooks for bare repos, so I don't
think I can actually do "git merge"!
I think my suggestion would work for OP, as long as they also run "git
show -c" alongside it. (And your suggestion would work, too, of
course).
- Sylvie
^ permalink raw reply
* [PATCH v2 3/4] introduce new format for git stash create
From: Thomas Gummerer @ 2017-01-29 20:16 UTC (permalink / raw)
To: git
Cc: Stephan Beyer, Junio C Hamano, Marc Strapetz, Jeff King,
Johannes Schindelin, Øyvind A . Holm, Jakub Narębski,
Thomas Gummerer
In-Reply-To: <20170129201604.30445-1-t.gummerer@gmail.com>
git stash create currently supports a positional argument for adding a
message. This is not quite in line with how git commands usually take
comments (using a -m flag).
Add a new syntax for adding a message to git stash create using a -m
flag. This is with the goal of deprecating the old style git stash
create with positional arguments.
This also adds a -u argument, for untracked files. This is already used
internally as another positional argument, but can now be used from the
command line.
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
Documentation/git-stash.txt | 1 +
git-stash.sh | 50 +++++++++++++++++++++++++++++++++++++++++----
t/t3903-stash.sh | 18 ++++++++++++++++
3 files changed, 65 insertions(+), 4 deletions(-)
diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index 0fc23c25ee..0bce33e3fc 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -17,6 +17,7 @@ SYNOPSIS
[-u|--include-untracked] [-a|--all] [<message>]]
'git stash' clear
'git stash' create [<message>]
+'git stash' create [-m <message>] [-u|--include-untracked <untracked|all>]
'git stash' store [-m|--message <message>] [-q|--quiet] <commit>
DESCRIPTION
diff --git a/git-stash.sh b/git-stash.sh
index 8528708f61..5f08b43967 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -56,8 +56,50 @@ clear_stash () {
}
create_stash () {
- stash_msg="$1"
- untracked="$2"
+ stash_msg=
+ untracked=
+ new_style=
+ while test $# != 0
+ do
+ case "$1" in
+ -m|--message)
+ shift
+ stash_msg="$1"
+ new_style=t
+ ;;
+ -u|--include-untracked)
+ shift
+ untracked="$1"
+ new_style=t
+ ;;
+ *)
+ if test -n "$new_style"
+ then
+ echo "invalid argument"
+ option="$1"
+ # TRANSLATORS: $option is an invalid option, like
+ # `--blah-blah'. The 7 spaces at the beginning of the
+ # second line correspond to "error: ". So you should line
+ # up the second line with however many characters the
+ # translation of "error: " takes in your language. E.g. in
+ # English this is:
+ #
+ # $ git stash save --blah-blah 2>&1 | head -n 2
+ # error: unknown option for 'stash save': --blah-blah
+ # To provide a message, use git stash save -- '--blah-blah'
+ eval_gettextln "error: unknown option for 'stash create': \$option"
+ usage
+ fi
+ break
+ ;;
+ esac
+ shift
+ done
+
+ if test -z "$new_style"
+ then
+ stash_msg="$*"
+ fi
git update-index -q --refresh
if no_changes
@@ -265,7 +307,7 @@ push_stash () {
git reflog exists $ref_stash ||
clear_stash || die "$(gettext "Cannot initialize stash")"
- create_stash "$stash_msg" $untracked
+ create_stash -m "$stash_msg" -u "$untracked"
store_stash -m "$stash_msg" -q $w_commit ||
die "$(gettext "Cannot save the current status")"
say "$(eval_gettext "Saved working directory and index state \$stash_msg")"
@@ -697,7 +739,7 @@ clear)
;;
create)
shift
- create_stash "$*" && echo "$w_commit"
+ create_stash "$@" && echo "$w_commit"
;;
store)
shift
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 0171b824c9..34e9610bb6 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -784,4 +784,22 @@ test_expect_success 'push -m shows right message' '
test_cmp expect actual
'
+test_expect_success 'deprecated version of stash create stores correct message' '
+ >foo &&
+ git add foo &&
+ STASH_ID=$(git stash create "create test message") &&
+ echo "On master: create test message" >expect &&
+ git show --pretty=%s ${STASH_ID} | head -n1 >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'new style stash create stores correct message' '
+ >foo &&
+ git add foo &&
+ STASH_ID=$(git stash create -m "create test message new style") &&
+ echo "On master: create test message new style" >expect &&
+ git show --pretty=%s ${STASH_ID} | head -n1 >actual &&
+ test_cmp expect actual
+'
+
test_done
--
2.11.0.297.g9a2118ac0b.dirty
^ permalink raw reply related
* [PATCH v2 0/4] stash: create filename argument
From: Thomas Gummerer @ 2017-01-29 20:16 UTC (permalink / raw)
To: git
Cc: Stephan Beyer, Junio C Hamano, Marc Strapetz, Jeff King,
Johannes Schindelin, Øyvind A . Holm, Jakub Narębski,
Thomas Gummerer
In-Reply-To: <20170121200804.19009-1-t.gummerer@gmail.com>
Previous round is at:
http://public-inbox.org/git/20170121200804.19009-1-t.gummerer@gmail.com/.
Thanks Junio, Peff, Øyvind, Jakub and Johannes for your feedback on
the previous round.
Changes since the previous round:
- Re-phrased the Documentation update.
- Added missing $ in 2/3
- Added an extra patch introducing a new syntax for git stash create,
where the message can be specified with the -m flag, instead of as a
positional argument
- Filenames with $IFS in their name are now supported. Added a test
for that as well.
Interdiff below:
diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index 871a3b246c..8306bac397 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -20,6 +20,8 @@ SYNOPSIS
[--] [<paths>...]
'git stash' clear
'git stash' create [<message>]
+'git stash' create [-m <message>] [-u|--include-untracked <untracked|all>]
+ [-- <paths>...]
'git stash' store [-m|--message <message>] [-q|--quiet] <commit>
DESCRIPTION
@@ -51,8 +53,8 @@ OPTIONS
save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] [<message>]::
push [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] [-m|--message <message>] [--] [<paths>...]::
- Save your local modifications to a new 'stash', and revert the
- the changes in the working tree to match the index.
+ Save your local modifications to a new 'stash' and roll them
+ back both in the working tree and in the index.
The <message> part is optional and gives
the description along with the stashed state. For quickly making
a snapshot, you can omit _both_ "save" and <message>, but giving
diff --git a/git-stash.sh b/git-stash.sh
index 7dcce629bd..0072a38b4c 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -56,25 +56,57 @@ clear_stash () {
}
create_stash () {
+ stash_msg=
+ untracked=
+ new_style=
files=
while test $# != 0
do
case "$1" in
+ -m|--message)
+ shift
+ stash_msg="$1"
+ new_style=t
+ ;;
+ -u|--include-untracked)
+ shift
+ untracked="$1"
+ new_style=t
+ ;;
--)
shift
+ files="$@"
+ new_style=t
break
;;
- --files)
- ;;
*)
- files="$1 $files"
+ if test -n "$new_style"
+ then
+ echo "invalid argument"
+ option="$1"
+ # TRANSLATORS: $option is an invalid option, like
+ # `--blah-blah'. The 7 spaces at the beginning of the
+ # second line correspond to "error: ". So you should line
+ # up the second line with however many characters the
+ # translation of "error: " takes in your language. E.g. in
+ # English this is:
+ #
+ # $ git stash save --blah-blah 2>&1 | head -n 2
+ # error: unknown option for 'stash save': --blah-blah
+ # To provide a message, use git stash save -- '--blah-blah'
+ eval_gettextln "error: unknown option for 'stash create': \$option"
+ usage
+ fi
+ break
;;
esac
shift
done
- stash_msg="$1"
- untracked="$2"
+ if test -z "$new_style"
+ then
+ stash_msg="$*"
+ fi
git update-index -q --refresh
if no_changes
@@ -284,7 +316,7 @@ push_stash () {
git reflog exists $ref_stash ||
clear_stash || die "$(gettext "Cannot initialize stash")"
- create_stash --files $files -- "$stash_msg" "$untracked"
+ create_stash -m "$stash_msg" -u "$untracked" -- $files
store_stash -m "$stash_msg" -q $w_commit ||
die "$(gettext "Cannot save the current status")"
say "$(eval_gettext "Saved working directory and index state \$stash_msg")"
@@ -293,9 +325,9 @@ push_stash () {
then
if test -n "$files"
then
- git reset -- $files
- git checkout HEAD -- $(git ls-files --modified -- $files)
- git clean --force --quiet -- $(git ls-files --others -- $files)
+ git ls-files -z -- "$@" | xargs -0 git reset --
+ git ls-files -z --modified -- "$@" | xargs -0 git checkout HEAD --
+ git ls-files -z --others -- "$@" | xargs -0 git clean --force --
else
git reset --hard ${GIT_QUIET:+-q}
fi
@@ -373,14 +405,9 @@ save_stash () {
shift
done
- # if test -n "$patch_mode" && test -n "$untracked"
- # then
- # die "$(gettext "Can't use --patch and --include-untracked or --all at the same time")"
- # fi
-
stash_msg="$*"
- if test -z stash_msg
+ if test -z "$stash_msg"
then
push_stash $push_options
else
@@ -728,7 +755,7 @@ clear)
;;
create)
shift
- create_stash -- "$*" && echo "$w_commit"
+ create_stash "$@" && echo "$w_commit"
;;
store)
shift
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 3e763ff766..ca4c44aa9c 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -784,6 +784,24 @@ test_expect_success 'push -m shows right message' '
test_cmp expect actual
'
+test_expect_success 'deprecated version of stash create stores correct message' '
+ >foo &&
+ git add foo &&
+ STASH_ID=$(git stash create "create test message") &&
+ echo "On master: create test message" >expect &&
+ git show --pretty=%s ${STASH_ID} | head -n1 >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'new style stash create stores correct message' '
+ >foo &&
+ git add foo &&
+ STASH_ID=$(git stash create -m "create test message new style") &&
+ echo "On master: create test message new style" >expect &&
+ git show --pretty=%s ${STASH_ID} | head -n1 >actual &&
+ test_cmp expect actual
+'
+
test_expect_success 'stash -- <filename> stashes and restores the file' '
>foo &&
>bar &&
@@ -811,4 +829,19 @@ test_expect_success 'stash with multiple filename arguments' '
test_path_is_file extra
'
+test_expect_success 'stash with file including $IFS character' '
+ >"foo bar" &&
+ >foo &&
+ >untracked &&
+ git add foo* &&
+ git stash push -- foo* &&
+ test_path_is_missing "foo bar" &&
+ test_path_is_missing foo &&
+ test_path_is_file untracked &&
+ git stash pop &&
+ test_path_is_file "foo bar" &&
+ test_path_is_file foo &&
+ test_path_is_file untracked
+'
+
test_done
Thomas Gummerer (4):
Documentation/stash: remove mention of git reset --hard
stash: introduce push verb
introduce new format for git stash create
stash: support filename argument
Documentation/git-stash.txt | 14 +++-
git-stash.sh | 154 ++++++++++++++++++++++++++++++++++++++++----
t/t3903-stash.sh | 69 ++++++++++++++++++++
3 files changed, 222 insertions(+), 15 deletions(-)
--
2.11.0.297.g9a2118ac0b.dirty
^ permalink raw reply related
* [PATCH v2 2/4] stash: introduce push verb
From: Thomas Gummerer @ 2017-01-29 20:16 UTC (permalink / raw)
To: git
Cc: Stephan Beyer, Junio C Hamano, Marc Strapetz, Jeff King,
Johannes Schindelin, Øyvind A . Holm, Jakub Narębski,
Thomas Gummerer
In-Reply-To: <20170129201604.30445-1-t.gummerer@gmail.com>
Introduce a new git stash push verb in addition to git stash save. The
push verb is used to transition from the current command line arguments
to a more conventional way, in which the message is specified after a -m
parameter instead of being a positional argument.
This allows introducing a new filename argument to stash single files.
Using that as a positional argument is much more consistent with the
rest of git, than using the positional argument for the message.
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
git-stash.sh | 76 +++++++++++++++++++++++++++++++++++++++++++++++++++++---
t/t3903-stash.sh | 9 +++++++
2 files changed, 82 insertions(+), 3 deletions(-)
diff --git a/git-stash.sh b/git-stash.sh
index 10c284d1aa..8528708f61 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -189,10 +189,11 @@ store_stash () {
return $ret
}
-save_stash () {
+push_stash () {
keep_index=
patch_mode=
untracked=
+ stash_msg=
while test $# != 0
do
case "$1" in
@@ -216,6 +217,10 @@ save_stash () {
-a|--all)
untracked=all
;;
+ -m|--message)
+ shift
+ stash_msg=$1
+ ;;
--help)
show_help
;;
@@ -251,8 +256,6 @@ save_stash () {
die "$(gettext "Can't use --patch and --include-untracked or --all at the same time")"
fi
- stash_msg="$*"
-
git update-index -q --refresh
if no_changes
then
@@ -291,6 +294,69 @@ save_stash () {
fi
}
+save_stash () {
+ push_options=
+ while test $# != 0
+ do
+ case "$1" in
+ -k|--keep-index)
+ push_options="-k $push_options"
+ ;;
+ --no-keep-index)
+ push_options="--no-keep-index $push_options"
+ ;;
+ -p|--patch)
+ push_options="-p $push_options"
+ ;;
+ -q|--quiet)
+ push_options="-q $push_options"
+ ;;
+ -u|--include-untracked)
+ push_options="-u $push_options"
+ ;;
+ -a|--all)
+ push_options="-a $push_options"
+ ;;
+ --help)
+ show_help
+ ;;
+ --)
+ shift
+ break
+ ;;
+ -*)
+ option="$1"
+ # TRANSLATORS: $option is an invalid option, like
+ # `--blah-blah'. The 7 spaces at the beginning of the
+ # second line correspond to "error: ". So you should line
+ # up the second line with however many characters the
+ # translation of "error: " takes in your language. E.g. in
+ # English this is:
+ #
+ # $ git stash save --blah-blah 2>&1 | head -n 2
+ # error: unknown option for 'stash save': --blah-blah
+ # To provide a message, use git stash save -- '--blah-blah'
+ eval_gettextln "error: unknown option for 'stash save': \$option
+ To provide a message, use git stash save -- '\$option'"
+ usage
+ ;;
+ *)
+ break
+ ;;
+ esac
+ shift
+ done
+
+ stash_msg="$*"
+
+ if test -z "$stash_msg"
+ then
+ push_stash $push_options
+ else
+ push_stash $push_options -m "$stash_msg"
+ fi
+}
+
have_stash () {
git rev-parse --verify --quiet $ref_stash >/dev/null
}
@@ -617,6 +683,10 @@ save)
shift
save_stash "$@"
;;
+push)
+ shift
+ push_stash "$@"
+ ;;
apply)
shift
apply_stash "$@"
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 2de3e18ce6..0171b824c9 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -775,4 +775,13 @@ test_expect_success 'stash is not confused by partial renames' '
test_path_is_missing file
'
+test_expect_success 'push -m shows right message' '
+ >foo &&
+ git add foo &&
+ git stash push -m "test message" &&
+ echo "stash@{0}: On master: test message" >expect &&
+ git stash list | head -n 1 >actual &&
+ test_cmp expect actual
+'
+
test_done
--
2.11.0.297.g9a2118ac0b.dirty
^ permalink raw reply related
* [PATCH v2 1/4] Documentation/stash: remove mention of git reset --hard
From: Thomas Gummerer @ 2017-01-29 20:16 UTC (permalink / raw)
To: git
Cc: Stephan Beyer, Junio C Hamano, Marc Strapetz, Jeff King,
Johannes Schindelin, Øyvind A . Holm, Jakub Narębski,
Thomas Gummerer
In-Reply-To: <20170129201604.30445-1-t.gummerer@gmail.com>
Don't mention git reset --hard in the documentation for git stash save.
It's an implementation detail that doesn't matter to the end user and
thus shouldn't be exposed to them. In addition it's not quite true for
git stash -p, and will not be true when a filename argument to limit the
stash to a few files is introduced.
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
Documentation/git-stash.txt | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index 2e9cef06e6..0fc23c25ee 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -47,8 +47,9 @@ OPTIONS
save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] [<message>]::
- Save your local modifications to a new 'stash', and run `git reset
- --hard` to revert them. The <message> part is optional and gives
+ Save your local modifications to a new 'stash' and roll them
+ back both in the working tree and in the index.
+ The <message> part is optional and gives
the description along with the stashed state. For quickly making
a snapshot, you can omit _both_ "save" and <message>, but giving
only <message> does not trigger this action to prevent a misspelled
--
2.11.0.297.g9a2118ac0b.dirty
^ permalink raw reply related
* [PATCH v2 4/4] stash: support filename argument
From: Thomas Gummerer @ 2017-01-29 20:16 UTC (permalink / raw)
To: git
Cc: Stephan Beyer, Junio C Hamano, Marc Strapetz, Jeff King,
Johannes Schindelin, Øyvind A . Holm, Jakub Narębski,
Thomas Gummerer
In-Reply-To: <20170129201604.30445-1-t.gummerer@gmail.com>
While working on a repository, it's often helpful to stash the changes
of a single or multiple files, and leave others alone. Unfortunately
git currently offers no such option. git stash -p can be used to work
around this, but it's often impractical when there are a lot of changes
over multiple files.
Add an optional filename argument to git stash push, which allows for
stashing a single (or multiple) files.
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
Documentation/git-stash.txt | 9 +++++++++
git-stash.sh | 30 +++++++++++++++++++++++-------
t/t3903-stash.sh | 42 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 74 insertions(+), 7 deletions(-)
diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index 0bce33e3fc..8306bac397 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -15,9 +15,13 @@ SYNOPSIS
'git stash' branch <branchname> [<stash>]
'git stash' [save [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]
[-u|--include-untracked] [-a|--all] [<message>]]
+'git stash' push [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]
+ [-u|--include-untracked] [-a|--all] [-m|--message <message>]]
+ [--] [<paths>...]
'git stash' clear
'git stash' create [<message>]
'git stash' create [-m <message>] [-u|--include-untracked <untracked|all>]
+ [-- <paths>...]
'git stash' store [-m|--message <message>] [-q|--quiet] <commit>
DESCRIPTION
@@ -47,6 +51,7 @@ OPTIONS
-------
save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] [<message>]::
+push [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] [-m|--message <message>] [--] [<paths>...]::
Save your local modifications to a new 'stash' and roll them
back both in the working tree and in the index.
@@ -56,6 +61,10 @@ save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q
only <message> does not trigger this action to prevent a misspelled
subcommand from making an unwanted stash.
+
+If the paths argument is given in 'git stash push', only these files
+are put in the new 'stash'. In addition only the indicated files are
+changed in the working tree to match the index.
++
If the `--keep-index` option is used, all changes already added to the
index are left intact.
+
diff --git a/git-stash.sh b/git-stash.sh
index 5f08b43967..0072a38b4c 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -41,7 +41,7 @@ no_changes () {
untracked_files () {
excl_opt=--exclude-standard
test "$untracked" = "all" && excl_opt=
- git ls-files -o -z $excl_opt
+ git ls-files -o -z $excl_opt -- $1
}
clear_stash () {
@@ -59,6 +59,7 @@ create_stash () {
stash_msg=
untracked=
new_style=
+ files=
while test $# != 0
do
case "$1" in
@@ -72,6 +73,12 @@ create_stash () {
untracked="$1"
new_style=t
;;
+ --)
+ shift
+ files="$@"
+ new_style=t
+ break
+ ;;
*)
if test -n "$new_style"
then
@@ -134,7 +141,7 @@ create_stash () {
# Untracked files are stored by themselves in a parentless commit, for
# ease of unpacking later.
u_commit=$(
- untracked_files | (
+ untracked_files $files | (
GIT_INDEX_FILE="$TMPindex" &&
export GIT_INDEX_FILE &&
rm -f "$TMPindex" &&
@@ -157,7 +164,7 @@ create_stash () {
git read-tree --index-output="$TMPindex" -m $i_tree &&
GIT_INDEX_FILE="$TMPindex" &&
export GIT_INDEX_FILE &&
- git diff-index --name-only -z HEAD -- >"$TMP-stagenames" &&
+ git diff-index --name-only -z HEAD -- $files >"$TMP-stagenames" &&
git update-index -z --add --remove --stdin <"$TMP-stagenames" &&
git write-tree &&
rm -f "$TMPindex"
@@ -171,7 +178,7 @@ create_stash () {
# find out what the user wants
GIT_INDEX_FILE="$TMP-index" \
- git add--interactive --patch=stash -- &&
+ git add--interactive --patch=stash -- $files &&
# state of the working tree
w_tree=$(GIT_INDEX_FILE="$TMP-index" git write-tree) ||
@@ -293,6 +300,8 @@ push_stash () {
shift
done
+ files="$*"
+
if test -n "$patch_mode" && test -n "$untracked"
then
die "$(gettext "Can't use --patch and --include-untracked or --all at the same time")"
@@ -307,18 +316,25 @@ push_stash () {
git reflog exists $ref_stash ||
clear_stash || die "$(gettext "Cannot initialize stash")"
- create_stash -m "$stash_msg" -u "$untracked"
+ create_stash -m "$stash_msg" -u "$untracked" -- $files
store_stash -m "$stash_msg" -q $w_commit ||
die "$(gettext "Cannot save the current status")"
say "$(eval_gettext "Saved working directory and index state \$stash_msg")"
if test -z "$patch_mode"
then
- git reset --hard ${GIT_QUIET:+-q}
+ if test -n "$files"
+ then
+ git ls-files -z -- "$@" | xargs -0 git reset --
+ git ls-files -z --modified -- "$@" | xargs -0 git checkout HEAD --
+ git ls-files -z --others -- "$@" | xargs -0 git clean --force --
+ else
+ git reset --hard ${GIT_QUIET:+-q}
+ fi
test "$untracked" = "all" && CLEAN_X_OPTION=-x || CLEAN_X_OPTION=
if test -n "$untracked"
then
- git clean --force --quiet -d $CLEAN_X_OPTION
+ git clean --force --quiet -d $CLEAN_X_OPTION -- $files
fi
if test "$keep_index" = "t" && test -n "$i_tree"
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 34e9610bb6..ca4c44aa9c 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -802,4 +802,46 @@ test_expect_success 'new style stash create stores correct message' '
test_cmp expect actual
'
+test_expect_success 'stash -- <filename> stashes and restores the file' '
+ >foo &&
+ >bar &&
+ git add foo bar &&
+ git stash push -- foo &&
+ test_path_is_file bar &&
+ test_path_is_missing foo &&
+ git stash pop &&
+ test_path_is_file foo &&
+ test_path_is_file bar
+'
+
+test_expect_success 'stash with multiple filename arguments' '
+ >foo &&
+ >bar &&
+ >extra &&
+ git add foo bar extra &&
+ git stash push -- foo bar &&
+ test_path_is_missing bar &&
+ test_path_is_missing foo &&
+ test_path_is_file extra &&
+ git stash pop &&
+ test_path_is_file foo &&
+ test_path_is_file bar &&
+ test_path_is_file extra
+'
+
+test_expect_success 'stash with file including $IFS character' '
+ >"foo bar" &&
+ >foo &&
+ >untracked &&
+ git add foo* &&
+ git stash push -- foo* &&
+ test_path_is_missing "foo bar" &&
+ test_path_is_missing foo &&
+ test_path_is_file untracked &&
+ git stash pop &&
+ test_path_is_file "foo bar" &&
+ test_path_is_file foo &&
+ test_path_is_file untracked
+'
+
test_done
--
2.11.0.297.g9a2118ac0b.dirty
^ permalink raw reply related
* git push failing when push.recurseSubmodules on-demand and git commit --amend was used in submodule.
From: Carlo Wood @ 2017-01-29 19:33 UTC (permalink / raw)
To: git
Hi,
there seems to be a problem with using 'git commit --amend' in
git submodules when using 'git push --recurse-submodules=on-demand'
in the parent.
The latter fails, saying "The following submodule paths contain changes
that can not be found on any remote:" for such submodule, even though
the submodule is clean, pushed and reports 'Everything up-to-date'
when trying to push it.
I believe that the reason has to be that the parent repository thinks
that the comment that was amended, but not pushed, must be on the remote
too, while the whole point of amend is that this commit is not pushed.
I wrote a little script that demonstrates the problem.
Please run in an empty directory.
START-OF-SCRIPT
#! /bin/bash
# This script demonstrates a bug in git where it reports
#
# The following submodule paths contain changes that can
# not be found on any remote:
#
# for a submodule that is clean and pushed.
#
# Create an empty directory, put this script in it
# and run the script.
#
# Carlo Wood, 2017/01/29
# Clean a possible previous run:
rm -rf parent remote.parent remote.subm
REMOTE_BASE="$(pwd)"
# Create a 'remote' for the submodule 'subm'.
mkdir remote.subm
pushd remote.subm
git init --bare
popd
# Create a 'remote' for the 'parent' repository.
mkdir remote.parent
pushd remote.parent
git init --bare
popd
# Create initial parent/subm directory structore.
mkdir -p parent/subm
# Create an initial subm git repository.
pushd parent/subm
git init
git remote add local "$REMOTE_BASE/remote.subm"
touch s ; git add s
git commit -m 'Initial commit.'
git push --set-upstream local master
popd
# Create an initial parent git repository with subm as submodule
# and push.recurseSubmodules = on-demand.
pushd parent
git init
git config push.recurseSubmodules on-demand
git remote add local "$REMOTE_BASE/remote.parent"
touch p ; git add p
git submodule add "$REMOTE_BASE/remote.subm" subm
git add .gitmodules subm
git commit -m 'Initial commit.'
git push --set-upstream local master
popd
# Do some commit in subm, but do not push it to the remote.
pushd parent/subm
echo "My frist commit." > s
git commit -a -m 'Change s'
popd
# Add the subm hash to the parent.
pushd parent
git add subm
git commit -m 'Updated subm.'
popd
# Amend the commit in subm (and optionally push it).
pushd parent/subm
echo "My first commit." > s
git commit -a --amend -m 'Change s'
popd
# Correct that in the parent too:
pushd parent
git add subm
git commit -m 'Updated subm.'
popd
# At this point nothing was published yes, so the
# amend shouldn't have caused a problem. But it did.
pushd parent
git push
popd
echo "THE ABOVE ERROR CAN NOW BE REPRODUCED INDEFINITELY,"
echo "FOR EXAMPLE, DO:"
echo
echo "cd parent/subm"
echo "git push"
echo "cd .."
echo "git push"
END-OF-SCRIPT
Tested with current master 4e59582ff70d299f5a88449891e78d15b4b3fabe
Regards,
Carlo
--
Carlo Wood <carlo@alinoe.com>
^ permalink raw reply
* Re: git push failing when push.recurseSubmodules on-demand and git commit --amend was used in submodule.
From: Junio C Hamano @ 2017-01-30 1:00 UTC (permalink / raw)
To: Carlo Wood, Heiko Voigt, Stefan Beller; +Cc: git
In-Reply-To: <20170129203348.1a8c0722@hikaru>
Carlo Wood <carlo@alinoe.com> writes:
> there seems to be a problem with using 'git commit --amend' in
> git submodules when using 'git push --recurse-submodules=on-demand'
> in the parent.
>
> The latter fails, saying "The following submodule paths contain changes
> that can not be found on any remote:" for such submodule, even though
> the submodule is clean, pushed and reports 'Everything up-to-date'
> when trying to push it.
>
> I believe that the reason has to be that the parent repository thinks
> that the comment that was amended, but not pushed, must be on the remote
> too, while the whole point of amend is that this commit is not pushed.
I am not super familiar with the actualy implementation of the
codepaths involved in this, so CC'ed the folks who can help you
better.
I suspect the submodule folks would say it is working as intended,
if \
- you made a commit in the submodule;
- recorded the resulting commit in the superproject;
- you amended the commit in the submodule; and then
- you did "push, while pushing out in the submodule as needed" from
the superproject.
There are two commits in the submodule that are involved in the
above scenario, and the first one before amending is needed by the
other participants of the project in order for them to check out
what you are trying to push in the superproject, because that is
what the superproject's tree records. You somehow need to make that
commit available to them, but after you amended, the original commit
may no longer be reachable from any branch in your submodule, so
even if you (or the "on-demand" mechanism) pushed any and all
branches out, that would not make the needed commit available to
others. If you push your top-level superproject out in such a
situation, you would break others.
I think you have two options.
1. If the amend was done to improve things in submodule but is not
quite ready, then get rid of that amended commit and restore the
branch in the submodule to the state before you amended, i.e.
the tip of the branch will become the same commit as the one
that is recorded in the superproject. Then push the submodule
and the superproject out. After that, move the submodule branch
to point at the amended commit (or record the amended commit as
a child of the commit you pushed out).
2. If the amend is good and ready to go, "git add" to update the
superproject to make that amended result the one that is needed
in the submodule.
^ permalink raw reply
* [PATCH v3] mingw: allow hooks to be .exe files
From: Johannes Schindelin @ 2017-01-30 12:28 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Stefan Beller
In-Reply-To: <9a27b90e771d4c97dc580d344e161d7cf8d632ce.1485433248.git.johannes.schindelin@gmx.de>
Executable files in Windows need to have the extension '.exe', otherwise
they do not work. Extend the hooks to not just look at the hard coded
names, but also at the names extended by the custom STRIP_EXTENSION,
which is defined as '.exe' in Windows.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
Published-As: https://github.com/dscho/git/releases/tag/exe-as-hook-v3
Fetch-It-Via: git fetch https://github.com/dscho/git exe-as-hook-v3
run-command.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/run-command.c b/run-command.c
index 73bfba7ef9..5227f78aea 100644
--- a/run-command.c
+++ b/run-command.c
@@ -871,8 +871,14 @@ const char *find_hook(const char *name)
strbuf_reset(&path);
strbuf_git_path(&path, "hooks/%s", name);
- if (access(path.buf, X_OK) < 0)
+ if (access(path.buf, X_OK) < 0) {
+#ifdef STRIP_EXTENSION
+ strbuf_addstr(&path, STRIP_EXTENSION);
+ if (access(path.buf, X_OK) >= 0)
+ return path.buf;
+#endif
return NULL;
+ }
return path.buf;
}
base-commit: 4e59582ff70d299f5a88449891e78d15b4b3fabe
--
2.11.1.windows.prerelease.2.9.g3014b57
^ permalink raw reply related
* Re: [PATCH] mingw: allow hooks to be .exe files
From: Johannes Schindelin @ 2017-01-30 12:29 UTC (permalink / raw)
To: Stefan Beller; +Cc: Junio C Hamano, Jeff King, git@vger.kernel.org
In-Reply-To: <CAGZ79kaa5WJmZkyFROfkfNb3++t37qAuAebKJTon2iD2bh+sWw@mail.gmail.com>
Hi Stefan,
On Fri, 27 Jan 2017, Stefan Beller wrote:
> On Fri, Jan 27, 2017 at 2:29 AM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> > Hi Junio,
> >
> > On Thu, 26 Jan 2017, Junio C Hamano wrote:
> >
> >> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> >>
> >> > On Wed, 25 Jan 2017, Jeff King wrote:
> >> >
> >> > v2 coming,
>
> The commit message, while technically correct, seems a bit off. This is
> because the commit message only talks about .exe extensions, but the
> change in code doesn't even have the string "exe" mentioned once.
>
> With this discussion here, it is easy for me to connect the dots, but it
> would be nice to have the full picture in the commit message.
I just sent out v3, using a slightly tweaked version of the commit message
you proposed.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] fixup! worktree move: new command
From: Johannes Schindelin @ 2017-01-30 12:24 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Nguyễn Thái Ngọc Duy
In-Reply-To: <xmqqwpdgz8zq.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Fri, 27 Jan 2017, Junio C Hamano wrote:
> The tip of 'pu' (or anything beyond the tip of 'jch') is not always
> expected to pass test or even build, [...]
This makes `pu` a lot less useful than it could be.
And we could easily improve the situation simply by changing the rule ever
so slightly: when a build, or a test, fails in `pu` and there exists a
fix, this fix should go into `pu` ASAP.
As you point out later in your mail, the fixup! or SQUASH! commit is a
very convenient reminder that a particular branch is still "under
construction". That is, changing the rule as I proposed above will not
only help the Continuous Integration [*1*] to avoid reporting duplicates,
it will also help us improve the project faster.
Ciao,
Johannes
Footnote *1*: It appears that there may be the misconception floating
around that Continuous Integration is designed to annoy developers by
pointing out unportable or unbuildable code. Once you realize, though,
that it detects and reports code that is below our existing code's
quality, no doubt you will agree that it is a convenient tool to relieve
reviewers from tedious work that can be done by a machine as well.
^ permalink raw reply
* [PATCH v2] help: improve is_executable() on Windows
From: Johannes Schindelin @ 2017-01-30 12:40 UTC (permalink / raw)
To: git; +Cc: Heiko Voigt, Junio C Hamano
In-Reply-To: <c1c6ccae4e60608259809914e8ff3d3d5e1ead5a.1485524999.git.johannes.schindelin@gmx.de>
From: Heiko Voigt <hvoigt@hvoigt.net>
On Windows, executables need to have the file extension `.exe`, or they
are not executables. Hence, to support scripts, Git for Windows also
looks for a she-bang line by opening the file in question, and executing
it via the specified script interpreter.
To figure out whether files in the `PATH` are executable, `git help` has
code that imitates this behavior. With one exception: it *always* opens
the files and looks for a she-bang line *or* an `MZ` tell-tale
(nevermind that files with the magic `MZ` but without file extension
`.exe` would still not be executable).
Opening this many files leads to performance problems that are even more
serious when a virus scanner is running. Therefore, let's change the
code to look for the file extension `.exe` early, and avoid opening the
file altogether if we already know that it is executable.
See the following measurements (in seconds) as an example, where we
execute a simple program that simply lists the directory contents and
calls open() on every listed file:
With virus scanner running (coldcache):
$ ./a.exe /libexec/git-core/
before open (git-add.exe): 0.000000
after open (git-add.exe): 0.412873
before open (git-annotate.exe): 0.000175
after open (git-annotate.exe): 0.397925
before open (git-apply.exe): 0.000243
after open (git-apply.exe): 0.399996
before open (git-archive.exe): 0.000147
after open (git-archive.exe): 0.397783
before open (git-bisect--helper.exe): 0.000160
after open (git-bisect--helper.exe): 0.397700
before open (git-blame.exe): 0.000160
after open (git-blame.exe): 0.399136
...
With virus scanner running (hotcache):
$ ./a.exe /libexec/git-core/
before open (git-add.exe): 0.000000
after open (git-add.exe): 0.000325
before open (git-annotate.exe): 0.000229
after open (git-annotate.exe): 0.000177
before open (git-apply.exe): 0.000167
after open (git-apply.exe): 0.000150
before open (git-archive.exe): 0.000154
after open (git-archive.exe): 0.000156
before open (git-bisect--helper.exe): 0.000132
after open (git-bisect--helper.exe): 0.000180
before open (git-blame.exe): 0.000718
after open (git-blame.exe): 0.000724
...
With this patch I get:
$ time git help git
Launching default browser to display HTML ...
real 0m8.723s
user 0m0.000s
sys 0m0.000s
and without
$ time git help git
Launching default browser to display HTML ...
real 1m37.734s
user 0m0.000s
sys 0m0.031s
both tests with cold cache and giving the machine some time to settle
down after restart.
[jes: adjusted the commit message]
Signed-off-by: Heiko Voigt <heiko.voigt@mahr.de>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
Published-As: https://github.com/dscho/git/releases/tag/help-is-exe-v2
Fetch-It-Via: git fetch https://github.com/dscho/git help-is-exe-v2
help.c | 21 ++++++++++++++++++---
1 file changed, 18 insertions(+), 3 deletions(-)
diff --git a/help.c b/help.c
index 53e2a67e00..bc6cd19cf3 100644
--- a/help.c
+++ b/help.c
@@ -105,7 +105,22 @@ static int is_executable(const char *name)
return 0;
#if defined(GIT_WINDOWS_NATIVE)
-{ /* cannot trust the executable bit, peek into the file instead */
+ /*
+ * On Windows there is no executable bit. The file extension
+ * indicates whether it can be run as an executable, and Git
+ * has special-handling to detect scripts and launch them
+ * through the indicated script interpreter. We test for the
+ * file extension first because virus scanners may make
+ * it quite expensive to open many files.
+ */
+ if (ends_with(name, ".exe"))
+ return S_IXUSR;
+
+{
+ /*
+ * Now that we know it does not have an executable extension,
+ * peek into the file instead.
+ */
char buf[3] = { 0 };
int n;
int fd = open(name, O_RDONLY);
@@ -113,8 +128,8 @@ static int is_executable(const char *name)
if (fd >= 0) {
n = read(fd, buf, 2);
if (n == 2)
- /* DOS executables start with "MZ" */
- if (!strcmp(buf, "#!") || !strcmp(buf, "MZ"))
+ /* look for a she-bang */
+ if (!strcmp(buf, "#!"))
st.st_mode |= S_IXUSR;
close(fd);
}
base-commit: 4e59582ff70d299f5a88449891e78d15b4b3fabe
--
2.11.1.windows.prerelease.2.9.g3014b57
^ permalink raw reply related
* Re: [PATCH] checkout: convert post_checkout_hook() to struct object_id
From: Johannes Schindelin @ 2017-01-30 13:01 UTC (permalink / raw)
To: René Scharfe; +Cc: Git List, Junio C Hamano, brian m. carlson
In-Reply-To: <b30e5d34-436a-af5f-dbad-b1df464bf303@web.de>
[-- Attachment #1: Type: text/plain, Size: 180 bytes --]
Hi René,
On Sat, 28 Jan 2017, René Scharfe wrote:
> Signed-off-by: Rene Scharfe <l.s.r@web.de>
These three SHA-1 -> OID patches all appear correct to me.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] help: correct behavior for is_executable on Windows
From: Johannes Schindelin @ 2017-01-30 12:44 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Heiko Voigt
In-Reply-To: <xmqqd1f8z6lt.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Fri, 27 Jan 2017, Junio C Hamano wrote:
> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
>
> > From: Heiko Voigt <hvoigt@hvoigt.net>
> >
> > The previous implementation said that the filesystem information on
> > Windows is not reliable to determine whether a file is executable. To
> > gather this information it was peeking into the first two bytes of a
> > file to see whether it looks executable.
> >
> > Apart from the fact that on Windows executables are defined as such by
> > their extension (and Git has special code to support "executing"
> > scripts when they have a she-bang line) it leads to serious
> > performance problems: not only do we have to open many files now, it
> > gets even slower when a virus scanner is running.
>
> Heiko, around here (before going into the details of how severe the
> problem is and how wonderful the result applying of this change is) is
> the best place to summarize the solution. E.g.
>
> Because the definition of "executable" on Windows is based
> on the file extension, update the function to declare that a
> file with ".exe" extension without opening and reading the
> early bytes from it. This avoids serious performance issues.
>
> I paraphrased the rest only so that the description of the solution
> (i.e. "instead of opening and peeking, we trust .exe suffix") fits well
> in the surrounding text; the important part is to say what the change
> does clearly.
I adjusted the commit message. It was tweaked a little differently from
what you suggested, as I preferred to condense the information a bit more.
> I agree with the reasoning and the execution of the patch, except
> that
>
> - "correct behaviour" in the title makes it appear that this is a
> correctness thing, but this is primarily a performance fix.
Primarily. But not only. The magic `MZ` without the file extension `.exe`
is pretty useless, as the file could not be executed, still.
To avoid further turnaround, though, I also edited the contentious
"correct" to read "improve" now.
> - It is a bit strange that "MZ" is dropped in the same patch
> without any mention.
I fixed that in the commit message.
Ciao,
Johannes
^ permalink raw reply
* Re: [PATCH v4] t/Makefile: add a rule to re-run previously-failed tests
From: Johannes Schindelin @ 2017-01-30 15:35 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Sverre Rabbelier,
Ævar Arnfjörð Bjarmason, Matthieu Moy
In-Reply-To: <xmqq4m0kz65d.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Fri, 27 Jan 2017, Junio C Hamano wrote:
> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
>
> > This patch automates the process of determining which tests failed
> > previously and re-running them.
> > ...
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> I stored both versions in files and compared them, and it seems the
> single word change in the proposed commit log message is the only
> difference. I would have written "Automate the process...", though.
Yes, we have different styles. Thanks for letting my commit keep my commit
message this time ;-)
> If you are resending, touching up to cover all points raised by a
> reviewer and doing nothing else, having "Reviewed-by: Jeff King
> <peff@peff.net>" would have been nicer.
TBH I am not at all sure that I know when to add those footers and when
not. After having been asked to remove such a footer, I decided to *not*
include them by default.
Having gray zones about the footers strikes me as similar to having gray
zones in the coding style guidelines: it sure gives the contributors more
freedom, but it also creates uncertainty and as a consequence takes up a
lot of reviewing space and time (hence taking away space and time from
reviewing the code for bugs).
In other words: while I appreciate the idea of giving contributors such as
myself a lot of leeway, I would love even more to be able to automate away
tedious and boring tasks (such as adding Tested-by: or Reviewed-by:
footers, or for that matter, addressing code style issues before any
reviewer has to shed bikes so that they can focus on the parts of the
review that no machine can do for them).
Ciao,
Johannes
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox