On Mon, Jul 6, 2015 at 1:30 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote: > Dave Borowitz <dborowitz@xxxxxxxxxx> writes: > >> I think I understand the confusion now. I think you and I are working >> from different assumptions about the client behavior. > > I agree that we now both understand where we come from ;-) > > And sorry for not being clear when I did the "push-cert" originally > in the documentation. As I already said, "packets between push-cert > and push-cert-end are contents of individual lines of the GPG signed > push certificate" This sentence makes sense to me now, but only because we now agree that "contents" does not include the LF. Different people may have different initial assumptions about whether the "contents" of a line includes the trailing newline or not. > was the design meant from day one, and a85b377d > (push: the beginning of "git push --signed", 2014-09-12) could have > made it clearer. > >> The problem with the documentation, then, is that the documentation >> does not say anything to indicate that the signed payload is anything >> other than what is on the wire. > > Yeah, that was untold assumption, as I considered "what is on the > wire" to include pkt-line framing when I wrote a85b377d (push: the > beginning of "git push --signed", 2014-09-12). > >> So maybe this series should include an explicit description of the >> singed payload outside of the context of a push. Then, in the push >> section, we can describe the set of transformations that the client >> MUST perform (splitting on LF; adding pkt-line headers) and MAY >> perform (adding LFs). > > Yes, and the latter is not limited to push-cert but anything sent on > pkt-line. > > That incidentally is the only point I deeply care about. I just > want to minimize "the protocol is this way in general, but only for > this one you must do it differently". Understood, and I'm glad we have finally come to an arrangement that is both consistent and easy to implement on the server side. > One example of "only for this one you must do it differently" is > another caveat for protocol implementors for the sending side (again > not limited to "push cert"). > > That existing implementations of the receivers treat an empty packet > (i.e. "0004") or "0005\n" ;) > as if it is the same as a flush packet (i.e. "0000"), > so even if the sending side chooses to ignore the "SHOULD terminate > each non-flush line using LF", it is strongly advised not to do so > when it wants to send an empty payload. This needs to be documented. > > The receiving end SHOULD NOT treat "0004" the same way as "0000". > I think that must be documented and implementations (including our > own) should be fixed. Agreed. > Thanks. -- 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