tree-walk.c: break circular dependency with unpack-trees
diff mbox series

Message ID 20200201113922.GE1864948@coredump.intra.peff.net
State New
Headers show
Series
  • tree-walk.c: break circular dependency with unpack-trees
Related show

Commit Message

Jeff King Feb. 1, 2020, 11:39 a.m. UTC
On Fri, Jan 31, 2020 at 10:52:30AM -0800, Elijah Newren wrote:

> As far as I can tell, before your change, we can remove the
>    #include "unpack-trees.h"
> from tree-walk.c; nothing uses it.  I do like snipping includes and
> dependencies where they aren't necessary, and this one seems like one
> that could be removed.

Yeah, I think that is the case.

> But, it's not a big deal; if you want to leave that for future work
> for someone else (perhaps even asking me to turn my paragraph above
> into a commit message that rips it out and defines one #define in
> terms of the other), that's fine.

Let's do it now while we're thinking about it. How about the patch below
on top of my series?

-- >8 --
Subject: [PATCH] tree-walk.c: break circular dependency with unpack-trees

The unpack-trees API depends on the tree-walk API. But we've recently
introduced a dependency in tree-walk.c on MAX_UNPACK_TREES, which
doesn't otherwise care about unpack-trees at all.

Let's break that dependency by reversing the constants: we'll introduce
a new MAX_TRAVERSE_TREES which belongs to the tree-walk API. And then we
can define MAX_UNPACK_TREES in terms of that (since unpack-trees cannot
possibly work with more trees than it can traverse at once via
tree-walk).

The value for both will remain at 8. This is somewhat arbitrary and
probably more than is necessary, per ca885a4fe6 (read-tree() and
unpack_trees(): use consistent limit, 2008-03-13), but there's not
really any pressing need to reduce it.

Suggested-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Jeff King <peff@peff.net>
---
 tree-walk.c    | 3 +--
 tree-walk.h    | 2 ++
 unpack-trees.h | 2 +-
 3 files changed, 4 insertions(+), 3 deletions(-)

Comments

Elijah Newren Feb. 1, 2020, 3:32 p.m. UTC | #1
On Sat, Feb 1, 2020 at 3:39 AM Jeff King <peff@peff.net> wrote:
>
> On Fri, Jan 31, 2020 at 10:52:30AM -0800, Elijah Newren wrote:
>
> > As far as I can tell, before your change, we can remove the
> >    #include "unpack-trees.h"
> > from tree-walk.c; nothing uses it.  I do like snipping includes and
> > dependencies where they aren't necessary, and this one seems like one
> > that could be removed.
>
> Yeah, I think that is the case.
>
> > But, it's not a big deal; if you want to leave that for future work
> > for someone else (perhaps even asking me to turn my paragraph above
> > into a commit message that rips it out and defines one #define in
> > terms of the other), that's fine.
>
> Let's do it now while we're thinking about it. How about the patch below
> on top of my series?
>
> -- >8 --
> Subject: [PATCH] tree-walk.c: break circular dependency with unpack-trees
>
> The unpack-trees API depends on the tree-walk API. But we've recently
> introduced a dependency in tree-walk.c on MAX_UNPACK_TREES, which
> doesn't otherwise care about unpack-trees at all.
>
> Let's break that dependency by reversing the constants: we'll introduce
> a new MAX_TRAVERSE_TREES which belongs to the tree-walk API. And then we
> can define MAX_UNPACK_TREES in terms of that (since unpack-trees cannot
> possibly work with more trees than it can traverse at once via
> tree-walk).
>
> The value for both will remain at 8. This is somewhat arbitrary and
> probably more than is necessary, per ca885a4fe6 (read-tree() and
> unpack_trees(): use consistent limit, 2008-03-13), but there's not
> really any pressing need to reduce it.
>
> Suggested-by: Elijah Newren <newren@gmail.com>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  tree-walk.c    | 3 +--
>  tree-walk.h    | 2 ++
>  unpack-trees.h | 2 +-
>  3 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/tree-walk.c b/tree-walk.c
> index 3093cf7098..bb0ad34c54 100644
> --- a/tree-walk.c
> +++ b/tree-walk.c
> @@ -1,6 +1,5 @@
>  #include "cache.h"
>  #include "tree-walk.h"
> -#include "unpack-trees.h"
>  #include "dir.h"
>  #include "object-store.h"
>  #include "tree.h"
> @@ -410,7 +409,7 @@ int traverse_trees(struct index_state *istate,
>                    struct traverse_info *info)
>  {
>         int error = 0;
> -       struct name_entry entry[MAX_UNPACK_TREES];
> +       struct name_entry entry[MAX_TRAVERSE_TREES];
>         int i;
>         struct tree_desc_x tx[ARRAY_SIZE(entry)];
>         struct strbuf base = STRBUF_INIT;
> diff --git a/tree-walk.h b/tree-walk.h
> index 826396c8ed..a5058469e9 100644
> --- a/tree-walk.h
> +++ b/tree-walk.h
> @@ -3,6 +3,8 @@
>
>  #include "cache.h"
>
> +#define MAX_TRAVERSE_TREES 8
> +
>  /**
>   * The tree walking API is used to traverse and inspect trees.
>   */
> diff --git a/unpack-trees.h b/unpack-trees.h
> index ca94a421a5..ae1557fb80 100644
> --- a/unpack-trees.h
> +++ b/unpack-trees.h
> @@ -6,7 +6,7 @@
>  #include "string-list.h"
>  #include "tree-walk.h"
>
> -#define MAX_UNPACK_TREES 8
> +#define MAX_UNPACK_TREES MAX_TRAVERSE_TREES
>
>  struct cache_entry;
>  struct unpack_trees_options;
> --
> 2.25.0.538.g1d5d7023b0

Looks great, thanks!

Patch
diff mbox series

diff --git a/tree-walk.c b/tree-walk.c
index 3093cf7098..bb0ad34c54 100644
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -1,6 +1,5 @@ 
 #include "cache.h"
 #include "tree-walk.h"
-#include "unpack-trees.h"
 #include "dir.h"
 #include "object-store.h"
 #include "tree.h"
@@ -410,7 +409,7 @@  int traverse_trees(struct index_state *istate,
 		   struct traverse_info *info)
 {
 	int error = 0;
-	struct name_entry entry[MAX_UNPACK_TREES];
+	struct name_entry entry[MAX_TRAVERSE_TREES];
 	int i;
 	struct tree_desc_x tx[ARRAY_SIZE(entry)];
 	struct strbuf base = STRBUF_INIT;
diff --git a/tree-walk.h b/tree-walk.h
index 826396c8ed..a5058469e9 100644
--- a/tree-walk.h
+++ b/tree-walk.h
@@ -3,6 +3,8 @@ 
 
 #include "cache.h"
 
+#define MAX_TRAVERSE_TREES 8
+
 /**
  * The tree walking API is used to traverse and inspect trees.
  */
diff --git a/unpack-trees.h b/unpack-trees.h
index ca94a421a5..ae1557fb80 100644
--- a/unpack-trees.h
+++ b/unpack-trees.h
@@ -6,7 +6,7 @@ 
 #include "string-list.h"
 #include "tree-walk.h"
 
-#define MAX_UNPACK_TREES 8
+#define MAX_UNPACK_TREES MAX_TRAVERSE_TREES
 
 struct cache_entry;
 struct unpack_trees_options;