Re: [RFC PATCH 0/2] Add named reference to latest push cert

From: Santiago Torres <santiago@nyu.edu>
Date: Sep 18, 2017, 14:22 UTC
Message-ID: <20170918142205.r5kwkq7ouy5zxisz@LykOS.localdomain>
In-Reply-To: <xmqqa81uumq6.fsf@gitster.mtv.corp.google.com>

Hello, everyone.

Sorry for being late in this thread, I was making sure I didn't say anything outrageously wrong.

> That's Stefan; I wouldn't have suggested any approach that uses the
> blob whose sole purpose is to serve as a temporary storage area to
> pass the information to the hook as part of the permanent record.
>

Hmm, as far as I understand *this* is the status quo. We get an ephemeral sha1/oid only if you have a hook attached. Otherwise, you will never see the object at all, even if you have --signed set.

I suggested preparing this RFC/Patch because of the following reasons (please let me know if my understanding of any of this is wrong...):

    - I find it weird that the cli allows for a --signed push and
      nowhere in the receive-pack's feedback you're told if the push
      certificate is compute/stored/handled at all. I think that, at the
      latest, receive pack should let the user know whether the signed
      push succeeded, or if there is no hook attached to handle it.

    - *if there is a hook* the blob is computed, but it is up to the
      hook itself to store it *somewhere*. This makes me feel like it's
      somewhat of a useless waste of computation if the hook is not
      meant to handle it anyway(which is just a post-receive hook). I
      find it rather weird that --signed is a builtin flag, and is
      handled on the server side only partially (just my two cents).

    - To me, the way push certificates are handled now feels like having
      git commit -S producing a detached signature that you have to
      embed somehow in the resulting commit object. (As a matter of
      fact, many points on [1] seem to back this notion, and even recall
      some drawbacks on push certificates the way they are handled
      today)

I understand the concurrency concerns, so I agree with stefan's solution, although I don't know how big of a deal it would be, if git already supports --atomic pushes (admittedly, I haven't checked if there are any guards in place for someone who pushes millions of refs atomically). It'd be completely understandable to have a setting to disable hadnling of --signed pushes and, ideally, a way to warn the user of this feature being disabled if --signed is sent.

Maybe I'm missing the bigger picture, but to me it feels that a named ref to the latest push certificate would make it easier to handle/tool/sync around the push certificate solution?

Thanks, -Santiago.

[1] https://public-inbox.org/git/CAJo=hJvWbjEM9E5AjPHgmQ=eY8xf=Q=xtukeu2Ur7auUqeabDg@mail.gmail.com/