Bug#833193: closed by Sean Whitton <spwhitton@spwhitton.name> (Closing inactive ITP RFSs)
Hello Sean,
Thank you for taking the time to review this package. I have addressed much
of the feedback that you have provided, as well as upgraded the package to the
latest Chapel release, 1.14.0.
An easily-browsable version of the source package contents can be here:
https://github.com/ben-albrecht/chapel-minimal-review
Alternatively, one can download the package with dget using this command:
dget -x https://mentors.debian.net/debian/pool/main/c/chapel-minimal/chapel-minimal_1.14.0-1.dsc
See responses below:
>=== Must-fixes
>1. The clean target isn't idempotent. I.e. if you try to run
>`debian/rules clean` when the package is already clean, it fails.
Corrected, by patching a Makefile that was accessing a directory that does not exist in the package.
>2. Please add proper headers to your patches, preferably in DEP-3
>format. The most important thing is to explain why the patch is
>necessary. You can also use the Forwarded: header to indicate that the
>patches are Debian-specific or not.
Corrected.
>3. debian/changelog contains a spurious space :)
Corrected.
>4. These lintian informational warnings should be fixed or overriden
>with explanations for why the warning is a false positive:
>
> I: chapel-minimal: hardening-no-pie usr/lib/chapel/bin/linux32/chpl
> I: chapel-minimal: hardening-no-bindnow usr/lib/chapel/bin/linux32/chpl
> I: chapel-minimal: hardening-no-fortify-functions usr/lib/chapel/bin/linux32/chpl
Corrected, by patching Makefile to recognize CPPFLAGS, and enabled hardening for
dpkg-buildflags.
>5. The UTF-8 decoder needs to be packaged separately -- Debian strongly
>discourages convenience copies of code. It might already be packaged
>for Debian, or you might have to prepare another RFS.
I have not made any changes in response to this feedback.
After discussing with others in the #debian-mentors channel, I don't
think packaging this separately is necessary.
UTF-8 Decoder is a 42 line header file, which requires modifications to use for
Chapel specifically. Modifications include renaming and static prefixes to
avoid collisions.
See https://codesearch.debian.net/search?q=bjoern.hoehrmann.de for a list
of existing packages that include UTF-8 decoder in their package.
>6. Your package is "inadequate" -- don't worry, it just means that it
>fails the tests performed by the adequate(1) tool. Please see if you
>can fix them, or explain why they shouldn't/can't be fixed:
>
> chapel-minimal: broken-symlink /usr/bin/chpl -> ../lib/chapel/bin/linux64/chpl
> chapel-minimal: py-file-not-bytecompiled /usr/lib/chapel/util/chplenv/__init__.py
> chapel-minimal: py-file-not-bytecompiled /usr/lib/chapel/util/chplenv/chpl_3p_gmp_configs.py
> chapel-minimal: py-file-not-bytecompiled /usr/lib/chapel/util/chplenv/chpl_3p_hwloc_configs.py
> chapel-minimal: py-file-not-bytecompiled /usr/lib/chapel/util/chplenv/chpl_3p_jemalloc_configs.py
> chapel-minimal: py-file-not-bytecompiled /usr/lib/chapel/util/chplenv/chpl_3p_massivethreads_configs.py
> chapel-minimal: py-file-not-bytecompiled /usr/lib/chapel/util/chplenv/chpl_3p_qthreads_configs.py
> chapel-minimal: py-file-not-bytecompiled /usr/lib/chapel/util/chplenv/chpl_3p_re2_configs.py
> chapel-minimal: py-file-not-bytecompiled /usr/lib/chapel/util/chplenv/third_party_utils.py
> chapel-minimal: py-file-not-bytecompiled /usr/lib/chapel/util/chplenv/utils.py
>
>I think these might be due to PYTHONDONTWRITEBYTECODE=1 in d/rules.
>Perhaps you could add a comment to the rules file explaining why you
>need that setting.
Corrected.
>7. The package is not compliant with the FHS. Almost everything is
>installed into /usr/lib/chapel, plus a symlink from /usr/bin/chpl into
>/usr/lib/chapel (only to the 64-bit version?). The main executable
>needs to be installed into /usr/bin/chpl. Please take a look at Debian
>policy section 9.1.1 and install the files appropriately.
I have not made any changes in response to this feedback.
The 'chpl' binary expects to live in $CHPL_HOME, and depends on
the contents of $CHPL_HOME/lib, $CHPL_HOME/make, $CHPL_HOME/modules, and
$CHPL_HOME/util. Therefore, I install this $CHPL_HOME directory as
/usr/lib/chapel, with the 'chpl' binary in $CHPL_HOME/bin/, so that it can
identify $CHPL_HOME without setting an environment variable. I read the 9.1.1
documentation on the FHS, and still am not sure if /usr/lib or /usr/share is
the right place for $CHPL_HOME.
Any guidance on this would be much appreciated.
>8. Hint for the previous item: check out dh_installchangelogs(1),
>dh_installdocs(1) and dh_installexamples(1).
Thanks for the pointers.
>9. As discussed on IRC, missing-sources is meant to be a directory
>containing the missing sources, not a list of the things that are
>missing. You need source code for everything in your package.
Corrected.
>10. You seem to be using dh_python2 but nothing gets installed to
>/usr/lib/python2/dist-packages. Is this deliberate?
Correct, I initially thought I needed dh_python2, since the installed
binary depends on a variety of Python scripts, but you are correct - it is not
needed. Corrected.
>11. I'm not an expert on multi-arch issues but this package seems to be
>targeted only at 32-bit and 64-bit Linux machines, just from looking at
>the 'linux32' and 'linux64' directories you have a lot of. Debian
>supports lots of other architectures, and the package should work on
>those. Is that something that can be fixed?
I have not made any changes in response to this feedback.
This is a way in which Chapel distinguishes platforms, which are independent
of the architectures Debian distinguishes. I would expect all Debian-supported
architectures to be categorized as either 'linux32' or 'linux64' platforms
for Chapel's purposes.
For a list of our platforms, see our documentation:
http://chapel.cray.com/docs/1.14/usingchapel/chplenv.html#chpl-host-platform
>=== Suggested improvements
>1. It would be nice to have some paragraph breaks in the extended
>description.
Added a line break.
>2. You could add Vcs-Git and Vcs-Browse fields to d/control.
Added these fields.
>There are probably more improvements that you could make but it is hard
>to review the package because of items 7--11 above. So I'll leave you
>with these things to work on for now and then take another look :)
Thanks again for your early feedback. Looking forward to your response.
Best,
Ben Albrecht
Chapel Team
Reply to: