Bug 214536 - www/chromium: fails to build with ICU 58.1
Summary: www/chromium: fails to build with ICU 58.1
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: freebsd-chromium (Nobody)
URL:
Keywords: patch, patch-ready
Depends on:
Blocks: 214384
  Show dependency treegraph
 
Reported: 2016-11-15 15:53 UTC by Jan Beich
Modified: 2016-11-19 18:24 UTC (History)
4 users (show)

See Also:


Attachments
patch-chromium-fix-build-icu-58.1 (2.12 KB, patch)
2016-11-15 16:44 UTC, Carlos J. Puga Medina
no flags Details | Diff
fix + commit message (7.13 KB, patch)
2016-11-15 18:42 UTC, Jan Beich
jbeich: maintainer-approval? (chromium)
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jan Beich freebsd_committer freebsd_triage 2016-11-15 15:53:50 UTC
../../components/url_formatter/url_formatter.cc:454:2: error: "Update aspirational_scripts per Unicode 9.0"
#error "Update aspirational_scripts per Unicode 9.0"
 ^
1 error generated.

http://package22.nyi.freebsd.org/data/103i386-default-PR214384/2016-11-14_20h14m36s/logs/errors/chromium-52.0.2743.116_3.log
https://chromium.googlesource.com/chromium/src.git/+/424584b4984bde7c831f42e9fb47f1ad583a1c46%5E%21/
Comment 1 Carlos J. Puga Medina freebsd_committer freebsd_triage 2016-11-15 16:02:44 UTC
(In reply to Jan Beich (mail not working) from comment #0)

We're preparing chromium update to 54.0.2840.100 release.

https://reviews.freebsd.org/D8517

Thanks for the patch, Jan.
Comment 2 Carlos J. Puga Medina freebsd_committer freebsd_triage 2016-11-15 16:44:16 UTC
Created attachment 177026 [details]
patch-chromium-fix-build-icu-58.1
Comment 3 clutton 2016-11-15 16:54:06 UTC
(In reply to Carlos J. Puga Medina from comment #2)

Do we need this patch? The chromium uses hardcoded icu because it needs some features only available in recent one, like emoji.

Either, use the icu from the ports (which is fine) or stick to the bundled one (which I did in 54). Don't remove chromium hardcoding, it was done for a reason.
Comment 4 Carlos J. Puga Medina freebsd_committer freebsd_triage 2016-11-15 17:00:11 UTC
(In reply to clutton from comment #3)

This patch only is needed for current 52.0.2743.116 version into the ports tree.

Meanwhile we launch chromium update to 54... it's needed.
Comment 5 Jan Beich freebsd_committer freebsd_triage 2016-11-15 17:15:23 UTC
Comment on attachment 177026 [details]
patch-chromium-fix-build-icu-58.1

../../third_party/WebKit/Source/platform/text/TextBreakIterator.cpp:182:1: error: static_assert failed "breakAllLineBreakClassTable should be consistent"
static_assert(WTF_ARRAY_LENGTH(breakAllLineBreakClassTable) == U_LB_COUNT, "breakAllLineBreakClassTable should be consistent");
^             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.

https://chromium.googlesource.com/chromium/src.git/+/e60b571faa3f14dd9119a6792dccf12f8bf80192%5E%21/
Comment 6 Jan Beich freebsd_committer freebsd_triage 2016-11-15 18:42:20 UTC
Created attachment 177031 [details]
fix + commit message

Chromium 52 built against ICU 58.1 on 101i386 runs fine on ja.wikipedia.org . Leaving testing build/runtime against ICU 57.1 (current) to someone else.

Note, similar code in Firefox had a security impact.
https://hg.mozilla.org/mozilla-central/rev/c17531742f63
Comment 7 commit-hook freebsd_committer freebsd_triage 2016-11-19 18:20:19 UTC
A commit references this bug:

Author: jbeich
Date: Sat Nov 19 18:20:13 UTC 2016
New revision: 426523
URL: https://svnweb.freebsd.org/changeset/ports/426523

Log:
  www/chromium: unbreak against ICU 58.1

  components/url_formatter/url_formatter.cc:454:2: error: "Update aspirational_scripts per Unicode 9.0"
   ^
  third_party/WebKit/Source/platform/text/TextBreakIterator.cpp:182:1: error: static_assert failed "breakAllLineBreakClassTable should be consistent"
  static_assert(WTF_ARRAY_LENGTH(breakAllLineBreakClassTable) == U_LB_COUNT, "breakAllLineBreakClassTable should be consistent");
  ^             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

  PR:		214536
  Submitted by:	cpm (based on)
  Obtained from:	upstream
  Approved by:	portmgr blanket

Changes:
  head/www/chromium/Makefile
  head/www/chromium/files/patch-components_url__formatter_url__formatter.cc
  head/www/chromium/files/patch-third__party_WebKit_Source_platform_text_TextBreakIterator.cpp