This service will be undergoing maintenance at 00:00 UTC, 2016-08-01. It is expected to last about 1 hours

Bug 464074

Summary: Review Request: cddlib - A library for generating all vertices in convex polyhedrons
Product: [Fedora] Fedora Reporter: Conrad Meyer <konrad>
Component: Package ReviewAssignee: Paulo Roma Cavalcanti <promac>
Status: CLOSED RAWHIDE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: fedora-package-review, kwizart, limburgher, notting, promac, roma, tcallawa, tremble
Target Milestone: ---Keywords: Reopened
Target Release: ---Flags: promac: fedora‑review+
kevin: fedora‑cvs+
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2009-04-24 19:05:08 EDT Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:

Description Conrad Meyer 2008-09-26 03:23:08 EDT
Spec URL: http://konradm.fedorapeople.org/fedora/SPECS/cddlib.spec
SRPM URL: http://konradm.fedorapeople.org/fedora/SRPMS/cddlib-094f-1.fc9.src.rpm
Description:
The C-library  cddlib is a C implementation of the Double Description 
Method of Motzkin et al. for generating all vertices (i.e. extreme points)
and extreme rays of a general convex polyhedron in R^d given by a system 
of linear inequalities:

   P = { x=(x1, ..., xd)^T :  b - A  x  >= 0 }

where A is a given m x d real matrix, b is a given m-vector 
and 0 is the m-vector of all zeros.

The program can be used for the reverse operation (i.e. convex hull
computation). This means that one can move back and forth between 
an inequality representation and a generator (i.e. vertex and ray) 
representation of a polyhedron with cdd. Also, cdd can solve a linear
programming problem, i.e. a problem of maximizing and minimizing 
a linear function over P.
Comment 1 Paulo Roma Cavalcanti 2008-10-25 19:32:03 EDT
Hi,

this package produces two rpms.
The first contains only the
documentation, with a single file:

cddlibman.dvi

I think this dvi file should be converted to
a pdf file, so people can read it with any pdf
reader installed on the system.

The second rpm is the devel rpm, and contains 7 .h and 
2 .a files (static libraries). You did the right
thing, putting these files in a devel package.

However, you suppressed the generation of the examples
in the spec file, but I think they are important for the users.

When I generated the examples, I saw that they were installed
in /usr/bin and that they were quite a few files (26).

I would suggest installing the examples in the main package,
in a directory examples, which can be part of the documentation.
Comment 2 Dominik 'Rathann' Mierzejewski 2008-10-25 19:40:55 EDT
All that should be in -devel, even the doc and the examples, if they aren't too big in size. Main package shouldn't contain just one doc file, it's counter-intuitive (to me). You can simply not produce the main package (by omitting its %files section) and put the doc in -doc subpackage if you must.
Comment 3 Conrad Meyer 2008-10-25 19:47:15 EDT
Example binaries shouldn't be in documentation! The source for them is already there (IIRC) or if not can be added there. Maybe they can be placed in %libexec if they are helpful, but I thought the source would be helpful, while the binaries would not be. But I will take all the other suggestions for the next spec.
Comment 4 Paulo Roma Cavalcanti 2008-10-25 20:07:47 EDT
  
Here are some of the examples created:

 /usr/bin/testlp2_gmp
 /usr/bin/testlp3
 /usr/bin/testlp3_gmp
 /usr/bin/testshoot
 /usr/bin/testshoot_gmp

It seems to me that this package requires gmp.

The README advises to use gmp for getting accurate
results.

"Whenever the time is not important, it is safer 
to use gmp rational arithmetic."

In the doc directory there is already a pdf version of
the documentation.

