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

pkg-config libraries without include directories or files to link to are not discovered #3777

Closed
xaizek opened this issue May 25, 2023 · 26 comments
Labels
Milestone

Comments

@xaizek
Copy link
Contributor

xaizek commented May 25, 2023

Xmake Version

v2.7.9+20230515

Operating System Version and Architecture

Slackware Linux 15.0 x86_64
pkg-config: v0.29.2

Describe Bug

Header-only libraries have empty output from pkg-config --ldflags and pkg-config --cflags which seems to make add_requires("CLI11") fail.

Expected Behavior

add_requires() should consider any library for which pkg-config --exists succeeds as present even if it doesn't need any additional compiler/linker flags.

Project Configuration

No response

Additional Information and Error Logs

You can test with https://github.com/CLIUtils/CLI11 which isn't found by add_requires() despite being installed.

@xaizek xaizek added the bug label May 25, 2023
@waruqi
Copy link
Member

waruqi commented May 26, 2023

? please provide your error logs with -vD and your xmake.lua configuration.

xmake f -cvD
xmake -vD

and rename CLI11 to cli11. add_requires("cli11")

@xaizek
Copy link
Contributor Author

xaizek commented May 26, 2023

and rename CLI11 to cli11. add_requires("cli11")

Then the failure is expected. The project is called CLI11, it creates /usr/share/pkgconfig/CLI11.pc with:

prefix=/usr
exec_prefix=${prefix}
includedir=${prefix}/include

Name: CLI11
Description: C++ command line parser
Version: 2.3.2

Cflags: -I${includedir}

And pkg-config is case-sensitive.


No project is necessary to see the issue and I've explained why (empty --cflags and --ldflags):

$ xmake l -vD find_package CLI11
running builtin module find_package with args:
dump from _print_vlog @@programdir/plugins/lua/main.lua:55
"CLI11"
dump from <anonymous> @@programdir/plugins/lua/main.lua:125
nil

With a "project":

set_project("test")
add_requires("CLI11")

Useless logs are:

$ xmake f -cvD
checking for platform ... linux
checking for architecture ... x86_64
checking for gcc ... /usr/bin/gcc
checkinfo: cannot runv(zig version), No such file or directory
checking for zig ... no
checkinfo: cannot runv(zig version), No such file or directory
checking for zig ... no
checking for unzip ... /usr/bin/unzip
checking for git ... /usr/bin/git
checking for gzip ... /usr/bin/gzip
checking for tar ... /usr/bin/tar
/usr/bin/git rev-parse HEAD
error: @programdir/core/main.lua:300: ...dir/core/sandbox/modules/import/core/package/package.lua:58: /home/xaizek/.xmake/repositories/xmake-repo/packages/c/cli11/xmake.lua: package(CLI11) not found!
stack traceback:
    [C]: in function 'error'
    [@programdir/core/base/os.lua:913]:
    [...dir/core/sandbox/modules/import/core/package/package.lua:58]:
    [@programdir/modules/private/action/require/impl/package.lua:764]: in function '_load_package'
    [@programdir/modules/private/action/require/impl/package.lua:896]: in function '_load_packages'
    [@programdir/modules/private/action/require/impl/package.lua:1195]: in function 'load_packages'
    [...modules/private/action/require/impl/install_packages.lua:659]:
    [@programdir/modules/private/action/require/install.lua:85]:
    [@programdir/actions/config/main.lua:387]:
    [C]: in function 'xpcall'
    [@programdir/core/base/utils.lua:280]:
    [@programdir/core/base/task.lua:501]: in function 'run'
    [@programdir/core/main.lua:298]: in function 'cotask'
    [@programdir/core/base/scheduler.lua:404]:

stack traceback:
        [C]: in function 'error'
        @programdir/core/base/os.lua:913: in function 'os.raiselevel'
        (...tail calls...)
        @programdir/core/main.lua:300: in upvalue 'cotask'
        @programdir/core/base/scheduler.lua:404: in function <@programdir/core/base/scheduler.lua:397>

@xaizek
Copy link
Contributor Author

xaizek commented May 26, 2023

Actually, updating Cflags: to get non-empty output of --cflags doesn't seem to affect the result, but adding Libs: -lbla fixes library detection.

@waruqi
Copy link
Member

waruqi commented May 26, 2023

And pkg-config is case-sensitive.

Because you used an uppercase package name, it does not exist in the xmake-repo repository. Only the lowercase cli11
If you want pkg-config to find it, you should open a pr to improve the cli11 package to make it better at finding system libraries. Something like this: https://github.com/xmake-io/xmake-repo/blob/a723eaef7144dcd2eec359a171629c82a631882d/packages/l/libxml2/xmake.lua#L21

/home/xaizek/.xmake/repositories/xmake-repo/packages/c/cli11/xmake.lua: package(CLI11) not found!
stack traceback:

Because you used an uppercase package name, it does not exist in the xmake-repo repository. Only the lowercase cli11

@xaizek
Copy link
Contributor Author

xaizek commented May 26, 2023

CLI11 was just an example which demonstrates the bug. The same issue happens with inputproto and anything else for which --cflags --libs produces no output. Look in

if libinfo.links or libinfo.includedirs then

This is why package with no --cflags or --libs output is considered as not present. I tried changing that line, got find_package working, but not add_requires(). Apparently some other place needs modification as well.

@xaizek
Copy link
Contributor Author

xaizek commented May 26, 2023

Or maybe add_requires("some library available via pkg-config") isn't supposed to work? I just thought that it should discover host libraries even when there is no corresponding package in xrepo. It seems to find chafa which isn't present in xrepo and is installed locally, but it's not header-only.

@waruqi
Copy link
Member

waruqi commented May 30, 2023

? I don't know what you mean. If there is no output from pkg-config, xmake cannot find the package.

About, For CLI11, you just need to open pr to improve package("cli11") and then add package:find_package("pkgconfig::CLI11") in on_fetch to improve the lookup rules to look for upper case package names. Or add add_extsources("pkgconfig::CLI11) in package.

@xaizek
Copy link
Contributor Author

xaizek commented May 30, 2023

? I don't know what you mean. If there is no output from pkg-config, xmake cannot find the package.

My point is that it should. Determining presence of a package based solely on the output of pkg-config is wrong because not all packages have the output (they just don't need extra flags). xmake should use exit code of pkg-config which is non-zero if there is no matching package and zero otherwise. Current implementation considers some existing packages as missing because it looks at output instead of exit code (or at least this is part of the problem).

About, For CLI11, you just need to open pr to improve package("cli11") and then add package:find_package("pkgconfig::CLI11") in on_fetch to improve the lookup rules to look for upper case package names. Or add add_extsources("pkgconfig::CLI11) in package.

Given that xmake l find_package CLI11 doesn't find it, I don't expect this change to make any difference.

@waruqi
Copy link
Member

waruqi commented May 30, 2023

My point is that it should. Determining presence of a package based solely on the output of pkg-config is wrong because not all packages have the output (they just don't need extra flags). xmake should use exit code of pkg-config which is non-zero if there is no matching package and zero otherwise. Current implementation considers some existing packages as missing because it looks at output instead of exit code (or at least this is part of the problem).

I haven't come across any available packages without any pkg-config output yet and can't test to verify it for now.

Given that xmake l find_package CLI11 doesn't find it, I don't expect this change to make any difference.

It works for me.

$ xmake l find_package CLI11
{ 
  includedirs = { 
    "/usr/local/include" 
  } 
}

@xaizek
Copy link
Contributor Author

xaizek commented May 30, 2023

It works for me.

I tried installing CLI11 v2.3.2 with /usr/local prefix and pkg-config --cflags CLI11 started to produce:

-I/usr/local/include

I think if you install CLI11 into /usr, there will be not output because pkg-config doesn't print -I/usr/include.

With CLI11 in /usr/local I do get the same output from xmake as you show above, but add_requires("CLI11") still fails, does it work for you?

@waruqi
Copy link
Member

waruqi commented May 30, 2023

With CLI11 in /usr/local I do get the same output from xmake as you show above, but add_requires("CLI11") still fails, does it work for you?

please see the previous reply.

About, For CLI11, you just need to open pr to improve package("cli11") and then add package:find_package("pkgconfig::CLI11") in on_fetch to improve the lookup rules to look for upper case package names. Or add add_extsources("pkgconfig::CLI11) in package.

@xaizek
Copy link
Contributor Author

xaizek commented May 30, 2023

please see the previous reply.

I'm asking about whether add_requires("CLI11") works for you in that it picks up system library, xrepo package is a separate thing.

@waruqi
Copy link
Member

waruqi commented May 30, 2023

please see the previous reply.

I'm asking about whether works for you in that it picks up system library, package is a separate thing.add_requires("CLI11")``xrepo

it can find system library by default. but you should pass lower case package name and improve package("cli11") to add add_extsources("pkgconfig::CLI11") or on_fetch .

package("cli11") -- lower case name
    add_extsources("pkgconfig::CLI11") --- real name (upper case)

or

package("cli11")
    on_fetch(function (package, opt)
        if opt.system then
            return package:find_package("pkgconfig:CLI11")     -- upper case name
        end
    end)

xmake.lua

add_requires("cli11") --- lower case name

please do not use upper case name.

About, For CLI11, you just need to open pr to improve package("cli11") and then add package:find_package("pkgconfig::CLI11") in on_fetch to improve the lookup rules to look for upper case package names. Or add add_extsources("pkgconfig::CLI11) in package.

@waruqi
Copy link
Member

waruqi commented May 30, 2023

So you need open a pr to improve and fix this package.

@SirLynix
Copy link
Member

from my understanding, add_requires is not compatible with uppercase names. if the lib was called cli11 and xrepo had no cli11 package, xmake would have find it by searching in pkgconfig.

The cli11 package should be improved, but I don't understand why uppercases names are an issue here, maybe xmake makes the name lowercase before searching the package using pkgconfig?

@waruqi
Copy link
Member

waruqi commented May 30, 2023

from my understanding, is not compatible with uppercase names. if the lib was called and xrepo had no cli11 package, xmake would have find it by searching in pkgconfig.add_requires``cli11

The cli11 package should be improved, but I don't understand why uppercases names are an issue here, maybe xmake makes the name lowercase before searching the package using pkgconfig?

Because xmake packages are not considered to support uppercase package names, users passing uppercase names to add_requires(), it will get some unknown errors.

add_requires("CLI11") -> xmake found xmake-repo/packages/c/cli11/xmake.lua -> try to load package("CLI11") not found.

if you pass other lower case name, even if it is not in the package, just the system library, it is still well supported.

add_requires("xxxx") -> no packages in xmake-repo -> xmake will find it from system libs . pkg-config, apt, pacman ..

There can be a lot of problems due to differences in case file systems on different systems, so please use lower case package names. xmake will not bother to handle any upper case package names very well either.

@SirLynix
Copy link
Member

Ok now I understand the behavior of xmake.

Maybe xmake shouldn't lowercase the name of packages passed to add_requires?

add_requires("CLI11") => xrepo package not found (cli11 != CLI11) not found => "CLI11" is passed to pkg-config which finds it.

(cli11 is just an example here, I also think the package should be improved and used instead).

@waruqi
Copy link
Member

waruqi commented May 30, 2023

Maybe xmake shouldn't lowercase the name of packages passed to add_requires?

No, the user should always pass the lowercase package name to add_requires.

However, the cli11 package should be improved by adding add_extsources("pkgconfig::CLI11") in upper case

@waruqi
Copy link
Member

waruqi commented May 30, 2023

I also think the package should be improved and used instead

The only thing that should be improved is that if the user passes in an upper case package name, there should be a more friendly error message to indicate how the user should go about improving the package name.

@SirLynix
Copy link
Member

I understand your point of view, however since add_requires is designed to work with other package managers (and not just xrepo), and those package managers may allow uppercase packages, this means that xmake can't find those packages (without requiring to write a dummy xmake package with add_extsources).

@waruqi
Copy link
Member

waruqi commented May 30, 2023

I understand your point of view, however since add_requires is designed to work with other package managers (and not just xrepo), and those package managers may allow uppercase packages, this means that xmake can't find those packages (without requiring to write a dummy xmake package with add_extsources).

add_extsources and on_fetch are required and the most reliable, relying on package names to find them is unreliable, even if upper case is supported.

Also, xmake package names are not necessarily related to the names of third-party repositories, they should not be coupled with each other.

@waruqi
Copy link
Member

waruqi commented May 30, 2023

In addition, the package names of different third-party repositories cannot be completely consistent, and it is impossible to use a fixed package name to be compatible with all other repository packages and system package names.

@SirLynix
Copy link
Member

fair enough.

I made a pull request to improve cli11: xmake-io/xmake-repo#2131

@xaizek
Copy link
Contributor Author

xaizek commented May 30, 2023

Also, xmake package names are not necessarily related to the names of third-party repositories, they should not be coupled with each other.

I thought add_requires() is a more generic interface when prefix is not specified. This explains your insistence on using cli11 name then. Then I should use add_requires("pkgconfig::CLI11") to use system-installed package (xrepo might not be available in package build environment which prohibits network usage, hence I need a way to use an already installed package).

However... If CLI11 is installed to /usr prefix and pkg-config --cflags --libs output is empty, add_requires("pkgconfig::CLI11") doesn't work. Installing it to /usr/local does fix this, but I still think there is a bug in here.

Because xmake packages are not considered to support uppercase package names, users passing uppercase names to add_requires(), it will get some unknown errors.

add_requires("CLI11") -> xmake found xmake-repo/packages/c/cli11/xmake.lua -> try to load package("CLI11") not found.

if you pass other lower case name, even if it is not in the package, just the system library, it is still well supported.

add_requires("xxxx") -> no packages in xmake-repo -> xmake will find it from system libs . pkg-config, apt, pacman ..

There can be a lot of problems due to differences in case file systems on different systems, so please use lower case package names. xmake will not bother to handle any upper case package names very well either.

OK, this inconsistent behaviour is quite confusing because something might work until a corresponding package is added to xrepo.

@waruqi
Copy link
Member

waruqi commented May 31, 2023

However... If CLI11 is installed to /usr prefix and pkg-config --cflags --libs output is empty, add_requires("pkgconfig::CLI11") doesn't work. Installing it to /usr/local does fix this, but I still think there is a bug in here.

I have improved it. #3797

OK, this inconsistent behaviour is quite confusing because something might work until a corresponding package is added to xrepo.

see xmake-io/xmake-repo#2131

please update repository.

$ xrepo update-repo

@waruqi waruqi added this to the v2.8.1 milestone May 31, 2023
waruqi added a commit that referenced this issue May 31, 2023
improve to find package from pkg-config #3777
@xaizek
Copy link
Contributor Author

xaizek commented May 31, 2023

xmake l find_package CLI11 and add_requires("pkgconfig::CLI11") work fine on dev. Thank you.

@xaizek xaizek closed this as completed May 31, 2023
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

3 participants