Summary: | [GTK] When building introspection files, add CMAKE_C_FLAGS to the compiler flags. | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Alexander Kanavin <alex.kanavin> | ||||||||
Component: | WebKitGTK | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | annulen, aperez, bugs-noreply, ews-watchlist, gyuyoung.kim, keith_miller, mark.lam, mcatanzaro, msaboff, ryuan.choi, saam, sergio, tzagallo | ||||||||
Priority: | P2 | ||||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Alexander Kanavin
2021-11-10 03:09:35 PST
Created attachment 443793 [details]
Patch
Comment on attachment 443793 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=443793&action=review Hi, thanks for your patch. Hm, it sure seems like the right thing to do, so I suppose we should try it. That said, this seems very familiar. Pretty sure we've discussed this before at some point, likely years ago. I wonder if this will surface some unexpected problem. Oh well, only one way to find out! r- is only because you've failed the style bot due to the "No new tests (OOPS!)." > Source/JavaScriptCore/ChangeLog:3 > + When building introspection files, add CMAKE_C_FLAGS to the compiler flags. [GTK] When building introspection files, add CMAKE_C_FLAGS to the compiler flags. > Source/WebKit/ChangeLog:3 > + When building introspection files, add CMAKE_C_FLAGS to the compiler flags. [GTK] When building introspection files, add CMAKE_C_FLAGS to the compiler flags. > Source/WebKit/ChangeLog:8 > + No new tests (OOPS!). You can simply remove this line in order to pass the style bot. Your change doesn't require adding new tests. > Source/JavaScriptCore/PlatformGTK.cmake:74 > + COMMAND CC=${CMAKE_C_COMPILER} CFLAGS=-Wno-deprecated-declarations\ ${CMAKE_C_FLAGS} LDFLAGS= Can you write it without a backslash as CFLAGS="-Wno-deprecated-declarations ${CMAKE_C_FLAGS}"? P.S. If you set the WebKitGTK component when reporting the bug, we'll see it faster. (In reply to Michael Catanzaro from comment #2) > Hi, thanks for your patch. Hm, it sure seems like the right thing to do, so > I suppose we should try it. That said, this seems very familiar. Pretty sure > we've discussed this before at some point, likely years ago. I wonder if > this will surface some unexpected problem. Oh well, only one way to find out! Actually it looks like gobject-introspection is supposed to handle this itself. Let me check with them. (In reply to Michael Catanzaro from comment #4) > Actually it looks like gobject-introspection is supposed to handle this > itself. Let me check with them. Sorry, I misunderstood the patch. This looks fine to me. Comment on attachment 443793 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=443793&action=review >> Source/JavaScriptCore/PlatformGTK.cmake:74 >> + COMMAND CC=${CMAKE_C_COMPILER} CFLAGS=-Wno-deprecated-declarations\ ${CMAKE_C_FLAGS} LDFLAGS= > > Can you write it without a backslash as CFLAGS="-Wno-deprecated-declarations ${CMAKE_C_FLAGS}"? Should work without the backslash, yup. Here it is harmless because escaping a space character in the shell results in a space character ("\ " → " ") but it would be nicer to remove it, for clarity. > Source/WebKit/PlatformGTK.cmake:677 > + COMMAND CC=${CMAKE_C_COMPILER} CFLAGS=-Wno-deprecated-declarations\ ${CMAKE_C_FLAGS} LDFLAGS= Same issue here with the backslash. > Source/WebKit/PlatformGTK.cmake:723 > + COMMAND CC=${CMAKE_C_COMPILER} CFLAGS=-Wno-deprecated-declarations\ ${CMAKE_C_FLAGS} ...and here. Created attachment 452217 [details]
Patch
(In reply to Alexander Kanavin from comment #7) > Created attachment 452217 [details] > Patch I think the patch needs a rebase, could you update it and resubmit it? Created attachment 452846 [details]
Patch
Committed r290312 (247635@main): <https://commits.webkit.org/247635@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 452846 [details]. |