gitgitgadget / git

GitGitGadget's Git fork. Open Pull Requests here to submit them to the Git mailing list
https://gitgitgadget.github.io/
Other
216 stars 134 forks source link

`git rm --cached <submodule>` does not stage changes to `.gitmodules` #750

Open phil-blain opened 4 years ago

phil-blain commented 4 years ago

Details here : https://lore.kernel.org/git/ea91c2ea29064079914f6a522db5115a@UUSALE0Z.utcmail.com/T/#u

This is indeed looks like a bug (in the sense that it does not follow what git-rm(1) describes).

Reproducer:

mkdir some_submodule
cd some_submodule
git init
echo hello > hello.txt
git add hello.txt
git commit -m 'First commit of submodule'
cd ..
mkdir top_repo
cd top_repo
git init
echo world > world.txt
git add world.txt
git commit -m 'First commit of top repo'
git submodule add ../some_submodule
git status  # both some_submodule and .gitmodules staged
git commit -m 'Added submodule'
git rm --cached some_submodule
git status  # only some_submodule staged
periperidip commented 3 years ago

Is anyone working on this issue @phil-blain ? Haven't been active on Git for a while thought might as well take this up to get back in the game.

periperidip commented 3 years ago

Also, another update on this scenario:

test_expect_success 'rm --cached leaves work tree of populated submodules and .gitmodules alone' '
    git reset --hard &&
    git submodule update &&
    git rm --cached submod &&
    test_path_is_dir submod &&
    test_path_is_file submod/.git &&
    git status -s -uno >actual &&
    test_cmp expect.cached actual &&
    git config -f .gitmodules submodule.sub.url &&
    git config -f .gitmodules submodule.sub.path

The above test 45 from the script t3600-rm.sh asserts that a git rm --cached <submodule pathspec> will not touch the .gitmodule(s) of the respective SMs. Something which will need correction as well.

phil-blain commented 3 years ago

I don’t think anybody is working on that specific issue, no.

periperidip commented 3 years ago

Should we lock this issue or something since it is very inconclusive as of now? I would love to work on this but I really don't know if this would be approved on the list or not. What do you say Phillipe?

phil-blain commented 3 years ago

Should we lock this issue or something since it is very inconclusive as of now?

I do not have the rights to lock issues here, and anyway, I do not think anyone should be locking issues here unless abusive behaviour is going on. What should be done is to link to the relevant messages on the list, so if people want to contribute to the discussion they can do so on the list: https://lore.kernel.org/git/20210207144144.GA42182@konoha/

 I would love to work on this but I really don't know if this would be approved on the list or not. What do you say Phillipe?

We never really know how patch series will be received by the reviewers on the list. If you feel that this change is good and should make its way into Git, then go ahead and code it up (which I see you did here) and see what comes up of it. ..