[PATCH 0/9] Add missing includes and forward declares

[PATCH 0/9] Add missing includes and forward declares

[Elijah Newren]

I was bit yet again by compilation errors when adding a #include of
some header file to some new place, and decided to determine which
header files were missing their own necessary #include's and forward
declarations.  This patch series is the result.  A few notes:

  * Patch 1: This patch is the bulk of the work; I included everything
    here except for special cases that need extra attention and the
    unrelated change in patch 2.

  * Patch 2: This patch is more of a while-at-it independent header
    fixup; it only changes two lines so I lumped it into the series.

  * Patch 3: I could have just added a #include of cache.h in branch.h
    instead, but opted to move things around slightly.  I'm unsure
    which is better.

  * Patches 4-5: Fix include guard issues

  * Patches 6-8: These patches might need to be submitted to separate
    projects elsewhere.  Let me know if so.

  * Patch 9: This patch includes the changes that conflict with other
    topics in next and pu.  I included these changes for completeness,
    but kept it separate because I wanted to make it easy to drop the
    stuff that conflicted with other things in flight (since I can
    just resubmit it later).

Elijah Newren (9):
  Add missing includes and forward declares
  alloc: make allocate_alloc_state and clear_alloc_state more consistent
  Move definition of enum branch_track from cache.h to branch.h
  urlmatch.h: fix include guard
  compat/precompose_utf8.h: use more common include guard style
  ewah/ewok.h: add missing include
  sha1dc/sha1.h: add missing include
  xdiff/xdiff.h: add missing include
  Add missing includes and forward declares

 alloc.c                  |  2 +-
 alloc.h                  |  4 +++-
 apply.h                  |  3 +++
 archive.h                |  4 ++++
 argv-array.h             |  2 ++
 attr.h                   |  1 +
 bisect.h                 |  2 ++
 branch.h                 | 13 +++++++++++++
 bulk-checkin.h           |  2 ++
 cache.h                  | 10 ----------
 color.h                  |  2 ++
 column.h                 |  1 +
 commit-graph.h           |  1 +
 compat/precompose_utf8.h |  3 ++-
 config.c                 |  1 +
 config.h                 |  6 ++++++
 connected.h              |  1 +
 convert.h                |  3 +++
 credential.h             |  1 +
 csum-file.h              |  2 ++
 delta.h                  |  2 ++
 diffcore.h               |  4 ++++
 dir-iterator.h           |  2 ++
 environment.c            |  1 +
 ewah/ewok.h              |  2 ++
 exec-cmd.h               |  2 ++
 fsck.h                   |  1 +
 fsmonitor.h              |  7 +++++++
 gpg-interface.h          |  4 ++++
 grep.h                   |  1 +
 hashmap.h                |  2 ++
 khash.h                  |  4 ++++
 kwset.h                  |  2 ++
 list-objects-filter.h    |  4 ++++
 list-objects.h           |  4 ++++
 ll-merge.h               |  2 ++
 mailinfo.h               |  2 ++
 mailmap.h                |  4 ++++
 mem-pool.h               |  2 ++
 merge-recursive.h        |  4 +++-
 notes-cache.h            |  1 +
 notes-merge.h            |  4 ++++
 notes-utils.h            |  4 ++++
 notes.h                  |  3 +++
 object-store.h           |  1 +
 object.h                 |  2 ++
 oidmap.h                 |  1 +
 pack-bitmap.h            |  3 +++
 pack-objects.h           |  1 +
 pack-revindex.h          |  2 ++
 parse-options.h          |  2 ++
 patch-ids.h              |  6 ++++++
 path.h                   |  3 +++
 pathspec.h               |  5 +++++
 pretty.h                 |  4 ++++
 progress.h               |  2 ++
 quote.h                  |  2 ++
 reachable.h              |  4 ++++
 reflog-walk.h            |  1 +
 refs.h                   |  4 ++++
 remote.h                 |  1 +
 repository.h             |  2 ++
 resolve-undo.h           |  2 ++
 revision.h               |  1 +
 send-pack.h              |  4 ++++
 sequencer.h              |  5 +++++
 sha1-lookup.h            |  2 ++
 sha1dc/sha1.h            |  1 +
 shortlog.h               |  3 +++
 sideband.h               |  2 ++
 strbuf.h                 |  2 ++
 submodule.h              | 10 ++++++++--
 tempfile.h               |  1 +
 trailer.h                |  3 +++
 tree-walk.h              |  2 ++
 unpack-trees.h           |  5 ++++-
 url.h                    |  2 ++
 urlmatch.h               |  3 +++
 utf8.h                   |  4 ++++
 varint.h                 |  2 ++
 worktree.h               |  1 +
 xdiff/xdiff.h            |  2 ++
 82 files changed, 218 insertions(+), 17 deletions(-)

-- 
2.18.0.549.gd4454f3f9b

[PATCH 1/9] Add missing includes and forward declares

[Elijah Newren]

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 alloc.h               |  2 ++
 apply.h               |  3 +++
 archive.h             |  4 ++++
 argv-array.h          |  2 ++
 attr.h                |  1 +
 branch.h              |  2 ++
 bulk-checkin.h        |  2 ++
 color.h               |  2 ++
 column.h              |  1 +
 commit-graph.h        |  1 +
 config.h              |  6 ++++++
 connected.h           |  1 +
 convert.h             |  3 +++
 credential.h          |  1 +
 csum-file.h           |  2 ++
 delta.h               |  2 ++
 diffcore.h            |  4 ++++
 dir-iterator.h        |  2 ++
 exec-cmd.h            |  2 ++
 fsck.h                |  1 +
 fsmonitor.h           |  7 +++++++
 gpg-interface.h       |  4 ++++
 grep.h                |  1 +
 hashmap.h             |  2 ++
 khash.h               |  4 ++++
 kwset.h               |  2 ++
 list-objects-filter.h |  4 ++++
 list-objects.h        |  4 ++++
 ll-merge.h            |  2 ++
 mailinfo.h            |  2 ++
 mailmap.h             |  4 ++++
 mem-pool.h            |  2 ++
 merge-recursive.h     |  4 +++-
 notes-cache.h         |  1 +
 notes-merge.h         |  4 ++++
 notes-utils.h         |  4 ++++
 notes.h               |  3 +++
 object-store.h        |  1 +
 object.h              |  2 ++
 oidmap.h              |  1 +
 pack-bitmap.h         |  3 +++
 pack-revindex.h       |  2 ++
 parse-options.h       |  2 ++
 patch-ids.h           |  6 ++++++
 path.h                |  3 +++
 pathspec.h            |  5 +++++
 pretty.h              |  4 ++++
 progress.h            |  2 ++
 quote.h               |  2 ++
 reachable.h           |  4 ++++
 reflog-walk.h         |  1 +
 refs.h                |  4 ++++
 remote.h              |  1 +
 repository.h          |  2 ++
 resolve-undo.h        |  2 ++
 revision.h            |  1 +
 send-pack.h           |  4 ++++
 sequencer.h           |  5 +++++
 sha1-lookup.h         |  2 ++
 shortlog.h            |  3 +++
 sideband.h            |  2 ++
 strbuf.h              |  2 ++
 submodule.h           | 10 ++++++++--
 tempfile.h            |  1 +
 trailer.h             |  3 +++
 tree-walk.h           |  2 ++
 unpack-trees.h        |  5 ++++-
 url.h                 |  2 ++
 urlmatch.h            |  1 +
 utf8.h                |  4 ++++
 varint.h              |  2 ++
 worktree.h            |  1 +
 72 files changed, 191 insertions(+), 4 deletions(-)

diff --git a/alloc.h b/alloc.h
index 3e4e828db4..7a41bb9eb3 100644
--- a/alloc.h
+++ b/alloc.h
@@ -1,9 +1,11 @@
 #ifndef ALLOC_H
 #define ALLOC_H
 
+struct alloc_state;
 struct tree;
 struct commit;
 struct tag;
+struct repository;
 
 void *alloc_blob_node(struct repository *r);
 void *alloc_tree_node(struct repository *r);
diff --git a/apply.h b/apply.h
index b80d8ba736..0b70e1f3f5 100644
--- a/apply.h
+++ b/apply.h
@@ -1,6 +1,9 @@
 #ifndef APPLY_H
 #define APPLY_H
 
