Yup, agreed.

Great work, thanks.

Gj

On Wed, 8 Nov 2017 at 21:36, Jan Lahoda <[email protected]> wrote:

> Thanks Ate, I tried to clean up the notice fragments here:
> https://github.com/apache/incubator-netbeans/pull/261
>
> Overall, what we need to fix seems to be:
> -the notices:
> https://github.com/apache/incubator-netbeans/pull/261
> -change the cluster.config in the source zip so that it only refers to the
> cluster config packed in the zip:
> https://github.com/apache/incubator-netbeans/pull/260
> -not copy the ide launchers to the platform (working on that, but not done
> yet)
> -delete inclusion of jsearch.jar in apisupport.harness (should be trivial
> by itself)
> -add a README - this I guess may be the trickiest fix, as that needs to be
> different for the platform and for the IDE
>
> Geertjan, it might make sense to cancel the vote and restart it when this
> is done (as these changes are significant).
>
> Jan
>
>
> On Wed, Nov 8, 2017 at 4:16 PM, Ate Douma <[email protected]> wrote:
>
> >
> >
> > On 2017-11-08 11:18, Jan Lahoda wrote:
> >
> >> On Wed, Nov 8, 2017 at 10:44 AM, Ate Douma <[email protected]> wrote:
> >>
> >> On 2017-11-08 07:54, Jan Lahoda wrote:
> >>>
> >>> Hi Ate,
> >>>>
> >>>> Thanks a lot for your review and comments.
> >>>>
> >>>> On Wed, Nov 8, 2017 at 12:04 AM, Ate Douma <[email protected]> wrote:
> >>>>
> >>>> I get a different build error:
> >>>>
> >>>>>
> >>>>> BUILD FAILED
> >>>>> /home/ate/tmp/netbeans/netbeans-platform-source-platform-9.
> >>>>> 0-alpha/nbbuild/build.xml:122:
> >>>>> Download of 276009D3F0A27079A80D241C3183EC
> >>>>> 712305A42A-net.java.html.boot.
> >>>>> script-1.5.jar
> >>>>> produced content with hash 5DE9DD70EDCD9B30E0671ACC813F6F0C80E951AA
> >>>>> when
> >>>>> 276009D3F0A27079A80D241C3183EC712305A42A was expected
> >>>>>
> >>>>>
> >>>>> Sorry, not sure what is happening, I tried to download:
> >>>> https://search.maven.org/remotecontent?filepath=org/netbeans
> >>>> /html/net.java.html.boot.script/1.5/net.java.html.boot.script-1.5.jar
> >>>>
> >>>> and it seems to have sha1sum:
> >>>> 276009d3f0a27079a80d241c3183ec712305a42a
> >>>>
> >>>>
> >>> Indeed, if I download the above manually I can confirm it to have the
> >>> expected sha1sum.
> >>>
> >>> The problem seems to be caused by my local maven repository already
> >>> having these artifacts, and *those* have different sha1sum...
> >>> Disabling my local maven repository 'fixed' this and the build
> succeeds.
> >>> And ant tryme does start the platform now.
> >>>
> >>>
> >>>
> >>>> But disregarding the above, I think a more serious, blocking, issue is
> >>>>
> >>>>> that AFAICT *not* all platform modules have been reviewed yet.
> >>>>> I notice this release candidate includes the apisupport.harness
> module.
> >>>>> And that still has status "To do". Furthermore, it depends on the
> *GPL*
> >>>>> external jsearch-2.0_05.jar which is a no-go... for the ASF.
> >>>>> See: https://www.apache.org/legal/resolved.html#category-x
> >>>>>
> >>>>>
> >>>>
> >>>> Oops, sorry I missed this one  I think there's no other solution than
> to
> >>>> simply remove that. The build of a platform application modules that
> >>>> have
> >>>> help will then (I believe) fail, but users should  be able to supply
> the
> >>>> jar manually.
> >>>>
> >>>>
> >>> Just provide a README (and not just for this reason, see my other
> >>> problem starting the binary) which explains users what they need to do
> >>> *if* they want/need to use help.
> >>>
> >>> With regards to the current release candidate I've to vote -1 based on
> >>> the current findings as IMO this one is blocking.
> >>>
> >>> (An alternative would be to make such a build pass with a
> >>>
> >>>> warning, but since that would lead to a wrongly built module, it is
> >>>> better
> >>>> to continue failing IMO.)
> >>>>
> >>>> (We may need to enhance the license checking task to fail on GPL.)
> >>>>
> >>>>
> >>> It most definitely should fail on encountering GPL.
> >>>
> >>>
> >>>
> >>>>
> >>>>> I haven't looked in more detail yet, but before spending more time on
> >>>>> it
> >>>>> I'd like to know if the inclusion of the harness module is an
> oversight
> >>>>> which can be quickly fixed. Regardless, I think this should be fixed
> >>>>> first.
> >>>>>
> >>>>>
> >>>>> I was thinking of removing the harness, but a) we would need to
> finish
> >>>> work
> >>>> on harness sometime soon anyway; b) I think the platform is more
> useful
> >>>> with the harness than without it. So, I was fixing problems in the
> >>>> harness
> >>>> modules I've noticed.
> >>>>
> >>>>
> >>>> Note that the binary also includes the jsearch jar.
> >>>>> And while the binary isn't the release itself, if/when a convenience
> >>>>> binary is provided *then* it needs to be in compliance with the
> >>>>> distribution rules. So no GPL artifacts allowed.
> >>>>>
> >>>>> I also see the bundled LICENSE and NOTICE files in the binary zip
> >>>>> are a bit weirdly formatted and the NOTICE file in particular.
> >>>>> Seems both are produced by concatenating several files together, but
> >>>>> the result is confusing, especially the NOTICE file.
> >>>>>
> >>>>>
> >>>>> Yes, the NOTICE is a concatenation of several files, as i suspect it
> is
> >>>> not
> >>>> feasible to have a manually updated NOTICE file for each distribution
> we
> >>>> do. The form can surely be improved.
> >>>>
> >>>>
> >>>> The NOTICE file should only contain what is needed, nothing
> >>>>> more. Of the current content only the OSGi Alliance notice seems to
> be
> >>>>> needed, beside the base NOTICE header.
> >>>>>
> >>>>>
> >>>>> It unfortunately wasn't clear which notices are needed and which not,
> >>>> so I
> >>>> was trying to err on the side of including them. Fortunately, it
> should
> >>>> be
> >>>> very easy to reduce the notices.
> >>>>
> >>>>
> >>>> A possible solution is to use/add an additional NOTICE-APPEND file,
> for
> >>> those modules that need it, which only contains the bits needing to be
> >>> appended, so without the standard Apache header.
> >>> Then the ant task just needs to aggregate/append those files (when
> >>> encountered) to the root NOTICE file.
> >>>
> >>>
> >> This is almost exactly what is done: there may be -notice.txt files in
> >> */external, which are merged (if the binary is used) to the NOTICE. I
> >> guess
> >> there is a problem with "the bits needing to be appended", which is
> still
> >> unclear to me (i.e. which exact notices need to be appended and which
> not,
> >> I've read the pages about that but still unclear to me).
> >>
> >
> > The canonical rule is defined here:
> >
> >   http://www.apache.org/dev/licensing-howto.html#mod-notice
> >
> > Which bottom line means: only add (and aggregate) notices when
> > explicitly required.
> > Either because specific copyright notices were transferred from source
> > files OR when including (in source, or as bundled dependency) external
> > resources which carry or require a NOTICE (file) themselves.
> > The latter *may* include artifacts from the project itself (e.g. in case
> > of the binary distribution) which contain a NOTICE file.
> > But even then, only the relevant bits need/should be merged/appended.
> > So we should not merge NOTICE fragments for bit not applicable/bundled,
> > nor does the standard ASF NOTICE header
> >
> >   <Project Foo>
> >   Copyright x-y The Apache Software Foundation
> >
> >   This product includes software developed at
> >   The Apache Software Foundation (http://www.apache.org/).
> >
> > be repeated.
> >
> > Following the above 'rules' should produce the correct and minimal
> > NOTICE file needed.
> >
> >
> >>
> >>
> >>>
> >>>
> >>>
> >>>> The LICENSE file correctly lists and appends the 3rd party licenses,
> >>>>> but:
> >>>>> a) The jsearch GPL-2-CP license obviously shouldn't be there (see
> >>>>> above)
> >>>>> b) The jemmy *external* libraries are available under CDDL or
> GPL-2-CP,
> >>>>>      in which case we simply 'pick' CDDL and don't need to
> >>>>> include/append
> >>>>>      the GPL-2CP license too.
> >>>>>
> >>>>>
> >>>>> Ok. It was not clear how to handle dual licenses. Stripping the GPL
> >>>> alternative should be simple.
> >>>>
> >>>>      This is (AFAIK) not blocking, but preferred to remove the
> GPL-2-CP
> >>>>
> >>>>      license text as it easily can 'trip' casual reviewers or even
> >>>>
> >>>>>      automatic license scanning tools which may draw the wrong
> >>>>> conclusion.
> >>>>>      (and why is jemmy an external dependency and not part of the
> code
> >>>>>       donation?)
> >>>>>
> >>>>> Finally, I tried to start the binary.
> >>>>> Sorry for probably being a noob here but I can't figure out if I'm
> >>>>> doing
> >>>>> something wrong or the binary itself has a problem.
> >>>>> If I try to just run the bin/netbeans shell script, I get the
> following
> >>>>> error:
> >>>>> WARNING [org.netbeans.core.startup.Main]
> >>>>> java.lang.NoClassDefFoundError: org.netbeans.license.AcceptLicense
> >>>>> starting from org.netbeans.MainImpl$BootClassLoader@1c20c684 with
> >>>>> possible defining loaders null and declared parents ]
> >>>>>           at org.netbeans.core.startup.Main.getKlass(Main.java:341)
> >>>>>
> >>>>>
> >>>>> Hm, I apparently was only starting this with ant tryme, which skips
> the
> >>>> license dialog :-( so i didn't notice. We need to fix this.
> >>>>
> >>>>
> >>> Yes, if the binary cannot be started/used then why provide it?
> >>>
> >>> IMO dome kind of instruction how to start/verify the binary should be
> >>> provided.
> >>> As a minimum in the vote thread, and when accepted in the release
> >>> announcement mail and release download page.
> >>> But preferably (also) in a bundled README, which then can provide more
> >>> details (see above).
> >>>
> >>>
> >> As Jarda noted in:
> >> https://issues.apache.org/jira/browse/NETBEANS-128
> >>
> >> the "netbeans" launchers should not be there. These are IDE launchers,
> and
> >> should not be in the platform. Platform applications delegate to
> >> platform/lib/nbexec, which probably should be the only launcher in the
> >> platform (+the templates in the harness).
> >>
> >> Jan
> >>
> >>
> >>
> >>>
> >>> Jan
> >>>>
> >>>>
> >>>>
> >>>>> Ate
> >>>>>
> >>>>>
> >>>>> On 2017-11-07 21:49, John McDonnell wrote:
> >>>>>
> >>>>> I want to vote yes, but should the sources be buildable?
> >>>>>
> >>>>>>
> >>>>>> If I execute ant the build fails?
> >>>>>>
> >>>>>> $ ant
> >>>>>> ...
> >>>>>> [insert-module-all-targets] This module is missing from checkout:
> >>>>>> maven.apisupport - at least can't find:
> >>>>>> /Users/john/Downloads/netbeans-platform-source-platform-9.0-
> >>>>>> alpha/maven.apisupport/build.xml
> >>>>>> [insert-module-all-targets] This module is missing from checkout:
> >>>>>> timers - at least can't find:
> >>>>>> /Users/john/Downloads/netbeans-platform-source-platform-9.0-
> >>>>>> alpha/timers/build.xml
> >>>>>>
> >>>>>> BUILD FAILED
> >>>>>> /Users/john/Downloads/netbeans-platform-source-platform-9.0-
> >>>>>> alpha/nbbuild/build.xml:156:
> >>>>>> Some modules according your cluster config 'basic' are missing from
> >>>>>> checkout, see messages above.
> >>>>>>
> >>>>>> I removed all but 2 of the errors.
> >>>>>>
> >>>>>> Am I missing something?  If I am then I'll happily vote +1 (non
> >>>>>> binding)
> >>>>>>
> >>>>>> Regards
> >>>>>>
> >>>>>> John
> >>>>>>
> >>>>>> On 7 November 2017 at 19:20, Benny Neugebauer <[email protected]> wrote:
> >>>>>>
> >>>>>> +1
> >>>>>>
> >>>>>>>
> >>>>>>> But the link to the artifact is broken. It should be:
> >>>>>>> https://builds.apache.org/view/Incubator%20Projects/job/incu
> >>>>>>> bator-netbeans-release/lastSuccessfulBuild/artifact/dist/
> >>>>>>> netbeans-platform-source-platform-9.0-alpha.zip
> >>>>>>>
> >>>>>>> On 2017-11-07 11:17, Geertjan Wielenga <[email protected]>
> wrote:
> >>>>>>>
> >>>>>>> Hi all,>
> >>>>>>>
> >>>>>>>>
> >>>>>>>> Please vote on releasing Apache NetBeans 9.0 Alpha (incubating)!
> If
> >>>>>>>> this>
> >>>>>>>> voting passes, another similar voting will be started on>
> >>>>>>>> [email protected], and if that passes too, then we can
> >>>>>>>> release>
> >>>>>>>> this version.>
> >>>>>>>>
> >>>>>>>> Apache NetBeans 9.0 Alpha (incubating) are the modules of Apache
> >>>>>>>> NetBeans>
> >>>>>>>> that provide the application framework of NetBeans, that is, the
> >>>>>>>> NetBeans>
> >>>>>>>> Platform.>
> >>>>>>>>
> >>>>>>>> Build artifacts:>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> https://builds.apache.org/view/Incubator%20Projects/job/incu
> >>>>>>>> bator-netbeans-release/>
> >>>>>>>>
> >>>>>>>> Source md5sum is 548058644764a07ef54568aa79a10aa1.>
> >>>>>>>>
> >>>>>>>> The artifact to be voted on:>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> https://builds.apache.org/view/Incubator%20Projects/job/incu
> >>>>>>>> bator-netbeans-release/lastSuccessfulBuild/artifact/dist/
> >>>>>>>> netbeans-platform-source-.zip>
> >>>>>>>>
> >>>>>>>> Rat report shows no unknown licenses:>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> https://builds.apache.org/view/Incubator%20Projects/job/incu
> >>>>>>>> bator-netbeans-release/lastSuccessfulBuild/artifact/rat-
> >>>>>>>> temp/nbbuild/build/rat-report.txt>
> >>>>>>>>
> >>>>>>>> Included as a convenience is a binary, unzip it and run it and
> >>>>>>>> you'll
> >>>>>>>> see>
> >>>>>>>> the NetBeans Platform:>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> https://builds.apache.org/view/Incubator%20Projects/job/incu
> >>>>>>>> bator-netbeans-release/lastSuccessfulBuild/artifact/dist/
> >>>>>>>> netbeans-platform-bin-.zip>
> >>>>>>>>
> >>>>>>>> Included in the above are the DEPENDENCIES%2
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>
> >>
>

Reply via email to