Bug 174253

Created attachment 314843 [details] Patch

Comment on attachment 314843 [details] Patch The pattern we've been following recently would be to create a GCrypt.cmake file with the GCrypt-specific targets. And guard the .cpp files with CMake conditionals so that they are not built at all when the feature is disabled.

(In reply to Michael Catanzaro from comment #2) > Comment on attachment 314843 [details] > Patch > > The pattern we've been following recently would be to create a GCrypt.cmake > file with the GCrypt-specific targets. And guard the .cpp files with CMake > conditionals so that they are not built at all when the feature is disabled. Where would the CMake file be placed? All such files are under platform/ for now, but it would be odd to add GCrypt.cmake there, and then from that file add files under crypto/ to the build.

Created attachment 315212 [details] Patch Also uses USE_GCRYPT guards, doesn't (yet) add a GCrypt.cmake file.

(In reply to Zan Dobersek from comment #3) > Where would the CMake file be placed? All such files are under platform/ for > now, That's exactly where I would put it. > but it would be odd to add GCrypt.cmake there, and then from that file > add files under crypto/ to the build. I don't see why it would be odd.

(In reply to Michael Catanzaro from comment #5) > (In reply to Zan Dobersek from comment #3) > > Where would the CMake file be placed? All such files are under platform/ for > > now, > > That's exactly where I would put it. > > > but it would be odd to add GCrypt.cmake there, and then from that file > > add files under crypto/ to the build. > > I don't see why it would be odd. OK, I don't care enough about this.

Created attachment 315640 [details] Patch

Comment on attachment 315640 [details] Patch Exactly! Sony starting splitting some libs they use out to separate CMake files already... we'll probably move more.

Comment on attachment 315640 [details] Patch Clearing flags on attachment: 315640 Committed r219556: <http://trac.webkit.org/changeset/219556>

All reviewed patches have been landed. Closing bug.

Reverted r219556 for reason: Broke build without WebCrypto Committed r219559: <http://trac.webkit.org/changeset/219559>

We'll have to try again. It broke the build without WebCrypto. Some of the stuff that you moved behind ENABLE_SUBTLE_CRYTPO is needed for CryptoDigest: lib/libPAL.a(lib/../Source/WebCore/PAL/pal/CMakeFiles/PAL.dir/crypto/gcrypt/CryptoDigestGCrypt.cpp.o):CryptoDigestGCrypt.cpp:function PAL::CryptoDigest::create(PAL::CryptoDigest::Algorithm): error: undefined reference to 'gcry_md_open' lib/libPAL.a(lib/../Source/WebCore/PAL/pal/CMakeFiles/PAL.dir/crypto/gcrypt/CryptoDigestGCrypt.cpp.o):CryptoDigestGCrypt.cpp:function PAL::CryptoDigest::computeHash(): error: undefined reference to 'gcry_md_get_algo_dlen' lib/libPAL.a(lib/../Source/WebCore/PAL/pal/CMakeFiles/PAL.dir/crypto/gcrypt/CryptoDigestGCrypt.cpp.o):CryptoDigestGCrypt.cpp:function PAL::CryptoDigest::computeHash(): error: undefined reference to 'gcry_md_ctl' lib/libPAL.a(lib/../Source/WebCore/PAL/pal/CMakeFiles/PAL.dir/crypto/gcrypt/CryptoDigestGCrypt.cpp.o):CryptoDigestGCrypt.cpp:function PAL::CryptoDigest::computeHash(): error: undefined reference to 'gcry_md_read' lib/libPAL.a(lib/../Source/WebCore/PAL/pal/CMakeFiles/PAL.dir/crypto/gcrypt/CryptoDigestGCrypt.cpp.o):CryptoDigestGCrypt.cpp:function PAL::CryptoDigest::computeHash(): error: undefined reference to 'gcry_md_close' lib/libPAL.a(lib/../Source/WebCore/PAL/pal/CMakeFiles/PAL.dir/crypto/gcrypt/CryptoDigestGCrypt.cpp.o):CryptoDigestGCrypt.cpp:function PAL::CryptoDigest::addBytes(void const*, unsigned long): error: undefined reference to 'gcry_md_write'

USE_GCRYPT has to be set and exposed to the build for both GTK+ and WPE since it's used in the libgcrypt-based CryptoDigest implementation that's used by the two ports unconditionally. The problem was libgcrypt include dirs and libraries weren't used during the build for the GTK+ port because that was done only in case USE_GCRYPT was enabled and GCrypt.cmake included in the build, but USE_GCRYPT wasn't enabled if ENABLE_SUBTLE_CRYPTO wasn't. The WPE port didn't suffer this because it added libgcrypt include dirs and libraries to the build for the needs of ClearKey support.

Created attachment 315916 [details] Patch

Comment on attachment 315916 [details] Patch Attachment 315916 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/4147899 New failing tests: fast/shadow-dom/ShadowRoot-mode.html

Created attachment 315918 [details] Archive of layout-test-results from ews121 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews121 Port: ios-simulator-wk2 Platform: Mac OS X 10.12.5

Comment on attachment 315916 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=315916&action=review > Source/WebCore/PlatformGTK.cmake:13 > +if (USE_GCRYPT) > + include(platform/GCrypt.cmake) > +endif () Since it's always true now, you should remove the conditional. > Source/WebCore/PlatformWPE.cmake:10 > +if (USE_GCRYPT) > + include(platform/GCrypt.cmake) > +endif () Ditto.

Committed r219676: <http://trac.webkit.org/changeset/219676>