+#include "lockfile.h"
+#include "string-list.h"
+
 enum apply_ws_error_action {
 	nowarn_ws_error,
 	warn_on_ws_error,
diff --git a/archive.h b/archive.h
index 1f9954f7cd..3455dde54d 100644
--- a/archive.h
+++ b/archive.h
@@ -1,8 +1,12 @@
 #ifndef ARCHIVE_H
 #define ARCHIVE_H
 
+#include "git-compat-util.h" /* For size_t, timestamp_t, etc. */
 #include "pathspec.h"
 
+struct object_id;
+enum object_type;
+
 struct archiver_args {
 	const char *base;
 	size_t baselen;
diff --git a/argv-array.h b/argv-array.h
index a39ba43f57..c46238784c 100644
--- a/argv-array.h
+++ b/argv-array.h
@@ -1,6 +1,8 @@
 #ifndef ARGV_ARRAY_H
 #define ARGV_ARRAY_H
 
+#include "git-compat-util.h"   /* for LAST_ARG_MUST_BE_NULL */
+
 extern const char *empty_argv[];
 
 struct argv_array {
diff --git a/attr.h b/attr.h
index 442d464db6..3185538bda 100644
--- a/attr.h
+++ b/attr.h
@@ -7,6 +7,7 @@ struct git_attr;
 /* opaque structures used internally for attribute collection */
 struct all_attrs_item;
 struct attr_stack;
+struct index_state;
 
 /*
  * Given a string, return the gitattribute object that
diff --git a/branch.h b/branch.h
index 473d0a93e9..7d9b330eba 100644
--- a/branch.h
+++ b/branch.h
@@ -1,6 +1,8 @@
 #ifndef BRANCH_H
 #define BRANCH_H
 
+struct strbuf;
+
 /* Functions for acting on the information about branches. */
 
 /*
diff --git a/bulk-checkin.h b/bulk-checkin.h
index a85527318b..e036231636 100644
--- a/bulk-checkin.h
+++ b/bulk-checkin.h
@@ -4,6 +4,8 @@
 #ifndef BULK_CHECKIN_H
 #define BULK_CHECKIN_H
 
+#include "cache.h" /* for object_type */
+
 extern int index_bulk_checkin(struct object_id *oid,
 			      int fd, size_t size, enum object_type type,
 			      const char *path, unsigned flags);
diff --git a/color.h b/color.h
index 5b744e1bc6..74edbffc9d 100644
--- a/color.h
+++ b/color.h
@@ -1,6 +1,8 @@
 #ifndef COLOR_H
 #define COLOR_H
 
+#include <stdio.h>
+
 struct strbuf;
 
 /*
diff --git a/column.h b/column.h
index 0a61917fa7..2567a5cf4d 100644
--- a/column.h
+++ b/column.h
@@ -36,6 +36,7 @@ static inline int column_active(unsigned int colopts)
 	return (colopts & COL_ENABLE_MASK) == COL_ENABLED;
 }
 
+struct string_list;
 extern void print_columns(const struct string_list *list, unsigned int colopts,
 			  const struct column_options *opts);
 
diff --git a/commit-graph.h b/commit-graph.h
index 76e098934a..b006e1b415 100644
--- a/commit-graph.h
+++ b/commit-graph.h
@@ -4,6 +4,7 @@
 #include "git-compat-util.h"
 #include "repository.h"
 #include "string-list.h"
+#include "cache.h" /* for object_id */
 
 struct commit;
 
diff --git a/config.h b/config.h
index bb2f506b27..ff14cc5d1c 100644
--- a/config.h
+++ b/config.h
@@ -1,6 +1,12 @@
 #ifndef CONFIG_H
 #define CONFIG_H
 
+#include "git-compat-util.h"  /* for size_t, off_t, timestamp_t, etc. */
+#include "hashmap.h"
+#include "string-list.h"
+
+struct object_id;
+
 /* git_config_parse_key() returns these negated: */
 #define CONFIG_INVALID_KEY 1
 #define CONFIG_NO_SECTION_OR_NAME 2
diff --git a/connected.h b/connected.h
index 322dc76372..e4c961817d 100644
--- a/connected.h
+++ b/connected.h
@@ -1,6 +1,7 @@
 #ifndef CONNECTED_H
 #define CONNECTED_H
 
+struct object_id;
 struct transport;
 
 /*
diff --git a/convert.h b/convert.h
index 01385d9288..b3b5a783b3 100644
--- a/convert.h
+++ b/convert.h
@@ -4,9 +4,12 @@
 #ifndef CONVERT_H
 #define CONVERT_H
 
+#include <stdlib.h>  /* for size_t */
 #include "string-list.h"
 
 struct index_state;
+struct object_id;
+struct strbuf;
 
 #define CONV_EOL_RNDTRP_DIE   (1<<0) /* Die if CRLF to LF to CRLF is different */
 #define CONV_EOL_RNDTRP_WARN  (1<<1) /* Warn if CRLF to LF to CRLF is different */
diff --git a/credential.h b/credential.h
index 6b0cd16be2..2b5ef76e43 100644
--- a/credential.h
+++ b/credential.h
@@ -1,6 +1,7 @@
 #ifndef CREDENTIAL_H
 #define CREDENTIAL_H
 
+#include <stdio.h>
 #include "string-list.h"
 
 struct credential {
diff --git a/csum-file.h b/csum-file.h
index c5a2e335e7..3bf7184736 100644
--- a/csum-file.h
+++ b/csum-file.h
@@ -1,6 +1,8 @@
 #ifndef CSUM_FILE_H
 #define CSUM_FILE_H
 
+#include "hash.h"
+
 struct progress;
 
 /* A SHA1-protected file */
diff --git a/delta.h b/delta.h
index 9b67531dfa..04ab4ba797 100644
--- a/delta.h
+++ b/delta.h
@@ -1,6 +1,8 @@
 #ifndef DELTA_H
 #define DELTA_H
 
+#include <stdlib.h>  /* for NULL */
+
 /* opaque object for delta index */
 struct delta_index;
 
diff --git a/diffcore.h b/diffcore.h
index a30da161da..5b9f80d1e6 100644
--- a/diffcore.h
+++ b/diffcore.h
@@ -4,6 +4,10 @@
 #ifndef DIFFCORE_H
 #define DIFFCORE_H
 
+#include "cache.h" /* for object_id */
+
+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-iterator.h b/dir-iterator.h
index 27739e6c29..970793d07a 100644
--- a/dir-iterator.h
+++ b/dir-iterator.h
@@ -1,6 +1,8 @@
 #ifndef DIR_ITERATOR_H
 #define DIR_ITERATOR_H
 
+#include "strbuf.h"
+
 /*
  * Iterate over a directory tree.
  *
diff --git a/exec-cmd.h b/exec-cmd.h
index 2522453cda..8737d84f3b 100644
--- a/exec-cmd.h
+++ b/exec-cmd.h
@@ -1,6 +1,8 @@
 #ifndef GIT_EXEC_CMD_H
 #define GIT_EXEC_CMD_H
 
+#include "git-compat-util.h"   /* for LAST_ARG_MUST_BE_NULL */
+
 struct argv_array;
 
 extern void git_set_exec_path(const char *exec_path);
diff --git a/fsck.h b/fsck.h
index c3cf5e0034..0c7e8c9428 100644
--- a/fsck.h
+++ b/fsck.h
@@ -6,6 +6,7 @@
 #define FSCK_IGNORE 3
 
 struct fsck_options;
+struct object;
 
 void fsck_set_msg_type(struct fsck_options *options,
 		const char *msg_id, const char *msg_type);
diff --git a/fsmonitor.h b/fsmonitor.h
index 65f3743636..09067abae5 100644
--- a/fsmonitor.h
+++ b/fsmonitor.h
@@ -1,6 +1,13 @@
 #ifndef FSMONITOR_H
 #define FSMONITOR_H
 
+#include "cache.h" /* for core_fsmonitor */
+#include "dir.h"   /* for untracked_cache_invalidate_path */
+
+struct cache_entry;
+struct index_state;
+struct strbuf;
+
 extern struct trace_key trace_fsmonitor;
 
 /*
diff --git a/gpg-interface.h b/gpg-interface.h
index 5ecff4aa0c..11e05e765b 100644
--- a/gpg-interface.h
+++ b/gpg-interface.h
@@ -1,6 +1,10 @@
 #ifndef GPG_INTERFACE_H
 #define GPG_INTERFACE_H
 
+#include <stdlib.h>
+
+struct strbuf;
+
 #define GPG_VERIFY_VERBOSE		1
 #define GPG_VERIFY_RAW			2
 #define GPG_VERIFY_OMIT_STATUS	4
diff --git a/grep.h b/grep.h
index 0ba62a11c5..0167a1054a 100644
--- a/grep.h
+++ b/grep.h
@@ -35,6 +35,7 @@ typedef int pcre2_jit_stack;
 #include "kwset.h"
 #include "thread-utils.h"
 #include "userdiff.h"
+#include "git-compat-util.h" /* for regex_t and uint32_t */
 
 enum grep_pat_token {
 	GREP_PATTERN,
diff --git a/hashmap.h b/hashmap.h
index d375d9cce7..855bbb5b5a 100644
--- a/hashmap.h
+++ b/hashmap.h
@@ -1,6 +1,8 @@
 #ifndef HASHMAP_H
 #define HASHMAP_H
 
+#include "git-compat-util.h" /* for BUG, size_t, memcpy, etc. */
+
 /*
  * Generic implementation of hash-based key-value mappings.
  *
diff --git a/khash.h b/khash.h
index c0da40daa7..c7e37790bb 100644
--- a/khash.h
+++ b/khash.h
@@ -26,6 +26,10 @@
 #ifndef __AC_KHASH_H
 #define __AC_KHASH_H
 
+#include "git-compat-util.h" /* for xcalloc, etc. */
+#include "hashmap.h" /* for sha1hash */
+#include "cache.h"   /* for hashcmp */
+
 #define AC_VERSION_KHASH_H "0.2.8"
 
 typedef uint32_t khint32_t;
diff --git a/kwset.h b/kwset.h
index 583f6268ef..d1b1835095 100644
--- a/kwset.h
+++ b/kwset.h
@@ -3,6 +3,8 @@
  * Git.
  */
 
+#include <stdlib.h>
+
 /* kwset.h - header declaring the keyword set library.
    Copyright (C) 1989, 1998, 2005 Free Software Foundation, Inc.
 
diff --git a/list-objects-filter.h b/list-objects-filter.h
index a963d0274c..a6f6b4990b 100644
--- a/list-objects-filter.h
+++ b/list-objects-filter.h
@@ -1,6 +1,10 @@
 #ifndef LIST_OBJECTS_FILTER_H
 #define LIST_OBJECTS_FILTER_H
 
+struct list_objects_filter_options;
+struct object;
+struct oidset;
+
 /*
  * During list-object traversal we allow certain objects to be
  * filtered (omitted) from the result.  The active filter uses
diff --git a/list-objects.h b/list-objects.h
index aa618d7f45..ad40762926 100644
--- a/list-objects.h
+++ b/list-objects.h
@@ -1,6 +1,10 @@
 #ifndef LIST_OBJECTS_H
 #define LIST_OBJECTS_H
 
+struct commit;
+struct object;
+struct rev_info;
+
 typedef void (*show_commit_fn)(struct commit *, void *);
 typedef void (*show_object_fn)(struct object *, const char *, void *);
 void traverse_commit_list(struct rev_info *, show_commit_fn, show_object_fn, void *);
diff --git a/ll-merge.h b/ll-merge.h
index 244a31f55a..3535c543d7 100644
--- a/ll-merge.h
+++ b/ll-merge.h
@@ -5,6 +5,8 @@
 #ifndef LL_MERGE_H
 #define LL_MERGE_H
 
+#include "xdiff/xdiff.h" /* for mmbuffer_t, mmfile_t */
+
 struct ll_merge_options {
 	unsigned virtual_ancestor : 1;
 	unsigned variant : 2;	/* favor ours, favor theirs, or union merge */
diff --git a/mailinfo.h b/mailinfo.h
index 04a25351d6..766c03dd1d 100644
--- a/mailinfo.h
+++ b/mailinfo.h
@@ -1,6 +1,8 @@
 #ifndef MAILINFO_H
 #define MAILINFO_H
 
+#include "strbuf.h"
+
 #define MAX_BOUNDARIES 5
 
 struct mailinfo {
diff --git a/mailmap.h b/mailmap.h
index ed7c93b05c..ce457858a7 100644
--- a/mailmap.h
+++ b/mailmap.h
@@ -1,6 +1,10 @@
 #ifndef MAILMAP_H
 #define MAILMAP_H
 
+#include <stdlib.h>
+
+struct string_list;
+
 int read_mailmap(struct string_list *map, char **repo_abbrev);
 void clear_mailmap(struct string_list *map);
 
diff --git a/mem-pool.h b/mem-pool.h
index 999d3c3a52..b055f602a1 100644
--- a/mem-pool.h
+++ b/mem-pool.h
@@ -1,6 +1,8 @@
 #ifndef MEM_POOL_H
 #define MEM_POOL_H
 
+#include "git-compat-util.h" /* for FLEX_ARRAY, etc. */
+
 struct mp_block {
 	struct mp_block *next_block;
 	char *next_free;
diff --git a/merge-recursive.h b/merge-recursive.h
index fa7bc6b683..0c46a5a4ff 100644
--- a/merge-recursive.h
+++ b/merge-recursive.h
@@ -1,8 +1,10 @@
 #ifndef MERGE_RECURSIVE_H
 #define MERGE_RECURSIVE_H
 
-#include "unpack-trees.h"
 #include "string-list.h"
+#include "unpack-trees.h"
+
+struct commit;
 
 struct merge_options {
 	const char *ancestor;
diff --git a/notes-cache.h b/notes-cache.h
index aeeee8409d..5ad82d9031 100644
--- a/notes-cache.h
+++ b/notes-cache.h
@@ -2,6 +2,7 @@
 #define NOTES_CACHE_H
 
 #include "notes.h"
+#include <stdlib.h>
 
 struct notes_cache {
 	struct notes_tree tree;
diff --git a/notes-merge.h b/notes-merge.h
index f815f23451..6c74e9385b 100644
--- a/notes-merge.h
+++ b/notes-merge.h
@@ -2,6 +2,10 @@
 #define NOTES_MERGE_H
 
 #include "notes-utils.h"
+#include "strbuf.h"
+
+struct commit;
+struct object_id;
 
 #define NOTES_MERGE_WORKTREE "NOTES_MERGE_WORKTREE"
 
diff --git a/notes-utils.h b/notes-utils.h
index 5d79cbef51..0476339677 100644
--- a/notes-utils.h
+++ b/notes-utils.h
@@ -2,6 +2,10 @@
 #define NOTES_UTILS_H
 
 #include "notes.h"
+#include <stdlib.h>
+
+struct commit_list;
+struct object_id;
 
 /*
  * Create new notes commit from the given notes tree
diff --git a/notes.h b/notes.h
index 0433f45db5..414bc6855a 100644
--- a/notes.h
+++ b/notes.h
@@ -3,6 +3,9 @@
 
 #include "string-list.h"
 
+struct object_id;
+struct strbuf;
+
 /*
  * Function type for combining two notes annotating the same object.
  *
diff --git a/object-store.h b/object-store.h
index e481f7ad41..05b962daba 100644
--- a/object-store.h
+++ b/object-store.h
@@ -1,6 +1,7 @@
 #ifndef OBJECT_STORE_H
 #define OBJECT_STORE_H
 
+#include "cache.h" /* for object_id */
 #include "oidmap.h"
 #include "list.h"
 #include "sha1-array.h"
diff --git a/object.h b/object.h
index 177b1a4571..020315279d 100644
--- a/object.h
+++ b/object.h
@@ -1,6 +1,8 @@
 #ifndef OBJECT_H
 #define OBJECT_H
 
+#include "cache.h"  /* For TYPE_BITS */
+
 struct buffer_slab;
 
 struct parsed_object_pool {
diff --git a/oidmap.h b/oidmap.h
index d3cd2bb590..09bbbbb15e 100644
--- a/oidmap.h
+++ b/oidmap.h
@@ -1,6 +1,7 @@
 #ifndef OIDMAP_H
 #define OIDMAP_H
 
+#include "cache.h" /* for object_id */
 #include "hashmap.h"
 
 /*
diff --git a/pack-bitmap.h b/pack-bitmap.h
index 4555907dee..8a04741e12 100644
--- a/pack-bitmap.h
+++ b/pack-bitmap.h
@@ -5,6 +5,9 @@
 #include "khash.h"
 #include "pack-objects.h"
 
+struct commit;
+struct rev_info;
+
 struct bitmap_disk_header {
 	char magic[4];
 	uint16_t version;
diff --git a/pack-revindex.h b/pack-revindex.h
index e262f3efe8..9dde731a90 100644
--- a/pack-revindex.h
+++ b/pack-revindex.h
@@ -1,6 +1,8 @@
 #ifndef PACK_REVINDEX_H
 #define PACK_REVINDEX_H
 
+#include "git-compat-util.h" /* for off_t */
+
 struct packed_git;
 
 struct revindex_entry {
diff --git a/parse-options.h b/parse-options.h
index dd14911a29..1ea55cb85c 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" /* for NORETURN, intptr_t */
+
 enum parse_opt_type {
 	/* special types */
 	OPTION_END,
diff --git a/patch-ids.h b/patch-ids.h
index bec0f727a7..79ac9a8498 100644
--- a/patch-ids.h
+++ b/patch-ids.h
@@ -1,6 +1,12 @@
 #ifndef PATCH_IDS_H
 #define PATCH_IDS_H
 
+#include "diff.h"
+#include "hashmap.h"
+
+struct commit;
+struct object_id;
+
 struct patch_id {
 	struct hashmap_entry ent;
 	struct object_id patch_id;
diff --git a/path.h b/path.h
index ed6732e5a2..021c6d8153 100644
--- a/path.h
+++ b/path.h
@@ -1,7 +1,10 @@
 #ifndef PATH_H
 #define PATH_H
 
+#include <stdlib.h>
+
 struct repository;
+struct strbuf;
 
 /*
  * The result to all functions which return statically allocated memory may be
diff --git a/pathspec.h b/pathspec.h
index 099a170c2e..2a750d7472 100644
--- a/pathspec.h
+++ b/pathspec.h
@@ -1,6 +1,11 @@
 #ifndef PATHSPEC_H
 #define PATHSPEC_H
 
+#include "string.h"   /* For str[n]cmp */
+#include "strings.h"  /* For str[n]casecmp */
+
+struct index_state;
+
 /* Pathspec magic */
 #define PATHSPEC_FROMTOP	(1<<0)
 #define PATHSPEC_MAXDEPTH	(1<<1)
diff --git a/pretty.h b/pretty.h
index 5c85d94e33..872c88d571 100644
--- a/pretty.h
+++ b/pretty.h
@@ -1,7 +1,11 @@
 #ifndef PRETTY_H
 #define PRETTY_H
 
+#include "cache.h" /* for date_mode */
+#include "string-list.h"
+
 struct commit;
+struct strbuf;
 
 /* Commit formats */
 enum cmit_fmt {
diff --git a/progress.h b/progress.h
index 70a4d4a0d6..9d80837f53 100644
--- a/progress.h
+++ b/progress.h
@@ -1,6 +1,8 @@
 #ifndef PROGRESS_H
 #define PROGRESS_H
 
+#include "git-compat-util.h" /* for uint64_t */
+
 struct progress;
 
 void display_throughput(struct progress *progress, uint64_t total);
diff --git a/quote.h b/quote.h
index ea992dcc91..407bf8a864 100644
--- a/quote.h
+++ b/quote.h
@@ -1,6 +1,8 @@
 #ifndef QUOTE_H
 #define QUOTE_H
 
+#include <stdio.h>
+
 struct strbuf;
 
 /* Help to copy the thing properly quoted for the shell safety.
diff --git a/reachable.h b/reachable.h
index 3c00fa0526..ef93f04911 100644
--- a/reachable.h
+++ b/reachable.h
@@ -1,7 +1,11 @@
 #ifndef REACHEABLE_H
 #define REACHEABLE_H
 
+#include "git-compat-util.h" /* for timestamp_t */
+
 struct progress;
+struct rev_info;
+
 extern int add_unseen_recent_objects_to_traversal(struct rev_info *revs,
 						  timestamp_t timestamp);
 extern void mark_reachable_objects(struct rev_info *revs, int mark_reflog,
diff --git a/reflog-walk.h b/reflog-walk.h
index 7553c448fe..cb3e73755d 100644
--- a/reflog-walk.h
+++ b/reflog-walk.h
@@ -3,6 +3,7 @@
 
 #include "cache.h"
 
+struct commit;
 struct reflog_walk_info;
 
 extern void init_reflog_walk(struct reflog_walk_info **info);
diff --git a/refs.h b/refs.h
index cc2fb4c68c..ab53d72609 100644
--- a/refs.h
+++ b/refs.h
@@ -1,10 +1,14 @@
 #ifndef REFS_H
 #define REFS_H
 
+#include "git-compat-util.h" /* for timestamp_t, strpbrk from string.h, etc. */
+
 struct object_id;
 struct ref_store;
+struct repository;
 struct strbuf;
 struct string_list;
+struct string_list_item;
 struct worktree;
 
 /*
diff --git a/remote.h b/remote.h
index 976292152c..b4fc87770d 100644
--- a/remote.h
+++ b/remote.h
@@ -1,6 +1,7 @@
 #ifndef REMOTE_H
 #define REMOTE_H
 
+#include "cache.h" /* for object_id */
 #include "parse-options.h"
 #include "hashmap.h"
 #include "refspec.h"
diff --git a/repository.h b/repository.h
index b9413be50c..112ae4c515 100644
--- a/repository.h
+++ b/repository.h
@@ -1,6 +1,8 @@
 #ifndef REPOSITORY_H
 #define REPOSITORY_H
 
+#include "path.h"  /* for path_cache */
+
 struct config_set;
 struct git_hash_algo;
 struct index_state;
diff --git a/resolve-undo.h b/resolve-undo.h
index 87291904bd..f48e0358ed 100644
--- a/resolve-undo.h
+++ b/resolve-undo.h
@@ -1,6 +1,8 @@
 #ifndef RESOLVE_UNDO_H
 #define RESOLVE_UNDO_H
 
+#include "cache.h" /* for object_id...and a bunch of other stuff */
+
 struct resolve_undo_info {
 	unsigned int mode[3];
 	struct object_id oid[3];
diff --git a/revision.h b/revision.h
index c599c34da9..7c88f9d8a4 100644
--- a/revision.h
+++ b/revision.h
@@ -1,6 +1,7 @@
 #ifndef REVISION_H
 #define REVISION_H
 
+#include "commit.h" /* for rev_sort_order */
 #include "parse-options.h"
 #include "grep.h"
 #include "notes.h"
diff --git a/send-pack.h b/send-pack.h
index 6af71f7008..e148fcd960 100644
--- a/send-pack.h
+++ b/send-pack.h
@@ -3,6 +3,10 @@
 
 #include "string-list.h"
 
+struct child_process;
+struct oid_array;
+struct ref;
+
 /* Possible values for push_cert field in send_pack_args. */
 #define SEND_PACK_PUSH_CERT_NEVER 0
 #define SEND_PACK_PUSH_CERT_IF_ASKED 1
diff --git a/sequencer.h b/sequencer.h
index c5787c6b56..a152dd46c5 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -1,6 +1,11 @@
 #ifndef SEQUENCER_H
 #define SEQUENCER_H
 
+#include "cache.h" /* for object_id */
+#include "strbuf.h"
+
+struct commit;
+
 const char *git_path_commit_editmsg(void);
 const char *git_path_seq_dir(void);
 
diff --git a/sha1-lookup.h b/sha1-lookup.h
index 7678b23b36..d332662fe0 100644
--- a/sha1-lookup.h
+++ b/sha1-lookup.h
@@ -1,6 +1,8 @@
 #ifndef SHA1_LOOKUP_H
 #define SHA1_LOOKUP_H
 
+#include "git-compat-util.h" /* for uint32_t, etc. */
+
 typedef const unsigned char *sha1_access_fn(size_t index, void *table);
 
 extern int sha1_pos(const unsigned char *sha1,
diff --git a/shortlog.h b/shortlog.h
index 5d64cfe929..cf09c38c9c 100644
--- a/shortlog.h
+++ b/shortlog.h
@@ -2,6 +2,9 @@
 #define SHORTLOG_H
 
 #include "string-list.h"
+#include <stdio.h>
+
+struct commit;
 
 struct shortlog {
 	struct string_list list;
diff --git a/sideband.h b/sideband.h
index 7a8146f161..a5a32be7d8 100644
--- a/sideband.h
+++ b/sideband.h
@@ -1,6 +1,8 @@
 #ifndef SIDEBAND_H
 #define SIDEBAND_H
 
+#include "git-compat-util.h" /* for ssize_t */
+
 #define SIDEBAND_PROTOCOL_ERROR -2
 #define SIDEBAND_REMOTE_ERROR -1
 
diff --git a/strbuf.h b/strbuf.h
index 60a35aef16..7ff545fef4 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -1,6 +1,8 @@
 #ifndef STRBUF_H
 #define STRBUF_H
 
+#include "git-compat-util.h"  /* for size_t, SWAP, strlen, off_t, etc. */
+
 struct string_list;
 
 /**
diff --git a/submodule.h b/submodule.h
index 4644683e6c..3adbc08915 100644
--- a/submodule.h
+++ b/submodule.h
@@ -1,11 +1,17 @@
 #ifndef SUBMODULE_H
 #define SUBMODULE_H
 
-struct repository;
-struct diff_options;
 struct argv_array;
+struct cache_entry;
+struct diff_options;
+struct index_state;
+struct object_id;
 struct oid_array;
+struct pathspec;
 struct remote;
+struct repository;
+struct string_list;
+struct strbuf;
 
 enum {
 	RECURSE_SUBMODULES_ONLY = -5,
diff --git a/tempfile.h b/tempfile.h
index 8959c5f1b5..36434eb6fa 100644
--- a/tempfile.h
+++ b/tempfile.h
@@ -2,6 +2,7 @@
 #define TEMPFILE_H
 
 #include "list.h"
+#include "strbuf.h"
 
 /*
  * Handle temporary files.
diff --git a/trailer.h b/trailer.h
index 6d7f8c2a52..1d05b9e625 100644
--- a/trailer.h
+++ b/trailer.h
@@ -2,6 +2,9 @@
 #define TRAILER_H
 
 #include "list.h"
+#include <stdlib.h>
+
+struct strbuf;
 
 enum trailer_where {
 	WHERE_DEFAULT,
diff --git a/tree-walk.h b/tree-walk.h
index 805f58f00f..196831007e 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 struct object_id *oid;
 	const char *path;
diff --git a/unpack-trees.h b/unpack-trees.h
index c2b434c606..70ffcc350e 100644
--- a/unpack-trees.h
+++ b/unpack-trees.h
@@ -1,11 +1,14 @@
 #ifndef UNPACK_TREES_H
 #define UNPACK_TREES_H
 
-#include "tree-walk.h"
 #include "argv-array.h"
+#include "cache.h"  /* for index_state */
+#include "string-list.h"
+#include "tree-walk.h"
 
 #define MAX_UNPACK_TREES 8
 
+struct cache_entry;
 struct unpack_trees_options;
 struct exclude_list;
 
diff --git a/url.h b/url.h
index abdaf6fa30..f311c40a46 100644
--- a/url.h
+++ b/url.h
@@ -1,6 +1,8 @@
 #ifndef URL_H
 #define URL_H
 
+struct strbuf;
+
 extern int is_url(const char *url);
 extern int is_urlschemechar(int first_flag, int ch);
 extern char *url_decode(const char *url);
diff --git a/urlmatch.h b/urlmatch.h
index 37ee5da85e..bb0f4dee8d 100644
--- a/urlmatch.h
+++ b/urlmatch.h
@@ -1,5 +1,6 @@
 #ifndef URL_MATCH_H
 #include "string-list.h"
+#include <stdlib.h>
 
 struct url_info {
 	/* normalized url on success, must be freed, otherwise NULL */
diff --git a/utf8.h b/utf8.h
index db73a2d8d3..26ca079762 100644
--- a/utf8.h
+++ b/utf8.h
@@ -1,6 +1,10 @@
 #ifndef GIT_UTF8_H
 #define GIT_UTF8_H
 
+#include "git-compat-util.h" /* for iconv_t */
+
+struct strbuf;
+
 typedef unsigned int ucs_char_t;  /* assuming 32bit int */
 
 size_t display_mode_esc_sequence_len(const char *s);
diff --git a/varint.h b/varint.h
index c1c44d9a6b..8fc15a54c8 100644
--- a/varint.h
+++ b/varint.h
@@ -1,6 +1,8 @@
 #ifndef VARINT_H
 #define VARINT_H
 
+#include "git-compat-util.h" /* for uintmax_t */
+
 extern int encode_varint(uintmax_t, unsigned char *);
 extern uintmax_t decode_varint(const unsigned char **);
 
diff --git a/worktree.h b/worktree.h
index fe38ce10c3..e850457208 100644
--- a/worktree.h
+++ b/worktree.h
@@ -1,6 +1,7 @@
 #ifndef WORKTREE_H
 #define WORKTREE_H
 
+#include "cache.h" /* for object_id */
 #include "refs.h"
 
 struct strbuf;
-- 
2.18.0.549.gd4454f3f9b

Re: [PATCH 1/9] Add missing includes and forward declares

[Jeff King]

On Fri, Aug 10, 2018 at 09:32:10PM -0700, Elijah Newren wrote:

> diff --git a/argv-array.h b/argv-array.h
> index a39ba43f57..c46238784c 100644
> --- a/argv-array.h
> +++ b/argv-array.h
> @@ -1,6 +1,8 @@
>  #ifndef ARGV_ARRAY_H
>  #define ARGV_ARRAY_H
>  
> +#include "git-compat-util.h"   /* for LAST_ARG_MUST_BE_NULL */

Following up on what I said in the other message, I think we should omit
this. All header files should assume the environment provided by
git-compat-util.h is already loaded (and all non-compat C files should
make sure they load it first).

> diff --git a/color.h b/color.h
> index 5b744e1bc6..74edbffc9d 100644
> --- a/color.h
> +++ b/color.h
> @@ -1,6 +1,8 @@
>  #ifndef COLOR_H
>  #define COLOR_H
>  
> +#include <stdio.h>

This one is more actively wrong than the git-compat-util.h one. Anybody
who already included git-compat-util.h would not be helped by this. And
anybody who _didn't_ may be hurt, because now git-compat-util.h might
run into ordering problems caused by headers already loaded as part of
stdio.h (and keep in mind that stdio.h often brings in other
system-dependent definitions, too).

> diff --git a/bulk-checkin.h b/bulk-checkin.h
> index a85527318b..e036231636 100644
> --- a/bulk-checkin.h
> +++ b/bulk-checkin.h
> @@ -4,6 +4,8 @@
>  #ifndef BULK_CHECKIN_H
>  #define BULK_CHECKIN_H
>  
> +#include "cache.h" /* for object_type */

As I said earlier, I don't mind includes like these that make
everybody's lives a little easier, and shouldn't introduce any funny
ordering constraints.

IMHO, though, comments like "for X" are probably a bad idea. That may be
accurate _now_, but it is very likely to grow stale without anybody
noticing. You only care about that "X" if you are thinking about
removing the include, and then the right answer is not to trust the
comment, but to remove the include and make sure there is no
other breakage.

The one exception is if there is some subtlety, like including a header
file that uses macros to replace a function, and removing it wouldn't
break compilation but produce a different result. But then:

  - I'd hope we don't have anything like that, because it sounds
    horrible ;)

  - you'd probably want a much more explanatory comment

> [...]

The rest of them are all variants on these three (or forward
declarations, which I put in the same boat as this third recursive
include; i.e., it's a good change).

-Peff

[PATCH 2/9] alloc: make allocate_alloc_state and clear_alloc_state more consistent

[Elijah Newren]

Since both functions are using the same data type, they should either both
refer to it as void *, or both use the real type (struct alloc_state *).
Opt for the latter.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 alloc.c | 2 +-
 alloc.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/alloc.c b/alloc.c
index c2fc5d6888..e7aa81b7aa 100644
--- a/alloc.c
+++ b/alloc.c
@@ -36,7 +36,7 @@ struct alloc_state {
 	int slab_nr, slab_alloc;
 };
 
-void *allocate_alloc_state(void)
+struct alloc_state *allocate_alloc_state(void)
 {
 	return xcalloc(1, sizeof(struct alloc_state));
 }
diff --git a/alloc.h b/alloc.h
index 7a41bb9eb3..ba356ed847 100644
--- a/alloc.h
+++ b/alloc.h
@@ -15,7 +15,7 @@ void *alloc_object_node(struct repository *r);
 void alloc_report(struct repository *r);
 unsigned int alloc_commit_index(struct repository *r);
 
-void *allocate_alloc_state(void);
+struct alloc_state *allocate_alloc_state(void);
 void clear_alloc_state(struct alloc_state *s);
 
 #endif
-- 
2.18.0.549.gd4454f3f9b

[PATCH 3/9] Move definition of enum branch_track from cache.h to branch.h

[Elijah Newren]

'branch_track' feels more closely related to branching, and it is
needed later in branch.h; rather than #include'ing cache.h in branch.h
for this small enum, just move the enum and the external declaration
for git_branch_track to branch.h.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 branch.h      | 11 +++++++++++
 cache.h       | 10 ----------
 config.c      |  1 +
 environment.c |  1 +
 4 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/branch.h b/branch.h
index 7d9b330eba..5cace4581f 100644
--- a/branch.h
+++ b/branch.h
@@ -3,6 +3,17 @@
 
 struct strbuf;
 
+enum branch_track {
+	BRANCH_TRACK_UNSPECIFIED = -1,
+	BRANCH_TRACK_NEVER = 0,
+	BRANCH_TRACK_REMOTE,
+	BRANCH_TRACK_ALWAYS,
+	BRANCH_TRACK_EXPLICIT,
+	BRANCH_TRACK_OVERRIDE
+};
+
+extern enum branch_track git_branch_track;
+
 /* Functions for acting on the information about branches. */
 
 /*
diff --git a/cache.h b/cache.h
index 8dc7134f00..a1c0c594fb 100644
--- a/cache.h
+++ b/cache.h
@@ -919,15 +919,6 @@ enum log_refs_config {
 };
 extern enum log_refs_config log_all_ref_updates;
 
-enum branch_track {
-	BRANCH_TRACK_UNSPECIFIED = -1,
-	BRANCH_TRACK_NEVER = 0,
-	BRANCH_TRACK_REMOTE,
-	BRANCH_TRACK_ALWAYS,
-	BRANCH_TRACK_EXPLICIT,
-	BRANCH_TRACK_OVERRIDE
-};
-
 enum rebase_setup_type {
 	AUTOREBASE_NEVER = 0,
 	AUTOREBASE_LOCAL,
@@ -944,7 +935,6 @@ enum push_default_type {
 	PUSH_DEFAULT_UNSPECIFIED
 };
 
-extern enum branch_track git_branch_track;
 extern enum rebase_setup_type autorebase;
 extern enum push_default_type push_default;
 
diff --git a/config.c b/config.c
index 66645047eb..b60d7c0308 100644
--- a/config.c
+++ b/config.c
@@ -5,6 +5,7 @@
  * Copyright (C) Johannes Schindelin, 2005
  *
  */
+#include "branch.h"
 #include "cache.h"
 #include "config.h"
 #include "repository.h"
diff --git a/environment.c b/environment.c
index 6cf0079389..920362900c 100644
--- a/environment.c
+++ b/environment.c
@@ -7,6 +7,7 @@
  * even if you might want to know where the git directory etc
  * are.
  */
+#include "branch.h"
 #include "cache.h"
 #include "repository.h"
 #include "config.h"
-- 
2.18.0.549.gd4454f3f9b

[PATCH 4/9] urlmatch.h: fix include guard

[Elijah Newren]

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 urlmatch.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/urlmatch.h b/urlmatch.h
index bb0f4dee8d..dfa1b2b71d 100644
--- a/urlmatch.h
+++ b/urlmatch.h
@@ -1,4 +1,6 @@
 #ifndef URL_MATCH_H
+#define URL_MATCH_H
+
 #include "string-list.h"
 #include <stdlib.h>
 
-- 
2.18.0.549.gd4454f3f9b

[PATCH 5/9] compat/precompose_utf8.h: use more common include guard style

[Elijah Newren]

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 compat/precompose_utf8.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/compat/precompose_utf8.h b/compat/precompose_utf8.h
index a94e7c4342..6f843d3e1a 100644
--- a/compat/precompose_utf8.h
+++ b/compat/precompose_utf8.h
@@ -1,4 +1,6 @@
 #ifndef PRECOMPOSE_UNICODE_H
+#define PRECOMPOSE_UNICODE_H
+
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <dirent.h>
@@ -41,5 +43,4 @@ int precompose_utf8_closedir(PREC_DIR *dirp);
 #define DIR PREC_DIR
 #endif /* PRECOMPOSE_UNICODE_C */
 
-#define  PRECOMPOSE_UNICODE_H
 #endif /* PRECOMPOSE_UNICODE_H */
-- 
2.18.0.549.gd4454f3f9b

[PATCH 6/9] ewah/ewok.h: add missing include

[Elijah Newren]

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 ewah/ewok.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/ewah/ewok.h b/ewah/ewok.h
index 84b2a29faa..67a68e291a 100644
--- a/ewah/ewok.h
+++ b/ewah/ewok.h
@@ -19,6 +19,8 @@
 #ifndef __EWOK_BITMAP_H__
 #define __EWOK_BITMAP_H__
 
+#include "../git-compat-util.h" /* for uint64_t, uint32_t, etc. */
+
 struct strbuf;
 typedef uint64_t eword_t;
 #define BITS_IN_EWORD (sizeof(eword_t) * 8)
-- 
2.18.0.549.gd4454f3f9b

[PATCH 7/9] sha1dc/sha1.h: add missing include

[Elijah Newren]

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 sha1dc/sha1.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/sha1dc/sha1.h b/sha1dc/sha1.h
index 1e4e94be54..e9ca7a83ac 100644
--- a/sha1dc/sha1.h
+++ b/sha1dc/sha1.h
@@ -14,6 +14,7 @@ extern "C" {
 
 #ifndef SHA1DC_NO_STANDARD_INCLUDES
 #include <stdint.h>
+#include <stdlib.h>
 #endif
 
 /* sha-1 compression function that takes an already expanded message, and additionally store intermediate states */
-- 
2.18.0.549.gd4454f3f9b

[PATCH 8/9] xdiff/xdiff.h: add missing include

[Elijah Newren]

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 xdiff/xdiff.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/xdiff/xdiff.h b/xdiff/xdiff.h
index 2356da5f78..5233550c9e 100644
--- a/xdiff/xdiff.h
+++ b/xdiff/xdiff.h
@@ -27,6 +27,8 @@
 extern "C" {
 #endif /* #ifdef __cplusplus */
 
+#include <stdlib.h>  /* for size_t */
+
 /* xpparm_t.flags */
 #define XDF_NEED_MINIMAL (1 << 0)
 
-- 
2.18.0.549.gd4454f3f9b

[PATCH 9/9] Add missing includes and forward declares

[Elijah Newren]

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 bisect.h       | 2 ++
 pack-objects.h | 1 +
 2 files changed, 3 insertions(+)

diff --git a/bisect.h b/bisect.h
index a5d9248a47..34df209351 100644
--- a/bisect.h
+++ b/bisect.h
@@ -1,6 +1,8 @@
 #ifndef BISECT_H
 #define BISECT_H
 
+struct commit_list;
+
 /*
  * Find bisection. If something is found, `reaches` will be the number of
  * commits that the best commit reaches. `all` will be the count of
diff --git a/pack-objects.h b/pack-objects.h
index edf74dabdd..919cdc211e 100644
--- a/pack-objects.h
+++ b/pack-objects.h
@@ -2,6 +2,7 @@
 #define PACK_OBJECTS_H
 
 #include "object-store.h"
+#include "pack.h" /* for pack_idx_entry */
 
 #define DEFAULT_DELTA_CACHE_SIZE (256 * 1024 * 1024)
 
-- 
2.18.0.549.gd4454f3f9b

Re: [PATCH 0/9] Add missing includes and forward declares

[Ævar Arnfjörð Bjarmason]

On Sat, Aug 11 2018, Elijah Newren wrote:

[CC'd sha1dc maintainers, for context the relevent patch is
https://public-inbox.org/git/20180811043218.31456-8-newren@gmail.com/T/#u]

>   * Patches 6-8: These patches might need to be submitted to separate
>     projects elsewhere.  Let me know if so.

Yes, for sha1dc (your 7/9) it's much better if we submit patches to
upstream and then submit a patch to git.git to update from upstream, as
my 23e37f8e9d ("sha1dc: update from upstream", 2018-08-02) sitting in
'next' does.

When we build that library we define SHA1DC_NO_STANDARD_INCLUDES, so we
don't use the codepath you patched, so your description of "I was bit
yet again by compilation errors[...]" doesn't add up in that case. I
assume you just started adding stdlib.h where you could grep for
stdint.h.

When I check out sha1collisiondetection.git stand-alone (which will use
that path) and compile it, it works fine for me. This is on GCC 8.1.0 on
Debian, so perhaps that patch isn't needed in that case.

Re: [PATCH 0/9] Add missing includes and forward declares

[Elijah Newren]

On Sat, Aug 11, 2018 at 1:30 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote
> On Sat, Aug 11 2018, Elijah Newren wrote:
>
> [CC'd sha1dc maintainers, for context the relevent patch is
> https://public-inbox.org/git/20180811043218.31456-8-newren@gmail.com/T/#u]
>
> >   * Patches 6-8: These patches might need to be submitted to separate
> >     projects elsewhere.  Let me know if so.
>
> Yes, for sha1dc (your 7/9) it's much better if we submit patches to
> upstream and then submit a patch to git.git to update from upstream, as
> my 23e37f8e9d ("sha1dc: update from upstream", 2018-08-02) sitting in
> 'next' does.
>
> When we build that library we define SHA1DC_NO_STANDARD_INCLUDES, so we
> don't use the codepath you patched, so your description of "I was bit
> yet again by compilation errors[...]" doesn't add up in that case. I
> assume you just started adding stdlib.h where you could grep for
> stdint.h.

The part of my story you snipped in the ellipsis is kind of important,
though: "...and decided to determine which header files were missing
their own necessary #include's and forward declarations."  The way I
did so was making a simple one-line .c file that included exactly one
header, and checked to see if I could compile it (without any special
defines), fixed it up as necessary, then repeated that process for
every toplevel header.

Most of the stdlib.h additions were for the definition of size_t,
sha1dc/sha1.h was no different; without my patch:

In file included from nukeme.c:1:0:
sha1dc/sha1.h:96:43: error: unknown type name ‘size_t’
 void SHA1DCUpdate(SHA1_CTX*, const char*, size_t);

Your point that we use SHA1DC_NO_STANDARD_INCLUDES normally when
compiling within git, though, does mean my patch #7 is kind of useless
to git.

> When I check out sha1collisiondetection.git stand-alone (which will use
> that path) and compile it, it works fine for me. This is on GCC 8.1.0 on
> Debian, so perhaps that patch isn't needed in that case.

I never said git or its subprojects failed to compile.  I said when I
added another #include of some header to some existing or new files, I
sometimes saw unknown symbol errors.  It's happened before, it
happened again today, and I got annoyed, so I set out to address it.
Sorry if that wasn't clear.

Re: [PATCH 0/9] Add missing includes and forward declares

[Jeff King]

On Sat, Aug 11, 2018 at 01:59:50AM -0700, Elijah Newren wrote:

> The part of my story you snipped in the ellipsis is kind of important,
> though: "...and decided to determine which header files were missing
> their own necessary #include's and forward declarations."  The way I
> did so was making a simple one-line .c file that included exactly one
> header, and checked to see if I could compile it (without any special
> defines), fixed it up as necessary, then repeated that process for
> every toplevel header.

The rule in Git has always been that your very first include must
always be "git-compat-util.h" or an equivalent header that includes it
first (like "cache.h"). This is mentioned in CodingGuidelines.

So I think the better test is a two-line .c file with:

  #include "git-compat-util.h"
  #include $header_to_check

And I'd be fine with fixing any compilation failures there, either with
forward declarations or recursive includes. I think the in the early
days there was some resistance to having a lot of recursive includes,
because it _can_ lead to confusion, but IMHO it mostly helps people. And
I don't recall much discussion on it in recent years.

Including "git-compat-util.h" in many more headers probably doesn't hurt
(after all, it's a noop for every .c file which is following the
existing rule). But I'd just as soon not sprinkle it everywhere, nor
imply that that people don't need to be including it. It's really
important that it comes first because it wants a clean slate and have
subtle effects on other header files due to macros. Worse still, some of
the effects are platform dependent, so we might not realize an ordering
problem until somebody on AIX complains.

I wonder if there's an easy way to check. I guess adding '-Dint=foo' to
the command line, and then putting '#undef int' at the top of
git-compat-util would catch just about any code the compiler sees that
doesn't have git-compat-util included. :)

-Peff

Re: [PATCH 0/9] Add missing includes and forward declares

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> The rule in Git has always been that your very first include must
> always be "git-compat-util.h" or an equivalent header that includes it
> first (like "cache.h"). This is mentioned in CodingGuidelines.

Glad to see that you already have written the above so I don't have
to ;-)

As things are slowly moving out of the so-far kitchen-sink "cache.h"
into more specific subsystem headers (like object-store.h), we may
actually want to tighten the "header that includes it first" part a
bit in the future, so that 'git grep cache.h' would give us a more
explicit and a better picture of what really depends on knowing what
the lowest level plumbing API are built around.

> So I think the better test is a two-line .c file with:
>
>   #include "git-compat-util.h"
>   #include $header_to_check

But until that tightening happens, I do not actually mind the
two-line .c file started with inclusion of cache.h instead of
git-compat-util.h.  That would limit the scope of this series
further.

> I wonder if there's an easy way to check. I guess adding '-Dint=foo' to
> the command line, and then putting '#undef int' at the top of
> git-compat-util would catch just about any code the compiler sees that
> doesn't have git-compat-util included. :)

;-).

Re: [PATCH 0/9] Add missing includes and forward declares

[Elijah Newren]

On Mon, Aug 13, 2018 at 11:24 AM Junio C Hamano <gitster@pobox.com> wrote:
> Jeff King <peff@peff.net> writes:

> As things are slowly moving out of the so-far kitchen-sink "cache.h"
> into more specific subsystem headers (like object-store.h), we may
> actually want to tighten the "header that includes it first" part a
> bit in the future, so that 'git grep cache.h' would give us a more
> explicit and a better picture of what really depends on knowing what
> the lowest level plumbing API are built around.
>
> > So I think the better test is a two-line .c file with:
> >
> >   #include "git-compat-util.h"
> >   #include $header_to_check
>
> But until that tightening happens, I do not actually mind the
> two-line .c file started with inclusion of cache.h instead of
> git-compat-util.h.  That would limit the scope of this series
> further.

Yes, this removes about 2/3 of patch #1.  But it makes things kind of
odd to me; do we change the relevant paragraph of
Documentation/CodingGuidelines to add an additional sentence so it
reads:

 - The first #include in C files, except in platform specific compat/
   implementations, must be either "git-compat-util.h", "cache.h" or
   "builtin.h".  You do not have to include more than one of these.
   However, note that including any of a few dozen header files
   may result in compilation failures if cache.h is not included first.

That seems annoying to state, but also annoying to leave undocumented.
I'd rather have the various structs (object_id, strbuf, index_state,
cache_entry, etc.) forward declared and/or relevant headers (e.g.
"cache.h", "strbuf.h", or "string-list.h") #include'd.  Are you sure
you really want me to remove these fixes?  Let me know, and if so I'll
do it, even if I'd prefer to leave them in.  Let me know how/if you'd
like that CodingGuidline to be updated too.


Also sha1dc_git.h fails to compile if cache.h is #included before it
(various redefinition errors, such as for "platform_SHA_CTX"); it's
possible that's expected, but it does make things a little weirder to
add implicit assumptions that cache.h is included first.

Re: [PATCH 0/9] Add missing includes and forward declares

[Junio C Hamano]

Elijah Newren <newren@gmail.com> writes:

> On Mon, Aug 13, 2018 at 11:24 AM Junio C Hamano <gitster@pobox.com> wrote:
>> Jeff King <peff@peff.net> writes:
>
>> As things are slowly moving out of the so-far kitchen-sink "cache.h"
>> into more specific subsystem headers (like object-store.h), we may
>> actually want to tighten the "header that includes it first" part a
>> bit in the future, so that 'git grep cache.h' would give us a more
>> explicit and a better picture of what really depends on knowing what
>> the lowest level plumbing API are built around.
>>
>> > So I think the better test is a two-line .c file with:
>> >
>> >   #include "git-compat-util.h"
>> >   #include $header_to_check
>>
>> But until that tightening happens, I do not actually mind the
>> two-line .c file started with inclusion of cache.h instead of
>> git-compat-util.h.  That would limit the scope of this series
>> further.
>
> Yes, this removes about 2/3 of patch #1.

Sorry for making a misleading comment.  I should have phrased "I
would not have minded if the series were looser by assuming
cache.h", implying that "but now the actual patch went extra mile to
be more complete, what we have is even better ;-)".

Re: [PATCH 0/9] Add missing includes and forward declares

[Elijah Newren]

On Tue, Aug 14, 2018 at 10:45 PM Junio C Hamano <gitster@pobox.com> wrote:
> Elijah Newren <newren@gmail.com> writes:
>
> > On Mon, Aug 13, 2018 at 11:24 AM Junio C Hamano <gitster@pobox.com> wrote:
> >> Jeff King <peff@peff.net> writes:
> >
> >> As things are slowly moving out of the so-far kitchen-sink "cache.h"
> >> into more specific subsystem headers (like object-store.h), we may
> >> actually want to tighten the "header that includes it first" part a
> >> bit in the future, so that 'git grep cache.h' would give us a more
> >> explicit and a better picture of what really depends on knowing what
> >> the lowest level plumbing API are built around.
> >>
> >> > So I think the better test is a two-line .c file with:
> >> >
> >> >   #include "git-compat-util.h"
> >> >   #include $header_to_check
> >>
> >> But until that tightening happens, I do not actually mind the
> >> two-line .c file started with inclusion of cache.h instead of
> >> git-compat-util.h.  That would limit the scope of this series
> >> further.
> >
> > Yes, this removes about 2/3 of patch #1.
>
> Sorry for making a misleading comment.  I should have phrased "I
> would not have minded if the series were looser by assuming
> cache.h", implying that "but now the actual patch went extra mile to
> be more complete, what we have is even better ;-)".

Ah, gotcha.  Thanks for the clarification.

Re: [PATCH 0/9] Add missing includes and forward declares

[Junio C Hamano]

Elijah Newren <newren@gmail.com> writes:

> On Tue, Aug 14, 2018 at 10:45 PM Junio C Hamano <gitster@pobox.com> wrote:
>> Elijah Newren <newren@gmail.com> writes:
>>
>> > On Mon, Aug 13, 2018 at 11:24 AM Junio C Hamano <gitster@pobox.com> wrote:
>> >> Jeff King <peff@peff.net> writes:
>> >
>> >> As things are slowly moving out of the so-far kitchen-sink "cache.h"
>> >> into more specific subsystem headers (like object-store.h), we may
>> >> actually want to tighten the "header that includes it first" part a
>> >> bit in the future, so that 'git grep cache.h' would give us a more
>> >> explicit and a better picture of what really depends on knowing what
>> >> the lowest level plumbing API are built around.
>> >>
>> >> > So I think the better test is a two-line .c file with:
>> >> >
>> >> >   #include "git-compat-util.h"
>> >> >   #include $header_to_check
>> >>
>> >> But until that tightening happens, I do not actually mind the
>> >> two-line .c file started with inclusion of cache.h instead of
>> >> git-compat-util.h.  That would limit the scope of this series
>> >> further.
>> >
>> > Yes, this removes about 2/3 of patch #1.
>>
>> Sorry for making a misleading comment.  I should have phrased "I
>> would not have minded if the series were looser by assuming
>> cache.h", implying that "but now the actual patch went extra mile to
>> be more complete, what we have is even better ;-)".
>
> Ah, gotcha.  Thanks for the clarification.

But please remind me not to merge this round down to 'next', for the
"enum" forward decl gotcha.

Re: [PATCH 0/9] Add missing includes and forward declares

[Elijah Newren]

On Wed, Aug 15, 2018 at 8:43 AM Junio C Hamano <gitster@pobox.com> wrote:
> Elijah Newren <newren@gmail.com> writes:
>
> > On Tue, Aug 14, 2018 at 10:45 PM Junio C Hamano <gitster@pobox.com> wrote:
> >> Elijah Newren <newren@gmail.com> writes:
> >>
> >> > On Mon, Aug 13, 2018 at 11:24 AM Junio C Hamano <gitster@pobox.com> wrote:
> >> >> Jeff King <peff@peff.net> writes:
> >> >
> >> >> As things are slowly moving out of the so-far kitchen-sink "cache.h"
> >> >> into more specific subsystem headers (like object-store.h), we may
> >> >> actually want to tighten the "header that includes it first" part a
> >> >> bit in the future, so that 'git grep cache.h' would give us a more
> >> >> explicit and a better picture of what really depends on knowing what
> >> >> the lowest level plumbing API are built around.
> >> >>
> >> >> > So I think the better test is a two-line .c file with:
> >> >> >
> >> >> >   #include "git-compat-util.h"
> >> >> >   #include $header_to_check
> >> >>
> >> >> But until that tightening happens, I do not actually mind the
> >> >> two-line .c file started with inclusion of cache.h instead of
> >> >> git-compat-util.h.  That would limit the scope of this series
> >> >> further.
> >> >
> >> > Yes, this removes about 2/3 of patch #1.
> >>
> >> Sorry for making a misleading comment.  I should have phrased "I
> >> would not have minded if the series were looser by assuming
> >> cache.h", implying that "but now the actual patch went extra mile to
> >> be more complete, what we have is even better ;-)".
> >
> > Ah, gotcha.  Thanks for the clarification.
>
> But please remind me not to merge this round down to 'next', for the
> "enum" forward decl gotcha.

I'll send out a new round shortly.  Would you like me to squash the
last patch (the one that had two hunks with minor conflicts with other
topics in next and pu) into the first patch, or would you rather I
dropped that patch and waited to submit it until later?

Re: [PATCH 0/9] Add missing includes and forward declares

[Junio C Hamano]

Elijah Newren <newren@gmail.com> writes:

>>
>> But please remind me not to merge this round down to 'next', for the
>> "enum" forward decl gotcha.
>
> I'll send out a new round shortly.  Would you like me to squash the
> last patch (the one that had two hunks with minor conflicts with other
> topics in next and pu) into the first patch, or would you rather I
> dropped that patch and waited to submit it until later?

Either way is fine, especially if you looked at the intergration
result from yesterday and resolution of these conflicts looked
reasonable to you.

Re: [PATCH 0/9] Add missing includes and forward declares

[Jeff King]

On Mon, Aug 13, 2018 at 11:24:37AM -0700, Junio C Hamano wrote:

> As things are slowly moving out of the so-far kitchen-sink "cache.h"
> into more specific subsystem headers (like object-store.h), we may
> actually want to tighten the "header that includes it first" part a
> bit in the future, so that 'git grep cache.h' would give us a more
> explicit and a better picture of what really depends on knowing what
> the lowest level plumbing API are built around.

Yeah, I agree that's a good long-term goal. I think "builtin.h" is
reasonable to remain as a magic header for builtins.

> > So I think the better test is a two-line .c file with:
> >
> >   #include "git-compat-util.h"
> >   #include $header_to_check
> 
> But until that tightening happens, I do not actually mind the
> two-line .c file started with inclusion of cache.h instead of
> git-compat-util.h.  That would limit the scope of this series
> further.

I can go either way on this. Using cache.h makes Elijah's current series
a bit more focused. But I think we'd eventually want to go there anyway.
I don't have a strong opinion on doing it now or later.

> > I wonder if there's an easy way to check. I guess adding '-Dint=foo' to
> > the command line, and then putting '#undef int' at the top of
> > git-compat-util would catch just about any code the compiler sees that
> > doesn't have git-compat-util included. :)
> 
> ;-).

So much to my amazement, my off-the-cuff suggestion actually worked
pretty well. The only failures were xdiff (expected, since it does its
own system-header stuff, though IMHO we might think about changing that)
and the programs in t/helper. Here's a patch to address the latter (and
you can see now why I said the above thing about "builtin.h"):

-- >8 --
Subject: [PATCH] test-tool.h: include git-compat-util.h

The test-tool programs include "test-tool.h" as their first
include, which breaks our CodingGuideline of "the first
include must be git-compat-util.h or an equivalent". This
isn't actually a problem, as test-tool.h doesn't define
anything tricky, but we should probably follow our own rule.

Rather than change them all, let's instead make test-tool.h
one of those equivalents, just like we do for builtin.h
(which many of the actual git builtins include first).

Signed-off-by: Jeff King <peff@peff.net>
---
 t/helper/test-tool.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h
index 80cbcf0857..75d7c782f0 100644
--- a/t/helper/test-tool.h
+++ b/t/helper/test-tool.h
@@ -1,6 +1,8 @@
 #ifndef __TEST_TOOL_H__
 #define __TEST_TOOL_H__
 
+#include "git-compat-util.h"
+
 int cmd__chmtime(int argc, const char **argv);
 int cmd__config(int argc, const char **argv);
 int cmd__ctype(int argc, const char **argv);
-- 
2.18.0.1070.g4763fa3c01

Re: [PATCH 0/9] Add missing includes and forward declares

[Jonathan Nieder]

Jeff King wrote:
> On Mon, Aug 13, 2018 at 11:24:37AM -0700, Junio C Hamano wrote:
>> Jeff King wrote:

>>> So I think the better test is a two-line .c file with:
>>>
>>>   #include "git-compat-util.h"
>>>   #include $header_to_check
>>
>> But until that tightening happens, I do not actually mind the
>> two-line .c file started with inclusion of cache.h instead of
>> git-compat-util.h.  That would limit the scope of this series
>> further.
>
> I can go either way on this. Using cache.h makes Elijah's current series
> a bit more focused. But I think we'd eventually want to go there anyway.
> I don't have a strong opinion on doing it now or later.

For what it's worth, I'd been assuming that any header that is not
self-contained after a #include of "git-compat-util.h" is a bug.

It's true that many .c files include git-compat-util.h via cache.h,
making such a bug harder to notice.  But the moment you write a source
file that doesn't include cache.h you'd run into that problem, so it's
worth fixing ahead-of-time anyway.

Thanks,
Jonathan

Re: [PATCH 0/9] Add missing includes and forward declares

[Jonathan Nieder]

Jeff King wrote:

> Subject: [PATCH] test-tool.h: include git-compat-util.h
>
> The test-tool programs include "test-tool.h" as their first
> include, which breaks our CodingGuideline of "the first
> include must be git-compat-util.h or an equivalent". This
> isn't actually a problem, as test-tool.h doesn't define
> anything tricky, but we should probably follow our own rule.
>
> Rather than change them all, let's instead make test-tool.h
> one of those equivalents, just like we do for builtin.h
> (which many of the actual git builtins include first).

I wonder if it would not be simpler to change them all.  It would be
one less special case.

That said,

> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  t/helper/test-tool.h | 2 ++
>  1 file changed, 2 insertions(+)

Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

Thanks.

[PATCHv2 0/6] Add missing includes and forward declares

[Elijah Newren]

This series fixes compilation errors when using a simple test.c file that
includes git-compat-util.h and then exactly one other header (and repeating
this for different headers of git).

Changes since v1:
  - Followed Peff's suggestion to make my simple test .c file first
    #include git-compat-util.h; removes the need for inclusions of this
    header or stdlib.h or stdio.h throughout my series.  (Patches 6-8 of
    v1 no longer necessary at all.)
  - Removed the explanatory comments about why various #include's were
    needed from v1.

Note: the final patch has minor conflicts with topics in flight in
next and pu; it can just be dropped and I can resubmit it later if
that is easier.

Elijah Newren (6):
  Add missing includes and forward declares
  alloc: make allocate_alloc_state and clear_alloc_state more consistent
  Move definition of enum branch_track from cache.h to branch.h
  urlmatch.h: fix include guard
  compat/precompose_utf8.h: use more common include guard style
  Add missing includes and forward declares

 alloc.c                  |  2 +-
 alloc.h                  |  4 +++-
 apply.h                  |  3 +++
 archive.h                |  3 +++
 attr.h                   |  1 +
 bisect.h                 |  2 ++
 branch.h                 | 13 +++++++++++++
 bulk-checkin.h           |  2 ++
 cache.h                  | 10 ----------
 column.h                 |  1 +
 commit-graph.h           |  1 +
 compat/precompose_utf8.h |  3 ++-
 config.c                 |  1 +
 config.h                 |  5 +++++
 connected.h              |  1 +
 convert.h                |  2 ++
 csum-file.h              |  2 ++
 diffcore.h               |  4 ++++
 dir-iterator.h           |  2 ++
 environment.c            |  1 +
 fsck.h                   |  1 +
 fsmonitor.h              |  7 +++++++
 gpg-interface.h          |  2 ++
 khash.h                  |  3 +++
 list-objects-filter.h    |  4 ++++
 list-objects.h           |  4 ++++
 ll-merge.h               |  2 ++
 mailinfo.h               |  2 ++
 mailmap.h                |  2 ++
 merge-recursive.h        |  4 +++-
 notes-merge.h            |  4 ++++
 notes-utils.h            |  3 +++
 notes.h                  |  3 +++
 object-store.h           |  1 +
 object.h                 |  2 ++
 oidmap.h                 |  1 +
 pack-bitmap.h            |  3 +++
 pack-objects.h           |  1 +
 patch-ids.h              |  6 ++++++
 path.h                   |  1 +
 pathspec.h               |  5 +++++
 pretty.h                 |  4 ++++
 reachable.h              |  2 ++
 reflog-walk.h            |  1 +
 refs.h                   |  2 ++
 remote.h                 |  1 +
 repository.h             |  2 ++
 resolve-undo.h           |  2 ++
 revision.h               |  1 +
 send-pack.h              |  4 ++++
 sequencer.h              |  5 +++++
 shortlog.h               |  2 ++
 submodule.h              | 10 ++++++++--
 tempfile.h               |  1 +
 trailer.h                |  2 ++
 tree-walk.h              |  2 ++
 unpack-trees.h           |  5 ++++-
 url.h                    |  2 ++
 urlmatch.h               |  2 ++
 utf8.h                   |  2 ++
 worktree.h               |  1 +
 61 files changed, 160 insertions(+), 17 deletions(-)

-- 
2.18.0.549.gd4454f3f9b

[PATCHv2 1/6] Add missing includes and forward declares

[Elijah Newren]

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 alloc.h               |  2 ++
 apply.h               |  3 +++
 archive.h             |  3 +++
 attr.h                |  1 +
 branch.h              |  2 ++
 bulk-checkin.h        |  2 ++
 column.h              |  1 +
 commit-graph.h        |  1 +
 config.h              |  5 +++++
 connected.h           |  1 +
 convert.h             |  2 ++
 csum-file.h           |  2 ++
 diffcore.h            |  4 ++++
 dir-iterator.h        |  2 ++
 fsck.h                |  1 +
 fsmonitor.h           |  7 +++++++
 gpg-interface.h       |  2 ++
 khash.h               |  3 +++
 list-objects-filter.h |  4 ++++
 list-objects.h        |  4 ++++
 ll-merge.h            |  2 ++
 mailinfo.h            |  2 ++
 mailmap.h             |  2 ++
 merge-recursive.h     |  4 +++-
 notes-merge.h         |  4 ++++
 notes-utils.h         |  3 +++
 notes.h               |  3 +++
 object-store.h        |  1 +
 object.h              |  2 ++
 oidmap.h              |  1 +
 pack-bitmap.h         |  3 +++
 patch-ids.h           |  6 ++++++
 path.h                |  1 +
 pathspec.h            |  5 +++++
 pretty.h              |  4 ++++
 reachable.h           |  2 ++
 reflog-walk.h         |  1 +
 refs.h                |  2 ++
 remote.h              |  1 +
 repository.h          |  2 ++
 resolve-undo.h        |  2 ++
 revision.h            |  1 +
 send-pack.h           |  4 ++++
 sequencer.h           |  5 +++++
 shortlog.h            |  2 ++
 submodule.h           | 10 ++++++++--
 tempfile.h            |  1 +
 trailer.h             |  2 ++
 tree-walk.h           |  2 ++
 unpack-trees.h        |  5 ++++-
 url.h                 |  2 ++
 utf8.h                |  2 ++
 worktree.h            |  1 +
 53 files changed, 138 insertions(+), 4 deletions(-)

diff --git a/alloc.h b/alloc.h
index 3e4e828db4..7a41bb9eb3 100644
--- a/alloc.h
+++ b/alloc.h
@@ -1,9 +1,11 @@
 #ifndef ALLOC_H
 #define ALLOC_H
 
+struct alloc_state;
 struct tree;
 struct commit;
 struct tag;
+struct repository;
 
 void *alloc_blob_node(struct repository *r);
 void *alloc_tree_node(struct repository *r);
diff --git a/apply.h b/apply.h
index b80d8ba736..0b70e1f3f5 100644
--- a/apply.h
+++ b/apply.h
@@ -1,6 +1,9 @@
 #ifndef APPLY_H
 #define APPLY_H
 
+#include "lockfile.h"
+#include "string-list.h"
+
 enum apply_ws_error_action {
 	nowarn_ws_error,
 	warn_on_ws_error,
diff --git a/archive.h b/archive.h
index 1f9954f7cd..64f0451e5c 100644
--- a/archive.h
+++ b/archive.h
@@ -3,6 +3,9 @@
 
 #include "pathspec.h"
 
+struct object_id;
+enum object_type;
+
 struct archiver_args {
 	const char *base;
 	size_t baselen;
diff --git a/attr.h b/attr.h
index 442d464db6..3185538bda 100644
--- a/attr.h
+++ b/attr.h
@@ -7,6 +7,7 @@ struct git_attr;
 /* opaque structures used internally for attribute collection */
 struct all_attrs_item;
 struct attr_stack;
+struct index_state;
 
 /*
  * Given a string, return the gitattribute object that
diff --git a/branch.h b/branch.h
index 473d0a93e9..7d9b330eba 100644
--- a/branch.h
+++ b/branch.h
@@ -1,6 +1,8 @@
 #ifndef BRANCH_H
 #define BRANCH_H
 
+struct strbuf;
+
 /* Functions for acting on the information about branches. */
 
 /*
diff --git a/bulk-checkin.h b/bulk-checkin.h
index a85527318b..f438f93811 100644
--- a/bulk-checkin.h
+++ b/bulk-checkin.h
@@ -4,6 +4,8 @@
 #ifndef BULK_CHECKIN_H
 #define BULK_CHECKIN_H
 
+#include "cache.h"
+
 extern int index_bulk_checkin(struct object_id *oid,
 			      int fd, size_t size, enum object_type type,
 			      const char *path, unsigned flags);
diff --git a/column.h b/column.h
index 0a61917fa7..2567a5cf4d 100644
--- a/column.h
+++ b/column.h
@@ -36,6 +36,7 @@ static inline int column_active(unsigned int colopts)
 	return (colopts & COL_ENABLE_MASK) == COL_ENABLED;
 }
 
+struct string_list;
 extern void print_columns(const struct string_list *list, unsigned int colopts,
 			  const struct column_options *opts);
 
diff --git a/commit-graph.h b/commit-graph.h
index 76e098934a..eea62f8c0e 100644
--- a/commit-graph.h
+++ b/commit-graph.h
@@ -4,6 +4,7 @@
 #include "git-compat-util.h"
 #include "repository.h"
 #include "string-list.h"
+#include "cache.h"
 
 struct commit;
 
diff --git a/config.h b/config.h
index bb2f506b27..75ba1d45ff 100644
--- a/config.h
+++ b/config.h
@@ -1,6 +1,11 @@
 #ifndef CONFIG_H
 #define CONFIG_H
 
+#include "hashmap.h"
+#include "string-list.h"
+
+struct object_id;
+
 /* git_config_parse_key() returns these negated: */
 #define CONFIG_INVALID_KEY 1
 #define CONFIG_NO_SECTION_OR_NAME 2
diff --git a/connected.h b/connected.h
index 322dc76372..e4c961817d 100644
--- a/connected.h
+++ b/connected.h
@@ -1,6 +1,7 @@
 #ifndef CONNECTED_H
 #define CONNECTED_H
 
+struct object_id;
 struct transport;
 
 /*
diff --git a/convert.h b/convert.h
index 01385d9288..76c654385d 100644
--- a/convert.h
+++ b/convert.h
@@ -7,6 +7,8 @@
 #include "string-list.h"
 
 struct index_state;
+struct object_id;
+struct strbuf;
 
 #define CONV_EOL_RNDTRP_DIE   (1<<0) /* Die if CRLF to LF to CRLF is different */
 #define CONV_EOL_RNDTRP_WARN  (1<<1) /* Warn if CRLF to LF to CRLF is different */
diff --git a/csum-file.h b/csum-file.h
index c5a2e335e7..3bf7184736 100644
--- a/csum-file.h
+++ b/csum-file.h
@@ -1,6 +1,8 @@
 #ifndef CSUM_FILE_H
 #define CSUM_FILE_H
 
+#include "hash.h"
+
 struct progress;
 
 /* A SHA1-protected file */
diff --git a/diffcore.h b/diffcore.h
index a30da161da..0dd1e9a63a 100644
--- a/diffcore.h
+++ b/diffcore.h
@@ -4,6 +4,10 @@
 #ifndef DIFFCORE_H
 #define DIFFCORE_H
 
+#include "cache.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-iterator.h b/dir-iterator.h
index 27739e6c29..970793d07a 100644
--- a/dir-iterator.h
+++ b/dir-iterator.h
@@ -1,6 +1,8 @@
 #ifndef DIR_ITERATOR_H
 #define DIR_ITERATOR_H
 
+#include "strbuf.h"
+
 /*
  * Iterate over a directory tree.
  *
diff --git a/fsck.h b/fsck.h
index c3cf5e0034..0c7e8c9428 100644
--- a/fsck.h
+++ b/fsck.h
@@ -6,6 +6,7 @@
 #define FSCK_IGNORE 3
 
 struct fsck_options;
+struct object;
 
 void fsck_set_msg_type(struct fsck_options *options,
 		const char *msg_id, const char *msg_type);
diff --git a/fsmonitor.h b/fsmonitor.h
index 65f3743636..69ac5a53b5 100644
--- a/fsmonitor.h
+++ b/fsmonitor.h
@@ -1,6 +1,13 @@
 #ifndef FSMONITOR_H
 #define FSMONITOR_H
 
+#include "cache.h"
+#include "dir.h"
+
+struct cache_entry;
+struct index_state;
+struct strbuf;
+
 extern struct trace_key trace_fsmonitor;
 
 /*
diff --git a/gpg-interface.h b/gpg-interface.h
index 5ecff4aa0c..acf50c4610 100644
--- a/gpg-interface.h
+++ b/gpg-interface.h
@@ -1,6 +1,8 @@
 #ifndef GPG_INTERFACE_H
 #define GPG_INTERFACE_H
 
+struct strbuf;
+
 #define GPG_VERIFY_VERBOSE		1
 #define GPG_VERIFY_RAW			2
 #define GPG_VERIFY_OMIT_STATUS	4
diff --git a/khash.h b/khash.h
index c0da40daa7..011c435163 100644
--- a/khash.h
+++ b/khash.h
@@ -26,6 +26,9 @@
 #ifndef __AC_KHASH_H
 #define __AC_KHASH_H
 
+#include "hashmap.h"
+#include "cache.h"
+
 #define AC_VERSION_KHASH_H "0.2.8"
 
 typedef uint32_t khint32_t;
diff --git a/list-objects-filter.h b/list-objects-filter.h
index a963d0274c..a6f6b4990b 100644
--- a/list-objects-filter.h
+++ b/list-objects-filter.h
@@ -1,6 +1,10 @@
 #ifndef LIST_OBJECTS_FILTER_H
 #define LIST_OBJECTS_FILTER_H
 
+struct list_objects_filter_options;
+struct object;
+struct oidset;
+
 /*
  * During list-object traversal we allow certain objects to be
  * filtered (omitted) from the result.  The active filter uses
diff --git a/list-objects.h b/list-objects.h
index aa618d7f45..ad40762926 100644
--- a/list-objects.h
+++ b/list-objects.h
@@ -1,6 +1,10 @@
 #ifndef LIST_OBJECTS_H
 #define LIST_OBJECTS_H
 
+struct commit;
+struct object;
+struct rev_info;
+
 typedef void (*show_commit_fn)(struct commit *, void *);
 typedef void (*show_object_fn)(struct object *, const char *, void *);
 void traverse_commit_list(struct rev_info *, show_commit_fn, show_object_fn, void *);
diff --git a/ll-merge.h b/ll-merge.h
index 244a31f55a..b72b19921e 100644
--- a/ll-merge.h
+++ b/ll-merge.h
@@ -5,6 +5,8 @@
 #ifndef LL_MERGE_H
 #define LL_MERGE_H
 
+#include "xdiff/xdiff.h"
+
 struct ll_merge_options {
 	unsigned virtual_ancestor : 1;
 	unsigned variant : 2;	/* favor ours, favor theirs, or union merge */
diff --git a/mailinfo.h b/mailinfo.h
index 04a25351d6..766c03dd1d 100644
--- a/mailinfo.h
+++ b/mailinfo.h
@@ -1,6 +1,8 @@
 #ifndef MAILINFO_H
 #define MAILINFO_H
 
+#include "strbuf.h"
+
 #define MAX_BOUNDARIES 5
 
 struct mailinfo {
diff --git a/mailmap.h b/mailmap.h
index ed7c93b05c..d0e65646cb 100644
--- a/mailmap.h
+++ b/mailmap.h
@@ -1,6 +1,8 @@
 #ifndef MAILMAP_H
 #define MAILMAP_H
 
+struct string_list;
+
 int read_mailmap(struct string_list *map, char **repo_abbrev);
 void clear_mailmap(struct string_list *map);
 
diff --git a/merge-recursive.h b/merge-recursive.h
index fa7bc6b683..0c46a5a4ff 100644
--- a/merge-recursive.h
+++ b/merge-recursive.h
@@ -1,8 +1,10 @@
 #ifndef MERGE_RECURSIVE_H
 #define MERGE_RECURSIVE_H
 
-#include "unpack-trees.h"
 #include "string-list.h"
+#include "unpack-trees.h"
+
+struct commit;
 
 struct merge_options {
 	const char *ancestor;
diff --git a/notes-merge.h b/notes-merge.h
index f815f23451..6c74e9385b 100644
--- a/notes-merge.h
+++ b/notes-merge.h
@@ -2,6 +2,10 @@
 #define NOTES_MERGE_H
 
 #include "notes-utils.h"
+#include "strbuf.h"
+
+struct commit;
+struct object_id;
 
 #define NOTES_MERGE_WORKTREE "NOTES_MERGE_WORKTREE"
 
diff --git a/notes-utils.h b/notes-utils.h
index 5d79cbef51..5408306528 100644
--- a/notes-utils.h
+++ b/notes-utils.h
@@ -3,6 +3,9 @@
 
 #include "notes.h"
 
+struct commit_list;
+struct object_id;
+
 /*
  * Create new notes commit from the given notes tree
  *
diff --git a/notes.h b/notes.h
index 0433f45db5..414bc6855a 100644
--- a/notes.h
+++ b/notes.h
@@ -3,6 +3,9 @@
 
 #include "string-list.h"
 
+struct object_id;
+struct strbuf;
+
 /*
  * Function type for combining two notes annotating the same object.
  *
diff --git a/object-store.h b/object-store.h
index e481f7ad41..b6f2c732c4 100644
--- a/object-store.h
+++ b/object-store.h
@@ -1,6 +1,7 @@
 #ifndef OBJECT_STORE_H
 #define OBJECT_STORE_H
 
+#include "cache.h"
 #include "oidmap.h"
 #include "list.h"
 #include "sha1-array.h"
diff --git a/object.h b/object.h
index 177b1a4571..6e28fdd0b4 100644
--- a/object.h
+++ b/object.h
@@ -1,6 +1,8 @@
 #ifndef OBJECT_H
 #define OBJECT_H
 
+#include "cache.h"
+
 struct buffer_slab;
 
 struct parsed_object_pool {
diff --git a/oidmap.h b/oidmap.h
index d3cd2bb590..72430b611e 100644
--- a/oidmap.h
+++ b/oidmap.h
@@ -1,6 +1,7 @@
 #ifndef OIDMAP_H
 #define OIDMAP_H
 
+#include "cache.h"
 #include "hashmap.h"
 
 /*
diff --git a/pack-bitmap.h b/pack-bitmap.h
index 4555907dee..8a04741e12 100644
--- a/pack-bitmap.h
+++ b/pack-bitmap.h
@@ -5,6 +5,9 @@
 #include "khash.h"
 #include "pack-objects.h"
 
+struct commit;
+struct rev_info;
+
 struct bitmap_disk_header {
 	char magic[4];
 	uint16_t version;
diff --git a/patch-ids.h b/patch-ids.h
index bec0f727a7..79ac9a8498 100644
--- a/patch-ids.h
+++ b/patch-ids.h
@@ -1,6 +1,12 @@
 #ifndef PATCH_IDS_H
 #define PATCH_IDS_H
 
+#include "diff.h"
+#include "hashmap.h"
+
+struct commit;
+struct object_id;
+
 struct patch_id {
 	struct hashmap_entry ent;
 	struct object_id patch_id;
diff --git a/path.h b/path.h
index ed6732e5a2..b654ea8ff5 100644
--- a/path.h
+++ b/path.h
@@ -2,6 +2,7 @@
 #define PATH_H
 
 struct repository;
+struct strbuf;
 
 /*
  * The result to all functions which return statically allocated memory may be
diff --git a/pathspec.h b/pathspec.h
index 099a170c2e..5a80c14b6b 100644
--- a/pathspec.h
+++ b/pathspec.h
@@ -1,6 +1,11 @@
 #ifndef PATHSPEC_H
 #define PATHSPEC_H
 
+#include "string.h"
+#include "strings.h"
+
+struct index_state;
+
 /* Pathspec magic */
 #define PATHSPEC_FROMTOP	(1<<0)
 #define PATHSPEC_MAXDEPTH	(1<<1)
diff --git a/pretty.h b/pretty.h
index 5c85d94e33..7359d318a9 100644
--- a/pretty.h
+++ b/pretty.h
@@ -1,7 +1,11 @@
 #ifndef PRETTY_H
 #define PRETTY_H
 
+#include "cache.h"
+#include "string-list.h"
+
 struct commit;
+struct strbuf;
 
 /* Commit formats */
 enum cmit_fmt {
diff --git a/reachable.h b/reachable.h
index 3c00fa0526..18b0f9f2f0 100644
--- a/reachable.h
+++ b/reachable.h
@@ -2,6 +2,8 @@
 #define REACHEABLE_H
 
 struct progress;
+struct rev_info;
+
 extern int add_unseen_recent_objects_to_traversal(struct rev_info *revs,
 						  timestamp_t timestamp);
 extern void mark_reachable_objects(struct rev_info *revs, int mark_reflog,
diff --git a/reflog-walk.h b/reflog-walk.h
index 7553c448fe..cb3e73755d 100644
--- a/reflog-walk.h
+++ b/reflog-walk.h
@@ -3,6 +3,7 @@
 
 #include "cache.h"
 
+struct commit;
 struct reflog_walk_info;
 
 extern void init_reflog_walk(struct reflog_walk_info **info);
diff --git a/refs.h b/refs.h
index cc2fb4c68c..bd52c1bbae 100644
--- a/refs.h
+++ b/refs.h
@@ -3,8 +3,10 @@
 
 struct object_id;
 struct ref_store;
+struct repository;
 struct strbuf;
 struct string_list;
+struct string_list_item;
 struct worktree;
 
 /*
diff --git a/remote.h b/remote.h
index 976292152c..88f8480c71 100644
--- a/remote.h
+++ b/remote.h
@@ -1,6 +1,7 @@
 #ifndef REMOTE_H
 #define REMOTE_H
 
+#include "cache.h"
 #include "parse-options.h"
 #include "hashmap.h"
 #include "refspec.h"
diff --git a/repository.h b/repository.h
index b9413be50c..13c8799678 100644
--- a/repository.h
+++ b/repository.h
@@ -1,6 +1,8 @@
 #ifndef REPOSITORY_H
 #define REPOSITORY_H
 
+#include "path.h"
+
 struct config_set;
 struct git_hash_algo;
 struct index_state;
diff --git a/resolve-undo.h b/resolve-undo.h
index 87291904bd..fbe348efac 100644
--- a/resolve-undo.h
+++ b/resolve-undo.h
@@ -1,6 +1,8 @@
 #ifndef RESOLVE_UNDO_H
 #define RESOLVE_UNDO_H
 
+#include "cache.h"
+
 struct resolve_undo_info {
 	unsigned int mode[3];
 	struct object_id oid[3];
diff --git a/revision.h b/revision.h
index c599c34da9..850e508783 100644
--- a/revision.h
+++ b/revision.h
@@ -1,6 +1,7 @@
 #ifndef REVISION_H
 #define REVISION_H
 
+#include "commit.h"
 #include "parse-options.h"
 #include "grep.h"
 #include "notes.h"
diff --git a/send-pack.h b/send-pack.h
index 6af71f7008..e148fcd960 100644
--- a/send-pack.h
+++ b/send-pack.h
@@ -3,6 +3,10 @@
 
 #include "string-list.h"
 
+struct child_process;
+struct oid_array;
+struct ref;
+
 /* Possible values for push_cert field in send_pack_args. */
 #define SEND_PACK_PUSH_CERT_NEVER 0
 #define SEND_PACK_PUSH_CERT_IF_ASKED 1
diff --git a/sequencer.h b/sequencer.h
index c5787c6b56..c751c9d6e4 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -1,6 +1,11 @@
 #ifndef SEQUENCER_H
 #define SEQUENCER_H
 
+#include "cache.h"
+#include "strbuf.h"
+
+struct commit;
+
 const char *git_path_commit_editmsg(void);
 const char *git_path_seq_dir(void);
 
diff --git a/shortlog.h b/shortlog.h
index 5d64cfe929..2fa61c4294 100644
--- a/shortlog.h
+++ b/shortlog.h
@@ -3,6 +3,8 @@
 
 #include "string-list.h"
 
+struct commit;
+
 struct shortlog {
 	struct string_list list;
 	int summary;
diff --git a/submodule.h b/submodule.h
index 4644683e6c..3adbc08915 100644
--- a/submodule.h
+++ b/submodule.h
@@ -1,11 +1,17 @@
 #ifndef SUBMODULE_H
 #define SUBMODULE_H
 
-struct repository;
-struct diff_options;
 struct argv_array;
+struct cache_entry;
+struct diff_options;
+struct index_state;
+struct object_id;
 struct oid_array;
+struct pathspec;
 struct remote;
+struct repository;
+struct string_list;
+struct strbuf;
 
 enum {
 	RECURSE_SUBMODULES_ONLY = -5,
diff --git a/tempfile.h b/tempfile.h
index 8959c5f1b5..36434eb6fa 100644
--- a/tempfile.h
+++ b/tempfile.h
@@ -2,6 +2,7 @@
 #define TEMPFILE_H
 
 #include "list.h"
+#include "strbuf.h"
 
 /*
  * Handle temporary files.
diff --git a/trailer.h b/trailer.h
index 6d7f8c2a52..9c10026c35 100644
--- a/trailer.h
+++ b/trailer.h
@@ -3,6 +3,8 @@
 
 #include "list.h"
 
+struct strbuf;
+
 enum trailer_where {
 	WHERE_DEFAULT,
 	WHERE_END,
diff --git a/tree-walk.h b/tree-walk.h
index 805f58f00f..196831007e 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 struct object_id *oid;
 	const char *path;
diff --git a/unpack-trees.h b/unpack-trees.h
index c2b434c606..fdb2e3443d 100644
--- a/unpack-trees.h
+++ b/unpack-trees.h
@@ -1,11 +1,14 @@
 #ifndef UNPACK_TREES_H
 #define UNPACK_TREES_H
 
-#include "tree-walk.h"
 #include "argv-array.h"
+#include "cache.h"
+#include "string-list.h"
+#include "tree-walk.h"
 
 #define MAX_UNPACK_TREES 8
 
+struct cache_entry;
 struct unpack_trees_options;
 struct exclude_list;
 
diff --git a/url.h b/url.h
index abdaf6fa30..f311c40a46 100644
--- a/url.h
+++ b/url.h
@@ -1,6 +1,8 @@
 #ifndef URL_H
 #define URL_H
 
+struct strbuf;
+
 extern int is_url(const char *url);
 extern int is_urlschemechar(int first_flag, int ch);
 extern char *url_decode(const char *url);
diff --git a/utf8.h b/utf8.h
index db73a2d8d3..dd5ee1052a 100644
--- a/utf8.h
+++ b/utf8.h
@@ -1,6 +1,8 @@
 #ifndef GIT_UTF8_H
 #define GIT_UTF8_H
 
+struct strbuf;
+
 typedef unsigned int ucs_char_t;  /* assuming 32bit int */
 
 size_t display_mode_esc_sequence_len(const char *s);
diff --git a/worktree.h b/worktree.h
index fe38ce10c3..df3fc30f73 100644
--- a/worktree.h
+++ b/worktree.h
@@ -1,6 +1,7 @@
 #ifndef WORKTREE_H
 #define WORKTREE_H
 
+#include "cache.h"
 #include "refs.h"
 
 struct strbuf;
-- 
2.18.0.549.gd4454f3f9b

[PATCHv2 2/6] alloc: make allocate_alloc_state and clear_alloc_state more consistent

[Elijah Newren]

Since both functions are using the same data type, they should either both
refer to it as void *, or both use the real type (struct alloc_state *).
Opt for the latter.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 alloc.c | 2 +-
 alloc.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/alloc.c b/alloc.c
index c2fc5d6888..e7aa81b7aa 100644
--- a/alloc.c
+++ b/alloc.c
@@ -36,7 +36,7 @@ struct alloc_state {
 	int slab_nr, slab_alloc;
 };
 
-void *allocate_alloc_state(void)
+struct alloc_state *allocate_alloc_state(void)
 {
 	return xcalloc(1, sizeof(struct alloc_state));
 }
diff --git a/alloc.h b/alloc.h
index 7a41bb9eb3..ba356ed847 100644
--- a/alloc.h
+++ b/alloc.h
@@ -15,7 +15,7 @@ void *alloc_object_node(struct repository *r);
 void alloc_report(struct repository *r);
 unsigned int alloc_commit_index(struct repository *r);
 
-void *allocate_alloc_state(void);
+struct alloc_state *allocate_alloc_state(void);
 void clear_alloc_state(struct alloc_state *s);
 
 #endif
-- 
2.18.0.549.gd4454f3f9b

[PATCHv2 3/6] Move definition of enum branch_track from cache.h to branch.h

[Elijah Newren]

'branch_track' feels more closely related to branching, and it is
needed later in branch.h; rather than #include'ing cache.h in branch.h
for this small enum, just move the enum and the external declaration
for git_branch_track to branch.h.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 branch.h      | 11 +++++++++++
 cache.h       | 10 ----------
 config.c      |  1 +
 environment.c |  1 +
 4 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/branch.h b/branch.h
index 7d9b330eba..5cace4581f 100644
--- a/branch.h
+++ b/branch.h
@@ -3,6 +3,17 @@
 
 struct strbuf;
 
+enum branch_track {
+	BRANCH_TRACK_UNSPECIFIED = -1,
+	BRANCH_TRACK_NEVER = 0,
+	BRANCH_TRACK_REMOTE,
+	BRANCH_TRACK_ALWAYS,
+	BRANCH_TRACK_EXPLICIT,
+	BRANCH_TRACK_OVERRIDE
+};
+
+extern enum branch_track git_branch_track;
+
 /* Functions for acting on the information about branches. */
 
 /*
diff --git a/cache.h b/cache.h
index 8dc7134f00..a1c0c594fb 100644
--- a/cache.h
+++ b/cache.h
@@ -919,15 +919,6 @@ enum log_refs_config {
 };
 extern enum log_refs_config log_all_ref_updates;
 
-enum branch_track {
-	BRANCH_TRACK_UNSPECIFIED = -1,
-	BRANCH_TRACK_NEVER = 0,
-	BRANCH_TRACK_REMOTE,
-	BRANCH_TRACK_ALWAYS,
-	BRANCH_TRACK_EXPLICIT,
-	BRANCH_TRACK_OVERRIDE
-};
-
 enum rebase_setup_type {
 	AUTOREBASE_NEVER = 0,
 	AUTOREBASE_LOCAL,
@@ -944,7 +935,6 @@ enum push_default_type {
 	PUSH_DEFAULT_UNSPECIFIED
 };
 
-extern enum branch_track git_branch_track;
 extern enum rebase_setup_type autorebase;
 extern enum push_default_type push_default;
 
diff --git a/config.c b/config.c
index 66645047eb..b60d7c0308 100644
--- a/config.c
+++ b/config.c
@@ -5,6 +5,7 @@
  * Copyright (C) Johannes Schindelin, 2005
  *
  */
+#include "branch.h"
 #include "cache.h"
 #include "config.h"
 #include "repository.h"
diff --git a/environment.c b/environment.c
index 6cf0079389..920362900c 100644
--- a/environment.c
+++ b/environment.c
@@ -7,6 +7,7 @@
  * even if you might want to know where the git directory etc
  * are.
  */
+#include "branch.h"
 #include "cache.h"
 #include "repository.h"
 #include "config.h"
-- 
2.18.0.549.gd4454f3f9b

Re: [PATCHv2 3/6] Move definition of enum branch_track from cache.h to branch.h

[Ramsay Jones]

On 11/08/18 21:50, Elijah Newren wrote:
> 'branch_track' feels more closely related to branching, and it is
> needed later in branch.h; rather than #include'ing cache.h in branch.h
> for this small enum, just move the enum and the external declaration
> for git_branch_track to branch.h.
> 
> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
>  branch.h      | 11 +++++++++++
>  cache.h       | 10 ----------
>  config.c      |  1 +
>  environment.c |  1 +
>  4 files changed, 13 insertions(+), 10 deletions(-)
> 
> diff --git a/branch.h b/branch.h
> index 7d9b330eba..5cace4581f 100644
> --- a/branch.h
> +++ b/branch.h
> @@ -3,6 +3,17 @@
>  
>  struct strbuf;
>  
> +enum branch_track {
> +	BRANCH_TRACK_UNSPECIFIED = -1,
> +	BRANCH_TRACK_NEVER = 0,
> +	BRANCH_TRACK_REMOTE,
> +	BRANCH_TRACK_ALWAYS,
> +	BRANCH_TRACK_EXPLICIT,
> +	BRANCH_TRACK_OVERRIDE
> +};
> +
> +extern enum branch_track git_branch_track;
> +
>  /* Functions for acting on the information about branches. */
>  
>  /*
> diff --git a/cache.h b/cache.h
> index 8dc7134f00..a1c0c594fb 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -919,15 +919,6 @@ enum log_refs_config {
>  };
>  extern enum log_refs_config log_all_ref_updates;
>  
> -enum branch_track {
> -	BRANCH_TRACK_UNSPECIFIED = -1,
> -	BRANCH_TRACK_NEVER = 0,
> -	BRANCH_TRACK_REMOTE,
> -	BRANCH_TRACK_ALWAYS,
> -	BRANCH_TRACK_EXPLICIT,
> -	BRANCH_TRACK_OVERRIDE
> -};
> -
>  enum rebase_setup_type {
>  	AUTOREBASE_NEVER = 0,
>  	AUTOREBASE_LOCAL,
> @@ -944,7 +935,6 @@ enum push_default_type {
>  	PUSH_DEFAULT_UNSPECIFIED
>  };
>  
> -extern enum branch_track git_branch_track;
>  extern enum rebase_setup_type autorebase;
>  extern enum push_default_type push_default;
>  
> diff --git a/config.c b/config.c
> index 66645047eb..b60d7c0308 100644
> --- a/config.c
> +++ b/config.c
> @@ -5,6 +5,7 @@
>   * Copyright (C) Johannes Schindelin, 2005
>   *
>   */
> +#include "branch.h"

git-compat-util.h, cache.h or builtin.h _must_ be the first
header file #included in a C file, so this new #include of
the "branch.h" header should be moved ...

>  #include "cache.h"

... after here.

>  #include "config.h"
>  #include "repository.h"
> diff --git a/environment.c b/environment.c
> index 6cf0079389..920362900c 100644
> --- a/environment.c
> +++ b/environment.c
> @@ -7,6 +7,7 @@
>   * even if you might want to know where the git directory etc
>   * are.
>   */
> +#include "branch.h"

ditto

>  #include "cache.h"
>  #include "repository.h"
>  #include "config.h"
> 

ATB,
Ramsay Jones

[PATCHv2 4/6] urlmatch.h: fix include guard

[Elijah Newren]

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 urlmatch.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/urlmatch.h b/urlmatch.h
index 37ee5da85e..e482148248 100644
--- a/urlmatch.h
+++ b/urlmatch.h
@@ -1,4 +1,6 @@
 #ifndef URL_MATCH_H
+#define URL_MATCH_H
+
 #include "string-list.h"
 
 struct url_info {
-- 
2.18.0.549.gd4454f3f9b

[PATCHv2 5/6] compat/precompose_utf8.h: use more common include guard style

[Elijah Newren]

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 compat/precompose_utf8.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/compat/precompose_utf8.h b/compat/precompose_utf8.h
index a94e7c4342..6f843d3e1a 100644
--- a/compat/precompose_utf8.h
+++ b/compat/precompose_utf8.h
@@ -1,4 +1,6 @@
 #ifndef PRECOMPOSE_UNICODE_H
+#define PRECOMPOSE_UNICODE_H
+
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <dirent.h>
@@ -41,5 +43,4 @@ int precompose_utf8_closedir(PREC_DIR *dirp);
 #define DIR PREC_DIR
 #endif /* PRECOMPOSE_UNICODE_C */
 
-#define  PRECOMPOSE_UNICODE_H
 #endif /* PRECOMPOSE_UNICODE_H */
-- 
2.18.0.549.gd4454f3f9b

[PATCHv2 6/6] Add missing includes and forward declares

[Elijah Newren]

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 bisect.h       | 2 ++
 pack-objects.h | 1 +
 2 files changed, 3 insertions(+)

diff --git a/bisect.h b/bisect.h
index a5d9248a47..34df209351 100644
--- a/bisect.h
+++ b/bisect.h
@@ -1,6 +1,8 @@
 #ifndef BISECT_H
 #define BISECT_H
 
+struct commit_list;
+
 /*
  * Find bisection. If something is found, `reaches` will be the number of
  * commits that the best commit reaches. `all` will be the count of
diff --git a/pack-objects.h b/pack-objects.h
index edf74dabdd..08c6b57d49 100644
--- a/pack-objects.h
+++ b/pack-objects.h
@@ -2,6 +2,7 @@
 #define PACK_OBJECTS_H
 
 #include "object-store.h"
+#include "pack.h"
 
 #define DEFAULT_DELTA_CACHE_SIZE (256 * 1024 * 1024)
 
-- 
2.18.0.549.gd4454f3f9b

[PATCHv3 0/6] Add missing includes and forward declares

[Elijah Newren]

This series fixes compilation errors when using a simple test.c file that
includes git-compat-util.h and then exactly one other header (and repeating
this for different headers of git).

Changes since v2 (full range-diff below):
  - Ensure the first #include in C files is either git-compat-util.h,
    cache.h, or builtin.h (pointed out by Ramsay).  Also, although not
    important, also tweaked a couple other #include orders to put cache.h
    first in some .h files.

Elijah Newren (6):
  Add missing includes and forward declares
  alloc: make allocate_alloc_state and clear_alloc_state more consistent
  Move definition of enum branch_track from cache.h to branch.h
  urlmatch.h: fix include guard
  compat/precompose_utf8.h: use more common include guard style
  Add missing includes and forward declares

 alloc.c                  |  2 +-
 alloc.h                  |  4 +++-
 apply.h                  |  3 +++
 archive.h                |  3 +++
 attr.h                   |  1 +
 bisect.h                 |  2 ++
 branch.h                 | 13 +++++++++++++
 bulk-checkin.h           |  2 ++
 cache.h                  | 10 ----------
 column.h                 |  1 +
 commit-graph.h           |  1 +
 compat/precompose_utf8.h |  3 ++-
 config.c                 |  1 +
 config.h                 |  5 +++++
 connected.h              |  1 +
 convert.h                |  2 ++
 csum-file.h              |  2 ++
 diffcore.h               |  4 ++++
 dir-iterator.h           |  2 ++
 environment.c            |  1 +
 fsck.h                   |  1 +
 fsmonitor.h              |  7 +++++++
 gpg-interface.h          |  2 ++
 khash.h                  |  3 +++
 list-objects-filter.h    |  4 ++++
 list-objects.h           |  4 ++++
 ll-merge.h               |  2 ++
 mailinfo.h               |  2 ++
 mailmap.h                |  2 ++
 merge-recursive.h        |  4 +++-
 notes-merge.h            |  4 ++++
 notes-utils.h            |  3 +++
 notes.h                  |  3 +++
 object-store.h           |  1 +
 object.h                 |  2 ++
 oidmap.h                 |  1 +
 pack-bitmap.h            |  3 +++
 pack-objects.h           |  1 +
 patch-ids.h              |  6 ++++++
 path.h                   |  1 +
 pathspec.h               |  5 +++++
 pretty.h                 |  4 ++++
 reachable.h              |  2 ++
 reflog-walk.h            |  1 +
 refs.h                   |  2 ++
 remote.h                 |  1 +
 repository.h             |  2 ++
 resolve-undo.h           |  2 ++
 revision.h               |  1 +
 send-pack.h              |  4 ++++
 sequencer.h              |  5 +++++
 shortlog.h               |  2 ++
 submodule.h              | 10 ++++++++--
 tempfile.h               |  1 +
 trailer.h                |  2 ++
 tree-walk.h              |  2 ++
 unpack-trees.h           |  5 ++++-
 url.h                    |  2 ++
 urlmatch.h               |  2 ++
 utf8.h                   |  2 ++
 worktree.h               |  1 +
 61 files changed, 160 insertions(+), 17 deletions(-)

1:  2bd09172ef ! 1:  f7d50cef3b Add missing includes and forward declares
    @@ -242,8 +242,8 @@
      #ifndef __AC_KHASH_H
      #define __AC_KHASH_H
      
    -+#include "hashmap.h"
     +#include "cache.h"
    ++#include "hashmap.h"
     +
      #define AC_VERSION_KHASH_H "0.2.8"
      
    @@ -693,8 +693,8 @@
      #define UNPACK_TREES_H
      
     -#include "tree-walk.h"
    - #include "argv-array.h"
     +#include "cache.h"
    + #include "argv-array.h"
     +#include "string-list.h"
     +#include "tree-walk.h"
      
2:  854860e967 = 2:  e46bf7d601 alloc: make allocate_alloc_state and clear_alloc_state more consistent
3:  c91c4eaa38 ! 3:  aec8ddda59 Move definition of enum branch_track from cache.h to branch.h
    @@ -63,22 +63,22 @@
     --- a/config.c
     +++ b/config.c
     @@
    -  * Copyright (C) Johannes Schindelin, 2005
       *
       */
    -+#include "branch.h"
      #include "cache.h"
    ++#include "branch.h"
      #include "config.h"
      #include "repository.h"
    + #include "lockfile.h"
     
     diff --git a/environment.c b/environment.c
     --- a/environment.c
     +++ b/environment.c
     @@
    -  * even if you might want to know where the git directory etc
       * are.
       */
    -+#include "branch.h"
      #include "cache.h"
    ++#include "branch.h"
      #include "repository.h"
      #include "config.h"
    + #include "refs.h"
4:  550568912b = 4:  14e33fb5ff urlmatch.h: fix include guard
5:  1186b37a3c = 5:  3e02a381af compat/precompose_utf8.h: use more common include guard style
6:  0642f8ecf0 = 6:  aca61bade1 Add missing includes and forward declares

-- 
2.18.0.549.gd4454f3f9b

[PATCHv3 1/6] Add missing includes and forward declares

[Elijah Newren]

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 alloc.h               |  2 ++
 apply.h               |  3 +++
 archive.h             |  3 +++
 attr.h                |  1 +
 branch.h              |  2 ++
 bulk-checkin.h        |  2 ++
 column.h              |  1 +
 commit-graph.h        |  1 +
 config.h              |  5 +++++
 connected.h           |  1 +
 convert.h             |  2 ++
 csum-file.h           |  2 ++
 diffcore.h            |  4 ++++
 dir-iterator.h        |  2 ++
 fsck.h                |  1 +
 fsmonitor.h           |  7 +++++++
 gpg-interface.h       |  2 ++
 khash.h               |  3 +++
 list-objects-filter.h |  4 ++++
 list-objects.h        |  4 ++++
 ll-merge.h            |  2 ++
 mailinfo.h            |  2 ++
 mailmap.h             |  2 ++
 merge-recursive.h     |  4 +++-
 notes-merge.h         |  4 ++++
 notes-utils.h         |  3 +++
 notes.h               |  3 +++
 object-store.h        |  1 +
 object.h              |  2 ++
 oidmap.h              |  1 +
 pack-bitmap.h         |  3 +++
 patch-ids.h           |  6 ++++++
 path.h                |  1 +
 pathspec.h            |  5 +++++
 pretty.h              |  4 ++++
 reachable.h           |  2 ++
 reflog-walk.h         |  1 +
 refs.h                |  2 ++
 remote.h              |  1 +
 repository.h          |  2 ++
 resolve-undo.h        |  2 ++
 revision.h            |  1 +
 send-pack.h           |  4 ++++
 sequencer.h           |  5 +++++
 shortlog.h            |  2 ++
 submodule.h           | 10 ++++++++--
 tempfile.h            |  1 +
 trailer.h             |  2 ++
 tree-walk.h           |  2 ++
 unpack-trees.h        |  5 ++++-
 url.h                 |  2 ++
 utf8.h                |  2 ++
 worktree.h            |  1 +
 53 files changed, 138 insertions(+), 4 deletions(-)

diff --git a/alloc.h b/alloc.h
index 3e4e828db4..7a41bb9eb3 100644
--- a/alloc.h
+++ b/alloc.h
@@ -1,9 +1,11 @@
 #ifndef ALLOC_H
 #define ALLOC_H
 
+struct alloc_state;
 struct tree;
 struct commit;
 struct tag;
+struct repository;
 
 void *alloc_blob_node(struct repository *r);
 void *alloc_tree_node(struct repository *r);
diff --git a/apply.h b/apply.h
index b80d8ba736..0b70e1f3f5 100644
--- a/apply.h
+++ b/apply.h
@@ -1,6 +1,9 @@
 #ifndef APPLY_H
 #define APPLY_H
 
+#include "lockfile.h"
+#include "string-list.h"
+
 enum apply_ws_error_action {
 	nowarn_ws_error,
 	warn_on_ws_error,
diff --git a/archive.h b/archive.h
index 1f9954f7cd..64f0451e5c 100644
--- a/archive.h
+++ b/archive.h
@@ -3,6 +3,9 @@
 
 #include "pathspec.h"
 
+struct object_id;
+enum object_type;
+
 struct archiver_args {
 	const char *base;
 	size_t baselen;
diff --git a/attr.h b/attr.h
index 442d464db6..3185538bda 100644
--- a/attr.h
+++ b/attr.h
@@ -7,6 +7,7 @@ struct git_attr;
 /* opaque structures used internally for attribute collection */
 struct all_attrs_item;
 struct attr_stack;
+struct index_state;
 
 /*
  * Given a string, return the gitattribute object that
diff --git a/branch.h b/branch.h
index 473d0a93e9..7d9b330eba 100644
--- a/branch.h
+++ b/branch.h
@@ -1,6 +1,8 @@
 #ifndef BRANCH_H
 #define BRANCH_H
 
+struct strbuf;
+
 /* Functions for acting on the information about branches. */
 
 /*
diff --git a/bulk-checkin.h b/bulk-checkin.h
index a85527318b..f438f93811 100644
--- a/bulk-checkin.h
+++ b/bulk-checkin.h
@@ -4,6 +4,8 @@
 #ifndef BULK_CHECKIN_H
 #define BULK_CHECKIN_H
 
+#include "cache.h"
+
 extern int index_bulk_checkin(struct object_id *oid,
 			      int fd, size_t size, enum object_type type,
 			      const char *path, unsigned flags);
diff --git a/column.h b/column.h
index 0a61917fa7..2567a5cf4d 100644
--- a/column.h
+++ b/column.h
@@ -36,6 +36,7 @@ static inline int column_active(unsigned int colopts)
 	return (colopts & COL_ENABLE_MASK) == COL_ENABLED;
 }
 
+struct string_list;
 extern void print_columns(const struct string_list *list, unsigned int colopts,
 			  const struct column_options *opts);
 
diff --git a/commit-graph.h b/commit-graph.h
index 76e098934a..eea62f8c0e 100644
--- a/commit-graph.h
+++ b/commit-graph.h
@@ -4,6 +4,7 @@
 #include "git-compat-util.h"
 #include "repository.h"
 #include "string-list.h"
+#include "cache.h"
 
 struct commit;
 
diff --git a/config.h b/config.h
index bb2f506b27..75ba1d45ff 100644
--- a/config.h
+++ b/config.h
@@ -1,6 +1,11 @@
 #ifndef CONFIG_H
 #define CONFIG_H
 
+#include "hashmap.h"
+#include "string-list.h"
+
+struct object_id;
+
 /* git_config_parse_key() returns these negated: */
 #define CONFIG_INVALID_KEY 1
 #define CONFIG_NO_SECTION_OR_NAME 2
diff --git a/connected.h b/connected.h
index 322dc76372..e4c961817d 100644
--- a/connected.h
+++ b/connected.h
@@ -1,6 +1,7 @@
 #ifndef CONNECTED_H
 #define CONNECTED_H
 
+struct object_id;
 struct transport;
 
 /*
diff --git a/convert.h b/convert.h
index 01385d9288..76c654385d 100644
--- a/convert.h
+++ b/convert.h
@@ -7,6 +7,8 @@
 #include "string-list.h"
 
 struct index_state;
+struct object_id;
+struct strbuf;
 
 #define CONV_EOL_RNDTRP_DIE   (1<<0) /* Die if CRLF to LF to CRLF is different */
 #define CONV_EOL_RNDTRP_WARN  (1<<1) /* Warn if CRLF to LF to CRLF is different */
diff --git a/csum-file.h b/csum-file.h
index c5a2e335e7..3bf7184736 100644
--- a/csum-file.h
+++ b/csum-file.h
@@ -1,6 +1,8 @@
 #ifndef CSUM_FILE_H
 #define CSUM_FILE_H
 
+#include "hash.h"
+
 struct progress;
 
 /* A SHA1-protected file */
diff --git a/diffcore.h b/diffcore.h
index a30da161da..0dd1e9a63a 100644
--- a/diffcore.h
+++ b/diffcore.h
@@ -4,6 +4,10 @@
 #ifndef DIFFCORE_H
 #define DIFFCORE_H
 
+#include "cache.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-iterator.h b/dir-iterator.h
index 27739e6c29..970793d07a 100644
--- a/dir-iterator.h
+++ b/dir-iterator.h
@@ -1,6 +1,8 @@
 #ifndef DIR_ITERATOR_H
 #define DIR_ITERATOR_H
 
+#include "strbuf.h"
+
 /*
  * Iterate over a directory tree.
  *
diff --git a/fsck.h b/fsck.h
index c3cf5e0034..0c7e8c9428 100644
--- a/fsck.h
+++ b/fsck.h
@@ -6,6 +6,7 @@
 #define FSCK_IGNORE 3
 
 struct fsck_options;
+struct object;
 
 void fsck_set_msg_type(struct fsck_options *options,
 		const char *msg_id, const char *msg_type);
diff --git a/fsmonitor.h b/fsmonitor.h
index 65f3743636..69ac5a53b5 100644
--- a/fsmonitor.h
+++ b/fsmonitor.h
@@ -1,6 +1,13 @@
 #ifndef FSMONITOR_H
 #define FSMONITOR_H
 
+#include "cache.h"
+#include "dir.h"
+
+struct cache_entry;
+struct index_state;
+struct strbuf;
+
 extern struct trace_key trace_fsmonitor;
 
 /*
diff --git a/gpg-interface.h b/gpg-interface.h
index 5ecff4aa0c..acf50c4610 100644
--- a/gpg-interface.h
+++ b/gpg-interface.h
@@ -1,6 +1,8 @@
 #ifndef GPG_INTERFACE_H
 #define GPG_INTERFACE_H
 
+struct strbuf;
+
 #define GPG_VERIFY_VERBOSE		1
 #define GPG_VERIFY_RAW			2
 #define GPG_VERIFY_OMIT_STATUS	4
diff --git a/khash.h b/khash.h
index c0da40daa7..07b4cc2e67 100644
--- a/khash.h
+++ b/khash.h
@@ -26,6 +26,9 @@
 #ifndef __AC_KHASH_H
 #define __AC_KHASH_H
 
+#include "cache.h"
+#include "hashmap.h"
+
 #define AC_VERSION_KHASH_H "0.2.8"
 
 typedef uint32_t khint32_t;
diff --git a/list-objects-filter.h b/list-objects-filter.h
index a963d0274c..a6f6b4990b 100644
--- a/list-objects-filter.h
+++ b/list-objects-filter.h
@@ -1,6 +1,10 @@
 #ifndef LIST_OBJECTS_FILTER_H
 #define LIST_OBJECTS_FILTER_H
 
+struct list_objects_filter_options;
+struct object;
+struct oidset;
+
 /*
  * During list-object traversal we allow certain objects to be
  * filtered (omitted) from the result.  The active filter uses
diff --git a/list-objects.h b/list-objects.h
index aa618d7f45..ad40762926 100644
--- a/list-objects.h
+++ b/list-objects.h
@@ -1,6 +1,10 @@
 #ifndef LIST_OBJECTS_H
 #define LIST_OBJECTS_H
 
+struct commit;
+struct object;
+struct rev_info;
+
 typedef void (*show_commit_fn)(struct commit *, void *);
 typedef void (*show_object_fn)(struct object *, const char *, void *);
 void traverse_commit_list(struct rev_info *, show_commit_fn, show_object_fn, void *);
diff --git a/ll-merge.h b/ll-merge.h
index 244a31f55a..b72b19921e 100644
--- a/ll-merge.h
+++ b/ll-merge.h
@@ -5,6 +5,8 @@
 #ifndef LL_MERGE_H
 #define LL_MERGE_H
 
+#include "xdiff/xdiff.h"
+
 struct ll_merge_options {
 	unsigned virtual_ancestor : 1;
 	unsigned variant : 2;	/* favor ours, favor theirs, or union merge */
diff --git a/mailinfo.h b/mailinfo.h
index 04a25351d6..766c03dd1d 100644
--- a/mailinfo.h
+++ b/mailinfo.h
@@ -1,6 +1,8 @@
 #ifndef MAILINFO_H
 #define MAILINFO_H
 
+#include "strbuf.h"
+
 #define MAX_BOUNDARIES 5
 
 struct mailinfo {
diff --git a/mailmap.h b/mailmap.h
index ed7c93b05c..d0e65646cb 100644
--- a/mailmap.h
+++ b/mailmap.h
@@ -1,6 +1,8 @@
 #ifndef MAILMAP_H
 #define MAILMAP_H
 
+struct string_list;
+
 int read_mailmap(struct string_list *map, char **repo_abbrev);
 void clear_mailmap(struct string_list *map);
 
diff --git a/merge-recursive.h b/merge-recursive.h
index fa7bc6b683..0c46a5a4ff 100644
--- a/merge-recursive.h
+++ b/merge-recursive.h
@@ -1,8 +1,10 @@
 #ifndef MERGE_RECURSIVE_H
 #define MERGE_RECURSIVE_H
 
-#include "unpack-trees.h"
 #include "string-list.h"
+#include "unpack-trees.h"
+
+struct commit;
 
 struct merge_options {
 	const char *ancestor;
diff --git a/notes-merge.h b/notes-merge.h
index f815f23451..6c74e9385b 100644
--- a/notes-merge.h
+++ b/notes-merge.h
@@ -2,6 +2,10 @@
 #define NOTES_MERGE_H
 
 #include "notes-utils.h"
+#include "strbuf.h"
+
+struct commit;
+struct object_id;
 
 #define NOTES_MERGE_WORKTREE "NOTES_MERGE_WORKTREE"
 
diff --git a/notes-utils.h b/notes-utils.h
index 5d79cbef51..5408306528 100644
--- a/notes-utils.h
+++ b/notes-utils.h
@@ -3,6 +3,9 @@
 
 #include "notes.h"
 
+struct commit_list;
+struct object_id;
+
 /*
  * Create new notes commit from the given notes tree
  *
diff --git a/notes.h b/notes.h
index 0433f45db5..414bc6855a 100644
--- a/notes.h
+++ b/notes.h
@@ -3,6 +3,9 @@
 
 #include "string-list.h"
 
+struct object_id;
+struct strbuf;
+
 /*
  * Function type for combining two notes annotating the same object.
  *
diff --git a/object-store.h b/object-store.h
index e481f7ad41..b6f2c732c4 100644
--- a/object-store.h
+++ b/object-store.h
@@ -1,6 +1,7 @@
 #ifndef OBJECT_STORE_H
 #define OBJECT_STORE_H
 
+#include "cache.h"
 #include "oidmap.h"
 #include "list.h"
 #include "sha1-array.h"
diff --git a/object.h b/object.h
index 177b1a4571..6e28fdd0b4 100644
--- a/object.h
+++ b/object.h
@@ -1,6 +1,8 @@
 #ifndef OBJECT_H
 #define OBJECT_H
 
+#include "cache.h"
+
 struct buffer_slab;
 
 struct parsed_object_pool {
diff --git a/oidmap.h b/oidmap.h
index d3cd2bb590..72430b611e 100644
--- a/oidmap.h
+++ b/oidmap.h
@@ -1,6 +1,7 @@
 #ifndef OIDMAP_H
 #define OIDMAP_H
 
+#include "cache.h"
 #include "hashmap.h"
 
 /*
diff --git a/pack-bitmap.h b/pack-bitmap.h
index 4555907dee..8a04741e12 100644
--- a/pack-bitmap.h
+++ b/pack-bitmap.h
@@ -5,6 +5,9 @@
 #include "khash.h"
 #include "pack-objects.h"
 
+struct commit;
+struct rev_info;
+
 struct bitmap_disk_header {
 	char magic[4];
 	uint16_t version;
diff --git a/patch-ids.h b/patch-ids.h
index bec0f727a7..79ac9a8498 100644
--- a/patch-ids.h
+++ b/patch-ids.h
@@ -1,6 +1,12 @@
 #ifndef PATCH_IDS_H
 #define PATCH_IDS_H
 
+#include "diff.h"
+#include "hashmap.h"
+
+struct commit;
+struct object_id;
+
 struct patch_id {
 	struct hashmap_entry ent;
 	struct object_id patch_id;
diff --git a/path.h b/path.h
index ed6732e5a2..b654ea8ff5 100644
--- a/path.h
+++ b/path.h
@@ -2,6 +2,7 @@
 #define PATH_H
 
 struct repository;
+struct strbuf;
 
 /*
  * The result to all functions which return statically allocated memory may be
diff --git a/pathspec.h b/pathspec.h
index 099a170c2e..5a80c14b6b 100644
--- a/pathspec.h
+++ b/pathspec.h
@@ -1,6 +1,11 @@
 #ifndef PATHSPEC_H
 #define PATHSPEC_H
 
+#include "string.h"
+#include "strings.h"
+
+struct index_state;
+
 /* Pathspec magic */
 #define PATHSPEC_FROMTOP	(1<<0)
 #define PATHSPEC_MAXDEPTH	(1<<1)
diff --git a/pretty.h b/pretty.h
index 5c85d94e33..7359d318a9 100644
--- a/pretty.h
+++ b/pretty.h
@@ -1,7 +1,11 @@
 #ifndef PRETTY_H
 #define PRETTY_H
 
+#include "cache.h"
+#include "string-list.h"
+
 struct commit;
+struct strbuf;
 
 /* Commit formats */
 enum cmit_fmt {
diff --git a/reachable.h b/reachable.h
index 3c00fa0526..18b0f9f2f0 100644
--- a/reachable.h
+++ b/reachable.h
@@ -2,6 +2,8 @@
 #define REACHEABLE_H
 
 struct progress;
+struct rev_info;
+
 extern int add_unseen_recent_objects_to_traversal(struct rev_info *revs,
 						  timestamp_t timestamp);
 extern void mark_reachable_objects(struct rev_info *revs, int mark_reflog,
diff --git a/reflog-walk.h b/reflog-walk.h
index 7553c448fe..cb3e73755d 100644
--- a/reflog-walk.h
+++ b/reflog-walk.h
@@ -3,6 +3,7 @@
 
 #include "cache.h"
 
+struct commit;
 struct reflog_walk_info;
 
 extern void init_reflog_walk(struct reflog_walk_info **info);
diff --git a/refs.h b/refs.h
index cc2fb4c68c..bd52c1bbae 100644
--- a/refs.h
+++ b/refs.h
@@ -3,8 +3,10 @@
 
 struct object_id;
 struct ref_store;
+struct repository;
 struct strbuf;
 struct string_list;
+struct string_list_item;
 struct worktree;
 
 /*
diff --git a/remote.h b/remote.h
index 976292152c..88f8480c71 100644
--- a/remote.h
+++ b/remote.h
@@ -1,6 +1,7 @@
 #ifndef REMOTE_H
 #define REMOTE_H
 
+#include "cache.h"
 #include "parse-options.h"
 #include "hashmap.h"
 #include "refspec.h"
diff --git a/repository.h b/repository.h
index b9413be50c..13c8799678 100644
--- a/repository.h
+++ b/repository.h
@@ -1,6 +1,8 @@
 #ifndef REPOSITORY_H
 #define REPOSITORY_H
 
+#include "path.h"
+
 struct config_set;
 struct git_hash_algo;
 struct index_state;
diff --git a/resolve-undo.h b/resolve-undo.h
index 87291904bd..fbe348efac 100644
--- a/resolve-undo.h
+++ b/resolve-undo.h
@@ -1,6 +1,8 @@
 #ifndef RESOLVE_UNDO_H
 #define RESOLVE_UNDO_H
 
+#include "cache.h"
+
 struct resolve_undo_info {
 	unsigned int mode[3];
 	struct object_id oid[3];
diff --git a/revision.h b/revision.h
index c599c34da9..850e508783 100644
--- a/revision.h
+++ b/revision.h
@@ -1,6 +1,7 @@
 #ifndef REVISION_H
 #define REVISION_H
 
+#include "commit.h"
 #include "parse-options.h"
 #include "grep.h"
 #include "notes.h"
diff --git a/send-pack.h b/send-pack.h
index 6af71f7008..e148fcd960 100644
--- a/send-pack.h
+++ b/send-pack.h
@@ -3,6 +3,10 @@
 
 #include "string-list.h"
 
+struct child_process;
+struct oid_array;
+struct ref;
+
 /* Possible values for push_cert field in send_pack_args. */
 #define SEND_PACK_PUSH_CERT_NEVER 0
 #define SEND_PACK_PUSH_CERT_IF_ASKED 1
diff --git a/sequencer.h b/sequencer.h
index c5787c6b56..c751c9d6e4 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -1,6 +1,11 @@
 #ifndef SEQUENCER_H
 #define SEQUENCER_H
 
+#include "cache.h"
+#include "strbuf.h"
+
+struct commit;
+
 const char *git_path_commit_editmsg(void);
 const char *git_path_seq_dir(void);
 
diff --git a/shortlog.h b/shortlog.h
index 5d64cfe929..2fa61c4294 100644
--- a/shortlog.h
+++ b/shortlog.h
@@ -3,6 +3,8 @@
 
 #include "string-list.h"
 
+struct commit;
+
 struct shortlog {
 	struct string_list list;
 	int summary;
diff --git a/submodule.h b/submodule.h
index 4644683e6c..3adbc08915 100644
--- a/submodule.h
+++ b/submodule.h
@@ -1,11 +1,17 @@
 #ifndef SUBMODULE_H
 #define SUBMODULE_H
 
-struct repository;
-struct diff_options;
 struct argv_array;
+struct cache_entry;
+struct diff_options;
+struct index_state;
+struct object_id;
 struct oid_array;
+struct pathspec;
 struct remote;
+struct repository;
+struct string_list;
+struct strbuf;
 
 enum {
 	RECURSE_SUBMODULES_ONLY = -5,
diff --git a/tempfile.h b/tempfile.h
index 8959c5f1b5..36434eb6fa 100644
--- a/tempfile.h
+++ b/tempfile.h
@@ -2,6 +2,7 @@
 #define TEMPFILE_H
 
 #include "list.h"
+#include "strbuf.h"
 
 /*
  * Handle temporary files.
diff --git a/trailer.h b/trailer.h
index 6d7f8c2a52..9c10026c35 100644
--- a/trailer.h
+++ b/trailer.h
@@ -3,6 +3,8 @@
 
 #include "list.h"
 
+struct strbuf;
+
 enum trailer_where {
 	WHERE_DEFAULT,
 	WHERE_END,
diff --git a/tree-walk.h b/tree-walk.h
index 805f58f00f..196831007e 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 struct object_id *oid;
 	const char *path;
diff --git a/unpack-trees.h b/unpack-trees.h
index c2b434c606..48bc69cb47 100644
--- a/unpack-trees.h
+++ b/unpack-trees.h
@@ -1,11 +1,14 @@
 #ifndef UNPACK_TREES_H
 #define UNPACK_TREES_H
 
-#include "tree-walk.h"
+#include "cache.h"
 #include "argv-array.h"
+#include "string-list.h"
+#include "tree-walk.h"
 
 #define MAX_UNPACK_TREES 8
 
+struct cache_entry;
 struct unpack_trees_options;
 struct exclude_list;
 
diff --git a/url.h b/url.h
index abdaf6fa30..f311c40a46 100644
--- a/url.h
+++ b/url.h
@@ -1,6 +1,8 @@
 #ifndef URL_H
 #define URL_H
 
+struct strbuf;
+
 extern int is_url(const char *url);
 extern int is_urlschemechar(int first_flag, int ch);
 extern char *url_decode(const char *url);
diff --git a/utf8.h b/utf8.h
index db73a2d8d3..dd5ee1052a 100644
--- a/utf8.h
+++ b/utf8.h
@@ -1,6 +1,8 @@
 #ifndef GIT_UTF8_H
 #define GIT_UTF8_H
 
+struct strbuf;
+
 typedef unsigned int ucs_char_t;  /* assuming 32bit int */
 
 size_t display_mode_esc_sequence_len(const char *s);
diff --git a/worktree.h b/worktree.h
index fe38ce10c3..df3fc30f73 100644
--- a/worktree.h
+++ b/worktree.h
@@ -1,6 +1,7 @@
 #ifndef WORKTREE_H
 #define WORKTREE_H
 
+#include "cache.h"
 #include "refs.h"
 
 struct strbuf;
-- 
2.18.0.549.gd4454f3f9b

Re: [PATCHv3 1/6] Add missing includes and forward declares

[Jonathan Nieder]

Elijah Newren wrote:

> Subject: Add missing includes and forward declares

nit: s/declares/declarations/

This is a huge patch.  Was it autogenerated or generated manually?
Can the commit message say something about methodology?

Is there an easy way to review it?  (Keep in mind that I'm super lazy.
;-))

> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
[...]
> --- a/alloc.h
> +++ b/alloc.h
> @@ -1,9 +1,11 @@
>  #ifndef ALLOC_H
>  #define ALLOC_H
>  
> +struct alloc_state;
>  struct tree;
>  struct commit;
>  struct tag;
> +struct repository;
>  
>  void *alloc_blob_node(struct repository *r);

That's reasonable.  Going forward, is there a way to tell if some of
these forward declarations are no longer needed at some point in the
future (e.g. can clang be convinced to warn us about it)?

[...]
> --- a/apply.h
> +++ b/apply.h
> @@ -1,6 +1,9 @@
>  #ifndef APPLY_H
>  #define APPLY_H
>  
> +#include "lockfile.h"
> +#include "string-list.h"
> +
>  enum apply_ws_error_action {

Here, to avoid strange behavior, we have to be careful to make sure
the headers don't #include apply.h back.  It's a pretty high-level
header so there's no risk of that *phew*.

[...]
> --- a/archive.h
> +++ b/archive.h
> @@ -3,6 +3,9 @@
>  
>  #include "pathspec.h"
>  
> +struct object_id;
> +enum object_type;

enums are of unknown size, so forward declarations don't work for
them.  See bb/pedantic for some examples.

enum object_type is defined in cache.h, so should this #include that?

[...]
> --- a/commit-graph.h
> +++ b/commit-graph.h
> @@ -4,6 +4,7 @@
>  #include "git-compat-util.h"
>  #include "repository.h"
>  #include "string-list.h"
> +#include "cache.h"

We can skip the #include of git-compat-util.h since all .c files
include it.

[...]
> --- a/fsmonitor.h
> +++ b/fsmonitor.h
> @@ -1,6 +1,13 @@
>  #ifndef FSMONITOR_H
>  #define FSMONITOR_H
>  
> +#include "cache.h"
> +#include "dir.h"
> +
> +struct cache_entry;
> +struct index_state;
> +struct strbuf;

cache_entry et al are defined in cache.h, right?  Are these forward
decls needed?

[...]
> --- a/merge-recursive.h
> +++ b/merge-recursive.h
> @@ -1,8 +1,10 @@
>  #ifndef MERGE_RECURSIVE_H
>  #define MERGE_RECURSIVE_H
>  
> -#include "unpack-trees.h"
>  #include "string-list.h"
> +#include "unpack-trees.h"

just curious, no need to change: where does this reordering come from?

[...]
> --- a/pathspec.h
> +++ b/pathspec.h
> @@ -1,6 +1,11 @@
>  #ifndef PATHSPEC_H
>  #define PATHSPEC_H
>  
> +#include "string.h"
> +#include "strings.h"

What are these headers?

The rest looks good.

Thanks and hope that helps,
Jonathan

Re: [PATCHv3 1/6] Add missing includes and forward declares

[Elijah Newren]

On Tue, Aug 14, 2018 at 10:10 PM Jonathan Nieder <jrnieder@gmail.com> wrote:
>
> Elijah Newren wrote:
>
> > Subject: Add missing includes and forward declares
>
> nit: s/declares/declarations/

Thanks.

> This is a huge patch.  Was it autogenerated or generated manually?
> Can the commit message say something about methodology?

Mostly manually.  I had a simple program that would create a dummy.c
file that included git-compat-util.h then exactly one header, compile
it, and spit any compile errors at me.  I repeated that through the
top-level headers.

I didn't want to repeat that description in all 6 patches, since all
six came from that, so I put it in the cover letter.  Since patch #1
has most that changes though, I guess it makes sense to include it at
least in that one?

> Is there an easy way to review it?  (Keep in mind that I'm super lazy.
> ;-))

I guess I could send you my hacky python script that loops through the
top-level header files and creates the dummy two-line c file, and you
could inspect it and run it.  But that only verifies that it compiles,
not that the changes I choose are "correct".

>
> > Signed-off-by: Elijah Newren <newren@gmail.com>
> > ---
> [...]
> > --- a/alloc.h
> > +++ b/alloc.h
> > @@ -1,9 +1,11 @@
> >  #ifndef ALLOC_H
> >  #define ALLOC_H
> >
> > +struct alloc_state;
> >  struct tree;
> >  struct commit;
> >  struct tag;
> > +struct repository;
> >
> >  void *alloc_blob_node(struct repository *r);
>
> That's reasonable.  Going forward, is there a way to tell if some of
> these forward declarations are no longer needed at some point in the
> future (e.g. can clang be convinced to warn us about it)?

I'm not aware of anything currently; while I could have easily missed
things, projects like
https://github.com/include-what-you-use/include-what-you-use (which
look active and have a July 2018 date on them) make me suspect there
isn't a good answer currently.

> [...]
> > --- a/apply.h
> > +++ b/apply.h
> > @@ -1,6 +1,9 @@
> >  #ifndef APPLY_H
> >  #define APPLY_H
> >
> > +#include "lockfile.h"
> > +#include "string-list.h"
> > +
> >  enum apply_ws_error_action {
>
> Here, to avoid strange behavior, we have to be careful to make sure
> the headers don't #include apply.h back.  It's a pretty high-level
> header so there's no risk of that *phew*.

:-)

> [...]
> > --- a/archive.h
> > +++ b/archive.h
> > @@ -3,6 +3,9 @@
> >
> >  #include "pathspec.h"
> >
> > +struct object_id;
> > +enum object_type;
>
> enums are of unknown size, so forward declarations don't work for
> them.  See bb/pedantic for some examples.

structs are also of unknown size; the size is irrelevant when the
function signature merely uses a pointer to the struct or enum.  The
enum forward declaration fixes a compilation bug.

> enum object_type is defined in cache.h, so should this #include that?

We could, but we don't need the definition; a forward declaration is sufficient.

> [...]
> > --- a/commit-graph.h
> > +++ b/commit-graph.h
> > @@ -4,6 +4,7 @@
> >  #include "git-compat-util.h"
> >  #include "repository.h"
> >  #include "string-list.h"
> > +#include "cache.h"
>
> We can skip the #include of git-compat-util.h since all .c files
> include it.

Good point.  Should I go through and remove all the inclusions of
git-compat-util.h in header files?

> [...]
> > --- a/fsmonitor.h
> > +++ b/fsmonitor.h
> > @@ -1,6 +1,13 @@
> >  #ifndef FSMONITOR_H
> >  #define FSMONITOR_H
> >
> > +#include "cache.h"
> > +#include "dir.h"
> > +
> > +struct cache_entry;
> > +struct index_state;
> > +struct strbuf;
>
> cache_entry et al are defined in cache.h, right?  Are these forward
> decls needed?

Good catch; they can be removed.  I'm pretty sure I added the forward
declarations first, then noticed it wasn't enough, added the cache.h
include, and forgot to clean up.

> [...]
> > --- a/merge-recursive.h
> > +++ b/merge-recursive.h
> > @@ -1,8 +1,10 @@
> >  #ifndef MERGE_RECURSIVE_H
> >  #define MERGE_RECURSIVE_H
> >
> > -#include "unpack-trees.h"
> >  #include "string-list.h"
> > +#include "unpack-trees.h"
>
> just curious, no need to change: where does this reordering come from?

Well, since I was manually editing anyway, I saw these and decided to
alphabetize it since it's a file I deal with a lot.  *shrug*

> [...]
> > --- a/pathspec.h
> > +++ b/pathspec.h
> > @@ -1,6 +1,11 @@
> >  #ifndef PATHSPEC_H
> >  #define PATHSPEC_H
> >
> > +#include "string.h"
> > +#include "strings.h"
>
> What are these headers?

The original patch[1] had explanations of why I added them:

+#include "string.h"   /* For str[n]cmp */
+#include "strings.h"  /* For str[n]casecmp */

But Peff requested that I remove the comments.

[1] https://public-inbox.org/git/20180811043218.31456-2-newren@gmail.com/

> The rest looks good.
>
> Thanks and hope that helps,
> Jonathan

Thanks for taking a look!

Re: [PATCHv3 1/6] Add missing includes and forward declares

[Jonathan Nieder]

Elijah Newren wrote:

> I didn't want to repeat that description in all 6 patches, since all
> six came from that, so I put it in the cover letter.  Since patch #1
> has most that changes though, I guess it makes sense to include it at
> least in that one?

Yes, that sounds sensible to me.

[...]
> On Tue, Aug 14, 2018 at 10:10 PM Jonathan Nieder <jrnieder@gmail.com> wrote:

>> Is there an easy way to review it?  (Keep in mind that I'm super lazy.
>> ;-))
>
> I guess I could send you my hacky python script that loops through the
> top-level header files and creates the dummy two-line c file, and you
> could inspect it and run it.  But that only verifies that it compiles,
> not that the changes I choose are "correct".

I suppose leaving it as an exercise to the reviewer is not terrible.
Maybe some reviewer will be inspired to make a test tool we can check
in, or configuration for an existing tool like iwyu.

[...]
>> enums are of unknown size, so forward declarations don't work for
>> them.  See bb/pedantic for some examples.
>
> structs are also of unknown size; the size is irrelevant when the
> function signature merely uses a pointer to the struct or enum.  The
> enum forward declaration fixes a compilation bug.

My rationale may miss the point but the standard and some real compilers
don't like this, unfortunately.

For structs, having an incomplete type is fine, but for enums we need
the full definition.  E.g. C99 sayeth (in section 6.7.2.3 "tags")

	A type specifier of the form

		enum identifier

	without an enumerator list shall only appear after the type it
	specifies is complete.

[...]
>>> --- a/commit-graph.h
>>> +++ b/commit-graph.h
>>> @@ -4,6 +4,7 @@
>>>  #include "git-compat-util.h"
>>>  #include "repository.h"
>>>  #include "string-list.h"
>>> +#include "cache.h"
>>
>> We can skip the #include of git-compat-util.h since all .c files
>> include it.
>
> Good point.  Should I go through and remove all the inclusions of
> git-compat-util.h in header files?

It's orthogonal to this series but might be a good change.

[...]
>>> --- a/pathspec.h
>>> +++ b/pathspec.h
>>> @@ -1,6 +1,11 @@
>>>  #ifndef PATHSPEC_H
>>>  #define PATHSPEC_H
>>>
>>> +#include "string.h"
>>> +#include "strings.h"
>>
>> What are these headers?
>
> The original patch[1] had explanations of why I added them:
>
> +#include "string.h"   /* For str[n]cmp */
> +#include "strings.h"  /* For str[n]casecmp */

Ah.  Please remove these #includes: they're part of the standard
library that we get implicitly via git-compat-util.h.

I was tripped up because they were in quotes instead of angle
brackets.

Thanks,
Jonathan

Re: [PATCHv3 1/6] Add missing includes and forward declares

[Elijah Newren]

On Tue, Aug 14, 2018 at 11:13 PM Jonathan Nieder <jrnieder@gmail.com> wrote:
>
> Elijah Newren wrote:
>
> > I didn't want to repeat that description in all 6 patches, since all
> > six came from that, so I put it in the cover letter.  Since patch #1
> > has most that changes though, I guess it makes sense to include it at
> > least in that one?
>
> Yes, that sounds sensible to me.

Will do.

> [...]
> >> enums are of unknown size, so forward declarations don't work for
> >> them.  See bb/pedantic for some examples.
> >
> > structs are also of unknown size; the size is irrelevant when the
> > function signature merely uses a pointer to the struct or enum.  The
> > enum forward declaration fixes a compilation bug.
>
> My rationale may miss the point but the standard and some real compilers
> don't like this, unfortunately.
>
> For structs, having an incomplete type is fine, but for enums we need
> the full definition.  E.g. C99 sayeth (in section 6.7.2.3 "tags")
>
>         A type specifier of the form
>
>                 enum identifier
>
>         without an enumerator list shall only appear after the type it
>         specifies is complete.

What about a type specifier of the form
  enum identifier *
?  Can that kind of type specifier appear before the full definition
of the enum?  (Or, alternatively, if the standard doesn't say, are
there any compilers that have a problem with that?)

If so, we can include cache.h instead.  We'll probably also have to
fix up packfile.h for the exact same issue (even the same enum name)
if that's the case.

> [...]
> >>> --- a/commit-graph.h
> >>> +++ b/commit-graph.h
> >>> @@ -4,6 +4,7 @@
> >>>  #include "git-compat-util.h"
> >>>  #include "repository.h"
> >>>  #include "string-list.h"
> >>> +#include "cache.h"
> >>
> >> We can skip the #include of git-compat-util.h since all .c files
> >> include it.
> >
> > Good point.  Should I go through and remove all the inclusions of
> > git-compat-util.h in header files?
>
> It's orthogonal to this series but might be a good change.

I think I'll leave it as #leftoverbits for someone else interested.  :-)

> [...]
> >>> --- a/pathspec.h
> >>> +++ b/pathspec.h
> >>> @@ -1,6 +1,11 @@
> >>>  #ifndef PATHSPEC_H
> >>>  #define PATHSPEC_H
> >>>
> >>> +#include "string.h"
> >>> +#include "strings.h"
> >>
> >> What are these headers?
> >
> > The original patch[1] had explanations of why I added them:
> >
> > +#include "string.h"   /* For str[n]cmp */
> > +#include "strings.h"  /* For str[n]casecmp */
>
> Ah.  Please remove these #includes: they're part of the standard
> library that we get implicitly via git-compat-util.h.
>
> I was tripped up because they were in quotes instead of angle
> brackets.

Indeed; will do.

Re: [PATCHv3 1/6] Add missing includes and forward declares

[Elijah Newren]

On Tue, Aug 14, 2018 at 11:51 PM Elijah Newren <newren@gmail.com> wrote:
> > [...]
> > >> enums are of unknown size, so forward declarations don't work for
> > >> them.  See bb/pedantic for some examples.
> > >
> > > structs are also of unknown size; the size is irrelevant when the
> > > function signature merely uses a pointer to the struct or enum.  The
> > > enum forward declaration fixes a compilation bug.
> >
> > My rationale may miss the point but the standard and some real compilers
> > don't like this, unfortunately.
> >
> > For structs, having an incomplete type is fine, but for enums we need
> > the full definition.  E.g. C99 sayeth (in section 6.7.2.3 "tags")
> >
> >         A type specifier of the form
> >
> >                 enum identifier
> >
> >         without an enumerator list shall only appear after the type it
> >         specifies is complete.
>
> What about a type specifier of the form
>   enum identifier *
> ?  Can that kind of type specifier appear before the full definition
> of the enum?  (Or, alternatively, if the standard doesn't say, are
> there any compilers that have a problem with that?)
>
> If so, we can include cache.h instead.  We'll probably also have to
> fix up packfile.h for the exact same issue (even the same enum name)
> if that's the case.

Digging a little further this morning, apparently C++ has defined a
forward declaration of an enum to either be useless (because it was
already defined), require an explicit size specifier, or be a
compilation error.

That seemed stupid to me, but a little more digging turned up
http://c-faq.com/null/machexamp.html , which states that sizeof(char*)
!= sizeof(int*) on some platforms.  That was a big surprise to me.
Since an enum could be a char or int (or long or...), knowing the size
of the enum thus is important to knowing the size of a pointer to an
enum, so we actually do need the full enum definition (or a C++ style
explicit size specifier).

What a crazy world.

So, I'll go with the inclusion of cache.h and also fix up packfile.h
the same way.  Thanks for pointing this out, Jonathan.

[PATCHv3 2/6] alloc: make allocate_alloc_state and clear_alloc_state more consistent

[Elijah Newren]

Since both functions are using the same data type, they should either both
refer to it as void *, or both use the real type (struct alloc_state *).
Opt for the latter.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 alloc.c | 2 +-
 alloc.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/alloc.c b/alloc.c
index c2fc5d6888..e7aa81b7aa 100644
--- a/alloc.c
+++ b/alloc.c
@@ -36,7 +36,7 @@ struct alloc_state {
 	int slab_nr, slab_alloc;
 };
 
-void *allocate_alloc_state(void)
+struct alloc_state *allocate_alloc_state(void)
 {
 	return xcalloc(1, sizeof(struct alloc_state));
 }
diff --git a/alloc.h b/alloc.h
index 7a41bb9eb3..ba356ed847 100644
--- a/alloc.h
+++ b/alloc.h
@@ -15,7 +15,7 @@ void *alloc_object_node(struct repository *r);
 void alloc_report(struct repository *r);
 unsigned int alloc_commit_index(struct repository *r);
 
-void *allocate_alloc_state(void);
+struct alloc_state *allocate_alloc_state(void);
 void clear_alloc_state(struct alloc_state *s);
 
 #endif
-- 
2.18.0.549.gd4454f3f9b

Re: [PATCHv3 2/6] alloc: make allocate_alloc_state and clear_alloc_state more consistent

[Jonathan Nieder]

Elijah Newren wrote:

> Since both functions are using the same data type, they should either both
> refer to it as void *, or both use the real type (struct alloc_state *).
> Opt for the latter.
> 
> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
>  alloc.c | 2 +-
>  alloc.h | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)

This API comes from 14ba97f8 (alloc: allow arbitrary repositories for
alloc functions, 2018-05-15).  The pointer returned by
allocate_alloc_state points to an initialized, usable object so
returning a struct instead of a void pointer is the right thing to do.
Thanks for noticing and fixing it.

That commit in turn was probably inspired by v1.5.2-rc0~16^2 (Clean up
object creation to use more common code, 2007-04-16), which was
following the same convention (void * for raw memory, struct blob *
for initialized object).

Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

[PATCHv3 3/6] Move definition of enum branch_track from cache.h to branch.h

[Elijah Newren]

'branch_track' feels more closely related to branching, and it is
needed later in branch.h; rather than #include'ing cache.h in branch.h
for this small enum, just move the enum and the external declaration
for git_branch_track to branch.h.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 branch.h      | 11 +++++++++++
 cache.h       | 10 ----------
 config.c      |  1 +
 environment.c |  1 +
 4 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/branch.h b/branch.h
index 7d9b330eba..5cace4581f 100644
--- a/branch.h
+++ b/branch.h
@@ -3,6 +3,17 @@
 
 struct strbuf;
 
+enum branch_track {
+	BRANCH_TRACK_UNSPECIFIED = -1,
+	BRANCH_TRACK_NEVER = 0,
+	BRANCH_TRACK_REMOTE,
+	BRANCH_TRACK_ALWAYS,
+	BRANCH_TRACK_EXPLICIT,
+	BRANCH_TRACK_OVERRIDE
+};
+
+extern enum branch_track git_branch_track;
+
 /* Functions for acting on the information about branches. */
 
 /*
diff --git a/cache.h b/cache.h
index 8dc7134f00..a1c0c594fb 100644
--- a/cache.h
+++ b/cache.h
@@ -919,15 +919,6 @@ enum log_refs_config {
 };
 extern enum log_refs_config log_all_ref_updates;
 
-enum branch_track {
-	BRANCH_TRACK_UNSPECIFIED = -1,
-	BRANCH_TRACK_NEVER = 0,
-	BRANCH_TRACK_REMOTE,
-	BRANCH_TRACK_ALWAYS,
-	BRANCH_TRACK_EXPLICIT,
-	BRANCH_TRACK_OVERRIDE
-};
-
 enum rebase_setup_type {
 	AUTOREBASE_NEVER = 0,
 	AUTOREBASE_LOCAL,
@@ -944,7 +935,6 @@ enum push_default_type {
 	PUSH_DEFAULT_UNSPECIFIED
 };
 
-extern enum branch_track git_branch_track;
 extern enum rebase_setup_type autorebase;
 extern enum push_default_type push_default;
 
diff --git a/config.c b/config.c
index 66645047eb..66dca7978a 100644
--- a/config.c
+++ b/config.c
@@ -6,6 +6,7 @@
  *
  */
 #include "cache.h"
+#include "branch.h"
 #include "config.h"
 #include "repository.h"
 #include "lockfile.h"
diff --git a/environment.c b/environment.c
index 6cf0079389..0c04a6da7a 100644
--- a/environment.c
+++ b/environment.c
@@ -8,6 +8,7 @@
  * are.
  */
 #include "cache.h"
+#include "branch.h"
 #include "repository.h"
 #include "config.h"
 #include "refs.h"
-- 
2.18.0.549.gd4454f3f9b

Re: [PATCHv3 3/6] Move definition of enum branch_track from cache.h to branch.h

[Jonathan Nieder]

Elijah Newren wrote:

> 'branch_track' feels more closely related to branching, and it is
> needed later in branch.h; rather than #include'ing cache.h in branch.h
> for this small enum, just move the enum and the external declaration
> for git_branch_track to branch.h.
>
> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
>  branch.h      | 11 +++++++++++
>  cache.h       | 10 ----------
>  config.c      |  1 +
>  environment.c |  1 +
>  4 files changed, 13 insertions(+), 10 deletions(-)

\o/

Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

[PATCHv3 4/6] urlmatch.h: fix include guard

[Elijah Newren]

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 urlmatch.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/urlmatch.h b/urlmatch.h
index 37ee5da85e..e482148248 100644
--- a/urlmatch.h
+++ b/urlmatch.h
@@ -1,4 +1,6 @@
 #ifndef URL_MATCH_H
+#define URL_MATCH_H
+
 #include "string-list.h"
 
 struct url_info {
-- 
2.18.0.549.gd4454f3f9b

Re: [PATCHv3 4/6] urlmatch.h: fix include guard

[Jonathan Nieder]

Elijah Newren wrote:

> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
>  urlmatch.h | 2 ++
>  1 file changed, 2 insertions(+)

Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

This has two benefits:

- avoids the definitions in this header from being evaluated twice

- many compilers recognize the #include guard idiom and also are able
  to avoid re-reading the text of the header twice, too

The header guard had been broken since day one.  Is there a tool that
can detect this class of problem?

[PATCHv3 5/6] compat/precompose_utf8.h: use more common include guard style

[Elijah Newren]

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 compat/precompose_utf8.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/compat/precompose_utf8.h b/compat/precompose_utf8.h
index a94e7c4342..6f843d3e1a 100644
--- a/compat/precompose_utf8.h
+++ b/compat/precompose_utf8.h
@@ -1,4 +1,6 @@
 #ifndef PRECOMPOSE_UNICODE_H
+#define PRECOMPOSE_UNICODE_H
+
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <dirent.h>
@@ -41,5 +43,4 @@ int precompose_utf8_closedir(PREC_DIR *dirp);
 #define DIR PREC_DIR
 #endif /* PRECOMPOSE_UNICODE_C */
 
-#define  PRECOMPOSE_UNICODE_H
 #endif /* PRECOMPOSE_UNICODE_H */
-- 
2.18.0.549.gd4454f3f9b

Re: [PATCHv3 5/6] compat/precompose_utf8.h: use more common include guard style

[Jonathan Nieder]

Elijah Newren wrote:

> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
>  compat/precompose_utf8.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

The more unusual style is less likely to be recognized by compilers, so
we can waste some I/O re-reading the header at compile time.

> --- a/compat/precompose_utf8.h
> +++ b/compat/precompose_utf8.h
> @@ -1,4 +1,6 @@
>  #ifndef PRECOMPOSE_UNICODE_H
> +#define PRECOMPOSE_UNICODE_H
> +
>  #include <sys/stat.h>
>  #include <sys/types.h>
>  #include <dirent.h>

Not about this patch: these headers are #include-d in git-compat-util.h
which we assume to be already included first before anything else.  Can
we remove these redundant #includes?

[PATCHv3 6/6] Add missing includes and forward declares

[Elijah Newren]

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 bisect.h       | 2 ++
 pack-objects.h | 1 +
 2 files changed, 3 insertions(+)

diff --git a/bisect.h b/bisect.h
index a5d9248a47..34df209351 100644
--- a/bisect.h
+++ b/bisect.h
@@ -1,6 +1,8 @@
 #ifndef BISECT_H
 #define BISECT_H
 
+struct commit_list;
+
 /*
  * Find bisection. If something is found, `reaches` will be the number of
  * commits that the best commit reaches. `all` will be the count of
diff --git a/pack-objects.h b/pack-objects.h
index edf74dabdd..08c6b57d49 100644
--- a/pack-objects.h
+++ b/pack-objects.h
@@ -2,6 +2,7 @@
 #define PACK_OBJECTS_H
 
 #include "object-store.h"
+#include "pack.h"
 
 #define DEFAULT_DELTA_CACHE_SIZE (256 * 1024 * 1024)
 
-- 
2.18.0.549.gd4454f3f9b

Re: [PATCHv3 6/6] Add missing includes and forward declares

[Jonathan Nieder]

Hi,

Elijah Newren wrote:

> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
>  bisect.h       | 2 ++
>  pack-objects.h | 1 +
>  2 files changed, 3 insertions(+)

Do you have more context about why these are in a separate commit?

For pack-objects.h, I think you might be making a fix to the
cc/delta-islands topic.  But for bisect.h, I don't see any recent
relevant changes.

Curious,
Jonathan