Improve loading optional dependencies#139
Improve loading optional dependencies#139ppisar wants to merge 2 commits intoPhilterPaper:masterfrom
Conversation
A code in LA_*() methods was identical to a check is image_() methods. This patch removes the in-lined code and calls the methods instead.
The old code first probed for a dependency (e.g. Graphics::TIFF) and then did not use it if the dependency was explicitly disabled (e.g. -nouseGT). This patch reverses the order to speed up the code and alleviate from loading unused modules.
|
Is the intent here to replace the hard-coded eval check with a call to the LA_* (Library Available) calls? Your new code behaves a little differently than the old code, returning -1 if "don't use the library" regardless of whether it's installed, instead of 0 if it's not installed. I will have to look at the usage and see whether this makes any difference. |
|
On Thu, Nov 05, 2020 at 07:09:04AM -0800, Phil Perry wrote:
Is the intent here to replace the hard-coded eval check with a call to the
LA_* (Library Available) calls?
Yes.
Your new code behaves a little differently than the old code, returning -1
if "don't use the library" regardless of whether it's installed, instead of
0 if it's not installed.
Which piece of code does return -1?
image_png() always returns an object reference.
LA_IPL() always returns 0 or 1. Never -1.
|
The segments you changed in Builder.pm set the library usage flag to -1 if the library is available but you choose not to use it, 0 if it's unavailable, or 1 if it's available and you choose to use it. Your change sets it to -1 even if the library is UNavailable. |
|
I see. It only affects printing the "Your system does not have..." warning.
But a comment above the warning reads:
# TBD give warning message once, unless silenced (-silent) or
# deliberately not using Graphics::TIFF (rc == -1)
So it actualy gets the code in line to the comment. Why should a user be
informed about missing the library if he decided that does not want the
library? I think it's an improvement.
|
|
You're misreading the comment and the original code. You see the message (once) if 1) If you feel that the existing comments and documentation are unclear, please list the problem(s) that I could try to do something about. |
|
OK, a bit of a compromise here. I now use |
2 participants
This patch set improves loading the optional dependencies for PNG and TIFF images.