Bug 799089 - Review Request: dyninst - An API for Run-time Code Generation
Summary: Review Request: dyninst - An API for Run-time Code Generation
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Frank Ch. Eigler
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2012-03-01 18:40 UTC by William Cohen
Modified: 2013-10-16 12:09 UTC (History)
6 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2012-07-18 16:15:46 UTC
Type: ---
Embargoed:
fche: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)
rpmlint results (67.60 KB, text/plain)
2012-03-12 19:38 UTC, Frank Ch. Eigler
no flags Details

Description William Cohen 2012-03-01 18:40:29 UTC
Spec URL: http://people.redhat.com/wcohen/dyninst/dyninst.spec
SRPM URL: http://people.redhat.com/wcohen/dyninst/dyninst-7.0.1-0.3.fc16.src.rpm
Description: 

Dyninst is an Application Program Interface (API) to permit the insertion of
code into a running program. The API also permits changing or removing
subroutine calls from the application program. Run-time code changes are
useful to support a variety of applications including debugging, performance
monitoring, and to support composing applications out of existing packages.
The goal of this API is to provide a machine independent interface to permit
the creation of tools and applications that use run-time code patching.

Comment 1 William Cohen 2012-03-01 18:41:42 UTC
I am able to build the package as a scratch build on f16:

http://koji.fedoraproject.org/koji/taskinfo?taskID=3843699

Comment 2 Frank Ch. Eigler 2012-03-12 19:38:55 UTC
Created attachment 569495 [details]
rpmlint results

Comment 3 Frank Ch. Eigler 2012-03-13 19:19:10 UTC
ReviewGuidelines MUST items failed:

MUST: packaging guidelines:
      - bundled librares, as below
      - make DESTDIR=%{buildroot} should work
      - %configure should be used

MUST: not bundle system libraries
      - boost is bundled, and is even installed into dyninst-devel.

ReviewGuidelines SHOULD items failed:
      /usr/bin/parseThat should have a man page

Other items in Packaging:ReviewGuidelines and Packaging:Guidelines appear OK.

