On 09.01.2015 17:52, Junio C Hamano wrote: > Stefan Beller <sbel...@google.com> writes: > >> If the server is configured to not advertise push certificates, >> a push certificate that gets pushed nevertheless will not be fully >> recorded because push_cert_nonce is NULL. > > Sorry, but I do not quite see what you are trying to get at. > > When we did not advertise that this feature is supported, we know > the incoming nonce is meaningless junk because we didn't supply the > correct answer the pusher can give us; we do not even have the > correct answer ourselves.
> > Besides, while reviewing the other patch, didn't we agree that we > should reject such a push? Then there is nothing to log anyway, no? Yes we did. This is unrelated to the previous patch. I stuffed it into this thread as I found it was touching the same feature. > > ... reads the patch and code beyond the context while scratching > head ... > > I notice that the above three lines do not correspond what you did > in this patch. The certificate is exported via the blob interface > fully with or without this change. I had problems with wording the commit message because I have no expertise with the feature. I am sorry for wasting your time there. > > What is not given to the hook is the push-cert-nonce-status. While > it is sufficient to tell the hook that we are not getting a good > nonce (i.e. the hook does not see GIT_PUSH_CERT_NONCE_STATUS=G), we > are not showing that nonce-status is "unsolicited", even though we > internally compute and decide that; is that what you are trying to > fix? yes that's what I was trying to hint at. The hook would just see it is unsolicited instead of not having the state available. > > Still puzzled... > >> >> The recording of GIT_PUSH_CERT_NONCE_STATUS should be dependent on >> the status being there instead of push_cert_nonce being non NULL. >> >> Without this patch an unsolicited nonce never makes to the stage when >> being exported with GIT_PUSH_CERT_NONCE_STATUS, because in the unsolicited >> case push_cert_nonce is always NULL. >> >> Signed-off-by: Stefan Beller <sbel...@google.com> >> --- >> builtin/receive-pack.c | 12 ++++++------ >> 1 file changed, 6 insertions(+), 6 deletions(-) >> >> diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c >> index 628d13a..0e4878e 100644 >> --- a/builtin/receive-pack.c >> +++ b/builtin/receive-pack.c >> @@ -504,18 +504,18 @@ static void prepare_push_cert_sha1(struct >> child_process *proc) >> sigcheck.key ? sigcheck.key : ""); >> argv_array_pushf(&proc->env_array, "GIT_PUSH_CERT_STATUS=%c", >> sigcheck.result); >> - if (push_cert_nonce) { >> + if (push_cert_nonce) >> argv_array_pushf(&proc->env_array, >> "GIT_PUSH_CERT_NONCE=%s", >> push_cert_nonce); >> + if (nonce_status) >> argv_array_pushf(&proc->env_array, >> "GIT_PUSH_CERT_NONCE_STATUS=%s", >> nonce_status); >> - if (nonce_status == NONCE_SLOP) >> - argv_array_pushf(&proc->env_array, >> - "GIT_PUSH_CERT_NONCE_SLOP=%ld", >> - nonce_stamp_slop); >> - } >> + if (nonce_status == NONCE_SLOP) >> + argv_array_pushf(&proc->env_array, >> + "GIT_PUSH_CERT_NONCE_SLOP=%ld", >> + nonce_stamp_slop); >> } >> } > -- > To unsubscribe from this list: send the line "unsubscribe git" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html