[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: sfcgal package for postgis and others



Sebastiaan Couwenberg <sebastic@xs4all.nl> wrote:

> The problematic quertbts is not used if you use reportbug with the
> options I gave earlier:
> 
>  reportbug -I -b wnpp

Ah OK.

> Please file the ITP for sfcgal and add the "(closes: #XXXXXX)" to the
> package changelog to close the ITP when the package enters the archive.

This will be easy then...

> Thanks for the push, I've reviewed the package and it needs some
> improvements before it can be uploaded to the archive.

I already expected this but looks lile it's even more bureaucracy
than I expected :(  I will try to fix that stuff and push a corrected
version, thanks for your patience.

Working on the package while writing the answer to this mail. Looks
like this seems to get a debian packaging journey for beginners who
thought that they already knew how to do this ;)

> - debian/changelog:
> 
> The changelog entry still needs to close the ITP bug as mentioned before.
> 
> You should also consider changing the urgency to medium so the package
> will migrate to testing after 5 days instead of 10.

Do I also need to change the distribution field from UNRELEASED to
unstable?

> The control file should be reformatted using the cme tool to have
> consistent formatting of the control file. See:
> 
> http://pkg-grass.alioth.debian.org/policy/policy.html#cme

OK, cme seems to be fine for now.

> The duplicate descriptions need to be fixed too, lintian complains about
> these.

lintian didn't complain about anything, I will retry using --pedantic

> You should also add a Files section for debian/* for your packaging
> license & copyright.

I would consider them a contribution to the original sources and thus
of course also GPL-2+.

> Also consider using stand-alone license specifications, because you'll
> likely use the same license for your Debian packaging. See:
> 
> https://www.debian.org/doc/packaging-manuals/copyright-format/1.0/#stand-alone-license-paragraph

Project Homepage states the following:

"SFCGAL is distributed under the terms of the GNU Lesser General Public License 2+."

So I went for "Example 3. Simple"
https://www.debian.org/doc/packaging-manuals/copyright-format/1.0/#examples 

cme check dpkg-copyright is fine with this now.

> The licensecheck tool reports several files licensed under the LGPL-2+ &
> LGPL-3+, you need to document these files in the copyright separately.

Those files in "patches" subdir are not part of sfcgal at all but
patches for older Versions of CGAL (4.2/4.3) and not used in the
package build anyway as we are building for CGAL 4.5 (debian 8.x)
and above.

> - debian/docs
> 
> Because the package is not specified for the docs, it will be applied to
> the first package in debian/control only, you should include the README
> in all binary packages.

OK, put them into all of them

> Instead of including the NEWS file in the documention, you should
> override dh_installchangelogs in debian/rules to install the NEWS file
> as the upstream changelog.

NACK. It does not look like the NEWS file is up-to date at all. I
would suggest to better get rid of it altogether.

> - debian/get-orig-source
> 
> This script should be removed in favor of plain uscan with the
> debian/watch file.
> 
> A get-orig-source script is only required if uscan cannot be used to
> download the upstream tarball.

Ah, just got this from your debian-gis skeleton. I updated
debian/watch according to your suggestions.

I had no Idea of the existance of this file up to now.

Is there some kind of automatism which will run uscan from time to
time or do I have to set this up for myself?

> - debian/*.dirs
> 
> These are not strictly required, the directories will be created for the
> files to install.

Well dh_make build them, so I added them. removed them now.

> - debian/libsfcgal-dev.install
> 
> You don't need to specify the prefixed / in the path for the man page.

*urgs* this was not by intention. However, it seemed to work anyway :)

> - debian/rules
> 
> Don't install the man page via the rules file, use
> debian/libsfcgal-dev.manpages to have dh_installman do the work for you.

Still not used to all that modern stuff. I build most of my debian
packages back in the times whe configure; make; make install have
been called directly from debian/rules.

> Why are the tests skipped?

Because the least important one did not work :( 

I have an Idea, but I did not figure out why. 

Looking at http://oslandia.github.io/SFCGAL/dev.html we are
talking about the "style test" This is actually calling the "astyle"
source code beautifier and for some reason the version from debian
stable does not seem to play well with their script:

--cut--
$ ./script/test-style-SFCGAL.sh

...

diff in ./viewer/src/SFCGAL/viewer/plugins/DataPlugin.cpp
diff in ./viewer/src/SFCGAL/viewer/plugins/GridPlugin.cpp
******* 298 errors found
--cut--

The other (more important) tests passed, but I did not find a good
way to disable only one of them.

Frankly, I have been bitten by broken tests (e.g. ones not running on
NFS home directories) way too often in recent times while building
backports.  That's why I know how to disable them in the first place :)

> Consider overriding dh_install to use --list-missing, this will report
> file in the build directory that are not being installed. This helps to
> ensure that new files are not silently discarded.

ACK

> You should also use "dh $@ --parallel" to enable parallel builds with
> DEB_BUILD_OPTIONS="parallel=N". This should speed up the build.

Nice, I would have needed this hint at the start of the journey.

> You also don't need to override dh_auto_configure if you set the flags
> in the CMAKE_OPTS environment variable.

This did not seem to work. Searching trough other debian/rules files on
http://codesearch.debian.net/ it looks like dh_auto_configure override
is still needed when using CMAKE_OPTS.

> I'm not sure how you've built the package, but it's highly recommended
> to always use pbuilder/cowbuilder to build the package in a clean
> chroot. See:
> 
> http://pkg-grass.alioth.debian.org/policy/packaging.html#git-pbuilder
> http://pkg-grass.alioth.debian.org/policy/packaging.html#git-build-package

I started with plain dpkg-buildpackage as I did not know anything
else, but checked with "gbp buildpackage --git-pbuilder" before uploading.

> Make sure to also enable the lintian hook as documented to have lintian
> report all issues with the package after the build. Currently lintian
> reports some typos for which you should add patches, also forward these
> patches upstream to not be required to carry the patches indefinitely.

Ah, have been running lintian manually which is probably the less
pedantic version.

Did not manage to run Lintioan from "gbp buildpackage" automatically.

running "lintian -I --show-overrides --pedantic" manually I get:

I: sfcgal source: duplicate-long-description libsfcgal1 libsfcgal-dev
P: sfcgal source: debian-watch-may-check-gpg-signature
I: sfcgal-bin: spelling-error-in-binary usr/bin/viewer-SFCGAL informations information
P: sfcgal-bin: no-upstream-changelog
I: sfcgal-bin: extended-description-is-probably-too-short
P: libsfcgal-dev: no-upstream-changelog
I: libsfcgal1: spelling-error-in-binary usr/lib/x86_64-linux-gnu/libSFCGAL.so.1.1.0 inconsistant inconsistent
I: libsfcgal1: hardening-no-fortify-functions usr/lib/x86_64-linux-gnu/libSFCGAL.so.1.1.0
P: libsfcgal1: no-upstream-changelog
I: libsfcgal1: no-symbols-control-file usr/lib/x86_64-linux-gnu/libSFCGAL.so.1.1.0

Way to late now to fix the rest, but I consider them mostly minor or
even ridiculous. Do you really want me to fix spelling errors in
function calls?

Pushed for now.

Regards

Sven

P.S.: Oh, and I also reverted the .gitignore changes. Reason I added them in
the first place was that I usually work in a way that git status does
not show something in red color.

-- 
"Das ist halt der Unterschied: Unix ist ein Betriebssystem mit Tradition,
 die anderen sind einfach von sich aus unlogisch."
                          (Anselm Lingnau in de.comp.os.unix.discussion)
/me ist giggls@ircnet, http://sven.gegg.us/ im WWW


Reply to: