[Bug 151] Review Request: systemc - Design and verification language for Hardware
http://bugzilla.rpmfusion.org/show_bug.cgi?id=151
--- Comment #6 from Orcan Ogetbil <orcanbahri(a)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.