Bug#824489: RFS: dwarfutils/20160507-1 [ITA] -- utility and library to work with DWARF debug information
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
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
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
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