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

[PATCH v2 0/8] In grep, no adding submodule ODB as alternates

From
JTJonathan Tan <jonathantanmy@google.com>
Date
Aug 13, 2021, 21:05 UTC
Message-ID
<cover.1628888668.git.jonathantanmy@google.com>
In-Reply-To
<cover.1628618950.git.jonathantanmy@google.com>
Thanks everyone for your reviews. I believe I've addressed all of them.

The main change is that I've also made a change in submodule-config to avoid registering submodule ODBs as alternates (thanks, Matheus, for noticing this).

Jonathan Tan (8):
  submodule: lazily add submodule ODBs as alternates
  grep: use submodule-ODB-as-alternate lazy-addition
  grep: typesafe versions of grep_source_init
  grep: read submodule entry with explicit repo
  grep: allocate subrepos on heap
  grep: add repository to OID grep sources
  submodule-config: pass repo upon blob config read
  t7814: show lack of alternate ODB-adding
 builtin/grep.c                     | 64 +++++++++++++++++++-----------
 config.c                           | 18 ++++++---
 config.h                           |  3 ++
 grep.c                             | 51 +++++++++++++++---------
 grep.h                             | 18 +++++++--
 object-file.c                      |  5 +++
 submodule-config.c                 |  5 ++-
 submodule.c                        | 25 +++++++++++-
 submodule.h                        |  8 ++++
 t/README                           | 10 +++++
 t/t7814-grep-recurse-submodules.sh |  3 ++
 11 files changed, 156 insertions(+), 54 deletions(-)
Range-diff against v1:
1:  5994a517e8 = 1:  5994a517e8 submodule: lazily add submodule ODBs as alternates
2:  31e9b914c4 = 2:  31e9b914c4 grep: use submodule-ODB-as-alternate lazy-addition
3:  e5e6a0dc1e ! 3:  aa3f1f3c89 grep: typesafe versions of grep_source_init
    @@ builtin/grep.c: static int grep_file(struct grep_opt *opt, const char *filename)
      	if (num_threads > 1) {
     
      ## grep.c ##
    -@@ grep.c: int grep_buffer(struct grep_opt *opt, char *buf, unsigned long size)
    +@@ grep.c: int grep_source(struct grep_opt *opt, struct grep_source *gs)
    + 	return grep_source_1(opt, gs, 0);
    + }
    + 
    ++static void grep_source_init_buf(struct grep_source *gs, char *buf,
    ++				 unsigned long size)
    ++{
    ++	gs->type = GREP_SOURCE_BUF;
    ++	gs->name = NULL;
    ++	gs->path = NULL;
    ++	gs->buf = buf;
    ++	gs->size = size;
    ++	gs->driver = NULL;
    ++	gs->identifier = NULL;
    ++}
    ++
    + int grep_buffer(struct grep_opt *opt, char *buf, unsigned long size)
    + {
      	struct grep_source gs;
      	int r;
      
     -	grep_source_init(&gs, GREP_SOURCE_BUF, NULL, NULL, NULL);
    -+	grep_source_init_buf(&gs);
    - 	gs.buf = buf;
    - 	gs.size = size;
    +-	gs.buf = buf;
    +-	gs.size = size;
    ++	grep_source_init_buf(&gs, buf, size);
    + 
    + 	r = grep_source(opt, &gs);
      
     @@ grep.c: int grep_buffer(struct grep_opt *opt, char *buf, unsigned long size)
      	return r;
    @@ grep.c: int grep_buffer(struct grep_opt *opt, char *buf, unsigned long size)
     +	gs->size = 0;
     +	gs->driver = NULL;
     +	gs->identifier = oiddup(oid);
    -+}
    -+
    -+void grep_source_init_buf(struct grep_source *gs)
    -+{
    -+	gs->type = GREP_SOURCE_BUF;
    -+	gs->name = NULL;
    -+	gs->path = NULL;
    -+	gs->buf = NULL;
    -+	gs->size = 0;
    -+	gs->driver = NULL;
    -+	gs->identifier = NULL;
      }
      
      void grep_source_clear(struct grep_source *gs)
    @@ grep.h: struct grep_source {
     +			   const char *path);
     +void grep_source_init_oid(struct grep_source *gs, const char *name,
     +			  const char *path, const struct object_id *oid);
    -+void grep_source_init_buf(struct grep_source *gs);
      void grep_source_clear_data(struct grep_source *gs);
      void grep_source_clear(struct grep_source *gs);
      void grep_source_load_driver(struct grep_source *gs,
4:  30ead880b3 = 4:  050deacfb7 grep: read submodule entry with explicit repo
5:  f62608e88f ! 5:  3f24815224 grep: allocate subrepos on heap
    @@ builtin/grep.c: static void work_done(struct work_item *w)
     +		free(repos_to_free[i]);
     +	}
     +	free(repos_to_free);
    ++	repos_to_free_nr = 0;
    ++	repos_to_free_alloc = 0;
     +}
     +
      static void *run(void *arg)
