On Sat, Jun 24, 2023 at 10:28:07PM +0200, Daniel Gröber wrote: > Hi Jonathan,
Hi Daniel. > Thanks for the detailed review and testing! > > On Sat, Jun 24, 2023 at 12:35:12PM +0200, Jonathan Neuschäfer wrote: > > [ Dropping the Debian mailing lists / bugtracker for now ] > > No need, I belive reviews should be public :) I've bounced your mail back > to BTS/pkg-electronics. > > > Some notes: > > > > prjtrellis: > > > > - I tried to build it from source (using the tarballs, with > > dpkg-buildpackage), > > but it failed because it can't find version.cpp. After a dirty hack to > > ensure > > that version.cpp is present and usable, and unpacking the orig-database > > tarball to the right location, the package builds for me. > > Did you build with git/gbp or using the dsc? I see that the build system > tries to extract the version using git commands. However I always test with > sbuild so my build environment should also have been a plain unpacked dsc > and not a git repo. Strange. I didn't use Git (except separately, as a reference for comparison). I extracted the orig and debian tarballs manually and then used dpkg-buildpackage. Perhaps not the recommended method :) > AFAICT I can define CURRENT_GIT_VERSION from d/rules and that should take > care of it. Sounds good. > > > apycula: > > > > - I think the source package name should rather be apicula (with 'i', > > not 'y'), which matches the upstream repo and readme.md. Apycula is > > the python binding, AFAIUI. > > NACK, what I'm packaging here is the a-py-cula pipy.org package which is > the main output of project a-pi-cula. Ah ok, that probably also explains the provenance of the pickle files. > > - The debian orig tarball contains a few GW1N*.pickle files (in the > > apycula subdirectory), that don't appear in the upstream git repo. > > They seem to be generated, and are later used in the nextpnr build, but > > it's unclear to me where they come from, where the source is. > > Indeed I agree it's a bit dodgy and probably warrants an explaination in > d/README.source. So what project apicula did is write fuzzers to > map/document what bits in the FPGA bitstream perform what function. > > They do this by running the proprietary FPGA-vendor toolchain using their > automated fuzzing tools. The pickle files encompass the output of these > tools and essentially provide a map of what logical function is where in > the hardware. Right, indeed. Other toolchains have the same problem, but solve it differently in Debian, for example with the prjtrellis_1.4.orig-database.tar.gz source tarball. I would expect that the apycula database is also maintained in an upstream git repo, akin to prjtrellis-db, but I haven't found one. > The a-pi-cula source repo contains these fuzzin tools and produces as > output the a-py-cula pipy package which is uploaded by the maintainer. > > I haven't consulted the sacred scriptures on the finer points yet but IMO > the pickle files contain facts about the physical world, not computer > code. My understanding is facts are not copyrightable in the first place so > the usual derrivative work considerations don't apply. > > You can imagine I'm looking forward to ftp-master review ;) In any case, apycula isn't the first to ship such data, as far as I understand it. > > nextpnr: > > > > - debian/upstream/metadata: Some of the authors have updated names that > > should be used instead of the old names. As far as I can gather: > > - Myrtle Shah[1] (a.k.a. gatecat) > > - Claire Xenia Wolf[2] (a.k.a. Claire Xen) > > This one is tricky. The license asks that the notices in the source be > preserved, so it's not clear to me we can honor such name changes without > explicit permission from the respective copyright holder. Ah well, ok, that's beyond my legal expertise and I can't speak for the authors. > > - Because I removed the GWIN*.pickle files of unknown origin, I skipped the > > gowin targets. > > Do note prjtrellis has the same layout information in the -database > component just in a more human readable format (json). Have a look at that > and you'll get an idea of what sort of data we're talking about here. Yep. It's just that I didn't find the files in the upsteam Git repo that I checked (and I didn't think to check the pypi package). > > > - Building multiple times in the same source directory fails, > > override_dh_auto_clean needs a > > "rm -rf debian/nextpnr-*-qt-chipdb/usr/share/nextpnr/chipdb-*.bin" or > > similar. > > Or maybe upstream "make clean" needs to be fixed. > > - Somehow the chipdb files are installed to /usr/share/nextpnr/chipdb-*.bin > > directly, while nextpnr expects them in per-arch subdirectories. As a > > quick > > work-around I'm symlinked the expected subdirectories to . > > > > > > After all this, I've built a simple test design for ECP5 (OrangeCrab), > > and it worked! > > Excellent news! > > > Thanks for your work! I hope my comments help to improve it further :) > > Definitely :) > > Thanks, > --Daniel Thanks again, and sorry for the delay (that was due to personal timing). Jonathan
