git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v6] submodule: absorb git dir instead of dying on deinit

From
Mugdha Pattnaik <mugdhapattnaik@gmail.com>
Date
Nov 17, 2021, 05:55 UTC
Message-ID
<CAA4dvxixLKAQeLTUnLFvnwrAzQg2w0zqH7gCUqYLXjf=SAP-wA@mail.gmail.com>
In-Reply-To
<xmqqy25o9bie.fsf@gitster.g>
On Wed, Nov 17, 2021 at 12:24 AM Junio C Hamano <gitster@pobox.com> wrote:
Show 34 quoted lines
>
> Mugdha Pattnaik <mugdhapattnaik@gmail.com> writes:
>
> >> OK.  That sounds like an improvement, albeit possibly an overly
> >> cautious one, as a casual "deinit" user will get an error as before
> >> without "--force", which may or may not be a good thing.  Requiring
> >> "--force" means it is safer by default by not changing the on-disk
> >> data.  But requiring "--force" also means we end up training users
> >> to say "--force" when it shouldn't have to be.
> >> ...
> >> Does "git submodule" currently reject a "deinit" request due to some
> >> _other_ conditions or safety concerns and require the "--force"
> >> option to continue?  Requiring the "--force" option to resolve ".git
> >> is a directory, and the user wants to make it absorbed" means that
> >> the user will be _forced_ to bypass these _other_ safety valves only
> >> to save the submodule repository from destruction when running
> >> "deinit", which may not be a good trade-off between the safety
> >> requirements of these _other_ conditions, if exists, and the one we
> >> are dealing with.
> >
> > This is definitely a situation we want to avoid. How about we try to run
> > a check for uncommitted local modifications first?
>
> I am not sure if I follow.  If we stop (ab)using "--force" for the
> situation (i.e. where today's "deinit" would die because .git needs
> to be absorbed first), then the user would not have to say "--force"
> which may override other safety valve.  You'd check if .git needs to
> be absorbed, make it absorbed as needed while reporting the fact
> that you did so, and then let the existing "deinit" code to take
> over.  If there are other safety checks that needs "--force" to be
> overridden, that is handled (presumably) correctly by the existing
> code, no?  So other than "do we need absorbing, and if so do it for
> the user" check, I do not think you'd need to add any new "we try to
> run a check for ..." at all.

Yes, I understand why "--force" should not be used. The reason why I suggested the check for local modifications is because I thought we should warn users that they have local modifications before we absorb the gitdir. But I see now that this is okay, considering deinit would die in such a situation anyway, and users would not lose their work. The only side-effect of running deinit despite users having local modifications, would be that the gitdir of the submodule has been absorbed and that should be okay.

-- 
Mugdha
Previous: Junio C HamanoNext: Mugdha Pattnaik via GitGitGadget
Message 15 of 18 in “submodule: absorb git dir instead of dying on deinit”
  1. submodule: absorb git dir instead of dying on deinitMugdha Pattnaik via GitGitGadget, Aug 26, 2021
  2. submodule: absorb git dir instead of dying on deinitMugdha Pattnaik via GitGitGadget, Aug 27, 2021
  3. Atharva RaykarAug 27, 2021
  4. Junio C HamanoAug 27, 2021
  5. submodule: absorb git dir instead of dying on deinitMugdha Pattnaik via GitGitGadget, Aug 27, 2021
  6. submodule: absorb git dir instead of dying on deinitMugdha Pattnaik via GitGitGadget, Aug 27, 2021
  7. Christian CouderSep 28, 2021
  8. Ævar Arnfjörð BjarmasonSep 28, 2021
  9. Mugdha PattnaikOct 6, 2021
  10. submodule: absorb git dir instead of dying on deinitMugdha Pattnaik via GitGitGadget, Oct 6, 2021
  11. submodule: absorb git dir instead of dying on deinitMugdha Pattnaik via GitGitGadget, Oct 6, 2021
  12. Junio C HamanoOct 7, 2021
  13. Mugdha PattnaikNov 16, 2021
  14. Junio C HamanoNov 16, 2021
  15. Mugdha PattnaikNov 17, 2021
  16. submodule: absorb git dir instead of dying on deinitMugdha Pattnaik via GitGitGadget, Nov 19, 2021
  17. Bagas SanjayaAug 27, 2021
  18. Mugdha PattnaikAug 27, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.