RESOLVED FIXED 171112
[GTK] Make the ENABLE_SUBTLE_CRYPTO option depend on libgcrypt 1.7.0 
https://bugs.webkit.org/show_bug.cgi?id=171112
Summary [GTK] Make the ENABLE_SUBTLE_CRYPTO option depend on libgcrypt 1.7.0
Zan Dobersek
Reported 2017-04-21 07:51:25 PDT
[GTK] Expose the ENABLE_SUBTLE_CRYPTO CMake option, have it depend on libgcrypt 1.7.0
Attachments
Patch (2.01 KB, patch)
2017-04-21 07:56 PDT, Zan Dobersek 		no flags
DetailsFormatted DiffDiff
Patch (1.47 KB, patch)
2017-04-23 03:51 PDT, Zan Dobersek 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Zan Dobersek
Comment 1 2017-04-21 07:56:00 PDT
Created attachment 307729 [details] Patch
Michael Catanzaro
Comment 2 2017-04-21 08:52:33 PDT
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.
Zan Dobersek
Comment 3 2017-04-23 03:07:48 PDT
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.
Zan Dobersek
Comment 4 2017-04-23 03:08:56 PDT
(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.
Zan Dobersek
Comment 5 2017-04-23 03:51:39 PDT
Created attachment 307929 [details] Patch
Zan Dobersek
Comment 6 2017-04-24 06:15:37 PDT
Comment on attachment 307929 [details] Patch Clearing flags on attachment: 307929 Committed r215681: <http://trac.webkit.org/changeset/215681>
Zan Dobersek
Comment 7 2017-04-24 06:15:41 PDT
All reviewed patches have been landed. Closing bug.
Michael Catanzaro
Comment 8 2017-04-24 09:20:05 PDT
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
Zan Dobersek
Comment 9 2017-04-24 11:10:30 PDT
(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
Note You need to log in before you can comment on or make changes to this bug.