8323672: Suppress unwanted autoconf added flags in CC and CXX

[TheShermanTanker]

JDK-8323008 reports unwanted autoconf flags being added to the command line, and solves the issue by filtering out the added flags by force. This is not optimal however, as doing so leaves the autoconf message that the flags were added still displaying during the configure process, which is confusing. Instead, setting a couple of fields to disable the autoconf internals responsible for the unwanted flag is a cleaner solution

---

Progress

• Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• Change must not contain extraneous whitespace
• Commit message must refer to an issue

Issue

• JDK-8323672: Suppress unwanted autoconf added flags in CC and CXX (Bug - P4)

Reviewers

• Magnus Ihse Bursie (@magicus - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/17401/head:pull/17401
$ git checkout pull/17401

Update a local copy of the PR:
$ git checkout pull/17401
$ git pull https://git.openjdk.org/jdk.git pull/17401/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 17401

View PR using the GUI difftool:
$ git pr show -t 17401

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/17401.diff

Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back jwaters! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

@TheShermanTanker The following label will be automatically applied to this pull request:

• build

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

[mlbridge]

Webrevs

• 07: Full (c478177b)
• 06: Full - Incremental (8d8d3749)
• 05: Full - Incremental (1e37103f)
• 04: Full - Incremental (71a93ea0)
• 03: Full - Incremental (776f8aa4)
• 02: Full - Incremental (0fb5734c)
• 01: Full - Incremental (0de14523)
• 00: Full (b39e0479)

[magicus]

I'm kind of on the fence for this one. On one hand filtering out stuff is really hacky, but so is setting internal undocumented variables. We have done some meddling with internal variable in a few places before, but I'd really like to keep that to a minumum. I'm not sure this is an improvement. I'd like to get input from other build folks on this.

[erikj79]

I'm kind of on the fence for this one. On one hand filtering out stuff is really hacky, but so is setting internal undocumented variables. We have done some meddling with internal variable in a few places before, but I'd really like to keep that to a minumum. I'm not sure this is an improvement. I'd like to get input from other build folks on this.

I was holding off for Magnus' opinion here. Both solutions are bad, but we are also trying to work around a pretty bad behavior in autoconf that has met quite a bit of opposition from what I've read. I don't see a clear winner, so I'm leaning towards sticking with the solution that is already integrated for now. If we see more related problems crop up we can re-evaluate then.

[TheShermanTanker]

I was thinking earlier this morning if I could roll a custom AC_PROG_CC for the JDK, since it could solve the issues that the current AC_PROG_CC and AC_PROG_CXX (including the addition of flags that we don't want). This way we could avoid hacks and messing with autoconf internals related to the problems that AC_PROG_CC presents (such as the one present here). Any thoughts on this?

[magicus]

The current use of AC_PROG_CC does not fit very well with out needs. It does a lot of testing that is not needed for us (and at least on Windows, this considerably slows down configure). So it would make sense to replace it with something that only does what we need. I have thought of this before, even without the context of this bug.

However, I have not pushed in this direction yet, partly because of lack of time, but also partly since I was afraid (well-grounded or not) that this was doing a lot of "autoconf black magic" that is needed elsewhere.

But if you can take the time to analyze and understand what the vanilla AC_PROG_CC macro actually does, and extract just the parts that are relevant for us, and argue convincingly that the parts we're leaving out is not going to affect us, then I think it would be good.

The more I've worked with autoconf, the less happy I've become with all its magic stuff, unchangable premises and bad fit for our use case.

[TheShermanTanker]

Out of curiosity: What exactly does AC_PROG_CC do that we need? I've begun writing a modified version of it, and I'm shocked with how much can be completely stripped away from the new version, and am unsure if I'm eliminating parts of it that we actually need

[TheShermanTanker]

What fun, I just found out that omitting the call to AC_PROG_CC and AC_PROG_CXX has no effect, and they're still called magically from somewhere by autoconf even if you don't call them. I'm genuinely at a loss for words :)

[magicus]

Then it sounds like this is not a viable approach.

Also, check your inbox for a personal invitation to The "wtf-autoconf-rly?" Club. ;-)

[TheShermanTanker]

Then it sounds like this is not a viable approach.

There's definitely gotta be a way to make autoconf do what we want that's out there, I'll work on it and post any findings back here. I've just found out that the implicit calls to AC_PROG_CC are due to AC_PROG_CPP calling AC_REQUIRE on AC_PROG_CC for instance

Also, check your inbox for a personal invitation to The "wtf-autoconf-rly?" Club. ;-)

I think I'm already a long time member of that club by this point...
(Sometimes I wonder whether Java could've gone the way V8 went and used Python or Java itself as the build system instead, oh well)

[magicus]

(Sometimes I wonder whether Java could've gone the way V8 went and used Python or Java itself as the build system instead, oh well)

While I think we're stuck with make for the long run, a long-term goal for me has been (and still is) to replace autoconf with a system written in Java. Fortunately, we have a well defined interface: The user runs configure with a set of well-know arguments, we check for a bunch of requirements and the validity of the requested build configuration, and generates a well-defined spec.gmk file as a result. This tight encapsulation means it is possible to make a drop-in replacement in Java. The challenge is "just" to make sure we don't loose any functionality, nor skip over any of the odd fixes and adaptation to real-world build systems that the current configure script has collected over the years.

[TheShermanTanker]

(Sometimes I wonder whether Java could've gone the way V8 went and used Python or Java itself as the build system instead, oh well)

While I think we're stuck with make for the long run, a long-term goal for me has been (and still is) to replace autoconf with a system written in Java. Fortunately, we have a well defined interface: The user runs configure with a set of well-know arguments, we check for a bunch of requirements and the validity of the requested build configuration, and generates a well-defined spec.gmk file as a result. This tight encapsulation means it is possible to make a drop-in replacement in Java. The challenge is "just" to make sure we don't loose any functionality, nor skip over any of the odd fixes and adaptation to real-world build systems that the current configure script has collected over the years.

That sounds like a huge potential improvement, at least to me. I've also just come across JEP-138 (Fascinating cache of JDK history within that JEP! Never knew the JDK used to only work with make), so it's interesting to see if autoconf can be phased out after its introduction 9 years ago. If I understand you correctly, this new hypothetical system would replace autoconf as such?

configure -> autoconf -> compiled configure -> make
to
configure -> compile Java -> Java configure program -> make

[erikj79]

What fun, I just found out that omitting the call to AC_PROG_CC and AC_PROG_CXX has no effect, and they're still called magically from somewhere by autoconf even if you don't call them. I'm genuinely at a loss for words :)

If I'm to guess, I think this is caused by AC_PROG_* being defined as AC_DEFUN_ONCE and every other autoconf macro that uses compilers for checks will implicitly call AC_PROG_* first if it hasn't been called yet. My understanding of these implicit calls is to make a more declarative programming style possible.

[TheShermanTanker]

What fun, I just found out that omitting the call to AC_PROG_CC and AC_PROG_CXX has no effect, and they're still called magically from somewhere by autoconf even if you don't call them. I'm genuinely at a loss for words :)

If I'm to guess, I think this is caused by AC_PROG_* being defined as AC_DEFUN_ONCE and every other autoconf macro that uses compilers for checks will implicitly call AC_PROG_* first if it hasn't been called yet. My understanding of these implicit calls is to make a more declarative programming style possible.

It's not so much the AC_DEFUN_ONCE, but actually due to other autoconf macros calling AC_REQUIRE([AC_PROG_CC]) and AC_REQUIRE([AC_PROG_CXX]), for instance our calls to AC_PROG_CPP and AC_PROG_CXXCPP will cause AC_PROG_CC and AC_PROG_CXX respectively to be implicitly called

[TheShermanTanker]

I found out that the issue of having AC_PROG_CC and AC_PROG_CXX being called by AC_REQUIRE can be solved by directly overwriting them in util.m4, rather than providing our own UTIL_PROG_CC and so on. Unsure how satisfactory this is, but I have here a stripped down version of both that omits:

For AC_PROG_CC

• The default check from a list of compilers if the compilers to search for list is empty, this now means AC_PROG_CC has to always be called with the argument at all times. We always do this, so shouldn't be a problem
• The check whether the compiler supports GNU C. This shouldn't be relevant
• The ill fated check that adds -std=gnu11. This was the one that the other issue wanted to get rid of

For AC_PROG_CXX

• Similarly, the default check from a list of compilers if the compilers to search for list is empty, and the check for CCC if it is set (We don't use CCC as fair as I can tell), this now means AC_PROG_CXX has to always be called with the argument at all times. We again always do this, so shouldn't be a problem
• The check whether the compiler supports GNU C++. This shouldn't be relevant
• The ill fated check that adds -std=gnu++11. This is also the one that the other issue wanted to get rid of

Notably, I left the check that checks if the compiler accepts -g in there, as I was unsure if we need it or not. I also tried removing the check for the object file extension, but apparently autoconf needs that to function properly. The exe file extension check is misleadingly named, because in actuality all of AC_PROG_CC and AC_PROG_CXX's functionality is in it,and it's also likely required for autoconf to work properly

[magicus]

@TheShermanTanker I think this is essentially done. All that needs are the testing. Can you do it or do you want help with it?

[TheShermanTanker]

@TheShermanTanker I think this is essentially done. All that needs are the testing. Can you do it or do you want help with it?

I'm able to do so myself, I've just been busy with university so far. I've only tested 2.71 and 2.72 though, 2.69 and 2.70 might still need testing. I'm still unhappy that I can't properly undefine the macros, as a side tangent

[bridgekeeper]

@TheShermanTanker This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

[bridgekeeper]

@TheShermanTanker This pull request has been inactive for more than 8 weeks and will now be automatically closed. If you would like to continue working on this pull request in the future, feel free to reopen it! This can be done using the /open pull request command.

[TheShermanTanker]

/open

[openjdk]

@TheShermanTanker This pull request is now open

[bridgekeeper]

@TheShermanTanker This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

[magicus]

Keep it for a while more, bot. I'll try to some testing of this on 2.69 and 2.70 if Julian is unable to.

[bridgekeeper]

@TheShermanTanker This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

[bridgekeeper]

@TheShermanTanker This pull request has been inactive for more than 8 weeks and will now be automatically closed. If you would like to continue working on this pull request in the future, feel free to reopen it! This can be done using the /open pull request command.

[TheShermanTanker]

/open

[openjdk]

@TheShermanTanker This pull request is now open

[magicus]

Ok. I have now tested this patch with autoconf 2.69 and 2.70, and afaict it works just fine. So this is finally ready to integrate. @TheShermanTanker Will you have the honors?

[TheShermanTanker]

Sorry for essentially having you do the testing on my behalf. I'm still very unhappy that I have to overwrite an AC_DEFUN function like this, but there doesn't seem to be any other way. I'll fix the conflicts first though

[TheShermanTanker]

/integrate

[openjdk]

Going to push as commit 5eae20f.
Since your change was applied there has been 1 commit pushed to the master branch:

• 6ed6dff: 8341871: Disable G1 for unsupported platforms after JDK-8334060

Your commit was automatically rebased without conflicts.

[openjdk]

@TheShermanTanker Pushed as commit 5eae20f.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.