[RFC PATCH 1/3] stylefix: pointers bind to the variable, not the type

[RFC PATCH 1/3] stylefix: pointers bind to the variable, not the type

[David Aguilar]

Signed-off-by: David Aguilar <davvid@gmail.com>
---
This is an RFC patch but this is probably fine as-is,
and is orthogonal to the next two patches.

 builtin/clone.c | 7 ++++---
 commit.c        | 2 +-
 commit.h        | 2 +-
 reflog-walk.c   | 2 +-
 reflog-walk.h   | 2 +-
 refs.h          | 2 +-
 remote-curl.c   | 2 +-
 7 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index bbd169c..315969d 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -685,9 +685,10 @@ static void write_config(struct string_list *config)
 	}
 }
 
-static void write_refspec_config(const char* src_ref_prefix,
-		const struct ref* our_head_points_at,
-		const struct ref* remote_head_points_at, struct strbuf* branch_top)
+static void write_refspec_config(const char *src_ref_prefix,
+		const struct ref *our_head_points_at,
+		const struct ref *remote_head_points_at,
+		struct strbuf *branch_top)
 {
 	struct strbuf key = STRBUF_INIT;
 	struct strbuf value = STRBUF_INIT;
diff --git a/commit.c b/commit.c
index ae7f2b1..4de6be4 100644
--- a/commit.c
+++ b/commit.c
@@ -1256,7 +1256,7 @@ static void parse_gpg_output(struct signature_check *sigc)
 	}
 }
 
-void check_commit_signature(const struct commit* commit, struct signature_check *sigc)
+void check_commit_signature(const struct commit *commit, struct signature_check *sigc)
 {
 	struct strbuf payload = STRBUF_INIT;
 	struct strbuf signature = STRBUF_INIT;
diff --git a/commit.h b/commit.h
index a8cbf52..268c9d7 100644
--- a/commit.h
+++ b/commit.h
@@ -346,7 +346,7 @@ extern void print_commit_list(struct commit_list *list,
  * at all.  This may allocate memory for sig->gpg_output, sig->gpg_status,
  * sig->signer and sig->key.
  */
-extern void check_commit_signature(const struct commit* commit, struct signature_check *sigc);
+extern void check_commit_signature(const struct commit *commit, struct signature_check *sigc);
 
 int compare_commits_by_commit_date(const void *a_, const void *b_, void *unused);
 
diff --git a/reflog-walk.c b/reflog-walk.c
index 9ce8b53..0e5174b 100644
--- a/reflog-walk.c
+++ b/reflog-walk.c
@@ -133,7 +133,7 @@ struct reflog_walk_info {
 	struct commit_reflog *last_commit_reflog;
 };
 
-void init_reflog_walk(struct reflog_walk_info** info)
+void init_reflog_walk(struct reflog_walk_info **info)
 {
 	*info = xcalloc(1, sizeof(struct reflog_walk_info));
 }
diff --git a/reflog-walk.h b/reflog-walk.h
index 50265f5..a9bd60e 100644
--- a/reflog-walk.h
+++ b/reflog-walk.h
@@ -5,7 +5,7 @@
 
 struct reflog_walk_info;
 
-extern void init_reflog_walk(struct reflog_walk_info** info);
+extern void init_reflog_walk(struct reflog_walk_info **info);
 extern int add_reflog_for_walk(struct reflog_walk_info *info,
 		struct commit *commit, const char *name);
 extern void fake_reflog_parent(struct reflog_walk_info *info,
diff --git a/refs.h b/refs.h
index ec46acd..00f209a 100644
--- a/refs.h
+++ b/refs.h
@@ -77,7 +77,7 @@ static inline const char *has_glob_specials(const char *pattern)
 extern int for_each_rawref(each_ref_fn, void *);
 
 extern void warn_dangling_symref(FILE *fp, const char *msg_fmt, const char *refname);
-extern void warn_dangling_symrefs(FILE *fp, const char *msg_fmt, const struct string_list* refnames);
+extern void warn_dangling_symrefs(FILE *fp, const char *msg_fmt, const struct string_list *refnames);
 
 /*
  * Lock the packed-refs file for writing.  Flags is passed to
diff --git a/remote-curl.c b/remote-curl.c
index 0fcf2ce..d2229e0 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -221,7 +221,7 @@ static int show_http_message(struct strbuf *type, struct strbuf *charset,
 	return 0;
 }
 
-static struct discovery* discover_refs(const char *service, int for_push)
+static struct discovery *discover_refs(const char *service, int for_push)
 {
 	struct strbuf exp = STRBUF_INIT;
 	struct strbuf type = STRBUF_INIT;
-- 
2.1.0.30.g0bdc89a

[RFC PATCH 2/3] headers: improve header dependencies

[David Aguilar]

Add missing includes or forward declarations where needed.

Signed-off-by: David Aguilar <davvid@gmail.com>
---
If enum date_type were moved to object.h then we wouldn't need
to include cache.h from commit.h, but this patch doesn't touch that.

If we want to avoid including cache.h, another possibility
is moving enum object_type to something like object-types.h so that
e.g. dir.h wouldn't need to include all of cache.h.

Do we prefer these forward declarations or full-on #includes
of the corresponding header?  Forward declarations seemed like
the least intrusive, which is why some of the files do that in
this patch.

 color.h         | 2 ++
 commit.h        | 2 +-
 diffcore.h      | 2 ++
 dir.h           | 1 +
 graph.h         | 3 +++
 object.h        | 2 ++
 parse-options.h | 2 ++
 pathspec.h      | 2 ++
 quote.h         | 2 ++
 refs.h          | 5 +++++
 remote.h        | 2 ++
 sequencer.h     | 4 ++++
 strbuf.h        | 2 ++
 submodule.h     | 3 +++
 tag.h           | 1 +
 tree-walk.h     | 2 ++
 tree.h          | 2 ++
 utf8.h          | 4 ++++
 18 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/color.h b/color.h
index 9a8495b..6d64e6d 100644
--- a/color.h
+++ b/color.h
@@ -1,6 +1,8 @@
 #ifndef COLOR_H
 #define COLOR_H
 
+#include "git-compat-util.h"
+
 struct strbuf;
 
 /*  2 + (2 * num_attrs) + 8 + 1 + 8 + 'm' + NUL */
diff --git a/commit.h b/commit.h
index 268c9d7..339c55d 100644
--- a/commit.h
+++ b/commit.h
@@ -1,7 +1,7 @@
 #ifndef COMMIT_H
 #define COMMIT_H
 
-#include "object.h"
+#include "cache.h"
 #include "tree.h"
 #include "strbuf.h"
 #include "decorate.h"
diff --git a/diffcore.h b/diffcore.h
index c876dac..43856de 100644
--- a/diffcore.h
+++ b/diffcore.h
@@ -4,6 +4,8 @@
 #ifndef DIFFCORE_H
 #define DIFFCORE_H
 
+struct diff_options;
+
 /* This header file is internal between diff.c and its diff transformers
  * (e.g. diffcore-rename, diffcore-pickaxe).  Never include this header
  * in anything else.
diff --git a/dir.h b/dir.h
index 6c45e9d..1cc9f90 100644
--- a/dir.h
+++ b/dir.h
@@ -3,6 +3,7 @@
 
 /* See Documentation/technical/api-directory-listing.txt */
 
+#include "cache.h"
 #include "strbuf.h"
 
 struct dir_entry {
diff --git a/graph.h b/graph.h
index 0be62bd..585ebe6 100644
--- a/graph.h
+++ b/graph.h
@@ -3,6 +3,9 @@
 
 /* A graph is a pointer to this opaque structure */
 struct git_graph;
+struct commit;
+struct rev_info;
+struct strbuf;
 
 /*
  * Set up a custom scheme for column colors.
diff --git a/object.h b/object.h
index 5e8d8ee..40bd3a8 100644
--- a/object.h
+++ b/object.h
@@ -1,6 +1,8 @@
 #ifndef OBJECT_H
 #define OBJECT_H
 
+enum object_type;
+
 struct object_list {
 	struct object *item;
 	struct object_list *next;
diff --git a/parse-options.h b/parse-options.h
index 7940bc7..933a1b7 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -1,6 +1,8 @@
 #ifndef PARSE_OPTIONS_H
 #define PARSE_OPTIONS_H
 
+#include "git-compat-util.h"
+
 enum parse_opt_type {
 	/* special types */
 	OPTION_END,
diff --git a/pathspec.h b/pathspec.h
index 0c11262..c92fafc 100644
--- a/pathspec.h
+++ b/pathspec.h
@@ -1,6 +1,8 @@
 #ifndef PATHSPEC_H
 #define PATHSPEC_H
 
+#include "git-compat-util.h"
+
 /* Pathspec magic */
 #define PATHSPEC_FROMTOP	(1<<0)
 #define PATHSPEC_MAXDEPTH	(1<<1)
diff --git a/quote.h b/quote.h
index 71dcc3a..f9ca9e2 100644
--- a/quote.h
+++ b/quote.h
@@ -1,6 +1,8 @@
 #ifndef QUOTE_H
 #define QUOTE_H
 
+#include "git-compat-util.h"
+
 struct strbuf;
 
 /* Help to copy the thing properly quoted for the shell safety.
diff --git a/refs.h b/refs.h
index 00f209a..889bf8d 100644
--- a/refs.h
+++ b/refs.h
@@ -1,6 +1,11 @@
 #ifndef REFS_H
 #define REFS_H
 
+#include "git-compat-util.h"
+
+struct strbuf;
+struct string_list;
+
 struct ref_lock {
 	char *ref_name;
 	char *orig_ref_name;
diff --git a/remote.h b/remote.h
index 917d383..4b1533f 100644
--- a/remote.h
+++ b/remote.h
@@ -3,6 +3,8 @@
 
 #include "parse-options.h"
 
+struct strbuf;
+
 enum {
 	REMOTE_CONFIG,
 	REMOTE_REMOTES,
diff --git a/sequencer.h b/sequencer.h
index db43e9c..8e30082 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -1,6 +1,10 @@
 #ifndef SEQUENCER_H
 #define SEQUENCER_H
 
+#include "git-compat-util.h"
+
+struct strbuf;
+
 #define SEQ_DIR		"sequencer"
 #define SEQ_HEAD_FILE	"sequencer/head"
 #define SEQ_TODO_FILE	"sequencer/todo"
diff --git a/strbuf.h b/strbuf.h
index a7c0192..be59d9e 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -1,6 +1,8 @@
 #ifndef STRBUF_H
 #define STRBUF_H
 
+#include "git-compat-util.h"
+
 /* See Documentation/technical/api-strbuf.txt */
 
 extern char strbuf_slopbuf[];
diff --git a/submodule.h b/submodule.h
index 7beec48..5295c01 100644
--- a/submodule.h
+++ b/submodule.h
@@ -1,8 +1,11 @@
 #ifndef SUBMODULE_H
 #define SUBMODULE_H
 
+#include "git-compat-util.h"
+
 struct diff_options;
 struct argv_array;
+struct string_list;
 
 enum {
 	RECURSE_SUBMODULES_ON_DEMAND = -1,
diff --git a/tag.h b/tag.h
index bc8a1e4..68b0334 100644
--- a/tag.h
+++ b/tag.h
@@ -1,6 +1,7 @@
 #ifndef TAG_H
 #define TAG_H
 
+#include "git-compat-util.h"
 #include "object.h"
 
 extern const char *tag_type;
diff --git a/tree-walk.h b/tree-walk.h
index ae7fb3a..d7612cf 100644
--- a/tree-walk.h
+++ b/tree-walk.h
@@ -1,6 +1,8 @@
 #ifndef TREE_WALK_H
 #define TREE_WALK_H
 
+struct strbuf;
+
 struct name_entry {
 	const unsigned char *sha1;
 	const char *path;
diff --git a/tree.h b/tree.h
index d84ac63..bfeef4f 100644
--- a/tree.h
+++ b/tree.h
@@ -3,6 +3,8 @@
 
 #include "object.h"
 
+struct pathspec;
+
 extern const char *tree_type;
 
 struct tree {
diff --git a/utf8.h b/utf8.h
index 65d0e42..4955866 100644
--- a/utf8.h
+++ b/utf8.h
@@ -1,6 +1,10 @@
 #ifndef GIT_UTF8_H
 #define GIT_UTF8_H
 
+#include "git-compat-util.h"
+
+struct strbuf;
+
 typedef unsigned int ucs_char_t;  /* assuming 32bit int */
 
 size_t display_mode_esc_sequence_len(const char *s);
-- 
2.1.0.30.g0bdc89a

[RFC PATCH 3/3] core: improve header dependencies

[David Aguilar]

Remove includes that have already been included by another header.

Signed-off-by: David Aguilar <davvid@gmail.com>
---
This patch is the culmination of the previous patches and
is a pure removal.

 builtin/add.c           | 3 ---
 builtin/blame.c         | 4 ----
 builtin/branch.c        | 4 ----
 builtin/checkout.c      | 4 ----
 builtin/diff-files.c    | 3 ---
 builtin/diff-index.c    | 3 ---
 builtin/diff.c          | 3 ---
 builtin/fast-export.c   | 7 -------
 builtin/fmt-merge-msg.c | 5 -----
 builtin/log.c           | 6 ------
 builtin/merge-base.c    | 4 ----
 builtin/merge.c         | 5 -----
 builtin/pack-objects.c  | 4 ----
 builtin/prune.c         | 4 ----
 builtin/reflog.c        | 3 ---
 builtin/rev-list.c      | 3 ---
 builtin/rev-parse.c     | 4 ----
 builtin/revert.c        | 3 ---
 builtin/shortlog.c      | 5 -----
 builtin/tag.c           | 4 ----
 bundle.c                | 4 ----
 combine-diff.c          | 3 ---
 commit.c                | 5 -----
 diff-lib.c              | 3 ---
 diff-no-index.c         | 4 ----
 graph.c                 | 3 ---
 http-push.c             | 3 ---
 line-log.c              | 7 -------
 list-objects.c          | 4 ----
 pack-bitmap-write.c     | 3 ---
 pack-bitmap.c           | 3 ---
 pretty.c                | 5 -----
 reachable.c             | 3 ---
 reflog-walk.c           | 4 ----
 remote.c                | 4 ----
 revision.c              | 6 ------
 sequencer.c             | 4 ----
 shallow.c               | 3 ---
 submodule.c             | 4 ----
 test-revision-walking.c | 3 ---
 transport-helper.c      | 4 ----
 upload-pack.c           | 5 -----
 wt-status.c             | 5 -----
 43 files changed, 173 deletions(-)

diff --git a/builtin/add.c b/builtin/add.c
index 4baf3a5..3e898d2 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -3,15 +3,12 @@
  *
  * Copyright (C) 2006 Linus Torvalds
  */
-#include "cache.h"
 #include "builtin.h"
 #include "dir.h"
 #include "pathspec.h"
 #include "exec_cmd.h"
 #include "cache-tree.h"
 #include "run-command.h"
-#include "parse-options.h"
-#include "diff.h"
 #include "diffcore.h"
 #include "revision.h"
 #include "bulk-checkin.h"
diff --git a/builtin/blame.c b/builtin/blame.c
index 17d30d0..84e44ff 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -8,19 +8,15 @@
 #include "cache.h"
 #include "builtin.h"
 #include "blob.h"
-#include "commit.h"
 #include "tag.h"
 #include "tree-walk.h"
-#include "diff.h"
 #include "diffcore.h"
 #include "revision.h"
 #include "quote.h"
 #include "xdiff-interface.h"
 #include "cache-tree.h"
-#include "string-list.h"
 #include "mailmap.h"
 #include "mergesort.h"
-#include "parse-options.h"
 #include "prio-queue.h"
 #include "utf8.h"
 #include "userdiff.h"
diff --git a/builtin/branch.c b/builtin/branch.c
index 0591b22..f4adb93 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -8,14 +8,10 @@
 #include "cache.h"
 #include "color.h"
 #include "refs.h"
-#include "commit.h"
 #include "builtin.h"
 #include "remote.h"
-#include "parse-options.h"
 #include "branch.h"
-#include "diff.h"
 #include "revision.h"
-#include "string-list.h"
 #include "column.h"
 #include "utf8.h"
 #include "wt-status.h"
diff --git a/builtin/checkout.c b/builtin/checkout.c
index f71e745..bc53824 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -1,9 +1,6 @@
 #include "cache.h"
 #include "builtin.h"
-#include "parse-options.h"
 #include "refs.h"
-#include "commit.h"
-#include "tree.h"
 #include "tree-walk.h"
 #include "cache-tree.h"
 #include "unpack-trees.h"
@@ -11,7 +8,6 @@
 #include "run-command.h"
 #include "merge-recursive.h"
 #include "branch.h"
-#include "diff.h"
 #include "revision.h"
 #include "remote.h"
 #include "blob.h"
diff --git a/builtin/diff-files.c b/builtin/diff-files.c
index 9200069..d538a38 100644
--- a/builtin/diff-files.c
+++ b/builtin/diff-files.c
@@ -3,9 +3,6 @@
  *
  * Copyright (C) Linus Torvalds, 2005
  */
-#include "cache.h"
-#include "diff.h"
-#include "commit.h"
 #include "revision.h"
 #include "builtin.h"
 #include "submodule.h"
diff --git a/builtin/diff-index.c b/builtin/diff-index.c
index ce15b23..7e998b0 100644
--- a/builtin/diff-index.c
+++ b/builtin/diff-index.c
@@ -1,6 +1,3 @@
-#include "cache.h"
-#include "diff.h"
-#include "commit.h"
 #include "revision.h"
 #include "builtin.h"
 #include "submodule.h"
diff --git a/builtin/diff.c b/builtin/diff.c
index 0f247d2..aad7b98 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -3,12 +3,9 @@
  *
  * Copyright (c) 2006 Junio C Hamano
  */
-#include "cache.h"
 #include "color.h"
-#include "commit.h"
 #include "blob.h"
 #include "tag.h"
-#include "diff.h"
 #include "diffcore.h"
 #include "revision.h"
 #include "log-tree.h"
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 92b4624..71c4309 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -4,18 +4,11 @@
  * Copyright (C) 2007 Johannes E. Schindelin
  */
 #include "builtin.h"
-#include "cache.h"
-#include "commit.h"
-#include "object.h"
 #include "tag.h"
-#include "diff.h"
 #include "diffcore.h"
 #include "log-tree.h"
 #include "revision.h"
-#include "decorate.h"
-#include "string-list.h"
 #include "utf8.h"
-#include "parse-options.h"
 #include "quote.h"
 #include "remote.h"
 
diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c
index 79df05e..85a2ba1 100644
--- a/builtin/fmt-merge-msg.c
+++ b/builtin/fmt-merge-msg.c
@@ -1,13 +1,8 @@
 #include "builtin.h"
-#include "cache.h"
-#include "commit.h"
-#include "diff.h"
 #include "revision.h"
 #include "tag.h"
-#include "string-list.h"
 #include "branch.h"
 #include "fmt-merge-msg.h"
-#include "gpg-interface.h"
 
 static const char * const fmt_merge_msg_usage[] = {
 	N_("git fmt-merge-msg [-m <message>] [--log[=<n>]|--no-log] [--file <file>]"),
diff --git a/builtin/log.c b/builtin/log.c
index 4389722..ab1f11f 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -4,10 +4,7 @@
  * (C) Copyright 2006 Linus Torvalds
  *		 2006 Junio Hamano
  */
-#include "cache.h"
 #include "color.h"
-#include "commit.h"
-#include "diff.h"
 #include "revision.h"
 #include "log-tree.h"
 #include "builtin.h"
@@ -17,14 +14,11 @@
 #include "run-command.h"
 #include "shortlog.h"
 #include "remote.h"
-#include "string-list.h"
-#include "parse-options.h"
 #include "line-log.h"
 #include "branch.h"
 #include "streaming.h"
 #include "version.h"
 #include "mailmap.h"
-#include "gpg-interface.h"
 
 /* Set a default date-time format for git log ("log.date" config variable) */
 static const char *default_date_mode = NULL;
diff --git a/builtin/merge-base.c b/builtin/merge-base.c
index 0ecde8d..08b9ba5 100644
--- a/builtin/merge-base.c
+++ b/builtin/merge-base.c
@@ -1,10 +1,6 @@
 #include "builtin.h"
-#include "cache.h"
-#include "commit.h"
 #include "refs.h"
-#include "diff.h"
 #include "revision.h"
-#include "parse-options.h"
 
 static int show_merge_base(struct commit **rev, int rev_nr, int show_all)
 {
diff --git a/builtin/merge.c b/builtin/merge.c
index ce82eb2..4f15ae6 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -6,13 +6,9 @@
  * Based on git-merge.sh by Junio C Hamano.
  */
 
-#include "cache.h"
-#include "parse-options.h"
 #include "builtin.h"
 #include "run-command.h"
-#include "diff.h"
 #include "refs.h"
-#include "commit.h"
 #include "diffcore.h"
 #include "revision.h"
 #include "unpack-trees.h"
@@ -27,7 +23,6 @@
 #include "resolve-undo.h"
 #include "remote.h"
 #include "fmt-merge-msg.h"
-#include "gpg-interface.h"
 
 #define DEFAULT_TWOHEAD (1<<0)
 #define DEFAULT_OCTOPUS (1<<1)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index b59f5d8..a0422a0 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -1,9 +1,6 @@
 #include "builtin.h"
-#include "cache.h"
 #include "attr.h"
-#include "object.h"
 #include "blob.h"
-#include "commit.h"
 #include "tag.h"
 #include "tree.h"
 #include "delta.h"
@@ -11,7 +8,6 @@
 #include "pack-revindex.h"
 #include "csum-file.h"
 #include "tree-walk.h"
-#include "diff.h"
 #include "revision.h"
 #include "list-objects.h"
 #include "pack-objects.h"
diff --git a/builtin/prune.c b/builtin/prune.c
index 144a3bd..c7c3472 100644
--- a/builtin/prune.c
+++ b/builtin/prune.c
@@ -1,10 +1,6 @@
-#include "cache.h"
-#include "commit.h"
-#include "diff.h"
 #include "revision.h"
 #include "builtin.h"
 #include "reachable.h"
-#include "parse-options.h"
 #include "progress.h"
 #include "dir.h"
 
diff --git a/builtin/reflog.c b/builtin/reflog.c
index e8a8fb1..6a55904 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -1,10 +1,7 @@
-#include "cache.h"
 #include "builtin.h"
-#include "commit.h"
 #include "refs.h"
 #include "dir.h"
 #include "tree-walk.h"
-#include "diff.h"
 #include "revision.h"
 #include "reachable.h"
 
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index ff84a82..7165d4d 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -1,6 +1,3 @@
-#include "cache.h"
-#include "commit.h"
-#include "diff.h"
 #include "revision.h"
 #include "list-objects.h"
 #include "pack.h"
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index d85e08c..959d316 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -3,13 +3,9 @@
  *
  * Copyright (C) Linus Torvalds, 2005
  */
-#include "cache.h"
-#include "commit.h"
 #include "refs.h"
 #include "quote.h"
 #include "builtin.h"
-#include "parse-options.h"
-#include "diff.h"
 #include "revision.h"
 #include "split-index.h"
 
diff --git a/builtin/revert.c b/builtin/revert.c
index f9ed5bd..51e78a6 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -1,7 +1,4 @@
-#include "cache.h"
 #include "builtin.h"
-#include "parse-options.h"
-#include "diff.h"
 #include "revision.h"
 #include "rerere.h"
 #include "dir.h"
diff --git a/builtin/shortlog.c b/builtin/shortlog.c
index 4b7e536..4a20169 100644
--- a/builtin/shortlog.c
+++ b/builtin/shortlog.c
@@ -1,13 +1,8 @@
 #include "builtin.h"
-#include "cache.h"
-#include "commit.h"
-#include "diff.h"
-#include "string-list.h"
 #include "revision.h"
 #include "utf8.h"
 #include "mailmap.h"
 #include "shortlog.h"
-#include "parse-options.h"
 
 static char const * const shortlog_usage[] = {
 	N_("git shortlog [<options>] [<revision range>] [[--] [<path>...]]"),
diff --git a/builtin/tag.c b/builtin/tag.c
index 19eb747..4776499 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -6,15 +6,11 @@
  * Based on git-tag.sh and mktag.c by Linus Torvalds.
  */
 
-#include "cache.h"
 #include "builtin.h"
 #include "refs.h"
 #include "tag.h"
 #include "run-command.h"
-#include "parse-options.h"
-#include "diff.h"
 #include "revision.h"
-#include "gpg-interface.h"
 #include "sha1-array.h"
 #include "column.h"
 
diff --git a/bundle.c b/bundle.c
index 71a21a6..bd14989 100644
--- a/bundle.c
+++ b/bundle.c
@@ -1,8 +1,4 @@
-#include "cache.h"
 #include "bundle.h"
-#include "object.h"
-#include "commit.h"
-#include "diff.h"
 #include "revision.h"
 #include "list-objects.h"
 #include "run-command.h"
diff --git a/combine-diff.c b/combine-diff.c
index 60cb4f8..98d2f0b 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -1,7 +1,4 @@
-#include "cache.h"
-#include "commit.h"
 #include "blob.h"
-#include "diff.h"
 #include "diffcore.h"
 #include "quote.h"
 #include "xdiff-interface.h"
diff --git a/commit.c b/commit.c
index 4de6be4..0c311c4 100644
--- a/commit.c
+++ b/commit.c
@@ -1,12 +1,7 @@
-#include "cache.h"
 #include "tag.h"
-#include "commit.h"
 #include "pkt-line.h"
 #include "utf8.h"
-#include "diff.h"
 #include "revision.h"
-#include "notes.h"
-#include "gpg-interface.h"
 #include "mergesort.h"
 #include "commit-slab.h"
 #include "prio-queue.h"
diff --git a/diff-lib.c b/diff-lib.c
index 875aff8..9816d7e 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -1,10 +1,7 @@
 /*
  * Copyright (C) 2005 Junio C Hamano
  */
-#include "cache.h"
 #include "quote.h"
-#include "commit.h"
-#include "diff.h"
 #include "diffcore.h"
 #include "revision.h"
 #include "cache-tree.h"
diff --git a/diff-no-index.c b/diff-no-index.c
index 265709b..1bbca98 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c
@@ -4,17 +4,13 @@
  * Copyright (c) 2008 by Junio C Hamano
  */
 
-#include "cache.h"
 #include "color.h"
-#include "commit.h"
 #include "blob.h"
 #include "tag.h"
-#include "diff.h"
 #include "diffcore.h"
 #include "revision.h"
 #include "log-tree.h"
 #include "builtin.h"
-#include "string-list.h"
 #include "dir.h"
 
 static int read_directory_contents(const char *path, struct string_list *list)
diff --git a/graph.c b/graph.c
index 6404331..cffaf2f 100644
--- a/graph.c
+++ b/graph.c
@@ -1,8 +1,5 @@
-#include "cache.h"
-#include "commit.h"
 #include "color.h"
 #include "graph.h"
-#include "diff.h"
 #include "revision.h"
 
 /* Internal API */
diff --git a/http-push.c b/http-push.c
index 952f8ed..4877efe 100644
--- a/http-push.c
+++ b/http-push.c
@@ -1,10 +1,7 @@
-#include "cache.h"
-#include "commit.h"
 #include "tag.h"
 #include "blob.h"
 #include "http.h"
 #include "refs.h"
-#include "diff.h"
 #include "revision.h"
 #include "exec_cmd.h"
 #include "remote.h"
diff --git a/line-log.c b/line-log.c
index 1008e72..a026ce0 100644
--- a/line-log.c
+++ b/line-log.c
@@ -1,15 +1,8 @@
-#include "git-compat-util.h"
 #include "line-range.h"
-#include "cache.h"
 #include "tag.h"
 #include "blob.h"
-#include "tree.h"
-#include "diff.h"
-#include "commit.h"
-#include "decorate.h"
 #include "revision.h"
 #include "xdiff-interface.h"
-#include "strbuf.h"
 #include "log-tree.h"
 #include "graph.h"
 #include "userdiff.h"
diff --git a/list-objects.c b/list-objects.c
index 3595ee7..7f1a55e 100644
--- a/list-objects.c
+++ b/list-objects.c
@@ -1,9 +1,5 @@
-#include "cache.h"
 #include "tag.h"
-#include "commit.h"
-#include "tree.h"
 #include "blob.h"
-#include "diff.h"
 #include "tree-walk.h"
 #include "revision.h"
 #include "list-objects.h"
diff --git a/pack-bitmap-write.c b/pack-bitmap-write.c
index 5f1791a..f2530df 100644
--- a/pack-bitmap-write.c
+++ b/pack-bitmap-write.c
@@ -1,7 +1,4 @@
-#include "cache.h"
-#include "commit.h"
 #include "tag.h"
-#include "diff.h"
 #include "revision.h"
 #include "list-objects.h"
 #include "progress.h"
diff --git a/pack-bitmap.c b/pack-bitmap.c
index 91e4101..a250c6e 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -1,7 +1,4 @@
-#include "cache.h"
-#include "commit.h"
 #include "tag.h"
-#include "diff.h"
 #include "revision.h"
 #include "progress.h"
 #include "list-objects.h"
diff --git a/pretty.c b/pretty.c
index 3a1da6f..fde24ab 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1,15 +1,10 @@
-#include "cache.h"
-#include "commit.h"
 #include "utf8.h"
-#include "diff.h"
 #include "revision.h"
-#include "string-list.h"
 #include "mailmap.h"
 #include "log-tree.h"
 #include "notes.h"
 #include "color.h"
 #include "reflog-walk.h"
-#include "gpg-interface.h"
 
 static char *user_format;
 static struct cmt_fmt_map {
diff --git a/reachable.c b/reachable.c
index 654a8c5..819de18 100644
--- a/reachable.c
+++ b/reachable.c
@@ -1,9 +1,6 @@
-#include "cache.h"
 #include "refs.h"
 #include "tag.h"
-#include "commit.h"
 #include "blob.h"
-#include "diff.h"
 #include "revision.h"
 #include "reachable.h"
 #include "cache-tree.h"
diff --git a/reflog-walk.c b/reflog-walk.c
index 0e5174b..a21529a 100644
--- a/reflog-walk.c
+++ b/reflog-walk.c
@@ -1,9 +1,5 @@
-#include "cache.h"
-#include "commit.h"
 #include "refs.h"
-#include "diff.h"
 #include "revision.h"
-#include "string-list.h"
 #include "reflog-walk.h"
 
 struct complete_reflogs {
diff --git a/remote.c b/remote.c
index 3d6c86a..682901e 100644
--- a/remote.c
+++ b/remote.c
@@ -1,12 +1,8 @@
-#include "cache.h"
 #include "remote.h"
 #include "refs.h"
-#include "commit.h"
-#include "diff.h"
 #include "revision.h"
 #include "dir.h"
 #include "tag.h"
-#include "string-list.h"
 #include "mergesort.h"
 
 enum map_direction { FROM_SRC, FROM_DST };
diff --git a/revision.c b/revision.c
index 2571ada..5de4299 100644
--- a/revision.c
+++ b/revision.c
@@ -1,18 +1,12 @@
-#include "cache.h"
 #include "tag.h"
 #include "blob.h"
-#include "tree.h"
-#include "commit.h"
-#include "diff.h"
 #include "refs.h"
 #include "revision.h"
 #include "graph.h"
 #include "grep.h"
 #include "reflog-walk.h"
 #include "patch-ids.h"
-#include "decorate.h"
 #include "log-tree.h"
-#include "string-list.h"
 #include "line-log.h"
 #include "mailmap.h"
 #include "commit-slab.h"
diff --git a/sequencer.c b/sequencer.c
index 3c060e0..c051d6c 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1,14 +1,10 @@
-#include "cache.h"
 #include "sequencer.h"
 #include "dir.h"
-#include "object.h"
-#include "commit.h"
 #include "tag.h"
 #include "run-command.h"
 #include "exec_cmd.h"
 #include "utf8.h"
 #include "cache-tree.h"
-#include "diff.h"
 #include "revision.h"
 #include "rerere.h"
 #include "merge-recursive.h"
diff --git a/shallow.c b/shallow.c
index de07709..3a0b0be 100644
--- a/shallow.c
+++ b/shallow.c
@@ -1,11 +1,8 @@
-#include "cache.h"
-#include "commit.h"
 #include "tag.h"
 #include "pkt-line.h"
 #include "remote.h"
 #include "refs.h"
 #include "sha1-array.h"
-#include "diff.h"
 #include "revision.h"
 #include "commit-slab.h"
 #include "sigchain.h"
diff --git a/submodule.c b/submodule.c
index c3a61e7..0ef37bf 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1,13 +1,9 @@
-#include "cache.h"
 #include "submodule.h"
 #include "dir.h"
-#include "diff.h"
-#include "commit.h"
 #include "revision.h"
 #include "run-command.h"
 #include "diffcore.h"
 #include "refs.h"
-#include "string-list.h"
 #include "sha1-array.h"
 #include "argv-array.h"
 #include "blob.h"
diff --git a/test-revision-walking.c b/test-revision-walking.c
index 3ade02c..5e25896 100644
--- a/test-revision-walking.c
+++ b/test-revision-walking.c
@@ -8,9 +8,6 @@
  * published by the Free Software Foundation.
  */
 
-#include "cache.h"
-#include "commit.h"
-#include "diff.h"
 #include "revision.h"
 
 static void print_commit(struct commit *commit)
diff --git a/transport-helper.c b/transport-helper.c
index 3d8fe7d..02ecc9f 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -1,13 +1,9 @@
-#include "cache.h"
 #include "transport.h"
 #include "quote.h"
 #include "run-command.h"
-#include "commit.h"
-#include "diff.h"
 #include "revision.h"
 #include "quote.h"
 #include "remote.h"
-#include "string-list.h"
 #include "thread-utils.h"
 #include "sigchain.h"
 #include "argv-array.h"
diff --git a/upload-pack.c b/upload-pack.c
index 01de944..b4e6565 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -1,19 +1,14 @@
-#include "cache.h"
 #include "refs.h"
 #include "pkt-line.h"
 #include "sideband.h"
 #include "tag.h"
-#include "object.h"
-#include "commit.h"
 #include "exec_cmd.h"
-#include "diff.h"
 #include "revision.h"
 #include "list-objects.h"
 #include "run-command.h"
 #include "connect.h"
 #include "sigchain.h"
 #include "version.h"
-#include "string-list.h"
 
 static const char upload_pack_usage[] = "git upload-pack [--strict] [--timeout=<n>] <dir>";
 
diff --git a/wt-status.c b/wt-status.c
index 27da529..37bc32e 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1,10 +1,6 @@
-#include "cache.h"
 #include "pathspec.h"
 #include "wt-status.h"
-#include "object.h"
 #include "dir.h"
-#include "commit.h"
-#include "diff.h"
 #include "revision.h"
 #include "diffcore.h"
 #include "quote.h"
@@ -14,7 +10,6 @@
 #include "refs.h"
 #include "submodule.h"
 #include "column.h"
-#include "strbuf.h"
 #include "utf8.h"
 
 static const char cut_line[] =
-- 
2.1.0.30.g0bdc89a

Re: [RFC PATCH 2/3] headers: improve header dependencies

[Junio C Hamano]

David Aguilar <davvid@gmail.com> writes:

> Add missing includes or forward declarations where needed.

Sorry, but I am not sure what the "missing includes" refers to in
the above.

As far as I know, we never aimed to make "gcc -c $header.h" happy.
I suspect that that is what you are trying to do here?

Re: [RFC PATCH 3/3] core: improve header dependencies

[Junio C Hamano]

David Aguilar <davvid@gmail.com> writes:

> Remove includes that have already been included by another header.

Hmm, I am not sure if that is a good move, and suspect that it is
incompatible with what your 2/3 attempts to do, at least at the
philosophical level.

I am guessing that your 2/3 wants to see

	gcc $header.h

to be happy.  One benefit from doing such a change is that sources
that want to use declaration made in $header.h have to include that
$header.h without having to worry about what other things the
implementation detail of $header.h needs.  If function F or type T
is declared in header H, you include H and you are done.

That is nice and tidy, but if that is the goal, then after making H
include its own dependency H1 that happen to declare functions F1,
F2 and types T1, T2 (which are necessary for H to be complete as
standalone), if the source that used to include both H and H1
because it uses F and F1 should still explicitly include H1, no?

For example, you dropped "diff.h" from builtin/add.c, but the
implementation of builtin/add.c needs access to diff_options struct,
which is in "diff.h", not whatever happened to include indirectly
that is already included by builtin/add.c.  I do not think it is a
good idea, and more importantly I suspect that it is not consistent
with what you tried to do with your 2/3.

But it is entirely possible I am misunderstanding the real
motivation behind these changes.  The log message justifies why
removal is safe i.e. "have already been included indirectly", and
the title claims it is an improvement, but there is no explanation
why it is an improvement (which would have also explained the
motivation behind it), so it is a bit hard for me to guess.

Re: [RFC PATCH 1/3] stylefix: pointers bind to the variable, not the type

[Junio C Hamano]

Thanks.

Re: [RFC PATCH 3/3] core: improve header dependencies

[David Aguilar]

On Tue, Sep 02, 2014 at 11:32:02AM -0700, Junio C Hamano wrote:
> David Aguilar <davvid@gmail.com> writes:
> 
> > Remove includes that have already been included by another header.
> 
> Hmm, I am not sure if that is a good move, and suspect that it is
> incompatible with what your 2/3 attempts to do, at least at the
> philosophical level.
> 
> I am guessing that your 2/3 wants to see
> 
> 	gcc $header.h
> 
> to be happy.  One benefit from doing such a change is that sources
> that want to use declaration made in $header.h have to include that
> $header.h without having to worry about what other things the
> implementation detail of $header.h needs.  If function F or type T
> is declared in header H, you include H and you are done.
> 
> That is nice and tidy, but if that is the goal, then after making H
> include its own dependency H1 that happen to declare functions F1,
> F2 and types T1, T2 (which are necessary for H to be complete as
> standalone), if the source that used to include both H and H1
> because it uses F and F1 should still explicitly include H1, no?
> 
> For example, you dropped "diff.h" from builtin/add.c, but the
> implementation of builtin/add.c needs access to diff_options struct,
> which is in "diff.h", not whatever happened to include indirectly
> that is already included by builtin/add.c.  I do not think it is a
> good idea, and more importantly I suspect that it is not consistent
> with what you tried to do with your 2/3.
> 
> But it is entirely possible I am misunderstanding the real
> motivation behind these changes.  The log message justifies why
> removal is safe i.e. "have already been included indirectly", and
> the title claims it is an improvement, but there is no explanation
> why it is an improvement (which would have also explained the
> motivation behind it), so it is a bit hard for me to guess.

The commit messages can certainly be improved.

Patch 2/3 is really a question in patch form:

Should we (a) forward-declare structs in headers that use
pointers to those structs, or (b) full-on #include the headers
that define those structs?

Making "gcc $header.h" happy might be good but that wasn't the
original motivation; approach (b) would satisfy that.

This old (and maybe uneditable these days?) wiki page contains
items that were the motivation for these patches:

https://git.wiki.kernel.org/index.php/Janitor

- Some Git headers depend on other headers to compile cleanly,
  in this case it might be a good idea to include the needed
  headers in the header that needs them."

- For example "revision.h" depends on "commit.h" and "diff.h",
  so "revision.h" should include them.

- Of course after that it makes sense to clean up the "*.c"
  source files that include all these headers, to remove the
  headers that are no more needed there.

Most importantly, though, is this part:

- Contact the mail list to see how to proceed.

;-)

Patch 3/3 is the item about cleaning up *.c files, but we may
want to reformulate these items and take a different approach.

Thoughts?
-- 
David