Add a ./Configure "use taint" option

[leonerd]

Updates the ./Configure script to allow control of whether we build taint support or not. Created from the underlying metaconfig change Perl/metaconfig#94.

Three reasons make me nervous here, looking for some good review:

1. I have almost no idea what I'm doing with metaconfig so this is all a wild guess
2. The first commit contains a diff that mconfig applied before I had even edited any files. Someone who knows these things ought to look into why that happened.
3. I'm not very happy with the inverted logic sense here; Configure sets the "USE_TAINT" macro which perl.h then inverts to create the NO_TAINT_SUPPORT which is used everywhere else in the code. I think it would be a lot neater for all the other code to just be conditional on USE_TAINT directly. But that's a rather larger edit than I really wanted to do here, plus the "SILENT_NO_TAINT_SUPPORT" complicates it further.

• This set of changes requires a perldelta entry, and I'll write something by the time I merge it

[leonerd]

And a fourth reason:
4. Something will have to be done about all the non-Configure based platforms (Windows, VMS, etc...?) but I again have no idea what that might be.

[leonerd]

Ahah; turns out point 4 may be more pressing than I expected. It fails sanity tests because of this:

$ (cd t; ../perl -I../lib porting/checkcfgvar.t)
1..14
ok 1 - Cross/config.sh-arm-linux sorted
not ok 2 - Cross/config.sh-arm-linux missing keys usetaint
ok 3 - Cross/config.sh-arm-linux-n770 sorted
not ok 4 - Cross/config.sh-arm-linux-n770 missing keys usetaint
ok 5 - Porting/config.sh sorted
not ok 6 - Porting/config.sh missing keys usetaint
ok 7 # skip configure.com doesn't need to be sorted
not ok 8 - configure.com missing keys usetaint
ok 9 - plan9/config_sh.sample sorted
not ok 10 - plan9/config_sh.sample missing keys usetaint
ok 11 - win32/config.gc sorted
not ok 12 - win32/config.gc missing keys usetaint
ok 13 - win32/config.vc sorted
not ok 14 - win32/config.vc missing keys usetaint

[leonerd]

Much failure all round. I wonder if @Tux might be able to offer any guidance here?

[Leont]

Most of these files just need a default value for the new variable, see 1f9097b for an example.

[leonerd]

Right. That's item 4 sorted. But items 1 to 3 remain.
Anyone want to review this.. @Tux ?

[Leont]

