Re: [PATCH 1/1] verify-tag/verify-commit should exit unsuccessfully when signature is not trusted

[Santiago Torres]

> > disable fetching keys from hkp servers. This way signature verification
> > should fail.
> > 
> > Thanks,
> > -Santiago.
> 
> This is not a deviation. GPG correctly recognizes difference between trusted,
> untrusted and unknown levels. git on the other hand does not. Well it did 
> until
> the commit a4cc18f29. That one removed GPG exit code propagation.

Oh wow. Sorry my assumption parted from looking at the code back in the
day where this happens. I assumed git was quietly propagating the gpg
error code and took it from there. 

Now that I think about it though, verify tag can verify more than one
tag. I assume that this would make it difficult to propagate individual
errors in trusting. I honestly don't know what's the best way to modify
this behavior then.

Thanks,
-Santiago


signature.asc
Description: PGP signature

Re: [PATCH 1/1] verify-tag/verify-commit should exit unsuccessfully when signature is not trusted

[Santiago Torres]

On Wed, Aug 01, 2018 at 12:19:42AM +, brian m. carlson wrote:
> On Tue, Jul 31, 2018 at 10:05:22PM +0200, Vojtech Myslivec wrote:
> > Hello,
> > 
> > me and my colleague are struggling with automation of verifying git
> > repositories and we have encountered that git verify-commit and
> > verify-tag accepts untrusted signatures and exit successfully.
> 
> I don't have strong feelings on your change one way or the other, but
> for automation it may be useful to use the --raw flag, which gives you
> the raw gpg output and much greater control.  For example, you can
> require that a subkey is or is not used or require certain algorithms.
> 
> I will say that most signatures are untrusted in my experience, so
> unless people are using TOFU mode or making local signatures, git will
> exit nonzero for most signatures.  I think the current status is to exit
> on a good signature, even if it isn't necessarily a valid signature.
> 
> I'm interested to hear others' thoughts on this.

I'd find it odd that we deviate from the gpg behavior, that returns 0
when verifyng an untrusted signatures. Tooling around gpg is generally
difficult for this reason, but using the raw output should be enough to
discard signatures with untrusted keys.

Another alternative is to use a keyring with trusted keys *only* and
disable fetching keys from hkp servers. This way signature verification
should fail.

Thanks,
-Santiago.

> -- 
> brian m. carlson: Houston, Texas, US
> OpenPGP: https://keybase.io/bk2204




signature.asc
Description: PGP signature

Re: does a new repo actually *need* default template content?

[Santiago Torres]

On Fri, Jun 01, 2018 at 03:06:10AM -0400, Jeff King wrote:
> On Mon, May 28, 2018 at 07:56:12PM +0200, Ævar Arnfjörð Bjarmason wrote:
> 
> > 
> > On Mon, May 28 2018, Robert P. J. Day wrote:
> > 
> > >   (apologies for more pedantic nitpickery, just little things i'm
> > > running across in my travels. aside: i actually teach git courses, so
> > > it's a bit embarrassing that i don't know some of this stuff. *sigh*.)
> > 
> > Aside from maybe the empty branches/ directory (see c8a58ac5a5 ("Revert
> > "Don't create the $GIT_DIR/branches directory on init"", 2009-10-31)),
> > none of this is needed.
> > 
> > I wish we didn't create any of this stuff, but have never been inclined
> > to make that my hill to die on.
> > 
> > I think we're much better off just shipping e.g. a single README file in
> > hooks/, or just nothing at all.
> 
> FWIW, that's my opinion, too (including the "hill to die on" part).
> 

+1. It'd probably be worth having the sample hooks be part of the
installation, but not as part of every repository (e.g., hold them under
/usr/share/git/samples/hooks/ or something along those lines).

> I also wish hooks were just shell snippets in the config files that
> could follow the usual config-precedence rules.

I like this idea, but I'd probably keep the snippets in a separate file
to keep things clean.

Thanks,
-Santiago.


signature.asc
Description: PGP signature

Re: GIT 2.3.1 - Code Execution Vulnerability

[Santiago Torres]

Hi, Christian.

They are probably talking about one of these[1][2]. I don't have access
to a solaris machine right now, so I don't know which is the latest
version they ship, but they probably backported patches. 

Here we can't do much more about it, given that the packagers for your
solaris version are the ones (possibly) packaging 2.3.1. I'd email or
open a ticket with Oracle after making sure they 1) haven't backported
patches to fix these, or 2) don't have a newer version in their
repositories.

Cheers!
-Santiago.


[1] https://security.archlinux.org/CVE-2017-1000117
[2] https://nvd.nist.gov/vuln/detail/CVE-2016-2324

On Thu, Jan 25, 2018 at 06:02:34PM +0100, christian.del.vecc...@zurich.com 
wrote:
> dear Team
> 
> I am Christian Del Vecchio,and i work in the infrastructure of Middleware on 
> Zurich.
> we have installed in our system Sun your product in order to connect to our 
> bitbucket repository.
> 
> we have followed the instruction provided on your Web Page:
> 
> https://git-scm.com/download/linux
> pkgutil -i git
> 
> the version installed is the 2.3.1, and actually it works.
> 
> but last week our security team informed that this software didn't pass the 
> check control due: Git Server and Client Remote Code Execution Vulnerability
> 
> 
> please, is it available a newer version that fix this problem?
> 
> our system is: Sun Solaris v10 sparc
> 
> best regards
> __ 
> 
> Christian Del Vecchio 
> Middleware SME 
> 
> Zurich Insurance Group Ltd. 
> bac de Roda 58, 
> Building C, 4th floor 
> 08019 Barcelona, Spain 
> 
> 64402 (internal) 
> +34 93 4465402 (direct) 
> christian.del.vecc...@zurich.com 
> http://www.zurich.com 


signature.asc
Description: PGP signature

Re: upstreaming https://github.com/cgwalters/git-evtag ?

[Santiago Torres]

> > push for hash-agnosticity. I don't know if git-evtag is hash agnostic,
> > but if it is not, then we have two transition plans to think about.
> 
> I don't think there's even a question here: Git has to transition off
> of SHA-1.
> 
> In that context, Stefan's comment is a welcome one: once we've
> transitioned off of SHA-1, having a separate evtag feature would make
> git more complicated without any benefit to match.  To put it another
> way, the gpgsig-sha256 field described in
> Documentation/technical/hash-function-transition.txt provides
> essentially the same functionality as an evtag.  What's missing is an
> implementation of it.
> 
> I'm happy to help in any way I can (reviews, advice, etc).

Same here, although I'm a bit swamped with other work... 

> 
> > Full disclosure, I published a "competing" solution a couple of years
> > ago[1] but, in my personal opinion, I think push certificates can
> > achieve the same security guarantees as my system with very little
> > changes.
> 
> Work to improve the usability of push certs would also be very very
> welcome.

I agree. I personally think that at least the sample hook work on here
would be a good candidate for this[1], although I don't know what's the
status of it. The way they are right now, they should at least warn when
push certificates are not enabled on the server side (i.e., there is no
hook to handle it).

> 
> Thanks and hope that helps,
> Jonathan

No, thanks to you :)

-Santiago.

[1] https://public-inbox.org/git/20171202091248.6037-1-r...@shikherverma.com/


signature.asc
Description: PGP signature

Re: upstreaming https://github.com/cgwalters/git-evtag ?

[Santiago Torres]

> > See Documentation/technical/hash-function-transition.txt
> > for how to do it.
> 
> evtag took me a day or two to write initially and doesn't
> impose any requirements on users except a small additional
> bit of software.

I agree that, in nature it shouldn't be difficult, but I also think that
things usually take longer when you try to minimize code reuse and
streamline the system's design.

> In contrast, working on hash-function-transition.txt?  That
> seems like it'd easily consume many person-months of work.
> And that plan only exists post-shatter.io, whereas git-evtag
> long predates both.

I think this is partly true. A hash transition has been brought up
multiple times pre-shattered. In my opinion shattered was a much-needed
PR push for SHA1 deprecation. In practice, things changed very little.

> > Personally I'd dislike to include ev-tags as it might send a signal
> > of "papering over sha1 issues instead of fixing it".
> 
> I don't agree.  I think it's pretty clear that a hash function transition
> would be a huge amount of work - not least because of course
> there are now at least two widely used implementations of git in C,
> plus https://www.eclipse.org/jgit/ plus...

I agree with Stefan here. I think it's better in the long-term to
push for hash-agnosticity. I don't know if git-evtag is hash agnostic,
but if it is not, then we have two transition plans to think about.

> 
> > push certificates are somewhat underdocumented, see the
> 
> Why not call them "git signed pushes"?  Junio's post
> even says "the signed push".

A signed push creates a push certificate.
> 
> And I just looked at this a little bit more but I'm not sure I
> see how this covers the same goal as evtags;

Correct me if I'm wrong (it's been a couple of years) but last time I
read about git evtags, they basically did the following:

1. Create a signed tag.
2. Create a signed statement of all the references.
3. Create a checksum of the checked out code on the tag.
4. Create a tarball of it.

I think 1) is already happening, 2) is very similar information to the
one contained in a push certificate. I don't know how necessary are 3)
and 4), but that's just my very opinionated take on it.

Full disclosure, I published a "competing" solution a couple of years
ago[1] but, in my personal opinion, I think push certificates can
achieve the same security guarantees as my system with very little
changes.

Cheers!
-Santiago.

[1] 
https://www.usenix.org/conference/usenixsecurity16/technical-sessions/presentation/torres-arias


signature.asc
Description: PGP signature

Re: upstreaming https://github.com/cgwalters/git-evtag ?

[Santiago Torres]

> Personally I'd dislike to include ev-tags as it might send a signal
> of "papering over sha1 issues instead of fixing it".

+1. I probably didn't convey it well, but this is what I was hoping for.
I think git has enough building blocks to provide something akin to git
evtags already.

Thanks,
-Santiago.


signature.asc
Description: PGP signature

Re: upstreaming https://github.com/cgwalters/git-evtag ?

[Santiago Torres]

Yeah, I see where you're coming from. I don't think push certificates
have caught on yet...

You can read on them on [1], and also under the
Documentation/git-push:147.

There's also another PR trying to make a sample hook for signed
pushes on [2].

The basic idea is to push a signed data structure with relevant git
reference information as a git object to avoid a server/mitm from moving
references around.

Cheers!
-Santiago.

[1] 
https://public-inbox.org/git/1408485987-3590-1-git-send-email-gits...@pobox.com/
[2] https://public-inbox.org/git/20171202091248.6037-1-r...@shikherverma.com/

On Mon, Jan 08, 2018 at 03:42:33PM -0500, Colin Walters wrote:
> 
> 
> On Mon, Jan 8, 2018, at 3:40 PM, Santiago Torres wrote:
> > Hi,
> > 
> > I personally like the idea of git-evtags, but I feel that they could be
> > made so that push certificates (and being hash-algorithm agnostic)
> > should provide the same functionality with less code.
> 
> What's a "push certificate"?  (I really tried to find it in Google,
> even going to page 4 where one can start to see tumbleweeds
> going by... I'm fairly certain you're not talking about something related
> to iOS notifications) 


signature.asc
Description: PGP signature

Re: upstreaming https://github.com/cgwalters/git-evtag ?

[Santiago Torres]

Hi,

I personally like the idea of git-evtags, but I feel that they could be
made so that push certificates (and being hash-algorithm agnostic)
should provide the same functionality with less code.

To me, a git evtag is basically a signed tag + a data structure similar
to a push certificate embedded in it. I wonder if, with the current
tooling in git, this could be done as a custom command...

Cheers!
-Santiago.

On Mon, Jan 08, 2018 at 03:12:00PM -0500, Colin Walters wrote:
> Hi, so quite a while ago I wrote this:
> https://github.com/cgwalters/git-evtag
> 
> Since I last posted about this on the list here, of course
> shattered.io happened.  It also looks
> like there was a node.js implementation written.
> 
> Any interest in having this in core git?  


signature.asc
Description: PGP signature

Re: git status always modifies index?

[Santiago Torres]

Ah, my bad. I missed this patch...

Good luck!
-Santiago.


signature.asc
Description: PGP signature

Re: git status always modifies index?

[Santiago Torres]

On Wed, Nov 22, 2017 at 09:37:09AM -0600, Nathan Neulinger wrote:
> What I'm meaning is - why does it need to write the index back out to disk?
> 
> From looking at the code in builtin/commit.c it looks like it takes a lock
> on the index, collects the status, and then unconditionally rewrites the
> index file.
>
Hmm, I just took a look at those lines and I see what you mean. From
what I understand, this is to cache the result of the index computation
for ensuing git calls.

> I'm proposing that the update_index_if_able call not actually be issued if
> it would result in a ownership change on the underlying file - such as a
> simple case of root user or other privileged account issuing 'git status' in
> a directory.

I don't think this would be a desire-able default behavior. I'd wager
that running git status using different accounts is not a great idea ---
specially interacting with a user repository as root.

> I understand completely that it would be expected to be altered if the
> privileged user did a commit/add or any other operation that was inherently
> a 'write' operation, but doesn't seem like status should be one of those
> cases.

I think it's because of the reasons above. That being said, I don't know
what the rest of the community would think of something akin to a
--no-update-index type flag.

Cheers!
-Santiago.


signature.asc
Description: PGP signature

Re: git status always modifies index?

[Santiago Torres]

Hi Nathan.

Do you mean git-status writing an index file? What would you suggest for
git-status to compute which files have changed without modifying an
index-file? Or are you suggesting git-status to fail if the index file
doesn't belong to the user-id who invoked the command...

Thanks,
-Santiago


signature.asc
Description: PGP signature

Re: [PATCH] t/lib-gpg: fix gpgconf stderr redirect to /dev/null

[Santiago Torres]

> If it's a small range of gnupg versions which fail badly when the GNUPGHOME
> dir is removed, then there's far less reason for git to do much more than
> make an effort to kill the agent.
> 

Yeah. FWIW, it may be reasonable to consider dropping the patch once we are
certain distros don't ship this range anymore.

> It seems like all the gnupg versions which may suffer from the bug also
> support gpgconf --kill, which would make any further change in the test to
> handle versions which lack the --kill option moot.
> 

This is also true. We should definitely not litter the stdout with ENOENT-like
error messages though...

Thanks again for catching this!
-Santiago.


signature.asc
Description: PGP signature

Re: [PATCH] t/lib-gpg: fix gpgconf stderr redirect to /dev/null

[Santiago Torres]

Quick followup.

The version that triggers this is at least 2.1.21[1]. I recall there was some
wiggle room on minor versions before it.

Thanks!
-Santiago.

[1] https://dev.gnupg.org/T3218

On Mon, Nov 13, 2017 at 06:02:02PM -0500, Santiago Torres wrote:
>  
> > Were the ENOENT errors you encountered in running the tests multiple times
> > easy to reproduce? 
> 
> If you had the right gpg2, it should be easy to repro with just re-running.
> 
> > If so, I can certainly try to reproduce them and then
> > run the tests with --reload in place of --kill to gpgconf.  If that worked
> > across the various gnupg 2.x releases, it would be a simple enough change to
> > make as a follow-up.
> 
> Let me dig up the exact versions. IIRC it was somewhere between 2.1.0 and 
> 2.2.x
> or so. I think somewhere within the patch re-rolls I had the exact versions.
> 
> Cheers!
> -Santiago.




signature.asc
Description: PGP signature

Re: [PATCH] t/lib-gpg: fix gpgconf stderr redirect to /dev/null

[Santiago Torres]

 
> Were the ENOENT errors you encountered in running the tests multiple times
> easy to reproduce? 

If you had the right gpg2, it should be easy to repro with just re-running.

> If so, I can certainly try to reproduce them and then
> run the tests with --reload in place of --kill to gpgconf.  If that worked
> across the various gnupg 2.x releases, it would be a simple enough change to
> make as a follow-up.

Let me dig up the exact versions. IIRC it was somewhere between 2.1.0 and 2.2.x
or so. I think somewhere within the patch re-rolls I had the exact versions.

Cheers!
-Santiago.


signature.asc
Description: PGP signature

Re: [PATCH] t/lib-gpg: fix gpgconf stderr redirect to /dev/null

[Santiago Torres]

 
> Of course, beyond getting stderr to /dev/null, there is the fact that on
> versions of gnupg < 2.1, gpgconf --kill is not available.  I noticed this with
> gnupg-2.0.14 on CentOS 6.  It also occurs on CentOS 7, which provides
> gnupg-2.0.22.
> 
> I don't know if there's much value in trying to better handle older gnupg-2.0
> systems. 

Hi Todd.

Thanks for catching the redirection issue! I agree that the other fixes feel
like overkill. Are you certain that switching to gpgconf --reload will have the
same effect as --kill? (I know that this is the case for scdaemon only).

Thanks again!
-Santiago.


signature.asc
Description: PGP signature

Re: Unable to use --patch with git add

[Santiago Torres]

It'd be helpful to know:

- What did you do?
- What did you expect to happen?
- What happened instead?

I suspect you are using --patch with a new file, so you probably need to first
add it with -N or so. This is just a shot in the dark though...

Thanks,
-Santiago.

On Wed, Oct 11, 2017 at 11:16:39PM +0530, Ayush Goel wrote:
> $ git --version
> git version 2.14.2
> 
> What more details can I provide to help debug this?
> 
> -- 
> Regards,
> Ayush Goel


signature.asc
Description: PGP signature

Re: git ls-tree -d doesn't work if the specified path is the repository root?

[Santiago Torres]

 
> Is there some reason for this? This behavior seems like a bug to me:
> it means that prior to calling ls-tree I need to check if the
> referenced path happens to be the root, and if so, find some other
> means (rev-parse?) of converting it to a treehash.

Hmm, interesting.

I see these four behaviors:

[santiago@LykOS bg_daemon]$ git ls-tree -d HEAD -- src
04 tree 238a62ca62527423fd3190d00917ddfef0d254a3src

[santiago@LykOS bg_daemon]$ git ls-tree -d HEAD -- src/
04 tree 767beaaf0927f89e630c52830b6fbac138ec039asrc/bg_daemon

[santiago@LykOS bg_daemon]$ git ls-tree -d HEAD -- .
04 tree 238a62ca62527423fd3190d00917ddfef0d254a3src
04 tree de12675d1023b1e743f7e11c2276fc417b3650a6tests

[santiago@LykOS bg_daemon]$ git ls-tree -d HEAD -- ./
04 tree 238a62ca62527423fd3190d00917ddfef0d254a3src
04 tree de12675d1023b1e743f7e11c2276fc417b3650a6tests

IMHO, I think it'd be more consistent if . returned the root treehash and ./
returned the treehash of the directories in root.

I don't personally know if there is a reason for this...

Thanks,
-Santiago.

> 
> Thanks,
> —Roy


signature.asc
Description: PGP signature

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

[Santiago Torres]

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=xtukeu2ur7auuqea...@mail.gmail.com/


signature.asc
Description: PGP signature

Re: signing commits using gpg2

[Santiago Torres]

On Sat, Sep 02, 2017 at 05:11:50PM -0400, shawn wilson wrote:
> tl;dr - how do I get git to use gpg2 to sign things?
> 
> I'm using gpg2 (so no agent options are configured but an agent is
> running) which is configured w/ a Nitrokey (Pro if it matters):
> 
>  % git commit -m "Initial."
> 
>  gits/bash-libs (master ⚡) localhost
> gpg: detected reader `Nitrokey Nitrokey Pro (3467) 00 00'
> gpg: pcsc_connect failed: sharing violation (0x801b)
> gpg: apdu_send_simple(0) failed: locking failed
> Please insert the card and hit return or enter 'c' to cancel:
> gpg: pcsc_connect failed: sharing violation (0x801b)
> gpg: pcsc_connect failed: sharing violation (0x801b)
> gpg: apdu_send_simple(0) failed: locking failed
> Please insert the card and hit return or enter 'c' to cancel: c
> gpg: selecting openpgp failed: general error
> gpg: signing failed: general error
> gpg: signing failed: general error
> error: gpg failed to sign the data
> fatal: failed to write commit object

This seems to be an issue with your gpg agent configuration (even if
there is none). 

I can't seem to reproduce, although I don't have a nitrokey, so this is
most likely an issue with either:

- the PIV/CCID interface of the nitrokey using gpg2. I"m not familiar
  enough with nitrokeys to debug this, but keys are usually super
  paranoid when signing arbitrary buffers.
- the fork call within git on gpg2. 

I think the second one is rather unlikely, but it's worth giving it a
try...

  ~ localhost
> -BEGIN PGP MESSAGE-
> Version: GnuPG v2
> [SNIPPED]
> -END PGP MESSAGE-
> 

I noticed you didn't try gpg2 -d foo.gpg? Am I missing something?

> However, if I try this w/ the old gpg:
> 
>  % gpg -ae -o foo.gpg foo
> 
>  ~ localhost
>  % gpg -d foo.gpg
> 
>  ~ localhost
> gpg: detected reader `Nitrokey Nitrokey Pro (3467) 00 00'
> gpg: pcsc_connect failed: sharing violation (0x801b)
> gpg: apdu_send_simple(0) failed: locking failed
> Please insert the card and hit return or enter 'c' to cancel: c
> gpg: selecting openpgp failed: general error
> gpg: encrypted with 3072-bit RSA key, ID 41826CFB, created 2017-03-13
>   "Shawn Wilson <ag4ve...@gmail.com>"
> gpg: public key decryption failed: general error
> gpg: decryption failed: secret key not available

This feels like an issue with the interface to the key itself. Can you
start a non-detached agent with --verbose to see exactly where it blows up?

We probably want to continue this offlist as this seems more of a gpg
issue rather than git. We can always come back if we figure out this is
something git related :)

Cheers!
-Santiago.


signature.asc
Description: PGP signature

Re: t5551 hangs ?

[Santiago Torres]

> No concurrency.
> That's what I do: ./t5551-http-fetch-smart.sh

Oh ok! I was just trying to weed out what could out of place. Just to
make sure, your old compute is also running debian 8 latest? 
> 
> > - You probably want to see the version of apache this is running/etc.
> 
> The one that comes with Debian -
> I am not an expert here, what is it that is interesting ?

I'm not sure, but sometimes it's helpful to look at changelogs between
versions of packages to get a poiner as to what could be wrong. I wonder
if it's apache the one that's failing on this case. Probably a paste of
dpkg-query --show apache would help me try to reproduce..

> 
> > - What happens if you kill the apache processes? 
> 
> I am left with these processes:
> /home/blabla/pu/git -C too-many-refs fetch -q --tags
> /home/blabla/pu/git-remote-http origin http://127.0.0.1:5551/smart/repo.git
> 

So it's probably not an issue with apache, as the socket should be
hopefully closed gracefully.

> > 
> > I can't reproduce on my side, but let me see if I can dig a little into
> > it.
> 
> Thanks, more tips or things I can do are welcome.
> t5551 seems to be flaky - from time to time.
> It seems that I have it reproducable unstable, so if someone has more
> ideas, please.

I'm still unable to reproduce. Do you think you can enable GIT_TRACE,
GIT_TRACE_PACK and GIT_TRACE_CURL and pastebin/paste what you see?

Cheers!
-Santiago.


signature.asc
Description: PGP signature

Re: t5551 hangs ?

[Santiago Torres]

Hello, Torsten. 

On Tue, Aug 22, 2017 at 07:10:59AM +0200, Torsten Bögershausen wrote:
> Hej,
> I found 2 threads about hanging t5551, one in 2016, and one question
> from Junio somewhen in 2017.
> I have it reproducable here:
> - Debian 8, 64 bit
> - SSD
> - Half-modern processor ;-)

I don't think the SSD/regular disk have anything to do with it. 

