[PATCH v2 0/3] submodule: port subcommand add from shell to C

Hello all,

This is the v2 of the patch with the same title, delivered more than a month ago as a part of my GSoC. Link to v1: https://lore.kernel.org/git/20200824090359.403944-1-shouryashukla.oo@gmail.com/

The changelog is as follows:

    1. Introduce PATCH[1/3](dir: change the scope of function
       'directory_exists_in_index()', 2020-10-06). This was done since
       the above mentioned function will be used in the patch that
       follows.

    2. There are multiple changes in this commit:

            A. Improve the part which checks if the 'path' given as
               argument exists or not. Implementing Kaartic's
               suggestions on the patch, I had to make sure that the
               case for checking if the path has tracked contents or
               not also works.

            B. Also, wrap the aforementioned segment in a function
               since it became very long. The function is called
               'check_sm_exists()'.

            C. Also, use the function 'is_nonbare_repository_dir()'
               instead of 'is_directory()' when trying to resolve
               gitlink.

            D. Append keyword 'fatal' in front of the expected output of
               test t7400.6 since the command die()s out in case of
               absence of commits in a submodule.

            E. Remove the extra `#include "dir.h"` from
               'submodule--helper.c'.

    3. Introduce PATCH[3/3] (t7400: add test to check 'submodule add'
       for tracked paths, 2020-10-07). Kaartic pointed out that a test
       for path with tracked contents did not exist and hence it was
       necessary to write one. Therefore, this commit introduces a new
       test 't7400.18: submodule add to path with tracked contents
       fails'.

Comments and feedback are appreciated. Sorry for the month long delay, I was on a vacation.

I am attaching a range-diff between v1 and v2 at the end of this mail.

Regards, Shourya Shukla -----

