JDK-8025705

[Keith McGuigan]

Hi Erik,

Sorry I posted this to the wrong list.

I agree that using CUSTOM_SRC_DIR, configured via 'configure', is a good
idea (Dave's #1).  I'm not so sure that file existence checks are the best
idea, though, for the reasons Erik points out -- change a filename and all
of sudden things just stop working because that happened to be the file
that the Makefile was looking for as a trigger for a particular feature.
 Probably better would be individual "feature" flags that are (again)
enabled or disabled via configure. This would allow build-time checking of
the existence of files and would not be as brittle.  But how many flags
would that be?  68 seems a bit much (though I'm sure we'd get some overlap
in features).  Maybe something in-between, where we check for some
top-level directory and turn features off or on based upon the existence of
that?

If there's a reasonable solution that I can start working on now, that'd be
great -- anything we do in this direction (even a partial solution) would
help reduce our ongoing maintenance costs and would be worth doing IMO.

--
- Keith







On Thu, Apr 17, 2014 at 3:28 AM, Erik Joelsson <erik.joelsson at oracle.com>wrote:

> (moving discussion to build-dev since this isn't directly part of the
> makefile rewrite project)
>
> Hello Keith,
>
> I certainly feel your pain in dealing with this, it's currently a mess.
> I'm not as opposed to the "ORACLEJDK" variable as David is, but I'm also
> not sure it will correctly express things in all current situations. In
> many cases, specific individual variables would probably make more sense.
> Also note that we have several references to OPENJDK in the Oracle closed
> makefiles that would need to be updated at the same time.
>
> I agree that we need to move away from explicitly writing "src/closed". We
> should definitely move as much of the Oracle specific parts of the build to
> closed makefiles, even though it's sometimes tricky.
>
> I don't think just having existence checks is enough. We do internally
> support the configure flag --enable-openjdk-only, which forces the build to
> ignore the non openjdk parts. It's not 100% functioning today, but I think
> it should be. This is also about consistency checking during the build. If
> parts of the build is optional just depending on existence of files, then a
> misspelled reference or other mistake can make those parts being silently
> excluded. At least for the common build scenarios/configurations I would
> like the build to know what needs to be built, which means we need explicit
> variables to control it.
>
> /Erik
>
>
> On 2014-04-17 06:52, David Holmes wrote:
>
>> Hi Keith,
>>
>> src/closed is Oracle's "custom source" location (hotspot calls it
>> alt_src). If we never saw src/closed in the makefiles, only CUSTOM_SRC_DIR,
>> and guarded with an existence test for a specific directory/file, then that
>> should address your problem. That would be a first step in moving things to
>> the custom makefiles where they belong.
>>
>> I'm opposed to the ORACLEJDK variable because I want to maintain the
>> pressure to get this fixed properly. If we hack around it then it will
>> never get cleaned up.
>>
>> I see 68 uses of src/closed across 14 files in the JDK repo. That seems
>> tractable.
>>
>> I think there are three things to be done here:
>>
>> 1. Replace all uses of src/closed with CUSTOM_SRC_DIR (similar to
>> CUSTOM_MAKE_DIR) which in turn is set via configure
>> 2. Guard all uses of CUSTOM_SRC_DIR in open makefiles with an existence
>> check
>> 3. Move all uses of CUSTOM_SRC_DIR to our closed makefiles
>>
>> Steps 1 and 2 can happen now. Step 3 is long term goal.
>>
>> ---
>>
>> The other problem I see with the OPENJDK, ORACLE_JDK, OTHER_JDK approach
>> is that you actually have to deal with the permutations. Something
>> currently flagged for OPENJDK really means !ORACLE_JDK - or does it? It
>> actually depends on what sources a given licensee has. Even for your custom
>> build you might want some OPENJDK items and not others. I'm not sure there
>> is a general solution, but using OPENJDK in combination with CUSTOM_SRC_DIR
>> is, I think, more flexible than trying to define discrete variables that
>> represent build "targets".
>>
>> David
>>
>> On 17/04/2014 1:31 PM, Keith McGuigan wrote:
>>
>>> On Wed, Apr 16, 2014 at 9:15 PM, David Holmes <david.holmes at oracle.com
>>> <mailto:david.holmes at oracle.com>> wrote:
>>>
>>>     Hi Keith,
>>>
>>>
>>>     On 17/04/2014 7:13 AM, Keith McGuigan wrote:
>>>
>>>         Hello,
>>>
>>>         I just added a comment to this bug -- see there for the details,
>>>         but in
>>>         short I'd like to update a number of tests in the makefiles that
>>>         check
>>>         OPENJDK and change them to check instead of the inverse
>>>         definition of some
>>>         new variable, such as ORACLEJDK.
>>>
>>>
>>>     Please no! It's bad enough this is implicitly in the build without
>>>     making it explicit!
>>>
>>>
>>> As I mentioned, I agree that moving all this to closed makefiles is the
>>> best solution (and is something that we could push for even if we took
>>> this partial step), but doing at least this step would be a vast
>>> improvement from our point of view and is much easier to implement,
>>> especially for someone like me who cannot do the make/closed refactoring.
>>>
>>>     Would file existence tests suffice? There should be a CUSTOM_SRC
>>>     variable for src/closed as there is CUSTOM_MAKE for make/closed.
>>>
>>>
>>> It's not really feasible for the jdk makefiles.  Almost each location
>>> where there is an OPENJDK test, when it is discovered that this isn't
>>> OpenJDK, it ends up referring to files in src/closed (which for us don't
>>> exist).  In Hotspot it's only a few makefiles, so not too bad there, but
>>> jdk is a different story.
>>>
>>> But really, there's three situations here, OpenJDK, OracleJDK, and
>>> OtherJDK/custom, which can't be encoded using one boolean makefile
>>> variable.  We really need at least one more here.  Why is ORACLEJDK so
>>> abhorent?
>>>
>>>         This would simply non-OpenJDK (i.e.,
>>>         src/closed builds), non-Oracle builds for those who are making
>>>         their own
>>>         distributions using the src/closed mechanism.  As you can guess,
>>>         that is
>>>         something we are doing here at Twitter :)
>>>
>>>
>>>     Hopefully you use src/custom (or whatever) not src/closed, as
>>>     otherwise there's no way to tell the difference between our custom
>>>     sources and yours.
>>>
>>>
>>> The makefiles are already setup to use src/closed, so really that's the
>>> most convenient way to add augmented sources to the build.  We'd very
>>> much like to avoid changing mainline code to reduce the
>>> maintenance/merge costs when things change.  I'm not sure it would help
>>> even if we did use a custom directory instead of 'closed' though --
>>> unless we went ahead and duplicated all of the 'closed' logic for
>>> 'custom' (which again, would incur maintenance costs and is not
>>> generally good engineering practice).
>>>
>>> --
>>> - Keith
>>>
>>
>