Bug 179910 - Review Request: commoncpp2 - commoncpp C++ framework
Summary: Review Request: commoncpp2 - commoncpp C++ framework
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review   
(Show other bugs)
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Ville Skyttä
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Keywords:
Depends On:
Blocks: FE-ACCEPT
TreeView+ depends on / blocked
 
Reported: 2006-02-03 20:06 UTC by Andreas Thienemann
Modified: 2015-01-08 16:22 UTC (History)
1 user (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-02-06 22:29:41 UTC
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Andreas Thienemann 2006-02-03 20:06:29 UTC
Spec Name or Url: http://helena.bawue.de/~ixs/commoncpp/commoncpp.spec
SRPM Name or Url: http://helena.bawue.de/~ixs/commoncpp/commoncpp-1.3.22-1.src.rpm
Description:
GNU Common C++ is a portable and highly optimized class framework for writing
C++ applications that need to use threads, sockets, XML parsing,
serialization, config files, etc. This framework offers a class foundation
that hides platform differences from your C++ application so that you need
not write platform specific code. GNU Common C++ has been ported to compile
nativily on most platforms which support posix threads.

Comment 1 Andreas Thienemann 2006-02-03 22:32:55 UTC
Note: The package name was changed after this bug was filed to commoncpp2.

It seems the gnu people refer to the software as commoncpp, even though the
tarball is named commoncpp2.
As the library is named commoncpp2-*.so as well, this is probably a more
sensible name.

Comment 2 Ed Hill 2006-02-04 00:20:51 UTC
Hi Andreas, this isn't a full review (yet!), just a handful of observations:

 - please shorten "A portable and highly optimized class framework for 
     writing C++ applications" to something shorter such as "A portable 
     and optimized framework for C++ applications"
 - "nativily" --> "natively"
 - please delete all *.la files (per the packaging guidelines)
 - the "/usr/include/cc++2" dir is unowned

Comment 3 Ville Skyttä 2006-02-04 16:31:27 UTC
More observations:

- 1.3.23 is out
- Most likely missing build dependencies: libxml2-devel, zlib-devel, doxygen
- Missing dependency on pkgconfig in -devel
- Missing install-info dependencies in -devel, "|| :" safeguards missing from 
  install-info invocations (breaks --excludedocs installs)
- Installation may try to modify /etc/ld.so.conf

Feel free to borrow fixes for the above and whatever you find worth taking from
my local package at http://cachalot.mine.nu/5/SRPMS/commoncpp2-1.3.23-v1.src.rpm

Comment 4 Ed Hill 2006-02-04 16:47:38 UTC
Hi Ville, I think you have a much better idea whats happening here so, if 
you want, please go ahead and replace me as the reviewer.  Otherwise, I'll 
do what I can to help the submitter get the good bits from your package
incorporated into this one.


Comment 5 Andreas Thienemann 2006-02-04 21:34:14 UTC
1.3.23 is out? hmmm. gonna update that then. The webpage referred to 1.3.22.
About the build-dependencies, mock does build the package as is. I checked.
The sagefuard is a good idea.
The modification of ld.so.conf is taken care of, the Makefile.in is being
patched accordingly.

Thanks so far for the suggestions. I'm gonna update the package.
Only thing I'm unsure about are the .la files. It seems they were being used by
ccrtp. If so, removing them is probably not a good idea.

Comment 6 Ville Skyttä 2006-02-05 09:04:24 UTC
(Reassigning to myself per comment 4)

Sure, mock probably builds it without the build dependencies, but obviously
leaves out zlib  and libxml2 support.  But if they're installed, the
corresponding features are built in.  So please either add the build
dependencies or explicitly disable these features so that the build is reproducible.

doxygen is a slightly different thing; with it installed the API docs are built
into doc/html/ and I think it would be good to include them in the devel package
(%doc doc/html).

If you're planning to include the *.la, please show a real case where they are
required (not "seems", "probably"...)

Comment 7 Andreas Thienemann 2006-02-05 12:49:10 UTC
Okay, Changes are incorporated.

Take a look please at http://home.bawue.de/~ixs/commoncpp/commoncpp2.spec and
http://home.bawue.de/~ixs/commoncpp/commoncpp2-1.3.23-2.src.rpm


Comment 8 Ville Skyttä 2006-02-05 21:26:28 UTC
Blockers:
- need fully versioned %{name} = %{version}-%{release} dependency in -devel.
- need Requires: libxml2-devel and zlib-devel in -devel (see ccgnu2-config
  --stdlibs and --extlibs, and libccext2.pc, and some installed *.h)

Suggestions:
- configure with --disable-dependency-tracking for possibly faster builds and
  cleaner build output
- Don't hardcode .gz in info filenames, install-info works without it and * can
  be used in %files
- install-info is not a "Requires", it's "Requires(post)" and "Requires(preun)"
- move %doc in -devel's %files after %defattr (actually, I'm a bit surprised 
  that it being before %defattr doesn't cause problems with files being owned by 
  the build owner here)

Comment 9 Andreas Thienemann 2006-02-06 11:22:35 UTC
Changes incorporated.

http://helena.bawue.de/~ixs/commoncpp/

I'm a bit surprised though, to notice that the external dependencies were not
taken up by rpm automagically.

Comment 10 Ville Skyttä 2006-02-06 20:39:35 UTC
Looks good.

Comment 11 Andreas Thienemann 2006-02-06 20:45:21 UTC
thx for the review.
as soon as this one is build, I'll put up the ccrtp for review. I'd be glad, if
you could take alook as well.

Comment 12 Christian Iseli 2006-10-18 13:18:04 UTC
Normalize summary field for easy parsing

Comment 13 Jan Grulich 2015-01-08 15:24:02 UTC
Package Change Request
======================
Package Name: commoncpp2
New Branches: epel7
Owners: ixs

Comment 14 Gwyn Ciesla 2015-01-08 16:22:39 UTC
Git done (by process-git-requests).


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