From: Junio C Hamano <hidden> Date: 2017-08-19 06:25:05
Stefan Beller [off-list ref] writes:
From: Lars Schneider <redacted>
Do not override the submodule configuration in the call to update
the submodules, but give a weaker default.
Reported-by: Lars Schneider <redacted>
Signed-off-by: Stefan Beller <redacted>
---
Personally I dislike this patch, but I have no better idea for the time
being.
The patch text from a cursory look seems reasonable to me.
It's not like you have 47 different codepaths that need to pay
attention to the .update config and they all have to pass the new
--default-update option, this is merely to fix one of them that
relates to the problem reported by Lars, and you need a similar fix
to other 46, right?
If you want the "--recurse-submodules" thing to always do the
"weaker default" thing in your project, you can choose not to set
.update to custom values in any of your submodules, so I do not
think the reason why you dislike this change is because it would
affect your use of submodules.
So I am a bit curious to learn which part of this change you dislike
and why.
On Fri, Aug 18, 2017 at 11:24:47PM -0700, Junio C Hamano wrote:
Stefan Beller [off-list ref] writes:
quoted
From: Lars Schneider <redacted>
Do not override the submodule configuration in the call to update
the submodules, but give a weaker default.
Reported-by: Lars Schneider <redacted>
Signed-off-by: Stefan Beller <redacted>
---
Personally I dislike this patch, but I have no better idea for the time
being.
The patch text from a cursory look seems reasonable to me.
It's not like you have 47 different codepaths that need to pay
attention to the .update config and they all have to pass the new
--default-update option, this is merely to fix one of them that
relates to the problem reported by Lars, and you need a similar fix
to other 46, right?
If you want the "--recurse-submodules" thing to always do the
"weaker default" thing in your project, you can choose not to set
.update to custom values in any of your submodules, so I do not
think the reason why you dislike this change is because it would
affect your use of submodules.
So I am a bit curious to learn which part of this change you dislike
and why.
I am also curious. Isn't this the same strategy we are using in other
places?
Cheers Heiko
From: Stefan Beller <hidden> Date: 2017-08-21 16:55:23
On Mon, Aug 21, 2017 at 9:20 AM, Heiko Voigt [off-list ref] wrote:
quoted
So I am a bit curious to learn which part of this change you dislike
and why.
I am also curious. Isn't this the same strategy we are using in other
places?
I dislike it because the UX feels crude. When reading the documentation,
it seems to me as if submodule.<name> can be one of the following
(none, checkout, rebase, merge, !<custom-command>)
This is perfect for "submodule-update", whose primary goal is
to update submodules *somehow*. However other commands
git rebase --recurse
git merge --recurse
git checkout --recurse
have a different primary mode of operation (note how their name
is one of the modes from the set above), so it may get confusing
for a user.
'none' and '!<custom-command>' seem like they would be okay
for any of the commands above but then:
git config submodule.<name>.update "!..."
git reset --hard --recurse
git status
# submodule is reported, because "!..." did not 'reset'.
Anyway. That dislike is just a minor gut feeling about the UX/UI
being horrible. I wrote the patch to keep the conversation going,
and if it fixes Lars problem, let's take it for now.
Thanks,
Stefan
From: Lars Schneider <hidden> Date: 2017-08-21 17:20:34
On 21 Aug 2017, at 18:55, Stefan Beller [off-list ref] wrote:
On Mon, Aug 21, 2017 at 9:20 AM, Heiko Voigt [off-list ref] wrote:
quoted
quoted
So I am a bit curious to learn which part of this change you dislike
and why.
I am also curious. Isn't this the same strategy we are using in other
places?
I dislike it because the UX feels crude. When reading the documentation,
it seems to me as if submodule.<name> can be one of the following
(none, checkout, rebase, merge, !<custom-command>)
This is perfect for "submodule-update", whose primary goal is
to update submodules *somehow*. However other commands
git rebase --recurse
git merge --recurse
git checkout --recurse
have a different primary mode of operation (note how their name
is one of the modes from the set above), so it may get confusing
for a user.
'none' and '!<custom-command>' seem like they would be okay
for any of the commands above but then:
git config submodule.<name>.update "!..."
git reset --hard --recurse
git status
# submodule is reported, because "!..." did not 'reset'.
Anyway. That dislike is just a minor gut feeling about the UX/UI
being horrible. I wrote the patch to keep the conversation going,
and if it fixes Lars problem, let's take it for now.
From: Stefan Beller <hidden> Date: 2017-08-21 17:48:15
On Mon, Aug 21, 2017 at 10:20 AM, Lars Schneider
[off-list ref] wrote:
quoted
On 21 Aug 2017, at 18:55, Stefan Beller [off-list ref] wrote:
On Mon, Aug 21, 2017 at 9:20 AM, Heiko Voigt [off-list ref] wrote:
quoted
quoted
So I am a bit curious to learn which part of this change you dislike
and why.
I am also curious. Isn't this the same strategy we are using in other
places?
I dislike it because the UX feels crude. When reading the documentation,
it seems to me as if submodule.<name> can be one of the following
(none, checkout, rebase, merge, !<custom-command>)
This is perfect for "submodule-update", whose primary goal is
to update submodules *somehow*. However other commands
git rebase --recurse
git merge --recurse
git checkout --recurse
have a different primary mode of operation (note how their name
is one of the modes from the set above), so it may get confusing
for a user.
'none' and '!<custom-command>' seem like they would be okay
for any of the commands above but then:
git config submodule.<name>.update "!..."
git reset --hard --recurse
git status
# submodule is reported, because "!..." did not 'reset'.
Anyway. That dislike is just a minor gut feeling about the UX/UI
being horrible. I wrote the patch to keep the conversation going,
and if it fixes Lars problem, let's take it for now.
(A) you need to set expect there as well, to have sub{2,4,5} be expected
there as well.
(B) That may hint at another (UX) bug.
The test case there uses "git submodule update --init".
The init flag will set all submodules to active.
Maybe you want
git config submodule.active ":(exclude)sub0"
git config --add submodule.active ":(exclude)sub2"
git config --add submodule.active "."
# Read: anything except sub0 and sub2 are interesting
git submodule update
# no init flag, needed even for new submodules IIUC
From: Brandon Williams <hidden> Date: 2017-08-21 18:21:19
On 08/21, Stefan Beller wrote:
On Mon, Aug 21, 2017 at 10:20 AM, Lars Schneider
[off-list ref] wrote:
quoted
quoted
On 21 Aug 2017, at 18:55, Stefan Beller [off-list ref] wrote:
On Mon, Aug 21, 2017 at 9:20 AM, Heiko Voigt [off-list ref] wrote:
quoted
quoted
So I am a bit curious to learn which part of this change you dislike
and why.
I am also curious. Isn't this the same strategy we are using in other
places?
I dislike it because the UX feels crude. When reading the documentation,
it seems to me as if submodule.<name> can be one of the following
(none, checkout, rebase, merge, !<custom-command>)
This is perfect for "submodule-update", whose primary goal is
to update submodules *somehow*. However other commands
git rebase --recurse
git merge --recurse
git checkout --recurse
have a different primary mode of operation (note how their name
is one of the modes from the set above), so it may get confusing
for a user.
'none' and '!<custom-command>' seem like they would be okay
for any of the commands above but then:
git config submodule.<name>.update "!..."
git reset --hard --recurse
git status
# submodule is reported, because "!..." did not 'reset'.
Anyway. That dislike is just a minor gut feeling about the UX/UI
being horrible. I wrote the patch to keep the conversation going,
and if it fixes Lars problem, let's take it for now.
Well, I need just a way to disable certain Submodules completely.
If you show me how "git config --local submodule.sub.active false"
works then I don't need this patch.
Yeah if you want to completely disable a submodule (as in not even check
it out) then setting .active to false would do that. But as stefan
pointed out and IIRC 'submodule update --init' with no pathspec sets all
submodules to be active. Perhaps it should only init submodules who
don't already have an explicit active flag set.
(A) you need to set expect there as well, to have sub{2,4,5} be expected
there as well.
(B) That may hint at another (UX) bug.
The test case there uses "git submodule update --init".
The init flag will set all submodules to active.
Maybe you want
git config submodule.active ":(exclude)sub0"
git config --add submodule.active ":(exclude)sub2"
git config --add submodule.active "."
# Read: anything except sub0 and sub2 are interesting
git submodule update
# no init flag, needed even for new submodules IIUC
From: Lars Schneider <hidden> Date: 2017-08-22 14:50:14
On 21 Aug 2017, at 20:21, Brandon Williams [off-list ref] wrote:
On 08/21, Stefan Beller wrote:
quoted
On Mon, Aug 21, 2017 at 10:20 AM, Lars Schneider
[off-list ref] wrote:
quoted
quoted
On 21 Aug 2017, at 18:55, Stefan Beller [off-list ref] wrote:
On Mon, Aug 21, 2017 at 9:20 AM, Heiko Voigt [off-list ref] wrote:
quoted
quoted
So I am a bit curious to learn which part of this change you dislike
and why.
I am also curious. Isn't this the same strategy we are using in other
places?
I dislike it because the UX feels crude. When reading the documentation,
it seems to me as if submodule.<name> can be one of the following
(none, checkout, rebase, merge, !<custom-command>)
This is perfect for "submodule-update", whose primary goal is
to update submodules *somehow*. However other commands
git rebase --recurse
git merge --recurse
git checkout --recurse
have a different primary mode of operation (note how their name
is one of the modes from the set above), so it may get confusing
for a user.
'none' and '!<custom-command>' seem like they would be okay
for any of the commands above but then:
git config submodule.<name>.update "!..."
git reset --hard --recurse
git status
# submodule is reported, because "!..." did not 'reset'.
Anyway. That dislike is just a minor gut feeling about the UX/UI
being horrible. I wrote the patch to keep the conversation going,
and if it fixes Lars problem, let's take it for now.
Well, I need just a way to disable certain Submodules completely.
If you show me how "git config --local submodule.sub.active false"
works then I don't need this patch.
Yeah if you want to completely disable a submodule (as in not even check
it out) then setting .active to false would do that. But as stefan
pointed out and IIRC 'submodule update --init' with no pathspec sets all
submodules to be active. Perhaps it should only init submodules who
don't already have an explicit active flag set.
OK. I change my scripts to use ".active" and it seems to work nicely.
I noticed one oddity, though:
If I clone a repo using `git clone --recursive <url>` then the local
Git config of the repo gets the following entry:
[submodule]
active = .
Is this intentional? Something in the git/git test harness seems to prevent
that. I was not able to write a test to replicate the issue.
Any idea?
Thanks,
Lars
From: Stefan Beller <hidden> Date: 2017-08-22 17:51:32
On Tue, Aug 22, 2017 at 7:50 AM, Lars Schneider
[off-list ref] wrote:
OK. I change my scripts to use ".active" and it seems to work nicely.
I noticed one oddity, though:
If I clone a repo using `git clone --recursive <url>` then the local
Git config of the repo gets the following entry:
[submodule]
active = .
bb62e0a99f (clone: teach --recurse-submodules to optionally take a
pathspec, 2017-03-17) makes it clear that this is intentional for
--recurse-submodules, but doesn't exactly state that --recurse will
behave the same. The idea here is that at clone time you can already
give
git clone --recurse=:(exclude)sub0 <url> <path>
and have your desired set of submodules there.
Combined with the changes in the attr system, b0db704652
(pathspec: allow querying for attributes, 2017-03-13)
you could make up things like this:
$ cat .gitattributes
/sub0 label0
/sub1
/sub2 label1 label2
/sub3 label1
/platform-specifc-subs/* label1 label2
and then get a clone via
git clone --recurse=:(attr:label2). <url> <path>
for example. The labeling via the attributes allows for
complex patterns, but a relatively easy command line, that you
can share with coworkers.
Is this intentional? Something in the git/git test harness seems to prevent
that. I was not able to write a test to replicate the issue.
Any idea?
I do not seem to understand the perceived bug?
The setting of submodule.active=<pathspec> seems intentional to me,
but how would you not reproduce it? Maybe Brandon has an idea.
Thanks,
Stefan
From: Brandon Williams <hidden> Date: 2017-08-22 18:55:51
On 08/22, Stefan Beller wrote:
On Tue, Aug 22, 2017 at 7:50 AM, Lars Schneider
[off-list ref] wrote:
quoted
OK. I change my scripts to use ".active" and it seems to work nicely.
I noticed one oddity, though:
If I clone a repo using `git clone --recursive <url>` then the local
Git config of the repo gets the following entry:
[submodule]
active = .
bb62e0a99f (clone: teach --recurse-submodules to optionally take a
pathspec, 2017-03-17) makes it clear that this is intentional for
--recurse-submodules, but doesn't exactly state that --recurse will
behave the same. The idea here is that at clone time you can already
give
git clone --recurse=:(exclude)sub0 <url> <path>
and have your desired set of submodules there.
Combined with the changes in the attr system, b0db704652
(pathspec: allow querying for attributes, 2017-03-13)
you could make up things like this:
$ cat .gitattributes
/sub0 label0
/sub1
/sub2 label1 label2
/sub3 label1
/platform-specifc-subs/* label1 label2
and then get a clone via
git clone --recurse=:(attr:label2). <url> <path>
for example. The labeling via the attributes allows for
complex patterns, but a relatively easy command line, that you
can share with coworkers.
quoted
Is this intentional? Something in the git/git test harness seems to prevent
that. I was not able to write a test to replicate the issue.
Any idea?
I do not seem to understand the perceived bug?
The setting of submodule.active=<pathspec> seems intentional to me,
but how would you not reproduce it? Maybe Brandon has an idea.
When adding '.active' we wanted it to be as flexible as possible. So
you can either use a pathspec with 'submodule.active' to catch multiple
submodules as being active or you can turn on/off individual submodules
with 'submodule.<name>.active' (this has precedent over the more general
'submodule.active' config).
The intent was if a user supplies --recurse-submodules (I believe i
removed the docs for --recursive in order to make the CLI more consistent
with other commands, so --recursive is just a synonym for
--recurse-submodules) then they clearly wanted all the submodules cloned
and checked out. With the '.active' config the way to specify this
is to make 'submodule.active = .'. In the old world every submodule
would need to have its URL copied into the config. This way the config
is kept cleaner as it only has a single entry added.
As stefan mentioned you can specify a value for 'submodule.active' to
take as an arg to --recurse-submodules (the default being '.' or all
submodules) so you can do clever things like group submodules using
attributes, you can even repeat the flag to provided a more complex
pathspec.
Hopefully that answers your question :D
--
Brandon Williams