icu: validate dat package file exists #14980

System-Arch · 2022-12-28T00:00:59Z

Specify library name and version: icu71.1

Addressed some issues that were preventing ICU use with Conan 2.0 as noted in #14979 and #12888

I've read the contributing guidelines.
I've followed the PEP8 style guides for Python code in the recipes.
I've used the latest Conan client version.
I've tried at least one configuration locally with the conan-center hook activated.

System-Arch · 2022-12-28T01:06:09Z

These changes now build on a sufficiently new version of Conan 2.0 (that is to say, newer that what CCI is presently running) but they fail the v2 migration lint checks because they rely on _get_gnu_triplet, which is not part of the published V2 API. While there are various threads on related topics (e.g., conan-io/conan#12546 and conan-io/conan#7565), it's not obvious (at least to me) that the solutions to these issues will obviate the need expressed in this recipe.

prince-chrismc

Two small question, I am not sure about the private import 🤔 so I asked the team to see since it's planned for the next release

recipes/icu/all/conanfile.py

prince-chrismc · 2023-01-04T21:33:52Z

recipes/icu/all/conanfile.py

 from conan.tools.env import Environment, VirtualBuildEnv
 from conan.tools.files import apply_conandata_patches, copy, export_conandata_patches, get, mkdir, rename, replace_in_file, rm, rmdir, save
 from conan.tools.gnu import Autotools, AutotoolsToolchain
+from conan.tools.gnu.get_gnu_triplet import _get_gnu_triplet # Ugly but not currently exported in Conan 2.0


conan-io/conan#12546 is marked for 1.57 so I poked the dev team to see what might come of it 🤞

recipes/icu/all/conanfile.py

Co-authored-by: Chris Mc <prince.chrismc@gmail.com>

…nan:conan-io#12884

Nekto89 · 2023-01-19T11:38:23Z

does this fix ios on mac m1?

recipes/icu/all/conanfile.py

Co-authored-by: Marian Klymov <nekto1989@gmail.com>

recipes/icu/all/conanfile.py

Co-authored-by: Marian Klymov <nekto1989@gmail.com>

ghost · 2023-02-06T14:27:36Z

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

icu: small fixes to support Conan 2.0 #15821

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

recipes/icu/all/conanfile.py

prince-chrismc

Few extra infos are there

recipes/icu/all/conanfile.py

prince-chrismc · 2023-02-08T05:30:22Z

recipes/icu/all/conanfile.py


-required_conan_version = ">=1.53.0"
+required_conan_version = ">=1.54.0"


Suggested change

required_conan_version = ">=1.54.0"

required_conan_version = ">=1.57.0"

prince-chrismc

3 tiny comments and FYI there's merge conflicts but otherwise this looks really good 👍

recipes/icu/all/conanfile.py

prince-chrismc · 2023-02-08T19:14:19Z

there's merge conflict and @jcar87 has a conflicting PR that's green just FYI

prince-chrismc · 2023-02-10T19:41:56Z

Trying to fix this 🤞

(its not used in CCI so lets ignore it)

prince-chrismc

I'm perfect

SpaceIm · 2023-02-10T21:20:37Z

recipes/icu/all/conanfile.py

+    def validate(self):
+        if self.options.dat_package_file:
+            if not os.path.exists(self.options.dat_package_file):
+                raise ConanInvalidConfiguration("Non-existent dat_package_file specified")


if it didn't exist it would fail in package_id() before hand anyway.

conan-center-bot · 2023-02-11T22:33:06Z

Conan v1 pipeline ✔️

All green in build 21 (eb31859b03d2433972d232ac896b79fa9132fb88):

icu/72.1@:
All packages built successfully! (All logs)
icu/69.1@:
All packages built successfully! (All logs)
icu/68.2@:
All packages built successfully! (All logs)
icu/71.1@:
All packages built successfully! (All logs)
icu/70.1@:
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 22 (eb31859b03d2433972d232ac896b79fa9132fb88):

icu/72.1@:
All packages built successfully! (All logs)
icu/71.1@:
All packages built successfully! (All logs)
icu/69.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)

recipes/icu/all/conanfile.py

SpaceIm · 2023-02-13T01:13:13Z

Not sure to understand the new scope of this PR now. As I said, this check is useless because if dat package file was not an existing file, it would fail anyway in package_id() which is called before validate().

ICU Conan 2.0 compatibility tweaks

f640d5b

conan-center-bot added the Failed label Dec 28, 2022

This comment has been minimized.

Sign in to view

System-Arch added 3 commits December 27, 2022 19:50

Fix lint issues

5af9fd6

Fixed typo

803ad74

Removed unused import

5069d3b

This comment has been minimized.

Sign in to view

Try dropping back to 1.54

e9bfc9f

This comment has been minimized.

Sign in to view

prince-chrismc reviewed Jan 4, 2023

View reviewed changes

Apply suggestions from code review

ccd3fa3

Co-authored-by: Chris Mc <prince.chrismc@gmail.com>

This comment has been minimized.

Sign in to view

Removed workarounds for conan-io/conan:conan-io#12642 and conan-io/co…

a7ebe4d

…nan:conan-io#12884

This comment has been minimized.

Sign in to view

Nekto89 reviewed Jan 26, 2023

View reviewed changes

recipes/icu/all/conanfile.py Outdated Show resolved Hide resolved

