Summary: | [EFL] Add subtle crypto to the build system | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Éva Balázsfalvi <evab.u-szeged> | ||||||||
Component: | Tools / Tests | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | buildbot, cgarcia, commit-queue, gyuyoung.kim, jh718.park, ossy, pnormand, rniwa | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Éva Balázsfalvi
2014-11-11 08:18:35 PST
Created attachment 241352 [details]
Patch
Attachment 241352 [details] did not pass style-queue:
ERROR: Source/WebCore/crypto/gnutls/CryptoDigestGnuTLS.cpp:31: Alphabetical sorting problem. [build/include_order] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmRSA_OAEPGnuTLS.cpp:38: CryptoAlgorithmRSA_OAEP::platformEncrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmRSA_OAEPGnuTLS.cpp:50: CryptoAlgorithmRSA_OAEP::platformDecrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmHMACGnuTLS.cpp:34: Alphabetical sorting problem. [build/include_order] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmAES_KWGnuTLS.cpp:37: CryptoAlgorithmAES_KW::platformEncrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmAES_KWGnuTLS.cpp:48: CryptoAlgorithmAES_KW::platformDecrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmRSASSA_PKCS1_v1_5GnuTLS.cpp:38: CryptoAlgorithmRSASSA_PKCS1_v1_5::platformSign is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmRSASSA_PKCS1_v1_5GnuTLS.cpp:51: CryptoAlgorithmRSASSA_PKCS1_v1_5::platformVerify is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmRSAES_PKCS1_v1_5GnuTLS.cpp:37: CryptoAlgorithmRSAES_PKCS1_v1_5::platformEncrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmRSAES_PKCS1_v1_5GnuTLS.cpp:48: CryptoAlgorithmRSAES_PKCS1_v1_5::platformDecrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmAES_CBCGnuTLS.cpp:38: CryptoAlgorithmAES_CBC::platformEncrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmAES_CBCGnuTLS.cpp:50: CryptoAlgorithmAES_CBC::platformDecrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
Total errors found: 12 in 20 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 241352 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=241352&action=review Looks fine for EFL port. However I think we need to get agreement from GTK port folks. > Source/WebCore/PlatformEfl.cmake:503 > + crypto/gnutls/CryptoAlgorithmRegistryGnuTLS.cpp Wrong alphabet sorting. These files should be placed in front of crypto/keys/ Comment on attachment 241352 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=241352&action=review > Source/cmake/OptionsEfl.cmake:107 > +WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_SUBTLE_CRYPTO OFF) BTW, is there any reason SUBTLE_CRYPTO is disabled for cmake build ? (In reply to comment #4) > Comment on attachment 241352 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=241352&action=review > > > Source/cmake/OptionsEfl.cmake:107 > > +WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_SUBTLE_CRYPTO OFF) > > BTW, is there any reason SUBTLE_CRYPTO is disabled for cmake build ? When it came to enabling SUBTLE_CRYPTO, I based my patch on bug 135798. There is a good reasoning why to disable it by default, and I thought we need this kind of solution in case of EFL, too. Created attachment 241586 [details]
Patch
Attachment 241586 [details] did not pass style-queue:
ERROR: Source/WebCore/crypto/gnutls/CryptoDigestGnuTLS.cpp:31: Alphabetical sorting problem. [build/include_order] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmRSA_OAEPGnuTLS.cpp:38: CryptoAlgorithmRSA_OAEP::platformEncrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmRSA_OAEPGnuTLS.cpp:50: CryptoAlgorithmRSA_OAEP::platformDecrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmHMACGnuTLS.cpp:34: Alphabetical sorting problem. [build/include_order] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmAES_KWGnuTLS.cpp:37: CryptoAlgorithmAES_KW::platformEncrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmAES_KWGnuTLS.cpp:48: CryptoAlgorithmAES_KW::platformDecrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmRSASSA_PKCS1_v1_5GnuTLS.cpp:38: CryptoAlgorithmRSASSA_PKCS1_v1_5::platformSign is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmRSASSA_PKCS1_v1_5GnuTLS.cpp:51: CryptoAlgorithmRSASSA_PKCS1_v1_5::platformVerify is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmRSAES_PKCS1_v1_5GnuTLS.cpp:37: CryptoAlgorithmRSAES_PKCS1_v1_5::platformEncrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmRSAES_PKCS1_v1_5GnuTLS.cpp:48: CryptoAlgorithmRSAES_PKCS1_v1_5::platformDecrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmAES_CBCGnuTLS.cpp:38: CryptoAlgorithmAES_CBC::platformEncrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmAES_CBCGnuTLS.cpp:50: CryptoAlgorithmAES_CBC::platformDecrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
Total errors found: 12 in 20 files
If any of these errors are false positives, please file a bug against check-webkit-style.
(In reply to comment #5) > (In reply to comment #4) > > Comment on attachment 241352 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=241352&action=review > > > > > Source/cmake/OptionsEfl.cmake:107 > > > +WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_SUBTLE_CRYPTO OFF) > > > > BTW, is there any reason SUBTLE_CRYPTO is disabled for cmake build ? > > When it came to enabling SUBTLE_CRYPTO, I based my patch on bug 135798. > There is a good reasoning why to disable it by default, and I thought we > need this kind of solution in case of EFL, too. Disabling in Options<Efl|Gtk>.cmake and enabling it in Features.pm is the common way for experimental / not completed features. Features.pm is only used by build-webkit, which is for developers and bots to be able test experimental features too. But production builds don't use build-webkit at all, they call cmake directly, so experimental features are disabled for production builds. Crypto seems to be in a very early stage now, so it shouldn't be enabled for production build yet. (In reply to comment #7) > Attachment 241586 [details] did not pass style-queue: > > > ERROR: Source/WebCore/crypto/gnutls/CryptoDigestGnuTLS.cpp:31: Alphabetical > sorting problem. [build/include_order] [4] > ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmRSA_OAEPGnuTLS.cpp:38: > CryptoAlgorithmRSA_OAEP::platformEncrypt is incorrectly named. Don't use > underscores in your identifier names. [readability/naming/underscores] [4] > ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmRSA_OAEPGnuTLS.cpp:50: > CryptoAlgorithmRSA_OAEP::platformDecrypt is incorrectly named. Don't use > underscores in your identifier names. [readability/naming/underscores] [4] > ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmHMACGnuTLS.cpp:34: > Alphabetical sorting problem. [build/include_order] [4] > ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmAES_KWGnuTLS.cpp:37: > CryptoAlgorithmAES_KW::platformEncrypt is incorrectly named. Don't use > underscores in your identifier names. [readability/naming/underscores] [4] > ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmAES_KWGnuTLS.cpp:48: > CryptoAlgorithmAES_KW::platformDecrypt is incorrectly named. Don't use > underscores in your identifier names. [readability/naming/underscores] [4] > ERROR: > Source/WebCore/crypto/gnutls/CryptoAlgorithmRSASSA_PKCS1_v1_5GnuTLS.cpp:38: > CryptoAlgorithmRSASSA_PKCS1_v1_5::platformSign is incorrectly named. Don't > use underscores in your identifier names. [readability/naming/underscores] > [4] > ERROR: > Source/WebCore/crypto/gnutls/CryptoAlgorithmRSASSA_PKCS1_v1_5GnuTLS.cpp:51: > CryptoAlgorithmRSASSA_PKCS1_v1_5::platformVerify is incorrectly named. Don't > use underscores in your identifier names. [readability/naming/underscores] > [4] > ERROR: > Source/WebCore/crypto/gnutls/CryptoAlgorithmRSAES_PKCS1_v1_5GnuTLS.cpp:37: > CryptoAlgorithmRSAES_PKCS1_v1_5::platformEncrypt is incorrectly named. Don't > use underscores in your identifier names. [readability/naming/underscores] > [4] > ERROR: > Source/WebCore/crypto/gnutls/CryptoAlgorithmRSAES_PKCS1_v1_5GnuTLS.cpp:48: > CryptoAlgorithmRSAES_PKCS1_v1_5::platformDecrypt is incorrectly named. Don't > use underscores in your identifier names. [readability/naming/underscores] > [4] > ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmAES_CBCGnuTLS.cpp:38: > CryptoAlgorithmAES_CBC::platformEncrypt is incorrectly named. Don't use > underscores in your identifier names. [readability/naming/underscores] [4] > ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmAES_CBCGnuTLS.cpp:50: > CryptoAlgorithmAES_CBC::platformDecrypt is incorrectly named. Don't use > underscores in your identifier names. [readability/naming/underscores] [4] > Total errors found: 12 in 20 files > > > If any of these errors are false positives, please file a bug against > check-webkit-style. We can ignore these style issues, the class names is come from the original design: Source/WebCore/crypto/algorithms/*.h . We shouldn't change them now. Created attachment 241712 [details]
Patch
Had to undo two, previously corrected 'alphabetical order' style issue, because we can't switch the order of #include <gnutls/crypto.h> and #include <gnutls/gnutls.h>, or else the build will fail.
Attachment 241712 [details] did not pass style-queue:
ERROR: Source/WebCore/crypto/gnutls/CryptoDigestGnuTLS.cpp:31: Alphabetical sorting problem. [build/include_order] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmRSA_OAEPGnuTLS.cpp:38: CryptoAlgorithmRSA_OAEP::platformEncrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmRSA_OAEPGnuTLS.cpp:50: CryptoAlgorithmRSA_OAEP::platformDecrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmHMACGnuTLS.cpp:34: Alphabetical sorting problem. [build/include_order] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmAES_KWGnuTLS.cpp:37: CryptoAlgorithmAES_KW::platformEncrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmAES_KWGnuTLS.cpp:48: CryptoAlgorithmAES_KW::platformDecrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmRSASSA_PKCS1_v1_5GnuTLS.cpp:38: CryptoAlgorithmRSASSA_PKCS1_v1_5::platformSign is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmRSASSA_PKCS1_v1_5GnuTLS.cpp:51: CryptoAlgorithmRSASSA_PKCS1_v1_5::platformVerify is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmRSAES_PKCS1_v1_5GnuTLS.cpp:37: CryptoAlgorithmRSAES_PKCS1_v1_5::platformEncrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmRSAES_PKCS1_v1_5GnuTLS.cpp:48: CryptoAlgorithmRSAES_PKCS1_v1_5::platformDecrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmAES_CBCGnuTLS.cpp:38: CryptoAlgorithmAES_CBC::platformEncrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
ERROR: Source/WebCore/crypto/gnutls/CryptoAlgorithmAES_CBCGnuTLS.cpp:50: CryptoAlgorithmAES_CBC::platformDecrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
Total errors found: 12 in 20 files
If any of these errors are false positives, please file a bug against check-webkit-style.
any feedback from GTK side? Ping? Comment on attachment 241712 [details]
Patch
lgtm
Comment on attachment 241712 [details]
Patch
It is good for EFL and GTK, so final r+.
Comment on attachment 241712 [details] Patch Clearing flags on attachment: 241712 Committed r176712: <http://trac.webkit.org/changeset/176712> All reviewed patches have been landed. Closing bug. It seems that there is an insignificant problem that Gtk and Efl depends on libgnutls28-dev when I see into Tools/efl/install-dependencies and Tools/gtk/install-dependencies. So once execute build-webkit, then it leads to version check fail because system installed gnutls version is 2.12.23 which is installed by install-dependencies, and its version is lower than the version subtle_crypto feature requires(3.0). So I think it will be needed to add gnutls(>=3.0) to install-dependencies in install-dependencies in port EFL and GTK. Please consider the comment I made above and if I missed something then please let me know. I don't know which distro you use but on Debian at least, libgnutls28 provides gnutls 3.x, I know it's confusing... pkg-config --modversion gnutls 3.3.8 I'm using Ubuntu 14.04, and even after sudo apt-get update && sudo apt-get upgrade it's not changed. In my case, pkg-config --modversion gnutls's conclusion is 2.12.23. I guess there's some divergence between distros when installing libgnutls28. Don't you think we need fixed version of gnutls for webkit efl and gtk? Maybe it might be an option to use jhbuild dependency package module, but I'm not sure it's ok because I've heard that jhbuild is for layout test originally. Even since 12.04 the libgnutls28-dev package provides version 3 of gnutls : http://packages.ubuntu.com/precise/libgnutls28-dev Make sure to have it installed... You can debug this with pkg-config --debug modversion gnutls On my Ubuntu 14.04: $ pkg-config --modversion gnutls 3.2.11 But I remember I had some issues on a 14.04 Ubuntu where another libgnutls was installed previously. As far as I remember removing the older one solved the version clashing problem. I think Csaba Osztrogonác's comment is right. As long as the older gnutls deppkg is remained even after I installed libgnutls28-dev the version of gnutls was not changed, but after remove the old one installing libgnutls28-dev changed the gnutls version to 3.2.11 at last. It seems ubuntu's insignificant bug. Thank you for your helps, Philippe Normand and Csaba Osztrogonác. :) |