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

autoconf: use API archives for pkg-config #81

Open
cnuke opened this issue Jan 16, 2024 · 16 comments
Open

autoconf: use API archives for pkg-config #81

cnuke opened this issue Jan 16, 2024 · 16 comments
Labels

Comments

@cnuke
Copy link
Member

cnuke commented Jan 16, 2024

While porting uacme I encountered configure problems because host libraries got picked up instead of the ones provides by Genode. Since we already went down the road of providing cmake files in the API archives doing the same for pkg-config feels natural.

@cnuke cnuke added the feature label Jan 16, 2024
cnuke added a commit to cnuke/goa that referenced this issue Jan 16, 2024
cnuke added a commit to cnuke/goa that referenced this issue Jan 16, 2024
cnuke added a commit to cnuke/genode that referenced this issue Jan 16, 2024
cnuke added a commit to cnuke/goa-projects that referenced this issue Jan 16, 2024
cnuke added a commit to cnuke/genode that referenced this issue Jan 16, 2024
@cnuke
Copy link
Member Author

cnuke commented Jan 16, 2024

The commits above show-case the approach but are not finalized yet.

@jschlatow
Copy link
Member

That looks very reasonable to me.

By the way, the more include-specific quirks I see being added to Goa, the more I feel the urge to make use of the import-*.mk files instead.

@nfeske
Copy link
Member

nfeske commented Jan 18, 2024

By the way, the more include-specific quirks I see being added to Goa, the more I feel the urge to make use of the import-*.mk files instead.

Alternatively, we could think about introducing conventions that API archives are expected to follow, similar to the convention of the Genode build system that incorporates include// into the include-search paths whereas the values depend on the architecture. Right now, Goa has only the single obvious convention that headers are searched in include/, which apparently doesn't cut it.

In the case of OpenSSL, it is a bit strange to see the include path pointing into src/. Maybe the archive could mirror the headers at include?

@jschlatow
Copy link
Member

Alternatively, we could think about introducing conventions that API archives are expected to follow, similar to the convention of the Genode build system that incorporates include// into the include-search paths whereas the values depend on the architecture. Right now, Goa has only the single obvious convention that headers are searched in include/, which apparently doesn't cut it.

Except for a few special cases (e.g. SDL/SDL2 which required include/SDL2 being added into the include-search paths) introducing architecture-specific conventions should eliminate most quirks.

In the case of OpenSSL, it is a bit strange to see the include path pointing into src/. Maybe the archive could mirror the headers at include?

That's a common pattern if the include files don't reside in the contrib directory but at src/lib/....

@jschlatow
Copy link
Member

Right now, Goa has only the single obvious convention that headers are searched in include/, which apparently doesn't cut it.

@nfeske I just noticed that Goa indeed already has a policy in place to add architecture-specific includes paths. The point of the existing quirks is actually that, for some libraries, we need additional subdirectories in the include paths.

@nfeske
Copy link
Member

nfeske commented Jan 31, 2024

Thanks @jschlatow for the clarification. So we are fine already in this respect. I'm sorry for the noise.

jschlatow added a commit to jschlatow/goa that referenced this issue Jan 31, 2024
If a used API contains an import-<api-name>.mk file, we evalute this in
order to set the include directories appropriately. This relieves us
from most quirks.

genodelabs#81
@jschlatow
Copy link
Member

I had a go at implementing support for import-*.mk files, which turned out to be pretty straightforward. @cnuke At any time that suits you, could you try whether commit b18045e would render 0f2a16c unnecessary?

cnuke added a commit to cnuke/genode that referenced this issue Mar 8, 2024
cnuke added a commit to cnuke/genode that referenced this issue Mar 8, 2024
cnuke added a commit to cnuke/genode that referenced this issue Mar 8, 2024
For now this import file is solely there to satisfy the mechansim
in Goa that collecteds an incorporates import files for used APIs.

Issue genodelabs/goa#81.
@cnuke
Copy link
Member Author

cnuke commented Mar 8, 2024

@jschlatow thanks for the commit - I've cherry-picked b18045e (and the fixup commit on your branch). It works fine but I had to adapt the openssl api archive as it is - naturally - the odd one out by not providing import-openssl.mk. As a quick-fix, commit 16300fc introduces this somewhat artificial file to remedy that for testing purposes.

@chelmuth
Copy link
Member

@cnuke I'm ready to merge the preparatory commits (curl, openssl) to genode staging. Please merge the following cleanup into the import-openssl.mk commit.

diff --git a/repos/libports/lib/import/import-libcrypto.mk b/repos/libports/lib/import/import-libcrypto.mk
index 3878ca4a8c8..bd8a17b9d39 100644
--- a/repos/libports/lib/import/import-libcrypto.mk
+++ b/repos/libports/lib/import/import-libcrypto.mk
@@ -1,9 +1,9 @@
 LIB_OPENSSL_DIR = $(call select_from_repositories,src/lib/openssl)
 
-ARCH = $(filter 32bit 64bit,$(SPECS))
-
 OPENSSL_DIR := $(call select_from_ports,openssl)
 
+ARCH = $(filter 32bit 64bit,$(SPECS))
+
 INC_DIR += $(OPENSSL_DIR)/include
 INC_DIR += $(LIB_OPENSSL_DIR)/spec/$(ARCH)
 
diff --git a/repos/libports/lib/import/import-libssl.mk b/repos/libports/lib/import/import-libssl.mk
index 54ea68accec..1c50108524d 100644
--- a/repos/libports/lib/import/import-libssl.mk
+++ b/repos/libports/lib/import/import-libssl.mk
@@ -2,9 +2,9 @@ LIB_OPENSSL_DIR = $(call select_from_repositories,src/lib/openssl)
 
 OPENSSL_DIR := $(call select_from_ports,openssl)
 
-LIBS += libcrypto
-
 ARCH = $(filter 32bit 64bit,$(SPECS))
 
 INC_DIR += $(OPENSSL_DIR)/include
 INC_DIR += $(LIB_OPENSSL_DIR)/spec/$(ARCH)
+
+LIBS += libcrypto

cnuke added a commit to cnuke/genode that referenced this issue Mar 12, 2024
For now this import file is solely there to satisfy the mechansim
in Goa that collects and incorporates import files for used APIs.

Issue genodelabs/goa#81.
@cnuke
Copy link
Member Author

cnuke commented Mar 12, 2024

@chelmuth commit b93f1f contains the cleaned-up commit.

chelmuth pushed a commit to genodelabs/genode that referenced this issue Mar 12, 2024
For now this import file is solely there to satisfy the mechansim
in Goa that collects and incorporates import files for used APIs.

Issue genodelabs/goa#81.
chelmuth pushed a commit to genodelabs/genode that referenced this issue Apr 12, 2024
For now this import file is solely there to satisfy the mechansim
in Goa that collects and incorporates import files for used APIs.

Issue genodelabs/goa#81.
chelmuth pushed a commit to chelmuth/genode that referenced this issue Jan 7, 2025
chelmuth pushed a commit to chelmuth/genode that referenced this issue Jan 7, 2025
@cnuke
Copy link
Member Author

cnuke commented Feb 11, 2025

I want to bump this issue as I have now encountered two additional projects that would benefit from .pc file handling that is more in line with expectations, namely cmus and irssi and I cannot recall if there were reasons against having .pc files around in the API archives.

(Naturally there are follow-up short-comings I encountered along the way but those can be discussed afterwards.)

@jschlatow
Copy link
Member

@cnuke Thanks for the reminder, I must admit that I totally lost track of this issue. In the meantime, @ssumpf added meson support in #94 for which he wrote a custom script to replace pkg-config. Do you think such an approach would be practical for autoconf as well? With the changes in #99 it will be a walk in the park to replace /usr/bin/pkg-config. My intuition is that we probably still need some .pc files (e.g. if version information is needed), yet a custom script/wrapper could allow us to cover the trivial cases. Either way I'd like to prevent maintaining two paths: one for autoconf, one for meson.

What do you think?

@cnuke
Copy link
Member Author

cnuke commented Feb 13, 2025

Well, now we are already in “Naturally there are follow-up short-comings…” territory ;-) but I think the TL;DR is having .pc files around for version check is agreeable, it stands to reason how and in what capacity they should be evaluated.

I what like to apologise for the following wall of text in advance - my primary intend is to raise awareness, taking immediate action is not required.

for which he wrote a custom script to replace pkg-config. Do you think such an approach would be practical for autoconf as well?

