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

[bug] PkgConfigDeps: -F <frameworkdir> on macOS leads to "clang: error: no such file or directory:" at link time #11867

Closed
SpaceIm opened this issue Aug 14, 2022 · 15 comments · Fixed by #12307
Assignees
Milestone

Comments

@SpaceIm
Copy link
Contributor

SpaceIm commented Aug 14, 2022

Environment Details (include every applicable attribute)

  • Operating System+version: macOS Monterey
  • Compiler+version: AppleClang 13
  • Conan version: 1.51.2
  • Python version: 3.9.13

Steps to reproduce (Include if Applicable)

glib build system is Meson, but it can be reproduced as well with a simple CMakeLists relying on pkg_check_modules() and a shared dependency (not tried with static).

If I tweak this PR by replacing PkgConfigDeps by pkg_config generator (also need to move these .pc files in generator folder expected by layout), the build succeeds. It fails afterwards in test_package since it also relies on PkgConfigDeps (with CMake build system).

Logs (Executed commands with output) (Include/Attach if Applicable)

[257/514] Linking target glib/libglib-2.0.0.dylib
FAILED: glib/libglib-2.0.0.dylib
clang  -o glib/libglib-2.0.0.dylib glib/libglib-2.0.0.dylib.p/deprecated_gallocator.c.o glib/libglib-2.0.0.dylib.p/deprecated_gcache.c.o glib/libglib-2.0.0.dylib.p/deprecated_gcompletion.c.o glib/libglib-2.0.0.dylib.p/deprecated_grel.c.o glib/libglib-2.0.0.dylib.p/deprecated_gthread-deprecated.c.o glib/libglib-2.0.0.dylib.p/garcbox.c.o glib/libglib-2.0.0.dylib.p/garray.c.o glib/libglib-2.0.0.dylib.p/gasyncqueue.c.o glib/libglib-2.0.0.dylib.p/gatomic.c.o glib/libglib-2.0.0.dylib.p/gbacktrace.c.o glib/libglib-2.0.0.dylib.p/gbase64.c.o glib/libglib-2.0.0.dylib.p/gbitlock.c.o glib/libglib-2.0.0.dylib.p/gbookmarkfile.c.o glib/libglib-2.0.0.dylib.p/gbytes.c.o glib/libglib-2.0.0.dylib.p/gcharset.c.o glib/libglib-2.0.0.dylib.p/gchecksum.c.o glib/libglib-2.0.0.dylib.p/gconvert.c.o glib/libglib-2.0.0.dylib.p/gdataset.c.o glib/libglib-2.0.0.dylib.p/gdate.c.o glib/libglib-2.0.0.dylib.p/gdatetime.c.o glib/libglib-2.0.0.dylib.p/gdir.c.o glib/libglib-2.0.0.dylib.p/genviron.c.o glib/libglib-2.0.0.dylib.p/gerror.c.o glib/libglib-2.0.0.dylib.p/gfileutils.c.o glib/libglib-2.0.0.dylib.p/ggettext.c.o glib/libglib-2.0.0.dylib.p/ghash.c.o glib/libglib-2.0.0.dylib.p/ghmac.c.o glib/libglib-2.0.0.dylib.p/ghook.c.o glib/libglib-2.0.0.dylib.p/ghostutils.c.o glib/libglib-2.0.0.dylib.p/giochannel.c.o glib/libglib-2.0.0.dylib.p/gkeyfile.c.o glib/libglib-2.0.0.dylib.p/glib-init.c.o glib/libglib-2.0.0.dylib.p/glib-private.c.o glib/libglib-2.0.0.dylib.p/glist.c.o glib/libglib-2.0.0.dylib.p/gmain.c.o glib/libglib-2.0.0.dylib.p/gmappedfile.c.o glib/libglib-2.0.0.dylib.p/gmarkup.c.o glib/libglib-2.0.0.dylib.p/gmem.c.o glib/libglib-2.0.0.dylib.p/gmessages.c.o glib/libglib-2.0.0.dylib.p/gnode.c.o glib/libglib-2.0.0.dylib.p/goption.c.o glib/libglib-2.0.0.dylib.p/gpattern.c.o glib/libglib-2.0.0.dylib.p/gpoll.c.o glib/libglib-2.0.0.dylib.p/gprimes.c.o glib/libglib-2.0.0.dylib.p/gqsort.c.o glib/libglib-2.0.0.dylib.p/gquark.c.o glib/libglib-2.0.0.dylib.p/gqueue.c.o glib/libglib-2.0.0.dylib.p/grand.c.o glib/libglib-2.0.0.dylib.p/grcbox.c.o glib/libglib-2.0.0.dylib.p/grefcount.c.o glib/libglib-2.0.0.dylib.p/grefstring.c.o glib/libglib-2.0.0.dylib.p/gregex.c.o glib/libglib-2.0.0.dylib.p/gscanner.c.o glib/libglib-2.0.0.dylib.p/gsequence.c.o glib/libglib-2.0.0.dylib.p/gshell.c.o glib/libglib-2.0.0.dylib.p/gslice.c.o glib/libglib-2.0.0.dylib.p/gslist.c.o glib/libglib-2.0.0.dylib.p/gstdio.c.o glib/libglib-2.0.0.dylib.p/gstrfuncs.c.o glib/libglib-2.0.0.dylib.p/gstring.c.o glib/libglib-2.0.0.dylib.p/gstringchunk.c.o glib/libglib-2.0.0.dylib.p/gstrvbuilder.c.o glib/libglib-2.0.0.dylib.p/gtestutils.c.o glib/libglib-2.0.0.dylib.p/gthread.c.o glib/libglib-2.0.0.dylib.p/gthreadpool.c.o glib/libglib-2.0.0.dylib.p/gtimer.c.o glib/libglib-2.0.0.dylib.p/gtimezone.c.o glib/libglib-2.0.0.dylib.p/gtrace.c.o glib/libglib-2.0.0.dylib.p/gtranslit.c.o glib/libglib-2.0.0.dylib.p/gtrashstack.c.o glib/libglib-2.0.0.dylib.p/gtree.c.o glib/libglib-2.0.0.dylib.p/guniprop.c.o glib/libglib-2.0.0.dylib.p/gutf8.c.o glib/libglib-2.0.0.dylib.p/gunibreak.c.o glib/libglib-2.0.0.dylib.p/gunicollate.c.o glib/libglib-2.0.0.dylib.p/gunidecomp.c.o glib/libglib-2.0.0.dylib.p/guri.c.o glib/libglib-2.0.0.dylib.p/gutils.c.o glib/libglib-2.0.0.dylib.p/guuid.c.o glib/libglib-2.0.0.dylib.p/gvariant.c.o glib/libglib-2.0.0.dylib.p/gvariant-core.c.o glib/libglib-2.0.0.dylib.p/gvariant-parser.c.o glib/libglib-2.0.0.dylib.p/gvariant-serialiser.c.o glib/libglib-2.0.0.dylib.p/gvarianttypeinfo.c.o glib/libglib-2.0.0.dylib.p/gvarianttype.c.o glib/libglib-2.0.0.dylib.p/gversion.c.o glib/libglib-2.0.0.dylib.p/gwakeup.c.o glib/libglib-2.0.0.dylib.p/gprintf.c.o glib/libglib-2.0.0.dylib.p/glib-unix.c.o glib/libglib-2.0.0.dylib.p/gspawn.c.o glib/libglib-2.0.0.dylib.p/giounix.c.o glib/libglib-2.0.0.dylib.p/gosxutils.m.o glib/libglib-2.0.0.dylib.p/gthread-posix.c.o -Wl,-dead_strip_dylibs -Wl,-headerpad_max_install_names -Wl,-undefined,error -shared -install_name @rpath/libglib-2.0.0.dylib -compatibility_version 7304 -current_version 7304.0 -arch x86_64 -Wl,-rpath,@loader_path/../subprojects/proxy-libintl -Wl,-rpath,/Users/spaceim/.conan/data/pcre2/10.40/_/_/package/58c75cc50ae61b8beaa095a6c3595e162e692eab/lib -Wl,-rpath,/Users/spaceim/.conan/data/zlib/1.2.12/_/_/package/a7fb9a1b45a50aff01ad6102760c848ff2ac80df/lib -Wl,-rpath,/Users/spaceim/.conan/data/bzip2/1.0.8/_/_/package/3dae332ff311e1d3bde4193d43b68fb94dbb4706/lib glib/libcharset/libcharset.a subprojects/proxy-libintl/libintl.8.dylib /Users/spaceim/.conan/data/pcre2/10.40/_/_/package/58c75cc50ae61b8beaa095a6c3595e162e692eab/lib/libpcre2-8.dylib -F /Users/spaceim/.conan/data/pcre2/10.40/_/_/package/58c75cc50ae61b8beaa095a6c3595e162e692eab/Frameworks /Users/spaceim/.conan/data/zlib/1.2.12/_/_/package/a7fb9a1b45a50aff01ad6102760c848ff2ac80df/lib/libz.dylib /Users/spaceim/.conan/data/zlib/1.2.12/_/_/package/a7fb9a1b45a50aff01ad6102760c848ff2ac80df/Frameworks /Users/spaceim/.conan/data/bzip2/1.0.8/_/_/package/3dae332ff311e1d3bde4193d43b68fb94dbb4706/lib/libbz2.dylib -liconv -framework Foundation -framework CoreFoundation -framework AppKit -framework Carbon -lm
clang: error: no such file or directory: '/Users/spaceim/.conan/data/zlib/1.2.12/_/_/package/a7fb9a1b45a50aff01ad6102760c848ff2ac80df/Frameworks'
@lasote lasote self-assigned this Aug 16, 2022
@franramirez688 franramirez688 self-assigned this Aug 16, 2022
@lasote lasote removed their assignment Aug 16, 2022
@franramirez688 franramirez688 added this to the 1.52 milestone Aug 17, 2022
@franramirez688
Copy link
Contributor

Hi @SpaceIm

Thanks for reporting the issue!

In a nutshell, the explanation is as follows:

  • The legacy pkg_config is not failing because it was using the old cpp_info.framework_paths, and those paths were being filtered automatically by Conan:
    @property
    def framework_paths(self):
        if self._framework_paths is None:
            self._framework_paths = self._filter_paths(self.frameworkdirs)
        return self._framework_paths
  • The new PkgConfigDeps is using the cpp_info.frameworkdirs because of the deprecation of framework_paths, so all the new cpp_info.***dirs are not filtered at all. That's the reason why it's coming with the default value.

Given that, the remaining question is, why is cpp_info.frameworkdirs coming with a default value? Because Conan 1.x puts it to Frameworks by default all the time.

So, I think that the workaround could come in two ways:

  • Defining an empty value self.cpp_info.frameworkdirs = [] in all the recipes.
            def package_info(self):
                self.cpp_info.frameworkdirs = []
  • Also, having a layout declared in all the dependencies' recipes. In that way, the default values for cpp_info.***dirs are None.

I was wondering if it could be possible to declare in Conan 1.x frameworkdirs = [] by default, but I guess it's a breaking change, so that's why it is in Conan 2.x.

@SpaceIm
Copy link
Contributor Author

SpaceIm commented Aug 17, 2022

  • Defining an empty value self.cpp_info.frameworkdirs = [] in all the recipes.
            def package_info(self):
                self.cpp_info.frameworkdirs = []

