[PATCH/v2] git-basis, a script to manage bases for git-bundle

[PATCH/v2] git-basis, a script to manage bases for git-bundle

[Adam Brewster]

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>
---
As promised, here's another patch with documentation.  The code is
identical to the previous version.

I know this is a minor patch, but I think the result is a more usable
git-bundle feature, and I'd like to see it included in future releases
if there are no objections.

 Documentation/git-basis.txt |   82 +++++++++++++++++++++++++++++++++++++++++++
 bundle.c                    |   22 ++++++++++-
 git-basis                   |   71 +++++++++++++++++++++++++++++++++++++
 3 files changed, 173 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/git-basis.txt
 create mode 100755 git-basis

diff --git a/Documentation/git-basis.txt b/Documentation/git-basis.txt
new file mode 100644
index 0000000..3624890
--- /dev/null
+++ b/Documentation/git-basis.txt
@@ -0,0 +1,82 @@
+git-basis(1)
+============
+
+NAME
+----
+git-basis - Track sets of references available on remote systems (bases)
+
+SYNOPSIS
+--------
+[verse]
+'git-basis' <basis> [<basis>...]
+'git-basis' --update <basis> [<basis>...] < <object list or bundle>
+
+DESCRIPTION
+-----------
+Maintains lists of objects that are known to be accessible on remote
+computer systems that are not accessible by network.
+
+OPTIONS
+-------
+
+basis::
+       List of bases to operate on.  Any valid filename can be
+       the name of a basis.  Bases that do not exist are taken
+       to be empty.
+
+--update::
+       Tells git-basis to read a list of objects from stdin and
+       add them to each of the given bases.  git-basis produces
+       no output when this option is given.  Bases will be created
+       if necessary.
+
+object list or bundle::
+       Git-basis --update reads object names, one per line from stdin.
+       Leading caret ("^") characters are ignored, as is anything
+       after the object name.  Lines that don't begin with an object
+       name are ignored.  The output of linkgit:git-ls-remote[1] or a
+       bundle created by linkgit:git-bundle[1] are both suitable input.
+
+DISCUSSION
+----------
+git-basis is probably only useful with linkgit:git-bundle[1].
+
+To create a bundle that excludes all objects that are part of my-basis,
+use
+
+git-basis my-basis | git-bundle create my-bundle --all --stdin
+
+To add the objects in my-bundle to my-basis, use
+
+git-basis --update my-basis < my-bundle
+
+DETAILS
+-------
+Bases are stored as plain text files under .git/bases/.  One object
+entry per line.
+
+git-basis without --update reads all of the basis names given on the
+command line, and outputs the intersection of them to stdout, with each
+object prefixed by "^".
+
+git-basis --update reads object names from stdin, and adds all of the
+references to each of the bases listed.  Duplicate references will not
+be listed twice, but otherwise redundant information will be included.
+
+BUGS
+----
+Likely.
+
+Bug reports are welcome, and patches are encouraged.
+
+SEE ALSO
+--------
+linkgit:git-bundle[1]
+
+AUTHOR
+------
+Written by Adam Brewster <asb@bu.edu>
+
+GIT
+---
+Part of the linkgit:git[1] suite
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

Re: [PATCH/v2] git-basis, a script to manage bases for git-bundle

[Jeff King]

On Mon, Jun 30, 2008 at 06:49:25PM -0400, Adam Brewster wrote:

> 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 don't use bundles myself, so I can't comment on how useful this is for
a bundle-based workflow. But it seems like a sensible idea in general.

A few comments:

> --- 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") ) {

When a new feature depends on other, more generic improvements
to existing code, it is usually split into two patches. E.g.,

  1/2: add --stdin to git-bundle
  2/2: add git-basis

with the advantages that:

 - it is slightly easier to review each change individually
 - it is easier for other features to build on the generic improvement
   without requiring part 2, especially if part 2 is questionable

As it happens in this case, I think in this case the change was already
easy to read, being logically separated by file, so I am nitpicking
somewhat. But splitting changes is a good habit to get into.

> +                               if (len && line[len - 1] == '\n')
> +                                       line[--len] = 0;

Style: we usually spell NUL as '\0'.

> diff --git a/git-basis b/git-basis
> new file mode 100755

This should be git-basis.perl, with accompanying Makefile changes.

> +if ( ! -d "$d/bases" ) {
> +    system( "mkdir '$d/bases'" );
> +}

Yikes. This fails if $d contains an apostrophe. You'd want to use
quotemeta to properly shell out. But there's no need at all to shell out
here, since perl has its own mkdir call.

