* [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
* [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
* 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
* (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:
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
* 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: 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:
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: 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: [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
* [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] 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 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
* [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 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
* 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
* Re: submodule network operations [WAS: Re: [RFC/PATCH 0/4] working tree operations: support superprefix]
From: Brian J. Davis @ 2017-01-25 7:14 UTC (permalink / raw)
To: Stefan Beller; +Cc: Brandon Williams, git@vger.kernel.org, David Turner
In-Reply-To: <CAGZ79kZzekMc=pUN3Q2+zmaSNo-BLJ6TxaQPN8ik1B2+ACy9dw@mail.gmail.com>
On 1/24/2017 12:49 PM, Stefan Beller wrote:
> On Sat, Jan 21, 2017 at 7:53 AM, Brian J. Davis <bitminer@gmail.com> wrote:
>> On 1/19/2017 7:22 PM, Stefan Beller wrote:
>>>>> Between the init and the update step you can modify the URLs.
>>>>> These commands are just a repetition from the first email, but the
>>>>> git commands can be viewed as moving from one state to another
>>>>> for submodules; submodules itself can be seen as a state machine
>>>>> according to that proposed documentation. Maybe such a state machine
>>>>> makes it easier to understand for some people.
>>>>
>>>> "Between the init and the update step you can modify the URLs." Yes I
>>>> can
>>>> and have to... wish it was not this way.
>>> So how would yo u rather want to do it?
>>> look at the .gitmodules file beforehand and then run a "submodule update"
>>> ?
>>> Or a thing like
>>>
>>> git -c url.https://internal.insteadOf git://github.com/ \
>>> -c submodule.record-rewritten-urls submodule update
>>>
>>> (no need for init there as theoretically there is not
>>> need for such an intermediate step)
>>>
>> "Yes please and thank you" ... both.
>>
>> My thought was to simply allow addition to .gitmodules. If I understand
>> correctly you are proposing, to override these at the command line and
>> possibly rewrite them on submodule update, but maybe not save or add to
>> .gitmodules. I would then propose both.
>> 1) allow user to add to .gitmodules for those who do not care that
>> "outsiders" know the internal dev server
>> and
>> 2) allow to rewrite while not saving to .gitmodules on fresh clone and
>> submodule update for thoes that do not want ousiders to known internal dev
>> server.
>> and possibly:
>> 3) allow at command line to add remote to .gitmodules on submodule commands
>> (note add optoin in -c <name> = <value> pair)
>>
>> .gitmodules before:
>>
>> [submodule "subprojects/wrangler"]
>> path = subprojects/wrangler
>> url = git://github.com/
>>
>> Then your adapted command:
>>
>> git -c url.https://internal.insteadOf git://github.com/ \
>> -c submodule.record-rewritten-urls=add,internal --add submodule
>> update
>>
>> would produce
>>
>> [submodule "subprojects/projname"]
>> path = subprojects/projname
>> remote.origin.url = git://github.com/
>> remote.internal.url =https://internal.insteadOf
>>
>> Or similar support.
> I think this was avoided until now as it would rewrite/add history.
> So what if you want ot "just mirror" a large project with a lot
> of submodules? You would want to do that without touching
> the history, hence we'd need to do such a configuration in a separate
> place. IIRC there was a proposal to have a ref e.g.
> "refs/submodule/config", that can overwrite/extend the .gitmodules
> file with your own configuration. It is a ref, such that mirroring would
> work, but not part of the main history, such that yoiu can still change it.
How would this handle the hashes for the submodules tracked in the super
project. This gets to my earlier statement that git submodules is not
really that well thought out. When/if /refs/submodule/config
overwrites/extends .gitmoduels what does git checkout for a hash code
(version) for the subproject? Maybe there are some simple ways of doing
this, but it really seems to me that git submodules needs an overhaul to
support multiple remotes. Would the refs/submodule/config also include
the versions hashes for the submodule for both remotes? When switching
between remotes would the git submodule update pull the correct hash
version for the subproject and pull the correct version of the subproject?
> I think to get it right we need to enable a workflow that allows easy
> "multi-step" mirroring, e.g. A (source of truth) can be mirrored to B,
> who can overlay the .gitmodules to point to server-B, which then can
> be mirrored by C, who can have its own serverC.
>
> When C forgot to configure all the submodules, it should fall back to
> serverB as that was closest, and if that is unconfigured it should
> fallback to A IMO.
>
>
>
>>>>> [remote "origin"]
>>>>> url = https://github.com/..
>>>>> [remote "inhouse"]
>>>>> url = https://inhouse.corp/..
>>>>>
>>>>> But where do we clone it from?
>>>>> (Or do we just do a "git init" on that submodule and fetch
>>>>> from both remotes? in which order?)
>>>> origin by default and inhouse if specified. There is already a implied
>>>> default (origin). The idea was not to do both but rather what is
>>>> specified.
>>>> Origin and inhouse are just names for remotes. If one wanted a
>>>> "--all-remotes" could pull from everywhere in the Ether if feature was to
>>>> be
>>>> implemented.
>>> How is origin implied to be the default?
>>> Should there be an order (e.g. if you cannot find it at inhouse get it
>>> from github,
>>> if they are down, get it from kernel.org)
>> As it is in the Highlander series... "there can be only one" (remote). So
>> that is what I mean by origin. The only remote allowed is the "origin"
>> unless changed by the user... but there can still only be one *currently*.
>> Though I see your point as it is not labeled "origin". It is not labeled at
>> all. Apologies for confusion there.
> "origin" is just a common name for a remote, like "master" is a common name
> for branches. There is nothing inherently special about these except for
> their automatic setup after cloning/initializing a repo.
Origin is the remote from where all was derived and yes it can certainly
be renamed or deleted (provided another remote name exists) in git.
Sadly "master" is a term used in git which always implies another term.
I thought term "master" was long since killed in the computer world with
the USB 1.1 to 2.0 debate, ... but here it still lives in git. What is
wrong with the term trunk? Trees don't have masters... they have
trunks... but i digress on the term "master". I will try and get my git
terms right even if IMO they are at times the wrong terms.
> So you can delete the master branch (which I did in a project
> that I am not the authoritative maintainer of; I only have feature
> branches), and
> the repository just works fine.
I too have deleted the "master" branches in previous projects.
^ permalink raw reply
* [PATCH v2] git-p4: Fix git-p4.mapUser on Windows
From: George Vanburgh @ 2017-01-25 9:17 UTC (permalink / raw)
To: git
In-Reply-To: <01020159c0e82598-e373cf0d-2bad-41bb-b455-6896ad183e22-000000@eu-west-1.amazonses.com>
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.
Reproduction Steps:
1. Add multiple git-p4.mapUser entries to git config on a Windows
machine
2. Attempt to clone a p4 repository
None of the user mappings will be applied.
This issue is actually caused by gitConfigList, using
split(os.linesep) to convert the output of git config --get-all into
a list. On Windows, os.linesep is equal to '\r\n' - however git.exe
returns configuration with a line seperator of '\n'. This leads to
the list returned by gitConfigList containing only one element - which
contains the full output of git config --get-all in string form, which
causes problems for the code introduced to getUserMapFromPerforceServer
in 10d08a1.
This issue should be caught by the test introduced in 10d08a1, however
would require running on Windows to reproduce.
Using splitlines solves this issue, by splitting config on all
typical delimiters ('\n', '\r\n' etc.)
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 related
* [PATCH v3 2/4] urlmatch: enable normalization of URLs with globs
From: Patrick Steinhardt @ 2017-01-25 9:56 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt, Junio C Hamano, Patrick Steinhardt,
Philip Oakley
In-Reply-To: <20170123130635.29577-1-patrick.steinhardt@elego.de>
The `url_normalize` function is used to validate and normalize URLs. As
such, it does not allow for some special characters to be part of the
URLs that are to be normalized. As we want to allow using globs in some
configuration keys making use of URLs, namely `http.<url>.<key>`, but
still normalize them, we need to somehow enable some additional allowed
characters.
To do this without having to change all callers of `url_normalize`,
where most do not actually want globbing at all, we split off another
function `url_normalize_1`. This function accepts an additional
parameter `allow_globs`, which is subsequently called by `url_normalize`
with `allow_globs=0`.
As of now, this function is not used with globbing enabled. A caller
will be added in the following commit.
Signed-off-by: Patrick Steinhardt <patrick.steinhardt@elego.de>
---
urlmatch.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/urlmatch.c b/urlmatch.c
index 132d342bc..d350478c0 100644
--- a/urlmatch.c
+++ b/urlmatch.c
@@ -63,7 +63,7 @@ static int append_normalized_escapes(struct strbuf *buf,
return 1;
}
-char *url_normalize(const char *url, struct url_info *out_info)
+static char *url_normalize_1(const char *url, struct url_info *out_info, char allow_globs)
{
/*
* Normalize NUL-terminated url using the following rules:
@@ -191,7 +191,12 @@ char *url_normalize(const char *url, struct url_info *out_info)
strbuf_release(&norm);
return NULL;
}
- spanned = strspn(url, URL_HOST_CHARS);
+
+ if (allow_globs)
+ spanned = strspn(url, URL_HOST_CHARS "*");
+ else
+ spanned = strspn(url, URL_HOST_CHARS);
+
if (spanned < colon_ptr - url) {
/* Host name has invalid characters */
if (out_info) {
@@ -380,6 +385,11 @@ char *url_normalize(const char *url, struct url_info *out_info)
return result;
}
+char *url_normalize(const char *url, struct url_info *out_info)
+{
+ return url_normalize_1(url, out_info, 0);
+}
+
static size_t url_match_prefix(const char *url,
const char *url_prefix,
size_t url_prefix_len)
--
2.11.0
^ permalink raw reply related
* [PATCH v3 0/4] urlmatch: allow regexp-based matches
From: Patrick Steinhardt @ 2017-01-25 9:56 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt, Junio C Hamano, Patrick Steinhardt,
Philip Oakley
In-Reply-To: <20170123130635.29577-1-patrick.steinhardt@elego.de>
Hi,
This is version three of my patch series. The previous version
can be found at [1]. The use case is to be able to configure an
HTTP proxy for all subdomains of a domain where there are
hundreds of subdomains.
This version addresses a comment by Philip Oakley regarding the
documentation. You can find the interdiff below.
Regards
Patrick
[1]: http://public-inbox.org/git/20170124170031.18069-1-patrick.steinhardt@elego.de/T/#u
Patrick Steinhardt (4):
mailmap: add Patrick Steinhardt's work address
urlmatch: enable normalization of URLs with globs
urlmatch: split host and port fields in `struct url_info`
urlmatch: allow globbing for the URL host part
.mailmap | 1 +
Documentation/config.txt | 5 +++-
t/t1300-repo-config.sh | 36 +++++++++++++++++++++++++
urlmatch.c | 68 +++++++++++++++++++++++++++++++++++++++++-------
urlmatch.h | 9 ++++---
5 files changed, 104 insertions(+), 15 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index a78921c2b..078e9b490 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1915,9 +1915,9 @@ http.<url>.*::
. Host/domain name (e.g., `example.com` in `https://example.com/`).
This field must match between the config key and the URL. It is
- possible to use globs in the config key to match all subdomains, e.g.
- `https://*.example.com/` to match all subdomains of `example.com`. Note
- that a glob only every matches a single part of the hostname.
+ possible to specify a `*` as part of the host name to match all subdomains
+ at this level. `https://*.example.com/` for example would match
+ `https://foo.example.com/`, but not `https://foo.bar.example.com/`.
. Port number (e.g., `8080` in `http://example.com:8080/`).
This field must match exactly between the config key and the URL.
--
2.11.0
^ permalink raw reply related
* [PATCH v3 4/4] urlmatch: allow globbing for the URL host part
From: Patrick Steinhardt @ 2017-01-25 9:56 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt, Junio C Hamano, Patrick Steinhardt,
Philip Oakley
In-Reply-To: <20170123130635.29577-1-patrick.steinhardt@elego.de>
The URL matching function computes for two URLs whether they match not.
The match is performed by splitting up the URL into different parts and
then doing an exact comparison with the to-be-matched URL.
The main user of `urlmatch` is the configuration subsystem. It allows to
set certain configurations based on the URL which is being connected to
via keys like `http.<url>.*`. A common use case for this is to set
proxies for only some remotes which match the given URL. Unfortunately,
having exact matches for all parts of the URL can become quite tedious
in some setups. Imagine for example a corporate network where there are
dozens or even hundreds of subdomains, which would have to be configured
individually.
This commit introduces the ability to use globbing in the host-part of
the URLs. A user can simply specify a `*` as part of the host name to
match all subdomains at this level. For example adding a configuration
key `http.https://*.example.com.proxy` will match all subdomains of
`https://example.com`.
Signed-off-by: Patrick Steinhardt <patrick.steinhardt@elego.de>
---
Documentation/config.txt | 5 ++++-
t/t1300-repo-config.sh | 36 ++++++++++++++++++++++++++++++++++++
urlmatch.c | 38 ++++++++++++++++++++++++++++++++++----
3 files changed, 74 insertions(+), 5 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 506431267..078e9b490 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1914,7 +1914,10 @@ http.<url>.*::
must match exactly between the config key and the URL.
. Host/domain name (e.g., `example.com` in `https://example.com/`).
- This field must match exactly between the config key and the URL.
+ This field must match between the config key and the URL. It is
+ possible to specify a `*` as part of the host name to match all subdomains
+ at this level. `https://*.example.com/` for example would match
+ `https://foo.example.com/`, but not `https://foo.bar.example.com/`.
. Port number (e.g., `8080` in `http://example.com:8080/`).
This field must match exactly between the config key and the URL.
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 923bfc5a2..ec545e092 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -1177,6 +1177,42 @@ test_expect_success 'urlmatch' '
test_cmp expect actual
'
+test_expect_success 'glob-based urlmatch' '
+ cat >.git/config <<-\EOF &&
+ [http]
+ sslVerify
+ [http "https://*.example.com"]
+ sslVerify = false
+ cookieFile = /tmp/cookie.txt
+ EOF
+
+ test_expect_code 1 git config --bool --get-urlmatch doesnt.exist https://good.example.com >actual &&
+ test_must_be_empty actual &&
+
+ echo true >expect &&
+ git config --bool --get-urlmatch http.SSLverify https://example.com >actual &&
+ test_cmp expect actual &&
+
+ echo true >expect &&
+ git config --bool --get-urlmatch http.SSLverify https://good-example.com >actual &&
+ test_cmp expect actual &&
+
+ echo true >expect &&
+ git config --bool --get-urlmatch http.sslverify https://deep.nested.example.com >actual &&
+ test_cmp expect actual &&
+
+ echo false >expect &&
+ git config --bool --get-urlmatch http.sslverify https://good.example.com >actual &&
+ test_cmp expect actual &&
+
+ {
+ echo http.cookiefile /tmp/cookie.txt &&
+ echo http.sslverify false
+ } >expect &&
+ git config --get-urlmatch HTTP https://good.example.com >actual &&
+ test_cmp expect actual
+'
+
# good section hygiene
test_expect_failure 'unsetting the last key in a section removes header' '
cat >.git/config <<-\EOF &&
diff --git a/urlmatch.c b/urlmatch.c
index e328905eb..53ff972a6 100644
--- a/urlmatch.c
+++ b/urlmatch.c
@@ -63,6 +63,38 @@ static int append_normalized_escapes(struct strbuf *buf,
return 1;
}
+static int match_host(const struct url_info *url_info,
+ const struct url_info *pattern_info)
+{
+ char *url = xmemdupz(url_info->url + url_info->host_off, url_info->host_len);
+ char *pat = xmemdupz(pattern_info->url + pattern_info->host_off, pattern_info->host_len);
+ char *url_tok, *pat_tok, *url_save, *pat_save;
+ int matching;
+
+ url_tok = strtok_r(url, ".", &url_save);
+ pat_tok = strtok_r(pat, ".", &pat_save);
+
+ for (; url_tok && pat_tok; url_tok = strtok_r(NULL, ".", &url_save),
+ pat_tok = strtok_r(NULL, ".", &pat_save)) {
+ if (!strcmp(pat_tok, "*"))
+ continue; /* a simple glob matches everything */
+
+ if (strcmp(url_tok, pat_tok)) {
+ /* subdomains do not match */
+ matching = 0;
+ break;
+ }
+ }
+
+ /* matching if both URL and pattern are at their ends */
+ matching = (url_tok == NULL && pat_tok == NULL);
+
+ free(url);
+ free(pat);
+
+ return matching;
+}
+
static char *url_normalize_1(const char *url, struct url_info *out_info, char allow_globs)
{
/*
@@ -467,9 +499,7 @@ static int match_urls(const struct url_info *url,
}
/* check the host */
- if (url_prefix->host_len != url->host_len ||
- strncmp(url->url + url->host_off,
- url_prefix->url + url_prefix->host_off, url->host_len))
+ if (!match_host(url, url_prefix))
return 0; /* host names do not match */
/* check the port */
@@ -512,7 +542,7 @@ int urlmatch_config_entry(const char *var, const char *value, void *cb)
struct url_info norm_info;
config_url = xmemdupz(key, dot - key);
- norm_url = url_normalize(config_url, &norm_info);
+ norm_url = url_normalize_1(config_url, &norm_info, 1);
free(config_url);
if (!norm_url)
return 0;
--
2.11.0
^ permalink raw reply related
* [PATCH v3 1/4] mailmap: add Patrick Steinhardt's work address
From: Patrick Steinhardt @ 2017-01-25 9:56 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt, Junio C Hamano, Patrick Steinhardt,
Philip Oakley
In-Reply-To: <20170123130635.29577-1-patrick.steinhardt@elego.de>
Signed-off-by: Patrick Steinhardt <patrick.steinhardt@elego.de>
---
.mailmap | 1 +
1 file changed, 1 insertion(+)
diff --git a/.mailmap b/.mailmap
index 9c87a3840..ea59205b9 100644
--- a/.mailmap
+++ b/.mailmap
@@ -177,6 +177,7 @@ Paolo Bonzini <bonzini@gnu.org> <paolo.bonzini@lu.unisi.ch>
Pascal Obry <pascal@obry.net> <pascal.obry@gmail.com>
Pascal Obry <pascal@obry.net> <pascal.obry@wanadoo.fr>
Pat Notz <patnotz@gmail.com> <pknotz@sandia.gov>
+Patrick Steinhardt <ps@pks.im> <patrick.steinhardt@elego.de>
Paul Mackerras <paulus@samba.org> <paulus@dorrigo.(none)>
Paul Mackerras <paulus@samba.org> <paulus@pogo.(none)>
Peter Baumann <waste.manager@gmx.de> <Peter.B.Baumann@stud.informatik.uni-erlangen.de>
--
2.11.0
^ permalink raw reply related
* [PATCH v3 3/4] urlmatch: split host and port fields in `struct url_info`
From: Patrick Steinhardt @ 2017-01-25 9:56 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt, Junio C Hamano, Patrick Steinhardt,
Philip Oakley
In-Reply-To: <20170123130635.29577-1-patrick.steinhardt@elego.de>
The `url_info` structure contains information about a normalized URL
with the URL's components being represented by different fields. The
host and port part though are to be accessed by the same `host` field,
so that getting the host and/or port separately becomes more involved
than really necessary.
To make the port more readily accessible, split up the host and port
fields. Namely, the `host_len` will not include the port length anymore
and a new `port_off` field has been added which includes the offset to
the port, if available.
The only user of these fields is `url_normalize_1`. This change makes it
easier later on to treat host and port differently when introducing
globs for domains.
Signed-off-by: Patrick Steinhardt <patrick.steinhardt@elego.de>
---
urlmatch.c | 16 ++++++++++++----
urlmatch.h | 9 +++++----
2 files changed, 17 insertions(+), 8 deletions(-)
diff --git a/urlmatch.c b/urlmatch.c
index d350478c0..e328905eb 100644
--- a/urlmatch.c
+++ b/urlmatch.c
@@ -104,7 +104,7 @@ static char *url_normalize_1(const char *url, struct url_info *out_info, char al
struct strbuf norm;
size_t spanned;
size_t scheme_len, user_off=0, user_len=0, passwd_off=0, passwd_len=0;
- size_t host_off=0, host_len=0, port_len=0, path_off, path_len, result_len;
+ size_t host_off=0, host_len=0, port_off=0, port_len=0, path_off, path_len, result_len;
const char *slash_ptr, *at_ptr, *colon_ptr, *path_start;
char *result;
@@ -263,6 +263,7 @@ static char *url_normalize_1(const char *url, struct url_info *out_info, char al
return NULL;
}
strbuf_addch(&norm, ':');
+ port_off = norm.len;
strbuf_add(&norm, url, slash_ptr - url);
port_len = slash_ptr - url;
}
@@ -270,7 +271,7 @@ static char *url_normalize_1(const char *url, struct url_info *out_info, char al
url = slash_ptr;
}
if (host_off)
- host_len = norm.len - host_off;
+ host_len = norm.len - host_off - (port_len ? port_len + 1 : 0);
/*
@@ -378,6 +379,7 @@ static char *url_normalize_1(const char *url, struct url_info *out_info, char al
out_info->passwd_len = passwd_len;
out_info->host_off = host_off;
out_info->host_len = host_len;
+ out_info->port_off = port_off;
out_info->port_len = port_len;
out_info->path_off = path_off;
out_info->path_len = path_len;
@@ -464,11 +466,17 @@ static int match_urls(const struct url_info *url,
usermatched = 1;
}
- /* check the host and port */
+ /* check the host */
if (url_prefix->host_len != url->host_len ||
strncmp(url->url + url->host_off,
url_prefix->url + url_prefix->host_off, url->host_len))
- return 0; /* host names and/or ports do not match */
+ return 0; /* host names do not match */
+
+ /* check the port */
+ if (url_prefix->port_len != url->port_len ||
+ strncmp(url->url + url->port_off,
+ url_prefix->url + url_prefix->port_off, url->port_len))
+ return 0; /* ports do not match */
/* check the path */
pathmatchlen = url_match_prefix(
diff --git a/urlmatch.h b/urlmatch.h
index 528862adc..0ea812b03 100644
--- a/urlmatch.h
+++ b/urlmatch.h
@@ -18,11 +18,12 @@ struct url_info {
size_t passwd_len; /* length of passwd; if passwd_off != 0 but
passwd_len == 0, an empty passwd was given */
size_t host_off; /* offset into url to start of host name (0 => none) */
- size_t host_len; /* length of host name; this INCLUDES any ':portnum';
+ size_t host_len; /* length of host name;
* file urls may have host_len == 0 */
- size_t port_len; /* if a portnum is present (port_len != 0), it has
- * this length (excluding the leading ':') at the
- * end of the host name (always 0 for file urls) */
+ size_t port_off; /* offset into url to start of port number (0 => none) */
+ size_t port_len; /* if a portnum is present (port_off != 0), it has
+ * this length (excluding the leading ':') starting
+ * from port_off (always 0 for file urls) */
size_t path_off; /* offset into url to the start of the url path;
* this will always point to a '/' character
* after the url has been normalized */
--
2.11.0
^ permalink raw reply related
* Re: [PATCH v2 4/4] urlmatch: allow globbing for the URL host part
From: Patrick Steinhardt @ 2017-01-25 9:57 UTC (permalink / raw)
To: Philip Oakley; +Cc: git, Patrick Steinhardt, Junio C Hamano
In-Reply-To: <5BF60E6DF2C04DF081466BC3BD3F954F@PhilipOakley>
[-- Attachment #1: Type: text/plain, Size: 2827 bytes --]
On Tue, Jan 24, 2017 at 05:52:39PM -0000, Philip Oakley wrote:
> From: "Patrick Steinhardt" <patrick.steinhardt@elego.de>
>
> a quick comment on the documentation part ..
>
> > The URL matching function computes for two URLs whether they match not.
> > The match is performed by splitting up the URL into different parts and
> > then doing an exact comparison with the to-be-matched URL.
> >
> > The main user of `urlmatch` is the configuration subsystem. It allows to
> > set certain configurations based on the URL which is being connected to
> > via keys like `http.<url>.*`. A common use case for this is to set
> > proxies for only some remotes which match the given URL. Unfortunately,
> > having exact matches for all parts of the URL can become quite tedious
> > in some setups. Imagine for example a corporate network where there are
> > dozens or even hundreds of subdomains, which would have to be configured
> > individually.
> >
> > This commit introduces the ability to use globbing in the host-part of
> > the URLs. A user can simply specify a `*` as part of the host name to
> > match all subdomains at this level. For example adding a configuration
> > key `http.https://*.example.com.proxy` will match all subdomains of
> > `https://example.com`.
> >
> > Signed-off-by: Patrick Steinhardt <patrick.steinhardt@elego.de>
> > ---
> > Documentation/config.txt | 5 ++++-
> > t/t1300-repo-config.sh | 36 ++++++++++++++++++++++++++++++++++++
> > urlmatch.c | 38 ++++++++++++++++++++++++++++++++++----
> > 3 files changed, 74 insertions(+), 5 deletions(-)
> >
> > diff --git a/Documentation/config.txt b/Documentation/config.txt
> > index 506431267..a78921c2b 100644
> > --- a/Documentation/config.txt
> > +++ b/Documentation/config.txt
> > @@ -1914,7 +1914,10 @@ http.<url>.*::
> > must match exactly between the config key and the URL.
> >
> > . Host/domain name (e.g., `example.com` in `https://example.com/`).
> > - This field must match exactly between the config key and the URL.
> > + This field must match between the config key and the URL. It is
> > + possible to use globs in the config key to match all subdomains, e.g.
> > + `https://*.example.com/` to match all subdomains of `example.com`. Note
> > + that a glob only every matches a single part of the hostname.
>
> [s/every/ever/ ?]
>
> the "match all subdomains" appears to contradict the "a glob only ever
> matches a single part ".
>
> Maybe borrow the example from the 0/4 cover letter
> "so for example `https://foo.bar.example.com` would not match in the case of
> `http.https://*.example.com` " (If I understood it correctly.
>
> A simple example often clarifies much better than more words.
Thanks! (Hopefully) improved this in v3.
Regards
Patrick
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* Fixing the warning about warning(""); was: Re: [PATCH] difftool.c: mark a file-local symbol with static
From: Johannes Schindelin @ 2017-01-25 10:36 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, David Aguilar, Ramsay Jones, GIT Mailing-list
In-Reply-To: <20170124230500.h3fasbvutjkkke5h@sigill.intra.peff.net>
Hi Peff,
On Tue, 24 Jan 2017, Jeff King wrote:
> 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.
I fail to see how this function, or this definition, makes the code better
than simply calling `warning("%s", "");` and be done with it.
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