For cmus I went down that road and extended the functionality of that too. At the moment pkg-config.tcl only supports the options that were encountered❶ so far (which is fine as it is inherent to the approach), always reports success❷ and derives the diagnostic pkg-config.log from the available ABI .lib.so files❸ and needs to operated from the buildsystem wrapper, e.g. build/meson.tcl❹.

❷ is a problem when configure uses pkg-config to probe for the existence of a feature and one needs to provide a list of --disable-* or --without-* options in configure_args as otherwise following steps might fail. (Than again that is nothing new.)

❶ is expected as the tool itself is extended on demand - with irssi the configure step tried to determine if version requirements are met and depending on the result might query other information. At this point those information are not available:

My intuition is that we probably still need some .pc files (e.g. if version information is needed), …

I have the same feeling and apparently we are not far off as irssi checks for >= 2.32.0 in glib-2.0.pc — ❸ I evluated the used_apis to determine (because I didn't want to create the abi symlink like meson does and rather use the whole project directory to collect the information) which libraries are available and sanitized the module name in question by stripping and applying tolower so, i.e. the check for SDL-1.2 is matched by sdl in this fictional example, and depending on the availability the appropriate exit-code is set. However, that does not cover an actual versions check because this information is not available (apart from the .port and/or import file or the contrib source directly in case of git checkouts).

For build/autotools.tcl ❹ I set the GENODE_PKG_CONFIG environment variable to tunnel the project directory into pkg-config.tcl as I cannot as easily as for meson generate the required configuration file beside PKG_CONFIG to configure to the tool.

With the changes in #99 it will be a walk in the park to replace /usr/bin/pkg-config

The autotools and also meson (at least I have seen it in one instance) already ask the “proper” pkg-config when they notice that cross-compiling is intended¹ - the former asks for x86_64-pc-elf-pkg-config while the later used genode-x86-pkg-config and they only try the build system's /usr/bin/pkg-config as fallback AFAICT. That makes me wonder if we could not make things easier down the road by playing ball in that regard, perhaps by making the required tools available as part of our toolchain.

Whether we use pkg-config (or even pkg-conf as a more lightweight replacement) or our own tool probably depends on what is more involved to maintain, i.e., what things do users of the buildsystem expect from such a tool.

That brings us to another short-coming - again nothing new here - in that all dependencies, used_apis, are used when compiling and linking. In case of a program like cmus its binary and all shared-objects are linked against all dependencies. From a practical point-of-view this is more or less cosmetical in nature but stands out nonetheless. In that regard .pc files could pave the way for a more fine grand management of compiler/linker flags.

Either way I'd like to prevent maintaining two paths: one for autoconf, one for meson.

I agree. From my perspective pkg-config looks like the lowest common denominator and currently we rather work around than with it.

¹) That might be different when we use goa on Genode directly and --host and --build are the same.

@jschlatow
Copy link
Member

but I think the TL;DR is having .pc files around for version check is agreeable, it stands to reason how and in what capacity they should be evaluated.

Considering that pkg-config is commonly used by many build systems, I believe having .pc files around will become very useful.

jschlatow added a commit to jschlatow/genode that referenced this issue Feb 21, 2025
Since Goa expects include files to reside at include/ or
include/spec/{x86,x86_64,arm_64,64bit}, the src/lib/.../opensslconf.h is
missed by Goa. This commit adds Goa compatibility for the openssl api
archive.

genodelabs/goa#81
@jschlatow
Copy link
Member

@cnuke I merged the autoconf support to staging (e0d360e).

With respect to the openssl quirks, I came to the conclusion that adding import-.mk support to Goa is not worthwhile when we consider that more ports are going to move to Goa and api archives created with Goa will not feature import-.mk files. Therefore, mirroring the required include files at the archive's include/ directory seems more appropriate. You'll find the corresponding api archive published on my depot.

@chelmuth Can you merge genodelabs/genode@375daa3 to staging?

jschlatow added a commit to jschlatow/genode that referenced this issue Feb 21, 2025
@jschlatow
Copy link
Member

@chelmuth I also noticed that curl relies on pkg-config to determine the availability of zlib. I therefore added the corresponding zlib.pc file to the api archive in genodelabs/genode@9598484.

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

No branches or pull requests

4 participants