Re: [PATCH 4/4] submodule: port submodule subcommand 'summary' from shell to C
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Aug 10, 2020, 13:24 UTC
- Message-ID
- <nycvar.QRO.7.76.6.2008101519090.50@tvgsbejvaqbjf.bet>
- In-Reply-To
- <20200715184153.GA52607@konoha>
Hi Shourya,
On Thu, 16 Jul 2020, Shourya Shukla wrote:
Show 39 quoted lines
> BTW in your review of my patch,
> https://lore.kernel.org/git/nycvar.QRO.7.76.6.2007031712160.50@tvgsbejvaqbjf.bet/
> you said this:
> --------------->8--------------
> > + */
> > + struct child_process cp_rev_parse = CHILD_PROCESS_INIT;
> > + struct strbuf sb_rev_parse = STRBUF_INIT;
> > +
> > + cp_rev_parse.git_cmd = 1;
> > + cp_rev_parse.no_stderr = 1;
> > + cp_rev_parse.dir = p->sm_path;
> > + prepare_submodule_repo_env(&cp_rev_parse.env_array);
> > +
> > + argv_array_pushl(&cp_rev_parse.args, "rev-parse",
> > + "HEAD", NULL);
> > + if (!capture_command(&cp_rev_parse, &sb_rev_parse, 0)) {
> > + strbuf_strip_suffix(&sb_rev_parse, "\n");
> > + get_oid_hex(sb_rev_parse.buf, &p->oid_dst);
> > + }
> > + strbuf_release(&sb_rev_parse);
> > + } else if (S_ISLNK(p->mod_dst) || S_ISREG(p->mod_dst)) {
> > + struct child_process cp_hash_object = CHILD_PROCESS_INIT;
> > + struct strbuf sb_hash_object = STRBUF_INIT;
> > +
> > + cp_hash_object.git_cmd = 1;
> > + argv_array_pushl(&cp_hash_object.args, "hash-object",
> > + p->sm_path, NULL);
> > + if (!capture_command(&cp_hash_object,
> > + &sb_hash_object, 0)) {
> > + strbuf_strip_suffix(&sb_hash_object, "\n");
> > + get_oid_hex(sb_hash_object.buf, &p->oid_dst);
> > + }
> > + strbuf_release(&sb_hash_object);
>
> It would probably be shorter, less error-prone, and quicker to use
> `index_fd()` directly.
> --------------->8--------------
>
> What exactly did you mean here and where should the index_fd() be used?Well, `index_fd()` is the function that `git hash-object` uses to calculate the hash of a given file (in this case, specified via the path `p->sm_path`).
You could open an fd to that file directly, use `index_fd()` to calculate the hash, and thereby avoid spawning another (totally unnecessary) process.
Ciao, Dscho