{"thread":{"id":"14300","subject":"[PATCH/v3] bundle.c: added --stdin option to git-bundle","startedAt":"2008-07-05T16:30:53Z","lastAt":"2008-07-06T14:28:58Z","messageCount":18,"participants":["Adam Brewster","Jakub Narebski","Junio C Hamano","Miklos Vajna"],"isPatch":true,"patchVersion":3,"patchTotal":null},"messages":[{"id":"82287","messageId":"c376da900807050930i6d1da898s624be58adc6f1751@mail.gmail.com","threadId":"14300","inReplyTo":null,"subject":"[PATCH/v3] bundle.c: added --stdin option to git-bundle","fromName":"Adam Brewster","fromEmail":"adambrewster@gmail.com","sentAt":"2008-07-05T16:30:53Z","receivedAt":"2008-07-05T16:30:53Z","isPatch":true,"sender":{"key":"adambrewster@gmail.com","avatar":"https://avatars.githubusercontent.com/u/223816?v=4"},"body":"Signed-off-by: Adam Brewster <asb@bu.edu>\n---\nIt seems that the consensus is that the other half of my original\npatch is no good.  You have some pretty good ideas about how to\ncorrectly address the problem I was trying to solve, and I look\nforward to seeing them actually implemented.\n\nFor now, I offer separately the modification I made to bundle.c to\nallow git-bundle to handle the --stdin option.  There is no\naccompanying change to the documentation because it already implies\nthat this option is available.\n\n bundle.c |   22 ++++++++++++++++++++--\n 1 files changed, 20 insertions(+), 2 deletions(-)\n\ndiff --git a/bundle.c b/bundle.c\nindex 0ba5df1..b44a4af 100644\n--- a/bundle.c\n+++ b/bundle.c\n@@ -227,8 +227,26 @@ int create_bundle(struct bundle_header *header,\nconst char *path,\n\n        /* write references */\n        argc = setup_revisions(argc, argv, &revs, NULL);\n-       if (argc > 1)\n-               return error(\"unrecognized argument: %s'\", argv[1]);\n+\n+       for (i = 1; i < argc; i++) {\n+               if ( !strcmp(argv[i], \"--stdin\") ) {\n+                       char line[1000];\n+                               while (fgets(line, sizeof(line),\nstdin) != NULL) {\n+                               int len = strlen(line);\n+                               if (len && line[len - 1] == '\\n')\n+                                       line[--len] = '\\0';\n+                               if (!len)\n+                                       break;\n+                               if (line[0] == '-')\n+                                       die(\"options not supported in\n--stdin mode\");\n+                               if (handle_revision_arg(line, &revs, 0, 1))\n+                                       die(\"bad revision '%s'\", line);\n+                       }\n+                       continue;\n+               }\n+\n+               return error(\"unrecognized argument: %s'\", argv[i]);\n+       }\n\n        for (i = 0; i < revs.pending.nr; i++) {\n                struct object_array_entry *e = revs.pending.objects + i;\n--\n1.5.5.1.211.g65ea3.dirty\n"},{"id":"82288","messageId":"200807051854.46798.jnareb@gmail.com","threadId":"14300","inReplyTo":"c376da900807050930i6d1da898s624be58adc6f1751@mail.gmail.com","subject":"Re: [PATCH/v3] bundle.c: added --stdin option to git-bundle","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-07-05T16:54:45Z","receivedAt":"2008-07-05T16:54:45Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Sat, 5 Jul 2008, Adam Brewster wrote:\n\n> It seems that the consensus is that the other half of my original\n> patch is no good.  You have some pretty good ideas about how to\n> correctly address the problem I was trying to solve, and I look\n> forward to seeing them actually implemented.\n\nIt is not that the other half is \"no good\", it is rather that there\nis no consensus how and in what way it should be implemented; be\nit separate git-bases command, git-bundle--bases helper script, or\nincorporated in git-bundle code; should it be written in Perl or\nas shell script (only in case of git-bases or git-bundle--bases),\nor should it be written in C.\n\nAdding some documentation, with example usage (example \"workflows\")\nwould help adding git-bases to git core... perhaps at start send\nit as script in contrib/ ?\n\n> For now, I offer separately the modification I made to bundle.c to\n> allow git-bundle to handle the --stdin option.  \n\nThat's the way it is preferred here on git mailing list, to not bundle \nnon-controversial change with the one that needs (or seem to need) some \nfurther discussion - do not hold features hostage ;-)\n\n> There is no accompanying change to the documentation because it\n> already implies that this option is available.\n\nAdding an example of using `--stdin' to \"[git-rev-list-args...]::\"\nin Documentation/git-bundle.txt would be good.\n\n-- \nJakub Narebski\nPoland\n"},{"id":"82294","messageId":"7vod5crydx.fsf@gitster.siamese.dyndns.org","threadId":"14300","inReplyTo":"c376da900807050930i6d1da898s624be58adc6f1751@mail.gmail.com","subject":"Re: [PATCH/v3] bundle.c: added --stdin option to git-bundle","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-07-05T18:15:06Z","receivedAt":"2008-07-05T18:15:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Adam Brewster\" <adambrewster@gmail.com> writes:\n\n> Subject: Re: [PATCH/v3] bundle.c: added --stdin option to git-bundle\n\nWhen the change is not about implementation detail (in which case you do\nwant to name the source file and perhaps even a function name), but about\na new feature that is visible to the end-users of a command, we'd want the\nmessage talk in terms of what the new feature does, not how the new\nfeature is invoked nor where it is implemented.  In other words, something\nlike these are preferred:\n\n\tgit-bundle: add --stdin\n        Teach git-bundle to read tips and basis from standard input\n\nand don't say \"You did\" in past tense --- say things in imperative mood\ninstead, as if you are commanding the person who applies the patch to make\nit happen.  Older log entries in our history (e.g. \"git log -n 20 v0.99\")\nmay give you a better feel.\n\nAnd give a few lines of obvious justfication in the body of the commit log\nmessage, e.g.\n\n\tThis patch allows the caller to feed the revision parameters to\n\tgit-bundle from its standard input.  This way, a script do not\n\thave to worry about limitation of the length of command line.\n\nto explain why this is good.  In order to explain that you may have to\ntalk about other things (like what it does and how it does it), but keep\nin mind that the primary thing you should talk about is _why_.\n\n> ... because it already implies that this option is available.\n\nIf that is the case, please mention in the commit log message something\nlike \"Even though the documentation said \"bundle --stdin\" is accepted it\ndidn't.  This patch teaches the option to the command\".\n\nBut I do not think there is no such implication.  \"bundle create\" may take\nlist of positive and negative refs as arguments or --branches, but it does\nnot take (and it shouldn't -- I do not think it should take --bisect\noption, for example) artibrary options that rev-list command accepts.\n\n>  bundle.c |   22 ++++++++++++++++++++--\n>  1 files changed, 20 insertions(+), 2 deletions(-)\n>\n> diff --git a/bundle.c b/bundle.c\n> index 0ba5df1..b44a4af 100644\n> --- a/bundle.c\n> +++ b/bundle.c\n> @@ -227,8 +227,26 @@ int create_bundle(struct bundle_header *header,\n> const char *path,\n\nWrapped.\n\n>         /* write references */\n>         argc = setup_revisions(argc, argv, &revs, NULL);\n> -       if (argc > 1)\n> -               return error(\"unrecognized argument: %s'\", argv[1]);\n> +\n> +       for (i = 1; i < argc; i++) {\n> +               if ( !strcmp(argv[i], \"--stdin\") ) {\n\nStyle.\n\n> +                       char line[1000];\n> +                               while (fgets(line, sizeof(line),\n> stdin) != NULL) {\n\nToo deep indentation.  Wrapped.\n\n> +                               int len = strlen(line);\n> +                               if (len && line[len - 1] == '\\n')\n> +                                       line[--len] = '\\0';\n> +                               if (!len)\n> +                                       break;\n> +                               if (line[0] == '-')\n> +                                       die(\"options not supported in\n> --stdin mode\");\n> +                               if (handle_revision_arg(line, &revs, 0, 1))\n> +                                       die(\"bad revision '%s'\", line);\n> +                       }\n> +                       continue;\n> +               }\n> +\n> +               return error(\"unrecognized argument: %s'\", argv[i]);\n> +       }\n\nHaving said that, I think copying and pasting read_revisions_from_stdin()\nin bundle.c is a wrong approach to take.  Probably the function can easily\nbe split out of builtin-rev-list.c and moved to revision.c or somewhere\n(which will be the first patch), and then a separate patch can add a few\nlines to call it from bundle.c.\n"},{"id":"82299","messageId":"1215290434-27694-1-git-send-email-adambrewster@gmail.com","threadId":"14300","inReplyTo":"7vod5crydx.fsf@gitster.siamese.dyndns.org","subject":"[PATCH v4 0/3]","fromName":"Adam Brewster","fromEmail":"adambrewster@gmail.com","sentAt":"2008-07-05T20:40:31Z","receivedAt":"2008-07-05T20:40:31Z","isPatch":true,"sender":{"key":"adambrewster@gmail.com","avatar":"https://avatars.githubusercontent.com/u/223816?v=4"},"body":"\nSorry for the idiotic wrapping problems in my last email.\n\nPreviously, I was trying to keep from changing any of the important stuff,\nlike git-rev-list, but I should know better than cut-and-pasting code.\n\nAs requested, I've broken the change into a multiple of patches.  First moving\nread_revisions_from_stdin to revision.c, next modifying git-bundle to handle\n--stdin, and finally a patch adding my old git-basis to contrib.\n\nI think I've corrected all of the style issues you pointed out, and I've also \ntried to craft more informative commit messages.\n"},{"id":"82301","messageId":"1215290434-27694-2-git-send-email-adambrewster@gmail.com","threadId":"14300","inReplyTo":"1215290434-27694-1-git-send-email-adambrewster@gmail.com","subject":"[PATCH] Move read_revisions_from_stdin from builtin-rev-list.c to revision.c","fromName":"Adam Brewster","fromEmail":"adambrewster@gmail.com","sentAt":"2008-07-05T20:40:32Z","receivedAt":"2008-07-05T20:40:32Z","isPatch":true,"sender":{"key":"adambrewster@gmail.com","avatar":"https://avatars.githubusercontent.com/u/223816?v=4"},"body":"Some other commands might like to support the --stdin option like\ngit-rev-list.  Since they don't want to depend on builtin-rev-list, the\nfunction has to be somewhere else.\n\nSigned-off-by: Adam Brewster <asb@bu.edu>\n---\n builtin-rev-list.c |   17 -----------------\n revision.c         |   17 +++++++++++++++++\n 2 files changed, 17 insertions(+), 17 deletions(-)\n mode change 100644 => 100755 builtin-rev-list.c\n mode change 100644 => 100755 revision.c\n\ndiff --git a/builtin-rev-list.c b/builtin-rev-list.c\nold mode 100644\nnew mode 100755\nindex 11a7eae..b4a2c44\n--- a/builtin-rev-list.c\n+++ b/builtin-rev-list.c\n@@ -575,23 +575,6 @@ static struct commit_list *find_bisection(struct commit_list *list,\n \treturn best;\n }\n \n-static void read_revisions_from_stdin(struct rev_info *revs)\n-{\n-\tchar line[1000];\n-\n-\twhile (fgets(line, sizeof(line), stdin) != NULL) {\n-\t\tint len = strlen(line);\n-\t\tif (len && line[len - 1] == '\\n')\n-\t\t\tline[--len] = 0;\n-\t\tif (!len)\n-\t\t\tbreak;\n-\t\tif (line[0] == '-')\n-\t\t\tdie(\"options not supported in --stdin mode\");\n-\t\tif (handle_revision_arg(line, revs, 0, 1))\n-\t\t\tdie(\"bad revision '%s'\", line);\n-\t}\n-}\n-\n int cmd_rev_list(int argc, const char **argv, const char *prefix)\n {\n \tstruct commit_list *list;\ndiff --git a/revision.c b/revision.c\nold mode 100644\nnew mode 100755\nindex 5a1a948..0191160\n--- a/revision.c\n+++ b/revision.c\n@@ -911,6 +911,23 @@ int handle_revision_arg(const char *arg, struct rev_info *revs,\n \treturn 0;\n }\n \n+void read_revisions_from_stdin(struct rev_info *revs)\n+{\n+\tchar line[1000];\n+\n+\twhile (fgets(line, sizeof(line), stdin) != NULL) {\n+\t\tint len = strlen(line);\n+\t\tif (len && line[len - 1] == '\\n')\n+\t\t\tline[--len] = '\\0';\n+\t\tif (!len)\n+\t\t\tbreak;\n+\t\tif (line[0] == '-')\n+\t\t\tdie(\"options not supported in --stdin mode\");\n+\t\tif (handle_revision_arg(line, revs, 0, 1))\n+\t\t\tdie(\"bad revision '%s'\", line);\n+\t}\n+}\n+\n static void add_grep(struct rev_info *revs, const char *ptn, enum grep_pat_token what)\n {\n \tif (!revs->grep_filter) {\n-- \n1.5.5.1.211.g65ea3.dirty\n"},{"id":"82300","messageId":"1215290434-27694-3-git-send-email-adambrewster@gmail.com","threadId":"14300","inReplyTo":"1215290434-27694-2-git-send-email-adambrewster@gmail.com","subject":"[PATCH] git-bundle: add --stdin","fromName":"Adam Brewster","fromEmail":"adambrewster@gmail.com","sentAt":"2008-07-05T20:40:33Z","receivedAt":"2008-07-05T20:40:33Z","isPatch":true,"sender":{"key":"adambrewster@gmail.com","avatar":"https://avatars.githubusercontent.com/u/223816?v=4"},"body":"Teach git-bundle to read revision arguments from stdin like git-rev-list.\n\nThis patch allows the caller to feed the revision parameters to git-bundle\nfrom its standard input.  This way, a script do not have to worry about\nlimitation of the length of command line.\n\nDocumentation/git-bundle.txt says that git-bundle takes arguments acceptable\nto git-rev-list.  Obviously some arguments that git-rev-list handles don't\nmake sense for git-bundle (e.g. --bisect) but --stdin is pretty reasonable.\n\nSigned-off-by: Adam Brewster <asb@bu.edu>\n---\n bundle.c |   13 +++++++++++--\n 1 files changed, 11 insertions(+), 2 deletions(-)\n mode change 100644 => 100755 bundle.c\n\ndiff --git a/bundle.c b/bundle.c\nold mode 100644\nnew mode 100755\nindex 0ba5df1..00b2aab\n--- a/bundle.c\n+++ b/bundle.c\n@@ -178,6 +178,7 @@ int create_bundle(struct bundle_header *header, const char *path,\n \tint i, ref_count = 0;\n \tchar buffer[1024];\n \tstruct rev_info revs;\n+\tint read_from_stdin = 0;\n \tstruct child_process rls;\n \tFILE *rls_fout;\n \n@@ -227,8 +228,16 @@ int create_bundle(struct bundle_header *header, const char *path,\n \n \t/* write references */\n \targc = setup_revisions(argc, argv, &revs, NULL);\n-\tif (argc > 1)\n-\t\treturn error(\"unrecognized argument: %s'\", argv[1]);\n+\n+\tfor (i = 1; i < argc; i++) {\n+\t\tif (!strcmp(argv[i], \"--stdin\")) {\n+\t\t\tif (read_from_stdin++)\n+\t\t\t\tdie(\"--stdin given twice?\");\n+\t\t\tread_revisions_from_stdin(&revs);\n+\t\t\tcontinue;\n+\t\t}\n+\t\treturn error(\"unrecognized argument: %s'\", argv[i]);\n+\t}\n \n \tfor (i = 0; i < revs.pending.nr; i++) {\n \t\tstruct object_array_entry *e = revs.pending.objects + i;\n-- \n1.5.5.1.211.g65ea3.dirty\n"},{"id":"82302","messageId":"1215290434-27694-4-git-send-email-adambrewster@gmail.com","threadId":"14300","inReplyTo":"1215290434-27694-3-git-send-email-adambrewster@gmail.com","subject":"[PATCH] Add git-basis.perl to contrib directory","fromName":"Adam Brewster","fromEmail":"adambrewster@gmail.com","sentAt":"2008-07-05T20:40:34Z","receivedAt":"2008-07-05T20:40:34Z","isPatch":true,"sender":{"key":"adambrewster@gmail.com","avatar":"https://avatars.githubusercontent.com/u/223816?v=4"},"body":"Git-basis is a perl script that remembers bases for use by git-bundle.\n\nThis script shouldn't be needed because git-push and git-remote should do this\nkind of work.  Unfortunately they currently don't so some might find this\nscript useful.\n\nSigned-off-by: Adam Brewster <asb@bu.edu>\n---\n contrib/basis/git-basis.perl |   77 ++++++++++++++++++++++++++++++++++++\n contrib/basis/git-basis.txt  |   90 ++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 167 insertions(+), 0 deletions(-)\n create mode 100755 contrib/basis/git-basis.perl\n create mode 100644 contrib/basis/git-basis.txt\n\ndiff --git a/contrib/basis/git-basis.perl b/contrib/basis/git-basis.perl\nnew file mode 100755\nindex 0000000..b3e753f\n--- /dev/null\n+++ b/contrib/basis/git-basis.perl\n@@ -0,0 +1,77 @@\n+#!/usr/bin/perl\n+\n+use strict;\n+\n+use Git;\n+\n+require Time::Local;\n+my $git_epoch = Time::Local::timegm(0, 0, 0, 1, 0, 70);\n+\n+my $r = Git->repository();\n+my $d = $r->repo_path();\n+\n+if ( ! -d \"$d/bases\" ) {\n+    mkdir(\"$d/bases\") || die \"Could not create $d/bases: $!\";\n+}\n+\n+if ( $#ARGV == -1 || ($#ARGV == 0 && $ARGV[0] eq '--update') ) {\n+    print STDERR \"usage: git-basis [--update] basis1...\\n\";\n+    exit;\n+} elsif ( $ARGV[0] eq '--update' ) {\n+    shift @ARGV;\n+\n+    my %new = ();\n+    while (<STDIN>) {\n+\tif (!/^^?([a-z0-9]{40})/) {next;}\n+\t$new{$1} = 1;\n+    }\n+\n+    foreach my $f (@ARGV) {\n+\tmy %these = ();\n+\tmy $fh;\n+\n+\topen $fh, \"+<$d/bases/$f\" || die \"Can't open bases/$f: $!\";\n+\twhile (<$fh>) {\n+\t    if (!/^([a-z0-9]{40})/) {next;}\n+\t    $these{$1} = 1;\n+\t}\n+\n+\tprint $fh \"# \", gmtime() - $git_epoch,\n+\t\t\" +0000 // \", scalar(localtime()), \"\\n\";\n+\n+\tforeach my $b (keys %new) {\n+\t    if (exists($these{$b})) {next;}\n+\t    print $fh \"$b\\n\";\n+\t}\n+\tclose $fh;\n+    }\n+} else {\n+    my $n = 0;\n+    my %basis = ();\n+\n+    my $f = shift @ARGV;\n+    open F, \"<$d/bases/$f\" || die \"Can't open bases/$f: $!\";\n+    while (<F>) {\n+\tif (!/^([a-z0-9]{40})/) {next;}\n+\t$basis{$1} = $n;\n+    }\n+    close F;\n+\n+    foreach $f (@ARGV) {\n+\topen F, \"<$d/bases/$f\" || die \"Can't open bases/$f: $!\";\n+\twhile (<F>) {\n+\t    if (!/^([a-z0-9]{40})/) {next;}\n+\t    if (!exists($basis{$1})) {next;}\n+\n+\t    if ($basis{$1} == $n) {$basis{$1}++;}\n+\t    else {delete $basis{$1};}\n+\t}\n+\tclose F;\n+\t$n++;\n+    }\n+\n+    foreach my $b (keys %basis) {\n+\tif ( $basis{$b} != $n ) {next;}\n+\tprint \"^$b\\n\";\n+    }\n+}\ndiff --git a/contrib/basis/git-basis.txt b/contrib/basis/git-basis.txt\nnew file mode 100644\nindex 0000000..97cfc20\n--- /dev/null\n+++ b/contrib/basis/git-basis.txt\n@@ -0,0 +1,90 @@\n+git-basis(1)\n+============\n+\n+NAME\n+----\n+git-basis - Track sets of references available on remote systems (bases)\n+\n+SYNOPSIS\n+--------\n+[verse]\n+'git-basis' <basis> [<basis>...]\n+'git-basis' --update <basis> [<basis>...] < <object list or bundle>\n+\n+DESCRIPTION\n+-----------\n+Maintains lists of objects that are known to be accessible on remote\n+computer systems that are not accessible by network.\n+\n+OPTIONS\n+-------\n+\n+basis::\n+\tList of bases to operate on.  Any valid filename can be\n+\tthe name of a basis.  Bases that do not exist are taken\n+\tto be empty.\n+\n+--update::\n+\tTells git-basis to read a list of objects from stdin and\n+\tadd them to each of the given bases.  git-basis produces\n+\tno output when this option is given.  Bases will be created\n+\tif necessary.\n+\n+object list or bundle::\n+\tGit-basis --update reads object names, one per line from stdin.\n+\tLeading caret (\"^\") characters are ignored, as is anything\n+\tafter the object name.  Lines that don't begin with an object\n+\tname are ignored.  The output of linkgit:git-ls-remote[1] or a\n+\tbundle created by linkgit:git-bundle[1] are both suitable input.\n+\n+DISCUSSION\n+----------\n+git-basis is probably only useful with linkgit:git-bundle[1].\n+\n+To create a bundle that excludes all objects that are part of my-basis,\n+use\n+\n+git-basis my-basis | git-bundle create my-bundle --all --stdin\n+\n+To add the objects in my-bundle to my-basis, use\n+\n+git-basis --update my-basis < my-bundle\n+\n+DETAILS\n+-------\n+Bases are stored as plain text files under .git/bases/.  One object\n+entry per line.\n+\n+git-basis without --update reads all of the basis names given on the\n+command line, and outputs the intersection of them to stdout, with each\n+object prefixed by \"^\".\n+\n+git-basis --update reads object names from stdin, and adds all of the\n+references to each of the bases listed.  Duplicate references will not\n+be listed twice, but otherwise redundant information will be included.\n+Each update is prefixed by a line with the current date.\n+\n+BUGS\n+----\n+Git-baisis has no undo function.  Once an object is added to a basis,\n+it will stay there forever.  If you need to remove objects from a basis,\n+use a text editor to alter the file .git/bases/<basis name>.\n+\n+Git-basis --update does not remove redundant information from bases.\n+(Having an object implies that it's parents are also available.)  This\n+is done intentionally to make sure git-basis --update is\n+non-destructive.\n+\n+Bug reports are welcome, and patches are encouraged.\n+\n+SEE ALSO\n+--------\n+linkgit:git-bundle[1]\n+\n+AUTHOR\n+------\n+Written by Adam Brewster <asb@bu.edu>\n+\n+GIT\n+---\n+Part of the linkgit:git[1] suite\n-- \n1.5.5.1.211.g65ea3.dirty\n"},{"id":"82303","messageId":"20080705204849.GJ4729@genesis.frugalware.org","threadId":"14300","inReplyTo":"1215290434-27694-2-git-send-email-adambrewster@gmail.com","subject":"Re: [PATCH] Move read_revisions_from_stdin from builtin-rev-list.c to revision.c","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2008-07-05T20:48:49Z","receivedAt":"2008-07-05T20:48:49Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"On Sat, Jul 05, 2008 at 04:40:32PM -0400, Adam Brewster <adambrewster@gmail.com> wrote:\n> Some other commands might like to support the --stdin option like\n> git-rev-list.  Since they don't want to depend on builtin-rev-list, the\n> function has to be somewhere else.\n\nI think it's fine to move such a function, but this is a false commit\nmessage, you can use read_revisions_from_stdin() from builtin-bundle if\nit lives in builtin-rev-list.c as well.\n\n>  mode change 100644 => 100755 builtin-rev-list.c\n>  mode change 100644 => 100755 revision.c\n\nHm? ;-)\n"},{"id":"82308","messageId":"1215293200-28199-1-git-send-email-adambrewster@gmail.com","threadId":"14300","inReplyTo":"20080705204849.GJ4729@genesis.frugalware.org","subject":"[PATCH v5]","fromName":"Adam Brewster","fromEmail":"adambrewster@gmail.com","sentAt":"2008-07-05T21:26:38Z","receivedAt":"2008-07-05T21:26:38Z","isPatch":true,"sender":{"key":"adambrewster@gmail.com","avatar":"https://avatars.githubusercontent.com/u/223816?v=4"},"body":"\nApparently I'm dumber than I thought.  Here's what they look like without \nrandom and unnecessary mode changes.  The patch to add git-basis under contrib \nis not affected.\n\nThe real reason read_revisions_from_stdin moved to revision.c is because I was \nasked to do it that way.  If my commit message doesn't accurately describe the \nreason for the change, go ahead and edit the message, or let me know what the \nreal reason is so I can provide a better message.\n\nAdam\n"},{"id":"82307","messageId":"1215293200-28199-2-git-send-email-adambrewster@gmail.com","threadId":"14300","inReplyTo":"1215293200-28199-1-git-send-email-adambrewster@gmail.com","subject":"[PATCH] Move read_revisions_from_stdin from builtin-rev-list.c to revision.c","fromName":"Adam Brewster","fromEmail":"adambrewster@gmail.com","sentAt":"2008-07-05T21:26:39Z","receivedAt":"2008-07-05T21:26:39Z","isPatch":true,"sender":{"key":"adambrewster@gmail.com","avatar":"https://avatars.githubusercontent.com/u/223816?v=4"},"body":"Some other commands might like to support the --stdin option like\ngit-rev-list.  Since they don't want to depend on builtin-rev-list, the\nfunction has to be somewhere else.\n\nSigned-off-by: Adam Brewster <asb@bu.edu>\n---\n builtin-rev-list.c |   17 -----------------\n revision.c         |   17 +++++++++++++++++\n 2 files changed, 17 insertions(+), 17 deletions(-)\n\ndiff --git a/builtin-rev-list.c b/builtin-rev-list.c\nindex 11a7eae..b4a2c44 100644\n--- a/builtin-rev-list.c\n+++ b/builtin-rev-list.c\n@@ -575,23 +575,6 @@ static struct commit_list *find_bisection(struct commit_list *list,\n \treturn best;\n }\n \n-static void read_revisions_from_stdin(struct rev_info *revs)\n-{\n-\tchar line[1000];\n-\n-\twhile (fgets(line, sizeof(line), stdin) != NULL) {\n-\t\tint len = strlen(line);\n-\t\tif (len && line[len - 1] == '\\n')\n-\t\t\tline[--len] = 0;\n-\t\tif (!len)\n-\t\t\tbreak;\n-\t\tif (line[0] == '-')\n-\t\t\tdie(\"options not supported in --stdin mode\");\n-\t\tif (handle_revision_arg(line, revs, 0, 1))\n-\t\t\tdie(\"bad revision '%s'\", line);\n-\t}\n-}\n-\n int cmd_rev_list(int argc, const char **argv, const char *prefix)\n {\n \tstruct commit_list *list;\ndiff --git a/revision.c b/revision.c\nindex 5a1a948..0191160 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -911,6 +911,23 @@ int handle_revision_arg(const char *arg, struct rev_info *revs,\n \treturn 0;\n }\n \n+void read_revisions_from_stdin(struct rev_info *revs)\n+{\n+\tchar line[1000];\n+\n+\twhile (fgets(line, sizeof(line), stdin) != NULL) {\n+\t\tint len = strlen(line);\n+\t\tif (len && line[len - 1] == '\\n')\n+\t\t\tline[--len] = '\\0';\n+\t\tif (!len)\n+\t\t\tbreak;\n+\t\tif (line[0] == '-')\n+\t\t\tdie(\"options not supported in --stdin mode\");\n+\t\tif (handle_revision_arg(line, revs, 0, 1))\n+\t\t\tdie(\"bad revision '%s'\", line);\n+\t}\n+}\n+\n static void add_grep(struct rev_info *revs, const char *ptn, enum grep_pat_token what)\n {\n \tif (!revs->grep_filter) {\n-- \n1.5.5.1.211.g65ea3.dirty\n"},{"id":"82309","messageId":"1215293200-28199-3-git-send-email-adambrewster@gmail.com","threadId":"14300","inReplyTo":"1215293200-28199-2-git-send-email-adambrewster@gmail.com","subject":"[PATCH] git-bundle: add --stdin","fromName":"Adam Brewster","fromEmail":"adambrewster@gmail.com","sentAt":"2008-07-05T21:26:40Z","receivedAt":"2008-07-05T21:26:40Z","isPatch":true,"sender":{"key":"adambrewster@gmail.com","avatar":"https://avatars.githubusercontent.com/u/223816?v=4"},"body":"Teach git-bundle to read revision arguments from stdin like git-rev-list.\n\nThis patch allows the caller to feed the revision parameters to git-bundle\nfrom its standard input.  This way, a script do not have to worry about\nlimitation of the length of command line.\n\nDocumentation/git-bundle.txt says that git-bundle takes arguments acceptable\nto git-rev-list.  Obviously some arguments that git-rev-list handles don't\nmake sense for git-bundle (e.g. --bisect) but --stdin is pretty reasonable.\n\nSigned-off-by: Adam Brewster <adambrewster@gmail.com>\n---\n bundle.c |   13 +++++++++++--\n 1 files changed, 11 insertions(+), 2 deletions(-)\n\ndiff --git a/bundle.c b/bundle.c\nindex 0ba5df1..00b2aab 100644\n--- a/bundle.c\n+++ b/bundle.c\n@@ -178,6 +178,7 @@ int create_bundle(struct bundle_header *header, const char *path,\n \tint i, ref_count = 0;\n \tchar buffer[1024];\n \tstruct rev_info revs;\n+\tint read_from_stdin = 0;\n \tstruct child_process rls;\n \tFILE *rls_fout;\n \n@@ -227,8 +228,16 @@ int create_bundle(struct bundle_header *header, const char *path,\n \n \t/* write references */\n \targc = setup_revisions(argc, argv, &revs, NULL);\n-\tif (argc > 1)\n-\t\treturn error(\"unrecognized argument: %s'\", argv[1]);\n+\n+\tfor (i = 1; i < argc; i++) {\n+\t\tif (!strcmp(argv[i], \"--stdin\")) {\n+\t\t\tif (read_from_stdin++)\n+\t\t\t\tdie(\"--stdin given twice?\");\n+\t\t\tread_revisions_from_stdin(&revs);\n+\t\t\tcontinue;\n+\t\t}\n+\t\treturn error(\"unrecognized argument: %s'\", argv[i]);\n+\t}\n \n \tfor (i = 0; i < revs.pending.nr; i++) {\n \t\tstruct object_array_entry *e = revs.pending.objects + i;\n-- \n1.5.5.1.211.g65ea3.dirty\n"},{"id":"82316","messageId":"7vbq1brfrb.fsf@gitster.siamese.dyndns.org","threadId":"14300","inReplyTo":"20080705204849.GJ4729@genesis.frugalware.org","subject":"Re: [PATCH] Move read_revisions_from_stdin from builtin-rev-list.c to revision.c","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-07-06T00:57:28Z","receivedAt":"2008-07-06T00:57:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Miklos Vajna <vmiklos@frugalware.org> writes:\n\n> I think it's fine to move such a function, but this is a false commit\n> message, you can use read_revisions_from_stdin() from builtin-bundle if\n> it lives in builtin-rev-list.c as well.\n\nAt the mechanical level, yes you _can_, but it is simply a bad taste to do\nso.  More library-ish files such as revision.c are better home for utility\nfunctions to be shared between builtins and commands.\n"},{"id":"82317","messageId":"7v63rjrfqz.fsf@gitster.siamese.dyndns.org","threadId":"14300","inReplyTo":"1215293200-28199-3-git-send-email-adambrewster@gmail.com","subject":"Re: Teach git-bundle to read revision arguments from stdin like git-rev-list","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-07-06T00:57:40Z","receivedAt":"2008-07-06T00:57:40Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Adam Brewster <adambrewster@gmail.com> writes:\n\n> @@ -227,8 +228,16 @@ int create_bundle(struct bundle_header *header, const char *path,\n>  \n>  \t/* write references */\n>  \targc = setup_revisions(argc, argv, &revs, NULL);\n> -\tif (argc > 1)\n> -\t\treturn error(\"unrecognized argument: %s'\", argv[1]);\n> +\n> +\tfor (i = 1; i < argc; i++) {\n> +\t\tif (!strcmp(argv[i], \"--stdin\")) {\n> +\t\t\tif (read_from_stdin++)\n> +\t\t\t\tdie(\"--stdin given twice?\");\n\nHmm, do we deeply care about this case?  What bad things coulc happen if\nyou call read_revisions_from_stdin() twice?\n\n> +\t\t\tread_revisions_from_stdin(&revs);\n> +\t\t\tcontinue;\n> +\t\t}\n> +\t\treturn error(\"unrecognized argument: %s'\", argv[i]);\n> +\t}\n>  \n>  \tfor (i = 0; i < revs.pending.nr; i++) {\n>  \t\tstruct object_array_entry *e = revs.pending.objects + i;\n"},{"id":"82323","messageId":"7vskunpyqz.fsf@gitster.siamese.dyndns.org","threadId":"14300","inReplyTo":"1215293200-28199-1-git-send-email-adambrewster@gmail.com","subject":"Re: [PATCH v5]","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-07-06T01:50:12Z","receivedAt":"2008-07-06T01:50:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Adam Brewster <adambrewster@gmail.com> writes:\n\n> The real reason read_revisions_from_stdin moved to revision.c is because I was \n> asked to do it that way.\n\nYeah, it is simply a bad taste to use helper in builtin-A from builtin-B.\nMore library-ish files such as revision.c are better home for utility\nfunctions to be shared between builtins and commands.\n\nHere is what I queued.\n\nBy the way did you compile test your fix before sending?\n\n-- >8 --\nMove read_revisions_from_stdin from builtin-rev-list.c to revision.c\n\nReading rev-list parameters from the command line can be reused by\ncommands other than rev-list.  Move this function to more \"library-ish\"\nplace to promote code reuse.\n\nSigned-off-by: Adam Brewster <asb@bu.edu>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n\ndiff --git a/builtin-rev-list.c b/builtin-rev-list.c\nindex 83a7b13..54b6672 100644\n--- a/builtin-rev-list.c\n+++ b/builtin-rev-list.c\n@@ -565,23 +565,6 @@ static struct commit_list *find_bisection(struct commit_list *list,\n \treturn best;\n }\n \n-static void read_revisions_from_stdin(struct rev_info *revs)\n-{\n-\tchar line[1000];\n-\n-\twhile (fgets(line, sizeof(line), stdin) != NULL) {\n-\t\tint len = strlen(line);\n-\t\tif (len && line[len - 1] == '\\n')\n-\t\t\tline[--len] = 0;\n-\t\tif (!len)\n-\t\t\tbreak;\n-\t\tif (line[0] == '-')\n-\t\t\tdie(\"options not supported in --stdin mode\");\n-\t\tif (handle_revision_arg(line, revs, 0, 1))\n-\t\t\tdie(\"bad revision '%s'\", line);\n-\t}\n-}\n-\n int cmd_rev_list(int argc, const char **argv, const char *prefix)\n {\n \tstruct commit_list *list;\ndiff --git a/revision.c b/revision.c\nindex fc66755..6ce6042 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -910,6 +910,23 @@ int handle_revision_arg(const char *arg, struct rev_info *revs,\n \treturn 0;\n }\n \n+void read_revisions_from_stdin(struct rev_info *revs)\n+{\n+\tchar line[1000];\n+\n+\twhile (fgets(line, sizeof(line), stdin) != NULL) {\n+\t\tint len = strlen(line);\n+\t\tif (len && line[len - 1] == '\\n')\n+\t\t\tline[--len] = '\\0';\n+\t\tif (!len)\n+\t\t\tbreak;\n+\t\tif (line[0] == '-')\n+\t\t\tdie(\"options not supported in --stdin mode\");\n+\t\tif (handle_revision_arg(line, revs, 0, 1))\n+\t\t\tdie(\"bad revision '%s'\", line);\n+\t}\n+}\n+\n static void add_grep(struct rev_info *revs, const char *ptn, enum grep_pat_token what)\n {\n \tif (!revs->grep_filter) {\ndiff --git a/revision.h b/revision.h\nindex abce500..83f364a 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -111,6 +111,8 @@ struct rev_info {\n #define REV_TREE_DIFFERENT\t2\n \n /* revision.c */\n+void read_revisions_from_stdin(struct rev_info *revs);\n+\n typedef void (*show_early_output_fn_t)(struct rev_info *, struct commit_list *);\n volatile show_early_output_fn_t show_early_output;\n \n"},{"id":"82324","messageId":"c376da900807051949y78161d5dv17f251567ba888da@mail.gmail.com","threadId":"14300","inReplyTo":"7vskunpyqz.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v5]","fromName":"Adam Brewster","fromEmail":"adambrewster@gmail.com","sentAt":"2008-07-06T02:49:31Z","receivedAt":"2008-07-06T02:49:31Z","isPatch":true,"sender":{"key":"adambrewster@gmail.com","avatar":"https://avatars.githubusercontent.com/u/223816?v=4"},"body":">\n> Yeah, it is simply a bad taste to use helper in builtin-A from builtin-B.\n> More library-ish files such as revision.c are better home for utility\n> functions to be shared between builtins and commands.\n>\n> Here is what I queued.\n>\n\nThank you.\n\n> By the way did you compile test your fix before sending?\n>\n\nI ran make and test, but I didn't notice the warnings that prompted\nthe question.  I also forgot to re-check it after re-working the\ncommits to put everything in order.\n\n> -- >8 --\n\nThank you again for your help and patience in dealing with my multiple\nfailed attempts to get this right.\n\nAdam\n"},{"id":"82368","messageId":"1215354538-1469-1-git-send-email-adambrewster@gmail.com","threadId":"14300","inReplyTo":"7v63rjrfqz.fsf@gitster.siamese.dyndns.org","subject":"Re: Teach git-bundle to read revision arguments from stdin like git-rev-list","fromName":"Adam Brewster","fromEmail":"adambrewster@gmail.com","sentAt":"2008-07-06T14:28:56Z","receivedAt":"2008-07-06T14:28:56Z","isPatch":false,"sender":{"key":"adambrewster@gmail.com","avatar":"https://avatars.githubusercontent.com/u/223816?v=4"},"body":"\nOn Sat, Jul 5, 2008 at 8:57 PM, Junio C Hamano <gitster@pobox.com> \n>> +                     if (read_from_stdin++)\n>> +                             die(\"--stdin given twice?\");\n>\n> Hmm, do we deeply care about this case?  What bad things coulc happen \nif\n> you call read_revisions_from_stdin() twice?\n>\n\nPresently, it'll actually try to read stdin twice and that won't work.\n\nAlso, if you want git-bundle to deal with --stdin --stdin, I'd say that \ngit-rev-list should do the same.  I don't really care how this \nparticular error case is handled, but I think git-rev-list and \ngit-bundle should do the same thing for any given input.\n\nIf you prefer to be liberal in what you accept, then you might like \nthese two patches that allow git-rev-list and git-bundle to deal with \n--stdin --stdin.\n\nBy the way, I'm not exactly sure on the format of these guys.  You said \nyou queued some changes yesterday, so these go on top of those.  If \nyou want me to start from scratch and give you the whole chain again, I \ncan do that too.\n"},{"id":"82369","messageId":"1215354538-1469-2-git-send-email-adambrewster@gmail.com","threadId":"14300","inReplyTo":"1215354538-1469-1-git-send-email-adambrewster@gmail.com","subject":"[PATCH] git-rev-list: tolerate multiple --stdin options","fromName":"Adam Brewster","fromEmail":"adambrewster@gmail.com","sentAt":"2008-07-06T14:28:57Z","receivedAt":"2008-07-06T14:28:57Z","isPatch":true,"sender":{"key":"adambrewster@gmail.com","avatar":"https://avatars.githubusercontent.com/u/223816?v=4"},"body":"There's no reason to fail if the user asks for --stdin twice.  Of course\nthere's only one stdin, and it can only be read once, and there's no\nreason to ask for it twice, but --all --all doesn't make sense, and\nthat's accepted, so accept this too.\n\nAlso, with read_revisions_from_stdin in revision.c where it might be\ncalled by other programs, it's better to check that stdin isn't at eof\nbefore trying to read it.\n\nSigned-off-by: Adam Brewster <asb@bu.edu>\n---\n builtin-rev-list.c |    3 ---\n revision.c         |    2 +-\n 2 files changed, 1 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin-rev-list.c b/builtin-rev-list.c\nindex b4a2c44..7f7c1a7 100644\n--- a/builtin-rev-list.c\n+++ b/builtin-rev-list.c\n@@ -579,7 +579,6 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)\n {\n \tstruct commit_list *list;\n \tint i;\n-\tint read_from_stdin = 0;\n \tint bisect_show_vars = 0;\n \tint bisect_find_all = 0;\n \tint quiet = 0;\n@@ -616,8 +615,6 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)\n \t\t\tcontinue;\n \t\t}\n \t\tif (!strcmp(arg, \"--stdin\")) {\n-\t\t\tif (read_from_stdin++)\n-\t\t\t\tdie(\"--stdin given twice?\");\n \t\t\tread_revisions_from_stdin(&revs);\n \t\t\tcontinue;\n \t\t}\ndiff --git a/revision.c b/revision.c\nindex 0191160..c1550c4 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -914,7 +914,7 @@ int handle_revision_arg(const char *arg, struct rev_info *revs,\n void read_revisions_from_stdin(struct rev_info *revs)\n {\n \tchar line[1000];\n-\n+\tif (feof(stdin)) return;\n \twhile (fgets(line, sizeof(line), stdin) != NULL) {\n \t\tint len = strlen(line);\n \t\tif (len && line[len - 1] == '\\n')\n-- \n1.5.5.1.211.g65ea3.dirty\n"},{"id":"82370","messageId":"1215354538-1469-3-git-send-email-adambrewster@gmail.com","threadId":"14300","inReplyTo":"1215354538-1469-2-git-send-email-adambrewster@gmail.com","subject":"[PATCH] Teach git-bundle to read revision arguments from stdin like","fromName":"Adam Brewster","fromEmail":"adambrewster@gmail.com","sentAt":"2008-07-06T14:28:58Z","receivedAt":"2008-07-06T14:28:58Z","isPatch":true,"sender":{"key":"adambrewster@gmail.com","avatar":"https://avatars.githubusercontent.com/u/223816?v=4"},"body":"This patch allows the caller to feed the revision parameters to\ngit-bundle from its standard input.  This way, a script do not have to\nworry about limitation of the length of command line.\n\nDocumentation/git-bundle.txt says that git-bundle takes arguments\nacceptable to git-rev-list.  Obviously some arguments that git-rev-list\nhandles don't make sense for git-bundle (e.g. --bisect) but --stdin is\npretty reasonable.\n\nSigned-off-by: Adam Brewster <asb@bu.edu>\n---\n bundle.c |   10 ++++++++--\n 1 files changed, 8 insertions(+), 2 deletions(-)\n\ndiff --git a/bundle.c b/bundle.c\nindex 0ba5df1..8d486f3 100644\n--- a/bundle.c\n+++ b/bundle.c\n@@ -227,8 +227,14 @@ int create_bundle(struct bundle_header *header, const char *path,\n \n \t/* write references */\n \targc = setup_revisions(argc, argv, &revs, NULL);\n-\tif (argc > 1)\n-\t\treturn error(\"unrecognized argument: %s'\", argv[1]);\n+\n+\tfor (i = 1; i < argc; i++) {\n+\t\tif (!strcmp(argv[i], \"--stdin\")) {\n+\t\t\tread_revisions_from_stdin(&revs);\n+\t\t\tcontinue;\n+\t\t}\n+\t\treturn error(\"unrecognized argument: %s'\", argv[i]);\n+\t}\n \n \tfor (i = 0; i < revs.pending.nr; i++) {\n \t\tstruct object_array_entry *e = revs.pending.objects + i;\n-- \n1.5.5.1.211.g65ea3.dirty\n"}]}