Re: [RFC PATCH] git-submodule.sh:cmd_update: if submodule branch exists, fetch that instead of default
2018-03-06 22:21 GMT+02:00 Jonathan Nieder : > > (cc list snipped) > Hi, > > Eddy Petrișor wrote: > > > Cc: [a lot of people] > > Can you say a little about how this cc list was created? E.g. should > "git send-email" get a feature to warn about long cc lists? I did it as advised by the documentation, git blame: https://github.com/git/git/blob/master/Documentation/SubmittingPatches#L264 I was suprised that there is no specialized script that does this, as it is for the kernel or u-boot, so I ran first git log --pretty=format:'%an <%ae>' git-submodule.sh | sort -u > mail-list-submodule then configure git to use that custom output and ignore the patch since it was trying to convert every line of it into a email address: git config sendemail.ccCmd 'cat mail-list-submodule # ' Then "git send-email 0001..." did the rest. > > > Signed-off-by: Eddy Petrișor > > --- > > > > There are projects such as llvm/clang which use several repositories, and > > they > > might be forked for providing support for various features such as adding > > Redox > > awareness to the toolchain. This typically means the superproject will use > > another branch than master, occasionally even use an old commit from that > > non-master branch. > > > > Combined with the fact that when incorporating such a hierachy of > > repositories > > usually the user is interested in just the exact commit specified in the > > submodule info, it follows that a desireable usecase is to be also able to > > provide '--depth 1' to avoid waiting for ages for the clone operation to > > finish. > > Some previous discussion is at > https://public-inbox.org/git/CAGZ79ka6UXKyVLmdLg_M5-sB1x96g8FRzRZy=eny5ajbqf9...@mail.gmail.com/. > > In theory this should be straightforward: Git protocol allows fetching > an arbitrary commit, so "git submodule update" and similar commands > could fetch the submodule commit by SHA-1 instead of by refname. Poof! > Problem gone. > > In practice, some complications: > > - some servers do not permit fetch-by-sha1. For example, github does >not permit it. This is governed by the >uploadpack.allowReachableSHA1InWant / uploadpack.allowAnySHA1InWant >configuration items. That is the problem I have faced since I tried to clone this repo which has at lest 2 levels of submodules: https://github.com/redox-os/redox The problematic modules are: rust @ https://github.com/redox-os/rust/tree/81c2bf4e51647295d3d92952dbb0464b460df0c3 - first level and rust/src/llvm @ https://github.com/rust-lang/llvm/tree/ba2edd794c7def715007931fcd1b4ce62aa711c8 > >That should be surmountable by making the behavior conditional, but >it's a complication. Which forced me to try to fetch a branch on which that commit exists, but also consider providing --depth for the submodule checkout, effectively minimizing the amount of brought in history. > > > - When the user passes --depth=, do they mean that to apply to >the superproject, to the submodules, or both? Documentation should >make the behavior clear. The supermodule clone already exists and that works OK; I was after providing something like 'git submodule update --depth 20 --recursive' or evn providing that 'depth' info via the .gitmodules parameters since 'shallow' is already used somehow, yet that is a bool value, not an integer, like depth expects: eddy@feodora:~/usr/src/redox/rust-eddy$ git config -f .gitmodules --list | egrep '(depth|shallow)' submodule.src/llvm.shallow=true submodule.src/rt/hoedown.shallow=true submodule.src/jemalloc.shallow=true submodule.src/liblibc.shallow=true submodule.src/doc/nomicon.shallow=true submodule.src/tools/cargo.shallow=true submodule.src/doc/reference.shallow=true submodule.src/doc/book.shallow=true submodule.src/tools/rls.shallow=true submodule.src/libcompiler_builtins.shallow=true submodule.src/tools/clippy.shallow=true submodule.src/tools/rustfmt.shallow=true submodule.src/tools/miri.shallow=true submodule.src/dlmalloc.shallow=true submodule.src/binaryen.shallow=true submodule.src/doc/rust-by-example.shallow=true submodule.src/llvm-emscripten.shallow=true submodule.src/tools/rust-installer.shallow=true > > Git submodule seems to be very stubborn and cloning master, although the > > wrapper script and the gitmodules-helper could work together to clone > > directly > > the branch specified in the .gitmodules file, if specified. > > This could make sense. For the same reason as --depth in the > superproject gives ambiguous signals about what should happen in > submodules, --single-branch in the superproject gives ambiguous > signals abo
Re: [RFC PATCH] git-submodule.sh:cmd_update: if submodule branch exists, fetch that instead of default
2018-03-08 21:29 GMT+02:00 Eddy Petrișor : > 2018-03-06 22:21 GMT+02:00 Jonathan Nieder : >> >> (cc list snipped) >> Hi, >> >> Eddy Petrișor wrote: >> >> > Cc: [a lot of people] >> >> Can you say a little about how this cc list was created? E.g. should >> "git send-email" get a feature to warn about long cc lists? > > > I did it as advised by the documentation, git blame: > > https://github.com/git/git/blob/master/Documentation/SubmittingPatches#L264 > > I was suprised that there is no specialized script that does this, as > it is for the kernel or u-boot, so I ran first > > git log --pretty=format:'%an <%ae>' git-submodule.sh | sort -u > > mail-list-submodule > > then configure git to use that custom output and ignore the patch > since it was trying to convert every line of it into a email address: > > git config sendemail.ccCmd 'cat mail-list-submodule # ' > > Then "git send-email 0001..." did the rest. > > >> >> > Signed-off-by: Eddy Petrișor >> > --- >> > >> > There are projects such as llvm/clang which use several repositories, and >> > they >> > might be forked for providing support for various features such as adding >> > Redox >> > awareness to the toolchain. This typically means the superproject will use >> > another branch than master, occasionally even use an old commit from that >> > non-master branch. >> > >> > Combined with the fact that when incorporating such a hierachy of >> > repositories >> > usually the user is interested in just the exact commit specified in the >> > submodule info, it follows that a desireable usecase is to be also able to >> > provide '--depth 1' to avoid waiting for ages for the clone operation to >> > finish. >> >> Some previous discussion is at >> https://public-inbox.org/git/CAGZ79ka6UXKyVLmdLg_M5-sB1x96g8FRzRZy=eny5ajbqf9...@mail.gmail.com/. >> >> In theory this should be straightforward: Git protocol allows fetching >> an arbitrary commit, so "git submodule update" and similar commands >> could fetch the submodule commit by SHA-1 instead of by refname. Poof! >> Problem gone. >> >> In practice, some complications: >> >> - some servers do not permit fetch-by-sha1. For example, github does >>not permit it. This is governed by the >>uploadpack.allowReachableSHA1InWant / uploadpack.allowAnySHA1InWant >>configuration items. > > > That is the problem I have faced since I tried to clone this repo > which has at lest 2 levels of submodules: > https://github.com/redox-os/redox > > The problematic modules are: > rust @ > https://github.com/redox-os/rust/tree/81c2bf4e51647295d3d92952dbb0464b460df0c3 > - first level > > and > > rust/src/llvm @ > https://github.com/rust-lang/llvm/tree/ba2edd794c7def715007931fcd1b4ce62aa711c8 > > >> >>That should be surmountable by making the behavior conditional, but >>it's a complication. > > > Which forced me to try to fetch a branch on which that commit exists, > but also consider providing --depth for the submodule checkout, > effectively minimizing the amount of brought in history. > >> >> >> - When the user passes --depth=, do they mean that to apply to >>the superproject, to the submodules, or both? Documentation should >>make the behavior clear. > > > The supermodule clone already exists and that works OK; I was after > providing something like 'git submodule update --depth 20 --recursive' > or evn providing that 'depth' info via the .gitmodules parameters > since 'shallow' is already used somehow, yet that is a bool value, not > an integer, like depth expects: > > > eddy@feodora:~/usr/src/redox/rust-eddy$ git config -f .gitmodules > --list | egrep '(depth|shallow)' > submodule.src/llvm.shallow=true > submodule.src/rt/hoedown.shallow=true > submodule.src/jemalloc.shallow=true > submodule.src/liblibc.shallow=true > submodule.src/doc/nomicon.shallow=true > submodule.src/tools/cargo.shallow=true > submodule.src/doc/reference.shallow=true > submodule.src/doc/book.shallow=true > submodule.src/tools/rls.shallow=true > submodule.src/libcompiler_builtins.shallow=true > submodule.src/tools/clippy.shallow=true > submodule.src/tools/rustfmt.shallow=true > submodule.src/tools/miri.shallow=true > submodule.src/dlmalloc.shallow=true > submodule.src/binaryen.shallow=true > submodule.src/doc/rust-by-example.shallow=true >
Is context/directory sensitive config possible or even a god idea?
Hello, In some situations one person could be involved in various multi-repository projects and need to use different user.email information, depending on the project. A simple example can be using the project specific email identity for all repositories part of Android, while using the personal email for personal projects on github. The developer has currently only 2 options, AFAICT: a) use --global identity for the personal email and set the Android identity in all project repos b) use the reverse, store Android identity in --global gitconfig, and make sure to set the identity at repo level for personal projects. Both solutions are suboptimal, especially if both the project in question has many repositories and the number github projects is high, because the user must remember to set the non-global user identity either in all github repos or in each Android repos, after checkout. In my view, there should be a middle ground, having a context/directory dependent setting could be a solution. My idea is somehting like "for all repos cloned in subdirs of directory ~/src/android, use identity johm@android.org", so the only thing to remember to make this work is to make sure all android checkouts are done under ~/src/android, which most people already do because they need a clean home dir. When I saw he include.path feature I thought this was exactly that, but when testing it did not work as I would have liked, the inluded config is simply added to the global config, if I add this to the ~/.gitconfig file and the target file exists. I understand the use case I am thinking of is not the one that was addressed by the include.file feature, so I am thinking git would need some other setting like 'contextsensitive.include=/home/johndoe/src/android/.gitconfig' which could mean include the settings from this config is we're in a dir on the same level a the file, or a directory which has that dir as parent or ancestor. What I see already: 1) it would be a perfomance hit if the test for "do we need to apply any context sensitive setting" must be done on every git command; for the identity usecase might be less of a problem if only the subcomands that need user.* would compare the prefix of the current dir with all all contextsensitive.inludes settings and applying only the longest one 2) what if the contextsensitive.inluclude file also includes another contextsensitive.include, could that lead to ambiguous situations? 3) having the feature could allow the user to set other project specific settings. Nice usecases: if Android uses --no-ff merge.ff=false in ~/src/andoid/.gitconfig contextsensitive.inlclude would ensure the expected strategy is used, and rebase is the preferred behaviour for pull, instead of merge) -- Eddy Petrișor
Fwd: [RFC PATCH] git-submodule.sh:cmd_update: if submodule branch exists, fetch that instead of default
mar., 27 mar. 2018, 02:07 Stefan Beller a scris:
>
> [snipped the cc list as well]
>
> On Tue, Mar 6, 2018 at 12:06 PM Eddy Petrișor
> wrote:
>
> > Signed-off-by: Eddy Petrișor
> > ---
>
> Did this go anywhere?
> (I just came back from a longer vacation, sorry for the delay on my site)
Not really. I am still unsure how is best to proceed. Details below.
> > There are projects such as llvm/clang which use several repositories, and
> they
> > might be forked for providing support for various features such as adding
> Redox
> > awareness to the toolchain. This typically means the superproject will use
> > another branch than master, occasionally even use an old commit from that
> > non-master branch.
>
> > Combined with the fact that when incorporating such a hierachy of
> repositories
> > usually the user is interested in just the exact commit specified in the
> > submodule info, it follows that a desireable usecase is to be also able to
> > provide '--depth 1' to avoid waiting for ages for the clone operation to
> > finish.
>
> Very sensible.
The only change is that I realized that hard coding the depth is not
necessary because the client can fetch more and more from the branch
until the commit hash is found or the entire history was fetched and
it wasn't found.
This is more robust but has a variable performance penalty and is
probably slower than single branch fetching from the start.
> > Git submodule seems to be very stubborn and cloning master, although the
> > wrapper script and the gitmodules-helper could work together to clone
> directly
> > the branch specified in the .gitmodules file, if specified.
>
> Also very sensible.
>
> So far so good, could you move these paragraphs before the triple dashed
> line
> and sign off so we record it as the commit message?
Sure, as long as the implementation and design makes sense.
> > Another wrinkle is that when the commit is not the tip of the branch, the
> depth
> > parameter should somehow be stored in the .gitmodules info, but any
> change in
> > the submodule will break the supermodule submodule depth info sooner or
> later,
> > which is definitly frigile.
>
> ... which is why I would not include that.
>
> git-fetch knows about --shallow-since or even better
> shallow-exclude which could be set to the (depth+1)-th commit
> (the boundary commit) recorded in the shallow information.
I am unsure what that means. Without yet looking in the docs, would
this --shallow-since be better than the try-until-found algorithm
explained above?
> > I tried digging into this section of the code and debugging with bashdb
> to see
> > where --depth might fit, but I got stuck on the shell-to-helper
> interaction and
> > the details of the submodule implementation, so I want to lay out this
> first
> > patch as starting point for the discussion in the hope somebody else
> picks it
> > up or can provide some inputs. I have the feeling there are multiple code
> paths
> > that are being ran, depending on the moment (initial clone, submodule
> > recursive, post-clone update etc.) and I have a gut feeling there
> shouldn't be
> > any code duplication just because the operation is different.
>
> > This first patch is only trying to use a non-master branch, I have some
> changes
> > for the --depth part, but I stopped working on it due to the "default
> depth"
> > issue above.
>
> > Does any of this sound reasonable?
> > Is this patch idea usable or did I managed to touch the part of the code
> that
> > should not be touched?
>
> This sounds reasonable. Thanks for writing the patch!
OK. Now I need to make it good, which is the hard part :)
> > diff --git a/git-submodule.sh b/git-submodule.sh
> > index 2491496..370f19e 100755
> > --- a/git-submodule.sh
> > +++ b/git-submodule.sh
> > @@ -589,8 +589,11 @@ cmd_update()
> > branch=$(git submodule--helper remote-branch
> "$sm_path")
> > if test -z "$nofetch"
> > then
> > + # non-default branch
> > + rbranch=$(git config -f .gitmodules
> submodule.$sm_path.branch)
> > +
> br_refspec=${rbanch:+"refs/heads/$rbranch:refs/heads/$rbranch"}
>
> Wouldn't we want to fetch into a remote tracking branch instead?
> Instead of computing all this by yourself, these two lines could be
>
> br_refspec=$(git submodule--helper remote-branch $sm_path)
>
> I would think.
I wasn't aware of this,
Re: [RFC PATCH v2] git-submodule.sh:cmd_update: if submodule branch exists, fetch that instead of default
(+list) 2018-03-30 1:55 GMT+03:00 : > From: Eddy Petrișor > > There are projects such as llvm/clang which use several repositories, and they > might be forked for providing support for various features such as adding > Redox > awareness to the toolchain. This typically means the superproject will use > another branch than master, occasionally even use an old commit from that > non-master branch. > > Combined with the fact that when incorporating such a hierachy of repositories > usually the user is interested in just the exact commit specified in the > submodule info, it follows that a desireable usecase is to be also able to > provide '--depth 1' or at least have a shallow clone to avoid waiting for ages > for the clone operation to finish. > > In theory, this should be straightforward since the git protocol allows > fetching an arbitary commit, but, in practice, some servers do not permit > fetch-by-sha1. > > Git submodule seems to be very stubborn and cloning master, although the > wrapper script and the gitmodules-helper could work together to clone directly > the branch specified in the .gitmodules file, if specified. > > Signed-off-by: Eddy Petrișor > --- > git-submodule.sh | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/git-submodule.sh b/git-submodule.sh > index 24914963c..65e3af08b 100755 > --- a/git-submodule.sh > +++ b/git-submodule.sh > @@ -589,8 +589,10 @@ cmd_update() > branch=$(git submodule--helper remote-branch > "$sm_path") > if test -z "$nofetch" > then > + # non-default branch refspec > + br_refspec=$(git submodule-helper > remote-branch $sm_path) > # Fetch remote before determining tracking > $sha1 > - fetch_in_submodule "$sm_path" $depth || > + fetch_in_submodule "$sm_path" $depth > $br_refspec || > die "$(eval_gettext "Unable to fetch in > submodule path '\$sm_path'")" > fi > remote_name=$(sanitize_submodule_env; cd "$sm_path" > && get_default_remote) > -- > 2.16.2 > I am planning to write a test case in the next few days, between the few drops of free time I have. I am still trying to figure out my way through the test code. -- Eddy Petrișor
[RFC PATCH v3 2/2] t7406: add test for non-default branch in submodule
From: Eddy Petrișor If a submodule uses a non-default branch and the branch info is versioned, on submodule update --recursive --init the correct branch should be checked out. Signed-off-by: Eddy Petrișor --- t/t7406-submodule-update.sh | 54 + 1 file changed, 54 insertions(+) diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh index 6f083c4d6..7b65f1dd1 100755 --- a/t/t7406-submodule-update.sh +++ b/t/t7406-submodule-update.sh @@ -259,6 +259,60 @@ test_expect_success 'submodule update --remote should fetch upstream changes wit ) ' +test_expect_success 'submodule update --remote --recursive --init should fetch module branch from .gitmodules' ' + git clone . super5 && + git clone super5 submodl2b2 && + git clone super5 submodl1b1 && + cd submodl2b2 && + echo linel2b2 > l2b2 && + git checkout -b b2 && + git add l2b2 && + test_tick && + git commit -m "commit on b2 branch in l2" && + git rev-parse --verify HEAD >../expectl2 && + git checkout master && + cd ../submodl1b1 && + git checkout -b b1 && + echo linel1b1 > l1b1 && + git add l1b1 && + test_tick && + git commit -m "commit on b1 branch in l1" && + git submodule add ../submodl2b2 submodl2b2 && + git config -f .gitmodules submodule."submodl2b2".branch b2 && + git add .gitmodules && + test_tick && + git commit -m "add l2 module with branch b2 in l1 module in branch b1" && + git submodule init submodl2b2 && + git rev-parse --verify HEAD >../expectl1 && + git checkout master && + cd ../super5 && + echo super_with_2_chained_modules > super5 && + git add super5 && + test_tick && + git commit -m "commit on default branch in super5" && + git submodule add ../submodl1b1 submodl1b1 && + git config -f .gitmodules submodule."submodl1b1".branch b1 && + git add .gitmodules && + test_tick && + git commit -m "add l1 module with branch b1 in super5" && + git submodule init submodl1b1 && + git clone super5 super && + ( + cd super && + git submodule update --recursive --init + ) && + ( + cd submodl1b1 && + git rev-parse --verify HEAD >../../actuall1 && + test_cmp ../../expectl1 ../../actuall1 + ) && + ( + cd submodl2b2 && + git rev-parse --verify HEAD >../../../actuall2 && + test_cmp ../../../expectl2 ../../../actuall2 + ) +' + test_expect_success 'local config should override .gitmodules branch' ' (cd submodule && git checkout test-branch && -- 2.16.2
[RFC PATCH v3 1/2] git-submodule.sh:cmd_update: if submodule branch exists, fetch that instead of default
From: Eddy Petrișor There are projects such as llvm/clang which use several repositories, and they might be forked for providing support for various features such as adding Redox awareness to the toolchain. This typically means the superproject will use another branch than master, occasionally even use an old commit from that non-master branch. Combined with the fact that when incorporating such a hierachy of repositories usually the user is interested in just the exact commit specified in the submodule info, it follows that a desireable usecase is to be also able to provide '--depth 1' or at least have a shallow clone to avoid waiting for ages for the clone operation to finish. In theory, this should be straightforward since the git protocol allows fetching an arbitary commit, but, in practice, some servers do not permit fetch-by-sha1. Git submodule seems to be very stubborn and cloning master, although the wrapper script and the gitmodules-helper could work together to clone directly the branch specified in the .gitmodules file, if specified. Signed-off-by: Eddy Petrișor --- git-submodule.sh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/git-submodule.sh b/git-submodule.sh index 24914963c..65e3af08b 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -589,8 +589,10 @@ cmd_update() branch=$(git submodule--helper remote-branch "$sm_path") if test -z "$nofetch" then + # non-default branch refspec + br_refspec=$(git submodule-helper remote-branch $sm_path) # Fetch remote before determining tracking $sha1 - fetch_in_submodule "$sm_path" $depth || + fetch_in_submodule "$sm_path" $depth $br_refspec || die "$(eval_gettext "Unable to fetch in submodule path '\$sm_path'")" fi remote_name=$(sanitize_submodule_env; cd "$sm_path" && get_default_remote) -- 2.16.2
Re: [RFC PATCH v3 2/2] t7406: add test for non-default branch in submodule
Notes: I am aware this test is not probably the best one and maybe I should have first one test that does a one level non-default, before trying a test with 2 levels of submodules, but I wanted to express the goal of the patch. Currently the test fails, so I am obviously missing something. Help would be appreciated. 2018-04-04 1:20 GMT+03:00 Eddy Petrișor : > From: Eddy Petrișor > > If a submodule uses a non-default branch and the branch info is versioned, on > submodule update --recursive --init the correct branch should be checked out. > > Signed-off-by: Eddy Petrișor > --- > t/t7406-submodule-update.sh | 54 > + > 1 file changed, 54 insertions(+) > > diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh > index 6f083c4d6..7b65f1dd1 100755 > --- a/t/t7406-submodule-update.sh > +++ b/t/t7406-submodule-update.sh > @@ -259,6 +259,60 @@ test_expect_success 'submodule update --remote should > fetch upstream changes wit > ) > ' > > +test_expect_success 'submodule update --remote --recursive --init should > fetch module branch from .gitmodules' ' > + git clone . super5 && > + git clone super5 submodl2b2 && > + git clone super5 submodl1b1 && > + cd submodl2b2 && > + echo linel2b2 > l2b2 && > + git checkout -b b2 && > + git add l2b2 && > + test_tick && > + git commit -m "commit on b2 branch in l2" && > + git rev-parse --verify HEAD >../expectl2 && > + git checkout master && > + cd ../submodl1b1 && > + git checkout -b b1 && > + echo linel1b1 > l1b1 && > + git add l1b1 && > + test_tick && > + git commit -m "commit on b1 branch in l1" && > + git submodule add ../submodl2b2 submodl2b2 && > + git config -f .gitmodules submodule."submodl2b2".branch b2 && > + git add .gitmodules && > + test_tick && > + git commit -m "add l2 module with branch b2 in l1 module in branch > b1" && > + git submodule init submodl2b2 && > + git rev-parse --verify HEAD >../expectl1 && > + git checkout master && > + cd ../super5 && > + echo super_with_2_chained_modules > super5 && > + git add super5 && > + test_tick && > + git commit -m "commit on default branch in super5" && > + git submodule add ../submodl1b1 submodl1b1 && > + git config -f .gitmodules submodule."submodl1b1".branch b1 && > + git add .gitmodules && > + test_tick && > + git commit -m "add l1 module with branch b1 in super5" && > + git submodule init submodl1b1 && > + git clone super5 super && > + ( > + cd super && > + git submodule update --recursive --init > + ) && > + ( > + cd submodl1b1 && > + git rev-parse --verify HEAD >../../actuall1 && > + test_cmp ../../expectl1 ../../actuall1 > + ) && > + ( > + cd submodl2b2 && > + git rev-parse --verify HEAD >../../../actuall2 && > + test_cmp ../../../expectl2 ../../../actuall2 > + ) > +' > + > test_expect_success 'local config should override .gitmodules branch' ' > (cd submodule && > git checkout test-branch && > -- > 2.16.2 > -- Eddy Petrișor
Re: [RFC PATCH v3 2/2] t7406: add test for non-default branch in submodule
2018-04-04 21:36 GMT+03:00 Stefan Beller : > On Tue, Apr 3, 2018 at 3:26 PM, Eddy Petrișor wrote: >> Notes: >> I am aware this test is not probably the best one and maybe I >> should have first one test that does a one level non-default, before >> trying a test with 2 levels of submodules, but I wanted to express the >> goal of the patch. > > This patch only contains the test, I presume this goes on top of > https://public-inbox.org/git/20180403222053.23132-1-eddy.petri...@codeaurora.org/ > which you plan to later submit as one patch including both the change as well > as > the test. Yes, not sure why the emails were not linked together, I specified the in-reply-to mesage id when I "git format-patch"-ed the patches. I wasn't actually planning to squash them in a single patch, will do, but I guess for now it helps to focus the discussion around the test since the implementation is still lacking, see below 2 lines comment. >> Currently the test fails, so I am obviously missing something. >> Help would be appreciated. >> >> >> 2018-04-04 1:20 GMT+03:00 Eddy Petrișor : >>> From: Eddy Petrișor [..] >>> --- a/t/t7406-submodule-update.sh >>> +++ b/t/t7406-submodule-update.sh >>> @@ -259,6 +259,60 @@ test_expect_success 'submodule update --remote should >>> fetch upstream changes wit >>> ) >>> ' >>> >>> +test_expect_success 'submodule update --remote --recursive --init should >>> fetch module branch from .gitmodules' ' >>> + git clone . super5 && > > I found adding "test_pause &&" > to be a great helper as it will drop you in a shell where > you can inspect the repository. Thanks for the pointer, I only looked at the post-failure state after adding --debug -i --verbose, but having "test_pause" to stop and inspect the interim stage should help a lot with debugging. > >>> + git clone super5 submodl2b2 && >>> + git clone super5 submodl1b1 && > > We may want to cleanup after the test is done: > > test_when_finished "rm submodl2b2" && > test_when_finished "rm submodl1b2" && Sure, will do. >>> + cd submodl2b2 && > > I'd think the test suite prefers subshells to operate in other dirs > instead of direct cd's, just like at the end of the test. After looking at other tests I was under the impression that the setup phase (e.g. creating the "server" side repos) of the test was done in the main context, while the test case (i.e. the client side, or what the user would do) is in subshells. > For short parts, we make heavy use of the -C option, > So for example the code below >( >cd super && >git submodule update --recursive --init >) && > > can be written as > > git -C super submodule update --recursive --init > > which is shorter. Nice. >>> + echo linel2b2 > l2b2 && > > style: The Git test suite prefers to have the redirect adjacent to the > file name: > echo hello >world I wasn't aware of that, it seems there are some inconsistencies, including in the modified test: eddy@feodora:~/usr/src/git/t$ grep '> ' -c t* 2>/dev/null | grep -v ':0$' | grep 7406 t7406-submodule-update.sh:24 eddy@feodora:~/usr/src/git/t$ grep '> ' -c t* 2>/dev/null | grep -v ':0$' | wc -l 325 I suspect that a minor patch correcting these inconsistencies would be faily fast reviewed adn accepted, of course, disconnected from this modification. >>> + git checkout -b b2 && >>> + git add l2b2 && >>> + test_tick && >>> + git commit -m "commit on b2 branch in l2" && >>> + git rev-parse --verify HEAD >../expectl2 && > > So until now we made a commit in a submodule on branch b2 > and wrote it out to an expect file. Yes, I was trying to replicate the redox-os case which has this structure: redox-os (master branch) + rust (@some commit on a non-default) + src/llvm (@some commit on a non-default branch) This section is making sure the b2 branch has other content than the default one and setting the expectation for level2 submodule branch, later. >>> + git checkout master && >>> + cd ../submodl1b1 && >>> + git checkout -b b1 && >>> + echo linel1b1 > l1b1 && >>> + git add l1b1 && >>> + test_tick && >&g
[RFC PATCH v4 9/9] fixup:t7406:change branches in submodules after the link is done
From: Eddy Petrișor Signed-off-by: Eddy Petrișor --- t/t7406-submodule-update.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh index f44872143..68c25ce0f 100755 --- a/t/t7406-submodule-update.sh +++ b/t/t7406-submodule-update.sh @@ -267,17 +267,16 @@ test_expect_success 'submodule update --remote --recursive --init should fetch m git checkout -b b2 && test_commit "l2_on_b2" && git rev-parse --verify HEAD >../expectl2 && - git checkout master && cd ../submodl1b1 && git checkout -b b1 && test_commit "l1_on_b1" && git submodule add ../submodl2b2 submodl2b2 && git config -f .gitmodules submodule."submodl2b2".branch b2 && git add .gitmodules && + git -C ../submodl2b2 checkout master && test_tick && git commit -m "add l2 (on b2) in l1 (on b1)" && git rev-parse --verify HEAD >../expectl1 && - git checkout master && cd ../super5 && test_commit super5_commit_before_2_chained_modules_on_default_branch && git submodule add ../submodl1b1 submodl1b1 && @@ -285,6 +284,7 @@ test_expect_success 'submodule update --remote --recursive --init should fetch m git add .gitmodules && test_tick && git commit -m "add l1 module with branch b1 in super5" && + git -C ../submodl1b1 checkout master && git submodule init submodl1b1 && cd .. && git clone super5 super_w && -- 2.16.2
[RFC PATCH v4 1/9] git-submodule.sh:cmd_update: if submodule branch exists, fetch that instead of default
From: Eddy Petrișor There are projects such as llvm/clang which use several repositories, and they might be forked for providing support for various features such as adding Redox awareness to the toolchain. This typically means the superproject will use another branch than master, occasionally even use an old commit from that non-master branch. Combined with the fact that when incorporating such a hierachy of repositories usually the user is interested in just the exact commit specified in the submodule info, it follows that a desireable usecase is to be also able to provide '--depth 1' or at least have a shallow clone to avoid waiting for ages for the clone operation to finish. In theory, this should be straightforward since the git protocol allows fetching an arbitary commit, but, in practice, some servers do not permit fetch-by-sha1. Git submodule seems to be very stubborn and cloning master, although the wrapper script and the gitmodules-helper could work together to clone directly the branch specified in the .gitmodules file, if specified. Signed-off-by: Eddy Petrișor --- git-submodule.sh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/git-submodule.sh b/git-submodule.sh index 24914963c..65e3af08b 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -589,8 +589,10 @@ cmd_update() branch=$(git submodule--helper remote-branch "$sm_path") if test -z "$nofetch" then + # non-default branch refspec + br_refspec=$(git submodule-helper remote-branch $sm_path) # Fetch remote before determining tracking $sha1 - fetch_in_submodule "$sm_path" $depth || + fetch_in_submodule "$sm_path" $depth $br_refspec || die "$(eval_gettext "Unable to fetch in submodule path '\$sm_path'")" fi remote_name=$(sanitize_submodule_env; cd "$sm_path" && get_default_remote) -- 2.16.2
[RFC PATCH v4 3/9] fixup:t7406: use test_commit instead of echo/add/commit as suggested by Stefan Beller
From: Eddy Petrișor Signed-off-by: Eddy Petrișor --- t/t7406-submodule-update.sh | 15 +++ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh index 7b65f1dd1..7fb370991 100755 --- a/t/t7406-submodule-update.sh +++ b/t/t7406-submodule-update.sh @@ -264,19 +264,13 @@ test_expect_success 'submodule update --remote --recursive --init should fetch m git clone super5 submodl2b2 && git clone super5 submodl1b1 && cd submodl2b2 && - echo linel2b2 > l2b2 && git checkout -b b2 && - git add l2b2 && - test_tick && - git commit -m "commit on b2 branch in l2" && + test_commit "l2_on_b2" && git rev-parse --verify HEAD >../expectl2 && git checkout master && cd ../submodl1b1 && git checkout -b b1 && - echo linel1b1 > l1b1 && - git add l1b1 && - test_tick && - git commit -m "commit on b1 branch in l1" && + test_commit "l1_on_b1" && git submodule add ../submodl2b2 submodl2b2 && git config -f .gitmodules submodule."submodl2b2".branch b2 && git add .gitmodules && @@ -286,10 +280,7 @@ test_expect_success 'submodule update --remote --recursive --init should fetch m git rev-parse --verify HEAD >../expectl1 && git checkout master && cd ../super5 && - echo super_with_2_chained_modules > super5 && - git add super5 && - test_tick && - git commit -m "commit on default branch in super5" && + test_commit super5_with_2_chained_modules_on_default_branch && git submodule add ../submodl1b1 submodl1b1 && git config -f .gitmodules submodule."submodl1b1".branch b1 && git add .gitmodules && -- 2.16.2
[RFC PATCH v4 7/9] fixup:t7406:supr5 commit is done before submodules chaining
From: Eddy Petrișor Signed-off-by: Eddy Petrișor --- t/t7406-submodule-update.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh index 32995e272..18328be73 100755 --- a/t/t7406-submodule-update.sh +++ b/t/t7406-submodule-update.sh @@ -279,7 +279,7 @@ test_expect_success 'submodule update --remote --recursive --init should fetch m git rev-parse --verify HEAD >../expectl1 && git checkout master && cd ../super5 && - test_commit super5_with_2_chained_modules_on_default_branch && + test_commit super5_commit_before_2_chained_modules_on_default_branch && git submodule add ../submodl1b1 submodl1b1 && git config -f .gitmodules submodule."submodl1b1".branch b1 && git add .gitmodules && -- 2.16.2
[RFC PATCH v4 5/9] fixup:t7406:cleanup chained submodules after test is done
From: Eddy Petrișor Signed-off-by: Eddy Petrișor --- t/t7406-submodule-update.sh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh index 974f949df..c5b412c46 100755 --- a/t/t7406-submodule-update.sh +++ b/t/t7406-submodule-update.sh @@ -298,7 +298,9 @@ test_expect_success 'submodule update --remote --recursive --init should fetch m cd submodl2b2 && git rev-parse --verify HEAD >../../../actuall2 && test_cmp ../../../expectl2 ../../../actuall2 - ) + ) && + test_when_finished "rm submodl2b2" && + test_when_finished "rm submodl1b1" ' test_expect_success 'local config should override .gitmodules branch' ' -- 2.16.2
[RFC PATCH v4 8/9] fixup:t7406:use super_w instead of the existing super
From: Eddy Petrișor Signed-off-by: Eddy Petrișor --- t/t7406-submodule-update.sh | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh index 18328be73..f44872143 100755 --- a/t/t7406-submodule-update.sh +++ b/t/t7406-submodule-update.sh @@ -286,10 +286,11 @@ test_expect_success 'submodule update --remote --recursive --init should fetch m test_tick && git commit -m "add l1 module with branch b1 in super5" && git submodule init submodl1b1 && - git clone super5 super && - git -C super submodule update --recursive --init && + cd .. && + git clone super5 super_w && + git -C super_w submodule update --recursive --init && ( - cd submodl1b1 && + cd super_w/submodl1b1 && git rev-parse --verify HEAD >../../actuall1 && test_cmp ../../expectl1 ../../actuall1 ) && @@ -298,6 +299,7 @@ test_expect_success 'submodule update --remote --recursive --init should fetch m git rev-parse --verify HEAD >../../../actuall2 && test_cmp ../../../expectl2 ../../../actuall2 ) && + test_when_finished "rm super_w" && test_when_finished "rm submodl2b2" && test_when_finished "rm submodl1b1" ' -- 2.16.2
[RFC PATCH v4 2/9] t7406: add test for non-default branch in submodule
From: Eddy Petrișor If a submodule uses a non-default branch and the branch info is versioned, on submodule update --recursive --init the correct branch should be checked out. Signed-off-by: Eddy Petrișor --- t/t7406-submodule-update.sh | 54 + 1 file changed, 54 insertions(+) diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh index 6f083c4d6..7b65f1dd1 100755 --- a/t/t7406-submodule-update.sh +++ b/t/t7406-submodule-update.sh @@ -259,6 +259,60 @@ test_expect_success 'submodule update --remote should fetch upstream changes wit ) ' +test_expect_success 'submodule update --remote --recursive --init should fetch module branch from .gitmodules' ' + git clone . super5 && + git clone super5 submodl2b2 && + git clone super5 submodl1b1 && + cd submodl2b2 && + echo linel2b2 > l2b2 && + git checkout -b b2 && + git add l2b2 && + test_tick && + git commit -m "commit on b2 branch in l2" && + git rev-parse --verify HEAD >../expectl2 && + git checkout master && + cd ../submodl1b1 && + git checkout -b b1 && + echo linel1b1 > l1b1 && + git add l1b1 && + test_tick && + git commit -m "commit on b1 branch in l1" && + git submodule add ../submodl2b2 submodl2b2 && + git config -f .gitmodules submodule."submodl2b2".branch b2 && + git add .gitmodules && + test_tick && + git commit -m "add l2 module with branch b2 in l1 module in branch b1" && + git submodule init submodl2b2 && + git rev-parse --verify HEAD >../expectl1 && + git checkout master && + cd ../super5 && + echo super_with_2_chained_modules > super5 && + git add super5 && + test_tick && + git commit -m "commit on default branch in super5" && + git submodule add ../submodl1b1 submodl1b1 && + git config -f .gitmodules submodule."submodl1b1".branch b1 && + git add .gitmodules && + test_tick && + git commit -m "add l1 module with branch b1 in super5" && + git submodule init submodl1b1 && + git clone super5 super && + ( + cd super && + git submodule update --recursive --init + ) && + ( + cd submodl1b1 && + git rev-parse --verify HEAD >../../actuall1 && + test_cmp ../../expectl1 ../../actuall1 + ) && + ( + cd submodl2b2 && + git rev-parse --verify HEAD >../../../actuall2 && + test_cmp ../../../expectl2 ../../../actuall2 + ) +' + test_expect_success 'local config should override .gitmodules branch' ' (cd submodule && git checkout test-branch && -- 2.16.2
[RFC PATCH v4 6/9] fixup:t7406:don't call init after add, is redundant
From: Eddy Petrișor Signed-off-by: Eddy Petrișor --- t/t7406-submodule-update.sh | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh index c5b412c46..32995e272 100755 --- a/t/t7406-submodule-update.sh +++ b/t/t7406-submodule-update.sh @@ -275,8 +275,7 @@ test_expect_success 'submodule update --remote --recursive --init should fetch m git config -f .gitmodules submodule."submodl2b2".branch b2 && git add .gitmodules && test_tick && - git commit -m "add l2 module with branch b2 in l1 module in branch b1" && - git submodule init submodl2b2 && + git commit -m "add l2 (on b2) in l1 (on b1)" && git rev-parse --verify HEAD >../expectl1 && git checkout master && cd ../super5 && -- 2.16.2
[RFC PATCH v4 4/9] fixup:t7404:use 'git -C' instead of cd .. && git
From: Eddy Petrișor Signed-off-by: Eddy Petrișor --- t/t7406-submodule-update.sh | 5 + 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh index 7fb370991..974f949df 100755 --- a/t/t7406-submodule-update.sh +++ b/t/t7406-submodule-update.sh @@ -288,10 +288,7 @@ test_expect_success 'submodule update --remote --recursive --init should fetch m git commit -m "add l1 module with branch b1 in super5" && git submodule init submodl1b1 && git clone super5 super && - ( - cd super && - git submodule update --recursive --init - ) && + git -C super submodule update --recursive --init && ( cd submodl1b1 && git rev-parse --verify HEAD >../../actuall1 && -- 2.16.2
Re: [RFC PATCH v4 1/9] git-submodule.sh:cmd_update: if submodule branch exists, fetch that instead of default
2018-04-19 2:53 GMT+03:00 Stefan Beller : > Hi Eddy, > > all the following patches 3-9 are touching the test as added in patch > 2, which would go best with this patch. > Could you squash all commits into one? Yes, I did not have time yesterday to put all changes into a single commit with an associated note (because note managment seems to be a huge pain), so I preferred small commits. But I wanted to get your feedback on something, I'll reply in thread arm where you actually suspected the problem. > There are a couple ways to do it: > > git reset --soft > git commit -a --reuse-commit-message=<...> > > or using > > git rebase --interactive origin/master > # and then marking all but the first as "fixup" I am aware of git rebase -i and use it regularly, that's why patches 3-9 have the 'fixup' prefix. > I think the end result looks good, but that is best reviewed as one > piece instead of 9 patches. -- Eddy Petrișor
Re: [RFC PATCH v3 2/2] t7406: add test for non-default branch in submodule
2018-04-05 0:41 GMT+03:00 Stefan Beller :
> On Wed, Apr 4, 2018 at 1:37 PM, Eddy Petrișor wrote:
>
>>> you plan to later submit as one patch including both the change as well as
>>> the test.
>>
>> Yes,
I did not forget about having a single patch. Will do it once the
details are ironed out.
>>>>> + cd ../super5 &&
>>>>> + echo super_with_2_chained_modules > super5 &&
>>>>> + git add super5 &&
>>>>> + test_tick &&
>>>>> + git commit -m "commit on default branch in super5" &&
>>>>> + git submodule add ../submodl1b1 submodl1b1 &&
>>>>> + git config -f .gitmodules submodule."submodl1b1".branch b1 &&
>>>>> + git add .gitmodules &&
>>>>> + test_tick &&
>>>>> + git commit -m "add l1 module with branch b1 in super5" &&
>>>
>>> now we do this again without the nested submodule, just one repo
>>> as a submodule?
>>
>> My intention was to have super5 -> submodl1b1@b1 -> submodl2b2@b2 on
>> the "server" side.
>> But are you saying I just implemented super5 -> sbmodl1b1@master due
>> to the previous master checkout in submodl1b1?
>
> No. I was a little confused about the code.
Actually you were 100% correct. In order to link to submodl1b1@b1 I
had to move the master branch checkout after the submobl2@b2 is added.
Otherwise the submodule is added with the last commit on master, not
the last one on b1 an b2, respectively.
I suspect that in the tests, because the "server side" repos are
local, the git fetch-by-sha1/cloning by hash will be done correctly,
without the need of a branch hint, but the problem will still exist
for servers such as github which do not support fetch-by-sha1.
In case I realize that a server-side repo that doesn't support
fetch-by-sha1 is needed, is there a mechanism to set that up in the
test case, or do I have to rethink my approach?
>>>>> + git submodule init submodl1b1 &&
>>>>> + git clone super5 super &&
>>>
>>> does super exist here already? (I did not check, but IIRC
>>> super and super{1-4} are there as we count upwards to
>>> find a name that is ok.
>>
>> I created it in the first step of the test with the intention to have
>> super5 as the "server" and "super" as the client clone.
>
> oh, ok.
After using test_pause I realized 'super' is left over by some other
test cases, so in my v4 (unjustifibly) long series I switch to using
super_w because I was getting all sorts of issues and wanted to not
interfere with the other tests.
>> As a general idea for a test, does it look sane?
>
> Yes, I think it is a sane approach. Thanks for writing such a test!
OK, thanks for the feedback.
>> Do you think I should I start with a just one level of submodule with
>> a non-default branch (super -> l1@b1), or it this OK?
>> In my view, having 2 levels makes sure the recursive part is also
>> addressed and verified.
>
> I totally agree.
--
Eddy Petrișor
