From: Philippe Blain <levraiphilippeblain@gmail.com>
To: "brian m. carlson" <sandals@crustytoothpaste.net>,
git@vger.kernel.org, Rose Kunkel <rose@rosekunkel.me>,
Emily Shaffer <emilyshaffer@google.com>,
Jonathan Tan <jonathantanmy@google.com>
Subject: Re: [PATCH] submodule: mark submodules with update=none as inactive
Date: Sat, 26 Jun 2021 11:12:07 -0400 [thread overview]
Message-ID: <67a6e80e-6eb6-10c2-2dcd-0610343884cd@gmail.com> (raw)
In-Reply-To: <YNZgoS9R1qam+62C@camp.crustytoothpaste.net>
Hi brian,
Le 2021-06-25 à 19:02, brian m. carlson a écrit :
> On 2021-06-22 at 03:45:45, Philippe Blain wrote:
>>> That will make us properly ignore the submodule
>>> when performing recursive operations.
>>>
>>> Note that we only do this when the --require-init option is passed,
>>> which is only passed during clone. That's because the update-clone
>>> submodule helper is also invoked during a user-invoked git submodule
>>> update, where we explicitly indicate that we override the configuration
>>> setting, so we don't want to set such a configuration option then.
>>
>> I'm not sure what you mean here by 'where we explicitely indicate that we
>> override the configuration setting'. For me, as I wrote above,
>> 'git clone --recurse-submodules' and 'git clone' followed by
>> 'git submodule update --init' should lead to mostly [*] the same end result.
>>
>> If you mean 'git submodule update --checkout', that indeed seems to sometimes override the 'update=none'
>> configuration (a fact which is absent from the documentation),
Note that I'm taking back that statemnt about the doc; the description of 'git submodule update'
states:
"The "updating" can be done in several ways depending on command line options
and the value of submodule.<name>.update configuration variable. The command line
option takes precedence over the configuration variable."
I was somehow under the impression that 'none' was a special case, but it's not.
then it's true that we
>> would not want to write 'active=false' at that invocation. As an aside, in my limited testing
>> I could not always get 'git submodule update --checkout' to clone and checkout 'update=none' submdules;
>> it would fail with "fatal: could not get a repository handle for submodule 'sub1'" because
>> 'git checkout/reset --recurse-submodules' leaves a bad .git/modules/sub1/config file
>> with the core.worktree setting when the command fails (this should not happen)...
>
> Yes, that's what I meant.
>
>> In any case, that leads me to think that maybe the right place to write the 'active' setting
>> would be during 'git submodule init', thus builtin/submodule--helper.c::init_submodule ?
>> This way it would lead to the same behaviour if the clone was recursive or not,
>> and it would not interfere with 'git submodule update --checkout'.
>
> Let me take a look at some other approaches and see if I can come up
> with something a little bit better.
I tested the following and it fixes the bug (for both recursive clone and
normal clone followed by 'git submodule update --init') and
t7406-submodule-update.sh still passes:
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index d55f6262e9..a4cd86c72f 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -686,6 +686,13 @@ static void init_submodule(const char *path, const char *prefix,
if (git_config_set_gently(sb.buf, upd))
die(_("Failed to register update mode for submodule path '%s'"), displaypath);
+
+ /* Mark update=none submodules as inactive */
+ if (sub->update_strategy.type == SM_UPDATE_NONE) {
+ strbuf_reset(&sb);
+ strbuf_addf(&sb, "submodule.%s.active", sub->name);
+ git_config_set_gently(sb.buf, "false");
+ }
}
strbuf_release(&sb);
free(displaypath);
>
> My apologies for the delay in response; I'm in the process of moving at
> the moment and my attention has been directed elsewhere than the list.
>
No problem of course!
Philippe.
[1] https://git-scm.com/docs/git-submodule#Documentation/git-submodule.txt-update--init--remote-N--no-fetch--no-recommend-shallow-f--force--checkout--rebase--merge--referenceltrepositorygt--depthltdepthgt--recursive--jobsltngt--no-single-branch--ltpathgt82308203
next prev parent reply other threads:[~2021-06-26 15:12 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-16 0:16 [BUG] `git reset --hard` fails with `update = none` submodules Rose Kunkel
2021-06-16 0:51 ` brian m. carlson
2021-06-16 0:57 ` Rose Kunkel
2021-06-16 1:03 ` Rose Kunkel
2021-06-16 1:15 ` Rose Kunkel
2021-06-16 1:25 ` brian m. carlson
2021-06-16 1:39 ` Rose Kunkel
2021-06-16 1:46 ` Rose Kunkel
2021-06-16 3:10 ` Junio C Hamano
2021-06-16 13:20 ` Philippe Blain
2021-06-17 23:52 ` brian m. carlson
2021-06-19 21:44 ` [PATCH] submodule: mark submodules with update=none as inactive brian m. carlson
2021-06-22 3:45 ` Philippe Blain
2021-06-25 23:02 ` brian m. carlson
2021-06-26 15:12 ` Philippe Blain [this message]
2021-07-01 22:51 ` [PATCH v2] " brian m. carlson
2021-07-09 20:26 ` Philippe Blain
2021-07-11 16:59 ` brian m. carlson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=67a6e80e-6eb6-10c2-2dcd-0610343884cd@gmail.com \
--to=levraiphilippeblain@gmail.com \
--cc=emilyshaffer@google.com \
--cc=git@vger.kernel.org \
--cc=jonathantanmy@google.com \
--cc=rose@rosekunkel.me \
--cc=sandals@crustytoothpaste.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.