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
Have Configure use pkg-config for system libtommath #1006
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks alright except for the one issue I noticed.
Configure.pl
Outdated
} | ||
if (index($config{lincludes}, '-L/usr/local/lib') == -1) { | ||
$config{lincludes} = join(' ', $config{lincludes}, '-L/usr/local/lib'); | ||
unless ($config{pkgconfig_works} && setup_native_library('libtommath')) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you use if
here instead of unless
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you saying you want it changed to if (!$config{pkgconfig_works} || !setup_native_library('libtommath'))
due to preference for if
over unless
, because the code within the block is the fallback if pkg-config
isn't available or if a .pc
file for libtommath isn't found. If setup_native_library
succeeds then cincludes
and lincludes
should be already be populated.
If you want it changed due to preference for if
, I'm happy to do so. I just want to be sure that I have what you are asking for right.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I'm asking for if
to be used over unless
but have it remain functionally identical.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay will do. Thanks for clarifying :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@samcv, I've replaced unless
with if
as you requested.
If pkg-config is available, use that to determine the lib and include paths/flags (with a fallback to a default if the .pc not found/available). This way, if platforms with irregular lib/header locations ship a .pc file, the system files can be found.
82f3758
to
135e52a
Compare
If pkg-config is available, use that to determine the lib and include paths/flags (with a fallback to a default if the .pc not found/available). This way, if platforms with irregular lib/header locations ship a .pc file, the system files can be found.
This doesn't totally fix the issue with building on ROSA linux seen here (as it seems it's libtommath-devel package does not include a .pc), but it does take a step to addressing platforms with irregular lib/header locations.
The libtommath source has a template for pkg-config (seen here) which can be bundled by package maintainers.