* [PATCH v2] fixing corner-cases with interpret_branch_name()
From: Jeff King @ 2017-03-02 8:21 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jacob Keller
This is a re-roll of the series from:
http://public-inbox.org/git/20170228120633.zkwfqms57fk7dkl5@sigill.intra.peff.net/
Thanks Junio and Jake for reviewing the original. This is mostly the
same, but:
- it fixes the case where "branch -r @{-1}" mistakes a local branch
for a remote (and adds a test)
- as a result of the above fix, the series needs to be applied on top
of jk/auto-namelen-in-interpret-branch-name.
- I clarified the history in the commit message of patch 4
- the commit message for patch 4 now explicitly mentions which
callers can be left alone (so anybody blaming the history won't
think they were simply forgotten).
With the exception of patch 6 flipping the failure/success bit on the
new test, the rest of the patches should be identical.
[1/8]: interpret_branch_name: move docstring to header file
[2/8]: strbuf_branchname: drop return value
[3/8]: strbuf_branchname: add docstring
[4/8]: interpret_branch_name: allow callers to restrict expansions
[5/8]: t3204: test git-branch @-expansion corner cases
[6/8]: branch: restrict @-expansions when deleting
[7/8]: strbuf_check_ref_format(): expand only local branches
[8/8]: checkout: restrict @-expansions when finding branch
builtin/branch.c | 5 +-
builtin/checkout.c | 2 +-
builtin/merge.c | 2 +-
cache.h | 32 +++++++-
refs.c | 2 +-
revision.c | 2 +-
sha1_name.c | 92 ++++++++++++-----------
strbuf.h | 21 +++++-
t/t3204-branch-name-interpretation.sh | 133 ++++++++++++++++++++++++++++++++++
9 files changed, 240 insertions(+), 51 deletions(-)
create mode 100755 t/t3204-branch-name-interpretation.sh
-Peff
^ permalink raw reply
* Re: [PATCH v5 24/24] t1406: new tests for submodule ref store
From: Michael Haggerty @ 2017-03-02 8:16 UTC (permalink / raw)
To: Duy Nguyen
Cc: Git Mailing List, Junio C Hamano, Johannes Schindelin,
Ramsay Jones, Stefan Beller, David Turner
In-Reply-To: <CACsJy8DyjWC5WUE7=dWK-mi_piGZJf7L1B0t5RUROvqDS+aP5g@mail.gmail.com>
On 03/02/2017 07:13 AM, Duy Nguyen wrote:
> On Wed, Mar 1, 2017 at 10:11 PM, Michael Haggerty <mhagger@alum.mit.edu> wrote:
>> By trial and error, I found that the test succeeds if I comment out the
>> "for_each_reflog()" test. By having that test write its results to
>> `/tmp` where they won't be deleted, I found that the problem is that the
>> `actual` results are not sorted correctly:
>>
>> refs/heads/new-master 0x0
>> refs/heads/master 0x0
>> HEAD 0x1
>>
>> I don't know why it's so Heisenbergish.
>
> It happens consistently on my other laptop. And yes it looks like
> sorting order problem, probably because of the underlying file system.
> I did wonder about that at some point but never asked. We do not
> guarantee any sorting order in the for-each api, do we?
The for-each-ref iteration is defined to be sorted by refname, but it's
true that the for-each-reflog order is undefined. So it was wrong for me
to say that the results are not sorted correctly. The problem is that
the actual output is ordered differently than the "expected" results,
which the test incorrectly considers to be an error.
Michael
^ permalink raw reply
* Re: [PATCH] README: create HTTP/HTTPS links from URLs in Markdown
From: Eric Wong @ 2017-03-02 7:34 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20170302071810.pbczh3p4w4bnyqzg@sigill.intra.peff.net>
Jeff King <peff@peff.net> wrote:
> On Wed, Mar 01, 2017 at 10:22:04PM +0000, Eric Wong wrote:
>
> > Markdown supports automatic links by surrounding URLs with
> > angle brackets, as documented in
> > <https://daringfireball.net/projects/markdown/syntax#autolink>
>
> One of the joys of markdown is that there are so many variants. A lot of
> them (including GitHub-flavored markdown) will linkify URLs even when
> they're not inside angle brackets.
>
> So I don't mind this patch, but I'm curious what's rendering the
> markdown you're seeing. I'd think online that one would either come
> across the raw text, or the GFM from https://github.com/git/git.
I was using Gruber's reference implementation from Debian stable
(1.0.1-7).
Also, pandoc (also Debian stable 1.12.4.2~dfsg-1+b14) seems to
require angle brackets by default. pandoc also supports the
"-f markdown_github" option where it seems to behave like GFM.
But yeah, I much prefer the unambiguity of angle brackets,
especially when URLs are followed immediately but punctuation
such as ',' or '.'.
^ permalink raw reply
* Re: [PATCH] README: create HTTP/HTTPS links from URLs in Markdown
From: Jeff King @ 2017-03-02 7:37 UTC (permalink / raw)
To: Eric Wong; +Cc: Junio C Hamano, git
In-Reply-To: <20170302073421.GA2528@whir>
On Thu, Mar 02, 2017 at 07:34:21AM +0000, Eric Wong wrote:
> Jeff King <peff@peff.net> wrote:
> > On Wed, Mar 01, 2017 at 10:22:04PM +0000, Eric Wong wrote:
> >
> > > Markdown supports automatic links by surrounding URLs with
> > > angle brackets, as documented in
> > > <https://daringfireball.net/projects/markdown/syntax#autolink>
> >
> > One of the joys of markdown is that there are so many variants. A lot of
> > them (including GitHub-flavored markdown) will linkify URLs even when
> > they're not inside angle brackets.
> >
> > So I don't mind this patch, but I'm curious what's rendering the
> > markdown you're seeing. I'd think online that one would either come
> > across the raw text, or the GFM from https://github.com/git/git.
>
> I was using Gruber's reference implementation from Debian stable
> (1.0.1-7).
OK. I guess my question more was "why are you doing that?". I'd expect
people to find the GFM rendering on GitHub, or just look at the text via
"less".
But it's not really my business why you would want to do it. :) It's
reasonable for us to cater to the common subset of renderers.
-Peff
^ permalink raw reply
* Re: [PATCH] Put sha1dc on a diet
From: Linus Torvalds @ 2017-03-01 19:10 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Marc Stevens, Dan Shumow, Git Mailing List
In-Reply-To: <20170301190711.nwlretmy6w675jvp@sigill.intra.peff.net>
On Wed, Mar 1, 2017 at 11:07 AM, Jeff King <peff@peff.net> wrote:
>
> So obviously the smaller object size is nice, and the diffstat is
> certainly satisfying. My only qualm would be whether this conflicts with
> the optimizations that Dan is working on (probably not conceptually, but
> textually).
Yeah. But I'll happily just re-apply the patch on any new version that
Dan posts. The patch is obviously trivial, even if size-wise it's a
fair number of lines.
So I wouldn't suggest using the patched version as some kind of
starting point. It's much easier to just take a new version of
upstream and repeat the diet patch on it.
.. and obviously later versions of the upstream sha1dc code may not
even need this at all since Dan and Marc are now aware of the issue.
Linus
^ permalink raw reply
* Re: [PATCH v5 24/24] t1406: new tests for submodule ref store
From: Duy Nguyen @ 2017-03-02 6:13 UTC (permalink / raw)
To: Michael Haggerty
Cc: Git Mailing List, Junio C Hamano, Johannes Schindelin,
Ramsay Jones, Stefan Beller, David Turner
In-Reply-To: <8f4ec174-9060-0896-5135-bb50229fe149@alum.mit.edu>
On Wed, Mar 1, 2017 at 10:11 PM, Michael Haggerty <mhagger@alum.mit.edu> wrote:
> By trial and error, I found that the test succeeds if I comment out the
> "for_each_reflog()" test. By having that test write its results to
> `/tmp` where they won't be deleted, I found that the problem is that the
> `actual` results are not sorted correctly:
>
> refs/heads/new-master 0x0
> refs/heads/master 0x0
> HEAD 0x1
>
> I don't know why it's so Heisenbergish.
It happens consistently on my other laptop. And yes it looks like
sorting order problem, probably because of the underlying file system.
I did wonder about that at some point but never asked. We do not
guarantee any sorting order in the for-each api, do we?
--
Duy
^ permalink raw reply
* Re: [PATCH] Put sha1dc on a diet
From: Duy Nguyen @ 2017-03-02 6:10 UTC (permalink / raw)
To: Jeff King
Cc: Linus Torvalds, Johannes Schindelin, Junio C Hamano, Marc Stevens,
Dan Shumow, Git Mailing List
In-Reply-To: <20170301231921.2puf7o7jkrujscwn@sigill.intra.peff.net>
On Thu, Mar 2, 2017 at 6:19 AM, Jeff King <peff@peff.net> wrote:
> You have to remember that some of the Git for Windows users are doing
> horrific things like using repositories with 450MB .git/index files, and
> the speed to compute the sha1 during an update is noticeable there.
We probably should separate this use case from the object hashing
anyway. Here we need a better, more reliable crc32 basically, to
detect bit flips. Even if we move to SHA-something, we can keep
staying with SHA-1 here (and with the fastest implementation)
--
Duy
^ permalink raw reply
* Re: [PATCH] mingw: use OpenSSL's SHA-1 routines
From: Johannes Sixt @ 2017-03-02 6:07 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, Jeff King, git, Jeff Hostetler
In-Reply-To: <xmqqmvdbz23d.fsf@gitster.mtv.corp.google.com>
Am 24.02.2017 um 22:54 schrieb Junio C Hamano:
> Johannes Sixt <j6t@kdbg.org> writes:
>> I'll use the patch for daily work for a while to see whether it hurts.
>
> Please ping this thread again when you have something to add. For
> now, I'll demote this patch from 'next' to 'pu' when we rewind and
> rebuild 'next' post 2.12 release.
As I already said in [1], I have no objection. I've used the patch in
production for some time now and did not notice any slowdowns.
[1]
https://public-inbox.org/git/0080edd6-a515-2fe9-6266-b6f6bbedfdde@kdbg.org/
-- Hannes
^ permalink raw reply
* Re: [PATCH 01/10] submodule: decouple url and submodule existence
From: Junio C Hamano @ 2017-03-02 5:43 UTC (permalink / raw)
To: Brandon Williams; +Cc: git, sbeller
In-Reply-To: <20170301200230.GA30622@google.com>
Brandon Williams <bmwill@google.com> writes:
> On 02/24, Junio C Hamano wrote:
>
>> Also as a grouping measure, submodule.active that lists submodule
>> paths feels hard to use. When switching between two branches in the
>> superproject that have the same submodule bound at two different
>> paths, who is responsible for updating submodule.active in
>> superproject's config? If it were a list of submodule names, this
>> objection does not apply, though.
>
> I agree that if you are listing every submodule path by hand then this
> may not be the best approach and would be difficult to use. The idea is
> that this would allow a user to set a general pathspec to identify a
> group of modules they are interested in. Perhaps once attributes can be
> used in pathspecs a user could group submodules by setting a particular
> attribute and then submodule.active would have a value like
> ":(attr:foo)" to indicate I'm interested in all submodules with the
> "foo" attribute.
OK. As .gitattributes is tracked just like .gitmodules in tree, the
project can adjust the path pattern that matches a renamed submodule
and gives it an attribute in .gitattributes in the same commit in
the superproject that moves the directory to which the submodule is
bound, just like it updates .gitmodules to adjust the name<->path
mapping. So that plan solves my specific worry about using "path"
for grouping and not "name".
Thanks.
^ permalink raw reply
* Re: [PATCH] Travis: also test on 32-bit Linux
From: Junio C Hamano @ 2017-03-02 5:06 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Git Mailing List, Lars Schneider
In-Reply-To: <c76a133a57514a332828099d342c9763fd946bfa.1488309430.git.johannes.schindelin@gmx.de>
On Tue, Feb 28, 2017 at 11:17 AM, Johannes Schindelin
<johannes.schindelin@gmx.de> wrote:
> .... This patch
> asks Travis CI to install a Docker image with 32-bit libraries and then
> goes on to build and test Git using this 32-bit setup.
>
> A big thank you to Lars Schneider without whose help this patch would
> not have happened.
This has been in 'pu' for a few days, and
https://travis-ci.org/git/git/builds shows that we have
a new build job running successfully.
Good job ;-)
Thanks.
^ permalink raw reply
* Re: [PATCH] Put sha1dc on a diet
From: Junio C Hamano @ 2017-03-02 4:38 UTC (permalink / raw)
To: Dan Shumow; +Cc: Linus Torvalds, Jeff King, Git Mailing List, Marc Stevens
In-Reply-To: <CY1PR0301MB21073D82F4A6AB0DAD8BF1FCC4280@CY1PR0301MB2107.namprd03.prod.outlook.com>
Dan Shumow <danshu@microsoft.com> writes:
> At this point, I would suggest that I take the C optimizations,
> clean them up and fold them in with the diet changes Linus has
> suggested. The slowdown is still 2x over block-sha1 and more over
> OpenSSL. But it is better than nothing. And then if there is
> interest Marc and I can investigate other processor specific
> optimizations like ASM or SIMD and circle back with those
> performance optimizations at a later date.
>
> Also, to Johannes Schindelin's point:
>> My concern is about that unexpected turn "oh, let's just switch
>> to C99 because, well, because my compiler canehandle it, and
>> everybody else should just switch tn a modern compiler". That
>> really sounded careless.
>
> While it will probably be a pain, if it is a requirement, we can
> modify the code to move away from any c99 specific stuff we have
> in here, if it makes adopting the code more palatable for Git.
I was assuming that we would treat your code just like how we treat
any other "borrowed code from elsewhere". The usual way for us to
do so is to take code that was released by the "upstream" (under a
license that allows us to use it---yours is MIT, which does) in the
style and language of upstream's choice, and then we in the Git
development community takes responsiblity for massaging the code to
match our style, for trimming what we won't use and for doing any
other customization to fit our needs.
As you and Marc seemed to be still working on speeding up, such a
customization work to fully adjust your code to our codebase was
premature, so I tentatively queued what we saw on the list as-is on
our 'pu' branch so that people can have a reference point. Which
unfortunately solicited a premature reaction by Johannes. Please do
not worry too much about the comment.
But if you are willing to help us by getting involved in the
"customization" part, too, that would be a very welcome news to us.
In that case, "welcome to the Git development community" ;-)
So,... from my point of view, we are OK either way. It is OK if you
are a third-party upstream that is not particularly interested in
Git project's specific requirement. We surely would be happier if
you and Marc, the upstream authors of the code in question, also act
as participants in the Git development community.
Either way, thanks for your great help.
^ permalink raw reply
* [PATCH 01/18] lib-submodule-update.sh: reorder create_lib_submodule_repo
From: Stefan Beller @ 2017-03-02 0:47 UTC (permalink / raw)
To: sbeller; +Cc: git, bmwill, novalis, sandals, hvoigt, gitster, jrnieder, ramsay
In-Reply-To: <20170302004759.27852-1-sbeller@google.com>
Redraw the ASCII art describing the setup using more space, such that
it is easier to understand. The leaf commits are now ordered the same
way the actual code is ordered.
Add empty lines to the setup code separating each of the leaf commits,
each starting with a "checkout -b".
Signed-off-by: Stefan Beller <sbeller@google.com>
---
t/lib-submodule-update.sh | 49 ++++++++++++++++++++++++++++-------------------
1 file changed, 29 insertions(+), 20 deletions(-)
diff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh
index 915eb4a7c6..5df528ea81 100755
--- a/t/lib-submodule-update.sh
+++ b/t/lib-submodule-update.sh
@@ -15,22 +15,27 @@
# - Tracked file replaced by submodule (replace_sub1_with_file =>
# replace_file_with_sub1)
#
-# --O-----O
-# / ^ replace_directory_with_sub1
-# / replace_sub1_with_directory
-# /----O
-# / ^
-# / modify_sub1
-# O------O-------O
-# ^ ^\ ^
-# | | \ remove_sub1
-# | | -----O-----O
-# | | \ ^ replace_file_with_sub1
-# | | \ replace_sub1_with_file
-# | add_sub1 --O-----O
-# no_submodule ^ valid_sub1
-# invalid_sub1
+# ----O
+# / ^
+# / remove_sub1
+# /
+# add_sub1 /-------O
+# | / ^
+# | / modify_sub1
+# v/
+# O------O-----------O---------O
+# ^ \ ^ replace_directory_with_sub1
+# | \ replace_sub1_with_directory
+# no_submodule \
+# --------O---------O
+# \ ^ replace_file_with_sub1
+# \ replace_sub1_with_file
+# \
+# ----O---------O
+# ^ valid_sub1
+# invalid_sub1
#
+
create_lib_submodule_repo () {
git init submodule_update_repo &&
(
@@ -49,10 +54,11 @@ create_lib_submodule_repo () {
git config submodule.sub1.ignore all &&
git add .gitmodules &&
git commit -m "Add sub1" &&
- git checkout -b remove_sub1 &&
+
+ git checkout -b remove_sub1 add_sub1 &&
git revert HEAD &&
- git checkout -b "modify_sub1" "add_sub1" &&
+ git checkout -b modify_sub1 add_sub1 &&
git submodule update &&
(
cd sub1 &&
@@ -67,7 +73,7 @@ create_lib_submodule_repo () {
git add sub1 &&
git commit -m "Modify sub1" &&
- git checkout -b "replace_sub1_with_directory" "add_sub1" &&
+ git checkout -b replace_sub1_with_directory add_sub1 &&
git submodule update &&
git -C sub1 checkout modifications &&
git rm --cached sub1 &&
@@ -75,22 +81,25 @@ create_lib_submodule_repo () {
git config -f .gitmodules --remove-section "submodule.sub1" &&
git add .gitmodules sub1/* &&
git commit -m "Replace sub1 with directory" &&
+
git checkout -b replace_directory_with_sub1 &&
git revert HEAD &&
- git checkout -b "replace_sub1_with_file" "add_sub1" &&
+ git checkout -b replace_sub1_with_file add_sub1 &&
git rm sub1 &&
echo "content" >sub1 &&
git add sub1 &&
git commit -m "Replace sub1 with file" &&
+
git checkout -b replace_file_with_sub1 &&
git revert HEAD &&
- git checkout -b "invalid_sub1" "add_sub1" &&
+ git checkout -b invalid_sub1 add_sub1 &&
git update-index --cacheinfo 160000 0123456789012345678901234567890123456789 sub1 &&
git commit -m "Invalid sub1 commit" &&
git checkout -b valid_sub1 &&
git revert HEAD &&
+
git checkout master
)
}
--
2.12.0.rc1.52.ge239d7e709.dirty
^ permalink raw reply related
* [PATCH 14/18] unpack-trees: check if we can perform the operation for submodules
From: Stefan Beller @ 2017-03-02 0:47 UTC (permalink / raw)
To: sbeller; +Cc: git, bmwill, novalis, sandals, hvoigt, gitster, jrnieder, ramsay
In-Reply-To: <20170302004759.27852-1-sbeller@google.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
unpack-trees.c | 131 +++++++++++++++++++++++++++++++++++++++++++++++++++++----
unpack-trees.h | 1 +
2 files changed, 123 insertions(+), 9 deletions(-)
diff --git a/unpack-trees.c b/unpack-trees.c
index 616a0ae4b2..8333da2cc9 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -10,6 +10,8 @@
#include "attr.h"
#include "split-index.h"
#include "dir.h"
+#include "submodule.h"
+#include "submodule-config.h"
/*
* Error messages expected by scripts out of plumbing commands such as
@@ -45,6 +47,9 @@ static const char *unpack_plumbing_errors[NB_UNPACK_TREES_ERROR_TYPES] = {
/* ERROR_WOULD_LOSE_ORPHANED_REMOVED */
"Working tree file '%s' would be removed by sparse checkout update.",
+
+ /* ERROR_WOULD_LOSE_SUBMODULE */
+ "Submodule '%s' cannot checkout new HEAD.",
};
#define ERRORMSG(o,type) \
@@ -161,6 +166,8 @@ void setup_unpack_trees_porcelain(struct unpack_trees_options *opts,
_("The following working tree files would be overwritten by sparse checkout update:\n%s");
msgs[ERROR_WOULD_LOSE_ORPHANED_REMOVED] =
_("The following working tree files would be removed by sparse checkout update:\n%s");
+ msgs[ERROR_WOULD_LOSE_SUBMODULE] =
+ _("Submodule '%s' cannot checkout new HEAD");
opts->show_all_errors = 1;
/* rejected paths may not have a static buffer */
@@ -240,12 +247,75 @@ static void display_error_msgs(struct unpack_trees_options *o)
fprintf(stderr, _("Aborting\n"));
}
+static int check_submodule_move_head(const struct cache_entry *ce,
+ const char *old_id,
+ const char *new_id,
+ struct unpack_trees_options *o)
+{
+ const struct submodule *sub = submodule_from_ce(ce);
+ if (!sub)
+ return 0;
+
+ switch (sub->update_strategy.type) {
+ case SM_UPDATE_UNSPECIFIED:
+ case SM_UPDATE_CHECKOUT:
+ if (submodule_move_head(ce->name, old_id, new_id, SUBMODULE_MOVE_HEAD_DRY_RUN))
+ return o->gently ? -1 :
+ add_rejected_path(o, ERROR_WOULD_LOSE_SUBMODULE, ce->name);
+ return 0;
+ case SM_UPDATE_NONE:
+ return 0;
+ case SM_UPDATE_REBASE:
+ case SM_UPDATE_MERGE:
+ case SM_UPDATE_COMMAND:
+ default:
+ warning(_("submodule update strategy not supported for submodule '%s'"), ce->name);
+ return -1;
+ }
+}
+
+static void reload_gitmodules_file(struct index_state *index,
+ struct checkout *state)
+{
+ int i;
+ for (i = 0; i < index->cache_nr; i++) {
+ struct cache_entry *ce = index->cache[i];
+ if (ce->ce_flags & CE_UPDATE) {
+ int r = strcmp(ce->name, ".gitmodules");
+ if (r < 0)
+ continue;
+ else if (r == 0) {
+ submodule_free();
+ checkout_entry(ce, state, NULL);
+ gitmodules_config();
+ git_config(submodule_config, NULL);
+ } else
+ break;
+ }
+ }
+}
+
/*
* Unlink the last component and schedule the leading directories for
* removal, such that empty directories get removed.
*/
static void unlink_entry(const struct cache_entry *ce)
{
+ const struct submodule *sub = submodule_from_ce(ce);
+ if (sub) {
+ switch (sub->update_strategy.type) {
+ case SM_UPDATE_UNSPECIFIED:
+ case SM_UPDATE_CHECKOUT:
+ case SM_UPDATE_REBASE:
+ case SM_UPDATE_MERGE:
+ submodule_move_head(ce->name, "HEAD", NULL,
+ SUBMODULE_MOVE_HEAD_FORCE);
+ break;
+ case SM_UPDATE_NONE:
+ case SM_UPDATE_COMMAND:
+ return; /* Do not touch the submodule. */
+ }
+ }
if (!check_leading_path(ce->name, ce_namelen(ce)))
return;
if (remove_or_warn(ce->ce_mode, ce->name))
@@ -301,6 +371,9 @@ static int check_updates(struct unpack_trees_options *o)
remove_marked_cache_entries(index);
remove_scheduled_dirs();
+ if (should_update_submodules() && o->update && !o->dry_run)
+ reload_gitmodules_file(index, &state);
+
for (i = 0; i < index->cache_nr; i++) {
struct cache_entry *ce = index->cache[i];
@@ -1358,17 +1431,26 @@ static int verify_uptodate_1(const struct cache_entry *ce,
if (!lstat(ce->name, &st)) {
int flags = CE_MATCH_IGNORE_VALID|CE_MATCH_IGNORE_SKIP_WORKTREE;
unsigned changed = ie_match_stat(o->src_index, ce, &st, flags);
+
+ if (submodule_from_ce(ce)) {
+ int r = check_submodule_move_head(ce,
+ "HEAD", oid_to_hex(&ce->oid), o);
+ if (r)
+ return o->gently ? -1 :
+ add_rejected_path(o, error_type, ce->name);
+ return 0;
+ }
+
if (!changed)
return 0;
/*
- * NEEDSWORK: the current default policy is to allow
- * submodule to be out of sync wrt the superproject
- * index. This needs to be tightened later for
- * submodules that are marked to be automatically
- * checked out.
+ * Historic default policy was to allow submodule to be out
+ * of sync wrt the superproject index. If the submodule was
+ * not considered interesting above, we don't care here.
*/
if (S_ISGITLINK(ce->ce_mode))
return 0;
+
errno = 0;
}
if (errno == ENOENT)
@@ -1412,7 +1494,11 @@ static int verify_clean_submodule(const char *old_sha1,
enum unpack_trees_error_types error_type,
struct unpack_trees_options *o)
{
- return 0;
+ if (!submodule_from_ce(ce))
+ return 0;
+
+ return check_submodule_move_head(ce, old_sha1,
+ oid_to_hex(&ce->oid), o);
}
static int verify_clean_subdirectory(const struct cache_entry *ce,
@@ -1578,9 +1664,15 @@ static int verify_absent_1(const struct cache_entry *ce,
path = xmemdupz(ce->name, len);
if (lstat(path, &st))
ret = error_errno("cannot stat '%s'", path);
- else
- ret = check_ok_to_remove(path, len, DT_UNKNOWN, NULL,
- &st, error_type, o);
+ else {
+ if (submodule_from_ce(ce))
+ ret = check_submodule_move_head(ce,
+ oid_to_hex(&ce->oid),
+ NULL, o);
+ else
+ ret = check_ok_to_remove(path, len, DT_UNKNOWN, NULL,
+ &st, error_type, o);
+ }
free(path);
return ret;
} else if (lstat(ce->name, &st)) {
@@ -1588,6 +1680,10 @@ static int verify_absent_1(const struct cache_entry *ce,
return error_errno("cannot stat '%s'", ce->name);
return 0;
} else {
+ if (submodule_from_ce(ce))
+ return check_submodule_move_head(ce, oid_to_hex(&ce->oid),
+ NULL, o);
+
return check_ok_to_remove(ce->name, ce_namelen(ce),
ce_to_dtype(ce), ce, &st,
error_type, o);
@@ -1643,6 +1739,15 @@ static int merged_entry(const struct cache_entry *ce,
return -1;
}
invalidate_ce_path(merge, o);
+
+ if (submodule_from_ce(ce)) {
+ int ret = check_submodule_move_head(ce, NULL,
+ oid_to_hex(&ce->oid),
+ o);
+ if (ret)
+ return ret;
+ }
+
} else if (!(old->ce_flags & CE_CONFLICTED)) {
/*
* See if we can re-use the old CE directly?
@@ -1663,6 +1768,14 @@ static int merged_entry(const struct cache_entry *ce,
update |= old->ce_flags & (CE_SKIP_WORKTREE | CE_NEW_SKIP_WORKTREE);
invalidate_ce_path(old, o);
}
+
+ if (submodule_from_ce(ce)) {
+ int ret = check_submodule_move_head(ce, oid_to_hex(&old->oid),
+ oid_to_hex(&ce->oid),
+ o);
+ if (ret)
+ return ret;
+ }
} else {
/*
* Previously unmerged entry left as an existence
diff --git a/unpack-trees.h b/unpack-trees.h
index 36a73a6d00..6c48117b84 100644
--- a/unpack-trees.h
+++ b/unpack-trees.h
@@ -21,6 +21,7 @@ enum unpack_trees_error_types {
ERROR_SPARSE_NOT_UPTODATE_FILE,
ERROR_WOULD_LOSE_ORPHANED_OVERWRITTEN,
ERROR_WOULD_LOSE_ORPHANED_REMOVED,
+ ERROR_WOULD_LOSE_SUBMODULE,
NB_UNPACK_TREES_ERROR_TYPES
};
--
2.12.0.rc1.52.ge239d7e709.dirty
^ permalink raw reply related
* [PATCH 05/18] lib-submodule-update.sh: define tests for recursing into submodules
From: Stefan Beller @ 2017-03-02 0:47 UTC (permalink / raw)
To: sbeller; +Cc: git, bmwill, novalis, sandals, hvoigt, gitster, jrnieder, ramsay
In-Reply-To: <20170302004759.27852-1-sbeller@google.com>
Currently lib-submodule-update.sh provides 2 functions
test_submodule_switch and test_submodule_forced_switch that are used by a
variety of tests to ensure that submodules behave as expected. The current
expected behavior is that submodules are not touched at all (see
42639d2317a for the exact setup).
In the future we want to teach all these commands to recurse
into submodules. To do that, we'll add two testing functions to
submodule-update-lib.sh: test_submodule_switch_recursing and
test_submodule_forced_switch_recursing.
These two functions behave in analogy to the already existing functions
just with a different expectation on submodule behavior. The submodule
in the working tree is expected to be updated to the recorded submodule
version. The behavior is analogous to e.g. the behavior of files in a
nested directory in the working tree, where a change to the working tree
handles any arising directory/file conflicts just fine.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
t/lib-submodule-update.sh | 506 +++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 504 insertions(+), 2 deletions(-)
diff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh
index 83202c54fc..aa196d4d42 100755
--- a/t/lib-submodule-update.sh
+++ b/t/lib-submodule-update.sh
@@ -4,6 +4,7 @@
# - New submodule (no_submodule => add_sub1)
# - Removed submodule (add_sub1 => remove_sub1)
# - Updated submodule (add_sub1 => modify_sub1)
+# - Updated submodule recursively (add_nested_sub => modify_sub1_recursively)
# - Submodule updated to invalid commit (add_sub1 => invalid_sub1)
# - Submodule updated from invalid commit (invalid_sub1 => valid_sub1)
# - Submodule replaced by tracked files in directory (add_sub1 =>
@@ -19,8 +20,8 @@
# / ^
# / remove_sub1
# /
-# add_sub1 /-------O
-# | / ^
+# add_sub1 /-------O---------O--------O modify_sub1_recursively
+# | / ^ add_nested_sub
# | / modify_sub1
# v/
# O------O-----------O---------O
@@ -48,6 +49,17 @@ create_lib_submodule_repo () {
git commit -m "Base inside first submodule" &&
git branch "no_submodule"
) &&
+ git init submodule_update_sub2 &&
+ (
+ cd submodule_update_sub2
+ echo "expect" >>.gitignore &&
+ echo "actual" >>.gitignore &&
+ echo "x" >file1 &&
+ echo "y" >file2 &&
+ git add .gitignore file1 file2 &&
+ git commit -m "nested submodule base" &&
+ git branch "no_submodule"
+ ) &&
git init submodule_update_repo &&
(
cd submodule_update_repo &&
@@ -84,6 +96,26 @@ create_lib_submodule_repo () {
git add sub1 &&
git commit -m "Modify sub1" &&
+ git checkout -b add_nested_sub modify_sub1 &&
+ git -C sub1 checkout -b "add_nested_sub" &&
+ git -C sub1 submodule add --branch no_submodule ../submodule_update_sub2 sub2 &&
+ git -C sub1 commit -a -m "add a nested submodule" &&
+ git add sub1 &&
+ git commit -a -m "update submodule, that updates a nested submodule" &&
+ git checkout -b modify_sub1_recursively &&
+ git -C sub1 checkout -b modify_sub1_recursively &&
+ git -C sub1/sub2 checkout -b modify_sub1_recursively &&
+ echo change >sub1/sub2/file3 &&
+ git -C sub1/sub2 add file3 &&
+ git -C sub1/sub2 commit -m "make a change in nested sub" &&
+ git -C sub1 add sub2 &&
+ git -C sub1 commit -m "update nested sub" &&
+ git add sub1 &&
+ git commit -m "update sub1, that updates nested sub" &&
+ git -C sub1 push origin modify_sub1_recursively &&
+ git -C sub1/sub2 push origin modify_sub1_recursively &&
+ git -C sub1 submodule deinit -f --all &&
+
git checkout -b replace_sub1_with_directory add_sub1 &&
git submodule update &&
git -C sub1 checkout modifications &&
@@ -150,6 +182,15 @@ test_git_directory_is_unchanged () {
)
}
+test_git_directory_exists() {
+ test -e ".git/modules/$1" &&
+ if test -f sub1/.git
+ then
+ # does core.worktree point at the right place?
+ test "$(git -C .git/modules/$1 config core.worktree)" = "../../../$1"
+ fi
+}
+
# Helper function to be executed at the start of every test below, it sets up
# the submodule repo if it doesn't exist and configures the most problematic
# settings for diff.ignoreSubmodules.
@@ -180,6 +221,27 @@ reset_work_tree_to () {
)
}
+reset_work_tree_to_interested () {
+ reset_work_tree_to $1 &&
+ # make the submodule git dirs available
+ if ! test -d submodule_update/.git/modules/sub1
+ then
+ mkdir -p submodule_update/.git/modules &&
+ cp -r submodule_update_repo/.git/modules/sub1 submodule_update/.git/modules/sub1
+ GIT_WORK_TREE=. git -C submodule_update/.git/modules/sub1 config --unset core.worktree
+ fi &&
+ if ! test -d submodule_update/.git/modules/sub1/modules/sub2
+ then
+ mkdir -p submodule_update/.git/modules/sub1/modules &&
+ cp -r submodule_update_repo/.git/modules/sub1/modules/sub2 submodule_update/.git/modules/sub1/modules/sub2
+ GIT_WORK_TREE=. git -C submodule_update/.git/modules/sub1/modules/sub2 config --unset core.worktree
+ fi &&
+ # indicate we are interested in the submodule:
+ git -C submodule_update config submodule.sub1.url "bogus" &&
+ # sub1 might not be checked out, so use the git dir
+ git -C submodule_update/.git/modules/sub1 config submodule.sub2.url "bogus"
+}
+
# Test that the superproject contains the content according to commit "$1"
# (the work tree must match the index for everything but submodules but the
# index must exactly match the given commit including any submodule SHA-1s).
@@ -700,3 +762,443 @@ test_submodule_forced_switch () {
)
'
}
+
+# Test that submodule contents are correctly updated when switching
+# between commits that change a submodule.
+# Test that the following transitions are correctly handled:
+# (These tests are also above in the case where we expect no change
+# in the submodule)
+# - Updated submodule
+# - New submodule
+# - Removed submodule
+# - Directory containing tracked files replaced by submodule
+# - Submodule replaced by tracked files in directory
+# - Submodule replaced by tracked file with the same name
+# - tracked file replaced by submodule
+#
+# New test cases
+# - Removing a submodule with a git directory absorbs the submodules
+# git directory first into the superproject.
+
+test_submodule_switch_recursing () {
+ command="$1"
+ ######################### Appearing submodule #########################
+ # Switching to a commit letting a submodule appear checks it out ...
+ test_expect_success "$command: added submodule is checked out" '
+ prolog &&
+ reset_work_tree_to_interested no_submodule &&
+ (
+ cd submodule_update &&
+ git branch -t add_sub1 origin/add_sub1 &&
+ $command add_sub1 &&
+ test_superproject_content origin/add_sub1 &&
+ test_submodule_content sub1 origin/add_sub1
+ )
+ '
+ # ... ignoring an empty existing directory ...
+ test_expect_success "$command: added submodule is checked out in empty dir" '
+ prolog &&
+ reset_work_tree_to_interested no_submodule &&
+ (
+ cd submodule_update &&
+ mkdir sub1 &&
+ git branch -t add_sub1 origin/add_sub1 &&
+ $command add_sub1 &&
+ test_superproject_content origin/add_sub1 &&
+ test_submodule_content sub1 origin/add_sub1
+ )
+ '
+ # ... unless there is an untracked file in its place.
+ test_expect_success "$command: added submodule doesn't remove untracked file with same name" '
+ prolog &&
+ reset_work_tree_to_interested no_submodule &&
+ (
+ cd submodule_update &&
+ git branch -t add_sub1 origin/add_sub1 &&
+ : >sub1 &&
+ test_must_fail $command add_sub1 &&
+ test_superproject_content origin/no_submodule &&
+ test_must_be_empty sub1
+ )
+ '
+ # ... but an ignored file is fine.
+ test_expect_success "$command: added submodule removes an untracked ignored file" '
+ test_when_finished "rm submodule_update/.git/info/exclude" &&
+ prolog &&
+ reset_work_tree_to_interested no_submodule &&
+ (
+ cd submodule_update &&
+ git branch -t add_sub1 origin/add_sub1 &&
+ : >sub1 &&
+ echo sub1 >.git/info/exclude
+ $command add_sub1 &&
+ test_superproject_content origin/add_sub1 &&
+ test_submodule_content sub1 origin/add_sub1
+ )
+ '
+ # Replacing a tracked file with a submodule produces a checked out submodule
+ test_expect_success "$command: replace tracked file with submodule checks out submodule" '
+ prolog &&
+ reset_work_tree_to_interested replace_sub1_with_file &&
+ (
+ cd submodule_update &&
+ git branch -t replace_file_with_sub1 origin/replace_file_with_sub1 &&
+ $command replace_file_with_sub1 &&
+ test_superproject_content origin/replace_file_with_sub1 &&
+ test_submodule_content sub1 origin/replace_file_with_sub1
+ )
+ '
+ # ... as does removing a directory with tracked files with a submodule.
+ test_expect_success "$command: replace directory with submodule" '
+ prolog &&
+ reset_work_tree_to_interested replace_sub1_with_directory &&
+ (
+ cd submodule_update &&
+ git branch -t replace_directory_with_sub1 origin/replace_directory_with_sub1 &&
+ $command replace_directory_with_sub1 &&
+ test_superproject_content origin/replace_directory_with_sub1 &&
+ test_submodule_content sub1 origin/replace_directory_with_sub1
+ )
+ '
+
+ ######################## Disappearing submodule #######################
+ # Removing a submodule removes its work tree ...
+ test_expect_success "$command: removed submodule removes submodules working tree" '
+ prolog &&
+ reset_work_tree_to_interested add_sub1 &&
+ (
+ cd submodule_update &&
+ git branch -t remove_sub1 origin/remove_sub1 &&
+ $command remove_sub1 &&
+ test_superproject_content origin/remove_sub1 &&
+ ! test -e sub1
+ )
+ '
+ # ... absorbing a .git directory along the way.
+ test_expect_success "$command: removed submodule absorbs submodules .git directory" '
+ prolog &&
+ reset_work_tree_to_interested add_sub1 &&
+ (
+ cd submodule_update &&
+ git branch -t remove_sub1 origin/remove_sub1 &&
+ replace_gitfile_with_git_dir sub1 &&
+ rm -rf .git/modules &&
+ $command remove_sub1 &&
+ test_superproject_content origin/remove_sub1 &&
+ ! test -e sub1 &&
+ test_git_directory_exists sub1
+ )
+ '
+ # Replacing a submodule with files in a directory must succeeds
+ # when the submodule is clean
+ test_expect_success "$command: replace submodule with a directory" '
+ prolog &&
+ reset_work_tree_to_interested add_sub1 &&
+ (
+ cd submodule_update &&
+ git branch -t replace_sub1_with_directory origin/replace_sub1_with_directory &&
+ $command replace_sub1_with_directory &&
+ test_superproject_content origin/replace_sub1_with_directory &&
+ test_submodule_content sub1 origin/replace_sub1_with_directory
+ )
+ '
+ # ... absorbing a .git directory.
+ test_expect_success "$command: replace submodule containing a .git directory with a directory must absorb the git dir" '
+ prolog &&
+ reset_work_tree_to_interested add_sub1 &&
+ (
+ cd submodule_update &&
+ git branch -t replace_sub1_with_directory origin/replace_sub1_with_directory &&
+ replace_gitfile_with_git_dir sub1 &&
+ rm -rf .git/modules &&
+ $command replace_sub1_with_directory &&
+ test_superproject_content origin/replace_sub1_with_directory &&
+ test_git_directory_exists sub1
+ )
+ '
+
+ # Replacing it with a file ...
+ test_expect_success "$command: replace submodule with a file" '
+ prolog &&
+ reset_work_tree_to_interested add_sub1 &&
+ (
+ cd submodule_update &&
+ git branch -t replace_sub1_with_file origin/replace_sub1_with_file &&
+ $command replace_sub1_with_file &&
+ test_superproject_content origin/replace_sub1_with_file &&
+ test -f sub1
+ )
+ '
+
+ # ... must check its local work tree for untracked files
+ test_expect_success "$command: replace submodule with a file must fail with untracked files" '
+ prolog &&
+ reset_work_tree_to_interested add_sub1 &&
+ (
+ cd submodule_update &&
+ git branch -t replace_sub1_with_file origin/replace_sub1_with_file &&
+ : >sub1/untrackedfile &&
+ test_must_fail $command replace_sub1_with_file &&
+ test_superproject_content origin/add_sub1 &&
+ test_submodule_content sub1 origin/add_sub1
+ )
+ '
+
+ # ... and ignored files are ignroed
+ test_expect_success "$command: replace submodule with a file works ignores ignored files in submodule" '
+ test_when_finished "rm submodule_update/.git/modules/sub1/info/exclude" &&
+ prolog &&
+ reset_work_tree_to_interested add_sub1 &&
+ (
+ cd submodule_update &&
+ git branch -t replace_sub1_with_file origin/replace_sub1_with_file &&
+ : >sub1/ignored &&
+ $command replace_sub1_with_file &&
+ test_superproject_content origin/replace_sub1_with_file &&
+ test -f sub1
+ )
+ '
+
+ ########################## Modified submodule #########################
+ # Updating a submodule sha1 updates the submodule's work tree
+ test_expect_success "$command: modified submodule updates submodule work tree" '
+ prolog &&
+ reset_work_tree_to_interested add_sub1 &&
+ (
+ cd submodule_update &&
+ git branch -t modify_sub1 origin/modify_sub1 &&
+ $command modify_sub1 &&
+ test_superproject_content origin/modify_sub1 &&
+ test_submodule_content sub1 origin/modify_sub1
+ )
+ '
+
+ # Updating a submodule to an invalid sha1 doesn't update the
+ # superproject nor the submodule's work tree.
+ test_expect_success "$command: updating to a missing submodule commit fails" '
+ prolog &&
+ reset_work_tree_to_interested add_sub1 &&
+ (
+ cd submodule_update &&
+ git branch -t invalid_sub1 origin/invalid_sub1 &&
+ test_must_fail $command invalid_sub1 &&
+ test_superproject_content origin/add_sub1 &&
+ test_submodule_content sub1 origin/add_sub1
+ )
+ '
+
+ test_expect_success "$command: modified submodule updates submodule recursively" '
+ prolog &&
+ reset_work_tree_to_interested add_nested_sub &&
+ (
+ cd submodule_update &&
+ git branch -t modify_sub1_recursively origin/modify_sub1_recursively &&
+ $command modify_sub1_recursively &&
+ test_superproject_content origin/modify_sub1_recursively &&
+ test_submodule_content sub1 origin/modify_sub1_recursively &&
+ test_submodule_content -C sub1 sub2 origin/modify_sub1_recursively
+ )
+ '
+}
+
+# Test that submodule contents are updated when switching between commits
+# that change a submodule, but throwing away local changes in
+# the superproject as well as the submodule is allowed.
+test_submodule_forced_switch_recursing () {
+ command="$1"
+ ######################### Appearing submodule #########################
+ # Switching to a commit letting a submodule appear creates empty dir ...
+ test_expect_success "$command: added submodule is checked out" '
+ prolog &&
+ reset_work_tree_to_interested no_submodule &&
+ (
+ cd submodule_update &&
+ git branch -t add_sub1 origin/add_sub1 &&
+ $command add_sub1 &&
+ test_superproject_content origin/add_sub1 &&
+ test_submodule_content sub1 origin/add_sub1
+ )
+ '
+ # ... and doesn't care if it already exists ...
+ test_expect_success "$command: added submodule ignores empty directory" '
+ prolog &&
+ reset_work_tree_to_interested no_submodule &&
+ (
+ cd submodule_update &&
+ git branch -t add_sub1 origin/add_sub1 &&
+ mkdir sub1 &&
+ $command add_sub1 &&
+ test_superproject_content origin/add_sub1 &&
+ test_submodule_content sub1 origin/add_sub1
+ )
+ '
+ # ... not caring about an untracked file either
+ test_expect_success "$command: added submodule does remove untracked unignored file with same name when forced" '
+ prolog &&
+ reset_work_tree_to_interested no_submodule &&
+ (
+ cd submodule_update &&
+ git branch -t add_sub1 origin/add_sub1 &&
+ >sub1 &&
+ $command add_sub1 &&
+ test_superproject_content origin/add_sub1 &&
+ test_submodule_content sub1 origin/add_sub1
+ )
+ '
+ # Replacing a tracked file with a submodule checks out the submodule
+ test_expect_success "$command: replace tracked file with submodule populates the submodule" '
+ prolog &&
+ reset_work_tree_to_interested replace_sub1_with_file &&
+ (
+ cd submodule_update &&
+ git branch -t replace_file_with_sub1 origin/replace_file_with_sub1 &&
+ $command replace_file_with_sub1 &&
+ test_superproject_content origin/replace_file_with_sub1 &&
+ test_submodule_content sub1 origin/replace_file_with_sub1
+ )
+ '
+ # ... as does removing a directory with tracked files with a
+ # submodule.
+ test_expect_success "$command: replace directory with submodule" '
+ prolog &&
+ reset_work_tree_to_interested replace_sub1_with_directory &&
+ (
+ cd submodule_update &&
+ git branch -t replace_directory_with_sub1 origin/replace_directory_with_sub1 &&
+ $command replace_directory_with_sub1 &&
+ test_superproject_content origin/replace_directory_with_sub1 &&
+ test_submodule_content sub1 origin/replace_directory_with_sub1
+ )
+ '
+
+ ######################## Disappearing submodule #######################
+ # Removing a submodule doesn't remove its work tree ...
+ test_expect_success "$command: removed submodule leaves submodule directory and its contents in place" '
+ prolog &&
+ reset_work_tree_to_interested add_sub1 &&
+ (
+ cd submodule_update &&
+ git branch -t remove_sub1 origin/remove_sub1 &&
+ $command remove_sub1 &&
+ test_superproject_content origin/remove_sub1 &&
+ ! test -e sub1
+ )
+ '
+ # ... especially when it contains a .git directory.
+ test_expect_success "$command: removed submodule leaves submodule containing a .git directory alone" '
+ prolog &&
+ reset_work_tree_to_interested add_sub1 &&
+ (
+ cd submodule_update &&
+ git branch -t remove_sub1 origin/remove_sub1 &&
+ replace_gitfile_with_git_dir sub1 &&
+ rm -rf .git/modules/sub1 &&
+ $command remove_sub1 &&
+ test_superproject_content origin/remove_sub1 &&
+ test_git_directory_exists sub1 &&
+ ! test -e sub1
+ )
+ '
+ # Replacing a submodule with files in a directory ...
+ test_expect_success "$command: replace submodule with a directory" '
+ prolog &&
+ reset_work_tree_to_interested add_sub1 &&
+ (
+ cd submodule_update &&
+ git branch -t replace_sub1_with_directory origin/replace_sub1_with_directory &&
+ test_must_fail $command replace_sub1_with_directory &&
+ test_superproject_content origin/replace_sub1_with_directory
+ )
+ '
+ # ... absorbing a .git directory.
+ test_expect_success "$command: replace submodule containing a .git directory with a directory must fail" '
+ prolog &&
+ reset_work_tree_to_interested add_sub1 &&
+ (
+ cd submodule_update &&
+ git branch -t replace_sub1_with_directory origin/replace_sub1_with_directory &&
+ replace_gitfile_with_git_dir sub1 &&
+ rm -rf .git/modules/sub1 &&
+ $command replace_sub1_with_directory &&
+ test_superproject_content origin/replace_sub1_with_directory &&
+ test_submodule_content sub1 origin/modify_sub1
+ test_git_directory_exists sub1
+ )
+ '
+ # Replacing it with a file
+ test_expect_success "$command: replace submodule with a file" '
+ prolog &&
+ reset_work_tree_to_interested add_sub1 &&
+ (
+ cd submodule_update &&
+ git branch -t replace_sub1_with_file origin/replace_sub1_with_file &&
+ $command replace_sub1_with_file &&
+ test_superproject_content origin/replace_sub1_with_file
+ )
+ '
+
+ # ... even if the submodule contains ignored files
+ test_expect_success "$command: replace submodule with a file ignoring ignored files" '
+ prolog &&
+ reset_work_tree_to_interested add_sub1 &&
+ (
+ cd submodule_update &&
+ git branch -t replace_sub1_with_file origin/replace_sub1_with_file &&
+ : >sub1/expect &&
+ $command replace_sub1_with_file &&
+ test_superproject_content origin/replace_sub1_with_file
+ )
+ '
+
+ # ... but stops for untracked files that would be lost
+ test_expect_success "$command: replace submodule with a file" '
+ prolog &&
+ reset_work_tree_to_interested add_sub1 &&
+ (
+ cd submodule_update &&
+ git branch -t replace_sub1_with_file origin/replace_sub1_with_file &&
+ : >sub1/untracked_file &&
+ test_must_fail $command replace_sub1_with_file &&
+ test_superproject_content origin/add_sub1 &&
+ test -f sub1/untracked_file
+ )
+ '
+
+ ########################## Modified submodule #########################
+ # Updating a submodule sha1 updates the submodule's work tree
+ test_expect_success "$command: modified submodule updates submodule work tree" '
+ prolog &&
+ reset_work_tree_to_interested add_sub1 &&
+ (
+ cd submodule_update &&
+ git branch -t modify_sub1 origin/modify_sub1 &&
+ $command modify_sub1 &&
+ test_superproject_content origin/modify_sub1 &&
+ test_submodule_content sub1 origin/modify_sub1
+ )
+ '
+ # Updating a submodule to an invalid sha1 doesn't update the
+ # submodule's work tree, subsequent update will fail
+ test_expect_success "$command: modified submodule does not update submodule work tree to invalid commit" '
+ prolog &&
+ reset_work_tree_to_interested add_sub1 &&
+ (
+ cd submodule_update &&
+ git branch -t invalid_sub1 origin/invalid_sub1 &&
+ test_must_fail $command invalid_sub1 &&
+ test_superproject_content origin/add_sub1 &&
+ test_submodule_content sub1 origin/add_sub1
+ )
+ '
+ # Updating a submodule from an invalid sha1 updates
+ test_expect_success "$command: modified submodule does not update submodule work tree from invalid commit" '
+ prolog &&
+ reset_work_tree_to_interested invalid_sub1 &&
+ (
+ cd submodule_update &&
+ git branch -t valid_sub1 origin/valid_sub1 &&
+ test_must_fail $command valid_sub1 &&
+ test_superproject_content origin/invalid_sub1
+ )
+ '
+}
--
2.12.0.rc1.52.ge239d7e709.dirty
^ permalink raw reply related
* [RFCv6 PATCH 00/18] Checkout aware of Submodules!
From: Stefan Beller @ 2017-03-02 0:47 UTC (permalink / raw)
To: sbeller; +Cc: git, bmwill, novalis, sandals, hvoigt, gitster, jrnieder, ramsay
In-Reply-To: <20170223225735.10994-1-sbeller@google.com/>
previous work:
https://public-inbox.org/git/20170223225735.10994-1-sbeller@google.com/
https://public-inbox.org/git/20161203003022.29797-1-sbeller@google.com/
v6:
* added support for read-tree (see last patch) to see how generic the
code of the previous patches is. I am pretty pleased with that patch.
* marked two functions static. Thanks Ramsay!
* fixed the recursive test; it still fails but it is the code that fails,
not the test. For this I had to change the setup code slightly.
* 2 new patches adding tiny refactoring to the submodule test lib.
* interdiff (to origin/sb/checkout-recurse-submodules, which is v5) below.
v5:
* as v4 was the first version queued by Junio, we do have an interdiff below!
* renamed functions
* changed the API, now the caller has to take care of the submodule strategy
themselves. (Note this can be different for different situations, e.g.
when a submodule is deleted, we can do that for any strategy except none and
!command. But for moving to a new state of the submodule we currently
only implement "checkout" [unspecified defaults to checkout]. warning about
the others, doing nothing there.)
v4:
* addressed all comments of Brian, Junio and Brandon.
Thanks!
* one major point of change is the introduction of another patch
"lib-submodule-update.sh: do not use ./. as submodule remote",
as that took some time to track down the existing bug.
v3:
* moved tests from t2013 to the generic submodule library.
* factored out the refactoring patches to be up front
* As I redid the complete implementation, I have the impression this time
it is cleaner than previous versions.
I think we still have to fix the corner cases of directory/file/submodule
conflicts before merging, but this serves as a status update on my current
way of thinking how to implement the worktree commands being aware of
submodules.
Thanks,
Stefan
v2:
* based on top of the series sent out an hour ago
"[PATCHv4 0/5] submodule embedgitdirs"
* Try to embed a submodule if we need to remove it.
* Strictly do not change behavior if not giving the new flag.
* I think I missed some review comments from v1, but I'd like to get
the current state out over the weekend, as a lot has changed so far.
On Monday I'll go through the previous discussion with a comb to see
if I missed something.
v1:
When working with submodules, nearly anytime after checking out
a different state of the projects, that has submodules changed
you'd run "git submodule update" with a current version of Git.
There are two problems with this approach:
* The "submodule update" command is dangerous as it
doesn't check for work that may be lost in the submodule
(e.g. a dangling commit).
* you may forget to run the command as checkout is supposed
to do all the work for you.
Integrate updating the submodules into git checkout, with the same
safety promises that git-checkout has, i.e. not throw away data unless
asked to. This is done by first checking if the submodule is at the same
sha1 as it is recorded in the superproject. If there are changes we stop
proceeding the checkout just like it is when checking out a file that
has local changes.
The integration happens in the code that is also used in other commands
such that it will be easier in the future to make other commands aware
of submodule.
This also solves d/f conflicts in case you replace a file/directory
with a submodule or vice versa.
The patches are still a bit rough, but the overall series seems
promising enough to me that I want to put it out here.
Any review, specifically on the design level welcome!
Thanks,
Stefan
Stefan Beller (14):
lib-submodule-update.sh: reorder create_lib_submodule_repo
lib-submodule-update.sh: define tests for recursing into submodules
make is_submodule_populated gently
connect_work_tree_and_git_dir: safely create leading directories
update submodules: add submodule config parsing
update submodules: add a config option to determine if submodules are
updated
update submodules: introduce is_interesting_submodule
update submodules: move up prepare_submodule_repo_env
update submodules: add submodule_go_from_to
unpack-trees: pass old oid to verify_clean_submodule
unpack-trees: check if we can perform the operation for submodules
read-cache: remove_marked_cache_entries to wipe selected submodules.
entry.c: update submodules when interesting
builtin/checkout: add --recurse-submodules switch
diff --git a/Documentation/git-read-tree.txt b/Documentation/git-read-tree.txt
index fa1d557e5b..ed9d63ef4a 100644
--- a/Documentation/git-read-tree.txt
+++ b/Documentation/git-read-tree.txt
@@ -115,6 +115,12 @@ OPTIONS
directories the index file and index output file are
located in.
+--[no-]recurse-submodules::
+ Using --recurse-submodules will update the content of all initialized
+ submodules according to the commit recorded in the superproject by
+ calling read-tree recursively, also setting the submodules HEAD to be
+ detached at that commit.
+
--no-sparse-checkout::
Disable sparse checkout support even if `core.sparseCheckout`
is true.
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 207ce09771..e9c5fcfaf8 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -29,8 +29,8 @@ static const char * const checkout_usage[] = {
NULL,
};
-int option_parse_recurse_submodules(const struct option *opt,
- const char *arg, int unset)
+static int option_parse_recurse_submodules(const struct option *opt,
+ const char *arg, int unset)
{
if (unset) {
recurse_submodules = RECURSE_SUBMODULES_OFF;
diff --git a/builtin/read-tree.c b/builtin/read-tree.c
index 8ba64bc785..2dc5cc06da 100644
--- a/builtin/read-tree.c
+++ b/builtin/read-tree.c
@@ -15,10 +15,13 @@
#include "builtin.h"
#include "parse-options.h"
#include "resolve-undo.h"
+#include "submodule.h"
+#include "submodule-config.h"
static int nr_trees;
static int read_empty;
static struct tree *trees[MAX_UNPACK_TREES];
+int recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
static int list_tree(unsigned char *sha1)
{
@@ -96,6 +99,23 @@ static int debug_merge(const struct cache_entry * const *stages,
return 0;
}
+static int option_parse_recurse_submodules(const struct option *opt,
+ const char *arg, int unset)
+{
+ if (unset) {
+ recurse_submodules = RECURSE_SUBMODULES_OFF;
+ return 0;
+ }
+ if (arg)
+ recurse_submodules =
+ parse_update_recurse_submodules_arg(opt->long_name,
+ arg);
+ else
+ recurse_submodules = RECURSE_SUBMODULES_ON;
+
+ return 0;
+}
+
static struct lock_file lock_file;
int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)
@@ -137,6 +157,9 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)
N_("skip applying sparse checkout filter")),
OPT_BOOL(0, "debug-unpack", &opts.debug_unpack,
N_("debug unpack-trees")),
+ { OPTION_CALLBACK, 0, "recurse-submodules", &recurse_submodules,
+ "checkout", "control recursive updating of submodules",
+ PARSE_OPT_OPTARG, option_parse_recurse_submodules },
OPT_END()
};
@@ -152,6 +175,12 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)
hold_locked_index(&lock_file, LOCK_DIE_ON_ERROR);
+ if (recurse_submodules != RECURSE_SUBMODULES_DEFAULT) {
+ gitmodules_config();
+ git_config(submodule_config, NULL);
+ set_config_update_recurse_submodules(RECURSE_SUBMODULES_ON);
+ }
+
prefix_set = opts.prefix ? 1 : 0;
if (1 < opts.merge + opts.reset + prefix_set)
die("Which one? -m, --reset, or --prefix?");
diff --git a/submodule.c b/submodule.c
index a2cf8c9376..bc5fecf8c5 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1257,7 +1257,7 @@ static int submodule_has_dirty_index(const struct submodule *sub)
return finish_command(&cp);
}
-void submodule_reset_index(const char *path)
+static void submodule_reset_index(const char *path)
{
struct child_process cp = CHILD_PROCESS_INIT;
prepare_submodule_repo_env_no_git_dir(&cp.env_array);
diff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh
index 54cd8a6366..429cd1041d 100755
--- a/t/lib-submodule-update.sh
+++ b/t/lib-submodule-update.sh
@@ -4,7 +4,7 @@
# - New submodule (no_submodule => add_sub1)
# - Removed submodule (add_sub1 => remove_sub1)
# - Updated submodule (add_sub1 => modify_sub1)
-# - Updated submodule recursively (modify_sub1 => modify_sub1_recursively)
+# - Updated submodule recursively (add_nested_sub => modify_sub1_recursively)
# - Submodule updated to invalid commit (add_sub1 => invalid_sub1)
# - Submodule updated from invalid commit (invalid_sub1 => valid_sub1)
# - Submodule replaced by tracked files in directory (add_sub1 =>
@@ -20,8 +20,8 @@
# / ^
# / remove_sub1
# /
-# add_sub1 /-------O---------O
-# | / ^ modify_sub1_recursive
+# add_sub1 /-------O---------O--------O modify_sub1_recursively
+# | / ^ add_nested_sub
# | / modify_sub1
# v/
# O------O-----------O---------O
@@ -96,12 +96,24 @@ create_lib_submodule_repo () {
git add sub1 &&
git commit -m "Modify sub1" &&
- git checkout -b modify_sub1_recursively modify_sub1 &&
+ git checkout -b add_nested_sub modify_sub1 &&
git -C sub1 checkout -b "add_nested_sub" &&
git -C sub1 submodule add --branch no_submodule ../submodule_update_sub2 sub2 &&
git -C sub1 commit -a -m "add a nested submodule" &&
git add sub1 &&
git commit -a -m "update submodule, that updates a nested submodule" &&
+ git checkout -b modify_sub1_recursively &&
+ git -C sub1 checkout -b modify_sub1_recursively &&
+ git -C sub1/sub2 checkout -b modify_sub1_recursively &&
+ echo change >sub1/sub2/file3 &&
+ git -C sub1/sub2 add file3 &&
+ git -C sub1/sub2 commit -m "make a change in nested sub" &&
+ git -C sub1 add sub2 &&
+ git -C sub1 commit -m "update nested sub" &&
+ git add sub1 &&
+ git commit -m "update sub1, that updates nested sub" &&
+ git -C sub1 push origin modify_sub1_recursively &&
+ git -C sub1/sub2 push origin modify_sub1_recursively &&
git -C sub1 submodule deinit -f --all &&
git checkout -b replace_sub1_with_directory add_sub1 &&
@@ -200,9 +212,9 @@ reset_work_tree_to () {
git checkout -f "$1" &&
git status -u -s >actual &&
test_must_be_empty actual &&
- sha1=$(git rev-parse --revs-only HEAD:sub1) &&
- if test -n "$sha1" &&
- test $(cd "../submodule_update_sub1" && git rev-parse --verify "$sha1^{commit}")
+ hash=$(git rev-parse --revs-only HEAD:sub1) &&
+ if test -n "$hash" &&
+ test $(cd "../submodule_update_sub1" && git rev-parse --verify "$hash^{commit}")
then
git submodule update --init --recursive "sub1"
fi
@@ -211,14 +223,23 @@ reset_work_tree_to () {
reset_work_tree_to_interested () {
reset_work_tree_to $1 &&
- # indicate we are interested in the submodule:
- git -C submodule_update config submodule.sub1.url "bogus" &&
- # also have it available:
+ # make the submodule git dirs available
if ! test -d submodule_update/.git/modules/sub1
then
mkdir -p submodule_update/.git/modules &&
cp -r submodule_update_repo/.git/modules/sub1 submodule_update/.git/modules/sub1
- fi
+ GIT_WORK_TREE=. git -C submodule_update/.git/modules/sub1 config --unset core.worktree
+ fi &&
+ if ! test -d submodule_update/.git/modules/sub1/modules/sub2
+ then
+ mkdir -p submodule_update/.git/modules/sub1/modules &&
+ cp -r submodule_update_repo/.git/modules/sub1/modules/sub2 submodule_update/.git/modules/sub1/modules/sub2
+ GIT_WORK_TREE=. git -C submodule_update/.git/modules/sub1/modules/sub2 config --unset core.worktree
+ fi &&
+ # indicate we are interested in the submodule:
+ git -C submodule_update config submodule.sub1.url "bogus" &&
+ # sub1 might not be checked out, so use the git dir
+ git -C submodule_update/.git/modules/sub1 config submodule.sub2.url "bogus"
}
# Test that the superproject contains the content according to commit "$1"
@@ -234,6 +255,11 @@ test_superproject_content () {
# Test that the given submodule at path "$1" contains the content according
# to the submodule commit recorded in the superproject's commit "$2"
test_submodule_content () {
+ if test "$1" == "-C"
+ then
+ cd "$2"
+ shift; shift;
+ fi
if test $# != 2
then
echo "test_submodule_content needs two arguments"
@@ -761,6 +787,11 @@ test_submodule_switch_recursing () {
then
RESULT=failure
fi
+
+ if test "$KNOWN_FAILURE_SUBMODULE_OVERWRITE_IGNORED_UNTRACKED" = 1
+ then
+ RESULT1=failure
+ fi
######################### Appearing submodule #########################
# Switching to a commit letting a submodule appear checks it out ...
test_expect_success "$command: added submodule is checked out" '
@@ -801,7 +832,7 @@ test_submodule_switch_recursing () {
)
'
# ... but an ignored file is fine.
- test_expect_success "$command: added submodule removes an untracked ignored file" '
+ test_expect_$RESULT1 "$command: added submodule removes an untracked ignored file" '
test_when_finished "rm submodule_update/.git/info/exclude" &&
prolog &&
reset_work_tree_to_interested no_submodule &&
@@ -966,21 +997,17 @@ test_submodule_switch_recursing () {
)
'
- # This test fails, due to missing setup, we do not clone sub2 into
- # submodule_update, because it doesn't exist in the 'add_sub1' version
- #
- test_expect_success "$command: modified submodule updates submodule recursively" '
+ # recursing deeper than one level doesn't work yet.
+ test_expect_failure "$command: modified submodule updates submodule recursively" '
prolog &&
- reset_work_tree_to_interested add_sub1 &&
+ reset_work_tree_to_interested add_nested_sub &&
(
cd submodule_update &&
git branch -t modify_sub1_recursively origin/modify_sub1_recursively &&
- test_must_fail $command modify_sub1_recursively &&
- test_superproject_content origin/add_sub1 &&
- test_submodule_content sub1 origin/add_sub1
- # test_superproject_content origin/modify_sub1_recursively &&
- # test_submodule_content sub1 origin/modify_sub1_recursively &&
- # test_submodule_content sub1/sub2 no_submodule
+ $command modify_sub1_recursively &&
+ test_superproject_content origin/modify_sub1_recursively &&
+ test_submodule_content sub1 origin/modify_sub1_recursively &&
+ test_submodule_content -C sub1 sub2 origin/modify_sub1_recursively
)
'
}
diff --git a/t/t1013-read-tree-submodule.sh b/t/t1013-read-tree-submodule.sh
index 20526aed34..7019d0a04f 100755
--- a/t/t1013-read-tree-submodule.sh
+++ b/t/t1013-read-tree-submodule.sh
@@ -5,6 +5,13 @@ test_description='read-tree can handle submodules'
. ./test-lib.sh
. "$TEST_DIRECTORY"/lib-submodule-update.sh
+KNOWN_FAILURE_DIRECTORY_SUBMODULE_CONFLICTS=1
+KNOWN_FAILURE_SUBMODULE_OVERWRITE_IGNORED_UNTRACKED=1
+
+test_submodule_switch_recursing "git read-tree --recurse-submodules -u -m"
+
+test_submodule_forced_switch_recursing "git read-tree --recurse-submodules -u --reset"
+
test_submodule_switch "git read-tree -u -m"
test_submodule_forced_switch "git read-tree -u --reset"
^ permalink raw reply related
* RE: [PATCH] Put sha1dc on a diet
From: Dan Shumow @ 2017-03-02 1:31 UTC (permalink / raw)
To: Linus Torvalds, Jeff King; +Cc: Junio C Hamano, Git Mailing List, Marc Stevens
In-Reply-To: <CA+55aFz4ixVKVURki8FeXjL5H51A_cQXsZpzKJ-N9n574Yy1rg@mail.gmail.com>
I played around tweaking the code a bit more and I got our performance down to a 2.077182x slowdown with check and a 1.055961x slowdown without checking. However, that slowdown is basically with the check turned off through our API. If I rip extraneous code for storing states and checking if we are doing collision detection out, I can reach performance parity with the block-sha1 implementation in the Git codebase, which basically tells me that is about as good as I can do for optimizing the C code.
SHA1 is more amenable to assembler implementation because its use of rotations, which are notoriously difficult to access through C code. And as this happens in the inner loop of the function, the inline asm tends to not cut it. This is one of the reasons that the OpenSSL SHA-1 runs like a scalded monkey, compared to the C implemenations. Marc and I have also discussed using SIMD operations to speed up the UBC checks, which could definitely help achieve better performance, but is highly dependent on processor support. It will take some time to do either a SIMD implementation of the UBC checks or an assembler implementation.
At this point, I would suggest that I take the C optimizations, clean them up and fold them in with the diet changes Linus has suggested. The slowdown is still 2x over block-sha1 and more over OpenSSL. But it is better than nothing. And then if there is interest Marc and I can investigate other processor specific optimizations like ASM or SIMD and circle back with those performance optimizations at a later date.
Also, to Johannes Schindelin's point:
> My concern is about that unexpected turn "oh, let's just switch to C99 because, well, because my compiler canehandle it, and everybody else should just switch tn a modern compiler". That really sounded careless.
While it will probably be a pain, if it is a requirement, we can modify the code to move away from any c99 specific stuff we have in here, if it makes adopting the code more palatable for Git.
Thanks,
Dan
^ permalink raw reply
* Re: [PATCH 03/18] lib-submodule-update: teach test_submodule_content the -C <dir> flag
From: Eric Wong @ 2017-03-02 2:11 UTC (permalink / raw)
To: Stefan Beller
Cc: git, bmwill, novalis, sandals, hvoigt, gitster, jrnieder, ramsay
In-Reply-To: <20170302004759.27852-4-sbeller@google.com>
Stefan Beller <sbeller@google.com> wrote:
> test_submodule_content () {
> + if test "$1" == "-C"
Use a single '=' for portability in sh. It's also a good idea
to prefix variables with 'x' or some such, since "$1" could be
"-n" or some other supported switch for test(1).
So, something like:
if test x"$1" = "x-C"
...or use a case statement.
On Debian systems, I use the "checkbashisms" from the
"devscripts" package to find and avoid bash dependencies.
Thanks.
^ permalink raw reply
* [PATCH 03/18] lib-submodule-update: teach test_submodule_content the -C <dir> flag
From: Stefan Beller @ 2017-03-02 0:47 UTC (permalink / raw)
To: sbeller; +Cc: git, bmwill, novalis, sandals, hvoigt, gitster, jrnieder, ramsay
In-Reply-To: <20170302004759.27852-1-sbeller@google.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
t/lib-submodule-update.sh | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh
index c0d6325133..a906c92cfb 100755
--- a/t/lib-submodule-update.sh
+++ b/t/lib-submodule-update.sh
@@ -193,6 +193,11 @@ test_superproject_content () {
# Test that the given submodule at path "$1" contains the content according
# to the submodule commit recorded in the superproject's commit "$2"
test_submodule_content () {
+ if test "$1" == "-C"
+ then
+ cd "$2"
+ shift; shift;
+ fi
if test $# != 2
then
echo "test_submodule_content needs two arguments"
--
2.12.0.rc1.52.ge239d7e709.dirty
^ permalink raw reply related
* [PATCH 11/18] update submodules: move up prepare_submodule_repo_env
From: Stefan Beller @ 2017-03-02 0:47 UTC (permalink / raw)
To: sbeller; +Cc: git, bmwill, novalis, sandals, hvoigt, gitster, jrnieder, ramsay
In-Reply-To: <20170302004759.27852-1-sbeller@google.com>
In a later patch we need to prepare the submodule environment with
another git directory, so split up the function.
Also move it up in the file such that we do not need to declare the
function later before using it.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
submodule.c | 29 +++++++++++++++++------------
1 file changed, 17 insertions(+), 12 deletions(-)
diff --git a/submodule.c b/submodule.c
index 8b2c0212be..0b2596e88a 100644
--- a/submodule.c
+++ b/submodule.c
@@ -356,6 +356,23 @@ static void print_submodule_summary(struct rev_info *rev, FILE *f,
strbuf_release(&sb);
}
+static void prepare_submodule_repo_env_no_git_dir(struct argv_array *out)
+{
+ const char * const *var;
+
+ for (var = local_repo_env; *var; var++) {
+ if (strcmp(*var, CONFIG_DATA_ENVIRONMENT))
+ argv_array_push(out, *var);
+ }
+}
+
+void prepare_submodule_repo_env(struct argv_array *out)
+{
+ prepare_submodule_repo_env_no_git_dir(out);
+ argv_array_pushf(out, "%s=%s", GIT_DIR_ENVIRONMENT,
+ DEFAULT_GIT_DIR_ENVIRONMENT);
+}
+
/* Helper function to display the submodule header line prior to the full
* summary output. If it can locate the submodule objects directory it will
* attempt to lookup both the left and right commits and put them into the
@@ -1390,18 +1407,6 @@ int parallel_submodules(void)
return parallel_jobs;
}
-void prepare_submodule_repo_env(struct argv_array *out)
-{
- const char * const *var;
-
- for (var = local_repo_env; *var; var++) {
- if (strcmp(*var, CONFIG_DATA_ENVIRONMENT))
- argv_array_push(out, *var);
- }
- argv_array_pushf(out, "%s=%s", GIT_DIR_ENVIRONMENT,
- DEFAULT_GIT_DIR_ENVIRONMENT);
-}
-
/*
* Embeds a single submodules git directory into the superprojects git dir,
* non recursively.
--
2.12.0.rc1.52.ge239d7e709.dirty
^ permalink raw reply related
* Re: Delta compression not so effective
From: Linus Torvalds @ 2017-03-02 0:43 UTC (permalink / raw)
To: Marius Storm-Olsen; +Cc: Git Mailing List
In-Reply-To: <603afdf2-159c-6bed-0e85-2824391185d1@gmail.com>
On Wed, Mar 1, 2017 at 4:12 PM, Marius Storm-Olsen <mstormo@gmail.com> wrote:
>
> No, the list of git verify-objects in the previous post was from the bottom
> of the sorted list, so those are the largest blobs, ~249MB..
.. so with a 6GB window, you should easily sill have 20+ objects. Not
a huge window, but it should find some deltas.
But a smaller window - _together_ with a suboptimal sorting choice -
could then result in lack of successful delta matches.
> So, this repo must be knocking several parts of Git's insides. I was curious
> about why it was so slow on the writing objects part, since the whole repo
> is on a 4x RAID 5, 7k spindels. Now, they are not SSDs sure, but the thing
> has ~400MB/s continuous throughput available.
>
> iostat -m 5 showed trickle read/write to the process, and 80-100% CPU single
> thread (since the "write objects" stage is single threaded, obviously).
So the writing phase isn't multi-threaded because it's not expected to
matter. But if you can't even generate deltas, you aren't just
*writing* much more data, you're compressing all that data with zlib
too.
So even with a fast disk subsystem, you won't even be able to saturate
the disk, simply because the compression will be slower (and
single-threaded).
> Filenames are fairly static, and the bulk of the 6000 biggest non-delta'ed
> blobs are the same DLLs (multiple of them)
I think the first thing you should test is to repack with fewer
threads, and a bigger pack window. Do somethinig like
-c pack.threads=4 --window-memory=30g
instead. Just to see if that starts finding deltas.
> Right, now on this machine, I really didn't notice much difference between
> standard zlib level and doing -9. The 203GB version was actually with
> zlib=9.
Don't. zlib has *horrible* scaling with higher compressions. It
doesn't actually improve the end result very much, and it makes things
*much* slower.
zlib was a reasonable choice when git started - well-known, stable, easy to use.
But realistically it's a relatively horrible choice today, just
because there are better alternatives now.
>> Hos sensitive is your material? Could you make a smaller repo with
>> some of the blobs that still show the symptoms? I don't think I want
>> to download 206GB of data even if my internet access is good.
>
> Pretty sensitive, and not sure how I can reproduce this reasonable well.
> However, I can easily recompile git with any recommended
> instrumentation/printfs, if you have any suggestions of good places to
> start? If anyone have good file/line numbers, I'll give that a go, and
> report back?
So the first thing you might want to do is to just print out the
objects after sorting them, and before it starts trying to finsd
deltas.
See prepare_pack() in builtin/pack-objects.c, where it does something like this:
if (nr_deltas && n > 1) {
unsigned nr_done = 0;
if (progress)
progress_state = start_progress(_("Compressing
objects"),
nr_deltas);
QSORT(delta_list, n, type_size_sort);
ll_find_deltas(delta_list, n, window+1, depth, &nr_done);
stop_progress(&progress_state);
and notice that QSORT() line: that's what sorts the objects. You can
do something like
for (i = 0; i < n; i++)
show_object_entry_details(delta_list[i]);
right after that QSORT(), and make that print out the object hash,
filename hash, and size (we don't have the filename that the object
was associated with any more at that stage - they take too much
space).
Save off that array for off-line processing: when you have the object
hash, you can see what the contents are, and match it up wuith the
file in the git history using something like
git log --oneline --raw -R --abbrev=40
which shows you the log, but also the "diff" in the form of "this
filename changed from SHA1 to SHA1", so you can match up the object
hashes with where they are in the tree (and where they are in
history).
So then you could try to figure out if that type_size_sort() heuristic
is just particularly horrible for you.
In fact, if your data is not *so* sensitive, and you're ok with making
the one-line commit logs and the filenames public, you could make just
those things available, and maybe I'll have time to look at it.
I'm in the middle of the kernel merge window, but I'm in the last
stretch, and because of the SHA1 thing I've been looking at git
lately. No promises, though.
Linus
^ permalink raw reply
* [PATCH 15/18] read-cache, remove_marked_cache_entries: wipe selected submodules.
From: Stefan Beller @ 2017-03-02 0:47 UTC (permalink / raw)
To: sbeller; +Cc: git, bmwill, novalis, sandals, hvoigt, gitster, jrnieder, ramsay
In-Reply-To: <20170302004759.27852-1-sbeller@google.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
read-cache.c | 27 +++++++++++++++++++++++++--
1 file changed, 25 insertions(+), 2 deletions(-)
diff --git a/read-cache.c b/read-cache.c
index 9054369dd0..9a2abacf7a 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -18,6 +18,8 @@
#include "varint.h"
#include "split-index.h"
#include "utf8.h"
+#include "submodule.h"
+#include "submodule-config.h"
/* Mask for the name length in ce_flags in the on-disk index */
@@ -520,6 +522,22 @@ int remove_index_entry_at(struct index_state *istate, int pos)
return 1;
}
+static void remove_submodule_according_to_strategy(const struct submodule *sub)
+{
+ switch (sub->update_strategy.type) {
+ case SM_UPDATE_UNSPECIFIED:
+ case SM_UPDATE_CHECKOUT:
+ case SM_UPDATE_REBASE:
+ case SM_UPDATE_MERGE:
+ submodule_move_head(sub->path, "HEAD", NULL, \
+ SUBMODULE_MOVE_HEAD_FORCE);
+ break;
+ case SM_UPDATE_NONE:
+ case SM_UPDATE_COMMAND:
+ ; /* Do not touch the submodule. */
+ }
+}
+
/*
* Remove all cache entries marked for removal, that is where
* CE_REMOVE is set in ce_flags. This is much more effective than
@@ -532,8 +550,13 @@ void remove_marked_cache_entries(struct index_state *istate)
for (i = j = 0; i < istate->cache_nr; i++) {
if (ce_array[i]->ce_flags & CE_REMOVE) {
- remove_name_hash(istate, ce_array[i]);
- save_or_free_index_entry(istate, ce_array[i]);
+ const struct submodule *sub = submodule_from_ce(ce_array[i]);
+ if (sub) {
+ remove_submodule_according_to_strategy(sub);
+ } else {
+ remove_name_hash(istate, ce_array[i]);
+ save_or_free_index_entry(istate, ce_array[i]);
+ }
}
else
ce_array[j++] = ce_array[i];
--
2.12.0.rc1.52.ge239d7e709.dirty
^ permalink raw reply related
* [PATCH 13/18] unpack-trees: pass old oid to verify_clean_submodule
From: Stefan Beller @ 2017-03-02 0:47 UTC (permalink / raw)
To: sbeller; +Cc: git, bmwill, novalis, sandals, hvoigt, gitster, jrnieder, ramsay
In-Reply-To: <20170302004759.27852-1-sbeller@google.com>
The check (which uses the old oid) is yet to be implemented, but this part
is just a refactor, so it can go separately first.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
unpack-trees.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/unpack-trees.c b/unpack-trees.c
index 3a8ee19fe8..616a0ae4b2 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -1407,7 +1407,8 @@ static void invalidate_ce_path(const struct cache_entry *ce,
* Currently, git does not checkout subprojects during a superproject
* checkout, so it is not going to overwrite anything.
*/
-static int verify_clean_submodule(const struct cache_entry *ce,
+static int verify_clean_submodule(const char *old_sha1,
+ const struct cache_entry *ce,
enum unpack_trees_error_types error_type,
struct unpack_trees_options *o)
{
@@ -1427,16 +1428,18 @@ static int verify_clean_subdirectory(const struct cache_entry *ce,
struct dir_struct d;
char *pathbuf;
int cnt = 0;
- unsigned char sha1[20];
- if (S_ISGITLINK(ce->ce_mode) &&
- resolve_gitlink_ref(ce->name, "HEAD", sha1) == 0) {
- /* If we are not going to update the submodule, then
+ if (S_ISGITLINK(ce->ce_mode)) {
+ unsigned char sha1[20];
+ int sub_head = resolve_gitlink_ref(ce->name, "HEAD", sha1);
+ /*
+ * If we are not going to update the submodule, then
* we don't care.
*/
- if (!hashcmp(sha1, ce->oid.hash))
+ if (!sub_head && !hashcmp(sha1, ce->oid.hash))
return 0;
- return verify_clean_submodule(ce, error_type, o);
+ return verify_clean_submodule(sub_head ? NULL : sha1_to_hex(sha1),
+ ce, error_type, o);
}
/*
--
2.12.0.rc1.52.ge239d7e709.dirty
^ permalink raw reply related
* Re: [PATCH 2/2] pretty: use fmt_output_email_subject()
From: Jeff King @ 2017-03-01 19:43 UTC (permalink / raw)
To: Junio C Hamano; +Cc: René Scharfe, Adrian Dudau, git@vger.kernel.org
In-Reply-To: <CAPc5daUyu_=5GeGqBAG_KvkZFE_0oD6Wa_HhoRQwZ2ANUwOF+Q@mail.gmail.com>
On Wed, Mar 01, 2017 at 11:38:57AM -0800, Junio C Hamano wrote:
> On Wed, Mar 1, 2017 at 11:32 AM, Jeff King <peff@peff.net> wrote:
> > I would think that future callers would just need to provide a dummy
> > pp->rev. I guess that logic could be pushed down into
> > fmt_output_email_subject(), so that it skips looking at
> > opt->subject_prefix, etc, when "opt" is NULL, and just hits the
> > "Subject:" case arm.
>
> The "flexibility" I was wondering about is that the current .subject can
> point at any caller-supplied string, not "Subject:".
Ah, I see. I don't think that is a huge loss, as nobody was using it.
And "Subject:" is already hard-coded in the nr/total counting bits,
which are what you'd want to reuse. I think it is fine to punt to the
future. If somebody really wants it later, the right fix is for them to
provide a string that fmt_output_email_subject() would use in place of
"Subject:" when it adds to the strbuf.
-Peff
^ permalink raw reply
* [PATCH 18/18] builtin/read-tree: add --recurse-submodules switch
From: Stefan Beller @ 2017-03-02 0:47 UTC (permalink / raw)
To: sbeller; +Cc: git, bmwill, novalis, sandals, hvoigt, gitster, jrnieder, ramsay
In-Reply-To: <20170302004759.27852-1-sbeller@google.com>
A new known failure mode is introduced[1], which is actually not
a failure but a feature in read-tree. Unlike checkout for which
the recursive submodule tests were originally written, read-tree does
warn about ignored untracked files that would be overwritten.
For the sake of keeping the test library for submodules genric, just
mark the test as a failure.
[1] KNOWN_FAILURE_SUBMODULE_OVERWRITE_IGNORED_UNTRACKED
Signed-off-by: Stefan Beller <sbeller@google.com>
---
Documentation/git-read-tree.txt | 6 ++++++
builtin/read-tree.c | 29 +++++++++++++++++++++++++++++
t/lib-submodule-update.sh | 7 ++++++-
t/t1013-read-tree-submodule.sh | 7 +++++++
4 files changed, 48 insertions(+), 1 deletion(-)
diff --git a/Documentation/git-read-tree.txt b/Documentation/git-read-tree.txt
index fa1d557e5b..ed9d63ef4a 100644
--- a/Documentation/git-read-tree.txt
+++ b/Documentation/git-read-tree.txt
@@ -115,6 +115,12 @@ OPTIONS
directories the index file and index output file are
located in.
+--[no-]recurse-submodules::
+ Using --recurse-submodules will update the content of all initialized
+ submodules according to the commit recorded in the superproject by
+ calling read-tree recursively, also setting the submodules HEAD to be
+ detached at that commit.
+
--no-sparse-checkout::
Disable sparse checkout support even if `core.sparseCheckout`
is true.
diff --git a/builtin/read-tree.c b/builtin/read-tree.c
index 8ba64bc785..2dc5cc06da 100644
--- a/builtin/read-tree.c
+++ b/builtin/read-tree.c
@@ -15,10 +15,13 @@
#include "builtin.h"
#include "parse-options.h"
#include "resolve-undo.h"
+#include "submodule.h"
+#include "submodule-config.h"
static int nr_trees;
static int read_empty;
static struct tree *trees[MAX_UNPACK_TREES];
+int recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
static int list_tree(unsigned char *sha1)
{
@@ -96,6 +99,23 @@ static int debug_merge(const struct cache_entry * const *stages,
return 0;
}
+static int option_parse_recurse_submodules(const struct option *opt,
+ const char *arg, int unset)
+{
+ if (unset) {
+ recurse_submodules = RECURSE_SUBMODULES_OFF;
+ return 0;
+ }
+ if (arg)
+ recurse_submodules =
+ parse_update_recurse_submodules_arg(opt->long_name,
+ arg);
+ else
+ recurse_submodules = RECURSE_SUBMODULES_ON;
+
+ return 0;
+}
+
static struct lock_file lock_file;
int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)
@@ -137,6 +157,9 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)
N_("skip applying sparse checkout filter")),
OPT_BOOL(0, "debug-unpack", &opts.debug_unpack,
N_("debug unpack-trees")),
+ { OPTION_CALLBACK, 0, "recurse-submodules", &recurse_submodules,
+ "checkout", "control recursive updating of submodules",
+ PARSE_OPT_OPTARG, option_parse_recurse_submodules },
OPT_END()
};
@@ -152,6 +175,12 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)
hold_locked_index(&lock_file, LOCK_DIE_ON_ERROR);
+ if (recurse_submodules != RECURSE_SUBMODULES_DEFAULT) {
+ gitmodules_config();
+ git_config(submodule_config, NULL);
+ set_config_update_recurse_submodules(RECURSE_SUBMODULES_ON);
+ }
+
prefix_set = opts.prefix ? 1 : 0;
if (1 < opts.merge + opts.reset + prefix_set)
die("Which one? -m, --reset, or --prefix?");
diff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh
index d71972c3db..5caae06bc5 100755
--- a/t/lib-submodule-update.sh
+++ b/t/lib-submodule-update.sh
@@ -787,6 +787,11 @@ test_submodule_switch_recursing () {
then
RESULT=failure
fi
+ RESULT1=success
+ if test "$KNOWN_FAILURE_SUBMODULE_OVERWRITE_IGNORED_UNTRACKED" = 1
+ then
+ RESULT1=failure
+ fi
######################### Appearing submodule #########################
# Switching to a commit letting a submodule appear checks it out ...
test_expect_success "$command: added submodule is checked out" '
@@ -827,7 +832,7 @@ test_submodule_switch_recursing () {
)
'
# ... but an ignored file is fine.
- test_expect_success "$command: added submodule removes an untracked ignored file" '
+ test_expect_$RESULT1 "$command: added submodule removes an untracked ignored file" '
test_when_finished "rm submodule_update/.git/info/exclude" &&
prolog &&
reset_work_tree_to_interested no_submodule &&
diff --git a/t/t1013-read-tree-submodule.sh b/t/t1013-read-tree-submodule.sh
index 20526aed34..7019d0a04f 100755
--- a/t/t1013-read-tree-submodule.sh
+++ b/t/t1013-read-tree-submodule.sh
@@ -5,6 +5,13 @@ test_description='read-tree can handle submodules'
. ./test-lib.sh
. "$TEST_DIRECTORY"/lib-submodule-update.sh
+KNOWN_FAILURE_DIRECTORY_SUBMODULE_CONFLICTS=1
+KNOWN_FAILURE_SUBMODULE_OVERWRITE_IGNORED_UNTRACKED=1
+
+test_submodule_switch_recursing "git read-tree --recurse-submodules -u -m"
+
+test_submodule_forced_switch_recursing "git read-tree --recurse-submodules -u --reset"
+
test_submodule_switch "git read-tree -u -m"
test_submodule_forced_switch "git read-tree -u --reset"
--
2.12.0.rc1.52.ge239d7e709.dirty
^ permalink raw reply related
* [PATCH 17/18] builtin/checkout: add --recurse-submodules switch
From: Stefan Beller @ 2017-03-02 0:47 UTC (permalink / raw)
To: sbeller; +Cc: git, bmwill, novalis, sandals, hvoigt, gitster, jrnieder, ramsay
In-Reply-To: <20170302004759.27852-1-sbeller@google.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
Documentation/git-checkout.txt | 7 +++++++
builtin/checkout.c | 28 ++++++++++++++++++++++++++++
t/lib-submodule-update.sh | 23 +++++++++++++++++------
t/t2013-checkout-submodule.sh | 5 +++++
4 files changed, 57 insertions(+), 6 deletions(-)
diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
index 8e2c0662dd..d6399c0af8 100644
--- a/Documentation/git-checkout.txt
+++ b/Documentation/git-checkout.txt
@@ -256,6 +256,13 @@ section of linkgit:git-add[1] to learn how to operate the `--patch` mode.
out anyway. In other words, the ref can be held by more than one
worktree.
+--[no-]recurse-submodules::
+ Using --recurse-submodules will update the content of all initialized
+ submodules according to the commit recorded in the superproject. If
+ local modifications in a submodule would be overwritten the checkout
+ will fail unless `-f` is used. If nothing (or --no-recurse-submodules)
+ is used, the work trees of submodules will not be updated.
+
<branch>::
Branch to checkout; if it refers to a branch (i.e., a name that,
when prepended with "refs/heads/", is a valid ref), then that
diff --git a/builtin/checkout.c b/builtin/checkout.c
index f174f50303..e9c5fcfaf8 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -21,12 +21,31 @@
#include "submodule-config.h"
#include "submodule.h"
+static int recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
+
static const char * const checkout_usage[] = {
N_("git checkout [<options>] <branch>"),
N_("git checkout [<options>] [<branch>] -- <file>..."),
NULL,
};
+static int option_parse_recurse_submodules(const struct option *opt,
+ const char *arg, int unset)
+{
+ if (unset) {
+ recurse_submodules = RECURSE_SUBMODULES_OFF;
+ return 0;
+ }
+ if (arg)
+ recurse_submodules =
+ parse_update_recurse_submodules_arg(opt->long_name,
+ arg);
+ else
+ recurse_submodules = RECURSE_SUBMODULES_ON;
+
+ return 0;
+}
+
struct checkout_opts {
int patch_mode;
int quiet;
@@ -1163,6 +1182,9 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
N_("second guess 'git checkout <no-such-branch>'")),
OPT_BOOL(0, "ignore-other-worktrees", &opts.ignore_other_worktrees,
N_("do not check if another worktree is holding the given ref")),
+ { OPTION_CALLBACK, 0, "recurse-submodules", &recurse_submodules,
+ "checkout", "control recursive updating of submodules",
+ PARSE_OPT_OPTARG, option_parse_recurse_submodules },
OPT_BOOL(0, "progress", &opts.show_progress, N_("force progress reporting")),
OPT_END(),
};
@@ -1193,6 +1215,12 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
git_xmerge_config("merge.conflictstyle", conflict_style, NULL);
}
+ if (recurse_submodules != RECURSE_SUBMODULES_OFF) {
+ git_config(submodule_config, NULL);
+ if (recurse_submodules != RECURSE_SUBMODULES_DEFAULT)
+ set_config_update_recurse_submodules(recurse_submodules);
+ }
+
if ((!!opts.new_branch + !!opts.new_branch_force + !!opts.new_orphan_branch) > 1)
die(_("-b, -B and --orphan are mutually exclusive"));
diff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh
index aa196d4d42..d71972c3db 100755
--- a/t/lib-submodule-update.sh
+++ b/t/lib-submodule-update.sh
@@ -782,6 +782,11 @@ test_submodule_forced_switch () {
test_submodule_switch_recursing () {
command="$1"
+ RESULT=success
+ if test "$KNOWN_FAILURE_DIRECTORY_SUBMODULE_CONFLICTS" = 1
+ then
+ RESULT=failure
+ fi
######################### Appearing submodule #########################
# Switching to a commit letting a submodule appear checks it out ...
test_expect_success "$command: added submodule is checked out" '
@@ -891,7 +896,7 @@ test_submodule_switch_recursing () {
'
# Replacing a submodule with files in a directory must succeeds
# when the submodule is clean
- test_expect_success "$command: replace submodule with a directory" '
+ test_expect_$RESULT "$command: replace submodule with a directory" '
prolog &&
reset_work_tree_to_interested add_sub1 &&
(
@@ -903,7 +908,7 @@ test_submodule_switch_recursing () {
)
'
# ... absorbing a .git directory.
- test_expect_success "$command: replace submodule containing a .git directory with a directory must absorb the git dir" '
+ test_expect_$RESULT "$command: replace submodule containing a .git directory with a directory must absorb the git dir" '
prolog &&
reset_work_tree_to_interested add_sub1 &&
(
@@ -931,7 +936,7 @@ test_submodule_switch_recursing () {
'
# ... must check its local work tree for untracked files
- test_expect_success "$command: replace submodule with a file must fail with untracked files" '
+ test_expect_$RESULT "$command: replace submodule with a file must fail with untracked files" '
prolog &&
reset_work_tree_to_interested add_sub1 &&
(
@@ -987,7 +992,8 @@ test_submodule_switch_recursing () {
)
'
- test_expect_success "$command: modified submodule updates submodule recursively" '
+ # recursing deeper than one level doesn't work yet.
+ test_expect_failure "$command: modified submodule updates submodule recursively" '
prolog &&
reset_work_tree_to_interested add_nested_sub &&
(
@@ -1006,6 +1012,11 @@ test_submodule_switch_recursing () {
# the superproject as well as the submodule is allowed.
test_submodule_forced_switch_recursing () {
command="$1"
+ RESULT=success
+ if test "$KNOWN_FAILURE_DIRECTORY_SUBMODULE_CONFLICTS" = 1
+ then
+ RESULT=failure
+ fi
######################### Appearing submodule #########################
# Switching to a commit letting a submodule appear creates empty dir ...
test_expect_success "$command: added submodule is checked out" '
@@ -1100,7 +1111,7 @@ test_submodule_forced_switch_recursing () {
)
'
# Replacing a submodule with files in a directory ...
- test_expect_success "$command: replace submodule with a directory" '
+ test_expect_$RESULT "$command: replace submodule with a directory" '
prolog &&
reset_work_tree_to_interested add_sub1 &&
(
@@ -1151,7 +1162,7 @@ test_submodule_forced_switch_recursing () {
'
# ... but stops for untracked files that would be lost
- test_expect_success "$command: replace submodule with a file" '
+ test_expect_$RESULT "$command: replace submodule with a file stops for untracked files" '
prolog &&
reset_work_tree_to_interested add_sub1 &&
(
diff --git a/t/t2013-checkout-submodule.sh b/t/t2013-checkout-submodule.sh
index 6847f75822..aa35223369 100755
--- a/t/t2013-checkout-submodule.sh
+++ b/t/t2013-checkout-submodule.sh
@@ -63,6 +63,11 @@ test_expect_success '"checkout <submodule>" honors submodule.*.ignore from .git/
! test -s actual
'
+KNOWN_FAILURE_DIRECTORY_SUBMODULE_CONFLICTS=1
+test_submodule_switch_recursing "git checkout --recurse-submodules"
+
+test_submodule_forced_switch_recursing "git checkout -f --recurse-submodules"
+
test_submodule_switch "git checkout"
test_submodule_forced_switch "git checkout -f"
--
2.12.0.rc1.52.ge239d7e709.dirty
^ permalink raw reply related
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