signing commits using gpg2

[shawn wilson]

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 works with gpg and ssh:
 % touch foo

 ~ localhost
 % gpg2 --sign foo

 ~ localhost
gpg: using "846FF490" as default secret key for signing
 % cat foo*

 ~ localhost
-BEGIN PGP MESSAGE-
Version: GnuPG v2

owEBuQFG/pANAwAKAYwdY7SEb/SQAcsJYgNmb29ZqxfviQGcBAABCgAGBQJZqxfv
AAoJEIwdY7SEb/SQAcEL/jonw+HymnlmfebtEwlvfx2Gl1Sbuw0xWWPpQ2Dtjljz
HtpD+LWczjpOSMTHFNK9xPR2kcs1WNY+mO8M45QI7iDgFkKRzaxEqeNUJkoyF/+I
81VMmXDQMXFs4+8jy00b+UxTdvwdXaHMsOtu+6YCtmCR5Bzohg07ADsnXnGGn3Sd
WTjVMzV6Dlh8LRF+coGJ8JuErBsRAI6vdNgJRVHYBULGNXci4uF/4a+58uiTL4/U
PvC4ruXCNxCKi89nMERhwlnOvglseX3TDR5ldrc4Hzb+pLsj/l6N4sBW0Zmb8UcE
9BG3WjOs4eZvnLmk5XHrwisD2CXuHvyWMl0yH7LTrg+m4Itj0PJ4Px4H9E5t/zfs
C1vcB/okcigeIyXnO06um02a5oZAYOKadB+6NRnBjULz5GvP2yxj/AO1VPmZprpt
budMuHZcA0zNE3uBmcnQY5+1tdkyTrlTxsL58lQrn/U3wvgah3AXMEvjRGqbYWHj
jDikQVJ7ESoevNqlfLPj8Q==
=hV6v
-END PGP MESSAGE-

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 "
gpg: public key decryption failed: general error
gpg: decryption failed: secret key not available
 % gpg2 -d foo.gpg

 ~ localhost
gpg: encrypted with 3072-bit RSA key, ID E27FA0B841826CFB, created 2017-03-13
  "Shawn Wilson "
foo

(yeah I added data to the file)

And just to prove basic competency checking:

 % git config --global -l | grep sign

 ~ localhost
user.signingkey=846FF490
filter.gitconfig-rmuser.clean=sed -e "s/^\( *email =\).*/\1 /" -e "s/^\( *name =\).*/\1 /" -e "s/^\(
*signingkey =\).*/\1 /"
filter.gitconfig-rmuser.smudge=egrep "^ *(email|name|signingkey) = "
commit.gpgsign=true

Re: [PATCH] config: use a static lock_file struct

[Jeff King]

On Sat, Sep 02, 2017 at 04:44:51AM -0400, Jeff King wrote:

> There are actually very few callers of create_tempfile (I count two).
> Everybody just uses lock_file(). So I think we could get away with just
> modifying the internals of the lock struct to hold a pointer, and then
> teaching commit_lock_file() et al to reset it to NULL after performing
> an operation that frees the tempfile. We could even modify
> rename_tempfile() and friends to take a pointer-to-pointer and NULL it
> itself. That would make it harder to get wrong.

I take it back.  It's true that there are very few create_tempfile()
callers, but we'd want to have a similar change for mks_tempfile_*, and
there are a lot more of those.

One solution would be to add an extra level of indirection, like:

  struct tempfile {
struct the_part_that_we_put_on_our_global_list *data;
  };

That lets us keep normal value semantics for "struct tempfile", which is
nice. And existing callers wouldn't have to be modified at all for the
creation/deletion bits. But I suspect it would also result in a lot of
replacements like s/temp->/temp.data->/ for any callers that look at the
underlying fields that have moved into the sub-struct.

-Peff

This is DAVID. am writing to inform you that am back from hoildays

[davidjohnofficef...@gmail.com]

Good Day

This is David john. am writing to inform you that am back from holidays, and 
regarding the last meeting it came out good, so the bank is read to transfer 
the fund into your account.I am only waiting for your directions

Best Regard
David john

I NEED YOUR HELP

[Cotte]

Greetings,

I am Miss elizabeth 22 years old i have every important things to discus with 
you. Please respond with your most secure email address to enable me give you 
the brief details of proposer, please write me on this email 
elizabethcot...@barid.com
Looking forward for your positive response.

Best Regards

I am looking forward to your prompt reply.
Elizabeth Cotte.

[no subject]

[Mr.Sheng Li Hung]

Hello  Dear

I am Mr.Sheng Li Hung, from china I got your information while search for
a reliable person, I have a very profitable business proposition for you
and i can assure you that you will not regret been part of this mutual
beneficial transaction after completion. Kindly get back to me for more
details on this email id: shengl...@outlook.com

Thanks
Sheng Li Hung

PLEASE READ

[Mrs Nora]

Hello,

I am Mrs Nora lin wong, I have an urgent and very confidential business 
proposition for you.

Regards,
Yours faithfully,
Mrs Nora lin wong.

  

Re: Bug report

