Hi Andrew, On Wed, 2022-02-09 at 03:54 +0000, Andrew Hughes wrote: > On 20:33 Thu 03 Feb , Severin Gehwolf wrote: > > On Wed, 2021-12-22 at 11:14 +0100, Severin Gehwolf wrote: > > > On Fri, 2021-12-10 at 15:11 +0100, Severin Gehwolf wrote: > > > > Hi, > > > > > > > > Please review this adaptation of the corresponding JDK 11 patch. The > > > > JDK 11u patch didn't apply because the build system is widely different > > > > between these two releases. > > > > > > > > The main difference is make/common/MakeBase.gmk (JDK 8) vs > > > > make/SourceRevision.gmk (JDK 11). I've basically rewritten those parts > > > > of the patch. All the SCM handling in JDK 8 is in MakeBase. > > > > FindAllReposAbs isn't present in JDK 8u. > > > > > > > > Bug: https://bugs.openjdk.java.net/browse/JDK-8210283 > > > > webrev: > > > > https://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8210283/01/webrev/ > > > > > > > > Testing: "make --trace source-tips" on mercurial tree as well as > > > > the git mirror. $IMAGE_DIR/release file contains the SHA of > > > > the sources it was built from with 'git:' or 'hg:' prefixes. > > > > > > > > Thoughts? > > > > > > Anyone? When building from the git read-only mirror the "release" file > > > no longer includes the git sha it was built from without this fix. > > > > Well, surely it's never contained a git SHA? :) > It doesn't have the source IDs because there is no Mercurial repository > information, so it's as if it was built from a source tarball or some such. > > > Anyone willing to review this? > > > > Thanks, > > Severin > > > > This looks ok to me. I would omit the line "Called from > jdk/make/closed/bundles.gmk" because this file doesn't exist in our > repository and we have no idea what it does or doesn't contain.
Removed that part of the comment. > I think it's also worth noting in the summary text that this removes > the forest handling, which wasn't part of the original change for > obvious reasons. Done. > JDK-8031567 was the change that introduced make/SourceRevision.gmk, > but that was at a time when Mercurial forests were still in use, and > it seems that the use of `hg id` has been adopted in 8u separately > form that bug anyway. > > I think this change is simple enough as is for what we need, though > I'll admit I've never made use of this functionality myself. Latest webrev: https://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8210283/02/webrev/ OK? Thanks, Severin
