* [PATCHv2 2/3] submodule-config: rename commit_sha1 to commit_or_tree
From: Stefan Beller @ 2016-11-21 23:27 UTC (permalink / raw)
To: git, gitster; +Cc: bmwill, jacob.keller, Stefan Beller
In-Reply-To: <20161121232709.8906-1-sbeller@google.com>
It is also possible to pass in a tree hash to lookup a submodule config.
Make it clear by naming the variables accrodingly. Looking up a submodule
config by tree hash will come in handy in a later patch.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
Documentation/technical/api-submodule-config.txt | 8 ++--
submodule-config.c | 47 ++++++++++++------------
submodule-config.h | 4 +-
t/t7411-submodule-config.sh | 11 ++++++
4 files changed, 41 insertions(+), 29 deletions(-)
diff --git a/Documentation/technical/api-submodule-config.txt b/Documentation/technical/api-submodule-config.txt
index 941fa178dd..768458580f 100644
--- a/Documentation/technical/api-submodule-config.txt
+++ b/Documentation/technical/api-submodule-config.txt
@@ -47,15 +47,15 @@ Functions
Can be passed to the config parsing infrastructure to parse
local (worktree) submodule configurations.
-`const struct submodule *submodule_from_path(const unsigned char *commit_sha1, const char *path)`::
+`const struct submodule *submodule_from_path(const unsigned char *commit_or_tree, const char *path)`::
- Lookup values for one submodule by its commit_sha1 and path.
+ Lookup values for one submodule by its commit_or_tree and path.
-`const struct submodule *submodule_from_name(const unsigned char *commit_sha1, const char *name)`::
+`const struct submodule *submodule_from_name(const unsigned char *commit_or_tree, const char *name)`::
The same as above but lookup by name.
-If given the null_sha1 as commit_sha1 the local configuration of a
+If given the null_sha1 as commit_or_tree the local configuration of a
submodule will be returned (e.g. consolidated values from local git
configuration and the .gitmodules file in the worktree).
diff --git a/submodule-config.c b/submodule-config.c
index 15ffab6af4..4c5f5d074b 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -263,12 +263,12 @@ int parse_push_recurse_submodules_arg(const char *opt, const char *arg)
return parse_push_recurse(opt, arg, 1);
}
-static void warn_multiple_config(const unsigned char *commit_sha1,
+static void warn_multiple_config(const unsigned char *commit_or_tree,
const char *name, const char *option)
{
const char *commit_string = "WORKTREE";
- if (commit_sha1)
- commit_string = sha1_to_hex(commit_sha1);
+ if (commit_or_tree)
+ commit_string = sha1_to_hex(commit_or_tree);
warning("%s:.gitmodules, multiple configurations found for "
"'submodule.%s.%s'. Skipping second one!",
commit_string, name, option);
@@ -276,7 +276,7 @@ static void warn_multiple_config(const unsigned char *commit_sha1,
struct parse_config_parameter {
struct submodule_cache *cache;
- const unsigned char *commit_sha1;
+ const unsigned char *commit_or_tree;
const unsigned char *gitmodules_sha1;
int overwrite;
};
@@ -300,7 +300,7 @@ static int parse_config(const char *var, const char *value, void *data)
if (!value)
ret = config_error_nonbool(var);
else if (!me->overwrite && submodule->path)
- warn_multiple_config(me->commit_sha1, submodule->name,
+ warn_multiple_config(me->commit_or_tree, submodule->name,
"path");
else {
if (submodule->path)
@@ -314,7 +314,7 @@ static int parse_config(const char *var, const char *value, void *data)
int die_on_error = is_null_sha1(me->gitmodules_sha1);
if (!me->overwrite &&
submodule->fetch_recurse != RECURSE_SUBMODULES_NONE)
- warn_multiple_config(me->commit_sha1, submodule->name,
+ warn_multiple_config(me->commit_or_tree, submodule->name,
"fetchrecursesubmodules");
else
submodule->fetch_recurse = parse_fetch_recurse(
@@ -324,7 +324,7 @@ static int parse_config(const char *var, const char *value, void *data)
if (!value)
ret = config_error_nonbool(var);
else if (!me->overwrite && submodule->ignore)
- warn_multiple_config(me->commit_sha1, submodule->name,
+ warn_multiple_config(me->commit_or_tree, submodule->name,
"ignore");
else if (strcmp(value, "untracked") &&
strcmp(value, "dirty") &&
@@ -340,7 +340,7 @@ static int parse_config(const char *var, const char *value, void *data)
if (!value) {
ret = config_error_nonbool(var);
} else if (!me->overwrite && submodule->url) {
- warn_multiple_config(me->commit_sha1, submodule->name,
+ warn_multiple_config(me->commit_or_tree, submodule->name,
"url");
} else {
free((void *) submodule->url);
@@ -351,21 +351,21 @@ static int parse_config(const char *var, const char *value, void *data)
ret = config_error_nonbool(var);
else if (!me->overwrite &&
submodule->update_strategy.type != SM_UPDATE_UNSPECIFIED)
- warn_multiple_config(me->commit_sha1, submodule->name,
+ warn_multiple_config(me->commit_or_tree, submodule->name,
"update");
else if (parse_submodule_update_strategy(value,
&submodule->update_strategy) < 0)
die(_("invalid value for %s"), var);
} else if (!strcmp(item.buf, "shallow")) {
if (!me->overwrite && submodule->recommend_shallow != -1)
- warn_multiple_config(me->commit_sha1, submodule->name,
+ warn_multiple_config(me->commit_or_tree, submodule->name,
"shallow");
else
submodule->recommend_shallow =
git_config_bool(var, value);
} else if (!strcmp(item.buf, "branch")) {
if (!me->overwrite && submodule->branch)
- warn_multiple_config(me->commit_sha1, submodule->name,
+ warn_multiple_config(me->commit_or_tree, submodule->name,
"branch");
else {
free((void *)submodule->branch);
@@ -379,18 +379,18 @@ static int parse_config(const char *var, const char *value, void *data)
return ret;
}
-static int gitmodule_sha1_from_commit(const unsigned char *commit_sha1,
+static int gitmodule_sha1_from_commit(const unsigned char *commit_or_tree,
unsigned char *gitmodules_sha1,
struct strbuf *rev)
{
int ret = 0;
- if (is_null_sha1(commit_sha1)) {
+ if (is_null_sha1(commit_or_tree)) {
hashclr(gitmodules_sha1);
return 1;
}
- strbuf_addf(rev, "%s:.gitmodules", sha1_to_hex(commit_sha1));
+ strbuf_addf(rev, "%s:.gitmodules", sha1_to_hex(commit_or_tree));
if (get_sha1(rev->buf, gitmodules_sha1) >= 0)
ret = 1;
@@ -402,7 +402,7 @@ static int gitmodule_sha1_from_commit(const unsigned char *commit_sha1,
* revisions.
*/
static const struct submodule *config_from(struct submodule_cache *cache,
- const unsigned char *commit_sha1, const char *key,
+ const unsigned char *commit_or_tree, const char *key,
enum lookup_type lookup_type)
{
struct strbuf rev = STRBUF_INIT;
@@ -418,7 +418,7 @@ static const struct submodule *config_from(struct submodule_cache *cache,
* return the first submodule. Can be used to check whether
* there are any submodules parsed.
*/
- if (!commit_sha1 || !key) {
+ if (!commit_or_tree || !key) {
struct hashmap_iter iter;
struct submodule_entry *entry;
@@ -428,7 +428,7 @@ static const struct submodule *config_from(struct submodule_cache *cache,
return entry->config;
}
- if (!gitmodule_sha1_from_commit(commit_sha1, sha1, &rev))
+ if (!gitmodule_sha1_from_commit(commit_or_tree, sha1, &rev))
goto out;
switch (lookup_type) {
@@ -448,7 +448,8 @@ static const struct submodule *config_from(struct submodule_cache *cache,
/* fill the submodule config into the cache */
parameter.cache = cache;
- parameter.commit_sha1 = commit_sha1;
+ // todo: get the actual tree here:
+ parameter.commit_or_tree = commit_or_tree;
parameter.gitmodules_sha1 = sha1;
parameter.overwrite = 0;
git_config_from_mem(parse_config, CONFIG_ORIGIN_SUBMODULE_BLOB, rev.buf,
@@ -484,7 +485,7 @@ int parse_submodule_config_option(const char *var, const char *value)
{
struct parse_config_parameter parameter;
parameter.cache = &the_submodule_cache;
- parameter.commit_sha1 = NULL;
+ parameter.commit_or_tree = NULL;
parameter.gitmodules_sha1 = null_sha1;
parameter.overwrite = 1;
@@ -492,18 +493,18 @@ int parse_submodule_config_option(const char *var, const char *value)
return parse_config(var, value, ¶meter);
}
-const struct submodule *submodule_from_name(const unsigned char *commit_sha1,
+const struct submodule *submodule_from_name(const unsigned char *commit_or_tree,
const char *name)
{
ensure_cache_init();
- return config_from(&the_submodule_cache, commit_sha1, name, lookup_name);
+ return config_from(&the_submodule_cache, commit_or_tree, name, lookup_name);
}
-const struct submodule *submodule_from_path(const unsigned char *commit_sha1,
+const struct submodule *submodule_from_path(const unsigned char *commit_or_tree,
const char *path)
{
ensure_cache_init();
- return config_from(&the_submodule_cache, commit_sha1, path, lookup_path);
+ return config_from(&the_submodule_cache, commit_or_tree, path, lookup_path);
}
void submodule_free(void)
diff --git a/submodule-config.h b/submodule-config.h
index d05c542d2c..99df8e593c 100644
--- a/submodule-config.h
+++ b/submodule-config.h
@@ -25,9 +25,9 @@ struct submodule {
int parse_fetch_recurse_submodules_arg(const char *opt, const char *arg);
int parse_push_recurse_submodules_arg(const char *opt, const char *arg);
int parse_submodule_config_option(const char *var, const char *value);
-const struct submodule *submodule_from_name(const unsigned char *commit_sha1,
+const struct submodule *submodule_from_name(const unsigned char *commit_or_tree,
const char *name);
-const struct submodule *submodule_from_path(const unsigned char *commit_sha1,
+const struct submodule *submodule_from_path(const unsigned char *commit_or_tree,
const char *path);
void submodule_free(void);
diff --git a/t/t7411-submodule-config.sh b/t/t7411-submodule-config.sh
index 47562ce465..301ed5e48f 100755
--- a/t/t7411-submodule-config.sh
+++ b/t/t7411-submodule-config.sh
@@ -93,6 +93,17 @@ test_expect_success 'error message contains blob reference' '
)
'
+test_expect_success 'using tree sha1 works' '
+ (
+ cd super &&
+ tree=$(git rev-parse HEAD^{tree}) &&
+ commit=$(git rev-parse HEAD^{commit}) &&
+ test-submodule-config $commit b >expect &&
+ test-submodule-config $tree b >actual &&
+ test_cmp expect actual
+ )
+'
+
cat >super/expect_url <<EOF
Submodule url: 'git@somewhere.else.net:a.git' for path 'b'
Submodule url: 'git@somewhere.else.net:submodule.git' for path 'submodule'
--
2.11.0.rc2.18.g0126045.dirty
^ permalink raw reply related
* [PATCHv2 1/3] submodule config: inline config_from_{name, path}
From: Stefan Beller @ 2016-11-21 23:27 UTC (permalink / raw)
To: git, gitster; +Cc: bmwill, jacob.keller, Stefan Beller
In-Reply-To: <20161121232709.8906-1-sbeller@google.com>
There is no other user of config_from_{name, path}, such that there is no
reason for the existence of these one liner functions. Just inline these
to increase readability.
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
submodule-config.c | 16 ++--------------
1 file changed, 2 insertions(+), 14 deletions(-)
diff --git a/submodule-config.c b/submodule-config.c
index 098085be69..15ffab6af4 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -471,18 +471,6 @@ static const struct submodule *config_from(struct submodule_cache *cache,
return submodule;
}
-static const struct submodule *config_from_path(struct submodule_cache *cache,
- const unsigned char *commit_sha1, const char *path)
-{
- return config_from(cache, commit_sha1, path, lookup_path);
-}
-
-static const struct submodule *config_from_name(struct submodule_cache *cache,
- const unsigned char *commit_sha1, const char *name)
-{
- return config_from(cache, commit_sha1, name, lookup_name);
-}
-
static void ensure_cache_init(void)
{
if (is_cache_init)
@@ -508,14 +496,14 @@ const struct submodule *submodule_from_name(const unsigned char *commit_sha1,
const char *name)
{
ensure_cache_init();
- return config_from_name(&the_submodule_cache, commit_sha1, name);
+ return config_from(&the_submodule_cache, commit_sha1, name, lookup_name);
}
const struct submodule *submodule_from_path(const unsigned char *commit_sha1,
const char *path)
{
ensure_cache_init();
- return config_from_path(&the_submodule_cache, commit_sha1, path);
+ return config_from(&the_submodule_cache, commit_sha1, path, lookup_path);
}
void submodule_free(void)
--
2.11.0.rc2.18.g0126045.dirty
^ permalink raw reply related
* [PATCHv2 0/3] submodule-config: clarify/cleanup docs and header
From: Stefan Beller @ 2016-11-21 23:27 UTC (permalink / raw)
To: git, gitster; +Cc: bmwill, jacob.keller, Stefan Beller
replacing sb/submodule-config-cleanup
v2:
addressed Jacobs concerns in patch2, fixing all occurrences of commit_sha1.
Thanks,
Stefan
interdiff to v1:
diff --git a/Documentation/technical/api-submodule-config.txt b/Documentation/technical/api-submodule-config.txt
index 1df7a827ff..a91c1f085e 100644
--- a/Documentation/technical/api-submodule-config.txt
+++ b/Documentation/technical/api-submodule-config.txt
@@ -49,7 +49,7 @@ Functions
`const struct submodule *submodule_from_path(const unsigned char *commit_or_tree, const char *path)`::
- Lookup values for one submodule by its commit_sha1 and path.
+ Lookup values for one submodule by its commit_or_tree and path.
`const struct submodule *submodule_from_name(const unsigned char *commit_or_tree, const char *name)`::
v1:
A small series that would have helped me understand the submodule config
once again.
Thanks,
Stefan
Stefan Beller (3):
submodule config: inline config_from_{name, path}
submodule-config: rename commit_sha1 to commit_or_tree
submodule-config: clarify parsing of null_sha1 element
Documentation/technical/api-submodule-config.txt | 13 ++++--
submodule-config.c | 59 ++++++++++--------------
submodule-config.h | 4 +-
t/t7411-submodule-config.sh | 11 +++++
4 files changed, 45 insertions(+), 42 deletions(-)
--
2.11.0.rc2.18.g0126045.dirty
^ permalink raw reply related
* Re: What's cooking in git.git (Nov 2016, #04; Mon, 21)
From: Stefan Beller @ 2016-11-21 23:19 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git@vger.kernel.org
In-Reply-To: <xmqqh970h1da.fsf@gitster.mtv.corp.google.com>
On Mon, Nov 21, 2016 at 3:14 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>
>> I did review both off and on list and I think the latest version is good.
>
> I thought that there were strange mixups of two enumeration types
> that are incompatible, at least. Is there an update that I didn't
> see, or you didn't read problems pointed out on list?
Oh right; there was no resolution to that one IIRC.
>
>>> * jt/use-trailer-api-in-commands (2016-11-02) 6 commits
..
>> From a cursory read (with the SQUASH applied)
>> this seems to be done to me.
>
> We are not all that in a hurry to move non-fix to 'next' only with a
> cursory read at this point in the cycle ;-).
But the cycle only applies to patches going to master, but when
asking for doneness I assumed you were asking for opinions on
the transition pu -> next, which I would support.
Thanks,
Stefan
^ permalink raw reply
* Re: What's cooking in git.git (Nov 2016, #04; Mon, 21)
From: Junio C Hamano @ 2016-11-21 23:14 UTC (permalink / raw)
To: Stefan Beller; +Cc: git@vger.kernel.org
In-Reply-To: <CAGZ79kbCebHojPDUdHn2mmjUwRe-FOcnJqtoa828PJwC6P5mNw@mail.gmail.com>
Stefan Beller <sbeller@google.com> writes:
>> * bw/grep-recurse-submodules (2016-11-18) 6 commits
>> - grep: search history of moved submodules
>> - grep: enable recurse-submodules to work on <tree> objects
>> - grep: optionally recurse into submodules
>> - grep: add submodules as a grep source type
>> - submodules: load gitmodules file from commit sha1
>> - submodules: add helper functions to determine presence of submodules
>>
>> "git grep" learns to optionally recurse into submodules
>>
>> Waiting for review.
>
> I did review both off and on list and I think the latest version is good.
I thought that there were strange mixups of two enumeration types
that are incompatible, at least. Is there an update that I didn't
see, or you didn't read problems pointed out on list?
>> * jt/use-trailer-api-in-commands (2016-11-02) 6 commits
>> - sequencer: use trailer's trailer layout
>> - trailer: have function to describe trailer layout
>> - trailer: avoid unnecessary splitting on lines
>> - commit: make ignore_non_trailer take buf/len
>> - SQUASH???
>> - trailer: be stricter in parsing separators
>>
>> Commands that operate on a log message and add lines to the trailer
>> blocks, such as "format-patch -s", "cherry-pick (-x|-s)", and
>> "commit -s", have been taught to use the logic of and share the
>> code with "git interpret-trailer".
>>
>> What's the doneness of this topic?
>
> From a cursory read (with the SQUASH applied)
> this seems to be done to me.
We are not all that in a hurry to move non-fix to 'next' only with a
cursory read at this point in the cycle ;-).
>> * sb/submodule-config-cleanup (2016-11-02) 3 commits
>> - submodule-config: clarify parsing of null_sha1 element
>> - submodule-config: rename commit_sha1 to commit_or_tree
>> - submodule config: inline config_from_{name, path}
>>
>> What's the doneness of this topic?
>
> Jake Keller reviewed this and it turns out I was not careful in patch 2/3.
>
> Will resend.
OK. Thanks.
^ permalink raw reply
* Re: What's cooking in git.git (Nov 2016, #04; Mon, 21)
From: Junio C Hamano @ 2016-11-21 23:05 UTC (permalink / raw)
To: Stefan Beller; +Cc: git@vger.kernel.org
In-Reply-To: <CAGZ79kbCebHojPDUdHn2mmjUwRe-FOcnJqtoa828PJwC6P5mNw@mail.gmail.com>
Stefan Beller <sbeller@google.com> writes:
>> * sb/push-make-submodule-check-the-default (2016-10-10) 2 commits
>> - push: change submodule default to check when submodules exist
>> - submodule add: extend force flag to add existing repos
>>
>> Turn the default of "push.recurseSubmodules" to "check" when
>> submodules seem to be in use.
>>
>> Will hold to wait for hv/submodule-not-yet-pushed-fix
>
> Which is cooking in next, so we'd want to include this into next as well?
Not really. One step at a time.
^ permalink raw reply
* Re: What's cooking in git.git (Nov 2016, #04; Mon, 21)
From: Stefan Beller @ 2016-11-21 22:57 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git@vger.kernel.org
In-Reply-To: <xmqqpoloh3xa.fsf@gitster.mtv.corp.google.com>
> * sb/push-make-submodule-check-the-default (2016-10-10) 2 commits
> - push: change submodule default to check when submodules exist
> - submodule add: extend force flag to add existing repos
>
> Turn the default of "push.recurseSubmodules" to "check" when
> submodules seem to be in use.
>
> Will hold to wait for hv/submodule-not-yet-pushed-fix
Which is cooking in next, so we'd want to include this into next as well?
When including this series, we get 2 benefits:
* the cooking of hv/submodule-not-yet-pushed-fix is greatly enhanced as
more submodule users will make use of it (as it would be the default).
* for non submodule users we would see if the approximated estimation
if the user cares about submodules produces false positives:
if (has_submodules_configured || file_exists(git_path("modules")) ||
(!is_bare_repository() && file_exists(".gitmodules")))
recurse_submodules = RECURSE_SUBMODULES_CHECK;
else
recurse_submodules = RECURSE_SUBMODULES_OFF;
This heuristic was introduced after we got burned and called out by Linus,
so I would expect this series to not stress non submodule users any more.
>
> * dt/empty-submodule-in-merge (2016-11-17) 1 commit
> - submodules: allow empty working-tree dirs in merge/cherry-pick
>
> An empty directory in a working tree that can simply be nuked used
> to interfere while merging or cherry-picking a change to create a
> submodule directory there, which has been fixed..
>
> Waiting for review.
I thought I had reviewed it, will do again and comment.
> * bw/grep-recurse-submodules (2016-11-18) 6 commits
> - grep: search history of moved submodules
> - grep: enable recurse-submodules to work on <tree> objects
> - grep: optionally recurse into submodules
> - grep: add submodules as a grep source type
> - submodules: load gitmodules file from commit sha1
> - submodules: add helper functions to determine presence of submodules
>
> "git grep" learns to optionally recurse into submodules
>
> Waiting for review.
I did review both off and on list and I think the latest version is good.
> * hv/submodule-not-yet-pushed-fix (2016-11-16) 4 commits
> (merged to 'next' on 2016-11-21 at 1a599af962)
> + submodule_needs_pushing(): explain the behaviour when we cannot answer
> + batch check whether submodule needs pushing into one call
> + serialize collection of refs that contain submodule changes
> + serialize collection of changed submodules
>
> The code in "git push" to compute if any commit being pushed in the
> superproject binds a commit in a submodule that hasn't been pushed
> out was overly inefficient, making it unusable even for a small
> project that does not have any submodule but have a reasonable
> number of refs.
>
> Will cook in 'next'.
Thanks!
>
> * jt/use-trailer-api-in-commands (2016-11-02) 6 commits
> - sequencer: use trailer's trailer layout
> - trailer: have function to describe trailer layout
> - trailer: avoid unnecessary splitting on lines
> - commit: make ignore_non_trailer take buf/len
> - SQUASH???
> - trailer: be stricter in parsing separators
>
> Commands that operate on a log message and add lines to the trailer
> blocks, such as "format-patch -s", "cherry-pick (-x|-s)", and
> "commit -s", have been taught to use the logic of and share the
> code with "git interpret-trailer".
>
> What's the doneness of this topic?
From a cursory read (with the SQUASH applied)
this seems to be done to me.
> * sb/submodule-config-cleanup (2016-11-02) 3 commits
> - submodule-config: clarify parsing of null_sha1 element
> - submodule-config: rename commit_sha1 to commit_or_tree
> - submodule config: inline config_from_{name, path}
>
> What's the doneness of this topic?
Jake Keller reviewed this and it turns out I was not careful in patch 2/3.
Will resend.
Thanks,
Stefan
^ permalink raw reply
* What's cooking in git.git (Nov 2016, #04; Mon, 21)
From: Junio C Hamano @ 2016-11-21 22:19 UTC (permalink / raw)
To: git
Here are the topics that have been cooking. Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'. The ones marked with '.' do not appear in any of
the integration branches, but I am still holding onto them.
You can find the changes described here in the integration branches
of the repositories listed at
http://git-blame.blogspot.com/p/git-public-repositories.html
--------------------------------------------------
[New Topics]
* jc/for-each-ref-head-segfault-fix (2016-11-18) 1 commit
(merged to 'next' on 2016-11-21 at 3c1f352316)
+ for-each-ref: do not segv with %(HEAD) on an unborn branch
Using a %(HEAD) placeholder in "for-each-ref --format=" option
caused the command to segfault when on an unborn branch.
Will cook in 'next'.
* js/prepare-sequencer (2016-11-21) 1 commit
(merged to 'next' on 2016-11-21 at 7cdf4ca421)
+ i18n: fix unmatched single quote in error message
Fix for an error message string.
Will merge to 'master'.
* js/rebase-i-commentchar-fix (2016-11-21) 3 commits
- rebase -i: handle core.commentChar=auto
- stripspace: respect repository config
- rebase -i: highlight problems with core.commentchar
"git rebase -i" did not work well with core.commentchar
configuration variable for two reasons, both of which have been
fixed.
Waiting for an ack for updates.
Hopefully we can merge this before 2.11 final, as one of the
breakages is a recent regression.
* jc/cache-tree-wip (2016-11-18) 4 commits
- cache-tree: freshen the tree object at the top level
- cache-tree: retire cache_tree_fully_valid() API function
- commit: remove redundant check for active_cache_changed
- freshen_object(): factor out a helper function
* jk/trailers-placeholder-in-pretty (2016-11-21) 2 commits
- ref-filter: add support to display trailers as part of contents
- pretty: add %(trailers) format for displaying trailers of a commit message
* jt/trailer-with-cruft (2016-11-21) 1 commit
- doc: mention user-configured trailers
* sb/submodule-intern-gitdir (2016-11-21) 3 commits
- submodule--helper: add intern-git-dir function
- test-lib-functions.sh: teach test_commit -C <dir>
- submodule: use absolute path for computing relative path connecting
--------------------------------------------------
[Graduated to "master"]
* jk/create-branch-remove-unused-param (2016-11-09) 1 commit
(merged to 'next' on 2016-11-16 at 621254c832)
+ create_branch: drop unused "head" parameter
Code clean-up.
* nd/worktree-lock (2016-11-13) 1 commit
(merged to 'next' on 2016-11-16 at 67b731de07)
+ git-worktree.txt: fix typo "to"/"two", and add comma
Typofix.
* tk/diffcore-delta-remove-unused (2016-11-14) 1 commit
(merged to 'next' on 2016-11-16 at 51e66c2fa7)
+ diffcore-delta: remove unused parameter to diffcore_count_changes()
Code cleanup.
--------------------------------------------------
[Stalled]
* sb/push-make-submodule-check-the-default (2016-10-10) 2 commits
- push: change submodule default to check when submodules exist
- submodule add: extend force flag to add existing repos
Turn the default of "push.recurseSubmodules" to "check" when
submodules seem to be in use.
Will hold to wait for hv/submodule-not-yet-pushed-fix
* jc/bundle (2016-03-03) 6 commits
- index-pack: --clone-bundle option
- Merge branch 'jc/index-pack' into jc/bundle
- bundle v3: the beginning
- bundle: keep a copy of bundle file name in the in-core bundle header
- bundle: plug resource leak
- bundle doc: 'verify' is not about verifying the bundle
The beginning of "split bundle", which could be one of the
ingredients to allow "git clone" traffic off of the core server
network to CDN.
While I think it would make it easier for people to experiment and
build on if the topic is merged to 'next', I am at the same time a
bit reluctant to merge an unproven new topic that introduces a new
file format, which we may end up having to support til the end of
time. It is likely that to support a "prime clone from CDN", it
would need a lot more than just "these are the heads and the pack
data is over there", so this may not be sufficient.
Will discard.
* mh/connect (2016-06-06) 10 commits
- connect: [host:port] is legacy for ssh
- connect: move ssh command line preparation to a separate function
- connect: actively reject git:// urls with a user part
- connect: change the --diag-url output to separate user and host
- connect: make parse_connect_url() return the user part of the url as a separate value
- connect: group CONNECT_DIAG_URL handling code
- connect: make parse_connect_url() return separated host and port
- connect: re-derive a host:port string from the separate host and port variables
- connect: call get_host_and_port() earlier
- connect: document why we sometimes call get_port after get_host_and_port
Rewrite Git-URL parsing routine (hopefully) without changing any
behaviour.
It has been two months without any support. We may want to discard
this.
* ec/annotate-deleted (2015-11-20) 1 commit
- annotate: skip checking working tree if a revision is provided
Usability fix for annotate-specific "<file> <rev>" syntax with deleted
files.
Has been waiting for a review for too long without seeing anything.
Will discard.
* dk/gc-more-wo-pack (2016-01-13) 4 commits
- gc: clean garbage .bitmap files from pack dir
- t5304: ensure non-garbage files are not deleted
- t5304: test .bitmap garbage files
- prepare_packed_git(): find more garbage
Follow-on to dk/gc-idx-wo-pack topic, to clean up stale
.bitmap and .keep files.
Has been waiting for a reroll for too long.
cf. <xmqq60ypbeng.fsf@gitster.mtv.corp.google.com>
Will discard.
* jc/diff-b-m (2015-02-23) 5 commits
. WIPWIP
. WIP: diff-b-m
- diffcore-rename: allow easier debugging
- diffcore-rename.c: add locate_rename_src()
- diffcore-break: allow debugging
"git diff -B -M" produced incorrect patch when the postimage of a
completely rewritten file is similar to the preimage of a removed
file; such a resulting file must not be expressed as a rename from
other place.
The fix in this patch is broken, unfortunately.
Will discard.
--------------------------------------------------
[Cooking]
* dt/empty-submodule-in-merge (2016-11-17) 1 commit
- submodules: allow empty working-tree dirs in merge/cherry-pick
An empty directory in a working tree that can simply be nuked used
to interfere while merging or cherry-picking a change to create a
submodule directory there, which has been fixed..
Waiting for review.
* bw/grep-recurse-submodules (2016-11-18) 6 commits
- grep: search history of moved submodules
- grep: enable recurse-submodules to work on <tree> objects
- grep: optionally recurse into submodules
- grep: add submodules as a grep source type
- submodules: load gitmodules file from commit sha1
- submodules: add helper functions to determine presence of submodules
"git grep" learns to optionally recurse into submodules
Waiting for review.
* dt/smart-http-detect-server-going-away (2016-11-18) 2 commits
(merged to 'next' on 2016-11-21 at d502523347)
+ upload-pack: optionally allow fetching any sha1
+ remote-curl: don't hang when a server dies before any output
When the http server gives an incomplete response to a smart-http
rpc call, it could lead to client waiting for a full response that
will never come. Teach the client side to notice this condition
and abort the transfer.
An improvement counterproposal has failed.
cf. <20161114194049.mktpsvgdhex2f4zv@sigill.intra.peff.net>
Will cook in 'next'.
* mm/push-social-engineering-attack-doc (2016-11-14) 1 commit
(merged to 'next' on 2016-11-16 at b7c1b27563)
+ doc: mention transfer data leaks in more places
Doc update on fetching and pushing.
Will cook in 'next'.
* nd/worktree-move (2016-11-12) 11 commits
. worktree remove: new command
. worktree move: refuse to move worktrees with submodules
. worktree move: accept destination as directory
. worktree move: new command
. worktree.c: add update_worktree_location()
. worktree.c: add validate_worktree()
. copy.c: convert copy_file() to copy_dir_recursively()
. copy.c: style fix
. copy.c: convert bb_(p)error_msg to error(_errno)
. copy.c: delete unused code in copy_file()
. copy.c: import copy_file() from busybox
"git worktree" learned move and remove subcommands.
Waiting for a reroll.
Seems to break a test or two.
* jc/compression-config (2016-11-15) 1 commit
- compression: unify pack.compression configuration parsing
Compression setting for producing packfiles were spread across
three codepaths, one of which did not honor any configuration.
Unify these so that all of them honor core.compression and
pack.compression variables the same way.
Needs tests for pack-objects and fast-import.
* mm/gc-safety-doc (2016-11-16) 1 commit
(merged to 'next' on 2016-11-17 at fc0d225b6b)
+ git-gc.txt: expand discussion of races with other processes
Doc update.
Will cook in 'next'.
* hv/submodule-not-yet-pushed-fix (2016-11-16) 4 commits
(merged to 'next' on 2016-11-21 at 1a599af962)
+ submodule_needs_pushing(): explain the behaviour when we cannot answer
+ batch check whether submodule needs pushing into one call
+ serialize collection of refs that contain submodule changes
+ serialize collection of changed submodules
The code in "git push" to compute if any commit being pushed in the
superproject binds a commit in a submodule that hasn't been pushed
out was overly inefficient, making it unusable even for a small
project that does not have any submodule but have a reasonable
number of refs.
Will cook in 'next'.
* kn/ref-filter-branch-list (2016-11-15) 18 commits
- for-each-ref: do not segv with %(HEAD) on an unborn branch
- branch: implement '--format' option
- branch: use ref-filter printing APIs
- branch, tag: use porcelain output
- ref-filter: allow porcelain to translate messages in the output
- ref-filter: add `:dir` and `:base` options for ref printing atoms
- ref-filter: make remote_ref_atom_parser() use refname_atom_parser_internal()
- ref-filter: introduce symref_atom_parser() and refname_atom_parser()
- ref-filter: introduce refname_atom_parser_internal()
- ref-filter: make "%(symref)" atom work with the ':short' modifier
- ref-filter: add support for %(upstream:track,nobracket)
- ref-filter: make %(upstream:track) prints "[gone]" for invalid upstreams
- ref-filter: introduce format_ref_array_item()
- ref-filter: move get_head_description() from branch.c
- ref-filter: modify "%(objectname:short)" to take length
- ref-filter: implement %(if:equals=<string>) and %(if:notequals=<string>)
- ref-filter: include reference to 'used_atom' within 'atom_value'
- ref-filter: implement %(if), %(then), and %(else) atoms
The code to list branches in "git branch" has been consolidated
with the more generic ref-filter API.
Rerolled, reviewed, looking good.
Expecting a reroll.
cf. <20161108201211.25213-1-Karthik.188@gmail.com>
cf. <CAOLa=ZQqe3vEj_428d41vd_4kfjzsm87Wam6Zm2dhXWkPdJ8Rw@mail.gmail.com>
cf. <xmqq7f84tqa7.fsf_-_@gitster.mtv.corp.google.com>
* bw/transport-protocol-policy (2016-11-09) 2 commits
(merged to 'next' on 2016-11-16 at 1391d3eeed)
+ transport: add protocol policy config option
+ lib-proto-disable: variable name fix
Finer-grained control of what protocols are allowed for transports
during clone/fetch/push have been enabled via a new configuration
mechanism.
Will cook in 'next'.
* jt/fetch-no-redundant-tag-fetch-map (2016-11-11) 1 commit
(merged to 'next' on 2016-11-16 at 5846c27cc5)
+ fetch: do not redundantly calculate tag refmap
Code cleanup to avoid using redundant refspecs while fetching with
the --tags option.
Will cook in 'next'.
* jc/retire-compaction-heuristics (2016-11-02) 3 commits
- SQUASH???
- SQUASH???
- diff: retire the original experimental "compaction" heuristics
Waiting for a reroll.
* jc/abbrev-autoscale-config (2016-11-01) 1 commit
- config.abbrev: document the new default that auto-scales
Waiting for a reroll.
* jk/nofollow-attr-ignore (2016-11-02) 5 commits
- exclude: do not respect symlinks for in-tree .gitignore
- attr: do not respect symlinks for in-tree .gitattributes
- exclude: convert "check_index" into a flags field
- attr: convert "macro_ok" into a flags field
- add open_nofollow() helper
As we do not follow symbolic links when reading control files like
.gitignore and .gitattributes from the index, match the behaviour
and not follow symbolic links when reading them from the working
tree. This also tightens security a bit by not leaking contents of
an unrelated file in the error messages when it is pointed at by
one of these files that is a symbolic link.
Perhaps we want to cover .gitmodules too with the same mechanism?
* sb/submodule-config-cleanup (2016-11-02) 3 commits
- submodule-config: clarify parsing of null_sha1 element
- submodule-config: rename commit_sha1 to commit_or_tree
- submodule config: inline config_from_{name, path}
What's the doneness of this topic?
* jc/push-default-explicit (2016-10-31) 2 commits
(merged to 'next' on 2016-11-01 at 8dc3a6cf25)
+ push: test pushing ambiguously named branches
+ push: do not use potentially ambiguous default refspec
A lazy "git push" without refspec did not internally use a fully
specified refspec to perform 'current', 'simple', or 'upstream'
push, causing unnecessary "ambiguous ref" errors.
Will cook in 'next'.
* jt/use-trailer-api-in-commands (2016-11-02) 6 commits
- sequencer: use trailer's trailer layout
- trailer: have function to describe trailer layout
- trailer: avoid unnecessary splitting on lines
- commit: make ignore_non_trailer take buf/len
- SQUASH???
- trailer: be stricter in parsing separators
Commands that operate on a log message and add lines to the trailer
blocks, such as "format-patch -s", "cherry-pick (-x|-s)", and
"commit -s", have been taught to use the logic of and share the
code with "git interpret-trailer".
What's the doneness of this topic?
* nd/rebase-forget (2016-10-28) 1 commit
- rebase: add --forget to cleanup rebase, leave HEAD untouched
"git rebase" learned "--forget" option, which allows a user to
remove the metadata left by an earlier "git rebase" that was
manually aborted without using "git rebase --abort".
Waiting for a reroll.
* jc/git-open-cloexec (2016-11-02) 3 commits
- sha1_file: stop opening files with O_NOATIME
- git_open_cloexec(): use fcntl(2) w/ FD_CLOEXEC fallback
- git_open(): untangle possible NOATIME and CLOEXEC interactions
The codeflow of setting NOATIME and CLOEXEC on file descriptors Git
opens has been simplified.
We may want to drop the tip one.
* jk/no-looking-at-dotgit-outside-repo-final (2016-10-26) 1 commit
(merged to 'next' on 2016-10-26 at 220e160451)
+ setup_git_env: avoid blind fall-back to ".git"
This is the endgame of the topic to avoid blindly falling back to
".git" when the setup sequence said we are _not_ in Git repository.
A corner case that happens to work right now may be broken by a
call to die("BUG").
Will cook in 'next'.
* jc/reset-unmerge (2016-10-24) 1 commit
- reset: --unmerge
After "git add" is run prematurely during a conflict resolution,
"git diff" can no longer be used as a way to sanity check by
looking at the combined diff. "git reset" learned a new
"--unmerge" option to recover from this situation.
Will discard.
This may not be needed, given that update-index has a similar
option.
* jc/merge-base-fp-only (2016-10-19) 8 commits
. merge-base: fp experiment
- merge: allow to use only the fp-only merge bases
- merge-base: limit the output to bases that are on first-parent chain
- merge-base: mark bases that are on first-parent chain
- merge-base: expose get_merge_bases_many_0() a bit more
- merge-base: stop moving commits around in remove_redundant()
- sha1_name: remove ONELINE_SEEN bit
- commit: simplify fastpath of merge-base
An experiment of merge-base that ignores common ancestors that are
not on the first parent chain.
Will discard.
The whole premise feels wrong.
* tb/convert-stream-check (2016-10-27) 2 commits
- convert.c: stream and fast search for binary
- read-cache: factor out get_sha1_from_index() helper
End-of-line conversion sometimes needs to see if the current blob
in the index has NULs and CRs to base its decision. We used to
always get a full statistics over the blob, but in many cases we
can return early when we have seen "enough" (e.g. if we see a
single NUL, the blob will be handled as binary). The codepaths
have been optimized by using streaming interface.
Waiting for review.
The tip seems to do too much in a single commit and may be better split.
cf. <20161012134724.28287-1-tboegi@web.de>
cf. <xmqqd1il5w4e.fsf@gitster.mtv.corp.google.com>
* pb/bisect (2016-10-18) 27 commits
- bisect--helper: remove the dequote in bisect_start()
- bisect--helper: retire `--bisect-auto-next` subcommand
- bisect--helper: retire `--bisect-autostart` subcommand
- bisect--helper: retire `--bisect-write` subcommand
- bisect--helper: `bisect_replay` shell function in C
- bisect--helper: `bisect_log` shell function in C
- bisect--helper: retire `--write-terms` subcommand
- bisect--helper: retire `--check-expected-revs` subcommand
- bisect--helper: `bisect_state` & `bisect_head` shell function in C
- bisect--helper: `bisect_autostart` shell function in C
- bisect--helper: retire `--next-all` subcommand
- bisect--helper: retire `--bisect-clean-state` subcommand
- bisect--helper: `bisect_next` and `bisect_auto_next` shell function in C
- t6030: no cleanup with bad merge base
- bisect--helper: `bisect_start` shell function partially in C
- bisect--helper: `get_terms` & `bisect_terms` shell function in C
- bisect--helper: `bisect_next_check` & bisect_voc shell function in C
- bisect--helper: `check_and_set_terms` shell function in C
- bisect--helper: `bisect_write` shell function in C
- bisect--helper: `is_expected_rev` & `check_expected_revs` shell function in C
- bisect--helper: `bisect_reset` shell function in C
- wrapper: move is_empty_file() and rename it as is_empty_or_missing_file()
- t6030: explicitly test for bisection cleanup
- bisect--helper: `bisect_clean_state` shell function in C
- bisect--helper: `write_terms` shell function in C
- bisect: rewrite `check_term_format` shell function in C
- bisect--helper: use OPT_CMDMODE instead of OPT_BOOL
Move more parts of "git bisect" to C.
Waiting for review.
* st/verify-tag (2016-10-10) 7 commits
- t/t7004-tag: Add --format specifier tests
- t/t7030-verify-tag: Add --format specifier tests
- builtin/tag: add --format argument for tag -v
- builtin/verify-tag: add --format to verify-tag
- tag: add format specifier to gpg_verify_tag
- ref-filter: add function to print single ref_array_item
- gpg-interface, tag: add GPG_VERIFY_QUIET flag
"git tag" and "git verify-tag" learned to put GPG verification
status in their "--format=<placeholders>" output format.
Waiting for a reroll.
cf. <20161007210721.20437-1-santiago@nyu.edu>
* sb/attr (2016-11-11) 35 commits
- completion: clone can initialize specific submodules
- clone: add --init-submodule=<pathspec> switch
- submodule update: add `--init-default-path` switch
- pathspec: allow escaped query values
- pathspec: allow querying for attributes
- pathspec: move prefix check out of the inner loop
- pathspec: move long magic parsing out of prefix_pathspec
- Documentation: fix a typo
- attr: keep attr stack for each check
- attr: convert to new threadsafe API
- attr: make git_check_attr_counted static
- attr.c: outline the future plans by heavily commenting
- attr.c: always pass check[] to collect_some_attrs()
- attr.c: introduce empty_attr_check_elems()
- attr.c: correct ugly hack for git_all_attrs()
- attr.c: rename a local variable check
- attr.c: pass struct git_attr_check down the callchain
- attr.c: add push_stack() helper
- attr: support quoting pathname patterns in C style
- attr: expose validity check for attribute names
- attr: add counted string version of git_check_attr()
- attr: retire git_check_attrs() API
- attr: convert git_check_attrs() callers to use the new API
- attr: convert git_all_attrs() to use "struct git_attr_check"
- attr: (re)introduce git_check_attr() and struct git_attr_check
- attr: rename function and struct related to checking attributes
- attr.c: plug small leak in parse_attr_line()
- attr.c: tighten constness around "git_attr" structure
- attr.c: simplify macroexpand_one()
- attr.c: mark where #if DEBUG ends more clearly
- attr.c: complete a sentence in a comment
- attr.c: explain the lack of attr-name syntax check in parse_attr()
- attr.c: update a stale comment on "struct match_attr"
- attr.c: use strchrnul() to scan for one line
- commit.c: use strchrnul() to scan for one line
The attributes API has been updated so that it can later be
optimized using the knowledge of which attributes are queried.
Building on top of the updated API, the pathspec machinery learned
to select only paths with given attributes set.
Waiting for review.
* va/i18n-perl-scripts (2016-11-11) 16 commits
- i18n: difftool: mark warnings for translation
- i18n: send-email: mark composing message for translation
- i18n: send-email: mark string with interpolation for translation
- i18n: send-email: mark warnings and errors for translation
- i18n: send-email: mark strings for translation
- i18n: add--interactive: mark status words for translation
- i18n: add--interactive: remove %patch_modes entries
- i18n: add--interactive: mark edit_hunk_manually message for translation
- i18n: add--interactive: i18n of help_patch_cmd
- i18n: add--interactive: mark patch prompt for translation
- i18n: add--interactive: mark plural strings
- i18n: clean.c: match string with git-add--interactive.perl
- i18n: add--interactive: mark strings with interpolation for translation
- i18n: add--interactive: mark simple here-documents for translation
- i18n: add--interactive: mark strings for translation
- Git.pm: add subroutines for commenting lines
Porcelain scripts written in Perl are getting internationalized.
Waiting for review.
* jc/latin-1 (2016-09-26) 2 commits
(merged to 'next' on 2016-09-28 at c8673e03c2)
+ utf8: accept "latin-1" as ISO-8859-1
+ utf8: refactor code to decide fallback encoding
Some platforms no longer understand "latin-1" that is still seen in
the wild in e-mail headers; replace them with "iso-8859-1" that is
more widely known when conversion fails from/to it.
Will hold to see if people scream.
* sg/fix-versioncmp-with-common-suffix (2016-09-08) 5 commits
- versioncmp: cope with common leading parts in versionsort.prereleaseSuffix
- versioncmp: pass full tagnames to swap_prereleases()
- t7004-tag: add version sort tests to show prerelease reordering issues
- t7004-tag: use test_config helper
- t7004-tag: delete unnecessary tags with test_when_finished
The prereleaseSuffix feature of version comparison that is used in
"git tag -l" did not correctly when two or more prereleases for the
same release were present (e.g. when 2.0, 2.0-beta1, and 2.0-beta2
are there and the code needs to compare 2.0-beta1 and 2.0-beta2).
Waiting for a reroll.
cf. <20160908223727.Horde.jVOOJ278ssZ3qkyjkmyqZD-@webmail.informatik.kit.edu>
* jc/pull-rebase-ff (2016-07-28) 1 commit
- pull: fast-forward "pull --rebase=true"
"git pull --rebase", when there is no new commits on our side since
we forked from the upstream, should be able to fast-forward without
invoking "git rebase", but it didn't.
Needs a real log message and a few tests.
* jc/merge-drop-old-syntax (2015-04-29) 1 commit
(merged to 'next' on 2016-10-11 at 8928c8b9b3)
+ merge: drop 'git merge <message> HEAD <commit>' syntax
Stop supporting "git merge <message> HEAD <commit>" syntax that has
been deprecated since October 2007, and issues a deprecation
warning message since v2.5.0.
It has been reported that git-gui still uses the deprecated syntax,
which needs to be fixed before this final step can proceed.
cf. <5671DB28.8020901@kdbg.org>
Will cook in 'next'.
^ permalink raw reply
* Re: [PATCH 3/3] submodule--helper: add intern-git-dir function
From: Brandon Williams @ 2016-11-21 21:56 UTC (permalink / raw)
To: Stefan Beller; +Cc: jrnieder, git, gitster, Jens.Lehmann, hvoigt
In-Reply-To: <20161121204146.13665-4-sbeller@google.com>
On 11/21, Stefan Beller wrote:
> diff --git a/t/t7412-submodule-interngitdirs.sh b/t/t7412-submodule-interngitdirs.sh
> new file mode 100755
> index 0000000000..8938a4c8b7
> --- /dev/null
> +++ b/t/t7412-submodule-interngitdirs.sh
> @@ -0,0 +1,41 @@
> +#!/bin/sh
> +
> +test_description='Test submodule interngitdirs
> +
> +This test verifies that `git submodue interngitdirs` moves a submodules git
> +directory into the superproject.
> +'
> +
> +. ./test-lib.sh
> +
> +test_expect_success 'setup a real submodule' '
> + git init sub1 &&
> + test_commit -C sub1 first &&
> + git submodule add ./sub1 &&
> + test_tick &&
> + git commit -m superproject
> +'
> +
> +test_expect_success 'intern the git dir' '
> + git submodule interngitdirs &&
> + test -f sub1/.git &&
> + test -d .git/modules/sub1 &&
> + # check that we did not break the repository:
> + git status
> +'
> +
> +test_expect_success 'setup a gitlink with missing .gitmodules entry' '
> + git init sub2 &&
> + test_commit -C sub2 first &&
> + git add sub2 &&
> + git commit -m superproject
> +'
> +
> +test_expect_success 'intern the git dir fails for incomplete submodules' '
> + test_must_fail git submodule interngitdirs &&
> + # check that we did not break the repository:
> + git status
> +'
> +
> +test_done
> +
Could we add a test which has nested submodules that need to be
migrated? Hopfully its just as easy as adding the test :)
--
Brandon Williams
^ permalink raw reply
* Re: [PATCH v15 15/27] bisect--helper: `bisect_next` and `bisect_auto_next` shell function in C
From: Stephan Beyer @ 2016-11-21 21:35 UTC (permalink / raw)
To: Pranit Bauva, git
In-Reply-To: <01020157c38b1af0-5d688c2e-868d-4d8c-a8fd-9a675f7f01da-000000@eu-west-1.amazonses.com>
Hi Pranit,
in this mail I review the "second part" of your patch: the transition of
bisect_next and bisect_auto_next to C.
On 10/14/2016 04:14 PM, Pranit Bauva wrote:
> diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
> index 1d3e17f..fcd7574 100644
> --- a/builtin/bisect--helper.c
> +++ b/builtin/bisect--helper.c
> @@ -408,6 +411,136 @@ static int bisect_terms(struct bisect_terms *terms, const char **argv, int argc)
> return 0;
> }
>
> +static int register_good_ref(const char *refname,
> + const struct object_id *oid, int flags,
> + void *cb_data)
> +{
> + struct string_list *good_refs = cb_data;
> + string_list_append(good_refs, oid_to_hex(oid));
> + return 0;
> +}
> +
> +static int bisect_next(struct bisect_terms *terms, const char *prefix)
> +{
> + int res, no_checkout;
> +
> + /*
> + * In case of mistaken revs or checkout error, or signals received,
> + * "bisect_auto_next" below may exit or misbehave.
> + * We have to trap this to be able to clean up using
> + * "bisect_clean_state".
> + */
The comment above makes no sense here, or does it?
> + if (bisect_next_check(terms, terms->term_good))
> + return -1;
> +
> + no_checkout = !is_empty_or_missing_file(git_path_bisect_head());
> +
> + /* Perform all bisection computation, display and checkout */
> + res = bisect_next_all(prefix , no_checkout);
Style: there is a space left of the comma.
> +
> + if (res == 10) {
> + FILE *fp = NULL;
> + unsigned char sha1[20];
> + struct commit *commit;
> + struct pretty_print_context pp = {0};
> + struct strbuf commit_name = STRBUF_INIT;
> + char *bad_ref = xstrfmt("refs/bisect/%s",
> + terms->term_bad);
> + int retval = 0;
> +
> + read_ref(bad_ref, sha1);
> + commit = lookup_commit_reference(sha1);
> + format_commit_message(commit, "%s", &commit_name, &pp);
> + fp = fopen(git_path_bisect_log(), "a");
> + if (!fp) {
> + retval = -1;
> + goto finish_10;
> + }
> + if (fprintf(fp, "# first %s commit: [%s] %s\n",
> + terms->term_bad, sha1_to_hex(sha1),
> + commit_name.buf) < 1){
> + retval = -1;
> + goto finish_10;
> + }
> + goto finish_10;
> + finish_10:
> + if (fp)
> + fclose(fp);
> + strbuf_release(&commit_name);
> + free(bad_ref);
> + return retval;
> + }
> + else if (res == 2) {
> + FILE *fp = NULL;
> + struct rev_info revs;
> + struct argv_array rev_argv = ARGV_ARRAY_INIT;
> + struct string_list good_revs = STRING_LIST_INIT_DUP;
> + struct pretty_print_context pp = {0};
> + struct commit *commit;
> + char *term_good = xstrfmt("%s-*", terms->term_good);
> + int i, retval = 0;
> +
> + fp = fopen(git_path_bisect_log(), "a");
> + if (!fp) {
> + retval = -1;
> + goto finish_2;
> + }
> + if (fprintf(fp, "# only skipped commits left to test\n") < 1) {
> + retval = -1;
> + goto finish_2;
> + }
> + for_each_glob_ref_in(register_good_ref, term_good,
> + "refs/bisect/", (void *) &good_revs);
> +
> + argv_array_pushl(&rev_argv, "skipped_commits", "refs/bisect/bad", "--not", NULL);
> + for (i = 0; i < good_revs.nr; i++)
> + argv_array_push(&rev_argv, good_revs.items[i].string);
> +
> + /* It is important to reset the flags used by revision walks
> + * as the previous call to bisect_next_all() in turn
> + * setups a revision walk.
> + */
> + reset_revision_walk();
> + init_revisions(&revs, NULL);
> + rev_argv.argc = setup_revisions(rev_argv.argc, rev_argv.argv, &revs, NULL);
> + argv_array_clear(&rev_argv);
> + string_list_clear(&good_revs, 0);
> + if (prepare_revision_walk(&revs))
> + die(_("revision walk setup failed\n"));
> +
> + while ((commit = get_revision(&revs)) != NULL) {
> + struct strbuf commit_name = STRBUF_INIT;
> + format_commit_message(commit, "%s",
> + &commit_name, &pp);
> + fprintf(fp, "# possible first %s commit: "
> + "[%s] %s\n", terms->term_bad,
> + oid_to_hex(&commit->object.oid),
> + commit_name.buf);
> + strbuf_release(&commit_name);
> + }
> + goto finish_2;
> + finish_2:
> + if (fp)
> + fclose(fp);
> + string_list_clear(&good_revs, 0);
> + argv_array_clear(&rev_argv);
> + free(term_good);
> + if (retval)
> + return retval;
> + else
> + return res;
> + }
> + return res;
> +}
It would be much nicer if you put the (res == 10) branch and the
(res == 2) branch into separate functions and just call them.
Then you also won't need ugly label naming like finish_10 or finish_2.
I'd also (again) recommend to use goto fail instead of setting retval to
-1 separately each time.
I'd also recommend to use a separate function to append to the bisect
log file. There is a lot of duplicated opening, checking, closing code;
IIRC such a function would also already be handy for some of the
previous patches.
> +
> +static int bisect_auto_next(struct bisect_terms *terms, const char *prefix)
> +{
> + if (!bisect_next_check(terms, NULL))
> + return bisect_next(terms, prefix);
> +
> + return 0;
> +}
Hmm, the handling of the return values is a little confusing. However,
if I understand the sh source correctly, it always returns success, no
matter if bisect_next failed or not. I do not know if you had something
special in mind here.
> int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
> @@ -643,6 +794,10 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
> N_("print out the bisect terms"), BISECT_TERMS),
> OPT_CMDMODE(0, "bisect-start", &cmdmode,
> N_("start the bisect session"), BISECT_START),
> + OPT_CMDMODE(0, "bisect-next", &cmdmode,
> + N_("find the next bisection commit"), BISECT_NEXT),
> + OPT_CMDMODE(0, "bisect-auto-next", &cmdmode,
> + N_("verify the next bisection state then find the next bisection state"), BISECT_AUTO_NEXT),
The next bisection *state* is found?
> diff --git a/git-bisect.sh b/git-bisect.sh
> index f0896b3..d574c44 100755
> --- a/git-bisect.sh
> +++ b/git-bisect.sh
> @@ -139,45 +119,7 @@ bisect_state() {
> *)
> usage ;;
> esac
> - bisect_auto_next
[...deleted lines...]
> + git bisect--helper --bisect-auto-next || exit
Why is the "|| exit" necessary?
> @@ -319,14 +260,15 @@ case "$#" in
> help)
> git bisect -h ;;
> start)
> - bisect_start "$@" ;;
> + git bisect--helper --bisect-start "$@" ;;
> bad|good|new|old|"$TERM_BAD"|"$TERM_GOOD")
> bisect_state "$cmd" "$@" ;;
> skip)
> bisect_skip "$@" ;;
> next)
> # Not sure we want "next" at the UI level anymore.
> - bisect_next "$@" ;;
> + get_terms
> + git bisect--helper --bisect-next "$@" || exit ;;
Why is the "|| exit" necessary? ;)
Furthermore:
Where is the bisect_autostart call from bisect_next() sh source gone?
Was it not necessary?
~Stephan
^ permalink raw reply
* Re: [PATCH] doc: mention user-configured trailers
From: Junio C Hamano @ 2016-11-21 21:22 UTC (permalink / raw)
To: Jonathan Tan; +Cc: git, jacob.e.keller
In-Reply-To: <1479761241-26284-1-git-send-email-jonathantanmy@google.com>
Jonathan Tan <jonathantanmy@google.com> writes:
> In commit 1462450 ("trailer: allow non-trailers in trailer block",
> 2016-10-21), functionality was added (and tested [1]) to allow
> non-trailer lines in trailer blocks, as long as those blocks contain at
> least one Git-generated or user-configured trailer, and consists of at
> least 25% trailers. The documentation was updated to mention this new
> functionality, but did not mention "user-configured trailer".
>
> Further update the documentation to also mention "user-configured
> trailer".
>
> [1] "with non-trailer lines mixed with a configured trailer" in
> t/t7513-interpret-trailers.sh
>
> Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
> ---
>
> Yes, mentioning a trailer in a Git config will cause interpret-trailers
> to treat it similarly to a Git-generated trailer (in that its presence
> causes a block partially consisting of trailers to be considered a
> trailer block). See the commit message above for a test case that
> verifies that.
>
> I took a look at the documentation, and it wasn't completely documented,
> so here is a patch to correct that.
Looks sensible. Thanks.
> Documentation/git-interpret-trailers.txt | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/git-interpret-trailers.txt b/Documentation/git-interpret-trailers.txt
> index e99bda6..09074c7 100644
> --- a/Documentation/git-interpret-trailers.txt
> +++ b/Documentation/git-interpret-trailers.txt
> @@ -49,7 +49,8 @@ will be added before the new trailer.
>
> Existing trailers are extracted from the input message by looking for
> a group of one or more lines that (i) are all trailers, or (ii) contains at
> -least one Git-generated trailer and consists of at least 25% trailers.
> +least one Git-generated or user-configured trailer and consists of at
> +least 25% trailers.
> The group must be preceded by one or more empty (or whitespace-only) lines.
> The group must either be at the end of the message or be the last
> non-whitespace lines before a line that starts with '---'. Such three
^ permalink raw reply
* Re: [PATCH 3/3] submodule--helper: add intern-git-dir function
From: Junio C Hamano @ 2016-11-21 21:14 UTC (permalink / raw)
To: Stefan Beller; +Cc: bmwill, jrnieder, git, Jens.Lehmann, hvoigt
In-Reply-To: <20161121204146.13665-4-sbeller@google.com>
Stefan Beller <sbeller@google.com> writes:
> +interngitdirs::
> + Move the git directory of submodules into its superprojects
> + `$GIT_DIR/modules` path and then connect the git directory and
> + its working directory by setting the `core.worktree` and adding
> + a .git file pointing to the git directory interned into the
> + superproject.
> ++
> + A repository that was cloned independently and later added
> + as a submodule or old setups have the submodules git directory
> + inside the submodule instead of the
> ++
> + This command is recursive by default.
Does this format correctly?
I somehow thought that second and subsequent paragraphs continued
with "+" want no indentation before them. See for example the
Values section in config.txt and see how entries for boolean:: and
color:: use multiple '+' paragraphs.
If we do not have to refrain from indenting the second and
subsequent paragraphs, that would be great for readability, but I
take the existing practice as telling me that we cannot do that X-<.
> +test_expect_success 'setup a gitlink with missing .gitmodules entry' '
> + git init sub2 &&
> + test_commit -C sub2 first &&
> + git add sub2 &&
> + git commit -m superproject
> +'
> +
> +test_expect_success 'intern the git dir fails for incomplete submodules' '
> + test_must_fail git submodule interngitdirs &&
> + # check that we did not break the repository:
> + git status
> +'
It is not clear what the last "git status" wants to test. In the
extreme, if the failed "git submodule" command did
rm -fr .git ?* && git init
wouldn't "git status" still succeed?
What are the minimum things that we expect from "did not break" to
see? sub2/.git is still a directory and is a valid repository? The
contents of the .git/modules/* before and after the "git submodule"
does not change? Some other things?
^ permalink raw reply
* Re: [PATCH 2/3] test-lib-functions.sh: teach test_commit -C <dir>
From: Junio C Hamano @ 2016-11-21 21:04 UTC (permalink / raw)
To: Stefan Beller; +Cc: bmwill, jrnieder, git, Jens.Lehmann, hvoigt
In-Reply-To: <20161121204146.13665-3-sbeller@google.com>
Stefan Beller <sbeller@google.com> writes:
> Specifically when setting up submodule tests, it comes in handy if
> we can create commits in repositories that are not at the root of
> the tested trash dir. Add "-C <dir>" similar to gits -C parameter
> that will perform the operation in the given directory.
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
Looks useful.
> t/test-lib-functions.sh | 20 +++++++++++++++-----
> 1 file changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> index fdaeb3a96b..579e812506 100644
> --- a/t/test-lib-functions.sh
> +++ b/t/test-lib-functions.sh
> @@ -157,16 +157,21 @@ debug () {
> GIT_TEST_GDB=1 "$@"
> }
>
> -# Call test_commit with the arguments "<message> [<file> [<contents> [<tag>]]]"
> +# Call test_commit with the arguments
> +# [-C <directory>] <message> [<file> [<contents> [<tag>]]]"
> #
> # This will commit a file with the given contents and the given commit
> # message, and tag the resulting commit with the given tag name.
> #
> # <file>, <contents>, and <tag> all default to <message>.
> +#
> +# If the first argument is "-C", the second argument is used as a path for
> +# the git invocations.
>
> test_commit () {
> notick= &&
> signoff= &&
> + indir= &&
> while test $# != 0
> do
> case "$1" in
> @@ -176,21 +181,26 @@ test_commit () {
> --signoff)
> signoff="$1"
> ;;
> + -C)
> + indir="$2"
> + shift
> + ;;
> *)
> break
> ;;
> esac
> shift
> done &&
> + indir=${indir:+"$indir"/} &&
> file=${2:-"$1.t"} &&
> - echo "${3-$1}" > "$file" &&
> - git add "$file" &&
> + echo "${3-$1}" > "$indir$file" &&
> + git ${indir:+ -C "$indir"} add "$file" &&
> if test -z "$notick"
> then
> test_tick
> fi &&
> - git commit $signoff -m "$1" &&
> - git tag "${4:-$1}"
> + git ${indir:+ -C "$indir"} commit $signoff -m "$1" &&
> + git ${indir:+ -C "$indir"} tag "${4:-$1}"
> }
>
> # Call test_merge with the arguments "<message> <commit>", where <commit>
^ permalink raw reply
* Re: [PATCH 1/3] submodule: use absolute path for computing relative path connecting
From: Stefan Beller @ 2016-11-21 21:03 UTC (permalink / raw)
To: Junio C Hamano
Cc: Brandon Williams, Jonathan Nieder, git@vger.kernel.org,
Jens Lehmann, Heiko Voigt
In-Reply-To: <xmqq8tscim31.fsf@gitster.mtv.corp.google.com>
On Mon, Nov 21, 2016 at 1:01 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>> Subject: Re: [PATCH 1/3] submodule: use absolute path for computing relative path connecting
>
> connecting what? IOW, has the subject line somehow truncated?
>
>> This addresses a similar concern as in f8eaa0ba98b (submodule--helper,
>> module_clone: always operate on absolute paths, 2016-03-31)
>>
>> When computing the relative path from one to another location, we
>> need to provide both locations as absolute paths to make sure the
>> computation of the relative path is correct.
>
> Can the effect of this change demonstrated in a new test? There
> must be a scenario where the current behaviour is broken and this
> change fixes an incorrect computation of relative path, no?
>
I found the latest patch of this series broken without this patch.
I'll try to find existing broken code, though.
^ permalink raw reply
* Re: [PATCH 1/3] submodule: use absolute path for computing relative path connecting
From: Junio C Hamano @ 2016-11-21 21:01 UTC (permalink / raw)
To: Stefan Beller; +Cc: bmwill, jrnieder, git, Jens.Lehmann, hvoigt
In-Reply-To: <20161121204146.13665-2-sbeller@google.com>
Stefan Beller <sbeller@google.com> writes:
> Subject: Re: [PATCH 1/3] submodule: use absolute path for computing relative path connecting
connecting what? IOW, has the subject line somehow truncated?
> This addresses a similar concern as in f8eaa0ba98b (submodule--helper,
> module_clone: always operate on absolute paths, 2016-03-31)
>
> When computing the relative path from one to another location, we
> need to provide both locations as absolute paths to make sure the
> computation of the relative path is correct.
Can the effect of this change demonstrated in a new test? There
must be a scenario where the current behaviour is broken and this
change fixes an incorrect computation of relative path, no?
>
> While at it, change `real_work_tree` to be non const as we own
> the memory.
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
> submodule.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/submodule.c b/submodule.c
> index 6f7d883de9..66c5ce5a24 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -1227,23 +1227,25 @@ void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir)
> {
> struct strbuf file_name = STRBUF_INIT;
> struct strbuf rel_path = STRBUF_INIT;
> - const char *real_work_tree = xstrdup(real_path(work_tree));
> + char *real_git_dir = xstrdup(real_path(git_dir));
> + char *real_work_tree = xstrdup(real_path(work_tree));
>
> /* Update gitfile */
> strbuf_addf(&file_name, "%s/.git", work_tree);
> write_file(file_name.buf, "gitdir: %s",
> - relative_path(git_dir, real_work_tree, &rel_path));
> + relative_path(real_git_dir, real_work_tree, &rel_path));
>
> /* Update core.worktree setting */
> strbuf_reset(&file_name);
> - strbuf_addf(&file_name, "%s/config", git_dir);
> + strbuf_addf(&file_name, "%s/config", real_git_dir);
> git_config_set_in_file(file_name.buf, "core.worktree",
> - relative_path(real_work_tree, git_dir,
> + relative_path(real_work_tree, real_git_dir,
> &rel_path));
>
> strbuf_release(&file_name);
> strbuf_release(&rel_path);
> - free((void *)real_work_tree);
> + free(real_work_tree);
> + free(real_git_dir);
> }
>
> int parallel_submodules(void)
^ permalink raw reply
* Re: [PATCH 0/3] Introduce `submodule interngitdirs`
From: Stefan Beller @ 2016-11-21 20:56 UTC (permalink / raw)
To: Junio C Hamano
Cc: Brandon Williams, Jonathan Nieder, git@vger.kernel.org,
Jens Lehmann, Heiko Voigt
In-Reply-To: <xmqqd1hoimov.fsf@gitster.mtv.corp.google.com>
On Mon, Nov 21, 2016 at 12:48 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>> The discussion of the submodule checkout series revealed to me
>> that a command is needed to move the git directory from the
>> submodules working tree to be embedded into the superprojects git
>> directory.
>
> You used "move" here, and "migrate" in the function name in 3/3,
> both of which make sense.
>
> But "intern" sounds funny. Who is confined as a prisoner here?
> North American English uses that verb as "serve as an intern", but
> that does not apply here. The verb also is used in Lisp-ish
> languages to mean the act of turning a string into a symbol, but
> that does not apply here, either.
I was inspired by the latter as we ask Git to make the submodule
"properly embedded" into the superproject, which is what I imagined
is similar to the lisp interning.
So I guess my imagination went to far and we rather want to invoke it via
"git submodule migrategitdirs" ?
But there you would ask "where are we migrating the git dirs to?", so
it would be reasonable to expect 2 parameters (just like the mv
command).
So maybe "git submodule embedgitdirs" ?
^ permalink raw reply
* Re: [PATCH 0/3] Introduce `submodule interngitdirs`
From: Junio C Hamano @ 2016-11-21 20:48 UTC (permalink / raw)
To: Stefan Beller; +Cc: bmwill, jrnieder, git, Jens.Lehmann, hvoigt
In-Reply-To: <20161121204146.13665-1-sbeller@google.com>
Stefan Beller <sbeller@google.com> writes:
> The discussion of the submodule checkout series revealed to me
> that a command is needed to move the git directory from the
> submodules working tree to be embedded into the superprojects git
> directory.
You used "move" here, and "migrate" in the function name in 3/3,
both of which make sense.
But "intern" sounds funny. Who is confined as a prisoner here?
North American English uses that verb as "serve as an intern", but
that does not apply here. The verb also is used in Lisp-ish
languages to mean the act of turning a string into a symbol, but
that does not apply here, either.
^ permalink raw reply
* [PATCH] doc: mention user-configured trailers
From: Jonathan Tan @ 2016-11-21 20:47 UTC (permalink / raw)
To: git; +Cc: Jonathan Tan, gitster, jacob.e.keller
In-Reply-To: <CA+P7+xrQEBYQQhqJQQCpLrs+4WOJOvH1X27w5Ou=2VPT=FegGQ@mail.gmail.com>
In commit 1462450 ("trailer: allow non-trailers in trailer block",
2016-10-21), functionality was added (and tested [1]) to allow
non-trailer lines in trailer blocks, as long as those blocks contain at
least one Git-generated or user-configured trailer, and consists of at
least 25% trailers. The documentation was updated to mention this new
functionality, but did not mention "user-configured trailer".
Further update the documentation to also mention "user-configured
trailer".
[1] "with non-trailer lines mixed with a configured trailer" in
t/t7513-interpret-trailers.sh
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
Yes, mentioning a trailer in a Git config will cause interpret-trailers
to treat it similarly to a Git-generated trailer (in that its presence
causes a block partially consisting of trailers to be considered a
trailer block). See the commit message above for a test case that
verifies that.
I took a look at the documentation, and it wasn't completely documented,
so here is a patch to correct that.
Documentation/git-interpret-trailers.txt | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/Documentation/git-interpret-trailers.txt b/Documentation/git-interpret-trailers.txt
index e99bda6..09074c7 100644
--- a/Documentation/git-interpret-trailers.txt
+++ b/Documentation/git-interpret-trailers.txt
@@ -49,7 +49,8 @@ will be added before the new trailer.
Existing trailers are extracted from the input message by looking for
a group of one or more lines that (i) are all trailers, or (ii) contains at
-least one Git-generated trailer and consists of at least 25% trailers.
+least one Git-generated or user-configured trailer and consists of at
+least 25% trailers.
The group must be preceded by one or more empty (or whitespace-only) lines.
The group must either be at the end of the message or be the last
non-whitespace lines before a line that starts with '---'. Such three
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* [PATCH 3/3] submodule--helper: add intern-git-dir function
From: Stefan Beller @ 2016-11-21 20:41 UTC (permalink / raw)
To: bmwill, jrnieder; +Cc: git, gitster, Jens.Lehmann, hvoigt, Stefan Beller
In-Reply-To: <20161121204146.13665-1-sbeller@google.com>
When a submodule has its git dir inside the working dir, the submodule
support for checkout that we plan to add in a later patch will fail.
Add functionality to migrate the git directory to be embedded
into the superprojects git directory.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
Documentation/git-submodule.txt | 15 ++++++++++++-
builtin/submodule--helper.c | 33 ++++++++++++++++++++++++++++-
git-submodule.sh | 7 ++++++-
submodule.c | 43 ++++++++++++++++++++++++++++++++++++++
submodule.h | 1 +
t/t7412-submodule-interngitdirs.sh | 41 ++++++++++++++++++++++++++++++++++++
6 files changed, 137 insertions(+), 3 deletions(-)
create mode 100755 t/t7412-submodule-interngitdirs.sh
diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index d841573475..80d55350eb 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -22,7 +22,7 @@ SYNOPSIS
[commit] [--] [<path>...]
'git submodule' [--quiet] foreach [--recursive] <command>
'git submodule' [--quiet] sync [--recursive] [--] [<path>...]
-
+'git submodule' [--quiet] interngitdirs [--] [<path>...]
DESCRIPTION
-----------
@@ -245,6 +245,19 @@ sync::
If `--recursive` is specified, this command will recurse into the
registered submodules, and sync any nested submodules within.
+interngitdirs::
+ Move the git directory of submodules into its superprojects
+ `$GIT_DIR/modules` path and then connect the git directory and
+ its working directory by setting the `core.worktree` and adding
+ a .git file pointing to the git directory interned into the
+ superproject.
++
+ A repository that was cloned independently and later added
+ as a submodule or old setups have the submodules git directory
+ inside the submodule instead of the
++
+ This command is recursive by default.
+
OPTIONS
-------
-q::
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 4beeda5f9f..256f8e9439 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1076,6 +1076,36 @@ static int resolve_remote_submodule_branch(int argc, const char **argv,
return 0;
}
+static int intern_git_dir(int argc, const char **argv, const char *prefix)
+{
+ int i;
+ struct pathspec pathspec;
+ struct module_list list = MODULE_LIST_INIT;
+
+ struct option intern_gitdir_options[] = {
+ OPT_END()
+ };
+
+ const char *const git_submodule_helper_usage[] = {
+ N_("git submodule--helper intern-git-dir [<path>...]"),
+ NULL
+ };
+
+ argc = parse_options(argc, argv, prefix, intern_gitdir_options,
+ git_submodule_helper_usage, 0);
+
+ gitmodules_config();
+ git_config(submodule_config, NULL);
+
+ if (module_list_compute(argc, argv, prefix, &pathspec, &list) < 0)
+ return 1;
+
+ for (i = 0; i < list.nr; i++)
+ migrate_submodule_gitdir(list.entries[i]->name);
+
+ return 0;
+}
+
struct cmd_struct {
const char *cmd;
int (*fn)(int, const char **, const char *);
@@ -1090,7 +1120,8 @@ static struct cmd_struct commands[] = {
{"resolve-relative-url", resolve_relative_url},
{"resolve-relative-url-test", resolve_relative_url_test},
{"init", module_init},
- {"remote-branch", resolve_remote_submodule_branch}
+ {"remote-branch", resolve_remote_submodule_branch},
+ {"intern-git-dir", intern_git_dir}
};
int cmd_submodule__helper(int argc, const char **argv, const char *prefix)
diff --git a/git-submodule.sh b/git-submodule.sh
index a024a135d6..747e934df2 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -1131,6 +1131,11 @@ cmd_sync()
done
}
+cmd_interngitdirs()
+{
+ git submodule--helper intern-git-dir "$@"
+}
+
# This loop parses the command line arguments to find the
# subcommand name to dispatch. Parsing of the subcommand specific
# options are primarily done by the subcommand implementations.
@@ -1140,7 +1145,7 @@ cmd_sync()
while test $# != 0 && test -z "$command"
do
case "$1" in
- add | foreach | init | deinit | update | status | summary | sync)
+ add | foreach | init | deinit | update | status | summary | sync | interngitdirs)
command=$1
;;
-q|--quiet)
diff --git a/submodule.c b/submodule.c
index 66c5ce5a24..99befdba85 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1263,3 +1263,46 @@ void prepare_submodule_repo_env(struct argv_array *out)
}
argv_array_push(out, "GIT_DIR=.git");
}
+
+/*
+ * Migrate the given submodule (and all its submodules recursively) from
+ * having its git directory within the working tree to the git dir nested
+ * in its superprojects git dir under modules/.
+ */
+void migrate_submodule_gitdir(const char *path)
+{
+ char *old_git_dir;
+ const char *new_git_dir;
+ const struct submodule *sub;
+ struct child_process cp = CHILD_PROCESS_INIT;
+
+ cp.git_cmd = 1;
+ cp.no_stdin = 1;
+ cp.dir = path;
+ argv_array_pushl(&cp.args, "submodule", "foreach", "--recursive",
+ "git", "submodule--helper" "intern-git-dir", NULL);
+
+ if (run_command(&cp))
+ die(_("Could not migrate git directory in submodule '%s'"),
+ path);
+
+ old_git_dir = xstrfmt("%s/.git", path);
+ if (read_gitfile(old_git_dir))
+ /* If it is an actual gitfile, it doesn't need migration. */
+ goto out;
+
+ sub = submodule_from_path(null_sha1, path);
+ if (!sub)
+ die(_("Could not lookup name for submodule '%s'"),
+ path);
+ new_git_dir = git_common_path("modules/%s", sub->name);
+ mkdir_in_gitdir(".git/modules");
+
+ if (rename(old_git_dir, new_git_dir) < 0)
+ die_errno(_("Could not migrate git directory from '%s' to '%s'"),
+ old_git_dir, new_git_dir);
+
+ connect_work_tree_and_git_dir(path, new_git_dir);
+out:
+ free(old_git_dir);
+}
diff --git a/submodule.h b/submodule.h
index d9e197a948..859026ecfa 100644
--- a/submodule.h
+++ b/submodule.h
@@ -75,4 +75,5 @@ int parallel_submodules(void);
*/
void prepare_submodule_repo_env(struct argv_array *out);
+extern void migrate_submodule_gitdir(const char *path);
#endif
diff --git a/t/t7412-submodule-interngitdirs.sh b/t/t7412-submodule-interngitdirs.sh
new file mode 100755
index 0000000000..8938a4c8b7
--- /dev/null
+++ b/t/t7412-submodule-interngitdirs.sh
@@ -0,0 +1,41 @@
+#!/bin/sh
+
+test_description='Test submodule interngitdirs
+
+This test verifies that `git submodue interngitdirs` moves a submodules git
+directory into the superproject.
+'
+
+. ./test-lib.sh
+
+test_expect_success 'setup a real submodule' '
+ git init sub1 &&
+ test_commit -C sub1 first &&
+ git submodule add ./sub1 &&
+ test_tick &&
+ git commit -m superproject
+'
+
+test_expect_success 'intern the git dir' '
+ git submodule interngitdirs &&
+ test -f sub1/.git &&
+ test -d .git/modules/sub1 &&
+ # check that we did not break the repository:
+ git status
+'
+
+test_expect_success 'setup a gitlink with missing .gitmodules entry' '
+ git init sub2 &&
+ test_commit -C sub2 first &&
+ git add sub2 &&
+ git commit -m superproject
+'
+
+test_expect_success 'intern the git dir fails for incomplete submodules' '
+ test_must_fail git submodule interngitdirs &&
+ # check that we did not break the repository:
+ git status
+'
+
+test_done
+
--
2.11.0.rc2.18.g0126045.dirty
^ permalink raw reply related
* [PATCH 0/3] Introduce `submodule interngitdirs`
From: Stefan Beller @ 2016-11-21 20:41 UTC (permalink / raw)
To: bmwill, jrnieder; +Cc: git, gitster, Jens.Lehmann, hvoigt, Stefan Beller
The discussion of the submodule checkout series revealed to me that a command
is needed to move the git directory from the submodules working tree to be
embedded into the superprojects git directory.
So I wrote the code to intern the submodules git dir into the superproject,
but whilst writing the code I realized this could be valueable for our use
in testing too. So I exposed it via the submodule--helper. But as the
submodule helper ought to be just an internal API, we could also
offer it via the proper submodule command.
The command as it is has little value to the end user for now, but
breaking it out of the submodule checkout series hopefully makes review easier.
Thanks,
Stefan
Stefan Beller (3):
submodule: use absolute path for computing relative path connecting
test-lib-functions.sh: teach test_commit -C <dir>
submodule--helper: add intern-git-dir function
Documentation/git-submodule.txt | 15 ++++++++++-
builtin/submodule--helper.c | 33 ++++++++++++++++++++++-
git-submodule.sh | 7 ++++-
submodule.c | 55 ++++++++++++++++++++++++++++++++++----
submodule.h | 1 +
t/t7412-submodule-interngitdirs.sh | 41 ++++++++++++++++++++++++++++
t/test-lib-functions.sh | 20 ++++++++++----
7 files changed, 159 insertions(+), 13 deletions(-)
create mode 100755 t/t7412-submodule-interngitdirs.sh
--
2.11.0.rc2.18.g0126045.dirty
^ permalink raw reply
* [PATCH 2/3] test-lib-functions.sh: teach test_commit -C <dir>
From: Stefan Beller @ 2016-11-21 20:41 UTC (permalink / raw)
To: bmwill, jrnieder; +Cc: git, gitster, Jens.Lehmann, hvoigt, Stefan Beller
In-Reply-To: <20161121204146.13665-1-sbeller@google.com>
Specifically when setting up submodule tests, it comes in handy if
we can create commits in repositories that are not at the root of
the tested trash dir. Add "-C <dir>" similar to gits -C parameter
that will perform the operation in the given directory.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
t/test-lib-functions.sh | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index fdaeb3a96b..579e812506 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -157,16 +157,21 @@ debug () {
GIT_TEST_GDB=1 "$@"
}
-# Call test_commit with the arguments "<message> [<file> [<contents> [<tag>]]]"
+# Call test_commit with the arguments
+# [-C <directory>] <message> [<file> [<contents> [<tag>]]]"
#
# This will commit a file with the given contents and the given commit
# message, and tag the resulting commit with the given tag name.
#
# <file>, <contents>, and <tag> all default to <message>.
+#
+# If the first argument is "-C", the second argument is used as a path for
+# the git invocations.
test_commit () {
notick= &&
signoff= &&
+ indir= &&
while test $# != 0
do
case "$1" in
@@ -176,21 +181,26 @@ test_commit () {
--signoff)
signoff="$1"
;;
+ -C)
+ indir="$2"
+ shift
+ ;;
*)
break
;;
esac
shift
done &&
+ indir=${indir:+"$indir"/} &&
file=${2:-"$1.t"} &&
- echo "${3-$1}" > "$file" &&
- git add "$file" &&
+ echo "${3-$1}" > "$indir$file" &&
+ git ${indir:+ -C "$indir"} add "$file" &&
if test -z "$notick"
then
test_tick
fi &&
- git commit $signoff -m "$1" &&
- git tag "${4:-$1}"
+ git ${indir:+ -C "$indir"} commit $signoff -m "$1" &&
+ git ${indir:+ -C "$indir"} tag "${4:-$1}"
}
# Call test_merge with the arguments "<message> <commit>", where <commit>
--
2.11.0.rc2.18.g0126045.dirty
^ permalink raw reply related
* [PATCH 1/3] submodule: use absolute path for computing relative path connecting
From: Stefan Beller @ 2016-11-21 20:41 UTC (permalink / raw)
To: bmwill, jrnieder; +Cc: git, gitster, Jens.Lehmann, hvoigt, Stefan Beller
In-Reply-To: <20161121204146.13665-1-sbeller@google.com>
This addresses a similar concern as in f8eaa0ba98b (submodule--helper,
module_clone: always operate on absolute paths, 2016-03-31)
When computing the relative path from one to another location, we
need to provide both locations as absolute paths to make sure the
computation of the relative path is correct.
While at it, change `real_work_tree` to be non const as we own
the memory.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
submodule.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/submodule.c b/submodule.c
index 6f7d883de9..66c5ce5a24 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1227,23 +1227,25 @@ void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir)
{
struct strbuf file_name = STRBUF_INIT;
struct strbuf rel_path = STRBUF_INIT;
- const char *real_work_tree = xstrdup(real_path(work_tree));
+ char *real_git_dir = xstrdup(real_path(git_dir));
+ char *real_work_tree = xstrdup(real_path(work_tree));
/* Update gitfile */
strbuf_addf(&file_name, "%s/.git", work_tree);
write_file(file_name.buf, "gitdir: %s",
- relative_path(git_dir, real_work_tree, &rel_path));
+ relative_path(real_git_dir, real_work_tree, &rel_path));
/* Update core.worktree setting */
strbuf_reset(&file_name);
- strbuf_addf(&file_name, "%s/config", git_dir);
+ strbuf_addf(&file_name, "%s/config", real_git_dir);
git_config_set_in_file(file_name.buf, "core.worktree",
- relative_path(real_work_tree, git_dir,
+ relative_path(real_work_tree, real_git_dir,
&rel_path));
strbuf_release(&file_name);
strbuf_release(&rel_path);
- free((void *)real_work_tree);
+ free(real_work_tree);
+ free(real_git_dir);
}
int parallel_submodules(void)
--
2.11.0.rc2.18.g0126045.dirty
^ permalink raw reply related
* Re: [PATCH 3/3] rebase -i: handle core.commentChar=auto
From: Junio C Hamano @ 2016-11-21 20:29 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Johannes Sixt
In-Reply-To: <20161121190514.18574-3-gitster@pobox.com>
Junio C Hamano <gitster@pobox.com> writes:
> @@ -93,8 +93,17 @@ eval '
> GIT_CHERRY_PICK_HELP="$resolvemsg"
> export GIT_CHERRY_PICK_HELP
>
> -comment_char=$(git config --get core.commentchar 2>/dev/null | cut -c1)
> -: ${comment_char:=#}
> +comment_char=$(git config --get core.commentchar 2>/dev/null)
> +case "$comment_char" in
> +'' | auto)
> + comment_char="#"
> + ;;
> +?)
> + ;;
> +*)
> + comment_char=$(echo "$comment_char" | cut -c1)
> + ;;
> +esac
Amended in is a fix for a typo the other Johannes noticed.
Thanks.
> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
> index d941f0a69f..5d0a7dca9d 100755
> --- a/t/t3404-rebase-interactive.sh
> +++ b/t/t3404-rebase-interactive.sh
> @@ -983,7 +983,7 @@ test_expect_success 'rebase -i respects core.commentchar' '
> test B = $(git cat-file commit HEAD^ | sed -ne \$p)
> '
>
> -test_expect_failure 'rebase -i respects core.commentchar=auto' '
> +test_expect_success 'rebase -i respects core.commentchar=auto' '
> test_config core.commentchar auto &&
> write_script copy-edit-script.sh <<-\EOF &&
> cp "$1" edit-script
^ permalink raw reply
* Re: [PATCH 2/3] stripspace: respect repository config
From: Junio C Hamano @ 2016-11-21 20:28 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin
In-Reply-To: <20161121190514.18574-2-gitster@pobox.com>
Junio C Hamano <gitster@pobox.com> writes:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> The way "git stripspace" reads the configuration was not quite
> correct, in that it forgot to probe for a possibly existing
> repository (note: stripspace is designed to be usable outside the
> repository as well) before doing so. Due to this, .git/config was
> read only when the command was run from the top-level of the working
> tree.
>
> A recent change b9605bc4f2 ("config: only read .git/config from
> configured repos", 2016-09-12) stopped reading the repository-local
> configuration file ".git/config" unless the repository discovery
> process is done, and ".git/config" is no longer read even when run
> from the top-level, which exposed the bug even more.
The above two paragraphs are rewritten from the original to explain
how this seemed to work (by accident) and its breakage surfaced in
"rebase -i" after b9605bc4f2 ("config: only read .git/config from
configured repos", 2016-09-12) better. The use of stripspace in
"rebase-i" was done after cd_to_toplevel and it happened to work
before that commit.
Otherwise there is no change from the original.
> When rebasing interactively with a commentChar defined in the
> current repository's config, the help text at the bottom of the edit
> script potentially used an incorrect comment character. This was not
> only funny-looking, but also resulted in tons of warnings like this
> one:
>
> Warning: the command isn't recognized in the following line
> - #
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> builtin/stripspace.c | 4 +++-
> t/t0030-stripspace.sh | 2 +-
> 2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/builtin/stripspace.c b/builtin/stripspace.c
> index 15e716ef43..1e62a008cb 100644
> --- a/builtin/stripspace.c
> +++ b/builtin/stripspace.c
> @@ -44,8 +44,10 @@ int cmd_stripspace(int argc, const char **argv, const char *prefix)
> if (argc)
> usage_with_options(stripspace_usage, options);
>
> - if (mode == STRIP_COMMENTS || mode == COMMENT_LINES)
> + if (mode == STRIP_COMMENTS || mode == COMMENT_LINES) {
> + setup_git_directory_gently(NULL);
> git_config(git_default_config, NULL);
> + }
>
> if (strbuf_read(&buf, 0, 1024) < 0)
> die_errno("could not read the input");
> diff --git a/t/t0030-stripspace.sh b/t/t0030-stripspace.sh
> index c1f6411eb2..bbf3e39e3d 100755
> --- a/t/t0030-stripspace.sh
> +++ b/t/t0030-stripspace.sh
> @@ -432,7 +432,7 @@ test_expect_success '-c with changed comment char' '
> test_cmp expect actual
> '
>
> -test_expect_failure '-c with comment char defined in .git/config' '
> +test_expect_success '-c with comment char defined in .git/config' '
> test_config core.commentchar = &&
> printf "= foo\n" >expect &&
> printf "foo" | (
^ permalink raw reply
* Re: [PATCH 1/3] rebase -i: highlight problems with core.commentchar
From: Junio C Hamano @ 2016-11-21 20:25 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin
In-Reply-To: <20161121190514.18574-1-gitster@pobox.com>
Junio C Hamano <gitster@pobox.com> writes:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> The interactive rebase does not currently play well with
> core.commentchar. Let's add some tests to highlight those problems
> that will be fixed in the remainder of the series.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
Sorry, I should have commented here after --- line what changes were
proposed by this set of amends.
> t/t0030-stripspace.sh | 9 +++++++++
> t/t3404-rebase-interactive.sh | 11 +++++++++++
> 2 files changed, 20 insertions(+)
>
> diff --git a/t/t0030-stripspace.sh b/t/t0030-stripspace.sh
> index 29e91d861c..c1f6411eb2 100755
> --- a/t/t0030-stripspace.sh
> +++ b/t/t0030-stripspace.sh
> @@ -432,6 +432,15 @@ test_expect_success '-c with changed comment char' '
> test_cmp expect actual
> '
>
> +test_expect_failure '-c with comment char defined in .git/config' '
> + test_config core.commentchar = &&
> + printf "= foo\n" >expect &&
> + printf "foo" | (
> + mkdir sub && cd sub && git stripspace -c
> + ) >actual &&
> + test_cmp expect actual
> +'
I noticed that the lack of "\n" at the end was merely copied from the
previous existing test (i.e. "-c with with changed comment char"),
which was already wrong the same way, so I kept that part as-is.
Running "stripspace" in a subdirectory is to avoid the ".git/config
was read without repository discovery as long as the command runs at
the top-level of the working tree" accident so that this highlights
the breakage with or without Peff's b9605bc4f2 ("config: only read
.git/config from configured repos", 2016-09-12).
> +
> test_expect_success 'avoid SP-HT sequence in commented line' '
> printf "#\tone\n#\n# two\n" >expect &&
> printf "\tone\n\ntwo\n" | git stripspace -c >actual &&
> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
> index d6d65a3a94..d941f0a69f 100755
> --- a/t/t3404-rebase-interactive.sh
> +++ b/t/t3404-rebase-interactive.sh
> @@ -983,6 +983,17 @@ test_expect_success 'rebase -i respects core.commentchar' '
> test B = $(git cat-file commit HEAD^ | sed -ne \$p)
> '
>
> +test_expect_failure 'rebase -i respects core.commentchar=auto' '
> + test_config core.commentchar auto &&
> + write_script copy-edit-script.sh <<-\EOF &&
> + cp "$1" edit-script
> + EOF
> + test_set_editor "$(pwd)/copy-edit-script.sh" &&
> + test_when_finished "git rebase --abort || :" &&
> + git rebase -i HEAD^ &&
> + test -z "$(grep -ve "^#" -e "^\$" -e "^pick" edit-script)"
> +'
> +
Removed from here is a leftover debugging bit (grep "^#"
edit-script).
^ 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