Re: [Rev 02] RFR: 8241476: Linux build warning issued on updated compilers (Ubuntu 18.04.4 / 20.04)
On Sun, 29 Mar 2020 00:38:20 GMT, Thiago Milczarek Sayao wrote: >> Simple fix to remove annoying warnings. > > Thiago Milczarek Sayao has updated the pull request incrementally with one > additional commit since the last revision: > > Sep cppFlags and cFlags The build changes in `linux.gradle` look good. I tested it, and all warnings are gone (including those in WebKit) except for the ones in the jfxmedia sub-project of javafx.media. Two comments: 1. I changed the bug title in JBS to refer to gcc 9 rather than Ubuntu to reflect the actual cause of the additional warnings. Can you update the title of this PR to match? 8241476: Linux build warnings issued on gcc 9 2. Can you add the following change to your PR as well? This will fix the remaining warnings in jfxmedia: --- a/modules/javafx.media/src/main/native/jfxmedia/projects/linux/Makefile +++ b/modules/javafx.media/src/main/native/jfxmedia/projects/linux/Makefile @@ -41,7 +41,6 @@ ifdef HOST_COMPILE -Wextra \ -Wformat-security \ -fstack-protector \ - -Werror=implicit-function-declaration \ -Werror=trampolines \ -msse2 \ -DGSTREAMER_LITE - PR: https://git.openjdk.java.net/jfx/pull/150
Re: [Rev 02] RFR: 8241476: Linux build warning issued on updated compilers (Ubuntu 18.04.4 / 20.04)
On Tue, 31 Mar 2020 20:45:44 GMT, Thiago Milczarek Sayao wrote: >> I took a look yesterday and came to the same conclusion that what we really >> want are separate C and C++ flags. For now, >> the only difference would be the presence or absence of >> `-Werror=implicit-function-declaration`, but it would allow for >> other differences in the future if needed. > > Please, let me know if this is the desired way to do it. If not, I will > rework it. Thanks. This is roughly what I had in mind. I want to test it, and also take a look at the WebKit and media builds as well. WebKit uses the flags from `linux.gradle`, and is mostly C++, so I think it needs no changes. Media does not use any flags from `linux.gradle` -- only the jfxmedia project has C++ files on Linux. I guess it should be fine, too, but I'll see. - PR: https://git.openjdk.java.net/jfx/pull/150
Re: [Rev 02] RFR: 8241476: Linux build warning issued on updated compilers (Ubuntu 18.04.4 / 20.04)
On Thu, 26 Mar 2020 11:21:06 GMT, Kevin Rushforth wrote: >> I think it's better to split the flags between CFLAGS and CPPFLAGS (as is >> done in OpenJDK). The >> `-Werror=implicit-function-declaration` is extremely useful for C files, and >> we don't want to risk that this somehow >> got lost. If we use CFLAGS and CPPFLAGS, we can pass the latter as CXXFLAGS >> to cmake, so that should be fairly easy. > > I took a look yesterday and came to the same conclusion that what we really > want are separate C and C++ flags. For now, > the only difference would be the presence or absence of > `-Werror=implicit-function-declaration`, but it would allow for > other differences in the future if needed. Please, let me know if this is the desired way to do it. If not, I will rework it. Thanks. - PR: https://git.openjdk.java.net/jfx/pull/150
Re: [Rev 02] RFR: 8241476: Linux build warning issued on updated compilers (Ubuntu 18.04.4 / 20.04)
> Simple fix to remove annoying warnings. Thiago Milczarek Sayao has updated the pull request incrementally with one additional commit since the last revision: Sep cppFlags and cFlags - Changes: - all: https://git.openjdk.java.net/jfx/pull/150/files - new: https://git.openjdk.java.net/jfx/pull/150/files/3164ab36..67adeaee Webrevs: - full: https://webrevs.openjdk.java.net/jfx/150/webrev.02 - incr: https://webrevs.openjdk.java.net/jfx/150/webrev.01-02 Stats: 22 lines in 1 file changed: 3 ins; 2 del; 17 mod Patch: https://git.openjdk.java.net/jfx/pull/150.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/150/head:pull/150 PR: https://git.openjdk.java.net/jfx/pull/150
Re: [Rev 01] RFR: 8241476: Linux build warning issued on updated compilers (Ubuntu 18.04.4 / 20.04)
On Thu, 26 Mar 2020 08:33:29 GMT, Johan Vos wrote: >> Like this? >> >> I think all C builds are covered. Gstreamer seems to have a Makefile with >> the flags. Not sure about libxml and libxlst >> inside javafx.web. > > I think it's better to split the flags between CFLAGS and CPPFLAGS (as is > done in OpenJDK). The > `-Werror=implicit-function-declaration` is extremely useful for C files, and > we don't want to risk that this somehow > got lost. If we use CFLAGS and CPPFLAGS, we can pass the latter as CXXFLAGS > to cmake, so that should be fairly easy. I took a look yesterday and came to the same conclusion that what we really want are separate C and C++ flags. For now, the only difference would be the presence or absence of `-Werror=implicit-function-declaration`, but it would allow for other differences in the future if needed. - PR: https://git.openjdk.java.net/jfx/pull/150
Re: [Rev 01] RFR: 8241476: Linux build warning issued on updated compilers (Ubuntu 18.04.4 / 20.04)
On Wed, 25 Mar 2020 00:40:01 GMT, Thiago Milczarek Sayao wrote: >> The problem is that gcc, for whatever reason, started issuing a (useless) >> warning if you pass the >> `-Werror=implicit-function-declaration` option to gcc for C++ files. I don't >> like the solution of removing that flag >> for C files. I think the better solution will be to have a set of options >> that are common to both C and C++ (all but >> this one currently), and then add `-Werror=implicit-function-declaration` >> only to the options for C. Until then, I >> think we live with this warning. It's better than losing the checking for C >> files. > > Like this? > > I think all C builds are covered. Gstreamer seems to have a Makefile with the > flags. Not sure about libxml and libxlst > inside javafx.web. I think it's better to split the flags between CFLAGS and CPPFLAGS (as is done in OpenJDK). The `-Werror=implicit-function-declaration` is extremely useful for C files, and we don't want to risk that this somehow got lost. If we use CFLAGS and CPPFLAGS, we can pass the latter as CXXFLAGS to cmake, so that should be fairly easy. - PR: https://git.openjdk.java.net/jfx/pull/150
Re: [Rev 01] RFR: 8241476: Linux build warning issued on updated compilers (Ubuntu 18.04.4 / 20.04)
On Tue, 24 Mar 2020 11:45:28 GMT, Kevin Rushforth wrote: >> Thiago Milczarek Sayao has updated the pull request with a new target base >> due to a merge or a rebase. The incremental >> webrev excludes the unrelated changes brought in by the merge/rebase. The >> pull request contains five additional commits >> since the last revision: >> - Merge branch 'master' into jdk_8241476 >> - Another approach for removing c++ warnings >> - Merge pull request #8 from openjdk/master >> >>Merge upstream >> - Remove build warning >> - Merge pull request #7 from openjdk/master >> >>merge from jfx > > The problem is that gcc, for whatever reason, started issuing a (useless) > warning if you pass the > `-Werror=implicit-function-declaration` option to gcc for C++ files. I don't > like the solution of removing that flag > for C files. I think the better solution will be to have a set of options > that are common to both C and C++ (all but > this one currently), and then add `-Werror=implicit-function-declaration` > only to the options for C. Until then, I > think we live with this warning. It's better than losing the checking for C > files. Like this? I think all C builds are covered. Gstreamer seems to have a Makefile with the flags. Not sure about libxml and libxlst inside javafx.web. - PR: https://git.openjdk.java.net/jfx/pull/150
Re: [Rev 01] RFR: 8241476: Linux build warning issued on updated compilers (Ubuntu 18.04.4 / 20.04)
> Simple fix to remove annoying warnings. Thiago Milczarek Sayao has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains five additional commits since the last revision: - Merge branch 'master' into jdk_8241476 - Another approach for removing c++ warnings - Merge pull request #8 from openjdk/master Merge upstream - Remove build warning - Merge pull request #7 from openjdk/master merge from jfx - Changes: - all: https://git.openjdk.java.net/jfx/pull/150/files - new: https://git.openjdk.java.net/jfx/pull/150/files/6e185d8a..3164ab36 Webrevs: - full: https://webrevs.openjdk.java.net/jfx/150/webrev.01 - incr: https://webrevs.openjdk.java.net/jfx/150/webrev.00-01 Stats: 1819 lines in 39 files changed: 1208 ins; 249 del; 362 mod Patch: https://git.openjdk.java.net/jfx/pull/150.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/150/head:pull/150 PR: https://git.openjdk.java.net/jfx/pull/150
Re: RFR: 8241476: Linux build warning issued on updated compilers (Ubuntu 18.04.4 / 20.04)
On Tue, 24 Mar 2020 01:36:29 GMT, Thiago Milczarek Sayao wrote: > Simple fix to remove annoying warnings. The problem is that gcc, for whatever reason, started issuing a (useless) warning if you pass the `-Werror=implicit-function-declaration` option to gcc for C++ files. I don't like the solution of removing that flag for C files. I think the better solution will be to have a set of options that are common to both C and C++ (all but this one currently), and then add `-Werror=implicit-function-declaration` only to the options for C. Until then, I think we live with this warning. It's better than losing the checking for C files. - Changes requested by kcr (Lead). PR: https://git.openjdk.java.net/jfx/pull/150
RFR: 8241476: Linux build warning issued on updated compilers (Ubuntu 18.04.4 / 20.04)
Simple fix to remove annoying warnings. - Commit messages: - Remove build warning - Merge pull request #7 from openjdk/master - Merge pull request #6 from openjdk/master - Merge pull request #5 from openjdk/master - Merge pull request #4 from openjdk/master Changes: https://git.openjdk.java.net/jfx/pull/150/files Webrev: https://webrevs.openjdk.java.net/jfx/150/webrev.00 Issue: https://bugs.openjdk.java.net/browse/JDK-8241476 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jfx/pull/150.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/150/head:pull/150 PR: https://git.openjdk.java.net/jfx/pull/150