> +if ( $#ARGV == -1 ) {
> +    print "usage: git-basis [--update] basis1...\n";
> +    exit;

Usage should probably go to STDERR.

> +    my %new = ();
> +    while (<STDIN>) {
> +       if (!/^^?([a-z0-9]{40})/) {next;}
> +       $new{$1} = 1;
> +    }

Why make a hash when the only thing we ever do with it is "keys %new"?
Shouldn't an array suffice?

> +    foreach my $f (@ARGV) {
> +       my %these = ();
> +       open F, "<$d/bases/$f" || die "Can't open bases/$f: $!";

Style: I know we are not consistent within git, but it is usually better
to use local variables for filehandles these days. I.e.,

  open my $fh, "<$d/bases/$f"

> +       open F, ">>$d/bases/$f" || die "Can't open bases/$f: $!";

So the basis just grows forever? That is, each time we do a bundle and
basis update, we add a line for every changed ref, and we never delete
any lines. But having a commit implies having all of its ancestors, so
in the normal case (i.e., no rewind or rebase) we can simply replace old
objects if we know they are a subset of the new ones (which you can
discover with git-merge-base). For the rewind/rebase case, probably
these lists should get pruned eventually for non-existent objects.

But maybe it is not worth worrying about this optimization at first, and
we can see if people complain. In that case, it is perhaps worth a note
in the 'Bugs' section (or 'Discussion' section) of the manpage.

> +       print F "\#" . `date`;

I don't think there are any portability issues with 'date' (especially
since it appears to be just a comment here, so we don't really care
about the format), but in general I think it is nicer to use perl's date
functions just for consistency's sake.

> --
> 1.5.5.1.211.g65ea3.dirty

Notably absent: any tests.

-Peff

Re: [PATCH/v2] git-basis, a script to manage bases for git-bundle

[Adam Brewster]

Hi Jeff,

Thank you for your feedback.  I have made most of the code changes you
suggested, and am in the process of writing tests, but it looks like
some others on the list have more serious objections, so I'll hold of
on that until I think it might actually be accepted.

In the mean time, I have a couple of responses to your comments below.

>
> When a new feature depends on other, more generic improvements
> to existing code, it is usually split into two patches. E.g.,
>
>  1/2: add --stdin to git-bundle
>  2/2: add git-basis
>
> with the advantages that:
>
>  - it is slightly easier to review each change individually
>  - it is easier for other features to build on the generic improvement
>   without requiring part 2, especially if part 2 is questionable
>
> As it happens in this case, I think in this case the change was already
> easy to read, being logically separated by file, so I am nitpicking
> somewhat. But splitting changes is a good habit to get into.
>

Makes sense,  I thought it was small enough for one commit, but I'll
split it up when I resubmit.

>> +                               if (len && line[len - 1] == '\n')
>> +                                       line[--len] = 0;
>
> Style: we usually spell NUL as '\0'.
>

Okay.  I can also include a third patch for the code I cut-and-pasted.

diff --git a/builtin-rev-list.c b/builtin-rev-list.c
index 11a7eae..73fe334 100644
--- a/builtin-rev-list.c
+++ b/builtin-rev-list.c
@@ -582,7 +582,7 @@ static void read_revisions_from_stdin(struct rev_info *revs)
        while (fgets(line, sizeof(line), stdin) != NULL) {
                int len = strlen(line);
                if (len && line[len - 1] == '\n')
-                       line[--len] = 0;
+                       line[--len] = '\0';
                if (!len)
                        break;
                if (line[0] == '-')


>> diff --git a/git-basis b/git-basis
>> new file mode 100755
>
> This should be git-basis.perl, with accompanying Makefile changes.
>
>> +if ( ! -d "$d/bases" ) {
>> +    system( "mkdir '$d/bases'" );
>> +}
>
> Yikes. This fails if $d contains an apostrophe. You'd want to use
> quotemeta to properly shell out. But there's no need at all to shell out
> here, since perl has its own mkdir call.
>

Made both of these changes.

>> +if ( $#ARGV == -1 ) {
>> +    print "usage: git-basis [--update] basis1...\n";
>> +    exit;
>
> Usage should probably go to STDERR.
>

Makes sense.

>> +    my %new = ();
>> +    while (<STDIN>) {
>> +       if (!/^^?([a-z0-9]{40})/) {next;}
>> +       $new{$1} = 1;
>> +    }
>
> Why make a hash when the only thing we ever do with it is "keys %new"?
> Shouldn't an array suffice?
>

It's probably a non-issue, but using a hash will prevent duplicates.

>> +    foreach my $f (@ARGV) {
>> +       my %these = ();
>> +       open F, "<$d/bases/$f" || die "Can't open bases/$f: $!";
>
> Style: I know we are not consistent within git, but it is usually better
> to use local variables for filehandles these days. I.e.,
>
>  open my $fh, "<$d/bases/$f"
>

Okay.

>> +       open F, ">>$d/bases/$f" || die "Can't open bases/$f: $!";
>
> So the basis just grows forever? That is, each time we do a bundle and
> basis update, we add a line for every changed ref, and we never delete
> any lines. But having a commit implies having all of its ancestors, so
> in the normal case (i.e., no rewind or rebase) we can simply replace old
> objects if we know they are a subset of the new ones (which you can
> discover with git-merge-base). For the rewind/rebase case, probably
> these lists should get pruned eventually for non-existent objects.
>

If all goes well then you're right, but I thought old objects should
be kept around  in case the user has some reason to manually delete
them.  As it is, you can go into the basis file and delete everything
past a given date line and be back where you were.  If I delete the
redundant objects, then that's not always possible.

It'd be nice if it could prune old objects (maybe older than 6 months,
or settable by git-config) that are redundant, but I currently have no
need for such functionality.

I also hadn't thought about rebasing.  Objects that don't exist
shouldn't hurt anything though.  Just a waste of a little disk space.
If pruning is ever put in, objects that don't exist can be deleted.

> But maybe it is not worth worrying about this optimization at first, and
> we can see if people complain. In that case, it is perhaps worth a note
> in the 'Bugs' section (or 'Discussion' section) of the manpage.
>

Agree.  I put it under bugs.

>> +       print F "\#" . `date`;
>
> I don't think there are any portability issues with 'date' (especially
> since it appears to be just a comment here, so we don't really care
> about the format), but in general I think it is nicer to use perl's date
> functions just for consistency's sake.
>

Maybe I'm a idiot, but I can't find any built-in date to string
functions that do nice things like print the date the way the user
says he likes to look at dates.

I updated the comment line to be "# <git-date> // `date`" where
git-date is as per git-fast-import (seconds since 1969 +/-TZ).  If
automatic pruning ever happens, the git-date will be used, so `date`
is just for humans.

>
> Notably absent: any tests.
>

Working on those.  I'll also include tests for git-bundle.

Adam

Re: [PATCH/v2] git-basis, a script to manage bases for git-bundle

[Jay Soffian]

On Tue, Jul 1, 2008 at 9:36 PM, Adam Brewster <adambrewster@gmail.com> wrote:
> Maybe I'm a idiot, but I can't find any built-in date to string
> functions that do nice things like print the date the way the user
> says he likes to look at dates.

perldoc -f localtime

j.

Re: [PATCH/v2] git-basis, a script to manage bases for git-bundle

[Adam Brewster]

On Tue, Jul 1, 2008 at 10:10 PM, Jay Soffian <jaysoffian@gmail.com> wrote:
> On Tue, Jul 1, 2008 at 9:36 PM, Adam Brewster <adambrewster@gmail.com> wrote:
>> Maybe I'm a idiot, but I can't find any built-in date to string
>> functions that do nice things like print the date the way the user
>> says he likes to look at dates.
>
> perldoc -f localtime
>

But of course one function returns two very different things depending
on what's on the left side of the equals sign.  That makes perfect
sense.

> j.
>

Thank you.
Adam

Re: [PATCH/v2] git-basis, a script to manage bases for git-bundle

[Jay Soffian]

On Tue, Jul 1, 2008 at 10:16 PM, Adam Brewster <adambrewster@gmail.com> wrote:
> But of course one function returns two very different things depending
> on what's on the left side of the equals sign.  That makes perfect
> sense.

Context should be the very first thing taught in any Perl tutorial,
lest ye end up in jail:

http://yro.slashdot.org/article.pl?sid=01/03/13/208259

:-)

j.

Re: [PATCH/v2] git-basis, a script to manage bases for git-bundle

[Jeff King]

On Tue, Jul 01, 2008 at 09:36:20PM -0400, Adam Brewster wrote:

> Makes sense,  I thought it was small enough for one commit, but I'll
> split it up when I resubmit.

I think in this instance it is not too big a deal either way.  I am just
trying to help encourage good habits. :)

