Re: [RFC PATCHv2 08/17] update submodules: add depopulate_submodule

From: Stefan Beller <sbeller@google.com>
Date: Dec 6, 2016, 00:18 UTC
Message-ID: <CAGZ79kaXqOjT_Jd_Q=GkDegbFJjoEQoxHJ-wvvCTkjALndBwFg@mail.gmail.com>
In-Reply-To: <832dcc3eec0d4237a1e2766e8df690ee@exmbdft7.ad.twosigma.com>

On Mon, Dec 5, 2016 at 3:37 PM, David Turner <David.Turner@twosigma.com> wrote:

Show 14 quoted lines

>> -----Original Message-----
>> From: Stefan Beller [mailto:sbeller@google.com]
>> Sent: Friday, December 02, 2016 7:30 PM
>> To: bmwill@google.com; David Turner
>> Cc: git@vger.kernel.org; sandals@crustytoothpaste.net; hvoigt@hvoigt.net;
>> gitster@pobox.com; Stefan Beller
>> Subject: [RFC PATCHv2 08/17] update submodules: add depopulate_submodule
>>
>> Implement the functionality needed to enable work tree manipulating
>> commands so that a deleted submodule should not only affect the index
>> (leaving all the files of the submodule in the work tree) but also to
>> remove the work tree of the superproject (including any untracked files).
>
> "including any untracked files" bothers me, I think.  Checkout is not usually willing to overwrite untracked files; it seems odd to me that it would be willing to do so in the submodule case.  I would be OK if they were both untracked and gitignored, I think.

I agree on being bothered, this is one of the things I thought how to solve. See the test in "checkout: recurse into submodules if asked to", which tests for untracked files and is still marked as a failure.

I think to address that issue, I'll add a flag to ok_to_remove_submodule which let's you specify which files you care about and which you don't.

Show 11 quoted lines

>> +                     warning(_("Cannot remove submodule '%s'\n"
>> +                               "because it (or one of its nested submodules)
>> has a git \n"
>> +                               "directory in the working tree, which could not
>> be embedded\n"
>> +                               "the superprojects git directory
>> automatically."), path);
>
> What if instead it couldn't run the command because you're out of file descriptors or pids or memory or something?
>
> I think this message should be in submodule--helper --embed-git-dirs instead, and we should just pass it through here.  Or, perhaps, instead of shelling out here, we should just call the functions directly?

heh, good point. Will call the function directly.