[Jeff King]

On Wed, Aug 30, 2017 at 11:25:00PM +0200, Aleksandar Pavic wrote:

>  I have a file
> 
>  app/Controller/CustomerCardVerificationController.php
> 
> And when I take a look at changes log, I get this (no matter which tool I
> use):
> 
> 2017-07-31 19:41 dule o membership renew payment email
> 2017-06-07 08:59 Dusan Tatic  o cc refund clean
> 2017-04-15 00:16 Miodrag Dragić   o refound admin payment
> 2017-03-20 12:02 Dusan Tatic  o CardVerification card connect
> 2017-03-16 15:59 Aleksandar Pavic o paypal
> 2017-03-10 13:34 Aleksandar Pavic o Production branch
> 2017-03-10 13:01 Aleksandar Pavic I Migrating dev
>
> However if I manually browse thru revisions and open revision from
> 03/27/2017 07:05 PM
> 
> I can see the change in that file which is unlisted above, at revision
> ff9f4946e109bd234d438e4db1d319b1f6cb6580

How are you invoking the log? Are you doing:

  git log app/Controller/CustomerCardVerificationController.php

or similar? If that is the case, then history simplification may be
causing the results you see. And even you don't _see_ any merges in the
output, that is because they were simplified away. And the commit you
are looking for may have been on a side branch that was simplified away.

If you do:

  git log --full-history app/...

does the commit you are interested in show up? If so, then it was
removed due to history simplification. And if you are surprised that a
side branch was simplified away, that is most likely because there is a
mis-merge in your history (some merge which threw away the changes on a
side branch).

Try:

  git log --graph --oneline --name-status --full-history app/...

to see the whole shape of history, including which commits touched the
file.

You can read more about it in the "History Simplification" section of
"git help log".

-Peff

Re: [PATCH] config: use a static lock_file struct

[Jeff King]

On Wed, Aug 30, 2017 at 09:07:53AM +0200, Michael Haggerty wrote:

> > So it's probably safer to just let tempfile.c handle the whole lifetime,
> > and have it put all live tempfiles on the heap, and free them when
> > they're deactivated. That means our creation signature becomes more
> > like:
> > 
> >   struct tempfile *create_tempfile(const char *path);
> > 
> > and delete_tempfile() actually calls free() on it (though we'd probably
> > want to skip the free() from a signal handler for the usual reasons).
> 
> I agree that the latter would be a nice, and relatively safe, design. It
> would involve some fairly intrusive changes to client code, though.
>
> I think it would be possible to implement the new API while leaving the
> old one intact, to avoid having to rewrite all clients at once, and
> potentially to allow clients to avoid a malloc if they already have a
> convenient place to embed a `struct tempfile` (except that now they'd be
> able to free it when done). For example, `create_tempfile(tempfile,
> path)` and its friends could accept NULL as the first argument, in which
> case it would malloc a `struct tempfile` itself, and mark it as being
> owned by the tempfile module. Such objects would be freed when
> deactivated. But if the caller passes in a non-NULL `tempfile` argument,
> the old behavior would be retained.

There are actually very few callers of create_tempfile (I count two).
Everybody just uses lock_file(). So I think we could get away with just
modifying the internals of the lock struct to hold a pointer, and then
teaching commit_lock_file() et al to reset it to NULL after performing
an operation that frees the tempfile. We could even modify
rename_tempfile() and friends to take a pointer-to-pointer and NULL it
itself. That would make it harder to get wrong.

In the long term I don't think there's a huge value to being able to
place a "struct tempfile" inside another struct, as opposed to holding a
pointer. It saves a malloc, but at the cost of opening up confusion
about lifetimes.

-Peff

Re: [PATCH] hashmap: add API to disable item counting when threaded

[Simon Ruderich]

On Wed, Aug 30, 2017 at 06:59:22PM +, Jeff Hostetler wrote:
> [snip]
> +/*
> + * Re-enable item couting when adding/removing items.
> + * If counting is currently disabled, it will force count them.

The code always recounts them. Either the comment or the
code should be adjusted.

> + * This might be used after leaving a threaded section.
> + */
> +static inline void hashmap_enable_item_counting(struct hashmap *map)
> +{
> + void *item;
> + unsigned int n = 0;
> + struct hashmap_iter iter;
> +
> + hashmap_iter_init(map, );
> + while ((item = hashmap_iter_next()))
> + n++;
> +
> + map->do_count_items = 1;
> + map->private_size = n;
> +}

Regards
Simon
-- 
+ privacy is necessary
+ using gnupg http://gnupg.org
+ public key id: 0x92FEFDB7E44C32F9

Re: [Bug] commit-tree shouldn't append an extra newline to commit messages

[Jeff King]

On Fri, Sep 01, 2017 at 02:58:52PM -0400, Ross Kabus wrote:

> When doing git commit-tree to manually create a commit object, it can be seen
> that the resulting commit's message has an extra appended newline (\n) that
> was not present in the input for either argument -m or -F. This is both
> undesirable and inconsistent with the git commit porcelain command.

Hmm. As a plumbing command, I'd expect commit-tree to generally _not_
perform such niceties. And definitely it does not when taking the
message in via stdin. In Git's original design, commit object bodies do
not even have to be text, though certainly the porcelain tools all
assume they are.

But I am confused by your "inconsistent with git commit porcelain"
comment. The porcelain git-commit definitely _does_ add a newline if one
isn't present (and in fact runs the whole thing through git-stripspace
to clean up whitespace oddities).

So I don't think "inconsistent with git-commit" is a compelling
argument, unless I'm missing something.

I _could_ see an argument for "commit-tree as plumbing should always
treat the message verbatim". But since "-F" and "-m" have done the
newline-completion since their inception, I'm not sure it's a good idea
to change them now. The current behavior also makes some sense, as it's
consistent with the matching options in git-commit (again, as far as I
can see; if you have a counter-example it would be interesting to see).

-Peff

[no subject]

[Mr.Sheng Li Hung]

Hello  Dear

I am Mr.Sheng Li Hung, from china I got your information while search for
a reliable person, I have a very profitable business proposition for you
and i can assure you that you will not regret been part of this mutual
beneficial transaction after completion. Kindly get back to me for more
details on this email id: shengl...@outlook.com

Thanks
Sheng Li Hung

Re: [PATCH] hashmap: add API to disable item counting when threaded

[Jeff King]

On Sat, Sep 02, 2017 at 01:31:19AM +0200, Johannes Schindelin wrote:

> > https://public-inbox.org/git/adb37b70139fd1e2bac18bfd22c8b96683ae18eb.1502780344.git.martin.ag...@gmail.com/
> [...]
> > Add API to hashmap to disable item counting and to disable automatic
> > rehashing.  Also include APIs to re-enable item counting and automatica
> > rehashing.
> [...]
> 
> The Git contribution process forces me to point out lines longer than 80
> columns. I wish there was already an automated tool to fix that, but we
> (as in "the core Git developers") have not yet managed to agree on one. So
> I'll have to ask you to identify and fix them manually.

Perhaps it would be helpful if you pointed out the lines that are too
long. Because I don't see any being added by the patch. There are two in
the commit message. One is a URL. For the other, which is 82 characters,
I'm not sure there is a better tool than "turn on text wrapping in your
editor".

> > +   /* TODO Consider counting them and returning that. */
> 
> I'd rather not. If counting is disabled, it is disabled.
> 
> > +   die("hashmap_get_size: size not set");
> 
> Before anybody can ask for this message to be wrapped in _(...) to be
> translateable, let me suggest instead to add the prefix "BUG: ".

Agreed on both (and Jonathan's suggestion to just use BUG()).

> > +static inline void hashmap_enable_item_counting(struct hashmap *map)
> > +{
> > +   void *item;
> > +   unsigned int n = 0;
> > +   struct hashmap_iter iter;
> > +
> > +   hashmap_iter_init(map, );
> > +   while ((item = hashmap_iter_next()))
> > +   n++;
> > +
> > +   map->do_count_items = 1;
> > +   map->private_size = n;
> > +}
> 
> BTW this made me think that we may have a problem in our code since
> switching from my original hashmap implementation to the bucket one added
> in 6a364ced497 (add a hashtable implementation that supports O(1) removal,
> 2013-11-14): while it is not expected that there are many collisions, the
> "grow_at" logic still essentially assumes the number of buckets to be
> equal to the number of hashmap entries.

I'm confused about what the problem is. If I am reading the code
correctly, "size" is always the number of elements and "grow_at" is the
table size times a load factor. Those are the same numbers you'd use to
decide to grow in an open-address table.

It's true that this does not take into account the actual number of
collisions we see (or the average per bucket, or however you want to
count it). But generally nor do open-address schemes (and certainly our
other hash tables just use load factor to decide when to grow).

Am I missing something?

-Peff

Re: [PATCH] hashmap: add API to disable item counting when threaded

[Jeff King]

On Wed, Aug 30, 2017 at 06:59:22PM +, Jeff Hostetler wrote:

> From: Jeff Hostetler 
> 
> This is to address concerns raised by ThreadSanitizer on the
> mailing list about threaded unprotected R/W access to map.size with my 
> previous
> "disallow rehash" change (0607e10009ee4e37cb49b4cec8d28a9dda1656a4).  See:
> https://public-inbox.org/git/adb37b70139fd1e2bac18bfd22c8b96683ae18eb.1502780344.git.martin.ag...@gmail.com/
> 
> Add API to hashmap to disable item counting and to disable automatic 
> rehashing.  
> Also include APIs to re-enable item counting and automatica rehashing.

It may be worth summarizing the discussion at that thread here.

At first glance, it looks like this is woefully inadequate for allowing
multi-threaded access. But after digging, the issue is that we're really
trying to accommodate one specific callers which is doing its own
per-bucket locking, and which needs the internals of the hashmap to be
truly read-only.

I suspect the code might be easier to follow if that pattern were pushed
into its own threaded_hashmap that disabled the size and handled the
mod-n locking, but I don't insist on that as a blocker to this fix.

-Peff