Re: RFS: ruby-mail-gpg 0.2.6-1

[ge...@riseup.net]

On 16-05-16 15:38:31, Jérémy Bobbio wrote:
> Please do not generate the keys on the fly. Generating keys takes time
> and can be flacky if entropy is low. I suggest having them stored in
> ASCII-armored form and importing them in a freshly created GNUPGHOME
> before running the tests.

Alright.

> You can totally avoid the patches by populating the new GNUPGHOME with
> the right configuration files as part of the same process. Keeping
> everything in the same place is probably going to be easier to
> maintain.

So...if I should store them in ASCII-armored form, and shouldn't use
patches for this, where should I store these? If quilt is a no-go, the
only place left is 'debian/', I guess? Should I use 'debian/tests/gpg'
for this? 


signature.asc
Description: Digital signature

Re: RFS: ruby-mail-gpg 0.2.6-1

[Jérémy Bobbio]

ge...@riseup.net:
> On 16-05-16 12:32:48, ge...@riseup.net wrote:
> > On 16-05-16 12:04:38, Jérémy Bobbio wrote:
> > > Both the hardcoded path and the binary files have the same root cause:
> > > this should all be generated on the fly as part of the test setup
> > > process. I really don't think it's a good thing to have GnuPG binary
> > > files shipped as part of the source.
> > > 
> > > Storing armored versions of the keys and importing them in a fresh
> > > temporary directory is a more robust approach, IMHO…  This is how I did
> > > it for Schleuder some time ago. The “around” block creates a new
> > > temporary directory, sets the right environment variable and import keys
> > > before running the tests. See:
> > > https://sources.debian.net/src/schleuder/2.2.1-2%2Bdeb7u1/spec/schleuder/crypt_spec.rb/?hl=28:45#L28
> > > 
> > > If that feels like too much changes, you could at least copy upstream
> > > binary files in a temporary and use that as the GNUPGHOME. Otherwise,
> > > there's a risk that testing twice in a row will give surprising results.
> > 
> > I could generate the keys on the fly sure...but, not sure if I'm missing
> > something here: Right now the quilt patch which injects gpg.conf and
> > gpg-agent.conf into the upstream provided gpghome inside the build dir.
> > Even if I would generate the keys myself: How would I get these two
> > configs into a temp dir, without the need to specify the path the configs
> > should be copied from?
> 
> ...or, if going the "generate all the stuff on the fly route": Should I
> create gpg.conf and gpg-agent.conf as well, and not use a patch for
> these?

Please do not generate the keys on the fly. Generating keys takes time
and can be flacky if entropy is low. I suggest having them stored in
ASCII-armored form and importing them in a freshly created GNUPGHOME
before running the tests.

You can totally avoid the patches by populating the new GNUPGHOME with
the right configuration files as part of the same process. Keeping
everything in the same place is probably going to be easier to maintain.

-- 
Lunar.''`. 
lu...@debian.org: :Ⓐ  :  # apt-get install anarchism
`. `'` 
  `-   


signature.asc
Description: Digital signature

Re: RFS: ruby-mail-gpg 0.2.6-1

[ge...@riseup.net]

On 16-05-16 12:32:48, ge...@riseup.net wrote:
> On 16-05-16 12:04:38, Jérémy Bobbio wrote:
> > Both the hardcoded path and the binary files have the same root cause:
> > this should all be generated on the fly as part of the test setup
> > process. I really don't think it's a good thing to have GnuPG binary
> > files shipped as part of the source.
> > 
> > Storing armored versions of the keys and importing them in a fresh
> > temporary directory is a more robust approach, IMHO…  This is how I did
> > it for Schleuder some time ago. The “around” block creates a new
> > temporary directory, sets the right environment variable and import keys
> > before running the tests. See:
> > https://sources.debian.net/src/schleuder/2.2.1-2%2Bdeb7u1/spec/schleuder/crypt_spec.rb/?hl=28:45#L28
> > 
> > If that feels like too much changes, you could at least copy upstream
> > binary files in a temporary and use that as the GNUPGHOME. Otherwise,
> > there's a risk that testing twice in a row will give surprising results.
> 
> I could generate the keys on the fly sure...but, not sure if I'm missing
> something here: Right now the quilt patch which injects gpg.conf and
> gpg-agent.conf into the upstream provided gpghome inside the build dir.
> Even if I would generate the keys myself: How would I get these two
> configs into a temp dir, without the need to specify the path the configs
> should be copied from?

...or, if going the "generate all the stuff on the fly route": Should I
create gpg.conf and gpg-agent.conf as well, and not use a patch for
these?


signature.asc
Description: Digital signature

Re: RFS: ruby-mail-gpg 0.2.6-1

[ge...@riseup.net]

On 16-05-16 12:04:38, Jérémy Bobbio wrote:
> ge...@riseup.net:
> > - I've made some progress: […]
> 
> Great! :)

Thanks for your input!

> I'm surprised you need to both launch and reload gpg-agent. I think it
> would also be much better to kill the agent once the tests have been
> run.

I guess just reloading is fine, gpg-agent will be launched if not
running already. Also, I'll kill the gpg-agent after the tests ran. I'll
fix this.

> > - AFAICT: Two last major showstoppers left, I need support with
> >   these, because I can't find a way on my own to solve these:
> > 
> >   - * One of the tests [1] fails. Running this gives:
> > Failure:  exception expected but none was 
> > thrown.
> > 
> > I guess the output of gpg changed during the transition from version
> > 1.x to 2.x - but I didn't find a way how to overcome this.
> > Still I'm unsure, because there is "no exception thrown".
> > 
> > Should I remove this part? Could someone of you have a look into
> > this?
> 
> After fiddling around, I've managed to understand what's happening: the
> agent is caching the passphrase. So it doesn't ask gpg to enter a new
> one, and so the wrong passphrase is never given to gpg. So I guess the
> agent needs to be told to forget the passphrase before this particular
> test can be run properly.

I see, I'll check if this will let the test pass.

> Both the hardcoded path and the binary files have the same root cause:
> this should all be generated on the fly as part of the test setup
> process. I really don't think it's a good thing to have GnuPG binary
> files shipped as part of the source.
> 
> Storing armored versions of the keys and importing them in a fresh
> temporary directory is a more robust approach, IMHO…  This is how I did
> it for Schleuder some time ago. The “around” block creates a new
> temporary directory, sets the right environment variable and import keys
> before running the tests. See:
> https://sources.debian.net/src/schleuder/2.2.1-2%2Bdeb7u1/spec/schleuder/crypt_spec.rb/?hl=28:45#L28
> 
> If that feels like too much changes, you could at least copy upstream
> binary files in a temporary and use that as the GNUPGHOME. Otherwise,
> there's a risk that testing twice in a row will give surprising results.

I could generate the keys on the fly sure...but, not sure if I'm missing
something here: Right now the quilt patch which injects gpg.conf and
gpg-agent.conf into the upstream provided gpghome inside the build dir.
Even if I would generate the keys myself: How would I get these two
configs into a temp dir, without the need to specify the path the configs
should be copied from?

Thanks,
Georg


signature.asc
Description: Digital signature

Re: RFS: ruby-mail-gpg 0.2.6-1

[Jérémy Bobbio]

ge...@riseup.net:
> On 16-05-11 09:53:06, Jérémy Bobbio wrote:
> > For this reason, I think you should find a way to restore
> > `debian/ruby-tests.rake` and make it work with gpg2 as gpg1 is likely
> > to go away in some near future…
> 
> - I've made some progress: […]

Great! :)

I'm surprised you need to both launch and reload gpg-agent. I think it
would also be much better to kill the agent once the tests have been
run.

> - AFAICT: Two last major showstoppers left, I need support with
>   these, because I can't find a way on my own to solve these:
> 
>   - * One of the tests [1] fails. Running this gives:
> Failure:  exception expected but none was 
> thrown.
> 
> I guess the output of gpg changed during the transition from version
> 1.x to 2.x - but I didn't find a way how to overcome this.
> Still I'm unsure, because there is "no exception thrown".
> 
> Should I remove this part? Could someone of you have a look into
> this?

After fiddling around, I've managed to understand what's happening: the
agent is caching the passphrase. So it doesn't ask gpg to enter a new
one, and so the wrong passphrase is never given to gpg. So I guess the
agent needs to be told to forget the passphrase before this particular
test can be run properly.

>   - In 'debian/ruby-tests.rake', I've to set two env vars to a path
> pointing at the upstream provided gpghome (this is: the upstream
> provided gpghome inside the build dir) and reload gpg-agent
> afterwards, so it picks up these.
> 
> I've added debian/gbp.conf and set the export-dir in there to
> '../build-area/'.
> 
> Right now, the path in 'debian/ruby-tests.rake' is hardcoded, aka
> "ruby-mail-gpg-0.2.6", which will fail obviously in the future.
>
> So...I'm searching for a way to get the Debian package version of
> the build.
> How should I deal with this? I've had a look into the env vars which
> are exported if using gbp buildpackage, but didn't found one which
> would help in this case (as long as I read the manpage correctly).
> 
> - Minor question:
> 
>   - In the upstream provided gpghome, there are two (binary) files,
> which, in my opinion, don't belong there: 'random_seed' and
> 'pubring.gpg~'. Using quilt gives, after adding the files to the
> patch, removing the files from the filesystem, and refreshing the
> patch, "diff failed". Doesn't this work for binary files? Or is it
> because one isn't supposed to remove the files from the fs? Again, I
> couldn't find something about this on the Internets..

Both the hardcoded path and the binary files have the same root cause:
this should all be generated on the fly as part of the test setup
process. I really don't think it's a good thing to have GnuPG binary
files shipped as part of the source.

Storing armored versions of the keys and importing them in a fresh
temporary directory is a more robust approach, IMHO…  This is how I did
it for Schleuder some time ago. The “around” block creates a new
temporary directory, sets the right environment variable and import keys
before running the tests. See:
https://sources.debian.net/src/schleuder/2.2.1-2%2Bdeb7u1/spec/schleuder/crypt_spec.rb/?hl=28:45#L28

If that feels like too much changes, you could at least copy upstream
binary files in a temporary and use that as the GNUPGHOME. Otherwise,
there's a risk that testing twice in a row will give surprising results.

Hope that helps,
-- 
Jérémy Bobbio.''`.
jeremy.bob...@irq7.fr   : :   : lu...@debian.org
`. `'`  lu...@torproject.org
  `-


signature.asc
Description: Digital signature

Re: RFS: ruby-mail-gpg 0.2.6-1

[ge...@riseup.net]

On 16-05-11 09:53:06, Jérémy Bobbio wrote:
> For this reason, I think you should find a way to restore
> `debian/ruby-tests.rake` and make it work with gpg2 as gpg1 is likely
> to go away in some near future…

- I've made some progress:

  - I've added a patch which injects gpg.conf and gpg-agent.conf
into the upstream provided gpghome. I need this to prevent pinentry
from spawning.

  - I've re-enabled i'debian/ruby-tests.rake'.

  - I've added 'ruby-shoulda-context' and 'ruby-actionmailer' as
Build-Depends:.

  - All of this makes the upstream tests pass* in a non-interactive way.

  - 'pry-nav' isn't needed for the tests, hence I'll close the ITPs I've
opened.

- AFAICT: Two last major showstoppers left, I need support with
  these, because I can't find a way on my own to solve these:

  - * One of the tests [1] fails. Running this gives:
Failure:  exception expected but none was 
thrown.

I guess the output of gpg changed during the transition from version
1.x to 2.x - but I didn't find a way how to overcome this.
Still I'm unsure, because there is "no exception thrown".

Should I remove this part? Could someone of you have a look into
this?

  - In 'debian/ruby-tests.rake', I've to set two env vars to a path
pointing at the upstream provided gpghome (this is: the upstream
provided gpghome inside the build dir) and reload gpg-agent
afterwards, so it picks up these.

I've added debian/gbp.conf and set the export-dir in there to
'../build-area/'.

Right now, the path in 'debian/ruby-tests.rake' is hardcoded, aka
"ruby-mail-gpg-0.2.6", which will fail obviously in the future.

So...I'm searching for a way to get the Debian package version of
the build.
How should I deal with this? I've had a look into the env vars which
are exported if using gbp buildpackage, but didn't found one which
would help in this case (as long as I read the manpage correctly).

- Minor question:

  - In the upstream provided gpghome, there are two (binary) files,
which, in my opinion, don't belong there: 'random_seed' and
'pubring.gpg~'. Using quilt gives, after adding the files to the
patch, removing the files from the filesystem, and refreshing the
patch, "diff failed". Doesn't this work for binary files? Or is it
because one isn't supposed to remove the files from the fs? Again, I
couldn't find something about this on the Internets..

Input highly appreciated,
thanks,
Georg


[1] 
https://anonscm.debian.org/cgit/pkg-ruby-extras/ruby-mail-gpg.git/tree/test/message_test.rb#n198


signature.asc
Description: Digital signature

Re: RFS: ruby-mail-gpg 0.2.6-1

[ge...@riseup.net]

On 16-05-13 13:54:25, Antonio Terceiro wrote:
> On Thu, May 12, 2016 at 08:16:50PM +0200, ge...@riseup.net wrote:
> > - Because I'll re-enable the tests, I had a look at the dependencies
> >   (again): mail-gpg depends on pry-nav [1], which depends on pry-remote 
> >   [2], both aren't packaged yet. So I'll do this as well..
> 
> I replied to the ITPs, but saying it here again: pry is a _development_
> tool. Requiring it at runtime, or even for the build, does not make
> sense. If the upstream code hardcodes that in a way that the build fails
> without it, I would rather patch it out than introduce these packages,
> because even if they are _loaded_ during the build, there is no point
> having a interactive console available.

I saw and read your replies already, thanks for this.

Actually, I've opened these ITPs because I _thought_, if gems are
specified as "development dependencies" in *.gemspec, which 'prv-nav'
is, that the src package should then depend on these. 

zeha said on IRC, I should check if the tests will run without these,
which I will do.

(In general, I've got more and more the feeling, that this task is maybe
"too big for me", because of my lack of knowledge regarding (Ruby)
Debian packaging. It feels more like "trial and error", instead of how I
normally work. Maybe this is because I'm just starting out, and it will
get better with me doing more stuff and time. Nearly each time I want to
do $foo, I'm asking myself "how to do this, where to find information
about this.)

Thanks for your input,
Georg


signature.asc
Description: Digital signature

Re: RFS: ruby-mail-gpg 0.2.6-1

[Antonio Terceiro]

On Thu, May 12, 2016 at 08:16:50PM +0200, ge...@riseup.net wrote:
> - Because I'll re-enable the tests, I had a look at the dependencies
>   (again): mail-gpg depends on pry-nav [1], which depends on pry-remote 
>   [2], both aren't packaged yet. So I'll do this as well..

I replied to the ITPs, but saying it here again: pry is a _development_
tool. Requiring it at runtime, or even for the build, does not make
sense. If the upstream code hardcodes that in a way that the build fails
without it, I would rather patch it out than introduce these packages,
because even if they are _loaded_ during the build, there is no point
having a interactive console available.


signature.asc
Description: PGP signature

Re: RFS: ruby-mail-gpg 0.2.6-1

[Christian Hofstaedtler]

* ge...@riseup.net  [160512 20:17]:
> - However, the options I've used assume gnupg-agent >= 2.1~, whereas in
>   jessie the current version is 2.0.26-6, which means that these won't
>   work in jessie. As this package won't get into jessie anyway: Is this
>   ok?

Would matter only if you intend to backport this to jessie. If you
don't, then it does not matter.

  C.

-- 
 ,''`.  Christian Hofstaedtler 
: :' :  Debian Developer
`. `'   7D1A CFFA D9E0 806C 9C4C  D392 5C13 D6DB 9305 2E03
  `-



signature.asc
Description: PGP signature

Re: RFS: ruby-mail-gpg 0.2.6-1

[Jérémy Bobbio]

ge...@riseup.net:
> On 16-05-11 09:53:06, Jérémy Bobbio wrote:
> > The only mistake I could find by looking at the source is in
> > `debian/copyright`:
> > 
> > “MIT” is an ambiguous term for the licence, see
> > .
> > The term most often refers to the Expat license:
> > 
> 
> Thanks, wasn't aware of this.
> 
> > Could you check and specify “Expat” if it's the case?
> 
> Well, actually I've just took the license the upstream author provided
> [1]. Should I clarify this with him?

As long as the full license text is there, it's correct. It's just that
many GitHub users refer to the Expat license as MIT, further nurtering
the ambiguity—or making the X11 license a corner case… depends on how
you view it. :)

Anyway, if upsteam license matches Expat, please use Expat in the
copyright file, as documented in the spec.

> > Have you tried to run autopkgtest for the package? I know it's one
> > more thing to setup, but it's now fairly easy and you won't regret it.
> > Uploading a package that breaks stuff is not a fun experience.
> 
> I didn't set up autopkgtest, because I disabled the tests.

There are two kinds of tests: build time tests and “runtime tests”
(autopkgtest). As `debian/control` still mentions autopkgtest, I got
confused.

I think both are indeed disabled for now. It might be worthwhile to
write extra autopkgtests on top of the standard Ruby ones so you test
integration with external modules better. But it's up to you.

And glad you got some tips from dkg! :)

-- 
Lunar.''`. 
lu...@debian.org: :Ⓐ  :  # apt-get install anarchism
`. `'` 
  `-   


signature.asc
Description: Digital signature

Re: RFS: ruby-mail-gpg 0.2.6-1

[ge...@riseup.net]

On 16-05-11 11:28:03, Christian Hofstaedtler wrote:
> Georg and me discussed this a bit on IRC [...]

While chatting, zeha recommended to speak with dkg about this, which I
just did. He gave me some pointers. I'm now trying to implement these
and will send a follow-up.

All the best,
Georg


signature.asc
Description: Digital signature

Re: RFS: ruby-mail-gpg 0.2.6-1

[ge...@riseup.net]

On 16-05-11 09:53:06, Jérémy Bobbio wrote:
> The only mistake I could find by looking at the source is in
> `debian/copyright`:
> 
> “MIT” is an ambiguous term for the licence, see
> .
> The term most often refers to the Expat license:
> 

Thanks, wasn't aware of this.

> Could you check and specify “Expat” if it's the case?

Well, actually I've just took the license the upstream author provided
[1]. Should I clarify this with him?

> Have you tried to run autopkgtest for the package? I know it's one
> more thing to setup, but it's now fairly easy and you won't regret it.
> Uploading a package that breaks stuff is not a fun experience.

I didn't set up autopkgtest, because I disabled the tests. This decision
was not an easy one, because it felt quite 'hakish'; I've spent something 
like four hours to try to find a way to make this work. I ran the tests
manually, in an environment with only gpg1 installed, and all passed. 

In the end, I've decided to disable test test _for now_, and planned 
to re-enable these "soon". But yes, maybe you're right, to fix these
first, and upload afterwards.

Thanks for your input Lunar!
Cheers,
Georg


[1] 
https://anonscm.debian.org/cgit/pkg-ruby-extras/ruby-mail-gpg.git/tree/LICENSE.txt?h=upstream#n3


signature.asc
Description: Digital signature

Re: RFS: ruby-mail-gpg 0.2.6-1

[Christian Hofstaedtler]

* Jérémy Bobbio  [160511 09:55]:
> For this reason, I think you should find a way to
> restore `debian/ruby-tests.rake` and make it work with gpg2 as gpg1 is
> likely to go away in some near future… Maybe this needs a fix in
> ruby-gpgme? Seems it's an old and unsolved issue:
> https://github.com/ueno/ruby-gpgme/issues/31

It needs a fix in gpg, see below.
In fact, ruby-gpgme as packaged in Debian leaves you no option to
use gpg1 today (in this context).

> Implementing only the required part of the agent protocol should be
> simple enough. It's documentation is available at:
> https://www.gnupg.org/documentation/manuals/gnupg/Agent-Protocol.html
> Then setting the `GPG_AGENT_INFO` variable to point at the custom agent
> should do the trick to feed gpg with the right passphrases.

Georg and me discussed this a bit on IRC; here are some thoughts
from me on that:

This is clearly a change in the functionality on the gpg side that
prevents automated use of gpg keys with passphrases.  (Someone
suggested this is an intentional change.)

Spawning an external program to output the passphrase to *work
around* that change in gpg is clearly not the way forward. If the
gpg authors believe this is a feature that should not be supported,
then I think we'll have to live with that.

Or talk to the gpg maintainers or gpg upstream if that's going to
change ...

(I'd see a tiny bit of value of removing passphrases from the test
keys, but I'm not sure that actually helps with the tests.)

-- 
 ,''`.  Christian Hofstaedtler 