> > Style: we usually spell NUL as '\0'.
> 
> Okay.  I can also include a third patch for the code I cut-and-pasted.

Heh. I said "usually", but I guess even Junio makes mistakes (unless I
am dreaming such a style directive, but ISTR it being mentioned before
on the list). I wouldn't bother with the style cleanup in rev-list
(usually for such small things, we just wait until touching that part of
the code).

> > Why make a hash when the only thing we ever do with it is "keys %new"?
> > Shouldn't an array suffice?
> 
> It's probably a non-issue, but using a hash will prevent duplicates.

Ah, true. And there will be duplicates here, if you have multiple refs
at the same spot in your bundle list. So it should remain as you have
it.

> If all goes well then you're right, but I thought old objects should
> be kept around  in case the user has some reason to manually delete
> them.  As it is, you can go into the basis file and delete everything
> past a given date line and be back where you were.  If I delete the
> redundant objects, then that's not always possible.

Hmm, and that might be useful. Probably the best thing would be to leave
it as-is for now, then, with a note. Then we can decide the best pruning
strategy if and when it becomes an issue.

> Maybe I'm a idiot, but I can't find any built-in date to string
> functions that do nice things like print the date the way the user
> says he likes to look at dates.
> 
> I updated the comment line to be "# <git-date> // `date`" where
> git-date is as per git-fast-import (seconds since 1969 +/-TZ).  If
> automatic pruning ever happens, the git-date will be used, so `date`
> is just for humans.

That sounds reasonable.

> > Notably absent: any tests.
> Working on those.  I'll also include tests for git-bundle.

Great. Glancing over Junio's comments, though, it might make sense to
integrate this more tightly with git-bundle, in which case the perl
stuff would go away. So I'll let you work out with him which is the best
route.

-Peff

Re: [PATCH/v2] git-basis, a script to manage bases for git-bundle

[Jakub Narebski]

On Wed, 2 July 2008, Jeff King wrote:
>
> Glancing over Junio's comments, though, it might make sense to
> integrate this more tightly with git-bundle, in which case the perl
> stuff would go away. So I'll let you work out with him which is the
> best route.

Well, there is one situation where either separate git-bases program
(which is a good start; it can be named git-bundle--bases; there are
some precedents for that ;-)), or allowing to create 'bases' file
without creating bundle would be good to have.  Namely situation
where two computers are _sometimes off-line (disconnected)_.  If you
want to transfer new commits from machine B to machine A, you would
generate 'bases' file on machine A, then transfer this file using some
off-line medium, then generate bundle on machine B using those bases,
etc.

