{"thread":{"id":"48583","subject":"[PATCH 0/2] minor show-index modernizations","startedAt":"2018-05-28T09:37:45Z","lastAt":"2018-05-28T17:58:00Z","messageCount":5,"participants":["Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"348652","messageId":"20180528093740.GA6902@sigill.intra.peff.net","threadId":"48583","inReplyTo":null,"subject":"[PATCH 0/2] minor show-index modernizations","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-05-28T09:37:40Z","receivedAt":"2018-05-28T09:37:45Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"While recommending show-index to somebody today, I noticed that it has a\nfew ancient warts. This series turns it into a builtin and fixes the\ndocumentation. I suspect it could do with some more modernization and\nfriendliness, like:\n\n - re-using the existing packfile.c code instead of re-implementing\n index-reading (though the packfile code would definitely need some\n refactoring to make this work)\n\n - it could probably handle arguments as files instead of requiring\n stdin redirection\n\nI'll leave those for now, as they're not worth the effort to me at this\npoint. But everybody who _doesn't_ use the command benefits from making\nit a builtin, so it seems like an easy win.\n\n [1/2]: make show-index a builtin\n [2/2]: show-index: update documentation for index v2\n\n Documentation/git-show-index.txt | 22 ++++++++++++++++------\n Makefile | 2 +-\n builtin.h | 1 +\n show-index.c => builtin/show-index.c | 2 +-\n git.c | 1 +\n 5 files changed, 20 insertions(+), 8 deletions(-)\n rename show-index.c => builtin/show-index.c (96%)\n\n-Peff\n"},{"id":"348653","messageId":"20180528093853.GA20229@sigill.intra.peff.net","threadId":"48583","inReplyTo":"20180528093740.GA6902@sigill.intra.peff.net","subject":"[PATCH 1/2] make show-index a builtin","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-05-28T09:38:53Z","receivedAt":"2018-05-28T09:38:58Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The git-show-index command is built as its own separate\nprogram. There's really no good reason for this, and it\nmeans we waste extra space on disk (and CPU time running the\nlinker). Let's fold it in to the main binary as a builtin.\n\nThe history here is actually a bit amusing. The program\nitself is mostly self-contained, and doesn't even use our\nnormal pack index code. In a5031214c4 (slim down \"git\nshow-index\", 2010-01-21), we even stopped using xmalloc() so\nthat it could avoid libgit.a entirely. But then 040a655116\n(cleanup: use internal memory allocation wrapper functions\neverywhere, 2011-10-06) switched that back to xmalloc, which\nlater become ALLOC_ARRAY().\n\nMaking it a builtin should give us the best of both worlds:\nno wasted space and no need to avoid the usual patterns.\n\nSigned-off-by: Jeff King \n---\n Makefile | 2 +-\n builtin.h | 1 +\n show-index.c => builtin/show-index.c | 2 +-\n git.c | 1 +\n 4 files changed, 4 insertions(+), 2 deletions(-)\n rename show-index.c => builtin/show-index.c (96%)\n\ndiff --git a/Makefile b/Makefile\nindex ad880d1fc5..766c5909bf 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -689,7 +689,6 @@ PROGRAM_OBJS += http-backend.o\n PROGRAM_OBJS += imap-send.o\n PROGRAM_OBJS += sh-i18n--envsubst.o\n PROGRAM_OBJS += shell.o\n-PROGRAM_OBJS += show-index.o\n PROGRAM_OBJS += remote-testsvn.o\n \n # Binary suffix, set to .exe for Windows builds\n@@ -1076,6 +1075,7 @@ BUILTIN_OBJS += builtin/send-pack.o\n BUILTIN_OBJS += builtin/serve.o\n BUILTIN_OBJS += builtin/shortlog.o\n BUILTIN_OBJS += builtin/show-branch.o\n+BUILTIN_OBJS += builtin/show-index.o\n BUILTIN_OBJS += builtin/show-ref.o\n BUILTIN_OBJS += builtin/stripspace.o\n BUILTIN_OBJS += builtin/submodule--helper.o\ndiff --git a/builtin.h b/builtin.h\nindex 4e0f64723e..0362f1ce25 100644\n--- a/builtin.h\n+++ b/builtin.h\n@@ -220,6 +220,7 @@ extern int cmd_serve(int argc, const char **argv, const char *prefix);\n extern int cmd_shortlog(int argc, const char **argv, const char *prefix);\n extern int cmd_show(int argc, const char **argv, const char *prefix);\n extern int cmd_show_branch(int argc, const char **argv, const char *prefix);\n+extern int cmd_show_index(int argc, const char **argv, const char *prefix);\n extern int cmd_status(int argc, const char **argv, const char *prefix);\n extern int cmd_stripspace(int argc, const char **argv, const char *prefix);\n extern int cmd_submodule__helper(int argc, const char **argv, const char *prefix);\ndiff --git a/show-index.c b/builtin/show-index.c\nsimilarity index 96%\nrename from show-index.c\nrename to builtin/show-index.c\nindex 1ead41e211..65fa86dd08 100644\n--- a/show-index.c\n+++ b/builtin/show-index.c\n@@ -4,7 +4,7 @@\n static const char show_index_usage[] =\n \"git show-index\";\n \n-int cmd_main(int argc, const char **argv)\n+int cmd_show_index(int argc, const char **argv, const char *prefix)\n {\n \tint i;\n \tunsigned nr;\ndiff --git a/git.c b/git.c\nindex 5771d62a32..c91e144d9a 100644\n--- a/git.c\n+++ b/git.c\n@@ -470,6 +470,7 @@ static struct cmd_struct commands[] = {\n \t{ \"shortlog\", cmd_shortlog, RUN_SETUP_GENTLY | USE_PAGER },\n \t{ \"show\", cmd_show, RUN_SETUP },\n \t{ \"show-branch\", cmd_show_branch, RUN_SETUP },\n+\t{ \"show-index\", cmd_show_index },\n \t{ \"show-ref\", cmd_show_ref, RUN_SETUP },\n \t{ \"stage\", cmd_add, RUN_SETUP | NEED_WORK_TREE },\n \t{ \"status\", cmd_status, RUN_SETUP | NEED_WORK_TREE },\n-- \n2.17.0.1391.g6fdbf40724\n\n"},{"id":"348654","messageId":"20180528093923.GB20229@sigill.intra.peff.net","threadId":"48583","inReplyTo":"20180528093740.GA6902@sigill.intra.peff.net","subject":"[PATCH 2/2] show-index: update documentation for index v2","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-05-28T09:39:23Z","receivedAt":"2018-05-28T09:39:28Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Commit 32637cdf4a (show-index.c: learn about index v2,\n2007-04-09) changed the output format of show-index to\ninclude the object CRC32 but didn't update the\ndocumentation. Let's fix that and generally describe the\noutput in more detail.\n\nThere are a few other fixes here while we're rewording:\n\n - refer to index-pack along with pack-objects, since either\n can create .idx files\n\n - use \"linkgit:\" for referring to other commands\n\n - expand the bit about verify-pack, giving reasons why you\n might want this command instead. I almost omitted this\n entirely, but referring to verify-pack might help a\n reader who is looking for more information.\n\nSigned-off-by: Jeff King \n---\n Documentation/git-show-index.txt | 26 ++++++++++++++++++++------\n 1 file changed, 20 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/git-show-index.txt b/Documentation/git-show-index.txt\nindex a8a9509e0e..424e4ba84c 100644\n--- a/Documentation/git-show-index.txt\n+++ b/Documentation/git-show-index.txt\n@@ -14,13 +14,27 @@ SYNOPSIS\n \n DESCRIPTION\n -----------\n-Read the idx file for a Git packfile created with\n-'git pack-objects' command from the standard input, and\n-dump its contents.\n+Read the `.idx` file for a Git packfile (created with\n+linkgit:git-pack-objects[1] or linkgit:git-index-pack[1]) from the\n+standard input, and dump its contents. The output consists of one object\n+per line, with each line containing two or three space-separated\n+columns:\n \n-The information it outputs is subset of what you can get from\n-'git verify-pack -v'; this command only shows the packfile\n-offset and SHA-1 of each object.\n+ - the first column is the offset in bytes of the object within the\n+ corresponding packfile\n+\n+ - the second column is the object id of the object\n+\n+ - if the index version is 2 or higher, the third column contains the\n+ CRC32 of the object data\n+\n+The objects are output in the order in which they are found in the index\n+file, which should be (in a correctly constructed file) sorted by object\n+id.\n+\n+Note that you can get more information on a packfile by calling\n+linkgit:git-verify-pack[1]. However, as this command considers only the\n+index file itself, it's both faster and more flexible.\n \n GIT\n ---\n-- \n2.17.0.1391.g6fdbf40724\n"},{"id":"348674","messageId":"xmqqlgc3iyom.fsf@gitster-ct.c.googlers.com","threadId":"48583","inReplyTo":"20180528093853.GA20229@sigill.intra.peff.net","subject":"Re: [PATCH 1/2] make show-index a builtin","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-05-28T15:31:53Z","receivedAt":"2018-05-28T15:32:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> diff --git a/show-index.c b/builtin/show-index.c\n> similarity index 96%\n> rename from show-index.c\n> rename to builtin/show-index.c\n> index 1ead41e211..65fa86dd08 100644\n> --- a/show-index.c\n> +++ b/builtin/show-index.c\n> @@ -4,7 +4,7 @@\n\nI squashed\n\n#include \"builtin.h\"\n\nnear the beginning of this file to squelch -DDEVELOPER extra\nwarnings. Otherwise looks obviously good.\n\n> static const char show_index_usage[] =\n> \"git show-index\";\n> \n> -int cmd_main(int argc, const char **argv)\n> +int cmd_show_index(int argc, const char **argv, const char *prefix)\n> {\n> \tint i;\n> \tunsigned nr;\n> diff --git a/git.c b/git.c\n> index 5771d62a32..c91e144d9a 100644\n> --- a/git.c\n> +++ b/git.c\n> @@ -470,6 +470,7 @@ static struct cmd_struct commands[] = {\n> \t{ \"shortlog\", cmd_shortlog, RUN_SETUP_GENTLY | USE_PAGER },\n> \t{ \"show\", cmd_show, RUN_SETUP },\n> \t{ \"show-branch\", cmd_show_branch, RUN_SETUP },\n> +\t{ \"show-index\", cmd_show_index },\n\nHmph, this does not need SETUP? Ah, of course, because its\nsubcommand body used to do everything itself, and this patch just\nturns cmd_main() to cmd_show_index().\n\n\n\n> \t{ \"show-ref\", cmd_show_ref, RUN_SETUP },\n> \t{ \"stage\", cmd_add, RUN_SETUP | NEED_WORK_TREE },\n> \t{ \"status\", cmd_status, RUN_SETUP | NEED_WORK_TREE },\n"},{"id":"348679","messageId":"20180528175754.GA20636@sigill.intra.peff.net","threadId":"48583","inReplyTo":"xmqqlgc3iyom.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 1/2] make show-index a builtin","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-05-28T17:57:55Z","receivedAt":"2018-05-28T17:58:00Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, May 29, 2018 at 12:31:53AM +0900, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > diff --git a/show-index.c b/builtin/show-index.c\n> > similarity index 96%\n> > rename from show-index.c\n> > rename to builtin/show-index.c\n> > index 1ead41e211..65fa86dd08 100644\n> > --- a/show-index.c\n> > +++ b/builtin/show-index.c\n> > @@ -4,7 +4,7 @@\n> \n> I squashed\n> \n> #include \"builtin.h\"\n> \n> near the beginning of this file to squelch -DDEVELOPER extra\n> warnings. Otherwise looks obviously good.\n\nThanks. I'm still using my custom build options, and obviously I need to\nadd -Wmissing-prototypes.\n\n> > diff --git a/git.c b/git.c\n> > index 5771d62a32..c91e144d9a 100644\n> > --- a/git.c\n> > +++ b/git.c\n> > @@ -470,6 +470,7 @@ static struct cmd_struct commands[] = {\n> > \t{ \"shortlog\", cmd_shortlog, RUN_SETUP_GENTLY | USE_PAGER },\n> > \t{ \"show\", cmd_show, RUN_SETUP },\n> > \t{ \"show-branch\", cmd_show_branch, RUN_SETUP },\n> > +\t{ \"show-index\", cmd_show_index },\n> \n> Hmph, this does not need SETUP? Ah, of course, because its\n> subcommand body used to do everything itself, and this patch just\n> turns cmd_main() to cmd_show_index().\n\nIt is not even that it does the setup itself. It does not need the setup\nat all, because it is purely a mechanical parsing of the stdin stream\nto text output. No repo required, and it does not know or care if the\nmatching packfile even exists.\n\n-Peff\n"}]}