Olivier Sallou <olivier.sal...@irisa.fr> writes:
> for info I could make a few changes ot blast+ packaging:
Please don't let the length of this message alarm you; as I mentioned,
you're off to a great start, but there are plenty of details to packaging
anything well, and BLAST is no exception. Moreover, my suggestions mostly
cover relatively minor issues that I probably wouldn't bother reporting if
not specifically asked to review the package.
At any rate, I'm all for enabling shared libraries and putting them in a
private directory, but adding that directory to the global search path
defeats the purpose of doing so. Instead, I'd suggest using the
--with-runpath=... option (easy to overlook among all the others) to build
in the right path, at which point you'll be good to go, with no need for
chrpath or ldconfig.
I propose some other changes to configure flags as well:
* Add --with-flat-makefile to make it easy to build just the libraries the
BLAST applications and tests need. (For this to be effective, it is also
necessary at the moment to run configure with LD_LIBRARY_PATH set.)
* Add --without-lzo to avoid picking up extra dependencies when its -dev
package happens to be installed.
* Add --without-autodep and --without-makefile-auto-update to
streamline the build and (in the latter case) simplify cleanup.
* Set an explicit build root ("BUILD") to which other rules can then refer.
* Substitute --with-optimization for --without-debug when DEB_BUILD_OPTIONS
contains nostrip. It would be cleaner to do so unconditionally, as that
change also switches from -DNDEBUG to -D_DEBUG, but if you don't have the
resources to link debug binaries, so be it.
* Drop --without-gbench, --without-internal, and --with-3psw=std:netopt
(superfluous) and --bindir=... and --libdir=... (in favor of manual
installation).
Passing --with-flat-makefile as proposed above yields a Makefile.flat that
override_dh_auto_build can proceed to invoke from the build directory with
a suitable all_projects setting (algo/blast/ app/ objmgr/
objtools/align_format/ objtools/blast/). You can also set
DLL_LDFLAGS=-Wl,--as-needed at that point to address some of
dpkg-shlibdeps's warnings about useless linking. (Alas, we can't do the
same for applications without changing library makefiles.)
I'd also suggest reporting test suite errors in more detail but otherwise
ignoring them, as some tests will likely fail on autobuilders that use
jails lacking network connectivity. Speaking of the test suite, adding a
build dependency on time should avoid the need for your existing fix_checks
patch, but other changes (freshly integrated upstream) are in order to
handle unpacking under .../src/... directories.
As I mentioned earlier, I'd install the binaries and libraries manually;
doing so is straightforward because upstream's build system collects them
all in one place and actually beats running upstream's installation rule,
which lacks DESTDIR support and needs a build-dependency on cpio so that it
can install headers that aren't of interest at the moment anyway. Also,
I'd leave a few more binaries out of /usr/bin and lose the /usr/bin
scripts' extensions per Policy 10.4. (Doing so calls for a corresponding
change to legacy.sh, though it can keep its name because it's an internal
script in a non-standard directory. Incidentally, there's no need for a
separate ncbi-blast+-legacy package if the commands it provides are in a
special directory anyway.)
Also, ncbiconf_unix.h is just the tip of the iceberg in terms of generated
files; override_dh_clean should take care of the others as well, and
build-depending on autotools-dev has no effect if you don't also invoke dh
--with autotools_dev.
I've attached a patch implementing all of these changes and some other
cleanups (notably to the build and runtime dependencies); would you like to
commit them, or should I? (In the latter case, should I factor it into
smaller commits?)
I've left some issues unaddressed, though, as they generally require less
experimentation to handle:
* As I alluded to, the linkage is a mess, with many libraries' DLL_[D]LIB
settings missing or incomplete; applications still build successfully
with default linker settings because they specify indirect dependencies
for the sake of static builds, but cannot safely use --as-needed. That's
not actively a problem, but it does generate a lot of warnings.
* There are no individual manpages, just one for the suite as a whole. In
the long term, adding them (perhaps based on -help or -xmlhelp output)
would be awesome; for the time being, symlinks to ncbi-blast+.1 would
still help.
* A couple of patches could stand to be renamed: lecagy_rename_rpsblast has
a typo, and fix_gcc46_include_error wound up with a second compatibility
fix as well.
* For that matter, you could perhaps rename the tree to match the package
name (which I'm glad to see you changed per my suggestion), but that's
really superficial.
As I said, though, you really are off to a great start, and I appreciate
all your work on the package; there are just a lot of details to packaging
anything.
--
Aaron M. Ucko, KB1CJC (amu at alum.mit.edu, ucko at debian.org)
http://www.mit.edu/~amu/ | http://stuff.mit.edu/cgi/finger/?a...@monk.mit.edu
Index: debian/control
===================================================================
--- debian/control (revision 6862)
+++ debian/control (working copy)
@@ -1,7 +1,7 @@
Source: ncbi-blast+
Section: science
Priority: optional
-Build-Depends: debhelper (>= 7.0.50~),autotools-dev,zlib1g,bzip2,libbz2-dev,libpcre++-dev,libpcre3,zlib1g-dev,chrpath,libboost-test-dev,cpio,quilt (>= 0.46-7~)
+Build-Depends: debhelper (>= 7.0.50~), autotools-dev, libbz2-dev, libpcre3-dev, zlib1g-dev, libboost-test-dev, quilt (>= 0.46-7~), time
Standards-Version: 3.9.1
Uploaders: Olivier Sallou <olivier.sal...@irisa.fr>
DM-Upload-Allowed: yes
@@ -13,8 +13,8 @@
Package: ncbi-blast+
Architecture: any
-Depends: ${shlibs:Depends}, ${misc:Depends},${perl:Depends},ncbi-data,bzip2,libpcre3,libpcre++0,zlib1g,python
-Description: The next generation suite of BLAST sequence search tools
+Depends: ${shlibs:Depends}, ${misc:Depends}, ${perl:Depends}, ${python:Depends}, ncbi-data
+Description: next generation suite of BLAST sequence search tools
The Basic Local Alignment Search Tool (BLAST) is the most widely
used sequence similarity tool. There are versions of BLAST that
compare protein queries to protein databases, nucleotide queries
@@ -30,7 +30,7 @@
Package: ncbi-blast+-legacy
Architecture: all
-Depends: ncbi-blast+,${misc:Depends}
+Depends: ncbi-blast+, ${misc:Depends}
Description: NCBI Blast legacy call script
This package adds some fake scripts to call NCBI+ programs
with the NCBI blast command line. It makes use of the
Index: debian/postinst
===================================================================
--- debian/postinst (revision 6862)
+++ debian/postinst (working copy)
@@ -1,48 +0,0 @@
-#!/bin/sh
-# postinst script for blast+
-#
-# see: dh_installdeb(1)
-
-set -e
-
-# summary of how this script can be called:
-# * <postinst> `configure' <most-recently-configured-version>
-# * <old-postinst> `abort-upgrade' <new version>
-# * <conflictor's-postinst> `abort-remove' `in-favour' <package>
-# <new-version>
-# * <postinst> `abort-remove'
-# * <deconfigured's-postinst> `abort-deconfigure' `in-favour'
-# <failed-install-package> <version> `removing'
-# <conflicting-package> <version>
-# for details, see http://www.debian.org/doc/debian-policy/ or
-# the debian-policy package
-
-
-case "$1" in
- configure)
- echo "/usr/lib/ncbi-blast+" > /etc/ld.so.conf.d/ncbi-blast+.conf
- ldconfig
- #grep "BLASTDB" /etc/ncbi/.ncbirc >/dev/null
- #if [ $? -eq 0 ];then
- # echo "/etc/ncbi/.ncbirc already configured"
- #else
- # echo "[BLAST]\n" >> /etc/ncbi/.ncbirc
- # echo "BLASTDB=/var/lib/ncbi-blast+/db\n" >> /etc/ncbi/.ncbirc
- #fi
- ;;
-
- abort-upgrade|abort-remove|abort-deconfigure)
- ;;
-
- *)
- echo "postinst called with unknown argument \`$1'" >&2
- exit 1
- ;;
-esac
-
-# dh_installdeb will replace this with shell code automatically
-# generated by other debhelper scripts.
-
-#DEBHELPER#
-
-exit 0
Index: debian/postrm
===================================================================
--- debian/postrm (revision 6862)
+++ debian/postrm (working copy)
@@ -1,41 +0,0 @@
-#!/bin/sh
-# postrm script for blast-plus
-#
-# see: dh_installdeb(1)
-
-set -e
-
-# summary of how this script can be called:
-# * <postrm> `remove'
-# * <postrm> `purge'
-# * <old-postrm> `upgrade' <new-version>
-# * <new-postrm> `failed-upgrade' <old-version>
-# * <new-postrm> `abort-install'
-# * <new-postrm> `abort-install' <old-version>
-# * <new-postrm> `abort-upgrade' <old-version>
-# * <disappearer's-postrm> `disappear' <overwriter>
-# <overwriter-version>
-# for details, see http://www.debian.org/doc/debian-policy/ or
-# the debian-policy package
-
-
-case "$1" in
- remove|purge)
- rm /etc/ld.so.conf.d/ncbi-blast+.conf
- ldconfig
- ;;
- upgrade|failed-upgrade|abort-install|abort-upgrade|disappear)
- ;;
-
- *)
- echo "postrm called with unknown argument \`$1'" >&2
- exit 1
- ;;
-esac
-
-# dh_installdeb will replace this with shell code automatically
-# generated by other debhelper scripts.
-
-#DEBHELPER#
-
-exit 0
Index: debian/patches/fix_checks
===================================================================
--- debian/patches/fix_checks (revision 6862)
+++ debian/patches/fix_checks (working copy)
@@ -1,35 +1,45 @@
-Subject: command time not found when using shell
+Subject: checks misreported as absent when unpacked under /*/src/*
- * scripts/common/check/check_make_unix.sh: use bash and remove exec usage
+ * scripts/common/check/check_add.sh: accept the relative source directory
+ * src/build-system/Makefile.meta.in: supply it to check_add.sh
-Author: Olivier Sallou <olivier.sal...@irisa.fr>
-Last-Update: 2011-05-03
---- a/c++/scripts/common/check/check_make_unix.sh
-+++ b/c++/scripts/common/check/check_make_unix.sh
-@@ -503,8 +503,8 @@
- # Also, process guard works better if used after "time -p".
- launch_sh="/var/tmp/launch.\$\$.sh"
- cat > \$launch_sh <<EOF_launch
--#! /bin/sh
--exec time -p \$check_exec \`eval echo \$xx_run\`
-+#! /bin/bash
-+time -p \$check_exec \`eval echo \$xx_run\`
- EOF_launch
- chmod a+x \$launch_sh
- \$launch_sh >\$x_log 2>&1
---- a/c++/src/serial/datatool/datatool.sh
-+++ b/c++/src/serial/datatool/datatool.sh
-@@ -1,4 +1,4 @@
--#! /bin/sh
-+#! /bin/bash
- # $Id: datatool.sh 79502 2006-03-23 19:45:38Z gouriano $
- #
+Author: Aaron M. Ucko <u...@debian.org>
+Last-Update: 2011-05-23
+--- a/c++/scripts/common/check/check_add.sh 2011-05-23 18:49:10.000000000 -0400
++++ b/c++/scripts/common/check/check_add.sh 2011-05-23 18:50:40.000000000 -0400
+@@ -28,14 +28,12 @@
+ x_srcdir=`(cd "$1"; pwd)`
+ x_test=$2
+ x_signature=$3
++x_srcdir_rel=$4
+ x_use_ignore_list=${CHECK_USE_IGNORE_LIST-Y}
+ x_delim=" ____ "
+ # Default timeout for check (in seconds)
+ x_timeout_default=200
---- a/c++/src/serial/datatool/datatool_xml.sh
-+++ b/c++/src/serial/datatool/datatool_xml.sh
-@@ -1,4 +1,4 @@
--#! /bin/sh
-+#! /bin/bash
- # $Id: datatool_xml.sh 141084 2008-09-23 19:53:09Z ivanovp $
- #
+-# Convert source dir to relative path
+-x_srcdir_rel=`echo "$x_srcdir" | perl -ne 's|^.*?/src/||; print'`
+-
+ # Check to necessity make test for this application
+ if test ! -f "$x_srcdir/Makefile.$x_test.app"; then
+ echo "Warning: File \"$x_srcdir/Makefile.$x_test.app\" not found."
+@@ -46,7 +44,7 @@
+ x_app=`grep '^ *APP[ =]' "$x_srcdir/Makefile.$x_test.app"`
+ x_app=`echo "$x_app" | sed -e 's/^.*=//' -e 's/^ *//'`
+-x_tpath=`echo "$x_srcdir/$x_test" | perl -ne 's|^.*?/src/||; print'`
++x_tpath=$x_srcdir_rel/$x_test
+ if grep -c '^ *CHECK_CMD' $x_srcdir/Makefile.$x_test.app > /dev/null ; then
+ # Check ignore list
+ x_use_ignore_list=`echo $x_use_ignore_list | tr '[a-z]' '[A-Z]' | sed -e 's/^\(.\).*/\1/g'`
+--- a/c++/src/build-system/Makefile.meta.in 2011-05-23 18:48:59.000000000 -0400
++++ b/c++/src/build-system/Makefile.meta.in 2011-05-23 18:49:20.000000000 -0400
+@@ -188,7 +188,7 @@
+ expendable=false ; \
+ for i in $$x_project ; do \
+ if test "x$$i" = "x-" ; then expendable=true ; continue ; fi ; \
+- $(check_add) $(abs_srcdir) $$i @signature@ @exe_ext@
++ $(check_add) $(abs_srcdir) $$i @signature@ $(subdir)
+ CHECK_ADD_KET = || exit 5 ; \
+ done ; \
+ fi
Index: debian/ncbi-blast+.dirs
===================================================================
--- debian/ncbi-blast+.dirs (revision 0)
+++ debian/ncbi-blast+.dirs (revision 0)
@@ -0,0 +1,2 @@
+usr/bin
+usr/lib/ncbi-blast+
Index: debian/rules
===================================================================
--- debian/rules (revision 6862)
+++ debian/rules (working copy)
@@ -9,35 +9,63 @@
# Uncomment this to turn on verbose mode.
#export DH_VERBOSE=1
-SOURCEDIR=${CURDIR}/c++
-export DEB_CONFIGURE_EXTRA_FLAGS= --with-dll --without-debug --with-mt --without-gbench --without-internal \
- --libdir=${CURDIR}/debian/ncbi-blast+/usr/lib/ncbi-blast+ --bindir=${CURDIR}/debian/ncbi-blast+/usr/bin \
- --includedir=$(SOURCEDIR)/include --with-3psw=std:netopt --without-dbapi
+DEB_CONFIGURE_EXTRA_FLAGS=--with-dll --with-mt --without-autodep \
+ --without-makefile-auto-update --with-flat-makefile --without-dbapi \
+ --without-lzo --with-runpath=/usr/lib/ncbi-blast+ --with-build-root=BUILD
+proj=algo/blast/ app/ objmgr/ objtools/align_format/ objtools/blast/
+# XXX - not quite right, as we get -DNDEBUG vs. -D_DEBUG
+ifeq (,$(findstring nostrip,$(DEB_BUILD_OPTIONS)))
+DEB_CONFIGURE_EXTRA_FLAGS += --without-debug
+else
+DEB_CONFIGURE_EXTRA_FLAGS += --with-optimization
+endif
+
override_dh_auto_configure:
- ( cd $(SOURCEDIR) ; ./configure ${DEB_CONFIGURE_EXTRA_FLAGS} --prefix=${SOURCEDIR}/debian/ncbi-blast+ )
+ cd c++ && LD_LIBRARY_PATH=$(CURDIR)/c++/BUILD/lib \
+ ./configure $(DEB_CONFIGURE_EXTRA_FLAGS)
+override_dh_auto_build:
+ cd c++/BUILD/build && make -f Makefile.flat \
+ all_projects="$(proj)" DLL_LDFLAGS=-Wl,--as-needed
+override_dh_auto_test:
+ -dh_auto_test
+ -c++/BUILD/build/check.sh concat_err
+ -cat c++/BUILD/build/check.sh.out_err
+
+instroot = debian/ncbi-blast+/usr
+override_dh_auto_install:
+ cp c++/BUILD/lib/*.so $(instroot)/lib/ncbi-blast+/
+ cp c++/BUILD/bin/* $(instroot)/bin/
+
override_dh_install:
- #TODO - can I just use -X.a -Xinclude instead of removing unneeded files later?
- dh_install
- cp debian/legacy/legacy.sh debian/ncbi-blast+-legacy/usr/share/ncbi-blast+/bin
- rm -f ${CURDIR}/debian/ncbi-blast+/usr/bin/*test*
- #mkdir -p ${CURDIR}/debian/ncbi-blast+-dev/usr/lib/ncbi-blast+
- rm -f ${CURDIR}/debian/ncbi-blast+/usr/lib/ncbi-blast+/*.a
- #mv ${CURDIR}/debian/ncbi-blast+/usr/lib/ncbi-blast+/*.a ${CURDIR}/debian/ncbi-blast+-dev/usr/lib/ncbi-blast+/
- #mv ${CURDIR}/debian/ncbi-blast+/usr/include ${CURDIR}/debian/ncbi-blast+-dev/usr/
- rm -rf ${CURDIR}/debian/ncbi-blast+/usr/include
- find ${CURDIR}/debian/ncbi-blast+/usr/bin/ -type f -not -name "*.p*" | xargs chrpath -d
- find ${CURDIR}/debian/ncbi-blast+/usr/lib/ncbi-blast+/*.so | xargs chrpath -d
- mv ${CURDIR}/debian/ncbi-blast+/usr/bin/rpsblast ${CURDIR}/debian/ncbi-blast+/usr/bin/rpsblast+
+ # dh_install
+ mv $(instroot)/bin/rpsblast $(instroot)/bin/rpsblast+
+ mv $(instroot)/bin/legacy_blast.pl $(instroot)/bin/legacy_blast
+ mv $(instroot)/bin/update_blastdb.pl $(instroot)/bin/update_blastdb
+ mv $(instroot)/bin/windowmasker_2.2.22_adapter.py \
+ $(instroot)/bin/windowmasker_2.2.22_adapter
+# Clean up tests, demos, and internal build tools
+ rm -f $(instroot)/bin/*test* $(instroot)/bin/seqdb_demo \
+ $(instroot)/bin/gene_info_reader $(instroot)/bin/datatool \
+ $(instroot)/bin/project_tree_builder \
+ $(instroot)/lib/ncbi-blast+/libtest_*.so
+ cp debian/legacy/legacy.sh \
+ debian/ncbi-blast+-legacy/usr/share/ncbi-blast+/bin/
+
override_dh_clean:
dh_clean
- find . -name ncbiconf_unix.h | xargs rm -f
+ -for x in c++/src/objects/*/*.files; do \
+ (cd `dirname $$x` && ../../../BUILD/build/new_module.sh \
+ `basename $$x .files`.module purge_sources); \
+ done
+ rm -rf c++/BUILD c++/compilers/dll c++/config.log c++/Makefile
+ rm -f c++/src/objects/blastxml/blastxml.module
%:
- dh $@ --with quilt --sourcedir=$(SOURCEDIR)
+ dh $@ -Dc++ --with autotools_dev --with quilt
get-orig-source:
. debian/get-orig-source
Index: debian/ncbi-blast+-legacy.dirs
===================================================================
--- debian/ncbi-blast+-legacy.dirs (revision 6862)
+++ debian/ncbi-blast+-legacy.dirs (working copy)
@@ -1 +1,2 @@
+/usr/bin
/usr/share/ncbi-blast+/bin
Index: debian/legacy/legacy.sh
===================================================================
--- debian/legacy/legacy.sh (revision 6862)
+++ debian/legacy/legacy.sh (working copy)
@@ -1,4 +1,4 @@
#!/bin/sh
# Execute legacy blast
-legacy_blast.pl ${0##*/} $@
+exec legacy_blast ${0##*/} $@
