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

Use of non-distro CPAN package Memory::Process without trouble? #370

Closed
hartwork opened this issue Dec 31, 2024 · 9 comments
Closed

Use of non-distro CPAN package Memory::Process without trouble? #370

hartwork opened this issue Dec 31, 2024 · 9 comments

Comments

@hartwork
Copy link
Contributor

Hi!

If I'm not mistaken, LCOV code uses CPAN module Memory::Process since at least LCOV 2.0 (precisely commit 5f659f6) while neither Debian nor Gentoo seem to have a distro package providing CPAN package Memory::Process. I do not understand yet how both distros are able to package LCOV >=2.0 without any dependency on a package providing Memory::Process files. If that runtime(?) dependency is not a problem, it would be interesting to understand how and why now. Thanks!

Best, Sebastian

@henry2cox
Copy link
Collaborator

Three reasons:

  • the tool only tries to use Memory::Process if parallelism is enabled and a memory constraint is applied.
    I suspect that most users are not using parallelism (disabled by default) - and are not trying to constrain resources, if they have enabled parallel processing.
  • if Memory::Process is not available (and the package error is ignored), then the tool falls back to try to use the /proc filesystem instead (...which will fail on windows, for example).

@hartwork
Copy link
Contributor Author

@henry2cox thanks for elaborating! Just thinking aloud:

  • are there any more widely package alternatives to Memory::Process in CPAN that would be worth switching to?
  • would it make sense to only use Memory::Process on Windows and always parse /proc on Unix-ish?
  • would it make sense to check for Memory::Process sooner at runtime so that packagers may notice lack at runtime (hopefully already when studying the change log of the next release) and package the missing dependency as a new package to depend on?

@henry2cox
Copy link
Collaborator

Hmm...my opinions, below (hopefully subject to change given convincing rational argument):

  • don't know of any such packages - but also haven't really looked.
    This could be more painful than expected as we want a package which is included (easily) on all the common distributions - including various linux flavors as well as cygwin, mac, windows, etc. That might be a high-ish bar.

  • My strong preference is to use standard libraries and packages when available - such that those packages hide platform differences behind their API. I expect those packages to be thoroughly tested on different platforms and versions - and thus to be more stable than whatever hack I might come up with.
    In this specific case: using /proc is guaranteed to fail on windows - which leads to an undesirable (to me) conditional path around the windows implementation.

  • The check for Memory::Process is in the init_parallel_params function - which gets called fairly early - right after command line and config file options are parsed - so any failure should happen before any significant computation starts.
    Do you observe some long delay in some case? (That would likely be a bug.)
    In general, though: yes. It is desirable for the tool to check for dependencies and fail quickly if something is missing. That behaviour may not be consistent across all dependencies - though I am not aware of any such issues (but I also haven't done a thorough check for them).

@hartwork
Copy link
Contributor Author

hartwork commented Jan 6, 2025

@henry2cox the check and import are guarded by a number of conditions though, which is why I never saw the error before. Here's a trigger:

# bin/genhtml -j 2 --memory 100 dummy
Attempting to retrieve memory size from /proc instead
genhtml: ERROR: (package) package Memory::Process is required to control memory consumption during parallel operations: genhtml: ERROR: Can't locate Memory/Process.pm in @INC (you may need to install the Memory::Process module) (@INC entries checked: [..])
        (use "genhtml --ignore-errors package ..." to bypass this error)

I guess we could take that command to distros and ask them to package Memory::Process given that they already have LCOV >=2.0 packaged:

# git tag --contains 5f659f63801ef7f94c50a0eb5cffa1ea70f73651 | sort -V
v2.0
v2.1
v2.1-beta
v2.1-beta2
v2.2
v2.2-beta
v2.3-beta

@hartwork
Copy link
Contributor Author

hartwork commented Jan 6, 2025

@henry2cox report sent to Gentoo: https://bugs.gentoo.org/947616

@hartwork
Copy link
Contributor Author

hartwork commented Jan 6, 2025

@hartwork
Copy link
Contributor Author

hartwork commented Jan 7, 2025

@henry2cox report sent to Gentoo: https://bugs.gentoo.org/947616

Now fixed through two new Perl packages:

  • dev-perl/Memory-Process (and its own dependency:)
  • dev-perl/Memory-Usage

@hartwork
Copy link
Contributor Author

@henry2cox should we close as "fixed in Gentoo, Debian notified, done"?

@henry2cox
Copy link
Collaborator

In general, I prefer that whoever opened the issue be the one to decide that things are complete and close it :-)
That way: we run less risk that my interpretation of the requirement is substantially different than the issue author.

That said: yeah - I think so. Sounds reasonable to me.

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

No branches or pull requests

2 participants