[Bug 2101] Review request: vbam - High compatibility Gameboy Advance Emulator combining many VBA developments

Sun Jan 8 13:31:02 CET 2012

https://bugzilla.rpmfusion.org/show_bug.cgi?id=2101

--- Comment #6 from Andrea Musuruane <musuruan at gmail.com> 2012-01-08 13:31:02 CET ---
Every time you modify the package you have to include in the post:
a) the URL of the updated spec file
b) the URL of the updated SRPM (the release tag must be increased)

(In reply to comment #5)
> Thanks a lot for your help! I've make as many changes as I can (and re-uploaded
> it) but I still have some questions/issues if you or some else can help:
> 
> -Just for clarification, is the group tag optional or unnecessary?

The group tag is deprecated in Fedora.

> -I guess I should submit fex for packaging in fedora? Assuming I don't enable
> rar, I'm pretty sure it should be LGPL with no fedora issues.

Yes and you'll have to patch vbam to use the system fex instead of the bundled
one.

> -Using %cmake doesn't seem to work for some odd reason; it gets to 72% and
> exits with an "error 2/error: Bad exit status". I can't seem to find the issue
> though, as it doesn't seem to appear in the output that I can see. I would
> upload the log but I don't know how to make/find it.

The root cause must be found. Without watching the compile output you cannot
see the CFLAGS used and I assure you they are not the Fedora optflags :(

I even tried to do what's in the debian rules:
%cmake . -DCMAKE_C_FLAGS="$(CFLAGS)" \
  -DCMAKE_LD_FLAGS="$(LDFLAGS)" \
  -DCMAKE_CXX_FLAGS="$(CXXFLAGS)" 

But CFLAGS (and not CCFLAGS as you set in the SPEC) and CXXFLAGS are not used.
It is always compiled with "-O3 -Wall -fPIC"

I tried to use the %cmake macro and I removed the %{?_smp_mflags} (a possible
cause of problems).

I got the following error at 25%:
Linking CXX shared library libfex.so
/usr/bin/cmake -E cmake_link_script CMakeFiles/fex.dir/link.txt --verbose=1
/usr/lib64/ccache/c++  -fPIC -O3 -Wall  -Wl,-z,relro  -shared
-Wl,-soname,libfex.so -o libfex.so CMakeFiles/fex.dir/fex/7z_C/7zAlloc.c.o
CMakeFiles/fex.dir/fex/7z_C/7zBuf.c.o CMakeFiles/fex.dir/fex/7z_C/7zCrc.c.o
CMakeFiles/fex.dir/fex/7z_C/7zCrcOpt.c.o
CMakeFiles/fex.dir/fex/7z_C/7zDecode.c.o
CMakeFiles/fex.dir/fex/7z_C/7zExtract.c.o
CMakeFiles/fex.dir/fex/7z_C/7zHeader.c.o CMakeFiles/fex.dir/fex/7z_C/7zIn.c.o
CMakeFiles/fex.dir/fex/7z_C/7zItem.c.o CMakeFiles/fex.dir/fex/7z_C/7zStream.c.o
CMakeFiles/fex.dir/fex/7z_C/Bcj2.c.o CMakeFiles/fex.dir/fex/7z_C/Bra.c.o
CMakeFiles/fex.dir/fex/7z_C/Bra86.c.o CMakeFiles/fex.dir/fex/7z_C/CpuArch.c.o
CMakeFiles/fex.dir/fex/7z_C/LzmaDec.c.o
CMakeFiles/fex.dir/fex/7z_C/Lzma2Dec.c.o
CMakeFiles/fex.dir/fex/fex/Binary_Extractor.cpp.o
CMakeFiles/fex.dir/fex/fex/blargg_common.cpp.o
CMakeFiles/fex.dir/fex/fex/blargg_errors.cpp.o
CMakeFiles/fex.dir/fex/fex/Data_Reader.cpp.o
CMakeFiles/fex.dir/fex/fex/fex.cpp.o
CMakeFiles/fex.dir/fex/fex/File_Extractor.cpp.o
CMakeFiles/fex.dir/fex/fex/Gzip_Extractor.cpp.o
CMakeFiles/fex.dir/fex/fex/Gzip_Reader.cpp.o
CMakeFiles/fex.dir/fex/fex/Rar_Extractor.cpp.o
CMakeFiles/fex.dir/fex/fex/Zip7_Extractor.cpp.o
CMakeFiles/fex.dir/fex/fex/Zip_Extractor.cpp.o
CMakeFiles/fex.dir/fex/fex/Zlib_Inflater.cpp.o 
CMakeFiles/fex.dir/fex/7z_C/7zIn.c.o:(.data+0x0): multiple definition of
`k7zSignature'
CMakeFiles/fex.dir/fex/7z_C/7zHeader.c.o:(.data+0x0): first defined here
CMakeFiles/fex.dir/fex/7z_C/7zItem.c.o: In function `SzCoderInfo_Init':
7zItem.c:(.text+0x0): multiple definition of `SzCoderInfo_Init'
CMakeFiles/fex.dir/fex/7z_C/7zIn.c.o:7zIn.c:(.text+0x0): first defined here
CMakeFiles/fex.dir/fex/7z_C/7zItem.c.o: In function `SzCoderInfo_Free':
7zItem.c:(.text+0x10): multiple definition of `SzCoderInfo_Free'
CMakeFiles/fex.dir/fex/7z_C/7zIn.c.o:7zIn.c:(.text+0x10): first defined here
CMakeFiles/fex.dir/fex/7z_C/7zItem.c.o: In function `SzFolder_Init':
7zItem.c:(.text+0x30): multiple definition of `SzFolder_Init'
CMakeFiles/fex.dir/fex/7z_C/7zIn.c.o:7zIn.c:(.text+0x30): first defined here
CMakeFiles/fex.dir/fex/7z_C/7zItem.c.o: In function
`SzFolder_GetNumOutStreams':
7zItem.c:(.text+0x80): multiple definition of `SzFolder_GetNumOutStreams'
CMakeFiles/fex.dir/fex/7z_C/7zIn.c.o:7zIn.c:(.text+0x100): first defined here
CMakeFiles/fex.dir/fex/7z_C/7zItem.c.o: In function
`SzFolder_FindBindPairForInStream':
7zItem.c:(.text+0xb0): multiple definition of
`SzFolder_FindBindPairForInStream'
CMakeFiles/fex.dir/fex/7z_C/7zIn.c.o:7zIn.c:(.text+0x130): first defined here
CMakeFiles/fex.dir/fex/7z_C/7zItem.c.o: In function `SzFolder_GetUnpackSize':
7zItem.c:(.text+0x100): multiple definition of `SzFolder_GetUnpackSize'
CMakeFiles/fex.dir/fex/7z_C/7zIn.c.o:7zIn.c:(.text+0x1d0): first defined here
CMakeFiles/fex.dir/fex/7z_C/7zItem.c.o: In function `SzFile_Init':
7zItem.c:(.text+0x180): multiple definition of `SzFile_Init'
CMakeFiles/fex.dir/fex/7z_C/7zIn.c.o:7zIn.c:(.text+0x230): first defined here
CMakeFiles/fex.dir/fex/7z_C/7zItem.c.o: In function `SzAr_Init':
7zItem.c:(.text+0x1a0): multiple definition of `SzAr_Init'
CMakeFiles/fex.dir/fex/7z_C/7zIn.c.o:7zIn.c:(.text+0x250): first defined here
CMakeFiles/fex.dir/fex/7z_C/7zItem.c.o: In function `SzAr_Free':
7zItem.c:(.text+0x1e0): multiple definition of `SzAr_Free'
CMakeFiles/fex.dir/fex/7z_C/7zIn.c.o:7zIn.c:(.text+0x290): first defined here
collect2: ld returned 1 exit status
make[2]: *** [libfex.so] Error 1
make[2]: Leaving directory `/home/andrea/rpmbuild/BUILD/vbam-1.8.0.1001'
make[1]: *** [CMakeFiles/fex.dir/all] Error 2
make[1]: Leaving directory `/home/andrea/rpmbuild/BUILD/vbam-1.8.0.1001'
make: *** [all] Error 2
errore: Stato d'uscita errato da /var/tmp/rpm-tmp.qxfGk3 (%build)

These problems must be patched (no CFLAGS used and this error). Try to work
with upstream to solve this issues.

> Also a note on the source link: I guess do to this being a pre-release, the
> team doesn't seem to be using a standard download link, plus they seem to be
> changing their organization version to version, but I have made use of name and
> version where ever I could.

It is fine the way you did.

*You can fix the desktop file using desktop-file-install (thus it will also
validate the file in a single command). E.g:

desktop-file-install \
  --remove-category Application \
  --dir %{buildroot}%{_datadir}/applications \
  %{name}.desktop

Check desktop-file-install options: 
man desktop-file-install

You can also look for other examples in Fedora git or RPM Fusion cvs.

* Do not use %{__install} or other program macros. Simply use install. It is
better for readability.

* You are already in %{_builddir}/%{buildsubdir}, so you can strip it

install -p -D -m 0644  debian/%{name}.1 %{buildroot}/%{_mandir}/man1/%{name}.1

* There is no need for this one:
ln -s %{name}.1.gz %{buildroot}/%{_mandir}/man1/g%{name}.1.gz

The man page is of vbam not of gvbam.

* Use this syntax for man pages:
%{_mandir}/man1/%{name}.1*

Thus you don't have to check if they are compressed or not.

* The %find_lang macro is usually called at the end of %install for obvious
reasons.

* A more compact form for this:
%{_datadir}/icons/hicolor/16x16/apps/%{name}.png
%{_datadir}/icons/hicolor/22x22/apps/%{name}.png
%{_datadir}/icons/hicolor/24x24/apps/%{name}.png
%{_datadir}/icons/hicolor/32x32/apps/%{name}.png
%{_datadir}/icons/hicolor/scalable/apps/%{name}.svg

is the following:
%{_datadir}/icons/hicolor/*/apps/%{name}.*

This is just for your knowledge. It is up to you to decide what to use. You
might prefer one form or the other.

* If you use rpmlint on the debug package you'll get:

$ rpmlint -i vbam-debuginfo-1.8.0.1001-1.fc16.x86_64.rpm
vbam-debuginfo.x86_64: E: debuginfo-without-sources
This debuginfo package appears to contain debug symbols but no source files.
This is often a sign of binaries being unexpectedly stripped too early during
the build, or being compiled without compiler debug flags (which again often
is a sign of distro's default compiler flags ignored which might have security
consequences), or other compiler flags which result in rpmbuild's debuginfo
extraction not working as expected.  Verify that the binaries are not
unexpectedly stripped and that the intended compiler flags are used.

This is a problem and it must be solved.

* Use %{buildroot} and %{optflags} or $RPM_BUILD_ROOT and $RPM_OPT_FLAGS. Do
not mix macro styles:
https://fedoraproject.org/wiki/Packaging:Guidelines#Using_.25.7Bbuildroot.7D_and_.25.7Boptflags.7D_vs_.24RPM_BUILD_ROOT_and_.24RPM_OPT_FLAGS

* You should generate two different packages one for gtk and the other for sdl
- so the user will choose the one it prefers.

-- 
Configure bugmail: https://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.