Re: [Debian-med-packaging] C++ help needed for centrifuge

2017-11-26 Thread Alexis Murzeau
Le 27/11/2017 à 00:39, Alexis Murzeau a écrit :
> Hi,
> 
> Le 26/11/2017 à 22:01, Fabian Klötzl a écrit :
>> Ho,
>>
>> On 26.11.2017 19:32, Walter Landry wrote:
>>> Andreas Tille  wrote:
 Unfortunately I've hit another issue:

 ...
 classifier.h:428:54: error: the value of 'rank' is not usable in a 
 constant expression
  while((uint8_t)_hitMap[i].rank < rank) {
   ^~~~
 classifier.h:424:21: note: 'uint8_t rank' is not const
  uint8_t rank = 0;
  ^~~~
>>>
>>> That is mysterious to me.  Is that the first error?
>>>
>>
>> That reminds me of an error where the `<` was mistaken for the opening
>> angle brackets of a template. Flipping the order of the comparison or
>> adding some extra parentheses might help.
> 
> Indeed, it seems gcc understand rank as std::rank, and maybe is trying
> to read _hitMap[i].std::rank ("using namespace std" is used,
> "rank" can also reference std::rank).
> 
> Reversing the order works: "while(rank > _hitMap[i].rank)".
> Removing "using namespace std" is probably a better long-term solution
> for upstream.
> 
> As a side note, I found the package to not build on 32 bits linux
> because the ASM instruction "popcntq" doesn't exists. To avoid that
> error, I used "export POPCNT_CAPABILITY=0" in debian/rules.
> But I'm not sure this program is designed to run with less than 2GB of
> memory anyway :) This flag is needed anyway if you want your package to
> build for non-amd64 architecture.
> 
> I got a deb file after the following modifications:
>  - Reversing the while condition (as said above)
>  - Patching Makefile to take into account the DESTDIR variable, used by
> Debian packaging scripts to set the target directory where to install
> and uninstall files (that is, $(DESTDIR)$(prefix) instead of just $(prefix))
>  - Add a dh_auto_install override to set "prefix" to /usr instead of
> default /usr/local
> 
> And the generated .deb got several lintian warnings/errors:
> W: centrifuge: wrong-bug-number-in-closes l3:#xxx
> W: centrifuge: new-package-should-close-itp-bug
> W: centrifuge: script-with-language-extension
> usr/bin/centrifuge-BuildSharedSequence.pl
> W: centrifuge: script-with-language-extension
> usr/bin/centrifuge-RemoveEmptySequence.pl
> W: centrifuge: script-with-language-extension usr/bin/centrifuge-RemoveN.pl
> W: centrifuge: script-with-language-extension usr/bin/centrifuge-compress.pl
> W: centrifuge: script-with-language-extension usr/bin/centrifuge-sort-nt.pl
> W: centrifuge: binary-without-manpage usr/bin/centrifuge
> W: centrifuge: binary-without-manpage
> usr/bin/centrifuge-BuildSharedSequence.pl
> W: centrifuge: binary-without-manpage
> usr/bin/centrifuge-RemoveEmptySequence.pl
> W: centrifuge: binary-without-manpage usr/bin/centrifuge-RemoveN.pl
> W: centrifuge: binary-without-manpage usr/bin/centrifuge-build
> W: centrifuge: binary-without-manpage usr/bin/centrifuge-build-bin
> W: centrifuge: binary-without-manpage usr/bin/centrifuge-class
> W: centrifuge: binary-without-manpage usr/bin/centrifuge-compress.pl
> W: centrifuge: binary-without-manpage usr/bin/centrifuge-download
> W: centrifuge: binary-without-manpage usr/bin/centrifuge-inspect
> W: centrifuge: binary-without-manpage usr/bin/centrifuge-inspect-bin
> W: centrifuge: binary-without-manpage usr/bin/centrifuge-sort-nt.pl
> E: centrifuge: python-script-but-no-python-dep usr/bin/centrifuge-build
> E: centrifuge: python-script-but-no-python-dep usr/bin/centrifuge-inspect
> W: centrifuge: script-not-executable
> usr/share/centrifuge/doc/strip_markdown.pl
> 
> So there is still some work to do after that ^^
> 
>>
>> I will do some more investigation, tomorrow.
>>
>> Fabian
>>
> 

I attached a dirty patch so you can see exactly what I have modified to
fix errors.

-- 
Alexis Murzeau
PGP: B7E6 0EBB 9293 7B06 BDBC  2787 E7BD 1904 F480 937F
diff --git a/debian/patches/0002-Fix-build-with-rank.patch 
b/debian/patches/0002-Fix-build-with-rank.patch
new file mode 100644
index 000..6e29a9c
--- /dev/null
+++ b/debian/patches/0002-Fix-build-with-rank.patch
@@ -0,0 +1,21 @@
+From: Alexis Murzeau 
+Date: Mon, 27 Nov 2017 00:04:21 +0100
+Subject: Fix build with rank
+
+---
+ classifier.h | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/classifier.h b/classifier.h
+index f61a7f0..13b110b 100644
+--- a/classifier.h
 b/classifier.h
+@@ -425,7 +425,7 @@ public:
+ while(_hitMap.size() > (size_t)rp.khits) {
+ _hitTaxCount.clear();
+ for(size_t i = 0; i < _hitMap.size(); i++) {
+-while(_hitMap[i].rank < rank) {
++while(rank > _hitMap[i].rank) {
+ if(_hitMap[i].rank + 1 >= _hitMap[i].path.size()) {
+ _hitMap[i].rank = 
std::numeric_limits::max();
+ break;

Re: [Debian-med-packaging] C++ help needed for centrifuge

2017-11-26 Thread Alexis Murzeau
Hi,

Le 26/11/2017 à 22:01, Fabian Klötzl a écrit :
> Ho,
> 
> On 26.11.2017 19:32, Walter Landry wrote:
>> Andreas Tille  wrote:
>>> Unfortunately I've hit another issue:
>>>
>>> ...
>>> classifier.h:428:54: error: the value of 'rank' is not usable in a constant 
>>> expression
>>>  while((uint8_t)_hitMap[i].rank < rank) {
>>>   ^~~~
>>> classifier.h:424:21: note: 'uint8_t rank' is not const
>>>  uint8_t rank = 0;
>>>  ^~~~
>>
>> That is mysterious to me.  Is that the first error?
>>
> 
> That reminds me of an error where the `<` was mistaken for the opening
> angle brackets of a template. Flipping the order of the comparison or
> adding some extra parentheses might help.

Indeed, it seems gcc understand rank as std::rank, and maybe is trying
to read _hitMap[i].std::rank ("using namespace std" is used,
"rank" can also reference std::rank).

Reversing the order works: "while(rank > _hitMap[i].rank)".
Removing "using namespace std" is probably a better long-term solution
for upstream.

As a side note, I found the package to not build on 32 bits linux
because the ASM instruction "popcntq" doesn't exists. To avoid that
error, I used "export POPCNT_CAPABILITY=0" in debian/rules.
But I'm not sure this program is designed to run with less than 2GB of
memory anyway :) This flag is needed anyway if you want your package to
build for non-amd64 architecture.

I got a deb file after the following modifications:
 - Reversing the while condition (as said above)
 - Patching Makefile to take into account the DESTDIR variable, used by
Debian packaging scripts to set the target directory where to install
and uninstall files (that is, $(DESTDIR)$(prefix) instead of just $(prefix))
 - Add a dh_auto_install override to set "prefix" to /usr instead of
default /usr/local

And the generated .deb got several lintian warnings/errors:
W: centrifuge: wrong-bug-number-in-closes l3:#xxx
W: centrifuge: new-package-should-close-itp-bug
W: centrifuge: script-with-language-extension
usr/bin/centrifuge-BuildSharedSequence.pl
W: centrifuge: script-with-language-extension
usr/bin/centrifuge-RemoveEmptySequence.pl
W: centrifuge: script-with-language-extension usr/bin/centrifuge-RemoveN.pl
W: centrifuge: script-with-language-extension usr/bin/centrifuge-compress.pl
W: centrifuge: script-with-language-extension usr/bin/centrifuge-sort-nt.pl
W: centrifuge: binary-without-manpage usr/bin/centrifuge
W: centrifuge: binary-without-manpage
usr/bin/centrifuge-BuildSharedSequence.pl
W: centrifuge: binary-without-manpage
usr/bin/centrifuge-RemoveEmptySequence.pl
W: centrifuge: binary-without-manpage usr/bin/centrifuge-RemoveN.pl
W: centrifuge: binary-without-manpage usr/bin/centrifuge-build
W: centrifuge: binary-without-manpage usr/bin/centrifuge-build-bin
W: centrifuge: binary-without-manpage usr/bin/centrifuge-class
W: centrifuge: binary-without-manpage usr/bin/centrifuge-compress.pl
W: centrifuge: binary-without-manpage usr/bin/centrifuge-download
W: centrifuge: binary-without-manpage usr/bin/centrifuge-inspect
W: centrifuge: binary-without-manpage usr/bin/centrifuge-inspect-bin
W: centrifuge: binary-without-manpage usr/bin/centrifuge-sort-nt.pl
E: centrifuge: python-script-but-no-python-dep usr/bin/centrifuge-build
E: centrifuge: python-script-but-no-python-dep usr/bin/centrifuge-inspect
W: centrifuge: script-not-executable
usr/share/centrifuge/doc/strip_markdown.pl

So there is still some work to do after that ^^

> 
> I will do some more investigation, tomorrow.
> 
> Fabian
>

-- 
Alexis Murzeau
PGP: B7E6 0EBB 9293 7B06 BDBC  2787 E7BD 1904 F480 937F



signature.asc
Description: OpenPGP digital signature


Re: [Debian-med-packaging] C++ help needed for centrifuge

2017-11-26 Thread Fabian Klötzl
Ho,

On 26.11.2017 19:32, Walter Landry wrote:
> Andreas Tille  wrote:
>> Unfortunately I've hit another issue:
>>
>> ...
>> classifier.h:428:54: error: the value of 'rank' is not usable in a constant 
>> expression
>>  while((uint8_t)_hitMap[i].rank < rank) {
>>   ^~~~
>> classifier.h:424:21: note: 'uint8_t rank' is not const
>>  uint8_t rank = 0;
>>  ^~~~
> 
> That is mysterious to me.  Is that the first error?
> 

That reminds me of an error where the `<` was mistaken for the opening
angle brackets of a template. Flipping the order of the comparison or
adding some extra parentheses might help.

I will do some more investigation, tomorrow.

Fabian



Re: C++ help needed for centrifuge

2017-11-26 Thread Walter Landry
Andreas Tille  wrote:
> Hi,
> 
> On Sat, Nov 25, 2017 at 01:39:03PM -0800, Walter Landry wrote:
>> > In file included from centrifuge_build.cpp:27:0:
>> > bt2_idx.h: In static member function 'static std::pair> > Ebwt*> Ebwt::fromStrings(const 
>> > EList&, bool, int, int, bool, 
>> > int32_t, int32_t, int32_t, const string&, bool, index_t, index_t, index_t, 
>> > int, uint32_t, bool, bool, bool)':
>> > bt2_idx.h:1053:3: warning: 'template class std::auto_ptr' is 
>> > deprecated [-Wdeprecated-declarations]
>> 
>> This is only a warning, so you can ignore it.  If you are feeling
>> ambitious, the recommended fix is to replace all auto_ptr's with
>> unique_ptr's and copies with moves().
> 
> I've applied this in
> 
>
> https://anonscm.debian.org/cgit/debian-med/centrifuge.git/tree/debian/patches/fix_auto_ptr_usage_in_gcc-7.patch

I think that is OK.  If I were in charge of this code, I would convert
it from pointers to value semantics, but that would be a much larger
change.

>> Apparently, clang-modernize can
>> do this automatically.
> 
> In what package can I find clang-modernize (apt-file search did not find
> anything - but I'm currently not on my development machine).

Sorry.  It has been renamed to clang-tidy.

> Unfortunately I've hit another issue:
> 
> ...
> classifier.h:428:54: error: the value of 'rank' is not usable in a constant 
> expression
>  while((uint8_t)_hitMap[i].rank < rank) {
>   ^~~~
> classifier.h:424:21: note: 'uint8_t rank' is not const
>  uint8_t rank = 0;
>  ^~~~

That is mysterious to me.  Is that the first error?

Walter Landry



Re: C++ help needed for centrifuge

2017-11-26 Thread Andreas Tille
Hi,

On Sat, Nov 25, 2017 at 01:39:03PM -0800, Walter Landry wrote:
> > In file included from centrifuge_build.cpp:27:0:
> > bt2_idx.h: In static member function 'static std::pair > Ebwt*> Ebwt::fromStrings(const 
> > EList&, bool, int, int, bool, 
> > int32_t, int32_t, int32_t, const string&, bool, index_t, index_t, index_t, 
> > int, uint32_t, bool, bool, bool)':
> > bt2_idx.h:1053:3: warning: 'template class std::auto_ptr' is 
> > deprecated [-Wdeprecated-declarations]
> 
> This is only a warning, so you can ignore it.  If you are feeling
> ambitious, the recommended fix is to replace all auto_ptr's with
> unique_ptr's and copies with moves().

I've applied this in

   
https://anonscm.debian.org/cgit/debian-med/centrifuge.git/tree/debian/patches/fix_auto_ptr_usage_in_gcc-7.patch

> Apparently, clang-modernize can
> do this automatically.

In what package can I find clang-modernize (apt-file search did not find
anything - but I'm currently not on my development machine).

Unfortunately I've hit another issue:

...
classifier.h:428:54: error: the value of 'rank' is not usable in a constant 
expression
 while((uint8_t)_hitMap[i].rank < rank) {
  ^~~~
classifier.h:424:21: note: 'uint8_t rank' is not const
 uint8_t rank = 0;
 ^~~~
...

I tried my luck with some type castings but failed. :-(

Kind regards

  Andreas.

-- 
http://fam-tille.de



Re: C++ help needed for centrifuge

2017-11-25 Thread Walter Landry
Andreas Tille  wrote:
> Hi,
> 
> I started packaging centrifuge[1] and hit a build error which
> is most probably caused by gcc-7 incompatibility:
> 
> ...
> In file included from centrifuge_build.cpp:27:0:
> bt2_idx.h: In static member function 'static std::pair Ebwt*> Ebwt::fromStrings(const 
> EList&, bool, int, int, bool, int32_t, 
> int32_t, int32_t, const string&, bool, index_t, index_t, index_t, int, 
> uint32_t, bool, bool, bool)':
> bt2_idx.h:1053:3: warning: 'template class std::auto_ptr' is 
> deprecated [-Wdeprecated-declarations]

This is only a warning, so you can ignore it.  If you are feeling
ambitious, the recommended fix is to replace all auto_ptr's with
unique_ptr's and copies with moves().  Apparently, clang-modernize can
do this automatically.

Walter Landry



Re: C++ help needed for centrifuge

2017-11-25 Thread Alexis Murzeau
Le 25/11/2017 à 21:56, Andreas Tille a écrit :
> Hi,
> 
> I started packaging centrifuge[1] and hit a build error which
> is most probably caused by gcc-7 incompatibility:
> 
> ...
> In file included from centrifuge_build.cpp:27:0:
> bt2_idx.h: In static member function 'static std::pair Ebwt*> Ebwt::fromStrings(const 
> EList&, bool, int, int, bool, int32_t, 
> int32_t, int32_t, const string&, bool, index_t, index_t, index_t, int, 
> uint32_t, bool, bool, bool)':
> bt2_idx.h:1053:3: warning: 'template class std::auto_ptr' is 
> deprecated [-Wdeprecated-declarations]
>auto_ptr ss(new stringstream());
>^~~~
> In file included from /usr/include/c++/7/memory:80:0,
>  from bt2_idx.h:28,
>  from centrifuge_build.cpp:27:
> /usr/include/c++/7/bits/unique_ptr.h:51:28: note: declared here
>template class auto_ptr;
> ^~~~
> In file included from centrifuge_build.cpp:27:0:
> bt2_idx.h:1057:3: warning: 'template class std::auto_ptr' is 
> deprecated [-Wdeprecated-declarations]
>auto_ptr fb(new FileBuf(ss.get()));
>^~~~
> In file included from /usr/include/c++/7/memory:80:0,
>  from bt2_idx.h:28,
>  from centrifuge_build.cpp:27:
> /usr/include/c++/7/bits/unique_ptr.h:51:28: note: declared here
>template class auto_ptr;
> ...
> 

Hi,

You can replace auto_ptr with unique_ptr which was introduced with
C++11. std::unique_ptr is declared in the same header as auto_ptr
().

One major difference is that std::unique_ptr cannot be copied but moved
instead.
For example:
std::unique_ptr fb(new FileBuf);
std::unique_ptr fb2 = std::move(p);

After that second line, p will be empty/null and p2 will contains the
FileBuf pointer.

But in bt2_idx.h, the auto_ptr variables are not copied around so you
probably just need to replace "auto_ptr" with "unique_ptr" and that's all.


-- 
Alexis Murzeau
PGP: B7E6 0EBB 9293 7B06 BDBC  2787 E7BD 1904 F480 937F



signature.asc
Description: OpenPGP digital signature


C++ help needed for centrifuge

2017-11-25 Thread Andreas Tille
Hi,

I started packaging centrifuge[1] and hit a build error which
is most probably caused by gcc-7 incompatibility:

...
In file included from centrifuge_build.cpp:27:0:
bt2_idx.h: In static member function 'static std::pair Ebwt::fromStrings(const 
EList&, bool, int, int, bool, int32_t, 
int32_t, int32_t, const string&, bool, index_t, index_t, index_t, int, 
uint32_t, bool, bool, bool)':
bt2_idx.h:1053:3: warning: 'template class std::auto_ptr' is deprecated 
[-Wdeprecated-declarations]
   auto_ptr ss(new stringstream());
   ^~~~
In file included from /usr/include/c++/7/memory:80:0,
 from bt2_idx.h:28,
 from centrifuge_build.cpp:27:
/usr/include/c++/7/bits/unique_ptr.h:51:28: note: declared here
   template class auto_ptr;
^~~~
In file included from centrifuge_build.cpp:27:0:
bt2_idx.h:1057:3: warning: 'template class std::auto_ptr' is deprecated 
[-Wdeprecated-declarations]
   auto_ptr fb(new FileBuf(ss.get()));
   ^~~~
In file included from /usr/include/c++/7/memory:80:0,
 from bt2_idx.h:28,
 from centrifuge_build.cpp:27:
/usr/include/c++/7/bits/unique_ptr.h:51:28: note: declared here
   template class auto_ptr;
...


Unfortunately I have no idea about C++.  Any idea how to fix this?

Kind regards

  Andreas.


[1] https://anonscm.debian.org/git/debian-med/centrifuge.git

-- 
http://fam-tille.de