git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/5] Header includes cleanup
@ 2009-03-30  9:55 Nathaniel P Dawson
  2009-03-30  9:55 ` [PATCH 1/5] " Nathaniel P Dawson
  2009-03-30 10:50 ` [PATCH 0/5] " Johannes Sixt
  0 siblings, 2 replies; 17+ messages in thread
From: Nathaniel P Dawson @ 2009-03-30  9:55 UTC (permalink / raw)
  To: git

This is just the beginning for this project. I'm slowly cleaning up the header includes one chunk at a time. I hope my patches aren't too messy, I've learned how to better utilize git to make patches and organize my commits logically so I'll submit neater chunks henceforth. You can expect patches from me nightly until I've finished this project.

Regards,
Nathaniel P Dawson

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 1/5] Header includes cleanup
  2009-03-30  9:55 [PATCH 0/5] Header includes cleanup Nathaniel P Dawson
@ 2009-03-30  9:55 ` Nathaniel P Dawson
  2009-03-30  9:55   ` [PATCH 2/5] " Nathaniel P Dawson
  2009-03-30 10:50 ` [PATCH 0/5] " Johannes Sixt
  1 sibling, 1 reply; 17+ messages in thread
From: Nathaniel P Dawson @ 2009-03-30  9:55 UTC (permalink / raw)
  To: git; +Cc: Nathaniel P Dawson

Deleted all header includes of git-compat-util.h, strbuf.h, cache.h, and commit.h where builtin.h was already included since builtin.h includes them.

Signed-off-by: Nathaniel P Dawson <nathaniel.dawson@gmail.com>
---
 builtin-add.c              |    1 -
 builtin-annotate.c         |    1 -
 builtin-apply.c            |    5 ++---
 builtin-archive.c          |    1 -
 builtin-blame.c            |    3 ---
 builtin-branch.c           |    5 +----
 builtin-bundle.c           |    1 -
 builtin-cat-file.c         |    3 +--
 builtin-check-attr.c       |    1 -
 builtin-check-ref-format.c |    3 +--
 builtin-checkout-index.c   |    1 -
 builtin-checkout.c         |    4 +---
 builtin-clean.c            |    1 -
 builtin-commit-tree.c      |    4 +---
 builtin-commit.c           |    5 +----
 builtin-config.c           |    1 -
 builtin-count-objects.c    |    3 +--
 builtin-describe.c         |    4 +---
 builtin-diff-files.c       |    4 +---
 builtin-diff-index.c       |    4 +---
 builtin-diff-tree.c        |    4 +---
 builtin-diff.c             |    6 ++----
 builtin-fast-export.c      |    2 --
 builtin-fetch--tool.c      |    2 --
 builtin-fetch.c            |    4 +---
 builtin-fmt-merge-msg.c    |    2 --
 builtin-for-each-ref.c     |    6 ++----
 builtin-fsck.c             |    2 --
 builtin-gc.c               |    1 -
 builtin-grep.c             |    4 +---
 builtin-help.c             |    1 -
 builtin-init-db.c          |    1 -
 builtin-log.c              |    4 +---
 builtin-ls-files.c         |    3 +--
 builtin-ls-remote.c        |    1 -
 builtin-ls-tree.c          |    4 +---
 builtin-mailinfo.c         |    2 --
 builtin-mailsplit.c        |    1 -
 builtin-merge-base.c       |    2 --
 builtin-merge-file.c       |    1 -
 builtin-merge-ours.c       |    1 -
 builtin-merge.c            |    4 +---
 builtin-mv.c               |    1 -
 builtin-name-rev.c         |    2 --
 builtin-pack-objects.c     |    2 --
 builtin-prune-packed.c     |    1 -
 builtin-prune.c            |    4 +---
 builtin-push.c             |    3 +--
 builtin-read-tree.c        |    3 +--
 builtin-reflog.c           |    2 --
 builtin-rerere.c           |    1 -
 builtin-rev-list.c         |    6 ++----
 builtin-revert.c           |    2 --
 builtin-rm.c               |    1 -
 builtin-shortlog.c         |    2 --
 builtin-show-branch.c      |    4 +---
 builtin-show-ref.c         |    1 -
 builtin-stripspace.c       |    1 -
 builtin-symbolic-ref.c     |    1 -
 builtin-tag.c              |    1 -
 builtin-tar-tree.c         |    2 --
 builtin-unpack-objects.c   |    2 --
 builtin-update-index.c     |    3 +--
 builtin-update-ref.c       |    3 +--
 builtin-upload-archive.c   |    1 -
 builtin-verify-pack.c      |    1 -
 builtin-verify-tag.c       |    1 -
 builtin-write-tree.c       |    1 -
 diff-no-index.c            |    4 +---
 fast-import.c              |    2 --
 git.c                      |    1 -
 help.c                     |    1 -
 merge-recursive.c          |    6 ++----
 73 files changed, 34 insertions(+), 145 deletions(-)

diff --git a/builtin-add.c b/builtin-add.c
index cb67d2c..4d61e72 100644
--- a/builtin-add.c
+++ b/builtin-add.c
@@ -3,7 +3,6 @@
  *
  * Copyright (C) 2006 Linus Torvalds
  */
-#include "cache.h"
 #include "builtin.h"
 #include "dir.h"
 #include "exec_cmd.h"
diff --git a/builtin-annotate.c b/builtin-annotate.c
index fc43eed..0a259a3 100644
--- a/builtin-annotate.c
+++ b/builtin-annotate.c
@@ -3,7 +3,6 @@
  *
  * Copyright (C) 2006 Ryan Anderson
  */
-#include "git-compat-util.h"
 #include "builtin.h"
 
 int cmd_annotate(int argc, const char **argv, const char *prefix)
diff --git a/builtin-apply.c b/builtin-apply.c
index 1926cd8..356db67 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -6,12 +6,11 @@
  * This applies patches on top of some (arbitrary) version of the SCM.
  *
  */
-#include "cache.h"
+#include "builtin.h"
+#include "blob.h"
 #include "cache-tree.h"
 #include "quote.h"
-#include "blob.h"
 #include "delta.h"
-#include "builtin.h"
 #include "string-list.h"
 #include "dir.h"
 #include "parse-options.h"
diff --git a/builtin-archive.c b/builtin-archive.c
index ab50ceb..f293d40 100644
--- a/builtin-archive.c
+++ b/builtin-archive.c
@@ -2,7 +2,6 @@
  * Copyright (c) 2006 Franck Bui-Huu
  * Copyright (c) 2006 Rene Scharfe
  */
-#include "cache.h"
 #include "builtin.h"
 #include "archive.h"
 #include "parse-options.h"
diff --git a/builtin-blame.c b/builtin-blame.c
index 83141fc..a91d350 100644
--- a/builtin-blame.c
+++ b/builtin-blame.c
@@ -3,11 +3,8 @@
  *
  * Copyright (c) 2006, Junio C Hamano
  */
-
-#include "cache.h"
 #include "builtin.h"
 #include "blob.h"
-#include "commit.h"
 #include "tag.h"
 #include "tree-walk.h"
 #include "diff.h"
diff --git a/builtin-branch.c b/builtin-branch.c
index 07a440e..2cb22c7 100644
--- a/builtin-branch.c
+++ b/builtin-branch.c
@@ -4,12 +4,9 @@
  * Copyright (c) 2006 Kristian Høgsberg <krh@redhat.com>
  * Based on git-branch.sh by Junio C Hamano.
  */
-
-#include "cache.h"
+#include "builtin.h"
 #include "color.h"
 #include "refs.h"
-#include "commit.h"
-#include "builtin.h"
 #include "remote.h"
 #include "parse-options.h"
 #include "branch.h"
diff --git a/builtin-bundle.c b/builtin-bundle.c
index 9b58152..b36456e 100644
--- a/builtin-bundle.c
+++ b/builtin-bundle.c
@@ -1,5 +1,4 @@
 #include "builtin.h"
-#include "cache.h"
 #include "bundle.h"
 
 /*
diff --git a/builtin-cat-file.c b/builtin-cat-file.c
index 8fad19d..70c9bda 100644
--- a/builtin-cat-file.c
+++ b/builtin-cat-file.c
@@ -3,11 +3,10 @@
  *
  * Copyright (C) Linus Torvalds, 2005
  */
-#include "cache.h"
+#include "builtin.h"
 #include "exec_cmd.h"
 #include "tag.h"
 #include "tree.h"
-#include "builtin.h"
 #include "parse-options.h"
 
 #define BATCH 1
diff --git a/builtin-check-attr.c b/builtin-check-attr.c
index 15a04b7..c8f0cb3 100644
--- a/builtin-check-attr.c
+++ b/builtin-check-attr.c
@@ -1,5 +1,4 @@
 #include "builtin.h"
-#include "cache.h"
 #include "attr.h"
 #include "quote.h"
 #include "parse-options.h"
diff --git a/builtin-check-ref-format.c b/builtin-check-ref-format.c
index 701de43..5289580 100644
--- a/builtin-check-ref-format.c
+++ b/builtin-check-ref-format.c
@@ -2,9 +2,8 @@
  * GIT - The information manager from hell
  */
 
-#include "cache.h"
-#include "refs.h"
 #include "builtin.h"
+#include "refs.h"
 
 int cmd_check_ref_format(int argc, const char **argv, const char *prefix)
 {
diff --git a/builtin-checkout-index.c b/builtin-checkout-index.c
index 0d534bc..fbdb37f 100644
--- a/builtin-checkout-index.c
+++ b/builtin-checkout-index.c
@@ -37,7 +37,6 @@
  * but get used to it in scripting!).
  */
 #include "builtin.h"
-#include "cache.h"
 #include "quote.h"
 #include "cache-tree.h"
 #include "parse-options.h"
diff --git a/builtin-checkout.c b/builtin-checkout.c
index fc55bbe..f8bb9ea 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -1,8 +1,7 @@
-#include "cache.h"
 #include "builtin.h"
+#include "blob.h"
 #include "parse-options.h"
 #include "refs.h"
-#include "commit.h"
 #include "tree.h"
 #include "tree-walk.h"
 #include "unpack-trees.h"
@@ -13,7 +12,6 @@
 #include "diff.h"
 #include "revision.h"
 #include "remote.h"
-#include "blob.h"
 #include "xdiff-interface.h"
 #include "ll-merge.h"
 
diff --git a/builtin-clean.c b/builtin-clean.c
index c5ad33d..46a483c 100644
--- a/builtin-clean.c
+++ b/builtin-clean.c
@@ -7,7 +7,6 @@
  */
 
 #include "builtin.h"
-#include "cache.h"
 #include "dir.h"
 #include "parse-options.h"
 #include "quote.h"
diff --git a/builtin-commit-tree.c b/builtin-commit-tree.c
index 0453425..ea83003 100644
--- a/builtin-commit-tree.c
+++ b/builtin-commit-tree.c
@@ -3,10 +3,8 @@
  *
  * Copyright (C) Linus Torvalds, 2005
  */
-#include "cache.h"
-#include "commit.h"
-#include "tree.h"
 #include "builtin.h"
+#include "tree.h"
 #include "utf8.h"
 
 #define BLOCKING (1ul << 14)
diff --git a/builtin-commit.c b/builtin-commit.c
index 46e649c..aba5660 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -5,20 +5,17 @@
  * Based on git-commit.sh by Junio C Hamano and Linus Torvalds
  */
 
-#include "cache.h"
+#include "builtin.h"
 #include "cache-tree.h"
 #include "color.h"
 #include "dir.h"
-#include "builtin.h"
 #include "diff.h"
 #include "diffcore.h"
-#include "commit.h"
 #include "revision.h"
 #include "wt-status.h"
 #include "run-command.h"
 #include "refs.h"
 #include "log-tree.h"
-#include "strbuf.h"
 #include "utf8.h"
 #include "parse-options.h"
 #include "string-list.h"
diff --git a/builtin-config.c b/builtin-config.c
index d8da72c..6a31764 100644
--- a/builtin-config.c
+++ b/builtin-config.c
@@ -1,5 +1,4 @@
 #include "builtin.h"
-#include "cache.h"
 #include "color.h"
 #include "parse-options.h"
 
diff --git a/builtin-count-objects.c b/builtin-count-objects.c
index b814fe5..a6f2337 100644
--- a/builtin-count-objects.c
+++ b/builtin-count-objects.c
@@ -4,9 +4,8 @@
  * Copyright (c) 2006 Junio C Hamano
  */
 
-#include "cache.h"
-#include "dir.h"
 #include "builtin.h"
+#include "dir.h"
 #include "parse-options.h"
 
 static void count_objects(DIR *d, char *path, int len, int verbose,
diff --git a/builtin-describe.c b/builtin-describe.c
index 3a007ed..159c1ad 100644
--- a/builtin-describe.c
+++ b/builtin-describe.c
@@ -1,8 +1,6 @@
-#include "cache.h"
-#include "commit.h"
+#include "builtin.h"
 #include "tag.h"
 #include "refs.h"
-#include "builtin.h"
 #include "exec_cmd.h"
 #include "parse-options.h"
 
diff --git a/builtin-diff-files.c b/builtin-diff-files.c
index 5b64011..4770aeb 100644
--- a/builtin-diff-files.c
+++ b/builtin-diff-files.c
@@ -3,11 +3,9 @@
  *
  * Copyright (C) Linus Torvalds, 2005
  */
-#include "cache.h"
+#include "builtin.h"
 #include "diff.h"
-#include "commit.h"
 #include "revision.h"
-#include "builtin.h"
 
 static const char diff_files_usage[] =
 "git diff-files [-q] [-0/-1/2/3 |-c|--cc] [<common diff options>] [<path>...]"
diff --git a/builtin-diff-index.c b/builtin-diff-index.c
index 0483749..7b8c813 100644
--- a/builtin-diff-index.c
+++ b/builtin-diff-index.c
@@ -1,8 +1,6 @@
-#include "cache.h"
+#include "builtin.h"
 #include "diff.h"
-#include "commit.h"
 #include "revision.h"
-#include "builtin.h"
 
 static const char diff_cache_usage[] =
 "git diff-index [-m] [--cached] "
diff --git a/builtin-diff-tree.c b/builtin-diff-tree.c
index 79cedb7..0b06748 100644
--- a/builtin-diff-tree.c
+++ b/builtin-diff-tree.c
@@ -1,8 +1,6 @@
-#include "cache.h"
+#include "builtin.h"
 #include "diff.h"
-#include "commit.h"
 #include "log-tree.h"
-#include "builtin.h"
 
 static struct rev_info log_tree_opt;
 
diff --git a/builtin-diff.c b/builtin-diff.c
index d75d69b..252c519 100644
--- a/builtin-diff.c
+++ b/builtin-diff.c
@@ -3,16 +3,14 @@
  *
  * Copyright (c) 2006 Junio C Hamano
  */
-#include "cache.h"
-#include "color.h"
-#include "commit.h"
+#include "builtin.h"
 #include "blob.h"
+#include "color.h"
 #include "tag.h"
 #include "diff.h"
 #include "diffcore.h"
 #include "revision.h"
 #include "log-tree.h"
-#include "builtin.h"
 
 struct blobinfo {
 	unsigned char sha1[20];
diff --git a/builtin-fast-export.c b/builtin-fast-export.c
index 34a419c..b62e48d 100644
--- a/builtin-fast-export.c
+++ b/builtin-fast-export.c
@@ -4,8 +4,6 @@
  * Copyright (C) 2007 Johannes E. Schindelin
  */
 #include "builtin.h"
-#include "cache.h"
-#include "commit.h"
 #include "object.h"
 #include "tag.h"
 #include "diff.h"
diff --git a/builtin-fetch--tool.c b/builtin-fetch--tool.c
index 29356d2..1863d5f 100644
--- a/builtin-fetch--tool.c
+++ b/builtin-fetch--tool.c
@@ -1,7 +1,5 @@
 #include "builtin.h"
-#include "cache.h"
 #include "refs.h"
-#include "commit.h"
 #include "sigchain.h"
 
 static char *get_stdin(void)
diff --git a/builtin-fetch.c b/builtin-fetch.c
index 3c998ea..c203286 100644
--- a/builtin-fetch.c
+++ b/builtin-fetch.c
@@ -1,10 +1,8 @@
 /*
  * "git fetch"
  */
-#include "cache.h"
-#include "refs.h"
-#include "commit.h"
 #include "builtin.h"
+#include "refs.h"
 #include "string-list.h"
 #include "remote.h"
 #include "transport.h"
diff --git a/builtin-fmt-merge-msg.c b/builtin-fmt-merge-msg.c
index a788369..772414f 100644
--- a/builtin-fmt-merge-msg.c
+++ b/builtin-fmt-merge-msg.c
@@ -1,6 +1,4 @@
 #include "builtin.h"
-#include "cache.h"
-#include "commit.h"
 #include "diff.h"
 #include "revision.h"
 #include "tag.h"
diff --git a/builtin-for-each-ref.c b/builtin-for-each-ref.c
index 5cbb4b0..ab9bfa4 100644
--- a/builtin-for-each-ref.c
+++ b/builtin-for-each-ref.c
@@ -1,11 +1,9 @@
 #include "builtin.h"
-#include "cache.h"
-#include "refs.h"
+#include "blob.h"
 #include "object.h"
+#include "refs.h"
 #include "tag.h"
-#include "commit.h"
 #include "tree.h"
-#include "blob.h"
 #include "quote.h"
 #include "parse-options.h"
 
diff --git a/builtin-fsck.c b/builtin-fsck.c
index 6436bc2..eae576a 100644
--- a/builtin-fsck.c
+++ b/builtin-fsck.c
@@ -1,6 +1,4 @@
 #include "builtin.h"
-#include "cache.h"
-#include "commit.h"
 #include "tree.h"
 #include "blob.h"
 #include "tag.h"
diff --git a/builtin-gc.c b/builtin-gc.c
index fc556ed..fae3fde 100644
--- a/builtin-gc.c
+++ b/builtin-gc.c
@@ -11,7 +11,6 @@
  */
 
 #include "builtin.h"
-#include "cache.h"
 #include "parse-options.h"
 #include "run-command.h"
 
diff --git a/builtin-grep.c b/builtin-grep.c
index 89489dd..aa5b973 100644
--- a/builtin-grep.c
+++ b/builtin-grep.c
@@ -3,13 +3,11 @@
  *
  * Copyright (c) 2006 Junio C Hamano
  */
-#include "cache.h"
+#include "builtin.h"
 #include "blob.h"
 #include "tree.h"
-#include "commit.h"
 #include "tag.h"
 #include "tree-walk.h"
-#include "builtin.h"
 #include "grep.h"
 
 #ifndef NO_EXTERNAL_GREP
diff --git a/builtin-help.c b/builtin-help.c
index 9b57a74..bdae8a3 100644
--- a/builtin-help.c
+++ b/builtin-help.c
@@ -3,7 +3,6 @@
  *
  * Builtin help command
  */
-#include "cache.h"
 #include "builtin.h"
 #include "exec_cmd.h"
 #include "common-cmds.h"
diff --git a/builtin-init-db.c b/builtin-init-db.c
index fc63d0f..97728cd 100644
--- a/builtin-init-db.c
+++ b/builtin-init-db.c
@@ -3,7 +3,6 @@
  *
  * Copyright (C) Linus Torvalds, 2005
  */
-#include "cache.h"
 #include "builtin.h"
 #include "exec_cmd.h"
 
diff --git a/builtin-log.c b/builtin-log.c
index c7a5772..5105c91 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -4,13 +4,11 @@
  * (C) Copyright 2006 Linus Torvalds
  *		 2006 Junio Hamano
  */
-#include "cache.h"
+#include "builtin.h"
 #include "color.h"
-#include "commit.h"
 #include "diff.h"
 #include "revision.h"
 #include "log-tree.h"
-#include "builtin.h"
 #include "tag.h"
 #include "reflog-walk.h"
 #include "patch-ids.h"
diff --git a/builtin-ls-files.c b/builtin-ls-files.c
index 88e2697..4efd46c 100644
--- a/builtin-ls-files.c
+++ b/builtin-ls-files.c
@@ -5,10 +5,9 @@
  *
  * Copyright (C) Linus Torvalds, 2005
  */
-#include "cache.h"
+#include "builtin.h"
 #include "quote.h"
 #include "dir.h"
-#include "builtin.h"
 #include "tree.h"
 #include "parse-options.h"
 
diff --git a/builtin-ls-remote.c b/builtin-ls-remote.c
index 78a88f7..5fe2af0 100644
--- a/builtin-ls-remote.c
+++ b/builtin-ls-remote.c
@@ -1,5 +1,4 @@
 #include "builtin.h"
-#include "cache.h"
 #include "transport.h"
 #include "remote.h"
 
diff --git a/builtin-ls-tree.c b/builtin-ls-tree.c
index 22008df..4516d14 100644
--- a/builtin-ls-tree.c
+++ b/builtin-ls-tree.c
@@ -3,12 +3,10 @@
  *
  * Copyright (C) Linus Torvalds, 2005
  */
-#include "cache.h"
+#include "builtin.h"
 #include "blob.h"
 #include "tree.h"
-#include "commit.h"
 #include "quote.h"
-#include "builtin.h"
 
 static int line_termination = '\n';
 #define LS_RECURSIVE 1
diff --git a/builtin-mailinfo.c b/builtin-mailinfo.c
index 1eeeb4d..0139a00 100644
--- a/builtin-mailinfo.c
+++ b/builtin-mailinfo.c
@@ -2,10 +2,8 @@
  * Another stupid program, this one parsing the headers of an
  * email to figure out authorship and subject
  */
-#include "cache.h"
 #include "builtin.h"
 #include "utf8.h"
-#include "strbuf.h"
 
 static FILE *cmitmsg, *patchfile, *fin, *fout;
 
diff --git a/builtin-mailsplit.c b/builtin-mailsplit.c
index 71f3b3b..ea22d8b 100644
--- a/builtin-mailsplit.c
+++ b/builtin-mailsplit.c
@@ -4,7 +4,6 @@
  * It just splits a mbox into a list of files: "0001" "0002" ..
  * so you can process them further from there.
  */
-#include "cache.h"
 #include "builtin.h"
 #include "string-list.h"
 
diff --git a/builtin-merge-base.c b/builtin-merge-base.c
index 03fc1c2..4f915e8 100644
--- a/builtin-merge-base.c
+++ b/builtin-merge-base.c
@@ -1,6 +1,4 @@
 #include "builtin.h"
-#include "cache.h"
-#include "commit.h"
 #include "parse-options.h"
 
 static int show_merge_base(struct commit **rev, int rev_nr, int show_all)
diff --git a/builtin-merge-file.c b/builtin-merge-file.c
index 96edb97..7589d61 100644
--- a/builtin-merge-file.c
+++ b/builtin-merge-file.c
@@ -1,5 +1,4 @@
 #include "builtin.h"
-#include "cache.h"
 #include "xdiff/xdiff.h"
 #include "xdiff-interface.h"
 #include "parse-options.h"
diff --git a/builtin-merge-ours.c b/builtin-merge-ours.c
index 8f5bbaf..33b85d0 100644
--- a/builtin-merge-ours.c
+++ b/builtin-merge-ours.c
@@ -7,7 +7,6 @@
  *
  * Pretend we resolved the heads, but declare our tree trumps everybody else.
  */
-#include "git-compat-util.h"
 #include "builtin.h"
 
 static const char *diff_index_args[] = {
diff --git a/builtin-merge.c b/builtin-merge.c
index 4c11935..cb4c645 100644
--- a/builtin-merge.c
+++ b/builtin-merge.c
@@ -6,13 +6,11 @@
  * Based on git-merge.sh by Junio C Hamano.
  */
 
-#include "cache.h"
-#include "parse-options.h"
 #include "builtin.h"
+#include "parse-options.h"
 #include "run-command.h"
 #include "diff.h"
 #include "refs.h"
-#include "commit.h"
 #include "diffcore.h"
 #include "revision.h"
 #include "unpack-trees.h"
diff --git a/builtin-mv.c b/builtin-mv.c
index 01270fe..659471b 100644
--- a/builtin-mv.c
+++ b/builtin-mv.c
@@ -3,7 +3,6 @@
  *
  * Copyright (C) 2006 Johannes Schindelin
  */
-#include "cache.h"
 #include "builtin.h"
 #include "dir.h"
 #include "cache-tree.h"
diff --git a/builtin-name-rev.c b/builtin-name-rev.c
index 08c8aab..386cafe 100644
--- a/builtin-name-rev.c
+++ b/builtin-name-rev.c
@@ -1,6 +1,4 @@
 #include "builtin.h"
-#include "cache.h"
-#include "commit.h"
 #include "tag.h"
 #include "refs.h"
 #include "parse-options.h"
diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index 2000d97..fe7a8de 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -1,9 +1,7 @@
 #include "builtin.h"
-#include "cache.h"
 #include "attr.h"
 #include "object.h"
 #include "blob.h"
-#include "commit.h"
 #include "tag.h"
 #include "tree.h"
 #include "delta.h"
diff --git a/builtin-prune-packed.c b/builtin-prune-packed.c
index 2d5b2cd..99f9fab 100644
--- a/builtin-prune-packed.c
+++ b/builtin-prune-packed.c
@@ -1,5 +1,4 @@
 #include "builtin.h"
-#include "cache.h"
 #include "progress.h"
 
 static const char prune_packed_usage[] =
diff --git a/builtin-prune.c b/builtin-prune.c
index 545e9c1..caf2751 100644
--- a/builtin-prune.c
+++ b/builtin-prune.c
@@ -1,8 +1,6 @@
-#include "cache.h"
-#include "commit.h"
+#include "builtin.h"
 #include "diff.h"
 #include "revision.h"
-#include "builtin.h"
 #include "reachable.h"
 #include "parse-options.h"
 #include "dir.h"
diff --git a/builtin-push.c b/builtin-push.c
index 2eabcd3..128bfd8 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -1,10 +1,9 @@
 /*
  * "git push"
  */
-#include "cache.h"
+#include "builtin.h"
 #include "refs.h"
 #include "run-command.h"
-#include "builtin.h"
 #include "remote.h"
 #include "transport.h"
 #include "parse-options.h"
diff --git a/builtin-read-tree.c b/builtin-read-tree.c
index 8e02738..dd4a9f6 100644
--- a/builtin-read-tree.c
+++ b/builtin-read-tree.c
@@ -4,14 +4,13 @@
  * Copyright (C) Linus Torvalds, 2005
  */
 
-#include "cache.h"
+#include "builtin.h"
 #include "object.h"
 #include "tree.h"
 #include "tree-walk.h"
 #include "cache-tree.h"
 #include "unpack-trees.h"
 #include "dir.h"
-#include "builtin.h"
 
 static int nr_trees;
 static struct tree *trees[MAX_UNPACK_TREES];
diff --git a/builtin-reflog.c b/builtin-reflog.c
index d95f515..fcd92e8 100644
--- a/builtin-reflog.c
+++ b/builtin-reflog.c
@@ -1,6 +1,4 @@
-#include "cache.h"
 #include "builtin.h"
-#include "commit.h"
 #include "refs.h"
 #include "dir.h"
 #include "tree-walk.h"
diff --git a/builtin-rerere.c b/builtin-rerere.c
index 020af73..d0a6ebb 100644
--- a/builtin-rerere.c
+++ b/builtin-rerere.c
@@ -1,5 +1,4 @@
 #include "builtin.h"
-#include "cache.h"
 #include "dir.h"
 #include "string-list.h"
 #include "rerere.h"
diff --git a/builtin-rev-list.c b/builtin-rev-list.c
index 40d5fcb..bc86e3c 100644
--- a/builtin-rev-list.c
+++ b/builtin-rev-list.c
@@ -1,14 +1,12 @@
-#include "cache.h"
+#include "builtin.h"
+#include "blob.h"
 #include "refs.h"
 #include "tag.h"
-#include "commit.h"
 #include "tree.h"
-#include "blob.h"
 #include "tree-walk.h"
 #include "diff.h"
 #include "revision.h"
 #include "list-objects.h"
-#include "builtin.h"
 #include "log-tree.h"
 #include "graph.h"
 
diff --git a/builtin-revert.c b/builtin-revert.c
index 3f2614e..90fea7d 100644
--- a/builtin-revert.c
+++ b/builtin-revert.c
@@ -1,7 +1,5 @@
-#include "cache.h"
 #include "builtin.h"
 #include "object.h"
-#include "commit.h"
 #include "tag.h"
 #include "wt-status.h"
 #include "run-command.h"
diff --git a/builtin-rm.c b/builtin-rm.c
index 269d608..7fb865a 100644
--- a/builtin-rm.c
+++ b/builtin-rm.c
@@ -3,7 +3,6 @@
  *
  * Copyright (C) Linus Torvalds 2006
  */
-#include "cache.h"
 #include "builtin.h"
 #include "dir.h"
 #include "cache-tree.h"
diff --git a/builtin-shortlog.c b/builtin-shortlog.c
index b28091b..9b6662b 100644
--- a/builtin-shortlog.c
+++ b/builtin-shortlog.c
@@ -1,6 +1,4 @@
 #include "builtin.h"
-#include "cache.h"
-#include "commit.h"
 #include "diff.h"
 #include "string-list.h"
 #include "revision.h"
diff --git a/builtin-show-branch.c b/builtin-show-branch.c
index 828e6f8..02c2c5c 100644
--- a/builtin-show-branch.c
+++ b/builtin-show-branch.c
@@ -1,7 +1,5 @@
-#include "cache.h"
-#include "commit.h"
-#include "refs.h"
 #include "builtin.h"
+#include "refs.h"
 
 static const char show_branch_usage[] =
 "git show-branch [--sparse] [--current] [--all] [--remotes] [--topo-order] [--more=count | --list | --independent | --merge-base ] [--topics] [<refs>...] | --reflog[=n[,b]] <branch>";
diff --git a/builtin-show-ref.c b/builtin-show-ref.c
index dc76c50..619b64e 100644
--- a/builtin-show-ref.c
+++ b/builtin-show-ref.c
@@ -1,5 +1,4 @@
 #include "builtin.h"
-#include "cache.h"
 #include "refs.h"
 #include "object.h"
 #include "tag.h"
diff --git a/builtin-stripspace.c b/builtin-stripspace.c
index d6e3896..126b37a 100644
--- a/builtin-stripspace.c
+++ b/builtin-stripspace.c
@@ -1,5 +1,4 @@
 #include "builtin.h"
-#include "cache.h"
 
 /*
  * Returns the length of a line, without trailing spaces.
diff --git a/builtin-symbolic-ref.c b/builtin-symbolic-ref.c
index 6ae6bcc..41814bf 100644
--- a/builtin-symbolic-ref.c
+++ b/builtin-symbolic-ref.c
@@ -1,5 +1,4 @@
 #include "builtin.h"
-#include "cache.h"
 #include "refs.h"
 #include "parse-options.h"
 
diff --git a/builtin-tag.c b/builtin-tag.c
index 01e7374..15f4ab7 100644
--- a/builtin-tag.c
+++ b/builtin-tag.c
@@ -6,7 +6,6 @@
  * Based on git-tag.sh and mktag.c by Linus Torvalds.
  */
 
-#include "cache.h"
 #include "builtin.h"
 #include "refs.h"
 #include "tag.h"
diff --git a/builtin-tar-tree.c b/builtin-tar-tree.c
index 0713bca..9707eea 100644
--- a/builtin-tar-tree.c
+++ b/builtin-tar-tree.c
@@ -1,8 +1,6 @@
 /*
  * Copyright (c) 2005, 2006 Rene Scharfe
  */
-#include "cache.h"
-#include "commit.h"
 #include "tar.h"
 #include "builtin.h"
 #include "quote.h"
diff --git a/builtin-unpack-objects.c b/builtin-unpack-objects.c
index 9a77323..4f5495d 100644
--- a/builtin-unpack-objects.c
+++ b/builtin-unpack-objects.c
@@ -1,10 +1,8 @@
 #include "builtin.h"
-#include "cache.h"
 #include "object.h"
 #include "delta.h"
 #include "pack.h"
 #include "blob.h"
-#include "commit.h"
 #include "tag.h"
 #include "tree.h"
 #include "tree-walk.h"
diff --git a/builtin-update-index.c b/builtin-update-index.c
index 1fde893..9a006bd 100644
--- a/builtin-update-index.c
+++ b/builtin-update-index.c
@@ -3,11 +3,10 @@
  *
  * Copyright (C) Linus Torvalds, 2005
  */
-#include "cache.h"
+#include "builtin.h"
 #include "quote.h"
 #include "cache-tree.h"
 #include "tree-walk.h"
-#include "builtin.h"
 #include "refs.h"
 
 /*
diff --git a/builtin-update-ref.c b/builtin-update-ref.c
index 378dc1b..cac86ef 100644
--- a/builtin-update-ref.c
+++ b/builtin-update-ref.c
@@ -1,6 +1,5 @@
-#include "cache.h"
-#include "refs.h"
 #include "builtin.h"
+#include "refs.h"
 #include "parse-options.h"
 
 static const char * const git_update_ref_usage[] = {
diff --git a/builtin-upload-archive.c b/builtin-upload-archive.c
index 0206b41..fe29d7b 100644
--- a/builtin-upload-archive.c
+++ b/builtin-upload-archive.c
@@ -1,7 +1,6 @@
 /*
  * Copyright (c) 2006 Franck Bui-Huu
  */
-#include "cache.h"
 #include "builtin.h"
 #include "archive.h"
 #include "pkt-line.h"
diff --git a/builtin-verify-pack.c b/builtin-verify-pack.c
index 0ee0a9a..070455f 100644
--- a/builtin-verify-pack.c
+++ b/builtin-verify-pack.c
@@ -1,5 +1,4 @@
 #include "builtin.h"
-#include "cache.h"
 #include "pack.h"
 #include "pack-revindex.h"
 
diff --git a/builtin-verify-tag.c b/builtin-verify-tag.c
index 729a159..039fe0a 100644
--- a/builtin-verify-tag.c
+++ b/builtin-verify-tag.c
@@ -5,7 +5,6 @@
  *
  * Based on git-verify-tag.sh
  */
-#include "cache.h"
 #include "builtin.h"
 #include "tag.h"
 #include "run-command.h"
diff --git a/builtin-write-tree.c b/builtin-write-tree.c
index 9d64050..f1ce5e5 100644
--- a/builtin-write-tree.c
+++ b/builtin-write-tree.c
@@ -4,7 +4,6 @@
  * Copyright (C) Linus Torvalds, 2005
  */
 #include "builtin.h"
-#include "cache.h"
 #include "tree.h"
 #include "cache-tree.h"
 
diff --git a/diff-no-index.c b/diff-no-index.c
index 42c1dd8..cb68b8d 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c
@@ -4,16 +4,14 @@
  * Copyright (c) 2008 by Junio C Hamano
  */
 
-#include "cache.h"
+#include "builtin.h"
 #include "color.h"
-#include "commit.h"
 #include "blob.h"
 #include "tag.h"
 #include "diff.h"
 #include "diffcore.h"
 #include "revision.h"
 #include "log-tree.h"
-#include "builtin.h"
 #include "string-list.h"
 
 static int read_directory(const char *path, struct string_list *list)
diff --git a/fast-import.c b/fast-import.c
index db44da3..e3c138f 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -141,11 +141,9 @@ Format of STDIN stream:
 */
 
 #include "builtin.h"
-#include "cache.h"
 #include "object.h"
 #include "blob.h"
 #include "tree.h"
-#include "commit.h"
 #include "delta.h"
 #include "pack.h"
 #include "refs.h"
diff --git a/git.c b/git.c
index c2b181e..b6c1884 100644
--- a/git.c
+++ b/git.c
@@ -1,6 +1,5 @@
 #include "builtin.h"
 #include "exec_cmd.h"
-#include "cache.h"
 #include "quote.h"
 #include "run-command.h"
 
diff --git a/help.c b/help.c
index fd87bb5..7edbc43 100644
--- a/help.c
+++ b/help.c
@@ -1,4 +1,3 @@
-#include "cache.h"
 #include "builtin.h"
 #include "exec_cmd.h"
 #include "levenshtein.h"
diff --git a/merge-recursive.c b/merge-recursive.c
index 3e1bc3e..352677c 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -3,11 +3,9 @@
  * Fredrik Kuivinen.
  * The thieves were Alex Riesen and Johannes Schindelin, in June/July 2006
  */
-#include "cache.h"
-#include "cache-tree.h"
-#include "commit.h"
-#include "blob.h"
 #include "builtin.h"
+#include "blob.h"
+#include "cache-tree.h"
 #include "tree-walk.h"
 #include "diff.h"
 #include "diffcore.h"
-- 
1.6.1.3

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 2/5] Header includes cleanup
  2009-03-30  9:55 ` [PATCH 1/5] " Nathaniel P Dawson
@ 2009-03-30  9:55   ` Nathaniel P Dawson
  2009-03-30  9:55     ` [PATCH 3/5] " Nathaniel P Dawson
  0 siblings, 1 reply; 17+ messages in thread
From: Nathaniel P Dawson @ 2009-03-30  9:55 UTC (permalink / raw)
  To: git; +Cc: Nathaniel P Dawson

Deleted header includes of object.h where blob.h was included since blob.h includes it.

Signed-off-by: Nathaniel P Dawson <nathaniel.dawson@gmail.com>
---
 alloc.c                  |    1 -
 builtin-for-each-ref.c   |    1 -
 builtin-pack-objects.c   |    1 -
 builtin-unpack-objects.c |    3 +--
 fast-import.c            |    1 -
 fsck.c                   |    1 -
 object.c                 |    1 -
 7 files changed, 1 insertions(+), 8 deletions(-)

diff --git a/alloc.c b/alloc.c
index 216c23a..5b3d4a2 100644
--- a/alloc.c
+++ b/alloc.c
@@ -10,7 +10,6 @@
  * for the new allocation is.
  */
 #include "cache.h"
-#include "object.h"
 #include "blob.h"
 #include "tree.h"
 #include "commit.h"
diff --git a/builtin-for-each-ref.c b/builtin-for-each-ref.c
index ab9bfa4..6cbfb03 100644
--- a/builtin-for-each-ref.c
+++ b/builtin-for-each-ref.c
@@ -1,6 +1,5 @@
 #include "builtin.h"
 #include "blob.h"
