[Bug 967620] Review Request: edelib - Small and portable C++ library for EDE

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



https://bugzilla.redhat.com/show_bug.cgi?id=967620

--- Comment #24 from Michael Schwendt <mschwendt@xxxxxxxxx> ---
Some questions based on another brief look at the new spec file:


> BuildRequires:     libX11

Not libX11-devel?


> %build
> CFLAGS="%{optflags}" CXXFLAGS="%{optflags}" \
> ./configure --enable-shared \
>     --prefix=%{buildroot}%{_prefix} \
>     --libdir=%{buildroot}%{_libdir}

Why isn't %configure used instead?

Could you avoid using %buildroot in this section? Passing %buildroot based
paths to a configure script is a common packaging pitfall, because when paths
are inserted into any built files, they would contain the %buildroot prefix.
Typically, you should not refer to %buildroot before the %install section.

> sed -i 's|%{buildroot}||' *.pc edelib/edelib-config.h

qed

> %install
> jam install

Is "jam install" capable of installing into a buildroot? That would be the
preferred solution.


> %files devel
> %{_libdir}/%{name}/sslib/*.ss

What files are they? When are they needed? At run-time or only during
development?  For example, src/Scheme.cpp refers to these files. There is a
hardcoded path in that file, too, which differs from the location you've
packaged, and it may need further patching for targets where %_lib is not /lib:
/lib/edelib/sslib


> %{_libdir}/%{name}/sslib/*.ss

Two "unowned" directories there:
https://fedoraproject.org/wiki/Packaging:Guidelines#File_and_Directory_Ownership

-- 
You are receiving this mail because:
You are on the CC list for the bug.
Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=KTVVbj0lxY&a=cc_unsubscribe
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review





[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]