Ping again. I still need code review for the other changes. The latest webrev is at
http://cr.openjdk.java.net/~weijun/8201815/webrev.01 The only change since webrev.00 is in GendataPublicSuffixList.gmk. Thanks Max > On May 24, 2018, at 8:47 PM, Magnus Ihse Bursie > <magnus.ihse.bur...@oracle.com> wrote: > > On 2018-05-24 06:52, Weijun Wang wrote: >> Good. It's now simply >> >> $(GENDATA_PUBLICSUFFIXLIST): $(GENDATA_PUBLICSUFFIXLIST_SRC) $(BUILD_TOOLS) >> $(call LogInfo, Generating public suffix list) >> $(call MakeDir, $(@D)) >> $(RM) $@ >> $(TOOL_PUBLICSUFFIXLIST) $< $@ >> $(CHMOD) 444 $@ >> >> Thanks >> Max > Great! Looks good to me now. > > I've forgotten that we started using .DELETE_ON_ERROR. Thanks for reminding > me Erik. :) > > /Magnus > >> >>> On May 24, 2018, at 12:30 AM, Erik Joelsson <erik.joels...@oracle.com> >>> wrote: >>> >>> On 2018-05-23 03:54, Magnus Ihse Bursie wrote: >>>> mv should not modify attributes. cp will, but mv should not. >>>> >>>> Your solution might fail in the (admittedly unlikely) case that the build >>>> is interrupted before the chmod but after the mv. In that case, an >>>> incremental rebuild will not see that anything is missing. >>>> >>>> I believe the other cases that you quote are also incorrect. >>>> >>>> But I'd like to hear Erik's input on this as well. >>> We have the pseudo target .DELETE_ON_ERROR defined globally (in >>> MakeBase.gmk) which causes make to delete the target of any failed or >>> interrupted recipe. This should actually remove any need for using .tmp >>> files and mv on the last line. We still have a lot of those constructs >>> around since forever though. The recipe copied here (and the two other >>> examples) are based on a template from very early build-infra makefile >>> history and do not represent current best practices. If anything I would >>> recommend getting rid of the .tmp and mv completely, but if you prefer both >>> belt and suspenders, putting the move last should be the correct construct. >>> >>> /Erik >>>> /Magnus >>>> >>>>> 23 maj 2018 kl. 02:01 skrev Weijun Wang <weijun.w...@oracle.com>: >>>>> >>>>> >>>>> >>>>>> On May 23, 2018, at 4:21 AM, Magnus Ihse Bursie >>>>>> <magnus.ihse.bur...@oracle.com> wrote: >>>>>> >>>>>> ... but you should switch order on the chmod and the mv in the new >>>>>> gensrc file, so the mv comes last. >>>>> I thought it's safer to call CHMOD last so MV won't change file mode >>>>> back. (I'm not saying it will, just afraid.) >>>>> >>>>> In below cases, CHMOD is called after MV/CP. >>>>> >>>>> gendata/Gendata-java.base.gmk >>>>> 59- $(MV) $@.tmp $@ >>>>> 60: $(CHMOD) 444 $@ >>>>> 61- >>>>> >>>>> common/JavaCompilation.gmk >>>>> 80- $(CP) $$< $$@ >>>>> 81: $(CHMOD) -f ug+w $$@ >>>>> >>>>> Thanks >>>>> Max >>>>> >>>>>> /Magnus >>>>>> >>>>>>> 22 maj 2018 kl. 17:44 skrev Erik Joelsson <erik.joels...@oracle.com>: >>>>>>> >>>>>>> Build changes look ok. >>>>>>> >>>>>>> /Erik >>>>>>> >>>>>>> >>>>>>>> On 2018-05-22 08:25, Weijun Wang wrote: >>>>>>>> Please take a review at >>>>>>>> >>>>>>>> http://cr.openjdk.java.net/~weijun/8201815/webrev.00/ >>>>>>>> >>>>>>>> With this change, We switch from a home-grown public suffix list >>>>>>>> (implemented in sun/net/RegisteredDomain.java) to Mozilla's PSL. The >>>>>>>> PSL data was re-encoded as a zip file with entries for different TLDs. >>>>>>>> >>>>>>>> There is no plan to update the data in a different channel other than >>>>>>>> a JDK release. >>>>>>>> >>>>>>>> Thanks >>>>>>>> Max >
