threads / patch / 12591

patch, 11 partsAllow for having for_each_ref() list some refs that aren't local

Subject: [PATCH 08/11] Allow for having for_each_ref() list some refs that aren't local

## tl;dr

5 messages between Mar 8, 2008 and Mar 9, 2008. Diffs are folded; open one to read it.

replies: 4people: 3as markdown or json

Daniel Barkalow· Mar 8, 2008, 23:04 UTC · lore

This is useful, for example, for listing the refs in a reference repository during clone, when you don't have your own refs that cover the objects that are in your alternate repository.

Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
---
 refs.c |   18 ++++++++++++++++++
 refs.h |    9 +++++++++
 2 files changed, 27 insertions(+), 0 deletions(-)

Show changes to 2 files +27 −0

refs.c, refs.h

diff --git a/refs.c b/refs.c
index 1b0050e..356396d 100644
--- a/refs.c
+++ b/refs.c
@@ -159,6 +159,8 @@ static struct cached_refs {
 } cached_refs;
 static struct ref_list *current_ref;
 
+static struct ref_list *extra_refs;
+
 static void free_ref_list(struct ref_list *list)
 {
 	struct ref_list *next;
@@ -215,6 +217,17 @@ static void read_packed_refs(FILE *f, struct cached_refs *cached_refs)
 	cached_refs->packed = sort_ref_list(list);
 }
 
+void add_extra_ref(const char *name, const unsigned char *sha1, int flag)
+{
+	extra_refs = add_ref(name, sha1, flag, extra_refs, NULL);
+}
+
+void clear_extra_refs(void)
+{
+	free_ref_list(extra_refs);
+	extra_refs = NULL;
+}
+
 static struct ref_list *get_packed_refs(void)
 {
 	if (!cached_refs.did_packed) {
@@ -535,6 +548,11 @@ static int do_for_each_ref(const char *base, each_ref_fn fn, int trim,
 	struct ref_list *packed = get_packed_refs();
 	struct ref_list *loose = get_loose_refs();
 
+	struct ref_list *extra;
+
+	for (extra = extra_refs; extra; extra = extra->next)
+		retval = do_one_ref(base, fn, trim, cb_data, extra);
+
 	while (packed && loose) {
 		struct ref_list *entry;
 		int cmp = strcmp(packed->name, loose->name);
diff --git a/refs.h b/refs.h
index 06abee1..06ad260 100644
--- a/refs.h
+++ b/refs.h
@@ -24,6 +24,15 @@ extern int for_each_tag_ref(each_ref_fn, void *);
 extern int for_each_branch_ref(each_ref_fn, void *);
 extern int for_each_remote_ref(each_ref_fn, void *);
 
+/*
+ * Extra refs will be listed by for_each_ref() before any actual refs
+ * for the duration of this process or until clear_extra_refs() is
+ * called. Only extra refs added before for_each_ref() is called will
+ * be listed on a given call of for_each_ref().
+ */
+extern void add_extra_ref(const char *refname, const unsigned char *sha1, int flags);
+extern void clear_extra_refs(void);
+
 extern int peel_ref(const char *, unsigned char *);
 
 /** Locks a "refs/" ref returning the lock on success and NULL on failure. **/

-- 
1.5.4.3.327.g614d7.dirty

Johannes Schindelin· Mar 8, 2008, 23:22 UTC · re: Daniel Barkalow · lore

Re: [PATCH 08/11] Allow for having for_each_ref() list some refs that aren't local

Hi,

On Sat, 8 Mar 2008, Daniel Barkalow wrote:

> This is useful, for example, for listing the refs in a reference 
> repository during clone, when you don't have your own refs that cover 
> the objects that are in your alternate repository.

How about this commit oneline instead:

	for_each_ref(): Allow adding extra refs to be traversed

Hmm?

You can explain in the body that the SHA-1s are not verified locally, so that you can add extra refs that do not reference local objects.

Ciao, Dscho

Daniel Barkalow· Mar 8, 2008, 23:25 UTC · re: Johannes Schindelin · lore

Re: [PATCH 08/11] Allow for having for_each_ref() list some refs that aren't local

On Sun, 9 Mar 2008, Johannes Schindelin wrote:

Show 16 quoted lines

> Hi,
> 
> On Sat, 8 Mar 2008, Daniel Barkalow wrote:
> 
> > This is useful, for example, for listing the refs in a reference 
> > repository during clone, when you don't have your own refs that cover 
> > the objects that are in your alternate repository.
> 
> How about this commit oneline instead:
> 
> 	for_each_ref(): Allow adding extra refs to be traversed
> 
> Hmm?
> 
> You can explain in the body that the SHA-1s are not verified locally, so 
> that you can add extra refs that do not reference local objects.

The non-local aspect is actually that the repository's refs doesn't actually include them. We actually do have these objects, in an alternate repo. Clearly I need a better commit message, with this info not trying to fit in the first line.

	-Daniel
*This .sig left intentionally blank*

Junio C Hamano· Mar 9, 2008, 03:57 UTC · re: Daniel Barkalow · lore

Re: [PATCH 08/11] Allow for having for_each_ref() list some refs that aren't local

Daniel Barkalow <barkalow@iabervon.org> writes:

> This is useful, for example, for listing the refs in a reference
> repository during clone, when you don't have your own refs that cover
> the objects that are in your alternate repository.

I hate to ask a question that I myself do not have a good answer to, but it is a bit worrysome that using these extra refs would cause for-each-ref to report different refs with the same name (the alternate's master branch and this one's master branch are both called refs/heads/master).

I think no caller that reads from for_each_ref() keeps track of names that it has seen and tries clever things like flagging duplicates as errors, but it still feels like it is inviting trouble.

Daniel Barkalow· Mar 9, 2008, 06:23 UTC · re: Junio C Hamano · lore

Re: [PATCH 08/11] Allow for having for_each_ref() list some refs that aren't local

On Sat, 8 Mar 2008, Junio C Hamano wrote:

Show 14 quoted lines

> Daniel Barkalow <barkalow@iabervon.org> writes:
> 
> > This is useful, for example, for listing the refs in a reference
> > repository during clone, when you don't have your own refs that cover
> > the objects that are in your alternate repository.
> 
> I hate to ask a question that I myself do not have a good answer to, but
> it is a bit worrysome that using these extra refs would cause for-each-ref
> to report different refs with the same name (the alternate's master branch
> and this one's master branch are both called refs/heads/master).
> 
> I think no caller that reads from for_each_ref() keeps track of names that
> it has seen and tries clever things like flagging duplicates as errors,
> but it still feels like it is inviting trouble.

I considered that, and didn't come to any particular conclusion about anything to do about it. In the particular case in this series, there are no non-extra refs, and the for_each_ref caller doesn't actually look at names at all, so it's really not a problem, but the interface could definitely generate odd results. For that matter, non-extra refs are sorted by name, and the extra refs are just done first. I think additional callers are needed before we can determine in more detail how the interface should work, since none of these details matters at all to this one.

	-Daniel
*This .sig left intentionally blank*