* Re: SubmittingPatches: drop temporal reference for PGP signing
From: Philip Oakley @ 2017-01-25 6:54 UTC (permalink / raw)
To: Stefan Beller, cornelius.weig
Cc: j6t, bitte.keine.werbung.einwerfen, git, gitster, thomas.braun,
john, Stefan Beller
In-Reply-To: <20170125002116.22111-1-sbeller@google.com>
From: "Stefan Beller" <sbeller@google.com>
>>
>>> Do not PGP sign your patch, at least *for now*. (...)
>>
>
> And maybe these 2 small words are the bug in the documentation?
> Shall we drop the "at least for now" part, like so:
>
> ---8<---
> From 2c4fe0e67451892186ff6257b20c53e088c9ec67 Mon Sep 17 00:00:00 2001
> From: Stefan Beller <sbeller@google.com>
> Date: Tue, 24 Jan 2017 16:19:13 -0800
> Subject: [PATCH] SubmittingPatches: drop temporal reference for PGP
> signing
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
> Documentation/SubmittingPatches | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/SubmittingPatches
> b/Documentation/SubmittingPatches
> index 08352deaae..28da4ad2d4 100644
> --- a/Documentation/SubmittingPatches
> +++ b/Documentation/SubmittingPatches
> @@ -216,12 +216,12 @@ that it will be postponed.
> Exception: If your mailer is mangling patches then someone may ask
> you to re-send them using MIME, that is OK.
>
> -Do not PGP sign your patch, at least for now. Most likely, your
> -maintainer or other people on the list would not have your PGP
> -key and would not bother obtaining it anyway. Your patch is not
> -judged by who you are; a good patch from an unknown origin has a
> -far better chance of being accepted than a patch from a known,
> -respected origin that is done poorly or does incorrect things.
> +Do not PGP sign your patch. Most likely, your maintainer or other
> +people on the list would not have your PGP key and would not bother
> +obtaining it anyway. Your patch is not judged by who you are; a good
> +patch from an unknown origin has a far better chance of being accepted
> +than a patch from a known, respected origin that is done poorly or
> +does incorrect things.
Wouldn't this also benefit from a forward reference to the section 5 on the
DOC signining? This would avoid Cornelius's case where he felt that section
5 no longer applied.
> If you really really really really want to do a PGP signed
> patch, format it as "multipart/signed", not a text/plain message
> --
> 2.11.0.495.g04f60290a0.dirty
--
Philip
^ permalink raw reply
* [PATCH v1 3/3] blame: add --color option
From: Edmundo Carmona Antoranz @ 2017-01-25 5:27 UTC (permalink / raw)
To: git; +Cc: apelisse, kevin, gitster, peff, Edmundo Carmona Antoranz
In-Reply-To: <20170125052734.17550-1-eantoranz@gmail.com>
Revision information will be highlighted so that it's easier
to tell from content of the file being "blamed".
Signed-off-by: Edmundo Carmona Antoranz <eantoranz@gmail.com>
---
Documentation/blame-options.txt | 5 +++++
Documentation/git-blame.txt | 2 +-
builtin/blame.c | 13 +++++++++++++
3 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt
index 64858a631..4abb4eb7e 100644
--- a/Documentation/blame-options.txt
+++ b/Documentation/blame-options.txt
@@ -57,6 +57,11 @@ include::line-range-format.txt[]
Show revision hints on aggregated information about the revision.
Implies --aggregate.
+--[no-]color::
+ Revision information will be highlighted on normal output
+ when running git from a terminal. This option allows
+ for color to be forcibly enabled or disabled.
+
--encoding=<encoding>::
Specifies the encoding used to output author names
and commit summaries. Setting it to `none` makes blame
diff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt
index ed8b119fa..d25cbc5ef 100644
--- a/Documentation/git-blame.txt
+++ b/Documentation/git-blame.txt
@@ -10,7 +10,7 @@ SYNOPSIS
[verse]
'git blame' [-c] [-b] [-l] [--root] [-t] [-f] [-n] [-s] [-e] [-p] [-w] [--incremental]
[-L <range>] [-S <revs-file>] [-M] [-C] [-C] [-C] [--since=<date>]
- [--progress] [--abbrev=<n>] [--aggregate] [--hint]
+ [--progress] [--abbrev=<n>] [--aggregate] [--hint] [--color]
[<rev> | --contents <file> | --reverse <rev>..<rev>] [--] <file>
DESCRIPTION
diff --git a/builtin/blame.c b/builtin/blame.c
index 7473b50e9..c661dd538 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1886,6 +1886,7 @@ static const char *format_time(unsigned long time, const char *tz_str,
#define OUTPUT_LINE_PORCELAIN 01000
#define OUTPUT_AGGREGATE 02000
#define OUTPUT_HINT 04000
+#define OUTPUT_COLOR 010000
static void emit_porcelain_details(struct origin *suspect, int repeat)
{
@@ -1943,6 +1944,8 @@ static void emit_porcelain(struct scoreboard *sb, struct blame_entry *ent,
static void print_revision_info(char* revision_hex, int revision_length, struct blame_entry* ent,
struct commit_info ci, int opt, int show_raw_time, int line_number)
{
+ if (opt & OUTPUT_COLOR)
+ printf("%s", GIT_COLOR_BOLD);
if (!line_number)
printf("\t");
int length = revision_length;
@@ -2008,6 +2011,8 @@ static void print_revision_info(char* revision_hex, int revision_length, struct
printf(" %s", ci.summary.buf);
printf("\n");
}
+ if (opt & OUTPUT_COLOR)
+ printf("%s", GIT_COLOR_RESET);
}
static void emit_other(struct scoreboard *sb, struct blame_entry *ent, int opt)
@@ -2608,6 +2613,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
char *final_commit_name = NULL;
enum object_type type;
struct commit *final_commit = NULL;
+ int show_color;
struct string_list range_list = STRING_LIST_INIT_NODUP;
int output_option = 0, opt = 0;
@@ -2647,6 +2653,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
OPT_STRING_LIST('L', NULL, &range_list, N_("n,m"), N_("Process only line range n,m, counting from 1")),
OPT_BIT(0, "aggregate", &output_option, N_("Aggregate output"), OUTPUT_AGGREGATE),
OPT_BIT(0, "hint", &output_option, N_("Show revision hints"), OUTPUT_HINT),
+ OPT_BOOL(0, "color", &show_color, N_("Show colors on revision information")),
OPT__ABBREV(&abbrev),
OPT_END()
};
@@ -2666,6 +2673,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
save_commit_buffer = 0;
dashdash_pos = 0;
show_progress = -1;
+ show_color = -1;
parse_options_start(&ctx, argc, argv, prefix, options,
PARSE_OPT_KEEP_DASHDASH | PARSE_OPT_KEEP_ARGV0);
@@ -2698,6 +2706,11 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
} else if (show_progress < 0)
show_progress = isatty(2);
+ if (show_color < 0)
+ show_color = isatty(1);
+ if (show_color)
+ output_option = output_option | OUTPUT_COLOR;
+
if (0 < abbrev && abbrev < GIT_SHA1_HEXSZ)
/* one more abbrev length is needed for the boundary commit */
abbrev++;
--
2.11.0.rc1
^ permalink raw reply related
* [PATCH v1 2/3] blame: add --hint option
From: Edmundo Carmona Antoranz @ 2017-01-25 5:27 UTC (permalink / raw)
To: git; +Cc: apelisse, kevin, gitster, peff, Edmundo Carmona Antoranz
In-Reply-To: <20170125052734.17550-1-eantoranz@gmail.com>
When printing aggregate information about revisions, this option
allows to add the 1-line summary of the revision to provide the
reader with some additional context about the revision itself.
Signed-off-by: Edmundo Carmona Antoranz <eantoranz@gmail.com>
---
Documentation/blame-options.txt | 4 ++++
Documentation/git-blame.txt | 2 +-
builtin/blame.c | 13 +++++++++++--
3 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt
index be2a327ff..64858a631 100644
--- a/Documentation/blame-options.txt
+++ b/Documentation/blame-options.txt
@@ -53,6 +53,10 @@ include::line-range-format.txt[]
Aggregate information about revisions. This option makes no
difference on incremental or porcelain format.
+--hint::
+ Show revision hints on aggregated information about the revision.
+ Implies --aggregate.
+
--encoding=<encoding>::
Specifies the encoding used to output author names
and commit summaries. Setting it to `none` makes blame
diff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt
index 385eaf7be..ed8b119fa 100644
--- a/Documentation/git-blame.txt
+++ b/Documentation/git-blame.txt
@@ -10,7 +10,7 @@ SYNOPSIS
[verse]
'git blame' [-c] [-b] [-l] [--root] [-t] [-f] [-n] [-s] [-e] [-p] [-w] [--incremental]
[-L <range>] [-S <revs-file>] [-M] [-C] [-C] [-C] [--since=<date>]
- [--progress] [--abbrev=<n>] [--aggregate]
+ [--progress] [--abbrev=<n>] [--aggregate] [--hint]
[<rev> | --contents <file> | --reverse <rev>..<rev>] [--] <file>
DESCRIPTION
diff --git a/builtin/blame.c b/builtin/blame.c
index 09b3b0c8a..7473b50e9 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1885,6 +1885,7 @@ static const char *format_time(unsigned long time, const char *tz_str,
#define OUTPUT_SHOW_EMAIL 0400
#define OUTPUT_LINE_PORCELAIN 01000
#define OUTPUT_AGGREGATE 02000
+#define OUTPUT_HINT 04000
static void emit_porcelain_details(struct origin *suspect, int repeat)
{
@@ -2001,8 +2002,12 @@ static void print_revision_info(char* revision_hex, int revision_length, struct
if (line_number && (opt & OUTPUT_AGGREGATE))
printf(opt & OUTPUT_ANNOTATE_COMPAT ? "%*d)" : " %*d) ",
max_digits, line_number);
- if (!line_number)
- printf(")\n");
+ if (!line_number) {
+ printf(")");
+ if (opt & OUTPUT_HINT)
+ printf(" %s", ci.summary.buf);
+ printf("\n");
+ }
}
static void emit_other(struct scoreboard *sb, struct blame_entry *ent, int opt)
@@ -2018,6 +2023,9 @@ static void emit_other(struct scoreboard *sb, struct blame_entry *ent, int opt)
get_commit_info(suspect->commit, &ci, 1);
sha1_to_hex_r(hex, suspect->commit->object.oid.hash);
+ if (~(opt & OUTPUT_AGGREGATE) & (opt & OUTPUT_HINT))
+ opt=opt|OUTPUT_AGGREGATE;
+
if (opt & OUTPUT_AGGREGATE)
print_revision_info(hex, revision_length, ent, ci, opt, show_raw_time, 0);
@@ -2638,6 +2646,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
{ OPTION_CALLBACK, 'M', NULL, &opt, N_("score"), N_("Find line movements within and across files"), PARSE_OPT_OPTARG, blame_move_callback },
OPT_STRING_LIST('L', NULL, &range_list, N_("n,m"), N_("Process only line range n,m, counting from 1")),
OPT_BIT(0, "aggregate", &output_option, N_("Aggregate output"), OUTPUT_AGGREGATE),
+ OPT_BIT(0, "hint", &output_option, N_("Show revision hints"), OUTPUT_HINT),
OPT__ABBREV(&abbrev),
OPT_END()
};
--
2.11.0.rc1
^ permalink raw reply related
* [PATCH v1 1/3] blame: add --aggregate option
From: Edmundo Carmona Antoranz @ 2017-01-25 5:27 UTC (permalink / raw)
To: git; +Cc: apelisse, kevin, gitster, peff, Edmundo Carmona Antoranz
To avoid taking up so much space on normal output printing duplicate
information on consecutive lines that "belong" to the same revision,
this option allows to print a single line with the information about
the revision before the lines of content themselves.
Signed-off-by: Edmundo Carmona Antoranz <eantoranz@gmail.com>
---
Documentation/blame-options.txt | 4 ++
Documentation/git-blame.txt | 4 +-
builtin/blame.c | 85 +++++++++++++++++++++++++++--------------
3 files changed, 63 insertions(+), 30 deletions(-)
diff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt
index 2669b87c9..be2a327ff 100644
--- a/Documentation/blame-options.txt
+++ b/Documentation/blame-options.txt
@@ -49,6 +49,10 @@ include::line-range-format.txt[]
Show the result incrementally in a format designed for
machine consumption.
+--aggregate::
+ Aggregate information about revisions. This option makes no
+ difference on incremental or porcelain format.
+
--encoding=<encoding>::
Specifies the encoding used to output author names
and commit summaries. Setting it to `none` makes blame
diff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt
index fdc3aea30..385eaf7be 100644
--- a/Documentation/git-blame.txt
+++ b/Documentation/git-blame.txt
@@ -10,8 +10,8 @@ SYNOPSIS
[verse]
'git blame' [-c] [-b] [-l] [--root] [-t] [-f] [-n] [-s] [-e] [-p] [-w] [--incremental]
[-L <range>] [-S <revs-file>] [-M] [-C] [-C] [-C] [--since=<date>]
- [--progress] [--abbrev=<n>] [<rev> | --contents <file> | --reverse <rev>..<rev>]
- [--] <file>
+ [--progress] [--abbrev=<n>] [--aggregate]
+ [<rev> | --contents <file> | --reverse <rev>..<rev>] [--] <file>
DESCRIPTION
-----------
diff --git a/builtin/blame.c b/builtin/blame.c
index 126b8c9e5..09b3b0c8a 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1884,6 +1884,7 @@ static const char *format_time(unsigned long time, const char *tz_str,
#define OUTPUT_NO_AUTHOR 0200
#define OUTPUT_SHOW_EMAIL 0400
#define OUTPUT_LINE_PORCELAIN 01000
+#define OUTPUT_AGGREGATE 02000
static void emit_porcelain_details(struct origin *suspect, int repeat)
{
@@ -1931,43 +1932,41 @@ static void emit_porcelain(struct scoreboard *sb, struct blame_entry *ent,
putchar('\n');
}
-static void emit_other(struct scoreboard *sb, struct blame_entry *ent, int opt)
-{
- int cnt;
- const char *cp;
- struct origin *suspect = ent->suspect;
- struct commit_info ci;
- char hex[GIT_SHA1_HEXSZ + 1];
- 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);
-
- cp = nth_line(sb, ent->lno);
- for (cnt = 0; cnt < ent->num_lines; cnt++) {
- char ch;
- int length = (opt & OUTPUT_LONG_OBJECT_NAME) ? GIT_SHA1_HEXSZ : abbrev;
-
- if (suspect->commit->object.flags & UNINTERESTING) {
+/**
+ * Print information about the revision.
+ * This information can be used in either aggregated output
+ * or prepending each line of the content of the file being blamed
+ *
+ * if line_number == 0, it's an aggregate line
+ */
+static void print_revision_info(char* revision_hex, int revision_length, struct blame_entry* ent,
+ struct commit_info ci, int opt, int show_raw_time, int line_number)
+{
+ if (!line_number)
+ printf("\t");
+ int length = revision_length;
+ if (!line_number || !(opt & OUTPUT_AGGREGATE)) {
+ if (ent->suspect->commit->object.flags & UNINTERESTING) {
if (blank_boundary)
- memset(hex, ' ', length);
+ memset(revision_hex, ' ', length);
else if (!(opt & OUTPUT_ANNOTATE_COMPAT)) {
length--;
putchar('^');
}
}
- printf("%.*s", length, hex);
+ printf("%.*s", length, revision_hex);
if (opt & OUTPUT_ANNOTATE_COMPAT) {
const char *name;
if (opt & OUTPUT_SHOW_EMAIL)
name = ci.author_mail.buf;
else
name = ci.author.buf;
- printf("\t(%10s\t%10s\t%d)", name,
+ printf("\t(%10s\t%10s", name,
format_time(ci.author_time, ci.author_tz.buf,
- show_raw_time),
- ent->lno + 1 + cnt);
+ show_raw_time));
+ if (line_number)
+ printf("\t%d)", line_number);
} else {
if (opt & OUTPUT_SHOW_SCORE)
printf(" %*d %02d",
@@ -1975,10 +1974,10 @@ static void emit_other(struct scoreboard *sb, struct blame_entry *ent, int opt)
ent->suspect->refcnt);
if (opt & OUTPUT_SHOW_NAME)
printf(" %-*.*s", longest_file, longest_file,
- suspect->path);
- if (opt & OUTPUT_SHOW_NUMBER)
+ ent->suspect->path);
+ if ((opt & OUTPUT_SHOW_NUMBER) && line_number)
printf(" %*d", max_orig_digits,
- ent->s_lno + 1 + cnt);
+ line_number);
if (!(opt & OUTPUT_NO_AUTHOR)) {
const char *name;
@@ -1994,9 +1993,38 @@ static void emit_other(struct scoreboard *sb, struct blame_entry *ent, int opt)
ci.author_tz.buf,
show_raw_time));
}
- printf(" %*d) ",
- max_digits, ent->lno + 1 + cnt);
+ if (line_number)
+ printf(" %*d) ",
+ max_digits, line_number);
}
+ }
+ if (line_number && (opt & OUTPUT_AGGREGATE))
+ printf(opt & OUTPUT_ANNOTATE_COMPAT ? "%*d)" : " %*d) ",
+ max_digits, line_number);
+ if (!line_number)
+ printf(")\n");
+}
+
+static void emit_other(struct scoreboard *sb, struct blame_entry *ent, int opt)
+{
+ int cnt;
+ const char *cp;
+ struct origin *suspect = ent->suspect;
+ struct commit_info ci;
+ char hex[GIT_SHA1_HEXSZ + 1];
+ int show_raw_time = !!(opt & OUTPUT_RAW_TIMESTAMP);
+ int revision_length = (opt & OUTPUT_LONG_OBJECT_NAME) ? GIT_SHA1_HEXSZ : abbrev;
+
+ get_commit_info(suspect->commit, &ci, 1);
+ sha1_to_hex_r(hex, suspect->commit->object.oid.hash);
+
+ if (opt & OUTPUT_AGGREGATE)
+ print_revision_info(hex, revision_length, ent, ci, opt, show_raw_time, 0);
+
+ cp = nth_line(sb, ent->lno);
+ char ch;
+ for (cnt = 0; cnt < ent->num_lines; cnt++) {
+ print_revision_info(hex, revision_length, ent, ci, opt, show_raw_time, ent->lno + 1 + cnt);
do {
ch = *cp++;
putchar(ch);
@@ -2609,6 +2637,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
{ OPTION_CALLBACK, 'C', NULL, &opt, N_("score"), N_("Find line copies within and across files"), PARSE_OPT_OPTARG, blame_copy_callback },
{ OPTION_CALLBACK, 'M', NULL, &opt, N_("score"), N_("Find line movements within and across files"), PARSE_OPT_OPTARG, blame_move_callback },
OPT_STRING_LIST('L', NULL, &range_list, N_("n,m"), N_("Process only line range n,m, counting from 1")),
+ OPT_BIT(0, "aggregate", &output_option, N_("Aggregate output"), OUTPUT_AGGREGATE),
OPT__ABBREV(&abbrev),
OPT_END()
};
--
2.11.0.rc1
^ permalink raw reply related
* Re: [PATCH] tag: add tag.createReflog option
From: Pranit Bauva @ 2017-01-25 5:06 UTC (permalink / raw)
To: cornelius.weig; +Cc: Git List, Jeff King, novalis, Duy Nguyen
In-Reply-To: <20170125001906.13916-1-cornelius.weig@tngtech.com>
Hey Cornelius,
On Wed, Jan 25, 2017 at 5:49 AM, <cornelius.weig@tngtech.com> wrote:
> From: Cornelius Weig <cornelius.weig@tngtech.com>
>
> Git does not create a history for tags, in contrast to common
> expectation to simply version everything. This can be changed by using
> the `--create-reflog` flag when creating the tag. However, a config
> option to enable this behavior by default is missing.
>
> This commit adds the configuration variable `tag.createReflog` which
> enables reflogs for new tags by default.
>
> Signed-off-by: Cornelius Weig <cornelius.weig@tngtech.com>
You have also added the option --no-create-reflog so would it be worth
to mention it in the commit message?
> ---
> Documentation/config.txt | 5 +++++
> Documentation/git-tag.txt | 8 +++++---
> builtin/tag.c | 6 +++++-
> t/t7004-tag.sh | 14 ++++++++++++++
> 4 files changed, 29 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index af2ae4c..9e5f6f6 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -2945,6 +2945,11 @@ submodule.alternateErrorStrategy
> as computed via `submodule.alternateLocation`. Possible values are
> `ignore`, `info`, `die`. Default is `die`.
>
> +tag.createReflog::
> + A boolean to specify whether newly created tags should have a reflog.
> + If `--[no-]create-reflog` is specified on the command line, it takes
> + precedence. Defaults to `false`.
This follows the convention, good! :)
> tag.forceSignAnnotated::
> A boolean to specify whether annotated tags created should be GPG signed.
> If `--annotate` is specified on the command line, it takes
> diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
> index 5055a96..f2ed370 100644
> --- a/Documentation/git-tag.txt
> +++ b/Documentation/git-tag.txt
> @@ -13,7 +13,7 @@ SYNOPSIS
> <tagname> [<commit> | <object>]
> 'git tag' -d <tagname>...
> 'git tag' [-n[<num>]] -l [--contains <commit>] [--points-at <object>]
> - [--column[=<options>] | --no-column] [--create-reflog] [--sort=<key>]
> + [--column[=<options>] | --no-column] [--[no-]create-reflog] [--sort=<key>]
> [--format=<format>] [--[no-]merged [<commit>]] [<pattern>...]
> 'git tag' -v <tagname>...
>
> @@ -149,8 +149,10 @@ This option is only applicable when listing tags without annotation lines.
> all, 'whitespace' removes just leading/trailing whitespace lines and
> 'strip' removes both whitespace and commentary.
>
> ---create-reflog::
> - Create a reflog for the tag.
> +--[no-]create-reflog::
> + Force to create a reflog for the tag, or no reflog if `--no-create-reflog`
> + is used. Unless the `tag.createReflog` config variable is set to true, no
> + reflog is created by default. See linkgit:git-config[1].
>
> <tagname>::
> The name of the tag to create, delete, or describe.
> diff --git a/builtin/tag.c b/builtin/tag.c
> index 73df728..1f13e4d 100644
> --- a/builtin/tag.c
> +++ b/builtin/tag.c
> @@ -30,6 +30,7 @@ static const char * const git_tag_usage[] = {
>
> static unsigned int colopts;
> static int force_sign_annotate;
> +static int create_reflog;
>
> static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, const char *format)
> {
> @@ -165,6 +166,10 @@ static int git_tag_config(const char *var, const char *value, void *cb)
> force_sign_annotate = git_config_bool(var, value);
> return 0;
> }
> + if (!strcmp(var, "tag.createreflog")) {
> + create_reflog = git_config_bool(var, value);
> + return 0;
> + }
>
> if (starts_with(var, "column."))
> return git_column_config(var, value, "tag", &colopts);
> @@ -325,7 +330,6 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
> const char *object_ref, *tag;
> struct create_tag_options opt;
> char *cleanup_arg = NULL;
> - int create_reflog = 0;
> int annotate = 0, force = 0;
> int cmdmode = 0, create_tag_object = 0;
> const char *msgfile = NULL, *keyid = NULL;
> diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
> index 1cfa8a2..67b39ec 100755
> --- a/t/t7004-tag.sh
> +++ b/t/t7004-tag.sh
> @@ -90,6 +90,20 @@ test_expect_success '--create-reflog does not create reflog on failure' '
> test_must_fail git reflog exists refs/tags/mytag
> '
>
> +test_expect_success 'option tag.createreflog creates reflog by default' '
> + test_when_finished "git tag -d tag_with_reflog" &&
> + git config tag.createReflog true &&
> + git tag tag_with_reflog &&
> + git reflog exists refs/tags/tag_with_reflog
> +'
> +
> +test_expect_success 'option tag.createreflog overridden by command line' '
> + test_when_finished "git tag -d tag_without_reflog" &&
> + git config tag.createReflog true &&
> + git tag --no-create-reflog tag_without_reflog &&
> + test_must_fail git reflog exists refs/tags/tag_without_reflog
> +'
> +
> test_expect_success 'listing all tags if one exists should succeed' '
> git tag -l &&
> git tag
> --
> 2.10.2
>
^ permalink raw reply
* [PATCH] gpg-interface: Add some output from gpg when it errors out.
From: Mike Hommey @ 2017-01-25 3:04 UTC (permalink / raw)
To: gitster; +Cc: git
When e.g. signing a tag fails, the user is left with the following
output on their console:
error: gpg failed to sign the data
error: unable to sign the tag
There is no indication of what specifically failed, and no indication
how they might solve the problem.
It turns out, gpg still does output some messages without a [GNUPG:]
prefix. The same messages it outputs when run standalone, in fact.
Those messages can be helpful to find what made the gpg command fail.
For instance, after changing my laptop for a new one, I copied my
configs, but had some environment differences that broke gpg.
With this change applied, the output becomes, on this new machine:
gpg: keyblock resource '/usr/share/keyrings/debian-keyring.gpg': No
such file or directory
error: gpg failed to sign the data
error: unable to sign the tag
which makes it clearer what's wrong.
Signed-off-by: Mike Hommey <mh@glandium.org>
---
gpg-interface.c | 28 ++++++++++++++++++++++++----
1 file changed, 24 insertions(+), 4 deletions(-)
diff --git a/gpg-interface.c b/gpg-interface.c
index e44cc27da..2768bb307 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -149,6 +149,26 @@ const char *get_signing_key(void)
return git_committer_info(IDENT_STRICT|IDENT_NO_DATE);
}
+static int pipe_gpg_command(struct child_process *cmd,
+ const char *in, size_t in_len,
+ struct strbuf *out, size_t out_hint,
+ struct strbuf *err, size_t err_hint)
+{
+ int ret = pipe_command(cmd, in, in_len, out, out_hint, err, err_hint);
+ /* Print out any line that doesn't start with [GNUPG:] if the gpg
+ * command failed. */
+ if (ret) {
+ struct strbuf **err_lines = strbuf_split(err, '\n');
+ for (struct strbuf **line = err_lines; *line; line++) {
+ if (memcmp((*line)->buf, "[GNUPG:]", 8)) {
+ strbuf_rtrim(*line);
+ fprintf(stderr, "%s\n", (*line)->buf);
+ }
+ }
+ strbuf_list_free(err_lines);
+ }
+ return ret;
+}
/*
* Create a detached signature for the contents of "buffer" and append
* it after "signature"; "buffer" and "signature" can be the same
@@ -175,8 +195,8 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *sig
* because gpg exits without reading and then write gets SIGPIPE.
*/
sigchain_push(SIGPIPE, SIG_IGN);
- ret = pipe_command(&gpg, buffer->buf, buffer->len,
- signature, 1024, &gpg_status, 0);
+ ret = pipe_gpg_command(&gpg, buffer->buf, buffer->len,
+ signature, 1024, &gpg_status, 0);
sigchain_pop(SIGPIPE);
ret |= !strstr(gpg_status.buf, "\n[GNUPG:] SIG_CREATED ");
@@ -232,8 +252,8 @@ int verify_signed_buffer(const char *payload, size_t payload_size,
gpg_status = &buf;
sigchain_push(SIGPIPE, SIG_IGN);
- ret = pipe_command(&gpg, payload, payload_size,
- gpg_status, 0, gpg_output, 0);
+ ret = pipe_gpg_command(&gpg, payload, payload_size,
+ gpg_status, 0, gpg_output, 0);
sigchain_pop(SIGPIPE);
delete_tempfile(&temp);
--
2.11.0.dirty
^ permalink raw reply related
* Re: [PATCH v2 7/7] Makefile: add a knob to enable the use of Asciidoctor
From: Øyvind A. Holm @ 2017-01-25 2:26 UTC (permalink / raw)
To: brian m. carlson, git, Johannes Schindelin, Jeff King
In-Reply-To: <20170123040917.lrd6ic6wb6nxulzf@genre.crustytoothpaste.net>
[-- Attachment #1: Type: text/plain, Size: 3869 bytes --]
On 2017-01-23 04:09:17, brian m. carlson wrote:
> On Mon, Jan 23, 2017 at 03:57:13AM +0100, Øyvind A. Holm wrote:
> > On 2017-01-22 02:41:56, brian m. carlson wrote:
> > > While Git has traditionally built its documentation using
> > > AsciiDoc, some people wish to use Asciidoctor for speed or other
> > > reasons. Add a Makefile knob, USE_ASCIIDOCTOR, that sets various
> > > options in order to produce acceptable output. For HTML output,
> > > XHTML5 was chosen, since the AsciiDoc options also produce XHTML,
> > > albeit XHTML 1.1.
> >
> > I applied and tested the patches on the current master, commit
> > 787f75f0567a ("Sixth batch for 2.12"), and "make doc" with
> > USE_ASCIIDOCTOR fails:
> >
> > [...]
> >
> > $ asciidoctor --version
> > Asciidoctor 0.1.4 [http://asciidoctor.org]
>
> I think you need a newer version of Asciidoctor. I fixed one or two
> issues upstream in 1.5.2, I think, that made it work properly.
I've tried on Linux Mint 18 with Asciidoctor 1.5.4 now, and it works
there, so the version is probably too old, yes.
> You could try to do the build with the "html5" target instead of
> "xhtml5" and see if that works. If so, we could switch to that
> instead if we want to support older Asciidoctor versions.
It went a little better, but after a while it died with
$ make doc USE_ASCIIDOCTOR=1
[Cut 249 lines]
GEN technical/api-index.txt
ASCIIDOC technical/api-index.html
ASCIIDOC git-init-db.xml
sed "s|@@MAN_BASE_URL@@|file:///home/sunny/share/doc/git-doc/|" manpage-base-url.xsl.in > manpage-base-url.xsl
XMLTO git-init-db.1
xmlto: /home/sunny/src/git/src-other/devel/git/git/Documentation/git-init-db.xml does not validate (status 3)
xmlto: Fix document syntax or use --skip-validation option
/home/sunny/src/git/src-other/devel/git/git/Documentation/git-init-db.xml:5: element article: validity error : root and DTD name do not match 'article' and 'manpage'
Document /home/sunny/src/git/src-other/devel/git/git/Documentation/git-init-db.xml does not validate
Makefile:343: recipe for target 'git-init-db.1' failed
make[1]: *** [git-init-db.1] Error 13
make[1]: Leaving directory '/home/sunny/src/git/src-other/devel/git/git/Documentation'
Makefile:2091: recipe for target 'doc' failed
make: *** [doc] Error 2
$
and that's fair enough, since the generated html isn't well-formed.
Adding --skip-validation to XMLTO_EXTRA gave a slightly different
result:
GEN technical/api-index.txt
ASCIIDOC technical/api-index.html
ASCIIDOC git-init-db.xml
sed "s|@@MAN_BASE_URL@@|file:///home/sunny/share/doc/git-doc/|" manpage-base-url.xsl.in > manpage-base-url.xsl
XMLTO git-init-db.1
Note: namesp. cut : stripped namespace before processing git-init-db(1)
Note: namesp. cut : processing stripped document git-init-db(1)
Erro: no refentry: No refentry elements found in "git-init-db(1) git-init-db(1)
Makefile:343: recipe for target 'git-init-db.1' failed
make[1]: *** [git-init-db.1] Error 1
make[1]: Leaving directory '/home/sunny/src/git/src-other/devel/git/git/Documentation'
Makefile:2091: recipe for target 'doc' failed
make: *** [doc] Error 2
$
But frankly, this probably isn't a showstopper. Even though this is the
newest stable version of Debian, Asciidoctor 0.1.4 was released
2013-09-05, 3y5m ago. USE_ASCIIDOCTOR isn't the default, so people can
build the docs with asciidoc, and that works in Debian 8.7.
Regards,
Øyvind
+-| Øyvind A. Holm <sunny@sunbase.org> - N 60.37604° E 5.33339° |-+
| OpenPGP: 0xFB0CBEE894A506E5 - http://www.sunbase.org/pubkey.asc |
| Fingerprint: A006 05D6 E676 B319 55E2 E77E FB0C BEE8 94A5 06E5 |
+------------| 1698e7f6-e257-11e6-bfa0-db5caa6d21d3 |-------------+
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* Re:
From: Eric Wong @ 2017-01-25 1:32 UTC (permalink / raw)
To: Linus Torvalds
Cc: Stefan Beller, cornelius.weig, Johannes Sixt,
bitte.keine.werbung.einwerfen, git, Junio C Hamano, thomas.braun,
John Keeping
In-Reply-To: <CA+55aFx_W500Ct6HuG18owG37FviirjsrJ_4zZpRcDD-0tmpFg@mail.gmail.com>
Linus Torvalds <torvalds@linux-foundation.org> wrote:
> On Tue, Jan 24, 2017 at 4:21 PM, Stefan Beller <sbeller@google.com> wrote:
> >
> > +Do not PGP sign your patch. Most likely, your maintainer or other
> > +people on the list would not have your PGP key and would not bother
> > +obtaining it anyway.
>
> I think even that could be further simplified - by just removing all
> comments about pgp email
>
> Because it's not that the PGP keys would be hard to get, it's that
> PGP-signed email is an abject failure, and nobody sane does it.
>
> Google for "phil zimmerman doesn't use pgp email".
>
> It's dead. So I'm not sure it's worth mentioning at all.
I disagree, we still see it, and Debian still advocates it.
In fact, we may also want to mention S/MIME in the same breath:
https://public-inbox.org/git/20170110004031.57985-2-hansenr@google.com/
Richard's more recent mails seem to indicate he's reformed :)
^ permalink raw reply
* Re:
From: Linus Torvalds @ 2017-01-25 0:54 UTC (permalink / raw)
To: Stefan Beller
Cc: cornelius.weig, Johannes Sixt, bitte.keine.werbung.einwerfen,
Git Mailing List, Junio C Hamano, thomas.braun, John Keeping
In-Reply-To: <20170125002116.22111-1-sbeller@google.com>
On Tue, Jan 24, 2017 at 4:21 PM, Stefan Beller <sbeller@google.com> wrote:
>
> +Do not PGP sign your patch. Most likely, your maintainer or other
> +people on the list would not have your PGP key and would not bother
> +obtaining it anyway.
I think even that could be further simplified - by just removing all
comments about pgp email
Because it's not that the PGP keys would be hard to get, it's that
PGP-signed email is an abject failure, and nobody sane does it.
Google for "phil zimmerman doesn't use pgp email".
It's dead. So I'm not sure it's worth mentioning at all.
You might as well talk about how you shouldn't use EBCDIC encoding for
your patches, or about why git assumes that an email address has an
'@' sign in it, instead of being an UUCP bang path address.
Linus
^ permalink raw reply
* Re:
From: Stefan Beller @ 2017-01-25 0:52 UTC (permalink / raw)
To: Cornelius Weig
Cc: Johannes Sixt, bitte.keine.werbung.einwerfen, git@vger.kernel.org,
Junio C Hamano, thomas.braun, John Keeping
In-Reply-To: <ec2a198e-edf5-68a1-523c-12a9d1333c58@tngtech.com>
On Tue, Jan 24, 2017 at 4:43 PM, Cornelius Weig
<cornelius.weig@tngtech.com> wrote:
> -(5) Sign your work
> +(5) Certify your work by signing off.
That sounds better than what I proposed.
Thanks,
Stefan
^ permalink raw reply
* Re: [PATCHv2 3/3] submodule absorbing: fix worktree/gitdir pointers recursively for non-moves
From: Brandon Williams @ 2017-01-25 0:46 UTC (permalink / raw)
To: Stefan Beller; +Cc: gitster, git, peff
In-Reply-To: <20170124235651.18749-4-sbeller@google.com>
On 01/24, Stefan Beller wrote:
> Consider having a submodule 'sub' and a nested submodule at 'sub/nested'.
> When nested is already absorbed into sub, but sub is not absorbed into
> its superproject, then we need to fixup the gitfile and core.worktree
> setting for 'nested' when absorbing 'sub', but we do not need to move
> its git dir around.
>
> Previously 'nested's gitfile contained "gitdir: ../.git/modules/nested";
> it has to be corrected to "gitdir: ../../.git/modules/sub1/modules/nested".
>
> An alternative I considered to do this work lazily, i.e. when resolving
> "../.git/modules/nested", we would notice the ".git" being a gitfile
> linking to another path. That seemed to be robuster by design, but harder
> to get the implementation right. Maybe we have to do that anyway once we
> try to have submodules and worktrees working nicely together, but for now
> just produce 'correct' (i.e. direct) pointers.
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
> submodule.c | 43 ++++++++++++++++++++++++++++++--------
> t/t7412-submodule-absorbgitdirs.sh | 27 ++++++++++++++++++++++++
> 2 files changed, 61 insertions(+), 9 deletions(-)
>
> diff --git a/submodule.c b/submodule.c
> index 4c4f033e8a..95437105bf 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -1437,22 +1437,47 @@ void absorb_git_dir_into_superproject(const char *prefix,
> const char *path,
> unsigned flags)
> {
> + int err_code;
> const char *sub_git_dir, *v;
> char *real_sub_git_dir = NULL, *real_common_git_dir = NULL;
> struct strbuf gitdir = STRBUF_INIT;
> -
> strbuf_addf(&gitdir, "%s/.git", path);
> - sub_git_dir = resolve_gitdir(gitdir.buf);
> + sub_git_dir = resolve_gitdir_gently(gitdir.buf, &err_code);
>
> /* Not populated? */
> - if (!sub_git_dir)
> - goto out;
> + if (!sub_git_dir) {
> + char *real_new_git_dir;
> + const char *new_git_dir;
> + const struct submodule *sub;
> +
> + if (err_code == READ_GITFILE_ERR_STAT_FAILED)
> + goto out; /* unpopulated as expected */
> + if (err_code != READ_GITFILE_ERR_NOT_A_REPO)
> + /* We don't know what broke here. */
> + read_gitfile_error_die(err_code, path, NULL);
>
> - /* Is it already absorbed into the superprojects git dir? */
> - real_sub_git_dir = real_pathdup(sub_git_dir);
> - real_common_git_dir = real_pathdup(get_git_common_dir());
> - if (!skip_prefix(real_sub_git_dir, real_common_git_dir, &v))
> - relocate_single_git_dir_into_superproject(prefix, path);
> + /*
> + * Maybe populated, but no git directory was found?
> + * This can happen if the superproject is a submodule
> + * itself and was just absorbed. The absorption of the
> + * superproject did not rewrite the git file links yet,
> + * fix it now.
> + */
> + sub = submodule_from_path(null_sha1, path);
> + if (!sub)
> + die(_("could not lookup name for submodule '%s'"), path);
> + new_git_dir = git_path("modules/%s", sub->name);
> + if (safe_create_leading_directories_const(new_git_dir) < 0)
> + die(_("could not create directory '%s'"), new_git_dir);
> + real_new_git_dir = real_pathdup(new_git_dir);
> + connect_work_tree_and_git_dir(path, real_new_git_dir);
Memory leak with 'real_new_git_dir'
> + } else {
> + /* Is it already absorbed into the superprojects git dir? */
> + real_sub_git_dir = real_pathdup(sub_git_dir);
> + real_common_git_dir = real_pathdup(get_git_common_dir());
The scope of 'real_sub_git_dir' and 'real_common_git_dir' variable can
be narrowed. Move their declaration here and free it prior to exiting
the else block.
> + if (!skip_prefix(real_sub_git_dir, real_common_git_dir, &v))
'v' isn't ever used, just use starts_with() and get rid of 'v'
> + relocate_single_git_dir_into_superproject(prefix, path);
> + }
>
There's a label 'out' at the end of the function which can be removed as
there is no 'goto' statement using it.
> if (flags & ABSORB_GITDIR_RECURSE_SUBMODULES) {
> struct child_process cp = CHILD_PROCESS_INIT;
> diff --git a/t/t7412-submodule-absorbgitdirs.sh b/t/t7412-submodule-absorbgitdirs.sh
> index 1c47780e2b..e2bbb449b6 100755
> --- a/t/t7412-submodule-absorbgitdirs.sh
> +++ b/t/t7412-submodule-absorbgitdirs.sh
> @@ -64,6 +64,33 @@ test_expect_success 'absorb the git dir in a nested submodule' '
> test_cmp expect.2 actual.2
> '
>
> +test_expect_success 're-setup nested submodule' '
> + # un-absorb the direct submodule, to test if the nested submodule
> + # is still correct (needs a rewrite of the gitfile only)
> + rm -rf sub1/.git &&
> + mv .git/modules/sub1 sub1/.git &&
> + GIT_WORK_TREE=. git -C sub1 config --unset core.worktree &&
> + # fixup the nested submodule
> + echo "gitdir: ../.git/modules/nested" >sub1/nested/.git &&
> + GIT_WORK_TREE=../../../nested git -C sub1/.git/modules/nested config \
> + core.worktree "../../../nested" &&
> + # make sure this re-setup is correct
> + git status --ignore-submodules=none
> +'
> +
> +test_expect_success 'absorb the git dir in a nested submodule' '
> + git status >expect.1 &&
> + git -C sub1/nested rev-parse HEAD >expect.2 &&
> + git submodule absorbgitdirs &&
> + test -f sub1/.git &&
> + test -f sub1/nested/.git &&
> + test -d .git/modules/sub1/modules/nested &&
> + git status >actual.1 &&
> + git -C sub1/nested rev-parse HEAD >actual.2 &&
> + test_cmp expect.1 actual.1 &&
> + test_cmp expect.2 actual.2
> +'
> +
> test_expect_success 'setup a gitlink with missing .gitmodules entry' '
> git init sub2 &&
> test_commit -C sub2 first &&
> --
> 2.11.0.495.g04f60290a0.dirty
>
You can squash them into your patch, assuming you agree with the changes
:)
--
Brandon Williams
--8<--
From 0336c4bee60a85d8ad319ff55deea95debb3ceda Mon Sep 17 00:00:00 2001
From: Brandon Williams <bmwill@google.com>
Date: Tue, 24 Jan 2017 16:44:43 -0800
Subject: [PATCH] SQUASH
Signed-off-by: Brandon Williams <bmwill@google.com>
---
submodule.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/submodule.c b/submodule.c
index 95437105b..0d9c4bbbe 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1438,8 +1438,7 @@ void absorb_git_dir_into_superproject(const char *prefix,
unsigned flags)
{
int err_code;
- const char *sub_git_dir, *v;
- char *real_sub_git_dir = NULL, *real_common_git_dir = NULL;
+ const char *sub_git_dir;
struct strbuf gitdir = STRBUF_INIT;
strbuf_addf(&gitdir, "%s/.git", path);
sub_git_dir = resolve_gitdir_gently(gitdir.buf, &err_code);
@@ -1471,12 +1470,18 @@ void absorb_git_dir_into_superproject(const char *prefix,
die(_("could not create directory '%s'"), new_git_dir);
real_new_git_dir = real_pathdup(new_git_dir);
connect_work_tree_and_git_dir(path, real_new_git_dir);
+
+ free(real_new_git_dir);
} else {
/* Is it already absorbed into the superprojects git dir? */
- real_sub_git_dir = real_pathdup(sub_git_dir);
- real_common_git_dir = real_pathdup(get_git_common_dir());
- if (!skip_prefix(real_sub_git_dir, real_common_git_dir, &v))
+ char *real_sub_git_dir = real_pathdup(sub_git_dir);
+ char *real_common_git_dir = real_pathdup(get_git_common_dir());
+
+ if (!starts_with(real_sub_git_dir, real_common_git_dir))
relocate_single_git_dir_into_superproject(prefix, path);
+
+ free(real_sub_git_dir);
+ free(real_common_git_dir);
}
if (flags & ABSORB_GITDIR_RECURSE_SUBMODULES) {
@@ -1506,6 +1511,4 @@ void absorb_git_dir_into_superproject(const char *prefix,
out:
strbuf_release(&gitdir);
- free(real_sub_git_dir);
- free(real_common_git_dir);
}
--
2.11.0.483.g087da7b7c-goog
^ permalink raw reply related
* Re:
From: Cornelius Weig @ 2017-01-25 0:43 UTC (permalink / raw)
To: Stefan Beller
Cc: j6t, bitte.keine.werbung.einwerfen, git, gitster, thomas.braun,
john
In-Reply-To: <20170125002116.22111-1-sbeller@google.com>
On 01/25/2017 01:21 AM, Stefan Beller wrote:
>>
>>> Do not PGP sign your patch, at least *for now*. (...)
>>
>
> And maybe these 2 small words are the bug in the documentation?
> Shall we drop the "at least for now" part, like so:
>
> ---8<---
> From 2c4fe0e67451892186ff6257b20c53e088c9ec67 Mon Sep 17 00:00:00 2001
> From: Stefan Beller <sbeller@google.com>
> Date: Tue, 24 Jan 2017 16:19:13 -0800
> Subject: [PATCH] SubmittingPatches: drop temporal reference for PGP signing
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
> Documentation/SubmittingPatches | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
> index 08352deaae..28da4ad2d4 100644
> --- a/Documentation/SubmittingPatches
> +++ b/Documentation/SubmittingPatches
> @@ -216,12 +216,12 @@ that it will be postponed.
> Exception: If your mailer is mangling patches then someone may ask
> you to re-send them using MIME, that is OK.
>
> -Do not PGP sign your patch, at least for now. Most likely, your
> -maintainer or other people on the list would not have your PGP
> -key and would not bother obtaining it anyway. Your patch is not
> -judged by who you are; a good patch from an unknown origin has a
> -far better chance of being accepted than a patch from a known,
> -respected origin that is done poorly or does incorrect things.
> +Do not PGP sign your patch. Most likely, your maintainer or other
> +people on the list would not have your PGP key and would not bother
> +obtaining it anyway. Your patch is not judged by who you are; a good
> +patch from an unknown origin has a far better chance of being accepted
> +than a patch from a known, respected origin that is done poorly or
> +does incorrect things.
>
> If you really really really really want to do a PGP signed
> patch, format it as "multipart/signed", not a text/plain message
>
It definitely is an improvement. Though it would still leave me puzzled
when finding a section about signing just below.
Is changing heading (5) too big a change? Like so:
diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index 08352de..71898dc 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -246,7 +246,7 @@ patch.
*2* The mailing list: git@vger.kernel.org
-(5) Sign your work
+(5) Certify your work by signing off.
To improve tracking of who did what, we've borrowed the
"sign-off" procedure from the Linux kernel project on patches
^ permalink raw reply related
* (no subject)
From: Stefan Beller @ 2017-01-25 0:21 UTC (permalink / raw)
To: cornelius.weig
Cc: j6t, bitte.keine.werbung.einwerfen, git, gitster, thomas.braun,
john, Stefan Beller
In-Reply-To: <923cd4e4-5c9c-4eaf-0fea-6deff6875b88@tngtech.com>
>
>> Do not PGP sign your patch, at least *for now*. (...)
>
And maybe these 2 small words are the bug in the documentation?
Shall we drop the "at least for now" part, like so:
---8<---
From 2c4fe0e67451892186ff6257b20c53e088c9ec67 Mon Sep 17 00:00:00 2001
From: Stefan Beller <sbeller@google.com>
Date: Tue, 24 Jan 2017 16:19:13 -0800
Subject: [PATCH] SubmittingPatches: drop temporal reference for PGP signing
Signed-off-by: Stefan Beller <sbeller@google.com>
---
Documentation/SubmittingPatches | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index 08352deaae..28da4ad2d4 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -216,12 +216,12 @@ that it will be postponed.
Exception: If your mailer is mangling patches then someone may ask
you to re-send them using MIME, that is OK.
-Do not PGP sign your patch, at least for now. Most likely, your
-maintainer or other people on the list would not have your PGP
-key and would not bother obtaining it anyway. Your patch is not
-judged by who you are; a good patch from an unknown origin has a
-far better chance of being accepted than a patch from a known,
-respected origin that is done poorly or does incorrect things.
+Do not PGP sign your patch. Most likely, your maintainer or other
+people on the list would not have your PGP key and would not bother
+obtaining it anyway. Your patch is not judged by who you are; a good
+patch from an unknown origin has a far better chance of being accepted
+than a patch from a known, respected origin that is done poorly or
+does incorrect things.
If you really really really really want to do a PGP signed
patch, format it as "multipart/signed", not a text/plain message
--
2.11.0.495.g04f60290a0.dirty
^ permalink raw reply related
* Re: [PATCH 7/7] completion: recognize more long-options
From: Cornelius Weig @ 2017-01-25 0:11 UTC (permalink / raw)
To: Stefan Beller
Cc: Junio C Hamano, Johannes Sixt, bitte.keine.werbung.einwerfen,
git@vger.kernel.org, thomas.braun, John Keeping
In-Reply-To: <CAGZ79ka0PSb9L71tkiacZS+FH=YbUBrQr6a5UQu7ochpihRqEQ@mail.gmail.com>
On 01/25/2017 12:43 AM, Stefan Beller wrote:
> On Tue, Jan 24, 2017 at 3:33 PM, Cornelius Weig
> <cornelius.weig@tngtech.com> wrote:
>> On 01/25/2017 12:24 AM, Junio C Hamano wrote:
>>> Cornelius Weig <cornelius.weig@tngtech.com> writes:
>>>
>>>>> Please study item (5) "Sign your work" in
>>>>> Documentation/SubmittingPatches and sign off your work.
>>>>
>>>> I followed the recommendations to submitting work, and in the first
>>>> round signing is discouraged.
>>>
>>> Just this point. You found a bug in our documentation if that is
>>> the case; it should not be giving that impression to you.
>>>
>>
>> Well, I am referring to par. (4) of Documentation/SubmittingPatches
>> (emphasis mine):
>>
>> <<<<<<<<<<<<<<
>> *Do not PGP sign your patch, at least for now*. Most likely, your
>> maintainer or other people on the list would not have your PGP
>> key and would not bother obtaining it anyway. Your patch is not
>> judged by who you are; a good patch from an unknown origin has a
>> far better chance of being accepted than a patch from a known,
>> respected origin that is done poorly or does incorrect things.
>> <<<<<<<<<<<<<<
>>
>> If first submissions should be signed as well, then I find this quite
>> misleading.
>>
>
> Please read on; While this part addresses PGP signing,
> which is discouraged at any round,
> later on we talk about another type of signing.
> (not cryptographic strong signing, but signing the intent;)
> the DCO in the commit message.
>
> So no PGP signing (in any round of the patch).
>
> But DCO signed (in anything that you deem useful for the
> project and that you are allowed to contribute)
>
Right, it's crystal clear now. What confused me was the combination of
> Do not PGP sign your patch, at least *for now*. (...)
and then the section with heading
> (5) *Sign* your work
So I didn't even bother to read (5) because I deemed it irrelevant. I
think if it had said `(5) *Certify* your work` this would not have happened.
^ permalink raw reply
* [PATCH] tag: add tag.createReflog option
From: cornelius.weig @ 2017-01-25 0:19 UTC (permalink / raw)
To: git; +Cc: peff, novalis, pclouds, Cornelius Weig
From: Cornelius Weig <cornelius.weig@tngtech.com>
Git does not create a history for tags, in contrast to common
expectation to simply version everything. This can be changed by using
the `--create-reflog` flag when creating the tag. However, a config
option to enable this behavior by default is missing.
This commit adds the configuration variable `tag.createReflog` which
enables reflogs for new tags by default.
Signed-off-by: Cornelius Weig <cornelius.weig@tngtech.com>
---
Documentation/config.txt | 5 +++++
Documentation/git-tag.txt | 8 +++++---
builtin/tag.c | 6 +++++-
t/t7004-tag.sh | 14 ++++++++++++++
4 files changed, 29 insertions(+), 4 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index af2ae4c..9e5f6f6 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2945,6 +2945,11 @@ submodule.alternateErrorStrategy
as computed via `submodule.alternateLocation`. Possible values are
`ignore`, `info`, `die`. Default is `die`.
+tag.createReflog::
+ A boolean to specify whether newly created tags should have a reflog.
+ If `--[no-]create-reflog` is specified on the command line, it takes
+ precedence. Defaults to `false`.
+
tag.forceSignAnnotated::
A boolean to specify whether annotated tags created should be GPG signed.
If `--annotate` is specified on the command line, it takes
diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index 5055a96..f2ed370 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -13,7 +13,7 @@ SYNOPSIS
<tagname> [<commit> | <object>]
'git tag' -d <tagname>...
'git tag' [-n[<num>]] -l [--contains <commit>] [--points-at <object>]
- [--column[=<options>] | --no-column] [--create-reflog] [--sort=<key>]
+ [--column[=<options>] | --no-column] [--[no-]create-reflog] [--sort=<key>]
[--format=<format>] [--[no-]merged [<commit>]] [<pattern>...]
'git tag' -v <tagname>...
@@ -149,8 +149,10 @@ This option is only applicable when listing tags without annotation lines.
all, 'whitespace' removes just leading/trailing whitespace lines and
'strip' removes both whitespace and commentary.
---create-reflog::
- Create a reflog for the tag.
+--[no-]create-reflog::
+ Force to create a reflog for the tag, or no reflog if `--no-create-reflog`
+ is used. Unless the `tag.createReflog` config variable is set to true, no
+ reflog is created by default. See linkgit:git-config[1].
<tagname>::
The name of the tag to create, delete, or describe.
diff --git a/builtin/tag.c b/builtin/tag.c
index 73df728..1f13e4d 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -30,6 +30,7 @@ static const char * const git_tag_usage[] = {
static unsigned int colopts;
static int force_sign_annotate;
+static int create_reflog;
static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, const char *format)
{
@@ -165,6 +166,10 @@ static int git_tag_config(const char *var, const char *value, void *cb)
force_sign_annotate = git_config_bool(var, value);
return 0;
}
+ if (!strcmp(var, "tag.createreflog")) {
+ create_reflog = git_config_bool(var, value);
+ return 0;
+ }
if (starts_with(var, "column."))
return git_column_config(var, value, "tag", &colopts);
@@ -325,7 +330,6 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
const char *object_ref, *tag;
struct create_tag_options opt;
char *cleanup_arg = NULL;
- int create_reflog = 0;
int annotate = 0, force = 0;
int cmdmode = 0, create_tag_object = 0;
const char *msgfile = NULL, *keyid = NULL;
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 1cfa8a2..67b39ec 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -90,6 +90,20 @@ test_expect_success '--create-reflog does not create reflog on failure' '
test_must_fail git reflog exists refs/tags/mytag
'
+test_expect_success 'option tag.createreflog creates reflog by default' '
+ test_when_finished "git tag -d tag_with_reflog" &&
+ git config tag.createReflog true &&
+ git tag tag_with_reflog &&
+ git reflog exists refs/tags/tag_with_reflog
+'
+
+test_expect_success 'option tag.createreflog overridden by command line' '
+ test_when_finished "git tag -d tag_without_reflog" &&
+ git config tag.createReflog true &&
+ git tag --no-create-reflog tag_without_reflog &&
+ test_must_fail git reflog exists refs/tags/tag_without_reflog
+'
+
test_expect_success 'listing all tags if one exists should succeed' '
git tag -l &&
git tag
--
2.10.2
^ permalink raw reply related
* [PATCHv2 3/3] submodule absorbing: fix worktree/gitdir pointers recursively for non-moves
From: Stefan Beller @ 2017-01-24 23:56 UTC (permalink / raw)
To: gitster; +Cc: git, bmwill, peff, Stefan Beller
In-Reply-To: <20170124235651.18749-1-sbeller@google.com>
Consider having a submodule 'sub' and a nested submodule at 'sub/nested'.
When nested is already absorbed into sub, but sub is not absorbed into
its superproject, then we need to fixup the gitfile and core.worktree
setting for 'nested' when absorbing 'sub', but we do not need to move
its git dir around.
Previously 'nested's gitfile contained "gitdir: ../.git/modules/nested";
it has to be corrected to "gitdir: ../../.git/modules/sub1/modules/nested".
An alternative I considered to do this work lazily, i.e. when resolving
"../.git/modules/nested", we would notice the ".git" being a gitfile
linking to another path. That seemed to be robuster by design, but harder
to get the implementation right. Maybe we have to do that anyway once we
try to have submodules and worktrees working nicely together, but for now
just produce 'correct' (i.e. direct) pointers.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
submodule.c | 43 ++++++++++++++++++++++++++++++--------
t/t7412-submodule-absorbgitdirs.sh | 27 ++++++++++++++++++++++++
2 files changed, 61 insertions(+), 9 deletions(-)
diff --git a/submodule.c b/submodule.c
index 4c4f033e8a..95437105bf 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1437,22 +1437,47 @@ void absorb_git_dir_into_superproject(const char *prefix,
const char *path,
unsigned flags)
{
+ int err_code;
const char *sub_git_dir, *v;
char *real_sub_git_dir = NULL, *real_common_git_dir = NULL;
struct strbuf gitdir = STRBUF_INIT;
-
strbuf_addf(&gitdir, "%s/.git", path);
- sub_git_dir = resolve_gitdir(gitdir.buf);
+ sub_git_dir = resolve_gitdir_gently(gitdir.buf, &err_code);
/* Not populated? */
- if (!sub_git_dir)
- goto out;
+ if (!sub_git_dir) {
+ char *real_new_git_dir;
+ const char *new_git_dir;
+ const struct submodule *sub;
+
+ if (err_code == READ_GITFILE_ERR_STAT_FAILED)
+ goto out; /* unpopulated as expected */
+ if (err_code != READ_GITFILE_ERR_NOT_A_REPO)
+ /* We don't know what broke here. */
+ read_gitfile_error_die(err_code, path, NULL);
- /* Is it already absorbed into the superprojects git dir? */
- real_sub_git_dir = real_pathdup(sub_git_dir);
- real_common_git_dir = real_pathdup(get_git_common_dir());
- if (!skip_prefix(real_sub_git_dir, real_common_git_dir, &v))
- relocate_single_git_dir_into_superproject(prefix, path);
+ /*
+ * Maybe populated, but no git directory was found?
+ * This can happen if the superproject is a submodule
+ * itself and was just absorbed. The absorption of the
+ * superproject did not rewrite the git file links yet,
+ * fix it now.
+ */
+ sub = submodule_from_path(null_sha1, path);
+ if (!sub)
+ die(_("could not lookup name for submodule '%s'"), path);
+ new_git_dir = git_path("modules/%s", sub->name);
+ if (safe_create_leading_directories_const(new_git_dir) < 0)
+ die(_("could not create directory '%s'"), new_git_dir);
+ real_new_git_dir = real_pathdup(new_git_dir);
+ connect_work_tree_and_git_dir(path, real_new_git_dir);
+ } else {
+ /* Is it already absorbed into the superprojects git dir? */
+ real_sub_git_dir = real_pathdup(sub_git_dir);
+ real_common_git_dir = real_pathdup(get_git_common_dir());
+ if (!skip_prefix(real_sub_git_dir, real_common_git_dir, &v))
+ relocate_single_git_dir_into_superproject(prefix, path);
+ }
if (flags & ABSORB_GITDIR_RECURSE_SUBMODULES) {
struct child_process cp = CHILD_PROCESS_INIT;
diff --git a/t/t7412-submodule-absorbgitdirs.sh b/t/t7412-submodule-absorbgitdirs.sh
index 1c47780e2b..e2bbb449b6 100755
--- a/t/t7412-submodule-absorbgitdirs.sh
+++ b/t/t7412-submodule-absorbgitdirs.sh
@@ -64,6 +64,33 @@ test_expect_success 'absorb the git dir in a nested submodule' '
test_cmp expect.2 actual.2
'
+test_expect_success 're-setup nested submodule' '
+ # un-absorb the direct submodule, to test if the nested submodule
+ # is still correct (needs a rewrite of the gitfile only)
+ rm -rf sub1/.git &&
+ mv .git/modules/sub1 sub1/.git &&
+ GIT_WORK_TREE=. git -C sub1 config --unset core.worktree &&
+ # fixup the nested submodule
+ echo "gitdir: ../.git/modules/nested" >sub1/nested/.git &&
+ GIT_WORK_TREE=../../../nested git -C sub1/.git/modules/nested config \
+ core.worktree "../../../nested" &&
+ # make sure this re-setup is correct
+ git status --ignore-submodules=none
+'
+
+test_expect_success 'absorb the git dir in a nested submodule' '
+ git status >expect.1 &&
+ git -C sub1/nested rev-parse HEAD >expect.2 &&
+ git submodule absorbgitdirs &&
+ test -f sub1/.git &&
+ test -f sub1/nested/.git &&
+ test -d .git/modules/sub1/modules/nested &&
+ git status >actual.1 &&
+ git -C sub1/nested rev-parse HEAD >actual.2 &&
+ test_cmp expect.1 actual.1 &&
+ test_cmp expect.2 actual.2
+'
+
test_expect_success 'setup a gitlink with missing .gitmodules entry' '
git init sub2 &&
test_commit -C sub2 first &&
--
2.11.0.495.g04f60290a0.dirty
^ permalink raw reply related
* [PATCHv2 2/3] cache.h: expose the dying procedure for reading gitlinks
From: Stefan Beller @ 2017-01-24 23:56 UTC (permalink / raw)
To: gitster; +Cc: git, bmwill, peff, Stefan Beller
In-Reply-To: <20170124235651.18749-1-sbeller@google.com>
In a later patch we want to react to only a subset of errors, defaulting
the rest to die as usual. Separate the block that takes care of dying
into its own function so we have easy access to it.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
cache.h | 1 +
setup.c | 48 ++++++++++++++++++++++++++----------------------
2 files changed, 27 insertions(+), 22 deletions(-)
diff --git a/cache.h b/cache.h
index cafa3d10ae..d55f5dccb1 100644
--- a/cache.h
+++ b/cache.h
@@ -507,6 +507,7 @@ extern int is_nonbare_repository_dir(struct strbuf *path);
#define READ_GITFILE_ERR_NO_PATH 6
#define READ_GITFILE_ERR_NOT_A_REPO 7
#define READ_GITFILE_ERR_TOO_LARGE 8
+extern void read_gitfile_error_die(int error_code, const char *path, const char *dir);
extern const char *read_gitfile_gently(const char *path, int *return_error_code);
#define read_gitfile(path) read_gitfile_gently((path), NULL)
extern const char *resolve_gitdir_gently(const char *suspect, int *return_error_code);
diff --git a/setup.c b/setup.c
index 4605fd3c3c..967f289f1e 100644
--- a/setup.c
+++ b/setup.c
@@ -486,6 +486,30 @@ int verify_repository_format(const struct repository_format *format,
return 0;
}
+void read_gitfile_error_die(int error_code, const char *path, const char *dir)
+{
+ switch (error_code) {
+ case READ_GITFILE_ERR_STAT_FAILED:
+ case READ_GITFILE_ERR_NOT_A_FILE:
+ /* non-fatal; follow return path */
+ break;
+ case READ_GITFILE_ERR_OPEN_FAILED:
+ die_errno("Error opening '%s'", path);
+ case READ_GITFILE_ERR_TOO_LARGE:
+ die("Too large to be a .git file: '%s'", path);
+ case READ_GITFILE_ERR_READ_FAILED:
+ die("Error reading %s", path);
+ case READ_GITFILE_ERR_INVALID_FORMAT:
+ die("Invalid gitfile format: %s", path);
+ case READ_GITFILE_ERR_NO_PATH:
+ die("No path in gitfile: %s", path);
+ case READ_GITFILE_ERR_NOT_A_REPO:
+ die("Not a git repository: %s", dir);
+ default:
+ die("BUG: unknown error code");
+ }
+}
+
/*
* Try to read the location of the git directory from the .git file,
* return path to git directory if found.
@@ -559,28 +583,8 @@ const char *read_gitfile_gently(const char *path, int *return_error_code)
cleanup_return:
if (return_error_code)
*return_error_code = error_code;
- else if (error_code) {
- switch (error_code) {
- case READ_GITFILE_ERR_STAT_FAILED:
- case READ_GITFILE_ERR_NOT_A_FILE:
- /* non-fatal; follow return path */
- break;
- case READ_GITFILE_ERR_OPEN_FAILED:
- die_errno("Error opening '%s'", path);
- case READ_GITFILE_ERR_TOO_LARGE:
- die("Too large to be a .git file: '%s'", path);
- case READ_GITFILE_ERR_READ_FAILED:
- die("Error reading %s", path);
- case READ_GITFILE_ERR_INVALID_FORMAT:
- die("Invalid gitfile format: %s", path);
- case READ_GITFILE_ERR_NO_PATH:
- die("No path in gitfile: %s", path);
- case READ_GITFILE_ERR_NOT_A_REPO:
- die("Not a git repository: %s", dir);
- default:
- assert(0);
- }
- }
+ else if (error_code)
+ read_gitfile_error_die(error_code, path, dir);
free(buf);
return error_code ? NULL : path;
--
2.11.0.495.g04f60290a0.dirty
^ permalink raw reply related
* [PATCHv2 1/3] Add gentle version of resolve_git_dir
From: Stefan Beller @ 2017-01-24 23:56 UTC (permalink / raw)
To: gitster; +Cc: git, bmwill, peff, Stefan Beller
In-Reply-To: <20170124235651.18749-1-sbeller@google.com>
This follows a93bedada (setup: add gentle version of read_gitfile,
2015-06-09), and assumes the same reasoning. resolve_git_dir is unsuited
for speculative calls, so we want to use the gentle version to find out
about potential errors.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
cache.h | 4 +++-
setup.c | 4 ++--
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/cache.h b/cache.h
index 00a029af36..cafa3d10ae 100644
--- a/cache.h
+++ b/cache.h
@@ -509,7 +509,9 @@ extern int is_nonbare_repository_dir(struct strbuf *path);
#define READ_GITFILE_ERR_TOO_LARGE 8
extern const char *read_gitfile_gently(const char *path, int *return_error_code);
#define read_gitfile(path) read_gitfile_gently((path), NULL)
-extern const char *resolve_gitdir(const char *suspect);
+extern const char *resolve_gitdir_gently(const char *suspect, int *return_error_code);
+#define resolve_gitdir(path) resolve_gitdir_gently((path), NULL)
+
extern void set_git_work_tree(const char *tree);
#define ALTERNATE_DB_ENVIRONMENT "GIT_ALTERNATE_OBJECT_DIRECTORIES"
diff --git a/setup.c b/setup.c
index 1b534a7508..4605fd3c3c 100644
--- a/setup.c
+++ b/setup.c
@@ -1017,11 +1017,11 @@ const char *setup_git_directory(void)
return setup_git_directory_gently(NULL);
}
-const char *resolve_gitdir(const char *suspect)
+const char *resolve_gitdir_gently(const char *suspect, int *return_error_code)
{
if (is_git_directory(suspect))
return suspect;
- return read_gitfile(suspect);
+ return read_gitfile_gently(suspect, return_error_code);
}
/* if any standard file descriptor is missing open it to /dev/null */
--
2.11.0.495.g04f60290a0.dirty
^ permalink raw reply related
* [PATCHv2 0/3] fix recursive submodule absorbing
From: Stefan Beller @ 2017-01-24 23:56 UTC (permalink / raw)
To: gitster; +Cc: git, bmwill, peff, Stefan Beller
In-Reply-To: <20170124221948.GB58021@google.com>
In this second iteration, only absorb_git_dir_into_superproject is touched,
which does the check if connect_work_tree_and_git_dir is needed.
Internally I also had a patch that converts is_submodule_populated
to be gentle, but it is not needed here, so I dropped it before sending out.
Thanks,
Stefan
Stefan Beller (3):
Add gentle version of resolve_git_dir
cache.h: expose the dying procedure for reading gitlinks
submodule absorbing: fix worktree/gitdir pointers recursively for
non-moves
cache.h | 5 +++-
setup.c | 52 ++++++++++++++++++++------------------
submodule.c | 43 ++++++++++++++++++++++++-------
t/t7412-submodule-absorbgitdirs.sh | 27 ++++++++++++++++++++
4 files changed, 93 insertions(+), 34 deletions(-)
--
2.11.0.495.g04f60290a0.dirty
^ permalink raw reply
* Re: [PATCH 7/7] completion: recognize more long-options
From: Stefan Beller @ 2017-01-24 23:43 UTC (permalink / raw)
To: Cornelius Weig
Cc: Junio C Hamano, Johannes Sixt, bitte.keine.werbung.einwerfen,
git@vger.kernel.org, thomas.braun, John Keeping
In-Reply-To: <a921bc92-4f20-3284-6577-344470a60c6f@tngtech.com>
On Tue, Jan 24, 2017 at 3:33 PM, Cornelius Weig
<cornelius.weig@tngtech.com> wrote:
> On 01/25/2017 12:24 AM, Junio C Hamano wrote:
>> Cornelius Weig <cornelius.weig@tngtech.com> writes:
>>
>>>> Please study item (5) "Sign your work" in
>>>> Documentation/SubmittingPatches and sign off your work.
>>>
>>> I followed the recommendations to submitting work, and in the first
>>> round signing is discouraged.
>>
>> Just this point. You found a bug in our documentation if that is
>> the case; it should not be giving that impression to you.
>>
>
> Well, I am referring to par. (4) of Documentation/SubmittingPatches
> (emphasis mine):
>
> <<<<<<<<<<<<<<
> *Do not PGP sign your patch, at least for now*. Most likely, your
> maintainer or other people on the list would not have your PGP
> key and would not bother obtaining it anyway. Your patch is not
> judged by who you are; a good patch from an unknown origin has a
> far better chance of being accepted than a patch from a known,
> respected origin that is done poorly or does incorrect things.
>>>>>>>>>>>>>>>
>
> If first submissions should be signed as well, then I find this quite
> misleading.
>
Please read on; While this part addresses PGP signing,
which is discouraged at any round,
later on we talk about another type of signing.
(not cryptographic strong signing, but signing the intent;)
the DCO in the commit message.
So no PGP signing (in any round of the patch).
But DCO signed (in anything that you deem useful for the
project and that you are allowed to contribute)
^ permalink raw reply
* Re: [PATCH 7/7] completion: recognize more long-options
From: Cornelius Weig @ 2017-01-24 23:33 UTC (permalink / raw)
To: Junio C Hamano
Cc: Johannes Sixt, bitte.keine.werbung.einwerfen, git, thomas.braun,
john
In-Reply-To: <xmqqd1fcca8n.fsf@gitster.mtv.corp.google.com>
On 01/25/2017 12:24 AM, Junio C Hamano wrote:
> Cornelius Weig <cornelius.weig@tngtech.com> writes:
>
>>> Please study item (5) "Sign your work" in
>>> Documentation/SubmittingPatches and sign off your work.
>>
>> I followed the recommendations to submitting work, and in the first
>> round signing is discouraged.
>
> Just this point. You found a bug in our documentation if that is
> the case; it should not be giving that impression to you.
>
Well, I am referring to par. (4) of Documentation/SubmittingPatches
(emphasis mine):
<<<<<<<<<<<<<<
*Do not PGP sign your patch, at least for now*. Most likely, your
maintainer or other people on the list would not have your PGP
key and would not bother obtaining it anyway. Your patch is not
judged by who you are; a good patch from an unknown origin has a
far better chance of being accepted than a patch from a known,
respected origin that is done poorly or does incorrect things.
>>>>>>>>>>>>>>
If first submissions should be signed as well, then I find this quite
misleading.
^ permalink raw reply
* Re: [PATCH 7/7] completion: recognize more long-options
From: Junio C Hamano @ 2017-01-24 23:24 UTC (permalink / raw)
To: Cornelius Weig
Cc: Johannes Sixt, bitte.keine.werbung.einwerfen, git, thomas.braun,
john
In-Reply-To: <967937ff-e5ff-2515-2f50-80a96683c068@tngtech.com>
Cornelius Weig <cornelius.weig@tngtech.com> writes:
>> Please study item (5) "Sign your work" in
>> Documentation/SubmittingPatches and sign off your work.
>
> I followed the recommendations to submitting work, and in the first
> round signing is discouraged.
Just this point. You found a bug in our documentation if that is
the case; it should not be giving that impression to you.
^ permalink raw reply
* Re: [PATCH] difftool.c: mark a file-local symbol with static
From: Jeff King @ 2017-01-24 23:05 UTC (permalink / raw)
To: Junio C Hamano
Cc: David Aguilar, Ramsay Jones, Johannes Schindelin,
GIT Mailing-list
In-Reply-To: <xmqqlgu0ceia.fsf@gitster.mtv.corp.google.com>
On Tue, Jan 24, 2017 at 01:52:13PM -0800, Junio C Hamano wrote:
> > I dunno. As ugly as the "%s" thing is in the source, at least it doesn't
> > change the output. Not that an extra space is the end of the world, but
> > it seems like it's letting the problem escape from the source code.
> >
> > Do people still care about resolving this? -Wno-format-zero-length is in
> > the DEVELOPER options. It wasn't clear to me if that was sufficient, or
> > if we're going to get a bunch of reports from people that need to be
> > directed to the right compiler options.
>
> I view both as ugly, but probably "%s", "" is lessor of the two
> evils.
>
> Perhaps
>
> #define JUST_SHOW_EMPTY_LINE "%s", ""
>
> ...
> warning(JUST_SHOW_EMPTY_LINE);
> ...
>
> or something silly like that?
Gross, but at least it's self documenting. :)
I guess a less horrible version of that is:
static inline warning_blank_line(void)
{
warning("%s", "");
}
We'd potentially need a matching one for error(), but at last it avoids
macro trickery.
-Peff
^ permalink raw reply
* Re: [PATCH] submodule absorbing: fix worktree/gitdir pointers recursively for non-moves
From: Stefan Beller @ 2017-01-24 22:13 UTC (permalink / raw)
To: Brandon Williams; +Cc: Junio C Hamano, git@vger.kernel.org, Jeff King
In-Reply-To: <20170124215851.GA58021@google.com>
On Tue, Jan 24, 2017 at 1:58 PM, Brandon Williams <bmwill@google.com> wrote:
> On 01/24, Stefan Beller wrote:
>> + if (read_gitfile_gently(old_git_dir, &err_code) ||
>> + err_code == READ_GITFILE_ERR_NOT_A_REPO) {
>> + /*
>> + * If it is an actual gitfile, it doesn't need migration,
>> + * however in case of a recursively nested submodule, the
>> + * gitfile content may be stale, as its superproject
>> + * (which may be a submodule of another superproject)
>> + * may have been moved. So expect a bogus pointer to be read,
>> + * which materializes as error READ_GITFILE_ERR_NOT_A_REPO.
>> + */
>> + connect_work_tree_and_git_dir(path, real_new_git_dir);
>
> So connect_work_tree_and_git_dir() will update the .gitfile if it is
> stale.
>
>> + return;
>> + }
>> +
>> + if (submodule_uses_worktrees(path))
>> + die(_("relocate_gitdir for submodule '%s' with "
>> + "more than one worktree not supported"), path);
>
> No current support for worktrees (yet!).
>
>> +
>> if (!prefix)
>> prefix = get_super_prefix();
>>
>> @@ -1437,22 +1448,14 @@ void absorb_git_dir_into_superproject(const char *prefix,
>> const char *path,
>> unsigned flags)
>> {
>> - const char *sub_git_dir, *v;
>> - char *real_sub_git_dir = NULL, *real_common_git_dir = NULL;
>> struct strbuf gitdir = STRBUF_INIT;
>> -
>> strbuf_addf(&gitdir, "%s/.git", path);
>> - sub_git_dir = resolve_gitdir(gitdir.buf);
>>
>> /* Not populated? */
>> - if (!sub_git_dir)
>> + if (!file_exists(gitdir.buf))
>> goto out;
>
> There should be a is_submodule_populated() function now, maybe
> we should start using it when performing population checks?
Yes I am aware of that, but the problem is we cannot use it here.
is_submodule_populated[1], just like the code here, uses
resolve_gitdir, which is
const char *resolve_gitdir(const char *suspect)
{
if (is_git_directory(suspect))
return suspect;
return read_gitfile(suspect);
}
And there you see the problem: read_gitfile will die on error.
we'd have to have use read_gitfile_gently(old_git_dir, &err_code),
and then allow READ_GITFILE_ERR_NOT_A_REPO to go through,
just as above.
And that is also the reason why we had to move submodule_uses_worktrees
down, as it also uses no gentle function to look for a git directory
(read: it would die as well). When you have bogus content in your
.git file, there is really nothing you can do to determine if the submodule
is part of a worktree setup, so it is fine to postpone the check until after we
fixed up the link.
So here is the bug you spotted: If it is a worktree already, then
read_gitfile_gently would work fine, no need to "fix" it.
I'll resend with logic as follows:
char *retvalue = read_gitfile_gently(old_git_dir, &err_code);
if (retvalue)
// return early; a worktree is fine here, no need to check
// because we do nothing
if (err_code == READ_GITFILE_ERR_NOT_A_REPO)
// connect; then check for worktree and return early;
// do the actual relocation.
[1] as found e.g. at
https://public-inbox.org/git/1481915002-162130-2-git-send-email-bmwill@google.com/
>
>>
>> - /* Is it already absorbed into the superprojects git dir? */
>> - real_sub_git_dir = real_pathdup(sub_git_dir);
>> - real_common_git_dir = real_pathdup(get_git_common_dir());
>> - if (!skip_prefix(real_sub_git_dir, real_common_git_dir, &v))
>> - relocate_single_git_dir_into_superproject(prefix, path);
>> + relocate_single_git_dir_into_superproject(prefix, path);
>
> So the check was just pushed into the relocation function.
The check was pushed down, so we can use the
connect_work_tree_and_git_dir instead.
>
>>
>> if (flags & ABSORB_GITDIR_RECURSE_SUBMODULES) {
>> struct child_process cp = CHILD_PROCESS_INIT;
>> @@ -1481,6 +1484,4 @@ void absorb_git_dir_into_superproject(const char *prefix,
>>
>> out:
>> strbuf_release(&gitdir);
>> - free(real_sub_git_dir);
>> - free(real_common_git_dir);
>> }
>> diff --git a/t/t7412-submodule-absorbgitdirs.sh b/t/t7412-submodule-absorbgitdirs.sh
>> index 1c47780e2b..e2bbb449b6 100755
>> --- a/t/t7412-submodule-absorbgitdirs.sh
>> +++ b/t/t7412-submodule-absorbgitdirs.sh
>> @@ -64,6 +64,33 @@ test_expect_success 'absorb the git dir in a nested submodule' '
>> test_cmp expect.2 actual.2
>> '
>>
>> +test_expect_success 're-setup nested submodule' '
>> + # un-absorb the direct submodule, to test if the nested submodule
>> + # is still correct (needs a rewrite of the gitfile only)
>> + rm -rf sub1/.git &&
>> + mv .git/modules/sub1 sub1/.git &&
>> + GIT_WORK_TREE=. git -C sub1 config --unset core.worktree &&
>> + # fixup the nested submodule
>> + echo "gitdir: ../.git/modules/nested" >sub1/nested/.git &&
>> + GIT_WORK_TREE=../../../nested git -C sub1/.git/modules/nested config \
>> + core.worktree "../../../nested" &&
>> + # make sure this re-setup is correct
>> + git status --ignore-submodules=none
>> +'
>> +
>> +test_expect_success 'absorb the git dir in a nested submodule' '
>> + git status >expect.1 &&
>> + git -C sub1/nested rev-parse HEAD >expect.2 &&
>> + git submodule absorbgitdirs &&
>> + test -f sub1/.git &&
>> + test -f sub1/nested/.git &&
>> + test -d .git/modules/sub1/modules/nested &&
>> + git status >actual.1 &&
>> + git -C sub1/nested rev-parse HEAD >actual.2 &&
>> + test_cmp expect.1 actual.1 &&
>> + test_cmp expect.2 actual.2
>> +'
>> +
>> test_expect_success 'setup a gitlink with missing .gitmodules entry' '
>> git init sub2 &&
>> test_commit -C sub2 first &&
>> --
>> 2.11.0.486.g67830dbe1c
>
>
> Aside from my one question the rest of this looks good to me.
>
> --
> Brandon Williams
^ permalink raw reply
* Re: [PATCH] submodule absorbing: fix worktree/gitdir pointers recursively for non-moves
From: Brandon Williams @ 2017-01-24 22:19 UTC (permalink / raw)
To: Stefan Beller; +Cc: Junio C Hamano, git@vger.kernel.org, Jeff King
In-Reply-To: <CAGZ79kYKkx441bbU5Oy9Ernb1FmbcTybYbL_M_+yWG_ycfPwrA@mail.gmail.com>
On 01/24, Stefan Beller wrote:
> On Tue, Jan 24, 2017 at 1:58 PM, Brandon Williams <bmwill@google.com> wrote:
> > On 01/24, Stefan Beller wrote:
> >> + if (read_gitfile_gently(old_git_dir, &err_code) ||
> >> + err_code == READ_GITFILE_ERR_NOT_A_REPO) {
> >> + /*
> >> + * If it is an actual gitfile, it doesn't need migration,
> >> + * however in case of a recursively nested submodule, the
> >> + * gitfile content may be stale, as its superproject
> >> + * (which may be a submodule of another superproject)
> >> + * may have been moved. So expect a bogus pointer to be read,
> >> + * which materializes as error READ_GITFILE_ERR_NOT_A_REPO.
> >> + */
> >> + connect_work_tree_and_git_dir(path, real_new_git_dir);
> >
> > So connect_work_tree_and_git_dir() will update the .gitfile if it is
> > stale.
> >
> >> + return;
> >> + }
> >> +
> >> + if (submodule_uses_worktrees(path))
> >> + die(_("relocate_gitdir for submodule '%s' with "
> >> + "more than one worktree not supported"), path);
> >
> > No current support for worktrees (yet!).
> >
> >> +
> >> if (!prefix)
> >> prefix = get_super_prefix();
> >>
> >> @@ -1437,22 +1448,14 @@ void absorb_git_dir_into_superproject(const char *prefix,
> >> const char *path,
> >> unsigned flags)
> >> {
> >> - const char *sub_git_dir, *v;
> >> - char *real_sub_git_dir = NULL, *real_common_git_dir = NULL;
> >> struct strbuf gitdir = STRBUF_INIT;
> >> -
> >> strbuf_addf(&gitdir, "%s/.git", path);
> >> - sub_git_dir = resolve_gitdir(gitdir.buf);
> >>
> >> /* Not populated? */
> >> - if (!sub_git_dir)
> >> + if (!file_exists(gitdir.buf))
> >> goto out;
> >
> > There should be a is_submodule_populated() function now, maybe
> > we should start using it when performing population checks?
>
> Yes I am aware of that, but the problem is we cannot use it here.
> is_submodule_populated[1], just like the code here, uses
> resolve_gitdir, which is
>
> const char *resolve_gitdir(const char *suspect)
> {
> if (is_git_directory(suspect))
> return suspect;
> return read_gitfile(suspect);
> }
>
> And there you see the problem: read_gitfile will die on error.
> we'd have to have use read_gitfile_gently(old_git_dir, &err_code),
> and then allow READ_GITFILE_ERR_NOT_A_REPO to go through,
> just as above.
Hmm, then maybe is_submodule_populated should be rewritten to not die on
an error then?
>
> And that is also the reason why we had to move submodule_uses_worktrees
> down, as it also uses no gentle function to look for a git directory
> (read: it would die as well). When you have bogus content in your
> .git file, there is really nothing you can do to determine if the submodule
> is part of a worktree setup, so it is fine to postpone the check until after we
> fixed up the link.
>
> So here is the bug you spotted: If it is a worktree already, then
> read_gitfile_gently would work fine, no need to "fix" it.
>
> I'll resend with logic as follows:
>
> char *retvalue = read_gitfile_gently(old_git_dir, &err_code);
> if (retvalue)
> // return early; a worktree is fine here, no need to check
> // because we do nothing
>
> if (err_code == READ_GITFILE_ERR_NOT_A_REPO)
> // connect; then check for worktree and return early;
>
> // do the actual relocation.
>
>
> [1] as found e.g. at
> https://public-inbox.org/git/1481915002-162130-2-git-send-email-bmwill@google.com/
>
> >
> >>
> >> - /* Is it already absorbed into the superprojects git dir? */
> >> - real_sub_git_dir = real_pathdup(sub_git_dir);
> >> - real_common_git_dir = real_pathdup(get_git_common_dir());
> >> - if (!skip_prefix(real_sub_git_dir, real_common_git_dir, &v))
> >> - relocate_single_git_dir_into_superproject(prefix, path);
> >> + relocate_single_git_dir_into_superproject(prefix, path);
> >
> > So the check was just pushed into the relocation function.
>
> The check was pushed down, so we can use the
> connect_work_tree_and_git_dir instead.
>
> >
> >>
> >> if (flags & ABSORB_GITDIR_RECURSE_SUBMODULES) {
> >> struct child_process cp = CHILD_PROCESS_INIT;
> >> @@ -1481,6 +1484,4 @@ void absorb_git_dir_into_superproject(const char *prefix,
> >>
> >> out:
> >> strbuf_release(&gitdir);
> >> - free(real_sub_git_dir);
> >> - free(real_common_git_dir);
> >> }
> >> diff --git a/t/t7412-submodule-absorbgitdirs.sh b/t/t7412-submodule-absorbgitdirs.sh
> >> index 1c47780e2b..e2bbb449b6 100755
> >> --- a/t/t7412-submodule-absorbgitdirs.sh
> >> +++ b/t/t7412-submodule-absorbgitdirs.sh
> >> @@ -64,6 +64,33 @@ test_expect_success 'absorb the git dir in a nested submodule' '
> >> test_cmp expect.2 actual.2
> >> '
> >>
> >> +test_expect_success 're-setup nested submodule' '
> >> + # un-absorb the direct submodule, to test if the nested submodule
> >> + # is still correct (needs a rewrite of the gitfile only)
> >> + rm -rf sub1/.git &&
> >> + mv .git/modules/sub1 sub1/.git &&
> >> + GIT_WORK_TREE=. git -C sub1 config --unset core.worktree &&
> >> + # fixup the nested submodule
> >> + echo "gitdir: ../.git/modules/nested" >sub1/nested/.git &&
> >> + GIT_WORK_TREE=../../../nested git -C sub1/.git/modules/nested config \
> >> + core.worktree "../../../nested" &&
> >> + # make sure this re-setup is correct
> >> + git status --ignore-submodules=none
> >> +'
> >> +
> >> +test_expect_success 'absorb the git dir in a nested submodule' '
> >> + git status >expect.1 &&
> >> + git -C sub1/nested rev-parse HEAD >expect.2 &&
> >> + git submodule absorbgitdirs &&
> >> + test -f sub1/.git &&
> >> + test -f sub1/nested/.git &&
> >> + test -d .git/modules/sub1/modules/nested &&
> >> + git status >actual.1 &&
> >> + git -C sub1/nested rev-parse HEAD >actual.2 &&
> >> + test_cmp expect.1 actual.1 &&
> >> + test_cmp expect.2 actual.2
> >> +'
> >> +
> >> test_expect_success 'setup a gitlink with missing .gitmodules entry' '
> >> git init sub2 &&
> >> test_commit -C sub2 first &&
> >> --
> >> 2.11.0.486.g67830dbe1c
> >
> >
> > Aside from my one question the rest of this looks good to me.
> >
> > --
> > Brandon Williams
--
Brandon Williams
^ 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