Heads up, BDB coming, needed to support fast configuration

[Chris Hegarty]

Updated webrev, comments inline...
   http://cr.openjdk.java.net/~chegar/jigsaw/bdb_webrev.01/webrev/

On 11/05/12 00:22, Mandy Chung wrote:
> ...
> Great, you built on all platforms + a bonus one (linux-arm)! Have you
> tried SKIP_BOOT_CYCLE=false build?

Yeap, SKIP_BOOT_CYCLE=false builds fine.

 > You said "Partial builds that import
> from another JDK are still not implemented" - you mean it always
> requires building the entire forest unless USE_SYSTEM_BDB or
> ALT_BDB_IMPORT_PATH is specified, right?

Right, you got it exactly. Since the import requires the db.h header it 
is not quite suited for import from a JDK ( the JDK doesn't contain the 
db.h ). Long term we might want to put this header in a default location.

I didn't spend too much time on this since I believe the new build 
system will not support partial builds as we are use to today. My 
understanding ( and I could be wrong ) is that a full forest will be 
required and full build initially. Partial builds will essentially be 
fast incremental builds, where untouched repos, like bdb for example, 
can be quickly by passed. I'll work with the build-infra folks to 
determine the best solution for this going forward.

> I can push this for you when it's ready. Minor comments:
>
> jdk/make/common/Sanity.gmk
> Do we plan to enable BDB_IMPORT_CHECK some day to
> to check the library when make sanity

BDB_IMPORT_CHECK will only be skipped ( false ) when running sanity from 
the top-level repo. The jdk sanity (when invoked form top) should not 
check for the header before it has been generated. For jdk (partial) 
builds then the sanity is always run. This is similar to the hotspot 
imports.

I seen an issue when trying to answer your question. The jdk sanity 
should always check for db.h when USE_SYSTEM_BDB is set. I updated 
jdk/make/common/Sanity.gmk and jdk/make/common/shared/Sanity.gmk to 
support this.

> PersistentTreeMap.java
> L52: "is" a typo? nit: would be good to add a space after //
Thanks, got it.

> L134: I assume the getStringAndInt method is a helper method
> for testing purpose only, right? Might be worth to add a comment.

Hmmm... I'm not sure about this. If it's ok, I'll leave it as it for 
now, we can revisit later if necessary.

-Chris.

>
> Thanks
> Mandy