kmcguigan at twitter.com
Thu Apr 17 11:53:42 UTC 2014
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
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.
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.
> 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
>> 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
>> 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".
>> 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:
>>> 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
>>> 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
>>> 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
More information about the build-dev