Comment 4 William Cohen 2012-03-15 21:30:36 UTC
MUST: packaging guidelines:
      - bundled librares, as below 
      - make DESTDIR=%{buildroot} should work
      ( DESTDIR=... isn't currently honored in the make)
      - %configure should be used
      (agreed, tried to use %configure, but because of the intermixed build/
       install. the build would attempt to install in /usr/lib and /usr/include
       because it ignored the staged installs, if is DESTDIR honored, might be
       able to use %configure)


MUST: not bundle system libraries
      - boost is bundled, and is even installed into dyninst-devel.
      (it looks like one can just add a BuildReqs: boost-deve and nuke the boost
       subdirectory)

ReviewGuidelines SHOULD items failed:
      /usr/bin/parseThat should have a man page

Comment 5 Josh Stone 2012-03-15 21:56:49 UTC
(In reply to comment #4)
> MUST: not bundle system libraries
>       - boost is bundled, and is even installed into dyninst-devel.
>       (it looks like one can just add a BuildReqs: boost-deve and nuke the
>        boost subdirectory)

Did you notice that boost actually exists twice?

$ find -name boost
./dyninstAPI/src/dyninst/external/boost
./dyninstAPI/src/dyninst/dynutil/h/dyn_detail/boost

The latter is just a subset, and a few files that I compared look identical, except namespaced everywhere as "dyn_detail::".  It appears to be included and used by that explicit path and namespace in quite a few places.  It may still be possible to patch that out, but not as trivial as nuking a subdirectory...

Comment 6 William Cohen 2012-03-21 14:54:15 UTC
Built a version minus one of the boost header files:

http://koji.fedoraproject.org/koji/taskinfo?taskID=3917629

Comment 8 William Cohen 2012-06-19 21:14:46 UTC
Most of the development work has been in the proccontrol branch of dyninst. This also has a number of improvements in the build process which would be difficult to backport to the older release of dyninst. Made tar.gz files of the git repositories following the guidelines to package the repos in a repeatable manner:

http://fedoraproject.org/wiki/Packaging/SourceURL

The resulting dyninst-7.99-0.12.fc17.src.rpm builds and installs on fedora 17.

The updated dyninst.spec file and dyninst-7.99-0.12.fc17.src.rpm are available at http://people.redhat.com/wcohen/dyninst

The rpmlint results look a bit better:

$ rpmlint  ../SRPMS/*7.99* ../RPMS/x86_64/*7.99*|more
dyninst.src:99: W: macro-in-comment %{_bindir}
dyninst.src:103: W: macro-in-comment %doc
dyninst.src: W: invalid-url Source1: dyninst-docs-7.99.tar.gz
dyninst.src: W: invalid-url Source0: dyninst-7.99.tar.gz
dyninst.x86_64: W: executable-stack /usr/lib64/dyninst/libdyninstAPI_RT_m32.so.7
.99
dyninst.x86_64: W: executable-stack /usr/lib64/dyninst/libdyninstAPI_RT.so.7.99
dyninst-devel.x86_64: W: only-non-binary-in-usr-lib
dyninst-devel.x86_64: W: no-documentation
dyninst-static.x86_64: W: no-documentation
5 packages and 0 specfiles checked; 0 errors, 9 warnings.

Comment 9 Frank Ch. Eigler 2012-06-22 05:53:05 UTC
Nice job, the basic requirements now appear to be met.

Comment 10 William Cohen 2012-06-22 13:14:29 UTC
New Package SCM Request
=======================
Package Name: dyninst
Short Description: An API for Run-time Code Generation
Owners: wcohen
Branches: devel
InitialCC: fche

Comment 11 Gwyn Ciesla 2012-06-22 13:22:55 UTC
Git done (by process-git-requests).

Comment 12 Frank Ch. Eigler 2012-07-18 16:15:46 UTC
rawhide includes dyninst-7.99-*

Comment 13 Michal Ambroz 2012-08-11 20:17:06 UTC
Hi please do you plan to branch for f17 ?

Comment 14 Frank Ch. Eigler 2012-08-11 22:41:17 UTC
Michal, Fedora does not normally backport brand new packages into existing releases.  The dyninst package being used in rawhide is already a pre-release of upstream dyninst 8.0, so we wouldn't want such a prerelease to spread even wider.  (You are of course welcome to grab the .src.rpm and build it for your own machines.)

Comment 15 Josh Stone 2012-08-13 19:42:34 UTC
I was already considering the creation of a private repo for older Fedoras, so now you've convinced me to just do it.  This is unofficial; your warranty is void; etc. etc.

http://repos.fedorapeople.org/repos/jistone/dyninst/

I'll try to keep this in sync with the packages we build for F18+.  As with everything else on repos.fedorapeople.org, don't bother bugzilla with any issues in these packages, but feel free to let me know personally and I'll try to help.

Comment 16 Orion Poplawski 2013-09-26 03:44:59 UTC
I would like to see this package in EPEL6.  I've requested the libdwarf dep in bug 492252.  Frank - would you be willing to maintain it there, or shall I?

Comment 17 Orion Poplawski 2013-09-26 03:50:45 UTC
Sorry, that should have been addressed to William (and other current co-maintainers)

Comment 18 Josh Stone 2013-09-26 17:26:44 UTC
Sorry, I guess it's obvious now that I've neglected my dyninst packages on repos.fpo.  That's moot for Fedora at this point, but still could help el6.

Orion, I don't mind if you want to maintain it as a proper EPEL6 package.

Comment 19 Orion Poplawski 2013-10-16 04:11:30 UTC
Package Change Request
======================
Package Name: dyninst
New Branches: el6
Owners: orion
InitialCC: 

will do

Comment 20 Gwyn Ciesla 2013-10-16 12:09:23 UTC
Git done (by process-git-requests).


Note You need to log in before you can comment on or make changes to this bug.