Bug#824489: RFS: dwarfutils/20160507-1 [ITA] -- utility and library to work with DWARF debug information

2016-05-18 Thread Fabian Wolff
On Wed, May 18, 2016 at 03:29:33PM +, Gianfranco Costamagna wrote:
> Hi,
> 
> >Done. I removed the override in debian/rules.
> 
> wonderful
> 
> >I went with a mixture of both patches you proposed. I also forwarded>the 
> >patches to upstream & removed the now useless exports in
> >debian/rules.
> >
> >I've reuploaded the new package to Mentors (same URL).
> 
> 
> actually that was mostly the same patch I did at the begin, but I didn't send 
> it
> to you by purpose :)
> I wanted to see your skills, and see if your conclusion were eventually the 
> same as mine.
> 
> I have to say I'm satisfied and happy with your changes, so I uploaded it on 
> unstable.

Great. Thank you!

> I put it on deferred/1, so the package will show up in ~24 hours or so.
> 
> Sorry for that deferred, but I would like to have one more day of testing :)

Sure.

> thanks for the nice contribution to Debian, and sorry for being so pedantic 
> in my
> reviews :)
> 
> please continue the nice job you did as new shiny maintainer of the package

I'll try my best!

Cheers,
Fabian



Bug#824489: RFS: dwarfutils/20160507-1 [ITA] -- utility and library to work with DWARF debug information

2016-05-18 Thread Fabian Wolff
On Wed, May 18, 2016 at 06:59:10AM +, Gianfranco Costamagna wrote:
> I don't see why it can't be patched to work like almost every other tool
> that uses a build system, but I don't care a lot about upstream.
> 
> The (possible) issue I foresee is: somebody updates the upstream build system
> to install the files too, you update the packaging without checking
> that, the Debian upload is buggy because some files are missing.
> 
> in short words, I prefer an useless call, rather than a broken package!

Done. I removed the override in debian/rules.

> I'm not fluent too, thanks for checking and replying about that, it was 
> already
> fine, but I just wanted to be sure there was a rationale for the change!
> 
> let me know your opinion about the install step, and I'll do the final checks 
> + sponsoring
> 
> 
> BTW, I like when a patch/fix can be upstream, so I propose another one (feel 
> free to reject of course, you are
> the maintainer, not me!)
> --- dwarfutils-20160507.orig/libdwarf/configure.in
> +++ dwarfutils-20160507/libdwarf/configure.in
> @@ -127,11 +127,11 @@ AC_TRY_COMPILE([#include ],[  Elf
> dnl default-disabled shared
> AC_SUBST(build_shared,[none])
> AC_SUBST(dwfpic,[])
> +AC_SUBST(dwfpic,[-fPIC])
> AC_ARG_ENABLE(shared,AC_HELP_STRING([--enable-shared],
> [build shared library libdwarf.so]))
> AS_IF([ test "x$enable_shared" = "xyes"], [
> AC_SUBST(build_shared,[libdwarf.so])
> -   AC_SUBST(dwfpic,[-fPIC])
> ])
> 
> dnl default-enabled nonshared
> 
> 
> maybe it makes sense to enable it alsofor static libs then anyway.
> (probably all the if block can be removed this way, but this is something 
> that upstream has to investigate ;))

I went with a mixture of both patches you proposed. I also forwarded
the patches to upstream & removed the now useless exports in
debian/rules.

I've reuploaded the new package to Mentors (same URL).



Bug#824489: RFS: dwarfutils/20160507-1 [ITA] -- utility and library to work with DWARF debug information

2016-05-18 Thread Gianfranco Costamagna
Hi Fabian


>dh_auto_install does not do anything useful here: `make install` for

>some reason does not actually install anything but just compiles some
>examples (that won't be part of the Debian package) and then finishes
>with the note
>
>"No install provided, see comments in the README"
>
>The README basically says that the installation should be performed
>manually by copying the desired files to the desired directories,
>which is what dh_install will do in the next step. Therefore, calling
>dh_auto_install probably won't hurt, but it does waste time, which is
>why I have overwritten it with an empty rule.


I don't see why it can't be patched to work like almost every other tool
that uses a build system, but I don't care a lot about upstream.

The (possible) issue I foresee is: somebody updates the upstream build system
to install the files too, you update the packaging without checking
that, the Debian upload is buggy because some files are missing.

in short words, I prefer an useless call, rather than a broken package!

>The hardening rules do export -fPIE, but not -fPIC. Without -fPIC, I
>get the following error when trying to link the static library into
>another shared library:
>
>/usr/bin/ld: /tmp/[...]/usr/lib/libdwarf.a(dwarf_alloc.o): relocation 
>R_X86_64_PC32 against symbol `dwarf_dealloc' can not be used when making a 
>shared object; >recompile with -fPIC
>/usr/bin/ld: final link failed: Bad value
>collect2: error: ld returned 1 exit status


ok

>Your patch doesn't fix that, either (although I might have missed
>something there; I'm not exactly fluent in Autotools), whereas if I'm
>building libdwarf with -fPIC, the shared library builds and links just

>fine.

I'm not fluent too, thanks for checking and replying about that, it was already
fine, but I just wanted to be sure there was a rationale for the change!

let me know your opinion about the install step, and I'll do the final checks + 
sponsoring


BTW, I like when a patch/fix can be upstream, so I propose another one (feel 
free to reject of course, you are
the maintainer, not me!)
--- dwarfutils-20160507.orig/libdwarf/configure.in
+++ dwarfutils-20160507/libdwarf/configure.in
@@ -127,11 +127,11 @@ AC_TRY_COMPILE([#include ],[  Elf
dnl default-disabled shared
AC_SUBST(build_shared,[none])
AC_SUBST(dwfpic,[])
+AC_SUBST(dwfpic,[-fPIC])
AC_ARG_ENABLE(shared,AC_HELP_STRING([--enable-shared],
[build shared library libdwarf.so]))
AS_IF([ test "x$enable_shared" = "xyes"], [
AC_SUBST(build_shared,[libdwarf.so])
-   AC_SUBST(dwfpic,[-fPIC])
])

dnl default-enabled nonshared


maybe it makes sense to enable it alsofor static libs then anyway.
(probably all the if block can be removed this way, but this is something that 
upstream has to investigate ;))

thanks!


Gianfranco



Bug#824489: RFS: dwarfutils/20160507-1 [ITA] -- utility and library to work with DWARF debug information

2016-05-16 Thread Fabian Wolff
Thanks for the review. I'm glad you liked it!

> 1) why an empty dh_[auto_]install override?

dh_auto_install does not do anything useful here: `make install` for
some reason does not actually install anything but just compiles some
examples (that won't be part of the Debian package) and then finishes
with the note

 "No install provided, see comments in the README"

The README basically says that the installation should be performed
manually by copying the desired files to the desired directories,
which is what dh_install will do in the next step. Therefore, calling
dh_auto_install probably won't hurt, but it does waste time, which is
why I have overwritten it with an empty rule.

> 2) why exporting fPIC manually?

The hardening rules do export -fPIE, but not -fPIC. Without -fPIC, I
get the following error when trying to link the static library into
another shared library:

 /usr/bin/ld: /tmp/[...]/usr/lib/libdwarf.a(dwarf_alloc.o): relocation 
R_X86_64_PC32 against symbol `dwarf_dealloc' can not be used when making a 
shared object; recompile with -fPIC
 /usr/bin/ld: final link failed: Bad value
 collect2: error: ld returned 1 exit status

Your patch doesn't fix that, either (although I might have missed
something there; I'm not exactly fluent in Autotools), whereas if I'm
building libdwarf with -fPIC, the shared library builds and links just
fine.

Thanks again,
Fabian