Re: [GSoC][Draft Proposal v2] Finish converting git submodule to builtin

From: Christian Couder <christian.couder@gmail.com>
Date: Apr 10, 2021, 12:59 UTC
Message-ID: <CAP8UFD2hhtpnz+WE2J9iLbzfRJ2k5EOtUMRW=QcH9xe1U6y69g@mail.gmail.com>
In-Reply-To: <F3446051-620F-4C7B-A306-5C36922A976C@gmail.com>

On Thu, Apr 8, 2021 at 12:19 PM Atharva Raykar <raykar.ath@gmail.com> wrote:

Show 8 quoted lines

>
> Here's my updated draft. Changes since v1:
>
> - Elaborated more on example porting strategy, stating how the patches
>    could be broken up.
> - Made language at the end of section 6 less ambiguous.
> - Updated status of microproject.
> - s/git/Git in several places.

Thanks for this summary of the changes since the previous version!

Show 6 quoted lines

> 3 Me and Git
> ============
>
>   Here are the various forms of contributions that I have made to Git:
>
>   - [Microproject] userdiff: userdiff: add support for Scheme Status: In

s/userdiff: userdiff/userdiff/

Show 5 quoted lines

>     progress, patch v3 requiring a review List:
>     <https://lore.kernel.org/git/20210408091442.22740-1-raykar.ath@gmail.com/>
>
>   - [Git Education] Conducted a workshop with attendance of hundreds of
>     students new to git, and increased the prevalence of of git's usage

s/git/Git/ s/of of git/of Git/

>     in my campus.
>     Photos: <https://photos.app.goo.gl/T7CPk1zkHdK7mx6v7> and
>     <https://photos.app.goo.gl/bzTgdHMttxDen6z9A>

[...]

Show 18 quoted lines

> 6 General implementation strategy
> =================================
>
>   The way to port the shell to C code for `submodule' will largely
>   remain the same. There already exists the builtin
>   `submodule--helper.c' which contains most of the previous commands'
>   ports. All that the shell script for `git-submodule.sh' is doing for
>   the previously completed ports is parsing the flags and then calling
>   the helper, which does all the business logic.
>
>   So I will be moving out all the business logic that the shell script
>   is performing to `submodule--helper.c'. Any reusable functionality
>   that is introduced during the port will be added to `submodule.c' in
>   the top level.
>
>       For example: The general strategy for converting `cmd_update()' would
>       be to have a call to `submodule--helper' in the shell script to a
>       function which would resemble something like `module_update()'.

Does module_update() already exists? It's hard to understand if you are referring to something that already exists (where?) or that you would create (how?) here. More details about this would be nice.

> This
>       would perform the work being done by the shell script past the flags
>       being parsed and make the necessary call to `update_clone()', which
>       returns information about the cloned modules.

How does it return information?

Show 10 quoted lines

> For each cloned module,
>       it will find out the update mode through `module_update_mode()', and
>       run the appropriate operation according to that mode (like a rebase,
>       if that was the update mode).
>
>       One possible way this work can be broken up into multiple patches, is
>       by moving over the shell code into C in a bottom-up manner.
>       For example: The shell part which retrieves the latest revision in the
>       remote (if --remote is specified) can be wrapped into a command of
>       `submodule--helper.c'.

Could you give an example of how the command would be named, what arguments it would take and how it could be used?

> Then we can move the part where we run the
>       update method (ie the `case' on line 611 onwards) into a C function.

Do you mean the code that does something like:

                       case "$update_module" in
                       checkout)
                               ...
                       rebase)
                               ...
                       merge)
                               ...
                       !*)
                               ...
                       *)
                               ...
                       esac

                       if (sanitize_submodule_env; cd "$sm_path" &&
$command "$sha1")
                       then
                               say "$say_msg"
                       elif test -n "$must_die_on_failure"
                       then
                               die_with_status 2 "$die_msg"
                       else
                               err="${err};$die_msg"
                               continue
                       fi

Could you also give an example of how the command would be named, what arguments it would take and how it could be used?

Show 18 quoted lines

>       Eventually, the shell part will just look like a bunch of invocations
>       to `submodule--helper', at which point, the whole thing can be
>       encapsulated in a single command called `git submodule--helper update'
>       (Bonus: Move the whole functionality to C, including the parsing of
>       flags, to work towards getting rid of `git-submodule.sh'). I believe
>       this is a fairly non-destructive and incremental way to work, and the
>       porting efforts by Stefan seem to follow this same kind of philosophy.
>       I will most likely end up tuning the size of these increments when I
>       get around to planning in my first phase of the project.
>
>   After this process, I will be adding the `add' and `update' command to
>   the commands array in `submodule--helper.c'. And since these two
>   functions are the last bit of functionality left to convert in
>   submodules, an extended goal can be to get rid of the shell script
>   altogether, and make the helper into the actual builtin [1].
>
>   [1]
>   <https://lore.kernel.org/git/nycvar.QRO.7.76.6.2011191327320.56@tvgsbejvaqbjf.bet/>