Skip to content
This repository has been archived by the owner on May 29, 2024. It is now read-only.

Add --disable-warnings-as-errors option to build_labsjdk.py #7

Closed
wants to merge 3 commits into from

Conversation

YaSuenag
Copy link
Contributor

I'd like to introduce --disable-warnings-as-errors to build_labsjdk.py.

I tried to build LabsJDK CE 11 on Fedora 32 which is installed GCC 10.1.1, but GCC reported some warnings, and they were considered to be errors.

configure script in OpenJDK has --disable-warnings-as-errors option to avoid them, so I want to pass it from build_labsjdk.py.

@dougxc dougxc self-assigned this Jul 16, 2020
@dougxc
Copy link
Member

dougxc commented Jul 16, 2020

It would be even better if we added a general purpose --configure-arg option that could be specified numerous times to allow for customization of any configure argument. Think you could try that instead?

@YaSuenag
Copy link
Contributor Author

To be honest I want to pass through all arguments to configure script in JDK, however it is difficult to use ArgumentParser because they are started with --, and also we need to consider original options which we cannot pass to (e.g. --clean-after-build ).

I want to pass configure options to arguments, but if it is difficult, we may set configure options to file and build_labsjdk.py would pickup it. What do you think?
For example:

configuration file:

--disable-warnings-as-errors
--with-vendor-name="Foo Bar"
--with-vendor-url=https://www.example.com/

Run build_labsjdk.py with configuration file:

$ python3 build_labsjdk.py --boot-jdk /path/to/jdk --configure-options /path/to/configuration/file

@dougxc
Copy link
Member

dougxc commented Jul 16, 2020

The configure options file idea sounds good.

Stepping back, can you say more about your use case for build_labsjdk.py, especially given https://bugs.openjdk.java.net/browse/JDK-8241193

@YaSuenag
Copy link
Contributor Author

I sometimes tweak configure options for OpenJDK e.g. --with-native-debug-symbols, --disable-warnings-as-errors, --with-extra-cxxflags. I think it is different from JDK-8241193 because JDK-8241193 (and JDK-8236921) is for static libraries.

I want to build / debug GraalVM on Fedora 32 because it provides modern C++ compiler and native debugger. But we need to tweak configure options for them due to compiler warnings.

@YaSuenag
Copy link
Contributor Author

Also I want to backport some tickets for jdk/jdk to build LabsJDK on Fedora 32 - I've listed them on #6.

@dougxc
Copy link
Member

dougxc commented Jul 16, 2020

I guess my real question is why you need to use build_labsjdk.py at all instead of sh configure; make. I've no objections to the proposed configure options file but am just wondering if you could save yourself some headache by bypassing build_labsjdk.py altogether.

@YaSuenag
Copy link
Contributor Author

README.md says LabsJDK can be built with build_labsjdk.py, and also it seems to need to pass valid JVMCI version to --with-version-opt. In addition we may need to consider other configure options in future when we use customized LabsJDK to build GraalVM. So I'd like to use build_labsjdk.py.

I'd like you to add (and maintain) what options should we pass to configure script in README.md if we do not change build_labsjdk.py.

@dougxc
Copy link
Member

dougxc commented Jul 17, 2020

Please update this PR to implement the configure options file and I'll integrate it.

@YaSuenag
Copy link
Contributor Author

I pushed new commit. Could you review it?

I introduced new option --configure-options and tested it with following options, then it works fine on my Fedora 32 (GCC 10).

--disable-warnings-as-errors
--with-native-debug-symbols=internal
--with-extra-cxxflags=-fcommon
--with-extra-cflags=-fcommon

@dougxc
Copy link
Member

dougxc commented Jul 17, 2020

Looks good - I'll integrate this.

@dougxc
Copy link
Member

dougxc commented Jul 20, 2020

If you don't mind, I will squash the commits in this PR when merging them.

@YaSuenag
Copy link
Contributor Author

@dougxc Thanks! please squash and merge this PR.

@dougxc
Copy link
Member

dougxc commented Jul 21, 2020

Resolved by 6bc9013

@dougxc dougxc closed this Jul 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants