public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed

From: Jonathan Nieder <jrnieder@gmail•com>
To: Stefan Beller <stefanbeller@gmail•com>
Cc: git@vger•kernel.org, jonathantanmy@google•com
Subject: Re: [PATCH 7/9] submodule: migrate get_next_submodule to use repository structs
Date: Fri, 1 Feb 2019 17:58:17 -0800	[thread overview]
Message-ID: <20190202015817.GA241226@google.com> (raw)
In-Reply-To: <20181129002756.167615-8-sbeller@google.com>

Hi,

Stefan Beller wrote:

> This patch tightens the check upfront, such that we do not need
> to spawn a child process to find out if the submodule is broken.

Sounds sensible.

[...]
> --- a/submodule.c
> +++ b/submodule.c
[...]
> @@ -1319,10 +1338,23 @@ static int get_next_submodule(struct child_process *cp,
>  			argv_array_push(&cp->args, default_argv);
>  			argv_array_push(&cp->args, "--submodule-prefix");
>  			argv_array_push(&cp->args, submodule_prefix.buf);
> +
> +			repo_clear(repo);
> +			free(repo);
>  			ret = 1;
> +		} else {
> +			/*
> +			 * An empty directory is normal,
> +			 * the submodule is not initialized
> +			 */
> +			if (S_ISGITLINK(ce->ce_mode) &&
> +			    !is_empty_dir(ce->name)) {

What if the directory is nonempty (e.g. contains build artifacts)?

> +				spf->result = 1;
> +				strbuf_addf(err,
> +					    _("Could not access submodule '%s'"),
> +					    ce->name);
> +			}

Should this exit the loop?  Otherwise, multiple "Could not access"
messages can go in the same err string a big concatenated line.

Thanks,
Jonathan

next prev parent reply	other threads:[~2019-02-02  1:58 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-29  0:27 [PATCHv2 0/9] Resending sb/submodule-recursive-fetch-gets-the-tip Stefan Beller
2018-11-29  0:27 ` [PATCH 1/9] sha1-array: provide oid_array_filter Stefan Beller
2018-11-29  0:27 ` [PATCH 2/9] submodule.c: fix indentation Stefan Beller
2018-11-29  0:27 ` [PATCH 3/9] submodule.c: sort changed_submodule_names before searching it Stefan Beller
2018-12-05  0:12   ` Jonathan Tan
2018-11-29  0:27 ` [PATCH 4/9] submodule.c: tighten scope of changed_submodule_names struct Stefan Beller
2018-11-29  0:27 ` [PATCH 5/9] submodule: store OIDs in changed_submodule_names Stefan Beller
2018-11-29  0:27 ` [PATCH 6/9] repository: repo_submodule_init to take a submodule struct Stefan Beller
2018-11-29  0:27 ` [PATCH 7/9] submodule: migrate get_next_submodule to use repository structs Stefan Beller
2018-12-05  0:17   ` Jonathan Tan
2019-02-02  1:58   ` Jonathan Nieder [this message]
2018-11-29  0:27 ` [PATCH 8/9] submodule.c: fetch in submodules git directory instead of in worktree Stefan Beller
2018-12-05  0:38   ` Jonathan Tan
2018-11-29  0:27 ` [PATCH 9/9] fetch: try fetching submodules if needed objects were not fetched Stefan Beller
2018-12-05  1:07   ` Jonathan Tan
2018-12-06 21:26     ` [PATCH] fetch: ensure submodule objects fetched Stefan Beller
2018-12-09  1:57       ` Junio C Hamano
2018-12-05  3:10 ` [PATCHv2 0/9] Resending sb/submodule-recursive-fetch-gets-the-tip Junio C Hamano
2018-12-06 21:59   ` Stefan Beller
2018-12-07  0:25 ` Josh Steadmon
2019-01-15  1:38   ` Jonathan Nieder

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=20190202015817.GA241226@google.com \
    --to=jrnieder@gmail$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=jonathantanmy@google$(echo .)com \
    --cc=stefanbeller@gmail$(echo .)com \
    /path/to/YOUR_REPLY

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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox