On (27/10/15 22:35), Lukas Slebodnik wrote:
On (27/10/15 17:57), Jakub Hrozek wrote:
>On Tue, Oct 27, 2015 at 05:42:29PM +0100, Jakub Hrozek wrote:
>> On Fri, Oct 23, 2015 at 01:06:36PM +0200, Lukas Slebodnik wrote:
>> > On (21/10/15 09:20), Sumit Bose wrote:
>> > >On Tue, Oct 20, 2015 at 10:15:06PM +0200, Jakub Hrozek wrote:
>> > >> On Tue, Oct 20, 2015 at 05:17:49PM +0300, Nikolai Kondrashov
wrote:
>> > >> > Hi Jakub,
>> > >> >
>> > >> > On 10/19/2015 09:43 PM, Jakub Hrozek wrote:
>> > >> > >I'm working on pam_sss.so tests[1] and I ran into a
problem that I don't
>> > >> > >know how to solve best.
>> > >> > >
>> > >> > >tl;dr, I would like to set different environment variables
for different
>> > >> > >tests in order to set up cwrap libraries differently
per-test.
>> > >> > >
>> > >> > >I can't use setenv() in the test itself, because
that's too late, I need
>> > >> > >the variables to be set when __attribute__(constructor) is
run, so normally
>> > >> > >at program startup, when the libraries are loaded.
>> > >> > >
>> > >> > >With cmake it's easy, use set(TEST_ENVIRONMENT). But
with autotools, I
>> > >> > >only found two ways:
>> > >> > > - TESTS_ENVIRONMENT - this is fine, but it's per
Makefile.am. So I
>> > >> > > would have to split the tests more, into
pam_wrapper tests that also
>> > >> > > require uid_wrapper, tests that only require
pam_wrapper, ...
>> > >> > > - LOG_COMPILER - this allows to run a wrapper script
before a test
>> > >> > > that receives the test name as argv. So this is
pretty much what I
>> > >> > > want except this is a feature new to automake 1.12,
which would
>> > >> > > rule out both RHEL-6 and Ubuntu Trusty (which is
used by Travis)
>> > >> > >
>> > >> > >So I'm really leaning towards creating a
src/tests/cwrap/pwrap/Makefile.am
>> > >> > >and src/tests/cwrap/pwrap_root/Makefile.am. The downside
of multiple
>> > >> > >Makefile.am files is that there is some code duplication
and the build
>> > >> > >takes longer. But I still think there is enough interest
(from us and from
>> > >> > >our users) to support git master on old platforms. I can
file a ticket to
>> > >> > >remove this and use LOG_COMPILER when we drop support for
RHEL-6 and old
>> > >> > >Ubuntu versions...
>> > >> > >
>> > >> > >If you disagree, please reply, otherwise I'm going to
send a patch with
>> > >> > >per-test Makefile...
>> > >> >
>> > >> > Ah, so these are unit tests, not integration tests?
>> > >>
>> > >> I'm working on both, actually. The first part is more or less
an
>> > >> isolated unit test of all the options that pam_sss supports. The
reason
>> > >> is that some options (2FA, smart cards, ...) are not really easily
>> > >> testable without a mock back end, at the moment we only have
openldap in
>> > >> the integration tests.
>> > >>
>> > >> The next step I will start right after I finish this part is
integration
>> > >> tests that will exercise LDAP authentication, password change and
maybe
>> > >> authorisation if there's time left.
>> > >>
>> > >> >
>> > >> > I'm not sure I understood everything right, sorry, but
perhaps you can find
>> > >> > something useful in contrib/ci/run, contrib/ci/make-check-wrap
and
>> > >> > contrib/ci/valgrind-condense where CI matches and handles
particular tests
>> > >> > differently regardless of whether LOG_COMPILER is supported or
not. Ping me
>> > >> > if you need help figuring out what's going on there.
>> > >>
>> > >> So more or less I wanted to have two tests and wanted to run the
first
>> > >> as (simplified):
>> > >> PAM_WRAPPER=1 ./src/tests/cwrap/pam_sss_wrapper-tests
>> > >> and other as:
>> > >> PAM_WRAPPER=1 UID_WRAPPER=1
./src/tests/cwrap/pam_sss_wrapper-root--tests
>> > >>
>> > >> but it occured to me that I can always start with UID wrapper, just
drop
>> > >> privileges if I need a strictly non-root test. It's a bit of a
hack :-)
>> > >> but since the root is fake anyway, I think it's acceptable.
>> > >>
>> > >> >
>> > >> > Granted this is from outside the build, but maybe you can
concoct something
>> > >> > from inside as well.
>> > >>
>> > >> I think this might work as well; thank you!
>> > >
>> > >I had a short look at libtool. Since we use it during 'make
check' not
>> > >the actual binaries are called but a libtool generated wrapper script.
>> > >If it would be possible to set the environment variables here they
would
>> > >be visible for the binary at startup.
>> > >
>> > >Libtool has the concept of 'executable wrappers' to support
cygwin and
>> > >similar environments but I didn't found an easy way to add own
wrapper
>> > >here.
>> > >
>> > >Adding the variables directly in the generated wrapper scripts would be
>> > >quite a hack. But maybe it would be possible if we add our own version
>> > >of build/ltmain.sh where the wrapper scripts are generated in
>> > >func_emit_wrapper()?
>> > >
>> > >So, I'm afraid this is not a direct answer to your question but
maybe
>> > >libtool might be useful here.
>> > >
>> >
>> > I read this thread after are came up with almost similar solution
>> > as Sumit. With a small difference. I did not decide to inject env
>> > variables to generated script but I decided to write yet another
>> > wrapper on top of and set env variable there.
>> >
>> > Here is a POC version. What do you think about such solution.
>> >
>> > LS
>>
>> > From 79e8f0e1cdd09ca791755bb03c1e8f38b8078dc3 Mon Sep 17 00:00:00 2001
>> > From: Lukas Slebodnik <lslebodn(a)redhat.com>
>> > Date: Fri, 23 Oct 2015 13:06:00 +0200
>> > Subject: [PATCH] temp
>>
>> Thank you, this works like a charm!
>>
>> ACK (but I had to apply with patch(1) in my pwrap branch, not sure if
>> the patch is applicable atop master...)
>
>Oh and of course please come up with a better commit message :-)
>
It was a "POC version" therefore such commit message.
I think tham make distcheck would not pass with my patch.
>if you prefer, I can resend along with the pam_wrapper patches...
go ahead
I was expecting that pam_wrapper patches will lend sooner.
Do you have a solution with env in WIP patches?
or should we do it before pam tests?
I just wnat to know what to do with this mail thread in patchwork.
LS