Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
phil-blain opened this issue Oct 12, 2020 · 5 comments
Open

Comments

@phil-blain
Copy link

phil-blain commented Oct 12, 2020

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
Copy link

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
Copy link

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
Copy link
Author

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

@periperidip
Copy link

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
Copy link
Author

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. ..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants