[GSoC,0/4] submodule: port 'summary' from Shell to C

Message

Shourya Shukla July 2, 2020, 7:24 p.m. UTC

Hello all!

This is third subcommand in line to be converted as a part of my GSoC
project. The other two were 'set-url' and 'set-branch' both of which
have been merged into Git. This port has been taken off from what
Prathamesh Chavan left as a part of his GSoC project in 2017 under
Stefan Beller and Christian Couder. As written in my proposal and
advised by my mentors Kaartic and Christian, it was decided to re-use
PC's work for multiple reasons.

Here is the link to the v1 of his patch:
https://lore.kernel.org/git/[e-mail]/

Here is the link to the v2 of his patch:
https://lore.kernel.org/git/[e-mail]/

Even though this patch series draws on Prathamesh' work, a lot extra
features had to be added along with some remodelling. A summary of the 4
commits in this patch are as follows: the patch consists of 3
preparatory patches preceding the main commit porting 'git submodule summary'.

Being a bit more verbose about the prepatory patches:

    1. a8dcc59a6a amends an extra linefeed between callback struct(s)
       macro(s) of subcommands 'init', 'status' and 'sync' so as to make
       the whole format of subcommands a bit more uniform and
       consistent.

    2. 5f17ca37e0 renames the helper functions
       'show_submodule_summary()', 'prepare_submodule_summary()' and
       'print_submodule_summary()' used by the 'builtin_diff()' function
       of diff.c This was done so as to avoid any ambiguity later on
       when the helper functions used by 'summary' will be defined in
       the upcoming port. The functions are renamed to
       '*_diff_submodule_summary()' so as to classify them as the one
       being used by the diff machinery.

    3. 5945ab4113 changes the scope of the 'count_lines()' function
       defined in diff.c so as to make it usable by other entities of
       the code. This was originally a patch authored by PC with a small
       change that the function instead of being defined as an 'extern
       int', it is now defined as 'int' since f758a7f8ac4 by Nguyễn Thái
       Ngọc Duy removed extern from the function declarations in diff.h

Now moving on to the main patch 4/4 that will port summary.

    17738e9df4 ports the subcommand summary from Shell to C. I will not
    be repetitive by stating how all the functions communicate since
    this is covered in the commit message. What I will focus on is the
    bits I had to tweak along with any extra functions added. Originally
    the functions were: module_summary(),
    compute_summary_submodule_list(), submodule_summary_callback(),
    prepare_submodule_summary(), generate_submodule_summary(),
    print_submodule_summary() and verify_submodule_object_name. I had to
    add another function called get_diff_cmd() which fetches a const
    char * (a diff-index or a diff-files) in response to the enum provided
    to it and bugs out in case of an unexpected enum value. I also had
    to tweak the enum declared by PC by NOT keeping it static and
    tweaking its definition just a little. These were suggested by
    Christian on PC's patch at the time.

    There were also some changes to function parameters such as those
    of index_fd() by passing the the_index parameter to it or the case
    of submodule_from_path() where it was needed to add another parameter
    of the_repository. This all must have been due to change of function
    defintions in the past 3 years.

    Even after all this, there were two problems: t7418 had been failing
    (something which I feel might have been overlooked/not tested by PC
    during his time) and the CI build failed. First let's cover the test
    failure.

    The main test for testing summary is t7401. t7418 is another test
    which involves sparse checkouts and submodules. The test creates a
    sparse checkout clone excluding the .gitmodules file to test the
    functionality of the '--for-status' option. After quite a thorough
    investigation me, Kaartic and Christian figured out that this port
    fails to handle cases where a gitfile exists instead of a .git dir
    since all the tests in t7401 did not any submodules which were
    cloned in any way but rather just exist in a nearby directory and
    are used in our tests (this is something I feel should be fixed and
    hence I am saving this for later). But anyway, we decided that it
    will be excellent to check for a non-bare repo instead of checking
    for a gitdir therefore replacing the function is_git_directory()
    with is_nonbare_repository() in the first non-nested if-check in
    generate_submodule_summary().

    Regarding the CI failure. The build was failing due to the
    function oidcmp() in the first if-check of
    generate_submodule_summary(). The CI build suggested that we use
    oideq() instead so that the build passes and on doing the change, it
    did!

    There were some other cosmetic changes such as wrapping of
    columns upto 80 chars (to follow the CodingGuidelines) and
    improving some error messages and option descriptions,

Finally, since it was originally PC's work, I have kept him as the author
for the final 2 commits and kept the date as-is (from 2017). This patch
is not perfect and hence needs the feedback from all of you :)

Here is a 'git log --oneline' of the commits:

    17738e9df4 submodule: port submodule subcommand 'summary' from shell to C
    5945ab4113 diff: change scope of the function count_lines()
    5f17ca37e0 submodule: rename helper functions to avoid ambiguity
    a8dcc59a6a submodule: amend extra line feed between callback struct and macro

Thanks,
Shourya Shukla


Prathamesh Chavan (2):
  diff: change scope of the function count_lines()
  submodule: port submodule subcommand 'summary' from shell to C

Shourya Shukla (2):
  submodule: amend extra line feed between callback struct and macro
  submodule: rename helper functions to avoid ambiguity

 builtin/submodule--helper.c | 454 +++++++++++++++++++++++++++++++++++-
 diff.c                      |   4 +-
 diff.h                      |   1 +
 git-submodule.sh            | 186 +--------------
 submodule.c                 |  10 +-
 submodule.h                 |   2 +-
 6 files changed, 461 insertions(+), 196 deletions(-)