threads / rfc / 14114

RFC patchCreated git-basis and modified git-bundle to accept --stdin.

Subject: [PATCH/RFC] Created git-basis and modified git-bundle to accept --stdin.

## tl;dr

6 messages between Jun 24, 2008 and Jun 25, 2008. Diffs are folded; open one to read it.

replies: 5people: 2as markdown or json

Adam Brewster· Jun 24, 2008, 02:21 UTC · lore

Git-basis is a perl script that remembers bases for use by git-bundle. Code from rev-parse was borrowed to allow git-bundle to handle --stdin.

Signed-off-by: Adam Brewster <adambrewster@gmail.com>
---
Git-bundle is a great tool to move repositories between computers on
different networks, but in order to create thin packs, it needs to be
given a list of objects that are available on the remote end.  I don't
like any of the options listed in the git-bundle documentation, so I did
this.

Git-basis is a script that maintains lists of objects that exist in various places.

With this patch, the command:
git-basis my-basis | git-bundle create my-bundle --all --stdin

will prepare a bundle containing everything except that which is a part of my-basis.

Then you can add the objects in the bundle to the basis, so they won't get included in the next pack like this:

git-basis --update my-basis < my-bundle

I'm sure that my implementation is crap, but I think this is a useful idea. Anybody agree? Disagree?

 bundle.c  |   22 +++++++++++++++++-
 git-basis |   71 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 91 insertions(+), 2 deletions(-)
 create mode 100755 git-basis
Show changes to 2 files +91 −4

bundle.c, git-basis

