| Summary: | [GTK] Make the ENABLE_SUBTLE_CRYPTO option depend on libgcrypt 1.7.0 | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Zan Dobersek <zan> | ||||||
| Component: | New Bugs | Assignee: | Zan Dobersek <zan> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | jiewen_tan, mcatanzaro | ||||||
| Priority: | P2 | ||||||||
| Version: | WebKit Nightly Build | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Attachments: |
|
||||||||
|
Description
Zan Dobersek
2017-04-21 07:51:25 PDT
Created attachment 307729 [details]
Patch
Comment on attachment 307729 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=307729&action=review I think this should be renamed to ENABLE_WEB_CRYPTO before it's made public. > Source/cmake/OptionsGTK.cmake:142 > +WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_SUBTLE_CRYPTO PUBLIC OFF) Not ready to turn it ON yet? I think we should make it public in the same patch that we turn it ON. The point of making it public is to provide users a way to turn it off if they don't have new enough libgcrypt. If it's not ON by default, then no point in making it public yet. Comment on attachment 307729 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=307729&action=review >> Source/cmake/OptionsGTK.cmake:142 >> +WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_SUBTLE_CRYPTO PUBLIC OFF) > > Not ready to turn it ON yet? > > I think we should make it public in the same patch that we turn it ON. The point of making it public is to provide users a way to turn it off if they don't have new enough libgcrypt. If it's not ON by default, then no point in making it public yet. No, we shouldn't turn it ON before completing the support and doing another round of reviews. (In reply to Michael Catanzaro from comment #2) > Comment on attachment 307729 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=307729&action=review > > I think this should be renamed to ENABLE_WEB_CRYPTO before it's made public. > OK. Created attachment 307929 [details]
Patch
Comment on attachment 307929 [details] Patch Clearing flags on attachment: 307929 Committed r215681: <http://trac.webkit.org/changeset/215681> All reviewed patches have been landed. Closing bug. Minor problem: cgarcia: Zan: btw, not a big deal but I have #define ENABLE_SUBTLE_CRYPTO 1 duplicated in cmakehader.h mcatanzaro: That's a regression from http://trac.webkit.org/changeset/215681/webkit mcatanzaro: We should not use SET_AND_EXPOSE_TO_BUILD there (In reply to Michael Catanzaro from comment #8) > Minor problem: > > cgarcia: Zan: btw, not a big deal but I have #define ENABLE_SUBTLE_CRYPTO 1 > duplicated in cmakehader.h > mcatanzaro: That's a regression from > http://trac.webkit.org/changeset/215681/webkit > mcatanzaro: We should not use SET_AND_EXPOSE_TO_BUILD there Fixed in r215684. https://trac.webkit.org/changeset/215684/webkit |