: :' :  Debian Developer
`. `'   7D1A CFFA D9E0 806C 9C4C  D392 5C13 D6DB 9305 2E03
  `-



signature.asc
Description: PGP signature

Re: RFS: ruby-mail-gpg 0.2.6-1

[Jérémy Bobbio]

ge...@riseup.net:
> The following package is ready to be reviewed / uploaded:
> 
>   ruby-mail-gpg 0.2.6-1
> 
> As this is my first package, I would be happy to get some pointers in
> case there is anything not in shape.

Thanks for your work.

The only mistake I could find by looking at the source is in
`debian/copyright`:

“MIT” is an ambiguous term for the licence, see
.
The term most often refers to the Expat license:

Could you check and specify “Expat” if it's the case?

Have you tried to run autopkgtest for the package?  I know it's one more
thing to setup, but it's now fairly easy and you won't regret it.
Uploading a package that breaks stuff is not a fun experience.

For this reason, I think you should find a way to
restore `debian/ruby-tests.rake` and make it work with gpg2 as gpg1 is
likely to go away in some near future… Maybe this needs a fix in
ruby-gpgme? Seems it's an old and unsolved issue:
https://github.com/ueno/ruby-gpgme/issues/31

Implementing only the required part of the agent protocol should be
simple enough. It's documentation is available at:
https://www.gnupg.org/documentation/manuals/gnupg/Agent-Protocol.html
Then setting the `GPG_AGENT_INFO` variable to point at the custom agent
should do the trick to feed gpg with the right passphrases.

-- 
Lunar.''`. 
lu...@debian.org: :Ⓐ  :  # apt-get install anarchism
`. `'` 
  `-   


signature.asc
Description: Digital signature