Re: [PATCH] build: get rid of the notion of a git library

[Felipe Contreras <felipe.contreras@gmail.com>]

On Sun, Jun 9, 2013 at 12:32 PM, John Keeping <john@keeping.me.uk> wrote:
> On Sun, Jun 09, 2013 at 12:13:41PM -0500, Felipe Contreras wrote:
>> On Sun, Jun 9, 2013 at 12:03 PM, Ramkumar Ramachandra
>> <artagnon@gmail.com> wrote:
>> > John Keeping wrote:
>> >> Calling across from one builtin/*.c file to another is just as wrong as
>> >> calling into a builtin/*.c file from a top-level file but the build
>> >> system happens not to enforce the former.
>> >
>> > So libgit.a is a collection of everything that is shared between
>> > builtins?  Does that correspond to reality?
>
> I think that's *precisely* what libgit.a is.

We don't care what libgit.a is; the important thing is what it *should* be.

> A quick check with "git log -S..." shows that most of these have barely
> been touched since the builtin/ directory was created.  So the reason
> they're not static is most likely because no one has tidied them up
> since the division between builtins was introduced.
>
> It is a fact of life that as we live and work with a system we realise
> that there may be a better way of doing something.  This doesn't mean
> that someone needs to immediately convert everything to the new way,
> it is often sufficient to do new things in the new way and slowly move
> existing things across as and when they are touched for other reasons.

Really?

builtin/ls-files.c:307:13: error: static declaration of
‘overlay_tree_on_cache’ follows non-static declaration
 static void overlay_tree_on_cache(const char *tree_name, const char *prefix)
             ^
In file included from builtin/ls-files.c:8:0:
./cache.h:1318:6: note: previous declaration of ‘overlay_tree_on_cache’ was here
 void overlay_tree_on_cache(const char *tree_name, const char *prefix);
      ^
builtin/ls-files.c:361:12: error: static declaration of
‘report_path_error’ follows non-static declaration
 static int report_path_error(const char *ps_matched, const char
**pathspec, const char *prefix)
            ^
In file included from builtin/ls-files.c:8:0:
./cache.h:1317:5: note: previous declaration of ‘report_path_error’ was here
 int report_path_error(const char *ps_matched, const char **pathspec,
const char *prefix);
     ^
make: *** [builtin/ls-files.o] Error 1
make: *** Waiting for unfinished jobs....
builtin/add.c:184:12: error: static declaration of
‘add_files_to_cache’ follows non-static declaration
 static int add_files_to_cache(const char *prefix, const char
**pathspec, int flags)
            ^
In file included from builtin/add.c:6:0:
./cache.h:1283:5: note: previous declaration of ‘add_files_to_cache’ was here
 int add_files_to_cache(const char *prefix, const char **pathspec, int flags);
     ^
builtin/add.c:280:12: error: static declaration of
‘run_add_interactive’ follows non-static declaration
 static int run_add_interactive(const char *revision, const char *patch_mode,
            ^
In file included from ./builtin.h:7:0,
                 from builtin/add.c:7:
./commit.h:187:12: note: previous declaration of ‘run_add_interactive’ was here
 extern int run_add_interactive(const char *revision, const char *patch_mode,
            ^
builtin/add.c:309:12: error: static declaration of ‘interactive_add’
follows non-static declaration
 static int interactive_add(int argc, const char **argv, const char
*prefix, int patch)
            ^
In file included from ./builtin.h:7:0,
                 from builtin/add.c:7:
./commit.h:186:12: note: previous declaration of ‘interactive_add’ was here
 extern int interactive_add(int argc, const char **argv, const char
*prefix, int patch);
            ^
builtin/add.c:184:12: warning: ‘add_files_to_cache’ defined but not
used [-Wunused-function]
 static int add_files_to_cache(const char *prefix, const char
**pathspec, int flags)
            ^
make: *** [builtin/add.o] Error 1
builtin/fmt-merge-msg.c:19:12: error: static declaration of
‘fmt_merge_msg_config’ follows non-static declaration
 static int fmt_merge_msg_config(const char *key, const char *value, void *cb)
            ^
In file included from builtin/fmt-merge-msg.c:9:0:
./fmt-merge-msg.h:5:12: note: previous declaration of
‘fmt_merge_msg_config’ was here
 extern int fmt_merge_msg_config(const char *key, const char *value, void *cb);
            ^
builtin/fmt-merge-msg.c:592:12: error: static declaration of
‘fmt_merge_msg’ follows non-static declaration
 static int fmt_merge_msg(struct strbuf *in, struct strbuf *out,
            ^
In file included from builtin/fmt-merge-msg.c:1:0:
./builtin.h:26:12: note: previous declaration of ‘fmt_merge_msg’ was here
 extern int fmt_merge_msg(struct strbuf *in, struct strbuf *out,
            ^
make: *** [builtin/fmt-merge-msg.o] Error 1
builtin/init-db.c:316:12: error: static declaration of
‘set_git_dir_init’ follows non-static declaration
 static int set_git_dir_init(const char *git_dir, const char *real_git_dir,
            ^
In file included from builtin/init-db.c:6:0:
./cache.h:421:12: note: previous declaration of ‘set_git_dir_init’ was here
 extern int set_git_dir_init(const char *git_dir, const char
*real_git_dir, int);
            ^
builtin/init-db.c:368:12: error: static declaration of ‘init_db’
follows non-static declaration
 static int init_db(const char *template_dir, unsigned int flags)
            ^
In file included from builtin/init-db.c:6:0:
./cache.h:422:12: note: previous declaration of ‘init_db’ was here
 extern int init_db(const char *template_dir, unsigned int flags);
            ^
make: *** [builtin/init-db.o] Error 1
builtin/shortlog.c:110:13: error: static declaration of
‘shortlog_add_commit’ follows non-static declaration
 static void shortlog_add_commit(struct shortlog *log, struct commit *commit)
             ^
In file included from builtin/shortlog.c:9:0:
./shortlog.h:24:6: note: previous declaration of ‘shortlog_add_commit’ was here
 void shortlog_add_commit(struct shortlog *log, struct commit *commit);
      ^
builtin/shortlog.c:207:13: error: static declaration of
‘shortlog_init’ follows non-static declaration
 static void shortlog_init(struct shortlog *log)
             ^
In file included from builtin/shortlog.c:9:0:
./shortlog.h:22:6: note: previous declaration of ‘shortlog_init’ was here
 void shortlog_init(struct shortlog *log);
      ^
builtin/shortlog.c:287:13: error: static declaration of
‘shortlog_output’ follows non-static declaration
 static void shortlog_output(struct shortlog *log)
             ^
In file included from builtin/shortlog.c:9:0:
./shortlog.h:26:6: note: previous declaration of ‘shortlog_output’ was here
 void shortlog_output(struct shortlog *log);
      ^
builtin/shortlog.c:287:13: warning: ‘shortlog_output’ defined but not
used [-Wunused-function]
 static void shortlog_output(struct shortlog *log)
             ^
make: *** [builtin/shortlog.o] Error 1
builtin/stripspace.c:36:13: error: static declaration of ‘stripspace’
follows non-static declaration
 static void stripspace(struct strbuf *sb, int skip_comments)
             ^
In file included from ./builtin.h:5:0,
                 from builtin/stripspace.c:1:
./strbuf.h:165:13: note: previous declaration of ‘stripspace’ was here
 extern void stripspace(struct strbuf *buf, int skip_comments);
             ^
make: *** [builtin/stripspace.o] Error 1
make[2]: `GIT-VERSION-FILE' is up to date.
builtin/prune-packed.c:37:13: error: static declaration of
‘prune_packed_objects’ follows non-static declaration
 static void prune_packed_objects(int opts)
             ^
In file included from builtin/prune-packed.c:1:0:
./builtin.h:18:13: note: previous declaration of ‘prune_packed_objects’ was here
 extern void prune_packed_objects(int);
             ^
make: *** [builtin/prune-packed.o] Error 1
builtin/notes.c:358:34: error: static declaration of
‘init_copy_notes_for_rewrite’ follows non-static declaration
 static struct notes_rewrite_cfg *init_copy_notes_for_rewrite(const char *cmd)
                                  ^
In file included from builtin/notes.c:11:0:
./builtin.h:39:27: note: previous declaration of
‘init_copy_notes_for_rewrite’ was here
 struct notes_rewrite_cfg *init_copy_notes_for_rewrite(const char *cmd);
                           ^
builtin/notes.c:396:12: error: static declaration of
‘copy_note_for_rewrite’ follows non-static declaration
 static int copy_note_for_rewrite(struct notes_rewrite_cfg *c,
            ^
In file included from builtin/notes.c:11:0:
./builtin.h:40:5: note: previous declaration of ‘copy_note_for_rewrite’ was here
 int copy_note_for_rewrite(struct notes_rewrite_cfg *c,
     ^
builtin/notes.c:406:13: error: static declaration of
‘finish_copy_notes_for_rewrite’ follows non-static declaration
 static void finish_copy_notes_for_rewrite(struct notes_rewrite_cfg *c)
             ^
In file included from builtin/notes.c:11:0:
./builtin.h:42:6: note: previous declaration of
‘finish_copy_notes_for_rewrite’ was here
 void finish_copy_notes_for_rewrite(struct notes_rewrite_cfg *c);
      ^
make: *** [builtin/notes.o] Error 1
builtin/blame.c:112:12: error: static declaration of ‘textconv_object’
follows non-static declaration
 static int textconv_object(const char *path,
            ^
In file included from builtin/blame.c:8:0:
./builtin.h:44:12: note: previous declaration of ‘textconv_object’ was here
 extern int textconv_object(const char *path, unsigned mode, const
unsigned char *sha1, int sha1_valid, char **buf, unsigned long
*buf_size);
            ^
make: *** [builtin/blame.o] Error 1

-- 
Felipe Contreras