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

[fontconfig] Fix x86-osx -> arm64-osx cross-compilation #18625

Closed
wants to merge 1 commit into from
Closed

[fontconfig] Fix x86-osx -> arm64-osx cross-compilation #18625

wants to merge 1 commit into from

Conversation

Lucius-Q-User
Copy link
Contributor

Pass host system cflags instead of target system cflags to tools that are supposed to run on host system.
Run host system version of fc-cache instead of target system.

  • What does your PR fix?

  • Which triplets are supported/not supported? Have you updated the CI baseline?

    x86-osx, arm64-osx No

  • Does your PR follow the maintainer guide?

    Yes

  • If you have added/updated a port: Have you run ./vcpkg x-add-version --all and committed the result?

    Yes

If you are still working on the PR, open it as a Draft: https://github.blog/2019-02-14-introducing-draft-pull-requests/

@Neumann-A
Copy link
Contributor

could you add:

if(NOT VCPKG_TARGET_IS_WINDOWS)
    list(APPEND OPTIONS "as_ln_s=ln -sr")
endif()

before vcpkg_configure_make

This also fixes the broken symlinks in fontconfig

@@ -7,7 +7,7 @@ index f0fa0ec50..24e5afd16 100644

$(TOOL): $(TSRC) $(ALIAS_FILES)
- $(AM_V_GEN) $(CC_FOR_BUILD) -o $(TOOL) $< $(AM_CPPFLAGS)
+ $(AM_V_GEN) $(CC_FOR_BUILD) -o $(TOOL) $< $(AM_CPPFLAGS) $(LIBINTL) $(CFLAGS)
+ $(AM_V_GEN) $(CC_FOR_BUILD) -o $(TOOL) $< $(AM_CPPFLAGS) $(LIBINTL) $(CFLAGS_FOR_BUILD)
Copy link
Contributor

Choose a reason for hiding this comment

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

Where does CFLAGS_FOR_BUILD come from, before #15605?

Copy link
Contributor

Choose a reason for hiding this comment

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

@Lucius-Q-User, I didn't find the CFLAGS_FOR_BUILD in fontconfig, so why do you change it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

CFLAGS refers to target system cflags, which can break the build, (e.g -arch flag for clang, or -march/-mcpu flags)

Copy link
Contributor

Choose a reason for hiding this comment

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

CFLAGS_FOR_BUILD might be set for cross builds after #15605, but for now you miss CFLAGS for native builds.
In addition, $(LIBINTL) can go wrong as well for cross builds.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alternatively, we can update the version of fontconfig that is currently in the tree. Newer versions made fc-case, which is the source of cross-compile problems into a python script.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

would it be ok to conditionally include cflags depending on whether we are cross-compiling or not?

@Lucius-Q-User
Copy link
Contributor Author

could you add:

if(NOT VCPKG_TARGET_IS_WINDOWS)
    list(APPEND OPTIONS "as_ln_s=ln -sr")
endif()

before vcpkg_configure_make

This also fixes the broken symlinks in fontconfig

That breaks the build, macos version of ln does not support -r

@PhoebeHui PhoebeHui added the category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist label Jun 25, 2021
@Lucius-Q-User Lucius-Q-User marked this pull request as ready for review June 28, 2021 08:53
@Lucius-Q-User
Copy link
Contributor Author

Looks like simage was broken even before the patch.

@PhoebeHui
Copy link
Contributor

simage failures would be fixed by PR #18645

@JackBoosY JackBoosY requested a review from PhoebeHui July 2, 2021 09:32
@PhoebeHui PhoebeHui added the depends:different-pr This PR or Issue depends on a PR which has been filed label Jul 6, 2021
@PhoebeHui PhoebeHui marked this pull request as draft December 2, 2021 06:26
@PhoebeHui
Copy link
Contributor

@Lucius-Q-User, the dependency PR has been closed, does this PR still be valid? can I close it?

@Lucius-Q-User
Copy link
Contributor Author

The dependency PR was closed because fontconfig was updated via a different PR. Since fontconfig was updated, it now uses a completely different build system (meson instead of automake) and this PR no longer applies.
The cross-compile is unfortunately still broken, but i believe that #21772 should be able to fix it.

@PhoebeHui
Copy link
Contributor

@Lucius-Q-User, thanks for your response! It looks PR #21772 aim to solve the problem.

Closing in favor of #21772.

@PhoebeHui PhoebeHui closed this Jan 12, 2022
@Lucius-Q-User Lucius-Q-User deleted the fix-fontconfig branch February 26, 2022 10:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist depends:different-pr This PR or Issue depends on a PR which has been filed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants