967: repository hostname lookups are not really canonical

psc***@vmwar***** (Google Code) (Is this you? Claim this profile.)
Sept. 3, 2009
Currently, post-review relies on calls to various socket module functions
in order to figure out the canonical FQDN of a Perforce or CVS repository.
This is not a great solution as these can actually be different on
different networks. 

For Perforce, at least, the host directly out of "p4 info" is a much much
better thing to use as a canonical name for the server, because it will
always be the same everywhere. (My team thinks that for older Perforce
servers this is the p4port argument when you start the server and for newer
servers this is whatever the FQDN is for the p4port argument when you start
the server.)

Looking back at the revision history for post-review in SVN, I think the
change was made to use the socket functions for Perforce since p4 info
might give you something other than the FQDN (like "perforce" instead of
"perforce.example.com"), which won't match up with what is likely recorded
as the repository path on the Review Board server. That actually points to
a larger problem, which is that the Review Board server's network, the
post-reviewer's network, and the repository itself may all have a different
canonical name for the repository.

It may be worth separating the canonical name used to figure out which
repository a review is for from the path the Review Board server uses to
connect back to that repository later. I would think it best to use
something self-reported from the repository itself (like the results from
p4 info) for the canonical name if at all possible.
david
#1 david
We cope with this much better for P4 and SVN now (using p4 info and the SVN UUID,
respectively). Therefore, I think this can be closed.
  • +Fixed