-#include "object.h"
 #include "refs.h"
 #include "tag.h"
 #include "tree.h"
diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index fe7a8de..c53147a 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -1,6 +1,5 @@
 #include "builtin.h"
 #include "attr.h"
-#include "object.h"
 #include "blob.h"
 #include "tag.h"
 #include "tree.h"
diff --git a/builtin-unpack-objects.c b/builtin-unpack-objects.c
index 4f5495d..674bede 100644
--- a/builtin-unpack-objects.c
+++ b/builtin-unpack-objects.c
@@ -1,8 +1,7 @@
 #include "builtin.h"
-#include "object.h"
+#include "blob.h"
 #include "delta.h"
 #include "pack.h"
-#include "blob.h"
 #include "tag.h"
 #include "tree.h"
 #include "tree-walk.h"
diff --git a/fast-import.c b/fast-import.c
index e3c138f..8d1d9f3 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -141,7 +141,6 @@ Format of STDIN stream:
 */
 
 #include "builtin.h"
-#include "object.h"
 #include "blob.h"
 #include "tree.h"
 #include "delta.h"
diff --git a/fsck.c b/fsck.c
index 511b82c..e903f6a 100644
--- a/fsck.c
+++ b/fsck.c
@@ -1,5 +1,4 @@
 #include "cache.h"
