Re: Fix for security/opensc regression

[Greg Troxel <gdt%lexort.com@localhost>]

Emmanuel Dreyfus <manu%netbsd.org@localhost> writes:

> When security/opensc was updated to 0.23.0, it gained a
> --enable-notify configure flag.
>
> The feature adds a libopensc dependency on libglib, which in turns 
> brings libpthread into the game.

interesting and I could see where this was going!

Big picture, everything needs to be threaded these days, unfortunately,
even if it is 1 thread.   But rule #1 is do not tilt at upstream
windmills (well, file a ticket to poke and then stop :-).

> The patch below turns the notify feature into a disabled by default
> option. There should probably be a way to warn the user about the 
> dire consequences of enabling it. A comment in options.mk?

Yes.  We do not really have a more friendly way.   Basically enabling
the option is at the moment broken, and is fodder for future work?  If
that's the case, perhaps don't even add it into SUGGESTED, with a
comment as to why.

> Beside this, any somment on the patch? It restores opensc-pkcs11.so
> functionnality with ssh at mine.

Sounds like the right thing.  (I'm speaking up because I used to
struggle with this world including threads at $DAYJOB because customers
tended to smartcards and pkix, and now I don't.)

> Additionally, users could be interested to learn that some drivers were
> disabled by default in OpenSC 0.23.0. The change is not pkgsrc-related
> and was done upstream. Enabling the drivers to match pre-0.23.0 behavior
> requires "card_drivers = old, internal" in opensc.conf's app default 
> section. I also have a patch that reverts this upstream's change, if 
> that is deemed useful.

I tend to say that upstream changes land in pkgsrc.  I think people with
a pkgsrc package installed should expect upstream behavior, and should
consult upstream docs to make things behave differently.  So I would say
let it be, and it would be buggy to change the defaults.

Unless they are somehow so terrible that this is a problem, in which
case there should be a ticket upstream to fix it, and enough piling on
and threats of forking if they don't.  But I bet that this not the case.


Patch is fine, other than that it should have a warning about threads in
the option code.   And if nothing useful happens if you turn it on, I
would remove it from SUPPORTED, making that line commented, with a
comment before saying that it pulls in threaded libs which blows up, and
thus is not possible to enable for now.