It means updating 1300+ recipes in conan-center :/

  • Also, having a layout declared in all the dependencies' recipes. In that way, the default values for cpp_info.***dirs are None.

zlib recipe has a layout: cmake_layout. If you mean some custom layout, it's like first suggestion, updating 1300+ recipes. Moreover, it's unnecessary boilerplate, and very error prone.

@lasote
Copy link
Contributor

lasote commented Aug 18, 2022

It means updating 1300+ recipes in conan-center :/

We can start with the recipes causing failures, I bet there won't be 1300.

The recipes were always declaring by default self.cpp_info.frameworkdirs = ["Frameworks"] in Conan 1.X, and the generators of 2.X will behave following that information without tricks (like skip non-existing folders that caused many many issues over time). We cannot change the 1.X behavior, we would need to revert it while breaking thousands of users in the companies.

@SpaceIm
Copy link
Contributor Author

SpaceIm commented Aug 18, 2022

It means updating 1300+ recipes in conan-center :/

We can start with the recipes causing failures, I bet there won't be 1300.

All recipes potentially cause failures for consumers, since it affects a generator. For the specific case of glib recipe migration to conan v2, indeed we have to fix few recipes only (at least pcre2, pcre & zlib). But hardcoding frameworksdirs = [] in all recipes is quite strange, since it's not the default behavior to create bundles.

I still don't understand: Do we need to hardcode self.cpp_info.frameworksdirs = [] in all recipes as a workaround to fix .pc files generated by PkgConfifDeps in conan v1 mode? Is it still necessary in conan v2 mode?

@lasote
Copy link
Contributor

lasote commented Aug 18, 2022

But hardcoding frameworksdirs = [] in all recipes is quite strange, since it's not the default behavior to create bundles.

I don't understand what you mean. For me it is not strange at all because Conan 1.X always had a default to frameworksdirs = ["Frameworks"] and it most of the recipes that is a bug because that directory doesn't even exist.

I still don't understand: Do we need to hardcode self.cpp_info.frameworksdirs = [] in all recipes as a workaround to fix .pc files generated by PkgConfifDeps in conan v1 mode? Is it still necessary in conan v2 mode?

It is needed because the consumers using v2 generators won't skip the non-existing folders.
In Conan v2 (or recipes using the layout()) won't need it because in that case the default would be self.cpp_info.frameworksdirs = [] that felt a better default, so we changed in 2.0 mode

@SpaceIm
Copy link
Contributor Author

SpaceIm commented Aug 18, 2022

In Conan v2 (or recipes using the layout()) won't need it because in that case the default would be self.cpp_info.frameworksdirs = [] that felt a better default, so we changed in 2.0 mode

When you say "In Conan v2 (OR recipes using the layout())", does it mean that in conan v1 mode, a recipe with a layout like cmake_layout shouldn't populate frameworksdirs by default? Because latest RREV of zlib has a cmake_layout, but still, its frameworksdirs is not empty.

@lasote
Copy link
Contributor

lasote commented Aug 18, 2022

Not True. With Conan 1.51.3, zlib/1.2.12 with layout, using CMakeDeps, and exploring the xxx-data.cmake:

set(zlib_FRAMEWORK_DIRS_RELEASE )

Without layout():

set(zlib_FRAMEWORK_DIRS_RELEASE "${zlib_PACKAGE_FOLDER_RELEASE}/Frameworks")

@lasote lasote removed this from the 1.52 milestone Aug 18, 2022
@SpaceIm
Copy link
Contributor Author

SpaceIm commented Aug 18, 2022

Indeed, I've checked in the latest log of conan-io/conan-center-index#12221, and I don't see zlib frameworks dirs injection anymore (I see bzip2 & pcre2 Frameworks dir. bzip2 recipe has been updated 2 days ago, so it should be fine now. Remains pcre2 recipe to update).
So the proper fix for conan v2 migration is to fix recipes of dependencies to use layout() (/cc @SSE4 @uilianries)

@SSE4
Copy link
Contributor

SSE4 commented Aug 18, 2022

@franramirez688 I think we talked about it before. I have experienced the same problem already in conan-io/conan-center-index#11139

@SpaceIm
Copy link
Contributor Author

SpaceIm commented Aug 18, 2022

@SSE4 Maybe a CCI hook should be implemented to enforce layout() in CCI recipes?

@SSE4
Copy link
Contributor

SSE4 commented Aug 18, 2022

yes, I think it can be enforced. I believe we already have layouts for most of the build helpers (AutTools, CMake, MSBuild and Meson). for others, hand-crafted layout could be used.

@lasote
Copy link
Contributor

lasote commented Aug 19, 2022

Note: In case you consider it. To activate the "2.0" mode an empty layout() method is enough.

@SSE4
Copy link
Contributor

SSE4 commented Aug 22, 2022

Note: In case you consider it. To activate the "2.0" mode an empty layout() method is enough.

for the record (probably, irrelevant for this issue), this behavior might be consuming for newcomers: having an empty method doing essentially nothing, but has some important side-effects. reference also currently says nothing about that "side-effect" :https://docs.conan.io/en/latest/reference/conanfile/methods.html#layout
I personally would prefer something more explicit (like attribute or method to activate new mode), but it's probably too late to change. so at least, it would be nice to reflect in reference documentation.

@theartful
Copy link
Contributor

theartful commented Oct 14, 2022

I don't think the bug here is what it seems it is. Simply providing non-existing directory for "-F" won't cause an error. The bug here is that "-F" flag and the proceeding directory are treated as different.
Specifically this part:

-F /Users/spaceim/.conan/data/pcre2/10.40/_/_/package/58c75cc50ae61b8beaa095a6c3595e162e692eab/Frameworks
/Users/spaceim/.conan/data/zlib/1.2.12/_/_/package/a7fb9a1b45a50aff01ad6102760c848ff2ac80df/lib/libz.dylib
/Users/spaceim/.conan/data/zlib/1.2.12/_/_/package/a7fb9a1b45a50aff01ad6102760c848ff2ac80df/Frameworks

This should be

-F /Users/spaceim/.conan/data/pcre2/10.40/_/_/package/58c75cc50ae61b8beaa095a6c3595e162e692eab/Frameworks
/Users/spaceim/.conan/data/zlib/1.2.12/_/_/package/a7fb9a1b45a50aff01ad6102760c848ff2ac80df/lib/libz.dylib
-F /Users/spaceim/.conan/data/zlib/1.2.12/_/_/package/a7fb9a1b45a50aff01ad6102760c848ff2ac80df/Frameworks

The first line will only cause a warning, but the third line will cause the error, because it should be preceeded by "-F" flag. This happens because autotools (in my case with imagemagick), pkgconfig and apparently meson reorder the flags and remove duplicates.
I opened this PR here to solve it
#12307

@theartful
Copy link
Contributor

In my case, doing a workaround patch like this, which just removed the space in between fixed the issue:

        if self.settings.os == "Macos":
            # FIXME workaround bug with pkgconfig files and apple frameworks
            # see https://github.com/conan-io/conan/pull/12307
            for pc in glob.glob(os.path.join(self.generators_folder, "*.pc")):
                files.replace_in_file(self, pc, "-F ", "-F", strict=False)
            files.replace_in_file(self,
                                  os.path.join(self.generators_folder,
                                               "conanautotoolsdeps.sh"),
                                  "-F ", "-F", strict=False)

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

Successfully merging a pull request may close this issue.

5 participants