Hi, Sorry for the lag I have other urgent projects to deal with as do we all I'm sure :)
Thanks again Christofer for taking an interest, I'm sure some of your points are indeed red flags and it's good to catch them. Replies inline... On Sun, 2022-11-06 at 14:50 +0100, Sander Striker wrote: > Hi Chris, > > On Sun, Nov 6, 2022 at 11:15 AM Christofer Dutz <[email protected]> > wrote: > > > Sorry I must do this … but … > > > > -1 (Chirs) > > > > To avoid confusion, posting your feedback in the future without adding a > "-1" is more helpful and appropriate such that it is not misconstrued as an > actual vote. > > > > [MINOR] Download all staged artifacts under the url specified in the > > release vote email. > > > > * Generally, we like our download artifacts to be prefixed with > > “apache-“ > > > > https://dist.apache.org/repos/dist/release/httpd/ as a counter example. > That said, I actually see a benefit to the apache- prefix here, given the > non ASF maintenance releases. It will make it easier to distinguish. I have reluctantly conceded to use "Apache BuildStream" in the release announcements in order to make this distinction, however, I want to be very clear that I am against rebranding of BuildStream, just because we are now under the Apache umbrella. I don't think this distinction is particularly useful seeing that a maintenance 1.6 support release will never appear on dist.apache.org at all. So I think no, there is no need for our release channel to be publishing "apache-" prefixed source tarballs. > > * Most projects generally use a {version}/{rc}/ directory structure > > with a KEYS file in the projects root > > [FAILED] Verify the signature is correct. > > > > I'm expecting us to put a KEYS file here > https://dist.apache.org/repos/dist/release/buildstream/. > Ok, I haven't seen this in the httpd example or in release process documentation. Sander: please spend time with me once the vote finally passes to ensure that we are meeting expectations regarding this detail. > > * No KEYS file containing the public signatures of the release-manager > > used to sign the release > > * Couldn’t find key on any public servers I searched > > [OK] Check if the signature references an Apache email address. > > [OK] Verify the SHA512 hashes. > > > > * Both Hashes match > > [OK] Unpack the archive. > > [OK] Verify the existence of LICENSE, NOTICE files in the extracted source > > bundle. > > [MINOR] Verify the content of LICENSE, NOTICE files in the extracted > > source bundle. > > > > * The NOTICE file of the plugins archive references 2021 > > [FAILED] [RM] Run RAT externally to ensure there are no surprises. > > Thanks, lets take an action on this :) > > * Main bundle: > > * 1924 Unknown Licenses for the main bundle (Attached as rat.txt) > > > > The bulk seems to be tests and docs? The docs should be more easily > addressable. To be clear, what is expected here ? Do we want to add license headings to all of the .rst files comprising the documentation sources ? I suppose I'll do this anyway, perhaps I can script it. > > > * Some sources seem to be GPL licensed: > > * > > > > BuildStream-1.95.4.dev0/src/buildstream/_scheduler/queues/cachequeryqueue.py > > > > That's a good catch - this one seems to have slipped through the cracks > back when the changes landed in January after iterating on them from > September 2021. Another action point, I'm not sure how this escaped grep :) > > * Some sources don’t seem to be having any header: > > * BuildStream-1.95.4.dev0/src/buildstream/_scheduler/resources.py > > > > I can see how this one happened, it didn't have a header since it was > introduced in a refactoring in 2018. Good catch as well. Another action point. > > > * Tests/integration/project/files/amhello.tar.gz (all other copies > > of this file too) is a binary file (which is generally not allowed) and > > contains GPL licensed content and is infringing that license by not > > distributing the license with it (which is even less allowed). > > > > Note that these files all note "This program is free software; the Free > Software Foundation gives unlimited permission to copy, distribute and > modify it.", in line with generated autotools files. That said, we could > have it not be in a tarball, and also given it is a test, we may be able to > download it at test time. > As Christofer pointed out, we have accidentally included some autotools generated files which are generated at build time, which include the GPL license heading. However, the autotools example itself is not GPL licensed and as stated above, the FSF gives unlimited permission to copy, distribute and modify this content. The content of these tarballs *should* only contain the files here: https://git.savannah.gnu.org/cgit/automake.git/tree/doc/amhello I will amend these tarballs (which are all in fact the same tiny tarball repeated in multiple test directories) to not include the unneeded generated GPL licensed files. I am not really amenable to refactoring the test suite to download these tarballs just for this detail, it's overkill. If it was the case that we did in fact require GPL code for these tests, I would be more happy to rewrite the content of these tarballs from scratch than to work out downloading these at test time. > > * Admittedly I stopped a detailed analysis of other problems > > as > > this is already enough for a -1 > > > > Your input is appreciated. > > > > * Plugin bundle: > > * Rat reports: 17 Unknown Licenses for the plugin bundle > > (Attached > > as rat-plugin.txt) > > [OK] Search for Copyright references, and if they are in headers, > > make > > sure these files containing them are mentioned in the LICENSE file. > > > > I see an .asf.yaml file which we don't need to distribute. There's 2 > empty > __init__.py files, 3 *_requirements.txt files. The egg-info > directories > are generated. > The setup.cfg file seems to have lost its header in the packaging > process, > as it is there in the source repo. > The PKG-INFO file actually needs its Authors reference updated. > Looking at this, the Author field already says "BuildStream Developers" which accurately reflects what is specified in setup.py. The Home-page, Author-email and License all also appear to be correct. However, some of the Project-URL entries have not yet been changed to point to github instead of gitlab. What would you expect to be listed as the "Author" field instead ? Action Summary ============== Here are the actions I intend to take for yet another try to get our release channel up and running: * Update Copyright year in buildstream-plugins NOTICE file * Fix inaccurate license heading in: src/buildstream/_scheduler/queues/cachequeryqueue.py * Add missing license heading in: src/buildstream/_scheduler/resources.py * Update remnant gitlab links in setup.py (and consequently PKG-INFO) * Regenerate the amhello.tar.gz tarballs to accurately reflect the public domain material offered by upstream autotools: https://git.savannah.gnu.org/cgit/automake.git/tree/doc/amhello * Add license headings to all of the documentation sources. Cheers, -Tristan
