Re: [GSoC][RFC][Proposal v4] Convert submodule to builtin

From: Christian Couder <christian.couder@gmail.com>
Date: Mar 24, 2020, 09:08 UTC
Message-ID: <CAP8UFD3BMEqt1+gGgGPHdMQ+BzY6Q84rGcp2UNK=WLusYQuy4g@mail.gmail.com>
In-Reply-To: <20200323171554.5254-1-shouryashukla.oo@gmail.com>

Hi,

On Mon, Mar 23, 2020 at 6:16 PM Shourya Shukla <shouryashukla.oo@gmail.com> wrote:

> Prathamesh implemented and improved the subcommands status[https://lore.kernel.org/git/20170713200538.25806-4-pc44800@gmail.com/], sync[https://lore.kernel.org/git/20180114211529.6391-2-pc44800@gmail.com/], deinit[https://lore.kernel.org/git/20180114211529.6391-3-pc44800@gmail.com/] and some more. The relevancy of this to my project is that some helper functions(located in submodule.c) such as print_submodule_summary(),prepare_submodule_summary(), etc. have been implemented beforehand. In the case of subcommand summary, use these functions, integrate them with the basic scaffolding(mentioned in the table below) and implement the module_summary() frontend function.

The last sentence above is not very clear to me. Do you mean "In the case of subcommand summary, I should use these functions, integrate them ..."? Or maybe "In the case of subcommand summary, the things left to do are to use these functions, integrate them ..."?

> He also ported various helper functions such as set_name_rev()[https://lore.kernel.org/git/20170619215025.10086-3-pc44800@gmail.com/]. He kept offering improvements to his conversions till around January of 2018.

Nice!

> Stefan Beller finished the implementation of the subcommand init[https://lore.kernel.org/git/1453418323-29587-1-git-send-email-sbeller@google.com/] as well as laid its foundation[https://lore.kernel.org/git/1453255396-31942-3-git-send-email-sbeller@google.com/]. He implemented foreach[https://lore.kernel.org/git/20180509002952.172347-5-sbeller@google.com/] and improved deinit[https://lore.kernel.org/git/20180327232824.112539-1-sbeller@google.com/] & update[https://lore.kernel.org/git/1444960333-16003-6-git-send-email-sbeller@google.com/] as well. He also ported various helper functions such as resolve_relative_url()[https://lore.kernel.org/git/1460767813-25916-2-git-send-email-sbeller@google.com/].

Nice!

Show 7 quoted lines

> #### Current Status of the subcommand and future vision
>
> The current status of the conversion as well as the direction I will take for the conversion of the subcommands are as follows:
>
>         add: pending conversion, full code needs to be written for the same. Need to implement callback macros and structures, i.e. struct add_cb,
>              ADD_CB_INIT, as well as frontend function module_add(). Other helper functions may be needed in the process as well. Compare with shell
>              script and try to “translate” it into C. I guesstimate around 400-500 lines of code for this(including helper functions).

Happy to see a guesstimation!

>         set-branch: pending conversion, full code needs to be written for the same. Need to implement macros and structures, i.e. struct setbranch,
>                     SETBRANCH_CB_INIT, as well as frontend function module_setbranch(). Other helper functions(such as remote_submodule_branch() &
>                     get_default_remote() which are already implemented may prove helpful later) may be needed in the process as well. Compare with shell
>                     script and try to “translate” it into C. This subcommand may take about 200 lines of C code to implement(including helper functions).

As it looks like it requires less lines of code, do you think it might be better to do this one before the 'add' sub-command?

Show 12 quoted lines

>         set-url: pending conversion, full code needs to be written for the same. Need to implement macros and structures, i.e. struct seturl,
>                  SETURL_CB_INIT, as well as frontend function module_seturl(). Other helper functions(such as remote_url() & resolve_resolve_url() which
>                  are already implemented may prove helpful later) may be needed in the process as well. Compare with shell script and try to “translate” it
>                  into C. It will have a similar implementation to set-branch because they are “setter” functions. This subcommand may take about 200 lines
>                  of C code to implement(including helper functions).
>
>         summary: pending conversion, work in progress; callback structures, functions and macros have been created, also, basic scaffolding of the command
>                  is done, i.e., functions module_summary(), summary_submodule(), summary_submodule_cb(). As this is a prototype, some functions may be scrapped
>                  or added later. Other functions to complement the subcommand have already been created; learn from Prathamesh's mistakes and implement a better code.
>                  After discussions with Junio C Hamano[https://lore.kernel.org/git/20200318163234.21628-1-shouryashukla.oo@gmail.com/T/#ma3912b761b6deda937691a19c1a070e5e9b34bd7],
>                  I intend to add a “--recursive” option as well for summary so as to obtain summaries of nested submodules as well.
>                  I estimate about 400 lines of code for this subcommand(excluding the  “--recursive” option, yet including the helper functions)

Nice!

> I aim to follow the same approach as Stefan and Prathamesh as mentioned above.

I am not sure that you talked much about their approach above.

> Though, there is about a 3 year gap between their work and mine, the model for porting seems to be consistent even if coding style may vary and might even give out improvements over previous implementations.

> #### Contribution process and interaction with the mentors
>
> I will keep committing changes on my GitHub fork[https://github.com/periperidip/git] and finally post a bundled patch on the Mailing List.

By "bundled patch" do you mean a patch series?

Show 11 quoted lines

> I will make sure to keep interacting with the community as well as the mentors regularly.    I aim to write weekly “progress report” blogs, which I will post on my website as well as the List. Apart from that, I will document anything new I learn as well as my journey in the GSoC program on my blogs and maybe as self-answered questions on StackOverflow with the aim that they will help me as well as others in case of reference.
>
>
> #### Project Timeline
>
> I have been studying the code of 'submodule.c', 'submodule--helper.c' and 'git-submodule.sh'
> since the submission of my microproject. After studying the codes, I tried to devise an effective
> conversion strategy for 'submodule'. I noticed that 'submodule.c' contains various helper functions
> for 'submodule--helper.c' whereas the latter houses the main "converted" command as of now.
>
> The subcommands ‘set-branch’ and ‘set-url’ will provide easy conversion due to the vast array of helper functions already available for them.

Nice that you found this!

> Hence, I have coupled them with other commands and tasks such as the conversion of ‘add’ and improving CLI parsing.

Hmm.

> In spite of the easiness of the aforementioned conversions, I have decided to convert ‘summary’ first because of the large variety of resources available for it

It is not very clear which resources you are talking about.

> as well as a direction of conversion available(as done by Prathamesh)

Do you mean that he already sent patches or wrote commits in his repo about that?

> which will aid me in learning from the mistakes committed before and thus help me offer an even more improved version of the subcommand.

Thanks, Christian.