[PATCH 2/3] git-bundle: die if a given ref is not included in bundle

[PATCH 2/3] git-bundle: die if a given ref is not included in bundle

[Johannes Schindelin]

The earlier patch tried to be nice by just warning, but it seems
more likely that the user wants to adjust the parameters.

Also, it prevents a bundle containing _all_ revisions in the case
when the user only gave one ref, but also rev-list options which
excluded the ref.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	It really was a dumb idea not to error out there. Sorry.

 builtin-bundle.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/builtin-bundle.c b/builtin-bundle.c
index 33b533f..ca3de60 100644
--- a/builtin-bundle.c
+++ b/builtin-bundle.c
@@ -328,11 +328,9 @@ static int create_bundle(struct bundle_header *header, const char *path,
 		 * other limiting options could have prevented all the tips
 		 * from getting output.
 		 */
-		if (!(e->item->flags & SHOWN)) {
-			warn("ref '%s' is excluded by the rev-list options",
+		if (!(e->item->flags & SHOWN))
+			die("ref '%s' is excluded by the rev-list options",
 				e->name);
-			continue;
-		}
 		write_or_die(bundle_fd, sha1_to_hex(e->item->sha1), 40);
 		write_or_die(bundle_fd, " ", 1);
 		write_or_die(bundle_fd, ref, strlen(ref));
-- 
1.5.0.3.2601.gc1e5-dirty

Re: [PATCH 2/3] git-bundle: die if a given ref is not included in bundle

[Mark Levedahl]

Johannes Schindelin wrote:
> The earlier patch tried to be nice by just warning, but it seems
> more likely that the user wants to adjust the parameters.
>
> Also, it prevents a bundle containing _all_ revisions in the case
> when the user only gave one ref, but also rev-list options which
> excluded the ref.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>
>   
This patch makes git-bundle greatly complicates one of my primary uses: 
a nightly update generated and emailed to other users. With this, I need 
to write code to separately explore each branch in the repository to 
find what changed, then include only those. Without, I can just do git 
bundle ... <list of refs> and those which have been updated get included.

So, I would really like an option to error out only if the bundle would 
be empty.

Mark

Re: [PATCH 2/3] git-bundle: die if a given ref is not included in bundle

[Junio C Hamano]

Mark Levedahl <mlevedahl@gmail.com> writes:

> ... Without, I
> can just do git bundle ... <list of refs> and those which have been
> updated get included.

I am not sure if the above is true.  How are you preventing the
command from bundling everything?  You must have some limiter at
the bottom, something like --since=25.hours (to account for cron
schedule skew), not just <list of refs>.

I do not think the max-age limiter works as you are expecting,
especially when merges and clock skew among committing machines
are involved (just as you are distributing from central repo, I
am assuming you are getting work back from worker-bee machines
to the central repo at some point, and doing sneakernet implies
to me that they are disconnected machines, not running ntp, but
this is just me guessing).

In any case, the semantics of --since=25.hours limiter is not
"show everything newer than 25.hours that are reachable from any
of these refs"; it is "start digging from these tips, and stop
exploring the path as soon as you hit something that is newer
than 25.hours".

It appears that for the past few days we have been spending
significant effort to make --max-count and --max-age work with
bundles, but my honest impression is they do not play well in
the real world, especially when clock skew is involved.  On the
other hand, revision ranges ("include these, exclude those") are
always precise, and that is what you would want to be using,
especially from an automated script.

If I were doing a nightly script, I would probably be doing
something like this:

	#!/bin/sh
	yesterday=$(git bundle list-heads yesterday.bdl | sed -e 's/ .*//')
	git bundle create today.bdl --all --not $yesterday
	# mail it out

After sending today's bundle out, you will rotate it out to
yesterday.bdl in order to prepare for the next round.  It is
likely that you would want to keep a few day's worth of bundles
for other reasons _anyway_ (say, some project members might be
out of office, or mail gets dropped, or whatever), I think the
above is a reasonably clean and easy thing to arrange.

Re: [PATCH 2/3] git-bundle: die if a given ref is not included in bundle

[Mark Levedahl]

Junio C Hamano wrote:
> Mark Levedahl <mlevedahl@gmail.com> writes:
> I am not sure if the above is true.  How are you preventing the
> command from bundling everything?  You must have some limiter at
> the bottom, something like --since=25.hours (to account for cron
> schedule skew), not just <list of refs>.
>   
Sorry, I trimmed too much. My typical command usage is something like
    git bundle create foo --since=10.days.ago --all

I include a very generous date range so that folks don't have to update 
daily, can cover a vacation with a single bundle, etc. I think this 
usage renders moot all practical issues with clock skew. I am being 
loose, if the bundle won't apply then get one from the previous week, 
apply, and go forward.

BTW, shouldn't git check for clock skew when creating a commit to assure 
the parents predate the child? Clock skew could allow this circumstance 
which would look suspicious when exploring a history.
> In any case, the semantics of --since=25.hours limiter is not
> "show everything newer than 25.hours that are reachable from any
> of these refs"; it is "start digging from these tips, and stop
> exploring the path as soon as you hit something that is newer
> than 25.hours".
>   
I presume you mean "older than 2.5" hours in the above.

> If I were doing a nightly script, I would probably be doing
> something like this:
>
> 	#!/bin/sh
> 	yesterday=$(git bundle list-heads yesterday.bdl | sed -e 's/ .*//')
> 	git bundle create today.bdl --all --not $yesterday
> 	# mail it out
>
> After sending today's bundle out, you will rotate it out to
> yesterday.bdl in order to prepare for the next round.  It is
> likely that you would want to keep a few day's worth of bundles
> for other reasons _anyway_ (say, some project members might be
> out of office, or mail gets dropped, or whatever), I think the
> above is a reasonably clean and easy thing to arrange.
>
>   
This is certainly a reliable method, but has the difficulty of how to 
get started and of course requires that history be kept for the entire 
range covered by each bundle. The first bundle is either a) the entire 
repository, or b) crafted by trying each and every ref to find which are 
legal to define. While all of this can be done, I think this cure is 
worse than the disease (seconds to minutes of clock skew).

