[CMake] ctest & git submodules

Marcus D. Hanwell marcus.hanwell at kitware.com
Tue Feb 26 19:06:11 EST 2013


On Tue, Feb 26, 2013 at 4:50 PM, Brad King <brad.king at kitware.com> wrote:
> On 2/26/2013 2:52 PM, Jean-Christophe Fillion-Robin wrote:
>> +1 to add these into CTest :) What would be the argument against it ?
>
> Not every project wants every submodule checked out all the time.
> A major use case for them is to have an umbrella project with many
> submodules and the developer may only checkout and work on some.
> Some may even be proprietary and inaccessible to some machines.

Agreed, there are different uses for submodules but it seems like we
are also neglecting a common use case.
>
>>     > However, it seems to me that ctest already does a
>>     > git submodule update --recurse
>
> This is the proper command to update everything that is already
> to configured to checkout in the source tree.
>
>>     > but its missing the --init flag to deal with changes to the
>>     > .gitmodules file.
>>     It also misses git submodule sync to deal with changes in git
>>     submodule URL,
>
> These are all intentionally missing for the above reason.  We
> should honor the user's configuration.  Maybe they intentionally
> use a custom url for a submodule for the branch they test.  We
> should not blow away their configuration by default.
>
>> and reset --hard etc.
>
> We do a reset --hard at the top level but I do not think we
> do it in the submodules.  That may be worth adding, perhaps
> with git submodule foreach.

This would be very helpful, and is a big inconsistency in the current behavior.
>
>>     We have just been dealing with a
>>     few of these issues and currently call git directly to sync, init, and
>>     then use submodule foreach to reset and clean all submodules before
>>     updating.
>
> That is the expected way to deal with it.  The local dashboard
> script knows if it needs to preserve the user config or not.
>
> We could also consider adding options for ctest_update to
> tell it to init, sync, etc., but it should not be the default.
>
I think adding an option would be totally fine, keep the current
default. It can be difficult getting the order right when scripting
and adding a few options to the update command could make the scripts
quite a bit simpler. If this seems like a reasonable path forward
maybe we could sketch out what those options should look like and we
might be able to put a patch together.

Our most common dashboard use case just wants a pristine clone, using
the latest submodule URLs and ideally should clean out any local
changes. We seem to spend quite a bit of time ensuring this all
happens in the right order, as I think others do.

Marcus


More information about the CMake mailing list