-#include "object.h"
 #include "blob.h"
 #include "tree.h"
 #include "tree-walk.h"
diff --git a/object.c b/object.c
index 7e6a92c..380652f 100644
--- a/object.c
+++ b/object.c
@@ -1,5 +1,4 @@
 #include "cache.h"
-#include "object.h"
 #include "blob.h"
 #include "tree.h"
 #include "commit.h"
-- 
1.6.1.3

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 3/5] Header includes cleanup
  2009-03-30  9:55   ` [PATCH 2/5] " Nathaniel P Dawson
@ 2009-03-30  9:55     ` Nathaniel P Dawson
  2009-03-30  9:55       ` [PATCH 4/5] " Nathaniel P Dawson
  0 siblings, 1 reply; 17+ messages in thread
From: Nathaniel P Dawson @ 2009-03-30  9:55 UTC (permalink / raw)
  To: git; +Cc: Nathaniel P Dawson

Deleted header includes of git-compat-util.h, strbuf.h, hash.h where cache.h was included since cache.h includes them.

Signed-off-by: Nathaniel P Dawson <nathaniel.dawson@gmail.com>
---
 builtin-clone.c   |    1 -
 builtin-remote.c  |    1 -
 connect.c         |    1 -
 diffcore-rename.c |    1 -
 editor.c          |    1 -
 hash.c            |    1 -
 parse-options.c   |    3 +--
 shell.c           |    1 -
 test-delta.c      |    3 +--
 9 files changed, 2 insertions(+), 11 deletions(-)

diff --git a/builtin-clone.c b/builtin-clone.c
index 0031b5f..50cb6ba 100644
--- a/builtin-clone.c
+++ b/builtin-clone.c
@@ -16,7 +16,6 @@
 #include "tree-walk.h"
 #include "unpack-trees.h"
 #include "transport.h"
-#include "strbuf.h"
 #include "dir.h"
 #include "pack-refs.h"
 #include "sigchain.h"
diff --git a/builtin-remote.c b/builtin-remote.c
index 9ef846f..5bf8042 100644
--- a/builtin-remote.c
+++ b/builtin-remote.c
@@ -3,7 +3,6 @@
 #include "transport.h"
 #include "remote.h"
 #include "string-list.h"
-#include "strbuf.h"
 #include "run-command.h"
 #include "refs.h"
 
diff --git a/connect.c b/connect.c
index 7636bf9..12a5c2b 100644
--- a/connect.c
+++ b/connect.c
@@ -1,4 +1,3 @@
-#include "git-compat-util.h"
 #include "cache.h"
 #include "pkt-line.h"
 #include "quote.h"
diff --git a/diffcore-rename.c b/diffcore-rename.c
index 0b0d6b8..00e1ce1 100644
--- a/diffcore-rename.c
+++ b/diffcore-rename.c
@@ -4,7 +4,6 @@
 #include "cache.h"
 #include "diff.h"
 #include "diffcore.h"
-#include "hash.h"
 
 /* Table of rename/copy destinations */
 
diff --git a/editor.c b/editor.c
index 4d469d0..22fd4c0 100644
--- a/editor.c
+++ b/editor.c
@@ -1,5 +1,4 @@
 #include "cache.h"
-#include "strbuf.h"
 #include "run-command.h"
 
 int launch_editor(const char *path, struct strbuf *buffer, const char *const *env)
diff --git a/hash.c b/hash.c
index 1cd4c9d..5a68fdf 100644
--- a/hash.c
+++ b/hash.c
@@ -2,7 +2,6 @@
  * Some generic hashing helpers.
  */
 #include "cache.h"
-#include "hash.h"
 
 /*
  * Look up a hash entry in the hash table. Return the pointer to
diff --git a/parse-options.c b/parse-options.c
index cf71bcf..bafeeea 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -1,6 +1,5 @@
-#include "git-compat-util.h"
-#include "parse-options.h"
 #include "cache.h"
+#include "parse-options.h"
 #include "commit.h"
 
 #define OPT_SHORT 1
diff --git a/shell.c b/shell.c
index e339369..37c52c2 100644
--- a/shell.c
+++ b/shell.c
@@ -1,7 +1,6 @@
 #include "cache.h"
 #include "quote.h"
 #include "exec_cmd.h"
-#include "strbuf.h"
 
 static int do_generic_cmd(const char *me, char *arg)
 {
diff --git a/test-delta.c b/test-delta.c
index 3d885ff..3d90611 100644
--- a/test-delta.c
+++ b/test-delta.c
@@ -8,9 +8,8 @@
  * published by the Free Software Foundation.
  */
 
-#include "git-compat-util.h"
-#include "delta.h"
 #include "cache.h"
+#include "delta.h"
 
 static const char usage_str[] =
 	"test-delta (-d|-p) <from_file> <data_file> <out_file>";
-- 
1.6.1.3

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 4/5] Header includes cleanup
  2009-03-30  9:55     ` [PATCH 3/5] " Nathaniel P Dawson
