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

Mark Levedahl [off-list ref] 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.

Junio C Hamano wrote:

Mark Levedahl [off-list ref] 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

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

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 <redacted>
---

	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

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