Unify freetype2 and freetype-android to one checkout |
||||
Issue descriptionWe have third_party/freetype2 as well as third_party/freetype-android. We should unify those, which has become possible after the freetype upgrade to 2.7.1 for Linux content shell.
,
Feb 28 2017
,
Mar 1 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4db6eb187710a109f98606588f4fa53b501e15ba commit 4db6eb187710a109f98606588f4fa53b501e15ba Author: drott <drott@chromium.org> Date: Wed Mar 01 12:30:06 2017 Unify freetype-android and freetype2 into one checkout Removing freetype2 for now, later renaming freetype-android to something more generic. Moving freetype-android from source_set to a component target so that it's generally usable in the other configurations. This should not change much on Android where component resolves to static_library. Also, adding myself to freetype-android OWNERS. TBR'ing jochen for the removal, as presubmit requests an OWNER. BUG= 697015 TBR=jochen Review-Url: https://codereview.chromium.org/2720823003 Cr-Commit-Position: refs/heads/master@{#453918} [modify] https://crrev.com/4db6eb187710a109f98606588f4fa53b501e15ba/DEPS [modify] https://crrev.com/4db6eb187710a109f98606588f4fa53b501e15ba/content/shell/BUILD.gn [modify] https://crrev.com/4db6eb187710a109f98606588f4fa53b501e15ba/third_party/WebKit/LayoutTests/platform/linux/fast/text/emoji-web-font-expected.png [modify] https://crrev.com/4db6eb187710a109f98606588f4fa53b501e15ba/third_party/WebKit/LayoutTests/platform/linux/fast/text/emoji-web-font-expected.txt [rename] https://crrev.com/4db6eb187710a109f98606588f4fa53b501e15ba/third_party/freetype-android/.clang-format [modify] https://crrev.com/4db6eb187710a109f98606588f4fa53b501e15ba/third_party/freetype-android/BUILD.gn [modify] https://crrev.com/4db6eb187710a109f98606588f4fa53b501e15ba/third_party/freetype-android/OWNERS [modify] https://crrev.com/4db6eb187710a109f98606588f4fa53b501e15ba/third_party/freetype-android/include/freetype-android-config/ftoption.h [delete] https://crrev.com/84b4215d6c4fa677341ba96dfdfa083a27345b74/third_party/freetype2/BUILD.gn [delete] https://crrev.com/84b4215d6c4fa677341ba96dfdfa083a27345b74/third_party/freetype2/OWNERS [delete] https://crrev.com/84b4215d6c4fa677341ba96dfdfa083a27345b74/third_party/freetype2/README.chromium [delete] https://crrev.com/84b4215d6c4fa677341ba96dfdfa083a27345b74/third_party/freetype2/include/freetype-custom-config/ftconfig.h [delete] https://crrev.com/84b4215d6c4fa677341ba96dfdfa083a27345b74/third_party/freetype2/include/freetype-custom-config/ftmodule.h [delete] https://crrev.com/84b4215d6c4fa677341ba96dfdfa083a27345b74/third_party/freetype2/include/freetype-custom-config/ftoption.h [delete] https://crrev.com/84b4215d6c4fa677341ba96dfdfa083a27345b74/third_party/freetype2/patches/freetype2_symbols_visibility.patch
,
Mar 1 2017
,
Mar 1 2017
I think this broke android component build (there's no cq coverage for it, and I use it locally) I'm getting a runtime error for not being able to load libfreetype.s.so: ContentShellActivity: Caused by: java.lang.UnsatisfiedLinkError: dalvik.system.PathClassLoader[DexPathList[[zip file "/system/framework/android.test.runner.jar", zip file "/data/app/org.chromium.content_shell_apk.tests-1/base.apk", zip file "/data/app/org.chromium.content_shell_apk-1/base.apk"],nativeLibraryDirectories=[/data/app/ org.chromium.content_shell_apk.tests-1/lib/arm, /data/app/org.chromium.content_shell_apk-1/lib/arm, /data/app/org.chromium.content_shell_apk.tests-1/base.apk!/lib/armeabi-v7a, /data/app/org.chromium.content_shell_apk-1/base.apk!/lib/armeabi-v7a, /vendor/lib, /system/lib]]] couldn't find "libfreetype.s.so"
,
Mar 1 2017
+agrieve to help with component build things, gn is still beyond me..
,
Mar 1 2017
It broke my non-component Android build, as well.
,
Mar 1 2017
> It broke my non-component Android build, as well. is_debug=true implies component build, so it was a component build
,
Mar 1 2017
I went and built a clean component Android build and note that the FreeType library is built as libfreetype.so.6, it appears this should be libfreetype.cr.so. Probably something to do with the .cr.so handling...
,
Mar 1 2017
the library is built, but not being put into the apk If you touch one of the freetype source files, and print out the stuff that ninja does in the incremental build, all it does is rebuild the .o and .so, but doesn't rebuild the apk. This sort of thing has happened before, but I don't remember how this all works :/
,
Mar 1 2017
boliu@, could you please try with this experimental fix https://codereview.chromium.org/2728463003 ?
,
Mar 1 2017
We need the library name to be libfreetype.so.6 on linux, so that it's found by the linker and shadows the system freetype when we build content_shell (which links against it), so I put the name in the BUILD.gn file explicitly. Doing this seems to conflict with the Android component build. The speculative fix only explicitly sets the name when is_linux.
,
Mar 1 2017
With the patch from https://codereview.chromium.org/2728463003 my component Android build now works.
,
Mar 1 2017
Great, thanks for checking, then let's merge this CL.
,
Mar 1 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1da0b00909796cae61b947e76830fdfa67d6802c commit 1da0b00909796cae61b947e76830fdfa67d6802c Author: twellington <twellington@chromium.org> Date: Wed Mar 01 21:12:40 2017 Revert of Unify freetype-android and freetype2 into one checkout (patchset #2 id:20001 of https://codereview.chromium.org/2720823003/ ) Reason for revert: This broke component builds on Android. Original issue's description: > Unify freetype-android and freetype2 into one checkout > > Removing freetype2 for now, later renaming freetype-android to something > more generic. > > Moving freetype-android from source_set to a component target so that > it's generally usable in the other configurations. This should not > change much on Android where component resolves to static_library. > > Also, adding myself to freetype-android OWNERS. > > TBR'ing jochen for the removal, as presubmit requests an OWNER. > > BUG= 697015 > TBR=jochen > > Review-Url: https://codereview.chromium.org/2720823003 > Cr-Commit-Position: refs/heads/master@{#453918} > Committed: https://chromium.googlesource.com/chromium/src/+/4db6eb187710a109f98606588f4fa53b501e15ba TBR=behdad@chromium.org,bungeman@chromium.org,dpranke@chromium.org,eae@chromium.org,jshin@chromium.org,drott@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= 697015 Review-Url: https://codereview.chromium.org/2729703002 Cr-Commit-Position: refs/heads/master@{#454028} [modify] https://crrev.com/1da0b00909796cae61b947e76830fdfa67d6802c/DEPS [modify] https://crrev.com/1da0b00909796cae61b947e76830fdfa67d6802c/content/shell/BUILD.gn [modify] https://crrev.com/1da0b00909796cae61b947e76830fdfa67d6802c/third_party/WebKit/LayoutTests/platform/linux/fast/text/emoji-web-font-expected.png [modify] https://crrev.com/1da0b00909796cae61b947e76830fdfa67d6802c/third_party/WebKit/LayoutTests/platform/linux/fast/text/emoji-web-font-expected.txt [modify] https://crrev.com/1da0b00909796cae61b947e76830fdfa67d6802c/third_party/freetype-android/BUILD.gn [modify] https://crrev.com/1da0b00909796cae61b947e76830fdfa67d6802c/third_party/freetype-android/OWNERS [modify] https://crrev.com/1da0b00909796cae61b947e76830fdfa67d6802c/third_party/freetype-android/include/freetype-android-config/ftoption.h [rename] https://crrev.com/1da0b00909796cae61b947e76830fdfa67d6802c/third_party/freetype2/.clang-format [add] https://crrev.com/1da0b00909796cae61b947e76830fdfa67d6802c/third_party/freetype2/BUILD.gn [add] https://crrev.com/1da0b00909796cae61b947e76830fdfa67d6802c/third_party/freetype2/OWNERS [add] https://crrev.com/1da0b00909796cae61b947e76830fdfa67d6802c/third_party/freetype2/README.chromium [add] https://crrev.com/1da0b00909796cae61b947e76830fdfa67d6802c/third_party/freetype2/include/freetype-custom-config/ftconfig.h [add] https://crrev.com/1da0b00909796cae61b947e76830fdfa67d6802c/third_party/freetype2/include/freetype-custom-config/ftmodule.h [add] https://crrev.com/1da0b00909796cae61b947e76830fdfa67d6802c/third_party/freetype2/include/freetype-custom-config/ftoption.h [add] https://crrev.com/1da0b00909796cae61b947e76830fdfa67d6802c/third_party/freetype2/patches/freetype2_symbols_visibility.patch
,
Mar 1 2017
I verified CL in #11 fixes this as well. Thanks!
,
Mar 1 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6b4e0da088089a5c4346c8e7546f106d4e5cbbbe commit 6b4e0da088089a5c4346c8e7546f106d4e5cbbbe Author: drott <drott@chromium.org> Date: Wed Mar 01 22:03:47 2017 Reland of Unify freetype-android and freetype2 into one checkout (patchset #1 id:1 of https://codereview.chromium.org/2729703002/ ) Reason for revert: Fix ready and reviewed https://codereview.chromium.org/2728463003 Original issue's description: > Revert of Unify freetype-android and freetype2 into one checkout (patchset #2 id:20001 of https://codereview.chromium.org/2720823003/ ) > > Reason for revert: > This broke component builds on Android. > > Original issue's description: > > Unify freetype-android and freetype2 into one checkout > > > > Removing freetype2 for now, later renaming freetype-android to something > > more generic. > > > > Moving freetype-android from source_set to a component target so that > > it's generally usable in the other configurations. This should not > > change much on Android where component resolves to static_library. > > > > Also, adding myself to freetype-android OWNERS. > > > > TBR'ing jochen for the removal, as presubmit requests an OWNER. > > > > BUG= 697015 > > TBR=jochen > > > > Review-Url: https://codereview.chromium.org/2720823003 > > Cr-Commit-Position: refs/heads/master@{#453918} > > Committed: https://chromium.googlesource.com/chromium/src/+/4db6eb187710a109f98606588f4fa53b501e15ba > > TBR=behdad@chromium.org,bungeman@chromium.org,dpranke@chromium.org,eae@chromium.org,jshin@chromium.org,drott@chromium.org > # Skipping CQ checks because original CL landed less than 1 days ago. > NOPRESUBMIT=true > NOTREECHECKS=true > NOTRY=true > BUG= 697015 > > Review-Url: https://codereview.chromium.org/2729703002 > Cr-Commit-Position: refs/heads/master@{#454028} > Committed: https://chromium.googlesource.com/chromium/src/+/1da0b00909796cae61b947e76830fdfa67d6802c TBR=behdad@chromium.org,bungeman@chromium.org,dpranke@chromium.org,eae@chromium.org,jshin@chromium.org,twellington@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= 697015 Review-Url: https://codereview.chromium.org/2730493002 Cr-Commit-Position: refs/heads/master@{#454056} [modify] https://crrev.com/6b4e0da088089a5c4346c8e7546f106d4e5cbbbe/DEPS [modify] https://crrev.com/6b4e0da088089a5c4346c8e7546f106d4e5cbbbe/content/shell/BUILD.gn [modify] https://crrev.com/6b4e0da088089a5c4346c8e7546f106d4e5cbbbe/third_party/WebKit/LayoutTests/platform/linux/fast/text/emoji-web-font-expected.png [modify] https://crrev.com/6b4e0da088089a5c4346c8e7546f106d4e5cbbbe/third_party/WebKit/LayoutTests/platform/linux/fast/text/emoji-web-font-expected.txt [rename] https://crrev.com/6b4e0da088089a5c4346c8e7546f106d4e5cbbbe/third_party/freetype-android/.clang-format [modify] https://crrev.com/6b4e0da088089a5c4346c8e7546f106d4e5cbbbe/third_party/freetype-android/BUILD.gn [modify] https://crrev.com/6b4e0da088089a5c4346c8e7546f106d4e5cbbbe/third_party/freetype-android/OWNERS [modify] https://crrev.com/6b4e0da088089a5c4346c8e7546f106d4e5cbbbe/third_party/freetype-android/include/freetype-android-config/ftoption.h [delete] https://crrev.com/cb16a69fc771ddc62b00da78161c2b55c4385b3a/third_party/freetype2/BUILD.gn [delete] https://crrev.com/cb16a69fc771ddc62b00da78161c2b55c4385b3a/third_party/freetype2/OWNERS [delete] https://crrev.com/cb16a69fc771ddc62b00da78161c2b55c4385b3a/third_party/freetype2/README.chromium [delete] https://crrev.com/cb16a69fc771ddc62b00da78161c2b55c4385b3a/third_party/freetype2/include/freetype-custom-config/ftconfig.h [delete] https://crrev.com/cb16a69fc771ddc62b00da78161c2b55c4385b3a/third_party/freetype2/include/freetype-custom-config/ftmodule.h [delete] https://crrev.com/cb16a69fc771ddc62b00da78161c2b55c4385b3a/third_party/freetype2/include/freetype-custom-config/ftoption.h [delete] https://crrev.com/cb16a69fc771ddc62b00da78161c2b55c4385b3a/third_party/freetype2/patches/freetype2_symbols_visibility.patch
,
Mar 1 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/be5b593038f45afaf9f39272b5d2c97fad02c766 commit be5b593038f45afaf9f39272b5d2c97fad02c766 Author: drott <drott@chromium.org> Date: Wed Mar 01 22:12:59 2017 Fix for Android component build after FreeType merge BUG= 697015 Review-Url: https://codereview.chromium.org/2728463003 Cr-Commit-Position: refs/heads/master@{#454062} [modify] https://crrev.com/be5b593038f45afaf9f39272b5d2c97fad02c766/third_party/freetype-android/BUILD.gn
,
Mar 1 2017
Alright, thanks everyone for the quick responses. The Android component build issue should be resolved now.
,
Mar 6 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fe896a85ac3ab250cc5cad0e857da7ef32283a11 commit fe896a85ac3ab250cc5cad0e857da7ef32283a11 Author: drott <drott@chromium.org> Date: Mon Mar 06 12:12:54 2017 Rename third_party/freetype-android to third_party/freetype Now that the FreeType checkouts are unified, we should no longer use the _android suffix. TBR'ing jochen for the rename, not adding any new library. BUG= 697015 TBR=jochen Review-Url: https://codereview.chromium.org/2726873002 Cr-Commit-Position: refs/heads/master@{#454855} [modify] https://crrev.com/fe896a85ac3ab250cc5cad0e857da7ef32283a11/DEPS [modify] https://crrev.com/fe896a85ac3ab250cc5cad0e857da7ef32283a11/build/linux/BUILD.gn [modify] https://crrev.com/fe896a85ac3ab250cc5cad0e857da7ef32283a11/content/shell/BUILD.gn [modify] https://crrev.com/fe896a85ac3ab250cc5cad0e857da7ef32283a11/skia/BUILD.gn [rename] https://crrev.com/fe896a85ac3ab250cc5cad0e857da7ef32283a11/third_party/freetype/.clang-format [rename] https://crrev.com/fe896a85ac3ab250cc5cad0e857da7ef32283a11/third_party/freetype/BUILD.gn [rename] https://crrev.com/fe896a85ac3ab250cc5cad0e857da7ef32283a11/third_party/freetype/OWNERS [rename] https://crrev.com/fe896a85ac3ab250cc5cad0e857da7ef32283a11/third_party/freetype/README.chromium [rename] https://crrev.com/fe896a85ac3ab250cc5cad0e857da7ef32283a11/third_party/freetype/include/freetype-custom-config/ftmodule.h [rename] https://crrev.com/fe896a85ac3ab250cc5cad0e857da7ef32283a11/third_party/freetype/include/freetype-custom-config/ftoption.h
,
Mar 6 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/84d67b491f426b51a8d19411d313738d0aa9e240 commit 84d67b491f426b51a8d19411d313738d0aa9e240 Author: dalecurtis <dalecurtis@chromium.org> Date: Mon Mar 06 20:31:18 2017 Fixup stale freetype .gitignore entries. Without this the tree is marked as dirty. BUG= 697015 Review-Url: https://codereview.chromium.org/2737613002 Cr-Commit-Position: refs/heads/master@{#454942} [modify] https://crrev.com/84d67b491f426b51a8d19411d313738d0aa9e240/third_party/.gitignore
,
Mar 7 2017
Ummmm so my repo got screwed up today, and I've traced it back to my .git/config file which -- somehow -- now lists the URL of my "origin" remote as "https://chromium.googlesource.com/chromium/src/third_party/freetype2.git". Could that have somehow been triggered by one of these CLs? I'm not sure when today this happened. I had done roughly the following steps: 1. git fetch origin master && git checkout origin/master 2. gclient sync 3. gn gen $OUTDIR && ninja -C $OUTDIR chrome 4. git status # suggested removing third_party/freetype2 and third_party/freetype-android 5. rm -rf third_party/freetype2 third_party/freetype-android 6. gclient sync and now `git remote get-url origin` returns the freetype2.git repo. How is that even possible?
,
Mar 8 2017
michaelpg@, are you sure git status suggested the removal, or was it gclient? Or how did git suggest it? By listing a lot of untracked files? third_party/freetype/src/.git/ exists, for the DEPS based checkout of FreeType, which correctly lists $ git remote get-url origin https://chromium.googlesource.com/chromium/src/third_party/freetype2.git when I execute this in third_party/freetype/src/ Could it be that the .git directories got confused somehow? From where were you calling the 'git remote get-url origin' command?
,
Mar 21 2017
Re #22, please see details in issue 703343 . The issue is, that with old checkouts had an old folder third_party/freetype with a .git folder in it, which needs to be cleaned up. |
||||
►
Sign in to add a comment |
||||
Comment 1 by drott@chromium.org
, Feb 28 2017