git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCHv3 3/3] submodule: implement `module-clone` as a builtin helper

From
Eric Sunshine <sunshine@sunshineco.com>
Date
Sep 1, 2015, 02:41 UTC
Message-ID
<CAPig+cRgSb5bD-pOH1rQ-ES61-aPKktXz8-LX-sB9kCqLrCWaw@mail.gmail.com>
In-Reply-To
<1441068029-19158-4-git-send-email-sbeller@google.com>
On Mon, Aug 31, 2015 at 8:40 PM, Stefan Beller <sbeller@google.com> wrote:
> This converts implements the helper `module_clone`. This functionality is
"converts implements"?
Show 17 quoted lines
> needed for converting `git submodule update` later on, which we want to
> add threading to.
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
> +static int module_clone(int argc, const char **argv, const char *prefix)
> +{
> +       const char *path = NULL, *name = NULL, *url = NULL;
> +       const char *reference = NULL, *depth = NULL;
> +       const char *alternative_path = NULL, *p;
> +       int quiet = 0;
> +       FILE *submodule_dot_git;
> +       char *sm_gitdir;
> +       struct strbuf rel_path = STRBUF_INIT;
> +       struct strbuf sb = STRBUF_INIT;
> +
> +       struct option module_update_options[] = {
s/update/clone/ maybe?
Show 23 quoted lines
> +               OPT_STRING(0, "prefix", &alternative_path,
> +                          N_("path"),
> +                          N_("alternative anchor for relative paths")),
> +               OPT_STRING(0, "path", &path,
> +                          N_("path"),
> +                          N_("where the new submodule will be cloned to")),
> +               OPT_STRING(0, "name", &name,
> +                          N_("string"),
> +                          N_("name of the new submodule")),
> +               OPT_STRING(0, "url", &url,
> +                          N_("string"),
> +                          N_("url where to clone the submodule from")),
> +               OPT_STRING(0, "reference", &reference,
> +                          N_("string"),
> +                          N_("reference repository")),
> +               OPT_STRING(0, "depth", &depth,
> +                          N_("string"),
> +                          N_("depth for shallow clones")),
> +               OPT__QUIET(&quiet, "Suppress output for cloning a submodule"),
> +               OPT_END()
> +       };
> +
> +       static const char * const git_submodule_helper_usage[] = {
You can still drop 'static'.
> +               N_("git submodule--helper update [--prefix=<path>] [--quiet] [--remote] [-N|--no-fetch]"
"update"?
> +                  "[-f|--force] [--rebase|--merge] [--reference <repository>]"
> +                  "[--depth <depth>] [--recursive] [--] [<path>...]"),
Also, what's all this "--force", "--rebase", "--remote", "--recursive" stuff?
Show 11 quoted lines
> +               NULL
> +       };
> +
> +       argc = parse_options(argc, argv, prefix, module_update_options,
> +                            git_submodule_helper_usage, 0);
> +
> +       strbuf_addf(&sb, "%s/modules/%s", get_git_dir(), name);
> +       sm_gitdir = strbuf_detach(&sb, NULL);
> +
> +       if (!file_exists(sm_gitdir)) {
> +               safe_create_leading_directories_const(sm_gitdir);

Still inconsistent checking of return value of safe_create_leading_directories_const() in this function.

In fact, it looks like pretty much all of my v2 review comments are still relevant to the remainder of this function. Rather than repeating them, I'll just give a reference[1].

[1]: http://article.gmane.org/gmane.comp.version-control.git/276952
Show 55 quoted lines
> +               if (clone_submodule(path, sm_gitdir, url, depth, reference, quiet))
> +                       die(N_("Clone of '%s' into submodule path '%s' failed"),
> +                           url, path);
> +       } else {
> +               safe_create_leading_directories_const(path);
> +               unlink(sm_gitdir);
> +       }
> +
> +       /* Write a .git file in the submodule to redirect to the superproject. */
> +       if (alternative_path && !strcmp(alternative_path, "")) {
> +               p = relative_path(path, alternative_path, &sb);
> +               strbuf_reset(&sb);
> +       } else
> +               p = path;
> +
> +       if (safe_create_leading_directories_const(p) < 0)
> +               die("Could not create directory '%s'", p);
> +
> +       strbuf_addf(&sb, "%s/.git", p);
> +
> +       if (safe_create_leading_directories_const(sb.buf) < 0)
> +               die(_("could not create leading directories of '%s'"), sb.buf);
> +       submodule_dot_git = fopen(sb.buf, "w");
> +       if (!submodule_dot_git)
> +               die ("Cannot open file '%s': %s", sb.buf, strerror(errno));
> +
> +       fprintf(submodule_dot_git, "gitdir: %s\n",
> +               relative_path(sm_gitdir, path, &rel_path));
> +       if (fclose(submodule_dot_git))
> +               die("Could not close file %s", sb.buf);
> +       strbuf_reset(&sb);
> +
> +       /* Redirect the worktree of the submodule in the superprojects config */
> +       if (!is_absolute_path(sm_gitdir)) {
> +               char *s = (char*)sm_gitdir;
> +               if (strbuf_getcwd(&sb))
> +                       die_errno("unable to get current working directory");
> +               strbuf_addf(&sb, "/%s", sm_gitdir);
> +               sm_gitdir = strbuf_detach(&sb, NULL);
> +               free(s);
> +       }
> +
> +       if (strbuf_getcwd(&sb))
> +               die_errno("unable to get current working directory");
> +       strbuf_addf(&sb, "/%s", path);
> +
> +       p = git_pathdup_submodule(path, "config");
> +       if (!p)
> +               die("Could not get submodule directory for '%s'", path);
> +       git_config_set_in_file(p, "core.worktree",
> +                              relative_path(sb.buf, sm_gitdir, &rel_path));
> +       strbuf_release(&sb);
> +       free(sm_gitdir);
> +       return 0;
> +}
Previous: Stefan BellerNext: Eric Sunshine
Message 20 of 21 in “[PATCHv2 0/3] submodule--helper: Have some refactoring only patches first”
  1. Stefan BellerAug 31, 2015
  2. 1/3 submodule: implement `module_list` as a builtin helperStefan Beller, Aug 31, 2015
  3. Eric SunshineAug 31, 2015
  4. 2/3 submodule: implement `module_name` as a builtin helperStefan Beller, Aug 31, 2015
  5. Junio C HamanoAug 31, 2015
  6. Stefan BellerAug 31, 2015
  7. Eric SunshineAug 31, 2015
  8. 3/3 submodule: implement `module_clone` as a builtin helperStefan Beller, Aug 31, 2015
  9. Eric SunshineAug 31, 2015
  10. Stefan BellerSep 1, 2015
  11. Eric SunshineSep 1, 2015
  12. Junio C HamanoAug 31, 2015
  13. 0/3 submodule--helper: Have some refactoring only patches firstStefan Beller, Sep 1, 2015
  14. 1/3 submodule: implement `module-list` as a builtin helperStefan Beller, Sep 1, 2015
  15. Eric SunshineSep 1, 2015
  16. 2/3 submodule: implement `module-name` as a builtin helperStefan Beller, Sep 1, 2015
  17. Eric SunshineSep 1, 2015
  18. Stefan BellerSep 1, 2015
  19. 3/3 submodule: implement `module-clone` as a builtin helperStefan Beller, Sep 1, 2015
  20. Eric SunshineSep 1, 2015
  21. Eric SunshineSep 1, 2015

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.