From: "Jeff King" <peff@xxxxxxxx>
On Wed, Oct 29, 2014 at 12:16:05PM -0700, Junio C Hamano wrote:
Probably three helper functions:
- The first is to find tops and bottoms (this translates fuzzy
specifications such as "--since 30.days" into a more concrete
revision range "^A ^B ... Z" to establish bundle prerequisites),
which is done by running a "rev-list --boundary".
- The second is to show refs, while paying attention to things like
"--10 maint master" which may result in the tip of 'maint' not
being shown at all. I am not sure if this part can/should take
advantage of revs.cmdline, though.
- The last is to create the actual pack data.
I agree with your analysis on the change in column.c and trailer.c
I was not planning to work on this, but since you did the first and
third bullet points, I think it makes sense to start the second one
with
this cleanup:
-- >8 --
Subject: bundle: split out ref writing from bundle_create
The bundle_create() function has a number of logical steps:
process the input, write the refs, and write the packfile.
Recent commits split the first and third into separate
sub-functions. It's worth splitting the middle step out,
too, if only because it makes the progression of the steps
more obvious.
Signed-off-by: Jeff King <peff@xxxxxxxx>
---
Obviously this should be dropped if somebody is actively working on
the
revs.cmdline thing you mentioned, as it would conflict horribly. But I
think it is a nice step for somebody working on it later, because the
revs.cmdline changes should be isolated to write_bundle_refs.
As a side project (slow time) I've been looking at the loss of the HEAD
symbolic ref when multiple heads are bundled that point at the same rev.
That is, when the HEAD detection heuristic fails.
What I've noticed so far is that a duplicated ref (added to the bundle
manually) simply creates a warning (currently, and the warning names
that ref) rather than failing when cloned (and by implication fetched).
Not only that the bundle verify command does not report any error for
such a bundle containing a duplicate ref.
Given that, if the refs were sorted, and HEAD was listed last (as is the
case with '--all'), then one could add the duplicate ref immediately
after HEAD, of it's symbolic ref. I.e if a duplicate ref is found after
HEAD then that ref is the true HEAD ref. This duplicate ref would only
need to be present if there are multiple (two or more) heads that point
to the same rev, and the HEAD isn't detatched. Sorting is necessary to
ensure that HEAD is last and its duplicate refs/head ref immediately
follows.
Thus the first step is to ensure that the positive refs list is sorted
such that HEAD (and it's ilk) is last.
I'd also planned an option '--HEAD' which would add such a duplicate ref
to a V2 bundle (resulting in a warning for older users which displays
the duplicate head ref!)
An option '--V3' would be the same as '--HEAD' but would also change the
bundle string version number (to V3), and so would not be acceptable to
older systems (for those that require such separation) but the
clone/fetch on a newer system would detect the V3 in the header string
and then use the extra ref rather than the heuristic to determine HEAD
(with appropriate checks).
I hadn't finished my studies of the refs.c code to fully understand what
I'd need to change, but hopefully the changes in this patch can be
aligned in the same direction (or the errors in my reasoning be pointed
out;-)
The need to sort the refs in this method would separate the
determination of the correct refs from the writing of the refs. All
assuming the idea has merit...
--
Philip
bundle.c | 97
++++++++++++++++++++++++++++++++++++++--------------------------
1 file changed, 58 insertions(+), 39 deletions(-)
diff --git a/bundle.c b/bundle.c
index 0ca8737..ca4803b 100644
--- a/bundle.c
+++ b/bundle.c
@@ -310,43 +310,22 @@ static int compute_and_write_prerequistes(int
bundle_fd,
return 0;
}
-int create_bundle(struct bundle_header *header, const char *path,
- int argc, const char **argv)
+/*
+ * Write out bundle refs based on the tips already
+ * parsed into revs.pending. As a side effect, may
+ * manipulate revs.pending to include additional
+ * necessary objects (like tags).
+ *
+ * Returns the number of refs written, or negative
+ * on error.
+ */
+static int write_bundle_refs(int bundle_fd, struct rev_info *revs)
{
- static struct lock_file lock;
- int bundle_fd = -1;
- int bundle_to_stdout;
- int i, ref_count = 0;
- struct rev_info revs;
-
- bundle_to_stdout = !strcmp(path, "-");
- if (bundle_to_stdout)
- bundle_fd = 1;
- else
- bundle_fd = hold_lock_file_for_update(&lock, path,
- LOCK_DIE_ON_ERROR);
-
- /* write signature */
- write_or_die(bundle_fd, bundle_signature, strlen(bundle_signature));
-
- /* init revs to list objects for pack-objects later */
- save_commit_buffer = 0;
- init_revisions(&revs, NULL);
-
- /* write prerequisites */
- if (compute_and_write_prerequistes(bundle_fd, &revs, argc, argv))
- return -1;
-
- /* write references */
- argc = setup_revisions(argc, argv, &revs, NULL);
-
- if (argc > 1)
- return error(_("unrecognized argument: %s"), argv[1]);
-
- object_array_remove_duplicates(&revs.pending);
+ int i;
+ int ref_count = 0;
- for (i = 0; i < revs.pending.nr; i++) {
- struct object_array_entry *e = revs.pending.objects + i;
+ for (i = 0; i < revs->pending.nr; i++) {
+ struct object_array_entry *e = revs->pending.objects + i;
unsigned char sha1[20];
char *ref;
const char *display_ref;
@@ -361,7 +340,7 @@ int create_bundle(struct bundle_header *header,
const char *path,
display_ref = (flag & REF_ISSYMREF) ? e->name : ref;
if (e->item->type == OBJ_TAG &&
- !is_tag_in_date_range(e->item, &revs)) {
+ !is_tag_in_date_range(e->item, revs)) {
e->item->flags |= UNINTERESTING;
continue;
}
@@ -407,7 +386,7 @@ int create_bundle(struct bundle_header *header,
const char *path,
*/
obj = parse_object_or_die(sha1, e->name);
obj->flags |= SHOWN;
- add_pending_object(&revs, obj, e->name);
+ add_pending_object(revs, obj, e->name);
}
free(ref);
continue;
@@ -420,11 +399,51 @@ int create_bundle(struct bundle_header *header,
const char *path,
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);
+ return ref_count;
+}
+
+int create_bundle(struct bundle_header *header, const char *path,
+ int argc, const char **argv)
+{
+ static struct lock_file lock;
+ int bundle_fd = -1;
+ int bundle_to_stdout;
+ int ref_count = 0;
+ struct rev_info revs;
+
+ bundle_to_stdout = !strcmp(path, "-");
+ if (bundle_to_stdout)
+ bundle_fd = 1;
+ else
+ bundle_fd = hold_lock_file_for_update(&lock, path,
+ LOCK_DIE_ON_ERROR);
+
+ /* write signature */
+ write_or_die(bundle_fd, bundle_signature, strlen(bundle_signature));
+
+ /* init revs to list objects for pack-objects later */
+ save_commit_buffer = 0;
+ init_revisions(&revs, NULL);
+
+ /* write prerequisites */
+ if (compute_and_write_prerequistes(bundle_fd, &revs, argc, argv))
+ return -1;
+
+ argc = setup_revisions(argc, argv, &revs, NULL);
+
+ if (argc > 1)
+ return error(_("unrecognized argument: %s"), argv[1]);
+
+ object_array_remove_duplicates(&revs.pending);
+
+ ref_count = write_bundle_refs(bundle_fd, &revs);
+ if (!ref_count)
+ die(_("Refusing to create empty bundle."));
+ else if (ref_count < 0)
+ return -1;
/* write pack */
if (write_pack_data(bundle_fd, &lock, &revs))
--
2.1.2.596.g7379948
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html