-- 
Jakub Narebski
Poland

Re: [PATCH/v2] git-basis, a script to manage bases for git-bundle

[Jeff King]

On Wed, Jul 02, 2008 at 11:44:45AM +0200, Jakub Narebski wrote:

> Well, there is one situation where either separate git-bases program
> (which is a good start; it can be named git-bundle--bases; there are
> some precedents for that ;-)), or allowing to create 'bases' file
> without creating bundle would be good to have.  Namely situation
> where two computers are _sometimes off-line (disconnected)_.  If you
> want to transfer new commits from machine B to machine A, you would
> generate 'bases' file on machine A, then transfer this file using some
> off-line medium, then generate bundle on machine B using those bases,
> etc.

Yes, certainly it is more flexible to have them split. I find Adam's
argument the most compelling, though. Think about moving commits as a
multi-step protocol:

  1. Local -> Remote: Here are some new commits, basis..current
  2. Remote -> Local: OK, I am now at current.
  3. Local: update basis to current

git-push has the luxury of asking for "basis" each time, so we know it
is correct. But with bundles, we can't do that. And failing to update
"basis" means we will send some extra commits next time. But updating
"basis" when we shouldn't means that the next bundle will be broken.

So I think even if people _do_ want to update "basis" when they create
the bundle (because it is more convenient, and they are willing to
accept the possibility of losing sync), it is trivial to create that
workflow on top of the separate components. But I can see why somebody
might prefer the separate components, and it is hard to create them if
the feature is lumped into "git-bundle" (meaning in such a way that you
cannot perform the steps separately; obviously git-bundle --basis would
be equivalent).

But I am not a bundle user, so that is just my outsider perspective.

-Peff

Re: [PATCH/v2] git-basis, a script to manage bases for git-bundle

[Adam Brewster]

>
> Yes, certainly it is more flexible to have them split. I find Adam's
> argument the most compelling, though. Think about moving commits as a
> multi-step protocol:
>
>  1. Local -> Remote: Here are some new commits, basis..current
>  2. Remote -> Local: OK, I am now at current.
>  3. Local: update basis to current
>
> git-push has the luxury of asking for "basis" each time, so we know it
> is correct. But with bundles, we can't do that. And failing to update
> "basis" means we will send some extra commits next time. But updating
> "basis" when we shouldn't means that the next bundle will be broken.
>
> So I think even if people _do_ want to update "basis" when they create
> the bundle (because it is more convenient, and they are willing to
> accept the possibility of losing sync), it is trivial to create that
> workflow on top of the separate components. But I can see why somebody
> might prefer the separate components, and it is hard to create them if
> the feature is lumped into "git-bundle" (meaning in such a way that you
> cannot perform the steps separately; obviously git-bundle --basis would
> be equivalent).
>
> But I am not a bundle user, so that is just my outsider perspective.
>
> -Peff
>

How does everybody feel about the following:

- Leave git-basis as a small perl script.

- Add a -b/--basis option in git-bundle that calls git-basis.  Any
objects mentioned in the output would be excluded from the bundle.
Multiple --basis options will call git-basis once with several
arguments to generate the intersection of specified bases.

- (maybe) Add an option "--update-bases" to automatically call
git-basis --update after the bundle is created successfully.

- Change the syntax a bit so git-basis --show does what git-basis
alone does now (because the user will no longer need to interact with
that command).

There's still plenty of potential for improvements, like a --gc mode
to clean up basis files, a --rewind option to undo an incorrect
--update, or improvements in the way it calculates intersections, but
I think that with these changes the system is as simple as possible
while maximizing flexibility, utility, and usability.

Adam

Re: [PATCH/v2] git-basis, a script to manage bases for git-bundle

[Johannes Schindelin]

Hi,

On Thu, 3 Jul 2008, Adam Brewster wrote:

> > Yes, certainly it is more flexible to have them split. I find Adam's 
> > argument the most compelling, though. Think about moving commits as a 
> > multi-step protocol:
> >
> >  1. Local -> Remote: Here are some new commits, basis..current
> >  2. Remote -> Local: OK, I am now at current.
> >  3. Local: update basis to current
> >
> > git-push has the luxury of asking for "basis" each time, so we know it 
> > is correct. But with bundles, we can't do that. And failing to update 
> > "basis" means we will send some extra commits next time. But updating 
> > "basis" when we shouldn't means that the next bundle will be broken.
> >
> > So I think even if people _do_ want to update "basis" when they create 
> > the bundle (because it is more convenient, and they are willing to 
> > accept the possibility of losing sync), it is trivial to create that 
> > workflow on top of the separate components. But I can see why somebody 
> > might prefer the separate components, and it is hard to create them if 
> > the feature is lumped into "git-bundle" (meaning in such a way that 
> > you cannot perform the steps separately; obviously git-bundle --basis 
> > would be equivalent).
> >
> > But I am not a bundle user, so that is just my outsider perspective.
> 
> How does everybody feel about the following:
> 
> - Leave git-basis as a small perl script.

I'd rather not.

> - Add a -b/--basis option in git-bundle that calls git-basis.  Any 
>   objects mentioned in the output would be excluded from the bundle.  
>   Multiple --basis options will call git-basis once with several 
>   arguments to generate the intersection of specified bases.

