git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Shourya Shukla <shouryashukla.oo@gmail.com>
To: git@vger.kernel.org
Cc: gitster@pobox.com, christian.couder@gmail.com,
	kaartic.sivaraam@gmail.com, Johannes.Schindelin@gmx.de,
	liu.denton@gmail.com, Shourya Shukla <shouryashukla.oo@gmail.com>
Subject: [PATCH v2 0/3] submodule: port subcommand add from shell to C
Date: Wed,  7 Oct 2020 13:15:35 +0530	[thread overview]
Message-ID: <20201007074538.25891-1-shouryashukla.oo@gmail.com> (raw)

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(-)

-- 
2.28.0


             reply	other threads:[~2020-10-07  7:45 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-07  7:45 Shourya Shukla [this message]
2020-10-07  7:45 ` [PATCH v2 1/3] dir: change the scope of function 'directory_exists_in_index()' Shourya Shukla
2020-10-07 18:05   ` Junio C Hamano
2020-10-12 10:11     ` Shourya Shukla
2020-11-18 23:25   ` Emily Shaffer
2020-10-07  7:45 ` [PATCH v2 2/3] submodule: port submodule subcommand 'add' from shell to C Shourya Shukla
2020-10-07 18:37   ` Junio C Hamano
2020-10-07 22:19   ` Junio C Hamano
2020-10-08 17:19   ` Junio C Hamano
2020-10-09  5:09   ` Junio C Hamano
2020-11-18 23:13     ` Jonathan Tan
2020-11-19  7:44       ` Ævar Arnfjörð Bjarmason
2020-11-19 12:38         ` Johannes Schindelin
2020-11-19 20:37           ` Junio C Hamano
2020-10-07  7:45 ` [PATCH v2 3/3] t7400: add test to check 'submodule add' for tracked paths Shourya Shukla
2020-11-19  0:03 ` [PATCH v2 0/3] submodule: port subcommand add from shell to C Josh Steadmon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201007074538.25891-1-shouryashukla.oo@gmail.com \
    --to=shouryashukla.oo@gmail.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=christian.couder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=kaartic.sivaraam@gmail.com \
    --cc=liu.denton@gmail.com \
    --subject='Re: [PATCH v2 0/3] submodule: port subcommand add from shell to C' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).