Bug 265180
| Summary: | www/chromium: actually use the system-installed ICU | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | Ports & Packages | Reporter: | Mikhail T. <freebsd-2024> | ||||||
| Component: | Individual Port(s) | Assignee: | freebsd-chromium (Nobody) <chromium> | ||||||
| Status: | Open --- | ||||||||
| Severity: | Affects Some People | CC: | grahamperrin, rene, robert | ||||||
| Priority: | --- | Flags: | bugzilla:
maintainer-feedback?
(chromium) |
||||||
| Version: | Latest | ||||||||
| Hardware: | Any | ||||||||
| OS: | Any | ||||||||
| Attachments: |
|
||||||||
(In reply to Mikhail T. from comment #0) Due to the fact that built-in ICU has several modifications, I would rather stick to it and remove the dependency from the port. The same goes for all the other deps that are picked up internally instead because there have been several incompatibility issues previously using an external library. (In reply to Robert Nagy from comment #1) > built-in ICU has several modifications If these modifications are good and useful, Google can/should submit them to IBM for inclusion into ICU releases. If IBM are too slow to process the contribution, FreeBSD can incorporate the changes ourselves -- they can be added (as a patch) to our devel/icu -- to benefit ALL of ICU-using software. Plenty of precedent there. To have ports use their own versions -- of ANYTHING -- is not right (see link above). The actual work on unbundling may be substantial -- which would explain, why it hasn't happened yet. But that such unbundling is a goal, is beyond dispute. Chromium port is not the first to stumble into this -- OpenOffice.org, for example, was quite notorious too some years ago. > The same goes for all the other deps that are picked up internally This view is held by a number of people, but FreeBSD Ports have a clearly-stated policy to the contrary: https://docs.freebsd.org/doc/10.1-RELEASE/usr/local/share/doc/freebsd/en/books/porters-handbook/bundled-libs.html Ideally, we'll get to the point, where the entire third_party/ subdirectory is not even extracted from the upstream's tar-ball. > there have been several incompatibility issues previously Frankly, this is FUD :-) (In reply to Mikhail T. from comment #2) > … <https://docs.freebsd.org/doc/10.1-RELEASE/usr/local/share/doc/freebsd/en/books/porters-handbook/bundled-libs.html> … In the current edition of the FreeBSD Porter's Handbook: <https://docs.freebsd.org/en/books/porters-handbook/book/#bundled-libs> Created attachment 235289 [details]
0001-www-chromium-unbundle-icu-to-use-icu-from-ports.patch
I am not against doing this but let's keep it as simple as possible.
Please test the attached patch.
(In reply to Robert Nagy from comment #4) This simple approach is, what I started with. However, this is dangerous -- because the bundled ICU header-files are used, instead of those installed by the icu-port. This is why my proposal includes removal of the bundled ICU... A commit in branch 2022Q3 references this bug: URL: https://cgit.FreeBSD.org/ports/commit/?id=b2193c3c801075a5b0d985d50b2e25dba4c9b6b4 commit b2193c3c801075a5b0d985d50b2e25dba4c9b6b4 Author: Robert Nagy <robert@openbsd.org> AuthorDate: 2022-07-16 10:15:36 +0000 Commit: Rene Ladan <rene@FreeBSD.org> CommitDate: 2022-07-19 19:56:43 +0000 www/chromium: unbundle icu to use icu from ports (cherry picked from commit 55b20cdeb6569e0e3c63d58aedd679901d15dc50) PR: 265180 www/chromium/Makefile | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) The committed change is insufficient -- instead of using the headers installed by devel/icu, the port still uses the bundled ones.
A simple experiment:
1. make patch
2. Edit the ${WRKSRC}/third_party/icu/source/common/unicode/utypes.h to insert the line at the beginning:
#error Meow-meow
3. make
4. Observe the build failing with:
../../third_party/icu/source/common/unicode/utypes.h:1:2: error: Meow-meow
As I mentioned in Comment #5, this -- taking declarations from one source and definitions from another -- is dangerous.
Please, consider using my patch -- or some derivative thereof -- instead.
|
Created attachment 235226 [details] Use the already-installed ICU Though the port lists devel/icu among the LIB_DEPENDS, the upstream code builds the bundled version of ICU anyway... This is true about a number of other bundled components too (AOM, AVIF, jsoncpp, node, v8, etc.), but ICU seems to be the easiest to fix. The proposed patch adds ICU to the existing (incomplete) list of possible dependencies, and, additionally, ensures, the icu/source folder is never even extracted from the tarball. For good measure... The post-patch gymnastics are only necessary because of the is_official_build=true setting -- do we really need it?