[v5,04/16] promisor-remote: implement promisor_remote_get_direct()

Commit Message

Christian Couder April 9, 2019, 4:11 p.m. UTC

From: Christian Couder <[e-mail]>

This is implemented for now by calling fetch_objects(). It fetches
from all the promisor remotes.

Helped-by: Ramsay Jones <[e-mail]>
Signed-off-by: Christian Couder <[e-mail]>
---
 promisor-remote.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++
 promisor-remote.h |  3 +++
 2 files changed, 69 insertions(+)

Comments

Derrick Stolee May 30, 2019, 5:21 p.m. UTC | #1

On 4/9/2019 12:11 PM, Christian Couder wrote:
> From: Christian Couder <[e-mail]>
> 
> This is implemented for now by calling fetch_objects(). It fetches
> from all the promisor remotes.

Hi Christian,

Sorry for jumping on the thread late, but I noticed some peculiarities
when looking at the test coverage report.

> +static int remove_fetched_oids(struct object_id **oids, int oid_nr, int to_free)

This method does not seem to be covered by the test suite at all.
Is this scenario difficult to set up for a test?

> +{
> +	int i, missing_nr = 0;
> +	int *missing = xcalloc(oid_nr, sizeof(*missing));
> +	struct object_id *old_oids = *oids;
> +	struct object_id *new_oids;
> +	int old_fetch_if_missing = fetch_if_missing;
> +
> +	fetch_if_missing = 0;

This global 'fetch_if_missing' swap seems very fragile. I'm guessing you are using
it to prevent a loop when calling oid_object_info_extended() below. Can you instead
pass a flag to the method that disables the fetch_if_missing behavior?

> +
> +	for (i = 0; i < oid_nr; i++)
> +		if (oid_object_info_extended(the_repository, &old_oids[i], NULL, 0)) {

A use of "the_repository" this deep in new code is asking for a refactor later to remove it.
Please try to pass a "struct repository *r" through your methods so we minimize references
to the_repository (and the amount of work required to remove them later).

> +			missing[i] = 1;
> +			missing_nr++;
> +		}
> +
> +	fetch_if_missing = old_fetch_if_missing;
> +
> +	if (missing_nr) {
> +		int j = 0;
> +		new_oids = xcalloc(missing_nr, sizeof(*new_oids));
> +		for (i = 0; i < oid_nr; i++)
> +			if (missing[i])
> +				oidcpy(&new_oids[j++], &old_oids[i]);
> +		*oids = new_oids;
> +		if (to_free)
> +			free(old_oids);
> +	}
> +
> +	free(missing);
> +
> +	return missing_nr;
> +}
> +
> +int promisor_remote_get_direct(const struct object_id *oids, int oid_nr)
> +{
> +	struct promisor_remote *r;
> +	struct object_id *missing_oids = (struct object_id *)oids;
> +	int missing_nr = oid_nr;

Note that for this method, "missing_nr" actually means "number of oids still in the list".

> +	int to_free = 0;
> +	int res = -1;
> +
> +	promisor_remote_init();
> +
> +	for (r = promisors; r; r = r->next) {
> +		if (fetch_objects(r->name, missing_oids, missing_nr) < 0) {

This block hits if we have any missing objects. This is not currently hit by the test
suite.

> +			if (missing_nr == 1)
> +				continue;

But we skip the call below if there is exactly one object in the list, as it must be the one
missing object. So, to be interesting we need to try fetching multiple objects.

> +			missing_nr = remove_fetched_oids(&missing_oids, missing_nr, to_free);

Here is the one call, and after this assignment "missing_nr" does mean the number of missing objects.
However, I do think this could be clarified by using remaining_nr and remaining_oids.

> +			if (missing_nr) {
> +				to_free = 1;
> +				continue;
> +			}

Now this block took a bit to grok. You use to_free in the if(to_free) free(missing_oids); below.
But it also changes the behavior of remove_fetched_oids(). This means that the first time
remove_fetched_oids() will preserve the list (because it is the input list) but all later
calls will free the newly-created intermediate list. This checks out.

What is confusing to me: is there any reason that missing_nr would be zero in this situation?
I guess if the fetch_objects() failed to find some objects, but we ended up having them locally
in a new call to oid_object_info_extended(). That's a fringe case that is worth guarding against
but I wouldn't worry about testing.

> +		}
> +		res = 0;
> +		break;
> +	}
> +
> +	if (to_free)
> +		free(missing_oids);
> +
> +	return res;
> +}

While the test coverage report brought this patch to my attention, it does seem correct.
I still think a test exposing this method would be good, especially one that requires
a fetch_objects() call to multiple remotes to really exercise the details of remove_fetched_oids().

Thanks,
-Stolee

Johannes Schindelin May 30, 2019, 8:46 p.m. UTC | #2

Hi,

On Thu, 30 May 2019, Derrick Stolee wrote:

> On 4/9/2019 12:11 PM, Christian Couder wrote:
> > From: Christian Couder <[e-mail]>
> >
> > +{
> > +	int i, missing_nr = 0;
> > +	int *missing = xcalloc(oid_nr, sizeof(*missing));
> > +	struct object_id *old_oids = *oids;
> > +	struct object_id *new_oids;
> > +	int old_fetch_if_missing = fetch_if_missing;
> > +
> > +	fetch_if_missing = 0;
>
> This global 'fetch_if_missing' swap seems very fragile. I'm guessing you
> are using it to prevent a loop when calling oid_object_info_extended()
> below. Can you instead pass a flag to the method that disables the
> fetch_if_missing behavior?

FWIW I mentioned the very same concern here:
https://public-inbox.org/git/[e-mail]/

The situation is *pretty* bad by now. I see `fetch_if_missing` mentioned
25 times in `master`, and all but one are in .c files or in cache.h.

The flag is actually used only in `oid_object_info_extended()`, and that
function accepts an `unsigned flags`, so one might think that it could be
extended to accept also a `OBJECT_INFO_LOOKUP_FETCH_IF_MISSING`. But then,
there are many callers of that function, some of them also pretty low in
the food chain. For example, `oid_object_info()` (does not accept `flags`)
or `read_object()` (does not accept flags either).

So it looks as if the idea to pass this flag down the call chain entailed
a pretty serious avalanche effect.

An alternative that strikes me as inelegant, still, but nevertheless
better would be to move `fetch_if_missing` into `struct repository`.

Ciao,
Dscho

Derrick Stolee May 30, 2019, 8:54 p.m. UTC | #3

On 5/30/2019 4:46 PM, Johannes Schindelin wrote:
> Hi,
> 
> On Thu, 30 May 2019, Derrick Stolee wrote:
> 
>> On 4/9/2019 12:11 PM, Christian Couder wrote:
>>> From: Christian Couder <[e-mail]>
>>>
>>> +{
>>> +	int i, missing_nr = 0;
>>> +	int *missing = xcalloc(oid_nr, sizeof(*missing));
>>> +	struct object_id *old_oids = *oids;
>>> +	struct object_id *new_oids;
>>> +	int old_fetch_if_missing = fetch_if_missing;
>>> +
>>> +	fetch_if_missing = 0;
>>
>> This global 'fetch_if_missing' swap seems very fragile. I'm guessing you
>> are using it to prevent a loop when calling oid_object_info_extended()
>> below. Can you instead pass a flag to the method that disables the
>> fetch_if_missing behavior?
> 
> FWIW I mentioned the very same concern here:
> https://public-inbox.org/git/[e-mail]/
> 
> The situation is *pretty* bad by now. I see `fetch_if_missing` mentioned
> 25 times in `master`, and all but one are in .c files or in cache.h.
> 
> The flag is actually used only in `oid_object_info_extended()`, and that
> function accepts an `unsigned flags`, so one might think that it could be
> extended to accept also a `OBJECT_INFO_LOOKUP_FETCH_IF_MISSING`. But then,
> there are many callers of that function, some of them also pretty low in
> the food chain. For example, `oid_object_info()` (does not accept `flags`)
> or `read_object()` (does not accept flags either).
>
> So it looks as if the idea to pass this flag down the call chain entailed
> a pretty serious avalanche effect.

It could be approached in small bits.

First, add an OBJECT_INFO_NEVER_FETCH_IF_MISSING flag that overrides fetch_if_missing,
and then use the flag in small places like this one. Then, build up to the other
methods as appropriate.

> An alternative that strikes me as inelegant, still, but nevertheless
> better would be to move `fetch_if_missing` into `struct repository`.

This is literally the _least_ we should do to reduce our dependence on
globals. Maybe this happens first, then the flag idea could be done bits
at a time.

Thanks,
-Stolee

Christian Couder May 31, 2019, 5:10 a.m. UTC | #4

Hi Stolee,

On Thu, May 30, 2019 at 7:21 PM Derrick Stolee <[e-mail]> wrote:
>
> On 4/9/2019 12:11 PM, Christian Couder wrote:
> > From: Christian Couder <[e-mail]>
> >
> > This is implemented for now by calling fetch_objects(). It fetches
> > from all the promisor remotes.
>
> Sorry for jumping on the thread late, but I noticed some peculiarities
> when looking at the test coverage report.

You are welcome. It needs review according to Junio, so it's
definitely a good thing that you take a look at it.

> > +static int remove_fetched_oids(struct object_id **oids, int oid_nr, int to_free)
>
> This method does not seem to be covered by the test suite at all.
> Is this scenario difficult to set up for a test?

I think so. If I remember correctly, I added this following a review
by Junio because it could be possible that a promisor/partial clone
remote only sends parts of the promisor objects it is asked. In this
case the objects that have been fetched should be removed from the
list of objects we try to fetch from the next promisor/partial clone
remote.

The issue is that now if a promisor/partial clone remote can send only
parts of the promisor objects it is asked, it should fail, as far as I
understand, which means that we will not actually get the objects it
should send. That's why I think it's not easy, or perhaps even not
possible, to test this.

> > +{
> > +     int i, missing_nr = 0;
> > +     int *missing = xcalloc(oid_nr, sizeof(*missing));
> > +     struct object_id *old_oids = *oids;
> > +     struct object_id *new_oids;
> > +     int old_fetch_if_missing = fetch_if_missing;
> > +
> > +     fetch_if_missing = 0;
>
> This global 'fetch_if_missing' swap seems very fragile. I'm guessing you are using
> it to prevent a loop when calling oid_object_info_extended() below. Can you instead
> pass a flag to the method that disables the fetch_if_missing behavior?

If such a flag existed when I wrote the function I would certainly
have used it, as I also dislike this kind of messing with a global
(and globals in general).

I will see if I can do something about it according to what you
suggest later in this thread.

> > +     for (i = 0; i < oid_nr; i++)
> > +             if (oid_object_info_extended(the_repository, &old_oids[i], NULL, 0)) {
>
> A use of "the_repository" this deep in new code is asking for a refactor later to remove it.
> Please try to pass a "struct repository *r" through your methods so we minimize references
> to the_repository (and the amount of work required to remove them later).

Ok, I will take a look at that.

> > +int promisor_remote_get_direct(const struct object_id *oids, int oid_nr)
> > +{
> > +     struct promisor_remote *r;
> > +     struct object_id *missing_oids = (struct object_id *)oids;
> > +     int missing_nr = oid_nr;
>
> Note that for this method, "missing_nr" actually means "number of oids still in the list".
>
> > +     int to_free = 0;
> > +     int res = -1;
> > +
> > +     promisor_remote_init();
> > +
> > +     for (r = promisors; r; r = r->next) {
> > +             if (fetch_objects(r->name, missing_oids, missing_nr) < 0) {
>
> This block hits if we have any missing objects. This is not currently hit by the test
> suite.
>
> > +                     if (missing_nr == 1)
> > +                             continue;
>
> But we skip the call below if there is exactly one object in the list, as it must be the one
> missing object. So, to be interesting we need to try fetching multiple objects.
>
> > +                     missing_nr = remove_fetched_oids(&missing_oids, missing_nr, to_free);
>
> Here is the one call, and after this assignment "missing_nr" does mean the number of missing objects.
> However, I do think this could be clarified by using remaining_nr and remaining_oids.

Ok, I will take a look at using "remaining_nr" and "remaining_oids".

> > +                     if (missing_nr) {
> > +                             to_free = 1;
> > +                             continue;
> > +                     }
>
> Now this block took a bit to grok. You use to_free in the if(to_free) free(missing_oids); below.
> But it also changes the behavior of remove_fetched_oids(). This means that the first time
> remove_fetched_oids() will preserve the list (because it is the input list) but all later
> calls will free the newly-created intermediate list. This checks out.
>
> What is confusing to me: is there any reason that missing_nr would be zero in this situation?

I don't think so but I will check again, and maybe add a comment.

> I guess if the fetch_objects() failed to find some objects, but we ended up having them locally
> in a new call to oid_object_info_extended(). That's a fringe case that is worth guarding against
> but I wouldn't worry about testing.
>
> > +             }
> > +             res = 0;
> > +             break;
> > +     }
> > +
> > +     if (to_free)
> > +             free(missing_oids);
> > +
> > +     return res;
> > +}
>
> While the test coverage report brought this patch to my attention, it does seem correct.
> I still think a test exposing this method would be good, especially one that requires
> a fetch_objects() call to multiple remotes to really exercise the details of remove_fetched_oids().

Yeah, I would like to actually test it. I will take another look at
what can be done to test this. Perhaps I will look at what can be done
to still get some objects when fetching from a promisor/partial clone
remote even when it doesn't have all of the objects we request.

Thanks for the review,
Christian.

Johannes Schindelin May 31, 2019, 11:35 a.m. UTC | #5

Hi Stolee,

On Thu, 30 May 2019, Derrick Stolee wrote:

> On 5/30/2019 4:46 PM, Johannes Schindelin wrote:
> >
> > On Thu, 30 May 2019, Derrick Stolee wrote:
> >
> >> On 4/9/2019 12:11 PM, Christian Couder wrote:
> >>> From: Christian Couder <[e-mail]>
> >>>
> >>> +{
> >>> +	int i, missing_nr = 0;
> >>> +	int *missing = xcalloc(oid_nr, sizeof(*missing));
> >>> +	struct object_id *old_oids = *oids;
> >>> +	struct object_id *new_oids;
> >>> +	int old_fetch_if_missing = fetch_if_missing;
> >>> +
> >>> +	fetch_if_missing = 0;
> >>
> >> This global 'fetch_if_missing' swap seems very fragile. I'm guessing you
> >> are using it to prevent a loop when calling oid_object_info_extended()
> >> below. Can you instead pass a flag to the method that disables the
> >> fetch_if_missing behavior?
> >
> > FWIW I mentioned the very same concern here:
> > https://public-inbox.org/git/[e-mail]/
> >
> > The situation is *pretty* bad by now. I see `fetch_if_missing` mentioned
> > 25 times in `master`, and all but one are in .c files or in cache.h.
> >
> > The flag is actually used only in `oid_object_info_extended()`, and that
> > function accepts an `unsigned flags`, so one might think that it could be
> > extended to accept also a `OBJECT_INFO_LOOKUP_FETCH_IF_MISSING`. But then,
> > there are many callers of that function, some of them also pretty low in
> > the food chain. For example, `oid_object_info()` (does not accept `flags`)
> > or `read_object()` (does not accept flags either).
> >
> > So it looks as if the idea to pass this flag down the call chain entailed
> > a pretty serious avalanche effect.
>
> It could be approached in small bits.
>
> First, add an OBJECT_INFO_NEVER_FETCH_IF_MISSING flag that overrides
> fetch_if_missing, and then use the flag in small places like this one.
> Then, build up to the other methods as appropriate.

That is a good idea. I fear that it will still take a Herculean effort to
get there, as some of the call paths strike me as rather deep...

> > An alternative that strikes me as inelegant, still, but nevertheless
> > better would be to move `fetch_if_missing` into `struct repository`.
>
> This is literally the _least_ we should do to reduce our dependence on
> globals. Maybe this happens first, then the flag idea could be done bits
> at a time.

Okay, then, I added https://github.com/gitgitgadget/git/issues/251 so we
won't forget.

BTW I am rather happy about the way the GitGitGadget issues turn out: I
added a couple of left-over bits, and could already close two tickets
after other developers pointed out that they had already been addressed,
something an unsuspecting GSoC student, for example, could not otherwise
have found out very easily (or for that matter, I myself...).

Ciao,
Dscho

Junio C Hamano May 31, 2019, 4:14 p.m. UTC | #6

Derrick Stolee <[e-mail]> writes:

>>> This global 'fetch_if_missing' swap seems very fragile. I'm guessing you
>>> are using it to prevent a loop when calling oid_object_info_extended()
>>> below. Can you instead pass a flag to the method that disables the
>>> fetch_if_missing behavior?
>>  ...
>> The flag is actually used only in `oid_object_info_extended()`, and that
>> function accepts an `unsigned flags`, so one might think that it could be
>> extended to accept also a `OBJECT_INFO_LOOKUP_FETCH_IF_MISSING`. But then,
>> there are many callers of that function, some of them also pretty low in
>> the food chain. For example, `oid_object_info()` (does not accept `flags`)
>> or `read_object()` (does not accept flags either).
>>
>> So it looks as if the idea to pass this flag down the call chain entailed
>> a pretty serious avalanche effect.
>
> It could be approached in small bits.
>
> First, add an OBJECT_INFO_NEVER_FETCH_IF_MISSING flag that overrides fetch_if_missing,
> and then use the flag in small places like this one. Then, build up to the other
> methods as appropriate.
>
>> An alternative that strikes me as inelegant, still, but nevertheless
>> better would be to move `fetch_if_missing` into `struct repository`.
>
> This is literally the _least_ we should do to reduce our dependence on
> globals. Maybe this happens first, then the flag idea could be done bits
> at a time.

The bit is not an attribute of a repository instance, and I agree it
is an ugly hack to take advantage of an unrelated fact that a repo
is getting passed throughout the codechain.  It is better than
nothing if we stop there and will not do anything more to the topic,
but in the longer term, it is not that better than a global, I am
afraid.  We may not be doing the save-flip-and-restore-the-bit dance
on the global anymore, but instead would be doing the same for the
field in the repository object, no?

In any case, thanks for taking a look at the topic; what it wants to
achieve is worthwhile, but its execution does look like it needs
quite a lot more polishing, which is helped by review comments like
these.

Christian Couder June 25, 2019, 1:50 p.m. UTC | #7

On Fri, May 31, 2019 at 7:10 AM Christian Couder
<[e-mail]> wrote:

> On Thu, May 30, 2019 at 7:21 PM Derrick Stolee <[e-mail]> wrote:
> >
> > On 4/9/2019 12:11 PM, Christian Couder wrote:

> > > +{
> > > +     int i, missing_nr = 0;
> > > +     int *missing = xcalloc(oid_nr, sizeof(*missing));
> > > +     struct object_id *old_oids = *oids;
> > > +     struct object_id *new_oids;
> > > +     int old_fetch_if_missing = fetch_if_missing;
> > > +
> > > +     fetch_if_missing = 0;
> >
> > This global 'fetch_if_missing' swap seems very fragile. I'm guessing you are using
> > it to prevent a loop when calling oid_object_info_extended() below. Can you instead
> > pass a flag to the method that disables the fetch_if_missing behavior?
>
> If such a flag existed when I wrote the function I would certainly
> have used it, as I also dislike this kind of messing with a global
> (and globals in general).
>
> I will see if I can do something about it according to what you
> suggest later in this thread.

In the V6 patch series I just sent, the new
OBJECT_INFO_SKIP_FETCH_OBJECT flag that you introduced is used.

> > > +     for (i = 0; i < oid_nr; i++)
> > > +             if (oid_object_info_extended(the_repository, &old_oids[i], NULL, 0)) {
> >
> > A use of "the_repository" this deep in new code is asking for a refactor later to remove it.
> > Please try to pass a "struct repository *r" through your methods so we minimize references
> > to the_repository (and the amount of work required to remove them later).
>
> Ok, I will take a look at that.

A "struct repository *r" is passed in V6. I forgot to mention that in
the cover letter.

> > > +                     missing_nr = remove_fetched_oids(&missing_oids, missing_nr, to_free);
> >
> > Here is the one call, and after this assignment "missing_nr" does mean the number of missing objects.
> > However, I do think this could be clarified by using remaining_nr and remaining_oids.
>
> Ok, I will take a look at using "remaining_nr" and "remaining_oids".

Done in V6 too.

> > > +                     if (missing_nr) {
> > > +                             to_free = 1;
> > > +                             continue;
> > > +                     }
> >
> > Now this block took a bit to grok. You use to_free in the if(to_free) free(missing_oids); below.
> > But it also changes the behavior of remove_fetched_oids(). This means that the first time
> > remove_fetched_oids() will preserve the list (because it is the input list) but all later
> > calls will free the newly-created intermediate list. This checks out.
> >
> > What is confusing to me: is there any reason that missing_nr would be zero in this situation?
>
> I don't think so but I will check again, and maybe add a comment.

Actually missing_nr, or now remaining_nr, would be 0 if all the
promised objects have been fetched.

> > > +             }
> > > +             res = 0;
> > > +             break;
> > > +     }
> > > +
> > > +     if (to_free)
> > > +             free(missing_oids);
> > > +
> > > +     return res;
> > > +}
> >
> > While the test coverage report brought this patch to my attention, it does seem correct.
> > I still think a test exposing this method would be good, especially one that requires
> > a fetch_objects() call to multiple remotes to really exercise the details of remove_fetched_oids().
>
> Yeah, I would like to actually test it. I will take another look at
> what can be done to test this. Perhaps I will look at what can be done
> to still get some objects when fetching from a promisor/partial clone
> remote even when it doesn't have all of the objects we request.

I haven't improved test coverage or looked at how we could better
handle a partial fetch. I plan to look at that soon.

Thanks,
Christian.

Patch

diff --git a/promisor-remote.c b/promisor-remote.c
index c249b80e02..289f1dd074 100644
--- a/promisor-remote.c
+++ b/promisor-remote.c
@@ -1,6 +1,8 @@ 
 #include "cache.h"
+#include "object-store.h"
 #include "promisor-remote.h"
 #include "config.h"
+#include "fetch-object.h"
 
 static struct promisor_remote *promisors;
 static struct promisor_remote **promisors_tail = &promisors;
@@ -90,3 +92,67 @@  int has_promisor_remote(void)
 {
 	return !!promisor_remote_find(NULL);
 }
+
+static int remove_fetched_oids(struct object_id **oids, int oid_nr, int to_free)
+{
+	int i, missing_nr = 0;
+	int *missing = xcalloc(oid_nr, sizeof(*missing));
+	struct object_id *old_oids = *oids;
+	struct object_id *new_oids;
+	int old_fetch_if_missing = fetch_if_missing;
+
+	fetch_if_missing = 0;
+
+	for (i = 0; i < oid_nr; i++)
+		if (oid_object_info_extended(the_repository, &old_oids[i], NULL, 0)) {
+			missing[i] = 1;
+			missing_nr++;
+		}
+
+	fetch_if_missing = old_fetch_if_missing;
+
+	if (missing_nr) {
+		int j = 0;
+		new_oids = xcalloc(missing_nr, sizeof(*new_oids));
+		for (i = 0; i < oid_nr; i++)
+			if (missing[i])
+				oidcpy(&new_oids[j++], &old_oids[i]);
+		*oids = new_oids;
+		if (to_free)
+			free(old_oids);
+	}
+
+	free(missing);
+
+	return missing_nr;
+}
+
+int promisor_remote_get_direct(const struct object_id *oids, int oid_nr)
+{
+	struct promisor_remote *r;
+	struct object_id *missing_oids = (struct object_id *)oids;
+	int missing_nr = oid_nr;
+	int to_free = 0;
+	int res = -1;
+
+	promisor_remote_init();
+
+	for (r = promisors; r; r = r->next) {
+		if (fetch_objects(r->name, missing_oids, missing_nr) < 0) {
+			if (missing_nr == 1)
+				continue;
+			missing_nr = remove_fetched_oids(&missing_oids, missing_nr, to_free);
+			if (missing_nr) {
+				to_free = 1;
+				continue;
+			}
+		}
+		res = 0;
+		break;
+	}
+
+	if (to_free)
+		free(missing_oids);
+
+	return res;
+}
diff --git a/promisor-remote.h b/promisor-remote.h
index 01dcdf4dc7..92650cfd4c 100644
--- a/promisor-remote.h
+++ b/promisor-remote.h
@@ -1,6 +1,8 @@ 
 #ifndef PROMISOR_REMOTE_H
 #define PROMISOR_REMOTE_H
 
+struct object_id;
+
 /*
  * Promisor remote linked list
  * Its information come from remote.XXX config entries.
@@ -12,5 +14,6 @@  struct promisor_remote {
 
 extern struct promisor_remote *promisor_remote_find(const char *remote_name);
 extern int has_promisor_remote(void);
+extern int promisor_remote_get_direct(const struct object_id *oids, int oid_nr);
 
 #endif /* PROMISOR_REMOTE_H */