6:  b2df245587 ! 6:  50c69a988b grep: add repository to OID grep sources
    @@ builtin/grep.c: static int grep_oid(struct grep_opt *opt, const struct object_id
      	strbuf_release(&pathbuf);
      
      	if (num_threads > 1) {
    +@@ builtin/grep.c: static int grep_submodule(struct grep_opt *opt,
    + 	repo_read_gitmodules(subrepo, 0);
    + 
    + 	/*
    +-	 * NEEDSWORK: This adds the submodule's object directory to the list of
    +-	 * alternates for the single in-memory object store.  This has some bad
    +-	 * consequences for memory (processed objects will never be freed) and
    +-	 * performance (this increases the number of pack files git has to pay
    +-	 * attention to, to the sum of the number of pack files in all the
    +-	 * repositories processed so far).  This can be removed once the object
    +-	 * store is no longer global and instead is a member of the repository
    +-	 * object.
    ++	 * All code paths tested by test code no longer need submodule ODBs to
    ++	 * be added as alternates, but add it to the list just in case.
    ++	 * Submodule ODBs added through add_submodule_odb_by_path() will be
    ++	 * lazily registered as alternates when needed (and except in an
    ++	 * unexpected code interaction, it won't be needed).
    + 	 */
    + 	add_submodule_odb_by_path(subrepo->objects->odb->path);
    + 	obj_read_unlock();
     
      ## grep.c ##
     @@ grep.c: void grep_source_init_file(struct grep_source *gs, const char *name,
    @@ grep.c: void grep_source_init_oid(struct grep_source *gs, const char *name,
     +	gs->repo = repo;
      }
      
    - void grep_source_init_buf(struct grep_source *gs)
    + void grep_source_clear(struct grep_source *gs)
     @@ grep.c: static int grep_source_load_oid(struct grep_source *gs)
      {
      	enum object_type type;
    @@ grep.c: static int grep_source_load_oid(struct grep_source *gs)
      			     gs->name,
     
      ## grep.h ##
    +@@ grep.h: struct grep_opt {
    + 	struct grep_pat *header_list;
    + 	struct grep_pat **header_tail;
    + 	struct grep_expr *pattern_expression;
    ++
    ++	/*
    ++	 * NEEDSWORK: See if we can remove this field, because the repository
    ++	 * should probably be per-source, not per-repo. This is potentially the
    ++	 * cause of at least one bug - "git grep" ignoring the textconv
    ++	 * attributes from submodules. See [1] for more information.
    ++	 * [1] https://lore.kernel.org/git/CAHd-oW5iEQarYVxEXoTG-ua2zdoybTrSjCBKtO0YT292fm0NQQ@mail.gmail.com/
    ++	 */
    + 	struct repository *repo;
    ++
    + 	const char *prefix;
    + 	int prefix_length;
    + 	regex_t regexp;
     @@ grep.h: struct grep_source {
      		GREP_SOURCE_BUF,
      	} type;
    @@ grep.h: struct grep_source {
     -			  const char *path, const struct object_id *oid);
     +			  const char *path, const struct object_id *oid,
     +			  struct repository *repo);
    - void grep_source_init_buf(struct grep_source *gs);
      void grep_source_clear_data(struct grep_source *gs);
      void grep_source_clear(struct grep_source *gs);
    + void grep_source_load_driver(struct grep_source *gs,
-:  ---------- > 7:  94db10a4e5 submodule-config: pass repo upon blob config read
7:  f1fc89894b = 8:  4a51fcfb77 t7814: show lack of alternate ODB-adding
-- 
2.33.0.rc1.237.g0d66db33f3-goog
Previous: Josh SteadmonNext: Jonathan Tan
Message 35 of 71 in “In grep, no adding submodule ODB as alternates”
  1. 0/7 In grep, no adding submodule ODB as alternatesJonathan Tan, Aug 10, 2021
  2. 4/7 grep: read submodule entry with explicit repoJonathan Tan, Aug 10, 2021
  3. Emily ShafferAug 11, 2021
  4. Jonathan TanAug 13, 2021
  5. 3/7 grep: typesafe versions of grep_source_initJonathan Tan, Aug 10, 2021
  6. Junio C HamanoAug 10, 2021
  7. Emily ShafferAug 11, 2021
  8. Ramsay JonesAug 11, 2021
  9. Jonathan TanAug 13, 2021
  10. Matheus Tavares BernardinoAug 11, 2021
  11. Junio C HamanoAug 12, 2021
  12. Jonathan TanAug 13, 2021
  13. 2/7 grep: use submodule-ODB-as-alternate lazy-additionJonathan Tan, Aug 10, 2021
  14. Emily ShafferAug 11, 2021
  15. Jonathan TanAug 13, 2021
  16. 1/7 submodule: lazily add submodule ODBs as alternatesJonathan Tan, Aug 10, 2021
  17. Junio C HamanoAug 10, 2021
  18. Jonathan TanAug 13, 2021
  19. Emily ShafferAug 11, 2021
  20. Jonathan TanAug 13, 2021
  21. 5/7 grep: allocate subrepos on heapJonathan Tan, Aug 10, 2021
  22. Emily ShafferAug 11, 2021
  23. Jonathan TanAug 13, 2021
  24. 6/7 grep: add repository to OID grep sourcesJonathan Tan, Aug 10, 2021
  25. Emily ShafferAug 11, 2021
  26. Jonathan TanAug 13, 2021
  27. Matheus Tavares BernardinoAug 11, 2021
  28. Jonathan TanAug 13, 2021
  29. 7/7 t7814: show lack of alternate ODB-addingJonathan Tan, Aug 10, 2021
  30. Emily ShafferAug 11, 2021
  31. Matheus Tavares BernardinoAug 11, 2021
  32. Jonathan TanAug 13, 2021
  33. Emily ShafferAug 11, 2021
  34. Josh SteadmonAug 11, 2021
  35. 0/8 In grep, no adding submodule ODB as alternatesJonathan Tan, Aug 13, 2021
  36. 2/8 grep: use submodule-ODB-as-alternate lazy-additionJonathan Tan, Aug 13, 2021
  37. 1/8 submodule: lazily add submodule ODBs as alternatesJonathan Tan, Aug 13, 2021
  38. 3/8 grep: typesafe versions of grep_source_initJonathan Tan, Aug 13, 2021
  39. Matheus Tavares BernardinoAug 16, 2021
  40. 4/8 grep: read submodule entry with explicit repoJonathan Tan, Aug 13, 2021
  41. 5/8 grep: allocate subrepos on heapJonathan Tan, Aug 13, 2021
  42. Junio C HamanoAug 13, 2021
  43. Jonathan TanAug 16, 2021
  44. 6/8 grep: add repository to OID grep sourcesJonathan Tan, Aug 13, 2021
  45. Matheus Tavares BernardinoAug 16, 2021
  46. Jonathan TanAug 16, 2021
  47. 7/8 submodule-config: pass repo upon blob config readJonathan Tan, Aug 13, 2021
  48. Matheus Tavares BernardinoAug 16, 2021
  49. Matheus Tavares BernardinoAug 16, 2021
  50. Jonathan TanAug 16, 2021
  51. Matheus Tavares BernardinoAug 16, 2021
  52. Jonathan TanAug 16, 2021
  53. Jonathan TanAug 16, 2021
  54. 8/8 t7814: show lack of alternate ODB-addingJonathan Tan, Aug 13, 2021
  55. Matheus Tavares BernardinoAug 16, 2021
  56. 0/8 In grep, no adding submodule ODB as alternatesJonathan Tan, Aug 16, 2021
  57. 1/8 submodule: lazily add submodule ODBs as alternatesJonathan Tan, Aug 16, 2021
  58. 2/8 grep: use submodule-ODB-as-alternate lazy-additionJonathan Tan, Aug 16, 2021
  59. 3/8 grep: typesafe versions of grep_source_initJonathan Tan, Aug 16, 2021
  60. 4/8 grep: read submodule entry with explicit repoJonathan Tan, Aug 16, 2021
  61. 5/8 grep: allocate subrepos on heapJonathan Tan, Aug 16, 2021
  62. 6/8 grep: add repository to OID grep sourcesJonathan Tan, Aug 16, 2021
  63. Ævar Arnfjörð BjarmasonSep 27, 2021
  64. Matheus TavaresSep 27, 2021
  65. Ævar Arnfjörð BjarmasonSep 27, 2021
  66. 7/8 submodule-config: pass repo upon blob config readJonathan Tan, Aug 16, 2021
  67. 8/8 t7814: show lack of alternate ODB-addingJonathan Tan, Aug 16, 2021
  68. Matheus Tavares BernardinoAug 17, 2021
  69. Junio C HamanoSep 8, 2021
  70. Matheus Tavares BernardinoSep 8, 2021
  71. Junio C HamanoSep 8, 2021

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.