Bug 618451

Note: the
  %global glibc_soversion
macro is intended to track the glibc version.  On F-12 this is 2.11.2; on F13 this is 2.11.90 etc.

The purpose of this is to ensure that the hooks get autoloaded by gdb when the debug information for glibc is loaded.

one question：

Why not use "%global module_install_path %{python_sitelib}" instead of "%global module_install_path %{_libdir}/gdb-heap"? Can gdb-heap work out of the box if we add python modules to a standard site?

My reasoning here is that gdb-heap uses the "gdb" module, which only exists in the python environment embedded inside /usr/bin/gdb, not in /usr/bin/python.

So it's not going to work if you try to use it from the regular /usr/bin/python environment.

Having said that, we could simply "claim" the "gdbheap" part of the namespace below python's site-packages (doing so might break pydoc and other tools that assume that all such modules are importable by /usr/bin/python, though).

Does this make sense?

The license tag should be:

License: LGPLv2+ and Python

Also, you're using mixed macros ($RPM_BUILD_ROOT vs %{buildroot}). I would suggest that you standardize on the latter.

I'm a little concerned about the forced dependency on /usr/lib/debug/lib*/ld-2.11.2.so.debug, as technically, the package that provides it (glibc-debuginfo) is not part of the normal repositories, so this package will not be installable without the user consciously enabling the debuginfo repo.

Perhaps a cleaner approach would be for the command to check on invocation for presence of that library, and if not found, prompt the user to run the appropriate "debuginfo-install" command, similar to gdb's behavior when it hits missing debuginfo during a debug run.

