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

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

Previous: Shourya Shukla
Message 21 of 21 in “submodule: port 'summary' from Shell to C”
  1. Shourya ShuklaJul 2, 2020
  2. 1/4 submodule: amend extra line feed between callback struct and macroShourya Shukla, Jul 2, 2020
  3. Johannes SchindelinJul 3, 2020
  4. Philip OakleyJul 3, 2020
  5. Shourya ShuklaJul 4, 2020
  6. Đoàn Trần Công DanhJul 4, 2020
  7. Philip OakleyJul 5, 2020
  8. 2/4 submodule: rename helper functions to avoid ambiguityShourya Shukla, Jul 2, 2020
  9. 3/4 diff: change scope of the function count_lines()Shourya Shukla, Jul 2, 2020
  10. Johannes SchindelinJul 3, 2020
  11. Shourya ShuklaJul 4, 2020
  12. Christian CouderJul 4, 2020
  13. 4/4 submodule: port submodule subcommand 'summary' from shell to CShourya Shukla, Jul 2, 2020
  14. Johannes SchindelinJul 3, 2020
  15. Shourya ShuklaJul 5, 2020
  16. Kaartic SivaraamJul 6, 2020
  17. Shourya ShuklaJul 6, 2020
  18. Johannes SchindelinJul 12, 2020
  19. Shourya ShuklaJul 15, 2020
  20. Shourya ShuklaJul 15, 2020
  21. Johannes SchindelinAug 10, 2020

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.