* [PATCH 0/2] recursive submodules: paths are hard
@ 2016-02-26 20:51 Stefan Beller
2016-02-26 20:51 ` [PATCH 1/2] Check recursive submodule update to have correct path from subdirectory Stefan Beller
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Stefan Beller @ 2016-02-26 20:51 UTC (permalink / raw)
To: gitster, git; +Cc: jacob.e.keller, Jens.Lehmann, Stefan Beller
I thought getting rid of the extra `prefix` argument in submodule helper
functions was easy up to the point of all but one test passing in the test
suite.
It turned out the implementation of that prefix patch was wrong, only caught
by tests, so we want to add tests for subtle details with submodule sync and
update which have not been tests yet.
For the record, I thought the 'prefix' patch was as easy as:
- git submodule--helper list --prefix "$wt_prefix" "$@" | {
+ git -C "$wt_prefix" submodule--helper list "$@" | {
for all occurrences of `submodule--helper list`.
This is not the case as by doing so the recursive functionality of submodules
is broken in some edge cases.
Consider this sequence:
mkdir untracked &&
cd untracked &&
git submodule <command> --recursive
The operation is run from within the work tree, so fro a normal submodule
operation (without --recursive) you expect the pathes to be adapted to be
prefixed by a `../` to make sense relative to the untracked directory.
In the case of recursive submodule operations, currently `git submodule`
usually does
if test -n "$recursive"
then
(
prefix="$prefix$sm_path/"
clear_local_git_env
cd "$sm_path" &&
eval cmd_update
)
By having a change of directory followed by the recursive call to the operation
we need to make sure the displayed path is still correctly referenced to where
the operation started.
By passing the prefix separately to git submodule--helper, this works currently
as the prefix is only used for calculating the displaypath. If it were passed
by the standard Git machinery, there is going on more, which
fails us at some point.
I think we may need to enable Git to pass in 'negative' pathes for the prefix,
i.e.
Although operating on this repository, your reference for displaying paths
should be '../untracked' for the example above, when the submodule is in the
root directory of the superproject.
This seems currently not possible with the standard way to pass down the prefix.
TL;DR: Most of the test is unrelated to the patch series, the patch series
adds some tests, which I would have found useful to stop me going the wrong
direction.
Thanks,
Stefan
Stefan Beller (2):
Check recursive submodule update to have correct path from
subdirectory
submodule sync: Test we can pass individual submodules
t/t7403-submodule-sync.sh | 13 +++++++++----
t/t7406-submodule-update.sh | 12 ++++++++++++
2 files changed, 21 insertions(+), 4 deletions(-)
--
2.7.2.368.g934fe14
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 1/2] Check recursive submodule update to have correct path from subdirectory
2016-02-26 20:51 [PATCH 0/2] recursive submodules: paths are hard Stefan Beller
@ 2016-02-26 20:51 ` Stefan Beller
2016-02-26 20:51 ` [PATCH 2/2] submodule sync: Test we can pass individual submodules Stefan Beller
2016-02-27 1:58 ` [PATCH 0/2] recursive submodules: paths are hard Duy Nguyen
2 siblings, 0 replies; 4+ messages in thread
From: Stefan Beller @ 2016-02-26 20:51 UTC (permalink / raw)
To: gitster, git; +Cc: jacob.e.keller, Jens.Lehmann, Stefan Beller
A similar test exists for `submodule sync` to behave well when being in
an unrelated subdirectory and performing operations on submodules.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
t/t7406-submodule-update.sh | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index 68ea31d..628da7f 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -774,4 +774,16 @@ test_expect_success 'submodule update --recursive drops module name before recur
test_i18ngrep "Submodule path .deeper/submodule/subsubmodule.: checked out" actual
)
'
+
+test_expect_success 'submodule update --recursive works from subdirectory' '
+ (cd super2 &&
+ (cd deeper/submodule/subsubmodule &&
+ git checkout HEAD^
+ ) &&
+ mkdir untracked &&
+ cd untracked &&
+ git submodule update --recursive >actual &&
+ test_i18ngrep "Submodule path .../deeper/submodule/subsubmodule.: checked out" actual
+ )
+'
test_done
--
2.7.2.368.g934fe14
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/2] submodule sync: Test we can pass individual submodules
2016-02-26 20:51 [PATCH 0/2] recursive submodules: paths are hard Stefan Beller
2016-02-26 20:51 ` [PATCH 1/2] Check recursive submodule update to have correct path from subdirectory Stefan Beller
@ 2016-02-26 20:51 ` Stefan Beller
2016-02-27 1:58 ` [PATCH 0/2] recursive submodules: paths are hard Duy Nguyen
2 siblings, 0 replies; 4+ messages in thread
From: Stefan Beller @ 2016-02-26 20:51 UTC (permalink / raw)
To: gitster, git; +Cc: jacob.e.keller, Jens.Lehmann, Stefan Beller
The parent patch made changes to the way `git submodule--helper list`
is called. From experience this is a sensitive topic and lots subtle things
can go wrong. As all submodule subcommands except `sync` are setup to
run `git submodule--helper list` in the original directory, I suspected
a possible breakage in `sync` not being able to specify an exact submodule
to run in, so let's add a test for that. Instead of adding a complete new
test we can modify an existing test to additionally test the additional
assertion of having just one submodule work fine.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
t/t7403-submodule-sync.sh | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/t/t7403-submodule-sync.sh b/t/t7403-submodule-sync.sh
index 79bc135..5dde123 100755
--- a/t/t7403-submodule-sync.sh
+++ b/t/t7403-submodule-sync.sh
@@ -28,6 +28,9 @@ test_expect_success setup '
git submodule add ../submodule submodule &&
test_tick &&
git commit -m "submodule"
+ git submodule add ../submodule submodule2 &&
+ test_tick &&
+ git commit -m "second submodule"
) &&
git clone super super-clone &&
(
@@ -149,15 +152,16 @@ test_expect_success 'reset submodule URLs' '
reset_submodule_urls super-clone
'
-test_expect_success '"git submodule sync" should update submodule URLs - subdirectory' '
+test_expect_success '"git submodule sync" should update specified submodule URLs - subdirectory' '
(
cd super-clone &&
git pull --no-recurse-submodules &&
mkdir -p sub &&
cd sub &&
- git submodule sync >../../output
+ git submodule sync ../submodule >../../output
) &&
grep "\\.\\./submodule" output &&
+ ! grep submodule2 output &&
test -d "$(
cd super-clone/submodule &&
git config remote.origin.url
@@ -177,7 +181,7 @@ test_expect_success '"git submodule sync" should update submodule URLs - subdire
)
'
-test_expect_success '"git submodule sync --recursive" should update all submodule URLs - subdirectory' '
+test_expect_success '"git submodule sync --recursive" should update all specified submodule URLs - subdirectory' '
(
cd super-clone &&
(
@@ -186,9 +190,10 @@ test_expect_success '"git submodule sync --recursive" should update all submodul
) &&
mkdir -p sub &&
cd sub &&
- git submodule sync --recursive >../../output
+ git submodule sync --recursive ../submodule >../../output
) &&
grep "\\.\\./submodule/sub-submodule" output &&
+ ! grep submodule2 output &&
test -d "$(
cd super-clone/submodule &&
git config remote.origin.url
--
2.7.2.368.g934fe14
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 0/2] recursive submodules: paths are hard
2016-02-26 20:51 [PATCH 0/2] recursive submodules: paths are hard Stefan Beller
2016-02-26 20:51 ` [PATCH 1/2] Check recursive submodule update to have correct path from subdirectory Stefan Beller
2016-02-26 20:51 ` [PATCH 2/2] submodule sync: Test we can pass individual submodules Stefan Beller
@ 2016-02-27 1:58 ` Duy Nguyen
2 siblings, 0 replies; 4+ messages in thread
From: Duy Nguyen @ 2016-02-27 1:58 UTC (permalink / raw)
To: Stefan Beller
Cc: Junio C Hamano, Git Mailing List, Jacob Keller, Jens Lehmann
On Sat, Feb 27, 2016 at 3:51 AM, Stefan Beller <sbeller@google.com> wrote:
> I think we may need to enable Git to pass in 'negative' pathes for the prefix,
> i.e.
> Although operating on this repository, your reference for displaying paths
> should be '../untracked' for the example above, when the submodule is in the
> root directory of the superproject.
>
> This seems currently not possible with the standard way to pass down the prefix.
The problem is, prefix is meant for inside worktree area only. Many
code paths are not ready for prefix "../". Worse, in some cases when
you launch outside worktree, prefix is empty and I think some builtin
commands rely on that. However, I think you can introduce "prefix v2"
that is used in parallel with "prefix v1".
--
Duy
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-02-27 1:59 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-26 20:51 [PATCH 0/2] recursive submodules: paths are hard Stefan Beller
2016-02-26 20:51 ` [PATCH 1/2] Check recursive submodule update to have correct path from subdirectory Stefan Beller
2016-02-26 20:51 ` [PATCH 2/2] submodule sync: Test we can pass individual submodules Stefan Beller
2016-02-27 1:58 ` [PATCH 0/2] recursive submodules: paths are hard Duy Nguyen
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).