(In reply to comment #4)
> The license tag should be:
> 
> License: LGPLv2+ and Python
Fixed

> Also, you're using mixed macros ($RPM_BUILD_ROOT vs %{buildroot}). I would
> suggest that you standardize on the latter.
Fixed


> I'm a little concerned about the forced dependency on
> /usr/lib/debug/lib*/ld-2.11.2.so.debug, as technically, the package that
> provides it (glibc-debuginfo) is not part of the normal repositories, so this
> package will not be installable without the user consciously enabling the
> debuginfo repo.
> 
> Perhaps a cleaner approach would be for the command to check on invocation for
> presence of that library, and if not found, prompt the user to run the
> appropriate "debuginfo-install" command, similar to gdb's behavior when it hits
> missing debuginfo during a debug run.    

This code is autoloaded by gdb when the libc.so.debug is loaded by gdb (in response to libc.so is loaded in the inferior process) i.e. when the DWARF debug info for glibc is loaded by gdb.  Hence if that file isn't present, the code is not autoloaded or run, and thus isn't able to issue a warning message.

I've changed this to a dependency on:
  Requires: glibc%{_isa} = %{glibc_version}
though this really ought to read:
  Requires: glibc-debuginfo%{_isa} = %{glibc_version}
as that's what it really needs.

Without the debuginfo package this code not only is useless, but won't load or run.

I don't know if it's possible to rig things up so that this autoloaded whenever gdb starts up (CCing tromey for gdb advice), that way we could issue a message.   (Though it strikes me that this ought to be handled at the rpm metadata level, not as a runtime failure, and that we ought to allow packages that require debuginfo to require that debuginfo).

Updated specfile:
http://dmalcolm.fedorapeople.org/python-packaging/gdb-heap.spec

Updated SRPM:
http://dmalcolm.fedorapeople.org/python-packaging/gdb-heap-0.1-2.fc12.src.rpm

Specfile changes:
http://dmalcolm.fedorapeople.org/python-packaging/from-0.1-1-to-0.1-2.diff

(In reply to comment #5)
> (In reply to comment #4)

> > I've changed this to a dependency on:
>   Requires: glibc%{_isa} = %{glibc_version}
> though this really ought to read:
>   Requires: glibc-debuginfo%{_isa} = %{glibc_version}
> as that's what it really needs.
> Without the debuginfo package this code not only is useless, but won't load or
> run.
> I don't know if it's possible to rig things up so that this autoloaded whenever
> gdb starts up (CCing tromey for gdb advice), that way we could issue a message.
>   (Though it strikes me that this ought to be handled at the rpm metadata
> level, not as a runtime failure, and that we ought to allow packages that
> require debuginfo to require that debuginfo).

Actually,  Requires: glibc%{_isa} is useless for this package, gdb-heap depends on python/gdb, python and gdb will pull in glibc as a dependency, I think adding a README to %doc will be more helpful.

Also, I suggest you to install head modules to %{_datadir}/gdb-heap, arch independent data should go into /usr/share in FHS. Many other packages(e.g. ibus) already install their private python modules to %{_datadir}

See
http://www.pathname.com/fhs/pub/fhs-2.3.html#USRSHAREARCHITECTUREINDEPENDENTDATA

(In reply to comment #6)
> Actually,  Requires: glibc%{_isa} is useless for this package, gdb-heap depends
> on python/gdb, python and gdb will pull in glibc as a dependency, I think
> adding a README to %doc will be more helpful.
The point of that "Requires" is to require a specific version of glibc.


> Also, I suggest you to install head modules to %{_datadir}/gdb-heap, arch
> independent data should go into /usr/share in FHS. Many other packages(e.g.
> ibus) already install their private python modules to %{_datadir}
> 
> See
> http://www.pathname.com/fhs/pub/fhs-2.3.html#USRSHAREARCHITECTUREINDEPENDENTDATA    

Thanks - good idea.

Is it possible to read /usr/lib/debug/lib64/ld-linux-x86-64.so.2.debug instead of /usr/lib/debug/lib64/ld-2.12.90.so.debug and /usr/lib/debug/lib/ld-linux.so.2.debug instead of /usr/lib/debug/lib/ld-2.12.90.so.debug? 

/usr/lib/debug/lib64/ld-linux-x86-64.so.2.debug -> /usr/lib/debug/lib64/ld-2.12.90.so.debug 

/usr/lib/debug/lib/ld-linux.so.2.debug -> /usr/lib/debug/lib/ld-2.12.90.so.debug

All current Fedora/RHEL release have /usr/lib/debug/lib64/ld-linux-x86-64.so.2.debug and /usr/lib/debug/lib/ld-linux.so.2.debug which is not restricted by glibc version.

(In reply to comment #8)
> Is it possible to read /usr/lib/debug/lib64/ld-linux-x86-64.so.2.debug instead
> of /usr/lib/debug/lib64/ld-2.12.90.so.debug and
> /usr/lib/debug/lib/ld-linux.so.2.debug instead of
> /usr/lib/debug/lib/ld-2.12.90.so.debug? 
> 
> /usr/lib/debug/lib64/ld-linux-x86-64.so.2.debug ->
> /usr/lib/debug/lib64/ld-2.12.90.so.debug 
> 
> /usr/lib/debug/lib/ld-linux.so.2.debug ->
> /usr/lib/debug/lib/ld-2.12.90.so.debug
I don't believe so, based on strace-ing gdb

Updated specfile:
http://dmalcolm.fedorapeople.org/python-packaging/gdb-heap.spec

Updated SRPM:
http://dmalcolm.fedorapeople.org/python-packaging/gdb-heap-0.2-1.fc12.src.rpm

I've moved the bulk of the code to below _datadir

I spoke with the RH gdb experts; my understanding is that at present there isn't a way to autoload a python script, except by placing it adjacent to either a DSO that's loaded (with a -gdb.py extension), or the .debug file.

Unfortunately, placing it next to the DSO itself leads to noise from ldconfig (see bug 562980), so placing it next to the .debug file (as done here) seems like the best approach to me.

This also contains a number of bugfixes in the underlying tarball

Just to make sure I understand:
This means that the gdb-heap code will never load unless the .debug file is present, right?

And since we can't guarantee that the glibc-debuginfo package is installed, what happens when glibc-debuginfo isn't installed and someone tries to invoke the "heap" command?

(In reply to comment #11)
> Just to make sure I understand:
> This means that the gdb-heap code will never load unless the .debug file is
> present, right?
That was my understanding.

However,
https://bugzilla.redhat.com/show_bug.cgi?id=562980#c3
suggests that it may be possible to install the loader code to 
  /usr/share/gdb/auto-load/lib/ld-%{glibc_version}.so-gdb.py
instead.

We'd still need a:
  Requires: glibc = %{version}
to ensure that we're in sync with glibc.

With this, we could be autoloaded provided that the correct glibc is loaded, and could issue an error message is glibc-debuginfo not available.

> And since we can't guarantee that the glibc-debuginfo package is installed,
> what happens when glibc-debuginfo isn't installed and someone tries to invoke
> the "heap" command?    
(gdb) heap
Undefined command: "heap".  Try "help".

(In reply to comment #12)

> However,
> https://bugzilla.redhat.com/show_bug.cgi?id=562980#c3
> suggests that it may be possible to install the loader code to 
>   /usr/share/gdb/auto-load/lib/ld-%{glibc_version}.so-gdb.py
> instead.
> 
> We'd still need a:
>   Requires: glibc = %{version}
> to ensure that we're in sync with glibc.
> 
> With this, we could be autoloaded provided that the correct glibc is loaded,
> and could issue an error message is glibc-debuginfo not available.

This sounds like the best option I've heard so far.

I've updated the packaging based on this idea, and implemented runtime detection of missing debuginfo:

(gdb) heap
Missing debuginfo for glibc
Suggested fix:
    debuginfo-install glibc

Updated specfile:
http://dmalcolm.fedorapeople.org/python-packaging/gdb-heap.spec

Updated SRPM:
http://dmalcolm.fedorapeople.org/python-packaging/gdb-heap-0.3-1.fc12.src.rpm

Specfile changes:
http://dmalcolm.fedorapeople.org/python-packaging/from-0.2-1-to-0.3-1.diff

Note to self, until I figure out git tagging:
  0.3 was at changeset bc630b438ca75106ead5cce671f44141db140fe5
  0.2 was at a35f2934f7b7afc2f305db2b6360a269d3e39901
  0.1 was at 853132392ba48695af4d2ce3cda2b646ba1fad88

A few points:

The comment at the top is no longer valid, but there is still no reason to create debuginfo here (it would be empty), as this is a architecture indepedent payload in an arch specific package (to match glibc). It would be nice to update that comment to reflect the actual reason for disabling debuginfo.

Also, the license tag is still invalid (should be LGPLv2+ and Python). Plus, you don't include copies of the License text. Given that you're upstream on this, you must do this (and package them as %doc). You should also do a proper header attribution of LGPLv2+ in your source files (with the exception of the Python licensed file(s), of course), like this:

    Copyright (C) <year>  <name of author>

    This library is free software; you can redistribute it and/or
    modify it under the terms of the GNU Lesser General Public
    License as published by the Free Software Foundation; either
    version 2.1 of the License, or (at your option) any later version.

    This library is distributed in the hope that it will be useful,
    but WITHOUT ANY WARRANTY; without even the implied warranty of
    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
    Lesser General Public License for more details.

    You should have received a copy of the GNU Lesser General Public
    License along with this library; if not, write to the Free Software
    Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA

Last, rpmlint says:

gdb-heap.src:47: W: macro-in-comment %{_isa}
gdb-heap.src:47: W: macro-in-comment %{glibc_version}

Should be harmless, but I would still advise that you %% out those macros in the comments.

=== REVIEW ===

MUST FIX:

- rpmlint checks return:
gdb-heap.src: W: invalid-license PSF
gdb-heap.x86_64: W: invalid-license PSF
License tag must be fixed before import, as described above.
- license (LGPLv2+ and Python) OKAY, but texts missing and files poorly attributed, see above.

Good:

- rpmlint checks return:
gdb-heap.src:47: W: macro-in-comment %{_isa}
gdb-heap.src:47: W: macro-in-comment %{glibc_version}
gdb-heap.x86_64: E: no-binary

Macros in comments should be cleaned up.

- package meets naming guidelines
- package meets packaging guidelines
- spec file legible, in am. english
- source matches upstream
- package compiles on devel (x86_64)
- no missing BR
- no unnecessary BR
- no locales
- not relocatable
- owns all directories that it creates
- no duplicate files
- permissions ok
- %clean ok
- macro use consistent
- code, not content
- no need for -docs
- nothing in %doc affects runtime
- no need for .desktop file 

Clean up those MUSTFIX items and I can approve this.

(In reply to comment #16)

Thanks.

> The comment at the top is no longer valid, but there is still no reason to
> create debuginfo here (it would be empty), as this is a architecture indepedent
> payload in an arch specific package (to match glibc). It would be nice to
> update that comment to reflect the actual reason for disabling debuginfo.

Fixed

>
> Also, the license tag is still invalid (should be LGPLv2+ and Python). Plus,
> you don't include copies of the License text. Given that you're upstream on
> this, you must do this (and package them as %doc). You should also do a proper
> header attribution of LGPLv2+ in your source files (with the exception of the
> Python licensed file(s), of course), like this:

Fixed: I've added license files to the upstream tarball and to %doc, and added license headers (see http://git.fedorahosted.org/git/?p=gdb-heap.git;a=commitdiff;h=08b5cf99c649d1e3c129b4350d67075f00fc5dd3 ) 

(snip)

> Last, rpmlint says:
> 
> gdb-heap.src:47: W: macro-in-comment %{_isa}
> gdb-heap.src:47: W: macro-in-comment %{glibc_version}

Fixed: I removed/rewrote that comment

(snip)

Updated specfile:
http://dmalcolm.fedorapeople.org/python-packaging/gdb-heap.spec

Updated SRPM:
http://dmalcolm.fedorapeople.org/python-packaging/gdb-heap-0.4-1.fc12.src.rpm

Changes in specfile:
http://dmalcolm.fedorapeople.org/python-packaging/gdb-heap-from-0.3-1-to-0.4-1.diff

Looks good. APPROVED.

New Package SCM Request
=======================
Package Name: gdb-heap
Short Description: Extensions to gdb for debugging dynamic memory allocation
Owners: dmalcolm
Branches: f14
InitialCC:

Git done (by process-git-requests).

gdb-heap-0.4-1.fc14 has been submitted as an update for Fedora 14.
http://admin.fedoraproject.org/updates/gdb-heap-0.4-1.fc14

gdb-heap-0.5-1.fc14 has been submitted as an update for Fedora 14.
http://admin.fedoraproject.org/updates/gdb-heap-0.5-1.fc14

gdb-heap-0.4-1.fc14 has been pushed to the Fedora 14 stable repository.  If problems still persist, please make note of it in this bug report.

gdb-heap-0.5-1.fc14 has been pushed to the Fedora 14 stable repository.  If problems still persist, please make note of it in this bug report.

Package Change Request
======================
Package Name: gdb-heap
New Branches: el6
Owners: dmalcolm

Git done (by process-git-requests).