I would prefer to just add a warning to the manual that when limiting by 
date, be generous to allow for all possible clock skew across the 
distributed set of computers.

Mark

Re: [PATCH 2/3] git-bundle: die if a given ref is not included in bundle

[Mark Levedahl]

Junio C Hamano wrote:
> If I were doing a nightly script, I would probably be doing
> something like this:
>
> 	#!/bin/sh
> 	yesterday=$(git bundle list-heads yesterday.bdl | sed -e 's/ .*//')
> 	git bundle create today.bdl --all --not $yesterday
> 	# mail it out
Thinking about this further, the above has a problem (or should, but see 
below). Consider a case where master is not updated since yesterday. 
Effectively, the above becomes

git bundle create today.bdl  master <other-refs> --not master <other-refs>

As ref master is excluded, the bundle creation should die because master 
cannot be included. Experimenting with next (299fcfbdcb5afd85) however, 
I get:

git>git bundle create t.bdl master --not master
Generating pack...
Done counting 0 objects.
Writing 0 objects.
Total 0 (delta 0), reused 0 (delta 0)
git>git ls-remote t.bdl
git>

e.g, an empty bundle is created without any error or warning. This is 
the one case I believe an error should result: there is no use to 
sending (or even creating) an empty bundle.

As a date limited bundle containing all updated refs is my basic use, I 
really want this case to not be hard, and it definitely should not 
require externally maintained history or scripting to create. Absent the 
"die if any ref wasn't updated in the given date range" logic, and 
adding always die if the resulting bundle is empty, git bundle in next 
accomplishes what I want.

Mark

[PATCH] git-bundle: only die if pack would be empty, warn if ref is skipped

[Johannes Schindelin]

A use case for git-bundle expected to be quite common is this:

	$ git bundle create daily.bundle --since=10.days.ago --all

The expected outcome is _not_ to error out if only a couple of the
refs were not changed during the last 10 days.