All the example sources are in the directory src.
At least these sources should be in the documentation.
Comment 5 Conrad Meyer 2008-10-25 21:38:44 EDT
(In reply to comment #4)
> ...
> It seems to me that this package requires gmp.

This builds one library that uses gmp and one that doesn't. I'm not terribly familiar with the use of the library so I felt it was best to include both options; I would appreciate input from anyone who is more informed.

> In the doc directory there is already a pdf version of
> the documentation.

You prefer pdf to dvi? They are both supposed to be portable printable formats, but I'm not sure which we prefer in Fedora.

> All the example sources are in the directory src.
> At least these sources should be in the documentation.

OK.
Comment 6 Paulo Roma Cavalcanti 2008-10-26 09:33:20 EDT
I would suggest a spec file like this:

Spec URL: http://orion.lcg.ufrj.br/RPMS/SPECS/cddlib.spec

Without "BR: gmp-devel" it does not build in mock...

and for developing with cddlib, gmp-devel is also necessary.
Comment 8 Paulo Roma Cavalcanti 2008-10-28 05:03:58 EDT
Hi,

Some of the source/example files
had the wrong permissions (executable bit on).

rpmlint outputs some warnings:

cddlib-devel.x86_64: W: spurious-executable-perm /usr/share/doc/cddlib-devel-094f/src-mathlink2/mathlink.h
cddlib-devel.x86_64: W: spurious-executable-perm /usr/share/doc/cddlib-devel-094f/src-mathlink/mathlink.h

.....

Specifying 0644 for files in defattr will fix the issue.
This package does not have any executable file, anyway. 
Even if it had, you could an extra defattr or a 
per file permission change. 

%files devel
%defattr(0644,root,root,0755)
%doc doc/cddlibman.pdf
%doc examples* src*
%{_includedir}/*.h
%{_libdir}/libcdd.a
%{_libdir}/libcddgmp.a
Comment 9 Conrad Meyer 2008-10-28 11:52:42 EDT
.a libs are supposed to be +x, I think this problem is easier to fix in prep stage. New URLs:
http://konradm.fedorapeople.org/fedora/SPECS/cddlib.spec
http://konradm.fedorapeople.org/fedora/SRPMS/cddlib-094f-3.fc9.src.rpm
Comment 10 Paulo Roma Cavalcanti 2008-10-28 12:52:38 EDT
(In reply to comment #9)
> .a libs are supposed to be +x, I think this problem is easier to fix in prep
> stage. New URLs:
> http://konradm.fedorapeople.org/fedora/SPECS/cddlib.spec
> http://konradm.fedorapeople.org/fedora/SRPMS/cddlib-094f-3.fc9.src.rpm

Not really. .a libs, generally, are 644 (ls -al /usr/lib64/*.a or /usr/lib/*.a)

But your way works, and you also removed the .* files, which was really good.

I am going to give you a practice review, which is NOT an official review.


------------------------------------------------------------------------


MUST: rpmlint must be run on every package. The output should be posted in the
review.

[lua:~/rhrpms] rpmlint cddlib-devel-094f-3.fc8.x86_64.rpm
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

Clean.

- MUST: The package must be named according to the Package Naming Guidelines .

OK.

- MUST: The spec file name must match the base package %{name}, in the format
%{name}.spec unless your package has an exemption on Package Naming Guidelines
.

OK.

- MUST: The package must meet the Packaging Guidelines .

OK.

- MUST: The package must be licensed with a Fedora approved license and meet
the Licensing Guidelines .

OK.

- MUST: The License field in the package spec file must match the actual
license.

OK.

- MUST: If (and only if) the source package includes the text of the license(s)
in its own file, then that file, containing the text of the license(s) for the
package must be included in %doc.

OK.

- MUST: The spec file must be written in American English.

OK.

- MUST: The spec file for the package MUST be legible. If the reviewer is
unable to read the spec file, it will be impossible to perform a review. Fedora
is not the place for entries into the Obfuscated Code Contest
(http://www.ioccc.org/).

OK.

- MUST: The sources used to build the package must match the upstream source,
as provided in the spec URL. Reviewers should use md5sum for this task. If no
upstream URL can be specified for this package, please see the Source URL
Guidelines for how to deal with this.

OK.

- MUST: The package must successfully compile and build into binary rpms on at
least one supported architecture.

Builds on F-8 x86_64 and F-9 i386 (using mock).

- MUST: If the package does not successfully compile, build or work on an
architecture, then those architectures should be listed in the spec in
ExcludeArch. Each architecture listed in ExcludeArch needs to have a bug filed
in bugzilla, describing the reason that the package does not compile/build/work
on that architecture. The bug number should then be placed in a comment, next
to the corresponding ExcludeArch line. New packages will not have bugzilla
entries during the review process, so they should put this description in the
comment until the package is approved, then file the bugzilla entry, and
replace the long explanation with the bug number. The bug should be marked as
blocking one (or more) of the following bugs to simplify tracking such issues:
FE-ExcludeArch-x86 , FE-ExcludeArch-x64 , FE-ExcludeArch-ppc ,
FE-ExcludeArch-ppc64

NA.

- MUST: All build dependencies must be listed in BuildRequires, except for any
that are listed in the exceptions section of the Packaging Guidelines ;
inclusion of those as BuildRequires is optional. Apply common sense.

OK.

- MUST: The spec file MUST handle locales properly. This is done by using the
%find_lang macro. Using %{_datadir}/locale/* is strictly forbidden.

NA.

- MUST: Every binary RPM package which stores shared library files (not just
symlinks) in any of the dynamic linker's default paths, must call ldconfig in
%post and %postun. If the package has multiple subpackages with libraries, each
subpackage should also have a %post/%postun section that calls /sbin/ldconfig.
An example of the correct syntax for this is:

%post -p /sbin/ldconfig

%postun -p /sbin/ldconfig


NA.

- MUST: If the package is designed to be relocatable, the packager must state
this fact in the request for review, along with the rationalization for
relocation of that specific package. Without this, use of Prefix: /usr is
considered a blocker.

NA.

- MUST: A package must own all directories that it creates. If it does not
create a directory that it uses, then it should require a package which does
create that directory. Refer to the Guidelines for examples.

OK.

- MUST: A package must not contain any duplicate files in the %files listing.

OK.

- MUST: Permissions on files must be set properly. Executables should be set
with executable permissions, for example. Every %files section must include a
%defattr(...) line.

OK.

- MUST: Each package must have a %clean section, which contains rm -rf
%{buildroot} ( or $RPM_BUILD_ROOT ).

OK.

- MUST: Each package must consistently use macros, as described in the macros
section of Packaging Guidelines .

OK.

- MUST: The package must contain code, or permissable content. This is
described in detail in the code vs. content section of Packaging Guidelines .

OK.

- MUST: Large documentation files should go in a -doc subpackage. (The
definition of large is left up to the packager's best judgement, but is not
restricted to size. Large can refer to either size or quantity)

Maybe. There is a considerable amount of files.

- MUST: If a package includes something as %doc, it must not affect the runtime
of the application. To summarize: If it is in %doc, the program must run
properly if it is not present.

OK.

- MUST: Header files must be in a -devel package.

OK.

- MUST: Static libraries must be in a -static package.

The package contains basically two static libraries.
The spec has a "Provides: %{name}-static = %{version}-%{release}" clause.
This is fine for me.

- MUST: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig'
(for directory ownership and usability).

NA.

- MUST: If a package contains library files with a suffix (e.g. libfoo.so.1.1),
then library files that end in .so (without suffix) must go in a -devel
package.

NA.

- MUST: In the vast majority of cases, devel packages must require the base
package using a fully versioned dependency: Requires: %{name} =
%{version}-%{release}

NA.

- MUST: Packages must NOT contain any .la libtool archives, these should be
removed in the spec.

NA.

- MUST: Packages containing GUI applications must include a %{name}.desktop
file, and that file must be properly installed with desktop-file-install in the
%install section. This is described in detail in the desktop files section of
the Packaging Guidelines . If you feel that your packaged GUI application does
not need a .desktop file, you must put a comment in the spec file with your
explanation.

NA.

- MUST: Packages must not own files or directories already owned by other
packages. The rule of thumb here is that the first package to be installed
should own the files or directories that other packages may rely upon. This
means, for example, that no package in Fedora should ever share ownership with
any of the files or directories owned by the filesystem or man package. If you
feel that you have a good reason to own a file or directory that another
package owns, then please present that at package review time.

OK.

- MUST: At the beginning of %install, each package MUST run rm -rf %{buildroot}
( or $RPM_BUILD_ROOT ). See Prepping BuildRoot For %install for details.

OK.


Summary:  the documentation contains several examples, which could be in a 
separate doc package, for clarity. See item above. But this is my personal opinion.
Comment 11 Conrad Meyer 2008-10-28 13:36:27 EDT
(In reply to comment #10)
> Not really. .a libs, generally, are 644 (ls -al /usr/lib64/*.a or /usr/lib/*.a)

Ah, when I checked a random lib I must have found a lib that was 755. My mistake! I am glad this way works ok also. I looked at the examples and source and they are only a few megs uncompressed and probably compress well, so I don't think it's a big deal having it all in one package. Besides, to use the library (i.e. link against it, it's static) you probably want the documentation anyway.

In the future doing reviews you might just omit the stuff that's OK or N/A and just tell me the stuff I'm doing wrong :D. But I appreciate the (practice) review.
Comment 12 Jon Ciesla 2008-10-28 13:52:22 EDT
rpmlint not clean for me:
cddlib-debuginfo.i386: E: empty-debuginfo-package
This debuginfo package contains no files.  This is often a sign of binaries
being unexpectedly stripped too early during the build, rpmbuild not being
able to strip the binaries, the package actually being a noarch one but
erratically packaged as arch dependent, or something else.  Verify what the
case is, and if there's no way to produce useful debuginfo out of it, disable
creation of the debuginfo package.

The file:
examples-ml/Combinatorica5.m
Cannot be included in Fedora cue to the following licensing clause:

This package may be copied in its entirety for nonprofit purposes only.
Sale, other than for the direct cost of the media, is prohibited.  This
copyright notice must accompany all copies.

Paulo, grep -i license * and */* and */*/* . . .etc will help you find this sort of thing.

Why -devel and not -static, as there are no solibs?

I'm ok with the docs/examples being bundled, they're not that big, left to packager discretion IMHO.

Otherwise, a good review.  Checking mock BRs, will be slightly delayed there due to system issues. :(
Comment 13 Conrad Meyer 2008-10-28 15:12:35 EDT
(In reply to comment #12)
> cddlib-debuginfo.i386: E: empty-debuginfo-package

http://fedoraproject.org/wiki/Packaging/Debuginfo suggests I should look for -s being passed to gcc or ld, or strip being called somewhere. I'm now building in mock to be able to grep over a build.log for these.

> The file:
> examples-ml/Combinatorica5.m
> Cannot be included in Fedora cue to the following licensing clause:
> 
> This package may be copied in its entirety for nonprofit purposes only.
> Sale, other than for the direct cost of the media, is prohibited.  This
> copyright notice must accompany all copies.

Good spot. Will rm in prep.

> Why -devel and not -static, as there are no solibs?

"2. Static libraries only.
When a package only provides static libraries you can place all the static library files in the *-devel subpackage. When doing this you also must have a virtual Provide for the *-static package." from http://fedoraproject.org/wiki/Packaging/Guidelines#Packaging_Static_Libraries

My personal reason? There are header files too, therefore it's useful for devel.

> I'm ok with the docs/examples being bundled, they're not that big, left to
> packager discretion IMHO.
> 
> Otherwise, a good review.  Checking mock BRs, will be slightly delayed there
> due to system issues. :(

Thanks for helping Paulo out. I appreciate the input from both of you!
Comment 14 Conrad Meyer 2008-10-28 15:27:37 EDT
Mock build.log on my machine: http://pastie.caboo.se/302506

I don't see any gcc -s or strip commands being run. However, I think mock/rpmbuild might be using *my* CFLAGS which differ from %{optflags}, so I'm exporting CFLAGS="%{optflags}" in %build now and seeing if that yields a non-empty debuginfo package.
Comment 15 Conrad Meyer 2008-10-28 15:28:12 EDT
(In reply to comment #14)
> Mock build.log on my machine: http://pastie.caboo.se/302506
> 
> I don't see any gcc -s or strip commands being run. However, I think
> mock/rpmbuild might be using *my* CFLAGS which differ from %{optflags}, so I'm
> exporting CFLAGS="%{optflags}" in %build now and seeing if that yields a
> non-empty debuginfo package.

Nope, still empty. I guess that means I should disable it?
Comment 16 Jon Ciesla 2008-10-28 16:13:05 EDT
>> The file:
>> examples-ml/Combinatorica5.m
>> Cannot be included in Fedora cue to the following licensing clause:
>> 
>> This package may be copied in its entirety for nonprofit purposes only.
>> Sale, other than for the direct cost of the media, is prohibited.  This
>> copyright notice must accompany all copies.
>
>Good spot. Will rm in prep.

No, you need a modified tarball, so it doesn't wind up in the SRPM.
See:https://fedoraproject.org/wiki/Packaging/SourceURL#When_Upstream_uses_Prohibited_Code

Yeah, probably disable.  And your -static/-devel explanation is IMHO sound.
Comment 17 Conrad Meyer 2008-10-28 16:33:16 EDT
(In reply to comment #16)
> >> The file:
> >> examples-ml/Combinatorica5.m
> >> Cannot be included in Fedora cue to the following licensing clause:
> >> 
> >> This package may be copied in its entirety for nonprofit purposes only.
> >> Sale, other than for the direct cost of the media, is prohibited.  This
> >> copyright notice must accompany all copies.
> >
> >Good spot. Will rm in prep.
> 
> No, you need a modified tarball, so it doesn't wind up in the SRPM.
> See:https://fedoraproject.org/wiki/Packaging/SourceURL#When_Upstream_uses_Prohibited_Code

That links says "Some upstream packages include patents or trademarks that we are not allowed to ship even as source code." It does not say we need to remove improperly licensed files from the source tarball.

> Yeah, probably disable.  And your -static/-devel explanation is IMHO sound.

Alright.
Comment 19 Jon Ciesla 2008-10-29 08:20:53 EDT
(In reply to comment #17)
> (In reply to comment #16)
> > >> The file:
> > >> examples-ml/Combinatorica5.m
> > >> Cannot be included in Fedora cue to the following licensing clause:
> > >> 
> > >> This package may be copied in its entirety for nonprofit purposes only.
> > >> Sale, other than for the direct cost of the media, is prohibited.  This
> > >> copyright notice must accompany all copies.
> > >
> > >Good spot. Will rm in prep.
> > 
> > No, you need a modified tarball, so it doesn't wind up in the SRPM.
> > See:https://fedoraproject.org/wiki/Packaging/SourceURL#When_Upstream_uses_Prohibited_Code
> 
> That links says "Some upstream packages include patents or trademarks that we
> are not allowed to ship even as source code." It does not say we need to remove
> improperly licensed files from the source tarball.

Read the next sentence in that paragraph: 

"In these cases you have to modify the source tarball to remove this code before you even upload it to the build system."

> > Yeah, probably disable.  And your -static/-devel explanation is IMHO sound.
> 
> Alright.
Comment 20 Conrad Meyer 2008-10-29 11:39:32 EDT
(In reply to comment #19)
> Read the next sentence in that paragraph: 
> 
> "In these cases you have to modify the source tarball to remove this code
> before you even upload it to the build system."

Yes, my point being the quoted sentence only applies if the first statement is true, which it is not. There is nothing patented or trademarked about that one file, it is simply improperly licensed. We can delete it and be on our merry way.
Comment 21 Jon Ciesla 2008-10-29 11:48:01 EDT
And if it's not removed from the tarball, it's distributed in the SRPM, which is a no-no.  Blocking FE-LEGAL for confirmation.
Comment 22 Conrad Meyer 2008-10-29 11:55:44 EDT
Ok.
Comment 23 Tom "spot" Callaway 2008-10-30 17:53:39 EDT
This is correct. That license text says that you cannot distribute it for profit, which would mean that someone putting the SRPM on a CD couldn't resel the CD to cover their costs because that file is in the original tarball, even if we patch it out or immediately delete it.

I know it is a pain, but you must make a modified tarball that doesn't have any bits under non-commercial licenses.

You should probably point this out to upstream and recommend that they pull this code out of their tarball, and offer it separately on their website instead.
Comment 25 Jon Ciesla 2008-10-31 11:25:10 EDT
Good, but it'd be best to comment on exactly how one may create the modified tarball from upstream, and not have the URL in the tag.  i.e.

Change:

Source0:        ftp://ftp.ifor.math.ethz.ch/pub/fukuda/cdd/%{name}-094f.tar.gz

to:

#Source0:        ftp://ftp.ifor.math.ethz.ch/pub/fukuda/cdd/%{name}-094f.tar.gz
#tar -xzf cddlib-094f.tar.gz
#rm foo
#tar -czf cddlib-094f/ cddlib-094f.tar.gz
Source0: cddlib-094f.tar.gz

This way someone testing md5sum will understand. . .
Comment 26 Dominik 'Rathann' Mierzejewski 2008-10-31 13:56:34 EDT
Or even rename the tarball to cddlib-094f-free.tar.gz.
Comment 27 Jon Ciesla 2008-10-31 14:03:13 EDT
(In reply to comment #26)
> Or even rename the tarball to cddlib-094f-free.tar.gz.

Which would go even farther in clarifying the issue.  Good idea.
Comment 29 Conrad Meyer 2008-11-05 20:35:38 EST
Any new comments?
Comment 30 Jon Ciesla 2008-11-06 08:50:06 EST
Paulo, were you doing to take this one and to the formal review now that you're sponsored, or shall I?

Conrad, if he doesn't want to or can't soon, I'll do it.
Comment 31 Paulo Roma Cavalcanti 2008-11-06 09:40:44 EST
Hi, Jon

I can finish the review. In fact, the only problem was the legal issued
you raised and Spot clarified for us.

I'll finish the review tonight.
Comment 32 Tom "spot" Callaway 2008-11-06 11:30:06 EST
Lifting FE-Legal, as this is now okay with the stripped tarball.
Comment 33 Paulo Roma Cavalcanti 2008-11-06 18:09:42 EST
I extracted the files from the .src.rpm:

rpm2cpio ../cddlib-094f-6.fc9.src.rpm | cpio -ivmud
cddlib-094f-free.tar.gz
cddlib.spec
2328 blocks

[cascavel:~/temp/temp] tar -ztvf cddlib-094f-free.tar.gz

gzip: stdin: not in gzip format
tar: Child returned status 1
tar: Error exit delayed from previous errors

Although the file has an extension .tar.gz it seems to be just an uncompressed 
tar archive (.tar).

Anyway, the file with the license issue has been removed.

[cascavel:~/temp/temp] tar -tvf cddlib-094f-free.tar.gz | grep Combinatorica5.m
[cascavel:~/temp/temp] 


and the debug package generation has been disabled.

Please, just check the .tar.gz file creation and the package will be ready.
Comment 34 Conrad Meyer 2008-11-06 18:29:37 EST
It's actually a .tar.bz2. I've renamed the SOURCE file and await your approval.
Comment 35 Paulo Roma Cavalcanti 2008-11-06 18:36:05 EST
Oh, I see. That is right, it is a tar.bz2

Did you update the links?

Also, the spec file should change

Source0:        %{name}-094f-free.tar.gz

for

Source0:        %{name}-094f-free.tar.bz2
Comment 36 Conrad Meyer 2008-11-06 19:00:58 EST
Yes, I fixed the spec file. No, I didn't bother uploading the changes to the same place because it is trivial. When you set "fedora-review" to "+" I can start getting it imported into Fedora.
Comment 37 Paulo Roma Cavalcanti 2008-11-06 19:05:32 EST
Approved.
Comment 38 Conrad Meyer 2008-11-06 19:17:13 EST
Thanks.

New Package CVS Request
=======================
Package Name: cddlib
Short Description: A library for generating all vertices in convex polyhedrons
Owners: konradm
Branches: F-10 F-9
InitialCC:
Comment 39 Dennis Gilmore 2008-11-06 21:25:28 EST
CVS Done
Comment 40 Conrad Meyer 2008-11-07 00:17:01 EST
**** Access denied: konradm is not in ACL for rpms/cddlib/devel
cvs commit: Pre-commit check failed


Hm?
Comment 41 Conrad Meyer 2008-11-28 20:45:50 EST
Apparently CVS issue was resolved. Built in rawhide: http://koji.fedoraproject.org/koji/taskinfo?taskID=957127 and closing.
Comment 42 Nicolas Chauvet (kwizart) 2008-11-28 21:09:11 EST
I haven't seen discussed why upstream only provides a static library instead of a Shared Object ?
Which Package will use this one (does a runtime test was done) ?
There is no needs to export CFLAGS before the %configure macro (as already done within the %configure).
Headers got installed in /usr/include which is the standard path. Does someone checked if there is possible namespace issue ? 
Also, headers should have been installed using install -p to prevent timestramp changes.

Just quick notes...
Comment 43 Conrad Meyer 2008-11-28 21:18:14 EST
I don't know why upstream chooses static v. shared. No packages depend on this one *yet*, though sage will when it is packaged.

The other issues will be addressed in the next update.
Comment 44 Paulo Roma Cavalcanti 2008-11-29 04:42:56 EST
(In reply to comment #42)
> I haven't seen discussed why upstream only provides a static library instead of
> a Shared Object ?

Some people still prefer static libraries.

> Which Package will use this one (does a runtime test was done) ?

All the examples have been run.

> There is no needs to export CFLAGS before the %configure macro (as already done
> within the %configure).

Agreed, but this is just redundant.

> Headers got installed in /usr/include which is the standard path. Does someone
> checked if there is possible namespace issue ? 

All the identifiers in the includes have prefixes, such as dd_ , ddd_ , ddf_ 
Very unlike they will conflict with anything else.


> Also, headers should have been installed using install -p to prevent timestramp
> changes.
>

What headers?
Comment 45 Conrad Meyer 2008-12-17 06:32:43 EST
Can we close this?
Comment 46 Paulo Roma Cavalcanti 2008-12-18 20:35:43 EST
For me, you can review the export CFLAGS,
and build the package.
Comment 47 Nicolas Chauvet (kwizart) 2009-01-22 10:26:21 EST
Using static libraries instead of shared libraries isn't a matter of preference.
Upstream should be educated to use shared library. Unless there is a very imperial technical reason (circle dependency, ABI un-stability, etc).

This package shouldn't make use of a static library instead of creating a shared object.


Futhermore src-mathlink and src-mathlink2 aren't compiled


Again, there is no reason for such package to install theses headers in /usr/include instead of /usr/include/cddlib to prevent namespace issues.
Also, Adding pkgconfig support should have been done to ease the -I/usr/include/cddlib addition.

cddlibman.pdf should have been generated from the .tex file.

How do you run the example ?
Comment 48 Conrad Meyer 2009-01-22 16:08:11 EST
(In reply to comment #47)
> Using static libraries instead of shared libraries isn't a matter of
> preference.
> Upstream should be educated to use shared library. Unless there is a very
> imperial technical reason (circle dependency, ABI un-stability, etc).
> 
> This package shouldn't make use of a static library instead of creating a
> shared object.

The last update from upstream was in early 2008 and the email address bounces.

> Futhermore src-mathlink and src-mathlink2 aren't compiled

Are these useful?

> Again, there is no reason for such package to install theses headers in
> /usr/include instead of /usr/include/cddlib to prevent namespace issues.

Agreed.

> Also, Adding pkgconfig support should have been done to ease the
> -I/usr/include/cddlib addition.

Unneeded work considering how many dependencies this library will have.

> cddlibman.pdf should have been generated from the .tex file.

Done.

> How do you run the example ?

$ gcc /usr/share/doc/cddlib-devel-094f/src/testcdd1.c -I/usr/include/cddlib -lcdd
$ ./a.out

When it asks you for a file, pass one of the files in /usr/share/doc/cddlib-devel-094f/examples/*.ine.
Comment 49 Kevin Kofler 2009-01-22 23:07:10 EST
The mathlink stuff is interfacing code to Mathematica. I don't know if mathlink is Free Software, Mathematica definitely isn't. :-) In any case, that code can't be built without mathlink.
Comment 50 Nicolas Chauvet (kwizart) 2009-04-14 10:45:00 EDT
Any other improvement ?
If this package isn't usable, it "have to" be removed from the Fedora package collection.
Comment 51 Conrad Meyer 2009-04-14 11:04:33 EDT
Uh, what? It's quite usable.
Comment 52 Nicolas Chauvet (kwizart) 2009-04-14 11:14:16 EDT
so, which package is using cddlib-devel then ?
Comment 53 Conrad Meyer 2009-04-14 11:26:35 EDT
Sage, which hasn't been admitted into Fedora yet. It's perfectly fine for libraries to be packaged for Fedora even if there is no software in Fedora uses them (yet).
Comment 54 Nicolas Chauvet (kwizart) 2009-04-14 11:42:48 EDT
(In reply to comment #53)
> Sage, which hasn't been admitted into Fedora yet. It's perfectly fine for
> libraries to be packaged for Fedora even if there is no software in Fedora uses
> them (yet).  
yep, even if said package itself is (for whatever reason) forbidden in fedora, there is a need to be assured that cddlib package itself works.

So how can I test that sage works against the current fedora cddlib package ?
even if Sage is at preliminary packaging step ...

BTW: I guess you didn't meant https://bugzilla.redhat.com/show_bug.cgi?id=198834
Comment 55 Conrad Meyer 2009-04-14 15:10:45 EDT
(In reply to comment #54)
> yep, even if said package itself is (for whatever reason) forbidden in fedora,
> there is a need to be assured that cddlib package itself works.
> 
> So how can I test that sage works against the current fedora cddlib package ?
> even if Sage is at preliminary packaging step ...

It doesn't yet.

> BTW: I guess you didn't meant
> https://bugzilla.redhat.com/show_bug.cgi?id=198834  

No. https://fedoraproject.org/wiki/SIGs/SciTech/SAGE
Comment 56 Nicolas Chauvet (kwizart) 2009-04-17 06:24:43 EDT
Anyway, I'm only interested in having this bug closed properly.
So... good luck for the next steps.
Comment 57 Nicolas Chauvet (kwizart) 2009-04-24 18:40:46 EDT
Shall we close this bug now that it is imported ?
Comment 58 Conrad Meyer 2009-04-24 19:05:08 EDT
That is my thought. Closing.
Comment 59 Mark Chappell 2010-03-17 04:08:13 EDT
Package Change Request
======================
Package Name: cddlib
New Branches: EL-5
Owners: tremble

Fedora owner not interested in EL:
https://bugzilla.redhat.com/show_bug.cgi?id=574082
Comment 60 Kevin Fenzi 2010-03-17 14:02:17 EDT
cvs done.