Update file hash per code review feedback

395647a

Co-authored-by: Marian Klymov <nekto1989@gmail.com>

This comment has been minimized.

Sign in to view

Nekto89 reviewed Jan 28, 2023

View reviewed changes

recipes/icu/all/conanfile.py Outdated Show resolved Hide resolved

Fix argument to open()

f735e10

Co-authored-by: Marian Klymov <nekto1989@gmail.com>

This comment has been minimized.

Sign in to view

self.version.major needs parens in Conan 1.x

8f1285d

conan-center-bot added Unexpected Error and removed Failed labels Jan 28, 2023

This comment has been minimized.

Sign in to view

conan-center-bot added Failed and removed Unexpected Error labels Jan 28, 2023

This comment has been minimized.

Sign in to view

Revert use of version.major until 1.x support is dropped

080af98

This was referenced Feb 6, 2023

[service] Potentially conflicting recipes #3571

Closed

icu: modernize more for conan v2 #15778

Merged

SpaceIm reviewed Feb 6, 2023

View reviewed changes

recipes/icu/all/conanfile.py Outdated Show resolved Hide resolved

SpaceIm reviewed Feb 6, 2023

View reviewed changes

recipes/icu/all/conanfile.py Outdated Show resolved Hide resolved

prince-chrismc reviewed Feb 8, 2023

View reviewed changes

recipes/icu/all/conanfile.py Outdated Show resolved Hide resolved

recipes/icu/all/conanfile.py Outdated Show resolved Hide resolved

prince-chrismc reviewed Feb 8, 2023

View reviewed changes

SSE4 reviewed Feb 8, 2023

View reviewed changes

recipes/icu/all/conanfile.py Outdated Show resolved Hide resolved

ghost mentioned this pull request Feb 8, 2023

icu: small fixes to support Conan 2.0 #15821

Merged

prince-chrismc marked this pull request as draft February 10, 2023 19:41

Chris Mc and others added 4 commits February 10, 2023 11:42

Merge branch 'master' into System-Arch-icu-1

a46b1d2

Apply suggestions from code review

6b37ff9

missing import and leave old cross build stuff

7fde1d5

(its not used in CCI so lets ignore it)

missing import

eb31859

prince-chrismc changed the title ~~ICU Conan 2.0 compatibility tweaks~~ icu: validate dat package file exists Feb 10, 2023

prince-chrismc marked this pull request as ready for review February 10, 2023 19:46

prince-chrismc approved these changes Feb 10, 2023

View reviewed changes

SpaceIm reviewed Feb 10, 2023

View reviewed changes

conan-center-bot added the Unexpected Error label Feb 11, 2023

This comment has been minimized.

Sign in to view

conan-center-bot removed the Unexpected Error label Feb 11, 2023

SpaceIm reviewed Feb 11, 2023

View reviewed changes

recipes/icu/all/conanfile.py Show resolved Hide resolved

prince-chrismc approved these changes Feb 12, 2023

View reviewed changes

jwillikers approved these changes Feb 14, 2023

View reviewed changes

conan-center-bot merged commit 5f9985b into conan-io:master Feb 14, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

icu: validate dat package file exists #14980

icu: validate dat package file exists #14980

System-Arch commented Dec 28, 2022

This comment has been minimized.

System-Arch commented Dec 28, 2022

This comment has been minimized.

This comment has been minimized.

prince-chrismc left a comment

prince-chrismc Jan 4, 2023

This comment has been minimized.

This comment has been minimized.

Nekto89 commented Jan 19, 2023

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

ghost commented Feb 6, 2023 •

edited by ghost

Loading

prince-chrismc left a comment

prince-chrismc Feb 8, 2023

prince-chrismc left a comment

prince-chrismc commented Feb 8, 2023

prince-chrismc commented Feb 10, 2023

prince-chrismc left a comment

SpaceIm Feb 10, 2023

This comment has been minimized.

conan-center-bot commented Feb 11, 2023

SpaceIm commented Feb 13, 2023


		required_conan_version = ">=1.53.0"
		required_conan_version = ">=1.54.0"

	required_conan_version = ">=1.54.0"
	required_conan_version = ">=1.57.0"

icu: validate dat package file exists #14980

icu: validate dat package file exists #14980

Conversation

System-Arch commented Dec 28, 2022

This comment has been minimized.

System-Arch commented Dec 28, 2022

This comment has been minimized.

This comment has been minimized.

prince-chrismc left a comment

Choose a reason for hiding this comment

prince-chrismc Jan 4, 2023

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

Nekto89 commented Jan 19, 2023

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

ghost commented Feb 6, 2023 • edited by ghost Loading

prince-chrismc left a comment

Choose a reason for hiding this comment

prince-chrismc Feb 8, 2023

Choose a reason for hiding this comment

prince-chrismc left a comment

Choose a reason for hiding this comment

prince-chrismc commented Feb 8, 2023

prince-chrismc commented Feb 10, 2023

prince-chrismc left a comment

Choose a reason for hiding this comment

SpaceIm Feb 10, 2023

Choose a reason for hiding this comment

This comment has been minimized.

conan-center-bot commented Feb 11, 2023

Conan v1 pipeline ✔️

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

SpaceIm commented Feb 13, 2023

ghost commented Feb 6, 2023 •

edited by ghost

Loading