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

Re: Interactive rebase with submodules

From
JKJohn Keeping <john@metanate.com>
Date
Jan 19, 2012, 10:48 UTC
Message-ID
<4F17F505.5030204@metanate.com>
In-Reply-To
<4F172DBB.4080303@web.de>
On 18/01/12 20:38, Jens Lehmann wrote:
Show 29 quoted lines
> Am 18.01.2012 12:21, schrieb John Keeping:
>> On 17/01/12 21:29, Jens Lehmann wrote:
>>> Am 17.01.2012 19:47, schrieb John Keeping:
>>>> I can understand not updating submodules while running the rebase, but I expected that having resolved a conflict and added my change to the index it would be applied by `git rebase --continue`, as indeed it is if there happen to be other (non-submodule) changes in the same commit.
>>>
>>> The irony is that you would have to update submodules (or at least
>>> their HEAD and use "--ignore-submodules=dirty") while running rebase
>>> to make that work in all cases ;-)
>>
>> I don't this this is the case, since diff-tree is being invoked with --cached won't it ignore changes in the work tree anyway?
>
> Right, thanks for nudging me with the clue bat ...
>
> I missed the "--cached" option and did not question if the code does
> what the commit message of 6848d58c6 (where the --ignore-submodules
> option was introduced) said:
>
>      Ignore dirty submodule states during rebase and stash
>
>      When rebasing or stashing, chances are that you do not care about
>      dirty submodules, since they are not updated by those actions anyway.
>      So ignore the submodules' states.
>
>      Note: the submodule states -- as committed in the superproject --
>      will still be stashed and rebased, it is _just_ the state of the
>      submodule in the working tree which is ignored.
>
> I think this logic misses the case when only submodule changes are left
> in a commit.
Yes, this is precisely the case that doesn't work as I expect.
Show 31 quoted lines
>>> But just updating the HEAD would be dangerous as you would have to be
>>> very careful to restore the submodules HEAD after the rebase, or the
>>> submodule's work tree will be out of sync.
>>
>> Just updating HEAD in the submodule without touching its work tree doesn't seem like a good idea.  I think it will cause a lot more confusion when running `git status` which will show unexpected modified content for the submodule.
>
> Yes, we agree here.
>
>> Since I did not expect rebase to perform a submodule update, I was not surprised to see unstaged submodule changes when rebasing, but I did expect rebase to commit anything I had added to the index.
>
> Right.
>
> I'll have to add some tests for that case, but I doubt I'll manage that
> today. Until I can provide a complete patch, this diff should fix your
> problem (no, I did not test if that change is enough to fix the problem,
> but at least it does not break the test suite ;-):
>
> ---------------8<--------------
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index 5812222..4546749 100644
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -672,7 +672,7 @@ rearrange_squash () {
>   case "$action" in
>   continue)
>          # do we have anything to commit?
> -       if git diff-index --cached --quiet --ignore-submodules HEAD --
> +       if git diff-index --cached --quiet HEAD --
>          then
>                  : Nothing to commit -- skip this
>          else
This patch does indeed fix the problem I was seeing.
Thanks,
-- 
John
Previous: Jens Lehmann
Message 5 of 5 in “Interactive rebase with submodules”
  1. John KeepingJan 17, 2012
  2. Jens LehmannJan 17, 2012
  3. John KeepingJan 18, 2012
  4. Jens LehmannJan 18, 2012
  5. John KeepingJan 19, 2012

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.