Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "Add Configure question for taint support" #19679

Merged
merged 2 commits into from
May 20, 2022

Conversation

leonerd
Copy link
Contributor

@leonerd leonerd commented May 1, 2022

This reverts commit 39f8eb4 which was added in PR #19541

This is because of a variety of issues discussed #19657 and at the PSC meeting 064 2022-04-22 https://www.nntp.perl.org/group/perl.perl5.porters/2022/04/msg263670.html

@leonerd leonerd requested a review from neilb May 1, 2022 08:57
@jkeenan
Copy link
Contributor

jkeenan commented May 1, 2022

To evaluate this p.r. I built blead at fc995cf with my regular configuration options, then built @leonerd's branch the same way, then compared their respective config.sh files. Results:

$ diff -w -I "# Configuration time" -I api_subversion -I api_version -I api_versionstring -I archlib -I archlibexp -I cf_time -I installarchlib -I installprivlib -I installsitearch -I installsitelib -I patchlevel -I perlpath -I privlib -I privlibexp -I sitearch -I sitearchexp -I sitelib -I sitelibexp -I startperl -I subversion -I version -I version_patchlevel_string -I PERL_API_VERSION -I PERL_API_SUBVERSION config.sh.blead config.sh.leonerd
1125d1124
< taint_support='define'

Looks good.

Just for reference, here's a similar diff starting with v5.34.1, thereby showing us our "net configuration changes from 5.34" assuming this p.r. is merged:

$ diff -w -I "# Configuration time" -I api_subversion -I api_version -I api_versionstring -I archlib -I archlibexp -I cf_time -I installarchlib -I installprivlib -I installsitearch -I installsitelib -I patchlevel -I perlpath -I privlib -I privlibexp -I sitearch -I sitearchexp -I sitelib -I sitelibexp -I startperl -I subversion -I version -I version_patchlevel_string -I PERL_API_VERSION -I PERL_API_SUBVERSION config.sh.v5.34.1 config.sh.leonerd
224a225,226
> d_ffs='define'
> d_ffsl='define'
431a434,435
> d_nl_langinfo_l='define'
> d_non_int_bitfields='define'
598a603
> d_strxfrm_l='define'
611a617
> d_thread_local='define'
986a993
> perl_thread_local='_Thread_local'
1097a1105,1106
> st_dev_sign='1'
> st_dev_size='8'
1215a1225
> xlocale_needed='define'
1221,1222c1231,1232
< PERL_VERSION=34
< PERL_SUBVERSION=1
---
> PERL_VERSION=35
> PERL_SUBVERSION=12

@leonerd
Copy link
Contributor Author

leonerd commented May 15, 2022

This is still needing some review. @neilb , @rjbs , anyone else?

@Leont
Copy link
Contributor

Leont commented May 15, 2022

I think this missed perlsec.pod, that needs a documentation update to also be reverted.

@neilb
Copy link
Contributor

neilb commented May 17, 2022

As @Leont noted, perlsec.pod needs the section on "Checking for taint suppot" removing, and the reference to that section earlier on.

The second paragraph in section "Strictness and warnings" of perlmodstyle should be removed.

In perlipc.pod remove this text:

Note that perl can be built without taint support,
in which case -T silently does nothing
(see L<perlsec> for how to check if your perl support taint checking).

replace it with: "Note that perl can be built without taint support. There are two different modes: in one, -T will silently do nothing. In the other mode -T results in a fatal error".

In perllocale.pod, remove this:

See L<perlsec> for how to tell if your perl supports taint checking.

In perlrun.pod, remove this:

(see L<perlsec>).

and this:

If your perl has been built without taint support, then this option
has no effect. See L<perlsec> for how to check whether your perl
supports taint checking.

This reverts commit 39f8eb4.

This is because of a variety of issues discussed Perl#19657 and at the PSC
meeting 064 2022-04-22

https://www.nntp.perl.org/group/perl.perl5.porters/2022/04/msg263670.html
@leonerd
Copy link
Contributor Author

leonerd commented May 19, 2022

@neilb I believe that's all the docs changes you listed. PTAL.

Should I squash these before merge, or leave them as two commits?

pod/perlipc.pod Outdated
Note that perl can be built without taint support,
in which case -T silently does nothing
(see L<perlsec> for how to check if your perl support taint checking).
Note that perl can be built without taint support. THere are two
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"THere"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't the preceding sentence be removed entirely?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hrm, that was in Neil's suggested text. I think it's still technically true, it's just you have go to the long way round with -Accflags=... rather than using a configure question.

@rjbs
Copy link
Member

rjbs commented May 19, 2022

At this point, what remains before this is merged? IMHO, this is the only known blocker.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants