[Bug 151] Review Request: systemc - Design and verification language for Hardware

[RPM Fusion Bugzilla]

http://bugzilla.rpmfusion.org/show_bug.cgi?id=151





--- Comment #6 from Orcan Ogetbil <orcanbahri at yahoo.com>  2008-11-30 01:54:41 ---
OK, I did a full review on this package (except, I still have to read the
license file)
(In reply to comment #5)
> (In reply to comment #4)
> 
> > -Why don't you use %{_usr} for prefix? Or just use %configure and it will take
> > care of it for you.
> 
> In this context, please do not confuse prefix with exec_prefix
> Here prefix is used as an install base location
> 
> It is not wise to opt another install base location as:
> - it will conflict with the whole set of fedora/rpmfusion packages
> 
> 
> > -I see you defining the $DESTDIR. But there is no reference to the $DESTDIR
> > anywhere in the Makefiles or the code.
> 
> Removed

* Then I think we should follow the gcc scheme rather than creating a systemc
directory directly inside /usr . gcc is installed in the path:
   /usr/lib/gcc/x86_64-redhat-linux/4.3.2/
I don't like putting %{buildroot} into the prefix. You can use a
   sed 's|\$(prefix)|\$(DESTDIR)/\$(prefix)|g'
on the Makefile* and then use the DESTDIR trick you just removed. e.g.
   %configure --prefix=%{_libdir}/%{name}/%{version}
(or something similar) and then
   %{__make} INSTALL="install -p" DESTDIR=%{buildroot} install

* I also wonder why you don't use
   gmake install

> 
> > -There are bunch of binary files in the source tarball. For instance, the
> > .DS_Store files. You should remove all of them during %prep
> 
> 
> done 
> 

* There are more precompiled binary files. e.g. .ncb files
Please run something like 
   for i in $(rpm -qpl systemc*.rpm); do file $i; done
to check for filetypes in an rpm.

And this is the rest of my review:

* Do we need to package the .vcproj files? They are for use of MS' visualC.

* The files README and doc/README are different. You are overwriting one the
way you package the doc files. (Maybe you should use docs instead of docs/* in
%doc or just rename one of these README files)

* This one is for making the SPEC file plainer: Check 
  rpm --eval %{optflags}
to see what flags need to be there for compilation. You are removing some of
these flags from the configure script. Removing them is unnecessary.

? The debuginfo package is empty?

* I am worried about the Requires. You are building this with g++34. Don't you
think we need to require g++34 for this package? If someone writes a code with
this library he needs to compile the code with g++34, right? And what about
other (possible) dependencies?


-- 
Configure bugmail: http://bugzilla.rpmfusion.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.
You are the assignee for the bug.