This makes the interface to SILENT_NO_TAINT_SUPPORT a bit awkward. Should that be a separate configuration flag? Should it be the default when usetaint is undefined? (you can't install most of CPAN without it)

[leonerd]

I could imagine adding a separate flag but if as you say, most of CPAN doesn't even install without either taint or SILENT_NO_TAINT, I can't imagine it being useful.

I think there's in practice only two useful states - taint on, or silent taint off. Both are now covered by the -D vs -U setting of usetaint.

[khwilliamson]

The units are automatically sorted in Configure by the mconfig command. You can influence the order with the ?Y command in the .U file. The sort is done minorly for consistency, but mostly to make sure that a unit doesn't rely on a unit that comes later in Configure.

You should be religiously using as a cookbook the README file in the metaconfig directory (above the U) one (patches welcome). It tells you you need to patch metaconfig.h, which hasn't been done here.

I'm unsure if you ran "Porting/checkcfgvar.pl", and similar tools listed in the README

[leonerd]

@khwilliamson

The units are automatically sorted in Configure by the mconfig command. You can influence the order with the ?Y command in the .U file. The sort is done minorly for consistency, but mostly to make sure that a unit doesn't rely on a unit that comes later in Configure.

Oh I don't mind what order these run in; it doesn't look like it really matters. My concern was simply that, starting from a clean checkout of both perl5 and metaconfig, simply rerunning the generator already produced changes when I expected there to be none. That confused me - I wasn't sure if it was expected.

You should be religiously using as a cookbook the README file in the metaconfig directory (above the U) one (patches welcome). It tells you you need to patch metaconfig.h, which hasn't been done here.

I did read the section about metaconfig.h, but that seemed to be explaining that you need to add it there if it doesn't turn up yet because no code is actually yet using the macro. Since I had already added code in perl.h which really does use the macro, that didn't appear to be necessary. But is it standard practice to list it there anyway even so?

I'm unsure if you ran "Porting/checkcfgvar.pl", and similar tools listed in the README

I did run checkcfgvar.pl, and it said nothing - I guess it's happy?

[khwilliamson]

As long as you ran all the things that cookbook says, and took care of any messages, I'm happy with that portion.

I'm trying to understand about metaconfig.h. I don't see anything in your PR nor blead that references this new symbol That would mean it needs to go in that file. (A bunch of symbols already there could now be removed.) But you're right that if perl.h already contained the symbol, metaconfig.h would not need it. But, again, I don't see it.

[khwilliamson]

I know from bitter experience that if you don't have that symbol defined, that everything seemingly works, but will fail to compile when starting with a clean workspace.

[leonerd]

I'm trying to understand about metaconfig.h. I don't see anything in your PR nor blead that references this new symbol That would mean it needs to go in that file. (A bunch of symbols already there could now be removed.) But you're right that if perl.h already contained the symbol, metaconfig.h would not need it. But, again, I don't see it.

Part of the PR; see perl.h line 947:

#ifndef PERL_USE_TAINT

I missed seeing the change to perl.h

[leonerd]

OK, well... As it stands it doesn't consider the SILENT version at all.

./Configure - equivalent to as it is now
./Configure -Uusetaint - equivalent to passing -Accflags=-DNO_TAINT_SUPPORT

There is no equivalent for setting the SILENT option. I don't have a good feel for what that should be - any suggestions?

[Leont]

There is no equivalent for setting the SILENT option. I don't have a good feel for what that should be - any suggestions?

IMNSHO the silent flag should be the default if -Uusetaint. Otherwise every test on CPAN with a -T in the shebang is going to fail, which effectively means such a perl is not viable.

Instead of a silent flag, we should have a loud flag as an option for people who want such behavior.

[leonerd]

IMNSHO the silent flag should be the default if -Uusetaint. Otherwise every test on CPAN with a -T in the shebang is going to fail, which effectively means such a perl is not viable.

Instead of a silent flag, we should have a loud flag as an option for people who want such behavior.

OK; latest behaviour: default taint is on. -Uusetaint sets the SILENT version of no-taint. There's not currently a way to get the noisy complaining version. I believe to add that would require a second -D flag.

[leonerd]

Ah. The -Uusetaint test suite fails, because:

#   Failed test 'taint flag is set'
#   at t/taint.t line 22.
# Looks like you failed 1 test of 2.
../cpan/Module-Metadata/t/taint.t .................................... 
Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/2 subtests

That test file starts with:

use Test::More $Config{ccflags} =~ /-DSILENT_NO_TAINT_SUPPORT/
    ? ( skip_all => 'No taint support' ) : ( tests => 2 );

as a way to detect if taint mode is supported, by looking in $Config{ccflags}. I wonder how many other CPAN modules there are out there which do it that way. All of those will fail with this approach.

[leonerd]

I wonder how many other CPAN modules there are out there which do it that way. All of those will fail with this approach.

Huh. Actually it turns out this appears to literally be the only module doing this:

https://grep.metacpan.org/search?q=DSILENT_NO_TAINT_SUPPORT&qft=*.t&qd=&qifl=

And no hits at all for the name without the leading "D".

I am slightly suspicious - what does anyone else do?

[Leont]

I wonder how many other CPAN modules there are out there which do it that way. All of those will fail with this approach.

MetaCPAN grep can only find one other dist affected, so I guess it's fine. They'll just have to adapt to the new situation (exists $Config{usetaint} and not $Config{usetaint}).

[Tux]

It is on my radar (now). I'll try to share some thought soon(ish)

[leonerd]

I've now updated the way that Module-Metadata's test works. Hopefully this is now sufficient to merge?