- Are you running tests concurrently? (e.g., with -j x)
- What happens if you run the test individually (i.e., cd t and
  ./t5551...)
- You probably want to see the version of apache this is running/etc.
- What happens if you kill the apache processes? 

I can't reproduce on my side, but let me see if I can dig a little into
it.

Cheers!
-Santiago.


signature.asc
Description: PGP signature

[PATCH v2] t: lib-gpg: flush gpg agent on startup

[santiago]

From: Santiago Torres <santi...@nyu.edu>

When running gpg-relevant tests, a gpg-daemon is spawned for each
GNUPGHOME used. This daemon may stay running after the test and cache
file descriptors for the trash directories, even after the trash
directory is removed. This leads to ENOENT errors when attempting to
create files if tests are run multiple times.

Add a cleanup script to force flushing the gpg-agent for that GNUPGHOME
(if any) before setting up the GPG relevant-environment.

Helped-by: Junio C Hamano <gits...@pobox.com>
Signed-off-by: Santiago Torres <santi...@nyu.edu>
---
 t/lib-gpg.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh
index ec2aa8f68..43679a4c6 100755
--- a/t/lib-gpg.sh
+++ b/t/lib-gpg.sh
@@ -31,6 +31,7 @@ then
chmod 0700 ./gpghome &&
GNUPGHOME="$(pwd)/gpghome" &&
export GNUPGHOME &&
+   (gpgconf --kill gpg-agent 2>&1 >/dev/null || : ) &&
gpg --homedir "${GNUPGHOME}" 2>/dev/null --import \
"$TEST_DIRECTORY"/lib-gpg/keyring.gpg &&
gpg --homedir "${GNUPGHOME}" 2>/dev/null --import-ownertrust \
-- 
2.13.3

Re: [PATCH] t: lib-gpg: flush gpg agent on startup

[Santiago Torres]

> With that "run it but ignore the outcome even if we failed to.", we
> do not have to worry about any of that ;-)

Oh right! thanks for the suggestion! Let me re-roll...

Thanks,
-Santiago.



signature.asc
Description: PGP signature

Re: [PATCH] t: lib-gpg: flush gpg agent on startup

[Santiago Torres]

This is the patch that stemmed from [1].

I tried to keep it simple and not noisy, alhtough it breaks the &&
chain, it needs to be run right before the --import command. I also
decided to drop the switch chain in case that regression was to be
introduced in the future in other versions (hopefully gpgconf goes
nowhere by then).

I was able to test this on debian oldstable/stable and arch.

Cheers!
-Santiago.

[1] https://public-inbox.org/git/xmqqvampmnmv@gitster.mtv.corp.google.com/

On Thu, Jul 20, 2017 at 12:58:14PM -0400, santi...@nyu.edu wrote:
> From: Santiago Torres <santi...@nyu.edu>
> 
> When running gpg-relevant tests, a gpg-daemon is spawned for each
> GNUPGHOME used. This daemon may stay running after the test and cache
> file descriptors for the trash directories, even after the trash
> directory is removed. This leads to ENOENT errors when attempting to
> create files if tests are run multiple times.
> 
> Add a cleanup script to force flushing the gpg-agent for that GNUPGHOME
> (if any) before setting up the GPG relevant-environment.
> 
> Helped-by: Junio C Hamano <gits...@pobox.com>
> Signed-off-by: Santiago Torres <santi...@nyu.edu>
> ---
>  t/lib-gpg.sh | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh
> index ec2aa8f68..7a6c7ee6f 100755
> --- a/t/lib-gpg.sh
> +++ b/t/lib-gpg.sh
> @@ -31,6 +31,7 @@ then
>   chmod 0700 ./gpghome &&
>   GNUPGHOME="$(pwd)/gpghome" &&
>   export GNUPGHOME &&
> + gpgconf --kill gpg-agent 2>&1 >/dev/null
>   gpg --homedir "${GNUPGHOME}" 2>/dev/null --import \
>   "$TEST_DIRECTORY"/lib-gpg/keyring.gpg &&
>   gpg --homedir "${GNUPGHOME}" 2>/dev/null --import-ownertrust \
> -- 
> 2.13.3
> 


signature.asc
Description: PGP signature

[PATCH] t: lib-gpg: flush gpg agent on startup

[santiago]

From: Santiago Torres <santi...@nyu.edu>

When running gpg-relevant tests, a gpg-daemon is spawned for each
GNUPGHOME used. This daemon may stay running after the test and cache
file descriptors for the trash directories, even after the trash
directory is removed. This leads to ENOENT errors when attempting to
create files if tests are run multiple times.

Add a cleanup script to force flushing the gpg-agent for that GNUPGHOME
(if any) before setting up the GPG relevant-environment.

Helped-by: Junio C Hamano <gits...@pobox.com>
Signed-off-by: Santiago Torres <santi...@nyu.edu>
---
 t/lib-gpg.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh
index ec2aa8f68..7a6c7ee6f 100755
--- a/t/lib-gpg.sh
+++ b/t/lib-gpg.sh
@@ -31,6 +31,7 @@ then
chmod 0700 ./gpghome &&
GNUPGHOME="$(pwd)/gpghome" &&
export GNUPGHOME &&
+   gpgconf --kill gpg-agent 2>&1 >/dev/null
gpg --homedir "${GNUPGHOME}" 2>/dev/null --import \
"$TEST_DIRECTORY"/lib-gpg/keyring.gpg &&
gpg --homedir "${GNUPGHOME}" 2>/dev/null --import-ownertrust \
-- 
2.13.3

Re: What's cooking in git.git (Jul 2017, #04; Thu, 13)

[Santiago Torres]

> Oh, wait, I can run "gpg" just fine, but I do not seem to have
> gpgconf.
> 
>   $ type gpgconf
>   bash: type: gpgconf: not found
> 
> The patch may need a bit more cross-version work, it seems.

Right, sorry about that. 

I was testing against Debian Stretch/Arch, who do ship gpg2 with
gpgconf. It seems Debian oldstable and other variants still ship gpg1,
which doesn't have it. Would it make sense to have a fallthrough branch
on the switch statement for gpg2.1 instead? something like the attached patch.

Thanks,
-Santiago.
From 07ab87c1ddb31197a3a5c124ad5a2462a460d4e3 Mon Sep 17 00:00:00 2001
From: Santiago Torres <santi...@nyu.edu>
Date: Tue, 18 Jul 2017 13:16:11 -0400
Subject: [RFC/PATCH] t: lib-gpg: flush gpg agent on startup

When running gpg-relevant tests, a gpg-daemon is spawned for each
GNUPGHOME used. This daemon may stay running after the test and cache
file descriptors for the trash directories, even after the trash
directory is removed. This leads to ENOENT errors when attempting to
create files if tests are run multiple times.

Add a cleanup script to force flushing the gpg-agent for that GNUPGHOME
(if any) before setting up the GPG relevant-environment.

Helped-by: Junio C Hamano <gits...@pobox.com>
Signed-off-by: Santiago Torres <santi...@nyu.edu>
---
 t/lib-gpg.sh | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh
index ec2aa8f68..ffb20a438 100755
--- a/t/lib-gpg.sh
+++ b/t/lib-gpg.sh
@@ -10,6 +10,8 @@ then
 	'gpg (GnuPG) 1.0.6'*)
 		say "Your version of gpg (1.0.6) is too buggy for testing"
 		;;
+	'gpg (GnuPG) 2.1'*)
+		GNUPGHOME="$(pwd)/gpghome" gpgconf --kill all ;&
 	*)
 		# Available key info:
 		# * Type DSA and Elgamal, size 2048 bits, no expiration date,
-- 
2.13.3



signature.asc
Description: PGP signature

Re: What's cooking in git.git (Jul 2017, #04; Thu, 13)

[Santiago Torres]

On Mon, Jul 17, 2017 at 03:09:44PM -0700, Junio C Hamano wrote:
> I am not sure if it is merely "if it's even necessary"; if there are
> two tests running in parallel, with their own separate
> $TRASH_DIRECTORY, and one of them say "kill the agent" at the
> beginning, would it affect the other test, depending on the timing?

This shouldn't happen, provided their respective GNUPGHOME env vars
aren't the same. A gpg-agent process is spawned per GNUPGHOME on the
versions that I've tested.

> 
> I would imagine that the sockets are kept per GNUPGHOME and they are
> not going to interfere, so if that is the case, I do not think we
> mind helping folks with a buggy versions of GnuPG by having a "let's
> be cautious and kill a leftover agent before starting to test"
> patch, as long as the reason why we do so is clearly understood and
> documented.

I double checked the patch/solutions/causes just to be sure I'm not
doing anything crazy. Here's a v2 of the patch that kills the agent upon
cleanup rather than startup.

Thanks!
-Santiago.
From 20491890b804d13f9edb0205c1cc21d080beffe2 Mon Sep 17 00:00:00 2001
From: Santiago Torres <santi...@nyu.edu>
Date: Tue, 18 Jul 2017 13:16:11 -0400
Subject: [RFC/PATCH] t: test-lib: flush gpg agent on cleanup

When running gpg-relevant tests, a gpg-daemon is spawned for each
GNUPGHOME used. This daemon may stay running after the testand cache ile
descriptors for the trash directories, even after the trash directory is
removed. This leads to ENOENT errors when attempting to create files if
tests are run multiple times.

Add a cleanup script to force flushing the gpg-agent when before
removing the trash directory if the test is GPG-relevant.

Helped-by: Junio C Hamano <gits...@pobox.com>
Signed-off-by: Santiago Torres <santi...@nyu.edu>
---
 t/test-lib.sh | 5 +
 1 file changed, 5 insertions(+)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 1b6e53f78..ed8796d7a 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -732,6 +732,11 @@ test_done () {
 		EOF
 	fi
 
+	if test_have_prereq GPG
+	then
+		GNUPGHOME="$TRASH_DIRECTORY/gpghome" gpgconf --kill all
+	fi
+
 	if test "$test_fixed" != 0
 	then
 		say_color error "# $test_fixed known breakage(s) vanished; please update test(s)"
-- 
2.13.3



signature.asc
Description: PGP signature

Re: What's cooking in git.git (Jul 2017, #04; Thu, 13)

[Santiago Torres]

> > I'll dig into this. This sounds a way more reasonable approach.
> 
> Thanks.  Another thing that may help, if it turns out that we do
> want to let agent run when it wants to

I did some digging on the reason as to why this was happening. It turns
out there is a bug on gnupg. As of gpg 2.1.21, all operations require an
agent running (BTW --no-use-agent is effectively a no-op now). 


Given that on versions 2.19-21, they changed the way sockets are
created to be stored on /run/user/UID/gnupg/... deleting the $GNUPG_HOME
directory wouldn't flush the agent socket, and would leave an agent
instance per test running, possibly forever. E.g., make test would
result in the following:

santiago at ~ ✔ pgrep -a gpg-agent
632 gpg-agent --homedir /git/t/trash directory.t6050-replace/gpghome 
--use-standard-socket --daemon
1192 /usr/bin/gpg-agent --supervised
2939 gpg-agent --homedir /git/t/trash 
directory.t5801-remote-helpers/gpghome --use-standard-socket --daemon
4656 gpg-agent --homedir /git/t/trash directory.t6300-for-each-ref/gpghome 
--use-standard-socket --daemon
5427 gpg-agent --homedir /git/t/trash directory.t7510-signed-commit/gpghome 
--use-standard-socket --daemon
5898 gpg-agent --homedir /git/t/trash 
directory.t6302-for-each-ref-filter/gpghome --use-standard-socket --daemon
7747 gpg-agent --homedir /git/t/trash directory.t7003-filter-branch/gpghome 
--use-standard-socket --daemon
12922 gpg-agent --homedir /git/t/trash directory.t7600-merge/gpghome 
--use-standard-socket --daemon
13572 gpg-agent --homedir /git/t/trash directory.t7004-tag/gpghome 
--use-standard-socket --daemon
14521 gpg-agent --homedir /git/t/trash directory.t5534-push-signed/gpghome 
--use-standard-socket --daemon
16563 gpg-agent --homedir /git/t/trash 
directory.t5541-http-push-smart/gpghome --use-standard-socket --daemon
17853 gpg-agent --homedir /git/t/trash directory.t7030-verify-tag/gpghome 
--use-standard-socket --daemon
29858 gpg-agent --homedir /git/t/trash 
directory.t7612-merge-verify-signatures/gpghome --use-standard-socket --daemon
31100 gpg-agent --homedir /git/t/trash directory.t4202-log/gpghome 
--use-standard-socket --daemon

Other projects such as notmuch opted for a solution that's simlar to
what I had suggested[1], but I wonder if it's even necessary to do.
There is already a fix on the master branch of gnupg[2], which I imagine
will show up to the next version of gpg2.

I don't think it would make sense to fix anything on our side, unless we
want to be extra sure the test suite is not leaking agents for all gpg
versions (including these minor versions). 

What is your take?

Thanks!
-Santiago.

[1] https://paste.debian.net/976970/
[2] https://dev.gnupg.org/T3218


signature.asc
Description: PGP signature

Re: What's cooking in git.git (Jul 2017, #04; Thu, 13)

[Santiago Torres]

Hi, Junio. Thanks for replying.

> I postponed it when I saw it the first time to see if anybody
> comments on it, and then it turns out nobody was interested, and it
> remained uninteresting to the list to this day.  
> 

True, that's what I was afraid of, but I wanted to give it some closure.

> Now, after looking at the message again, from the patch description,
> I would believe you that you experienced _some_ trouble when the
> gpg-agent that is auto-spawned by gpg gets left behind (as I do not
> see any hits from "git grep gpg-agent t/", we are not deliberately
> using the agent). 

True, this is what I could gather from asking people over at #gnupg. The
agent spawns a socket for a GNUPGHOME and leaves it open outside of the
home folder (and it caches the inode for the olddir or so).

> However, I could not convince myself that the
> solution is credible.  

I think you're right on this. I'd rather have more people reproduce the
issue (some of my colleagues were able to do so, but we all were running
the latest GPG, vanilla conf) and find the root of the issue too.

> but the current directory of this part is the $TRASH_DIRECTORY,
> which is always created anew from the beginning in a test.  What
> agent process could possibly be running there immedately after
> creating ./gpghome (which happens with "mkdir gpghome &&" without
> "-p" just before the context of this hunk---if the agent was running
> already, the directory would have been there, and mkdir would have
> failed, which would have caused "test_set_prereq GPG" at the end of
> the "&&" cascade to be skipped.  In other words, it is unclear to
> me, and your log message does not sufficiently explain, why this is
> the right solution (or what the exact cause of the symptom is, for
> that matter).

I see. What is causing this (as far as my current understanding goes)
is:

1) First iteration of the tests is run, the .gpghome is created and a
unix socket is created too. The keychain is imported etc. Tests run
normally.

2) The test ends, and the trash directory (along with the .gpghome) are
flushed, but the agent is not aware of this. The socket is still
open.

3) The second iteration of the tests is run, the new .gpghome is
created, but when the keychain fails to import and the agent errors
out with ENOENT. The and-chain fails and test_set_preqreq GPG is
skipped (as you pointed out).

This last bit is apparently a result of the agent trying to be clever
with the paths. 

> 
> Or perhaps the gpg-agent socket is created somewhere outside the
> GNUPGHOME and stays behind even after a previous run of the same
> test finishes and $TRASH_DIRECTORY gets cleared (I am guessing the
> "what the exact cause is" part, as the log message did not explain
> it)?  If that is the case, it makes me wonder if either of the two
> alternative may be a more robust solution: (1) running gpg tests
> telling gpg never to use agent, or (2) telling gpg and gpg-agent to
> use a socket inside GNUPGHOME.

I agree. In hindsight this solution seems rather naive. I'll dig into
the root cause, as well as to try to isolate the issue from a
gpg-version and gpg-config viewpoint.

> After all, "kill"ing agent blindly like the above patch would mean
> you do not know what other party is relying on the proper operation
> of the thing you are killing.  That sounds more like a workaround
> that a solution (unless it is explained with a solid reason why that
> is the right way to run more than one instances of GPG).

I agree. It is probably better to seek the solutions that you suggested
above.

> 
> Perhaps everybody else is running these gpg tests without having to
> worry about gpg-agent already because their environment is more
> vanilla, but you have some configuration or environment that cause
> gpg to use agent, and that is the reason why nobody is interested
> (because they have never seen the symptom)?  It is possible that the
> part of t/test-lib.sh that tries to cleanse environment variables
> and other "external influence" to give us a predictable and
> repeatable test is unaware of such a knob that only some developers
> (including you) have and the rest of us were merely lucky.  Perhaps
> we need to throw GPG_AGENT_INFO SSH_AUTH_SOCK etc. into the list of
> envirionment variables to nuke there?
> 
> Combined with the unknown-ness of the root cause of the issue, I can
> only say that the patch may be raising an issue worth addressing,
> but it is too sketchy to tell if it is a right solution or what the
> exact problem being solved is.

I'll dig into this. This sounds a way more reasonable approach.

Thanks for the feedback!
-Santiago.


signature.asc
Description: PGP signature

Re: What's cooking in git.git (Jul 2017, #04; Thu, 13)

[Santiago Torres]

> Here are the topics that have been cooking.  

Hi, 

I sent (a patch almost a week ago) that would probably[1] be labeled
as "uninteresting" (as per the notes from the maintainer), but I wanted
to make sure it wasn't lost in the noise -- I see that theres a lot of
active development lately. I checked the latest iterations of "what's
cooking" to see if it was going to be discarded or so, but I see no
mention of it.

Thanks!
-Santiago

[1] 
https://public-inbox.org/git/20170707220729.a3xrsju3rf4guyzs@LykOS.localdomain/T/#t


signature.asc
Description: PGP signature

Re: [RFC PATCH] t: lib-gpg: flush agent sockets on startup

[Santiago Torres]

Hello all,

I don't know if this is a desired feature, but I noticed that, one some
versions of gpg, gpg tests are skipped when I run a test suite multiple
times.

Cheers!
-Santiago.

On Fri, Jul 07, 2017 at 06:01:59PM -0400, santi...@nyu.edu wrote:
> From: Santiago Torres <santi...@nyu.edu>
> 
> When running gpg-relevant tests, a gpg-daemon is ran for a
> trash_directory-specific GNUPGHOME. This daemon creates a unix socket on
> the target host, and it will be used on subsequent runs of the same test
> script.  Add a call to kill the agent and flush the sockets of the
> relevant trash directory.
> 
> Signed-off-by: Santiago Torres <santi...@nyu.edu>
> ---
>  t/lib-gpg.sh | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh
> index ec2aa8f68..22ef2fa87 100755
> --- a/t/lib-gpg.sh
> +++ b/t/lib-gpg.sh
> @@ -31,6 +31,7 @@ then
>   chmod 0700 ./gpghome &&
>   GNUPGHOME="$(pwd)/gpghome" &&
>   export GNUPGHOME &&
> + gpgconf --kill gpg-agent &&
>   gpg --homedir "${GNUPGHOME}" 2>/dev/null --import \
>   "$TEST_DIRECTORY"/lib-gpg/keyring.gpg &&
>   gpg --homedir "${GNUPGHOME}" 2>/dev/null --import-ownertrust \
> -- 
> 2.13.2
> 


signature.asc
Description: PGP signature

[RFC PATCH] t: lib-gpg: flush agent sockets on startup

[santiago]

From: Santiago Torres <santi...@nyu.edu>

When running gpg-relevant tests, a gpg-daemon is ran for a
trash_directory-specific GNUPGHOME. This daemon creates a unix socket on
the target host, and it will be used on subsequent runs of the same test
script.  Add a call to kill the agent and flush the sockets of the
relevant trash directory.

Signed-off-by: Santiago Torres <santi...@nyu.edu>
---
 t/lib-gpg.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh
index ec2aa8f68..22ef2fa87 100755
--- a/t/lib-gpg.sh
+++ b/t/lib-gpg.sh
@@ -31,6 +31,7 @@ then
chmod 0700 ./gpghome &&
GNUPGHOME="$(pwd)/gpghome" &&
export GNUPGHOME &&
+   gpgconf --kill gpg-agent &&
gpg --homedir "${GNUPGHOME}" 2>/dev/null --import \
"$TEST_DIRECTORY"/lib-gpg/keyring.gpg &&
gpg --homedir "${GNUPGHOME}" 2>/dev/null --import-ownertrust \
-- 
2.13.2

Re: [PATCH 3/3] t7004, t7030: fix here-doc syntax errors

[Santiago Torres]

On Thu, Mar 23, 2017 at 03:00:08PM -0700, Junio C Hamano wrote:
> Santiago Torres <santi...@nyu.edu> writes:
 
> OK, so has everybody agreed what the next step would be? 

I believe it is, although I imagine getting a confirmation from Peff
would be adequate.

> Is the patch below a good first step (I still need to get it signed
> off)?

I'm adding a signoff to the patch below.

Thanks,
-Santiago

-- >8 --
Subject: t7004, t7030: fix here-doc syntax errors
From: Santiago Torres <santi...@nyu.edu>

Jan Palus noticed that some here-doc are spelled incorrectly,
resulting the entire remainder of the test as if it were data
slurped into the "expect" file, e.g. in this sequence

cat >expect <actual &&
test_cmp expect actual

the last command of the test is "cat" that sends everything to
'expect' and succeeds.

Fixing these issues in t7004 and t7030 reveals that "git tag -v"
and "git verify-tag" with their --format option do not work as the
test was expecting originally.  Instead of showing both valid tags
and tags with incorrect signatures on their output, tags that do not
pass verification are omitted from the output.

Arguably, that is a safer behaviour, and because the format
specifiers like %(tag) do not have a way to show if the signature
verifies correctly, the command with the --format option cannot be
used to get a list of tags annotated with their signature validity
anyway.

For now, let's fix the here-doc syntax and update the expectation to
match the reality.  Maybe later when we extend the --format language
available to "git tag -v" and "git verify-tag" to include things
like "%(gpg:status)", we may want to change the behaviour so that
piping a list of tag names into

xargs git verify-tag --format='%(gpg:status) %(tag)'

becomes a good way to produce such a list, but that is a separate
topic.

Signed-off-by: Santiago Torres <santi...@nyu.edu>
Noticed-by: Jan Palus <jan.pa...@gmail.com>
Helped-by: Jeff King <p...@peff.net>
---
 t/t7004-tag.sh| 10 --
 t/t7030-verify-tag.sh | 10 --
 2 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index b4698ab5f5..0581053a06 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -896,17 +896,15 @@ test_expect_success GPG 'verifying a forged tag should 
fail' '
 '
 
 test_expect_success 'verifying a proper tag with --format pass and format 
accordingly' '
-   cat >expect <<-\EOF
+   cat >expect <<-\EOF &&
tagname : signed-tag
-   EOF &&
+   EOF
git tag -v --format="tagname : %(tag)" "signed-tag" >actual &&
test_cmp expect actual
 '
 
-test_expect_success 'verifying a forged tag with --format fail and format 
accordingly' '
-   cat >expect <<-\EOF
-   tagname : forged-tag
-   EOF &&
+test_expect_success 'verifying a forged tag with --format should fail 
silently' '
+   >expect &&
test_must_fail git tag -v --format="tagname : %(tag)" "forged-tag" 
>actual &&
test_cmp expect actual
 '
diff --git a/t/t7030-verify-tag.sh b/t/t7030-verify-tag.sh
index d62ccbb98e..79864a3411 100755
--- a/t/t7030-verify-tag.sh
+++ b/t/t7030-verify-tag.sh
@@ -126,17 +126,15 @@ test_expect_success GPG 'verify multiple tags' '
 '
 
 test_expect_success 'verifying tag with --format' '
-   cat >expect <<-\EOF
+   cat >expect <<-\EOF &&
tagname : fourth-signed
-   EOF &&
+   EOF
git verify-tag --format="tagname : %(tag)" "fourth-signed" >actual &&
test_cmp expect actual
 '
 
-test_expect_success 'verifying a forged tag with --format fail and format 
accordingly' '
-   cat >expect <<-\EOF
-   tagname : 7th forged-signed
-   EOF &&
+test_expect_success 'verifying a forged tag with --format should fail 
silently' '
+   >expect &&
test_must_fail git verify-tag --format="tagname : %(tag)" $(cat 
forged1.tag) >actual-forged &&
test_cmp expect actual-forged
 '




signature.asc
Description: PGP signature

Re: [PATCH 3/3] t7004, t7030: fix here-doc syntax errors

[Santiago Torres]

On Wed, Mar 22, 2017 at 06:41:24PM -0400, Jeff King wrote:
> > In that case, something like this would be closer to the desired
> > behavior?
> 
> Yes, though you can spell:
> 
>   cat >expect <<-\EOF
>   EOF
> 
> as just:
> 
>   >expect

Ah, that sounds like a better way to fix this with a smaller diff.
> 
> > I'm also unsure on what would be the right thing to put on the commit
> > message.
> 
> I think the argument is:
> 
>   1. It's safer not to expound on tags that have failed verification (so
>  that the caller cannot accidentally use them). Especially since the
>  --format cannot tell anything about the GPG status.
> 
>  That means that
> 
>tag=$(git verify-tag --format='%(tag)' foo)
> 
>  can use a non-blank $tag without having to wonder whether it is
>  valid or not.
> 
> and
> 
>   2. That's what we've done since the feature was released.
> 
> The only thing that would give me pause is if were to later add
> %G-like formatters, and then:
> 
>   xargs git verify-tag --format='%(gpg:status) %(tag)' |
>   while read status tag
>   do
>  ...
>   done
> 
> would become useful, but we'd be tied to the behavior that we omit the
> tag when the gpg verification failed (for backwards compatibility).
> OTOH, we could perhaps make the rule "ignored unless %(gpg) formatters
> are used". Which would be backwards-compatible and safe for old formats,
> and work correctly for new ones.

This sounds like a helpful addition to implement. We could update/add
tests for compliance on this once the feature is addded and fix the
ambiguous behavior in the tests now.

Thanks,
-Santiago.

---
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index b4698ab5f..0581053a0 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -896,17 +896,15 @@ test_expect_success GPG 'verifying a forged tag should 
fail' '
 '
 
 test_expect_success 'verifying a proper tag with --format pass and format 
accordingly' '
-   cat >expect <<-\EOF
+   cat >expect <<-\EOF &&
tagname : signed-tag
-   EOF &&
+   EOF
git tag -v --format="tagname : %(tag)" "signed-tag" >actual &&
test_cmp expect actual
 '
 
-test_expect_success 'verifying a forged tag with --format fail and format 
accordingly' '
-   cat >expect <<-\EOF
-   tagname : forged-tag
-   EOF &&
+test_expect_success 'verifying a forged tag with --format should fail 
silently' '
+   >expect &&
test_must_fail git tag -v --format="tagname : %(tag)" "forged-tag" 
>actual &&
test_cmp expect actual
 '
diff --git a/t/t7030-verify-tag.sh b/t/t7030-verify-tag.sh
index d62ccbb98..173a88e89 100755
--- a/t/t7030-verify-tag.sh
+++ b/t/t7030-verify-tag.sh
@@ -126,17 +126,15 @@ test_expect_success GPG 'verify multiple tags' '
 '
 
 test_expect_success 'verifying tag with --format' '
-   cat >expect <<-\EOF
+   cat >expect <<-\EOF &&
tagname : fourth-signed
-   EOF &&
+   EOF
git verify-tag --format="tagname : %(tag)" "fourth-signed" >actual &&
test_cmp expect actual
 '
 
-test_expect_success 'verifying a forged tag with --format fail and format 
accordingly' '
-   cat >expect <<-\EOF
-   tagname : 7th forged-signed
-   EOF &&
+test_expect_success 'verifying a forged tag with --format should fail 
silently' '
+   >expect &&
test_must_fail git verify-tag --format="tagname : %(tag)" $(cat 
forged1.tag) >actual-forged &&
test_cmp expect actual-forged
 '


signature.asc
Description: PGP signature

Re: [PATCH 3/3] t7004, t7030: fix here-doc syntax errors

[Santiago Torres]

> I worked up the patch to do that (see below), but I got stumped trying
> to write the commit message. Perhaps that is what the test intended, but
> I don't think tag's --format understands "%G" codes at all. So you
> cannot tell from the output if a tag was valid or not; you have to check
> the exit code separately.
> 
> And if you do something like:
> 
>   git tag -v --format='%(tag)' foo bar |
>   while read tag
>   do
>  ...
>   done
> 
> you cannot tell at all which ones are bogus. Whereas with the current
> behavior, the bogus ones are quietly omitted. Which can also be
> confusing, but I'd think would generally err on the side of caution.

In that case, something like this would be closer to the desired
behavior?

I'm also unsure on what would be the right thing to put on the commit
message.

-Santiago.

---
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index b4698ab5f..70f6d4646 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -896,19 +896,18 @@ test_expect_success GPG 'verifying a forged tag should 
fail' '
 '
 
 test_expect_success 'verifying a proper tag with --format pass and format 
accordingly' '
-   cat >expect <<-\EOF
+   cat >expect <<-\EOF &&
tagname : signed-tag
-   EOF &&
+   EOF
git tag -v --format="tagname : %(tag)" "signed-tag" >actual &&
test_cmp expect actual
 '
 
-test_expect_success 'verifying a forged tag with --format fail and format 
accordingly' '
-   cat >expect <<-\EOF
+test_expect_success 'verifying a forged tag with --format should fail 
silently' '
+   cat >expect <<-\EOF &&
tagname : forged-tag
-   EOF &&
-   test_must_fail git tag -v --format="tagname : %(tag)" "forged-tag" 
>actual &&
-   test_cmp expect actual
+   EOF
+   test_must_fail git tag -v --format="tagname : %(tag)" "forged-tag"
 '
 
 # blank and empty messages for signed tags:
diff --git a/t/t7030-verify-tag.sh b/t/t7030-verify-tag.sh
index d62ccbb98..ba0aafa1f 100755
--- a/t/t7030-verify-tag.sh
+++ b/t/t7030-verify-tag.sh
@@ -126,19 +126,18 @@ test_expect_success GPG 'verify multiple tags' '
 '
 
 test_expect_success 'verifying tag with --format' '
-   cat >expect <<-\EOF
+   cat >expect <<-\EOF &&
tagname : fourth-signed
-   EOF &&
+   EOF
git verify-tag --format="tagname : %(tag)" "fourth-signed" >actual &&
test_cmp expect actual
 '
 
-test_expect_success 'verifying a forged tag with --format fail and format 
accordingly' '
-   cat >expect <<-\EOF
+test_expect_success 'verifying a forged tag with --format should fail 
silently' '
+   cat >expect <<-\EOF &&
tagname : 7th forged-signed
-   EOF &&
-   test_must_fail git verify-tag --format="tagname : %(tag)" $(cat 
forged1.tag) >actual-forged &&
-   test_cmp expect actual-forged
+   EOF
+   test_must_fail git verify-tag --format="tagname : %(tag)" $(cat 
forged1.tag)
 '
 
 test_done


signature.asc
Description: PGP signature

Re: [PATCH 3/3] t7004, t7030: fix here-doc syntax errors

[Santiago Torres]

