Skip to content

[CMakeToolchain][env] Added PKG_CONFIG_PATH env variable and PKG_CONFIG_EXECUTABLE one - #12513

Merged
czoido merged 11 commits into
conan-io:developfrom
franramirez688:feature/pkg_config_path_var
Nov 16, 2022
Merged

[CMakeToolchain][env] Added PKG_CONFIG_PATH env variable and PKG_CONFIG_EXECUTABLE one#12513
czoido merged 11 commits into
conan-io:developfrom
franramirez688:feature/pkg_config_path_var

Conversation

@franramirez688

@franramirez688 franramirez688 commented Nov 11, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

Changelog: Feature: Added generators folder to PKG_CONFIG_PATH environment variable in CMakeToolchain.
Changelog: Feature: Ensure that CMakeToolchain will enforce usage of pkg-config executable set in tools.gnu:pkg_config config.
Closes: #11962
Partial implementation of #12354
Docs: conan-io/docs#2832

Note: Originally started by #12355

@franramirez688 franramirez688 added this to the 1.55 milestone Nov 11, 2022
@franramirez688
franramirez688 force-pushed the feature/pkg_config_path_var branch from 8ea1999 to 021ae22 Compare November 15, 2022 13:46
franramirez688
franramirez688 commented Nov 15, 2022
View reviewed changes
Comment thread conan/tools/cmake/toolchain/blocks.py Outdated
user_pkg_config_path = os.getenv("PKG_CONFIG_PATH")
if user_pkg_config_path:
# Prepending the Conan one
pkg_config_path = pkg_config_path + os.pathsep + user_pkg_config_path

@franramirez688 franramirez688 Nov 15, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I use this approach from EnvVars?

        self._subsystem = deduce_subsystem(conanfile, scope)

    @property
    def _pathsep(self):
        return ":" if self._subsystem != WINDOWS else ";"

@czoido czoido Nov 15, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think the correct way to do this is to deduce the subsystem and then aplying the path separator according to that

@franramirez688 franramirez688 Nov 15, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The question is: should I assume that scope == "build"? Here, we don't have any scope defined.

czoido
czoido reviewed Nov 15, 2022
View reviewed changes
Comment thread conan/tools/cmake/toolchain/blocks.py Outdated
czoido
czoido reviewed Nov 15, 2022
View reviewed changes
Comment thread conans/test/functional/toolchains/cmake/test_cmake_toolchain.py
memsharded
memsharded approved these changes Nov 15, 2022
View reviewed changes
Comment thread conans/test/functional/toolchains/cmake/test_cmake_toolchain.py
@pytest.mark.tool_pkg_config
def test_cmaketoolchain_and_pkg_config_path():
"""
Lightweight test which is loading a dependency as a *.pc file through

@memsharded memsharded Nov 15, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice job with this test!

czoido
czoido reviewed Nov 16, 2022
View reviewed changes
Comment thread conan/tools/cmake/toolchain/blocks.py Outdated
@franramirez688 franramirez688 mentioned this pull request Nov 16, 2022
Closed
5 tasks
@franramirez688 franramirez688 changed the title [CMakeToolchain][Environment] Added PKG_CONFIG_PATH env variable [CMakeToolchain][Environment] Added PKG_CONFIG_PATH env variable and PKG_CONFIG_EXECUTABLE one Nov 16, 2022
@franramirez688 franramirez688 changed the title [CMakeToolchain][Environment] Added PKG_CONFIG_PATH env variable and PKG_CONFIG_EXECUTABLE one [CMakeToolchain][env] Added PKG_CONFIG_PATH env variable and PKG_CONFIG_EXECUTABLE one Nov 16, 2022
@czoido
czoido merged commit 559bca1 into conan-io:develop Nov 16, 2022
jcar87
jcar87 reviewed Nov 16, 2022
View reviewed changes
Comment thread conan/tools/cmake/toolchain/blocks.py
# hardcoding scope as "build"
subsystem = deduce_subsystem(self._conanfile, "build")
pathsep = ":" if subsystem != WINDOWS else ";"
pkg_config_path = pkg_config_path.replace("\\", "/") + pathsep

@jcar87 jcar87 Nov 16, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this! :D Matches the documentation - and still allows (a few lines above in the template block) to fall back on anything the user may have previously specified

PKG_CONFIG_PATH

A colon-separated (on Windows, semicolon-separated) list of directories to search for .pc files. The default directory will always be searched after searching the path; the default is libdir/pkgconfig:datadir/pkgconfig where libdir is the libdir where pkg-config and datadir is the datadir where pkg-config was installed.

@franramirez688 franramirez688 mentioned this pull request Nov 29, 2022
Merged
@SpaceIm SpaceIm mentioned this pull request Jan 22, 2023
Merged
@hoyhoy

hoyhoy commented Jun 14, 2023 •
edited
Loading

Copy link
Copy Markdown

I'm running conan version 1.60.1 and I'm seeing self.conf.get("tools.gnu:pkg_config", check_type=str) == None when my conan.conf is set with the following.

[tools]
gnu:pkg_config = /usr/local/bin/pkgconf

Also, if I set pkg_config with the conan config command, it corrupts conan.conf.

conan config set tools.gnu:pkg_config=/usr/local/bin/pkgconf

gnu = pkg_config = /usr/local/bin/pkgconf

It only seems to work if I put a [conf] section in my conan profile.

@memsharded

memsharded commented Jun 15, 2023

Copy link
Copy Markdown
Member

Hi @hoyhoy

This is a closed issue from 8 months ago, it would be better to report this as a new ticket.
There are some known issues, if the pkgconf is called at package_info() method it will not work, because environment or conf is not injected that way at that point.

Also [tools] is not a valid section, and conan.conf is not the right place to put it (in any case, it would be global.conf. Likewise gnu:pkg_config is not a conf, but it must be tools.gnu:pkg_config=value, the full string, and in global.conf

@hoyhoy

hoyhoy commented Jun 15, 2023 •
edited
Loading

Copy link
Copy Markdown

Hmmm, conan config list doesn't refer to conan.conf? conan config set absolutely changes that file. Also, self.conf.get("tools.gnu:pkg_config", check_type=str) doesn't refer to conan.conf? If it's global.conf, why does conan config write to conan.conf? I'm not sure how to write an issue. It definitely seems like something is wrong.

@memsharded

memsharded commented Jun 15, 2023

Copy link
Copy Markdown
Member

Hmmm, conan config list doesn't refer to conan.conf? conan config set absolutely changes that file. Also, self.conf.get("tools.gnu:pkg_config", check_type=str) doesn't refer to conan.conf? If it's global.conf, why does conan config write to conan.conf?

No, you are mixing things. In Conan 1.X the two different confs coexist:

  • conan.conf is the old, legacy one. conan config set/update writes to it, but those commands are also removed in Conan 2.0. It contains the legacy configuration items
  • global.conf or profile files are the new ones. conan config list list the possible values, but they should go to global.conf or profiles. tools.gnu:pkg_config is a new configuration listed here, it should be defined in global.conf or profile.

@danimtb danimtb mentioned this pull request Nov 20, 2024
Closed
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jcar87 jcar87 jcar87 left review comments
@czoido czoido czoido approved these changes
@memsharded memsharded memsharded approved these changes

Assignees

@czoido czoido

Labels

None yet

Projects

None yet

Milestone

1.55

Development

Successfully merging this pull request may close these issues.

add the PKG_CONFIG_PATH via CMake build helper or even CMakeToolchain

6 participants

@franramirez688 @hoyhoy @memsharded @jcar87 @czoido @SpaceIm