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

icu: small fixes to support Conan 2.0 #15821

Merged

Conversation

jcar87
Copy link
Contributor

@jcar87 jcar87 commented Feb 8, 2023

Specify library name and version: icu/all

Summary of changes

  • Use str(self.ref) to support cross-building in Conan 2
  • Compose build and host triplets directly when cross-building for iOS/watchOS/tvOS, while fixing an issue cross-building from an Apple Silicon mac (the build/host triplets would be the same and the scripts were not correctly detecting cross compilation).
  • Remove logic for cross-building on Windows as the issue mentioned in the comments is now resolved in 1.57. This, however, still requires a small nudge to the build scripts, which is specific to the icu build system (telling the build scripts that the mh-msys-msvc config fragment is needed).

Other changes

  • Clean-up logic to check that the version of Visual Studio/msvc is the minimum required to pass the -FS flag - this is now possible with check_min_vs when passing raise_invalid=False.
  • Remove older versions not referenced elsewhere in Conan Center

Manual testing, both working correctly:

  • Conan 2.0: native build on macOS and cross-build to iOS with arm64 and armv7s
  • Conan 1.x: Windows, native build and cross-build to ARM64

@ghost
Copy link

ghost commented Feb 8, 2023

I detected other pull requests that are modifying icu/all recipe:

This message is automatically generated by https://github.com/ericLemanissier/conan-center-conflicting-prs so don't hesitate to report issues/improvements there.

Comment on lines +132 to +133
host_triplet = f"{str(self.settings.arch)}-apple-darwin"
build_triplet = f"{str(self._settings_build.arch)}-apple"
Copy link
Contributor

Choose a reason for hiding this comment

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

Are they valid triplet? I mean if arch is armv8, triplet should be aarch64-apple not armv8-apple.

Copy link
Contributor

Choose a reason for hiding this comment

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

I've tested and icu doesn't seem to complain.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed - in a config.sub this is likely to be appear like:

| arm | arm[lb]e | arme[lb] | armv* \

so it will work as long as the build scripts don't have further conditionals on the specific arch part of the triple. ICU doesn't seem too bothered. Even when building natively on apple silicon, the arch is autodetected as arm (and not aarch64 like a more modern config.sub would do).

@conan-center-bot
Copy link
Collaborator

Conan v1 pipeline ✔️

All green in build 1 (f4e2ec7bd692d68284caee707008365e8d1e8054):

  • icu/71.1@:
    All packages built successfully! (All logs)

  • icu/69.1@:
    All packages built successfully! (All logs)

  • icu/72.1@:
    All packages built successfully! (All logs)

  • icu/70.1@:
    All packages built successfully! (All logs)

  • icu/68.2@:
    All packages built successfully! (All logs)


Conan v2 pipeline (informative, not required for merge) ✔️

Note: Conan v2 builds are informative and they are not required for the PR to be merged.

All green in build 1 (f4e2ec7bd692d68284caee707008365e8d1e8054):

  • icu/68.2@:
    All packages built successfully! (All logs)

  • icu/70.1@:
    All packages built successfully! (All logs)

  • icu/72.1@:
    All packages built successfully! (All logs)

  • icu/69.1@:
    All packages built successfully! (All logs)

  • icu/71.1@:
    All packages built successfully! (All logs)

@conan-center-bot conan-center-bot merged commit 029722d into conan-io:master Feb 9, 2023
@jcar87 jcar87 deleted the lcc/maintenance/icu-v2-small-fixes branch February 9, 2023 11:52
sabelka pushed a commit to sabelka/conan-center-index that referenced this pull request Feb 12, 2023
* icu: fix issues with Conan 2

* icu: remove unnecessary line
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants