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.joels...@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.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 >>> >> >