diff --git a/bundle.c b/bundle.c
index 0ba5df1..0af12d7 100644
--- a/bundle.c
+++ b/bundle.c
@@ -227,8 +227,26 @@ int create_bundle(struct bundle_header *header,
const char *path,

       /* write references */
       argc = setup_revisions(argc, argv, &revs, NULL);
-       if (argc > 1)
-               return error("unrecognized argument: %s'", argv[1]);
+
+       for (i = 1; i < argc; i++) {
+               if ( !strcmp(argv[i], "--stdin") ) {
+                       char line[1000];
+                               while (fgets(line, sizeof(line),
stdin) != NULL) {
+                               int len = strlen(line);
+                               if (len && line[len - 1] == '\n')
+                                       line[--len] = 0;
+                               if (!len)
+                                       break;
+                               if (line[0] == '-')
+                                       die("options not supported in
--stdin mode");
+                               if (handle_revision_arg(line, &revs, 0, 1))
+                                       die("bad revision '%s'", line);
+                       }
+                       continue;
+               }
+
+               return error("unrecognized argument: %s'", argv[i]);
+       }

       for (i = 0; i < revs.pending.nr; i++) {
               struct object_array_entry *e = revs.pending.objects + i;
diff --git a/git-basis b/git-basis
new file mode 100755
index 0000000..891635c
--- /dev/null
+++ b/git-basis
@@ -0,0 +1,71 @@
+#!/usr/bin/perl
+
+use strict;
+
+use Git;
+
+my $r = Git->repository();
+my $d = $r->repo_path();
+
+if ( ! -d "$d/bases" ) {
+    system( "mkdir '$d/bases'" );
+}
+
+if ( $#ARGV == -1 ) {
+    print "usage: git-basis [--update] basis1...\n";
+    exit;
+} elsif ( $ARGV[0] eq '--update' ) {
+    shift @ARGV;
+
+    my %new = ();
+    while (<STDIN>) {
+       if (!/^^?([a-z0-9]{40})/) {next;}
+       $new{$1} = 1;
+    }
+
+    foreach my $f (@ARGV) {
+       my %these = ();
+       open F, "<$d/bases/$f" || die "Can't open bases/$f: $!";
+       while (<F>) {
+           if (!/^([a-z0-9]{40})/) {next;}
+           $these{$1} = 1;
+       }
+       close F;
+       open F, ">>$d/bases/$f" || die "Can't open bases/$f: $!";
+       print F "\#" . `date`;
+       foreach my $b (keys %new) {
+           if (exists($these{$b})) {next;}
+           print F "$b\n";
+       }
+       close F;
+    }
+} else {
+    my $n = 0;
+    my %basis = ();
+
+    my $f = shift @ARGV;
+    open F, "<$d/bases/$f" || die "Can't open bases/$f: $!";
+    while (<F>) {
+       if (!/^([a-z0-9]{40})/) {next;}
+       $basis{$1} = $n;
+    }
+    close F;
+
+    foreach $f (@ARGV) {
+       open F, "<$d/bases/$f" || die "Can't open bases/$f: $!";
+       while (<F>) {
+           if (!/^([a-z0-9]{40})/) {next;}
+           if (!exists($basis{$1})) {next;}
+
+           if ($basis{$1} == $n) {$basis{$1}++;}
+           else {delete $basis{$1};}
+       }
+       close F;
+       $n++;
+    }
+
+    foreach my $b (keys %basis) {
+       if ( $basis{$b} != $n ) {next;}
+       print "^$b\n";
+    }
+}
--
1.5.5.1.211.g65ea3.dirty
Jakub Narebski· Jun 24, 2008, 08:09 UTC · re: Adam Brewster · lore

Re: [PATCH/RFC] Created git-basis and modified git-bundle to accept --stdin.

"Adam Brewster" <adam@adambrewster.com> writes:
> Git-basis is a perl script that remembers bases for use by git-bundle.
> Code from rev-parse was borrowed to allow git-bundle to handle --stdin.

I'd rather you follow git-pack-objects, and use `--revs` for the name of this option (or even '--not --revs'). The name `--stdin` might imply that you are providing objects names on stdin of git-bundle.

But perhaps I am worrying over nothing.
 
[...]
> Then you can add the objects in the bundle to the basis, so they won't
> get included in the next pack like this:
> 
>  $ git-basis --update my-basis < my-bundle
Why not use "$(git ls-remote my-bundle)" somewhere in the invocation
creating new bundle instead?
 
> I'm sure that my implementation is crap, but I think this is a useful
> idea.  Anybody agree?  Disagree?

Documentation, please? Especially that it looks like '--stdin' option is a bit tricky...

-- 
Jakub Narebski
Poland
ShadeHawk on #git
Adam Brewster· Jun 24, 2008, 15:30 UTC · re: Jakub Narebski · lore

Re: [PATCH/RFC] Created git-basis and modified git-bundle to accept --stdin.

Show 9 quoted lines
>> Git-basis is a perl script that remembers bases for use by git-bundle.
>> Code from rev-parse was borrowed to allow git-bundle to handle --stdin.
>
> I'd rather you follow git-pack-objects, and use `--revs` for the name
> of this option (or even '--not --revs').  The name `--stdin` might
> imply that you are providing objects names on stdin of git-bundle.
>
> But perhaps I am worrying over nothing.
>
This seems like a fine idea.
Show 9 quoted lines
> [...]
>> Then you can add the objects in the bundle to the basis, so they won't
>> get included in the next pack like this:
>>
>>  $ git-basis --update my-basis < my-bundle
>
> Why not use "$(git ls-remote my-bundle)" somewhere in the invocation
> creating new bundle instead?
>

You could use "git ls-remote my-bundle | git-basis --update my-basis" to do the same thing as the command I gave above.

Show 6 quoted lines
>> I'm sure that my implementation is crap, but I think this is a useful
>> idea.  Anybody agree?  Disagree?
>
> Documentation, please?  Especially that it looks like '--stdin' option
> is a bit tricky...
>

I wanted to test the waters and make sure that someone was at least vaguely interested in this (no need to document code that is never going to leave my machine).

I'll prepare another patch with documentation and changing --stdin to --revs when I get a chance.

Thank you, Adam Brewster

Jakub Narebski· Jun 24, 2008, 18:55 UTC · re: Adam Brewster · lore

Re: [PATCH/RFC] Created git-basis and modified git-bundle to accept --stdin.

Adam Brewster wrote:
Show 12 quoted lines
>> [...]
>>> Then you can add the objects in the bundle to the basis, so they won't
>>> get included in the next pack like this:
>>>
>>>  $ git-basis --update my-basis < my-bundle
>>
>> Why not use "$(git ls-remote my-bundle)" somewhere in the invocation
>> creating new bundle instead?
>>
> 
> You could use "git ls-remote my-bundle | git-basis --update my-basis"
> to do the same thing as the command I gave above.
I was thinking about
  $ git bundle create my-bundle --all --not $(git ls-remote my-bundle | cut -f1)

(or `--branches' instead of `--all'). Or, if you don't want to keep bundle, but only save basis, just use

  $ git bundle create my-bundle --all --not $(git basis --show my-basis)

No need for `--stdin' / `--revs' / `--basis' option to git-bundle, at the cost of little shell trickery.

Or even
  $ git ls-remote my-bundle | cut -f1 > my-bases
  [...]
  $ git bundle create my-bundle --all --not $(cat my-bases)
Show 9 quoted lines
>>> I'm sure that my implementation is crap, but I think this is a useful
>>> idea.  Anybody agree?  Disagree?
>>
>> Documentation, please?  Especially that it looks like '--stdin' option
>> is a bit tricky...
> 
> I wanted to test the waters and make sure that someone was at least
> vaguely interested in this (no need to document code that is never
> going to leave my machine).
See above example, to check if this would be enough...
> I'll prepare another patch with documentation and changing --stdin to
> --revs when I get a chance.

I'm not sure if another name, like --bases=<filename / basename> wouldn't be better. Perhaps --stdin is a good name...

-- 
Jakub Narebski
Poland
Jakub Narebski· Jun 25, 2008, 00:21 UTC · lore

Re: [PATCH/RFC] Created git-basis and modified git-bundle to accept --stdin.

On 6/25/08, Adam Brewster <adam@adambrewster.com> wrote:
Show 8 quoted lines
>>
>> I was thinking about
>>
>>  $ git bundle create my-bundle --all --not $(git ls-remote my-bundle | cut -f1)
>
> Not a bad idea, but it seems like I might be close to the maximum
>  length for a command line (about 3200 objects) in a couple of cases
>  (git-svn creates lots of branches).

Ah, sorry, I have forgot about that. This is definitely, both in the case of revisions packed in bundle, and basis (cutoff) of bundle, case for some kind of --stdin.

>  This also means that I have to remember where I save the old bundles.
That's what second solution was to save only bases in some file.
Show 11 quoted lines
>> Or even
>>
>>  $ git ls-remote my-bundle | cut -f1 > my-bases
>>  [...]
>>  $ git bundle create my-bundle --all --not $(cat my-bases)
>>
>
>
> That's basically all the perl script does, except it will also create
>  an intersection of several bases (I don't want to remember the options
>  to comm).

Does it prefix bases with ^, i.e. ^basis (negative revision)? I think it should, then there would be no need for strange --stdin semantic, or implementing --stdin in such way that --not--stdin works as expected.

>  Now that you mention it, a --basis option in setup_revisions would be
>  handy, but if I can't sell this, then I probably can't sell that
>  either.

I think that --stdin would be better idea, as it can be used to enumerate all refs to send, even if they are too many or they have too long names to use arguments.

[...]
Show 15 quoted lines
>>> I'll prepare another patch with documentation and changing --stdin to
>>> --revs when I get a chance.
>>
>> I'm not sure if another name, like --bases=<filename / basename> wouldn't
>> be better.  Perhaps --stdin is a good name...
>
> On second thought I'm going to defend my choice of --stdin now that I
>  remember why I chose it.
>
>  The docs for git-bundle specify the syntax is "git-bundle create
>  <file> <git-rev-list args>", where <git-rev-list args> is defined as
>  "A list of arguments, acceptable to git-rev-parse and git-rev-list,
>  that specify the specific objects and references to transport. ..."
>  Git-rev-list takes --stdin, so it seems reasonable to expect that
>  git-bundle will as well.

Ahh... git-rev-list... well, I though about git-pack-objects and its different --stin semantic (which might make no sense for git-bundle).

[Sorry for bad wrapping, I'm sending it from GMail Web interface]
-- 
Jakub Narebski
Adam Brewster· Jun 25, 2008, 01:14 UTC · re: Jakub Narebski · lore

Re: [PATCH/RFC] Created git-basis and modified git-bundle to accept --stdin.

Show 6 quoted lines
>
> Does it prefix bases with ^, i.e. ^basis (negative revision)?
> I think it should, then there would be no need for strange --stdin
> semantic, or implementing --stdin in such way that --not--stdin
> works as expected.
>
Yes.

I figure that patching to make --not --stdin work how I'd like would break something else, and implementing --not-stdin seems hackish.

Adam Brewster

← back to recent threads