From: Junio C Hamano <gitster@pobox•com>
To: Prathamesh Chavan <pc44800@gmail•com>
Cc: git@vger•kernel.org, sbeller@google•com, christian.couder@gmail•com
Subject: Re: [GSoC][PATCH 4/5 v3] submodule: port submodule subcommand 'status' from shell to C
Date: Fri, 30 Jun 2017 16:08:42 -0700 [thread overview]
Message-ID: <xmqq60fdoyyt.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <20170630194727.29787-4-pc44800@gmail.com> (Prathamesh Chavan's message of "Sat, 1 Jul 2017 01:17:26 +0530")
Prathamesh Chavan <pc44800@gmail•com> writes:
> + argv_array_pushl(&diff_files_args, "diff-files",
> + "--ignore-submodules=dirty", "--quiet", "--",
> + list_item->name, NULL);
> +
> + if (!cmd_diff_files(diff_files_args.argc, diff_files_args.argv,
> + info->prefix)) {
Essentially we'd only want to run ce_match_stat() on list_item and
the current on-filesystem submodule. We not just know which path we
are interested in, but we already have the cache entry for it, so it
feels quite wasteful to go over the_index.cache[] once again to find
the cache entry that matches list_item->name. Yes, that is how the
scripted Porcelain did it, but it feels way overkill now we have
ready access to the lower level machinery.
Having said all that, I think it is a good idea to stick to a
faithful conversion in this series. I just think a NEEDSWORK
comment may be a good idea to indicate future optimization
opportunities.
A similar optimization is already happening in this patch, actually.
Instead of doing a stat of "$sm_path/.git" and checking test -d/-f,
the code just calls is_submodule_initialized(). Which is good ;-)
> + } else {
> + if (!info->cached) {
> + struct child_process cp = CHILD_PROCESS_INIT;
> + struct strbuf sb = STRBUF_INIT;
> +
> + prepare_submodule_repo_env(&cp.env_array);
> + cp.git_cmd = 1;
> + cp.dir = list_item->name;
> +
> + argv_array_pushl(&cp.args, "rev-parse",
> + "--verify", "HEAD", NULL);
> +
> + if (capture_command(&cp, &sb, 0))
> + die(_("could not run 'git rev-parse --verify"
> + "HEAD' in submodule %s"),
> + list_item->name);
> +
> + strbuf_strip_suffix(&sb, "\n");
Likewise. This is merely resolving a ref inside a submodule;
calling something like head_ref_submodule() may be sufficient
as an optimization and at least deserves to be mentioned in a
NEEDSWORK comment.
> + print_status(info, '+', list_item->name, sb.buf,
> + displaypath);
> + strbuf_release(&sb);
> + } else {
> + print_status(info, '+', list_item->name, sub_sha1,
> + displaypath);
> + }
> + }
next prev parent reply other threads:[~2017-06-30 23:08 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-06-26 23:09 [GSoC] Update: Week 6 Prathamesh Chavan
2017-06-26 23:11 ` [GSoC][PATCH 1/6 v2] submodule--helper: introduce for_each_submodule_list Prathamesh Chavan
2017-06-26 23:11 ` [GSoC][PATCH 2/6 v2] submodule: port subcommand foreach from shell to C Prathamesh Chavan
2017-06-27 6:06 ` Christian Couder
2017-06-26 23:11 ` [GSoC][PATCH 3/6 v2] submodule: port set_name_rev " Prathamesh Chavan
2017-06-26 23:11 ` [GSoC][PATCH 4/6 v2] submodule: port submodule subcommand status Prathamesh Chavan
2017-06-26 23:11 ` [GSoC][PATCH 5/6 v2] submodule: port submodule subcommand sync from shell to C Prathamesh Chavan
2017-06-27 6:37 ` Christian Couder
2017-06-26 23:11 ` [GSoC][PATCH 6/6 v2] submodule: port submodule subcommand 'deinit' " Prathamesh Chavan
2017-06-27 7:18 ` Christian Couder
2017-06-28 19:53 ` [GSoC][PATCH 1/6 v2] submodule--helper: introduce for_each_submodule_list Junio C Hamano
2017-06-29 11:47 ` Prathamesh Chavan
2017-06-30 19:47 ` [GSoC][PATCH 1/5 v3] submodule--helper: introduce get_submodule_displaypath() Prathamesh Chavan
2017-06-30 19:47 ` [GSoC][PATCH 2/5 v3] submodule--helper: introduce for_each_submodule_list() Prathamesh Chavan
2017-06-30 19:47 ` [GSoC][PATCH 3/5 v3] submodule: port set_name_rev() from shell to C Prathamesh Chavan
2017-06-30 22:49 ` Junio C Hamano
2017-06-30 19:47 ` [GSoC][PATCH 4/5 v3] submodule: port submodule subcommand 'status' " Prathamesh Chavan
2017-06-30 23:08 ` Junio C Hamano [this message]
2017-06-30 19:47 ` [GSoC][PATCH 5/5 v3] submodule: port submodule subcommand 'sync' " Prathamesh Chavan
2017-06-30 20:11 ` Stefan Beller
2017-06-30 23:19 ` Junio C Hamano
2017-06-30 22:37 ` [GSoC][PATCH 1/5 v3] submodule--helper: introduce get_submodule_displaypath() Junio C Hamano
2017-07-13 20:05 ` [GSoC][PATCH 1/5 v4] " Prathamesh Chavan
2017-07-13 20:05 ` [GSoC][PATCH 2/5 v4] submodule--helper: introduce for_each_submodule_list() Prathamesh Chavan
2017-07-13 20:05 ` [GSoC][PATCH 3/5 v4] submodule: port set_name_rev() from shell to C Prathamesh Chavan
2017-07-13 20:05 ` [GSoC][PATCH 4/5 v4] submodule: port submodule subcommand 'status' " Prathamesh Chavan
2017-07-13 22:44 ` Stefan Beller
2017-07-13 20:05 ` [GSoC][PATCH 5/5 v4] submodule: port submodule subcommand 'sync' " Prathamesh Chavan
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=xmqq60fdoyyt.fsf@gitster.mtv.corp.google.com \
--to=gitster@pobox$(echo .)com \
--cc=christian.couder@gmail$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=pc44800@gmail$(echo .)com \
--cc=sbeller@google$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox