Skip to content

Problem: configure fails on system without pkg-config#491

Merged
jimklimov merged 3 commits intonetworkupstools:masterfrom
jimklimov:pkg-config
Aug 14, 2018
Merged

Problem: configure fails on system without pkg-config#491
jimklimov merged 3 commits intonetworkupstools:masterfrom
jimklimov:pkg-config

Conversation

@jimklimov
Copy link
Copy Markdown
Member

Solution: detect presence and usability of pkg-config program
and autotools macros (they end up as unexpanded tokens if
pkg-config is not installed), and use this knowledge to
proceed with search for libcppunit - or not. When the test is
optional, we should not kill the build if we can not make it
due to missing tools.

Solution: detect presence and usability of pkg-config program
and autotools macros (they end up as unexpanded tokens if
pkg-config is not installed), and use this knowledge to
proceed with search for libcppunit - or not. When the test is
optional, we should not kill the build if we can not make it
due to missing tools.
@jimklimov jimklimov requested review from aquette and clepple October 20, 2017 17:43
@clepple
Copy link
Copy Markdown
Member

clepple commented Oct 27, 2017

@jimklimov I'm not sure I understand when this situation would arise. Are we talking about building from a tarball? If from Git, we have some assumptions about using recent autotools that I am not sure if this patch addresses.

I just did a tarball build (tarball built with make dist from master@704459f89) on OS X where pkg-config was not in the PATH, and things configured and built fine. Just a single data point, but there seems to be a lot of code here for a small corner case.

@jimklimov
Copy link
Copy Markdown
Member Author

jimklimov commented Oct 28, 2017

I was recently building on a minimized-footprint system, so adding autoconf was not sufficient - had to find and add the pkg-config package too. Note that those are different projects, so without pkg-config installed, the PKG_* macros are not defined in m4 and remain verbatim tokens including parentheses and producing shell script parsing failure.

Actual use of pkg-config that I've found in NUT is currently constrained to cppunit scaffolding (so far at least), so actual NUT project code can live and build quite happily on systems without pkg-config.

@clepple
Copy link
Copy Markdown
Member

clepple commented Oct 28, 2017

I use the term "autotools" loosely, and I guess I assume that if people are using autogen.sh, then they should have their distribution's version of build-essential installed (including pkg-config). It's not that I think we should ignore minimal systems, but that they are better served by installing from tarballs.

FWIW, the libusb-1.0 branch uses pkg-config.

I don't want to spend too much time bikeshedding this, so if you want to merge this, I guess I don't have a specific objection, but let's not set a precedent here - I really don't want the NUT configure.ac to basically reinvent the portions of autoconf/automake/pkg-config that are already available in later versions. As has been done for other auxiliary tools, if a widely-available developer tool is missing during the run of autogen.sh, I think we should error out with a message that shows what is missing, rather than just warning.

@jimklimov
Copy link
Copy Markdown
Member Author

jimklimov commented Oct 29, 2017 via email

@clepple
Copy link
Copy Markdown
Member

clepple commented Oct 29, 2017 via email

@jimklimov
Copy link
Copy Markdown
Member Author

jimklimov commented Oct 29, 2017 via email

@jimklimov
Copy link
Copy Markdown
Member Author

jimklimov commented Nov 19, 2017

Thinking more about this, the opinion became stronger: building on a system that lacks pkg-config causes the generated configure script to fail in a way that is hard to debug. This PR fixes that part of the problem, and can work for any checkout from git without relying on tarballs. It also allows the builds to pass on systems that do not have pkg-config (e.g. proprietary Unix deployments where one may not install much).

@aquette : do you have an opinion on this?

@jimklimov
Copy link
Copy Markdown
Member Author

@aquette @clepple : How about adding this to 2.7.5? I believe the added logic should not break anything, but can fix builds out of the box on certain systems. Which should be good PR for a release :)

@clepple
Copy link
Copy Markdown
Member

clepple commented Dec 4, 2017 via email

@jimklimov jimklimov force-pushed the pkg-config branch 3 times, most recently from c6541cd to 4543b15 Compare December 5, 2017 01:10
@jimklimov
Copy link
Copy Markdown
Member Author

Tried to stage the test, see https://travis-ci.org/networkupstools/nut/jobs/311621280 - but found that in Debian the*.pc files are part of *-dev packages, along with headers etc... so removing the pkg-config package also removes the dependencies NUT wants to build against.

@clepple
Copy link
Copy Markdown
Member

clepple commented Dec 5, 2017 via email

@jimklimov
Copy link
Copy Markdown
Member Author

"without those libraries" it had problems passing configure (no headers of things to build against)... I'm now trying a different approach at removing pkg-config package contents while having the needed rest installed...

@jimklimov
Copy link
Copy Markdown
Member Author

jimklimov commented May 22, 2018

@clepple : I've finally crafted a way to stage this situation (lack of pkg-config) on Travis.

This was a bit difficult because modern autoconf somehow expects/relies on pkg-config's pkg.m4 being present while it generates the configure script, although I did not unravel why - saw no direct references during cursory review.

On older systems that did not have it sort of always, their older autotools probably did not rely on it either, and/or the release tarballs could be used which include a copy of pre-generated configure script.

You can see an example log of successful build (not finding a number of components where it defaults to looking by pkg-config files, but not crashing with invalid syntax of the configure script itself - as it did before the PR) e.g. here : https://api.travis-ci.org/v3/job/382089663/log.txt

Side note: compared to initial premise of this being needed for libcppunit, it seems that pkgconfig metadata is also consulted to look for quite a few other dependencies, as at least one data source about those.

@jimklimov jimklimov added the ready / code review Author (and CI) consider the PR worthy of human rewievers' time label May 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready / code review Author (and CI) consider the PR worthy of human rewievers' time

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants