Bug#835253: RFS: steghide/0.5.1-11 [QA]

[Fernando Seiti Furusato]

Hello, Gianfranco.

Thanks a lot for taking some time to verify this.

I tried to follow your recommendations:


On 09/12/2016 05:05 PM, Gianfranco Costamagna wrote:
> BTW, what about having the documentation in a separate -doc package?
> (and in the doc standard location)

I tried to to that. But then I had to add doxygen as a build-dep.

> 1) std-version is now 3.9.8

Corrected that.

>
> 2) 
> -rw-r--r-- root/root  6816 2016-08-22 23:03 
> ./usr/share/doc/steghide/COPYING.gz
>
>
> now this file is installed (and probably useless)
> quoting changelog:
>
> +  * Refixed so that INSTALL and COPYING information is not in the
> +deb archive.
>
>
>
> quoting old rules:
>
> +   $(MAKE) install prefix=`pwd`/debian/steghide/usr
> +   rm -rf `pwd`/debian/steghide/usr/doc
> +   rm -f `pwd`/debian/steghide/usr/share/doc/steghide/INSTALL
> +   rm -f `pwd`/debian/steghide/usr/share/doc/steghide/INSTALL.gz
> +   rm -f `pwd`/debian/steghide/usr/share/doc/steghide/COPYING
>
>
> so, I would try to remove it again

Well, I just used the same commands, just not the make install part.

>
> 3) this RFS has been picked up by Breno, so he should have the final words 
> about the
> package :)
> (I hope he don't mind my review, and hope he won't ask you to change 
> something back)

I talked to Breno last week, he said he might not have time to do this
and it would
be fine if you sponsored this one.

If you could take a look I would appreciate.

It can be found at the same place:

  dget -x
https://mentors.debian.net/debian/pool/main/s/steghide/steghide_0.5.1-11.dsc

Thanks!

-- 
Fernando Seiti Furusato
IBM Linux Technology Center




signature.asc
Description: OpenPGP digital signature

Bug#835253: RFS: steghide/0.5.1-11 [QA]

[Gianfranco Costamagna]

Hi,



>I was actually referring to the original version 0.5.1-10 from the
>Debian archive.
>When you have doxygen it builds the same way you have in the version I
>uploaded. I was just reproducing the same result.
>I was not clear about it, sorry.
>In any case:


ok this makes sense now :)

BTW, what about having the documentation in a separate -doc package?
(and in the doc standard location)


>dget -x 
>https://mentors.debian.net/debian/pool/main/s/steghide/steghide_0.5.1-11.dsc



seems really better, however I have some last complains
1) std-version is now 3.9.8

2) 
-rw-r--r-- root/root  6816 2016-08-22 23:03 
./usr/share/doc/steghide/COPYING.gz


now this file is installed (and probably useless)
quoting changelog:

+  * Refixed so that INSTALL and COPYING information is not in the
+deb archive.



quoting old rules:

+   $(MAKE) install prefix=`pwd`/debian/steghide/usr
+   rm -rf `pwd`/debian/steghide/usr/doc
+   rm -f `pwd`/debian/steghide/usr/share/doc/steghide/INSTALL
+   rm -f `pwd`/debian/steghide/usr/share/doc/steghide/INSTALL.gz
+   rm -f `pwd`/debian/steghide/usr/share/doc/steghide/COPYING


so, I would try to remove it again

3) this RFS has been picked up by Breno, so he should have the final words 
about the
package :)
(I hope he don't mind my review, and hope he won't ask you to change something 
back)

G.

Bug#835253: RFS: steghide/0.5.1-11 [QA]

[Fernando Seiti Furusato]

Hi.

On 09/12/2016 12:31 PM, Gianfranco Costamagna wrote:
> dpkg -c steghide_0.5.1-11_amd64.deb  |grep man |wc -l
> 308

I was actually referring to the original version 0.5.1-10 from the
Debian archive.
When you have doxygen it builds the same way you have in the version I
uploaded. I was just reproducing the same result.
I was not clear about it, sorry.
In any case:

> this is something that should be installed as a doxygen documentation, 
> somewhere else.

I removed the manpages file that I created from debian directory
and unset doxygen as a build-dep.

> probably the spec file is autogenerated, not sure

Yes, in fact, the spec and other files in the diff.gz.
I am removing them from the patches.

I have just uploaded it to mentors again.

dget -x 
https://mentors.debian.net/debian/pool/main/s/steghide/steghide_0.5.1-11.dsc

Thanks!

-- 
Fernando Seiti Furusato
IBM Linux Technology Center




signature.asc
Description: OpenPGP digital signature

Bug#835253: RFS: steghide/0.5.1-11 [QA]

[Gianfranco Costamagna]

Hi,




>This is actually because of lack of dependency on doxygen.
>When you do not have doxygen installed, the manpages are simply not
>created with
>no errors whatsoever.
>Although when you have it there is some difference, it drops to 6 lines,
>instead

>of 300.

-> Considering build-dep doxygen
-> Trying doxygen
-> Installing  debhelper libmcrypt-dev libmhash-dev zlib1g-dev libtool 
libtool-bin libjpeg-dev dh-autoreconf doxygen


I'm installing it

dpkg -c steghide_0.5.1-11_amd64.deb  |grep man |wc -l
308



no, you can't pollute the usr/share/man/man3 namespace with all the functions 
of your library.
-rw-r--r-- root/root   275 2016-08-22 23:03 
./usr/share/man/man3/WavFormatChunk.h.3.gz
-rw-r--r-- root/root   869 2016-08-22 23:03 
./usr/share/man/man3/WavPCMSampleValue.3.gz
-rw-r--r-- root/root   291 2016-08-22 23:03 
./usr/share/man/man3/WavPCMSampleValue.h.3.gz
-rw-r--r-- root/root   808 2016-08-22 23:03 
./usr/share/man/man3/WavPCMSampleValueTest.3.gz
-rw-r--r-- root/root   271 2016-08-22 23:03 
./usr/share/man/man3/WavPCMSampleValueTest.h.3.gz
-rw-r--r-- root/root   385 2016-08-22 23:03 
./usr/share/man/man3/WavPCMSmaller.3.gz
-rw-r--r-- root/root   849 2016-08-22 23:03 
./usr/share/man/man3/common.h.3.gz
-rw-r--r-- root/root   383 2016-08-22 23:03 
./usr/share/man/man3/error.h.3.gz
-rw-r--r-- root/root   517 2016-08-22 23:03 
./usr/share/man/man3/gettext.h.3.gz
-rw-r--r-- root/root   304 2016-08-22 23:03 ./usr/share/man/man3/msg.h.3.gz
-rw-r--r-- root/root   374 2016-08-22 23:03 
./usr/share/man/man3/utcommon.h.3.gz
-rw-r--r-- root/root   206 2016-08-22 23:03 
./usr/share/man/man3/wrapper_hash_map.h.3.gz
-rw-r--r-- root/root   205 2016-08-22 23:03 
./usr/share/man/man3/wrapper_hash_set.h.3.gz


this is something that should be installed as a doxygen documentation, 
somewhere else.

>Ok, this might be something I messed up when removing usage of mkinstalldirs
>script.


wonderful
>Yes, there is a grammar correction for po/de.po indeed.
>Now for the spec file I kind of ignored it intentionally.
>I will review them all.


probably the spec file is autogenerated, not sure

>Right, I was not sure about this.
>At first the dh_clean broke badly so I cleaned it up manually then did this.
>I will fix it.


usually it breaks when you kill the configure script, you have to manually
do: fakeroot debian/rules clean, and maybe delete files created when configure
fails, e.g. config.log

I hope this helps,

G.

Bug#835253: RFS: steghide/0.5.1-11 [QA]

[Fernando Seiti Furusato]

Hello Gianfranco.

On 09/08/2016 02:43 PM, Gianfranco Costamagna wrote:
> Hi, while waiting for Bruno's review, I'm adding another little (unasked)
> one.
Your reviews are welcome!
>
> debdiff steghide_0.5.1-10_amd64.deb steghide_0.5.1-11_amd64.deb
>
> you are installing too many manpages
> -rw-r--r--  root/root   /usr/share/man/man3/BmpFile.3.gz
> -rw-r--r--  root/root   /usr/share/man/man3/BmpFile.h.3.gz
>
> -rw-r--r--  root/root   /usr/share/man/cc/man3/error.3.gz
> -rw-r--r--  root/root   /usr/share/man/cc/man3/msg.3.gz
>
> and so on for ~300 lines
This is actually because of lack of dependency on doxygen.
When you do not have doxygen installed, the manpages are simply not
created with
no errors whatsoever.
Although when you have it there is some difference, it drops to 6 lines,
instead
of 300.
>
> something broke the po files
> before:
>
>
> installing es.gmo as 
> /build/steghide-0.5.1/debian/steghide/usr/share/locale/es/LC_MESSAGES/steghide.mo
>
>
> now:
> /usr/bin/install: cannot create regular file 
> '/build/steghide-0.5.1/debian/steghide/usr/share/locale/es/LC_MESSAGES/steghide.mo':
>  No such file or directory
>
> and debdiff shows:
> Files in first .deb but not in second
> -
> -rw-r--r--  root/root   /usr/share/locale/de/LC_MESSAGES/steghide.mo
> -rw-r--r--  root/root   /usr/share/locale/es/LC_MESSAGES/steghide.mo
> -rw-r--r--  root/root   /usr/share/locale/fr/LC_MESSAGES/steghide.mo
> -rw-r--r--  root/root   /usr/share/locale/ro/LC_MESSAGES/steghide.mo
>
> this seems because of a missing patch on po/Makefile.in.in directory
> +-$(mkinstalldirs) $(DESTDIR)$$dir; \
>
>
> you need to patch it to create the directory.
>
> (this is a bad effect of the migration to autoreconf.
> Patching autogenerated files is bad, because autoreconf overrides the 
> changes).
>
> Please try to patch the source Makefile.in.in and let autoreconf to the 
> remaining stuff :D
Ok, this might be something I messed up when removing usage of mkinstalldirs
script.
>
> debdiff between sources dsc files shows changes not converted in patch
> e.g.
> po/de.po
> doxygen and spec files
> (and probably more)
Yes, there is a grammar correction for po/de.po indeed.
Now for the spec file I kind of ignored it intentionally.
I will review them all.
>
> override_dh_auto_clean:
> if [ -e Makefile ]; then $(MAKE) distclean; fi
>
>
> ^^ the above should be useless, and partially wrong
> (in fact it seems to be not working to me)
>
>
> if [ -f Makefile]; then dh_auto_clean; fi
> might be better, even if I would just remove that line
Right, I was not sure about this.
At first the dh_clean broke badly so I cleaned it up manually then did this.
I will fix it.

Thanks!

-- 
Fernando Seiti Furusato
IBM Linux Technology Center




signature.asc
Description: OpenPGP digital signature

Bug#835253: RFS: steghide/0.5.1-11 [QA]

[Gianfranco Costamagna]

Hi, while waiting for Bruno's review, I'm adding another little (unasked)
one.



debdiff steghide_0.5.1-10_amd64.deb steghide_0.5.1-11_amd64.deb

you are installing too many manpages
-rw-r--r--  root/root   /usr/share/man/man3/BmpFile.3.gz
-rw-r--r--  root/root   /usr/share/man/man3/BmpFile.h.3.gz

-rw-r--r--  root/root   /usr/share/man/cc/man3/error.3.gz
-rw-r--r--  root/root   /usr/share/man/cc/man3/msg.3.gz

and so on for ~300 lines

something broke the po files
before:


installing es.gmo as 
/build/steghide-0.5.1/debian/steghide/usr/share/locale/es/LC_MESSAGES/steghide.mo


now:
/usr/bin/install: cannot create regular file 
'/build/steghide-0.5.1/debian/steghide/usr/share/locale/es/LC_MESSAGES/steghide.mo':
 No such file or directory

and debdiff shows:
Files in first .deb but not in second
-
-rw-r--r--  root/root   /usr/share/locale/de/LC_MESSAGES/steghide.mo
-rw-r--r--  root/root   /usr/share/locale/es/LC_MESSAGES/steghide.mo
-rw-r--r--  root/root   /usr/share/locale/fr/LC_MESSAGES/steghide.mo
-rw-r--r--  root/root   /usr/share/locale/ro/LC_MESSAGES/steghide.mo

this seems because of a missing patch on po/Makefile.in.in directory
+-$(mkinstalldirs) $(DESTDIR)$$dir; \


you need to patch it to create the directory.

(this is a bad effect of the migration to autoreconf.
Patching autogenerated files is bad, because autoreconf overrides the changes).

Please try to patch the source Makefile.in.in and let autoreconf to the 
remaining stuff :D


debdiff between sources dsc files shows changes not converted in patch
e.g.
po/de.po
doxygen and spec files
(and probably more)


override_dh_auto_clean:
if [ -e Makefile ]; then $(MAKE) distclean; fi


^^ the above should be useless, and partially wrong
(in fact it seems to be not working to me)


if [ -f Makefile]; then dh_auto_clean; fi
might be better, even if I would just remove that line

hope this helps,

G.

Bug#835253: RFS: steghide/0.5.1-11 [QA]

[Fernando Seiti Furusato]

It was actually missing doxygen so the manpages would be built.
I have just uploaded it with the new change.

To access further information about this package, please visit the following 
URL:

https://mentors.debian.net/package/steghide

Alternatively, one can download the package with dget using this command:

dget -x 
https://mentors.debian.net/debian/pool/main/s/steghide/steghide_0.5.1-11.dsc

  Changes since the last upload:

  * QA upload.
  * debian/compat:
- Bumped compat level to 9
  * debian/source/format:
- File added with "3.0 (Quilt)"
  * debian/control:
- Added dh-autoreconf to build-depends.
- Added doxygen as a build-depend to build manpages.
- ${misc:Depends} to binary package dependency.
  * debian/rules:
- Changed to dh $@ to make debian build simpler.
- Build with --with-autoreconf to update config.{guess,sub} and fix
  FTBFS on architectures such as ppc64el. Closes: #759453, #535842
  * debian/steghide.manpages: added this file to debian in order for
dh_installman to work.
  * debian/patches:
- Updated to quilt format.
- Some patches to enable build system to run with autoreconf.

Regards.
-- 

Fernando Seiti Furusato
IBM Linux Technology Center




signature.asc
Description: OpenPGP digital signature

Bug#835253: RFS: steghide/0.5.1-11 [QA]

[Fernando Seiti Furusato]

Gentlemen, please disregard the last update I sent.
The installation of manpages do not really work when in a clean environment.
I will try again and report back.

Thanks.

-- 
Fernando Seiti Furusato
IBM Linux Technology Center




signature.asc
Description: OpenPGP digital signature

Bug#835253: RFS: steghide/0.5.1-11 [QA]

[Fernando Seiti Furusato]

Hello!

Could you please take a look now?

I reworked the rules file, changed to 3.0 (quilt) so the diff.gz would
not be included anymore.
I also changed some things in the Makefiles* so the build would work
more smoothly.
And now it is using autoreconf.
The debdiff output is huge because I reversed the diff.gz before running
debuild.

   * QA upload.
   * debian/compat:
 - Bumped compat level to 9
   * debian/source/format:
 - File added with "3.0 (Quilt)"
   * debian/control:
 - Added dh-autoreconf to build-depends.
 - ${misc:Depends} to binary package dependency.
   * debian/rules:
 - Changed to dh $@ to make debian build simpler.
 - Build with --with-autoreconf to update config.{guess,sub} and fix
   FTBFS on architectures such as ppc64el. Closes: #759453, #535842
   * debian/steghide.manpages: added this file to debian in order for
 dh_installman to work.
   * debian/patches:
 - Updated to quilt format.
 - Some patches to enable build system to run with autoreconf.

Thanks!

-- 
Fernando Seiti Furusato
IBM Linux Technology Center




signature.asc
Description: OpenPGP digital signature

Bug#835253: RFS: steghide/0.5.1-11 [QA]

[Fernando Seiti Furusato]

Hello Gianfranco.

Thanks for your input on this!

On 08/25/2016 02:56 PM, Gianfranco Costamagna wrote:

what about using autoreconf? it should fix the issue in a better(TM) way :)
https://wiki.debian.org/Autoreconf

Yes, I initially tried to add the usage of dh-autoreconf, but I rolled back
because it didn't build and clean smoothly.
Anyway I will work on it then.


BTW converting the rules file in the new dh calls seems to be trivial too ;)

Ok, let me give it a try.

Regards.

--
Fernando Seiti Furusato
IBM Linux Technology Center

Bug#835253: RFS: steghide/0.5.1-11 [QA]

[Gianfranco Costamagna]

Hi

sorry for stepping in :)

> - added autotools-dev to build-depends.
> - added dh_autotools-dev_updateconfig and
>   dh_autotools-dev_restoreconfig to fix FTBFS on new architectures
>   such as ppc64el. Closes: #759453, #535842
what about using autoreconf? it should fix the issue in a better(TM) way :)
https://wiki.debian.org/Autoreconf

BTW converting the rules file in the new dh calls seems to be trivial too ;)

(I'm not requesting you to change anything, I might do also a subsequent upload
when I bump debhelper compat level, and refactor the packaging to be more easily
maintainable)

thanks for caring you both :)

G.

Bug#835253: RFS: steghide/0.5.1-11 [QA]

[Fernando Seiti Furusato]

Hello.

I have just addressed the points raised.

dget 
-xhttps://mentors.debian.net/debian/pool/main/s/steghide/steghide_0.5.1-11.dsc

I also updated the changelog:
  
  * QA upload.

  * debian/control:
- added autotools-dev to build-depends.
- ${misc:Depends} to binary package dependency.

  [ Aurelien Jarno ]
  * debian/rules:
- added dh_autotools-dev_updateconfig and
  dh_autotools-dev_restoreconfig to fix FTBFS on new architectures
  such as ppc64el. Closes: #759453, #535842

Thanks!

--
Fernando Seiti Furusato
IBM Linux Technology Center

Re: Bug#835253: RFS: steghide/0.5.1-11 [QA]

[Fernando Seiti Furusato]

Hi, Breno.


On 08/24/2016 10:14 PM, Breno Leitao wrote:

During my tests with your submission, I found that the package currently
depends on libtool binary, but it does not build-depends on libtool-bin,
which causes the build to fail (FTBFS). I opened a bug to track it under
Bug#835378. Check it for more details.

This is my fault. I removed it by mistake from the build-depends.
I don't think a bug is needed in this case, because originally the 
package had a

dependency on it.

Also, I understand that your fix also closes the Bug#535842.  What do
you think?

Indeed, it seems to be the same problem.

Thank you,
Breno


Thanks for your response. I will work on it and re-upload it.

--
Fernando Seiti Furusato
IBM Linux Technology Center

Bug#835253: RFS: steghide/0.5.1-11 [QA]

[Breno Leitao]

Hello Fernando,

On Tue, Aug 23, 2016 at 04:20:47PM -0400, Fernando Seiti Furusato wrote:
>   Changes since the last upload:
>   
>   * QA upload.
> 
>   [ Aurelien Jarno ]
>   * debian/rules: added dh_autotools-dev_updateconfig and
> dh_autotools-dev_restoreconfig to fix FTBFS on new architectures
> such as arm64 and ppc64el. Closes: #759453

Thanks for contributing to Debian. Your changes seem fine to me, and I
am willing to sponsor it, but I would like to ask you something else.

During my tests with your submission, I found that the package currently
depends on libtool binary, but it does not build-depends on libtool-bin,
which causes the build to fail (FTBFS). I opened a bug to track it under
Bug#835378. Check it for more details.

Also, I understand that your fix also closes the Bug#535842.  What do
you think?

Thank you,
Breno


signature.asc
Description: PGP signature

Bug#835253: RFS: steghide/0.5.1-11 [QA]

[Fernando Seiti Furusato]

Package: sponsorship-requests
Severity: normal

Dear mentors,

  I am looking for a sponsor for the package "steghide"

 * Package name: steghide
   Version : 0.5.1-11
   Upstream Author : Stefan Hetzl 
 * URL : http://steghide.sourceforge.net/
 * License : GPL
   Section : misc

  It builds those binary package:

steghide   - A steganography hiding tool

  To access further information about this package, please visit the following 
URL:

  https://mentors.debian.net/package/steghide

  Alternatively, one can download the package with dget using this command:

dget -x 
https://mentors.debian.net/debian/pool/main/s/steghide/steghide_0.5.1-11.dsc

  Changes since the last upload:
  
  * QA upload.

  [ Aurelien Jarno ]
  * debian/rules: added dh_autotools-dev_updateconfig and
dh_autotools-dev_restoreconfig to fix FTBFS on new architectures
such as arm64 and ppc64el. Closes: #759453

  Regards,
   Fernando Seiti Furusato