-:  ---------- > 1:  bdac00494e dir: change the scope of function 'directory_exists_in_index()'
1:  b08d81e179 ! 2:  3e20d0fe04 submodule: port submodule subcommand 'add' from shell to C
    @@ Commit message
         'git-submodule.sh'.

         Also, since the command die()s out in case of absence of commits in the
    -    submodule and exits with exit status 1 when we try adding a submodule
    -    which is mentioned in .gitignore, the keyword 'fatal' is prefixed in the
    -    error messages. Therefore, prepend the keyword in the expected outputs
    -    of tests t7400.6 and t7400.16.
    +    submodule, the keyword 'fatal' is prefixed in the error messages.
    +    Therefore, prepend the keyword in the expected output of test t7400.6.
    +
    +    While at it, eliminate the extra preprocessor directive
    +    `#include "dir.h"` at the start of 'submodule--helper.c'.

         Mentored-by: Christian Couder <chriscool@tuxfamily.org>
         Mentored-by: Stefan Beller <stefanbeller@gmail.com>
    @@ Commit message
         Signed-off-by: Shourya Shukla <shouryashukla.oo@gmail.com>

      ## builtin/submodule--helper.c ##
    +@@
    + #include "diffcore.h"
    + #include "diff.h"
    + #include "object-store.h"
    +-#include "dir.h"
    + #include "advice.h"
    +
    + #define OPT_QUIET (1 << 0)
     @@ builtin/submodule--helper.c: static int module_set_branch(int argc, const char **argv, const char *prefix)
        return !!ret;
      }
    @@ builtin/submodule--helper.c: static int module_set_branch(int argc, const char *
     +  free(url);
     +}
     +
    ++static int check_sm_exists(unsigned int force, const char *path) {
    ++
    ++  int cache_pos, dir_in_cache = 0;
    ++  if (read_cache() < 0)
    ++          die(_("index file corrupt"));
    ++
    ++  cache_pos = cache_name_pos(path, strlen(path));
    ++  if(cache_pos < 0 && (directory_exists_in_index(&the_index,
    ++     path, strlen(path)) == index_directory))
    ++          dir_in_cache = 1;
    ++
    ++  if (!force) {
    ++          if (cache_pos >= 0 || dir_in_cache)
    ++                  die(_("'%s' already exists in the index"), path);
    ++  } else {
    ++          struct cache_entry *ce = NULL;
    ++          if (cache_pos >= 0)
    ++                  ce = the_index.cache[cache_pos];
    ++          if (dir_in_cache || (ce && !S_ISGITLINK(ce->ce_mode)))
    ++                  die(_("'%s' already exists in the index and is not a "
    ++                        "submodule"), path);
    ++  }
    ++  return 0;
    ++}
    ++
     +static void modify_remote_v(struct strbuf *sb)
     +{
     +  int i;
    @@ builtin/submodule--helper.c: static int module_set_branch(int argc, const char *
     +  if (is_dir_sep(path[strlen(path) -1]))
     +          path[strlen(path) - 1] = '\0';
     +
    -+  if (!force) {
    -+          if (is_directory(path) && submodule_from_path(the_repository, &null_oid, path))
    -+                  die(_("'%s' already exists in the index"), path);
    -+  } else {
    -+          int err;
    -+          if (index_name_pos(&the_index, path, strlen(path)) >= 0 &&
    -+              !is_submodule_populated_gently(path, &err))
    -+                  die(_("'%s' already exists in the index and is not a "
    -+                        "submodule"), path);
    -+  }
    ++  if (check_sm_exists(force, path))
    ++          return 1;
     +
     +  strbuf_addstr(&sb, path);
    -+  if (is_directory(path)) {
    ++  if (is_nonbare_repository_dir(&sb)) {
     +          struct object_id oid;
     +          if (resolve_gitlink_ref(path, "HEAD", &oid) < 0)
     +                  die(_("'%s' does not have a commit checked out"), path);
    @@ builtin/submodule--helper.c: static int module_set_branch(int argc, const char *
     +          cp.no_stdout = 1;
     +          strvec_pushl(&cp.args, "add", "--dry-run", "--ignore-missing",
     +                       "--no-warn-embedded-repo", path, NULL);
    -+          if (pipe_command(&cp, NULL, 0, NULL, 0, &sb, 0))
    -+                  die(_("%s"), sb.buf);
    ++          if (pipe_command(&cp, NULL, 0, NULL, 0, &sb, 0)) {
    ++                  fprintf(stderr, _("%s"), sb.buf);
    ++                  return 1;
    ++          }
     +          strbuf_release(&sb);
     +  }
     +
    @@ t/t7400-submodule-basic.sh: test_expect_success 'submodule update aborts on miss
        EOF
        git init repo-no-commits &&
        test_must_fail git submodule add ../a ./repo-no-commits 2>actual &&
    -@@ t/t7400-submodule-basic.sh: test_expect_success 'submodule add to .gitignored path fails' '
    -   (
    -           cd addtest-ignore &&
    -           cat <<-\EOF >expect &&
    --          The following paths are ignored by one of your .gitignore files:
    -+          fatal: The following paths are ignored by one of your .gitignore files:
    -           submod
    -           hint: Use -f if you really want to add them.
    -           hint: Turn this message off by running
    -           hint: "git config advice.addIgnoredFile false"
    -+
    -           EOF
    -           # Does not use test_commit due to the ignore
    -           echo "*" > .gitignore &&
-:  ---------- > 3:  98b05eb46d t7400: add test to check 'submodule add' for tracked paths
-----

Prathamesh Chavan (1):
  submodule: port submodule subcommand 'add' from shell to C

Shourya Shukla (2):
  dir: change the scope of function 'directory_exists_in_index()'
  t7400: add test to check 'submodule add' for tracked paths

 builtin/submodule--helper.c | 391 +++++++++++++++++++++++++++++++++++-
 dir.c                       |  10 +-
 dir.h                       |   9 +
 git-submodule.sh            | 161 +--------------
 t/t7400-submodule-basic.sh  |  13 +-
 5 files changed, 414 insertions(+), 170 deletions(-)