So the only function of -b would be to fork() && exec() a _shell_ script?  
I don't like that at all.

> - (maybe) Add an option "--update-bases" to automatically call git-basis 
>   --update after the bundle is created successfully.

Rather, have it as a feature to auto-detect if there is a ".basis" file of 
the same basename (or, rather ".state", a I find "basis" less than 
descriptive), and rewrite it if it was there.

It could be forced by a to-be-introduced "--state" option to git-bundle.

> There's still plenty of potential for improvements, like a --gc mode
> to clean up basis files,

umm, why?  "rm" is not simple enough?

> a --rewind option to undo an incorrect --update,

Rather hard, would you not think?  The information is either not there, or 
you store loads of cruft in the .state file.

> or improvements in the way it calculates intersections,

Umm.  How so?

> but I think that with these changes the system is as simple as possible 
> while maximizing flexibility, utility, and usability.

I am not convinced.  This sort of feature belongs into git-bundle.  It 
certainly does not deserve being blessed by yet-another git-* command, 
when we are constantly being bashed for having _way_ too many _already_.

Ciao,
Dscho

Re: [PATCH/v2] git-basis, a script to manage bases for git-bundle

[Adam Brewster]

>>
>> How does everybody feel about the following:
>>
>> - Leave git-basis as a small perl script.
>
> I'd rather not.
>

I'm not exactly sure what your objection is here, but I guess it's
either you don't want a separate tool called git-basis that does all
of the things I've described, or you don't like my choice of perl to
do the work.

If the former, I suggest that my approach is consistent with the
philosophy of doing one thing and doing it well.  I acknowledge that
it could do it's one job better (see potential improvements in my last
email), but that doesn't seem to be your complaint.

If the latter, I wonder what practical advantage comes from redoing
what I've already done in C.  It would be slightly faster, but I'm not
worried about saving a few milliseconds in a process that takes at
least a couple of minutes (considering of course the time it takes to
walk to whatever remote system).

>> - Add a -b/--basis option in git-bundle that calls git-basis.  Any
>>   objects mentioned in the output would be excluded from the bundle.
>>   Multiple --basis options will call git-basis once with several
>>   arguments to generate the intersection of specified bases.
>
> So the only function of -b would be to fork() && exec() a _shell_ script?
> I don't like that at all.
>

Not quite.  It would be more like a shortcut for

git-basis --show basis | git-bundle create bundle -all --stdin or
git-bundle create bundle --all <( git-basis --show basis )

(the latter of which of course wouldn't work because git-basis doesn't
take filenames.

The bundle creation is still done by git-bundle.  git-basis is just
deciding what should (not) be included in the bundle.

It seems like similar architectures have been accepted to support the
-i/--interactive options or git-add and git-rebase.

>> - (maybe) Add an option "--update-bases" to automatically call git-basis
>>   --update after the bundle is created successfully.
>
> Rather, have it as a feature to auto-detect if there is a ".basis" file of
> the same basename (or, rather ".state", a I find "basis" less than
> descriptive), and rewrite it if it was there.
>
> It could be forced by a to-be-introduced "--state" option to git-bundle.
>

Just because I'm creating a bundle, doesn't guarantee that the bundle
will be installed on any particular remote system, so I think that
updating the basis without being told to do so by the user is a bad
idea.  For example, when creating a bundle, I find it's best to
exclude objects that I know exist on ALL of my systems, so that I can
pull from it anywhere, but usually I only end up sending it to one
system.

With regard to the use of the word basis, it comes from the
documentation of git-bundle.  It's been a while since I took linear
algebra, but if I remember correctly, a basis is a set of vectors that
describe a vector space, such that a combination of those vectors
yields any point in the space.  The analogy isn't perfect, but I think
it's pretty close.  The word state seems very generic, as the state of
a repository includes much more than a list of objects that are known
to be available.

>> There's still plenty of potential for improvements, like a --gc mode
>> to clean up basis files,
>
> umm, why?  "rm" is not simple enough?
>

rm leaves the files a little cleaner than I'd like.  A basis is really
a list of objects.  --gc should make sure that the objects in the list
actually exist, and aren't redundant (if I know that a remote system
has a given commit, I also know that it has all of the ancestors, so I
could delete them from the basis file without losing (much)
information.

>> a --rewind option to undo an incorrect --update,
>
> Rather hard, would you not think?  The information is either not there, or
> you store loads of cruft in the .state file.
>

There's some information that you might describe as cruft.  It is
specifically engineered to enable this operation, and the purpose of
--gc is to reduce the volume of that cruft.

>> or improvements in the way it calculates intersections,
>
> Umm.  How so?
>

In a tree with three commits, where A (the root) is a parent of X and
Y, if only ommit X is in basisX and only commit Y is in basisY, then
the intersection of basisX and basisY should include A.  Currently
git-basis will return nothing, because it doesn't care about ancestry.

I don't consider this a serious flaw, because it results in extra
information being included in the bundle, but should never cause
broken bundles that are missing information.

>> but I think that with these changes the system is as simple as possible
>> while maximizing flexibility, utility, and usability.
>
> I am not convinced.  This sort of feature belongs into git-bundle.  It
> certainly does not deserve being blessed by yet-another git-* command,
> when we are constantly being bashed for having _way_ too many _already_.
>

I disagree.  I think the --basis option seems like a logical addition
to git-bundle, but I don't think git-bundle is the right interface to
update the basis files.

In other news, it seems, the change to make git-bundle accept --stdin
is less controversial, so I'll submit that as a small patch.

> Ciao,
> Dscho
>
>



Adam Brewster

Re: [PATCH/v2] git-basis, a script to manage bases for git-bundle

[Mark Levedahl]

Johannes Schindelin wrote:
> I am not convinced.  This sort of feature belongs into git-bundle.  It 
> certainly does not deserve being blessed by yet-another git-* command, 
> when we are constantly being bashed for having _way_ too many _already_.
>
> Ciao,
> Dscho
>
>   
Actually, I would like to see "normal" interface for git-bundle handled 
by git-push, git-fetch, and git-remote. We fixed the retrieve side to 
use git-fetch before first integration, but didn't understand the 
semantics for creation well-enough to put into git-push. Right now, we 
can do "git remote add bundle-nick <path-to-bundle>" and set up the remote.

We should have "git push bundle-nick"  create the new bundle, updating 
the basis refs kept somewhere in refs/* (possibly refs/remotes, possibly 
refs/bundles?).

However, we need two helpers to maintain the basis refs, both I believe 
should be sub-commands of git-remote:

- a "rewind" function to roll the refs back to a previous state because 
the bundle didn't get applied, whatever. This is well supported by 
reflogs, is "expire anything *newer* than time", and for convenience 
should apply to all refs for the given remote so the user doesn't have 
to invoke per branch on the remote. e.g., "git remote rewind bundle-nick 
3.days.ago".

- a "prune" function to remove any branch for the remote that is not 
known to the local refs/heads/* hierarchy. This is needed to support 
cleaning up pruned topic branches. Could be a special behavior of "git 
remote prune" triggered by the remote being a bundle, but that might be 
confusing a perhaps need a new sub-command name. Perhaps, "git remote 
prune-non-local bundle-nick"

If we did the above, then git-bundle can be relegated to plumbing and 
bundles become better integrated to the porcelain.

Mark

Re: [PATCH/v2] git-basis, a script to manage bases for git-bundle

[Jakub Narebski]

Mark Levedahl wrote:

> We should have "git push bundle-nick"  create the new bundle, updating 
> the basis refs kept somewhere in refs/* (possibly refs/remotes, possibly 
> refs/bundles?).

I think that because "git push directory" (local push) with error in
directory name, which otherwise would result in error, can be mistaken
for "git push bundle", that we would want to either use pseudo-protocol
("git push bundle://path/to/bundle") or some extension...

-- 
Jakub Narebski
Poland

Re: [PATCH/v2] git-basis, a script to manage bases for git-bundle

[Jeff King]

On Thu, Jul 03, 2008 at 07:38:21PM -0400, Adam Brewster wrote:

> There's still plenty of potential for improvements, like a --gc mode
> to clean up basis files, a --rewind option to undo an incorrect
> --update, or improvements in the way it calculates intersections, but
> I think that with these changes the system is as simple as possible
> while maximizing flexibility, utility, and usability.

I was thinking about Mark's approach, and I think there are two distinct
differences from yours:

  1. he updates the basis upon bundle creation, rather than as a
     separate step (and I have already commented on this)

  2. he stores the basis in the refs hierarchy

I actually think '2' makes a lot of sense. Storing the basis as refs
gets you:

  - an easy implementation; you use existing git tools

  - correct reachability analysis, since the refs will be taken into
    account by git-fsck, meaning you won't ever accidentally prune
    your basis objects

  - free logging of your basis history, in the form of reflogs

  - free gc in the usual reflog way

IIRC, Mark suggested putting them under refs/remotes/<bundle>, and you
objected that you didn't want to clutter that hierarchy. If that is a
problem, you can always use refs/basis/<bundle>, which will be ignored
by gitk and "git branch -a", but will be correctly handled by other
tools.

And then suddenly your perl script gets a lot simpler, and is either a
short shell script, or even better, can be written in C as part of
git-bundle. So you would have something like "git bundle --update-basis
<basis>" instead of "git-basis", and a config option like
"bundle.autoUpdateBasis" to update the basis whenever you create a
bundle.

-Peff

Re: [PATCH/v2] git-basis, a script to manage bases for git-bundle

[Junio C Hamano]

"Adam Brewster" <adambrewster@gmail.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.
>
> Signed-off-by: Adam Brewster <adambrewster@gmail.com>
> ---
> As promised, here's another patch with documentation.  The code is
> identical to the previous version.
>
> I know this is a minor patch, but I think the result is a more usable
> git-bundle feature, and I'd like to see it included in future releases
> if there are no objections.

Well, I have a moderately strong objection to this.

This very much feels like adding a missing feature to "git bundle" command
itself.  Why isn't it a new option to it?

For that matter, I am not sure how this integrates to a larger workflow.
You have a site (or more) to "push" your changes to, and you would need to
remember up to which revisions you have given out bundles to.  To remember
which site is at what basis level, you would need an extra infrastructure
than what this separate command offers (and "I'll have a yet another layer
of wrapper to this script" is not a good answer.  That wrapper can simply
read the tips from the bundle and record them without your script, and the
wrapper can use the previously recorded information to use the new bottom
refs when creating a new bundle again without using your script).

Perhaps it would be sufficient to have a new option to git-bundle.  "write
basis information under this name, so that I can reuse it in the next
invocation", and "I am not giving the bottom refs to create this bundle;
read them from the existing basis with this name".  It probably is easiest
to operate if these two are simply a new single option, like this...

diff --git a/Documentation/git-bundle.txt b/Documentation/git-bundle.txt
index f6a0612..d3e0716 100644
--- a/Documentation/git-bundle.txt
+++ b/Documentation/git-bundle.txt
@@ -9,7 +9,7 @@ git-bundle - Move objects and refs by archive
 SYNOPSIS
 --------
 [verse]
-'git-bundle' create <file> <git-rev-list args>
+'git-bundle' create [--basis=<filename>] <file> <git-rev-list args>
 'git-bundle' verify <file>
 'git-bundle' list-heads <file> [refname...]
 'git-bundle' unbundle <file> [refname...]
@@ -37,6 +37,17 @@ create <file>::
        Used to create a bundle named 'file'.  This requires the
        git-rev-list arguments to define the bundle contents.
 
+--basis=<filename>;;
+	Record the tips of resulting bundle to the file whie creating the
+        bundle, so that the information can be used when later creating a
+        new bundle to incrementally update a repository that resulting
+        bundle has already been applied to.
++
+If the named file exists, it should name the file given to this
+option in a previous invocation of 'git-bundle create' command.
+The tips of history recorded in the file is read and the resulting
+bundle will require them.
+
 verify <file>::
        Used to check that a bundle file is valid and will apply
        cleanly to the current repository.  This includes checks on the
@@ -165,6 +176,26 @@ $ git pull bundle
 would treat it as if it is talking with a remote side over the
 network.
 
+- Use basis file to keep track.
+
+------------
+$ git bundle create --basis=siteA.basis 2008-07-01.bndl master
+------------
+
+The new file `siteA.basis` records the tip commits in the created
+bundle.  Give `2008-07-01.bndl` to 'site A'.  Then later:
+
+------------
+$ git bundle create --basis=siteA.basis 2008-08-01.bndl master
+------------
+
+This invocation will read the existing `siteA.basis` file, reads the tip
+commits recorded there, and excludes the history reachable from these
+commits from the resulting `2008-08-01.bndl` bundle.  You can give this to
+'site A'; as long as they have extracted the previous bundle, it is
+sufficient to bring them up-to-date.
+
+
 Author
 ------
 Written by Mark Levedahl <mdl123@verizon.net>

Hmm?

Re: [PATCH/v2] git-basis, a script to manage bases for git-bundle

[Mark Levedahl]

Junio C Hamano wrote:

> 
> Well, I have a moderately strong objection to this.
> 
> This very much feels like adding a missing feature to "git bundle" command
> itself.  Why isn't it a new option to it?
> 

I have implemented (in script form) a different approach: basically, I just keep 
a local copy of the refs pushed out via bundle in refs/remotes/*, just as for 
any other remote, and then use those as the basis for later bundles. My longer 
term goal is to integrate this into git push, so that with a properly configured 
remote "git push foo" will create a bundle based upon the local knowledge of the 
remote's basis and update the local copy of the refs.


For reference, this is the script I currently use ...

#!/bin/sh
# usage
if test $# -lt 4
then
     echo "usage: $0 repoDirectory bundleName remote [git-for-each-ref args]"
     exit 1
fi

# must be at toplevel
cd $1 || exit 1
cd ./$(git rev-parse --show-cdup) || exit 1

bundleName=$2
remote=$3
shift 3

# get list of what we want to bundle up
newrefs=$(git for-each-ref --format="%(refname)" $*)

# get list of the current bundle
basis=$(git for-each-ref --format="^%(objectname)" refs/remotes/$remote)

# create the bundle
if git bundle create "$bundleName" $newrefs $basis
then
     # update our record of basis from the bundle
     git bundle list-heads "$bundleName" | \
     while read sha1 refname
     do
         git update-ref refs/remotes/"$remote"/"${refname##refs/}" $sha1
     done
else
     rm -f "$bundleName"
fi

Mark

Re: [PATCH/v2] git-basis, a script to manage bases for git-bundle

[Adam Brewster]

Hi Mark,

Thank you for your help, and I'm sorry I didn't get back to you sooner.

>
> I have implemented (in script form) a different approach: basically, I just
> keep a local copy of the refs pushed out via bundle in refs/remotes/*, just
> as for any other remote, and then use those as the basis for later bundles.
> My longer term goal is to integrate this into git push, so that with a
> properly configured remote "git push foo" will create a bundle based upon
> the local knowledge of the remote's basis and update the local copy of the
> refs.
>
> [...]

That's a good way to do things.  I tend to like my system better
because it's a little more flexible and it doesn't pollute git-branch
-a and gitk --all, also as I said before I like the bundle creation
and the basis update to be separated.

How do you deal with the case where you want to include remote refs in
the bundle?  Don't they get saved as
refs/remotes/remote/remotes/somewhere-else/master?

Adam

Re: [PATCH/v2] git-basis, a script to manage bases for git-bundle

[Mark Levedahl]

Adam Brewster wrote:
> Hi Mark,
>
> Thank you for your help, and I'm sorry I didn't get back to you sooner.
>
> That's a good way to do things.  I tend to like my system better
> because it's a little more flexible and it doesn't pollute git-branch
> -a and gitk --all, also as I said before I like the bundle creation
> and the basis update to be separated.
>
> How do you deal with the case where you want to include remote refs in
> the bundle?  Don't they get saved as
> refs/remotes/remote/remotes/somewhere-else/master?
>
> Adam
>
>   
My script is based upon experience with breakage from keeping the basis 
separate: absent keeping the refs in the git repo, there is nothing to 
guarantee that the referenced commits continue to exist. We make 
extensive use of short lived topic branches that are often rebased 
multiple times before being incorporated into a stable branch, so an 
externally stored basis would frequently have invalid commits. The 
solutions to this are to 1) filter them out, one by one, with a "git 
rev-parse $commit" or some such, or 2) keep the refs in tree so git will 
not remove the objects.

I'm using this for sneaker-netting, so I know the bundles are being 
applied - clearly the create bundle and update in-tree basis steps could 
be separated, but I don't use this for cases where I'm not sure the 
bundle will be applied. In the latter case, I just use a basis 
containing only previous stable branches.

You are correct in the name for a remote in the pushed bundle, the names 
do get convoluted, but then I'm not sure what the "correct" syntax would 
be to refer to a remote repo's idea of a remote branch.

Mark

Re: [PATCH/v2] git-basis, a script to manage bases for git-bundle

[Johannes Schindelin]

Hi,

On Fri, 4 Jul 2008, Mark Levedahl wrote:

> I'm using this for sneaker-netting, so I know the bundles are being 
> applied - clearly the create bundle and update in-tree basis steps could 
> be separated, but I don't use this for cases where I'm not sure the 
> bundle will be applied.

/me wonders if it would not make sense to support "git push <bundle>", 
then.  Maybe with a running counter, i.e.

	$ git push the-bundle-5.bundle master

would create the-bundle-6.bundle with everything needed in addition to 
the-bundle-5.bundle to have the current "master".

Just an idea,
Dscho

Re: [PATCH/v2] git-basis, a script to manage bases for git-bundle

[Mark Levedahl]

Johannes Schindelin wrote:
> Hi,
> 
> On Fri, 4 Jul 2008, Mark Levedahl wrote:
> 
> 
> /me wonders if it would not make sense to support "git push <bundle>", 
> then.  Maybe with a running counter, i.e.
> 
> 	$ git push the-bundle-5.bundle master
> 
> would create the-bundle-6.bundle with everything needed in addition to 
> the-bundle-5.bundle to have the current "master".
> 
> Just an idea,
> Dscho
> 

I think the trouble here is that the-bundle-5.bundle does not necessarily 
contain the basis. Consider the general case of pushing master, next, pu, and 
only pu has updated since the last push. Now, the-bundle-6.bundle will only 
contain pu, not master nor next as there is nothing new, and thus is not a good 
basis for creating the-bundle-7.bundle. This leads to a need for a meta-storage 
of basis or a redefinition of how git-bundle deals with refs that are equal to 
the given basis (currently, it excludes such refs).

Mark

Re: [PATCH/v2] git-basis, a script to manage bases for git-bundle

[Adam Brewster]

>
> Well, I have a moderately strong objection to this.
>
> This very much feels like adding a missing feature to "git bundle" command
> itself.  Why isn't it a new option to it?
>

Mostly because this was an easy way to accomplish the same thing.  If
this is popular, then it can be added to git-bundle.

> For that matter, I am not sure how this integrates to a larger workflow.
> You have a site (or more) to "push" your changes to, and you would need to
> remember up to which revisions you have given out bundles to.  To remember
> which site is at what basis level, you would need an extra infrastructure
> than what this separate command offers (and "I'll have a yet another layer
> of wrapper to this script" is not a good answer.  That wrapper can simply
> read the tips from the bundle and record them without your script, and the
> wrapper can use the previously recorded information to use the new bottom
> refs when creating a new bundle again without using your script).

The intent is to use one basis per site, not one per bundle, so the
first iteration is

A$ git-bundle create package.git --all
B$ git-clone package.git package
A$ git-bundle --update siteB < package.git

and thereafter it's

A$ git-bundle siteB | git-bundle create package.git --all --stdin
B$ git-pull
A$ git-bundle --update siteB < package.git

There's no issue of remembering which site is at which basis level,
because each site gets it's own basis.

If you're worried about hundred of sites, this is a bad solution.  I
happen to be worried about three sites, so this works well for me.

>
> Perhaps it would be sufficient to have a new option to git-bundle.  "write
> basis information under this name, so that I can reuse it in the next
> invocation", and "I am not giving the bottom refs to create this bundle;
> read them from the existing basis with this name".  It probably is easiest
> to operate if these two are simply a new single option, like this...
>
> [...]

I agree than git-bundle --basis is a better syntax than git-basis |
git-bundle --stdin.

I do, however, think that creating the bundle and updating the basis
should be two separate steps.  Mostly because the fact that I created
a bundle and planned to install it on another machine does not
guarantee that the resources of that bundle exist on the other
machine.  (I may need to stop for coffee and ... who knows?) Also, in
practice, I always use the intersection of all of my remote bases when
I create a bundle, and I frequently use them in places other than
where I intended.  Yes it's a pain to go back to git-basis --update,
but it's better than trying to git-pull from a bundle that's missing
objects.

Adam