Johannes Schindelin <Johannes.Schindelin@xxxxxx> writes:
> The function for_each_remote() does exactly what the name suggests.
>
> The function remote_find_tracking() was extended to be able to search
> remote refs for a given local ref. You have to set the parameter
> "reverse" to true for that behavior.
>
> Both changes are required for the next step: simplification of
> git-branch's --track functionality.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@xxxxxx>
> ---
>
> You're right. I completely missed that functionality. Well, a
> few tweaks were needed. If this clashes too seriously with
> Daniel's work, I will gladly redo it after his changes are
> in "next".
No offence meant to Daniel, but I am inclined to postpone the
current round of changes from him to move the stuff further
to get us closer to built-in git-fetch until 1.5.3 final is
done. The amount of C code changes otherwise would be a bit too
much for me to be comfortable between -rc0 and -rc1.
> diff --git a/remote.c b/remote.c
> index cf98a44..21adb0d 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -279,6 +279,26 @@ struct remote *remote_get(const char *name)
> return ret;
> }
>
> +int for_each_remote(each_remote_fn fn, void *priv)
> +{
> ...
> + if ((result = fn(r, priv)))
> + break;
Just a minor style, but (you know what comes)...
> @@ -289,34 +309,36 @@ int remote_has_uri(struct remote *remote, const char *uri)
> return 0;
> }
>
> +int remote_find_tracking(struct remote *remote, struct refspec *refspec,
> + int reverse)
> {
> int i;
> for (i = 0; i < remote->fetch_refspec_nr; i++) {
> struct refspec *fetch = &remote->fetch[i];
> + const char *src = reverse ? fetch->dst : fetch->src;
> + const char *dst = reverse ? fetch->src : fetch->dst;
I have to agree with Daniel here --- variable names src and dst
are quite confusing. It seems to mean that "we search with
'src' to fill 'dst', but if reverse incoming refspec is given
reversed so matching refspec->src with what in fact is 'dst' in
the configuration file is fine". Utterly confusing.
Even though this is a good opportunity for you to improve your
comment ratio in ohloh stats, I doubt any amount of explanation
can unconfuse readers of this code.
> if (!fetch->dst)
> continue;
>
> if (fetch->pattern) {
> + if (!prefixcmp(refspec->src, src)) {
> refspec->dst =
> + xmalloc(strlen(dst) +
> strlen(refspec->src) -
> + strlen(src) + 1);
> + strcpy(refspec->dst, dst);
> + strcpy(refspec->dst + strlen(dst),
> + refspec->src + strlen(src));
> refspec->force = fetch->force;
> return 0;
> }
> } else {
> + if (!strcmp(refspec->src, src)) {
> + refspec->dst = xstrdup(dst);
> refspec->force = fetch->force;
> return 0;
> }
> }
> }
> - refspec->dst = NULL;
> return -1;
> }
The original remote_find_tracking() took a single remote and a
refspec with src filled, and returned the given refspec after
filling its dst, if an appropriate tracking was configured for
the remote. What you want to do is from the same remote
information and a refspec with dst filled to find a src branch
that would be stored to that dst.
In either case, incoming refspec would not have both src and dst
filled. The caller has one side of the information and asking
for the other. As Daniel suggests, the 'reverse' parameter is
not needed. If you must have it, please do not call it
'reverse' -- it is more like "find_by_dst".
Following Daniel's suggestion might make the code a bit
lengthier, but I think that would not be as confusing.
> diff --git a/send-pack.c b/send-pack.c
> index fecbda9..9fdd7b4 100644
> --- a/send-pack.c
> +++ b/send-pack.c
> @@ -305,8 +305,7 @@ static int send_pack(int in, int out, struct remote *remote, int nr_refspec, cha
> if (remote) {
> struct refspec rs;
> rs.src = ref->name;
> - remote_find_tracking(remote, &rs);
> - if (rs.dst) {
> + if (!remote_find_tracking(remote, &rs, 0)) {
> struct ref_lock *lock;
> fprintf(stderr, " Also local %s\n", rs.dst);
> if (will_delete_ref) {
-
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
