Re: [PATCH v3 2/4] remote: use remote_state parameter internally

Junio C Hamano [off-list ref] writes:

Glen Choo [off-list ref] writes:

quoted

Junio C Hamano [off-list ref] writes:

quoted

quoted

c) Replace the backpointer with a remote_state parameter. Expressive and
   fits the paradigm of "defaulting to the repo when needed", but
   interfaces are repetitive and shifts the responsibility of
   correctness to the caller (see v2).

Hopefully.  Of course I think the implementation of the safety would
actually be done, not by iterationg over branches[] array, but just
checking branch->remote_state == remote_state pointer equality.

With (c), I plan to eliminate the backpointer altogether, so such a
check is not possible. However, I plan to add a branches_hash to
remote_state in the same way that we have remotes_hash for remote_state.

quoted

This "longer term direction" sounds like what I envisioned with (e). I
agree that detached HEAD is a state that should be expressed with more
than just NULL, though I'm not sure that "struct branch" is the correct
abstraction. No point bikeshedding now of course, we'll cross that
bridge when we get there ;)

I actually was hoping that the time to cross the bridge was now,
though ;-)

Hm, well I don't want to get too lost in the weeds here and lose sight
of the short-term objective. I have a few strands of thought, but
nothing concrete enough to propose a full alternative:

- It seems odd that the branches and the "current_branch" are handled by
  the remotes system, perhaps it would be clearer to move it elsewhere.
  Separating branches from "branch remote-tracking configuration" might
  clarify our thinking.
- branch_get(NULL) and branch_get("HEAD") are not entirely coherent with
  the idea of "getting a branch by name", perhaps we should just move
  this functionality into a different function.
- Similarly, variants of remote_for_branch() are misleading because they
  behave inconsistently when branch is NULL. 

I might not want to take action on these ideas though, e.g.
branch_get("HEAD") or remote_for_branch(NULL) are very convenient for
callers even though they behave slightly inconsisently. I'll propose a
longer term direction when I have a better grasp of the system and the
tradeoffs.