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 check3. 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.hol...@oracle.com <mailto:david.hol...@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
