Noisy warnings from submodule deinit on git >= 2.21.0 #3293
Comments
|
Thanks for the thorough report. I don't know much about git submodules, but the solution you're proposing seems reasonable, would you be able to create a PR? |
|
Hm. I've been searching, and I don't know if there's a good way to test whether a submodule has been initialized. There's a way to loop through all initialized submodules using diff --git a/lib/bundler/source/git/git_proxy.rb b/lib/bundler/source/git/git_proxy.rb
index 2a4d7138a..98bf591ac 100644
--- a/lib/bundler/source/git/git_proxy.rb
+++ b/lib/bundler/source/git/git_proxy.rb
@@ -146,7 +146,7 @@ module Bundler
if submodules
git_retry "submodule update --init --recursive"
elsif Gem::Version.create(version) >= Gem::Version.create("2.9.0")
- git_retry "submodule deinit --all --force"
+ git_retry "submodule foreach --quiet 'cd $toplevel; git submodule deinit --force $sm_path'"
end
end
endThis fixes the noisy warnings for me locally. But |
|
I think it should be fine, I think the specific command should be ok for all shells? Maybe we could do Regarding the test, I was writing down some tips for you but I was not sure whether I was in the right track, so I ended up writing the test myself :). It would be something like: diff --git a/spec/install/gemfile/git_spec.rb b/spec/install/gemfile/git_spec.rb
index 00f8e9662..a3b7def6c 100644
--- a/spec/install/gemfile/git_spec.rb
+++ b/spec/install/gemfile/git_spec.rb
@@ -845,6 +845,26 @@ RSpec.describe "bundle install with git sources" do
expect(the_bundle).not_to include_gems "has_submodule 1.0"
end
+ it "does not warn when deiniting submodules" do
+ build_git "submodule", "1.0"
+ build_git "has_submodule", "1.0"
+
+ Dir.chdir(lib_path("has_submodule-1.0")) do
+ sys_exec "git submodule add #{lib_path("submodule-1.0")} submodule-1.0"
+ `git commit -m "submodulator"`
+ end
+
+ install_gemfile <<-G
+ git "#{lib_path("has_submodule-1.0")}" do
+ gem "has_submodule"
+ end
+ G
+ expect(err).to be_empty
+
+ expect(the_bundle).to include_gems "has_submodule 1.0"
+ expect(the_bundle).to_not include_gems "submodule 1.0"
+ end
+ |
|
Hi @ajvondrak, are you still interested in fixing this or should we take it over? |
|
I transformed the discussion here into a PR: #3969. |
|
@deivid-rodriguez I know I'm very late to reply, but just wanted to say thanks so much for taking care of this. ❤️ If you couldn't tell by my radio silence, I was not in a place to focus on fixing this. 😅 |
Since Bundler v1.13.0, bundler does an automatic
git submodule deinit --all --force, per rubygems/bundler#4717. To this day, bundler still just fires thedeinitall the time when:submodulesisfalse: https://github.com/bundler/bundler/blob/f6045fcf93cd3076cdd3d5e15622927c87df2db5/lib/bundler/source/git/git_proxy.rb#L146-L150Git v2.21.0 started maintaining an invariant where the
core.worktreeconfiguration gets removed ongit submodule deinit, per the release notes. This was implemented by git/git@898c2e6:With the above implementation, though, git spits out a warning if the submodule has already been
deinit'd.Here's a minimal setup:
We
deinitonce, no problem:When we
deinitagain, git can't open the lock file, so it spits out warnings:What's more, the same warning is given if you never
init'd in the first place - like in the freshgit clone(sans--recurse-submodules) that Bundler does:Note: the fact that the "repo" example doesn't have a gemspec is immaterial to the git behavior. I encountered this error on a git-based gem (which had submodules, even though I still opted for
submodules: false) that installed just fine despite issuing the abovedeinitwarning aboutcore.worktree.Running the above with some more verbose tracing:
Solution: I think that Bundler could probably do something to avoid the
git submodule deinit --all --forceif there are no submodules in the cloned repo anyway. This keeps Git from issuing confusing warnings on newer versions.The text was updated successfully, but these errors were encountered: