Perl / perl5

🐪 The Perl programming language
https://dev.perl.org/perl5/
Other
1.92k stars 549 forks source link

[doc] ExtUtils::ParseXS - typemap search path documentation inconsistency #18682

Open benkasminbullock opened 3 years ago

benkasminbullock commented 3 years ago

Where dist/ExtUtils-ParseXS/lib/ExtUtils/ParseXS.pod https://github.com/Perl/perl5/blob/blead/dist/ExtUtils-ParseXS/lib/ExtUtils/ParseXS/Utilities.pm#L66-L155

Description ExtUtils::ParseXS describes that the path where typemaps are looked for is

../../../typemap:../../typemap:../typemap:typemap

on line 39, but that disagrees with the documentation and implementation in ExtUtils::ParseXS::Utilities, where the documentation and implementation of standard_typemap_location gives another ../, so four, as well as looking in lib/ExtUtils/.

Incidentally the function standard_typemap_locations appears to be unused outside of Utilities.pm and doesn't need to be imported into ExtUtils::ParseXS, or even exported. It's used excusively by process_typemaps and in the test files of ExtUtils::ParseXS. In the test files it could just be qualified as ExtUtils::ParseXS::Utilities::standard_typemap_locations.

jkeenan commented 2 years ago

Where dist/ExtUtils-ParseXS/lib/ExtUtils/ParseXS.pod https://github.com/Perl/perl5/blob/blead/dist/ExtUtils-ParseXS/lib/ExtUtils/ParseXS/Utilities.pm#L66-L155

Description ExtUtils::ParseXS describes that the path where typemaps are looked for is

../../../typemap:../../typemap:../typemap:typemap

on line 39, but that disagrees with the documentation and implementation in ExtUtils::ParseXS::Utilities, where the documentation and implementation of standard_typemap_location gives another ../, so four, as well as looking in lib/ExtUtils/.

Incidentally the function standard_typemap_locations appears to be unused outside of Utilities.pm and doesn't need to be imported into ExtUtils::ParseXS, or even exported. It's used excusively by process_typemaps and in the test files of ExtUtils::ParseXS. In the test files it could just be qualified as ExtUtils::ParseXS::Utilities::standard_typemap_locations.

@sisyphus, would you be able to take a look at this ticket and see if there's any overlap with the problem you are describing in https://github.com/Perl/perl5/issues/19294?

Thank you very much. Jim Keenan

sisyphus commented 2 years ago

No overlap, @jkeenan. This one is a straightforward case of another behaviour vs documentation mismatch.

The documentation says that any files called "typemap" that are in either '.' or '..' or '../..' or '../../..' will be found automatically. In reality, however, files named "typemap" that are located in '../../../..' will also be found automatically, courtesy of standard_typemap_locations( \@INC) doing what it does. (Perhaps, the documentation was originally correct, and then the extra directory was added but the docs not updated ... I don't know.)

@benkasminbullock's observation about there being no need to export standard_typemap_locations seems to be correct, though I can't rule out the possibility that there's code somewhere that relies on the current behaviour.

In the process_typemaps subroutine, it's illuminating to replace:

push @tm, standard_typemap_locations( \@INC );

with something like:

  warn "\n\n PWD: $pwd\n";
  my @s = standard_typemap_locations ( \@INC );
  for(@tm) { warn " TM : $_\n" }
  for(@s) { warn " S  : $_\n" } 
  push @tm, @s;

IME (which does not extend to Module::Build driven applications), we then get to see that anything picked up by standard_typemap_locations( \@INC) is already in code>@tm</code to begin with, which makes the push() pointless and potentially destructive. But that's an observation more pertinent to #19294 than to this thread.

Cheers, Rob