On Wed, Mar 22, 2017 at 03:04:32PM -0700, Junio C Hamano wrote:
> Jeff King <p...@peff.net> writes:
> 
> > On Wed, Mar 22, 2017 at 01:08:05PM -0700, Junio C Hamano wrote:
> >
> >> From: Jan Palus <jan.pa...@gmail.com>
> >> 
> >> These all came as part of an earlier st/verify-tag topic that was
> >> merged to 2.12.
> >> 
> >> Signed-off-by: Junio C Hamano <gits...@pobox.com>
> >> ---
> >> 
> >>  * This should be applied on top of 4fea72f4 ("t/t7004-tag: Add
> >>--format specifier tests", 2017-01-17)
> >> 
> >>  t/t7004-tag.sh| 8 
> >>  t/t7030-verify-tag.sh | 8 
> >>  2 files changed, 8 insertions(+), 8 deletions(-)
> >
> > Like 2/3, this one also produces test failures for me. It looks like
> > "verify-tag" does not show a tag which has been forged. I'm not sure if
> > that's intentional (and the test is wrong) or a bug.  +cc Santiago
> 
> It appears that the test expected a broken one to be shown, and my
> reading of its log message is that the change expected --format= to
> be used with %G? so that scripts can tell between pass and fail?  
> 
> So if I have to judge, the code becoming silent for a tag that does
> not pass verification is not doing what the commit wanted it to do.
> 

Yes, considering the test name is:

    "verifying a forged tag with --format fail and format accordingly" 

It feels as if the behavior of verify-tag/tag -v is not the one
intended. I could add two patches on top of those two commits.

Would this be enough?
-Santiago.


signature.asc
Description: PGP signature

Re: [PATCH 3/3] t7004, t7030: fix here-doc syntax errors

[Santiago Torres]

On Wed, Mar 22, 2017 at 05:10:03PM -0400, Jeff King wrote:
> On Wed, Mar 22, 2017 at 01:08:05PM -0700, Junio C Hamano wrote:
> 
> > From: Jan Palus <jan.pa...@gmail.com>
> > 
> > These all came as part of an earlier st/verify-tag topic that was
> > merged to 2.12.
> > 
> > Signed-off-by: Junio C Hamano <gits...@pobox.com>
> > ---
> > 
> >  * This should be applied on top of 4fea72f4 ("t/t7004-tag: Add
> >--format specifier tests", 2017-01-17)
> > 
> >  t/t7004-tag.sh| 8 
> >  t/t7030-verify-tag.sh | 8 
> >  2 files changed, 8 insertions(+), 8 deletions(-)
> 

On my side, these patches make tests fail. I'm wondering if this is an
issue with the underlying shell (probably the version)? Let me try to
figure out what is exactly going on here.

> Like 2/3, this one also produces test failures for me. It looks like
> "verify-tag" does not show a tag which has been forged. I'm not sure if
> that's intentional (and the test is wrong) or a bug. 

I see that offending code would be [1]. Changing this behavior should be
trivial (dropping the continue), although I'm not sure if this is what
we want?

> 
> -Peff

Thanks,
-Santiago.

[1] 
https://github.com/git/git/blob/master/builtin/verify-tag.c#L6://github.com/git/git/blob/master/builtin/verify-tag.c#L67


signature.asc
Description: PGP signature

Re: SHA1 collisions found

[Santiago Torres]

On Fri, Feb 24, 2017 at 11:47:46PM +0100, Jakub Narębski wrote:
> I have just read on ArsTechnica[1] that while Git repository could be
> corrupted (though this would require attackers to spend great amount
> of resources creating their own collision, while as said elsewhere
> in this thread allegedly easy to detect), putting two proof-of-concept
> different PDFs with same size and SHA-1 actually *breaks* Subversion.
> Repository can become corrupt, and stop accepting new commits.  

From what I understood in the thread[1], it was the combination of svn +
git-svn together. I think Arstechnica may be a little bit
sensationalistic here.

Cheers!
-Santiago.

[1] https://bugs.webkit.org/show_bug.cgi?id=168774#c27


signature.asc
Description: PGP signature

SHAttered (the first practical SHA1 attack)

[Santiago Torres]

Hello all,

I ran into this website presenting the "first practical attack on
sha1"[1]. I don't recall seeing this on the ML, so I'm sharing this just
in case. I know there are proposals to move out of sha1 already. I
wonder if this affects the timeline for their adoption?

Thanks,
-Santiago.

[1] https://shattered.io/


signature.asc
Description: PGP signature

Re: idea: light-weight pull requests

[Santiago Torres]

Hello, pabs.

IMHO, the notion of a PR/MR is more specific to Git repository
management tools (e.g., GitHub, GitLab). They all have specific
concepts/ways to manage the way how their hosted repositories behave ---
and I believe this flexibility is one of the beauties in Git . I could
see how this could be implemented by tools like this rather easily
(e.g., using symlinks + inotify or something less hacky).

I'm wondering if standardizing this would be more interesting to those
communities?

I would like to see what becomes of this.

Cheers!
-Santiago.

On Tue, Feb 07, 2017 at 08:32:17AM +0800, Paul Wise wrote:
> Hi all,
> 
> I had an idea that might be interesting to git folks:
> 
> light-weight pull requests:
> 
> Anonymous and ssh/http-identified users should be able to push just
> using git, within the refs/pull/ namespace, to any non-existent branch
> name or to a branch they created when previously identified, without
> forking off a new repository.
> 
> Advantages:
> 
> Removes the need to look up who to send the pull request notification
> to since configuring that is up to the project itself.
> 
> Removes the annoying scenario of having lots of remotes that have been
> removed after the corresponding pull request was closed.
> 
> Moves popular git hosting services from primarily a model of forests of
> forks to one of contributions to well maintained ongoing projects.
> 
> Allows users to use their preferred git clients to issue pull requests
> instead of using web interfaces of popular git hosting services.
> 
> Creates a new standard for contributing to repositories on all git
> repository hosting services.
> 
> Contributions from people without an account on those services are
> possible.
> 
> Contributions from people without any git repository hosting of their
> own are possible.
> 
> Contributions from people who don't use or dislike MUAs are possible.
> 
> Disadvantages:
> 
> Pull request spam, could be mitigated with configuration options.
> 
> Extra configuration and complexity on the server side. This is once
> only and means less complexity on the pull requester side.
> 
> Will not work with typical setups where the git/http/ssh user does not
> have write access to the repositories. A workaround could be some sort
> of hybrid-repository setup with the new refs and objects in a second
> repository which would be shared by all pull requesters.
> 
> -- 
> bye,
> pabs
> 
> http://bonedaddy.net/pabs3/




signature.asc
Description: PGP signature

Re: git clone problem

[Santiago Torres]

Hello, Jordi.

Hmm, it should've cloned in the "whatever" directory.

Can you post your git version/configs and maybe the output verbatim of
the command when you run it?

If you can reproduce in an empty dictionary that'd be better

$ mkdir test && cd test

$ git clone --recursive https://github.com/...

$ ls 

Thanks,
-Santiago

On Wed, Jan 25, 2017 at 05:58:58PM +0100, Jordi Durban wrote:
> Hi all! Not sure if that will reach the goal, but let's it a try.
> 
> I have a problem with the git clone command: when I try to clone a remote
> repository with the following:
> 
> git clone --recursive https://github.com/whatever.git
> 
> what I actually obtain is a copy of my own files in the current directory.
> 
> I mean:
> 
> In the current directory:
> 
> $ls
> 
> -rwxr-xr-x 1  1,6K oct 24 17:29 get_fasta.pl
> -rwxr-xr-x 1  1,6K set  5 13:05 script_clus_miRNA_c95.pl
> 
> $git clone --recursive https://github.com/whatever.git WHATEVER
> 
> $ls
> 
> -rwxr-xr-x 1  1,6K oct 24 17:29 get_fasta.pl
> -rwxr-xr-x 1  1,6K set  5 13:05 script_clus_miRNA_c95.pl
> 
> -rwxr-xr-x 1  1,6K set  5 13:05 WHATEVER
> 
> $ls WHATEVER
> 
> -rwxr-xr-x 1  1,6K oct 24 17:29 get_fasta.pl
> -rwxr-xr-x 1  1,6K set  5 13:05 script_clus_miRNA_c95.pl
> 
> I am really confused with that.
> 
> Any help will be appreciated. Thank you
> 


signature.asc
Description: PGP signature

Re: [PATCH v6 4/6] builtin/tag: add --format argument for tag -v

[Santiago Torres]

On Wed, Jan 18, 2017 at 10:44:03AM -0800, Junio C Hamano wrote:
> Santiago Torres <santi...@nyu.edu> writes:
> 
Was:

Thanks!

Would you want me to re-roll really quick? or would you rather apply
this on your side?

Thanks,
-Santiago.
> 
> Eric, I've noticed that this message
> 
>   
> http://public-inbox.org/git/20170118182831.pkhqu2np3bh2puei@LykOS.localdomain/
> 
> and all messages from Santiago appear empty when they come via
> public-inbox.org; the reason I suspect we haven't heard much
> complaints is because nobody else around here sends multipart/signed
> disposition inline other than Santiago.
> 

Interesting, I thought I wasn't inlining the .asc. I guess I can disable
signing for this ML for the time being. 

Thanks for letting me know.
-Santiago.

Re: [PATCH v6 4/6] builtin/tag: add --format argument for tag -v

[Santiago Torres]

On Wed, Jan 18, 2017 at 10:25:59AM -0800, Junio C Hamano wrote:
> Junio C Hamano <gits...@pobox.com> writes:
> 
> > santi...@nyu.edu writes:
> >
> >> @@ -428,9 +443,12 @@ int cmd_tag(int argc, const char **argv, const char 
> >> *prefix)
> >>if (filter.merge_commit)
> >>die(_("--merged and --no-merged option are only allowed with 
> >> -l"));
> >>if (cmdmode == 'd')
> >> -  return for_each_tag_name(argv, delete_tag);
> >> -  if (cmdmode == 'v')
> >> -  return for_each_tag_name(argv, verify_tag);
> >> +  return for_each_tag_name(argv, delete_tag, NULL);
> >> +  if (cmdmode == 'v') {
> >> +  if (format)
> >> +  verify_ref_format(format);
> >> +  return for_each_tag_name(argv, verify_tag, format);
> >> +  }
> >
> > This triggers:
> >
> > builtin/tag.c: In function 'cmd_tag':
> > builtin/tag.c:451:3: error: passing argument 3 of
> > 'for_each_tag_name' discards 'const' qualifier from pointer target type 
> > [-Werror]
> >return for_each_tag_name(argv, verify_tag, format);
> >
> > Either for-each-tag-name's new parameter needs to be typed
> > correctly, or the type of the "format" variable needs to be updated.
> 
> Squashing the following into this commit solves this issue with the
> former approach.  The lines it touches are all from 4/6 and I view
> all of it as general improvement, including type correctness and
> code formatting.

Thanks!

Should I re-roll this really quick? Or would you rather apply this on
your tree directly? 

-Santiago.


signature.asc
Description: PGP signature

[PATCH v6 2/6] ref-filter: add function to print single ref_array_item

[santiago]

From: Lukas Puehringer 

ref-filter functions are useful for printing git object information
using a format specifier. However, some other modules may not want to use
this functionality on a ref-array but only print a single item.

Expose a pretty_print_ref function to create, pretty print and free
individual ref-items.

Signed-off-by: Lukas Puehringer 
---
 ref-filter.c | 27 +--
 ref-filter.h |  7 +++
 2 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index 1a978405e..5f4b08792 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1361,7 +1361,7 @@ static struct ref_array_item *new_ref_array_item(const 
char *refname,
return ref;
 }
 
-static int filter_ref_kind(struct ref_filter *filter, const char *refname)
+static int ref_kind_from_refname(const char *refname)
 {
unsigned int i;
 
@@ -1374,11 +1374,7 @@ static int filter_ref_kind(struct ref_filter *filter, 
const char *refname)
{ "refs/tags/", FILTER_REFS_TAGS}
};
 
-   if (filter->kind == FILTER_REFS_BRANCHES ||
-   filter->kind == FILTER_REFS_REMOTES ||
-   filter->kind == FILTER_REFS_TAGS)
-   return filter->kind;
-   else if (!strcmp(refname, "HEAD"))
+   if (!strcmp(refname, "HEAD"))
return FILTER_REFS_DETACHED_HEAD;
 
for (i = 0; i < ARRAY_SIZE(ref_kind); i++) {
@@ -1389,6 +1385,15 @@ static int filter_ref_kind(struct ref_filter *filter, 
const char *refname)
return FILTER_REFS_OTHERS;
 }
 
+static int filter_ref_kind(struct ref_filter *filter, const char *refname)
+{
+   if (filter->kind == FILTER_REFS_BRANCHES ||
+   filter->kind == FILTER_REFS_REMOTES ||
+   filter->kind == FILTER_REFS_TAGS)
+   return filter->kind;
+   return ref_kind_from_refname(refname);
+}
+
 /*
  * A call-back given to for_each_ref().  Filter refs and keep them for
  * later object processing.
@@ -1671,6 +1676,16 @@ void show_ref_array_item(struct ref_array_item *info, 
const char *format, int qu
putchar('\n');
 }
 
+void pretty_print_ref(const char *name, const unsigned char *sha1,
+   const char *format)
+{
+   struct ref_array_item *ref_item;
+   ref_item = new_ref_array_item(name, sha1, 0);
+   ref_item->kind = ref_kind_from_refname(name);
+   show_ref_array_item(ref_item, format, 0);
+   free_array_item(ref_item);
+}
+
 /*  If no sorting option is given, use refname to sort as default */
 struct ref_sorting *ref_default_sorting(void)
 {
diff --git a/ref-filter.h b/ref-filter.h
index fc55fa357..7b05592ba 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -109,4 +109,11 @@ struct ref_sorting *ref_default_sorting(void);
 /*  Function to parse --merged and --no-merged options */
 int parse_opt_merge_filter(const struct option *opt, const char *arg, int 
unset);
 
+/*
+ * Print a single ref, outside of any ref-filter. Note that the
+ * name must be a fully qualified refname.
+ */
+void pretty_print_ref(const char *name, const unsigned char *sha1,
+   const char *format);
+
 #endif /*  REF_FILTER_H  */
-- 
2.11.0

[PATCH v6 6/6] t/t7004-tag: Add --format specifier tests

[santiago]

From: Santiago Torres <santi...@nyu.edu>

tag -v now supports --format specifiers to inspect the contents of a tag
upon verification. Add two tests to ensure this behavior is respected in
future changes.

Signed-off-by: Santiago Torres <santi...@nyu.edu>
---
 t/t7004-tag.sh | 16 
 1 file changed, 16 insertions(+)

diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 07869b0c0..ba88b556b 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -874,6 +874,22 @@ test_expect_success GPG 'verifying a forged tag should 
fail' '
test_must_fail git tag -v forged-tag
 '
 
+test_expect_success 'verifying a proper tag with --format pass and format 
accordingly' '
+   cat >expect <<-\EOF
+   tagname : signed-tag
+   EOF &&
+   git tag -v --format="tagname : %(tag)" "signed-tag" >actual &&
+   test_cmp expect actual
+'
+
+test_expect_success 'verifying a forged tag with --format fail and format 
accordingly' '
+   cat >expect <<-\EOF
+   tagname : forged-tag
+   EOF &&
+   test_must_fail git tag -v --format="tagname : %(tag)" "forged-tag" 
>actual &&
+   test_cmp expect actual
+'
+
 # blank and empty messages for signed tags:
 
 get_tag_header empty-signed-tag $commit commit $time >expect
-- 
2.11.0

[PATCH v6 4/6] builtin/tag: add --format argument for tag -v

[santiago]

From: Lukas Puehringer 

Adding --format to git tag -v mutes the default output of the GPG
verification and instead prints the formatted tag object.
This allows callers to cross-check the tagname from refs/tags with
the tagname from the tag object header upon GPG verification.

The callback function for for_each_tag_name() didn't allow callers to
pass custom data to their callback functions. Add a new opaque pointer
to each_tag_name_fn's parameter to allow this.

Signed-off-by: Lukas Puehringer 
---
 Documentation/git-tag.txt |  2 +-
 builtin/tag.c | 38 --
 2 files changed, 29 insertions(+), 11 deletions(-)

diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index 76cfe40d9..586aaa315 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -15,7 +15,7 @@ SYNOPSIS
 'git tag' [-n[]] -l [--contains ] [--points-at ]
[--column[=] | --no-column] [--create-reflog] [--sort=]
[--format=] [--[no-]merged []] [...]
-'git tag' -v ...
+'git tag' -v [--format=] ...
 
 DESCRIPTION
 ---
diff --git a/builtin/tag.c b/builtin/tag.c
index 73df72811..b9da72761 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -24,7 +24,7 @@ static const char * const git_tag_usage[] = {
N_("git tag -d ..."),
N_("git tag -l [-n[]] [--contains ] [--points-at ]"
"\n\t\t[--format=] [--[no-]merged []] 
[...]"),
-   N_("git tag -v ..."),
+   N_("git tag -v [--format=] ..."),
NULL
 };
 
@@ -66,15 +66,17 @@ static int list_tags(struct ref_filter *filter, struct 
ref_sorting *sorting, con
 }
 
 typedef int (*each_tag_name_fn)(const char *name, const char *ref,
-   const unsigned char *sha1);
+   const unsigned char *sha1, void *cb_data);
 
-static int for_each_tag_name(const char **argv, each_tag_name_fn fn)
+static int for_each_tag_name(const char **argv, each_tag_name_fn fn,
+   void *cb_data)
 {
const char **p;
char ref[PATH_MAX];
int had_error = 0;
unsigned char sha1[20];
 
+
for (p = argv; *p; p++) {
if (snprintf(ref, sizeof(ref), "refs/tags/%s", *p)
>= sizeof(ref)) {
@@ -87,14 +89,14 @@ static int for_each_tag_name(const char **argv, 
each_tag_name_fn fn)
had_error = 1;
continue;
}
-   if (fn(*p, ref, sha1))
+   if (fn(*p, ref, sha1, cb_data))
had_error = 1;
}
return had_error;
 }
 
 static int delete_tag(const char *name, const char *ref,
-   const unsigned char *sha1)
+   const unsigned char *sha1, void *cb_data)
 {
if (delete_ref(ref, sha1, 0))
return 1;
@@ -103,9 +105,22 @@ static int delete_tag(const char *name, const char *ref,
 }
 
 static int verify_tag(const char *name, const char *ref,
-   const unsigned char *sha1)
+   const unsigned char *sha1, void *cb_data)
 {
-   return gpg_verify_tag(sha1, name, GPG_VERIFY_VERBOSE);
+   int flags;
+   char *fmt_pretty = cb_data;
+   flags = GPG_VERIFY_VERBOSE;
+
+   if (fmt_pretty)
+   flags = GPG_VERIFY_OMIT_STATUS;
+
+   if (gpg_verify_tag(sha1, name, flags))
+   return -1;
+
+if (fmt_pretty)
+   pretty_print_ref(name, sha1, fmt_pretty);
+
+   return 0;
 }
 
 static int do_sign(struct strbuf *buffer)
@@ -428,9 +443,12 @@ int cmd_tag(int argc, const char **argv, const char 
*prefix)
if (filter.merge_commit)
die(_("--merged and --no-merged option are only allowed with 
-l"));
if (cmdmode == 'd')
-   return for_each_tag_name(argv, delete_tag);
-   if (cmdmode == 'v')
-   return for_each_tag_name(argv, verify_tag);
+   return for_each_tag_name(argv, delete_tag, NULL);
+   if (cmdmode == 'v') {
+   if (format)
+   verify_ref_format(format);
+   return for_each_tag_name(argv, verify_tag, format);
+   }
 
if (msg.given || msgfile) {
if (msg.given && msgfile)
-- 
2.11.0

[PATCH v6 3/6] builtin/verify-tag: add --format to verify-tag

[santiago]

From: Santiago Torres <santi...@nyu.edu>

Callers of verify-tag may want to cross-check the tagname from refs/tags
with the tagname from the tag object header upon GPG verification. This
is to avoid tag refs that point to an incorrect object.

Add a --format parameter to git verify-tag to print the formatted tag
object header in addition to or instead of the --verbose or --raw GPG
verification output.

Signed-off-by: Santiago Torres <santi...@nyu.edu>
---
 Documentation/git-verify-tag.txt |  2 +-
 builtin/verify-tag.c | 23 ---
 2 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-verify-tag.txt b/Documentation/git-verify-tag.txt
index d590edceb..0b8075dad 100644
--- a/Documentation/git-verify-tag.txt
+++ b/Documentation/git-verify-tag.txt
@@ -8,7 +8,7 @@ git-verify-tag - Check the GPG signature of tags
 SYNOPSIS
 
 [verse]
-'git verify-tag' ...
+'git verify-tag' [--format=] ...
 
 DESCRIPTION
 ---
diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index 99f8148cf..18443bf9f 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -12,12 +12,14 @@
 #include 
 #include "parse-options.h"
 #include "gpg-interface.h"
+#include "ref-filter.h"
 
 static const char * const verify_tag_usage[] = {
-   N_("git verify-tag [-v | --verbose] ..."),
+   N_("git verify-tag [-v | --verbose] [--format=] 
..."),
NULL
 };
 
+
 static int git_verify_tag_config(const char *var, const char *value, void *cb)
 {
int status = git_gpg_config(var, value, cb);
@@ -30,9 +32,11 @@ int cmd_verify_tag(int argc, const char **argv, const char 
*prefix)
 {
int i = 1, verbose = 0, had_error = 0;
unsigned flags = 0;
+   char *fmt_pretty = NULL;
const struct option verify_tag_options[] = {
OPT__VERBOSE(, N_("print tag contents")),
OPT_BIT(0, "raw", , N_("print raw gpg status output"), 
GPG_VERIFY_RAW),
+   OPT_STRING(  0 , "format", _pretty, N_("format"), 
N_("format to use for the output")),
OPT_END()
};
 
@@ -46,13 +50,26 @@ int cmd_verify_tag(int argc, const char **argv, const char 
*prefix)
if (verbose)
flags |= GPG_VERIFY_VERBOSE;
 
+   if (fmt_pretty) {
+   verify_ref_format(fmt_pretty);
+   flags |= GPG_VERIFY_OMIT_STATUS;
+   }
+
while (i < argc) {
unsigned char sha1[20];
const char *name = argv[i++];
-   if (get_sha1(name, sha1))
+   if (get_sha1(name, sha1)) {
had_error = !!error("tag '%s' not found.", name);
-   else if (gpg_verify_tag(sha1, name, flags))
+   continue;
+   }
+
+   if (gpg_verify_tag(sha1, name, flags)) {
had_error = 1;
+   continue;
+   }
+
+   if (fmt_pretty)
+   pretty_print_ref(name, sha1, fmt_pretty);
}
return had_error;
 }
-- 
2.11.0

[PATCH v6 1/6] gpg-interface,tag: add GPG_VERIFY_OMIT_STATUS flag

[santiago]

From: Lukas Puehringer 

Functions that print git object information may require that the
gpg-interface functions be silent. Add GPG_VERIFY_OMIT_STATUS flag and
prevent print_signature_buffer from being called if flag is set.

Signed-off-by: Lukas Puehringer 
---
 gpg-interface.h | 5 +++--
 tag.c   | 5 -
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/gpg-interface.h b/gpg-interface.h
index ea68885ad..d2d4fd3a6 100644
--- a/gpg-interface.h
+++ b/gpg-interface.h
@@ -1,8 +1,9 @@
 #ifndef GPG_INTERFACE_H
 #define GPG_INTERFACE_H
 
-#define GPG_VERIFY_VERBOSE 1
-#define GPG_VERIFY_RAW 2
+#define GPG_VERIFY_VERBOSE 1
+#define GPG_VERIFY_RAW 2
+#define GPG_VERIFY_OMIT_STATUS 4
 
 struct signature_check {
char *payload;
diff --git a/tag.c b/tag.c
index d1dcd18cd..243d1fdbb 100644
--- a/tag.c
+++ b/tag.c
@@ -3,6 +3,7 @@
 #include "commit.h"
 #include "tree.h"
 #include "blob.h"
+#include "gpg-interface.h"
 
 const char *tag_type = "tag";
 
@@ -24,7 +25,9 @@ static int run_gpg_verify(const char *buf, unsigned long 
size, unsigned flags)
 
ret = check_signature(buf, payload_size, buf + payload_size,
size - payload_size, );
-   print_signature_buffer(, flags);
+
+   if (!(flags & GPG_VERIFY_OMIT_STATUS))
+   print_signature_buffer(, flags);
 
signature_check_clear();
return ret;
-- 
2.11.0

[PATCH v6 5/6] t/t7030-verify-tag: Add --format specifier tests

[santiago]

From: Santiago Torres <santi...@nyu.edu>

Verify-tag now provides --format specifiers to inspect and ensure the
contents of the tag are proper. We add two tests to ensure this
functionality works as expected: the return value should indicate if
verification passed, and the format specifiers must be respected.

Signed-off-by: Santiago Torres <santi...@nyu.edu>
---
 t/t7030-verify-tag.sh | 16 
 1 file changed, 16 insertions(+)

diff --git a/t/t7030-verify-tag.sh b/t/t7030-verify-tag.sh
index 07079a41c..d62ccbb98 100755
--- a/t/t7030-verify-tag.sh
+++ b/t/t7030-verify-tag.sh
@@ -125,4 +125,20 @@ test_expect_success GPG 'verify multiple tags' '
test_cmp expect.stderr actual.stderr
 '
 
+test_expect_success 'verifying tag with --format' '
+   cat >expect <<-\EOF
+   tagname : fourth-signed
+   EOF &&
+   git verify-tag --format="tagname : %(tag)" "fourth-signed" >actual &&
+   test_cmp expect actual
+'
+
+test_expect_success 'verifying a forged tag with --format fail and format 
accordingly' '
+   cat >expect <<-\EOF
+   tagname : 7th forged-signed
+   EOF &&
+   test_must_fail git verify-tag --format="tagname : %(tag)" $(cat 
forged1.tag) >actual-forged &&
+   test_cmp expect actual-forged
+'
+
 test_done
-- 
2.11.0

[PATCH v6 0/6] Add --format to tag verification

[santiago]

From: Santiago Torres <santi...@nyu.edu>

This is the sixth iteration of [1][2][3][4][5], and as a result of the
discussion in [5]. The main goal of this patch series is to bring
--format to git tag verification so that upper-layer tools can inspect
the content of a tag and make decisions based on it.

In this re-woll we:

* Changed the call interface so printing is done outside of verification. 

* Fixed a couple of whitespace issues and whatnot. 

Thanks,
-Santiago

[1] http://public-inbox.org/git/20170115184705.10376-1-santi...@nyu.edu/
[2] http://public-inbox.org/git/20161007210721.20437-1-santi...@nyu.edu/
[3] http://public-inbox.org/git/20160930221806.3398-1-santi...@nyu.edu/
[4] http://public-inbox.org/git/20160922185317.349-1-santi...@nyu.edu/
[5] http://public-inbox.org/git/20160926224233.32702-1-santi...@nyu.edu/
[6] http://public-inbox.org/git/20160607195608.16643-1-santi...@nyu.edu/
[7] 
http://public-inbox.org/git/20161019203546.dfqmi2czcxopg...@sigill.intra.peff.net/
[8] 
http://public-inbox.org/git/20161019203943.epjxnfci7vcqg...@sigill.intra.peff.net/

Lukas Puehringer (3):
  gpg-interface,tag: add GPG_VERIFY_OMIT_STATUS flag
  ref-filter: add function to print single ref_array_item
  builtin/tag: add --format argument for tag -v

Santiago Torres (3):
  builtin/verify-tag: add --format to verify-tag
  t/t7030-verify-tag: Add --format specifier tests
  t/t7004-tag: Add --format specifier tests

 Documentation/git-tag.txt|  2 +-
 Documentation/git-verify-tag.txt |  2 +-
 builtin/tag.c| 38 --
 builtin/verify-tag.c | 23 ---
 gpg-interface.h  |  5 +++--
 ref-filter.c | 27 +--
 ref-filter.h |  7 +++
 t/t7004-tag.sh   | 16 
 t/t7030-verify-tag.sh| 16 
 tag.c|  5 -
 10 files changed, 117 insertions(+), 24 deletions(-)

-- 
2.11.0

Re: [PATCH v5 5/7] builtin/tag: add --format argument for tag -v

[Santiago Torres]

> VERBOSE|QUIET _does_ have a meaning, which is "show the payload, but do
> not print the signature buffer". Perhaps just renaming QUIET to
> OMIT_STATUS or something would make it more clear.
> 
Let me give this a go too. OMIT_STATUS does sound less confusing.

Thanks,
-Santiago.

> -Peff


signature.asc
Description: PGP signature

Re: [PATCH v5 3/7] tag: add format specifier to gpg_verify_tag

[Santiago Torres]

Yeah, this actually looks more cleaner.

Let me give it a go.

Thanks!
-Santiago.

On Tue, Jan 17, 2017 at 12:30:04PM -0500, Jeff King wrote:
> On Tue, Jan 17, 2017 at 12:25:31PM -0500, Jeff King wrote:
> 
> > Actually, looking at the callsites, I think they are fine to just call
> > pretty_print_ref() themselves, and I don't think it actually matters if
> > it happens before or after the verification.
> 
> Oh, sorry, I misread it. We do indeed early-return from the verification
> and skip the printing in that case. So it'd be more like:
> 
> diff --git a/builtin/tag.c b/builtin/tag.c
> index 9da11e0c2..068f392b6 100644
> --- a/builtin/tag.c
> +++ b/builtin/tag.c
> @@ -114,7 +114,11 @@ static int verify_tag(const char *name, const char *ref,
>   if (fmt_pretty)
>   flags = GPG_VERIFY_QUIET;
>  
> - return verify_and_format_tag(sha1, ref, fmt_pretty, flags);
> + if (gpg_verify_tag(sha1, ref, flags))
> + return -1;
> +
> + pretty_print_ref(name, sha1, fmt_pretty);
> + return 0;
>  }
>  
>  static int do_sign(struct strbuf *buffer)
> diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
> index 212449f47..b3f08f705 100644
> --- a/builtin/verify-tag.c
> +++ b/builtin/verify-tag.c
> @@ -58,10 +58,19 @@ int cmd_verify_tag(int argc, const char **argv, const 
> char *prefix)
>   while (i < argc) {
>   unsigned char sha1[20];
>   const char *name = argv[i++];
> - if (get_sha1(name, sha1))
> +
> + if (get_sha1(name, sha1)) {
>   had_error = !!error("tag '%s' not found.", name);
> - else if (verify_and_format_tag(sha1, name, fmt_pretty, flags))
> + continue;
> + }
> +
> + if (gpg_verify_tag(sha1, name, flags)) {
>   had_error = 1;
> + continue;
> + }
> +
> + if (fmt_pretty)
> + pretty_print_ref(name, sha1, fmt_pretty);
>   }
>   return had_error;
>  }
> 
> which I think is still an improvement (the printing, rather than being
> an obscure parameter to the overloaded verify_and_format_tag()
> function, is clearly a first-class operation).
> 
> -Peff


signature.asc
Description: PGP signature

Re: [PATCH v5 5/7] builtin/tag: add --format argument for tag -v

[Santiago Torres]

> >  {
> > -   return verify_and_format_tag(sha1, name, NULL, GPG_VERIFY_VERBOSE);
> > +   int flags;
> > +   char *fmt_pretty = cb_data;
> > +   flags = GPG_VERIFY_VERBOSE;
> > +
> > +   if (fmt_pretty)
> > +   flags = GPG_VERIFY_QUIET;
> > +
> > +   return verify_and_format_tag(sha1, ref, fmt_pretty, flags);
> 
> It seems funny that VERBOSE and QUIET are bit-flags. What happens when
> you ask for GPG_VERIFY_VERBOSE|GPG_VERIFY_QUIET?

If I'm not mistaken, the way the code works right now this is not
possible (GPG_VERIFY_VERBOSE will be unset when GPG_VERIFY_QUIET). I
would have to re-read the patch to make sure this is the case then.

GPG_VERIFY_QUIET was added to suppress any VERBOSE|RAW flags, we could
defeault to QUIET if flags are not set. What do you think?

Thanks!
-Santiago


signature.asc
Description: PGP signature

Re: [PATCH v5 3/7] tag: add format specifier to gpg_verify_tag

[Santiago Torres]

> > Hrm. Maybe I am missing something, but what does:
> > 
> >   verify_and_format_tag(sha1, name, fmt, flags);
> > 
> > get you over:
> > 
> >   gpg_verify_tag(sha1, name, flags);
> >   pretty_print_ref(name, sha1, fmt);
> > 
> > ? The latter seems much more flexible, and I do not see how the
> > verification step impacts the printing at all (or vice versa).
> > 
> > I could understand it more if there were patches later in the series
> > that somehow used the format and verification results together. But I
> > didn't see that.
> 
> Having read through the rest of the series, it looks like you'd
> sometimes have to do:

IIRC, we did this to make the diff "simpler". It also feeds odd that the
call chain is the following:

verify_and_format_tag()
gpg_verify_tag()
run_gpg_verification()

I'm afraid that adding yet another wrapper would further convolute the
call chain. If you think this is not an issue, I could easily do it. Do
you have any suggested name for the wrapper?

Thanks!
-Santiago


signature.asc
Description: PGP signature

[PATCH v5 3/7] tag: add format specifier to gpg_verify_tag

[santiago]

From: Lukas Puehringer 

Calling functions for gpg_verify_tag() may desire to print relevant
information about the header for further verification. Add an optional
format argument to print any desired information after GPG verification.

Signed-off-by: Lukas Puehringer 
---
 builtin/tag.c|  2 +-
 builtin/verify-tag.c |  2 +-
 tag.c| 17 +++--
 tag.h|  4 ++--
 4 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/builtin/tag.c b/builtin/tag.c
index 73df72811..880677df5 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -105,7 +105,7 @@ static int delete_tag(const char *name, const char *ref,
 static int verify_tag(const char *name, const char *ref,
const unsigned char *sha1)
 {
-   return gpg_verify_tag(sha1, name, GPG_VERIFY_VERBOSE);
+   return verify_and_format_tag(sha1, name, NULL, GPG_VERIFY_VERBOSE);
 }
 
 static int do_sign(struct strbuf *buffer)
diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index 99f8148cf..de10198c4 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -51,7 +51,7 @@ int cmd_verify_tag(int argc, const char **argv, const char 
*prefix)
const char *name = argv[i++];
if (get_sha1(name, sha1))
had_error = !!error("tag '%s' not found.", name);
-   else if (gpg_verify_tag(sha1, name, flags))
+   else if (verify_and_format_tag(sha1, name, NULL, flags))
had_error = 1;
}
return had_error;
diff --git a/tag.c b/tag.c
index 291073f6e..d5a7cfb20 100644
--- a/tag.c
+++ b/tag.c
@@ -4,6 +4,7 @@
 #include "tree.h"
 #include "blob.h"
 #include "gpg-interface.h"
+#include "ref-filter.h"
 
 const char *tag_type = "tag";
 
@@ -33,8 +34,8 @@ static int run_gpg_verify(const char *buf, unsigned long 
size, unsigned flags)
return ret;
 }
 
-int gpg_verify_tag(const unsigned char *sha1, const char *name_to_report,
-   unsigned flags)
+int verify_and_format_tag(const unsigned char *sha1, const char *name,
+   const char *fmt_pretty, unsigned flags)
 {
enum object_type type;
char *buf;
@@ -44,21 +45,25 @@ int gpg_verify_tag(const unsigned char *sha1, const char 
*name_to_report,
type = sha1_object_info(sha1, NULL);
if (type != OBJ_TAG)
return error("%s: cannot verify a non-tag object of type %s.",
-   name_to_report ?
-   name_to_report :
+   name ?
+   name :
find_unique_abbrev(sha1, DEFAULT_ABBREV),
typename(type));
 
buf = read_sha1_file(sha1, , );
if (!buf)
return error("%s: unable to read file.",
-   name_to_report ?
-   name_to_report :
+   name ?
+   name :
find_unique_abbrev(sha1, DEFAULT_ABBREV));
 
ret = run_gpg_verify(buf, size, flags);
 
free(buf);
+
+   if (fmt_pretty)
+   pretty_print_ref(name, sha1, fmt_pretty);
+
return ret;
 }
 
diff --git a/tag.h b/tag.h
index a5721b673..896b9c2dc 100644
--- a/tag.h
+++ b/tag.h
@@ -17,7 +17,7 @@ extern int parse_tag_buffer(struct tag *item, const void 
*data, unsigned long si
 extern int parse_tag(struct tag *item);
 extern struct object *deref_tag(struct object *, const char *, int);
 extern struct object *deref_tag_noverify(struct object *);
-extern int gpg_verify_tag(const unsigned char *sha1,
-   const char *name_to_report, unsigned flags);
+extern int verify_and_format_tag(const unsigned char *sha1, const char *name,
+   const char *fmt_pretty, unsigned flags);
 
 #endif /* TAG_H */
-- 
2.11.0

[PATCH v5 4/7] builtin/verify-tag: add --format to verify-tag

[santiago]

From: Santiago Torres <santi...@nyu.edu>

Callers of verify-tag may want to cross-check the tagname from refs/tags
with the tagname from the tag object header upon GPG verification. This
is to avoid tag refs that point to an incorrect object.

Add a --format parameter to git verify-tag to print the formatted tag
object header in addition to or instead of the --verbose or --raw GPG
verification output.

Signed-off-by: Santiago Torres <santi...@nyu.edu>
---
 Documentation/git-verify-tag.txt |  2 +-
 builtin/verify-tag.c | 13 +++--
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-verify-tag.txt b/Documentation/git-verify-tag.txt
index d590edceb..0b8075dad 100644
--- a/Documentation/git-verify-tag.txt
+++ b/Documentation/git-verify-tag.txt
@@ -8,7 +8,7 @@ git-verify-tag - Check the GPG signature of tags
 SYNOPSIS
 
 [verse]
-'git verify-tag' ...
+'git verify-tag' [--format=] ...
 
 DESCRIPTION
 ---
diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index de10198c4..212449f47 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -12,12 +12,14 @@
 #include 
 #include "parse-options.h"
 #include "gpg-interface.h"
+#include "ref-filter.h"
 
 static const char * const verify_tag_usage[] = {
-   N_("git verify-tag [-v | --verbose] ..."),
+   N_("git verify-tag [-v | --verbose] [--format=] 
..."),
NULL
 };
 
+
 static int git_verify_tag_config(const char *var, const char *value, void *cb)
 {
int status = git_gpg_config(var, value, cb);
@@ -30,9 +32,11 @@ int cmd_verify_tag(int argc, const char **argv, const char 
*prefix)
 {
int i = 1, verbose = 0, had_error = 0;
unsigned flags = 0;
+   char *fmt_pretty = NULL;
const struct option verify_tag_options[] = {
OPT__VERBOSE(, N_("print tag contents")),
OPT_BIT(0, "raw", , N_("print raw gpg status output"), 
GPG_VERIFY_RAW),
+   OPT_STRING(  0 , "format", _pretty, N_("format"), 
N_("format to use for the output")),
OPT_END()
};
 
@@ -46,12 +50,17 @@ int cmd_verify_tag(int argc, const char **argv, const char 
*prefix)
if (verbose)
flags |= GPG_VERIFY_VERBOSE;
 
+   if (fmt_pretty) {
+   verify_ref_format(fmt_pretty);
+   flags |= GPG_VERIFY_QUIET;
+   }
+
while (i < argc) {
unsigned char sha1[20];
const char *name = argv[i++];
if (get_sha1(name, sha1))
had_error = !!error("tag '%s' not found.", name);
-   else if (verify_and_format_tag(sha1, name, NULL, flags))
+   else if (verify_and_format_tag(sha1, name, fmt_pretty, flags))
had_error = 1;
}
return had_error;
-- 
2.11.0

[PATCH v5 0/7] Add --format to tag verification

[santiago]

From: Santiago Torres <santi...@nyu.edu>

This is the fifth iteration of [1][2][3][4], and as a result of the
discussion in [5]. The main goal of this patch series is to bring
--format to git tag verification so that upper-layer tools can inspect
the content of a tag and make decisions based on it.

In this re-woll we:

* Squashed Peff's first patch[6] with the second patch of the series. The commit
  message may need work.

* Applied the relevant segment's of Peff's second patch[7] on the rest of the
  series

* Rebased so these patches apply to the tip of the master branch.

Thanks,
-Santiago

[1] http://public-inbox.org/git/20161007210721.20437-1-santi...@nyu.edu/
[2] http://public-inbox.org/git/20160930221806.3398-1-santi...@nyu.edu/
[3] http://public-inbox.org/git/20160922185317.349-1-santi...@nyu.edu/
[4] http://public-inbox.org/git/20160926224233.32702-1-santi...@nyu.edu/
[5] http://public-inbox.org/git/20160607195608.16643-1-santi...@nyu.edu/
[6] 
http://public-inbox.org/git/20161019203546.dfqmi2czcxopg...@sigill.intra.peff.net/
[7] 
http://public-inbox.org/git/20161019203943.epjxnfci7vcqg...@sigill.intra.peff.net/

Lukas Puehringer (4):
  gpg-interface, tag: add GPG_VERIFY_QUIET flag
  ref-filter: add function to print single ref_array_item
  tag: add format specifier to gpg_verify_tag
  builtin/tag: add --format argument for tag -v

Santiago Torres (3):
  builtin/verify-tag: add --format to verify-tag
  t/t7030-verify-tag: Add --format specifier tests
  t/t7004-tag: Add --format specifier tests

 Documentation/git-tag.txt|  2 +-
 Documentation/git-verify-tag.txt |  2 +-
 builtin/tag.c| 32 ++--
 builtin/verify-tag.c | 13 +++--
 gpg-interface.h  |  1 +
 ref-filter.c | 27 +--
 ref-filter.h |  7 +++
 t/t7004-tag.sh   | 16 
 t/t7030-verify-tag.sh| 16 
 tag.c| 22 +++---
 tag.h|  4 ++--
 11 files changed, 113 insertions(+), 29 deletions(-)

-- 
2.11.0

[PATCH v5 1/7] gpg-interface, tag: add GPG_VERIFY_QUIET flag

[santiago]

From: Lukas Puehringer 

Functions that print git object information may require that the
gpg-interface functions be silent. Add GPG_VERIFY_QUIET flag and prevent
print_signature_buffer from being called if flag is set.

Signed-off-by: Lukas Puehringer 
---
 gpg-interface.h | 1 +
 tag.c   | 5 -
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/gpg-interface.h b/gpg-interface.h
index ea68885ad..85dc9820d 100644
--- a/gpg-interface.h
+++ b/gpg-interface.h
@@ -3,6 +3,7 @@
 
 #define GPG_VERIFY_VERBOSE 1
 #define GPG_VERIFY_RAW 2
+#define GPG_VERIFY_QUIET   4
 
 struct signature_check {
char *payload;
diff --git a/tag.c b/tag.c
index d1dcd18cd..291073f6e 100644
--- a/tag.c
+++ b/tag.c
@@ -3,6 +3,7 @@
 #include "commit.h"
 #include "tree.h"
 #include "blob.h"
+#include "gpg-interface.h"
 
 const char *tag_type = "tag";
 
@@ -24,7 +25,9 @@ static int run_gpg_verify(const char *buf, unsigned long 
size, unsigned flags)
 
ret = check_signature(buf, payload_size, buf + payload_size,
size - payload_size, );
-   print_signature_buffer(, flags);
+
+   if (!(flags & GPG_VERIFY_QUIET))
+   print_signature_buffer(, flags);
 
signature_check_clear();
return ret;
-- 
2.11.0

[PATCH v5 6/7] t/t7030-verify-tag: Add --format specifier tests

[santiago]

From: Santiago Torres <santi...@nyu.edu>

Verify-tag now provides --format specifiers to inspect and ensure the
contents of the tag are proper. We add two tests to ensure this
functionality works as expected: the return value should indicate if
verification passed, and the format specifiers must be respected.

Signed-off-by: Santiago Torres <santi...@nyu.edu>
---
 t/t7030-verify-tag.sh | 16 
 1 file changed, 16 insertions(+)

diff --git a/t/t7030-verify-tag.sh b/t/t7030-verify-tag.sh
index 07079a41c..d62ccbb98 100755
--- a/t/t7030-verify-tag.sh
+++ b/t/t7030-verify-tag.sh
@@ -125,4 +125,20 @@ test_expect_success GPG 'verify multiple tags' '
test_cmp expect.stderr actual.stderr
 '
 
+test_expect_success 'verifying tag with --format' '
+   cat >expect <<-\EOF
+   tagname : fourth-signed
+   EOF &&
+   git verify-tag --format="tagname : %(tag)" "fourth-signed" >actual &&
+   test_cmp expect actual
+'
+
+test_expect_success 'verifying a forged tag with --format fail and format 
accordingly' '
+   cat >expect <<-\EOF
+   tagname : 7th forged-signed
+   EOF &&
+   test_must_fail git verify-tag --format="tagname : %(tag)" $(cat 
forged1.tag) >actual-forged &&
+   test_cmp expect actual-forged
+'
+
 test_done
-- 
2.11.0

[PATCH v5 7/7] t/t7004-tag: Add --format specifier tests

[santiago]

From: Santiago Torres <santi...@nyu.edu>

tag -v now supports --format specifiers to inspect the contents of a tag
upon verification. Add two tests to ensure this behavior is respected in
future changes.

Signed-off-by: Santiago Torres <santi...@nyu.edu>
---
 t/t7004-tag.sh | 16 
 1 file changed, 16 insertions(+)

diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 07869b0c0..b2b81f203 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -874,6 +874,22 @@ test_expect_success GPG 'verifying a forged tag should 
fail' '
test_must_fail git tag -v forged-tag
 '
 
+test_expect_success 'verifying a proper tag with --format pass and format 
accordingly' '
+   cat >expect <<-\EOF 
+   tagname : signed-tag
+   EOF &&
+   git tag -v --format="tagname : %(tag)" "signed-tag" >actual &&
+   test_cmp expect actual
+'
+
+test_expect_success 'verifying a forged tag with --format fail and format 
accordingly' '
+   cat >expect <<-\EOF 
+   tagname : forged-tag
+   EOF &&
+   test_must_fail git tag -v --format="tagname : %(tag)" "forged-tag" 
>actual &&
+   test_cmp expect actual
+'
+
 # blank and empty messages for signed tags:
 
 get_tag_header empty-signed-tag $commit commit $time >expect
-- 
2.11.0

[PATCH v5 2/7] ref-filter: add function to print single ref_array_item

[santiago]

From: Lukas Puehringer 

ref-filter functions are useful for printing git object information
using a format specifier. However, some other modules may not want to use
this functionality on a ref-array but only print a single item.

Expose a pretty_print_ref function to create, pretty print and free
individual ref-items.

Signed-off-by: Lukas Puehringer 
---
 ref-filter.c | 27 +--
 ref-filter.h |  7 +++
 2 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index 1a978405e..5f4b08792 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1361,7 +1361,7 @@ static struct ref_array_item *new_ref_array_item(const 
char *refname,
return ref;
 }
 
-static int filter_ref_kind(struct ref_filter *filter, const char *refname)
+static int ref_kind_from_refname(const char *refname)
 {
unsigned int i;
 
@@ -1374,11 +1374,7 @@ static int filter_ref_kind(struct ref_filter *filter, 
const char *refname)
{ "refs/tags/", FILTER_REFS_TAGS}
};
 
-   if (filter->kind == FILTER_REFS_BRANCHES ||
-   filter->kind == FILTER_REFS_REMOTES ||
-   filter->kind == FILTER_REFS_TAGS)
-   return filter->kind;
-   else if (!strcmp(refname, "HEAD"))
+   if (!strcmp(refname, "HEAD"))
return FILTER_REFS_DETACHED_HEAD;
 
for (i = 0; i < ARRAY_SIZE(ref_kind); i++) {
@@ -1389,6 +1385,15 @@ static int filter_ref_kind(struct ref_filter *filter, 
const char *refname)
return FILTER_REFS_OTHERS;
 }
 
+static int filter_ref_kind(struct ref_filter *filter, const char *refname)
+{
+   if (filter->kind == FILTER_REFS_BRANCHES ||
+   filter->kind == FILTER_REFS_REMOTES ||
+   filter->kind == FILTER_REFS_TAGS)
+   return filter->kind;
+   return ref_kind_from_refname(refname);
+}
+
 /*
  * A call-back given to for_each_ref().  Filter refs and keep them for
  * later object processing.
@@ -1671,6 +1676,16 @@ void show_ref_array_item(struct ref_array_item *info, 
const char *format, int qu
putchar('\n');
 }
 
+void pretty_print_ref(const char *name, const unsigned char *sha1,
+   const char *format)
+{
+   struct ref_array_item *ref_item;
+   ref_item = new_ref_array_item(name, sha1, 0);
+   ref_item->kind = ref_kind_from_refname(name);
+   show_ref_array_item(ref_item, format, 0);
+   free_array_item(ref_item);
+}
+
 /*  If no sorting option is given, use refname to sort as default */
 struct ref_sorting *ref_default_sorting(void)
 {
diff --git a/ref-filter.h b/ref-filter.h
index fc55fa357..7b05592ba 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -109,4 +109,11 @@ struct ref_sorting *ref_default_sorting(void);
 /*  Function to parse --merged and --no-merged options */
 int parse_opt_merge_filter(const struct option *opt, const char *arg, int 
unset);
 
+/*
+ * Print a single ref, outside of any ref-filter. Note that the
+ * name must be a fully qualified refname.
+ */
+void pretty_print_ref(const char *name, const unsigned char *sha1,
+   const char *format);
+
 #endif /*  REF_FILTER_H  */
-- 
2.11.0

[PATCH v5 5/7] builtin/tag: add --format argument for tag -v

[santiago]

From: Lukas Puehringer 

Adding --format to git tag -v mutes the default output of the GPG
verification and instead prints the formatted tag object.
This allows callers to cross-check the tagname from refs/tags with
the tagname from the tag object header upon GPG verification.

The callback function for for_each_tag_name() didn't allow callers to
pass custom data to their callback functions. Add a new opaque pointer
to each_tag_name_fn's parameter to allow this.

Signed-off-by: Lukas Puehringer 
---
 Documentation/git-tag.txt |  2 +-
 builtin/tag.c | 32 ++--
 2 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index 76cfe40d9..586aaa315 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -15,7 +15,7 @@ SYNOPSIS
 'git tag' [-n[]] -l [--contains ] [--points-at ]
[--column[=] | --no-column] [--create-reflog] [--sort=]
[--format=] [--[no-]merged []] [...]
-'git tag' -v ...
+'git tag' -v [--format=] ...
 
 DESCRIPTION
 ---
diff --git a/builtin/tag.c b/builtin/tag.c
index 880677df5..9da11e0c2 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -24,7 +24,7 @@ static const char * const git_tag_usage[] = {
N_("git tag -d ..."),
N_("git tag -l [-n[]] [--contains ] [--points-at ]"
"\n\t\t[--format=] [--[no-]merged []] 
[...]"),
-   N_("git tag -v ..."),
+   N_("git tag -v [--format=] ..."),
NULL
 };
 
@@ -66,15 +66,17 @@ static int list_tags(struct ref_filter *filter, struct 
ref_sorting *sorting, con
 }
 
 typedef int (*each_tag_name_fn)(const char *name, const char *ref,
-   const unsigned char *sha1);
+   const unsigned char *sha1, void *cb_data);
 
-static int for_each_tag_name(const char **argv, each_tag_name_fn fn)
+static int for_each_tag_name(const char **argv, each_tag_name_fn fn,
+   void *cb_data)
 {
const char **p;
char ref[PATH_MAX];
int had_error = 0;
unsigned char sha1[20];
 
+
for (p = argv; *p; p++) {
if (snprintf(ref, sizeof(ref), "refs/tags/%s", *p)
>= sizeof(ref)) {
@@ -87,14 +89,14 @@ static int for_each_tag_name(const char **argv, 
each_tag_name_fn fn)
had_error = 1;
continue;
}
-   if (fn(*p, ref, sha1))
+   if (fn(*p, ref, sha1, cb_data))
had_error = 1;
}
return had_error;
 }
 
 static int delete_tag(const char *name, const char *ref,
-   const unsigned char *sha1)
+   const unsigned char *sha1, void *cb_data)
 {
if (delete_ref(ref, sha1, 0))
return 1;
@@ -103,9 +105,16 @@ static int delete_tag(const char *name, const char *ref,
 }
 
 static int verify_tag(const char *name, const char *ref,
-   const unsigned char *sha1)
+   const unsigned char *sha1, void *cb_data)
 {
-   return verify_and_format_tag(sha1, name, NULL, GPG_VERIFY_VERBOSE);
+   int flags;
+   char *fmt_pretty = cb_data;
+   flags = GPG_VERIFY_VERBOSE;
+
+   if (fmt_pretty)
+   flags = GPG_VERIFY_QUIET;
+
+   return verify_and_format_tag(sha1, ref, fmt_pretty, flags);
 }
 
 static int do_sign(struct strbuf *buffer)
@@ -428,9 +437,12 @@ int cmd_tag(int argc, const char **argv, const char 
*prefix)
if (filter.merge_commit)
die(_("--merged and --no-merged option are only allowed with 
-l"));
if (cmdmode == 'd')
-   return for_each_tag_name(argv, delete_tag);
-   if (cmdmode == 'v')
-   return for_each_tag_name(argv, verify_tag);
+   return for_each_tag_name(argv, delete_tag, NULL);
+   if (cmdmode == 'v') {
+   if (format)
+   verify_ref_format(format);
+   return for_each_tag_name(argv, verify_tag, format);
+   }
 
if (msg.given || msgfile) {
if (msg.given && msgfile)
-- 
2.11.0

Re: [PATCH 2/2] tag: send fully qualified refnames to verify_tag_and_format

[Santiago Torres]

On Wed, Oct 19, 2016 at 04:39:44PM -0400, Jeff King wrote:
> The ref-filter code generally expects to see fully qualified
> refs, so that things like "%(refname)" and "%(refname:short)"
> work as expected. We can do so easily from git-tag, which
> always works with refnames in the refs/tags namespace. As a
> bonus, we can drop the "kind" parameter from
> pretty_print_ref() and just deduce it automatically.
> 
> Unfortunately, things are not so simple for verify-tag,
> which takes an arbitrary sha1 expression. It has no clue if
> a refname as used or not, and whether it was in the
> refs/tags namespace.
> 
> In an ideal world, get_sha1_with_context() would optionally
> tell us about any refs we resolved while it was working, and
> we could just feed that refname (and then in cases where we
> didn't use a ref at all, like a bare sha1, we could fallback
> to just showing the sha1 name the user gave us).
> 
> Signed-off-by: Jeff King 
> ---
> I think you'd really just squash the various bits of this into your
> series at the right spots, though I don't mind it on top, either.
> 
> The big question is to what degree we should care about the verify-tag
> case. I don't think it's any worse off with this change than it is with
> your series (its "kind" becomes "OTHER", but I don't think that is
> actually used for display at all; the name remains the same). I'd be OK
> with leaving it like this, as a known bug, until get_sha1_with_context()
> learns to tell us about the ref. It's an unhandled corner case in a
> brand-new feature, not a regression in an existing one.

I see now, I think I can sprinkle some of these changes on 2/7 then. The
rest should be doing 4/7 and 5/7. Does this sound ok?


signature.asc
Description: PGP signature

Re: [PATCH v4 2/7] ref-filter: add function to print single ref_array_item

[Santiago Torres]

On Wed, Oct 19, 2016 at 05:16:42AM -0400, Jeff King wrote:
> On Wed, Oct 19, 2016 at 04:55:43AM -0400, Jeff King wrote:
> 
> > > diff --git a/ref-filter.h b/ref-filter.h
> > > index 14d435e..3d23090 100644
> > > --- a/ref-filter.h
> > > +++ b/ref-filter.h
> > > @@ -107,4 +107,7 @@ struct ref_sorting *ref_default_sorting(void);
> > >  /*  Function to parse --merged and --no-merged options */
> > >  int parse_opt_merge_filter(const struct option *opt, const char *arg, 
> > > int unset);
> > >  
> > > +void pretty_print_ref(const char *name, const unsigned char *sha1,
> > > + const char *format, unsigned kind);
> > > +
> > 
> > What are the possible values for "kind"? I guess these should come from
> > FILTER_REFS_TAGS, BRANCHES, etc. It's probably worth documenting that.
> > Alternatively, is it possible to just determine this from the name? It
> > looks like filter_ref_kind() is how it happens for a normal ref-filter.
> 
> I guess that may complicate things for the caller you add in this
> series, which may not have a fully-qualified refname (which is obviously
> how filter_ref_kind() figures it out). I'd argue that is a bug, though,
> as things like "%(refname)" are generally expected to print out the
> fully refname ("git tag --format=%(refname)" does so, and you can use
> "%(refname:short)" if you want the shorter part).

Hmm, I hadn't actually noticed that. Do you have any suggestions in how to
address this?

In general this feels like a consequence of disambiguating .git/tags/*
within builtin/tag.c rather than letting ref-filter figure it out.

Thanks,
-Santiago.


signature.asc
Description: PGP signature

Re: What's cooking in git.git (Oct 2016, #03; Tue, 11)

[Santiago Torres]

> * st/verify-tag (2016-10-10) 7 commits
>  - t/t7004-tag: Add --format specifier tests
>  - t/t7030-verify-tag: Add --format specifier tests
>  - builtin/tag: add --format argument for tag -v
>  - builtin/verify-tag: add --format to verify-tag
>  - tag: add format specifier to gpg_verify_tag
>  - ref-filter: add function to print single ref_array_item
>  - gpg-interface, tag: add GPG_VERIFY_QUIET flag
> 
>  "git tag" and "git verify-tag" learned to put GPG verification
>  status in their "--format=" output format.
> 
>  Is this ready for 'next'?

Hi, I saw this on the previous "what's cooking." Is there anything I
need to do on my side to make sure this is ready for next?

Thanks!
-Santiago.


signature.asc
Description: PGP signature

Re: [PATCH v4 0/7] Add --format to tag verification

[Santiago Torres]

Hi,

I noticed there were no replies for this thread. I was curious if it got
buried because I sent it on the Friday evening before a long weekend.

I don't mean to pressure or anything.

Thanks!
-Santiago.


On Fri, Oct 07, 2016 at 05:07:14PM -0400, santi...@nyu.edu wrote:
> From: Santiago Torres <santi...@nyu.edu>
> 
> This is the fourth iteration of the series in [1][2][3], which comes as a
> result of the discussion in [4]. The main goal of this patch series is to 
> bring
> --format to git tag verification so that upper-layer tools can inspect the
> content of a tag and make decisions based on those contents.
> 
> In this re-woll we:
> 
> * Fixed the author fields and signed off by's throughout the patch
>   series
> 
> * Added two more patches with unit tests to ensure the format specifier
>   behaves as expected
> 
> * Fixed a missing initialization for the format specifier in verify-tag which
>   caused a crash.
> 
> * Fixed an outdated git commit message that had the previous name of a
>   function definition.
> 
> Thanks,
> -Santiago
> 
> [1] http://public-inbox.org/git/20160930221806.3398-1-santi...@nyu.edu/
> [2] http://public-inbox.org/git/20160922185317.349-1-santi...@nyu.edu/
> [3] http://public-inbox.org/git/20160926224233.32702-1-santi...@nyu.edu/
> [4] http://public-inbox.org/git/20160607195608.16643-1-santi...@nyu.edu/
> 
> 
> Lukas Puehringer (4):
>   tag: add format specifier to gpg_verify_tag
>   gpg-interface, tag: add GPG_VERIFY_QUIET flag
>   ref-filter: add function to print single ref_array_item
>   builtin/tag: add --format argument for tag -v
> 
> Santiago Torres (3):
>   builtin/verify-tag: add --format to verify-tag
>   t/t7030-verify-tag: Add --format specifier tests
>   t/t7004-tag: Add --format specifier tests
> 
>  Documentation/git-tag.txt|  2 +-
>  Documentation/git-verify-tag.txt |  2 +-
>  builtin/tag.c| 34 +++---
>  builtin/verify-tag.c | 13 +++--
>  gpg-interface.h  |  1 +
>  ref-filter.c | 10 ++
>  ref-filter.h |  3 +++
>  t/t7004-tag.sh   | 16 
>  t/t7030-verify-tag.sh| 16 
>  tag.c| 22 +++---
>  tag.h|  4 ++--
>  11 files changed, 99 insertions(+), 24 deletions(-)
> 
> -- 
> 2.10.0
> 


signature.asc
Description: PGP signature

[PATCH v4 5/7] builtin/tag: add --format argument for tag -v

[santiago]

From: Lukas Puehringer 

Adding --format to git tag -v mutes the default output of the GPG
verification and instead prints the formatted tag object.
This allows callers to cross-check the tagname from refs/tags with
the tagname from the tag object header upon GPG verification.

The callback function for for_each_tag_name() didn't allow callers to
pass custom data to their callback functions. Add a new opaque pointer
to each_tag_name_fn's parameter to allow this.

Signed-off-by: Lukas Puehringer 
---
 Documentation/git-tag.txt |  2 +-
 builtin/tag.c | 34 +++---
 builtin/verify-tag.c  |  2 +-
 3 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index 7ecca8e..3bb5e3c 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -15,7 +15,7 @@ SYNOPSIS
 'git tag' [-n[]] -l [--contains ] [--points-at ]
[--column[=] | --no-column] [--create-reflog] [--sort=]
[--format=] [--[no-]merged []] [...]
-'git tag' -v ...
+'git tag' -v [--format=] ...
 
 DESCRIPTION
 ---
diff --git a/builtin/tag.c b/builtin/tag.c
index 14f3b48..7730fd0 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -24,7 +24,7 @@ static const char * const git_tag_usage[] = {
N_("git tag -d ..."),
N_("git tag -l [-n[]] [--contains ] [--points-at ]"
"\n\t\t[--format=] [--[no-]merged []] 
[...]"),
-   N_("git tag -v ..."),
+   N_("git tag -v [--format=] ..."),
NULL
 };
 
@@ -66,15 +66,17 @@ static int list_tags(struct ref_filter *filter, struct 
ref_sorting *sorting, con
 }
 
 typedef int (*each_tag_name_fn)(const char *name, const char *ref,
-   const unsigned char *sha1);
+   const unsigned char *sha1, void *cb_data);
 
-static int for_each_tag_name(const char **argv, each_tag_name_fn fn)
+static int for_each_tag_name(const char **argv, each_tag_name_fn fn,
+   void *cb_data)
 {
const char **p;
char ref[PATH_MAX];
int had_error = 0;
unsigned char sha1[20];
 
+
for (p = argv; *p; p++) {
if (snprintf(ref, sizeof(ref), "refs/tags/%s", *p)
>= sizeof(ref)) {
@@ -87,14 +89,14 @@ static int for_each_tag_name(const char **argv, 
each_tag_name_fn fn)
had_error = 1;
continue;
}
-   if (fn(*p, ref, sha1))
+   if (fn(*p, ref, sha1, cb_data))
had_error = 1;
}
return had_error;
 }
 
 static int delete_tag(const char *name, const char *ref,
-   const unsigned char *sha1)
+   const unsigned char *sha1, void *cb_data)
 {
if (delete_ref(ref, sha1, 0))
return 1;
@@ -103,9 +105,16 @@ static int delete_tag(const char *name, const char *ref,
 }
 
 static int verify_tag(const char *name, const char *ref,
-   const unsigned char *sha1)
+   const unsigned char *sha1, void *cb_data)
 {
-   return verify_and_format_tag(sha1, name, NULL, GPG_VERIFY_VERBOSE);
+   int flags;
+   char *fmt_pretty = cb_data;
+   flags = GPG_VERIFY_VERBOSE;
+
+   if (fmt_pretty)
+   flags = GPG_VERIFY_QUIET;
+
+   return verify_and_format_tag(sha1, name, fmt_pretty, flags);
 }
 
 static int do_sign(struct strbuf *buffer)
@@ -334,7 +343,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
struct strbuf err = STRBUF_INIT;
struct ref_filter filter;
static struct ref_sorting *sorting = NULL, **sorting_tail = 
-   const char *format = NULL;
+   char *format = NULL;
struct option options[] = {
OPT_CMDMODE('l', "list", , N_("list tag names"), 'l'),
{ OPTION_INTEGER, 'n', NULL, , N_("n"),
@@ -424,9 +433,12 @@ int cmd_tag(int argc, const char **argv, const char 
*prefix)
if (filter.merge_commit)
die(_("--merged and --no-merged option are only allowed with 
-l"));
if (cmdmode == 'd')
-   return for_each_tag_name(argv, delete_tag);
-   if (cmdmode == 'v')
-   return for_each_tag_name(argv, verify_tag);
+   return for_each_tag_name(argv, delete_tag, NULL);
+   if (cmdmode == 'v') {
+   if (format)
+   verify_ref_format(format);
+   return for_each_tag_name(argv, verify_tag, format);
+   }
 
if (msg.given || msgfile) {
if (msg.given && msgfile)
-- 
2.10.0

[PATCH v4 7/7] t/t7004-tag: Add --format specifier tests

[santiago]

From: Santiago Torres <santi...@nyu.edu>

tag -v now supports --format specifiers to inspect the contents of a tag
upon verification. Add two tests to ensure this behavior is respected in
future changes.

Signed-off-by: Santiago Torres <santi...@nyu.edu>
---
 t/t7004-tag.sh | 16 
 1 file changed, 16 insertions(+)

diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 8b0f71a..633b089 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -847,6 +847,22 @@ test_expect_success GPG 'verifying a forged tag should 
fail' '
test_must_fail git tag -v forged-tag
 '
 
+test_expect_success 'verifying a proper tag with --format pass and format 
accordingly' '
+   cat >expect <<-\EOF &&
+   tagname : signed-tag
+   EOF
+   git tag -v --format="tagname : %(tag)" "signed-tag" >actual &&
+   test_cmp expect actual
+'
+
+test_expect_success 'verifying a forged tag with --format fail and format 
accordingly' '
+   cat >expect <<-\EOF &&
+   tagname : forged-tag
+   EOF
+   test_must_fail git tag -v --format="tagname : %(tag)" "forged-tag" 
>actual &&
+   test_cmp expect actual
+'
+
 # blank and empty messages for signed tags:
 
 get_tag_header empty-signed-tag $commit commit $time >expect
-- 
2.10.0

[PATCH v4 6/7] t/t7030-verify-tag: Add --format specifier tests

[santiago]

From: Santiago Torres <santi...@nyu.edu>

Verify-tag now provides --format specifiers to inspect and ensure the
contents of the tag are proper. We add two tests to ensure this
functionality works as expected: the return value should indicate if
verification passed, and the format specifiers must be respected.

Signed-off-by: Santiago Torres <santi...@nyu.edu>
---
 t/t7030-verify-tag.sh | 16 
 1 file changed, 16 insertions(+)

diff --git a/t/t7030-verify-tag.sh b/t/t7030-verify-tag.sh
index 07079a4..ce37fd9 100755
--- a/t/t7030-verify-tag.sh
+++ b/t/t7030-verify-tag.sh
@@ -125,4 +125,20 @@ test_expect_success GPG 'verify multiple tags' '
test_cmp expect.stderr actual.stderr
 '
 
+test_expect_success 'verifying tag with --format' '
+   cat >expect <<-\EOF &&
+   tagname : fourth-signed
+   EOF
+   git verify-tag --format="tagname : %(tag)" "fourth-signed" >actual &&
+   test_cmp expect actual
+'
+
+test_expect_success 'verifying a forged tag with --format fail and format 
accordingly' '
+   cat >expect <<-\EOF &&
+   tagname : 7th forged-signed
+   EOF
+   test_must_fail git verify-tag --format="tagname : %(tag)" $(cat 
forged1.tag) >actual-forged &&
+   test_cmp expect actual-forged
+'
+
 test_done
-- 
2.10.0

[PATCH v4 0/7] Add --format to tag verification

[santiago]

From: Santiago Torres <santi...@nyu.edu>

This is the fourth iteration of the series in [1][2][3], which comes as a
result of the discussion in [4]. The main goal of this patch series is to bring
--format to git tag verification so that upper-layer tools can inspect the
content of a tag and make decisions based on those contents.

In this re-woll we:

* Fixed the author fields and signed off by's throughout the patch
  series

* Added two more patches with unit tests to ensure the format specifier
  behaves as expected

* Fixed a missing initialization for the format specifier in verify-tag which
  caused a crash.

* Fixed an outdated git commit message that had the previous name of a
  function definition.

Thanks,
-Santiago

[1] http://public-inbox.org/git/20160930221806.3398-1-santi...@nyu.edu/
[2] http://public-inbox.org/git/20160922185317.349-1-santi...@nyu.edu/
[3] http://public-inbox.org/git/20160926224233.32702-1-santi...@nyu.edu/
[4] http://public-inbox.org/git/20160607195608.16643-1-santi...@nyu.edu/


Lukas Puehringer (4):
  tag: add format specifier to gpg_verify_tag
  gpg-interface, tag: add GPG_VERIFY_QUIET flag
  ref-filter: add function to print single ref_array_item
  builtin/tag: add --format argument for tag -v

Santiago Torres (3):
  builtin/verify-tag: add --format to verify-tag
  t/t7030-verify-tag: Add --format specifier tests
  t/t7004-tag: Add --format specifier tests

 Documentation/git-tag.txt|  2 +-
 Documentation/git-verify-tag.txt |  2 +-
 builtin/tag.c| 34 +++---
 builtin/verify-tag.c | 13 +++--
 gpg-interface.h  |  1 +
 ref-filter.c | 10 ++
 ref-filter.h |  3 +++
 t/t7004-tag.sh   | 16 
 t/t7030-verify-tag.sh| 16 
 tag.c| 22 +++---
 tag.h|  4 ++--
 11 files changed, 99 insertions(+), 24 deletions(-)

-- 
2.10.0

[PATCH v4 1/7] gpg-interface, tag: add GPG_VERIFY_QUIET flag

[santiago]

From: Lukas Puehringer 

Functions that print git object information may require that the
gpg-interface functions be silent. Add GPG_VERIFY_QUIET flag and prevent
print_signature_buffer from being called if flag is set.

Signed-off-by: Lukas Puehringer 
---
 gpg-interface.h | 1 +
 tag.c   | 5 -
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/gpg-interface.h b/gpg-interface.h
index ea68885..85dc982 100644
--- a/gpg-interface.h
+++ b/gpg-interface.h
@@ -3,6 +3,7 @@
 
 #define GPG_VERIFY_VERBOSE 1
 #define GPG_VERIFY_RAW 2
+#define GPG_VERIFY_QUIET   4
 
 struct signature_check {
char *payload;
diff --git a/tag.c b/tag.c
index d1dcd18..291073f 100644
--- a/tag.c
+++ b/tag.c
@@ -3,6 +3,7 @@
 #include "commit.h"
 #include "tree.h"
 #include "blob.h"
+#include "gpg-interface.h"
 
 const char *tag_type = "tag";
 
@@ -24,7 +25,9 @@ static int run_gpg_verify(const char *buf, unsigned long 
size, unsigned flags)
 
ret = check_signature(buf, payload_size, buf + payload_size,
size - payload_size, );
-   print_signature_buffer(, flags);
+
+   if (!(flags & GPG_VERIFY_QUIET))
+   print_signature_buffer(, flags);
 
signature_check_clear();
return ret;
-- 
2.10.0

[PATCH v4 2/7] ref-filter: add function to print single ref_array_item

[santiago]

From: Lukas Puehringer 

ref-filter functions are useful for printing git object information
using a format specifier. However, some other modules may not want to use
this functionality on a ref-array but only print a single item.

Expose a pretty_print_ref function to create, pretty print and free
individual ref-items.

Signed-off-by: Lukas Puehringer 
---
 ref-filter.c | 10 ++
 ref-filter.h |  3 +++
 2 files changed, 13 insertions(+)

diff --git a/ref-filter.c b/ref-filter.c
index 9adbb8a..cfbcd73 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1637,6 +1637,16 @@ void show_ref_array_item(struct ref_array_item *info, 
const char *format, int qu
putchar('\n');
 }
 
+void pretty_print_ref(const char *name, const unsigned char *sha1,
+   const char *format, unsigned kind)
+{
+   struct ref_array_item *ref_item;
+   ref_item = new_ref_array_item(name, sha1, 0);
+   ref_item->kind = kind;
+   show_ref_array_item(ref_item, format, 0);
+   free_array_item(ref_item);
+}
+
 /*  If no sorting option is given, use refname to sort as default */
 struct ref_sorting *ref_default_sorting(void)
 {
diff --git a/ref-filter.h b/ref-filter.h
index 14d435e..3d23090 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -107,4 +107,7 @@ struct ref_sorting *ref_default_sorting(void);
 /*  Function to parse --merged and --no-merged options */
 int parse_opt_merge_filter(const struct option *opt, const char *arg, int 
unset);
 
+void pretty_print_ref(const char *name, const unsigned char *sha1,
+   const char *format, unsigned kind);
+
 #endif /*  REF_FILTER_H  */
-- 
2.10.0

[PATCH v4 3/7] tag: add format specifier to gpg_verify_tag

[santiago]

From: Lukas Puehringer 

Calling functions for gpg_verify_tag() may desire to print relevant
information about the header for further verification. Add an optional
format argument to print any desired information after GPG verification.

Signed-off-by: Lukas Puehringer 
---
 builtin/tag.c|  2 +-
 builtin/verify-tag.c |  2 +-
 tag.c| 17 +++--
 tag.h|  4 ++--
 4 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/builtin/tag.c b/builtin/tag.c
index 50e4ae5..14f3b48 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -105,7 +105,7 @@ static int delete_tag(const char *name, const char *ref,
 static int verify_tag(const char *name, const char *ref,
const unsigned char *sha1)
 {
-   return gpg_verify_tag(sha1, name, GPG_VERIFY_VERBOSE);
+   return verify_and_format_tag(sha1, name, NULL, GPG_VERIFY_VERBOSE);
 }
 
 static int do_sign(struct strbuf *buffer)
diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index 99f8148..de10198 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -51,7 +51,7 @@ int cmd_verify_tag(int argc, const char **argv, const char 
*prefix)
const char *name = argv[i++];
if (get_sha1(name, sha1))
had_error = !!error("tag '%s' not found.", name);
-   else if (gpg_verify_tag(sha1, name, flags))
+   else if (verify_and_format_tag(sha1, name, NULL, flags))
had_error = 1;
}
return had_error;
diff --git a/tag.c b/tag.c
index 291073f..d3512c0 100644
--- a/tag.c
+++ b/tag.c
@@ -4,6 +4,7 @@
 #include "tree.h"
 #include "blob.h"
 #include "gpg-interface.h"
+#include "ref-filter.h"
 
 const char *tag_type = "tag";
 
@@ -33,8 +34,8 @@ static int run_gpg_verify(const char *buf, unsigned long 
size, unsigned flags)
return ret;
 }
 
-int gpg_verify_tag(const unsigned char *sha1, const char *name_to_report,
-   unsigned flags)
+int verify_and_format_tag(const unsigned char *sha1, const char *name,
+   const char *fmt_pretty, unsigned flags)
 {
enum object_type type;
char *buf;
@@ -44,21 +45,25 @@ int gpg_verify_tag(const unsigned char *sha1, const char 
*name_to_report,
type = sha1_object_info(sha1, NULL);
if (type != OBJ_TAG)
return error("%s: cannot verify a non-tag object of type %s.",
-   name_to_report ?
-   name_to_report :
+   name ?
+   name :
find_unique_abbrev(sha1, DEFAULT_ABBREV),
typename(type));
 
buf = read_sha1_file(sha1, , );
if (!buf)
return error("%s: unable to read file.",
-   name_to_report ?
-   name_to_report :
+   name ?
+   name :
find_unique_abbrev(sha1, DEFAULT_ABBREV));
 
ret = run_gpg_verify(buf, size, flags);
 
free(buf);
+
+   if (fmt_pretty)
+   pretty_print_ref(name, sha1, fmt_pretty, FILTER_REFS_TAGS);
+
return ret;
 }
 
diff --git a/tag.h b/tag.h
index a5721b6..896b9c2 100644
--- a/tag.h
+++ b/tag.h
@@ -17,7 +17,7 @@ extern int parse_tag_buffer(struct tag *item, const void 
*data, unsigned long si
 extern int parse_tag(struct tag *item);
 extern struct object *deref_tag(struct object *, const char *, int);
 extern struct object *deref_tag_noverify(struct object *);
-extern int gpg_verify_tag(const unsigned char *sha1,
-   const char *name_to_report, unsigned flags);
+extern int verify_and_format_tag(const unsigned char *sha1, const char *name,
+   const char *fmt_pretty, unsigned flags);
 
 #endif /* TAG_H */
-- 
2.10.0

[PATCH v4 4/7] builtin/verify-tag: add --format to verify-tag

[santiago]

From: Santiago Torres <santi...@nyu.edu>

Callers of verify-tag may want to cross-check the tagname from refs/tags
with the tagname from the tag object header upon GPG verification. This
is to avoid tag refs that point to an incorrect object.

Add a --format parameter to git verify-tag to print the formatted tag
object header in addition to or instead of the --verbose or --raw GPG
verification output.

Signed-off-by: Santiago Torres <santi...@nyu.edu>
---
 Documentation/git-verify-tag.txt |  2 +-
 builtin/verify-tag.c | 13 +++--
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-verify-tag.txt b/Documentation/git-verify-tag.txt
index d590edc..0b8075d 100644
--- a/Documentation/git-verify-tag.txt
+++ b/Documentation/git-verify-tag.txt
@@ -8,7 +8,7 @@ git-verify-tag - Check the GPG signature of tags
 SYNOPSIS
 
 [verse]
-'git verify-tag' ...
+'git verify-tag' [--format=] ...
 
 DESCRIPTION
 ---
diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index de10198..745b6a6 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -12,12 +12,14 @@
 #include 
 #include "parse-options.h"
 #include "gpg-interface.h"
+#include "ref-filter.h"
 
 static const char * const verify_tag_usage[] = {
-   N_("git verify-tag [-v | --verbose] ..."),
+   N_("git verify-tag [-v | --verbose] [--format=] 
..."),
NULL
 };
 
+
 static int git_verify_tag_config(const char *var, const char *value, void *cb)
 {
int status = git_gpg_config(var, value, cb);
@@ -30,9 +32,11 @@ int cmd_verify_tag(int argc, const char **argv, const char 
*prefix)
 {
int i = 1, verbose = 0, had_error = 0;
unsigned flags = 0;
+   char *fmt_pretty = NULL;
const struct option verify_tag_options[] = {
OPT__VERBOSE(, N_("print tag contents")),
OPT_BIT(0, "raw", , N_("print raw gpg status output"), 
GPG_VERIFY_RAW),
+   OPT_STRING(  0 , "format", _pretty, N_("format"), 
N_("format to use for the output")),
OPT_END()
};
 
@@ -46,12 +50,17 @@ int cmd_verify_tag(int argc, const char **argv, const char 
*prefix)
if (verbose)
flags |= GPG_VERIFY_VERBOSE;
 
+   if (fmt_pretty) {
+   verify_ref_format(fmt_pretty);
+   flags |= GPG_VERIFY_QUIET;
+   }
+
while (i < argc) {
unsigned char sha1[20];
const char *name = argv[i++];
if (get_sha1(name, sha1))
had_error = !!error("tag '%s' not found.", name);
-   else if (verify_and_format_tag(sha1, name, NULL, flags))
+   else if (verify_and_format_tag(sha1, name, fmt_pretty, flags))
had_error = 1;
}
return had_error;
-- 
2.10.0

Re: [PATCH v3 0/5] Add --format to tag verification

[Santiago Torres]

Hi, Junio. 
> I however notice that there is no new tests to protect these two new
> features from future breakages.  Perhaps you want to add some in
> [6/5]?

I'll be working on this. I spent some time looking around for example
tests for format. Are there any that I should pay special attention to?
(I'm looking at t7004 mostly right now).

Thanks!
-Santiago.


signature.asc
Description: PGP signature

[PATCH v3 3/5] tag: add format specifier to gpg_verify_tag

[santiago]

From: Lukas P 

Calling functions for gpg_verify_tag() may desire to print relevant
information about the header for further verification. Add an optional
format argument to print any desired information after GPG verification.

Signed-off-by: Lukas Puehringer 
---
 builtin/tag.c|  2 +-
 builtin/verify-tag.c |  2 +-
 tag.c| 17 +++--
 tag.h|  4 ++--
 4 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/builtin/tag.c b/builtin/tag.c
index 50e4ae5..14f3b48 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -105,7 +105,7 @@ static int delete_tag(const char *name, const char *ref,
 static int verify_tag(const char *name, const char *ref,
const unsigned char *sha1)
 {
-   return gpg_verify_tag(sha1, name, GPG_VERIFY_VERBOSE);
+   return verify_and_format_tag(sha1, name, NULL, GPG_VERIFY_VERBOSE);
 }
 
 static int do_sign(struct strbuf *buffer)
diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index 99f8148..de10198 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -51,7 +51,7 @@ int cmd_verify_tag(int argc, const char **argv, const char 
*prefix)
const char *name = argv[i++];
if (get_sha1(name, sha1))
had_error = !!error("tag '%s' not found.", name);
-   else if (gpg_verify_tag(sha1, name, flags))
+   else if (verify_and_format_tag(sha1, name, NULL, flags))
had_error = 1;
}
return had_error;
diff --git a/tag.c b/tag.c
index 291073f..d3512c0 100644
--- a/tag.c
+++ b/tag.c
@@ -4,6 +4,7 @@
 #include "tree.h"
 #include "blob.h"
 #include "gpg-interface.h"
+#include "ref-filter.h"
 
 const char *tag_type = "tag";
 
@@ -33,8 +34,8 @@ static int run_gpg_verify(const char *buf, unsigned long 
size, unsigned flags)
return ret;
 }
 
-int gpg_verify_tag(const unsigned char *sha1, const char *name_to_report,
-   unsigned flags)
+int verify_and_format_tag(const unsigned char *sha1, const char *name,
+   const char *fmt_pretty, unsigned flags)
 {
enum object_type type;
char *buf;
@@ -44,21 +45,25 @@ int gpg_verify_tag(const unsigned char *sha1, const char 
*name_to_report,
type = sha1_object_info(sha1, NULL);
if (type != OBJ_TAG)
return error("%s: cannot verify a non-tag object of type %s.",
-   name_to_report ?
-   name_to_report :
+   name ?
+   name :
find_unique_abbrev(sha1, DEFAULT_ABBREV),
typename(type));
 
buf = read_sha1_file(sha1, , );
if (!buf)
return error("%s: unable to read file.",
-   name_to_report ?
-   name_to_report :
+   name ?
+   name :
find_unique_abbrev(sha1, DEFAULT_ABBREV));
 
ret = run_gpg_verify(buf, size, flags);
 
free(buf);
+
+   if (fmt_pretty)
+   pretty_print_ref(name, sha1, fmt_pretty, FILTER_REFS_TAGS);
+
return ret;
 }
 
diff --git a/tag.h b/tag.h
index a5721b6..896b9c2 100644
--- a/tag.h
+++ b/tag.h
@@ -17,7 +17,7 @@ extern int parse_tag_buffer(struct tag *item, const void 
*data, unsigned long si
 extern int parse_tag(struct tag *item);
 extern struct object *deref_tag(struct object *, const char *, int);
 extern struct object *deref_tag_noverify(struct object *);
-extern int gpg_verify_tag(const unsigned char *sha1,
-   const char *name_to_report, unsigned flags);
+extern int verify_and_format_tag(const unsigned char *sha1, const char *name,
+   const char *fmt_pretty, unsigned flags);
 
 #endif /* TAG_H */
-- 
2.10.0

[PATCH v3 5/5] builtin/tag: add --format argument for tag -v

[santiago]

From: Lukas Puehringer 

Adding --format to git tag -v mutes the default output of the GPG
verification and instead prints the formatted tag object.
This allows callers to cross-check the tagname from refs/tags with
the tagname from the tag object header upon GPG verification.

The callback function for for_each_tag_name() didn't allow callers to
pass custom data to their callback functions. Add a new opaque pointer
to each_tag_name_fn's parameter to allow this.

Signed-off-by: Lukas Puehringer 
---
 Documentation/git-tag.txt |  2 +-
 builtin/tag.c | 34 +++---
 2 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index 7ecca8e..3bb5e3c 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -15,7 +15,7 @@ SYNOPSIS
 'git tag' [-n[]] -l [--contains ] [--points-at ]
[--column[=] | --no-column] [--create-reflog] [--sort=]
[--format=] [--[no-]merged []] [...]
-'git tag' -v ...
+'git tag' -v [--format=] ...
 
 DESCRIPTION
 ---
diff --git a/builtin/tag.c b/builtin/tag.c
index 14f3b48..7730fd0 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -24,7 +24,7 @@ static const char * const git_tag_usage[] = {
N_("git tag -d ..."),
N_("git tag -l [-n[]] [--contains ] [--points-at ]"
"\n\t\t[--format=] [--[no-]merged []] 
[...]"),
-   N_("git tag -v ..."),
+   N_("git tag -v [--format=] ..."),
NULL
 };
 
@@ -66,15 +66,17 @@ static int list_tags(struct ref_filter *filter, struct 
ref_sorting *sorting, con
 }
 
 typedef int (*each_tag_name_fn)(const char *name, const char *ref,
-   const unsigned char *sha1);
+   const unsigned char *sha1, void *cb_data);
 
-static int for_each_tag_name(const char **argv, each_tag_name_fn fn)
+static int for_each_tag_name(const char **argv, each_tag_name_fn fn,
+   void *cb_data)
 {
const char **p;
char ref[PATH_MAX];
int had_error = 0;
unsigned char sha1[20];
 
+
for (p = argv; *p; p++) {
if (snprintf(ref, sizeof(ref), "refs/tags/%s", *p)
>= sizeof(ref)) {
@@ -87,14 +89,14 @@ static int for_each_tag_name(const char **argv, 
each_tag_name_fn fn)
had_error = 1;
continue;
}
-   if (fn(*p, ref, sha1))
+   if (fn(*p, ref, sha1, cb_data))
had_error = 1;
}
return had_error;
 }
 
 static int delete_tag(const char *name, const char *ref,
-   const unsigned char *sha1)
+   const unsigned char *sha1, void *cb_data)
 {
if (delete_ref(ref, sha1, 0))
return 1;
@@ -103,9 +105,16 @@ static int delete_tag(const char *name, const char *ref,
 }
 
 static int verify_tag(const char *name, const char *ref,
-   const unsigned char *sha1)
+   const unsigned char *sha1, void *cb_data)
 {
-   return verify_and_format_tag(sha1, name, NULL, GPG_VERIFY_VERBOSE);
+   int flags;
+char *fmt_pretty = cb_data;
+   flags = GPG_VERIFY_VERBOSE;
+
+   if (fmt_pretty)
+   flags = GPG_VERIFY_QUIET;
+
+   return verify_and_format_tag(sha1, name, fmt_pretty, flags);
 }
 
 static int do_sign(struct strbuf *buffer)
@@ -334,7 +343,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
struct strbuf err = STRBUF_INIT;
struct ref_filter filter;
static struct ref_sorting *sorting = NULL, **sorting_tail = 
-   const char *format = NULL;
+   char *format = NULL;
struct option options[] = {
OPT_CMDMODE('l', "list", , N_("list tag names"), 'l'),
{ OPTION_INTEGER, 'n', NULL, , N_("n"),
@@ -424,9 +433,12 @@ int cmd_tag(int argc, const char **argv, const char 
*prefix)
if (filter.merge_commit)
die(_("--merged and --no-merged option are only allowed with 
-l"));
if (cmdmode == 'd')
-   return for_each_tag_name(argv, delete_tag);
-   if (cmdmode == 'v')
-   return for_each_tag_name(argv, verify_tag);
+   return for_each_tag_name(argv, delete_tag, NULL);
+   if (cmdmode == 'v') {
+   if (format)
+   verify_ref_format(format);
+   return for_each_tag_name(argv, verify_tag, format);
+   }
 
if (msg.given || msgfile) {
if (msg.given && msgfile)
-- 
2.10.0

[PATCH v3 2/5] ref-filter: add function to print single ref_array_item

[santiago]

From: Lukas Puehringer 

ref-filter functions are useful for printing git object information
using a format specifier. However, some other modules may not want to use
this functionality on a ref-array but only print a single item.

Expose a format_ref function to create, pretty print and free individual
ref-items.

Signed-off-by: Lukas Puehringer 
---
 ref-filter.c | 10 ++
 ref-filter.h |  3 +++
 2 files changed, 13 insertions(+)

diff --git a/ref-filter.c b/ref-filter.c
index bc551a7..ee3ed67 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1655,6 +1655,16 @@ void show_ref_array_item(struct ref_array_item *info, 
const char *format, int qu
putchar('\n');
 }
 
+void pretty_print_ref(const char *name, const unsigned char *sha1,
+   const char *format, unsigned kind)
+{
+   struct ref_array_item *ref_item;
+   ref_item = new_ref_array_item(name, sha1, 0);
+   ref_item->kind = kind;
+   show_ref_array_item(ref_item, format, 0);
+   free_array_item(ref_item);
+}
+
 /*  If no sorting option is given, use refname to sort as default */
 struct ref_sorting *ref_default_sorting(void)
 {
diff --git a/ref-filter.h b/ref-filter.h
index 14d435e..3d23090 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -107,4 +107,7 @@ struct ref_sorting *ref_default_sorting(void);
 /*  Function to parse --merged and --no-merged options */
 int parse_opt_merge_filter(const struct option *opt, const char *arg, int 
unset);
 
+void pretty_print_ref(const char *name, const unsigned char *sha1,
+   const char *format, unsigned kind);
+
 #endif /*  REF_FILTER_H  */
-- 
2.10.0

[PATCH v3 4/5] builtin/verify-tag: add --format to verify-tag

[santiago]

From: Santiago Torres <santi...@nyu.edu>

Callers of verify-tag may want to cross-check the tagname from refs/tags
with the tagname from the tag object header upon GPG verification. This
is to avoid tag refs that point to an incorrect object.

Add a --format parameter to git verify-tag to print the formatted tag
object header in addition to or instead of the --verbose or --raw GPG
verification output.

Signed-off-by: Santiago Torres <santi...@nyu.edu>
---
 Documentation/git-verify-tag.txt |  2 +-
 builtin/verify-tag.c | 13 +++--
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-verify-tag.txt b/Documentation/git-verify-tag.txt
index d590edc..0b8075d 100644
--- a/Documentation/git-verify-tag.txt
+++ b/Documentation/git-verify-tag.txt
@@ -8,7 +8,7 @@ git-verify-tag - Check the GPG signature of tags
 SYNOPSIS
 
 [verse]
-'git verify-tag' ...
+'git verify-tag' [--format=] ...
 
 DESCRIPTION
 ---
diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index de10198..745b6a6 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -12,12 +12,14 @@
 #include 
 #include "parse-options.h"
 #include "gpg-interface.h"
+#include "ref-filter.h"
 
 static const char * const verify_tag_usage[] = {
-   N_("git verify-tag [-v | --verbose] ..."),
+   N_("git verify-tag [-v | --verbose] [--format=] 
..."),
NULL
 };
 
+
 static int git_verify_tag_config(const char *var, const char *value, void *cb)
 {
int status = git_gpg_config(var, value, cb);
@@ -30,9 +32,11 @@ int cmd_verify_tag(int argc, const char **argv, const char 
*prefix)
 {
int i = 1, verbose = 0, had_error = 0;
unsigned flags = 0;
+char *fmt_pretty;
const struct option verify_tag_options[] = {
OPT__VERBOSE(, N_("print tag contents")),
OPT_BIT(0, "raw", , N_("print raw gpg status output"), 
GPG_VERIFY_RAW),
+   OPT_STRING(  0 , "format", _pretty, N_("format"), 
N_("format to use for the output")),
OPT_END()
};
 
@@ -46,12 +50,17 @@ int cmd_verify_tag(int argc, const char **argv, const char 
*prefix)
if (verbose)
flags |= GPG_VERIFY_VERBOSE;
 
+   if (fmt_pretty) {
+   verify_ref_format(fmt_pretty);
+   flags |= GPG_VERIFY_QUIET;
+   }
+
while (i < argc) {
unsigned char sha1[20];
const char *name = argv[i++];
if (get_sha1(name, sha1))
had_error = !!error("tag '%s' not found.", name);
-   else if (verify_and_format_tag(sha1, name, NULL, flags))
+   else if (verify_and_format_tag(sha1, name, fmt_pretty, flags))
had_error = 1;
}
return had_error;
-- 
2.10.0

[PATCH v3 0/5] Add --format to tag verification

[santiago]

From: Santiago Torres <santi...@nyu.edu>

This is the third iteration of [1][2], and as a result of the discussion
in [3].

In this re-roll we:

* Fixed all the signed-off-by's

[0002]
* Renamed the function format_ref to pretty_print_ref instead, which
  is a more descriptive name 

[0004] 
* Added the respective line for the new --format parameter in the
  documentation.

[0005] 
* Added mention of the --format flag in the documentation files. 
* Fixed the function signatures, now they take an opaque void *cb_data pointer
  so it can be used in a more general way (by e.g., delete_tag).

This patch applies to 2.10.0 and master.

[1] http://public-inbox.org/git/20160922185317.349-1-santi...@nyu.edu/
[2] http://public-inbox.org/git/20160926224233.32702-1-santi...@nyu.edu/
[3] http://public-inbox.org/git/20160607195608.16643-1-santi...@nyu.edu/

Lukas Puehringer (4):
  gpg-interface, tag: add GPG_VERIFY_QUIET flag
  ref-filter: add function to print single ref_array_item
  tag: add format specifier to gpg_verify_tag
  builtin/tag: add --format argument for tag -v

Santiago Torres (1):
  builtin/verify-tag: add --format to verify-tag

 Documentation/git-tag.txt|  2 +-
 Documentation/git-verify-tag.txt |  2 +-
 builtin/tag.c| 34 +++---
 builtin/verify-tag.c | 13 +++--
 gpg-interface.h  |  1 +
 ref-filter.c | 10 ++
 ref-filter.h |  3 +++
 tag.c| 22 +++---
 tag.h|  4 ++--
 9 files changed, 67 insertions(+), 24 deletions(-)

-- 
2.10.0

[PATCH v3 1/5] gpg-interface, tag: add GPG_VERIFY_QUIET flag

[santiago]

From: Lukas Puehringer 

Functions that print git object information may require that the
gpg-interface functions be silent. Add GPG_VERIFY_QUIET flag and prevent
print_signature_buffer from being called if flag is set.

Signed-off-by: Lukas Puehringer 
---
 gpg-interface.h | 1 +
 tag.c   | 5 -
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/gpg-interface.h b/gpg-interface.h
index ea68885..85dc982 100644
--- a/gpg-interface.h
+++ b/gpg-interface.h
@@ -3,6 +3,7 @@
 
 #define GPG_VERIFY_VERBOSE 1
 #define GPG_VERIFY_RAW 2
+#define GPG_VERIFY_QUIET   4
 
 struct signature_check {
char *payload;
diff --git a/tag.c b/tag.c
index d1dcd18..291073f 100644
--- a/tag.c
+++ b/tag.c
@@ -3,6 +3,7 @@
 #include "commit.h"
 #include "tree.h"
 #include "blob.h"
+#include "gpg-interface.h"
 
 const char *tag_type = "tag";
 
@@ -24,7 +25,9 @@ static int run_gpg_verify(const char *buf, unsigned long 
size, unsigned flags)
 
ret = check_signature(buf, payload_size, buf + payload_size,
size - payload_size, );
-   print_signature_buffer(, flags);
+
+   if (!(flags & GPG_VERIFY_QUIET))
+   print_signature_buffer(, flags);
 
signature_check_clear();
return ret;
-- 
2.10.0

Re: [PATCH v2 4/5] builtin/verify-tag: add --format to verify-tag

[Santiago Torres]

> > static const char * const verify_tag_usage[] = {
> > - N_("git verify-tag [-v | --verbose] ..."),
> > + N_("git verify-tag [-v | --verbose] [--format=] ..."),
> 
> Does this require a corresponding documentation change? (also 5/5)
> 

Yes, I'll work on this while I wait for more reviews.

Thanks!
-Santiago.


signature.asc
Description: PGP signature

[PATCH v2 5/5] builtin/tag: add --format argument for tag -v

[santiago]

From: Lukas P 

Adding --format to git tag -v mutes the default output of the GPG
verification and instead prints the formatted tag object.
This allows callers to cross-check the tagname from refs/tags with
the tagname from the tag object header upon GPG verification.

Caveat: The change adds a format specifier argument to the
(*each_tag_name_fn) function pointer, i.e. delete_tag now receives this
too, although it does not need it.

Signed-off-by: Lukas P 
---
 builtin/tag.c | 30 --
 1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/builtin/tag.c b/builtin/tag.c
index 14f3b48..f53227e 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -24,7 +24,7 @@ static const char * const git_tag_usage[] = {
N_("git tag -d ..."),
N_("git tag -l [-n[]] [--contains ] [--points-at ]"
"\n\t\t[--format=] [--[no-]merged []] 
[...]"),
-   N_("git tag -v ..."),
+   N_("git tag -v [--format=] ..."),
NULL
 };
 
@@ -66,9 +66,10 @@ static int list_tags(struct ref_filter *filter, struct 
ref_sorting *sorting, con
 }
 
 typedef int (*each_tag_name_fn)(const char *name, const char *ref,
-   const unsigned char *sha1);
+   const unsigned char *sha1, const char 
*fmt_pretty);
 
-static int for_each_tag_name(const char **argv, each_tag_name_fn fn)
+static int for_each_tag_name(const char **argv, each_tag_name_fn fn,
+   const char *fmt_pretty)
 {
const char **p;
char ref[PATH_MAX];
@@ -87,14 +88,14 @@ static int for_each_tag_name(const char **argv, 
each_tag_name_fn fn)
had_error = 1;
continue;
}
-   if (fn(*p, ref, sha1))
+   if (fn(*p, ref, sha1, fmt_pretty))
had_error = 1;
}
return had_error;
 }
 
 static int delete_tag(const char *name, const char *ref,
-   const unsigned char *sha1)
+   const unsigned char *sha1, const char 
*fmt_pretty)
 {
if (delete_ref(ref, sha1, 0))
return 1;
@@ -103,9 +104,15 @@ static int delete_tag(const char *name, const char *ref,
 }
 
 static int verify_tag(const char *name, const char *ref,
-   const unsigned char *sha1)
+   const unsigned char *sha1, const char 
*fmt_pretty)
 {
-   return verify_and_format_tag(sha1, name, NULL, GPG_VERIFY_VERBOSE);
+   int flags;
+   flags = GPG_VERIFY_VERBOSE;
+
+   if (fmt_pretty)
+   flags = GPG_VERIFY_QUIET;
+
+   return verify_and_format_tag(sha1, name, fmt_pretty, flags);
 }
 
 static int do_sign(struct strbuf *buffer)
@@ -424,9 +431,12 @@ int cmd_tag(int argc, const char **argv, const char 
*prefix)
if (filter.merge_commit)
die(_("--merged and --no-merged option are only allowed with 
-l"));
if (cmdmode == 'd')
-   return for_each_tag_name(argv, delete_tag);
-   if (cmdmode == 'v')
-   return for_each_tag_name(argv, verify_tag);
+   return for_each_tag_name(argv, delete_tag, NULL);
+   if (cmdmode == 'v') {
+   if (format)
+   verify_ref_format(format);
+   return for_each_tag_name(argv, verify_tag, format);
+   }
 
if (msg.given || msgfile) {
if (msg.given && msgfile)
-- 
2.10.0

[PATCH v2 4/5] builtin/verify-tag: add --format to verify-tag

[santiago]

From: Santiago Torres <santi...@nyu.edu>

Callers of verify-tag may want to cross-check the tagname from refs/tags
with the tagname from the tag object header upon GPG verification. This
is to avoid tag refs that point to an incorrect object.

Add a --format parameter to git verify-tag to print the formatted tag
object header in addition to or instead of the --verbose or --raw GPG
verification output.

Signed-off-by: Santiago Torres <santi...@nyu.edu>
---
 builtin/verify-tag.c | 13 +++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index de10198..a941053 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -12,12 +12,15 @@
 #include 
 #include "parse-options.h"
 #include "gpg-interface.h"
+#include "ref-filter.h"
 
 static const char * const verify_tag_usage[] = {
-   N_("git verify-tag [-v | --verbose] ..."),
+   N_("git verify-tag [-v | --verbose] [--format=] 
..."),
NULL
 };
 
+static char *fmt_pretty;
+
 static int git_verify_tag_config(const char *var, const char *value, void *cb)
 {
int status = git_gpg_config(var, value, cb);
@@ -33,6 +36,7 @@ int cmd_verify_tag(int argc, const char **argv, const char 
*prefix)
const struct option verify_tag_options[] = {
OPT__VERBOSE(, N_("print tag contents")),
OPT_BIT(0, "raw", , N_("print raw gpg status output"), 
GPG_VERIFY_RAW),
+   OPT_STRING(  0 , "format", _pretty, N_("format"), 
N_("format to use for the output")),
OPT_END()
};
 
@@ -46,12 +50,17 @@ int cmd_verify_tag(int argc, const char **argv, const char 
*prefix)
if (verbose)
flags |= GPG_VERIFY_VERBOSE;
 
+   if (fmt_pretty) {
+   verify_ref_format(fmt_pretty);
+   flags |= GPG_VERIFY_QUIET;
+   }
+
while (i < argc) {
unsigned char sha1[20];
const char *name = argv[i++];
if (get_sha1(name, sha1))
had_error = !!error("tag '%s' not found.", name);
-   else if (verify_and_format_tag(sha1, name, NULL, flags))
+   else if (verify_and_format_tag(sha1, name, fmt_pretty, flags))
had_error = 1;
}
return had_error;
-- 
2.10.0

[PATCH v2 0/5] Add --format to tag verification

[santiago]

From: Santiago Torres <santi...@nyu.edu>

This is the second iteration of [1], and as a result of the discussion
in [2].

In this re-roll we:

* Dropped the commit to move the format string parameter to a global
  variable on builtin/tag. We had to change the signature of
  for_each_name_fn to do this.

* Fixed the signed-off-by lines to match the author/committer

[0001]
* Moved the GPG_VERIFY_QUIET flag check into tag.c instead of
  gpg_interface

[0002] 
* Changed the exported function to "format_ref". This way, we can print
  a single formatted ref element. This also made the patch
  cleaner/easier to read.

[0003] 
* We fixed style/formatting errors that we had introduced

This patch applies to 2.10.0 and master.

[1] http://public-inbox.org/git/20160922185317.349-1-santi...@nyu.edu/
[2] http://public-inbox.org/git/20160607195608.16643-1-santi...@nyu.edu/

Lukas P (4):
  gpg-interface, tag: add GPG_VERIFY_QUIET flag
  ref-filter: add function to print single ref_array_item
  tag: add format specifier to gpg_verify_tag
  builtin/tag: add --format argument for tag -v

Santiago Torres (1):
  builtin/verify-tag: add --format to verify-tag

 builtin/tag.c| 30 --
 builtin/verify-tag.c | 16 ++--
 gpg-interface.h  |  1 +
 ref-filter.c | 10 ++
 ref-filter.h |  4 
 tag.c| 22 +++---
 tag.h|  4 ++--
 7 files changed, 66 insertions(+), 21 deletions(-)

-- 
2.10.0

[PATCH v2 3/5] tag: add format specifier to gpg_verify_tag

[santiago]

From: Lukas P 

Calling functions for gpg_verify_tag() may desire to print relevant
information about the header for further verification. Add an optional
format argument to print any desired information after GPG verification.

Signed-off-by: Lukas P 
---
 builtin/tag.c|  2 +-
 builtin/verify-tag.c |  2 +-
 tag.c| 17 +++--
 tag.h|  4 ++--
 4 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/builtin/tag.c b/builtin/tag.c
index 50e4ae5..14f3b48 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -105,7 +105,7 @@ static int delete_tag(const char *name, const char *ref,
 static int verify_tag(const char *name, const char *ref,
const unsigned char *sha1)
 {
-   return gpg_verify_tag(sha1, name, GPG_VERIFY_VERBOSE);
+   return verify_and_format_tag(sha1, name, NULL, GPG_VERIFY_VERBOSE);
 }
 
 static int do_sign(struct strbuf *buffer)
diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index 99f8148..de10198 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -51,7 +51,7 @@ int cmd_verify_tag(int argc, const char **argv, const char 
*prefix)
const char *name = argv[i++];
if (get_sha1(name, sha1))
had_error = !!error("tag '%s' not found.", name);
-   else if (gpg_verify_tag(sha1, name, flags))
+   else if (verify_and_format_tag(sha1, name, NULL, flags))
had_error = 1;
}
return had_error;
diff --git a/tag.c b/tag.c
index 291073f..65e1a96 100644
--- a/tag.c
+++ b/tag.c
@@ -4,6 +4,7 @@
 #include "tree.h"
 #include "blob.h"
 #include "gpg-interface.h"
+#include "ref-filter.h"
 
 const char *tag_type = "tag";
 
@@ -33,8 +34,8 @@ static int run_gpg_verify(const char *buf, unsigned long 
size, unsigned flags)
return ret;
 }
 
-int gpg_verify_tag(const unsigned char *sha1, const char *name_to_report,
-   unsigned flags)
+int verify_and_format_tag(const unsigned char *sha1, const char *name,
+   const char *fmt_pretty, unsigned flags)
 {
enum object_type type;
char *buf;
@@ -44,21 +45,25 @@ int gpg_verify_tag(const unsigned char *sha1, const char 
*name_to_report,
type = sha1_object_info(sha1, NULL);
if (type != OBJ_TAG)
return error("%s: cannot verify a non-tag object of type %s.",
-   name_to_report ?
-   name_to_report :
+   name ?
+   name :
find_unique_abbrev(sha1, DEFAULT_ABBREV),
typename(type));
 
buf = read_sha1_file(sha1, , );
if (!buf)
return error("%s: unable to read file.",
-   name_to_report ?
-   name_to_report :
+   name ?
+   name :
find_unique_abbrev(sha1, DEFAULT_ABBREV));
 
ret = run_gpg_verify(buf, size, flags);
 
free(buf);
+
+   if (fmt_pretty)
+   format_ref(name, sha1, fmt_pretty, FILTER_REFS_TAGS);
+
return ret;
 }
 
diff --git a/tag.h b/tag.h
index a5721b6..0b6e458 100644
--- a/tag.h
+++ b/tag.h
@@ -17,7 +17,7 @@ extern int parse_tag_buffer(struct tag *item, const void 
*data, unsigned long si
 extern int parse_tag(struct tag *item);
 extern struct object *deref_tag(struct object *, const char *, int);
 extern struct object *deref_tag_noverify(struct object *);
-extern int gpg_verify_tag(const unsigned char *sha1,
-   const char *name_to_report, unsigned flags);
+extern int verify_and_format_tag(const unsigned char *sha1, const char *name,
+   const char *fmt_pretty, unsigned flags);
 
 #endif /* TAG_H */
-- 
2.10.0

[PATCH v2 1/5] gpg-interface, tag: add GPG_VERIFY_QUIET flag

[santiago]

From: Lukas P 

Functions that print git object information may require that the
gpg-interface functions be silent. Add GPG_VERIFY_QUIET flag and prevent
print_signature_buffer from being called if flag is set.

Signed-off-by: Lukas P 
---
 gpg-interface.h | 1 +
 tag.c   | 5 -
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/gpg-interface.h b/gpg-interface.h
index ea68885..85dc982 100644
--- a/gpg-interface.h
+++ b/gpg-interface.h
@@ -3,6 +3,7 @@
 
 #define GPG_VERIFY_VERBOSE 1
 #define GPG_VERIFY_RAW 2
+#define GPG_VERIFY_QUIET   4
 
 struct signature_check {
char *payload;
diff --git a/tag.c b/tag.c
index d1dcd18..291073f 100644
--- a/tag.c
+++ b/tag.c
@@ -3,6 +3,7 @@
 #include "commit.h"
 #include "tree.h"
 #include "blob.h"
+#include "gpg-interface.h"
 
 const char *tag_type = "tag";
 
@@ -24,7 +25,9 @@ static int run_gpg_verify(const char *buf, unsigned long 
size, unsigned flags)
 
ret = check_signature(buf, payload_size, buf + payload_size,
size - payload_size, );
-   print_signature_buffer(, flags);
+
+   if (!(flags & GPG_VERIFY_QUIET))
+   print_signature_buffer(, flags);
 
signature_check_clear();
return ret;
-- 
2.10.0

[PATCH v2 2/5] ref-filter: add function to print single ref_array_item

[santiago]

From: Lukas P 

ref-filter functions are useful for printing git object information
using a format specifier. However, some other modules may not want to use
this functionality on a ref-array but only print a single item.

Expose a format_ref function to create, pretty print and free individual
ref-items.

Signed-off-by: Lukas P 
---
 ref-filter.c | 10 ++
 ref-filter.h |  4 
 2 files changed, 14 insertions(+)

diff --git a/ref-filter.c b/ref-filter.c
index bc551a7..e0aaf5f 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1655,6 +1655,16 @@ void show_ref_array_item(struct ref_array_item *info, 
const char *format, int qu
putchar('\n');
 }
 
+void format_ref(const char *name, const unsigned char *sha1, const char 
*format,
+   unsigned kind)
+{
+   struct ref_array_item *ref_item;
+   ref_item = new_ref_array_item(name, sha1, 0);
+   ref_item->kind = kind;
+   show_ref_array_item(ref_item, format, 0);
+   free_array_item(ref_item);
+}
+
 /*  If no sorting option is given, use refname to sort as default */
 struct ref_sorting *ref_default_sorting(void)
 {
diff --git a/ref-filter.h b/ref-filter.h
index 14d435e..1ef7999 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -107,4 +107,8 @@ struct ref_sorting *ref_default_sorting(void);
 /*  Function to parse --merged and --no-merged options */
 int parse_opt_merge_filter(const struct option *opt, const char *arg, int 
unset);
 
+/* Pretty-print a single ref */
+void format_ref(const char *name, const unsigned char *sha1, const char 
*format,
+   unsigned kind);
+
 #endif /*  REF_FILTER_H  */
-- 
2.10.0

Re: [PATCH 4/6] tag: add format specifier to gpg_verify_tag

[Santiago Torres]

On Thu, Sep 22, 2016 at 01:58:06PM -0700, Junio C Hamano wrote:
> santi...@nyu.edu writes:
> 
> > Calling functions for gpg_verify_tag() may desire to print relevant
> > information about the header for further verification. Add an optional
> > format argument to print any desired information after GPG verification.
> 
> > diff --git a/builtin/tag.c b/builtin/tag.c
> > index dbf271f..94ed8a2 100644
> > --- a/builtin/tag.c
> > +++ b/builtin/tag.c
> > @@ -106,7 +106,7 @@ static int delete_tag(const char *name, const char *ref,
> >  static int verify_tag(const char *name, const char *ref,
> > const unsigned char *sha1)
> >  {
> > -   return gpg_verify_tag(sha1, name, GPG_VERIFY_VERBOSE);
> > +   return verify_and_format_tag(sha1, name, NULL, GPG_VERIFY_VERBOSE);
> >  }
> >  
> >  static int do_sign(struct strbuf *buffer)
> > diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
> > index 99f8148..7a1121b 100644
> > --- a/builtin/verify-tag.c
> > +++ b/builtin/verify-tag.c
> > @@ -51,8 +51,10 @@ int cmd_verify_tag(int argc, const char **argv, const 
> > char *prefix)
> > const char *name = argv[i++];
> > if (get_sha1(name, sha1))
> > had_error = !!error("tag '%s' not found.", name);
> > -   else if (gpg_verify_tag(sha1, name, flags))
> > -   had_error = 1;
> > +   else {
> > +   if (verify_and_format_tag(sha1, name, NULL, flags))
> > +   had_error = 1;
> > +   }
> 
> Revert the unnecessary reformatting here.
> 
> > @@ -56,6 +57,15 @@ int gpg_verify_tag(const unsigned char *sha1, const char 
> > *name_to_report,
> > ret = run_gpg_verify(buf, size, flags);
> >  
> > free(buf);
> > +
> > +   if (fmt_pretty) {
> > +   struct ref_array_item *ref_item;
> > +   ref_item = new_ref_item(name_to_report, sha1, 0);
> > +   ref_item->kind = FILTER_REFS_TAGS;
> > +   show_ref_item(ref_item, fmt_pretty, 0);
> > +   free_ref_item(ref_item);
> > +   }
> 
> I haven't seen 5/6 and 6/6, but if this is the only user of the 3/6,
> it would be much better to have a single function to format a ref
> exported from ref-filter.[ch] so that this one can say
> 
>   if (fmt_pretty)
>   format_ref(name_to_report, sha1, FILTER_REFS_TAGS);
> 
> or something like that, instead of doing three that will always be
> used together in quick succession in the above pattern.

Oh, this sounds like a better alternative. This would be instead of 0003
right? 

Thanks,
-Santiago.


signature.asc
Description: PGP signature

Re: [PATCH 5/6] builtin/verify-tag: Add --format to verify-tag

[Santiago Torres]

On Thu, Sep 22, 2016 at 02:16:21PM -0700, Junio C Hamano wrote:
> santi...@nyu.edu writes:
> 
> > From: Santiago Torres <santi...@nyu.edu>
> >
> > Callers of verify-tag may want to cross-check the tagname from refs/tags
> > with the tagname from the tag object header upon GPG verification. This
> > is to avoid tag refs that point to an incorrect object.
> >
> > Add a --format parameter to git verify-tag to print the formatted tag
> > object header in addition to or instead of the --verbose or --raw GPG
> > verification output.
> >
> > Signed-off-by: Santiago Torres <santi...@nyu.edu>
> > ---
> >  builtin/verify-tag.c | 13 +++--
> >  1 file changed, 11 insertions(+), 2 deletions(-)
> >
> > diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
> > index 7a1121b..319d469 100644
> > --- a/builtin/verify-tag.c
> > +++ b/builtin/verify-tag.c
> > @@ -12,12 +12,15 @@
> >  #include 
> >  #include "parse-options.h"
> >  #include "gpg-interface.h"
> > +#include "ref-filter.h"
> >  
> >  static const char * const verify_tag_usage[] = {
> > -   N_("git verify-tag [-v | --verbose] ..."),
> > +   N_("git verify-tag [-v | --verbose] [--format=] 
> > ..."),
> > NULL
> >  };
> >  
> > +char *fmt_pretty;
> 
> Does this have to be extern?  I do not think so; prepend "static "
> in front of it.
> 
> > while (i < argc) {
> > unsigned char sha1[20];
> > const char *name = argv[i++];
> > if (get_sha1(name, sha1))
> > had_error = !!error("tag '%s' not found.", name);
> > else {
> > -   if (verify_and_format_tag(sha1, name, NULL, flags))
> > +   if (verify_and_format_tag(sha1, name, fmt_pretty, 
> > flags))
> 
> OK.  The callchain from here is
> 
> verify_and_format_tag()
> -> run_gpg_verify()
>   -> print_signature_buffer()
> 
> so not cramming QUIET into the flags parameter that is already
> passed is cumbersome.  As I said in my earlier review, it would make
> more sense to have the conditional NOT in print_signature_buffer()
> but in its caller, but it still is OK to add GPG_VERIFY_QUIET bit
> to the flag, which you would check in run_gpg_verify() to decide not
> to call print_signature_buffer().
> 

Yeah, in retrospect, this sounds like a more reasonable approach than
doing it on gpg-nterface. I'll keep the QUIET bit then.


signature.asc
Description: PGP signature

Re: [PATCH 6/6] builtin/tag: add --format argument for tag -v

[Santiago Torres]

> OK, you said something about for_each_ref() in an earlier commit,
> but what you meant was this one, which takes each_tag_name_fn.

Oh yeah, sorry for the confusion.

> 
> The function for_each_tag_name(), the type each_tag_name_fn, and the
> function of that type verify_tag(), are ALL file-scope static in
> this single file, builtin/tag.c.  It seems to me that it is not
> necessary to make the format string global at all.

Oh, ok. I was thinking that this was preferred over changing the
signature of those functions. (I drew my conclusion from log.c). I'll
take this other road then.
> 
> ... 
> 
> There are minor implementation and design issues I spotted, but
> overall I think the feature the series attempts to add may be a good
> thing to have.
> 

Thanks for the review! I'll re-roll shortly.
-Santiago. 


> Thanks.


signature.asc
Description: PGP signature

[PATCH 5/6] builtin/verify-tag: Add --format to verify-tag

[santiago]

From: Santiago Torres <santi...@nyu.edu>

Callers of verify-tag may want to cross-check the tagname from refs/tags
with the tagname from the tag object header upon GPG verification. This
is to avoid tag refs that point to an incorrect object.

Add a --format parameter to git verify-tag to print the formatted tag
object header in addition to or instead of the --verbose or --raw GPG
verification output.

Signed-off-by: Santiago Torres <santi...@nyu.edu>
---
 builtin/verify-tag.c | 13 +++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index 7a1121b..319d469 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -12,12 +12,15 @@
 #include 
 #include "parse-options.h"
 #include "gpg-interface.h"
+#include "ref-filter.h"
 
 static const char * const verify_tag_usage[] = {
-   N_("git verify-tag [-v | --verbose] ..."),
+   N_("git verify-tag [-v | --verbose] [--format=] 
..."),
NULL
 };
 
+char *fmt_pretty;
+
 static int git_verify_tag_config(const char *var, const char *value, void *cb)
 {
int status = git_gpg_config(var, value, cb);
@@ -33,6 +36,7 @@ int cmd_verify_tag(int argc, const char **argv, const char 
*prefix)
const struct option verify_tag_options[] = {
OPT__VERBOSE(, N_("print tag contents")),
OPT_BIT(0, "raw", , N_("print raw gpg status output"), 
GPG_VERIFY_RAW),
+   OPT_STRING(  0 , "format", _pretty, N_("format"), 
N_("format to use for the output")),
OPT_END()
};
 
@@ -46,13 +50,18 @@ int cmd_verify_tag(int argc, const char **argv, const char 
*prefix)
if (verbose)
flags |= GPG_VERIFY_VERBOSE;
 
+   if (fmt_pretty) {
+   verify_ref_format(fmt_pretty);
+   flags |= GPG_VERIFY_QUIET;
+   }
+
while (i < argc) {
unsigned char sha1[20];
const char *name = argv[i++];
if (get_sha1(name, sha1))
had_error = !!error("tag '%s' not found.", name);
else {
-   if (verify_and_format_tag(sha1, name, NULL, flags))
+   if (verify_and_format_tag(sha1, name, fmt_pretty, 
flags))
had_error = 1;
}
}
-- 
2.10.0

[PATCH 2/6] gpg-interface: add GPG_VERIFY_QUIET flag

[santiago]

From: Lukas P 

Functions that print git object information may require that the
gpg-interface functions be silent. Add a GPG_VERIFY_QUIET to prevent
functions such as `print_signature_buffer` from printing any output and
only return whether signature verification passed or not.

Signed-off-by: Lukas Puehringer 
---
 gpg-interface.c | 3 +++
 gpg-interface.h | 1 +
 2 files changed, 4 insertions(+)

diff --git a/gpg-interface.c b/gpg-interface.c
index 8672eda..b82bc50 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -88,6 +88,9 @@ int check_signature(const char *payload, size_t plen, const 
char *signature,
 
 void print_signature_buffer(const struct signature_check *sigc, unsigned flags)
 {
+   if (flags & GPG_VERIFY_QUIET)
+   return;
+
const char *output = flags & GPG_VERIFY_RAW ?
sigc->gpg_status : sigc->gpg_output;
 
diff --git a/gpg-interface.h b/gpg-interface.h
index ea68885..85dc982 100644
--- a/gpg-interface.h
+++ b/gpg-interface.h
@@ -3,6 +3,7 @@
 
 #define GPG_VERIFY_VERBOSE 1
 #define GPG_VERIFY_RAW 2
+#define GPG_VERIFY_QUIET   4
 
 struct signature_check {
char *payload;
-- 
2.10.0

[PATCH 1/6] builtin/tag: move format specifier to global var

[santiago]

From: Santiago Torres <santi...@nyu.edu>

The format specifier will be likely used in other functions throughout
git tag. One likely candidate to require format strings in the future is
the gpg_verify_tag function. However, changing the signature of
functions such as for_each_ref or verify_tag would be quite burdensome.

Instead, we move the format string specifier to a static global variable
that modules can access in the same way git-log and other modules handle
this case.

Signed-off-by: Santiago Torres <santi...@nyu.edu>
---
 builtin/tag.c | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/builtin/tag.c b/builtin/tag.c
index 50e4ae5..dbf271f 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -30,8 +30,9 @@ static const char * const git_tag_usage[] = {
 
 static unsigned int colopts;
 static int force_sign_annotate;
+static const char *fmt_pretty;
 
-static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, 
const char *format)
+static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting)
 {
struct ref_array array;
char *to_free = NULL;
@@ -42,23 +43,23 @@ static int list_tags(struct ref_filter *filter, struct 
ref_sorting *sorting, con
if (filter->lines == -1)
filter->lines = 0;
 
-   if (!format) {
+   if (!fmt_pretty) {
if (filter->lines) {
to_free = xstrfmt("%s %%(contents:lines=%d)",
  "%(align:15)%(refname:strip=2)%(end)",
  filter->lines);
-   format = to_free;
+   fmt_pretty = to_free;
} else
-   format = "%(refname:strip=2)";
+   fmt_pretty = "%(refname:strip=2)";
}
 
-   verify_ref_format(format);
+   verify_ref_format(fmt_pretty);
filter->with_commit_tag_algo = 1;
filter_refs(, filter, FILTER_REFS_TAGS);
ref_array_sort(sorting, );
 
for (i = 0; i < array.nr; i++)
-   show_ref_array_item(array.items[i], format, 0);
+   show_ref_array_item(array.items[i], fmt_pretty, 0);
ref_array_clear();
free(to_free);
 
@@ -334,7 +335,6 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
struct strbuf err = STRBUF_INIT;
struct ref_filter filter;
static struct ref_sorting *sorting = NULL, **sorting_tail = 
-   const char *format = NULL;
struct option options[] = {
OPT_CMDMODE('l', "list", , N_("list tag names"), 'l'),
{ OPTION_INTEGER, 'n', NULL, , N_("n"),
@@ -369,7 +369,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
OPTION_CALLBACK, 0, "points-at", _at, 
N_("object"),
N_("print only tags of the object"), 0, 
parse_opt_object_name
},
-   OPT_STRING(  0 , "format", , N_("format"), N_("format to 
use for the output")),
+   OPT_STRING(  0 , "format", _pretty, N_("format"), 
N_("format to use for the output")),
OPT_END()
};
 
@@ -410,7 +410,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
run_column_filter(colopts, );
}
filter.name_patterns = argv;
-   ret = list_tags(, sorting, format);
+   ret = list_tags(, sorting);
if (column_active(colopts))
stop_column_filter();
return ret;
-- 
2.10.0

[PATCH 4/6] tag: add format specifier to gpg_verify_tag

[santiago]

From: Lukas P 

Calling functions for gpg_verify_tag() may desire to print relevant
information about the header for further verification. Add an optional
format argument to print any desired information after GPG verification.

Signed-off-by: Lukas Puehringer 
---
 builtin/tag.c|  2 +-
 builtin/verify-tag.c |  6 --
 tag.c| 14 --
 tag.h|  4 ++--
 4 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/builtin/tag.c b/builtin/tag.c
index dbf271f..94ed8a2 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -106,7 +106,7 @@ static int delete_tag(const char *name, const char *ref,
 static int verify_tag(const char *name, const char *ref,
const unsigned char *sha1)
 {
-   return gpg_verify_tag(sha1, name, GPG_VERIFY_VERBOSE);
+   return verify_and_format_tag(sha1, name, NULL, GPG_VERIFY_VERBOSE);
 }
 
 static int do_sign(struct strbuf *buffer)
diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index 99f8148..7a1121b 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -51,8 +51,10 @@ int cmd_verify_tag(int argc, const char **argv, const char 
*prefix)
const char *name = argv[i++];
if (get_sha1(name, sha1))
had_error = !!error("tag '%s' not found.", name);
-   else if (gpg_verify_tag(sha1, name, flags))
-   had_error = 1;
+   else {
+   if (verify_and_format_tag(sha1, name, NULL, flags))
+   had_error = 1;
+   }
}
return had_error;
 }
diff --git a/tag.c b/tag.c
index d1dcd18..e08da51 100644
--- a/tag.c
+++ b/tag.c
@@ -3,6 +3,7 @@
 #include "commit.h"
 #include "tree.h"
 #include "blob.h"
+#include "ref-filter.h"
 
 const char *tag_type = "tag";
 
@@ -30,8 +31,8 @@ static int run_gpg_verify(const char *buf, unsigned long 
size, unsigned flags)
return ret;
 }
 
-int gpg_verify_tag(const unsigned char *sha1, const char *name_to_report,
-   unsigned flags)
+int verify_and_format_tag(const unsigned char *sha1, const char 
*name_to_report,
+   const char *fmt_pretty, unsigned flags)
 {
enum object_type type;
char *buf;
@@ -56,6 +57,15 @@ int gpg_verify_tag(const unsigned char *sha1, const char 
*name_to_report,
ret = run_gpg_verify(buf, size, flags);
 
free(buf);
+
+   if (fmt_pretty) {
+   struct ref_array_item *ref_item;
+   ref_item = new_ref_item(name_to_report, sha1, 0);
+   ref_item->kind = FILTER_REFS_TAGS;
+   show_ref_item(ref_item, fmt_pretty, 0);
+   free_ref_item(ref_item);
+   }
+
return ret;
 }
 
diff --git a/tag.h b/tag.h
index a5721b6..460b6f9 100644
--- a/tag.h
+++ b/tag.h
@@ -17,7 +17,7 @@ extern int parse_tag_buffer(struct tag *item, const void 
*data, unsigned long si
 extern int parse_tag(struct tag *item);
 extern struct object *deref_tag(struct object *, const char *, int);
 extern struct object *deref_tag_noverify(struct object *);
-extern int gpg_verify_tag(const unsigned char *sha1,
-   const char *name_to_report, unsigned flags);
+extern int verify_and_format_tag(const unsigned char *sha1,
+   const char *name_to_report, const char *fmt_pretty, unsigned 
flags);
 
 #endif /* TAG_H */
-- 
2.10.0

[PATCH 6/6] builtin/tag: add --format argument for tag -v

[santiago]

From: Lukas P 

Adding --format to git tag -v mutes the default output of the GPG
verification and instead prints the formatted tag object.

This allows callers to cross-check the tagname from refs/tags with
the tagname from the tag object header upon GPG verification.

Signed-off-by: Lukas Puehringer 
---
 builtin/tag.c | 15 ---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/builtin/tag.c b/builtin/tag.c
index 94ed8a2..3dd1e65 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -24,7 +24,7 @@ static const char * const git_tag_usage[] = {
N_("git tag -d ..."),
N_("git tag -l [-n[]] [--contains ] [--points-at ]"
"\n\t\t[--format=] [--[no-]merged []] 
[...]"),
-   N_("git tag -v ..."),
+   N_("git tag -v [--format=] ..."),
NULL
 };
 
@@ -106,7 +106,13 @@ static int delete_tag(const char *name, const char *ref,
 static int verify_tag(const char *name, const char *ref,
const unsigned char *sha1)
 {
-   return verify_and_format_tag(sha1, name, NULL, GPG_VERIFY_VERBOSE);
+   int flags;
+   flags = GPG_VERIFY_VERBOSE;
+
+   if (fmt_pretty)
+   flags = GPG_VERIFY_QUIET;
+
+   return verify_and_format_tag(sha1, name, fmt_pretty, flags);
 }
 
 static int do_sign(struct strbuf *buffer)
@@ -425,8 +431,11 @@ int cmd_tag(int argc, const char **argv, const char 
*prefix)
die(_("--merged and --no-merged option are only allowed with 
-l"));
if (cmdmode == 'd')
return for_each_tag_name(argv, delete_tag);
-   if (cmdmode == 'v')
+   if (cmdmode == 'v') {
+   if (fmt_pretty)
+   verify_ref_format(fmt_pretty);
return for_each_tag_name(argv, verify_tag);
+   }
 
if (msg.given || msgfile) {
if (msg.given && msgfile)
-- 
2.10.0

[PATCH 3/6] ref-filter: Expose wrappers for ref_item functions

[santiago]

From: Lukas P 

Ref-filter functions are useful for printing git object information
without a format specifier. However, some functions may not want to use
a complete ref-array, and just a single item instead. Expose
create/show/free functions for ref_array_items through wrappers around
the original functions.

Signed-off-by: Lukas Puehringer 
---
 ref-filter.c | 20 
 ref-filter.h | 10 ++
 2 files changed, 30 insertions(+)

diff --git a/ref-filter.c b/ref-filter.c
index 9adbb8a..b013799 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1329,6 +1329,14 @@ static struct ref_array_item *new_ref_array_item(const 
char *refname,
return ref;
 }
 
+/* Wrapper: Create ref_array_item w/o referencing container in function name */
+struct ref_array_item *new_ref_item(const char *refname,
+const unsigned char 
*objectname,
+int flag)
+{
+   return new_ref_array_item(refname, objectname, flag);
+}
+
 static int filter_ref_kind(struct ref_filter *filter, const char *refname)
 {
unsigned int i;
@@ -1426,6 +1434,12 @@ static void free_array_item(struct ref_array_item *item)
free(item);
 }
 
+/* Wrapper: Free ref_array_item w/o referencing container in function name */
+void free_ref_item(struct ref_array_item *ref_item)
+{
+   free_array_item(ref_item);
+}
+
 /* Free all memory allocated for ref_array */
 void ref_array_clear(struct ref_array *array)
 {
@@ -1637,6 +1651,12 @@ void show_ref_array_item(struct ref_array_item *info, 
const char *format, int qu
putchar('\n');
 }
 
+/* Wrapper: Show ref_array_item w/o referencing container in function name */
+void show_ref_item(struct ref_array_item *ref_item, const char *format, int 
quote_style)
+{
+   show_ref_array_item(ref_item, format, quote_style);
+}
+
 /*  If no sorting option is given, use refname to sort as default */
 struct ref_sorting *ref_default_sorting(void)
 {
diff --git a/ref-filter.h b/ref-filter.h
index 14d435e..0f0ffe9 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -107,4 +107,14 @@ struct ref_sorting *ref_default_sorting(void);
 /*  Function to parse --merged and --no-merged options */
 int parse_opt_merge_filter(const struct option *opt, const char *arg, int 
unset);
 
+/*
+ * Wrappers exposing the ref_array_item data structure independently
+ * of the container ref_array, e.g. to format-print individual refs.
+ */
+struct ref_array_item *new_ref_item(const char *refname,
+   const unsigned char *objectname, int flag);
+void show_ref_item(struct ref_array_item *ref_item, const char *format,
+   int quote_style);
+void free_ref_item(struct ref_array_item *ref_item);
+
 #endif /*  REF_FILTER_H  */
-- 
2.10.0

[RFC/PATCH 0/6] Add --format to tag verification

[santiago]

From: Santiago Torres <santi...@nyu.edu>

Hello everyone,

This is a followup on [1]. There we discussed what would be the best way
to provide automated scripts with mechanisms to inspect the contents of
a tag upon verification.

We struggled a little bit with how to make this fit the current git
codebase in the best way. Specifically, we are not sure if adding the
GPG_QUIET flags and/or exposing the primitives to allocate individual
git_ref_item's would be the best way forward.

This applies on the current HEAD as well as v 2.9.10.

Thanks!
-Santiago.

P.S. Gmane seems to be broken for git after it was rebooted. Should we ping
them about it?

[1] http://lists-archives.com/git/869122-verify-tag-add-check-name-flag.html

Lukas P (4):
  gpg-interface: add GPG_VERIFY_QUIET flag
  ref-filter: Expose wrappers for ref_item functions
  tag: add format specifier to gpg_verify_tag
  builtin/tag: add --format argument for tag -v

Santiago Torres (2):
  builtin/tag: move format specifier to global var
  builtin/verify-tag: Add --format to verify-tag

 builtin/tag.c| 33 +
 builtin/verify-tag.c | 17 ++---
 gpg-interface.c  |  3 +++
 gpg-interface.h  |  1 +
 ref-filter.c | 20 
 ref-filter.h | 10 ++
 tag.c| 14 --
 tag.h|  4 ++--
 8 files changed, 83 insertions(+), 19 deletions(-)

-- 
2.10.0

Re: Bug

[Santiago Torres]

Hi, Michael.

It would be helpful to get more context on what triggered this bug. I'm
not a 'core' dev, so there may be a better way to send this. In general,
you want to state the following:

0) Information about your git installation, host system, etc.
1) Information about your repo (was it GitHub? local? self-hosted?)
2) What did you do? (git push origin master? git push?)
3) What happened instead of working? (the error message would be
   helpful.

Hope this helps.

Cheers!
-Santiago.

On Tue, Sep 13, 2016 at 01:18:52PM -0400, Mike Hawes wrote:
> To whom this may concern,
> 
> I found a bug in git while trying to push my website.
> 
> I redid the process and it happened again.
> 
> I also tried it on another computer and it happened again.
> 
> I was wondering how to claim a bug?
> 
> Thank you,
> 
> 
> Michael Hawes