This patch complains loudly about refs which are skipped due to the
pack not containing the corresponding objects, but dies only if
no objects would be in the pack _at all_.

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
---

	On Fri, 9 Mar 2007, Mark Levedahl wrote:

	> Junio C Hamano wrote:
	> > If I were doing a nightly script, I would probably be doing
	> > something like this:
	> > 
	> > 	#!/bin/sh
	> > 	yesterday=$(git bundle list-heads yesterday.bdl | sed -e 's/ .*//')
	> > 	git bundle create today.bdl --all --not $yesterday
	> > 	# mail it out
	>
	> Thinking about this further, the above has a problem (or should, 
	> but see below). [...]

	I see another problem, too: if at least one ref was not updated 
	since yesterday, "create" would fail with the latest patches.

	This fixes it.

	BTW I had a little laugh when seeing what git-describe made of my 
	current version :-)

 builtin-bundle.c |   11 ++++++++---
 1 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/builtin-bundle.c b/builtin-bundle.c
index 55f6d0a..7868080 100644
--- a/builtin-bundle.c
+++ b/builtin-bundle.c
@@ -263,7 +263,7 @@ static int create_bundle(struct bundle_header *header, const char *path,
 	int bundle_fd = -1;
 	const char **argv_boundary = xmalloc((argc + 4) * sizeof(const char *));
 	const char **argv_pack = xmalloc(5 * sizeof(const char *));
-	int pid, in, out, i, status;
+	int pid, in, out, i, status, ref_count = 0;
 	char buffer[1024];
 	struct rev_info revs;
 
@@ -328,15 +328,20 @@ static int create_bundle(struct bundle_header *header, const char *path,
 		 * other limiting options could have prevented all the tips
 		 * from getting output.
 		 */
-		if (!(e->item->flags & SHOWN))
-			die("ref '%s' is excluded by the rev-list options",
+		if (!(e->item->flags & SHOWN)) {
+			warn("ref '%s' is excluded by the rev-list options",
 				e->name);
+			continue;
+		}
+		ref_count++;
 		write_or_die(bundle_fd, sha1_to_hex(e->item->sha1), 40);
 		write_or_die(bundle_fd, " ", 1);
 		write_or_die(bundle_fd, ref, strlen(ref));
 		write_or_die(bundle_fd, "\n", 1);
 		free(ref);
 	}
+	if (!ref_count)
+		die ("Refusing to create empty bundle.");
 
 	/* end header */
 	write_or_die(bundle_fd, "\n", 1);
-- 
1.5.0.3.2621.gaaaa-dirty

Re: [PATCH] git-bundle: only die if pack would be empty, warn if ref is skipped

[Mark Levedahl]

Johannes Schindelin wrote:
> A use case for git-bundle expected to be quite common is this:
>
> 	$ git bundle create daily.bundle --since=10.days.ago --all
>
>   
This patch works for me... Thanks.

Mark

Re: [PATCH 2/3] git-bundle: die if a given ref is not included in bundle

[Junio C Hamano]

Mark Levedahl <mlevedahl@gmail.com> writes:

> git>git bundle create t.bdl master --not master
> Generating pack...
> Done counting 0 objects.
> Writing 0 objects.
> Total 0 (delta 0), reused 0 (delta 0)
> git>git ls-remote t.bdl
> git>
>
> e.g, an empty bundle is created without any error or warning. This is
> the one case I believe an error should result: there is no use to
> sending (or even creating) an empty bundle.

I agree that erroring on an empty output is a sensible _option_
just like pack-objects has --no-empty option.

The above is actually an interesting example in a different
sense.  When somebody did the following, what should be output?

	$ edit; git commit -a ;# on master
        $ git checkout -b side
        $ edit; git commit -a ;# on side
        $ git bundle create foo.bdl master side ^master
	$ git bundle verify foo.bdl

My answer is that it should list master and side as the
available heads and master itself as also a prerequisite (which
is not what the current code does).  

I think unbundling foo.bdl should be the moral equivalent of
fetching from the originating repository by somebody who has its
prerequisites as tips of some branches.  So I think foo.bdl
should list refs/heads/master as one of the available heads to
fetch/pull from, while requiring the same commit as prerequisite
of the bundle.  It is as if you tried "git fetch" and found out
that you are up to date.  Listing where the 'master' tip is,
even though you did not have to include any commits from that
branch, gives you a useful bit of information ("I am up to date
with respect to that branch").

So I think if you did this instead in the above sequence: 

	$ git bundle create foo.bdl master ^master

it would be sensible to have an option to error out because of
empty pack, but at the same time it would equally be sensible to
have an option to still create a bundle with an empty pack
contents.  In either case, the head and prerequisite section
should include the tip of the master.  Earlier, I said we should
error out if we do not find 'master' in the list of shown
objects, but I think it is more sensible to add it to both the
list of head _and_ prerequisites.

Re: [PATCH 2/3] git-bundle: die if a given ref is not included in bundle

[Mark Levedahl]

Junio C Hamano wrote:
> My answer is that it should list master and side as the
> available heads and master itself as also a prerequisite (which
> is not what the current code does).
>   
So, practically speaking the proposal is that for each ref where 
git-bundle currently issues a warning, instead the ref should be added 
to the prerequisites list, and that all refs given on the command line 
are in the bundle's defined refs.

Sounds reasonable to me.

Mark

Re: [PATCH 2/3] git-bundle: die if a given ref is not included in bundle

[Johannes Schindelin]

Hi,

On Fri, 9 Mar 2007, Junio C Hamano wrote:

> Mark Levedahl <mlevedahl@gmail.com> writes:
> 
> > git>git bundle create t.bdl master --not master
> > Generating pack...
> > Done counting 0 objects.
> > Writing 0 objects.
> > Total 0 (delta 0), reused 0 (delta 0)
> > git>git ls-remote t.bdl
> > git>
> >
> > e.g, an empty bundle is created without any error or warning. This is
> > the one case I believe an error should result: there is no use to
> > sending (or even creating) an empty bundle.
> 
> I agree that erroring on an empty output is a sensible _option_
> just like pack-objects has --no-empty option.
> 
> The above is actually an interesting example in a different
> sense.  When somebody did the following, what should be output?
> 
> 	$ edit; git commit -a ;# on master
>         $ git checkout -b side
>         $ edit; git commit -a ;# on side
>         $ git bundle create foo.bdl master side ^master

IMHO saying "master ^master" should blow into the user's face. If she says 
"I want it" _and_ "I don't want it", she should sorta expect it not to 
work.

Ciao,
Dscho

Re: [PATCH 2/3] git-bundle: die if a given ref is not included in bundle

[Mark Levedahl]

Johannes Schindelin wrote:
> Hi,
>   
> IMHO saying "master ^master" should blow into the user's face. If she says 
> "I want it" _and_ "I don't want it", she should sorta expect it not to 
> work.
>
> Ciao,
> Dscho
>
>   
The command
    git-bundle create foo next ^master

is legitimate, even if next points to the same commit as master. The 
current logic would reject this, and should not as we might want to push 
out the base of a new development branch in this manner. Consider that 
git-fetch <url> would happily update next in this case, git bundle / 
git-fetch should as well.

I think we should think of the git-bundle command as accepting two lists 
of rev-args
1 - the list of heads to define in the bundle (possibly --all, should 
also accept refs/heads/*)
2 - the list of commits to require as prerequisites for applying the 
bundle (possibly defined as --since=, possibly defined as a list of 
commits, etc).

As long as the lists are syntactically acceptable (all exist), we should 
just create the bundle with the given refs and prerequisites. The 
resulting bundle will apply cleanly and conforms to general git 
semantics. So, I think git-bundle should error out only if a ref does 
not exist or if no refs are defined. An empty pack file is legitimate.

Mark

Re: [PATCH 2/3] git-bundle: die if a given ref is not included in bundle

[Johannes Schindelin]

Hi,

On Sat, 10 Mar 2007, Mark Levedahl wrote:

> Johannes Schindelin wrote:
>
> > IMHO saying "master ^master" should blow into the user's face. If she 
> > says "I want it" _and_ "I don't want it", she should sorta expect it 
> > not to work.
>
> [...]
>
> As long as the lists are syntactically acceptable (all exist), we should 
> just create the bundle with the given refs and prerequisites.

So, what do you do if some of your users do, and some others do not, have 
the "blue-sky" branch? If you say "git bundle create new.bundle --all 
-10", your bundle will list "blue-sky" as a prerequisite.

Boom.

Some of your users -- those without "blue-sky" -- will _not_ be able 
to fetch _anything_ from the bundle. They are lacking the prerequisites.

The semantics of git-bundle used to be so clear and sensible, since they 
exactly reflected what git-pack-objects would do.

Now they are no longer?

Ciao,
Dscho

Re: [PATCH 2/3] git-bundle: die if a given ref is not included in bundle

[Mark Levedahl]

Johannes Schindelin wrote:
> Hi,
>   
> So, what do you do if some of your users do, and some others do not, have 
> the "blue-sky" branch? If you say "git bundle create new.bundle --all 
> -10", your bundle will list "blue-sky" as a prerequisite.
>
> Boom.
>
> Some of your users -- those without "blue-sky" -- will _not_ be able 
> to fetch _anything_ from the bundle. They are lacking the prerequisites.
>   
Those who have the prerequisites can apply the bundle. Those who do not, 
cannot. This is unchanged, and completely unrelated to whether the 
bundle defines 0 objects or 10,000. If you do not have the 
prerequisites, you need a different bundle.

Mark

Re: [PATCH 2/3] git-bundle: die if a given ref is not included in bundle

[Johannes Schindelin]

Hi,

On Sat, 10 Mar 2007, Mark Levedahl wrote:

> Johannes Schindelin wrote:
>
> > So, what do you do if some of your users do, and some others do not, 
> > have the "blue-sky" branch? If you say "git bundle create new.bundle 
> > --all -10", your bundle will list "blue-sky" as a prerequisite.
> > 
> > Boom.
> > 
> > Some of your users -- those without "blue-sky" -- will _not_ be able 
> > to fetch _anything_ from the bundle. They are lacking the 
> > prerequisites.
>
> Those who have the prerequisites can apply the bundle. Those who do not, 
> cannot. This is unchanged, and completely unrelated to whether the 
> bundle defines 0 objects or 10,000. If you do not have the 
> prerequisites, you need a different bundle.

Only that I suspect that you want to stick more than one ref into the 
bundle. And at some point you -- or any other user -- will expect the 
bundle to work _with several refs_, even if different recipients will pick 
_only one_ of them.

Basically, I am saying that this whole bundle concept is not thought 
through, that it is too loosely defined, and that it will result in unmet 
expectations sooner or later. (Which usually means sooner.)

So, either we have to rethink how to handle prerequisites (so that only 
those are checked which are strictly necessary for _the one_ ref you are 
updating), or we have to make it _very_ obvious to (human) users of 
git-bundle that you should _not_ bundle two unrelated -- or only remotely 
related -- refs into one bundle.

Ciao,
Dscho

Re: [PATCH 2/3] git-bundle: die if a given ref is not included in bundle

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Basically, I am saying that this whole bundle concept is not thought 
> through, that it is too loosely defined, and that it will result in unmet 
> expectations sooner or later. (Which usually means sooner.)

Earlier I thought you said that bundle had a clearly defined
semantics, which I did not quite understand, but now you are
agreeing with me...

> So, either we have to rethink how to handle prerequisites (so that only 
> those are checked which are strictly necessary for _the one_ ref you are 
> updating), or we have to make it _very_ obvious to (human) users of 
> git-bundle that you should _not_ bundle two unrelated -- or only remotely 
> related -- refs into one bundle.

I've been wondering if we can define prereqs per listed head.

Re: [PATCH 2/3] git-bundle: die if a given ref is not included in bundle

[Johannes Schindelin]

Hi,

On Sat, 10 Mar 2007, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > Basically, I am saying that this whole bundle concept is not thought 
> > through, that it is too loosely defined, and that it will result in 
> > unmet expectations sooner or later. (Which usually means sooner.)
> 
> Earlier I thought you said that bundle had a clearly defined semantics, 
> which I did not quite understand, but now you are agreeing with me...

git-bundle (as is in "next") has clearly defined semantics.

But the bundle concept is not thought through. Obviously, the clearly 
defined semantics of git-bundle do not match what people want to use 
bundles for.

> > So, either we have to rethink how to handle prerequisites (so that 
> > only those are checked which are strictly necessary for _the one_ ref 
> > you are updating), or we have to make it _very_ obvious to (human) 
> > users of git-bundle that you should _not_ bundle two unrelated -- or 
> > only remotely related -- refs into one bundle.
> 
> I've been wondering if we can define prereqs per listed head.

That is a substantial change, and it makes creating a bundle potentially 
very expensive. We would have to find out reachability for every boundary 
commit from each ref.

Granted, we could do that while finding the boundary objects, but we only 
have 28-16 bits left to store from which ref the commit is reachable, 
which means only 12 refs, and if you have more (which is likely when you 
say "--all" and have a reasonable number of tags) you need to allocate a 
bitfield for the remaining refs _for each_ traversed commit's parents.

So I wonder if there is not a better way.

Ciao,
Dscho

Re: [PATCH 2/3] git-bundle: die if a given ref is not included in bundle

[Mark Levedahl]

Johannes Schindelin wrote:
> git-bundle (as is in "next") has clearly defined semantics.
>   
git-bundle on next with the patch in 
<Pine.LNX.4.63.0703091726530.22628@wbgn013.biozentrum.uni-wuerzburg.de> 
works well enough for me, but absent that latter patch is too punishing.
> But the bundle concept is not thought through. Obviously, the clearly 
> defined semantics of git-bundle do not match what people want to use 
> bundles for.
>
>   
>> I've been wondering if we can define prereqs per listed head.
>>     
Currently, a bundle has a single pack. The bundle's prerequisites are 
that pack's dependencies. Splitting prerequisites per head requires 
either creating one pack per head or unpacking at the receiving and and 
extracting only the objects needed for the selected head. I'm not sure 
that either is warranted, and my uses of bundle do not require this 
regardless.

I think we need to restate purpose here. git-bundle is an alternate 
transport mechanism: git-bundle + git-fetch over sneakernet allows doing 
what git-push or git-fetch can do when directly connected. However, 
there are limitations due to the lack of direct connect, specifically 
the user of git-bundle needs to specify the prerequisites as the 
protocol cannot negotiate these. The exchange needs to be robust in that 
git-bundle+git-fetch must never result in leaving a repository in a 
corrupted state: the current prerequisites list + use of git-fetch seem 
to satisfy this.

 From a given connected repo, I can do:

    git fetch -f <source url> master:master

and nothing complains, even if no update occurs (remote master is up to 
date). I can also do
        git fetch -f <source url> master:next

and the new ref is created without complaint even if no new objects need 
to be defined or if the new definition is completely unrelated to the old.

With appropriate remote settings in .git/config, I can have git-fetch 
get all branches, or all branches and tags, and never complain when no 
update is required for something.

What I desire is similar functionality across sneakernet, and this is 
where git-bundle steps in. I cannot know what is on the destination 
repository exactly, so I need an imprecise way to specify prerequisites 
(e.g., --since=10.days.ago): I *know* that this is not exactly correct 
and *must* be conservative so that the bundle likely can be used. As the 
system is distributed and I don't control the recipients of the bundle, 
there is *no* way to know exactly what exists, the previous bundle is 
not definitive, it might not have been applied, they might have received 
data by a side-channel communication, etc.

So, a date range is the best method I have found to specify a bundle's 
prerequisites. However, I should not have to know which refs have been 
updated within the date range: this is too punishing. Directly connected 
git-fetch does not abort when the refspec says "get everything" but some 
ref in "everything" has not changed, why should git-bundle complain? 
Absent the latest patch (i.e., what is now on next), git-bundle will 
error out which is extremely unfriendly and unhelpful.

The single disconnect for the above with latest git-bundle + patch is 
that we cannot package a ref whose commit object is directly a bundle 
prerequisite. (I cannot do the equivalent of "git fetch remote 
master:next", where I already have all the objects for master). These 
instead result in a string of warning messages with the latest patch: I 
can live with this limitation (though I don't think this should even be 
a warning, git-fetch/git-push do not warn here). Absent the latest 
patch, git-bundle errors out: this is too punishing to the user.

While it is possible to fetch a particular ref from the bundle rather 
than taking all, the monolithic pack structure and protocol dictates 
that you will get all objects regardless. I do not see this as a 
problem: the bundle came from a single repository, everything in the 
bundle is therefore related, excess is easily trimmed by git-repack. 
This is really just a limitation of the disconnected protocol that 
cannot optimize the pack for the exact transfer required.

At some point, we have to make a clear distinction between what rules 
the protocol should enforce for "correctness" vs what an "intelligent" 
use of bundle is, and not try to enforce the latter in the software. 
What practices are useful or good vary considerably from business to 
business (I have many times been told that things I find essential to my 
work are "bad practice," usually stated by people who didn't have to 
solve a problem given constraints I actually face). The only requests 
git-bundle/git-fetch should refuse are things that will corrupt a 
git-repository, and the pair should endeavor to enable any information 
transfer that can be done with git-push or git-fetch given direct 
connections.

Bottom line, I strongly advocate Dscho's last patch + what is on next be 
promoted to master. We can revisit how well it is working and refine it 
after it gets some usage from others defining additional use-cases.

Mark

Re: [PATCH 2/3] git-bundle: die if a given ref is not included in bundle

[Junio C Hamano]

Mark Levedahl <mlevedahl@gmail.com> writes:

> Bottom line, I strongly advocate Dscho's last patch + what is on next
> be promoted to master.

I agree that is a very sensible thing to do.  Let's do that.

Re: [PATCH 2/3] git-bundle: die if a given ref is not included in bundle

[Johannes Schindelin]

Hi,

On Sun, 11 Mar 2007, Mark Levedahl wrote:

> Johannes Schindelin wrote:
> > git-bundle (as is in "next") has clearly defined semantics.
>
> git-bundle on next with the patch in 
> <Pine.LNX.4.63.0703091726530.22628@wbgn013.biozentrum.uni-wuerzburg.de> 
> works well enough for me, but absent that latter patch is too punishing.

I don't want to punish you! But I want to prevent "Huh?" moments early on.

> > I've been wondering if we can define prereqs per listed head.
>
> Currently, a bundle has a single pack. The bundle's prerequisites 
> are that pack's dependencies. Splitting prerequisites per head requires 
> either creating one pack per head or unpacking at the receiving and and 
> extracting only the objects needed for the selected head. I'm not sure 
> that either is warranted, and my uses of bundle do not require this 
> regardless.

You can just store the pack and update the ref (at least as long as the 
pack is not thin): we have a long-standing tradition of guaranteeing the 
integrity of commit chains _only_ when the commits are reachable by at 
least one ref.

> I think we need to restate purpose here. git-bundle is an alternate 
> transport mechanism: git-bundle + git-fetch over sneakernet allows doing 
> what git-push or git-fetch can do when directly connected.

But would that not mean that we expect the user of the bundle to update 
_all_ refs contained in the bundle?

> However, there are limitations due to the lack of direct connect, 
> specifically the user of git-bundle needs to specify the prerequisites 
> as the protocol cannot negotiate these. The exchange needs to be robust 
> in that git-bundle+git-fetch must never result in leaving a repository 
> in a corrupted state: the current prerequisites list + use of git-fetch 
> seem to satisfy this.

Yes. I think we definitely need to ensure the integrity _before_ updating 
the refs (and not rely on the prereqs in the bundle). And yes, that means 
I changed my mind from when I argued against your use of fsck, and _for_ 
prereqs.

> With appropriate remote settings in .git/config, I can have git-fetch 
> get all branches, or all branches and tags, and never complain when no 
> update is required for something.

Makes sense.

> While it is possible to fetch a particular ref from the bundle rather 
> than taking all, the monolithic pack structure and protocol dictates 
> that you will get all objects regardless.

... but a subsequent git-gc will strip them away.

> At some point, we have to make a clear distinction between what rules 
> the protocol should enforce for "correctness" vs what an "intelligent" 
> use of bundle is, and not try to enforce the latter in the software.

Concur. But I think that we have to make sure that the "correctness" 
enforcing means that you do not end up with something that people want to 
use for what it cannot be used for.

> Bottom line, I strongly advocate Dscho's last patch + what is on next be 
> promoted to master. We can revisit how well it is working and refine it 
> after it gets some usage from others defining additional use-cases.

FWIW my plans are to make the pack thin _only_ when there is only one 
prereq and/or ref in the bundle (this prevents a _wanted_ object being 
deltified against a not-wanted object).

Also, as mentioned above, I think that we have to check that "git rev-list 
--objects <new-refs> --not --all" does not result in missing objects.

Ciao,
Dscho

Re: [PATCH 2/3] git-bundle: die if a given ref is not included in bundle

[Mark Levedahl]

Johannes Schindelin wrote:
> FWIW my plans are to make the pack thin _only_ when there is only one 
> prereq and/or ref in the bundle (this prevents a _wanted_ object being 
> deltified against a not-wanted object).
>
>   
I am not sure that this is really necessary or accomplishes additional 
safety. The prerequisites must exist and be well connected in the target 
repo before the pack file is indexed: presumably, the reference objects 
all exist if the checks hold, or there is a logic flaw in the thin-pack 
generation.

If the prereq test is removed, then avoiding a thin pack might allow the 
pack file to be applied to a repo that held only the prereqs for a 
single head out of many in the bundle, but there is no info for the user 
to understand how or when to do this and I don't really think that is a 
good practice to encourage. I suggest waiting for a well defined 
use-case that really demands being able to apply only part of a pack 
file before implementing.

> Also, as mentioned above, I think that we have to check that "git rev-list 
> --objects <new-refs> --not --all" does not result in missing objects.
>   
This is certainly a good safety check: even though the prereqs are 
satisfied and all *should* be ok, some error might still exist and it is 
better to be safe.
> Ciao,
> Dscho
>   

[PATCH] git-bundle: die if the bundle is empty.

[Mark Levedahl]

A common use of bundle is to schedule nightly emailed updates to a remote
system containing all updates made over a given period. We want bundle
to complain only if the resulting bundle would be empty, but not if at
least one of the given refs has an update resulting in a non-empty bundle.

Signed-off-by: Mark Levedahl <mdl123@verizon.net>
---
 builtin-bundle.c |   17 +++++++++--------
 1 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/builtin-bundle.c b/builtin-bundle.c
index c85f996..b888a9f 100644
--- a/builtin-bundle.c
+++ b/builtin-bundle.c
@@ -261,6 +261,7 @@ static int create_bundle(struct bundle_header *header, const char *path,
 		int argc, const char **argv)
 {
 	int bundle_fd = -1;
+	int isempty = 1;
 	const char **argv_boundary = xmalloc((argc + 4) * sizeof(const char *));
 	const char **argv_pack = xmalloc(5 * sizeof(const char *));
 	int pid, in, out, i, status;
@@ -328,17 +329,17 @@ static int create_bundle(struct bundle_header *header, const char *path,
 		 * other limiting options could have prevented all the tips
 		 * from getting output.
 		 */
-		if (!(e->item->flags & SHOWN)) {
-			warn("ref '%s' is excluded by the rev-list options",
-				e->name);
-			continue;
+		if ((e->item->flags & SHOWN)) {
+			isempty = 0;
+			write_or_die(bundle_fd, sha1_to_hex(e->item->sha1), 40);
+			write_or_die(bundle_fd, " ", 1);
+			write_or_die(bundle_fd, ref, strlen(ref));
+			write_or_die(bundle_fd, "\n", 1);
 		}
-		write_or_die(bundle_fd, sha1_to_hex(e->item->sha1), 40);
-		write_or_die(bundle_fd, " ", 1);
-		write_or_die(bundle_fd, ref, strlen(ref));
-		write_or_die(bundle_fd, "\n", 1);
 		free(ref);
 	}
+	if (isempty)
+		die("Bundle is empty");
 
 	/* end header */
 	write_or_die(bundle_fd, "\n", 1);
-- 
1.5.0.3.927.g2432c-dirty