linux-test-project / lcov

LCOV
GNU General Public License v2.0
867 stars 235 forks source link

2.0.0: lib/lcovutil.pm not installed as perl module #223

Closed wuch-g2v closed 1 year ago

wuch-g2v commented 1 year ago

Looks like some script stated use lcovutil module present in lib/lcovutil.pm source tree. IMO it would be good to install that as regular perl module in per tree as lcov::lcovutil.

henry2cox commented 1 year ago

Is there an error that you see when trying to use some lcov component (lcov, genhtml, geninfo)? Or are you asking for some change as a matter of style?

We consider the 'lcovutil' module to be private interfaces of the implementation - and we feel free to change them at any point, for any reason. As a result: we do not want to export/publish the current interfaces and then be stuck supporting them forever.

Could you clarify your request, and explain what things would look like if they exactly met your expectation. Thanks Henry

wuch-g2v commented 1 year ago

To be able use lcovutil module it is necessary to add path to it.

https://github.com/linux-test-project/lcov/blob/521e31949b5571d9093e9e85462cb137dded05b4/bin/lcov#L77

If it will be installed as regular module that hack will be not necessary.

wuch-g2v commented 1 year ago

Generally if that module would be installed as regular perl module on packaging lcov as rpm package it will be scanned by automatic dependencies generator. Currently it is not the case.

henry2cox commented 1 year ago

So the complaint is that the RPM system is not correctly picking up the dependencies. That is probably fixable - though I don't know how to fix it without some research.

As mentioned above: we don't want to export the lcovutil module in a way that would encourage people to use it in their code - as we may change the existing interfaces at any time.

wuch-g2v commented 1 year ago

dependencies generator checks only files in installvendorarch and installvendorlib

[tkloczko@pers-jacek SPECS]$ perl -V:installvendorlib
installvendorlib='/usr/share/perl5/vendor_perl';
[tkloczko@pers-jacek SPECS]$ perl -V:installvendorarch
installvendorarch='/usr/lib64/perl5/vendor_perl';
oberpar commented 1 year ago

As @henry2cox stated, lcovutil.pm is deliberately not installed as a generic Perl module. Commit 1161379f8f823b321c8bee1dc7e45301d28d3acb specifically instructs automatic RPM dependency generators to exclude lcovutil.pm from the requires/provides list.

Also during RPM generation/make install, the FindBin calls used to locate lcovutil.pm are replaced by the specification of the absolute module installation path to make sure that the module is found regardless of any module search paths.

Example:

@@ -70,11 +70,10 @@
                               file_name_is_absolute rootdir splitdir splitpath/;
 use Cwd qw /abs_path getcwd/;
 use POSIX qw (floor);
-use FindBin;
 use Storable;
 use Time::HiRes;    # for profiling

-use lib "$FindBin::RealBin/../lib";
+use lib "/usr/lib/lcov";
 use lcovutil qw ($tool_name $tool_dir $lcov_version $lcov_url
                  define_errors parse_ignore_errors ignorable_error
                  info set_info_callback init_verbose_flag $verbose

So unless there is an actual run-time problem, or a violation of documented packaging rules, this behavior is working as designed.

henry2cox commented 1 year ago

Generally if that module would be installed as regular perl module on packaging lcov as rpm package it will be scanned by automatic dependencies generator. Currently it is not the case.

Dependencies seem properly identified:

$ rpm -qpR lcov-2.0-1.noarch.rpm /usr/bin/perl /usr/bin/python3 config(lcov) = 2.0-1 perl >= 5.8.8 perl(Capture::Tiny) perl(Carp) perl(Cwd) perl(Date::Parse) perl(DateTime) perl(Digest::MD5) perl(Exporter) perl(File::Basename) perl(File::Copy) perl(File::Find) perl(File::Path) perl(File::Spec) perl(File::Spec::Functions) perl(File::Temp) perl(FileHandle) perl(Getopt::Long) perl(JSON) perl(Module::Load) perl(Module::Load::Conditional) perl(POSIX) perl(Storable) perl(Time::HiRes) perl(constant) perl(lib) perl(strict) perl(warnings) rpmlib(CompressedFileNames) <= 3.0.4-1 rpmlib(FileDigests) <= 4.6.0-1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1

Unless there is some other problem, I believe that this issue can be closed.

henry2cox commented 1 year ago

Closing this issue now. It appears that there is not actual problem or functional bug.

If there is still a problem, please either reopen this issue or file a new one. Please include a detailed description of the problem and include a testcase which exhibits the issue if at all possible Thanks Henry

GitMensch commented 1 year ago

Hm, after installing lcov 2.0 from source I get an error using it:

lcov  --capture --no-checksum --compat-libtool --rc lcov_branch_coverage=1 --gcov-tool "gcov"
Can't locate Capture/Tiny.pm in @INC (you may need to install the Capture::Tiny module) (@INC contains: /usr/local/lib/lcov /etc/perl /usr/local/lib/x86_64-linux-gnu/perl/5.30.0 /usr/local/share/perl/5.30.0 /usr/lib/x86_64-linux-gnu/perl5/5.30 /usr/share/perl5 /usr/lib/x86_64-linux-gnu/perl/5.30 /usr/share/perl/5.30 /usr/local/lib/site_perl /usr/lib/x86_64-linux-gnu/perl-base) at /usr/local/lib/lcov/lcovutil.pm line 14.
BEGIN failed--compilation aborted at /usr/local/lib/lcov/lcovutil.pm line 14.
Compilation failed in require at /usr/local/bin/lcov line 102.
BEGIN failed--compilation aborted at /usr/local/bin/lcov line 102

So it looks like that lcovutil.pm is installed and uses a perl module that is newer / a new dependency?

henry2cox commented 1 year ago

So it looks like that lcovutil.pm is installed and uses a perl module that is newer / a new dependency?

Yes. There is a list of the required packages in section 3 of the README.

GitMensch commented 1 year ago

Ah, I see, this list got actually added with 2.0: https://github.com/linux-test-project/lcov/compare/v1.16...v2.0#diff-2b7814d3fca2e99e56c51b6ff2aa313ea6e9da6424804240aa8ad891fdfe0900 - can you please edit the 2.0 release at GitHub and pointing out that there are new dependencies, possibly linking to to https://github.com/linux-test-project/lcov/blob/v2.0/README#L86?

oberpar commented 1 year ago

As suggested I updated the release text to mention the new module dependencies. Thanks for pointing this out.

henry2cox commented 1 year ago

Arguably, also a good idea to add something like:

See section 6 of the README for a brief description of the above features, as well as the man pages for details.

...to the release description as well. Not sure if users will go look if not explicitly directed (or if anyone will go look, even if explicitly directed).