@ 2009-03-30  9:55       ` Nathaniel P Dawson
  2009-03-30  9:55         ` [PATCH 5/5] " Nathaniel P Dawson
  0 siblings, 1 reply; 17+ messages in thread
From: Nathaniel P Dawson @ 2009-03-30  9:55 UTC (permalink / raw)
  To: git; +Cc: Nathaniel P Dawson

Deleted header includes of object.h, tree.h, strbuf.h, and decorate.h where commit.h was included since commit.h includes them.

Signed-off-by: Nathaniel P Dawson <nathaniel.dawson@gmail.com>
---
 alloc.c                |    1 -
 builtin-receive-pack.c |    1 -
 builtin-reset.c        |    2 --
 builtin.h              |    2 --
 bundle.c               |    1 -
 fsck.c                 |    3 +--
 index-pack.c           |    1 -
 list-objects.c         |    1 -
 object.c               |    1 -
 read-cache.c           |    1 -
 revision.c             |    2 --
 server-info.c          |    1 -
 sha1_file.c            |    1 -
 sha1_name.c            |    1 -
 tag.c                  |    1 -
 tree.c                 |    3 +--
 upload-pack.c          |    1 -
 walker.c               |    1 -
 wt-status.c            |    3 +--
 19 files changed, 3 insertions(+), 25 deletions(-)

diff --git a/alloc.c b/alloc.c
index 5b3d4a2..af0dd88 100644
--- a/alloc.c
+++ b/alloc.c
@@ -11,7 +11,6 @@
  */
 #include "cache.h"
 #include "blob.h"
-#include "tree.h"
 #include "commit.h"
 #include "tag.h"
 
diff --git a/builtin-receive-pack.c b/builtin-receive-pack.c
index a970b39..e95aa41 100644
--- a/builtin-receive-pack.c
+++ b/builtin-receive-pack.c
@@ -5,7 +5,6 @@
 #include "run-command.h"
 #include "exec_cmd.h"
 #include "commit.h"
-#include "object.h"
 #include "remote.h"
 #include "transport.h"
 
diff --git a/builtin-reset.c b/builtin-reset.c
index c0cb915..d44c935 100644
--- a/builtin-reset.c
+++ b/builtin-reset.c
@@ -9,13 +9,11 @@
  */
 #include "cache.h"
 #include "tag.h"
-#include "object.h"
 #include "commit.h"
 #include "run-command.h"
 #include "refs.h"
 #include "diff.h"
 #include "diffcore.h"
-#include "tree.h"
 #include "branch.h"
 #include "parse-options.h"
 
diff --git a/builtin.h b/builtin.h
index 1495cf6..b9ef9fc 100644
--- a/builtin.h
+++ b/builtin.h
@@ -1,8 +1,6 @@
 #ifndef BUILTIN_H
 #define BUILTIN_H
 
-#include "git-compat-util.h"
-#include "strbuf.h"
 #include "cache.h"
 #include "commit.h"
 
diff --git a/bundle.c b/bundle.c
index d0dd818..14966f8 100644
--- a/bundle.c
+++ b/bundle.c
@@ -1,6 +1,5 @@
 #include "cache.h"
 #include "bundle.h"
-#include "object.h"
 #include "commit.h"
 #include "diff.h"
 #include "revision.h"
diff --git a/fsck.c b/fsck.c
index e903f6a..8cbb6aa 100644
--- a/fsck.c
+++ b/fsck.c
@@ -1,8 +1,7 @@
 #include "cache.h"
 #include "blob.h"
-#include "tree.h"
-#include "tree-walk.h"
 #include "commit.h"
+#include "tree-walk.h"
 #include "tag.h"
 #include "fsck.h"
 
diff --git a/index-pack.c b/index-pack.c
index 7546822..ac6fc84 100644
--- a/index-pack.c
+++ b/index-pack.c
@@ -5,7 +5,6 @@
 #include "blob.h"
 #include "commit.h"
 #include "tag.h"
-#include "tree.h"
 #include "progress.h"
 #include "fsck.h"
 #include "exec_cmd.h"
diff --git a/list-objects.c b/list-objects.c
index c8b8375..6446f81 100644
--- a/list-objects.c
+++ b/list-objects.c
@@ -1,7 +1,6 @@
 #include "cache.h"
 #include "tag.h"
 #include "commit.h"
-#include "tree.h"
 #include "blob.h"
 #include "diff.h"
 #include "tree-walk.h"
diff --git a/object.c b/object.c
index 380652f..83f786c 100644
--- a/object.c
+++ b/object.c
@@ -1,6 +1,5 @@
 #include "cache.h"
 #include "blob.h"
-#include "tree.h"
 #include "commit.h"
 #include "tag.h"
 
diff --git a/read-cache.c b/read-cache.c
index 3f58711..b50b02e 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -8,7 +8,6 @@
 #include "cache-tree.h"
 #include "refs.h"
 #include "dir.h"
-#include "tree.h"
 #include "commit.h"
 #include "diff.h"
 #include "diffcore.h"
diff --git a/revision.c b/revision.c
index f5771c7..827108d 100644
--- a/revision.c
+++ b/revision.c
@@ -1,7 +1,6 @@
 #include "cache.h"
 #include "tag.h"
 #include "blob.h"
-#include "tree.h"
 #include "commit.h"
 #include "diff.h"
 #include "refs.h"
@@ -10,7 +9,6 @@
 #include "grep.h"
 #include "reflog-walk.h"
 #include "patch-ids.h"
-#include "decorate.h"
 #include "log-tree.h"
 
 volatile show_early_output_fn_t show_early_output;
diff --git a/server-info.c b/server-info.c
index 66b0d9d..3d54c44 100644
--- a/server-info.c
+++ b/server-info.c
@@ -1,6 +1,5 @@
 #include "cache.h"
 #include "refs.h"
-#include "object.h"
 #include "commit.h"
 #include "tag.h"
 
diff --git a/sha1_file.c b/sha1_file.c
index 54972f9..77e6f54 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -12,7 +12,6 @@
 #include "blob.h"
 #include "commit.h"
 #include "tag.h"
-#include "tree.h"
 #include "refs.h"
 #include "pack-revindex.h"
 #include "sha1-lookup.h"
diff --git a/sha1_name.c b/sha1_name.c
index 2f75179..c98cd30 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -1,7 +1,6 @@
 #include "cache.h"
 #include "tag.h"
 #include "commit.h"
-#include "tree.h"
 #include "blob.h"
 #include "tree-walk.h"
 #include "refs.h"
diff --git a/tag.c b/tag.c
index 4470d2b..94e2903 100644
--- a/tag.c
+++ b/tag.c
@@ -1,7 +1,6 @@
 #include "cache.h"
 #include "tag.h"
 #include "commit.h"
-#include "tree.h"
 #include "blob.h"
 
 const char *tag_type = "tag";
diff --git a/tree.c b/tree.c
index 25d2e29..b9e427d 100644
--- a/tree.c
+++ b/tree.c
@@ -1,8 +1,7 @@
 #include "cache.h"
 #include "cache-tree.h"
-#include "tree.h"
-#include "blob.h"
 #include "commit.h"
+#include "blob.h"
 #include "tag.h"
 #include "tree-walk.h"
 
diff --git a/upload-pack.c b/upload-pack.c
index a49d872..9a91b14 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -3,7 +3,6 @@
 #include "pkt-line.h"
 #include "sideband.h"
 #include "tag.h"
-#include "object.h"
 #include "commit.h"
 #include "exec_cmd.h"
 #include "diff.h"
diff --git a/walker.c b/walker.c
index e57630e..67ddcf6 100644
--- a/walker.c
+++ b/walker.c
@@ -1,7 +1,6 @@
 #include "cache.h"
 #include "walker.h"
 #include "commit.h"
-#include "tree.h"
 #include "tree-walk.h"
 #include "tag.h"
 #include "blob.h"
diff --git a/wt-status.c b/wt-status.c
index 929b00f..125a4c4 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1,9 +1,8 @@
 #include "cache.h"
 #include "wt-status.h"
 #include "color.h"
-#include "object.h"
-#include "dir.h"
 #include "commit.h"
+#include "dir.h"
 #include "diff.h"
 #include "revision.h"
 #include "diffcore.h"
-- 
1.6.1.3

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 5/5] Header includes cleanup
  2009-03-30  9:55       ` [PATCH 4/5] " Nathaniel P Dawson
@ 2009-03-30  9:55         ` Nathaniel P Dawson
  0 siblings, 0 replies; 17+ messages in thread
From: Nathaniel P Dawson @ 2009-03-30  9:55 UTC (permalink / raw)
  To: git; +Cc: Nathaniel P Dawson

Deleted header includes of tree-walk.h where diff.h was included since diff.h includes it.

Signed-off-by: Nathaniel P Dawson <nathaniel.dawson@gmail.com>
---
 builtin-blame.c        |    1 -
 builtin-checkout.c     |    3 +--
 builtin-pack-objects.c |    1 -
 builtin-reflog.c       |    1 -
 builtin-rev-list.c     |    1 -
 list-objects.c         |    1 -
 merge-recursive.c      |    1 -
 7 files changed, 1 insertions(+), 8 deletions(-)

diff --git a/builtin-blame.c b/builtin-blame.c
index a91d350..bce181c 100644
--- a/builtin-blame.c
+++ b/builtin-blame.c
@@ -6,7 +6,6 @@
 #include "builtin.h"
 #include "blob.h"
 #include "tag.h"
-#include "tree-walk.h"
 #include "diff.h"
 #include "diffcore.h"
 #include "revision.h"
diff --git a/builtin-checkout.c b/builtin-checkout.c
index f8bb9ea..3000e87 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -3,13 +3,12 @@
 #include "parse-options.h"
 #include "refs.h"
 #include "tree.h"
-#include "tree-walk.h"
+#include "diff.h"
 #include "unpack-trees.h"
 #include "dir.h"
 #include "run-command.h"
 #include "merge-recursive.h"
 #include "branch.h"
-#include "diff.h"
 #include "revision.h"
 #include "remote.h"
 #include "xdiff-interface.h"
diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index c53147a..9d87fc7 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -7,7 +7,6 @@
 #include "pack.h"
 #include "pack-revindex.h"
 #include "csum-file.h"
-#include "tree-walk.h"
 #include "diff.h"
 #include "revision.h"
 #include "list-objects.h"
diff --git a/builtin-reflog.c b/builtin-reflog.c
index fcd92e8..4c122d6 100644
--- a/builtin-reflog.c
+++ b/builtin-reflog.c
@@ -1,7 +1,6 @@
 #include "builtin.h"
 #include "refs.h"
 #include "dir.h"
-#include "tree-walk.h"
 #include "diff.h"
 #include "revision.h"
 #include "reachable.h"
diff --git a/builtin-rev-list.c b/builtin-rev-list.c
index bc86e3c..cedfe9a 100644
--- a/builtin-rev-list.c
+++ b/builtin-rev-list.c
@@ -3,7 +3,6 @@
 #include "refs.h"
 #include "tag.h"
 #include "tree.h"
-#include "tree-walk.h"
 #include "diff.h"
 #include "revision.h"
 #include "list-objects.h"
diff --git a/list-objects.c b/list-objects.c
index 6446f81..715e06d 100644
--- a/list-objects.c
+++ b/list-objects.c
@@ -3,7 +3,6 @@
 #include "commit.h"
 #include "blob.h"
 #include "diff.h"
-#include "tree-walk.h"
 #include "revision.h"
 #include "list-objects.h"
 
diff --git a/merge-recursive.c b/merge-recursive.c
index 352677c..370151e 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -6,7 +6,6 @@
 #include "builtin.h"
 #include "blob.h"
 #include "cache-tree.h"
-#include "tree-walk.h"
 #include "diff.h"
 #include "diffcore.h"
 #include "tag.h"
-- 
1.6.1.3

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH 0/5] Header includes cleanup
  2009-03-30  9:55 [PATCH 0/5] Header includes cleanup Nathaniel P Dawson
  2009-03-30  9:55 ` [PATCH 1/5] " Nathaniel P Dawson
@ 2009-03-30 10:50 ` Johannes Sixt
  2009-03-30 17:33   ` Nathaniel P Dawson
  2009-03-31  5:53   ` Christian Couder
  1 sibling, 2 replies; 17+ messages in thread
From: Johannes Sixt @ 2009-03-30 10:50 UTC (permalink / raw)
  To: Nathaniel P Dawson; +Cc: git

Please wrap your lines at ca. 75 columns.

Nathaniel P Dawson schrieb:
> This is just the beginning for this project. I'm slowly cleaning up
> the header includes one chunk at a time. I hope my patches aren't too
> messy, I've learned how to better utilize git to make patches and
> organize my commits logically so I'll submit neater chunks henceforth.
> You can expect patches from me nightly until I've finished this project.

You have removed includes that are implied by other includes, i.e. if
foo.h includes bar.h, then you removed #include "bar.h" from *.c if there
is #include "foo.h".

IMO, this is not a good guiding principle to reduce includes. A better
principle is to keep #include "bar.h" in a source or header file iff a
feature that is declared or defined in bar.h is *used* *directly* in that
source or header file, regardless of whether bar.h is included in foo.h
that is itself included in that source or header file.

If this latter principle is obeyed, then the build won't break by removing
the include of foo.h (for the reason that nothing of foo.h is *use*
*directly* anymore).

-- Hannes

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 0/5] Header includes cleanup
  2009-03-30 10:50 ` [PATCH 0/5] " Johannes Sixt
@ 2009-03-30 17:33   ` Nathaniel P Dawson
  2009-03-31  6:59     ` Christian Couder
  2009-03-31  5:53   ` Christian Couder
  1 sibling, 1 reply; 17+ messages in thread
From: Nathaniel P Dawson @ 2009-03-30 17:33 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: git

[-- Attachment #1: Type: text/plain, Size: 1556 bytes --]

I'm sorry, I must have misunderstood the task on the janitor
list. Could you explain it a bit better so I would be able to do it
properly?

And as for line wrapping I apologize, emacs on my laptop was
apparently not turning on auto-fill but has been rectified.

Regards,
Nathaniel P Dawson

On Mon, Mar 30, 2009 at 12:50:07PM +0200, Johannes Sixt wrote:
> Please wrap your lines at ca. 75 columns.
> 
> Nathaniel P Dawson schrieb:
> > This is just the beginning for this project. I'm slowly cleaning up
> > the header includes one chunk at a time. I hope my patches aren't too
> > messy, I've learned how to better utilize git to make patches and
> > organize my commits logically so I'll submit neater chunks henceforth.
> > You can expect patches from me nightly until I've finished this project.
> 
> You have removed includes that are implied by other includes, i.e. if
> foo.h includes bar.h, then you removed #include "bar.h" from *.c if there
> is #include "foo.h".
> 
> IMO, this is not a good guiding principle to reduce includes. A better
> principle is to keep #include "bar.h" in a source or header file iff a
> feature that is declared or defined in bar.h is *used* *directly* in that
> source or header file, regardless of whether bar.h is included in foo.h
> that is itself included in that source or header file.
> 
> If this latter principle is obeyed, then the build won't break by removing
> the include of foo.h (for the reason that nothing of foo.h is *use*
> *directly* anymore).
> 
> -- Hannes
> 

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 0/5] Header includes cleanup
  2009-03-30 10:50 ` [PATCH 0/5] " Johannes Sixt
  2009-03-30 17:33   ` Nathaniel P Dawson
@ 2009-03-31  5:53   ` Christian Couder
  1 sibling, 0 replies; 17+ messages in thread
From: Christian Couder @ 2009-03-31  5:53 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Nathaniel P Dawson, git

Le lundi 30 mars 2009, Johannes Sixt a écrit :
> Please wrap your lines at ca. 75 columns.
>
> Nathaniel P Dawson schrieb:
> > This is just the beginning for this project. I'm slowly cleaning up
> > the header includes one chunk at a time. I hope my patches aren't too
> > messy, I've learned how to better utilize git to make patches and
> > organize my commits logically so I'll submit neater chunks henceforth.
> > You can expect patches from me nightly until I've finished this
> > project.
>
> You have removed includes that are implied by other includes, i.e. if
> foo.h includes bar.h, then you removed #include "bar.h" from *.c if there
> is #include "foo.h".
>
> IMO, this is not a good guiding principle to reduce includes. A better
> principle is to keep #include "bar.h" in a source or header file iff a
> feature that is declared or defined in bar.h is *used* *directly* in that
> source or header file, regardless of whether bar.h is included in foo.h
> that is itself included in that source or header file.

In theory, it's perhaps the best way to do it. But we don't do that for 
system header includes. And I think it makes it very difficult to cleanup 
header includes, because you have to check what is needed for each feature 
in a file. So I think we have to choose between basically no header include 
cleanup or a cleanup like the one Nathaniel started.

> If this latter principle is obeyed, then the build won't break by
> removing the include of foo.h (for the reason that nothing of foo.h is
> *use* *directly* anymore).

I think it's more likely that we will add features rather than remove some.
And it's not very difficult if we remove some, to remove "foo.h" and 
add "bar.h" at the same time.

For example, many files include "cache.h", does it really make sense to try 
to remove these #include because we could replace them by only includes 
of "git-compat-util.h" and "hash.h"? I don't think so.

Best regards,
Christian.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 0/5] Header includes cleanup
  2009-03-30 17:33   ` Nathaniel P Dawson
@ 2009-03-31  6:59     ` Christian Couder
  2009-03-31 16:04       ` Junio C Hamano
  0 siblings, 1 reply; 17+ messages in thread
From: Christian Couder @ 2009-03-31  6:59 UTC (permalink / raw)
  To: Nathaniel P Dawson; +Cc: Johannes Sixt, git

Le lundi 30 mars 2009, Nathaniel P Dawson a écrit :
> I'm sorry, I must have misunderstood the task on the janitor
> list. Could you explain it a bit better so I would be able to do it
> properly?

Actually I added this task to the Janitor page on the wiki because I was 
tired of having to find which other includes are needed each time I 
include "revision.h". And I think there are other worthwile include cleanup 
that could be done.

I think it's a good thing that you started working on it even if in the end 
we decide that we want these cleanup to be done otherwise. At least we will 
hopefully have clarified our include header policy.

Thanks,
Christian.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 0/5] Header includes cleanup
  2009-03-31  6:59     ` Christian Couder
@ 2009-03-31 16:04       ` Junio C Hamano
  2009-04-02  3:57         ` Christian Couder
  0 siblings, 1 reply; 17+ messages in thread
From: Junio C Hamano @ 2009-03-31 16:04 UTC (permalink / raw)
  To: Christian Couder; +Cc: Nathaniel P Dawson, Johannes Sixt, git

Christian Couder <chriscool@tuxfamily.org> writes:

> I think it's a good thing that you started working on it even if in the end 
> we decide that we want these cleanup to be done otherwise. At least we will 
> hopefully have clarified our include header policy.

Before seeing a lot of patches to change #include all over the place, I'd
like to see a simple guiding principle described, not just a subjective "I
think this makes things better" but with "... because of X and Y and Z".

The document Documentation/CodingGuidelines describes the only policy that
exists currently: git-compat-util.h must be the first thing the compiler
sees.  The language should probably be stronger than what appears there:

 - The first #include in C files, except in platform specific
   compat/ implementations, should be git-compat-util.h or another
   well-known header file that includes it at the beginning, namely
   cache.h or builtin.h.

Even though http.h may include "cache.h" at the very beginning, I'd rather
not to see http-walker.c lose inclusion of "cache.h".  It will force us to
remember that http.h is Ok to include as the first file, and that won't
scale.

The reason git-compat-util.h must be the first is because inclusion of all
the system header files is supposed to happen there, and there are some
platforms that have broken system header dependencies we do not want
application writers to care about, and the compat-util header knows about
them.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 0/5] Header includes cleanup
  2009-03-31 16:04       ` Junio C Hamano
@ 2009-04-02  3:57         ` Christian Couder
  2009-04-02  5:25           ` Junio C Hamano
  0 siblings, 1 reply; 17+ messages in thread
From: Christian Couder @ 2009-04-02  3:57 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Nathaniel P Dawson, Johannes Sixt, git

Le mardi 31 mars 2009, Junio C Hamano a écrit :
> Christian Couder <chriscool@tuxfamily.org> writes:
> > I think it's a good thing that you started working on it even if in the
> > end we decide that we want these cleanup to be done otherwise. At least
> > we will hopefully have clarified our include header policy.
>
> Before seeing a lot of patches to change #include all over the place, I'd
> like to see a simple guiding principle described, not just a subjective
> "I think this makes things better" but with "... because of X and Y and
> Z".
>
> The document Documentation/CodingGuidelines describes the only policy
> that exists currently: git-compat-util.h must be the first thing the
> compiler sees.  The language should probably be stronger than what
> appears there:
>
>  - The first #include in C files, except in platform specific
>    compat/ implementations, should be git-compat-util.h or another
>    well-known header file that includes it at the beginning, namely
>    cache.h or builtin.h.
>
> Even though http.h may include "cache.h" at the very beginning, I'd
> rather not to see http-walker.c lose inclusion of "cache.h".  It will
> force us to remember that http.h is Ok to include as the first file, and
> that won't scale.
>
> The reason git-compat-util.h must be the first is because inclusion of
> all the system header files is supposed to happen there, and there are
> some platforms that have broken system header dependencies we do not want
> application writers to care about, and the compat-util header knows about
> them.

Ok, so I suggest the following simple guiding principles:

- git-compat-util.h or cache.h or builtin.h should always be the first 
#include in C files,

- header files should include other incude files if and only if the other 
includes are needed to compile them,

- a header file should be included in a C file only if it is needed to 
compile the C file (it is not ok to include it only because it includes 
many other headers that are needed)

- other than the above rules, it is ok to reduce the number of includes as 
much as possible

What do you think?

By the way, Nathaniel, you should try to give more meaningful titles to the 
individual patches in your patch series and perhaps make them smaller so 
that they are easier to review and have less conflicts with other patches.

For example your patch "[PATCH 1/5] Header includes cleanup" could be 
splitted and the resulting patches could be renamed like this

"delete includes of 'git-compat-util.h' where 'builtin.h' is included"
"delete includes of 'cache.h' where 'builtin.h' is included"
"delete includes of 'strbuf.h' where 'builtin.h' is included"
"delete includes of 'commit.h' where 'builtin.h' is included"

Or perhaps Junio would prefer that you work on a C file by C file basis? 
Like for example:

"delete useless includes in 'builtin-diff-files.c'"
"delete useless includes in 'builtin-diff-index.c'"
...

Thanks,
Christian.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 0/5] Header includes cleanup
  2009-04-02  3:57         ` Christian Couder
@ 2009-04-02  5:25           ` Junio C Hamano
  2009-04-02 11:27             ` Jeff King
  2009-04-03  3:58             ` Christian Couder
  0 siblings, 2 replies; 17+ messages in thread
From: Junio C Hamano @ 2009-04-02  5:25 UTC (permalink / raw)
  To: Christian Couder; +Cc: Nathaniel P Dawson, Johannes Sixt, git

Christian Couder <chriscool@tuxfamily.org> writes:

> Ok, so I suggest the following simple guiding principles:
>
> - git-compat-util.h or cache.h or builtin.h should always be the first 
> #include in C files,
>
> - header files should include other incude files if and only if the other 
> includes are needed to compile them,

This is unclear.

Long before I started touching git, I used to be religious about making
header files self contained.  For a header file frotz.h in the project, I
used to insist that

	$ cat >1.c <<\EOF
	#include "frotz.h"
        EOF
        $ cc -Wall -c 1.c

did not fail.  

Are you talking about that by "to compile them"?

I stopped doing that long time ago, partly because the rule was cumbersome
to enforce, but primarily because it was not helping much in the larger
picture.

Such a rule, together with strict rules such as the order of including
other header files in the header files themselves, may make life easier
for programmers who touch .c files but never .h files, because they can
include only the necessary .h files and in any order.  But in practice,
people need to touch both .h and .c files and when they need to include
new system header files, they need to follow the inclusion order rule
somewhere anyway---at that point, it does not matter much if the rule
applies to only .h files or both .h and .c files.

So for example, you cannot compile

	$ cat >1.c <<\EOF
        #include "revision.h"
        EOF
        $ cc -Wll -c 1.c

in git.git project, but I do not think it is a problem.

> - a header file should be included in a C file only if it is needed to 
> compile the C file (it is not ok to include it only because it includes 
> many other headers that are needed)

If that is the rule, perhaps the problem lies not in a .c program that
includes such a .h header, but in the .h itself that includes many other
header files.

> - other than the above rules, it is ok to reduce the number of includes as 
> much as possible
>
> What do you think?

What you did not write and I forgot to mention, which is a logical
conclusion of the first rule, is that C files should not directly include
common system header files such as unistd, sys/stat, etc.

There are exceptions to any rule.  For example, inclusion of syslog.h in
daemon.c is OK because most of the rest of the system does not even use
syslog.  If we later find a platform whose syslog.h has some funny
inter-header dependencies, however, we will need to include it in the
git-compat-util.h and resolve the dependencies there, like we do for other
system header files.

> Or perhaps Junio would prefer that you work on a C file by C file basis? 
> Like for example:
>
> "delete useless includes in 'builtin-diff-files.c'"
> "delete useless includes in 'builtin-diff-index.c'"

If the series does not involve .h file clean-up, then a series that
consists of one patch per .c file would be easier to handle, I think.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 0/5] Header includes cleanup
  2009-04-02  5:25           ` Junio C Hamano
@ 2009-04-02 11:27             ` Jeff King
  2009-04-03  4:14               ` Christian Couder
  2009-04-03  3:58             ` Christian Couder
  1 sibling, 1 reply; 17+ messages in thread
From: Jeff King @ 2009-04-02 11:27 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Christian Couder, Nathaniel P Dawson, Johannes Sixt, git

On Wed, Apr 01, 2009 at 10:25:09PM -0700, Junio C Hamano wrote:

> > - a header file should be included in a C file only if it is needed to 
> > compile the C file (it is not ok to include it only because it includes 
> > many other headers that are needed)
> 
> If that is the rule, perhaps the problem lies not in a .c program that
> includes such a .h header, but in the .h itself that includes many other
> header files.

If this were combined with splitting gigantic .h files (like cache.h)
into smaller logical units, then we could in theory speed up
recompilation times with make (we would also need to correctly track
header dependencies, but gcc -M can do this fairly easily).

But it does come at the price of actually having to consider which
include files are necessary. I can't think of more than half a dozen
times in the last year I have actually had to add a #include while
working on a git .c file, mostly because everything and the kitchen sink
is included by cache.h.

So I don't know if it is worth it.

-Peff

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 0/5] Header includes cleanup
  2009-04-02  5:25           ` Junio C Hamano
  2009-04-02 11:27             ` Jeff King
@ 2009-04-03  3:58             ` Christian Couder
  1 sibling, 0 replies; 17+ messages in thread
From: Christian Couder @ 2009-04-03  3:58 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Nathaniel P Dawson, Johannes Sixt, git

Le jeudi 2 avril 2009, Junio C Hamano a écrit :
> Christian Couder <chriscool@tuxfamily.org> writes:
> > Ok, so I suggest the following simple guiding principles:
> >
> > - git-compat-util.h or cache.h or builtin.h should always be the first
> > #include in C files,
> >
> > - header files should include other incude files if and only if the
> > other includes are needed to compile them,
>
> This is unclear.
>
> Long before I started touching git, I used to be religious about making
> header files self contained.  For a header file frotz.h in the project, I
> used to insist that
>
> 	$ cat >1.c <<\EOF
> 	#include "frotz.h"
>         EOF
>         $ cc -Wall -c 1.c
>
> did not fail.
>
> Are you talking about that by "to compile them"?

Well, I wanted to say that for a header file frotz.h in the project:

	$ cat >1.c <<\EOF
	#include "cache.h"
	#include "frotz.h"
	EOF
	$ cc -Wall -c 1.c
	
should not fail.

(Ok, in practice, let's say that something like:

$ cc -Wall -DSHA1_HEADER='<openssl/sha.h>' -c 1.c

should not fail.)

> I stopped doing that long time ago, partly because the rule was
> cumbersome to enforce, but primarily because it was not helping much in
> the larger picture.
>
> Such a rule, together with strict rules such as the order of including
> other header files in the header files themselves, may make life easier
> for programmers who touch .c files but never .h files, because they can
> include only the necessary .h files and in any order.  But in practice,
> people need to touch both .h and .c files and when they need to include
> new system header files, they need to follow the inclusion order rule
> somewhere anyway---at that point, it does not matter much if the rule
> applies to only .h files or both .h and .c files.
>
> So for example, you cannot compile
>
> 	$ cat >1.c <<\EOF
>         #include "revision.h"
>         EOF
>         $ cc -Wll -c 1.c
>
> in git.git project, but I do not think it is a problem.

I think it may become a problem if a header needs more headers to be 
included before it, and those latter headers again need more headers and so 
on.

The advantage of having and using "cache.h" is that things are quite simple. 
You include cache.h and then a few more headers for special features you 
need, and, here you go, you can code up something quite interesting without 
worrying too much about includes. (Though the compilation time is perhaps a 
little longer than it could be.)

If we loose this simplicity because we don't take care or for some 
theoretical reason, then I think we have lost everything that really 
matters.

A simple patch like this:

---------8<------------
$ gdiff
diff --git a/revision.h b/revision.h
index 5adfc91..495d7eb 100644
--- a/revision.h
+++ b/revision.h
@@ -3,6 +3,8 @@

 #include "parse-options.h"
 #include "grep.h"
+#include "diff.h"
+#include "commit.h"

 #define SEEN           (1u<<0)
 #define UNINTERESTING   (1u<<1)
---------8<------------

makes the following just work(tm):

	$ cat >1.c <<\EOF
	#include "cache.h"
	#include "revision.h"
	EOF
	$ cc -Wall -DSHA1_HEADER='<openssl/sha.h>' -c 1.c
	$

(By the way it's this kind of patches that I hoped would get posted first.)

> > - a header file should be included in a C file only if it is needed to
> > compile the C file (it is not ok to include it only because it includes
> > many other headers that are needed)

I should have said "a header file other than git-compat-util.h or cache.h 
should be included ..."

> If that is the rule, perhaps the problem lies not in a .c program that
> includes such a .h header, but in the .h itself that includes many other
> header files.

It depends if the .h that includes many other header files does that for a 
good reason or not.

> > - other than the above rules, it is ok to reduce the number of includes
> > as much as possible
> >
> > What do you think?
>
> What you did not write and I forgot to mention, which is a logical
> conclusion of the first rule, is that C files should not directly include
> common system header files such as unistd, sys/stat, etc.

Yeah, I forgot to restate that.

> There are exceptions to any rule.  For example, inclusion of syslog.h in
> daemon.c is OK because most of the rest of the system does not even use
> syslog.  If we later find a platform whose syslog.h has some funny
> inter-header dependencies, however, we will need to include it in the
> git-compat-util.h and resolve the dependencies there, like we do for
> other system header files.

Ok, I will state that too.

If you agree I will post a patch to "Documentation/CodingGuidelines" with 
the above clarifications. (I can also post an improved version of my patch 
to "revision.h".)

Thanks,
Christian.

> > Or perhaps Junio would prefer that you work on a C file by C file
> > basis? Like for example:
> >
> > "delete useless includes in 'builtin-diff-files.c'"
> > "delete useless includes in 'builtin-diff-index.c'"
>
> If the series does not involve .h file clean-up, then a series that
> consists of one patch per .c file would be easier to handle, I think.

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH 0/5] Header includes cleanup
  2009-04-02 11:27             ` Jeff King
@ 2009-04-03  4:14               ` Christian Couder
  2009-04-03 12:24                 ` Jeff King
  0 siblings, 1 reply; 17+ messages in thread
From: Christian Couder @ 2009-04-03  4:14 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, Nathaniel P Dawson, Johannes Sixt, git

Hi,

Le jeudi 2 avril 2009, Jeff King a écrit :
> On Wed, Apr 01, 2009 at 10:25:09PM -0700, Junio C Hamano wrote:
> > > - a header file should be included in a C file only if it is needed
> > > to compile the C file (it is not ok to include it only because it
> > > includes many other headers that are needed)
> >
> > If that is the rule, perhaps the problem lies not in a .c program that
> > includes such a .h header, but in the .h itself that includes many
> > other header files.
>
> If this were combined with splitting gigantic .h files (like cache.h)
> into smaller logical units, then we could in theory speed up
> recompilation times with make (we would also need to correctly track
> header dependencies, but gcc -M can do this fairly easily).
>
> But it does come at the price of actually having to consider which
> include files are necessary. I can't think of more than half a dozen
> times in the last year I have actually had to add a #include while
> working on a git .c file, mostly because everything and the kitchen sink
> is included by cache.h.

Yeah, I think the best feature of the actual design is the simplicity, and 
that's why we don't have to add new #include very often. So let's keep this 
simplicity by having and applying rules to keep things simple for the 
developer.

> So I don't know if it is worth it.

I am not sure what you are talking about here, but if you mean that you 
don't think it's worth splitting gigantic .h files (like cache.h) into 
smaller logical units, then I agree.

Best regards,
Christian.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 0/5] Header includes cleanup
  2009-04-03  4:14               ` Christian Couder
@ 2009-04-03 12:24                 ` Jeff King
  0 siblings, 0 replies; 17+ messages in thread
From: Jeff King @ 2009-04-03 12:24 UTC (permalink / raw)
  To: Christian Couder; +Cc: Junio C Hamano, Nathaniel P Dawson, Johannes Sixt, git

On Fri, Apr 03, 2009 at 06:14:26AM +0200, Christian Couder wrote:

> > So I don't know if it is worth it.
> 
> I am not sure what you are talking about here, but if you mean that you 
> don't think it's worth splitting gigantic .h files (like cache.h) into 
> smaller logical units, then I agree.

What I meant was "there is a tradeoff between faster compilation times
and potentially more programmer effort, and I don't know if that
tradeoff is worth it". So I think you understood what I was saying, and
I think your point is reasonable.

-Peff

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2009-04-03 12:26 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-03-30  9:55 [PATCH 0/5] Header includes cleanup Nathaniel P Dawson
2009-03-30  9:55 ` [PATCH 1/5] " Nathaniel P Dawson
2009-03-30  9:55   ` [PATCH 2/5] " Nathaniel P Dawson
2009-03-30  9:55     ` [PATCH 3/5] " Nathaniel P Dawson
2009-03-30  9:55       ` [PATCH 4/5] " Nathaniel P Dawson
2009-03-30  9:55         ` [PATCH 5/5] " Nathaniel P Dawson
2009-03-30 10:50 ` [PATCH 0/5] " Johannes Sixt
2009-03-30 17:33   ` Nathaniel P Dawson
2009-03-31  6:59     ` Christian Couder
2009-03-31 16:04       ` Junio C Hamano
2009-04-02  3:57         ` Christian Couder
2009-04-02  5:25           ` Junio C Hamano
2009-04-02 11:27             ` Jeff King
2009-04-03  4:14               ` Christian Couder
2009-04-03 12:24                 ` Jeff King
2009-04-03  3:58             ` Christian Couder
2009-03-31  5:53   ` Christian Couder

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).