{"thread":{"id":"34068","subject":"[PATCH] build: get rid of the notion of a git library","startedAt":"2013-06-08T17:29:34Z","lastAt":"2013-06-13T18:50:46Z","messageCount":59,"participants":["Felipe Contreras","Ramkumar Ramachandra","John Keeping","Vincent van Ravesteijn","Jeff King","Junio C Hamano","Linus Torvalds","Johan Herland","Andreas Krey"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"219844","messageId":"1370712574-27688-1-git-send-email-felipe.contreras@gmail.com","threadId":"34068","inReplyTo":null,"subject":"[PATCH] build: get rid of the notion of a git library","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-06-08T17:29:34Z","receivedAt":"2013-06-08T17:29:34Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"There's no libgit, and there will never be, every object file in Git is\nthe same, and there's wish to organize them in any way; they are *all*\nfor the 'git' binary and its builtin commands.\n\nSo let's shatter any hopes of ever having a library, and be clear about\nit; both the top-level objects (./*.o) and the builtin objects\n(./builtin/*.o) go into git.a, which is not a library, merely a\nconvenient way to stash objects together.\n\nThis way there will not be linking issues when top-level objects try to\naccess functions of builtin objects.\n\nLIB_OBJS and LIB_H imply a library, but there isn't one, and never will\nbe; so give them proper names; just a bunch of headers and objects.\n\nSigned-off-by: Felipe Contreras \n---\n Makefile | 564 ++++++++++++++++++++++++++++++++-------------------------------\n 1 file changed, 283 insertions(+), 281 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 03524d0..63451b1 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -435,8 +435,8 @@ XDIFF_OBJS =\n VCSSVN_OBJS =\n GENERATED_H =\n EXTRA_CPPFLAGS =\n-LIB_H =\n-LIB_OBJS =\n+HEADERS =\n+OBJS =\n PROGRAM_OBJS =\n PROGRAMS =\n SCRIPT_PERL =\n@@ -629,270 +629,270 @@ endif\n export PERL_PATH\n export PYTHON_PATH\n \n-LIB_FILE = libgit.a\n+GIT_LIB = git.a\n XDIFF_LIB = xdiff/lib.a\n VCSSVN_LIB = vcs-svn/lib.a\n \n GENERATED_H += common-cmds.h\n \n-LIB_H += advice.h\n-LIB_H += archive.h\n-LIB_H += argv-array.h\n-LIB_H += attr.h\n-LIB_H += bisect.h\n-LIB_H += blob.h\n-LIB_H += branch.h\n-LIB_H += builtin.h\n-LIB_H += bulk-checkin.h\n-LIB_H += bundle.h\n-LIB_H += cache-tree.h\n-LIB_H += cache.h\n-LIB_H += color.h\n-LIB_H += column.h\n-LIB_H += commit.h\n-LIB_H += compat/bswap.h\n-LIB_H += compat/cygwin.h\n-LIB_H += compat/mingw.h\n-LIB_H += compat/obstack.h\n-LIB_H += compat/poll/poll.h\n-LIB_H += compat/precompose_utf8.h\n-LIB_H += compat/terminal.h\n-LIB_H += compat/win32/dirent.h\n-LIB_H += compat/win32/pthread.h\n-LIB_H += compat/win32/syslog.h\n-LIB_H += connected.h\n-LIB_H += convert.h\n-LIB_H += credential.h\n-LIB_H += csum-file.h\n-LIB_H += decorate.h\n-LIB_H += delta.h\n-LIB_H += diff.h\n-LIB_H += diffcore.h\n-LIB_H += dir.h\n-LIB_H += exec_cmd.h\n-LIB_H += fetch-pack.h\n-LIB_H += fmt-merge-msg.h\n-LIB_H += fsck.h\n-LIB_H += gettext.h\n-LIB_H += git-compat-util.h\n-LIB_H += gpg-interface.h\n-LIB_H += graph.h\n-LIB_H += grep.h\n-LIB_H += hash.h\n-LIB_H += help.h\n-LIB_H += http.h\n-LIB_H += kwset.h\n-LIB_H += levenshtein.h\n-LIB_H += line-log.h\n-LIB_H += line-range.h\n-LIB_H += list-objects.h\n-LIB_H += ll-merge.h\n-LIB_H += log-tree.h\n-LIB_H += mailmap.h\n-LIB_H += merge-blobs.h\n-LIB_H += merge-recursive.h\n-LIB_H += mergesort.h\n-LIB_H += notes-cache.h\n-LIB_H += notes-merge.h\n-LIB_H += notes.h\n-LIB_H += object.h\n-LIB_H += pack-revindex.h\n-LIB_H += pack.h\n-LIB_H += parse-options.h\n-LIB_H += patch-ids.h\n-LIB_H += pathspec.h\n-LIB_H += pkt-line.h\n-LIB_H += progress.h\n-LIB_H += prompt.h\n-LIB_H += quote.h\n-LIB_H += reachable.h\n-LIB_H += reflog-walk.h\n-LIB_H += refs.h\n-LIB_H += remote.h\n-LIB_H += rerere.h\n-LIB_H += resolve-undo.h\n-LIB_H += revision.h\n-LIB_H += run-command.h\n-LIB_H += send-pack.h\n-LIB_H += sequencer.h\n-LIB_H += sha1-array.h\n-LIB_H += sha1-lookup.h\n-LIB_H += shortlog.h\n-LIB_H += sideband.h\n-LIB_H += sigchain.h\n-LIB_H += strbuf.h\n-LIB_H += streaming.h\n-LIB_H += string-list.h\n-LIB_H += submodule.h\n-LIB_H += tag.h\n-LIB_H += tar.h\n-LIB_H += thread-utils.h\n-LIB_H += transport.h\n-LIB_H += tree-walk.h\n-LIB_H += tree.h\n-LIB_H += unpack-trees.h\n-LIB_H += url.h\n-LIB_H += userdiff.h\n-LIB_H += utf8.h\n-LIB_H += varint.h\n-LIB_H += vcs-svn/fast_export.h\n-LIB_H += vcs-svn/line_buffer.h\n-LIB_H += vcs-svn/repo_tree.h\n-LIB_H += vcs-svn/sliding_window.h\n-LIB_H += vcs-svn/svndiff.h\n-LIB_H += vcs-svn/svndump.h\n-LIB_H += walker.h\n-LIB_H += wildmatch.h\n-LIB_H += wt-status.h\n-LIB_H += xdiff-interface.h\n-LIB_H += xdiff/xdiff.h\n-LIB_H += xdiff/xdiffi.h\n-LIB_H += xdiff/xemit.h\n-LIB_H += xdiff/xinclude.h\n-LIB_H += xdiff/xmacros.h\n-LIB_H += xdiff/xprepare.h\n-LIB_H += xdiff/xtypes.h\n-LIB_H += xdiff/xutils.h\n-\n-LIB_OBJS += abspath.o\n-LIB_OBJS += advice.o\n-LIB_OBJS += alias.o\n-LIB_OBJS += alloc.o\n-LIB_OBJS += archive.o\n-LIB_OBJS += archive-tar.o\n-LIB_OBJS += archive-zip.o\n-LIB_OBJS += argv-array.o\n-LIB_OBJS += attr.o\n-LIB_OBJS += base85.o\n-LIB_OBJS += bisect.o\n-LIB_OBJS += blob.o\n-LIB_OBJS += branch.o\n-LIB_OBJS += bulk-checkin.o\n-LIB_OBJS += bundle.o\n-LIB_OBJS += cache-tree.o\n-LIB_OBJS += color.o\n-LIB_OBJS += column.o\n-LIB_OBJS += combine-diff.o\n-LIB_OBJS += commit.o\n-LIB_OBJS += compat/obstack.o\n-LIB_OBJS += compat/terminal.o\n-LIB_OBJS += config.o\n-LIB_OBJS += connect.o\n-LIB_OBJS += connected.o\n-LIB_OBJS += convert.o\n-LIB_OBJS += copy.o\n-LIB_OBJS += credential.o\n-LIB_OBJS += csum-file.o\n-LIB_OBJS += ctype.o\n-LIB_OBJS += date.o\n-LIB_OBJS += decorate.o\n-LIB_OBJS += diffcore-break.o\n-LIB_OBJS += diffcore-delta.o\n-LIB_OBJS += diffcore-order.o\n-LIB_OBJS += diffcore-pickaxe.o\n-LIB_OBJS += diffcore-rename.o\n-LIB_OBJS += diff-delta.o\n-LIB_OBJS += diff-lib.o\n-LIB_OBJS += diff-no-index.o\n-LIB_OBJS += diff.o\n-LIB_OBJS += dir.o\n-LIB_OBJS += editor.o\n-LIB_OBJS += entry.o\n-LIB_OBJS += environment.o\n-LIB_OBJS += exec_cmd.o\n-LIB_OBJS += fetch-pack.o\n-LIB_OBJS += fsck.o\n-LIB_OBJS += gettext.o\n-LIB_OBJS += gpg-interface.o\n-LIB_OBJS += graph.o\n-LIB_OBJS += grep.o\n-LIB_OBJS += hash.o\n-LIB_OBJS += help.o\n-LIB_OBJS += hex.o\n-LIB_OBJS += ident.o\n-LIB_OBJS += kwset.o\n-LIB_OBJS += levenshtein.o\n-LIB_OBJS += line-log.o\n-LIB_OBJS += line-range.o\n-LIB_OBJS += list-objects.o\n-LIB_OBJS += ll-merge.o\n-LIB_OBJS += lockfile.o\n-LIB_OBJS += log-tree.o\n-LIB_OBJS += mailmap.o\n-LIB_OBJS += match-trees.o\n-LIB_OBJS += merge.o\n-LIB_OBJS += merge-blobs.o\n-LIB_OBJS += merge-recursive.o\n-LIB_OBJS += mergesort.o\n-LIB_OBJS += name-hash.o\n-LIB_OBJS += notes.o\n-LIB_OBJS += notes-cache.o\n-LIB_OBJS += notes-merge.o\n-LIB_OBJS += object.o\n-LIB_OBJS += pack-check.o\n-LIB_OBJS += pack-revindex.o\n-LIB_OBJS += pack-write.o\n-LIB_OBJS += pager.o\n-LIB_OBJS += parse-options.o\n-LIB_OBJS += parse-options-cb.o\n-LIB_OBJS += patch-delta.o\n-LIB_OBJS += patch-ids.o\n-LIB_OBJS += path.o\n-LIB_OBJS += pathspec.o\n-LIB_OBJS += pkt-line.o\n-LIB_OBJS += preload-index.o\n-LIB_OBJS += pretty.o\n-LIB_OBJS += progress.o\n-LIB_OBJS += prompt.o\n-LIB_OBJS += quote.o\n-LIB_OBJS += reachable.o\n-LIB_OBJS += read-cache.o\n-LIB_OBJS += reflog-walk.o\n-LIB_OBJS += refs.o\n-LIB_OBJS += remote.o\n-LIB_OBJS += replace_object.o\n-LIB_OBJS += rerere.o\n-LIB_OBJS += resolve-undo.o\n-LIB_OBJS += revision.o\n-LIB_OBJS += run-command.o\n-LIB_OBJS += send-pack.o\n-LIB_OBJS += sequencer.o\n-LIB_OBJS += server-info.o\n-LIB_OBJS += setup.o\n-LIB_OBJS += sha1-array.o\n-LIB_OBJS += sha1-lookup.o\n-LIB_OBJS += sha1_file.o\n-LIB_OBJS += sha1_name.o\n-LIB_OBJS += shallow.o\n-LIB_OBJS += sideband.o\n-LIB_OBJS += sigchain.o\n-LIB_OBJS += strbuf.o\n-LIB_OBJS += streaming.o\n-LIB_OBJS += string-list.o\n-LIB_OBJS += submodule.o\n-LIB_OBJS += symlinks.o\n-LIB_OBJS += tag.o\n-LIB_OBJS += trace.o\n-LIB_OBJS += transport.o\n-LIB_OBJS += transport-helper.o\n-LIB_OBJS += tree-diff.o\n-LIB_OBJS += tree.o\n-LIB_OBJS += tree-walk.o\n-LIB_OBJS += unpack-trees.o\n-LIB_OBJS += url.o\n-LIB_OBJS += usage.o\n-LIB_OBJS += userdiff.o\n-LIB_OBJS += utf8.o\n-LIB_OBJS += varint.o\n-LIB_OBJS += version.o\n-LIB_OBJS += walker.o\n-LIB_OBJS += wildmatch.o\n-LIB_OBJS += wrapper.o\n-LIB_OBJS += write_or_die.o\n-LIB_OBJS += ws.o\n-LIB_OBJS += wt-status.o\n-LIB_OBJS += xdiff-interface.o\n-LIB_OBJS += zlib.o\n+HEADERS += advice.h\n+HEADERS += archive.h\n+HEADERS += argv-array.h\n+HEADERS += attr.h\n+HEADERS += bisect.h\n+HEADERS += blob.h\n+HEADERS += branch.h\n+HEADERS += builtin.h\n+HEADERS += bulk-checkin.h\n+HEADERS += bundle.h\n+HEADERS += cache-tree.h\n+HEADERS += cache.h\n+HEADERS += color.h\n+HEADERS += column.h\n+HEADERS += commit.h\n+HEADERS += compat/bswap.h\n+HEADERS += compat/cygwin.h\n+HEADERS += compat/mingw.h\n+HEADERS += compat/obstack.h\n+HEADERS += compat/poll/poll.h\n+HEADERS += compat/precompose_utf8.h\n+HEADERS += compat/terminal.h\n+HEADERS += compat/win32/dirent.h\n+HEADERS += compat/win32/pthread.h\n+HEADERS += compat/win32/syslog.h\n+HEADERS += connected.h\n+HEADERS += convert.h\n+HEADERS += credential.h\n+HEADERS += csum-file.h\n+HEADERS += decorate.h\n+HEADERS += delta.h\n+HEADERS += diff.h\n+HEADERS += diffcore.h\n+HEADERS += dir.h\n+HEADERS += exec_cmd.h\n+HEADERS += fetch-pack.h\n+HEADERS += fmt-merge-msg.h\n+HEADERS += fsck.h\n+HEADERS += gettext.h\n+HEADERS += git-compat-util.h\n+HEADERS += gpg-interface.h\n+HEADERS += graph.h\n+HEADERS += grep.h\n+HEADERS += hash.h\n+HEADERS += help.h\n+HEADERS += http.h\n+HEADERS += kwset.h\n+HEADERS += levenshtein.h\n+HEADERS += line-log.h\n+HEADERS += line-range.h\n+HEADERS += list-objects.h\n+HEADERS += ll-merge.h\n+HEADERS += log-tree.h\n+HEADERS += mailmap.h\n+HEADERS += merge-blobs.h\n+HEADERS += merge-recursive.h\n+HEADERS += mergesort.h\n+HEADERS += notes-cache.h\n+HEADERS += notes-merge.h\n+HEADERS += notes.h\n+HEADERS += object.h\n+HEADERS += pack-revindex.h\n+HEADERS += pack.h\n+HEADERS += parse-options.h\n+HEADERS += patch-ids.h\n+HEADERS += pathspec.h\n+HEADERS += pkt-line.h\n+HEADERS += progress.h\n+HEADERS += prompt.h\n+HEADERS += quote.h\n+HEADERS += reachable.h\n+HEADERS += reflog-walk.h\n+HEADERS += refs.h\n+HEADERS += remote.h\n+HEADERS += rerere.h\n+HEADERS += resolve-undo.h\n+HEADERS += revision.h\n+HEADERS += run-command.h\n+HEADERS += send-pack.h\n+HEADERS += sequencer.h\n+HEADERS += sha1-array.h\n+HEADERS += sha1-lookup.h\n+HEADERS += shortlog.h\n+HEADERS += sideband.h\n+HEADERS += sigchain.h\n+HEADERS += strbuf.h\n+HEADERS += streaming.h\n+HEADERS += string-list.h\n+HEADERS += submodule.h\n+HEADERS += tag.h\n+HEADERS += tar.h\n+HEADERS += thread-utils.h\n+HEADERS += transport.h\n+HEADERS += tree-walk.h\n+HEADERS += tree.h\n+HEADERS += unpack-trees.h\n+HEADERS += url.h\n+HEADERS += userdiff.h\n+HEADERS += utf8.h\n+HEADERS += varint.h\n+HEADERS += vcs-svn/fast_export.h\n+HEADERS += vcs-svn/line_buffer.h\n+HEADERS += vcs-svn/repo_tree.h\n+HEADERS += vcs-svn/sliding_window.h\n+HEADERS += vcs-svn/svndiff.h\n+HEADERS += vcs-svn/svndump.h\n+HEADERS += walker.h\n+HEADERS += wildmatch.h\n+HEADERS += wt-status.h\n+HEADERS += xdiff-interface.h\n+HEADERS += xdiff/xdiff.h\n+HEADERS += xdiff/xdiffi.h\n+HEADERS += xdiff/xemit.h\n+HEADERS += xdiff/xinclude.h\n+HEADERS += xdiff/xmacros.h\n+HEADERS += xdiff/xprepare.h\n+HEADERS += xdiff/xtypes.h\n+HEADERS += xdiff/xutils.h\n+\n+OBJS += abspath.o\n+OBJS += advice.o\n+OBJS += alias.o\n+OBJS += alloc.o\n+OBJS += archive.o\n+OBJS += archive-tar.o\n+OBJS += archive-zip.o\n+OBJS += argv-array.o\n+OBJS += attr.o\n+OBJS += base85.o\n+OBJS += bisect.o\n+OBJS += blob.o\n+OBJS += branch.o\n+OBJS += bulk-checkin.o\n+OBJS += bundle.o\n+OBJS += cache-tree.o\n+OBJS += color.o\n+OBJS += column.o\n+OBJS += combine-diff.o\n+OBJS += commit.o\n+OBJS += compat/obstack.o\n+OBJS += compat/terminal.o\n+OBJS += config.o\n+OBJS += connect.o\n+OBJS += connected.o\n+OBJS += convert.o\n+OBJS += copy.o\n+OBJS += credential.o\n+OBJS += csum-file.o\n+OBJS += ctype.o\n+OBJS += date.o\n+OBJS += decorate.o\n+OBJS += diffcore-break.o\n+OBJS += diffcore-delta.o\n+OBJS += diffcore-order.o\n+OBJS += diffcore-pickaxe.o\n+OBJS += diffcore-rename.o\n+OBJS += diff-delta.o\n+OBJS += diff-lib.o\n+OBJS += diff-no-index.o\n+OBJS += diff.o\n+OBJS += dir.o\n+OBJS += editor.o\n+OBJS += entry.o\n+OBJS += environment.o\n+OBJS += exec_cmd.o\n+OBJS += fetch-pack.o\n+OBJS += fsck.o\n+OBJS += gettext.o\n+OBJS += gpg-interface.o\n+OBJS += graph.o\n+OBJS += grep.o\n+OBJS += hash.o\n+OBJS += help.o\n+OBJS += hex.o\n+OBJS += ident.o\n+OBJS += kwset.o\n+OBJS += levenshtein.o\n+OBJS += line-log.o\n+OBJS += line-range.o\n+OBJS += list-objects.o\n+OBJS += ll-merge.o\n+OBJS += lockfile.o\n+OBJS += log-tree.o\n+OBJS += mailmap.o\n+OBJS += match-trees.o\n+OBJS += merge.o\n+OBJS += merge-blobs.o\n+OBJS += merge-recursive.o\n+OBJS += mergesort.o\n+OBJS += name-hash.o\n+OBJS += notes.o\n+OBJS += notes-cache.o\n+OBJS += notes-merge.o\n+OBJS += object.o\n+OBJS += pack-check.o\n+OBJS += pack-revindex.o\n+OBJS += pack-write.o\n+OBJS += pager.o\n+OBJS += parse-options.o\n+OBJS += parse-options-cb.o\n+OBJS += patch-delta.o\n+OBJS += patch-ids.o\n+OBJS += path.o\n+OBJS += pathspec.o\n+OBJS += pkt-line.o\n+OBJS += preload-index.o\n+OBJS += pretty.o\n+OBJS += progress.o\n+OBJS += prompt.o\n+OBJS += quote.o\n+OBJS += reachable.o\n+OBJS += read-cache.o\n+OBJS += reflog-walk.o\n+OBJS += refs.o\n+OBJS += remote.o\n+OBJS += replace_object.o\n+OBJS += rerere.o\n+OBJS += resolve-undo.o\n+OBJS += revision.o\n+OBJS += run-command.o\n+OBJS += send-pack.o\n+OBJS += sequencer.o\n+OBJS += server-info.o\n+OBJS += setup.o\n+OBJS += sha1-array.o\n+OBJS += sha1-lookup.o\n+OBJS += sha1_file.o\n+OBJS += sha1_name.o\n+OBJS += shallow.o\n+OBJS += sideband.o\n+OBJS += sigchain.o\n+OBJS += strbuf.o\n+OBJS += streaming.o\n+OBJS += string-list.o\n+OBJS += submodule.o\n+OBJS += symlinks.o\n+OBJS += tag.o\n+OBJS += trace.o\n+OBJS += transport.o\n+OBJS += transport-helper.o\n+OBJS += tree-diff.o\n+OBJS += tree.o\n+OBJS += tree-walk.o\n+OBJS += unpack-trees.o\n+OBJS += url.o\n+OBJS += usage.o\n+OBJS += userdiff.o\n+OBJS += utf8.o\n+OBJS += varint.o\n+OBJS += version.o\n+OBJS += walker.o\n+OBJS += wildmatch.o\n+OBJS += wrapper.o\n+OBJS += write_or_die.o\n+OBJS += ws.o\n+OBJS += wt-status.o\n+OBJS += xdiff-interface.o\n+OBJS += zlib.o\n \n BUILTIN_OBJS += builtin/add.o\n BUILTIN_OBJS += builtin/annotate.o\n@@ -990,7 +990,9 @@ BUILTIN_OBJS += builtin/verify-pack.o\n BUILTIN_OBJS += builtin/verify-tag.o\n BUILTIN_OBJS += builtin/write-tree.o\n \n-GITLIBS = $(LIB_FILE) $(XDIFF_LIB)\n+OBJS += $(BUILTIN_OBJS)\n+\n+GITLIBS = $(GIT_LIB) $(XDIFF_LIB)\n EXTLIBS =\n \n GIT_USER_AGENT = git/$(GIT_VERSION)\n@@ -1365,16 +1367,16 @@ else\n endif\n endif\n ifdef NO_INET_NTOP\n-\tLIB_OBJS += compat/inet_ntop.o\n+\tOBJS += compat/inet_ntop.o\n \tBASIC_CFLAGS += -DNO_INET_NTOP\n endif\n ifdef NO_INET_PTON\n-\tLIB_OBJS += compat/inet_pton.o\n+\tOBJS += compat/inet_pton.o\n \tBASIC_CFLAGS += -DNO_INET_PTON\n endif\n ifndef NO_UNIX_SOCKETS\n-\tLIB_OBJS += unix-socket.o\n-\tLIB_H += unix-socket.h\n+\tOBJS += unix-socket.o\n+\tHEADERS += unix-socket.h\n \tPROGRAM_OBJS += credential-cache.o\n \tPROGRAM_OBJS += credential-cache--daemon.o\n endif\n@@ -1397,13 +1399,13 @@ endif\n \n ifdef BLK_SHA1\n \tSHA1_HEADER = \"block-sha1/sha1.h\"\n-\tLIB_OBJS += block-sha1/sha1.o\n-\tLIB_H += block-sha1/sha1.h\n+\tOBJS += block-sha1/sha1.o\n+\tHEADERS += block-sha1/sha1.h\n else\n ifdef PPC_SHA1\n \tSHA1_HEADER = \"ppc/sha1.h\"\n-\tLIB_OBJS += ppc/sha1.o ppc/sha1ppc.o\n-\tLIB_H += ppc/sha1.h\n+\tOBJS += ppc/sha1.o ppc/sha1ppc.o\n+\tHEADERS += ppc/sha1.h\n else\n ifdef APPLE_COMMON_CRYPTO\n \tCOMPAT_CFLAGS += -DCOMMON_DIGEST_FOR_OPENSSL\n@@ -1442,7 +1444,7 @@ ifdef NO_PTHREADS\n else\n \tBASIC_CFLAGS += $(PTHREAD_CFLAGS)\n \tEXTLIBS += $(PTHREAD_LIBS)\n-\tLIB_OBJS += thread-utils.o\n+\tOBJS += thread-utils.o\n endif\n \n ifdef HAVE_PATHS_H\n@@ -1590,7 +1592,7 @@ LIBS = $(GITLIBS) $(EXTLIBS)\n \n BASIC_CFLAGS += -DSHA1_HEADER='$(SHA1_HEADER_SQ)' \\\n \t$(COMPAT_CFLAGS)\n-LIB_OBJS += $(COMPAT_OBJS)\n+OBJS += $(COMPAT_OBJS)\n \n # Quote for C\n \n@@ -1677,7 +1679,7 @@ strip: $(PROGRAMS) git$X\n \n # The generic compilation pattern rule and automatically\n # computed header dependencies (falling back to a dependency on\n-# LIB_H) are enough to describe how most targets should be built,\n+# HEADERS) are enough to describe how most targets should be built,\n # but some targets are special enough to need something a little\n # different.\n #\n@@ -1712,9 +1714,9 @@ git.sp git.s git.o: EXTRA_CPPFLAGS = \\\n \t'-DGIT_MAN_PATH=\"$(mandir_relative_SQ)\"' \\\n \t'-DGIT_INFO_PATH=\"$(infodir_relative_SQ)\"'\n \n-git$X: git.o GIT-LDFLAGS $(BUILTIN_OBJS) $(GITLIBS)\n+git$X: git.o GIT-LDFLAGS $(GITLIBS)\n \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ git.o \\\n-\t\t$(BUILTIN_OBJS) $(ALL_LDFLAGS) $(LIBS)\n+\t\t$(ALL_LDFLAGS) $(LIBS)\n \n help.sp help.s help.o: common-cmds.h\n \n@@ -1892,7 +1894,7 @@ VCSSVN_OBJS += vcs-svn/svndiff.o\n VCSSVN_OBJS += vcs-svn/svndump.o\n \n TEST_OBJS := $(patsubst test-%$X,test-%.o,$(TEST_PROGRAMS))\n-OBJECTS := $(LIB_OBJS) $(BUILTIN_OBJS) $(PROGRAM_OBJS) $(TEST_OBJS) \\\n+OBJECTS := $(OBJS) $(PROGRAM_OBJS) $(TEST_OBJS) \\\n \t$(XDIFF_OBJS) \\\n \t$(VCSSVN_OBJS) \\\n \tgit.o\n@@ -1998,7 +2000,7 @@ else\n # should _not_ be included here, since they are necessary even when\n # building an object for the first time.\n \n-$(OBJECTS): $(LIB_H)\n+$(OBJECTS): $(HEADERS)\n endif\n \n exec_cmd.sp exec_cmd.s exec_cmd.o: GIT-PREFIX\n@@ -2066,8 +2068,8 @@ $(REMOTE_CURL_PRIMARY): remote-curl.o http.o http-walker.o GIT-LDFLAGS $(GITLIBS\n \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \\\n \t\t$(LIBS) $(CURL_LIBCURL) $(EXPAT_LIBEXPAT)\n \n-$(LIB_FILE): $(LIB_OBJS)\n-\t$(QUIET_AR)$(RM) $@ && $(AR) rcs $@ $(LIB_OBJS)\n+$(GIT_LIB): $(OBJS)\n+\t$(QUIET_AR)$(RM) $@ && $(AR) rcs $@ $(OBJS)\n \n $(XDIFF_LIB): $(XDIFF_OBJS)\n \t$(QUIET_AR)$(RM) $@ && $(AR) rcs $@ $(XDIFF_OBJS)\n@@ -2102,7 +2104,7 @@ XGETTEXT_FLAGS_C = $(XGETTEXT_FLAGS) --language=C \\\n XGETTEXT_FLAGS_SH = $(XGETTEXT_FLAGS) --language=Shell \\\n \t--keyword=gettextln --keyword=eval_gettextln\n XGETTEXT_FLAGS_PERL = $(XGETTEXT_FLAGS) --keyword=__ --language=Perl\n-LOCALIZED_C := $(C_OBJ:o=c) $(LIB_H) $(GENERATED_H)\n+LOCALIZED_C := $(C_OBJ:o=c) $(HEADERS) $(GENERATED_H)\n LOCALIZED_SH := $(SCRIPT_SH)\n LOCALIZED_PERL := $(SCRIPT_PERL)\n \n@@ -2480,7 +2482,7 @@ profile-clean:\n \n clean: profile-clean coverage-clean\n \t$(RM) *.o *.res block-sha1/*.o ppc/*.o compat/*.o compat/*/*.o xdiff/*.o vcs-svn/*.o \\\n-\t\tbuiltin/*.o $(LIB_FILE) $(XDIFF_LIB) $(VCSSVN_LIB)\n+\t\tbuiltin/*.o $(GIT_LIB) $(XDIFF_LIB) $(VCSSVN_LIB)\n \t$(RM) $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) git$X\n \t$(RM) $(TEST_PROGRAMS)\n \t$(RM) -r bin-wrappers $(dep_dirs)\n-- \n1.8.3.698.g079b096\n"},{"id":"219849","messageId":"CALkWK0mA7MXQv1k5bFpZLARDOHxU5kzKFXzcyUfb6NLZZY-=FA@mail.gmail.com","threadId":"34068","inReplyTo":"1370712574-27688-1-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH] build: get rid of the notion of a git library","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-08T18:02:09Z","receivedAt":"2013-06-08T18:02:09Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Felipe Contreras wrote:\n> There's no libgit, and there will never be, every object file in Git is\n> the same, and there's wish to organize them in any way; they are *all*\n> for the 'git' binary and its builtin commands.\n\nNice joke patch to illustrate your point ;)\n\nOn a more serious note, please be a little more patient while everyone\ncopes with what you're attempting. I've already made it clear that\nI'm in favor of moving forward with your plan to lib'ify git. The\nproblem is that you're sending your changes in fragmented comments and\ndiffs, and nobody is able to piece together what the big picture is.\n\nPlease write one cogent email (preferably with code included)\nexplaining your plan.\n"},{"id":"219850","messageId":"CAMP44s0cozMsTo7KQAjnqkqmvMwMw9D3SZrVxg48MOXkH9UQJQ@mail.gmail.com","threadId":"34068","inReplyTo":"CALkWK0mA7MXQv1k5bFpZLARDOHxU5kzKFXzcyUfb6NLZZY-=FA@mail.gmail.com","subject":"Re: [PATCH] build: get rid of the notion of a git library","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-06-08T18:22:41Z","receivedAt":"2013-06-08T18:22:41Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sat, Jun 8, 2013 at 1:02 PM, Ramkumar Ramachandra wrote:\n> Felipe Contreras wrote:\n>> There's no libgit, and there will never be, every object file in Git is\n>> the same, and there's wish to organize them in any way; they are *all*\n>> for the 'git' binary and its builtin commands.\n>\n> Nice joke patch to illustrate your point ;)\n\nIt's not a joke. This is seriously the direction the others say is the\ncorrect one.\n\nOne direction or the other, the problem that top-level objects can't\naccess code from builtin objects must be fixed. And if the others\ndon't want to fix it by properly splitting code between library-like\nobjects, and builtin objects, there's only one other way to fix it;\nthis way.\n\n> On a more serious note, please be a little more patient while everyone\n> copes with what you're attempting.\n\nI don't think patience will help. What do you suggest? Wait until the\nproblem fixes itself? (I'll be waiting until the end times). Wait\nuntil somebody changed their opinion by themselves? (I don't see that\nhappening).\n\n> I've already made it clear that\n> I'm in favor of moving forward with your plan to lib'ify git.\n\nUnfortunately you are the only one.\n\n> The\n> problem is that you're sending your changes in fragmented comments and\n> diffs, and nobody is able to piece together what the big picture is.\n>\n> Please write one cogent email (preferably with code included)\n> explaining your plan.\n\nThe plan is simple; make libgit.a a proper library, starting by\nclarifying what goes into libgit.a, and what doesn't. If there's any\nhopes of ever having a public library, it's clear what code doesn't\nbelong in libgit.a; code that is meant for builtins, that code belongs\nin builtins/lib.a, or similar.\n\nBut to be honest, I don't really care, all I want is the problem of\nthe bogus split to be solved. One way to solve it is going the proper\nlibrary way, but the other is to stash everything together into git.a.\nBoth ways solve the problem.\n\nGive this a try:\n\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -14,6 +14,7 @@\n #include \"merge-recursive.h\"\n #include \"refs.h\"\n #include \"argv-array.h\"\n+#include \"builtin.h\"\n\n #define GIT_REFLOG_ACTION \"GIT_REFLOG_ACTION\"\n\n@@ -102,6 +103,17 @@ static int has_conforming_footer(struct strbuf\n*sb, struct strbuf *sob,\n return 1;\n }\n\n+static void copy_notes(const char *name)\n+{\n+ struct notes_rewrite_cfg *cfg;\n+\n+ cfg = init_copy_notes_for_rewrite(name);\n+ if (!cfg)\n+ return;\n+\n+ finish_copy_notes_for_rewrite(cfg);\n+}\n+\n static void remove_sequencer_state(void)\n {\n struct strbuf seq_dir = STRBUF_INIT;\n@@ -997,6 +1009,8 @@ static int pick_commits(struct commit_list\n*todo_list, struct replay_opts *opts)\n return res;\n }\n\n+ copy_notes(\"cherry-pick\");\n+\n /*\n * Sequence of picks finished successfully; cleanup by\n * removing the .git/sequencer directory\n\nWhat happens?\n\nlibgit.a(sequencer.o): In function `copy_notes':\n/home/felipec/dev/git/sequencer.c:110: undefined reference to\n`init_copy_notes_for_rewrite'\n/home/felipec/dev/git/sequencer.c:114: undefined reference to\n`finish_copy_notes_for_rewrite'\n\nIt is not the first time, nor the last that top-level code needs\nbuiltin code, and the solution is easy; organize the code. Alas, this\nsimple solution reject on the basis that we shouldn't organize the\ncode, because the code is not meant to be organized.\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"219962","messageId":"CALkWK0=7PRndNc7XQ-PCPbVCp9vck909bA561JhQG6uXXj1n4g@mail.gmail.com","threadId":"34068","inReplyTo":"CAMP44s0cozMsTo7KQAjnqkqmvMwMw9D3SZrVxg48MOXkH9UQJQ@mail.gmail.com","subject":"Re: [PATCH] build: get rid of the notion of a git library","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-09T14:56:32Z","receivedAt":"2013-06-09T14:56:32Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Felipe Contreras wrote:\n> The plan is simple; make libgit.a a proper library, starting by\n> clarifying what goes into libgit.a, and what doesn't. If there's any\n> hopes of ever having a public library, it's clear what code doesn't\n> belong in libgit.a; code that is meant for builtins, that code belongs\n> in builtins/lib.a, or similar.\n>\n> Give this a try:\n>\n> --- a/sequencer.c\n> +++ b/sequencer.c\n>\n> libgit.a(sequencer.o): In function `copy_notes':\n> /home/felipec/dev/git/sequencer.c:110: undefined reference to\n> `init_copy_notes_for_rewrite'\n> /home/felipec/dev/git/sequencer.c:114: undefined reference to\n> `finish_copy_notes_for_rewrite'\n\nThis is a good example: yes, I'm convinced that the code does need to\nbe reorganized. Please resend your {sequencer.c ->\nbuiltin/sequencer.c} patch with this example as the rationale, and\nlet's work towards improving libgit.a.\n"},{"id":"219963","messageId":"20130609151235.GA22905@serenity.lan","threadId":"34068","inReplyTo":"CALkWK0=7PRndNc7XQ-PCPbVCp9vck909bA561JhQG6uXXj1n4g@mail.gmail.com","subject":"Re: [PATCH] build: get rid of the notion of a git library","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2013-06-09T15:12:35Z","receivedAt":"2013-06-09T15:12:35Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"On Sun, Jun 09, 2013 at 08:26:32PM +0530, Ramkumar Ramachandra wrote:\n> Felipe Contreras wrote:\n> > The plan is simple; make libgit.a a proper library, starting by\n> > clarifying what goes into libgit.a, and what doesn't. If there's any\n> > hopes of ever having a public library, it's clear what code doesn't\n> > belong in libgit.a; code that is meant for builtins, that code belongs\n> > in builtins/lib.a, or similar.\n> >\n> > Give this a try:\n> >\n> > --- a/sequencer.c\n> > +++ b/sequencer.c\n> >\n> > libgit.a(sequencer.o): In function `copy_notes':\n> > /home/felipec/dev/git/sequencer.c:110: undefined reference to\n> > `init_copy_notes_for_rewrite'\n> > /home/felipec/dev/git/sequencer.c:114: undefined reference to\n> > `finish_copy_notes_for_rewrite'\n> \n> This is a good example: yes, I'm convinced that the code does need to\n> be reorganized. Please resend your {sequencer.c ->\n> builtin/sequencer.c} patch with this example as the rationale, and\n> let's work towards improving libgit.a.\n\nWhy should sequencer.c move into builtin/ to solve this? Why not pull\ninit_copy_notes_for_rewrite and finish_copy_notes_for_rewrite up into\nnotes.c?\n"},{"id":"219965","messageId":"CAMP44s0L9nQxp5OeK8uT4Ls5WUerCjVpR9uONUcOwvTD6k7Jfg@mail.gmail.com","threadId":"34068","inReplyTo":"20130609151235.GA22905@serenity.lan","subject":"Re: [PATCH] build: get rid of the notion of a git library","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-06-09T15:40:32Z","receivedAt":"2013-06-09T15:40:32Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sun, Jun 9, 2013 at 10:12 AM, John Keeping wrote:\n> On Sun, Jun 09, 2013 at 08:26:32PM +0530, Ramkumar Ramachandra wrote:\n>> Felipe Contreras wrote:\n>> > The plan is simple; make libgit.a a proper library, starting by\n>> > clarifying what goes into libgit.a, and what doesn't. If there's any\n>> > hopes of ever having a public library, it's clear what code doesn't\n>> > belong in libgit.a; code that is meant for builtins, that code belongs\n>> > in builtins/lib.a, or similar.\n>> >\n>> > Give this a try:\n>> >\n>> > --- a/sequencer.c\n>> > +++ b/sequencer.c\n>> >\n>> > libgit.a(sequencer.o): In function `copy_notes':\n>> > /home/felipec/dev/git/sequencer.c:110: undefined reference to\n>> > `init_copy_notes_for_rewrite'\n>> > /home/felipec/dev/git/sequencer.c:114: undefined reference to\n>> > `finish_copy_notes_for_rewrite'\n>>\n>> This is a good example: yes, I'm convinced that the code does need to\n>> be reorganized. Please resend your {sequencer.c ->\n>> builtin/sequencer.c} patch with this example as the rationale, and\n>> let's work towards improving libgit.a.\n>\n> Why should sequencer.c move into builtin/ to solve this? Why not pull\n> init_copy_notes_for_rewrite and finish_copy_notes_for_rewrite up into\n> notes.c?\n\nBecause finish_copy_notes_for_rewrite is only useful for builtin\ncommands, so it belongs in builtin/. If there's any meaning to the\n./*.o vs. builtin/*.o divide, it's for that. Otherwise we should just\nsquash all objects into libgit.a and be done with it.\n\n-- \nFelipe Contreras\n"},{"id":"219966","messageId":"CAMP44s1PkL51QQ2T7qC3uVcBGv7ftarazR+LgJtV4X0yVAcj2A@mail.gmail.com","threadId":"34068","inReplyTo":"CALkWK0=7PRndNc7XQ-PCPbVCp9vck909bA561JhQG6uXXj1n4g@mail.gmail.com","subject":"Re: [PATCH] build: get rid of the notion of a git library","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-06-09T15:41:24Z","receivedAt":"2013-06-09T15:41:24Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sun, Jun 9, 2013 at 9:56 AM, Ramkumar Ramachandra wrote:\n> Felipe Contreras wrote:\n>> The plan is simple; make libgit.a a proper library, starting by\n>> clarifying what goes into libgit.a, and what doesn't. If there's any\n>> hopes of ever having a public library, it's clear what code doesn't\n>> belong in libgit.a; code that is meant for builtins, that code belongs\n>> in builtins/lib.a, or similar.\n>>\n>> Give this a try:\n>>\n>> --- a/sequencer.c\n>> +++ b/sequencer.c\n>>\n>> libgit.a(sequencer.o): In function `copy_notes':\n>> /home/felipec/dev/git/sequencer.c:110: undefined reference to\n>> `init_copy_notes_for_rewrite'\n>> /home/felipec/dev/git/sequencer.c:114: undefined reference to\n>> `finish_copy_notes_for_rewrite'\n>\n> This is a good example: yes, I'm convinced that the code does need to\n> be reorganized. Please resend your {sequencer.c ->\n> builtin/sequencer.c} patch with this example as the rationale, and\n> let's work towards improving libgit.a.\n\nI already explained this multiple times; code from ./*.o can't access\ncode from ./builtin/*.o.\n\n-- \nFelipe Contreras\n"},{"id":"219967","messageId":"20130609160225.GB22905@serenity.lan","threadId":"34068","inReplyTo":"CAMP44s0L9nQxp5OeK8uT4Ls5WUerCjVpR9uONUcOwvTD6k7Jfg@mail.gmail.com","subject":"Re: [PATCH] build: get rid of the notion of a git library","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2013-06-09T16:02:25Z","receivedAt":"2013-06-09T16:02:25Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"On Sun, Jun 09, 2013 at 10:40:32AM -0500, Felipe Contreras wrote:\n> On Sun, Jun 9, 2013 at 10:12 AM, John Keeping wrote:\n> > On Sun, Jun 09, 2013 at 08:26:32PM +0530, Ramkumar Ramachandra wrote:\n> >> Felipe Contreras wrote:\n> >> > The plan is simple; make libgit.a a proper library, starting by\n> >> > clarifying what goes into libgit.a, and what doesn't. If there's any\n> >> > hopes of ever having a public library, it's clear what code doesn't\n> >> > belong in libgit.a; code that is meant for builtins, that code belongs\n> >> > in builtins/lib.a, or similar.\n> >> >\n> >> > Give this a try:\n> >> >\n> >> > --- a/sequencer.c\n> >> > +++ b/sequencer.c\n> >> >\n> >> > libgit.a(sequencer.o): In function `copy_notes':\n> >> > /home/felipec/dev/git/sequencer.c:110: undefined reference to\n> >> > `init_copy_notes_for_rewrite'\n> >> > /home/felipec/dev/git/sequencer.c:114: undefined reference to\n> >> > `finish_copy_notes_for_rewrite'\n> >>\n> >> This is a good example: yes, I'm convinced that the code does need to\n> >> be reorganized. Please resend your {sequencer.c ->\n> >> builtin/sequencer.c} patch with this example as the rationale, and\n> >> let's work towards improving libgit.a.\n> >\n> > Why should sequencer.c move into builtin/ to solve this? Why not pull\n> > init_copy_notes_for_rewrite and finish_copy_notes_for_rewrite up into\n> > notes.c?\n> \n> Because finish_copy_notes_for_rewrite is only useful for builtin\n> commands, so it belongs in builtin/. If there's any meaning to the\n> ./*.o vs. builtin/*.o divide, it's for that. Otherwise we should just\n> squash all objects into libgit.a and be done with it.\n\nHow is it only useful for builtin commands? By that logic everything\nbelongs in builtin/ because it's all only used by builtin commands (I\nrealise that is what you're arguing towards).\n\nBut we make a distinction between things that are specific to one\ncommand (especially argument parsing and user interaction) and more\ngenerally useful features. Copying notes around in the notes tree is\ngenerally useful so why shouldn't it be in notes.c with the other note\nmanipulation functions? The current API may not be completely suitable\nbut that doesn't mean that it cannot be extracted into notes.c. In\nfact, other than the commit message saying \"Notes added by 'git notes\ncopy'\" I don't see what's wrong with the current functions being\nextracted as-is.\n"},{"id":"219968","messageId":"CAMP44s0Zsejk4Ex6QfzOFOom3cyWv_hziWGkAK-LawSUkT9V3Q@mail.gmail.com","threadId":"34068","inReplyTo":"20130609160225.GB22905@serenity.lan","subject":"Re: [PATCH] build: get rid of the notion of a git library","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-06-09T16:22:06Z","receivedAt":"2013-06-09T16:22:06Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sun, Jun 9, 2013 at 11:02 AM, John Keeping wrote:\n> On Sun, Jun 09, 2013 at 10:40:32AM -0500, Felipe Contreras wrote:\n>> On Sun, Jun 9, 2013 at 10:12 AM, John Keeping wrote:\n>> > On Sun, Jun 09, 2013 at 08:26:32PM +0530, Ramkumar Ramachandra wrote:\n>> >> Felipe Contreras wrote:\n>> >> > The plan is simple; make libgit.a a proper library, starting by\n>> >> > clarifying what goes into libgit.a, and what doesn't. If there's any\n>> >> > hopes of ever having a public library, it's clear what code doesn't\n>> >> > belong in libgit.a; code that is meant for builtins, that code belongs\n>> >> > in builtins/lib.a, or similar.\n>> >> >\n>> >> > Give this a try:\n>> >> >\n>> >> > --- a/sequencer.c\n>> >> > +++ b/sequencer.c\n>> >> >\n>> >> > libgit.a(sequencer.o): In function `copy_notes':\n>> >> > /home/felipec/dev/git/sequencer.c:110: undefined reference to\n>> >> > `init_copy_notes_for_rewrite'\n>> >> > /home/felipec/dev/git/sequencer.c:114: undefined reference to\n>> >> > `finish_copy_notes_for_rewrite'\n>> >>\n>> >> This is a good example: yes, I'm convinced that the code does need to\n>> >> be reorganized. Please resend your {sequencer.c ->\n>> >> builtin/sequencer.c} patch with this example as the rationale, and\n>> >> let's work towards improving libgit.a.\n>> >\n>> > Why should sequencer.c move into builtin/ to solve this? Why not pull\n>> > init_copy_notes_for_rewrite and finish_copy_notes_for_rewrite up into\n>> > notes.c?\n>>\n>> Because finish_copy_notes_for_rewrite is only useful for builtin\n>> commands, so it belongs in builtin/. If there's any meaning to the\n>> ./*.o vs. builtin/*.o divide, it's for that. Otherwise we should just\n>> squash all objects into libgit.a and be done with it.\n>\n> How is it only useful for builtin commands? By that logic everything\n> belongs in builtin/ because it's all only used by builtin commands (I\n> realise that is what you're arguing towards).\n\nWhich is precisely the point of this patch. If everything is for\nbuiltin commands, then we don't have a git library, and git.a should\ncontain everything under builtin/*.o.\n\n> But we make a distinction between things that are specific to one\n> command (especially argument parsing and user interaction) and more\n> generally useful features.\n\nNo, we don't. Everything under ./*.o goes to libgit.a, and everything\nunder ./builtin/*.o goes to 'git'. So builtin/commit.o can access code\nfrom builtin/notes.o, but sequencer.o can't.\n\n-- \nFelipe Contreras\n"},{"id":"219970","messageId":"CALkWK0koyDS3m2B9fdnwv5e_gT3zp9y+MB9TwVN4kHEr9HkRZw@mail.gmail.com","threadId":"34068","inReplyTo":"20130609160225.GB22905@serenity.lan","subject":"Re: [PATCH] build: get rid of the notion of a git library","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-09T16:36:54Z","receivedAt":"2013-06-09T16:36:54Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"John Keeping wrote:\n> How is it only useful for builtin commands? By that logic everything\n> belongs in builtin/ because it's all only used by builtin commands (I\n> realise that is what you're arguing towards).\n\nsequencer.c is merely a common API for builtins to invoke\n\"continuations\": i.e. stop the program persisting enough state to let\nto user continue, allow the user to do whatever conflict resolutions\nusing whatever tools, allow the user to continue the original\noperation. sequencer.c provides a uniform UI\n(--continue|--skip|--abort), and a uniform way to persist state\n(.git/sequencer/todo). It mainly abstracts out the boring details of\nreading/writing the todo lines.\n\nCurrently, sequencer.c has no callers other than those in\nbuiltin/revert.c. In its current shape, it's incapable of being used\nby anything else: while an external ruby script (possibly a rebase\nimplementation) could call into the sequencer to persist state, I\ndon't think it is going to happen anytime soon.\n\nWe might get a proper public api sequencer sometime in the distant\nfuture, but don't confuse that with the current shape of sequencer.c.\n\n> But we make a distinction between things that are specific to one\n> command (especially argument parsing and user interaction) and more\n> generally useful features. Copying notes around in the notes tree is\n> generally useful so why shouldn't it be in notes.c with the other note\n> manipulation functions? The current API may not be completely suitable\n> but that doesn't mean that it cannot be extracted into notes.c. In\n> fact, other than the commit message saying \"Notes added by 'git notes\n> copy'\" I don't see what's wrong with the current functions being\n> extracted as-is.\n\nSure, notes could have a better public api and so could a lot of other\nthings: worktree operations like reset and checkout come to mind.\n\nThe problem is that we seem to be at some frozen in some sort of\nstalemate, and some reorganization is definitely required. What would\nyou suggest?\n"},{"id":"220016","messageId":"20130609164248.GD22905@serenity.lan","threadId":"34068","inReplyTo":"CAMP44s0Zsejk4Ex6QfzOFOom3cyWv_hziWGkAK-LawSUkT9V3Q@mail.gmail.com","subject":"Re: [PATCH] build: get rid of the notion of a git library","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2013-06-09T16:42:49Z","receivedAt":"2013-06-09T16:42:49Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"On Sun, Jun 09, 2013 at 11:22:06AM -0500, Felipe Contreras wrote:\n> On Sun, Jun 9, 2013 at 11:02 AM, John Keeping wrote:\n> > But we make a distinction between things that are specific to one\n> > command (especially argument parsing and user interaction) and more\n> > generally useful features.\n> \n> No, we don't. Everything under ./*.o goes to libgit.a, and everything\n> under ./builtin/*.o goes to 'git'. So builtin/commit.o can access code\n> from builtin/notes.o, but sequencer.o can't.\n\nI would argue that it was a mistake not to extract these functions from\nbuiltin/notes.c to notes.c when builtin/commit.c started using them.\nCalling across from one builtin/*.c file to another is just as wrong as\ncalling into a builtin/*.c file from a top-level file but the build\nsystem happens not to enforce the former.\n"},{"id":"220020","messageId":"CALkWK0k=39-Cq3vNdrpLPTWa0wgkqLM=7c=cAmL0nvx0MT5mkA@mail.gmail.com","threadId":"34068","inReplyTo":"20130609164248.GD22905@serenity.lan","subject":"Re: [PATCH] build: get rid of the notion of a git library","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-09T17:03:52Z","receivedAt":"2013-06-09T17:03:52Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"John Keeping wrote:\n> Calling across from one builtin/*.c file to another is just as wrong as\n> calling into a builtin/*.c file from a top-level file but the build\n> system happens not to enforce the former.\n\nSo libgit.a is a collection of everything that is shared between\nbuiltins? Does that correspond to reality?\n\n $ ls *.h | sed 's/.h$/.c/' | xargs file\n\nAn example violation: builtin/log.c uses functions defined in\nbuiltin/shortlog.c.\n\nWhat is the point of all this separation, if no external scripts are\never going to use libgit.a?\n"},{"id":"220024","messageId":"CALkWK0=CW+tTRy0oPFCQpV6a0VnWQb6SUKtSPaj+4JoeG+J6uw@mail.gmail.com","threadId":"34068","inReplyTo":"CALkWK0k=39-Cq3vNdrpLPTWa0wgkqLM=7c=cAmL0nvx0MT5mkA@mail.gmail.com","subject":"Re: [PATCH] build: get rid of the notion of a git library","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-09T17:12:41Z","receivedAt":"2013-06-09T17:12:41Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Ramkumar Ramachandra wrote:\n> So libgit.a is a collection of everything that is shared between\n> builtins?\n\nThat is not to say that we shouldn't share things between builtins.\nWe can do it in builtin/lib.a, as Felipe has demonstrated here [1].\n\n[1]: http://article.gmane.org/gmane.comp.version-control.git/226975\n"},{"id":"220025","messageId":"CAMP44s1PENiKMy03_mgZ_myiGP5+qpaE2bvo0LN3X3mZhSvT2g@mail.gmail.com","threadId":"34068","inReplyTo":"CALkWK0k=39-Cq3vNdrpLPTWa0wgkqLM=7c=cAmL0nvx0MT5mkA@mail.gmail.com","subject":"Re: [PATCH] build: get rid of the notion of a git library","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-06-09T17:13:41Z","receivedAt":"2013-06-09T17:13:41Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sun, Jun 9, 2013 at 12:03 PM, Ramkumar Ramachandra\n wrote:\n> John Keeping wrote:\n>> Calling across from one builtin/*.c file to another is just as wrong as\n>> calling into a builtin/*.c file from a top-level file but the build\n>> system happens not to enforce the former.\n>\n> So libgit.a is a collection of everything that is shared between\n> builtins? Does that correspond to reality?\n>\n> $ ls *.h | sed 's/.h$/.c/' | xargs file\n>\n> An example violation: builtin/log.c uses functions defined in\n> builtin/shortlog.c.\n>\n> What is the point of all this separation, if no external scripts are\n> ever going to use libgit.a?\n\nAnd all the functions should be static, which doesn't seem to be the case:\n\n00000000000003c0 T add_files_to_cache\n0000000000000530 T interactive_add\n0000000000000410 T run_add_interactive\n0000000000001920 T textconv_object\n00000000000005b0 T fmt_merge_msg\n0000000000000090 T fmt_merge_msg_config\n0000000000000c00 T init_db\n0000000000000b40 T set_git_dir_init\n0000000000000360 T overlay_tree_on_cache\n0000000000000500 T report_path_error\n00000000000011a0 T copy_note_for_rewrite\n0000000000001210 T finish_copy_notes_for_rewrite\n0000000000001060 T init_copy_notes_for_rewrite\n0000000000000000 T prune_packed_objects\n0000000000000510 T shortlog_add_commit\n00000000000006b0 T shortlog_init\n0000000000000780 T shortlog_output\n0000000000000000 T stripspace\n\n-- \nFelipe Contreras\n"},{"id":"220037","messageId":"51B4BBB7.8060807@lyx.org","threadId":"34068","inReplyTo":"CAMP44s0L9nQxp5OeK8uT4Ls5WUerCjVpR9uONUcOwvTD6k7Jfg@mail.gmail.com","subject":"Re: [PATCH] build: get rid of the notion of a git library","fromName":"Vincent van Ravesteijn","fromEmail":"vfr@lyx.org","sentAt":"2013-06-09T17:30:31Z","receivedAt":"2013-06-09T17:30:31Z","isPatch":true,"sender":{"key":"vfr@lyx.org","avatar":"https://avatars.githubusercontent.com/u/687868?v=4"},"body":"Op 9-6-2013 17:40, Felipe Contreras schreef:\n> On Sun, Jun 9, 2013 at 10:12 AM, John Keeping wrote:\n>> On Sun, Jun 09, 2013 at 08:26:32PM +0530, Ramkumar Ramachandra wrote:\n>>> Felipe Contreras wrote:\n>>>> The plan is simple; make libgit.a a proper library, starting by\n>>>> clarifying what goes into libgit.a, and what doesn't. If there's any\n>>>> hopes of ever having a public library, it's clear what code doesn't\n>>>> belong in libgit.a; code that is meant for builtins, that code belongs\n>>>> in builtins/lib.a, or similar.\n>>>>\n>>>> Give this a try:\n>>>>\n>>>> --- a/sequencer.c\n>>>> +++ b/sequencer.c\n>>>>\n>>>> libgit.a(sequencer.o): In function `copy_notes':\n>>>> /home/felipec/dev/git/sequencer.c:110: undefined reference to\n>>>> `init_copy_notes_for_rewrite'\n>>>> /home/felipec/dev/git/sequencer.c:114: undefined reference to\n>>>> `finish_copy_notes_for_rewrite'\n>>> This is a good example: yes, I'm convinced that the code does need to\n>>> be reorganized. Please resend your {sequencer.c ->\n>>> builtin/sequencer.c} patch with this example as the rationale, and\n>>> let's work towards improving libgit.a.\n>> Why should sequencer.c move into builtin/ to solve this? Why not pull\n>> init_copy_notes_for_rewrite and finish_copy_notes_for_rewrite up into\n>> notes.c?\n> Because finish_copy_notes_for_rewrite is only useful for builtin\n> commands, so it belongs in builtin/. If there's any meaning to the\n> ./*.o vs. builtin/*.o divide, it's for that. Otherwise we should just\n> squash all objects into libgit.a and be done with it.\n>\nI think that libgit.a should contain all code to be able to carry out \nall functions of git. The stuff in builtin/ is just a command-line user \ninterface. Whether or not sequencer should be in builtin depends on \nwhether the sequencer is only part of this command-line user interface.\n\nI think that the sequencer code is at the moment unusable if you do not \nuse the code in builtin/ so that would advocate to move it into \nbuiltin/. If sequencer is in libgit, and I write my own (graphical) user \ninterface, I expect to be able to use it.\n\nVincent\n"},{"id":"220040","messageId":"20130609173257.GE22905@serenity.lan","threadId":"34068","inReplyTo":"CAMP44s1PENiKMy03_mgZ_myiGP5+qpaE2bvo0LN3X3mZhSvT2g@mail.gmail.com","subject":"Re: [PATCH] build: get rid of the notion of a git library","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2013-06-09T17:32:58Z","receivedAt":"2013-06-09T17:32:58Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"On Sun, Jun 09, 2013 at 12:13:41PM -0500, Felipe Contreras wrote:\n> On Sun, Jun 9, 2013 at 12:03 PM, Ramkumar Ramachandra\n> wrote:\n> > John Keeping wrote:\n> >> Calling across from one builtin/*.c file to another is just as wrong as\n> >> calling into a builtin/*.c file from a top-level file but the build\n> >> system happens not to enforce the former.\n> >\n> > So libgit.a is a collection of everything that is shared between\n> > builtins? Does that correspond to reality?\n\nI think that's *precisely* what libgit.a is. It doesn't currently\ncorrespond exactly to reality, but that's mostly for historic reasons\n(see below).\n\n> > $ ls *.h | sed 's/.h$/.c/' | xargs file\n> >\n> > An example violation: builtin/log.c uses functions defined in\n> > builtin/shortlog.c.\n> >\n> > What is the point of all this separation, if no external scripts are\n> > ever going to use libgit.a?\n\nWhy do we structure code in a certain way at all? The reason libgit.a\nwas introduced (according to commit 0a02ce7) is:\n\n This introduces the concept of git \"library\" objects that\n the real programs use, and makes it easier to add such\n things to a \"libgit.a\".\n\n> And all the functions should be static, which doesn't seem to be the case:\n> \n> 00000000000003c0 T add_files_to_cache\n> 0000000000000530 T interactive_add\n> 0000000000000410 T run_add_interactive\n> 0000000000001920 T textconv_object\n> 00000000000005b0 T fmt_merge_msg\n> 0000000000000090 T fmt_merge_msg_config\n> 0000000000000c00 T init_db\n> 0000000000000b40 T set_git_dir_init\n> 0000000000000360 T overlay_tree_on_cache\n> 0000000000000500 T report_path_error\n> 00000000000011a0 T copy_note_for_rewrite\n> 0000000000001210 T finish_copy_notes_for_rewrite\n> 0000000000001060 T init_copy_notes_for_rewrite\n> 0000000000000000 T prune_packed_objects\n> 0000000000000510 T shortlog_add_commit\n> 00000000000006b0 T shortlog_init\n> 0000000000000780 T shortlog_output\n> 0000000000000000 T stripspace\n\nA quick check with \"git log -S...\" shows that most of these have barely\nbeen touched since the builtin/ directory was created. So the reason\nthey're not static is most likely because no one has tidied them up\nsince the division between builtins was introduced.\n\nIt is a fact of life that as we live and work with a system we realise\nthat there may be a better way of doing something. This doesn't mean\nthat someone needs to immediately convert everything to the new way,\nit is often sufficient to do new things in the new way and slowly move\nexisting things across as and when they are touched for other reasons.\n"},{"id":"220042","messageId":"CAMP44s2Sg3D4kjXM2v0_kU+Y_OeTMbiEtbSWcLQj1bWuRPOxhw@mail.gmail.com","threadId":"34068","inReplyTo":"51B4BBB7.8060807@lyx.org","subject":"Re: [PATCH] build: get rid of the notion of a git library","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-06-09T17:35:04Z","receivedAt":"2013-06-09T17:35:04Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sun, Jun 9, 2013 at 12:30 PM, Vincent van Ravesteijn wrote:\n> Op 9-6-2013 17:40, Felipe Contreras schreef:\n>\n>> On Sun, Jun 9, 2013 at 10:12 AM, John Keeping wrote:\n>>>\n>>> On Sun, Jun 09, 2013 at 08:26:32PM +0530, Ramkumar Ramachandra wrote:\n>>>>\n>>>> Felipe Contreras wrote:\n>>>>>\n>>>>> The plan is simple; make libgit.a a proper library, starting by\n>>>>> clarifying what goes into libgit.a, and what doesn't. If there's any\n>>>>> hopes of ever having a public library, it's clear what code doesn't\n>>>>> belong in libgit.a; code that is meant for builtins, that code belongs\n>>>>> in builtins/lib.a, or similar.\n>>>>>\n>>>>> Give this a try:\n>>>>>\n>>>>> --- a/sequencer.c\n>>>>> +++ b/sequencer.c\n>>>>>\n>>>>> libgit.a(sequencer.o): In function `copy_notes':\n>>>>> /home/felipec/dev/git/sequencer.c:110: undefined reference to\n>>>>> `init_copy_notes_for_rewrite'\n>>>>> /home/felipec/dev/git/sequencer.c:114: undefined reference to\n>>>>> `finish_copy_notes_for_rewrite'\n>>>>\n>>>> This is a good example: yes, I'm convinced that the code does need to\n>>>> be reorganized. Please resend your {sequencer.c ->\n>>>> builtin/sequencer.c} patch with this example as the rationale, and\n>>>> let's work towards improving libgit.a.\n>>>\n>>> Why should sequencer.c move into builtin/ to solve this? Why not pull\n>>> init_copy_notes_for_rewrite and finish_copy_notes_for_rewrite up into\n>>> notes.c?\n>>\n>> Because finish_copy_notes_for_rewrite is only useful for builtin\n>> commands, so it belongs in builtin/. If there's any meaning to the\n>> ./*.o vs. builtin/*.o divide, it's for that. Otherwise we should just\n>> squash all objects into libgit.a and be done with it.\n>>\n> I think that libgit.a should contain all code to be able to carry out all\n> functions of git. The stuff in builtin/ is just a command-line user\n> interface. Whether or not sequencer should be in builtin depends on whether\n> the sequencer is only part of this command-line user interface.\n\nThe sequencer is only part of the command-line user interface.\n\n> I think that the sequencer code is at the moment unusable if you do not use\n> the code in builtin/ so that would advocate to move it into builtin/. If\n> sequencer is in libgit, and I write my own (graphical) user interface, I\n> expect to be able to use it.\n\nAs do I, but it appears all other Git developers disagree; libgit.a is\nnot really a library, and will never be used by anything other than\nGit's core.\n\nHence this patch.\n\n-- \nFelipe Contreras\n"},{"id":"220049","messageId":"CAMP44s2h1Oj=qFkrsH9L4cZ0VZYbRHbo4eqmDxoTe36HiHXsxQ@mail.gmail.com","threadId":"34068","inReplyTo":"20130609173257.GE22905@serenity.lan","subject":"Re: [PATCH] build: get rid of the notion of a git library","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-06-09T17:45:07Z","receivedAt":"2013-06-09T17:45:07Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sun, Jun 9, 2013 at 12:32 PM, John Keeping wrote:\n> On Sun, Jun 09, 2013 at 12:13:41PM -0500, Felipe Contreras wrote:\n>> On Sun, Jun 9, 2013 at 12:03 PM, Ramkumar Ramachandra\n>> wrote:\n>> > John Keeping wrote:\n>> >> Calling across from one builtin/*.c file to another is just as wrong as\n>> >> calling into a builtin/*.c file from a top-level file but the build\n>> >> system happens not to enforce the former.\n>> >\n>> > So libgit.a is a collection of everything that is shared between\n>> > builtins? Does that correspond to reality?\n>\n> I think that's *precisely* what libgit.a is.\n\nWe don't care what libgit.a is; the important thing is what it *should* be.\n\n> A quick check with \"git log -S...\" shows that most of these have barely\n> been touched since the builtin/ directory was created. So the reason\n> they're not static is most likely because no one has tidied them up\n> since the division between builtins was introduced.\n>\n> It is a fact of life that as we live and work with a system we realise\n> that there may be a better way of doing something. This doesn't mean\n> that someone needs to immediately convert everything to the new way,\n> it is often sufficient to do new things in the new way and slowly move\n> existing things across as and when they are touched for other reasons.\n\nReally?\n\nbuiltin/ls-files.c:307:13: error: static declaration of\n‘overlay_tree_on_cache’ follows non-static declaration\n static void overlay_tree_on_cache(const char *tree_name, const char *prefix)\n ^\nIn file included from builtin/ls-files.c:8:0:\n./cache.h:1318:6: note: previous declaration of ‘overlay_tree_on_cache’ was here\n void overlay_tree_on_cache(const char *tree_name, const char *prefix);\n ^\nbuiltin/ls-files.c:361:12: error: static declaration of\n‘report_path_error’ follows non-static declaration\n static int report_path_error(const char *ps_matched, const char\n**pathspec, const char *prefix)\n ^\nIn file included from builtin/ls-files.c:8:0:\n./cache.h:1317:5: note: previous declaration of ‘report_path_error’ was here\n int report_path_error(const char *ps_matched, const char **pathspec,\nconst char *prefix);\n ^\nmake: *** [builtin/ls-files.o] Error 1\nmake: *** Waiting for unfinished jobs....\nbuiltin/add.c:184:12: error: static declaration of\n‘add_files_to_cache’ follows non-static declaration\n static int add_files_to_cache(const char *prefix, const char\n**pathspec, int flags)\n ^\nIn file included from builtin/add.c:6:0:\n./cache.h:1283:5: note: previous declaration of ‘add_files_to_cache’ was here\n int add_files_to_cache(const char *prefix, const char **pathspec, int flags);\n ^\nbuiltin/add.c:280:12: error: static declaration of\n‘run_add_interactive’ follows non-static declaration\n static int run_add_interactive(const char *revision, const char *patch_mode,\n ^\nIn file included from ./builtin.h:7:0,\n from builtin/add.c:7:\n./commit.h:187:12: note: previous declaration of ‘run_add_interactive’ was here\n extern int run_add_interactive(const char *revision, const char *patch_mode,\n ^\nbuiltin/add.c:309:12: error: static declaration of ‘interactive_add’\nfollows non-static declaration\n static int interactive_add(int argc, const char **argv, const char\n*prefix, int patch)\n ^\nIn file included from ./builtin.h:7:0,\n from builtin/add.c:7:\n./commit.h:186:12: note: previous declaration of ‘interactive_add’ was here\n extern int interactive_add(int argc, const char **argv, const char\n*prefix, int patch);\n ^\nbuiltin/add.c:184:12: warning: ‘add_files_to_cache’ defined but not\nused [-Wunused-function]\n static int add_files_to_cache(const char *prefix, const char\n**pathspec, int flags)\n ^\nmake: *** [builtin/add.o] Error 1\nbuiltin/fmt-merge-msg.c:19:12: error: static declaration of\n‘fmt_merge_msg_config’ follows non-static declaration\n static int fmt_merge_msg_config(const char *key, const char *value, void *cb)\n ^\nIn file included from builtin/fmt-merge-msg.c:9:0:\n./fmt-merge-msg.h:5:12: note: previous declaration of\n‘fmt_merge_msg_config’ was here\n extern int fmt_merge_msg_config(const char *key, const char *value, void *cb);\n ^\nbuiltin/fmt-merge-msg.c:592:12: error: static declaration of\n‘fmt_merge_msg’ follows non-static declaration\n static int fmt_merge_msg(struct strbuf *in, struct strbuf *out,\n ^\nIn file included from builtin/fmt-merge-msg.c:1:0:\n./builtin.h:26:12: note: previous declaration of ‘fmt_merge_msg’ was here\n extern int fmt_merge_msg(struct strbuf *in, struct strbuf *out,\n ^\nmake: *** [builtin/fmt-merge-msg.o] Error 1\nbuiltin/init-db.c:316:12: error: static declaration of\n‘set_git_dir_init’ follows non-static declaration\n static int set_git_dir_init(const char *git_dir, const char *real_git_dir,\n ^\nIn file included from builtin/init-db.c:6:0:\n./cache.h:421:12: note: previous declaration of ‘set_git_dir_init’ was here\n extern int set_git_dir_init(const char *git_dir, const char\n*real_git_dir, int);\n ^\nbuiltin/init-db.c:368:12: error: static declaration of ‘init_db’\nfollows non-static declaration\n static int init_db(const char *template_dir, unsigned int flags)\n ^\nIn file included from builtin/init-db.c:6:0:\n./cache.h:422:12: note: previous declaration of ‘init_db’ was here\n extern int init_db(const char *template_dir, unsigned int flags);\n ^\nmake: *** [builtin/init-db.o] Error 1\nbuiltin/shortlog.c:110:13: error: static declaration of\n‘shortlog_add_commit’ follows non-static declaration\n static void shortlog_add_commit(struct shortlog *log, struct commit *commit)\n ^\nIn file included from builtin/shortlog.c:9:0:\n./shortlog.h:24:6: note: previous declaration of ‘shortlog_add_commit’ was here\n void shortlog_add_commit(struct shortlog *log, struct commit *commit);\n ^\nbuiltin/shortlog.c:207:13: error: static declaration of\n‘shortlog_init’ follows non-static declaration\n static void shortlog_init(struct shortlog *log)\n ^\nIn file included from builtin/shortlog.c:9:0:\n./shortlog.h:22:6: note: previous declaration of ‘shortlog_init’ was here\n void shortlog_init(struct shortlog *log);\n ^\nbuiltin/shortlog.c:287:13: error: static declaration of\n‘shortlog_output’ follows non-static declaration\n static void shortlog_output(struct shortlog *log)\n ^\nIn file included from builtin/shortlog.c:9:0:\n./shortlog.h:26:6: note: previous declaration of ‘shortlog_output’ was here\n void shortlog_output(struct shortlog *log);\n ^\nbuiltin/shortlog.c:287:13: warning: ‘shortlog_output’ defined but not\nused [-Wunused-function]\n static void shortlog_output(struct shortlog *log)\n ^\nmake: *** [builtin/shortlog.o] Error 1\nbuiltin/stripspace.c:36:13: error: static declaration of ‘stripspace’\nfollows non-static declaration\n static void stripspace(struct strbuf *sb, int skip_comments)\n ^\nIn file included from ./builtin.h:5:0,\n from builtin/stripspace.c:1:\n./strbuf.h:165:13: note: previous declaration of ‘stripspace’ was here\n extern void stripspace(struct strbuf *buf, int skip_comments);\n ^\nmake: *** [builtin/stripspace.o] Error 1\nmake[2]: `GIT-VERSION-FILE' is up to date.\nbuiltin/prune-packed.c:37:13: error: static declaration of\n‘prune_packed_objects’ follows non-static declaration\n static void prune_packed_objects(int opts)\n ^\nIn file included from builtin/prune-packed.c:1:0:\n./builtin.h:18:13: note: previous declaration of ‘prune_packed_objects’ was here\n extern void prune_packed_objects(int);\n ^\nmake: *** [builtin/prune-packed.o] Error 1\nbuiltin/notes.c:358:34: error: static declaration of\n‘init_copy_notes_for_rewrite’ follows non-static declaration\n static struct notes_rewrite_cfg *init_copy_notes_for_rewrite(const char *cmd)\n ^\nIn file included from builtin/notes.c:11:0:\n./builtin.h:39:27: note: previous declaration of\n‘init_copy_notes_for_rewrite’ was here\n struct notes_rewrite_cfg *init_copy_notes_for_rewrite(const char *cmd);\n ^\nbuiltin/notes.c:396:12: error: static declaration of\n‘copy_note_for_rewrite’ follows non-static declaration\n static int copy_note_for_rewrite(struct notes_rewrite_cfg *c,\n ^\nIn file included from builtin/notes.c:11:0:\n./builtin.h:40:5: note: previous declaration of ‘copy_note_for_rewrite’ was here\n int copy_note_for_rewrite(struct notes_rewrite_cfg *c,\n ^\nbuiltin/notes.c:406:13: error: static declaration of\n‘finish_copy_notes_for_rewrite’ follows non-static declaration\n static void finish_copy_notes_for_rewrite(struct notes_rewrite_cfg *c)\n ^\nIn file included from builtin/notes.c:11:0:\n./builtin.h:42:6: note: previous declaration of\n‘finish_copy_notes_for_rewrite’ was here\n void finish_copy_notes_for_rewrite(struct notes_rewrite_cfg *c);\n ^\nmake: *** [builtin/notes.o] Error 1\nbuiltin/blame.c:112:12: error: static declaration of ‘textconv_object’\nfollows non-static declaration\n static int textconv_object(const char *path,\n ^\nIn file included from builtin/blame.c:8:0:\n./builtin.h:44:12: note: previous declaration of ‘textconv_object’ was here\n extern int textconv_object(const char *path, unsigned mode, const\nunsigned char *sha1, int sha1_valid, char **buf, unsigned long\n*buf_size);\n ^\nmake: *** [builtin/blame.o] Error 1\n\n-- \nFelipe Contreras\n"},{"id":"220388","messageId":"20130610214504.GG13333@sigill.intra.peff.net","threadId":"34068","inReplyTo":"51B4BBB7.8060807@lyx.org","subject":"Re: [PATCH] build: get rid of the notion of a git library","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-06-10T21:45:04Z","receivedAt":"2013-06-10T21:45:04Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Jun 09, 2013 at 07:30:31PM +0200, Vincent van Ravesteijn wrote:\n\n> I think that libgit.a should contain all code to be able to carry out\n> all functions of git. The stuff in builtin/ is just a command-line\n> user interface. Whether or not sequencer should be in builtin depends\n> on whether the sequencer is only part of this command-line user\n> interface.\n\nOne code organization issue I have not seen mentioned is that there is\nmore CLI than what is in builtin, and libgit.a does more than simply\nprovide code for the sources in builtin/. There are also external\ncommands shipped in git.git that are not linked against git.c or the\nother builtins.\n\nOnce upon a time, all commands were that way, and that was the origin of\nlibgit.a: the set of common code used by all of the C commands in\ngit.git. Over time, those commands became builtins (mostly to keep the\nsize of the libexec dir down). These days there are only a handful of\nexternal commands left, mostly ones that have startup time overhead from\nthe dynamic loader (e.g., remote-curl, http-push, imap-send).\n\nThat is what libgit.a _is_ now. I do not mean to imply any additional\njudgement on what it could be. But if the goal is to make libgit.a\n\"functions that programs outside git.git would want, and nothing else\",\nwe may want to additionally split out a \"libgit-internal.a\" consisting\nof code that is used by multiple externals in git, but which would not\nbe appropriate for clients to use.\n\nFor example, I think most of \"http.c\" is in that boat, as it is full of\nwrappers for curl code that are of enough quality to reuse within git,\nbut a little too half-baked to be part of a stable API.\n\nWe can always link directly against http.o, too, of course. The point of\nputting the files into a static library is that it makes the link\nfaster, and if there are only a handful of such links, it may not be\nworth the effort.\n\n-Peff\n"},{"id":"220392","messageId":"CAMP44s2-94LTu54oX1_m14tnE3KfwK+N=pPxgUSqGCgd51EA5A@mail.gmail.com","threadId":"34068","inReplyTo":"20130610214504.GG13333@sigill.intra.peff.net","subject":"Re: [PATCH] build: get rid of the notion of a git library","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-06-10T21:52:57Z","receivedAt":"2013-06-10T21:52:57Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Mon, Jun 10, 2013 at 4:45 PM, Jeff King wrote:\n\n> That is what libgit.a _is_ now. I do not mean to imply any additional\n> judgement on what it could be. But if the goal is to make libgit.a\n> \"functions that programs outside git.git would want, and nothing else\",\n> we may want to additionally split out a \"libgit-internal.a\" consisting\n> of code that is used by multiple externals in git, but which would not\n> be appropriate for clients to use.\n\nThat might make sense, but that still doesn't clarify what belongs in\n./*.o, and what belongs in ./builtin/*.o. And right now that creates a\nmess where you have code shared between ./builtin/*.o that is defined\nin cache.h (overlay_tree_on_cache), and some in builtin.h\n(init_copy_notes_for_rewrite). And it's not clear what should be done\nwhen code in ./*.o needs to access functionality in ./builtin/*.o,\nspecially if that code is only useful for git builtins, and nothing\nelse.\n\n-- \nFelipe Contreras\n"},{"id":"220394","messageId":"20130610220627.GB28345@sigill.intra.peff.net","threadId":"34068","inReplyTo":"CAMP44s2-94LTu54oX1_m14tnE3KfwK+N=pPxgUSqGCgd51EA5A@mail.gmail.com","subject":"Re: [PATCH] build: get rid of the notion of a git library","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-06-10T22:06:27Z","receivedAt":"2013-06-10T22:06:27Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jun 10, 2013 at 04:52:57PM -0500, Felipe Contreras wrote:\n\n> On Mon, Jun 10, 2013 at 4:45 PM, Jeff King wrote:\n> \n> > That is what libgit.a _is_ now. I do not mean to imply any additional\n> > judgement on what it could be. But if the goal is to make libgit.a\n> > \"functions that programs outside git.git would want, and nothing else\",\n> > we may want to additionally split out a \"libgit-internal.a\" consisting\n> > of code that is used by multiple externals in git, but which would not\n> > be appropriate for clients to use.\n> \n> That might make sense, but that still doesn't clarify what belongs in\n> ./*.o, and what belongs in ./builtin/*.o. And right now that creates a\n> mess where you have code shared between ./builtin/*.o that is defined\n> in cache.h (overlay_tree_on_cache), and some in builtin.h\n> (init_copy_notes_for_rewrite). And it's not clear what should be done\n> when code in ./*.o needs to access functionality in ./builtin/*.o,\n> specially if that code is only useful for git builtins, and nothing\n> else.\n\nMy general impression of the goal of our current code organization is:\n\n 1. builtin/*.c should each contain a single builtin command and its\n supporting static functions. Each file gets linked into git.o to\n make the \"main\" git executable.\n\n 2. ./*.c is one of:\n\n a. Shared code usable by externals and builtins, which gets\n linked into libgit.a\n\n b. An external command itself, with its own main(). It gets\n linked against libgit.a.\n\n 3. Functions in libgit.a should be defined in a header file specific\n to their module (e.g., refs.h). cache.h picks up the slack for\n things that are general, or too small to get their own header file,\n or otherwise don't group well.\n\nI said it was a \"goal\", because I know that we do not follow that in\nmany places, so it is certainly easy to find counter-examples (and nor\ndo I think it cannot be changed; I am just trying to describe the\ncurrent rationale). Under that organization, there is no place for \"code\nthat does not go into libgit.a, but is not a builtin command in itself\".\nThere was never a need in the past, because libgit.a was a bit of a\ndumping ground for linkable functions, and nobody cared that it had\neverything and the kitchen sink.\n\nIf we want to start caring, then we probably need to create a separate\n\"kitchen sink\"-like library, with the rule that things in libgit.a\ncannot depend on it. In other words, a support library for Git's\ncommands, for the parts that are not appropriate to expose as part of a\nlibrary API.\n\n-Peff\n"},{"id":"220395","messageId":"CAMP44s0H4ET_Bfc0tFuxSagFO+ycq_U3RY65fqGsqh=0Y-YKPw@mail.gmail.com","threadId":"34068","inReplyTo":"20130610220627.GB28345@sigill.intra.peff.net","subject":"Re: [PATCH] build: get rid of the notion of a git library","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-06-10T22:22:00Z","receivedAt":"2013-06-10T22:22:00Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Mon, Jun 10, 2013 at 5:06 PM, Jeff King wrote:\n> On Mon, Jun 10, 2013 at 04:52:57PM -0500, Felipe Contreras wrote:\n>\n>> On Mon, Jun 10, 2013 at 4:45 PM, Jeff King wrote:\n>>\n>> > That is what libgit.a _is_ now. I do not mean to imply any additional\n>> > judgement on what it could be. But if the goal is to make libgit.a\n>> > \"functions that programs outside git.git would want, and nothing else\",\n>> > we may want to additionally split out a \"libgit-internal.a\" consisting\n>> > of code that is used by multiple externals in git, but which would not\n>> > be appropriate for clients to use.\n>>\n>> That might make sense, but that still doesn't clarify what belongs in\n>> ./*.o, and what belongs in ./builtin/*.o. And right now that creates a\n>> mess where you have code shared between ./builtin/*.o that is defined\n>> in cache.h (overlay_tree_on_cache), and some in builtin.h\n>> (init_copy_notes_for_rewrite). And it's not clear what should be done\n>> when code in ./*.o needs to access functionality in ./builtin/*.o,\n>> specially if that code is only useful for git builtins, and nothing\n>> else.\n>\n> My general impression of the goal of our current code organization is:\n>\n> 1. builtin/*.c should each contain a single builtin command and its\n> supporting static functions. Each file gets linked into git.o to\n> make the \"main\" git executable.\n\nWe already know this is not the case. Maybe this should be fixed by\nmoving all the shared code between builtins to libgit.a, but maybe we\nalready know at some level this is not wise, and that's why we haven't\ndone so.\n\n> If we want to start caring, then we probably need to create a separate\n> \"kitchen sink\"-like library, with the rule that things in libgit.a\n> cannot depend on it. In other words, a support library for Git's\n> commands, for the parts that are not appropriate to expose as part of a\n> library API.\n\nYes, that's clearly what we should be doing, which is precisely what\nmy patch that creates builtin/lib.a does.\n\nSo we have two options:\n\na) Do what we clearly should do; create builtin/lib.a, and move code\nthere that is specific to builtin commands.\n\nb) Do what we think we have been doing; and move _all_ shared code to\nlibgit.a (which shouldn't be called libgit, because it's not really a\nlibrary), and cleanup builtin/*.c so they don't share anything among\nthemselves directly.\n\nI vote for a), not only because we already know that's what we\n_should_ do, but because we are basically already there.\n\nLeaving things as they are is not really an option; that's a mess.\n\n-- \nFelipe Contreras\n"},{"id":"220399","messageId":"7vk3m1efda.fsf@alter.siamese.dyndns.org","threadId":"34068","inReplyTo":"20130610220627.GB28345@sigill.intra.peff.net","subject":"Re: [PATCH] build: get rid of the notion of a git library","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-10T23:05:21Z","receivedAt":"2013-06-10T23:05:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> My general impression of the goal of our current code organization is:\n>\n> 1. builtin/*.c should each contain a single builtin command and its\n> supporting static functions. Each file gets linked into git.o to\n> make the \"main\" git executable.\n\nCorrect; that is what we aimed for when we made builtin-*.c (later\nmoved to builtin/*.c). Some builtin/*.c files can contain more than\none cmd_foo implementations, so \"single\" is not a solid rule, and it\ndoes not have to be, because all of them are expected to be linked\ninto the main binary together with git.c to be called from main().\n\nAnd as you hinted, if some global data or functions in it turns out\nto be useful for standalone binaries, their definitions must migrate\nout of buitlin/*.c to ./*.c files, because standalone binaries with\ntheir own main() definition cannot be linked with builtin/*.o, the\nlatter of which requires to be linked with git.o with its own main().\n\n> 2. ./*.c is one of:\n>\n> a. Shared code usable by externals and builtins, which gets\n> linked into libgit.a\n>\n> b. An external command itself, with its own main(). It gets\n> linked against libgit.a.\n>\n> 3. Functions in libgit.a should be defined in a header file specific\n> to their module (e.g., refs.h). cache.h picks up the slack for\n> things that are general, or too small to get their own header file,\n> or otherwise don't group well.\n>\n> I said it was a \"goal\", because I know that we do not follow that in\n> many places, so it is certainly easy to find counter-examples (and nor\n> do I think it cannot be changed; I am just trying to describe the\n> current rationale). Under that organization, there is no place for \"code\n> that does not go into libgit.a, but is not a builtin command in itself\".\n> There was never a need in the past, because libgit.a was a bit of a\n> dumping ground for linkable functions, and nobody cared that it had\n> everything and the kitchen sink.\n\nThe rationale behind libgit.a was so that make targets for the\nstandalone binaries (note: all of them were standalone in the\nbeginning) do not have to list *.o files that each of them needs to\nbe linked with. It was primary done as a convenient way to have the\nlinker figure out the dependency and link only what was needed.\n"},{"id":"220401","messageId":"7v8v2hedou.fsf@alter.siamese.dyndns.org","threadId":"34068","inReplyTo":"7vk3m1efda.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] build: get rid of the notion of a git library","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-10T23:41:37Z","receivedAt":"2013-06-10T23:41:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Jeff King writes:\n>\n>> My general impression of the goal of our current code organization is:\n>>\n>> 1. builtin/*.c should each contain a single builtin command and its\n>> supporting static functions. Each file gets linked into git.o to\n>> make the \"main\" git executable.\n>\n> Correct; that is what we aimed for when we made builtin-*.c (later\n> moved to builtin/*.c). Some builtin/*.c files can contain more than\n> one cmd_foo implementations, so \"single\" is not a solid rule, and it\n> does not have to be, because all of them are expected to be linked\n> into the main binary together with git.c to be called from main().\n>\n> And as you hinted, if some global data or functions in it turns out\n> to be useful for standalone binaries, their definitions must migrate\n> out of buitlin/*.c to ./*.c files, because standalone binaries with\n> their own main() definition cannot be linked with builtin/*.o, the\n> latter of which requires to be linked with git.o with its own main().\n> ...\n> The rationale behind libgit.a was so that make targets for the\n> standalone binaries (note: all of them were standalone in the\n> beginning) do not have to list *.o files that each of them needs to\n> be linked with. It was primary done as a convenient way to have the\n> linker figure out the dependency and link only what was needed.\n\nFor the particular case of trying to make sequencer.o, which does\nnot currently have dependencies on builtin/*.o, depend on something\nthat is in builtin/notes.o, the link phase of standalone that wants\nanything from revision.o (which is pretty much everything ;-) goes\nlike this:\n\n upload-pack.c wants handle_revision_opt etc.\n revision.c provides handle_revision_opt\n wants name_decoration etc.\n log-tree.c provides name_decoration\n wants append_signoff\n sequencer.c provides append_signoff\n\nSo sequencer.o _is_ meant to be usable from standalone and belongs\nto libgit.a\n\nIf sequencer.o wants to call init_copy_notes_for_rewrite() and its\nfriends [*1*] that are currently in builtin/notes.o, first the\ncalled function(s) should be moved outside builtin/notes.o to\nnotes.o or somewhere more library-ish place to be included in\nlibgit.a, which is meant to be usable from standalone.\n\n\n[Footnote]\n\n*1* ... which is a very reasonable thing to do. But moving\n sequencer.o to builtin/sequencer.o is *not* the way to do this.\n"},{"id":"220403","messageId":"CAMP44s1HM0zFvkGmaHrX2Wq2JSzDNk8uwNSz3bNo12eWxDcL8A@mail.gmail.com","threadId":"34068","inReplyTo":"7v8v2hedou.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] build: get rid of the notion of a git library","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-06-10T23:51:42Z","receivedAt":"2013-06-10T23:51:42Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Mon, Jun 10, 2013 at 6:41 PM, Junio C Hamano wrote:\n\n> For the particular case of trying to make sequencer.o, which does\n> not currently have dependencies on builtin/*.o, depend on something\n> that is in builtin/notes.o, the link phase of standalone that wants\n> anything from revision.o (which is pretty much everything ;-) goes\n> like this:\n>\n> upload-pack.c wants handle_revision_opt etc.\n> revision.c provides handle_revision_opt\n> wants name_decoration etc.\n> log-tree.c provides name_decoration\n> wants append_signoff\n> sequencer.c provides append_signoff\n>\n> So sequencer.o _is_ meant to be usable from standalone and belongs\n> to libgit.a\n\nNot after my patch. It moves append_signoff *out* of sequencer, which\nin fact has nothing to do with the sequencer in the first place.\n\n> If sequencer.o wants to call init_copy_notes_for_rewrite() and its\n> friends [*1*] that are currently in builtin/notes.o, first the\n> called function(s) should be moved outside builtin/notes.o to\n> notes.o or somewhere more library-ish place to be included in\n> libgit.a, which is meant to be usable from standalone.\n>\n>\n> [Footnote]\n>\n> *1* ... which is a very reasonable thing to do. But moving\n> sequencer.o to builtin/sequencer.o is *not* the way to do this.\n\nBy now we all know what is the *CURRENT* way to do this; in other\nwords, the status quo, which is BTW all messed up, because builtin/*.o\nobjects depend on each other already.\n\nWe are discussing the way it *SHOULD* be. Why aren't you leaning on that?\n\n-- \nFelipe Contreras\n"},{"id":"220404","messageId":"7v4nd5ecmy.fsf@alter.siamese.dyndns.org","threadId":"34068","inReplyTo":"CAMP44s1HM0zFvkGmaHrX2Wq2JSzDNk8uwNSz3bNo12eWxDcL8A@mail.gmail.com","subject":"Re: [PATCH] build: get rid of the notion of a git library","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-11T00:04:21Z","receivedAt":"2013-06-11T00:04:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n>> *1* ... which is a very reasonable thing to do. But moving\n>> sequencer.o to builtin/sequencer.o is *not* the way to do this.\n>\n> By now we all know what is the *CURRENT* way to do this; in other\n> words, the status quo, which is BTW all messed up, because builtin/*.o\n> objects depend on each other already.\n\nbuiltin/*.o are allowed to depend on each other. They are by\ndefinition builtins, meant to be linked into a single binary.\n\n> We are discussing the way it *SHOULD* be. Why aren't you leaning on that?\n\nAnd I do not see the reason why builtin/*.o should not depend on\neach other. It is not messed up at all. They are meant to be\nlinked into a single binary---that is what being \"built-in\" is.\n\nA good way forward, the way it *SHOULD* be, is to slim the builtin/*.o\nby moving parts that do not have to be in the single \"git\" binary\nbut are also usable in standalone binaries out of them.\n\nAnd that is what I just suggested.\n"},{"id":"220406","messageId":"7vwqq1ct0g.fsf@alter.siamese.dyndns.org","threadId":"34068","inReplyTo":"7v4nd5ecmy.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] build: get rid of the notion of a git library","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-11T01:53:35Z","receivedAt":"2013-06-11T01:53:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> And I do not see the reason why builtin/*.o should not depend on\n> each other. It is not messed up at all. They are meant to be\n> linked into a single binary---that is what being \"built-in\" is.\n>\n> A good way forward, the way it *SHOULD* be, is to slim the builtin/*.o\n> by moving parts that do not have to be in the single \"git\" binary\n> but are also usable in standalone binaries out of them.\n\nActually, as long as these pieces are currently used by builtins,\nmoving them (e.g. init_copy_notes_for_rewrite()) out of builtin/*.o \nwill not make these parts not to be in the single \"git\" binary at\nall, so the above is grossly misstated.\n\n - There may be pieces of usefully reusable code buried in\n builtin/*.o;\n\n - By definition, any code (piece of data or function definition) in\n builtin/*.o cannot be used in standalone binaries, because all of\n builtin/*.o expect to link with git.o and expect their cmd_foo()\n getting called from main in it;\n\n - By moving the useful reusable pieces ont of builtin/*.o and\n adding them to libgit.a, these pieces become usable from\n standalone binaries as well.\n\nAnd that is the reason why slimming builtin/*.o is the way it\n*SHOULD* be.\n\nAnother thing to think about is looking at pieces of data and\nfunctions defined in each *.o files and moving things around within\nthem. For example, looking at the dependency chain I quoted earlier\nfor sequencer.o to build upload-pack, which is about responding to\n\"git fetch\" on the sending side:\n\n upload-pack.c wants handle_revision_opt etc.\n revision.c provides handle_revision_opt\n wants name_decoration etc.\n log-tree.c provides name_decoration\n wants append_signoff\n sequencer.c provides append_signoff\n\nIt is already crazy. There is no reason for the pack sender to be\nlinking with the sequencer interpreter machinery. If the function\ndefinition (and possibly other ones) are split into separate source\nfiles (still in libgit.a), git-upload-pack binary does not have to\npull in the whole sequencer.c at all.\n\nComing back to the categorization Peff earlier made in the thread, I\nthink I am OK with adding new two subdirectories to the root level,\ni.e.\n\n builtin/\t- the ones that define cmd_foo()\n commands/ - the ones that has main() for standalone commands\n libsrc/ - the ones that go in libgit.a\n\nWe may also want to add another subdirectory to hold scripted\nPorcelains, but the primary topic of this thread is what to do about\nthe C library, so it is orthogonal in that sense, but if we were to\ngo in the \"group things in subdirectories to slim the root level\"\ndirection, it may be worth considering doing so at the same time.\n"},{"id":"220408","messageId":"CAMP44s03iXPVnunBdFT8etvZ-ew-D15A7mCV3wAAFXMNCpRAgA@mail.gmail.com","threadId":"34068","inReplyTo":"7v4nd5ecmy.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] build: get rid of the notion of a git library","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-06-11T04:04:52Z","receivedAt":"2013-06-11T04:04:52Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Mon, Jun 10, 2013 at 7:04 PM, Junio C Hamano wrote:\n> Felipe Contreras writes:\n>\n>>> *1* ... which is a very reasonable thing to do. But moving\n>>> sequencer.o to builtin/sequencer.o is *not* the way to do this.\n>>\n>> By now we all know what is the *CURRENT* way to do this; in other\n>> words, the status quo, which is BTW all messed up, because builtin/*.o\n>> objects depend on each other already.\n>\n> builtin/*.o are allowed to depend on each other. They are by\n> definition builtins, meant to be linked into a single binary.\n\nThat's not what John Keeping said[1]. I'm going to assume he was\nwrong, but I don't think that's relevant for my point.\n\nEither way, the meaning of builtin/ should probably be explained somewhere.\n\n>> We are discussing the way it *SHOULD* be. Why aren't you leaning on that?\n>\n> And I do not see the reason why builtin/*.o should not depend on\n> each other. It is not messed up at all. They are meant to be\n> linked into a single binary---that is what being \"built-in\" is.\n>\n> A good way forward, the way it *SHOULD* be, is to slim the builtin/*.o\n> by moving parts that do not have to be in the single \"git\" binary\n> but are also usable in standalone binaries out of them.\n>\n> And that is what I just suggested.\n\nBut init_copy_notes_for_rewrite() can *not* be used by anything other\nthan git builtins. Standalone binaries will never use such a function,\ntherefore it doesn't belong in libgit.a. Another example is\nalias_lookup(). They belong in builtin/lib.a.\n\n[1] http://article.gmane.org/gmane.comp.version-control.git/227017\n\n-- \nFelipe Contreras\n"},{"id":"220409","messageId":"CAMP44s0r96ByEs3+N1Qo+O18rOmT72rHk4zAEFAyFdU_DsQ8wA@mail.gmail.com","threadId":"34068","inReplyTo":"7vwqq1ct0g.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] build: get rid of the notion of a git library","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-06-11T04:15:14Z","receivedAt":"2013-06-11T04:15:14Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Mon, Jun 10, 2013 at 8:53 PM, Junio C Hamano wrote:\n> Junio C Hamano writes:\n>\n>> And I do not see the reason why builtin/*.o should not depend on\n>> each other. It is not messed up at all. They are meant to be\n>> linked into a single binary---that is what being \"built-in\" is.\n>>\n>> A good way forward, the way it *SHOULD* be, is to slim the builtin/*.o\n>> by moving parts that do not have to be in the single \"git\" binary\n>> but are also usable in standalone binaries out of them.\n>\n> Actually, as long as these pieces are currently used by builtins,\n> moving them (e.g. init_copy_notes_for_rewrite()) out of builtin/*.o\n> will not make these parts not to be in the single \"git\" binary at\n> all, so the above is grossly misstated.\n>\n> - There may be pieces of usefully reusable code buried in\n> builtin/*.o;\n>\n> - By definition, any code (piece of data or function definition) in\n> builtin/*.o cannot be used in standalone binaries, because all of\n> builtin/*.o expect to link with git.o and expect their cmd_foo()\n> getting called from main in it;\n>\n> - By moving the useful reusable pieces ont of builtin/*.o and\n> adding them to libgit.a, these pieces become usable from\n> standalone binaries as well.\n\nWhat if these reusable pieces should not be used by standalone binaries?\n\n> And that is the reason why slimming builtin/*.o is the way it\n> *SHOULD* be.\n>\n> Another thing to think about is looking at pieces of data and\n> functions defined in each *.o files and moving things around within\n> them. For example, looking at the dependency chain I quoted earlier\n> for sequencer.o to build upload-pack, which is about responding to\n> \"git fetch\" on the sending side:\n>\n> upload-pack.c wants handle_revision_opt etc.\n> revision.c provides handle_revision_opt\n> wants name_decoration etc.\n> log-tree.c provides name_decoration\n> wants append_signoff\n> sequencer.c provides append_signoff\n>\n> It is already crazy. There is no reason for the pack sender to be\n> linking with the sequencer interpreter machinery. If the function\n> definition (and possibly other ones) are split into separate source\n> files (still in libgit.a), git-upload-pack binary does not have to\n> pull in the whole sequencer.c at all.\n\nAgreed, which is precisely why my patches move that code out of\nsequencer.c. Maybe log-tree.c is not the right destination, but it is\na step in the right direction.\n\n> Coming back to the categorization Peff earlier made in the thread, I\n> think I am OK with adding new two subdirectories to the root level,\n> i.e.\n>\n> builtin/ - the ones that define cmd_foo()\n\nAs is the case right now.\n\n> commands/ - the ones that has main() for standalone commands\n\nGood.\n\n> libsrc/ - the ones that go in libgit.a\n\nlib/ is probably descriptive enough.\n\nBut this doesn't answer the question; what about code that is shared\nbetween builtins, but cannot be used by standalone programs?\n\nI'd wager it belongs to builtin/ and should be linked to\nbuiltin/lib.a. Maybe you would like to have a separate builtin/lib/\ndirectory, but I think that's overkill.\n\n> We may also want to add another subdirectory to hold scripted\n> Porcelains, but the primary topic of this thread is what to do about\n> the C library, so it is orthogonal in that sense, but if we were to\n> go in the \"group things in subdirectories to slim the root level\"\n> direction, it may be worth considering doing so at the same time.\n\nAgreed. Plus there's completions, shell prompt, and other script-like\ntools that shouldn't really belong in contrib/, and probably installed\nby default.\n\n-- \nFelipe Contreras\n"},{"id":"220470","messageId":"7vtxl4blht.fsf@alter.siamese.dyndns.org","threadId":"34068","inReplyTo":"CAMP44s0r96ByEs3+N1Qo+O18rOmT72rHk4zAEFAyFdU_DsQ8wA@mail.gmail.com","subject":"Re: [PATCH] build: get rid of the notion of a git library","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-11T17:33:34Z","receivedAt":"2013-06-11T17:33:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n>> - There may be pieces of usefully reusable code buried in\n>> builtin/*.o;\n>>\n>> - By definition, any code (piece of data or function definition) in\n>> builtin/*.o cannot be used in standalone binaries, because all of\n>> builtin/*.o expect to link with git.o and expect their cmd_foo()\n>> getting called from main in it;\n>>\n>> - By moving the useful reusable pieces ont of builtin/*.o and\n>> adding them to libgit.a, these pieces become usable from\n>> standalone binaries as well.\n>\n> What if these reusable pieces should not be used by standalone binaries?\n\nI am not sure what you mean. A piece is either reusable or not.\nWhen would one piece _be_ reusable and should *not* be used in one\ncontext but not in another?\n\nThere are distinctions between being \"useful\" and \"usable\", but I\nthink the zeroth order of approximation when thinking about this is\nto think builtin/*.o as set of subroutines called by git.c::main().\n\nThese set of subroutines may call out to more generic helper\nfunctions that are usable from anywhere both within builtins and\nalso from standalone. They may also call to their own helper\nfunctions that were originally designed to support only their use\nby the original caller from somewhere in builtin/*.o (most commonly\nin the same file, marked as static).\n\nThe general direction, if we want to have an improve libgit.a,\nshould be to see if the functions and their data that are private\nto builtin/*.o can be used from standalone, either as they are or\nwith more generalization, and turn them from helpers specific to one\ncmd_foo() into more generally useful library-ish functions.\n\nThere may be pieces in the callchain from that entry point to\ncmd_foo() that are implementation details of git.c::main(); for\nexample the loop that does command dispatching to check with\nbuiltins, external commands that begin with git-, and aliases, is\none of them, and would not be usable (nor it is useful) outside the\ncontext of \"git\" aggregate binary. But there are things that ought\nto be usable that are currently in builtin/*.o, which prevents them\nfrom being used by standalone binaries. If a remote helper binary\nthat is standalone wants to call \"create_note()\", it is not\nsufficient to make it non-static in builtin/notes.o, for example.\n\nBut if it is moved outside builtin/notes.o, it becomes usable.\n\nI think the \"git notes\", being one of the most recent additions,\nhaven't gone through enough round of refactoring to come to the best\nseparation between library-ish part (i.e. could be in notes.o, even\nthough it is mostly about the underlying data structure manipulation\nand contains no higher-level operations like actually creating and\ncopying, which might want to be in a separate source notes-lib.o)\nand its CLI implementation \"builtin/notes.o\".\n\n> But this doesn't answer the question; what about code that is shared\n> between builtins, but cannot be used by standalone programs?\n\nAgain, I do not know what you mean by \"cannot\" here. My tentative\nanswer to that question is \"the eventual goal should be not to have\nany code in that class, and that is a reasonable goal we can achieve\nonce we refactor what ought to be reusable out of builtin/*.o\".\n\nWhat are the examples you have in mind, code that we want to forbid\nstandalone from using?\n"},{"id":"220472","messageId":"CAMP44s02PqGFNmrGEcJVT6xcQHx8k4NYqJ_TtOTUEY8XHPj0BA@mail.gmail.com","threadId":"34068","inReplyTo":"7vtxl4blht.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] build: get rid of the notion of a git library","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-06-11T17:41:06Z","receivedAt":"2013-06-11T17:41:06Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Tue, Jun 11, 2013 at 12:33 PM, Junio C Hamano wrote:\n> Felipe Contreras writes:\n>\n>>> - There may be pieces of usefully reusable code buried in\n>>> builtin/*.o;\n>>>\n>>> - By definition, any code (piece of data or function definition) in\n>>> builtin/*.o cannot be used in standalone binaries, because all of\n>>> builtin/*.o expect to link with git.o and expect their cmd_foo()\n>>> getting called from main in it;\n>>>\n>>> - By moving the useful reusable pieces ont of builtin/*.o and\n>>> adding them to libgit.a, these pieces become usable from\n>>> standalone binaries as well.\n>>\n>> What if these reusable pieces should not be used by standalone binaries?\n>\n> I am not sure what you mean. A piece is either reusable or not.\n\nIt can be reusable for A, but not for B. A being the 'git' binary, B\nbeing other standalone binaries.\n\n>> But this doesn't answer the question; what about code that is shared\n>> between builtins, but cannot be used by standalone programs?\n>\n> Again, I do not know what you mean by \"cannot\" here. My tentative\n> answer to that question is \"the eventual goal should be not to have\n> any code in that class, and that is a reasonable goal we can achieve\n> once we refactor what ought to be reusable out of builtin/*.o\".\n>\n> What are the examples you have in mind, code that we want to forbid\n> standalone from using?\n\ninit_copy_notes_for_rewrite(). Nothing outside the 'git' binary would\nneed that. If you disagree, show me an example.\n\n-- \nFelipe Contreras\n"},{"id":"220474","messageId":"7vppvsbkc3.fsf@alter.siamese.dyndns.org","threadId":"34068","inReplyTo":"CAMP44s02PqGFNmrGEcJVT6xcQHx8k4NYqJ_TtOTUEY8XHPj0BA@mail.gmail.com","subject":"Re: [PATCH] build: get rid of the notion of a git library","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-11T17:58:36Z","receivedAt":"2013-06-11T17:58:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n>> What are the examples you have in mind, code that we want to forbid\n>> standalone from using?\n>\n> init_copy_notes_for_rewrite(). Nothing outside the 'git' binary would\n> need that. If you disagree, show me an example.\n\n\"Nothing would need that\", if you are talking about the current\ncodebase, I would agree that nothing would link to it.\n\nBut that is not a good justification for closing door to others that\ncome later who may want to have a standalone that would want to use\nit. Think about rewriting filter-branch.sh in C but not as a\nbuilt-in, for example.\n"},{"id":"220475","messageId":"CAMP44s02KaMaMUz4618n5RqVqVSXzr_D9rPS1uesy2XEdqnq5A@mail.gmail.com","threadId":"34068","inReplyTo":"7vppvsbkc3.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] build: get rid of the notion of a git library","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-06-11T18:06:27Z","receivedAt":"2013-06-11T18:06:27Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Tue, Jun 11, 2013 at 12:58 PM, Junio C Hamano wrote:\n> Felipe Contreras writes:\n>\n>>> What are the examples you have in mind, code that we want to forbid\n>>> standalone from using?\n>>\n>> init_copy_notes_for_rewrite(). Nothing outside the 'git' binary would\n>> need that. If you disagree, show me an example.\n>\n> \"Nothing would need that\", if you are talking about the current\n> codebase, I would agree that nothing would link to it.\n>\n> But that is not a good justification for closing door to others that\n> come later who may want to have a standalone that would want to use\n> it. Think about rewriting filter-branch.sh in C but not as a\n> built-in, for example.\n\nWhy would anybody rewrite filter-branch, and not make it a builtin? It\nshould be a builtin. That's the whole point of builtins.\n\nMoreover, if you are going to argue that we shouldn't be closing the\ndoor, then why not link ./builtin/*.o to libgit.a? If you are\nseriously considering the highly unlikely hypothetical standalone\ngit-filter-branch scenario, you should consider the even more likely\nscenario where somebody needs to access code from ./builtin/*.o; that\nscenario is not even hypothetical, we know it's happened multiple\ntimes, and we know it's going to happen again.\n\n-- \nFelipe Contreras\n"},{"id":"220478","messageId":"CA+55aFwYAFuz5p0=8QiAFDy4e66f1pF3v=D5nnL6+3um7Z3L2g@mail.gmail.com","threadId":"34068","inReplyTo":"CAMP44s02KaMaMUz4618n5RqVqVSXzr_D9rPS1uesy2XEdqnq5A@mail.gmail.com","subject":"Re: [PATCH] build: get rid of the notion of a git library","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2013-06-11T18:14:54Z","receivedAt":"2013-06-11T18:14:54Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"On Tue, Jun 11, 2013 at 11:06 AM, Felipe Contreras\n wrote:\n>\n> Moreover, if you are going to argue that we shouldn't be closing the\n> door [...]\n\nFelipe, you saying \"if you are going to argue ...\" to anybody else is\nkind of ironic.\n\nWhy is it every thread I see you in, you're being a dick and arguing\nfor some theoretical thing that nobody else cares about?\n\nThis whole thread has been one long argument about totally pointless\nthings that wouldn't improve anything one way or the other. It's\nbikeshedding of the worst kind. Just let it go.\n\n Linus\n"},{"id":"220479","messageId":"7vd2rsbjgr.fsf@alter.siamese.dyndns.org","threadId":"34068","inReplyTo":"CAMP44s02KaMaMUz4618n5RqVqVSXzr_D9rPS1uesy2XEdqnq5A@mail.gmail.com","subject":"Re: [PATCH] build: get rid of the notion of a git library","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-11T18:17:24Z","receivedAt":"2013-06-11T18:17:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n> Moreover, if you are going to argue that we shouldn't be closing the\n> door, then why not link ./builtin/*.o to libgit.a?\n\nHuh? It does not make any sense. builtin/*.o files have cmd_foo()\nthat are expected to be called from git.c::main(), but libgit.a\nfiles are linked with no constraints whose main() they are linking\nwith.\n\n> If you are\n> seriously considering the highly unlikely hypothetical standalone\n> git-filter-branch scenario, you should consider the even more likely\n> scenario where somebody needs to access code from ./builtin/*.o; that\n> scenario is not even hypothetical, we know it's happened multiple\n> times, and we know it's going to happen again.\n\nThat is exactly why I said that builtin/*.o should be refactored to\npick \"does not have to be in builtin\" bits, which will result in a\nbetter division of labor. Reusable bits should live in the library,\nwhile a particular implementation of command remain in builtin/*\nthat utilize the reusable bits.\n\nYou still haven't justified why we have to _forbid_ any outside\ncallers from calling copy_notes_for_rewrite().\n"},{"id":"220494","messageId":"CAMP44s3SL7qs-Pmfz=kV-4U5OnPedK2RDLZDOyU-eq8WebLt+Q@mail.gmail.com","threadId":"34068","inReplyTo":"7vd2rsbjgr.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] build: get rid of the notion of a git library","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-06-11T19:01:15Z","receivedAt":"2013-06-11T19:01:15Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Tue, Jun 11, 2013 at 1:17 PM, Junio C Hamano wrote:\n> Felipe Contreras writes:\n>\n>> Moreover, if you are going to argue that we shouldn't be closing the\n>> door, then why not link ./builtin/*.o to libgit.a?\n>\n> Huh? It does not make any sense. builtin/*.o files have cmd_foo()\n> that are expected to be called from git.c::main(), but libgit.a\n> files are linked with no constraints whose main() they are linking\n> with.\n\n\n>> If you are\n>> seriously considering the highly unlikely hypothetical standalone\n>> git-filter-branch scenario, you should consider the even more likely\n>> scenario where somebody needs to access code from ./builtin/*.o; that\n>> scenario is not even hypothetical, we know it's happened multiple\n>> times, and we know it's going to happen again.\n>\n> That is exactly why I said that builtin/*.o should be refactored to\n> pick \"does not have to be in builtin\" bits, which will result in a\n> better division of labor. Reusable bits should live in the library,\n> while a particular implementation of command remain in builtin/*\n> that utilize the reusable bits.\n>\n> You still haven't justified why we have to _forbid_ any outside\n> callers from calling copy_notes_for_rewrite().\n\nBecause only builtins _should_ use it. I asked you for an example, and\nyou said a hypothetical standalone 'git-filter-branch' might use it,\nbut you have not explained why it should be standalone, when it's\nclear it should be a builtin.\n\nIf we assume your argument is valid for the hypothetical\n'git-filter-branch', if that's the case, then it would be even more\nreasonable to assume that there will be other standalone binaries that\nwould want to use all sort of functions from ./builtin/*.o. Let's put\nan example: reset_index(). Some standalone program wants to use that\nfunction. What do you we do?\n\nThe shortest route is to make it non-static, and add it to builtin.h.\nBut that would not be enough, we need the infrastructure prepared for\nthat; link libgit.a with ./builtin/*.o.\n\nI don't think that's the way to go, but your line of argumentation\nleads directly there; if we are worrying about anything that any\npotential standalone program could want, then we should worry about\nreset_index() not being easily accessible to them.\n\nIMO we should be clear and say no; standalone programs should not\naccess copy_notes_for_rewrite(), that's for builtins. If we move all\nthe code that potential standalone programs could want to libgit.a, it\nwouldn't be a library at all, and it would basically contain\neverything.\n\n-- \nFelipe Contreras\n"},{"id":"220496","messageId":"CAMP44s3QUjs_uOEF++NfSQbNSHae8y1Nxt48CWtHi8YdEiq_zA@mail.gmail.com","threadId":"34068","inReplyTo":"CA+55aFwYAFuz5p0=8QiAFDy4e66f1pF3v=D5nnL6+3um7Z3L2g@mail.gmail.com","subject":"Re: [PATCH] build: get rid of the notion of a git library","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-06-11T19:15:12Z","receivedAt":"2013-06-11T19:15:12Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Tue, Jun 11, 2013 at 1:14 PM, Linus Torvalds\n wrote:\n> On Tue, Jun 11, 2013 at 11:06 AM, Felipe Contreras\n> wrote:\n>>\n>> Moreover, if you are going to argue that we shouldn't be closing the\n>> door [...]\n>\n> Felipe, you saying \"if you are going to argue ...\" to anybody else is\n> kind of ironic.\n\nSupposing the other side's argument is correct is a standard\ndiscussing technique.\n\n> Why is it every thread I see you in, you're being a dick and arguing\n> for some theoretical thing that nobody else cares about?\n\nI don't know. I've sent 800 patches in the last three months (patches,\nnot email comments), and you pick this one to reply to. Maybe because\nyou enjoy insulting people?\n\n> This whole thread has been one long argument about totally pointless\n> things that wouldn't improve anything one way or the other. It's\n> bikeshedding of the worst kind. Just let it go.\n\nWhy don't you ask Junio to let it go? If it's irrelevant, than it\ndoesn't matter if this patch is applied or not. You say it's\nbike-shedding, that implies that Junio likes red, and I like blue, and\nboth are equally useless. So let's go for blue then.\n\nPresumably Junio doesn't agree with you, he does truly think it should\nbe red, in fact, he doesn't think it's just a color, it's something\nimportant, and I agree, and apparently other people in the mailing\nlist also agree, as most of them have voiced their opinion that red is\nthe color.\n\nNow, do you have something of value to say which of the two options\nshould be, or are you just going to engage in double standards and\npersonal attacks?\n\nIf you truly think this is bikeshedding, at least be fair and complain\nabout that to the people that argue for red, not just the ones that\nargue for blue.\n\n-- \nFelipe Contreras\n"},{"id":"220498","messageId":"7vobbca1sr.fsf@alter.siamese.dyndns.org","threadId":"34068","inReplyTo":"CAMP44s3SL7qs-Pmfz=kV-4U5OnPedK2RDLZDOyU-eq8WebLt+Q@mail.gmail.com","subject":"Re: [PATCH] build: get rid of the notion of a git library","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-11T19:24:20Z","receivedAt":"2013-06-11T19:24:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n> On Tue, Jun 11, 2013 at 1:17 PM, Junio C Hamano wrote:\n>> Felipe Contreras writes:\n>>\n>>> Moreover, if you are going to argue that we shouldn't be closing the\n>>> door, then why not link ./builtin/*.o to libgit.a?\n>>\n>> Huh? It does not make any sense. builtin/*.o files have cmd_foo()\n>> that are expected to be called from git.c::main(), but libgit.a\n>> files are linked with no constraints whose main() they are linking\n>> with.\n> ...\n>> That is exactly why I said that builtin/*.o should be refactored to\n>> pick \"does not have to be in builtin\" bits, which will result in a\n>> better division of labor. Reusable bits should live in the library,\n>> while a particular implementation of command remain in builtin/*\n>> that utilize the reusable bits.\n>>\n>> You still haven't justified why we have to _forbid_ any outside\n>> callers from calling copy_notes_for_rewrite().\n>\n> Because only builtins _should_ use it.\n\nAnd there is no justification behind that \"_should_\" claim; you are\nnot making any technical argument to explain it.\n\n> I asked you for an example, and\n> you said a hypothetical standalone 'git-filter-branch' might use it,\n\nOf course it has to be hypothetical; I already said with the current\ncode no standalone does use it---it is not arranged to be doable so\nthere is no user. If you want to have examples of future possible\ncallers, they have to be hypothetical---the future by definition\nhasn't happened. But that does not mean hypothetical is impractical\nnor useless.\n\nThere are out-of-tree programs like cgit that will not be built-in\nbut already link with libgit.a. Moving things that can be used by\noutside people out of builtin/*.o to libgit.a would allow uses that\nyou and I cannot imagine offhand. I do not see a reason for us to\nforbid a filter-branch replacement out of tree as a standalone.\n\nI do not see a point in continuing to discuss this (or any design\nlevel issues) with you. You seem to go into a wrong direction to\nbreak the design of the overall system, not in a direction to\nimprove anything. I do not know, and at this point I do not care,\nif you are doing so deliberately to sabotage Git. Just stop.\n"},{"id":"220502","messageId":"CAMP44s0M6Jf_=bL+X06t7Vam+WypL_3JAt1odBkXCpMgqANDiQ@mail.gmail.com","threadId":"34068","inReplyTo":"7vobbca1sr.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] build: get rid of the notion of a git library","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-06-11T19:49:39Z","receivedAt":"2013-06-11T19:49:39Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Tue, Jun 11, 2013 at 2:24 PM, Junio C Hamano wrote:\n> Felipe Contreras writes:\n>\n>> On Tue, Jun 11, 2013 at 1:17 PM, Junio C Hamano wrote:\n>>> Felipe Contreras writes:\n>>>\n>>>> Moreover, if you are going to argue that we shouldn't be closing the\n>>>> door, then why not link ./builtin/*.o to libgit.a?\n>>>\n>>> Huh? It does not make any sense. builtin/*.o files have cmd_foo()\n>>> that are expected to be called from git.c::main(), but libgit.a\n>>> files are linked with no constraints whose main() they are linking\n>>> with.\n>> ...\n>>> That is exactly why I said that builtin/*.o should be refactored to\n>>> pick \"does not have to be in builtin\" bits, which will result in a\n>>> better division of labor. Reusable bits should live in the library,\n>>> while a particular implementation of command remain in builtin/*\n>>> that utilize the reusable bits.\n>>>\n>>> You still haven't justified why we have to _forbid_ any outside\n>>> callers from calling copy_notes_for_rewrite().\n>>\n>> Because only builtins _should_ use it.\n>\n> And there is no justification behind that \"_should_\" claim; you are\n> not making any technical argument to explain it.\n\nThe first argument of init_copy_notes_for_rewrite() is the name of the\nbuiltin command. There hardly could be any more justification.\n\n>> I asked you for an example, and\n>> you said a hypothetical standalone 'git-filter-branch' might use it,\n>\n> Of course it has to be hypothetical; I already said with the current\n> code no standalone does use it---it is not arranged to be doable so\n> there is no user. If you want to have examples of future possible\n> callers, they have to be hypothetical---the future by definition\n> hasn't happened. But that does not mean hypothetical is impractical\n> nor useless.\n\nSo? It's still hypothetical, which is what I said. What are you\ncomplaining about? About the fact that I made a correct assessment?\n\n> There are out-of-tree programs like cgit that will not be built-in\n> but already link with libgit.a. Moving things that can be used by\n> outside people out of builtin/*.o to libgit.a would allow uses that\n> you and I cannot imagine offhand. I do not see a reason for us to\n> forbid a filter-branch replacement out of tree as a standalone.\n\nYeah, I already ran that argument, and you conveniently chose to\nescape the next logical conclusion that I already put forward:\n\n--- a/Makefile\n+++ b/Makefile\n@@ -990,6 +990,8 @@ BUILTIN_OBJS += builtin/verify-pack.o\n BUILTIN_OBJS += builtin/verify-tag.o\n BUILTIN_OBJS += builtin/write-tree.o\n\n+LIB_OBJS += $(BUILTIN_OBJS)\n+\n GITLIBS = $(LIB_FILE) $(XDIFF_LIB)\n EXTLIBS =\n\nI don't think that's the right direction.\n\n> I do not see a point in continuing to discuss this (or any design\n> level issues) with you. You seem to go into a wrong direction to\n> break the design of the overall system, not in a direction to\n> improve anything. I do not know, and at this point I do not care,\n> if you are doing so deliberately to sabotage Git. Just stop.\n\nThat's your opinion, and it's not shared by others (outside of the Git\nproject). If you were right and Git was moving in the right direction,\nthere would be no need for libgit2.\n\n-- \nFelipe Contreras\n"},{"id":"220505","messageId":"7vehc8a05n.fsf@alter.siamese.dyndns.org","threadId":"34068","inReplyTo":"CA+55aFwYAFuz5p0=8QiAFDy4e66f1pF3v=D5nnL6+3um7Z3L2g@mail.gmail.com","subject":"Re: [PATCH] build: get rid of the notion of a git library","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-11T19:59:48Z","receivedAt":"2013-06-11T19:59:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds writes:\n\n> This whole thread has been one long argument about totally pointless\n> things that wouldn't improve anything one way or the other. It's\n> bikeshedding of the worst kind. Just let it go.\n\nThe proposal to move sequencer.c to builtins/sequencer.c and then\nadding a filter in Makefile to exclude so that \"git-sequencer\" is\nnot built is \"it wouldn't improve anything one way or the other\".\nIt is to throw in something into a set to which it does not belong,\nand then working around that mistake with another kludge.\n\nThe problem that triggered the wrong solution actually is real,\nhowever.\n\nA function that sequencer.c (in libgit.a so that it could be used by\nstandalone) may want to use in the future currently lives in\nbuiltin/notes.c. If you add a call to that function to sequencer.c\nwithout doing anything else, standalones like git-upload-pack will\nstop linking correctly. The git-upload-pack wants the revision\ntraversal machinery in revision.o, which in turn wants to be able to\nsee log-tree.o, which in turn wants to link with sequencer.o to see\none global variable (there may be other dependencies). All of these\nobjects are currently in libgit.a so that both builtins and standalones\ncan use them.\n\nMoving sequencer.c to builtin/ is not even a solution. Linking\ngit-upload-pack will still pull in builtin/notes.o along with\ncmd_notes(), which is not called from main(); as you remember,\ncmd_foo() in all builtin/*.o are designed to be called from\ngit.c::main().\n\nThere is only one right solution. If a useful function is buried in\nbuiltin/*.o as a historical accident (i.e. it started its life as a\nhelper for that particular command, and nobody else used it from\noutside so far) and that makes it impossible to use the function\nfrom outside builtin/*.o, refactor the function and its callers and\nmove it to libgit.a.\n\nSo I do not think this is not even a bikeshedding. Just one side\nbeing right, and the other side continuing to repeat nonsense\nwithout listening.\n"},{"id":"220506","messageId":"CAMP44s0ytu39thUo4Jfpy_rEdMKyTu6AtXW+4SBWHoRj+G-TwA@mail.gmail.com","threadId":"34068","inReplyTo":"7vehc8a05n.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] build: get rid of the notion of a git library","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-06-11T20:12:58Z","receivedAt":"2013-06-11T20:12:58Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Tue, Jun 11, 2013 at 2:59 PM, Junio C Hamano wrote:\n> Linus Torvalds writes:\n>\n>> This whole thread has been one long argument about totally pointless\n>> things that wouldn't improve anything one way or the other. It's\n>> bikeshedding of the worst kind. Just let it go.\n>\n> The proposal to move sequencer.c to builtins/sequencer.c and then\n> adding a filter in Makefile to exclude so that \"git-sequencer\" is\n> not built is \"it wouldn't improve anything one way or the other\".\n> It is to throw in something into a set to which it does not belong,\n\nIn your opinion.\n\n> and then working around that mistake with another kludge.\n\nIn your opinion.\n\nYou continually use absolutist rhetoric to try to convince yourself\nand others that what you say is absolute 100% fact. But it's not, it's\nyour opinion.\n\n> So I do not think this is not even a bikeshedding. Just one side\n> being right, and the other side continuing to repeat nonsense\n> without listening.\n\nGeorge W. Bush said history would prove him right, but saying so\ndoesn't make it so. At least he had the decency to acknowledge that\nother people had different valid opinions.\n\nbuiltin/lib.a makes perfect sense, and it's the first logical step in\nmoving libgit.a towards libgit2.\n\n-- \nFelipe Contreras\n"},{"id":"220576","messageId":"1370995981-1553-1-git-send-email-johan@herland.net","threadId":"34068","inReplyTo":"7vehc8a05n.fsf@alter.siamese.dyndns.org","subject":"[PATCH 0/3] Refactor useful notes functions into notes-utils.[ch]","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2013-06-12T00:12:58Z","receivedAt":"2013-06-12T00:12:58Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"> There is only one right solution. If a useful function is buried in\n> builtin/*.o as a historical accident (i.e. it started its life as a\n> helper for that particular command, and nobody else used it from\n> outside so far) and that makes it impossible to use the function\n> from outside builtin/*.o, refactor the function and its callers and\n> move it to libgit.a.\n\nHere goes...\n\n...Johan\n\nJohan Herland (3):\n finish_copy_notes_for_rewrite(): Let caller provide commit message\n Move copy_note_for_rewrite + friends from builtin/notes.c to notes-utils.c\n Move create_notes_commit() from notes-merge.c into notes-utils.c\n\n Makefile | 2 +\n builtin.h | 16 ------\n builtin/commit.c | 3 +-\n builtin/notes.c | 136 ++---------------------------------------------\n notes-merge.c | 27 +---------\n notes-merge.h | 14 -----\n notes-utils.c | 157 +++++++++++++++++++++++++++++++++++++++++++++++++++++++\n notes-utils.h | 37 +++++++++++++\n 8 files changed, 203 insertions(+), 189 deletions(-)\n create mode 100644 notes-utils.c\n create mode 100644 notes-utils.h\n\n-- \n1.8.1.3.704.g33f7d4f\n"},{"id":"220577","messageId":"1370995981-1553-2-git-send-email-johan@herland.net","threadId":"34068","inReplyTo":"1370995981-1553-1-git-send-email-johan@herland.net","subject":"[PATCH 1/3] finish_copy_notes_for_rewrite(): Let caller provide commit message","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2013-06-12T00:12:59Z","receivedAt":"2013-06-12T00:12:59Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"When copying notes for a rewritten object, the resulting notes commit\nwould have the following hardcoded commit message:\n\n Notes added by 'git notes copy'\n\nThis is obviously bogus when the notes rewriting is performed by\n'git commit --amend'.\n\nTherefore, let the caller specify an appropriate notes commit message\ninstead of hardcoding it. The above message is used for 'git notes copy',\nbut when calling finish_copy_notes_for_rewrite() from builtin/commit.c,\nwe use the following message instead:\n\n Notes added by 'git commit --amend'\n\nCc: Thomas Rast \nSigned-off-by: Johan Herland \n---\n builtin.h | 2 +-\n builtin/commit.c | 2 +-\n builtin/notes.c | 9 +++++----\n 3 files changed, 7 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin.h b/builtin.h\nindex faef559..78fb14d 100644\n--- a/builtin.h\n+++ b/builtin.h\n@@ -36,7 +36,7 @@ struct notes_rewrite_cfg {\n struct notes_rewrite_cfg *init_copy_notes_for_rewrite(const char *cmd);\n int copy_note_for_rewrite(struct notes_rewrite_cfg *c,\n \t\t\t const unsigned char *from_obj, const unsigned char *to_obj);\n-void finish_copy_notes_for_rewrite(struct notes_rewrite_cfg *c);\n+void finish_copy_notes_for_rewrite(struct notes_rewrite_cfg *c, const char *msg);\n \n extern int textconv_object(const char *path, unsigned mode, const unsigned char *sha1, int sha1_valid, char **buf, unsigned long *buf_size);\n \ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex d2f30d9..f8df8ca 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -1591,7 +1591,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \t\tif (cfg) {\n \t\t\t/* we are amending, so current_head is not NULL */\n \t\t\tcopy_note_for_rewrite(cfg, current_head->object.sha1, sha1);\n-\t\t\tfinish_copy_notes_for_rewrite(cfg);\n+\t\t\tfinish_copy_notes_for_rewrite(cfg, \"Notes added by 'git commit --amend'\");\n \t\t}\n \t\trun_rewrite_hook(current_head->object.sha1, sha1);\n \t}\ndiff --git a/builtin/notes.c b/builtin/notes.c\nindex 57748a6..6a80714 100644\n--- a/builtin/notes.c\n+++ b/builtin/notes.c\n@@ -403,11 +403,11 @@ int copy_note_for_rewrite(struct notes_rewrite_cfg *c,\n \treturn ret;\n }\n \n-void finish_copy_notes_for_rewrite(struct notes_rewrite_cfg *c)\n+void finish_copy_notes_for_rewrite(struct notes_rewrite_cfg *c, const char *msg)\n {\n \tint i;\n \tfor (i = 0; c->trees[i]; i++) {\n-\t\tcommit_notes(c->trees[i], \"Notes added by 'git notes copy'\");\n+\t\tcommit_notes(c->trees[i], msg);\n \t\tfree_notes(c->trees[i]);\n \t}\n \tfree(c->trees);\n@@ -420,6 +420,7 @@ static int notes_copy_from_stdin(int force, const char *rewrite_cmd)\n \tstruct notes_rewrite_cfg *c = NULL;\n \tstruct notes_tree *t = NULL;\n \tint ret = 0;\n+\tconst char *msg = \"Notes added by 'git notes copy'\";\n \n \tif (rewrite_cmd) {\n \t\tc = init_copy_notes_for_rewrite(rewrite_cmd);\n@@ -461,10 +462,10 @@ static int notes_copy_from_stdin(int force, const char *rewrite_cmd)\n \t}\n \n \tif (!rewrite_cmd) {\n-\t\tcommit_notes(t, \"Notes added by 'git notes copy'\");\n+\t\tcommit_notes(t, msg);\n \t\tfree_notes(t);\n \t} else {\n-\t\tfinish_copy_notes_for_rewrite(c);\n+\t\tfinish_copy_notes_for_rewrite(c, msg);\n \t}\n \treturn ret;\n }\n-- \n1.8.1.3.704.g33f7d4f\n"},{"id":"220578","messageId":"1370995981-1553-3-git-send-email-johan@herland.net","threadId":"34068","inReplyTo":"1370995981-1553-1-git-send-email-johan@herland.net","subject":"[PATCH 2/3] Move copy_note_for_rewrite + friends from builtin/notes.c to notes-utils.c","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2013-06-12T00:13:00Z","receivedAt":"2013-06-12T00:13:00Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"This is a pure code movement of the machinery for copying notes to\nrewritten objects. This code was located in builtin/notes.c for\nhistorical reasons. In order to make it available to builtin/commit.c\nit was declared in builtin.h. This was more of an accident of history\nthan a concious design, and we now want to make this machinery more\nwidely available.\n\nHence, this patch moves the code into the new notes-utils.[hc] files\nwhich are included into libgit.a. Except for adjusting #includes\naccordingly, this patch merely moves the relevant functions verbatim\ninto the new files.\n\nCc: Thomas Rast \nSigned-off-by: Johan Herland \n---\n Makefile | 2 +\n builtin.h | 16 -------\n builtin/commit.c | 1 +\n builtin/notes.c | 131 +-----------------------------------------------------\n notes-utils.c | 132 +++++++++++++++++++++++++++++++++++++++++++++++++++++++\n notes-utils.h | 23 ++++++++++\n 6 files changed, 159 insertions(+), 146 deletions(-)\n create mode 100644 notes-utils.c\n create mode 100644 notes-utils.h\n\ndiff --git a/Makefile b/Makefile\nindex 0f931a2..22deee1 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -682,6 +682,7 @@ LIB_H += merge-recursive.h\n LIB_H += mergesort.h\n LIB_H += notes-cache.h\n LIB_H += notes-merge.h\n+LIB_H += notes-utils.h\n LIB_H += notes.h\n LIB_H += object.h\n LIB_H += pack-refs.h\n@@ -815,6 +816,7 @@ LIB_OBJS += name-hash.o\n LIB_OBJS += notes.o\n LIB_OBJS += notes-cache.o\n LIB_OBJS += notes-merge.o\n+LIB_OBJS += notes-utils.o\n LIB_OBJS += object.o\n LIB_OBJS += pack-check.o\n LIB_OBJS += pack-refs.o\ndiff --git a/builtin.h b/builtin.h\nindex 78fb14d..72bb2a8 100644\n--- a/builtin.h\n+++ b/builtin.h\n@@ -5,7 +5,6 @@\n #include \"strbuf.h\"\n #include \"cache.h\"\n #include \"commit.h\"\n-#include \"notes.h\"\n \n #define DEFAULT_MERGE_LOG_LEN 20\n \n@@ -23,21 +22,6 @@ struct fmt_merge_msg_opts {\n extern int fmt_merge_msg(struct strbuf *in, struct strbuf *out,\n \t\t\t struct fmt_merge_msg_opts *);\n \n-struct notes_rewrite_cfg {\n-\tstruct notes_tree **trees;\n-\tconst char *cmd;\n-\tint enabled;\n-\tcombine_notes_fn combine;\n-\tstruct string_list *refs;\n-\tint refs_from_env;\n-\tint mode_from_env;\n-};\n-\n-struct notes_rewrite_cfg *init_copy_notes_for_rewrite(const char *cmd);\n-int copy_note_for_rewrite(struct notes_rewrite_cfg *c,\n-\t\t\t const unsigned char *from_obj, const unsigned char *to_obj);\n-void finish_copy_notes_for_rewrite(struct notes_rewrite_cfg *c, const char *msg);\n-\n extern int textconv_object(const char *path, unsigned mode, const unsigned char *sha1, int sha1_valid, char **buf, unsigned long *buf_size);\n \n extern int cmd_add(int argc, const char **argv, const char *prefix);\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex f8df8ca..ce40176 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -29,6 +29,7 @@\n #include \"gpg-interface.h\"\n #include \"column.h\"\n #include \"sequencer.h\"\n+#include \"notes-utils.h\"\n \n static const char * const builtin_commit_usage[] = {\n \tN_(\"git commit [options] [--] ...\"),\ndiff --git a/builtin/notes.c b/builtin/notes.c\nindex 6a80714..9ed2508 100644\n--- a/builtin/notes.c\n+++ b/builtin/notes.c\n@@ -18,9 +18,7 @@\n #include \"parse-options.h\"\n #include \"string-list.h\"\n #include \"notes-merge.h\"\n-\n-static void commit_notes(struct notes_tree *t, const char *msg);\n-static combine_notes_fn parse_combine_notes_fn(const char *v);\n+#include \"notes-utils.h\"\n \n static const char * const git_notes_usage[] = {\n \tN_(\"git notes [--ref ] [list []]\"),\n@@ -287,133 +285,6 @@ static int parse_reedit_arg(const struct option *opt, const char *arg, int unset\n \treturn parse_reuse_arg(opt, arg, unset);\n }\n \n-static void commit_notes(struct notes_tree *t, const char *msg)\n-{\n-\tstruct strbuf buf = STRBUF_INIT;\n-\tunsigned char commit_sha1[20];\n-\n-\tif (!t)\n-\t\tt = &default_notes_tree;\n-\tif (!t->initialized || !t->ref || !*t->ref)\n-\t\tdie(_(\"Cannot commit uninitialized/unreferenced notes tree\"));\n-\tif (!t->dirty)\n-\t\treturn; /* don't have to commit an unchanged tree */\n-\n-\t/* Prepare commit message and reflog message */\n-\tstrbuf_addstr(&buf, msg);\n-\tif (buf.buf[buf.len - 1] != '\\n')\n-\t\tstrbuf_addch(&buf, '\\n'); /* Make sure msg ends with newline */\n-\n-\tcreate_notes_commit(t, NULL, &buf, commit_sha1);\n-\tstrbuf_insert(&buf, 0, \"notes: \", 7); /* commit message starts at index 7 */\n-\tupdate_ref(buf.buf, t->ref, commit_sha1, NULL, 0, DIE_ON_ERR);\n-\n-\tstrbuf_release(&buf);\n-}\n-\n-static combine_notes_fn parse_combine_notes_fn(const char *v)\n-{\n-\tif (!strcasecmp(v, \"overwrite\"))\n-\t\treturn combine_notes_overwrite;\n-\telse if (!strcasecmp(v, \"ignore\"))\n-\t\treturn combine_notes_ignore;\n-\telse if (!strcasecmp(v, \"concatenate\"))\n-\t\treturn combine_notes_concatenate;\n-\telse if (!strcasecmp(v, \"cat_sort_uniq\"))\n-\t\treturn combine_notes_cat_sort_uniq;\n-\telse\n-\t\treturn NULL;\n-}\n-\n-static int notes_rewrite_config(const char *k, const char *v, void *cb)\n-{\n-\tstruct notes_rewrite_cfg *c = cb;\n-\tif (!prefixcmp(k, \"notes.rewrite.\") && !strcmp(k+14, c->cmd)) {\n-\t\tc->enabled = git_config_bool(k, v);\n-\t\treturn 0;\n-\t} else if (!c->mode_from_env && !strcmp(k, \"notes.rewritemode\")) {\n-\t\tif (!v)\n-\t\t\tconfig_error_nonbool(k);\n-\t\tc->combine = parse_combine_notes_fn(v);\n-\t\tif (!c->combine) {\n-\t\t\terror(_(\"Bad notes.rewriteMode value: '%s'\"), v);\n-\t\t\treturn 1;\n-\t\t}\n-\t\treturn 0;\n-\t} else if (!c->refs_from_env && !strcmp(k, \"notes.rewriteref\")) {\n-\t\t/* note that a refs/ prefix is implied in the\n-\t\t * underlying for_each_glob_ref */\n-\t\tif (!prefixcmp(v, \"refs/notes/\"))\n-\t\t\tstring_list_add_refs_by_glob(c->refs, v);\n-\t\telse\n-\t\t\twarning(_(\"Refusing to rewrite notes in %s\"\n-\t\t\t\t\" (outside of refs/notes/)\"), v);\n-\t\treturn 0;\n-\t}\n-\n-\treturn 0;\n-}\n-\n-\n-struct notes_rewrite_cfg *init_copy_notes_for_rewrite(const char *cmd)\n-{\n-\tstruct notes_rewrite_cfg *c = xmalloc(sizeof(struct notes_rewrite_cfg));\n-\tconst char *rewrite_mode_env = getenv(GIT_NOTES_REWRITE_MODE_ENVIRONMENT);\n-\tconst char *rewrite_refs_env = getenv(GIT_NOTES_REWRITE_REF_ENVIRONMENT);\n-\tc->cmd = cmd;\n-\tc->enabled = 1;\n-\tc->combine = combine_notes_concatenate;\n-\tc->refs = xcalloc(1, sizeof(struct string_list));\n-\tc->refs->strdup_strings = 1;\n-\tc->refs_from_env = 0;\n-\tc->mode_from_env = 0;\n-\tif (rewrite_mode_env) {\n-\t\tc->mode_from_env = 1;\n-\t\tc->combine = parse_combine_notes_fn(rewrite_mode_env);\n-\t\tif (!c->combine)\n-\t\t\t/* TRANSLATORS: The first %s is the name of the\n-\t\t\t environment variable, the second %s is its value */\n-\t\t\terror(_(\"Bad %s value: '%s'\"), GIT_NOTES_REWRITE_MODE_ENVIRONMENT,\n-\t\t\t\t\trewrite_mode_env);\n-\t}\n-\tif (rewrite_refs_env) {\n-\t\tc->refs_from_env = 1;\n-\t\tstring_list_add_refs_from_colon_sep(c->refs, rewrite_refs_env);\n-\t}\n-\tgit_config(notes_rewrite_config, c);\n-\tif (!c->enabled || !c->refs->nr) {\n-\t\tstring_list_clear(c->refs, 0);\n-\t\tfree(c->refs);\n-\t\tfree(c);\n-\t\treturn NULL;\n-\t}\n-\tc->trees = load_notes_trees(c->refs);\n-\tstring_list_clear(c->refs, 0);\n-\tfree(c->refs);\n-\treturn c;\n-}\n-\n-int copy_note_for_rewrite(struct notes_rewrite_cfg *c,\n-\t\t\t const unsigned char *from_obj, const unsigned char *to_obj)\n-{\n-\tint ret = 0;\n-\tint i;\n-\tfor (i = 0; c->trees[i]; i++)\n-\t\tret = copy_note(c->trees[i], from_obj, to_obj, 1, c->combine) || ret;\n-\treturn ret;\n-}\n-\n-void finish_copy_notes_for_rewrite(struct notes_rewrite_cfg *c, const char *msg)\n-{\n-\tint i;\n-\tfor (i = 0; c->trees[i]; i++) {\n-\t\tcommit_notes(c->trees[i], msg);\n-\t\tfree_notes(c->trees[i]);\n-\t}\n-\tfree(c->trees);\n-\tfree(c);\n-}\n-\n static int notes_copy_from_stdin(int force, const char *rewrite_cmd)\n {\n \tstruct strbuf buf = STRBUF_INIT;\ndiff --git a/notes-utils.c b/notes-utils.c\nnew file mode 100644\nindex 0000000..2ae5cb2\n--- /dev/null\n+++ b/notes-utils.c\n@@ -0,0 +1,132 @@\n+#include \"cache.h\"\n+#include \"commit.h\"\n+#include \"refs.h\"\n+#include \"notes-utils.h\"\n+#include \"notes-merge.h\" // need create_notes_commit()\n+\n+void commit_notes(struct notes_tree *t, const char *msg)\n+{\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tunsigned char commit_sha1[20];\n+\n+\tif (!t)\n+\t\tt = &default_notes_tree;\n+\tif (!t->initialized || !t->ref || !*t->ref)\n+\t\tdie(_(\"Cannot commit uninitialized/unreferenced notes tree\"));\n+\tif (!t->dirty)\n+\t\treturn; /* don't have to commit an unchanged tree */\n+\n+\t/* Prepare commit message and reflog message */\n+\tstrbuf_addstr(&buf, msg);\n+\tif (buf.buf[buf.len - 1] != '\\n')\n+\t\tstrbuf_addch(&buf, '\\n'); /* Make sure msg ends with newline */\n+\n+\tcreate_notes_commit(t, NULL, &buf, commit_sha1);\n+\tstrbuf_insert(&buf, 0, \"notes: \", 7); /* commit message starts at index 7 */\n+\tupdate_ref(buf.buf, t->ref, commit_sha1, NULL, 0, DIE_ON_ERR);\n+\n+\tstrbuf_release(&buf);\n+}\n+\n+static combine_notes_fn parse_combine_notes_fn(const char *v)\n+{\n+\tif (!strcasecmp(v, \"overwrite\"))\n+\t\treturn combine_notes_overwrite;\n+\telse if (!strcasecmp(v, \"ignore\"))\n+\t\treturn combine_notes_ignore;\n+\telse if (!strcasecmp(v, \"concatenate\"))\n+\t\treturn combine_notes_concatenate;\n+\telse if (!strcasecmp(v, \"cat_sort_uniq\"))\n+\t\treturn combine_notes_cat_sort_uniq;\n+\telse\n+\t\treturn NULL;\n+}\n+\n+static int notes_rewrite_config(const char *k, const char *v, void *cb)\n+{\n+\tstruct notes_rewrite_cfg *c = cb;\n+\tif (!prefixcmp(k, \"notes.rewrite.\") && !strcmp(k+14, c->cmd)) {\n+\t\tc->enabled = git_config_bool(k, v);\n+\t\treturn 0;\n+\t} else if (!c->mode_from_env && !strcmp(k, \"notes.rewritemode\")) {\n+\t\tif (!v)\n+\t\t\tconfig_error_nonbool(k);\n+\t\tc->combine = parse_combine_notes_fn(v);\n+\t\tif (!c->combine) {\n+\t\t\terror(_(\"Bad notes.rewriteMode value: '%s'\"), v);\n+\t\t\treturn 1;\n+\t\t}\n+\t\treturn 0;\n+\t} else if (!c->refs_from_env && !strcmp(k, \"notes.rewriteref\")) {\n+\t\t/* note that a refs/ prefix is implied in the\n+\t\t * underlying for_each_glob_ref */\n+\t\tif (!prefixcmp(v, \"refs/notes/\"))\n+\t\t\tstring_list_add_refs_by_glob(c->refs, v);\n+\t\telse\n+\t\t\twarning(_(\"Refusing to rewrite notes in %s\"\n+\t\t\t\t\" (outside of refs/notes/)\"), v);\n+\t\treturn 0;\n+\t}\n+\n+\treturn 0;\n+}\n+\n+\n+struct notes_rewrite_cfg *init_copy_notes_for_rewrite(const char *cmd)\n+{\n+\tstruct notes_rewrite_cfg *c = xmalloc(sizeof(struct notes_rewrite_cfg));\n+\tconst char *rewrite_mode_env = getenv(GIT_NOTES_REWRITE_MODE_ENVIRONMENT);\n+\tconst char *rewrite_refs_env = getenv(GIT_NOTES_REWRITE_REF_ENVIRONMENT);\n+\tc->cmd = cmd;\n+\tc->enabled = 1;\n+\tc->combine = combine_notes_concatenate;\n+\tc->refs = xcalloc(1, sizeof(struct string_list));\n+\tc->refs->strdup_strings = 1;\n+\tc->refs_from_env = 0;\n+\tc->mode_from_env = 0;\n+\tif (rewrite_mode_env) {\n+\t\tc->mode_from_env = 1;\n+\t\tc->combine = parse_combine_notes_fn(rewrite_mode_env);\n+\t\tif (!c->combine)\n+\t\t\t/* TRANSLATORS: The first %s is the name of the\n+\t\t\t environment variable, the second %s is its value */\n+\t\t\terror(_(\"Bad %s value: '%s'\"), GIT_NOTES_REWRITE_MODE_ENVIRONMENT,\n+\t\t\t\t\trewrite_mode_env);\n+\t}\n+\tif (rewrite_refs_env) {\n+\t\tc->refs_from_env = 1;\n+\t\tstring_list_add_refs_from_colon_sep(c->refs, rewrite_refs_env);\n+\t}\n+\tgit_config(notes_rewrite_config, c);\n+\tif (!c->enabled || !c->refs->nr) {\n+\t\tstring_list_clear(c->refs, 0);\n+\t\tfree(c->refs);\n+\t\tfree(c);\n+\t\treturn NULL;\n+\t}\n+\tc->trees = load_notes_trees(c->refs);\n+\tstring_list_clear(c->refs, 0);\n+\tfree(c->refs);\n+\treturn c;\n+}\n+\n+int copy_note_for_rewrite(struct notes_rewrite_cfg *c,\n+\t\t\t const unsigned char *from_obj, const unsigned char *to_obj)\n+{\n+\tint ret = 0;\n+\tint i;\n+\tfor (i = 0; c->trees[i]; i++)\n+\t\tret = copy_note(c->trees[i], from_obj, to_obj, 1, c->combine) || ret;\n+\treturn ret;\n+}\n+\n+void finish_copy_notes_for_rewrite(struct notes_rewrite_cfg *c, const char *msg)\n+{\n+\tint i;\n+\tfor (i = 0; c->trees[i]; i++) {\n+\t\tcommit_notes(c->trees[i], msg);\n+\t\tfree_notes(c->trees[i]);\n+\t}\n+\tfree(c->trees);\n+\tfree(c);\n+}\ndiff --git a/notes-utils.h b/notes-utils.h\nnew file mode 100644\nindex 0000000..0661e99\n--- /dev/null\n+++ b/notes-utils.h\n@@ -0,0 +1,23 @@\n+#ifndef NOTES_UTILS_H\n+#define NOTES_UTILS_H\n+\n+#include \"notes.h\"\n+\n+void commit_notes(struct notes_tree *t, const char *msg);\n+\n+struct notes_rewrite_cfg {\n+\tstruct notes_tree **trees;\n+\tconst char *cmd;\n+\tint enabled;\n+\tcombine_notes_fn combine;\n+\tstruct string_list *refs;\n+\tint refs_from_env;\n+\tint mode_from_env;\n+};\n+\n+struct notes_rewrite_cfg *init_copy_notes_for_rewrite(const char *cmd);\n+int copy_note_for_rewrite(struct notes_rewrite_cfg *c,\n+\t\t\t const unsigned char *from_obj, const unsigned char *to_obj);\n+void finish_copy_notes_for_rewrite(struct notes_rewrite_cfg *c, const char *msg);\n+\n+#endif\n-- \n1.8.1.3.704.g33f7d4f\n"},{"id":"220579","messageId":"1370995981-1553-4-git-send-email-johan@herland.net","threadId":"34068","inReplyTo":"1370995981-1553-1-git-send-email-johan@herland.net","subject":"[PATCH 3/3] Move create_notes_commit() from notes-merge.c into notes-utils.c","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2013-06-12T00:13:01Z","receivedAt":"2013-06-12T00:13:01Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"create_notes_commit() is needed by both the notes-merge code, and by\ncommit_notes() in notes-utils. Since it is generally useful, and not\nbound to the notes-merge machinery, we move it from (the more specific)\nnotes-merge to (the more general) notes-utils.\n\nSigned-off-by: Johan Herland \n---\n notes-merge.c | 27 +--------------------------\n notes-merge.h | 14 --------------\n notes-utils.c | 27 ++++++++++++++++++++++++++-\n notes-utils.h | 14 ++++++++++++++\n 4 files changed, 41 insertions(+), 41 deletions(-)\n\ndiff --git a/notes-merge.c b/notes-merge.c\nindex 0f67bd3..ab18857 100644\n--- a/notes-merge.c\n+++ b/notes-merge.c\n@@ -9,6 +9,7 @@\n #include \"notes.h\"\n #include \"notes-merge.h\"\n #include \"strbuf.h\"\n+#include \"notes-utils.h\"\n \n struct notes_merge_pair {\n \tunsigned char obj[20], base[20], local[20], remote[20];\n@@ -530,32 +531,6 @@ static int merge_from_diffs(struct notes_merge_options *o,\n \treturn conflicts ? -1 : 1;\n }\n \n-void create_notes_commit(struct notes_tree *t, struct commit_list *parents,\n-\t\t\t const struct strbuf *msg, unsigned char *result_sha1)\n-{\n-\tunsigned char tree_sha1[20];\n-\n-\tassert(t->initialized);\n-\n-\tif (write_notes_tree(t, tree_sha1))\n-\t\tdie(\"Failed to write notes tree to database\");\n-\n-\tif (!parents) {\n-\t\t/* Deduce parent commit from t->ref */\n-\t\tunsigned char parent_sha1[20];\n-\t\tif (!read_ref(t->ref, parent_sha1)) {\n-\t\t\tstruct commit *parent = lookup_commit(parent_sha1);\n-\t\t\tif (!parent || parse_commit(parent))\n-\t\t\t\tdie(\"Failed to find/parse commit %s\", t->ref);\n-\t\t\tcommit_list_insert(parent, &parents);\n-\t\t}\n-\t\t/* else: t->ref points to nothing, assume root/orphan commit */\n-\t}\n-\n-\tif (commit_tree(msg, tree_sha1, parents, result_sha1, NULL, NULL))\n-\t\tdie(\"Failed to commit notes tree to database\");\n-}\n-\n int notes_merge(struct notes_merge_options *o,\n \t\tstruct notes_tree *local_tree,\n \t\tunsigned char *result_sha1)\ndiff --git a/notes-merge.h b/notes-merge.h\nindex 0c11b17..1d01f6a 100644\n--- a/notes-merge.h\n+++ b/notes-merge.h\n@@ -26,20 +26,6 @@ struct notes_merge_options {\n void init_notes_merge_options(struct notes_merge_options *o);\n \n /*\n- * Create new notes commit from the given notes tree\n- *\n- * Properties of the created commit:\n- * - tree: the result of converting t to a tree object with write_notes_tree().\n- * - parents: the given parents OR (if NULL) the commit referenced by t->ref.\n- * - author/committer: the default determined by commmit_tree().\n- * - commit message: msg\n- *\n- * The resulting commit SHA1 is stored in result_sha1.\n- */\n-void create_notes_commit(struct notes_tree *t, struct commit_list *parents,\n-\t\t\t const struct strbuf *msg, unsigned char *result_sha1);\n-\n-/*\n * Merge notes from o->remote_ref into o->local_ref\n *\n * The given notes_tree 'local_tree' must be the notes_tree referenced by the\ndiff --git a/notes-utils.c b/notes-utils.c\nindex 2ae5cb2..9107c37 100644\n--- a/notes-utils.c\n+++ b/notes-utils.c\n@@ -2,7 +2,32 @@\n #include \"commit.h\"\n #include \"refs.h\"\n #include \"notes-utils.h\"\n-#include \"notes-merge.h\" // need create_notes_commit()\n+\n+void create_notes_commit(struct notes_tree *t, struct commit_list *parents,\n+\t\t\t const struct strbuf *msg, unsigned char *result_sha1)\n+{\n+\tunsigned char tree_sha1[20];\n+\n+\tassert(t->initialized);\n+\n+\tif (write_notes_tree(t, tree_sha1))\n+\t\tdie(\"Failed to write notes tree to database\");\n+\n+\tif (!parents) {\n+\t\t/* Deduce parent commit from t->ref */\n+\t\tunsigned char parent_sha1[20];\n+\t\tif (!read_ref(t->ref, parent_sha1)) {\n+\t\t\tstruct commit *parent = lookup_commit(parent_sha1);\n+\t\t\tif (!parent || parse_commit(parent))\n+\t\t\t\tdie(\"Failed to find/parse commit %s\", t->ref);\n+\t\t\tcommit_list_insert(parent, &parents);\n+\t\t}\n+\t\t/* else: t->ref points to nothing, assume root/orphan commit */\n+\t}\n+\n+\tif (commit_tree(msg, tree_sha1, parents, result_sha1, NULL, NULL))\n+\t\tdie(\"Failed to commit notes tree to database\");\n+}\n \n void commit_notes(struct notes_tree *t, const char *msg)\n {\ndiff --git a/notes-utils.h b/notes-utils.h\nindex 0661e99..b4cb1bf 100644\n--- a/notes-utils.h\n+++ b/notes-utils.h\n@@ -3,6 +3,20 @@\n \n #include \"notes.h\"\n \n+/*\n+ * Create new notes commit from the given notes tree\n+ *\n+ * Properties of the created commit:\n+ * - tree: the result of converting t to a tree object with write_notes_tree().\n+ * - parents: the given parents OR (if NULL) the commit referenced by t->ref.\n+ * - author/committer: the default determined by commmit_tree().\n+ * - commit message: msg\n+ *\n+ * The resulting commit SHA1 is stored in result_sha1.\n+ */\n+void create_notes_commit(struct notes_tree *t, struct commit_list *parents,\n+\t\t\t const struct strbuf *msg, unsigned char *result_sha1);\n+\n void commit_notes(struct notes_tree *t, const char *msg);\n \n struct notes_rewrite_cfg {\n-- \n1.8.1.3.704.g33f7d4f\n"},{"id":"220580","messageId":"CAMP44s2pUW_+w6B_R-A=vxOg1Ay6iLmc4MQsA_sfDF+GP-XsWw@mail.gmail.com","threadId":"34068","inReplyTo":"1370995981-1553-3-git-send-email-johan@herland.net","subject":"Re: [PATCH 2/3] Move copy_note_for_rewrite + friends from builtin/notes.c to notes-utils.c","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-06-12T00:32:40Z","receivedAt":"2013-06-12T00:32:40Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Tue, Jun 11, 2013 at 7:13 PM, Johan Herland wrote:\n> This is a pure code movement of the machinery for copying notes to\n> rewritten objects. This code was located in builtin/notes.c for\n> historical reasons. In order to make it available to builtin/commit.c\n> it was declared in builtin.h. This was more of an accident of history\n> than a concious design, and we now want to make this machinery more\n> widely available.\n>\n> Hence, this patch moves the code into the new notes-utils.[hc] files\n> which are included into libgit.a. Except for adjusting #includes\n> accordingly, this patch merely moves the relevant functions verbatim\n> into the new files.\n>\n> Cc: Thomas Rast \n> Signed-off-by: Johan Herland \n\nI wonder where you got that idea from. Did you come up with that out thin air?\n\nAnd here goes my bet; nobody will ever use these notes-utils outside\nof the git binary. Ever.\n\n-- \nFelipe Contreras\n"},{"id":"220585","messageId":"CALKQrgfxrKz5bB=AAmL1ZtBFRK2Bx6TrRd1AsMEVv8bTAH0KCg@mail.gmail.com","threadId":"34068","inReplyTo":"CAMP44s2pUW_+w6B_R-A=vxOg1Ay6iLmc4MQsA_sfDF+GP-XsWw@mail.gmail.com","subject":"Re: [PATCH 2/3] Move copy_note_for_rewrite + friends from builtin/notes.c to notes-utils.c","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2013-06-12T07:10:58Z","receivedAt":"2013-06-12T07:10:58Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Wed, Jun 12, 2013 at 2:32 AM, Felipe Contreras\n wrote:\n> On Tue, Jun 11, 2013 at 7:13 PM, Johan Herland wrote:\n>> This is a pure code movement of the machinery for copying notes to\n>> rewritten objects. This code was located in builtin/notes.c for\n>> historical reasons. In order to make it available to builtin/commit.c\n>> it was declared in builtin.h. This was more of an accident of history\n>> than a concious design, and we now want to make this machinery more\n>> widely available.\n>>\n>> Hence, this patch moves the code into the new notes-utils.[hc] files\n>> which are included into libgit.a. Except for adjusting #includes\n>> accordingly, this patch merely moves the relevant functions verbatim\n>> into the new files.\n>>\n>> Cc: Thomas Rast \n>> Signed-off-by: Johan Herland \n>\n> I wonder where you got that idea from. Did you come up with that out thin air?\n\nObviously not. I should add\n\nSuggested-by: Junio C Hamano \n\n\n...Johan\n\n-- \nJohan Herland, \nwww.herland.net\n"},{"id":"220653","messageId":"7v8v2f5jdy.fsf@alter.siamese.dyndns.org","threadId":"34068","inReplyTo":"1370995981-1553-2-git-send-email-johan@herland.net","subject":"Re: [PATCH 1/3] finish_copy_notes_for_rewrite(): Let caller provide commit message","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-12T17:27:53Z","receivedAt":"2013-06-12T17:27:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johan Herland writes:\n\n> When copying notes for a rewritten object, the resulting notes commit\n> would have the following hardcoded commit message:\n>\n> Notes added by 'git notes copy'\n>\n> This is obviously bogus when the notes rewriting is performed by\n> 'git commit --amend'.\n>\n> Therefore, let the caller specify an appropriate notes commit message\n> instead of hardcoding it. The above message is used for 'git notes copy',\n> but when calling finish_copy_notes_for_rewrite() from builtin/commit.c,\n> we use the following message instead:\n>\n> Notes added by 'git commit --amend'\n>\n> Cc: Thomas Rast \n> Signed-off-by: Johan Herland \n\nMakes sense. Thanks.\n"},{"id":"220655","messageId":"CAMP44s3KAeDPo1Cw8eFsU=A6H7oUGmf+eLAMvGV+R2_hPXHLbw@mail.gmail.com","threadId":"34068","inReplyTo":"CALKQrgfxrKz5bB=AAmL1ZtBFRK2Bx6TrRd1AsMEVv8bTAH0KCg@mail.gmail.com","subject":"Re: [PATCH 2/3] Move copy_note_for_rewrite + friends from builtin/notes.c to notes-utils.c","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-06-12T18:28:05Z","receivedAt":"2013-06-12T18:28:05Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Wed, Jun 12, 2013 at 2:10 AM, Johan Herland wrote:\n> On Wed, Jun 12, 2013 at 2:32 AM, Felipe Contreras\n> wrote:\n>> On Tue, Jun 11, 2013 at 7:13 PM, Johan Herland wrote:\n>>> This is a pure code movement of the machinery for copying notes to\n>>> rewritten objects. This code was located in builtin/notes.c for\n>>> historical reasons. In order to make it available to builtin/commit.c\n>>> it was declared in builtin.h. This was more of an accident of history\n>>> than a concious design, and we now want to make this machinery more\n>>> widely available.\n>>>\n>>> Hence, this patch moves the code into the new notes-utils.[hc] files\n>>> which are included into libgit.a. Except for adjusting #includes\n>>> accordingly, this patch merely moves the relevant functions verbatim\n>>> into the new files.\n>>>\n>>> Cc: Thomas Rast \n>>> Signed-off-by: Johan Herland \n>>\n>> I wonder where you got that idea from. Did you come up with that out thin air?\n>\n> Obviously not. I should add\n>\n> Suggested-by: Junio C Hamano \n\nYou are still not explaining where the idea came from. And you are\ndoing that with the express purpose of annoying.\n\nWhere did the idea come from?\n\n-- \nFelipe Contreras\n"},{"id":"220659","messageId":"CALKQrgfPktWOcUKnWecQcE-wMVwTqMES112nHcqnCrZzLLqOeg@mail.gmail.com","threadId":"34068","inReplyTo":"CAMP44s3KAeDPo1Cw8eFsU=A6H7oUGmf+eLAMvGV+R2_hPXHLbw@mail.gmail.com","subject":"Re: [PATCH 2/3] Move copy_note_for_rewrite + friends from builtin/notes.c to notes-utils.c","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2013-06-12T19:14:43Z","receivedAt":"2013-06-12T19:14:43Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Wed, Jun 12, 2013 at 8:28 PM, Felipe Contreras\n wrote:\n> On Wed, Jun 12, 2013 at 2:10 AM, Johan Herland wrote:\n>> On Wed, Jun 12, 2013 at 2:32 AM, Felipe Contreras wrote:\n>>> On Tue, Jun 11, 2013 at 7:13 PM, Johan Herland wrote:\n>>>> This is a pure code movement of the machinery for copying notes to\n>>>> rewritten objects. This code was located in builtin/notes.c for\n>>>> historical reasons. In order to make it available to builtin/commit.c\n>>>> it was declared in builtin.h. This was more of an accident of history\n>>>> than a concious design, and we now want to make this machinery more\n>>>> widely available.\n>>>>\n>>>> Hence, this patch moves the code into the new notes-utils.[hc] files\n>>>> which are included into libgit.a. Except for adjusting #includes\n>>>> accordingly, this patch merely moves the relevant functions verbatim\n>>>> into the new files.\n>>>>\n>>>> Cc: Thomas Rast \n>>>> Signed-off-by: Johan Herland \n>>>\n>>> I wonder where you got that idea from. Did you come up with that out thin air?\n>>\n>> Obviously not. I should add\n>>\n>> Suggested-by: Junio C Hamano \n>\n> You are still not explaining where the idea came from. And you are\n> doing that with the express purpose of annoying.\n\nTruly, I am not trying to annoy anyone. I have not followed the\npreceding discussion closely, and I wrote the patch based solely on\none paragraph from Junio's email[1].\n\n> Where did the idea come from?\n\nI got it from Junio. I do not know if I might have accidentally\nplagiarized something you already submitted to the mailing list,\nalthough I would be surprised if that was the case, since - as far as\nI understand - you are opposed to this solution. Furthermore, I\nthought you might not like having your name mentioned in a patch you\ndo not agree with, but if you think differently I have no problem\nadding your name. I don't know what kind of attribution you would\nprefer though:\n\nOriginally-envisioned-by: Felipe Contreras ?\nNAKed-by: Felipe Contreras ?\nSomething else?\n\n\n...Johan\n\n\n[1]: Quoted from <7vehc8a05n.fsf@alter.siamese.dyndns.org>:\nOn Tue, Jun 11, 2013 at 9:59 PM, Junio C Hamano wrote:\n> Linus Torvalds writes:\n> There is only one right solution. If a useful function is buried in\n> builtin/*.o as a historical accident (i.e. it started its life as a\n> helper for that particular command, and nobody else used it from\n> outside so far) and that makes it impossible to use the function\n> from outside builtin/*.o, refactor the function and its callers and\n> move it to libgit.a.\n\n-- \nJohan Herland, \nwww.herland.net\n"},{"id":"220662","messageId":"51b8c9816155a_501d1297e8483820@nysa.mail","threadId":"34068","inReplyTo":"CALKQrgfPktWOcUKnWecQcE-wMVwTqMES112nHcqnCrZzLLqOeg@mail.gmail.com","subject":"Re: [PATCH 2/3] Move copy_note_for_rewrite + friends from builtin/notes.c to notes-utils.c","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-06-12T19:18:25Z","receivedAt":"2013-06-12T19:18:25Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Johan Herland wrote:\n> On Wed, Jun 12, 2013 at 8:28 PM, Felipe Contreras\n> wrote:\n> > On Wed, Jun 12, 2013 at 2:10 AM, Johan Herland wrote:\n> >> On Wed, Jun 12, 2013 at 2:32 AM, Felipe Contreras wrote:\n> >>> On Tue, Jun 11, 2013 at 7:13 PM, Johan Herland wrote:\n> >>>> This is a pure code movement of the machinery for copying notes to\n> >>>> rewritten objects. This code was located in builtin/notes.c for\n> >>>> historical reasons. In order to make it available to builtin/commit.c\n> >>>> it was declared in builtin.h. This was more of an accident of history\n> >>>> than a concious design, and we now want to make this machinery more\n> >>>> widely available.\n> >>>>\n> >>>> Hence, this patch moves the code into the new notes-utils.[hc] files\n> >>>> which are included into libgit.a. Except for adjusting #includes\n> >>>> accordingly, this patch merely moves the relevant functions verbatim\n> >>>> into the new files.\n> >>>>\n> >>>> Cc: Thomas Rast \n> >>>> Signed-off-by: Johan Herland \n> >>>\n> >>> I wonder where you got that idea from. Did you come up with that out thin air?\n> >>\n> >> Obviously not. I should add\n> >>\n> >> Suggested-by: Junio C Hamano \n> >\n> > You are still not explaining where the idea came from. And you are\n> > doing that with the express purpose of annoying.\n> \n> Truly, I am not trying to annoy anyone. I have not followed the\n> preceding discussion closely, and I wrote the patch based solely on\n> one paragraph from Junio's email[1].\n\nHere is another pagraph:\n\n> Moving sequencer.c to builtin/ is not even a solution. Linking\n> git-upload-pack will still pull in builtin/notes.o along with cmd_notes(),\n> which is not called from main(); as you remember, cmd_foo() in all\n> builtin/*.o are designed to be called from git.c::main().\n\nWhich clearly refers to:\nhttp://article.gmane.org/gmane.comp.version-control.git/226752\n\n> > Where did the idea come from?\n> \n> I got it from Junio. I do not know if I might have accidentally\n> plagiarized something you already submitted to the mailing list,\n> although I would be surprised if that was the case, since - as far as\n> I understand - you are opposed to this solution.\n\nYou are aware I opposed this *solution*, yet were not aware that I sent the\nfirst patch in this thread, which clearly states the *problem*?\n\n> This way there will not be linking issues when top-level objects try to\n> access functions of builtin objects.\n\nhttp://article.gmane.org/gmane.comp.version-control.git/226845\n\n> Originally-envisioned-by: Felipe Contreras ?\n\nDo I have to do it for you? Your commit message is all wrong, because nowhere\nare you pointing out *why* you are making the change.\n\n---\nMove copy_note_for_rewrite + friends to notes-utils.c\n\nIn order to make these functionas available to top-level objects (e.g.\nsequencer.o), we need to move them out of the builtin/ subdirectory.\n\nReported-by: Felipe Contreras \n---\n\n-- \nFelipe Contreras\n"},{"id":"220665","messageId":"7v7ghz2j3s.fsf@alter.siamese.dyndns.org","threadId":"34068","inReplyTo":"1370995981-1553-3-git-send-email-johan@herland.net","subject":"Re: [PATCH 2/3] Move copy_note_for_rewrite + friends from builtin/notes.c to notes-utils.c","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-12T20:02:15Z","receivedAt":"2013-06-12T20:02:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johan Herland writes:\n\n> This is a pure code movement of the machinery for copying notes to\n> rewritten objects. This code was located in builtin/notes.c for\n> historical reasons. In order to make it available to builtin/commit.c\n> it was declared in builtin.h. This was more of an accident of history\n> than a concious design, and we now want to make this machinery more\n> widely available.\n>\n> Hence, this patch moves the code into the new notes-utils.[hc] files\n> which are included into libgit.a. Except for adjusting #includes\n> accordingly, this patch merely moves the relevant functions verbatim\n> into the new files.\n>\n> Cc: Thomas Rast \n> Signed-off-by: Johan Herland \n> ---\n> Makefile | 2 +\n> builtin.h | 16 -------\n> builtin/commit.c | 1 +\n> builtin/notes.c | 131 +-----------------------------------------------------\n> notes-utils.c | 132 +++++++++++++++++++++++++++++++++++++++++++++++++++++++\n> notes-utils.h | 23 ++++++++++\n> 6 files changed, 159 insertions(+), 146 deletions(-)\n> create mode 100644 notes-utils.c\n> create mode 100644 notes-utils.h\n\nOutput from \"git show -C1 --stat\" after applying this patch shows\nmostly removals (i.e. builtin/notes.c loses what was lifted from it,\nnotes-utils.c starts its life as a copy of the former and the patch\nshows removal of what should not move to notes-utils.c). After\ninspecting \"added\" lines to these two files, I did not spot anything\nsuspicious, except for one C++/C99 comment (will locally touch-up).\n\nThanks.\n"},{"id":"220666","messageId":"7vzjuv14ir.fsf@alter.siamese.dyndns.org","threadId":"34068","inReplyTo":"1370995981-1553-1-git-send-email-johan@herland.net","subject":"Re: [PATCH 0/3] Refactor useful notes functions into notes-utils.[ch]","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-12T20:02:36Z","receivedAt":"2013-06-12T20:02:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johan Herland writes:\n\n>> There is only one right solution. If a useful function is buried in\n>> builtin/*.o as a historical accident (i.e. it started its life as a\n>> helper for that particular command, and nobody else used it from\n>> outside so far) and that makes it impossible to use the function\n>> from outside builtin/*.o, refactor the function and its callers and\n>> move it to libgit.a.\n>\n> Here goes...\n>\n> ...Johan\n\nWith these three patches, if you apply the following skeleton patch\n(lifted from $gmane/226851 and adjusted minimally to the change\nthese patches introduce), we can see that the link breakage Felipe\nobserved in the message:\n\n Felipe Contreras writes in $gmane/226851:\n > What happens?\n > \n > libgit.a(sequencer.o): In function `copy_notes':\n > /home/felipec/dev/git/sequencer.c:110: undefined reference to\n > `init_copy_notes_for_rewrite'\n > /home/felipec/dev/git/sequencer.c:114: undefined reference to\n > `finish_copy_notes_for_rewrite'\n\nis gone.\n\n > It is not the first time, nor the last that top-level code needs\n > builtin code, and the solution is easy; organize the code.\n\nAnd as I already said, the above is correct. The problem and the\ngeneral approach to solve it correctly were identified in the\nmessage.\n\nBut what followed was a nonsense, which ended up wastign everybody's\ntime:\n\n> ... Alas, this\n> simple solution reject on the basis that we shouldn't organize the\n> code, because the code is not meant to be organized.\n\nThe proposed patch was rejected on the basis that it was organized\nthe code in a wrong way. And your patch shows how it should be\ndone.\n\nThanks for doing it right.\n\n-- skeleton patch --\n\n sequencer.c | 14 ++++++++++++++\n 1 file changed, 14 insertions(+)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex ab6f8a7..4281466 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -14,6 +14,7 @@\n #include \"merge-recursive.h\"\n #include \"refs.h\"\n #include \"argv-array.h\"\n+#include \"notes-utils.h\"\n \n #define GIT_REFLOG_ACTION \"GIT_REFLOG_ACTION\"\n \n@@ -979,6 +980,17 @@ static void save_opts(struct replay_opts *opts)\n \t}\n }\n \n+static void copy_notes(const char *name, const char *msg)\n+{\n+ struct notes_rewrite_cfg *cfg;\n+\n+ cfg = init_copy_notes_for_rewrite(name);\n+ if (!cfg)\n+ return;\n+\n+ finish_copy_notes_for_rewrite(cfg, msg);\n+}\n+\n static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)\n {\n \tstruct commit_list *cur;\n@@ -997,6 +1009,8 @@ static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)\n \t\t\treturn res;\n \t}\n \n+\tcopy_notes(\"cherry-pick\", \"notes copied by cherry-pick\");\n+\n \t/*\n \t * Sequence of picks finished successfully; cleanup by\n \t * removing the .git/sequencer directory\n"},{"id":"220668","messageId":"CAMP44s3jnyds45UGfbig1=evbqP-rztcn7GTZ8puVa2zzA7HGg@mail.gmail.com","threadId":"34068","inReplyTo":"7vzjuv14ir.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 0/3] Refactor useful notes functions into notes-utils.[ch]","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-06-12T20:11:59Z","receivedAt":"2013-06-12T20:11:59Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Wed, Jun 12, 2013 at 3:02 PM, Junio C Hamano wrote:\n\n>> ... Alas, this\n>> simple solution reject on the basis that we shouldn't organize the\n>> code, because the code is not meant to be organized.\n>\n> The proposed patch was rejected on the basis that it was organized\n> the code in a wrong way. And your patch shows how it should be\n> done.\n\nIn your opinion.\n\nThe fact that nobody outside of 'git' will ever use\ninit_copy_notes_for_rewrite() still remains. Therefore this\n\"organization\" is wrong.\n\n-- \nFelipe Contreras\n"},{"id":"220694","messageId":"20130613064521.GA21707@inner.h.apk.li","threadId":"34068","inReplyTo":"CAMP44s3KAeDPo1Cw8eFsU=A6H7oUGmf+eLAMvGV+R2_hPXHLbw@mail.gmail.com","subject":"Re: [PATCH 2/3] Move copy_note_for_rewrite + friends from builtin/notes.c to notes-utils.c","fromName":"Andreas Krey","fromEmail":"a.krey@gmx.de","sentAt":"2013-06-13T06:45:21Z","receivedAt":"2013-06-13T06:45:21Z","isPatch":true,"sender":{"key":"a.krey@gmx.de","avatar":"https://avatars.githubusercontent.com/u/37810?v=4"},"body":"On Wed, 12 Jun 2013 13:28:05 +0000, Felipe Contreras wrote:\n...\n> And you are\n> doing that with the express purpose of annoying.\n\nWhere did 'assume good faith' go to today?\n\nAndreas\n\n-- \n\"Totally trivial. Famous last words.\"\nFrom: Linus Torvalds \nDate: Fri, 22 Jan 2010 07:29:21 -0800\n"},{"id":"220720","messageId":"CAMP44s0Ng=d_h2dewZzSDk3LcXHNmz_8mGRXL43LE=iWOigN_w@mail.gmail.com","threadId":"34068","inReplyTo":"20130613064521.GA21707@inner.h.apk.li","subject":"Re: [PATCH 2/3] Move copy_note_for_rewrite + friends from builtin/notes.c to notes-utils.c","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-06-13T13:13:03Z","receivedAt":"2013-06-13T13:13:03Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Thu, Jun 13, 2013 at 1:45 AM, Andreas Krey wrote:\n> On Wed, 12 Jun 2013 13:28:05 +0000, Felipe Contreras wrote:\n> ...\n>> And you are\n>> doing that with the express purpose of annoying.\n>\n> Where did 'assume good faith' go to today?\n\nDid you read the last part?\n\n\"This does not mean that one should continue to assume good faith when\nthere's evidence to the contrary.\"\n\nThat being said, my evidence was not solid, and while there is still\nthe possibility that he was indeed acting in good faith, I've received\nno response from him, and Junio has committed the change without any\nmentioning of where the idea come from.\n\nEither way, I bet you my good faith suggestion will *not* end up in\nthe official guidelines, nor will any suggestion of mine.\n\n-- \nFelipe Contreras\n"},{"id":"220743","messageId":"7vsj0lvs8f.fsf@alter.siamese.dyndns.org","threadId":"34068","inReplyTo":"CAMP44s3jnyds45UGfbig1=evbqP-rztcn7GTZ8puVa2zzA7HGg@mail.gmail.com","subject":"Re: [PATCH 0/3] Refactor useful notes functions into notes-utils.[ch]","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-13T17:24:32Z","receivedAt":"2013-06-13T17:24:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n> On Wed, Jun 12, 2013 at 3:02 PM, Junio C Hamano wrote:\n>\n>> The proposed patch was rejected on the basis that it was organized\n>> the code in a wrong way. And your patch shows how it should be\n>> done.\n>\n> In your opinion.\n>\n> The fact that nobody outside of 'git' will ever use\n> init_copy_notes_for_rewrite() still remains. Therefore this\n> \"organization\" is wrong.\n\nThat is a fact?\n\nIt is your opinion on what might happen in the future. \n\nAnd you ignored external projects that may want to link with\nlibgit.a, and closed the door for future improvements. Johan's\nimplementation has the same effect of allowing sequencer.c to call\nthese functions without doing so.\n\nAnyway, I have a more important thing to say.\n\nYou sometimes identify the right problem to tackle, but often the\ndiscussions on your patches go in a wrong direction that does not\nhelp solving the original problem at all. The two examples I can\nimmediately recall offhand are:\n\n (1) a possible \"blame\" enhancement, where gitk, that currently runs\n two passes of it to identify where each line ultimately came\n from and to identify where each line was moved to the current\n place, could ask it to learn both with a single run.\n\n (2) refactoring builtin/notes.c to make it possible for sequencer\n machinery can also call useful helper functions buried in it.\n\nbut I am sure other reviewers can recall other instances in the\nrecent past.\n\nYour patches were wrong in both cases, but that is not an issue. If\nyou are not familiar with the area you are trying to improve, it is\nunderstandable that initial attempts may try to solve the right\nproblem in a wrong way. That is perfectly normal.\n\nThat is what the patch review process is there to help.\n\nReviewers who are more familiar with the area (either the code flow\nand data structure used in blame, or how the object files are laid\nout in the source tree and the build procedure is designed to link\nthem to which binary) can point the contributor in a direction that\nwould take us to a better result in the end. During the discussion,\nit may turn out that reviewers have overlooked issues that also need\nto be addressed, or there may be further adjustments needed that are\ninitially overlooked by everyone. The solution to these problems is\nfor contributors and reviewers to _collaborate_ to come up with a\nbetter end result, which is often different from both the original\npatch and the suggestions in the initial review.\n\nWhen it is your patch, however, we repeatedly saw that the review\nprocess got derailed in the middle.\n\nThe reviewers tried to reach a good end result in the same way as\nthey interact with other contributors, i.e. by showing a way they\nthink is better, trying to make the contributor realize why it is\nbetter by rephrasing and coming up with other examples.\n\nThis iteration takes a lot of resources, but the reviewers are\nhoping that we will see a good result at the end of the review and\neverybody wins. They are trying to collaborate.\n\nIf there is no will to collaborate on the contributor's end,\nhowever, and the primary thing the contributor wants to do is to\nendlessly argue, the efforts by reviewers are all wasted. We do not\nget anywhere.\n\nThat is how I perceive what happens to many of your patches. I am\nsure you will say \"that is your opinion\", but I do not think I am\nalone. And I am also sure you will then say \"majority is not always\nright\".\n\nBut the thing is, that majority is what writes the majority of the\ncode and does the majority of the reviews, so as maintainer I *do*\nhave to give their opinion a lot of weight, not to mention my own\nopinion about how to help keep the community the most productive.\n\nAnd I have to conclude that the cost of having to deal with you\noutweighs the benefit the project gets out of having you around.\nTherefore I have ask you to leave and not bother us anymore.\n\nGoodbye.\n"},{"id":"220756","messageId":"CAMP44s3atPW-SE1yQzep-F6M13g1fPP_q2RqHKofPL0B8=JfYQ@mail.gmail.com","threadId":"34068","inReplyTo":"7vsj0lvs8f.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 0/3] Refactor useful notes functions into notes-utils.[ch]","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-06-13T18:16:55Z","receivedAt":"2013-06-13T18:16:55Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Thu, Jun 13, 2013 at 12:24 PM, Junio C Hamano wrote:\n> Felipe Contreras writes:\n>\n>> On Wed, Jun 12, 2013 at 3:02 PM, Junio C Hamano wrote:\n>>\n>>> The proposed patch was rejected on the basis that it was organized\n>>> the code in a wrong way. And your patch shows how it should be\n>>> done.\n>>\n>> In your opinion.\n>>\n>> The fact that nobody outside of 'git' will ever use\n>> init_copy_notes_for_rewrite() still remains. Therefore this\n>> \"organization\" is wrong.\n>\n> That is a fact?\n\nNo, it's not.\n\n> It is your opinion on what might happen in the future.\n\nThat's right, an informed opinion.\n\n> And you ignored external projects that may want to link with\n> libgit.a,\n\nLike which project?\n\nMoreover:\n\n% find /opt/git -name '*.a'\n\nReturns nothing. The cannot link to libgit.a, and besides, we don't\nprovide a public API at all.\n\n> and closed the door for future improvements. Johan's\n> implementation has the same effect of allowing sequencer.c to call\n> these functions without doing so.\n\nThat would be closing the door to ghosts.\n\nDo you want to bet? Five years from now nobody will be using\ninit_copy_notes_for_rewrite().\n\nYou loose, we move it to builtin/lib.a, you win, we don't.\n\n> Anyway, I have a more important thing to say.\n>\n> You sometimes identify the right problem to tackle, but often the\n> discussions on your patches go in a wrong direction that does not\n> help solving the original problem at all.\n\nSo what? I'm a human, am I not allowed to make mistakes?\n\nYou make mistakes too.\n\n> The two examples I can immediately recall offhand are:\n>\n> (1) a possible \"blame\" enhancement, where gitk, that currently runs\n> two passes of it to identify where each line ultimately came\n> from and to identify where each line was moved to the current\n> place, could ask it to learn both with a single run.\n\nYes, *I ACKNOWLEDGED* the direction was not the right one, and I\ndidn't have the time nor the patience to go into such a tedious\ndirection.\n\n>From my recommended guideline:\n\n* Accept comments on your reviews gracefully. If the original patch\nsubmitter doesn't agree with your review, don't take offense. Don't\nassume the submitter has to automatically modify the patches according\nto your comments, or even necessarily seek a compromise. The submitter\nis entitled to his opinion, and so are you. Also, remember that each\nperson has their own priorities in life, and it might take time before\nthe submitter has time to implement the changes, if ever. The changes\nyou request might be beyond the time the submitter is willing to\nspend, and it's OK for him to decide to drop the patches as a result.\nYou can help by picking the patches yourself in those situations.\n\n> (2) refactoring builtin/notes.c to make it possible for sequencer\n> machinery can also call useful helper functions buried in it.\n\nYou are wrong. My patches did solve the original problem, I know\nbecause I was the one that found the original problem.\n\n> The solution to these problems is\n> for contributors and reviewers to _collaborate_ to come up with a\n> better end result, which is often different from both the original\n> patch and the suggestions in the initial review.\n\nCollaboration requires both sides to work on the problem. Not one side\npointing fingers and the other side doing all the work.\n\n> When it is your patch, however, we repeatedly saw that the review\n> process got derailed in the middle.\n\nWhen working collaboratively it's fine to disagree, and it's fine to\nhave two sides come up with two different patches.\n\nIf you disagree with the other side, send a patch that does it properly.\n\nIf the other side doesn't do *exactly* what you want, that's not the\nreview process being derailed.\n\n> If there is no will to collaborate on the contributor's end,\n> however, and the primary thing the contributor wants to do is to\n> endlessly argue, the efforts by reviewers are all wasted. We do not\n> get anywhere.\n\nIn order to have and endless argument, *both sides* need to be engaged\nin the argument. If you decide that a disagreement has been reached,\nthe argument ends in a disagreement.\n\n> That is how I perceive what happens to many of your patches. I am\n> sure you will say \"that is your opinion\", but I do not think I am\n> alone.\n\nThe opinion of a billion people is still an opinion.\n\n> But the thing is, that majority is what writes the majority of the\n> code and does the majority of the reviews, so as maintainer I *do*\n> have to give their opinion a lot of weight, not to mention my own\n> opinion about how to help keep the community the most productive.\n\nIndeed, but that doesn't make it a fact. It remains an opinion.\n\n> And I have to conclude that the cost of having to deal with you\n> outweighs the benefit the project gets out of having you around.\n> Therefore I have ask you to leave and not bother us anymore.\n\nWe shall see.\n\n[1] http://article.gmane.org/gmane.comp.version-control.git/225325\n\n-- \nFelipe Contreras\n"},{"id":"220766","messageId":"CAMP44s1dDsRwk5q4NikA5-O86sKDQ_FQsPB2QnGZBqQZ6tpL1A@mail.gmail.com","threadId":"34068","inReplyTo":"CAMP44s3atPW-SE1yQzep-F6M13g1fPP_q2RqHKofPL0B8=JfYQ@mail.gmail.com","subject":"Re: [PATCH 0/3] Refactor useful notes functions into notes-utils.[ch]","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-06-13T18:50:46Z","receivedAt":"2013-06-13T18:50:46Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Thu, Jun 13, 2013 at 1:16 PM, Felipe Contreras\n wrote:\n> On Thu, Jun 13, 2013 at 12:24 PM, Junio C Hamano wrote:\n\n>> But the thing is, that majority is what writes the majority of the\n>> code and does the majority of the reviews, so as maintainer I *do*\n>> have to give their opinion a lot of weight, not to mention my own\n>> opinion about how to help keep the community the most productive.\n>\n> Indeed, but that doesn't make it a fact. It remains an opinion.\n\nAnd just to make it clear, I didn't deny you are the only one with\ncommit access, and therefore you make all the shots. You made a\ndecision, fine, I never said you can't do that.\n\nWhat I said is that you should not use words to imply that your\n*opinion* is a fact. The fact that you make a decision doesn't make\nyour opinion a fact, and the fact that many people share your opinion\ndoesn't make it a fact either.\n\nSo, instead of saying:\n\n\"Just one side being right, and the other side continuing to repeat\nnonsense without listening.\"\n\nYou should say:\n\n\"Simply a matter of disagreement where the code belongs.\"\n\n-- \nFelipe Contreras\n"}]}