|
Description
Alex Lancaster
2009-12-31 08:22:59 CET
(In reply to comment #0) [FWIW: I have a spec for xbmc of my own, but do not indent to publish it for several, different reasons. The comments below are based on what I am observing without having looked into your package.] > Note that XBMC is a big package, the SRPM is 135 MB (although the binary is > only 32 MB). Much of the size stems from upstream including windows and osx binaries. I'd recommend to prune them from their tarball. > 1. The first issue (as revealed in the rpmlint log, below) is that xbmc > installs no-arch independent code in /usr/share/xbmc/. We probably need s/need/MUST/ No go without having this fixed. > to > work with upstream to see how best to relocate these binaries to /usr/lib{64}/. I have a patch addressing this issue. But am not yet sure my patch is complete, because there are several places in xbmc which have /usr/share hard-wired. > The non-executable script messages result from plugins .py files that are run > from within xbmc, but not by users, I believe. > > # rpmlint xbmc-9.11-3.fc12.src.rpm > xbmc.src: W: no-cleaning-of-buildroot %clean > 1 packages and 0 specfiles checked; 0 errors, 1 warnings. > > Only just noticed this warning after the upload: will fix in next iteration. There are other majors issues I am aware about: * the spyce stuff is outdated and breaks with python-2.6 (Earlier versions of my spec failed to build in mock because of this - I have a patch, I grabbed somewhere on the net.) * parts of xbmc don't acknowledge RPM_OPT_FLAGS * Lack of DESTDIR support. Created attachment 331 [details]
Patch to fix the spyce issues
Created attachment 332 [details]
1st patch to acknowledge libdir/bindir
Created attachment 333 [details]
2nd patch to acknknowledge bindir/libdir
Created attachment 334 [details]
3rd patch to acknowledge bindir/libdir
Created attachment 335 [details]
Bug fix to an apparent bug in xbmc
Thanks for the patches. Have you tried opening up trac tickets with upstream xbmc with any of these issues before? If not, I will start trying to feed them upstream as soon as possible, probably once the package is in a good state. I assume the Debian folks trying to package xbmc have also run into similar issues, so I'll try to ping them too. The more distros make upstream aware of the necessity of using /usr/lib/ hopefully that will encourage them to follow more standard practice on Linux. (In reply to comment #1) > (In reply to comment #0) > > [FWIW: I have a spec for xbmc of my own, but do not indent to publish it for > several, different reasons. The comments below are based on what I am observing > without having looked into your package.] Any reason why you hadn't submitted a package review for XBMC yourself then? > > Note that XBMC is a big package, the SRPM is 135 MB (although the binary is > > only 32 MB). > Much of the size stems from upstream including windows and osx binaries. > I'd recommend to prune them from their tarball. Yes, I had noticed that, although I believe that Fedora prefers pristine tarballs where possible. Of course if any .dlls or binaries are in there, they need to be stripped out in any case since they may not be distributable, so we may as well remove anything that isn't strictly necessary. > > 1. The first issue (as revealed in the rpmlint log, below) is that xbmc > > installs no-arch independent code in /usr/share/xbmc/. We probably need > > s/need/MUST/ > No go without having this fixed. Right, the "probably" was in reference to the fact that it would be good to enlist upstream's advice in fixing this properly, versus only applying patches which at least for me is more risky since I don't know the xbmc codebase that well, not to whether it was necessary for the package review. > > to > > work with upstream to see how best to relocate these binaries to /usr/lib{64}/. > I have a patch addressing this issue. But am not yet sure my patch is complete, > because there are several places in xbmc which have /usr/share hard-wired. Right, and again why probably working with upstream who knows the codebase will probably help out. > There are other majors issues I am aware about: > > * the spyce stuff is outdated and breaks with python-2.6 (Earlier versions of > my spec failed to build in mock because of this - I have a patch, I grabbed > somewhere on the net.) > > * parts of xbmc don't acknowledge RPM_OPT_FLAGS > > * Lack of DESTDIR support. OK, so it seems that your patches fix the DESTDIR issue at least. (In reply to comment #8) > (In reply to comment #1) > > (In reply to comment #0) > > > > [FWIW: I have a spec for xbmc of my own, but do not indent to publish it for > > several, different reasons. The comments below are based on what I am observing > > without having looked into your package.] > > Any reason why you hadn't submitted a package review for XBMC yourself then? As I said, there "several different reasons": 1) I am not sure, I be will using xbmc in longer terms. All I did was to recently pick up Rolf F.'s spec and to try bringing into shape to get an impression about xbmc, myself. 2) Christmas holidays interferred (and still interfer) - I haven't had any chance to submit these patches anywhere, yet. > > > Note that XBMC is a big package, the SRPM is 135 MB (although the binary is > > > only 32 MB). > > Much of the size stems from upstream including windows and osx binaries. > > I'd recommend to prune them from their tarball. > > Yes, I had noticed that, although I believe that Fedora prefers pristine > tarballs where possible. Of course if any .dlls or binaries are in there, they > need to be stripped out in any case since they may not be distributable, so we > may as well remove anything that isn't strictly necessary. Well, what upstream calls "packaging", leaves a lot of room for improvement ;) > > > 1. The first issue (as revealed in the rpmlint log, below) is that xbmc > > > installs no-arch independent code in /usr/share/xbmc/. We probably need > > > > s/need/MUST/ > > No go without having this fixed. > > Right, the "probably" was in reference to the fact that it would be good to > enlist upstream's advice in fixing this properly, versus only applying patches > which at least for me is more risky since I don't know the xbmc codebase that > well, not to whether it was necessary for the package review. Neither do I. It's just that I have been wading though upstream's mess and try to address obvious issues. > > There are other majors issues I am aware about: > > > > * the spyce stuff is outdated and breaks with python-2.6 (Earlier versions of > > my spec failed to build in mock because of this - I have a patch, I grabbed > > somewhere on the net.) IIRC, this issue pops up when building with debug-infos enabled and with libdir fixed. Some of rpm's python-scripts will choke on the scripts, because these are using keywords which are reserved in python-2.6. > > * parts of xbmc don't acknowledge RPM_OPT_FLAGS > > > > * Lack of DESTDIR support. > > OK, so it seems that your patches fix the DESTDIR issue at least. Yes, this was a fallout from addressing the bindir/libdir issue. A new update (SRPM is in the process of being uploaded): * Thu Dec 31 2009 Alex Lancaster <alexlan[AT]fedoraproject org> - 9.11-4 - Patches to fix installation paths to /usr/lib/ (thanks Ralf Corsepius) - Patch to fix other issues: random number library (thanks Ralf Corsepius) Spec file: http://alexlan.fedorapeople.org/rpmfusion/xbmc.spec SRPM: http://alexlan.fedorapeople.org/rpmfusion/xbmc-9.11-4.fc12.src.rpm Quiets rpmlint a bit: # rpmlint xbmc-9.11-4.fc12.x86_64.rpm xbmc.x86_64: E: non-executable-script /usr/lib64/xbmc/system/python/spyce/spyce.py.spyce 0644 /usr/bin/env xbmc.x86_64: E: non-executable-script /usr/lib64/xbmc/system/python/spyce/run_spyceCmd.py 0644 /usr/bin/env xbmc.x86_64: W: devel-file-in-non-devel-package /usr/lib64/xbmc/visualisations/xbmc_vis.h xbmc.x86_64: E: non-executable-script /usr/lib64/xbmc/system/python/spyce/spyceCGI.py 0644 /usr/bin/env xbmc.x86_64: E: non-executable-script /usr/lib64/xbmc/system/python/spyce/spyce.py 0644 /usr/bin/env xbmc.x86_64: E: non-executable-script /usr/lib64/xbmc/system/python/spyce/run_spyceModpy.py 0644 /usr/bin/env xbmc.x86_64: E: non-executable-script /usr/lib64/xbmc/system/python/spyce/verchk.py 0644 /usr/bin/env xbmc.x86_64: E: non-executable-script /usr/lib64/xbmc/system/python/spyce/run_spyceCGI.py 0644 /usr/bin/env xbmc.x86_64: E: non-executable-script /usr/lib64/xbmc/system/python/spyce/spyceCmd.py 0644 /usr/bin/env 1 packages and 0 specfiles checked; 8 errors, 1 warnings. Some further remarks/comments: * Could you explain this fragment from your spec: %ifarch x86_64 CFLAGS="-I/usr/lib64/dbus-1.0/include -I/usr/lib64/glib-2.0/include" make %else AFAICT, dbus is being picked up by the toplevel configure script and propagated correctly to the files trying to access dbus headers => IMO, these CFLAGS are redundant and superfluous. Proposal: Remove these CFLAGS. * Silent make rules are harmful: Some cmake-based parts of the package are compiled silently, hiding away the protentally broken compiler calls they are using underneath [In this case, they indeed are broken - They are passing around bogus defines. I haven't checked yet whether this is causing actual problems.] Proposal: Use "make VERBOSE=1" instead of "make" * Redundant "BR: mysql-libs" BR: mysql-devel should be sufficient. ... more to come ... Good points.
The reason for the ifarch's was a good one when I tried to make xbmc build on Fedora 11, but apparently there's no reason anymore. So:
[rolf@home07 SPECS]$ diff -ruN xbmc.spec.sav xbmc.spec
--- xbmc.spec.sav 2010-01-03 11:00:15.027212350 +0100
+++ xbmc.spec 2010-01-03 15:28:14.446072042 +0100
@@ -48,7 +48,6 @@
BuildRequires: freetype-devel
BuildRequires: libXinerama-devel
BuildRequires: fontconfig-devel
-BuildRequires: mysql-libs
BuildRequires: mysql-devel
BuildRequires: jasper-devel
BuildRequires: faac-devel
@@ -152,18 +151,10 @@
done
%build
-CFLAGS="-I$PWD//xbmc/lib/cximage-6.0/jpeg/"
-
-%ifarch x86_64
-LDIR=lib64
-%else
-LDIR=lib
-%endif
-CFLAGS="-fPIC -I/usr/$LDIR/dbus-1.0/include -I/usr/$LDIR/glib-2.0/include"
-CFLAGS="$CFLAGS -I/usr/include/ffmpeg/"
+CLFAGS="-fPIC -I/usr/include/ffmpeg/"
CXXFLAGS="$CFLAGS"
LDFLAGS="-fPIC"
-LIBS="-L/usr/$LDIR/mysql $LIBS"
+LIBS="-L%{_libdir}/mysql $LIBS"
export CFLAGS
export CXXFLAGS
export LDFLAGS
@@ -174,11 +165,7 @@
export ASFLAGS=-fPIC
%configure --enable-external-libraries --enable-goom
-%ifarch x86_64
-CFLAGS="-I/usr/lib64/dbus-1.0/include -I/usr/lib64/glib-2.0/include" make
-%else
-make
-%endif
+make VERBOSE=1
%install
rm -rf $RPM_BUILD_ROOT
[rolf@home07 SPECS]$
Alex may include this in the next build.
Created attachment 337 [details] Fix to some nasty GCC warning. This patch is the result of tracking down some ugly warnings GCC emits while building the rpms ("transposed arguments" NptStdCTime). Checking upstream, I noticed upstream has applied a very similar patch, but slightely more comprehensive patch to xbmc-trunk (http://xbmc.org/trac/changeset/26191). For reasons, I don't know, they haven't applied it to their 9.11-branch ;) This patch is a partial backport of upstream's patch to trunk to xbmc-9.11. Alex, I read your spec file. Here is my suggestion. Could you plan to separate some binary rpms for xbmc? Look at xbmc on Ubuntu. XBMC are separated more than one debs. Not all features are needed by users. So I think that multi rpm packages depending on features may be the best choice. (In reply to comment #14) > Alex, > > I read your spec file. Here is my suggestion. > > Could you plan to separate some binary rpms for xbmc? Look at xbmc on Ubuntu. > XBMC are separated more than one debs. Not all features are needed by users. So > I think that multi rpm packages depending on features may be the best choice. It would be helpful if you could suggest how to split it up. There can only be one actual executable, but you could split out the skins perhaps, but not much else because most things are compiled into the binary AFAICT. Anyway, I think that could be done later once in the repo, the focus is getting the thing past package review for the moment. New version (-5), SRPM is in the process of being uploaded: Tue Jan 5 2010 Alex Lancaster <alexlan[AT]fedoraproject org> - 9.11-5 - Remove unnecessary BR: mysql-libs - Prune out unneeded stuff from build (thanks Rolf Fokkens) - Remove libraries not compiled (thanks Rolf Fokkens) - Patch to some nasty GCC warnings (thanks Ralf Corsepius) Spec file: http://alexlan.fedorapeople.org/rpmfusion/xbmc.spec SRPM: http://alexlan.fedorapeople.org/rpmfusion/xbmc-9.11-5.fc12.src.rpm Note that Rolf has made a start on pruning out the unneeded libraries from build, ultimately we can adapt this into a script to prune the pristine tarball along the lines of: https://fedoraproject.org/wiki/Packaging:SourceURL#When_Upstream_uses_Prohibited_Code More comments:
* Package contains a patch backup:
/usr/lib64/xbmc/system/python/spyce/spyce.py.spyce
Proposal: Don't use patch backups (I personally consider them to be harmful).
* This file is bogus:
/usr/lib64/xbmc/visualisations/xbmc_vis.h
Proposal: Add
rm -f $RPM_BUILD_ROOT%{_libdir}/xbmc/visualisations/xbmc_vis.h
to %install
(I thought this already had been in Rolf's original *.spec ?)
* Building goom triggers this warning:
...
/builddir/build/BUILD/xbmc-9.11/xbmc/visualizations/Goom/goom2k4-0/missing: Unknown `--run' option
Try `/builddir/build/BUILD/xbmc-9.11/xbmc/visualizations/Goom/goom2k4-0/missing --help' for more information
configure: WARNING: `missing' script is too old or missing
...
The solution to this issue would be to either upgrade/patch xbmc/visualizations/Goom/goom2k4-0/missing to a newer version of "missing" or to extend "bootstrap" to be run inside of xbmc/visualizations/Goom/goom2k4-0 (Dangerous!)
* RPM_OPT_FLAGS are still not acknowledged correctly in all places.
* xbmc's webserver frequently dumps core on x86_64.
No actual surprise, because it's based on GoAhead, a piece of SW which is known not to be 64bit compliant.
I am currently testing a hack to GoAhead with my local rpm and keep you posted.
* xbmc still applies local files from its local copies of zlib.
[I am working on it - Patch to come soon.]
* I am not convinced about Rolf's "win patch".
Instead, I am using this in %prep:
...
# The tarball contains prebuilt binaries
find \( -name '*.so' -o -name '*.DLL' -o -name '*.dll' -o -name '*.lib' -o -name '*.zlib' -o -name '*.obj' -o -name '*.exe' -o -name '*.vis' \) | xargs rm -f
# Force using the system-wide versions
rm -rf lib
...
* Has anybody tested "xbmc-standalone" rsp. the xbmc.desktop?
My attempts were without much success. AFAICT, xbmc-standalone doesn't seem to be applicable on Fedora and needs to be reworked.
* /usr/bin/xbmc contains some magic to produce "verbose core dump logs".
I am not sure, this logic is useful in general and on Fedora in particular (abrt already takes care about it), but I don't have a strong opinion on it.
Created attachment 338 [details]
Patch to make libGoAhead more 64bit compliant
This is before-mentioned libGoAhead patch. It is the result of going after gcc warnings related to int<->pointer casts of variables of different sizes.
(In reply to comment #17) > * Has anybody tested "xbmc-standalone" rsp. the xbmc.desktop? > My attempts were without much success. AFAICT, xbmc-standalone doesn't seem to > be applicable on Fedora and needs to be reworked. Same here. I have never got this working under fedora. > * /usr/bin/xbmc contains some magic to produce "verbose core dump logs". > I am not sure, this logic is useful in general and on Fedora in particular > (abrt already takes care about it), but I don't have a strong opinion on it. When reporting a bug upstream they tend to insist on copies of these logs, so maybe worth keeping it in there? I noticed upstream have been working on porting python 2.6.4 to xbmc. Is anyone working with them to ensure that the external Fedora python will work? Currently if you build with external python (as required by Fedora) then python based plugins (i.e. most plugins) fail to work. It would seem to be a good time to encourage upstream to ensure xbmc will work with Fedora python as the plugins are heavily used. (In reply to comment #19) > (In reply to comment #17) > > * Has anybody tested "xbmc-standalone" rsp. the xbmc.desktop? > > My attempts were without much success. AFAICT, xbmc-standalone doesn't seem to > > be applicable on Fedora and needs to be reworked. > > Same here. I have never got this working under fedora. What do mean "not working"? Does the script not run? I've managed to use xbmc-standalone under fvwm and it does handle mounting usb drives etc. which is what is supposed to do, i.e. it takes over managing mounting in lieu of hal and gnome etc. Are you referring maybe to the /usr/share/xsessions/XBMC.desktop functionality where it launches an entire session from GDM? xbmc-standalone, the binary seems to work at least. Hi, I had already done some work packaging xbmc for my personal use (I had to go back on using external python as plugins don't work, as someone mentioned previously). I do have 1 suggestion though that's rather low priority. on the configure line %configure --enable-external-libraries --enable-goom If you change it to %configure --enable-external-libraries --enable-goom SVN_REV="24886" (For example for release 9.11) It works better with the plugins which need that variable set so they can determine if your version of xbmc can use the plugin or not. Obviously not a big deal seeing as no plugins work at the moment due to python core dumping. Giving the srpm a compile/go now and will feedback anything I find. (In reply to comment #20) > (In reply to comment #19) > > (In reply to comment #17) > > > * Has anybody tested "xbmc-standalone" rsp. the xbmc.desktop? > > > My attempts were without much success. AFAICT, xbmc-standalone doesn't seem to > > > be applicable on Fedora and needs to be reworked. > > > > Same here. I have never got this working under fedora. > > What do mean "not working"? Different kinds of audio related issues. From xbmc issueing "failed to initialize audio device" messages, over "being unable to control audio volume" to always "full volume"/"very low volume". What exactly happens seems to be machine-, account- and xbmc-built/version/patch dependent. AFAIU, xbmc in a standalone-session tries to use alsa => I am likely facing the zoo of problems related to initializing alsa. > Does the script not run? The script tries to launch pulseaudio and then to fallback to "native xbmc" if pulseaudio is not available, using "pulse-session". pulse-session doesn't exist under Fedora => xbmc-standalone always falls back to using "native xbmc". => If we indent to always use pulse-audio, this script needs to be modified. May-be something along the lines of: /usr/bin/pulseaudio --start --log-target=syslog --use-pid-file /usr/bin/xbmc -fs --standalone "$@" /usr/bin/pulseaudio --kill --use-pid-file I am not sure. > Are you referring maybe to the /usr/share/xsessions/XBMC.desktop functionality > where it launches an entire session from GDM? Yes, this is what I am referring to. > xbmc-standalone, the binary > seems to work at least. Well, not for me: Testcase: Reboot into GDM; login into an xbmc-session; try to play an audio file. On one machine this appears to work, on another one I am receiving "failed to initialize audio device" warnings or am unable to control the audio volume. What exactly happens seems to depend upon the account I using to launch such an xbmx-session. On freshly created accounts, I am receiving "faild to initialize audio device", on previously used accounts, I typically end up with sound but without ability to control the audio volume. (In reply to comment #22) > > What do mean "not working"? > Different kinds of audio related issues. > > From xbmc issueing "failed to initialize audio device" messages, over "being > unable to control audio volume" to always "full volume"/"very low volume". > What exactly happens seems to be machine-, account- and > xbmc-built/version/patch dependent. > AFAIU, xbmc in a standalone-session tries to use alsa => I am likely facing the > zoo of problems related to initializing alsa. Yes, it does seem a little flaky, alsa not sure if pulseaudio is working. > > Does the script not run? > The script tries to launch pulseaudio and then to fallback to "native xbmc" if > pulseaudio is not available, using "pulse-session". > > pulse-session doesn't exist under Fedora > => xbmc-standalone always falls back to using "native xbmc". > => If we indent to always use pulse-audio, this script needs to be modified. > May-be something along the lines of: > /usr/bin/pulseaudio --start --log-target=syslog --use-pid-file > /usr/bin/xbmc -fs --standalone "$@" > /usr/bin/pulseaudio --kill --use-pid-file > I am not sure. If you've got some specific patches, sure, let's put them in. > > Are you referring maybe to the /usr/share/xsessions/XBMC.desktop functionality > > where it launches an entire session from GDM? > Yes, this is what I am referring to. > > > xbmc-standalone, the binary > > seems to work at least. > Well, not for me: > > Testcase: Reboot into GDM; login into an xbmc-session; try to play an audio > file. > > On one machine this appears to work, on another one I am receiving "failed to > initialize audio device" warnings or am unable to control the audio volume. > What exactly happens seems to depend upon the account I using to launch such an > xbmx-session. On freshly created accounts, I am receiving "faild to initialize > audio device", on previously used accounts, I typically end up with sound but > without ability to control the audio volume. I haven't tried using the XBMC.xsession feature from GDM to start xbmc-standalone. I simply use a regular GDM login and have it use an ~/.xsession file which starts the fvwm window manager and then start xbmc-standalone on top of that. This is because I switch between apps and starting xbmc-standalone directly from GDM would mean that the whole session would end when xbmc-standalone ends. "Working" for me, simply means that xbmc-standalone works. A lot of these issues are probably upstream issues that need trac bugs, I guess. We can fix what we can as far as packaging goes, especially any Fedora-specific mods, but for general bugs, we will probably have to rely on working with upstream. I have noticed a general dislike of pulseaudio and XBMC on some of the IRC channels and forums, not sure if this is shared by the devs concerned or not. Hi guys, is there an rpm to test it? I have downloaded src.rpm from http://alexlan.fedorapeople.org/rpmfusion/xbmc-9.11-5.fc12.src.rpm but when trying to build rpm there are lots of devel depndencies that I need to install and I would like not to do that if possible. An already build rpm would be really appreciated for testing. Cheers! (In reply to comment #24) > Hi guys, is there an rpm to test it? I have downloaded src.rpm from > http://alexlan.fedorapeople.org/rpmfusion/xbmc-9.11-5.fc12.src.rpm but when > trying to build rpm there are lots of devel depndencies that I need to install > and I would like not to do that if possible. Not at this point, my fedorapeople.org web space quota is already maxed out with the SRPM (~ 135 MB). My advice would be to just install all the deps and built it yourself for the moment. Unfortunately RPM Fusion doesn't have a build system that can handle "scratch" builds the way koji can do in Fedora proper. (In reply to comment #18) > Created an attachment (id=338) [details] > Patch to make libGoAhead more 64bit compliant > > This is before-mentioned libGoAhead patch. It is the result of going after gcc > warnings related to int<->pointer casts of variables of different sizes. Thanks for the patch. Has this been added to upstream trac? Seems like a straight bug fix that should be for all platforms. If not, would you be happy if I submitted it? New version (-6), SRPM is in the process of being uploaded: - Add patch for web server segfaults on 64-bit (thanks Ralf Corsepius) - Drop patch backup for .spyce, causes packaging problems (thanks Ralf Corsepius) - Remove bogus header from install (thanks Ralf Corsepius) - More comprehensive pre-built Win32 binary removal (thanks Ralf Corsepius) - Add SVN_REV to configure line for plugins (thanks Graeme Gillies) Spec file: http://alexlan.fedorapeople.org/rpmfusion/xbmc.spec SRPM: http://alexlan.fedorapeople.org/rpmfusion/xbmc-9.11-6.fc12.src.rpm (In reply to comment #17) > * Package contains a patch backup: > /usr/lib64/xbmc/system/python/spyce/spyce.py.spyce > > Proposal: Don't use patch backups (I personally consider them to be harmful). Removed "-b" for this patch at least. Not sure about removing them all completely at this point. They do serve a role in documenting what the patch is which is helpful when renumbering because they don't require copying separate comments around. > Proposal: Add > rm -f $RPM_BUILD_ROOT%{_libdir}/xbmc/visualisations/xbmc_vis.h > to %install > (I thought this already had been in Rolf's original *.spec ?) Fixed. > * Building goom triggers this warning: > The solution to this issue would be to either upgrade/patch > xbmc/visualizations/Goom/goom2k4-0/missing to a newer version of "missing" or > to extend "bootstrap" to be run inside of xbmc/visualizations/Goom/goom2k4-0 > (Dangerous!) Not sure what the best thing would be to do here. Perhaps upstream can be convinced to fix this. > * RPM_OPT_FLAGS are still not acknowledged correctly in all places. OK, let me know if you have any patches that could fix this. > * xbmc's webserver frequently dumps core on x86_64. > * xbmc still applies local files from its local copies of zlib. > [I am working on it - Patch to come soon.] Applied your patch below. > * I am not convinced about Rolf's "win patch". > Instead, I am using this in %prep: > ... Implemented this in place of Rolf's version, seems to work for me. I plan to add a script with all this logic (removing binaries, unneeded bundled code and any prohibited code) that modifies the upstream tarball and repacks it in the next iteration which will reduce the SRPM size considerably. > * /usr/bin/xbmc contains some magic to produce "verbose core dump logs". > I am not sure, this logic is useful in general and on Fedora in particular > (abrt already takes care about it), but I don't have a strong opinion on it. Does abrt work with filing bugs on bugzillas for with 3rd party repos like RPM Fusion? (In reply to comment #19) > When reporting a bug upstream they tend to insist on copies of these logs, so > maybe worth keeping it in there? I'll leave this in for the moment anyway. > I noticed upstream have been working on porting python 2.6.4 to xbmc. Is anyone > working with them to ensure that the external Fedora python will work? > Currently if you build with external python (as required by Fedora) then python > based plugins (i.e. most plugins) fail to work. It would seem to be a good time > to encourage upstream to ensure xbmc will work with Fedora python as the > plugins are heavily used. I think other distro packagers (Debian etc.) have been leaning on upstream XBMC to fix this, but I don't think it's a high priority for the devs when I've discussed this on IRC. Some don't see the point in distro packages and recommend "XBMC Live", others just don't have time to work on fixing the Python bugs. Obviously I'm sure they'd be happy to take patches that fix this, however I don't have the time to work on the innards of the python-xbmc bindings myself. Was there an upstream trac bug that's dedicated to fixing this, or a forum post that outlines this porting effort that you know about? Re: Python 2.6.4 issue, I found: http://www.xbmc.org/trac/ticket/7754 New version (-7), SRPM is in the process of being uploaded, this adds a script that strips tarball before making SRPM, reducing the SRPM size considerably - Remove bundled copies of libraries, and code that we can't distribute from upstream tarball with script - Drop patch against ffmpeg which we removed from tarball - Trim description Spec file: http://alexlan.fedorapeople.org/rpmfusion/xbmc.spec SRPM: http://alexlan.fedorapeople.org/rpmfusion/xbmc-9.11-7.fc12.src.rpm (In reply to comment #25) > (In reply to comment #24) > > Hi guys, is there an rpm to test it? I have downloaded src.rpm from > > http://alexlan.fedorapeople.org/rpmfusion/xbmc-9.11-5.fc12.src.rpm but when > > trying to build rpm there are lots of devel depndencies that I need to install > > and I would like not to do that if possible. > > Not at this point, my fedorapeople.org web space quota is already maxed out > with the SRPM (~ 135 MB). My advice would be to just install all the deps and > built it yourself for the moment. Unfortunately RPM Fusion doesn't have a > build system that can handle "scratch" builds the way koji can do in Fedora > proper. Now that the SRPM has been trimmed in size a bit to ~80 MB, I have space to upload only the x86_64 build (32 MB): http://alexlan.fedorapeople.org/rpmfusion/xbmc-9.11-7.fc12.x86_64.rpm Valent: please note that there are several known issues: 1. Many, if not most, Python plugins won't work. This is because we compile using our external system Python and using the internal Python support is a no-no, this is a known issue and requires upstream to fix. 2. DVD playback probably won't work. We can't ship the css stuff in RPM Fusion (it's now stripped out of the source) and I'm not sure whether XBMC can detect the presence of the livna dvdcss shared library and use it if it found or whether it needs to be compiled in from the start. For the sake of this(In reply to comment #32) > Now that the SRPM has been trimmed in size a bit to ~80 MB, I have space to > upload only the x86_64 build (32 MB): For the review, you can eventually provide a nosrc.rpm That can be done using that after Source0: NoSource: 0 Quick note: it could be fine to have upstream bug report to be referenced alog with the patch, whenever they are available. DVD works fine over here (Eagles (Hell Freezes Over) and Roger Waters (In The Flesh) anyway): [rolf@home07 new]$ rpm -q xbmc libdvdcss xbmc-9.11-7.fc12.x86_64 libdvdcss-1.2.10-1.x86_64 [rolf@home07 new]$ xbmc libdvdread: Using libdvdcss version 1.2.10 for DVD access libdvdread: Attempting to retrieve all CSS keys libdvdread: This can take a _long_ time, please be patient libdvdread: Get key for /VIDEO_TS/VIDEO_TS.VOB at 0x00000160 libdvdread: Elapsed time 0 libdvdread: Get key for /VIDEO_TS/VTS_01_0.VOB at 0x000037c0 libdvdread: Elapsed time 1 libdvdread: Get key for /VIDEO_TS/VTS_01_1.VOB at 0x000037e0 libdvdread: Elapsed time 0 libdvdread: Get key for /VIDEO_TS/VTS_02_0.VOB at 0x00046f40 libdvdread: Elapsed time 0 libdvdread: Get key for /VIDEO_TS/VTS_02_1.VOB at 0x0007548a libdvdread: Elapsed time 0 libdvdread: Get key for /VIDEO_TS/VTS_03_0.VOB at 0x003d8b80 libdvdread: Elapsed time 0 libdvdread: Get key for /VIDEO_TS/VTS_03_1.VOB at 0x003d8ba0 libdvdread: Elapsed time 0 libdvdread: Found 3 VTS's libdvdread: Elapsed time 1 No accelerated IMDCT transform found Makes sense as I patched the RPM to use rpmfusion libdvdread before this review request was started by Alex. (In reply to comment #34) > DVD works fine over here (Eagles (Hell Freezes Over) and Roger Waters (In The > Flesh) anyway): > > [rolf@home07 new]$ rpm -q xbmc libdvdcss > xbmc-9.11-7.fc12.x86_64 > libdvdcss-1.2.10-1.x86_64 > [rolf@home07 new]$ xbmc > libdvdread: Using libdvdcss version 1.2.10 for DVD access > > libdvdread: Attempting to retrieve all CSS keys > libdvdread: This can take a _long_ time, please be patient > > libdvdread: Get key for /VIDEO_TS/VIDEO_TS.VOB at 0x00000160 > libdvdread: Elapsed time 0 > libdvdread: Get key for /VIDEO_TS/VTS_01_0.VOB at 0x000037c0 > libdvdread: Elapsed time 1 > libdvdread: Get key for /VIDEO_TS/VTS_01_1.VOB at 0x000037e0 > libdvdread: Elapsed time 0 > libdvdread: Get key for /VIDEO_TS/VTS_02_0.VOB at 0x00046f40 > libdvdread: Elapsed time 0 > libdvdread: Get key for /VIDEO_TS/VTS_02_1.VOB at 0x0007548a > libdvdread: Elapsed time 0 > libdvdread: Get key for /VIDEO_TS/VTS_03_0.VOB at 0x003d8b80 > libdvdread: Elapsed time 0 > libdvdread: Get key for /VIDEO_TS/VTS_03_1.VOB at 0x003d8ba0 > libdvdread: Elapsed time 0 > libdvdread: Found 3 VTS's > libdvdread: Elapsed time 1 > No accelerated IMDCT transform found > > Makes sense as I patched the RPM to use rpmfusion libdvdread before this review > request was started by Alex. That's what I was hoping would happen. I just wasn't sure if support could be detected at run-time, so that's good news that it does and therefore we can strip out all libdvdcss from the tarball. (In reply to comment #21) > %configure --enable-external-libraries --enable-goom SVN_REV="24886" > > (For example for release 9.11) I don't understand this. AFAICT, 9.11 is svn revision 26017 on xbmc.org's 9.11_Camelot branch (https://xbmc.svn.sourceforge.net/svnroot/xbmc/branches/9.11_Camelot) (In reply to comment #28) > > * /usr/bin/xbmc contains some magic to produce "verbose core dump logs". > > I am not sure, this logic is useful in general and on Fedora in particular > > (abrt already takes care about it), but I don't have a strong opinion on it. > > Does abrt work with filing bugs on bugzillas for with 3rd party repos like RPM > Fusion? Well, I've seen abrt popping up when xbmc went down. Whether this is usable/useful is a different question. Anyway, so far, I've never experienced abrt doing anything useful anywhere - not even with original Fedora bugzilla.redhat.com hosted Fedora packages. If you really want to keep the wrapper, then * you'd have to introduce at least "Require: gdb" (A silly idea, IMO) and a "Requires: redhat-lsb" (Pulls in a long series of dependencies) or to check for presence of them at runtime. * fix the script into a location which is more likely to be writeable (e.g. $(HOME)) than $(pwd) Finally, I just noticed that xbmc.sh still accesses /usr/share/xbmc/xbmc.bin. Created attachment 345 [details] replacement patch for attachment 334 [details] Changes to 334: - Use hard-coded /usr/bin/gdb instead of plain "gdb", to make sure the "right" gdb is being used. - Replace hardcoded (bogus) /usr/share/xbmc/xbmc.bin with @libdir@/xmbc/xbmc.bin (In reply to comment #36) > (In reply to comment #28) > > Does abrt work with filing bugs on bugzillas for with 3rd party repos like RPM > > Fusion? I asked one of the abrt devs on PM. This is what he replied: >> How does abrt handle segfaults in packages outside of Fedora? >> [...] I have seen it popping up on segfaults in packages >> from 3rd party repos. > > ABRT detects it, creates backtrace, but will fail to report it, > because it doesn't (yet) support 3rd party bug tracking systems. Comment on attachment 345 [details] replacement patch for attachment 334 [details] Fails to apply for me: Patch #9 (xbmc-9.11-xbmc.sh.diff): + /bin/cat /home/alex/rpmbuild/SOURCES/xbmc-9.11-xbmc.sh.diff + /usr/bin/patch -s -p1 -b --suffix .xbmcsh --fuzz=0 1 out of 4 hunks FAILED -- saving rejects to file tools/Linux/xbmc.sh.in.rej error: Bad exit status from /var/tmp/rpm-tmp.gD7HYA (%prep) (In reply to comment #39) > (From update of attachment 345 [details]) > Fails to apply for me: > > Patch #9 (xbmc-9.11-xbmc.sh.diff): > + /bin/cat /home/alex/rpmbuild/SOURCES/xbmc-9.11-xbmc.sh.diff > + /usr/bin/patch -s -p1 -b --suffix .xbmcsh --fuzz=0 > 1 out of 4 hunks FAILED -- saving rejects to file tools/Linux/xbmc.sh.in.rej > error: Bad exit status from /var/tmp/rpm-tmp.gD7HYA (%prep) > OK, this was a result of interference with patch2: xbmc-9.11-a2-xbmc.sh.env.patch Now that I dropped that, it seems to apply cleanly. New version (-8), SRPM is in the process of being uploaded, made a start on contributing upstream tickets, where appropriate: * Thu Jan 21 2010 Alex Lancaster <alexlan[AT]fedoraproject org> - 9.11-8 - Update xbmc script patch (thanks Ralf C) - Reorder patches, add upstream tickets where possible - Drop SVN_REV from line until exact version clarified Spec: http://alexlan.fedorapeople.org/rpmfusion/xbmc.spec SRPM: http://alexlan.fedorapeople.org/rpmfusion/xbmc-9.11-8.fc12.src.rpm OK, time to do a quick patch audit, Rolf and Ralf, could you clarify the following points? Thanks. Rolf: is this necessary, it seems to add a single "-I ../jpeg", I thought that xbmc is using the system libjpeg at this point, should it be submitted upstream? Patch0: xbmc-9.11-a2-libjpeg.patch Rolf: has this been submitted upstream, will this be fixed in the 10.x release? Patch1: xbmc-9.11-a2-gcc440.patch This patch look for libdca, rather than libdts, there is already: http://xbmc.org/trac/ticket/8040 (although patch slightly different to this one) Patch2: xbmc-9.11-b1-config.patch Rolf, is this necessary if we remove dvdcss from source? Patch3: xbmc-9.11-b1-dvdlibs-external.patch This had been submitted upstream: http://xbmc.org/trac/ticket/8026 (should be in next release): Patch4: xbmc-9.11-use_cdio_system_headers_on_non_win32.patch I added next 3 patches to trac: http://xbmc.org/trac/ticket/8590 Patch5: xbmc-9.11-fix-Makefile.in.patch Patch6: xbmc-9.11-Makefile.include.in.diff Patch7: xbmc-9.11-xbmc.sh.diff Rolf: could you clarify the purposes of the next 3 patches be submitted upstream? Remlibs appears to force using external libass: Patch8: xbmc-9.11-spyce.diff Patch9: xbmc-9.11-RandomNumberGenerators.hpp.diff Patch10: xbmc-9.11-remlibs.patch Ralf this fixes GCC warning: partial backport to 9.11, will this be fixed in next release? Do we need to report upstream? Patch11: xbmc-9.11-changeset-26191.diff This fixes problems with web server segfaulting on x86_64, I submitted http://xbmc.org/trac/ticket/8591 Patch12: xbmc-9.11-GoAhead.diff (In reply to comment #42) > OK, time to do a quick patch audit, Rolf and Ralf, could you clarify the > following points? Thanks. For convenience I uploaded all the patches here: http://alexlan.fedorapeople.org/rpmfusion/xbmc-patches/ New binary for x86_64: http://alexlan.fedorapeople.org/rpmfusion/xbmc-9.11-8.fc12.x86_64.rpm (In reply to comment #42) > I added next 3 patches to trac: http://xbmc.org/trac/ticket/8590 Thanks. > Patch8: xbmc-9.11-spyce.diff This fixes python-2.6 syntax errors (the syntax of the sources xbmc has merged into their source-tree is invalid) > Patch9: xbmc-9.11-RandomNumberGenerators.hpp.diff This is a fix to a minor but quite obious bug - Without this patch, the code is more or less useless. > Ralf this fixes GCC warning: partial backport to 9.11, will this be fixed in > next release? Well, much of this patch is from xbxc's svn-trunk. It is not fixed on 9.11_branch. > Do we need to report upstream? I think so. This bug (a memory-leak) is still in 9.11_branch. > Patch11: xbmc-9.11-changeset-26191.diff > > This fixes problems with web server segfaulting on x86_64, I submitted > http://xbmc.org/trac/ticket/8591 > Patch12: xbmc-9.11-GoAhead.diff Thanks - FWIW: I am having some problems in keeping keeping the pace/synch-ing my packages with you, because I have several further patches applied locally and am testing them. (In reply to comment #45) > (In reply to comment #42) > > > > I added next 3 patches to trac: http://xbmc.org/trac/ticket/8590 > Thanks. > > > Patch8: xbmc-9.11-spyce.diff > This fixes python-2.6 syntax errors (the syntax of the sources xbmc has merged > into their source-tree is invalid) OK, is it fixed in trunk? > > Patch9: xbmc-9.11-RandomNumberGenerators.hpp.diff > This is a fix to a minor but quite obious bug - Without this patch, the code is > more or less useless. There are plans to move to libprojectM 2.0.1 in trunk, I think, but I don't think they have as yet: http://xbmc.org/trac/ticket/8277 and ultimately we should be compiling against the external version: http://xbmc.org/trac/ticket/8408 so is it still worth submitting this patch upstream?? > > Ralf this fixes GCC warning: partial backport to 9.11, will this be fixed in > > next release? > Well, much of this patch is from xbxc's svn-trunk. > It is not fixed on 9.11_branch. > > Do we need to report upstream? > I think so. This bug (a memory-leak) is still in 9.11_branch. > > Patch11: xbmc-9.11-changeset-26191.diff OK, but the next 10.x version will presumably be from trunk, so I don't know if there is a point in resubmitting it as a trac ticket upstream, they'd probably just close as being fixed in trunk. I don't think xbmc upstream make point releases based on the branches AFAIK, except for the "Live" version, and in this case, we are effectively "Live". > > This fixes problems with web server segfaulting on x86_64, I submitted > > http://xbmc.org/trac/ticket/8591 > > Patch12: xbmc-9.11-GoAhead.diff This was submitted and they immediately closed it, apparently goahead is about to be replaced: http://xbmc.org/trac/ticket/8591#comment:1 > Thanks - FWIW: I am having some problems in keeping keeping the pace/synch-ing > my packages with you, because I have several further patches applied locally > and am testing them. I'll leave updating the spec for a while to give you a chance to test them. Perhaps you could just make the modifications relative to my last (-8) spec file, update the changelog and then attach it (and any new patches) right to this bug. Then I can roll a new srpm. Also: are you willing to actually review this? Or is there somebody who is, since nobody has yet volunteered. (In reply to comment #46) > (In reply to comment #45) > > (In reply to comment #42) > > > > > > > I added next 3 patches to trac: http://xbmc.org/trac/ticket/8590 > > Thanks. > > > > > Patch8: xbmc-9.11-spyce.diff > > This fixes python-2.6 syntax errors (the syntax of the sources xbmc has merged > > into their source-tree is invalid) > > OK, is it fixed in trunk? Last time I checked, it wasn't fixed - IIRC, it had been reported before and similar patches be proposed before. However, spyce, as many other parts in xbmc, actually is a standalone project with a (seemingly dead) upstream of its own (http://spyce.sourceforge.net). > There are plans to move to libprojectM 2.0.1 in trunk, I think, but I don't > think they have as yet: > > http://xbmc.org/trac/ticket/8277 > > and ultimately we should be compiling against the external version: Yep. I already had considered to propose a projectM1/compat-projectM1 package in fedora, but abandoned this plan ;) > http://xbmc.org/trac/ticket/8408 > > so is it still worth submitting this patch upstream?? I don't know - If can easily be realized, then why not, if not ... no biggy, IMO. > > > Patch11: xbmc-9.11-changeset-26191.diff > > OK, but the next 10.x version will presumably be from trunk, so I don't know if > there is a point in resubmitting it as a trac ticket upstream, they'd probably > just close as being fixed in trunk. I don't think xbmc upstream make point > releases based on the branches AFAIK, except for the "Live" version, and in > this case, we are effectively "Live" Well, ... I think xbmc needs to reconsider their release procedures, but provided what I am observing in trunk, I would expect the worst (e.g. AFAICT, they recently added mingw32 binaries, ...) > > > This fixes problems with web server segfaulting on x86_64, I submitted > > > http://xbmc.org/trac/ticket/8591 > > > Patch12: xbmc-9.11-GoAhead.diff > > This was submitted and they immediately closed it, Ah, OK ... > apparently goahead is about > to be replaced: http://xbmc.org/trac/ticket/8591#comment:1 Ah, ... I noticed somebody recently pointed out to upstream xbmc that GoAhead's license is quite problematic ... > Perhaps you could just make the modifications relative to my last (-8) spec > file, update the changelog and then attach it (and any new patches) right to > this bug. That's what I am already trying ... but working on this package in my spare time, I resync'ing occassionally is tedious. > Also: are you willing to actually review this? Yes, I would, ... but we aren't there yet. Current showstopper: RPM_OPT_FLAGS (This one topic, I am trying to address. So far I've caught much of it, but not yet all - It is the patch which collides with your patches :) ). Also: Has anybody tried to build this beast on ppc-*'s? I would not expect it to build at all. > Or is there somebody who is, > since nobody has yet volunteered. I intentionally did not assign this BZ to me to leave room to others to step in. > OK, time to do a quick patch audit, Rolf and Ralf, could you clarify the > following points? Thanks. > Rolf: is this necessary, it seems to add a single "-I ../jpeg", I thought that > xbmc is using the system libjpeg at this point, should it be submitted > upstream? > Patch0: xbmc-9.11-a2-libjpeg.patch In another galaxy long ago (alpha2? Fedora11?) it had a purpose, but it's no nonger needed. > Rolf: has this been submitted upstream, will this be fixed in the 10.x release? > Patch1: xbmc-9.11-a2-gcc440.patch Somehow we can no do without this one as well, on Fedora 12 anyhow. > This patch look for libdca, rather than libdts, there is already: > http://xbmc.org/trac/ticket/8040 (although patch slightly different to this > one) > Patch2: xbmc-9.11-b1-config.patch I renamed it to xbmc-9.11-libdca.patch. I tried the upstream patch, but it somehow makes make fail on "make -C libdts". > Rolf, is this necessary if we remove dvdcss from source? > Patch3: xbmc-9.11-b1-dvdlibs-external.patch Without this patch configure fails in an attempt to descend to xbmc/cores/dvdplayer/Codecs/libdvd/libdvdcss and xbmc/cores/dvdplayer/Codecs/libdvd/libdvdread which no longer exist. > Rolf: could you clarify the purposes of the next 3 patches be submitted > upstream? Remlibs appears to force using external libass: > Patch8: xbmc-9.11-spyce.diff > Patch9: xbmc-9.11-RandomNumberGenerators.hpp.diff The purposes of the previous patches have been clarified by Ralf > Patch10: xbmc-9.11-remlibs.patch Without this patch config.status attempts to descend to open xbmc/lib/libass/xbmc/Makefile.in which is obsolete (and has been removed) as we use external libass now. I renamed it to xbmc-9.11-remlibass.patch, and removed an irrelevant part. I changed Source0 to be based on xz compression, it reduces the src.rpm size! Both src.rpm and nosrc.rpm can be found at: http://rolffokkens.dyndns.org/xbmc-9.11-9.fc12/ --- xbmc.spec.orig 2010-01-21 08:47:27.000000000 +0100 +++ xbmc.spec 2010-01-21 21:52:56.394548815 +0100 @@ -1,9 +1,10 @@ Name: xbmc Version: 9.11 -Release: 8%{?dist} +Release: 9%{?dist} URL: http://www.xbmc.org/ -Source0: %{name}-%{version}-patched.tar.gz +Source0: %{name}-%{version}-patched.tar.xz +NoSource: 0 # xbmc contains code that we cannot ship, as well as redundant private # copies of upstream libraries that we already distribute. Therefore # we use this script to remove the code before shipping it. @@ -12,16 +13,16 @@ # and invoke this script while in the directory where the tarball is located: # sh xbmc-generate-tarball.sh <version> # where <version> is the particular version being used -Source1: xbmc-generate-tarball.sh +Source1: xbmc-generate-tarball-xz.sh -Patch0: xbmc-9.11-a2-libjpeg.patch +#Patch0: xbmc-9.11-a2-libjpeg.patch # check if this is in SVN -Patch1: xbmc-9.11-a2-gcc440.patch +#Patch1: xbmc-9.11-a2-gcc440.patch # look for libdca, rather than libdts -# http://xbmc.org/trac/ticket/8040 (although patch slightly different to this one) -Patch2: xbmc-9.11-b1-config.patch +# http://xbmc.org/trac/ticket/8040 +Patch2: xbmc-9.11-libdca.patch # is this necessary if we remove dvdcss from source? Patch3: xbmc-9.11-b1-dvdlibs-external.patch @@ -38,7 +39,7 @@ # check to see if the next 3 can be submitted upstream Patch8: xbmc-9.11-spyce.diff Patch9: xbmc-9.11-RandomNumberGenerators.hpp.diff -Patch10: xbmc-9.11-remlibs.patch +Patch10: xbmc-9.11-remlibass.patch # Fixes GCC warning: partial backport to 9.11 # (should be fixed in next release) @@ -128,9 +129,9 @@ %setup -q -n %{name}-%{version} -%patch0 -p1 -b .libjpeg -%patch1 -p1 -b .gcc440 -%patch2 -p1 -b .config +## %patch0 -p1 -b .libjpeg +## %patch1 -p1 -b .gcc440 +%patch2 -p1 -b .dtsdca %patch3 -p1 -b .dvdlibs %patch4 -p1 -b .cdio %patch5 -p0 -b .makefilein @@ -142,7 +143,6 @@ %patch11 -p1 -b .gccwarning %patch12 -p1 -b .goahead - %build CFLAGS="-fPIC -I/usr/include/ffmpeg/" CXXFLAGS="$CFLAGS" @@ -158,7 +158,7 @@ export ASFLAGS=-fPIC %configure --enable-external-libraries --enable-goom -make VERBOSE=1 +make %{?_smp_mflags} VERBOSE=1 %install rm -rf $RPM_BUILD_ROOT (In reply to comment #48) > I changed Source0 to be based on xz compression, it reduces the src.rpm size! > > Both src.rpm and nosrc.rpm can be found at: > http://rolffokkens.dyndns.org/xbmc-9.11-9.fc12/ Rolf: Thanks for the updated srpm and spec. Just one small request: could you also update the %changelog with a detailed list of changes (as I do) when you spin a new spec with the changes, it saves me having to write out one, then I can simply add to it, or upload a new one. Thanks! Ralf: would you like me to update to the new spec now, or should I wait until you have done your passthrough? Created attachment 346 [details]
Use external zlib
This patch
* Removes hard-coded references to internal zlib headers.
This is supposed to allow building xbmc with all "zlib" directories physically removed.
* Adds zlib to the set of "external libs" when xbmc is being built with external libs enabled. I haven't tested whether this patch works without external zlibs and when wanting to use the bundled zlibs, only.
This patch is against Alex's xbmc-9.11-8 sources (Applied as "Patch13" after all of Alex patches).
Created attachment 347 [details]
Update gooms's missing
This patch updates goom's "missing" to a later version.
This avoids goom's configure script to warn about its "missing" being too old.
As mentioned before, an alternative to this patch would be run autoreconf -fi in "goom". As this is would impose risks to breaking the configuration, I prefer patching "missing".
Alex, the following diff may be of help. I assume that several people are working on the spec, so it makes no sense to provide a "latest and greatest" - though I'll upload the latest spec as well. The following diff however may help you to cut 'n' paste the spec you're maintaining.
[root@home07 SPECS]# diff -ruN ~rolf/rpmbuild/SPECS/xbmc.spec xbmc.spec
--- /home/rolf/rpmbuild/SPECS/xbmc.spec 2010-01-21 08:47:27.000000000 +0100
+++ xbmc.spec 2010-01-22 09:04:24.377603593 +0100
@@ -1,9 +1,10 @@
Name: xbmc
Version: 9.11
-Release: 8%{?dist}
+Release: 9%{?dist}
URL: http://www.xbmc.org/
-Source0: %{name}-%{version}-patched.tar.gz
+Source0: %{name}-%{version}-patched.tar.xz
+NoSource: 0
# xbmc contains code that we cannot ship, as well as redundant private
# copies of upstream libraries that we already distribute. Therefore
# we use this script to remove the code before shipping it.
@@ -12,18 +13,16 @@
# and invoke this script while in the directory where the tarball is located:
# sh xbmc-generate-tarball.sh <version>
# where <version> is the particular version being used
-Source1: xbmc-generate-tarball.sh
-
-Patch0: xbmc-9.11-a2-libjpeg.patch
-
-# check if this is in SVN
-Patch1: xbmc-9.11-a2-gcc440.patch
+Source1: xbmc-generate-tarball-xz.sh
# look for libdca, rather than libdts
-# http://xbmc.org/trac/ticket/8040 (although patch slightly different to this one)
-Patch2: xbmc-9.11-b1-config.patch
-
-# is this necessary if we remove dvdcss from source?
+# http://xbmc.org/trac/ticket/8040 also has a patch, but that one doesn't
+# compile well
+Patch2: xbmc-9.11-libdca.patch
+
+# dvdread and dvdcss have been removed from source, but without this patch
+# configure fails in an attempt to enter the removed dirs of dvdread and
+# dvdcss
Patch3: xbmc-9.11-b1-dvdlibs-external.patch
# http://xbmc.org/trac/ticket/8026 (this will be in next release)
@@ -38,7 +37,10 @@
# check to see if the next 3 can be submitted upstream
Patch8: xbmc-9.11-spyce.diff
Patch9: xbmc-9.11-RandomNumberGenerators.hpp.diff
-Patch10: xbmc-9.11-remlibs.patch
+
+# we removed libass from source, and need the next patch to stop
+# config.status opening libass/xbmc/Makefile.in
+Patch10: xbmc-9.11-remlibass.patch
# Fixes GCC warning: partial backport to 9.11
# (should be fixed in next release)
@@ -128,9 +130,7 @@
%setup -q -n %{name}-%{version}
-%patch0 -p1 -b .libjpeg
-%patch1 -p1 -b .gcc440
-%patch2 -p1 -b .config
+%patch2 -p1 -b .dtsdca
%patch3 -p1 -b .dvdlibs
%patch4 -p1 -b .cdio
%patch5 -p0 -b .makefilein
@@ -142,7 +142,6 @@
%patch11 -p1 -b .gccwarning
%patch12 -p1 -b .goahead
-
%build
CFLAGS="-fPIC -I/usr/include/ffmpeg/"
CXXFLAGS="$CFLAGS"
@@ -158,7 +157,7 @@
export ASFLAGS=-fPIC
%configure --enable-external-libraries --enable-goom
-make VERBOSE=1
+make %{?_smp_mflags} VERBOSE=1
%install
rm -rf $RPM_BUILD_ROOT
@@ -185,6 +184,13 @@
%{_datadir}/pixmaps/xbmc.png
%changelog
+* Thu Jan 21 2010 Rolf Fokkens <rolf fokkens[AT]wanadoo nl> - 9.11-9
+- increase compression ratio of tarball by compressing with cx
+ the src.rpm is now about 20% smaller
+- remove patch0 and patch1, they are obsolete now
+- rename patch2 and patch10 to more meaningful name
+
+%changelog
* Thu Jan 21 2010 Alex Lancaster <alexlan[AT]fedoraproject org> - 9.11-8
- Update xbmc script patch (thanks Ralf C)
- Reorder patches, add upstream tickets where possible
Created attachment 348 [details] Proposed spec related to Comment #52 This is an improved spec file related comment #52 Another issue: Alex, building with your spec triggers a lengthy series of configure script warnings, similar to this: ... checking libavcodec/avcodec.h usability... yes checking libavcodec/avcodec.h presence... no checking for libavcodec/avcodec.h... configure: WARNING: libavcodec/avcodec.h: accepted by the compiler, rejected by the preprocessor! configure: WARNING: libavcodec/avcodec.h: proceeding with the compiler's result ... The cause of this is xbmc's configuration not correctly honoring CPPFLAGS. More precisely: * autoconf expects -I<..> in CPPFLAGS * xbmc's Makefile*in's expects -I<..> in CFLAGS Result it is the mess above. One work-around to this would be to explicitly pass all -I<...>'s contained in CFLAGS, also in CPPFLAGS to configure. Proposal: Append CPPFLAGS to %configure: %configure .... CPPFLAGS="-I/usr/include/ffmpeg" I have a suggestion that we can split the skins to another RPMs. As XBMC have several options of skin, we can try to mimic that the XBMC team make the package for the Ubuntu. This may apply to another option like the plugins. In this case, we can make the package slimmer and less download for who does not interest on pm3-hd skin. But the default Conflence is still needed as it's fallback skin. As I'm maintaining Spotnet repositories for XBMC packages, I made the spliting of the skins. I'll attach my SPEC file, but to be noted, it's not based on RPMFusion library, as I was use ATRPMS. I'm just suggest to use the part %files and below for the skin packages. With the spec that I made, it will produce 3 rpm: xbmc-9.11.final-1.fc12.i686.rpm xbmc-skin-Confluence-9.11.final-1.fc12.noarch.rpm xbmc-skin-PM3HD-9.11.final-1.fc12.noarch.rpm I'll attach for the other SPEC for the skins too. Created attachment 349 [details] Sample SPEC file for splitting skin into different RPM (Comment #55) View Comment #55 Created attachment 350 [details]
SPEC file for Media Stream 1.00. It may need to be updated for new version 1.5
Created attachment 351 [details]
SPEC file for Media Stream Redux Skin
Created attachment 352 [details]
SPEC file for MiniMeedia
Created attachment 353 [details]
SPEC file for Rapier
Created attachment 354 [details]
SPEC file for Transparency
Last SPEC file that I have.
Just checked the XBMC Skin download, there is Back Row and AEON that I do not have the SPEC file
Forgot 1 more thing to mention. On x86_64 achitecture, there is 1 more file to be included in the SPEC File:
%{_datadir}/xbmc/FEH.pyc
I'm not too familiar with creating SPEC file, and XBMC is my first packages. So I have 2 SPEC file for xbmc i686 and x86_64. And the differ is just include the FEH.pyc that only appear in BUILDROOT for x86_64 machine
Created attachment 355 [details]
Weather pane fixes
This patch addresses 2 minor issues:
1. A bogus initialization.
2. Change the weather pane to print an empty string instead of "N/A"
1) is a bug fix and should be applied.
2) is essentially a matter of personal preference.
[Alex, I am trying to work off my patch queue, before diving
further into RPM_OPT_FLAGS].
(In reply to comment #56) > Created an attachment (id=349) [details] > Sample SPEC file for splitting skin into different RPM (Comment #55) > > View Comment #55 Thanks for the suggested modifications to the spec to split out the skins. These can probably be split out into subpackages, however all the other specs are separate packages (separate tarballs) and will need to have a separate review open for each. At this point my main focus is getting the main xbmc package reviewed and in the system, once that is done I help with packaging the other skins (which need to be reviewed, especially for various license issues). But thanks again. Created attachment 356 [details]
Patch to let rsxs acknowledge RPM_OPT_FLAGS
This patch addresses xbmc/screensavers/rsxs-0.9 not acknowledging RPM_OPT_FLAGS.
The cause for this is xbmc/screensavers/rsxs-0.9/* overriding CFLAGS and xbmc's toplevel configure not passing all required configuration settings down to rsxs-0.9.
Unfortunately, the autotool source in xbmc/screensavers/rsxs-0.9 are broken and incompatible to modern autotools, furthermore the generated files were hacked up by upstream xbmc.
=> It's not possible to regenerate the generated files nor to run autoreconf (Running autoreconf in rpm specs is harmful - This case is an example for this) on Fedora without having to dig out the ancient autotools upstream xbmc rsp. upstream rsxs had used.
Therefore I am hacking the files directly.
=> This patch need fixing up timestamps.
I am using this in %prep:
# Prevent rerunning the autotools.
touch -r xbmc/screensavers/rsxs-0.9/aclocal.m4 \
$(find xbmc/screensavers/rsxs-0.9 \( -name 'configure.*' -o -name 'Makefile.*' \))
Created attachment 357 [details]
Let XBMCProjectM acknowledge RPM_OPT_FLAGS
This patch addresses libprojectM not receiving RPM_OPT_FLAGS and other configuration settings.
Here, once again, the cause is xbmc's toplevel configure not passing down its settings to subdirectories.
Thanks for the RPM_OPT_FLAGS patches, are they suitable to send upstream? I would like to send all patches that aren't strictly Fedora-specific upstream. (In reply to comment #67) > Thanks for the RPM_OPT_FLAGS patches, There are more to come ;-) > are they suitable to send upstream? Though some of them actually are hacks, I hope so. [IMO, xbmc actually has massive design problems wrt. its configuration and would need a major overhaul, but this would be way beyond a Fedora review ;-)] > I > would like to send all patches that aren't strictly Fedora-specific upstream. Feel free to submit them upstream. Upstream here (theuni). It would be easier on us if a tracker ticket were created on our trac for all outstanding upstream issues (patches included), and linked to this page. Then we have all fedora-related issues in one place for easy reference. Any volunteers? fwiw, as of ~2 days ago, fedora is compiling with no patches needed. See here for nightly build status and logs, not sure if it helps: http://buildbot.xbmc.org/builders/fedora-nightly (In reply to comment #69) > Upstream here (theuni). Hello. Good to hear from you! > It would be easier on us if a tracker ticket were created on our trac for all > outstanding upstream issues (patches included), and linked to this page. Then > we have all fedora-related issues in one place for easy reference. Any > volunteers? I have started a few already, but I'll start submitting the remaining patches (which should hopefully be useful for all distros and even non-Linux OSes) to a single trac ticket. Some of the patches may need additional modification before applying to trunk, but they should be a starting point. > fwiw, as of ~2 days ago, fedora is compiling with no patches needed. See here > for nightly build status and logs, not sure if it helps: > http://buildbot.xbmc.org/builders/fedora-nightly That's good to hear. However, most of the patches aren't just to get xbmc to compile (9.11 will "compile" very roughly with almost no patches), but are fixes to ensure that compiler flags get passed around correctly, and to conform to Linux specifications like the FHS and to use as many external system libraries as possible (ideally configurable). New version (-10), SRPM is in the process of being uploaded (please note I renumbered the patches to be consecutive): * Mon Jan 25 2010 Alex Lancaster <alexlan[AT]fedoraproject org> - 9.11-10 - Patches for RPM_OPT_FLAGS being recognised throughout (thanks Ralf C) - Patch for goom (Ralf C) - Patch for using external zlib (Ralf C) - Pass CPPFLAGS to configure (Ralf C) Spec: http://alexlan.fedorapeople.org/rpmfusion/xbmc.spec SRPM: http://alexlan.fedorapeople.org/rpmfusion/xbmc-9.11-10.fc12.src.rpm (In reply to comment #48) > > This patch look for libdca, rather than libdts, there is already: > > http://xbmc.org/trac/ticket/8040 (although patch slightly different to this > > one) > > Patch2: xbmc-9.11-b1-config.patch > I renamed it to xbmc-9.11-libdca.patch. I tried the upstream patch, but it > somehow makes make fail on "make -C libdts". OK, ideally (from upstream POV) it this would detect either libdca or libdts and choose the relevant one. > > Rolf, is this necessary if we remove dvdcss from source? > > Patch3: xbmc-9.11-b1-dvdlibs-external.patch > Without this patch configure fails in an attempt to descend to > xbmc/cores/dvdplayer/Codecs/libdvd/libdvdcss and > xbmc/cores/dvdplayer/Codecs/libdvd/libdvdread which no longer exist. OK, that seems to just remove the directories from the build, so this doesn't really work for upstream because they need to be able to build those directory if the external library support is disabled. Would it be possible to enhance that patch. > The purposes of the previous patches have been clarified by Ralf > > Patch10: xbmc-9.11-remlibs.patch > Without this patch config.status attempts to descend to open > xbmc/lib/libass/xbmc/Makefile.in which is obsolete (and has been removed) as > we use external libass now. I renamed it to xbmc-9.11-remlibass.patch, and > removed an irrelevant part. OK, that's good but has the same problem as above: i.e. it can't be applied to trunk because it needs to be able to work in internal and external mode. I'll probably hold off on submitting these two patches upstream until they can be made to work in non-external mode as well. > I changed Source0 to be based on xz compression, it reduces the src.rpm size! > > Both src.rpm and nosrc.rpm can be found at: > http://rolffokkens.dyndns.org/xbmc-9.11-9.fc12/ Great thanks! That's very helpful. (In reply to comment #70) > (In reply to comment #69) > > Upstream here (theuni). > > Any > > volunteers? > > I have started a few already, but I'll start submitting the remaining patches > (which should hopefully be useful for all distros and even non-Linux OSes) to a > single trac ticket. Some of the patches may need additional modification > before applying to trunk, but they should be a starting point. OK, I started a new trac ticket for submitting these patches here: http://www.xbmc.org/trac/ticket/8629 more will be added from here as they become more suitable for upstream consumption, a few some need more work (see commment #72). (In reply to comment #69) > fwiw, as of ~2 days ago, fedora is compiling with no patches needed. See here > for nightly build status and logs, not sure if it helps: > http://buildbot.xbmc.org/builders/fedora-nightly Ah, I just noticed that it uses the ATRPMS for deps which is incompatible with RPM Fusion, that isn't so helpful for us here at RPM Fusion. Would it be possible to add nightly builds using RPM Fusion repos? > Ah, I just noticed that it uses the ATRPMS for deps which is incompatible with
> RPM Fusion, that isn't so helpful for us here at RPM Fusion. Would it be
> possible to add nightly builds using RPM Fusion repos?
Yes, that's probably doable. I'll try to get to this in the next few days.
Also, sure I realize that just compiling isn't much, but that was my
requirement for adding to the buildbot. From here we can help to make things
more compliant (for other distros as well). And ofcourse we realize that the
buildsystem needs some work.
Please keep in mind that our default (for now) is to use internal libs, so no
patch will be considered that does not take this into account. You'll notice
that over the last few days we've moved a few internal libs out by default.
Hopefully that will remain the trend.
Thanks for opening the tracking ticket, this should make things much more visibile. The devs tend to be cranky about major build/install changes, but I'll stay after em to keep an open mind ;)
Created attachment 361 [details]
Let Makefile's acknowledge CFLAGS/CXXFLAGS
RPM_OPT_FLAGS related patch:
Addresses 2 issues at once:
- Makefiles overriding CFLAGS/CXXFLAGS
- Makefiles not using CFLAGS/CXXFLAGS for linking
(This is necessary in multilib'ed environments - building on ppc/ppc64 or
sparc/sparc64, rsp. a building for i386 (-m32) on x86_64 would choke on this.)
Though I currently believe this patch to be complete, I am pretty sure there are still spots I missed ;)
More patches to come ...
@Alex Buildslave has been switched to rpmfusion. Nightly builds can still be found here: http://buildbot.xbmc.org/builders/fedora-nightly Cory, why are you marking my attachments as obsolete? Though you might have applied them upstream, these still are part of the patches we will want to apply for building xbmc-9.11 for Fedora. Legitimate reasons to marking them obsolete would be: * General agreement on them not being used/useful/broken. * Upstream releasing a new tarball and us re-basing our works against it. * The patches being replaced with other patches (e.g. upstream having merged them their release branch and us rebasing our patches against their VCS's contents). My apologies, I've misunderstood. I'll stay on my turf :) (In reply to comment #77) > @Alex > Buildslave has been switched to rpmfusion. > Nightly builds can still be found here: > http://buildbot.xbmc.org/builders/fedora-nightly Great, looks good. I wonder if you might be able to create an additional build configuration that would specifically enable the external libraries feature as well as goom, i.e. mimic what our current spec does: INCLUDEDIRS="-I/usr/include/ffmpeg/" CFLAGS="-fPIC $INCLUDEDIRS" CXXFLAGS="$CFLAGS" LDFLAGS="-fPIC" LIBS="-L%{_libdir}/mysql $LIBS" export CFLAGS export CXXFLAGS export LDFLAGS export LIBS chmod +x bootstrap ./bootstrap export ASFLAGS=-fPIC ./configure --enable-external-libraries --enable-goom CPPFLAGS="$INCLUDEDIRS" make That way we'd be able to quickly diagnose errors or regressions in the external library functionality in RPM Fusion/Fedora. At this time I think external library support will work for all libraries except for the libdts because Fedora has switched to the new libdca, so it may require the xbmc-9.11-libdca.patch for external library support. In addition have you added all the packages listed in BuildRequires in the spec as packages to install, if not it may not be using all the external libraries XBMC supports. New version (-11), SRPM is in the process of being uploaded: * Tue Jan 26 2010 Alex Lancaster <alexlan[AT]fedoraproject org> - 9.11-11 - Add another patch to ensure Makefile's pass compiler flags properly (thanks Ralf C) - Remove commands no longer needed in install Spec: http://alexlan.fedorapeople.org/rpmfusion/xbmc.spec SRPM: http://alexlan.fedorapeople.org/rpmfusion/xbmc-9.11-11.fc12.src.rpm Ping? Any comments on my latest spec? Anyone willing to give this an initial review? (In reply to comment #82) > Ping? Patience please - More important topics have kept me busy throughout this week (E.g. wading through the mess this broken Fedora thunderbird-3 has left behind). I intend to get back to xmbc, either later today or tomorrow ;) (In reply to comment #83) > (In reply to comment #82) > > Ping? > > Patience please - More important topics have kept me busy throughout this week > (E.g. wading through the mess this broken Fedora thunderbird-3 has left > behind). Sure, although my ping wasn't necessarily directed at you, but to any RPM Fusion maintainer who might be interested in reviewing this package and/or any others Cc'ed on this bug who have made comments. > I intend to get back to xmbc, either later today or tomorrow ;) No problem. Sorry for interrupting, I've just registered* to ask you kindly to also compile and include the cwiid-based Wiimote event client in `tools/EventClients/Clients/WiiRemote', adding BuildRequires cwiid-devel and bluez-libs-devel, adding our default CFLAGS to `tools/EventClient/Makefile' and doing `make wiimote' in said directory should do the job.
The Debian-based package includes the binary as %{_bindir}/xbmc-wiiremote, man page can be found in `./docs/manpages/xbmc-wiiremote.1'. It would also be nice to be able starting up the client without tinkering with the provided xbmc-standalone script or firing up a whole DE first.
I've tested the client together with your build, works like a charm.
(* I'm a Fedora Packager but not @RPMFusion)
Created attachment 366 [details]
Misc. xbmc-9.1 fixes
These patches address further RPM_OPT_FLAGS issues,
address a bug in one of my previous patch and contains further bugs fixes to xbmc itself.
This patch contains all xbmc patches, I currently have accumulated so far.
Actually, each of them actually should be split into individual patches, but for reasons of simplicity (lazyness :-) ), I am sending them in one "bunch". :-)
Created attachment 367 [details]
Further extensions to xbmc-generate-tarball-xz.sh
[I hope this patch is self-explanatory :-]
Created attachment 368 [details]
Diffs between Alex's spec and mine
This patch is contains the diff between your and my current xbmc.spec.
Differences to your spec:
* Passing *FLAGS etc. to configure, because xbmc Makefile's pick up *FLAGS from the "air" (enviroment) and append them anew in each single Makefile.
This causes each make recursion to append *FLAGS one more,
i.e. in a 3-level recursion, CFLAGS will have piled up 3 times (passed to the compiler 3 times)
* Touch rsxs Makefiles to prevent rerunning the autotools.
* Add missing %clean
This is the last change, I currently have pending.
(In reply to comment #86) > Created an attachment (id=366) [details] > Misc. xbmc-9.1 fixes > > These patches address further RPM_OPT_FLAGS issues, > address a bug in one of my previous patch and contains further bugs fixes to > xbmc itself. > > This patch contains all xbmc patches, I currently have accumulated so far. > > Actually, each of them actually should be split into individual patches, but > for reasons of simplicity (lazyness :-) ), I am sending them in one "bunch". > :-) Is each diff supposed to be a separate patch, or are some of them grouped logically? If you could tell me which one's are together and give a (rough) description I will also submit them upstream to the trac ticket. (In reply to comment #89) > Is each diff supposed to be a separate patch, or are some of them grouped > logically? Each diff is a separate patch. > If you could tell me which one's are together and give a (rough) > description I will also submit them upstream to the trac ticket. The actually interesting part for us is this: * configure.in: *FLAGS, configure-argument hacks. Incremental patch against previous patches on configure.in. Should probably be merged with its predecessors. All other patches address more or less minor issues with xbmc, I tripped over when repeatedly building xbmc: * xbmc/cores/paplayer/GYMCodec/ym2612.h: Clean up of a signed/unsigned char handling sloppiness. * xbmc/cores/paplayer/MACDll/Makefile.in: Remove -pedantic from CFLAGS. Passing -pedantic in a production SW's Makefile doesn't make any sense. * xbmc/cores/paplayer/MACDll/Source/MACLib/Prepare.cpp: Use <number>U instead of <number> for unsigned <number> constant initialization to avoid potential C-dialect issues (Improve portability). GCC warns about the original code to change behavior depending up --stdc=.. variant being used. Actually, I'd assume the original code in xbmc to be dysfunctional. However as this code is part of a codec of an exotic format (at least on Linux), I'd assume potential issues caused by this code might have gotten away unnoticed so far. * xbmc/lib/libid3tag/libid3tag/metadata.c: Add missing prototypes for free/malloc, ... etc. Original code is non-posix compilant. * xbmc/lib/libRTV/Makefile.in: Remove -D__unix__. __unix__ is an internal system define, all unix-compilers (comprising linux) internally define. It's a violation of POSIX to specify it on a compiler's command line. New version (-12), SRPM is in the process of being uploaded: * Mon Feb 8 2010 Alex Lancaster <alexlan[AT]fedoraproject org> - 9.11-12 - More patches from Ralf Corsepius - Update configure line, specify *FLAGS directly in configure line (thanks Ralf) - Touch rsxs Makefiles to prevent rerunning the autotools (thanks Ralf) - Add missing %%clean (thanks Ralf) - Remove even more unused library copies in generate tarball script Spec: http://alexlan.fedorapeople.org/rpmfusion/xbmc.spec SRPM: http://alexlan.fedorapeople.org/rpmfusion/xbmc-9.11-12.fc12.src.rpm New binary for x86_64 to match the -12 SRPM: http://alexlan.fedorapeople.org/rpmfusion/xbmc-9.11-12.fc12.x86_64.rpm Most of the patches on http://trac.xbmc.org/ticket/8629 (note the new URL scheme for upstream XBMC trac) have been either applied in SVN trunk or are now otherwise obsolete. There still seems to be a bit of misunderstanding upstream of the purpose of DESTDIR with respect to the changes for /usr/{share,lib} at: http://trac.xbmc.org/ticket/8590. We should probably split the patch into: 1. a patch that adds DESTDIR support 2. rework the rest of the patch so that only truly arch-dep files such as .so and .vis (?) files go in /usr/lib{64}/xbmc/ and the rest stay in /usr/share/xbmc/ Then at least (1) could be committed while working on (2). (In reply to comment #93) > Most of the patches on http://trac.xbmc.org/ticket/8629 (note the new URL > scheme for upstream XBMC trac) have been either applied in SVN trunk or are now > otherwise obsolete. > > There still seems to be a bit of misunderstanding upstream of the purpose of > DESTDIR with respect to the changes for /usr/{share,lib} at: > http://trac.xbmc.org/ticket/8590. Yes, upstream seems to a have lot of problems in understanding system integration concepts. > We should probably split the patch into: > > 1. a patch that adds DESTDIR support > > 2. rework the rest of the patch so that only truly arch-dep files such as .so > and .vis (?) files go in /usr/lib{64}/xbmc/ and the rest stay in > /usr/share/xbmc/ AFAICT, this is non-trivial, because at least 9.11 lacks a support of the concepts of "pkgdatadir" (/usr/share/xmbc) and "pkglibdir" (/usr/{lib|lib64}/xbmc). It only seems to support the concept of a "single root" ("prefix"). I haven't checked how difficult implementing this would be. It would likely be a massive work of several of xbmc's internals. > Then at least (1) could be committed while working on (2). Hmm, the problem with this approach is that such patches would have to be incremental because adding DESTDIR and datadir/libdir both affect the same pieces of source-code. (In reply to comment #94) > I haven't checked how difficult implementing this would be. It would likely be > a massive work of several of xbmc's internals. Upstream alludes to how this might be done here: http://trac.xbmc.org/ticket/8590#comment:8 > > Then at least (1) could be committed while working on (2). > Hmm, the problem with this approach is that such patches would have to be > incremental because adding DESTDIR and datadir/libdir both affect the same > pieces of source-code. Sure, I realise that there is overlap, but at least we could get that added to SVN trunk, then the next patch for the next version of XBMC would be against the (new) SVN version, so at least we would have that functionality built in. All this is really only useful for the next 10.xx version of XBMC though, not for the current 9.11 version. Any chance you could pickup the formal review, or should I perhaps canvas some others on Fedora and/or rpmfusion devel list? It may be more productive to hold off on a review until the next version when many of these issues have been addressed and resolved upstream. Your patches have already helped in svn quite a bit. We know there's work to be done and it's a priority for Camelot+1. (In reply to comment #95) > (In reply to comment #94) > > > I haven't checked how difficult implementing this would be. It would likely be > > a massive work of several of xbmc's internals. > > Upstream alludes to how this might be done here: > > http://trac.xbmc.org/ticket/8590#comment:8 Well, ... I think, xmbc configuation is foobar'd and could use a re-write from scratch :) However, provided the answers I read in this xbmc ticket, I'd assume this to be close to impossible to implement. > Any chance you could pickup the formal review, I will try to so ASAP, but I am currently having real problems in finding a free time slot. > or should I perhaps canvas some > others on Fedora and/or rpmfusion devel list? Feel free to do so ... (In reply to comment #96) > It may be more productive to hold off on a review until the next version Any ETA? I am hesitant to wait for Camelot+1, because I'd expect Camelot+1 to introduce further configuration issues, similar to those we already addressed, i.e. I'd expect us having to start a review from the beginning, while I believe to be close to let the current package into rpmfusion. (In reply to comment #96) > It may be more productive to hold off on a review until the next version when > many of these issues have been addressed and resolved upstream. Your patches > have already helped in svn quite a bit. > We know there's work to be done and it's a priority for Camelot+1. I think it's still worth getting a package out there before then, otherwise users will be waiting for a long-time and the review process (which has got some momentum right now) is likely to stall (I've seen this happen before). I think we have got a package that should be able to pass a preliminary review with the patches as-is. Ok, I'm willing to review this, but so far the only packages I've reviewed for Fedora have been softballs and this looks like it will be pretty massive. Is -12 current? (In reply to comment #99) > Ok, I'm willing to review this, but so far the only packages I've reviewed for > Fedora have been softballs and this looks like it will be pretty massive. Excellent! I'm sure others will also step in to help out. Also there's been a fair amount of "reviewing" already especially by Ralf (see the previous almost 100 comments!) so the package has already been through the ringer already and some of the trickier issues have already been addressed. > Is -12 current? Yep: -12 is the most recent. (In reply to comment #99) > Ok, I'm willing to review this, but so far the only packages I've reviewed for > Fedora have been softballs and this looks like it will be pretty massive. Hi Jonathan, just a quick ping to see if you are still interested in reviewing this. (In reply to comment #101) > Hi Jonathan, just a quick ping to see if you are still interested in reviewing > this. Yes, I'm hoping to start tomorrow. This last week was a bit crazier than I expected. Ralf's summary of the review to this point (posted to rpmfusion-devel): ------------------------------------------------------------------------ * Technically: *-12 doesn't build for FC13 ;) - An API change between rpmfusion's FC12 and FC13's ffmpeg breaks xbmc. - xbmc is victim of the DSO changes in FC13. - There is a subtile configure script bug somewhere causing it to (silently) not to work for FC13. I have dirty hacks addressing the 1st and 2nd issues pending, but am still investigating the latter, yet. Could be one these "autoreconf is harmful" cases, could also be a side-effect of the DSO-changes, could be something else, ... I don't know yet. * Usability-wise: - Verify that python works sufficiently. There have been reports that xbmc's python scripts (python2.4) don't work on Fedora (python2.6). I haven't see any such python breakdown yet, so I don't know how to reproduce such breakdown. - Decide about what to do with xbmc-standalone. IMO, it's dysfunctional. - Decide about what to do with /usr/bin/xbmc's "core dump feature". To me, it's nothing but silly. * Perform a legal review. - AFAICT, even if putting patent issues aside, xbmc is not [L]GPL'ed, because it contains subpackages/libraries which are not [L]GPL-compatible. The original xbmc code certainly is "free", but I am having strong doubts if all of the libraries they have bundled, are (e.g. GoAHead, UnRar). In Fedora, I would reject this package for "improper licensing" and/or delegate it for legal review to FE-LEGAL. No idea, about what rpmfusion wants to do about it. - One detail: xbmc contains fonts, which suspiciously look like "bundled msttcorefonts", but I haven't checked the details, yet. * Packaging-wise/FPG-compliance-wise: xbmc contains many "bundled" libraries. Ralf found the problem with xbmc not building under F13: """ It's a case of "running autoreconf during builts is harmful". "In this case, it's xbmc's configure.ac hitting a bug in autoconf-2.65 (Upstream autoconf is aware about this issue.) Bug against fedora's autoconf filed: https://bugzilla.redhat.com/show_bug.cgi?id=568039 """ I would really like to see a number of the libraries in xbmc/lib split out and packaged separately (preferably in Fedora). The ones I'm looking at are: * cximage (could go into Fedora, I think) * libcmyth (could also go into Fedora as it doesn't need to actually be built against MythTV, at least as far as I can see). * libexif (mentioned earlier, already in Fedora) * libhdhomerun (already in Fedora) * libid3tag (already in Fedora, not sure if it includes fixes mentioned in xbmc/lib/libid3tag/readme.txt) * librtmp (could go into Fedora, I think) * libshout (already in Fedora) * libsquish (could go into Fedora unless there are patent problems, not sure myself) * libupnp (aka Platinum UPnP, could go into Fedora) * libxdaap (based on libopendaap, could either package fork or push changes back upstream) * UnrarXLib (can we ditch this and include patches to move to libunrar?) I'd also like to see xbmc using system library headers whenever possible rather than bundled headers: * boost * libglew * libiconv (though /usr/include/iconv.h is totally different from the bundled one; don't know how that works!) * libportaudio (though we're missing pa_mac_core; unneeded for Linux, perhaps?) * sqLite (it seems to be the sqlite3 header and some code having to do with datasets; if we can at least ditch the header, that would be great) * zlib Finally, the following may not be appropriate for Fedora or RPM Fusion because of problematic licenses: * libGoAhead (Not quite sure what to make of the license, but I think this is going away in the next release anyway) * libhts (Not sure where upstream is or what it's licensed under) I would happily review any of these that need to go into either Fedora or RPM Fusion, and will also try to help split some of these out. Hi some quick answers I hope. (In reply to comment #105) > I'd also like to see xbmc using system library headers whenever possible rather > than bundled headers: > > * boost Should be system for linux, just windows that needs it. > * libportaudio (though we're missing pa_mac_core; unneeded for Linux, perhaps?) Unneeded on all, just left, can be removed. > * zlib Afaik same as with boost > > Finally, the following may not be appropriate for Fedora or RPM Fusion because > of problematic licenses: > > * libGoAhead (Not quite sure what to make of the license, but I think this is > going away in the next release anyway) To confirm, latest SVN does not have libGoAhead and uses libmicrohttpd instead > * libhts (Not sure where upstream is or what it's licensed under) TBH I'm not sure, will ask, here is one http://trac.lonelycoder.com/hts/browser/trunk/libs/libhts?rev=2547 Cheers, Tobias a.k.a topfs2 (In reply to comment #106) > > * zlib > > Afaik same as with boost The original sources use the bundled zlibs, however should already be unused with our patches. > > Finally, the following may not be appropriate for Fedora or RPM Fusion because > > of problematic licenses: > > > > * libGoAhead (Not quite sure what to make of the license, but I think this is > > going away in the next release anyway) > > To confirm, latest SVN does not have libGoAhead and uses libmicrohttpd instead This likely doesn't help us. The licening xbmc ships with its GoAhead sources * puts us under a legal risk of being sued by GoAhead, because we are patching GoAhead. * Very likely invalidates xbmc's licensing, i.e. xbmc's sources very likely are *NOT GPL'ed*. (In reply to comment #107) > (In reply to comment #106) > > > > * zlib > > > > Afaik same as with boost > The original sources use the bundled zlibs, however should already be unused > with our patches. Right, ditto with many of the other bundles, which have been removed. > > > Finally, the following may not be appropriate for Fedora or RPM Fusion because > > > of problematic licenses: > > > > > > * libGoAhead (Not quite sure what to make of the license, but I think this is > > > going away in the next release anyway) > > > > To confirm, latest SVN does not have libGoAhead and uses libmicrohttpd instead > > This likely doesn't help us. The licening xbmc ships with its GoAhead sources > * puts us under a legal risk of being sued by GoAhead, because we are patching > GoAhead. > * Very likely invalidates xbmc's licensing, i.e. xbmc's sources very likely are > *NOT GPL'ed*. We should just remove all the GoAhead stuff from the tarball, can we just build the 9.11 without GoAhead at all? switching to SVN is likely to be a major pain at this stage. (In reply to comment #105) > I would really like to see a number of the libraries in xbmc/lib split out and > packaged separately (preferably in Fedora). > > The ones I'm looking at are: > > * cximage (could go into Fedora, I think) > * libcmyth (could also go into Fedora as it doesn't need to actually be built > against MythTV, at least as far as I can see). Are these 2 actual separate projects with URLs? > * libexif (mentioned earlier, already in Fedora) This actually isn't *the* libexif, it's totally separate code, just has the same name. > * libhdhomerun (already in Fedora) > * libshout (already in Fedora) OK, not sure if upstream has facility to use external versions of these 2, or if it would be easy to patch. > * libid3tag (already in Fedora, not sure if it includes fixes mentioned in > xbmc/lib/libid3tag/readme.txt) I don't think it does include the fixes, I have raised this on: http://trac.xbmc.org/ticket/8331 > * libxdaap (based on libopendaap, could either package fork or push changes > back upstream) Is this used by any other package other than XBMC? if not, I don't see a need. > * UnrarXLib (can we ditch this and include patches to move to libunrar?) I believe upstream has already done so in SVN, I think? > * librtmp (could go into Fedora, I think) > * libsquish (could go into Fedora unless there are patent problems, not sure > myself) > * libupnp (aka Platinum UPnP, could go into Fedora) These 3 would require new packages, are there other packages that use these currently, or would likely in the future? Are they *all* necessary for basic XBMC functionality We should look at what Debian does here. Some of these, like libid3tag, have been heavily patched by upstream (I have reported trac ticket) and libid3tag is more or less orphaned as a separate project anyway. Ideally, yes it would be good to switch out many of these, and in fact we have already made a huge effort to do so with other libraries (see the previous patches by Ralf), but might it be possible to do so an incremental basis? If the whole package has to wait until every library is packaged, we'll never get this done, partly because upstream often needs to patch said packages. There are precedents in Fedora for splitting out stuff post-review. > I'd also like to see xbmc using system library headers whenever possible rather > than bundled headers: > > * boost > * libglew > * libiconv (though /usr/include/iconv.h is totally different from the bundled > one; don't know how that works!) > * libportaudio (though we're missing pa_mac_core; unneeded for Linux, perhaps?) > * sqLite (it seems to be the sqlite3 header and some code having to do with > datasets; if we can at least ditch the header, that would be great) > * zlib I think Ralf's patches addressed most of these. Ralf, can you confirm which one's the package still uses? > I would happily review any of these that need to go into either Fedora or RPM > Fusion, and will also try to help split some of these out. That would definitely help, thanks! I think the major hassle is going to be getting XBMC to "see" the external libs. Although I emphasise the significant work that's already been done on this package, and I'm concerned if the review hangs on one or 2 very minor external libs (which may offer only optional functionality) currently being bundled that are difficult to get XBMC to recognize as "external libs", that the whole process might stall and people will give up. Yes, in an ideal world, everything would be split out (and working with upstream we will do so: upstream SVN is making steps towards that in any case) but there is a tradeoff between insisting on dotting every i and t to meet the review guidelines and the amount of volunteer motivation. :-) The only real showstopper should be the legal one. (In reply to comment #109) > (In reply to comment #105) > > I would really like to see a number of the libraries in xbmc/lib split out and > > packaged separately (preferably in Fedora). > > > > The ones I'm looking at are: > > > > * cximage (could go into Fedora, I think) > > * libcmyth (could also go into Fedora as it doesn't need to actually be built > > against MythTV, at least as far as I can see). > > Are these 2 actual separate projects with URLs? Yes. cximage: http://www.xdp.it/cximage/600/cximage600_full.zip libcmyth is part of the mvpmc project (and should probably be split out upstream): http://sourceforge.net/projects/mvpmc/files/mvpmc/mvpmc-0.3.4/mvpmc-0.3.4.tar.gz/download Of the two, cximage is the most likely to be used elsewhere. For the record, cximage hasn't been updated since 2008 and libcmyth since 2007. > > * libexif (mentioned earlier, already in Fedora) > > This actually isn't *the* libexif, it's totally separate code, just has the > same name. Sorry, my mistake. > > * libhdhomerun (already in Fedora) > > * libshout (already in Fedora) > > OK, not sure if upstream has facility to use external versions of these 2, or > if it would be easy to patch. I'll take a look at these and see if we can split them out with minimal fuss. > > * libid3tag (already in Fedora, not sure if it includes fixes mentioned in > > xbmc/lib/libid3tag/readme.txt) > > I don't think it does include the fixes, I have raised this on: > > http://trac.xbmc.org/ticket/8331 > > > * libxdaap (based on libopendaap, could either package fork or push changes > > back upstream) > > Is this used by any other package other than XBMC? if not, I don't see a need. I don't think so, so I suppose we could just leave that in. > > * UnrarXLib (can we ditch this and include patches to move to libunrar?) > > I believe upstream has already done so in SVN, I think? Brilliant, no point in reinventing the wheel for 9.11. > > * librtmp (could go into Fedora, I think) > > * libsquish (could go into Fedora unless there are patent problems, not sure > > myself) > > * libupnp (aka Platinum UPnP, could go into Fedora) > > These 3 would require new packages, are there other packages that use these > currently, or would likely in the future? Are they *all* necessary for basic > XBMC functionality We should look at what Debian does here. None of them are currently in Fedora, but they all could be used by other packages in the future. If any are currently being used by Fedora packages, they are bundled as well. > Some of these, like libid3tag, have been heavily patched by upstream (I have > reported trac ticket) and libid3tag is more or less orphaned as a separate > project anyway. > > Ideally, yes it would be good to switch out many of these, and in fact we have > already made a huge effort to do so with other libraries (see the previous > patches by Ralf), but might it be possible to do so an incremental basis? If > the whole package has to wait until every library is packaged, we'll never get > this done, partly because upstream often needs to patch said packages. There > are precedents in Fedora for splitting out stuff post-review. > > > I'd also like to see xbmc using system library headers whenever possible rather > > than bundled headers: > > > > * boost > > * libglew > > * libiconv (though /usr/include/iconv.h is totally different from the bundled > > one; don't know how that works!) > > * libportaudio (though we're missing pa_mac_core; unneeded for Linux, perhaps?) > > * sqLite (it seems to be the sqlite3 header and some code having to do with > > datasets; if we can at least ditch the header, that would be great) > > * zlib > > I think Ralf's patches addressed most of these. Ralf, can you confirm which > one's the package still uses? > > > I would happily review any of these that need to go into either Fedora or RPM > > Fusion, and will also try to help split some of these out. > > That would definitely help, thanks! I think the major hassle is going to be > getting XBMC to "see" the external libs. > > Although I emphasise the significant work that's already been done on this > package, and I'm concerned if the review hangs on one or 2 very minor external > libs (which may offer only optional functionality) currently being bundled that > are difficult to get XBMC to recognize as "external libs", that the whole > process might stall and people will give up. > > Yes, in an ideal world, everything would be split out (and working with > upstream we will do so: upstream SVN is making steps towards that in any case) > but there is a tradeoff between insisting on dotting every i and t to meet the > review guidelines and the amount of volunteer motivation. :-) The only real > showstopper should be the legal one. True enough. I've just had a nasty episode with bundled zlib in deltarpm, so I'd love to make sure we're doing our best to unbundle what we can. Except for the legal stuff, I don't consider any of these showstopppers. My biggest concern with cximage is that *it's* bundling loads of libraries itself. Honestly, apart from libGoAhead, cximage is the one bundled library that scares me the most because of that. If we could get it packaged for Fedora, and have XBMC using it, that would make me feel much better. (In reply to comment #110) > > > * libshout (already in Fedora) > > > > OK, not sure if upstream has facility to use external versions of these 2, or > > if it would be easy to patch. > > I'll take a look at these and see if we can split them out with minimal fuss. I just had a look into libshout. AFAICT, xbmc's libshout is something entirely different as the libshout in Fedora. (In reply to comment #110) > > Are these 2 actual separate projects with URLs? > > Yes. > cximage: http://www.xdp.it/cximage/600/cximage600_full.zip > libcmyth is part of the mvpmc project (and should probably be split out > upstream): > http://sourceforge.net/projects/mvpmc/files/mvpmc/mvpmc-0.3.4/mvpmc-0.3.4.tar.gz/download > > Of the two, cximage is the most likely to be used elsewhere. > > For the record, cximage hasn't been updated since 2008 and libcmyth since 2007. I regularly see commits for libcmyth in SVN trunk, so perhaps XBMC has become the defacto upstream for this library? Hopefully upstream XBMC will chime on this. Also hopefully they will chime on the feasibility of using external cximage. > > > > * libhdhomerun (already in Fedora) > > > * libshout (already in Fedora) > I'll take a look at these and see if we can split them out with minimal fuss. Excellent. You can post any patches here, and I'll submit them to the upstream trac ticket for Fedora patches: http://trac.xbmc.org/ticket/8629 > > > * libxdaap > > Is this used by any other package other than XBMC? if not, I don't see a need. > > I don't think so, so I suppose we could just leave that in. OK, will do so for the moment. > > > * UnrarXLib (can we ditch this and include patches to move to libunrar?) > > > > I believe upstream has already done so in SVN, I think? > > Brilliant, no point in reinventing the wheel for 9.11. Hopefully upstream can check on this. > > > * librtmp (could go into Fedora, I think) > > > * libsquish (could go into Fedora unless there are patent problems, not sure myself) > > > * libupnp (aka Platinum UPnP, could go into Fedora) > > > > These 3 would require new packages, are there other packages that use these > > currently, or would likely in the future? Are they *all* necessary for basic > > XBMC functionality We should look at what Debian does here. > None of them are currently in Fedora, but they all could be used by other > packages in the future. If any are currently being used by Fedora packages, > they are bundled as well. Again not sure if they are necessary for basic XBMC functionality. > > Yes, in an ideal world, everything would be split out (and working with > > upstream we will do so: upstream SVN is making steps towards that in any case) > > but there is a tradeoff between insisting on dotting every i and t to meet the > > review guidelines and the amount of volunteer motivation. :-) The only real > > showstopper should be the legal one. > > True enough. I've just had a nasty episode with bundled zlib in deltarpm, so > I'd love to make sure we're doing our best to unbundle what we can. Except for > the legal stuff, I don't consider any of these showstopppers. Sure. @Alex Lancaster: Would you *please* include the Wiimote binary and manual as already described at comment #85? The changes/additions are really trivial and I'd send you a patch myself if I had the time to prepare the build environment for XBMC right now. (In reply to comment #114) > @Alex Lancaster: Would you *please* include the Wiimote binary and manual as > already described at comment #85? The changes/additions are really trivial and > I'd send you a patch myself if I had the time to prepare the build environment > for XBMC right now. Sorry, we have bigger fish to fry right now (like getting the package through review at all). Once that's done, I'll look into it. Patches will be considered once review is done. We've actually removed libportaudio from our code. It is no longer used on any platform. Kyle Hill (aka monkeyman) Created attachment 379 [details]
Add Xext check to configure
This hack adds -lXext to LIBS.
Without it xbmc fails to link with a indirect shared DSO error on FC13.
Created attachment 380 [details]
hack to work around ffmpeg sws_scale() incompatibility
This is a brutal hack to work around xbmc failing to build against rpmfusion's ffmpeg for F13.
Background: An API change in libswscale:
ffmpeg (/usr/include/ffmpeg/libswscale/swscale.h
in rpmfusion for F13 uses
int sws_scale(struct SwsContext *context, const uint8_t* srcSlice[], int srcStride[], int srcSliceY, int srcSliceH, uint8_t* dst[], int dstStride[]);
while sws_scale in ffmpeg for FC12 uses:
int sws_scale(struct SwsContext *context, uint8_t* srcSlice[], int srcStride[],
int srcSliceY, int srcSliceH, uint8_t* dst[], int dstStride[]);
This API change breaks C++ code using libswscale.
This patch is more a minimally invasive hack to work around this incompatiblity
but a real fix, which likely would be to revamp xbmc's API to ffmpeg.
(In reply to comment #111) > My biggest concern with cximage is that *it's* bundling loads of libraries > itself. Honestly, apart from libGoAhead, cximage is the one bundled library > that scares me the most because of that. If we could get it packaged for > Fedora, and have XBMC using it, that would make me feel much better. OK, I have some feedback on cximage (as well as some other libraries), from upstream: "1) XBMC wraps cximage in a dll interface. There are some minor changes to it, in particular to jpeg loading where we resample on load. Any changes (including the dll interface) could probably be pushed upstream, assuming there is an upstream. 3) Yes, XBMC is the upstream for libRTMP. It's currently internal to XBMC. Making it an external library would be something we'd welcome. 4) libsquish has been altered only slightly, so using upstream is probably feasible. XBMC is the upstream of libUPnP which wraps Neptune (which has it's own upstream I think?) " (taken from http://trac.xbmc.org/ticket/8629#comment:21) (In reply to comment #119) > OK, I have some feedback on cximage (as well as some other libraries), from > upstream: > > "1) XBMC wraps cximage in a dll interface. This is the actual cause behind much of xbmc's bloat: Instead of using a system's dynamic loader, they implemented an application-specific dll-loader ;) (In reply to comment #120) > (In reply to comment #119) > > OK, I have some feedback on cximage (as well as some other libraries), from > > upstream: > > > > "1) XBMC wraps cximage in a dll interface. > This is the actual cause behind much of xbmc's bloat: Instead of using a > system's dynamic loader, they implemented an application-specific dll-loader ;) No doubt, but the question is, what should we do about it for the moment in terms of packaging? I can't see packaging it separately and getting xbmc to compile against it in the short-term. More clarifications from upstream are here http://trac.xbmc.org/ticket/8629#comment:23. My original questions followed by their, in some cases, somewhat terse answers (i.e. it would be good to know why unrarlib couldn't be used): 2. similarly is XBMC the defacto upstream for libcmyth? > 2) nope. i believe libcmyth upstream is active? we do patch it though. 5. can unrarlib be used in place of UnrarXlib? > 5) no. 6. would it be possible to use an external libhdhomerun? > 6) probably, doubt we have patched it much. 7. can you clarify the source/upstream/license of of libhts? > 7) http://www.lonelycoder.com/hts/, very much active. it's gplv3 so building with it enabled makes the xbmc binary gplv3. $ rpmlint xbmc-9.11-12.fc12.i686.rpm xbmc.i686: W: spelling-error %description -l en_US playlist -> play list, play-list, playtime xbmc.i686: W: spelling-error %description -l en_US slideshow -> sideshow, slide show, slide-show xbmc.i686: W: spelling-error %description -l en_US plugins -> plug ins, plug-ins, plugging xbmc.i686: W: invalid-url URL: http://www.xbmc.org/ <urlopen error timed out> xbmc.i686: W: executable-stack /usr/lib/xbmc/system/players/paplayer/SNESAPU-i486-linux.so xbmc.i686: E: non-executable-script /usr/lib/xbmc/system/python/spyce/verchk.py 0644 /usr/bin/env xbmc.i686: E: non-executable-script /usr/lib/xbmc/system/python/spyce/run_spyceCGI.py 0644 /usr/bin/env xbmc.i686: E: non-executable-script /usr/lib/xbmc/system/python/spyce/run_spyceModpy.py 0644 /usr/bin/env xbmc.i686: E: non-executable-script /usr/lib/xbmc/system/python/spyce/spyce.py 0644 /usr/bin/env xbmc.i686: E: non-executable-script /usr/lib/xbmc/system/python/spyce/run_spyceCmd.py 0644 /usr/bin/env xbmc.i686: E: non-executable-script /usr/lib/xbmc/system/python/spyce/spyceCGI.py 0644 /usr/bin/env xbmc.i686: E: non-executable-script /usr/lib/xbmc/system/python/spyce/spyceCmd.py 0644 /usr/bin/env 1 packages and 0 specfiles checked; 7 errors, 5 warnings. $ rpmlint xbmc-9.11-12.fc12.src.rpm xbmc.src: W: spelling-error %description -l en_US playlist -> play list, play-list, playtime xbmc.src: W: spelling-error %description -l en_US slideshow -> sideshow, slide show, slide-show xbmc.src: W: spelling-error %description -l en_US plugins -> plug ins, plug-ins, plugging xbmc.src: W: strange-permission xbmc-generate-tarball-xz.sh 0755 xbmc.src: W: invalid-url Source0: xbmc-9.11-patched.tar.xz 1 packages and 0 specfiles checked; 0 errors, 5 warnings. We can ignore the spelling errors, and I'm not sure why I'm consistently getting a "urlopen error timed out" on the url; it looks reasonable. The executable stack error may be a problem with SELinux, but I don't have SELinux enabled on my machines, so I can't test it. The non-executable-script are actual errors and should be fixed, either by making the scripts executable or removing their shebang lines. As for strange-permission...I think it's fine, and as you explain where we get the source from, invalid-url is also not a problem. (Note: I'm running rpmlint in an up-to-date F12 box, so I'm not sure why my output differs from yours.) (In reply to comment #123) > xbmc.i686: W: executable-stack > /usr/lib/xbmc/system/players/paplayer/SNESAPU-i486-linux.so ... > The executable stack error may be a problem with SELinux, but I don't have > SELinux enabled on my machines, so I can't test it. Is this object built from code that includes some assembly language section? If so, it's possible that this could go away simply by adding -Wa,--execstack to the compiler options. It's described in more detail here (see Appendix A): http://people.redhat.com/drepper/nonselsec.pdf The document also contains alternative approaches to fixing the problem. X = good
- = not so good
? = not sure of status
N/A = not applicable
[ X ] MUST: rpmlint must be run on every package
[ X ] MUST: The package must be named according to the Package Naming
Guidelines
[ X ] MUST: The spec file name must match the base package %{name} [...]
[ - ] MUST: The package must meet the Packaging Guidelines
There are loads of bundled libraries, some internal to the project and therefore
no problem, but others are external libraries and really should be packaged
separately. See previous ten or so comments. For guidelines, see
http://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries
[ - ] MUST: The package must be licensed with a Fedora approved license
and meet the Licensing Guidelines
While the main package is GPLv2+, some bundled libraries have...odd...licenses.
As mentioned in earlier comments, we really need to do something about them,
especially libGoAhead.
[ X ] MUST: The License field in the package spec file must match the
actual license
[ X ] MUST: The text of the license for the package must be included in %doc
[ X ] MUST: The spec file must be written in American English.
[ X ] MUST: The spec file for the package MUST be legible.
[ - ] MUST: The sources used to build the package must match the upstream
source, as provided in the spec URL.
The current source in the SRPM *doesn't* match that generated using
xbmc-generate-tarball-xz.sh. There are several directories in xbmc/lib that
shouldn't be there.
[ X ] MUST: The package MUST successfully compile and build into binary
rpms on at least one primary architecture (tested on i686, x86_64)
[ N/A ] 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.
[ X ] MUST: All build dependencies must be listed in BuildRequires, except
for any that are listed in the exceptions section of the Packaging
Guidelines.
[ ? ] MUST: The spec file MUST handle locales properly. This is done by
using the %find_lang macro. Using %{_datadir}/locale/* is strictly
forbidden
XBMC stores its language files as xml files. I'm not sure how that affects
this rule.
[ N/A ] MUST: Every binary RPM package (or subpackage) which stores shared
library files (not just symlinks) in any of the dynamic linker's
default paths, must call ldconfig in %post and %postun.
[ N/A ] MUST: If the package is designed to be relocatable, the packager must
state this fact in the request for review.
[ X ] MUST: A package must own all directories that it creates.
[ X ] MUST: A package must not contain any duplicate files in the %files
listing.
[ X ] MUST: Permissions on files must be set properly.
[ X ] MUST: Each package must have a %clean section, which contains rm -rf
$RPM_BUILD_ROOT.
[ X ] MUST: Each package must consistently use macros.
[ X ] MUST: The package must contain code, or permissable content.
[ N/A ] MUST: Large documentation files must go in a -doc subpackage.
[ X ] MUST: If a package includes something as %doc, it must not affect the
runtime of the application.
[ N/A ] MUST: Header files must be in a -devel package.
[ N/A ] MUST: Static libraries must be in a -static package.
[ N/A ] MUST: Packages containing pkgconfig(.pc) files must 'Requires:
pkgconfig'.
[ N/A ] 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.
[ N/A ] MUST: In the vast majority of cases, devel packages must require the
base package using a fully versioned dependency.
[ X ] MUST: Packages must NOT contain any .la libtool archives, these must
be removed in the spec if they are built.
[ - ] 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.
The file is installed in the proper location, but I don't see desktop-
file-install being run, which means it won't show up in the menus until the
user logs out, and then in again (at least as far as I understand it).
[ X ] MUST: Packages must not own files or directories already owned by
other packages.
[ X ] MUST: At the beginning of %install, each package MUST run rm -rf
$RPM_BUILD_ROOT.
[ X ] MUST: All filenames in rpm packages must be valid UTF-8.
[ X ] SHOULD: query upstream to include it. (At least, I'm assuming upstream is
involved based on the upstream comments in this bug report.)
[ N/A ] SHOULD: The description and summary sections in the package spec file
should contain translations for supported Non-English languages, if
available.
[ - ] SHOULD: The reviewer should test that the package builds in mock. (Don't have
a fast enough Internet connection).
[ X ] SHOULD: The package should compile and build into binary rpms on all
supported architectures.
[ X ] SHOULD: The reviewer should test that the package functions as
described. A package should not segfault instead of running, for
example.
[ N/A ] SHOULD: If scriptlets are used, those scriptlets must be sane. This is
vague, and left up to the reviewers judgement to determine sanity.
[ N/A ] SHOULD: Usually, subpackages other than devel should require the base
package using a fully versioned dependency.
[ N/A ] SHOULD: The placement of pkgconfig(.pc) files depends on their
usecase, and this is usually for development purposes, so should be
placed in a -devel pkg.
[ N/A ] SHOULD: If the package has file dependencies outside of /etc, /bin,
/sbin, /usr/bin, or /usr/sbin consider requiring the package which
provides the file instead of the file itself.
[ N/A ] SHOULD: your package should contain man pages for binaries/scripts. If
it doesn't, work with upstream to add them where they make sense.
Nuts! Inconsistent column length between comment box and bugzilla formatting! [ - ] SHOULD: The reviewer should test that the package builds in mock. I have build it in mock (i686), it builds fine. (In reply to comment #127) > [ - ] SHOULD: The reviewer should test that the package builds in mock. > > I have build it in mock (i686), it builds fine. > Excellent. We can change that to a + now. (In reply to comment #128) > (In reply to comment #127) > > [ - ] SHOULD: The reviewer should test that the package builds in mock. > > > > I have build it in mock (i686), it builds fine. > > > > Excellent. We can change that to a + now. > 1) *-12 does NOT build in FC13's mock 2) FC13 generates broken configure scripts, i.e the result is miscompiled (In reply to comment #129) > > > > Excellent. We can change that to a + now. > > > > 1) *-12 does NOT build in FC13's mock > 2) FC13 generates broken configure scripts, i.e the result is miscompiled Indeed, I need to spin a -13 version that incorporates your patches, will try to do that ASAP, and hopefully start addressing some of the other points in Jonathan's formal review checklist. (In reply to comment #117) > Created an attachment (id=379) [details] > Add Xext check to configure > This hack adds -lXext to LIBS. > Without it xbmc fails to link with a indirect shared DSO error on FC13. (In reply to comment #118) > Created an attachment (id=380) [details] > hack to work around ffmpeg sws_scale() incompatibility > > This is a brutal hack to work around xbmc failing to build against rpmfusion's > ffmpeg for F13. > This patch is more a minimally invasive hack to work around this incompatiblity > but a real fix, which likely would be to revamp xbmc's API to ffmpeg. Is it worth submitting either of these patches upstream? How should upstream solve the missing library issue? And the real fix for the ffmpeg might be fixed in xbmc SVN trunk, but I haven't checked. New version (-13), SRPM is in the process of being uploaded: * Fri Mar 5 2010 Alex Lancaster <alexlan[AT]fedoraproject org> - 9.11-13 - Patches to add -lXext to LIBS (thanks Ralf Corsepius) - Hack to work around ffmpeg sws_scale() incompatibility (thanks Ralf) - Remove bundled boost, libportaudio, libglew and extra unused zlib header from tarball - Call desktop-file-install as per review guidelines - Make spyce files executable, quiets rpmlint Spec: http://alexlan.fedorapeople.org/rpmfusion/xbmc.spec SRPM: http://alexlan.fedorapeople.org/rpmfusion/xbmc-9.11-13.fc12.src.rpm x86_64 binary: http://alexlan.fedorapeople.org/rpmfusion/xbmc-9.11-13.fc12.x86_64.rpm rpmlint ../RPMS/x86_64/xbmc-9.11-13.fc12.x86_64.rpm ../SRPMS/xbmc-9.11-13.fc12.src.rpm xbmc.x86_64: W: spelling-error %description -l en_US playlist -> play list, play-list, playtime xbmc.x86_64: W: spelling-error %description -l en_US slideshow -> sideshow, slide show, slide-show xbmc.x86_64: W: spelling-error %description -l en_US plugins -> plug ins, plug-ins, plugging xbmc.src: W: spelling-error %description -l en_US playlist -> play list, play-list, playtime xbmc.src: W: spelling-error %description -l en_US slideshow -> sideshow, slide show, slide-show xbmc.src: W: spelling-error %description -l en_US plugins -> plug ins, plug-ins, plugging xbmc.src: W: strange-permission xbmc-generate-tarball-xz.sh 0755 xbmc.src: W: invalid-url Source0: xbmc-9.11-patched.tar.xz 2 packages and 0 specfiles checked; 0 errors, 8 warnings. Some additional notes regarding review > [ - ] MUST: The package must meet the Packaging Guidelines > There are loads of bundled libraries, some internal to the project and therefore no problem, but others are external libraries and really should be packaged separately. See previous ten or so comments. For guidelines, se http://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries Many have been split out and I have removed more bundled copies of libraries and headers in -13, notably boost, libportaudio, libglew. Some of the other internal only libraries that are used either only by XBMC can probably stay there, although we are developing a firm plan to work with upstream to split them out. There is only or two packages that are currently in Fedora that XBMC could potentially use, one is libhdhomerunner, not sure if using the external version is non-trivial. Other packages like cmyth and cximage are probably non-trivial because of modifications by XBMC or stagnant upstream (like cmyth), and they aren't used by any other Fedora package as far as I am aware, so I think for this round probably safe to leave in. So for the next round, I'd like to get a sense from the reviewer of which bundles are acceptable to remain in for the moment, as getting every last one removed is not feasible for 9.11 at least since they may require non-trivial amounts of modifications to XBMC to get XBMC to recognize or compile against the "external" version. In some cases, these have been fixed in SVN upstream and waiting for 10.5 is likely to not be feasible either. > [ - ] MUST: The package must be licensed with a Fedora approved license > and meet the Licensing Guidelines > While the main package is GPLv2+, some bundled libraries have ...odd... licenses. As mentioned in earlier comments, we really need to do something about them, especially libGoAhead. I think libGoAhead is the most problematic of them, I will try to remove it in -14. libhts as noted in reply from upstream in comment #122 is GPLv3+, so should be OK to use with GPLv2+ code if I understand https://fedoraproject.org/wiki/Licensing#GPL_Compatibility_Matrix correctly, it basically means that XBMC is then effectively GPLv3. > [ - ] MUST: The sources used to build the package must match the upstream > source, as provided in the spec URL. > The current source in the SRPM *doesn't* match that generated using xbmc-generate-tarball-xz.sh. There are several directories in xbmc/lib that shouldn't be there. Should be fixed in -13. > [ ? ] MUST: The spec file MUST handle locales properly. This is done by > using the %find_lang macro. Using %{_datadir}/locale/* is strictly > forbidden > XBMC stores its language files as xml files. I'm not sure how that affects this rule. Nor I, not sure if this is that important. > [ - ] 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. > The file is installed in the proper location, but I don't see desktop-file-install being run, which means it won't show up in the menus until the user logs out, and then in again (at least as far as I understand it). Should be fixed in -13 > [ - ] SHOULD: The reviewer should test that the package builds in mock. (Don't have a fast enough Internet connection). Will attempt to try this, should work now in branched/F-13 I think with Ralf's patches for the DSO and other linker changes. (In reply to comment #132) > > [ - ] SHOULD: The reviewer should test that the package builds in mock. (Don't have a fast enough Internet connection). > > Will attempt to try this, should work now in branched/F-13 I think with Ralf's > patches for the DSO and other linker changes. OK, builds fine in mock for rawhide and branched/F-13, except that since faac has been determined to be non-free and moved to non-free (bug #780), I had to remove faac as BuildRequires. So in order to retain faac support, we would have to move xbmc to non-free as well until such time as xbmc can dlopen() faac dynamically. Does anybody have an strong opinions on this? I guess without faac support we would lose the ability to encode AAC audio. That would mean that you couldn't rip a CD to AAC, which may not be such a great loss since the CD encoding in XBMC is pretty rudimentary at best, so probably not worth moving the whole of XBMC just for that. But I think we still retain decoding support for AAC via faad2, is that correct? That would be a greater loss. New version (-14), SRPM and RPM are in the process of being uploaded: * Sat Mar 6 2010 Alex Lancaster <alexlan[AT]fedoraproject org> - 9.11-14 - Add patch that removes all webserver GoAhead functionality due to problematic license - Drop BR: faac-devel since it was moved to nonfree Spec: http://alexlan.fedorapeople.org/rpmfusion/xbmc.spec SRPM: http://alexlan.fedorapeople.org/rpmfusion/xbmc-9.11-14.fc12.src.rpm x86_64 binary: http://alexlan.fedorapeople.org/rpmfusion/xbmc-9.11-14.fc12.x86_64.rpm This fixes the problematic license issue for libGoAhead identified in review and package now builds in mock for F-13 and rawhide (F-14 which doesn't yet exist for RPM Fusion). (In reply to comment #134) > New version (-14), SRPM and RPM are in the process of being uploaded: > > * Sat Mar 6 2010 Alex Lancaster <alexlan[AT]fedoraproject org> - 9.11-14 > - Add patch that removes all webserver GoAhead functionality due to > problematic license Note that this also removes the libGoAhead directory from the source tarball too. (In reply to comment #134) > - Drop BR: faac-devel since it was moved to nonfree I consider this change to be a bad move, because you are crippling xbmc. Instead, xmbc should be moved to nonfree. Afaik, we don't patch libhdhomerun at all. Seems a reasonable candidate to break out. (In reply to comment #136) > (In reply to comment #134) > > > - Drop BR: faac-devel since it was moved to nonfree > I consider this change to be a bad move, because you are crippling xbmc. > > Instead, xmbc should be moved to nonfree. OK, clearly you have strong opinions (as I requested in comment #133), so that's one data point. Does anybody else? If there's a consensus, I'm happy to move it to non-free. Keeping a package in free is always preferable, if possible. I'm not sure how often encoding/ripping CDs using XBMC is actually done by users, since it's currently obscure and difficult for the user to actually use (see http://trac.xbmc.org/ticket/8510), and this would only mean the loss of AAC encoding, not mp3 or ogg. Of course if there are other uses for AAC encoding other than ripping CDs that XBMC can perform, then that loss would be more significant. (In reply to comment #137) > Afaik, we don't patch libhdhomerun at all. Seems a reasonable candidate to > break out. Thanks for looking into it! I wonder if, when an upstream XBMC developer adds new library is added to the trunk (especially if that library is unmodified), that there could be a rule upstream that the developer write an configure-time check for that as a matter of course, at least for Linux. It would make packaging XBMC a lot easier on us downstream packagers. (In reply to comment #138) > (In reply to comment #136) > > I consider this change to be a bad move, because you are crippling xbmc. > > > > Instead, xmbc should be moved to nonfree. > > OK, clearly you have strong opinions (as I requested in comment #133), so > that's one data point. Does anybody else? If there's a consensus, I'm happy > to move it to non-free. I have no preference either way. I won't be ripping CD's to AAC, and I'm not sure how common it is. I also have both -free and -nonfree enabled on my computer. (In reply to comment #140) > (In reply to comment #138) > > (In reply to comment #136) > > > I consider this change to be a bad move, because you are crippling xbmc. > > > > > > Instead, xmbc should be moved to nonfree. > > > > OK, clearly you have strong opinions (as I requested in comment #133), so > > that's one data point. rpmfusion's job is to provide US patent-law encumbered packages to the free world, not to cater US patent laws. I.e. I consider rpmfusion having ripped out faac encoding from most of its packages to be a failure of rpmfusion and their leadership. > > Does anybody else? If there's a consensus, I'm happy > > to move it to non-free. > > I have no preference either way. I won't be ripping CD's to AAC, and I'm not > sure how common it is. Neither do I, nor do I know what xbmc actually uses faac for, to me it's a matter principles: If rpmfusion was doing its job properly, it would ship packages with faac enabled and would not cripple their packages - as they currently do!!!!!! (In reply to comment #141) > > > OK, clearly you have strong opinions (as I requested in comment #133), so > > > that's one data point. > rpmfusion's job is to provide US patent-law encumbered packages to the free > world, not to cater US patent laws. I.e. I consider rpmfusion having ripped out > faac encoding from most of its packages to be a failure of rpmfusion and their > leadership. Actually my read on http://www.audiocoding.com/faac.html and a license review on Ubuntu: https://bugs.launchpad.net/ubuntu/+source/faac/+bug/374900 is that the issue is that the code isn't really LGPL (even if it is redistributable), rather than because of patent issues, at least not directly. But I could be wrong in my interpretation. (In reply to comment #136) > (In reply to comment #134) > > - Drop BR: faac-devel since it was moved to nonfree > I consider this change to be a bad move, because you are crippling xbmc. > Instead, xmbc should be moved to nonfree. I agree with you that xbmc should be moved to nonfree without moving faac which will give better user experience for fedora end users. The founding principle of rpmfusion is shipping softwares that have legal problems in fedora. We should be lenient with software that have the patent and licensing issues. (In reply to comment #143) > The founding principle of rpmfusion is shipping softwares that have legal > problems in fedora. We should be lenient with software that have the patent and > licensing issues. Patent issues are one thing because they don't apply in all jurisdictions, but we obviously can't distribute anything that is not distributable period, since copyright still applies and there is much more worldwide standardisation when it comes to copyright law. So we have to care about licensing issues in that sense. e.g. we have to make sure that licenses are at least compatible and the like (e.g. we can't mix GPLv2-only and GPLv3+ code). In any case, although I would dispute the "user experience" be degraded because of the issues I outlined in comment #138 (I think the effect would actually be pretty minimal), I'm happy to move to this to the non-free repo. I suspect that there may be other issues that crop up that might necessitate a move to non-free at a later point anyway. New update * Sun Mar 07 2010 Rolf Fokkens <rolf fokkens[AT]wanadoo nl> - 9.11-14 - forced hdhomerun external, had to create a hdhomerun-devel package first :-( Spec file: http://xbmc.rolffokkens.nl/xbmc/xbmc-9.11-14.fc12/xbmc.spec SRPM1: http://xbmc.rolffokkens.nl/xbmc/xbmc-9.11-14.fc12/xbmc-9.11-14.fc12/xbmc-9.11-14.fc12.src.rpm SRPM2: http://xbmc.rolffokkens.nl/xbmc/xbmc-9.11-14.fc12/xbmc-9.11-14.fc12/xbmc-9.11-14.fc12.nosrc.rpm Have a look here for the hdhomerun packages as well: http://xbmc.rolffokkens.nl/xbmc/xbmc-9.11-14.fc12/ Created a bug report for the hdhomerun problem: https://bugzilla.redhat.com/show_bug.cgi?id=571139 (In reply to comment #145) > New update > > * Sun Mar 07 2010 Rolf Fokkens <rolf fokkens[AT]wanadoo nl> - 9.11-14 > - forced hdhomerun external, had to create a hdhomerun-devel package > first :-( Thanks, but could you merge with my -14 to create a -15? Just attach your spec and patches to the bug and I'll roll the new spec/srpm with links, so that all specs/rpms are funnelled through me until such time as the package is actually in the repo. Versioning race condition, New packages at: http://xbmc.rolffokkens.nl/xbmc/xbmc-9.11-15.fc12/ Created attachment 383 [details]
Diff between Alex 9.11-14 spec and my 9.110-15 spec
This diff tells what changes I made to force the use of "external" hdhomerun
Created attachment 384 [details]
spec file for xbmc-9.11-15
Created attachment 385 [details]
new xbmc-generate-tarball script that removes hdhomerun
This script generates a tarball from the original one. This version also removes hdhomerun from the original source.
Created attachment 386 [details]
New cflags patch to work withoud hdhomerun
New patch, which works now hdhomerun has gone
Created attachment 387 [details]
patch to make XBMC use Fedora's hdhomerun instead of the included one
Several changes to make xbmc use the 'external' hdhomerun
(In reply to comment #139) > (In reply to comment #137) > > Afaik, we don't patch libhdhomerun at all. Seems a reasonable candidate to > > break out. > > Thanks for looking into it! I wonder if, when an upstream XBMC developer adds > new library is added to the trunk (especially if that library is unmodified), > that there could be a rule upstream that the developer write an configure-time > check for that as a matter of course, at least for Linux. It would make > packaging XBMC a lot easier on us downstream packagers. > Most of those libs are leftovers from the xbox days. Current way of doing is that we use system libs were its possible (so we try to move old non-patched, or just xbox patched to system libs). Some platforms need the libs though, OSX and win32, these are placed in libs and can safely be disregarded on linux, the xbmc/libs should only contain our own libs were we are upstream, although these might not be possible to move to external since they may interact with xbmc way to closely (jsonrpc is an example of this). As said, we are quite a bit in a tumble and still a lot of the xbox nastiness is left :) Cheers, Tobias Arrskog aka topfs2 (In reply to comment #132) > So for the next round, I'd like to get a sense from the reviewer of which > bundles are acceptable to remain in for the moment, as getting every last one > removed is not feasible for 9.11 at least since they may require non-trivial > amounts of modifications to XBMC to get XBMC to recognize or compile against > the "external" version. In some cases, these have been fixed in SVN upstream > and waiting for 10.5 is likely to not be feasible either. * cximage - As this has a separate upstream, I would *really* like to see this packaged separately. However, Alex, if you're willing to commit to trying to get this into Fedora after we finish getting XBMC into RPM Fusion, that would be good enough for me. * libsquish - ditto * libhts - ditto * libcmyth - if it actually has an active upstream, then ditto > I think libGoAhead is the most problematic of them, I will try to remove it in > -14. libhts as noted in reply from upstream in comment #122 is GPLv3+, so > should be OK to use with GPLv2+ code if I understand > https://fedoraproject.org/wiki/Licensing#GPL_Compatibility_Matrix correctly, it > basically means that XBMC is then effectively GPLv3. Does that mean we should be changing the listed license to GPLv3+? > > [ ? ] MUST: The spec file MUST handle locales properly. This is done by > > using the %find_lang macro. Using %{_datadir}/locale/* is strictly > > forbidden > > > XBMC stores its language files as xml files. I'm not sure how that affects this rule. > > Nor I, not sure if this is that important. Ok, let's consider this unrelated unless someone else tells us something different. (In reply to comment #155) > * cximage - As this has a separate upstream, I would *really* like to see this > packaged separately. I tried a stab at this last weekend. Unfortunately, this has proven to be tedious and difficult, for several reasons: * Upstream CxImage is a Windows only package, cluttered with and depending on Windows-proprietary stuff. * Upstream CxImage contains a hacked-up version of dcraw, which collides with the version in Fedora. * xbmc's cximage is a hacked-up version of CxImage-6.0.0, which seemingly has been brutally hacked up to make "most parts of it kind-a-compile" under non-Windows systems (i.e. it's far from being a proper port). Also, AFAICT, CxImage upstream (http://www.xdp.it) appears to be dead. The latest updates seem to be from 2008. => IMO, splitting out cximage into a separate package likely would be a major rework of cximage, which very likely would end up as a real fork. => For now, I'd say, let's keep the bundled cximage. (In reply to comment #156) > => IMO, splitting out cximage into a separate package likely would be a major > rework of cximage, which very likely would end up as a real fork. > > => For now, I'd say, let's keep the bundled cximage. Yeah, I wasn't aware it would be so nasty. Sounds good. I'll try to get to incorporating the latest comments and review feedback today or tomorrow, and hopefully we can get this through review very soon, but this is a heads-up that I will be on vacation for about 10 days after tomorrow: https://fedoraproject.org/wiki/Vacation Created attachment 390 [details] New hdhomerun patch to force usage of system hdhomerun A hdhomerun-devel package is now available in fedora-testing: https://bugzilla.redhat.com/show_bug.cgi?id=571139 This is a more recent hdhomerun version which makes the previous patch result in compile errors. This is caused by the fact that the new hdhomerun functions somtimes returns int where they did return void before. This formally is an API change, which may be an issue. I am not able to test hdhomerun, so I'm not sure. xbmc now compiles by ignoring the new int return values. Hi, just wanted to inform you that I've just finished a patch that removes the CxImage dependancy in favour for Magick++. I'm not sure if the patches will be applicable for camelot but you can reconsider breaking it out as a seperate lib since it will be deprecated soon :) Cheers, Tobias OK, back from vacation. New version (-16) which incorporates Rolf's -15 version, SRPM and RPM are in the process of being uploaded: * Wed Mar 24 2010 Alex Lancaster <alexlan[AT]fedoraproject org> - 9.11-16 - Add BuildRequires: hdhomerun-devel * Sun Mar 7 2010 Rolf Fokkens <rolf fokkens[AT]wanadoo nl> - 9.11-15 - Add patch for force using hdhomerun external, had to create a hdhomerun-devel package first Spec: http://alexlan.fedorapeople.org/rpmfusion/xbmc.spec SRPM: http://alexlan.fedorapeople.org/rpmfusion/xbmc-9.11-16.fc12.src.rpm x86_64 binary: http://alexlan.fedorapeople.org/rpmfusion/xbmc-9.11-16.fc12.x86_64.rpm This allows builds against external hdhomerun (available on F-12 in updates-testing). In response the remaining review comments: (In reply to comment #155) > * cximage - As this has a separate upstream, I would *really* like to see this > packaged separately. However, Alex, if you're willing to commit to trying to > get this into Fedora after we finish getting XBMC into RPM Fusion, that would > be good enough for me. As indicated by upstream in comment #160, looks like cximage will be going away in new release and would be very difficult to split out as indicated by Ralf, so should be OK to carry for this release only. > * libsquish - ditto > * libhts - ditto > * libcmyth - if it actually has an active upstream, then ditto I will commit to looking into these packages if they can be split out post-review. I suspect libcmyth is more or less dead upstream, hopefully upstream XBMC can tell us whether they are now the defacto upstream. > > I think libGoAhead is the most problematic of them, I will try to remove it in > > -14. libhts as noted in reply from upstream in comment #122 is GPLv3+, so > > should be OK to use with GPLv2+ code if I understand > > https://fedoraproject.org/wiki/Licensing#GPL_Compatibility_Matrix correctly, it > > basically means that XBMC is then effectively GPLv3. > > Does that mean we should be changing the listed license to GPLv3+? I think that the license refers to the source code, not necessarily to resultant binary. I think we should probably change the license to: "GPLv2+ and GPLv3+" to indicate that there are sources under both licenses. If approved, I will do this immediately before committing. > > > XBMC stores its language files as xml files. I'm not sure how that affects this rule. > Ok, let's consider this unrelated unless someone else tells us something > different. OK. I think this version should hopefully address most, if not all, of the remaining review concerns. (In reply to comment #159) > Created an attachment (id=390) [details] > New hdhomerun patch to force usage of system hdhomerun > > A hdhomerun-devel package is now available in fedora-testing: > https://bugzilla.redhat.com/show_bug.cgi?id=571139 > > This is a more recent hdhomerun version which makes the previous patch result > in compile errors. This is caused by the fact that the new hdhomerun functions > somtimes returns int where they did return void before. > > This formally is an API change, which may be an issue. I am not able to test > hdhomerun, so I'm not sure. xbmc now compiles by ignoring the new int return > values. Rolf, are these patches ready to push upstream? I'd like to submit them if possible, at least the parts that are ready for prime-time. I guess this would also include the part of the CFLAGS patch that is specific to hdhomerun (the rest of that patch has already been applied in upstream SVN). Ok, I'm just going through the items that were [ - ] in #125.
[ X ] MUST: The package must meet the Packaging Guidelines
We've gotten the worst offenders, and most of the rest are private
XBMC-specific libraries anyway.
[ X ] MUST: The package must be licensed with a Fedora approved license
and meet the Licensing Guidelines
GoAhead is now stripped from the package, so I think we're good.
[ X ] MUST: The sources used to build the package must match the upstream
source, as provided in the spec URL.
This has been fixed
[ X ] 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 has been fixed
[ X ] SHOULD: The reviewer should test that the package builds in mock.
The reviewer hasn't tested this, but others have.
This package is APPROVED!
/me looks for fedora-review flag :)
(In reply to comment #163) > Ok, I'm just going through the items that were [ - ] in #125. > > [ X ] MUST: The package must meet the Packaging Guidelines > > We've gotten the worst offenders, and most of the rest are private > XBMC-specific libraries anyway. > > [ X ] MUST: The package must be licensed with a Fedora approved license > and meet the Licensing Guidelines > > GoAhead is now stripped from the package, so I think we're good. > > [ X ] MUST: The sources used to build the package must match the upstream > source, as provided in the spec URL. > > This has been fixed > > [ X ] 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 has been fixed > > [ X ] SHOULD: The reviewer should test that the package builds in mock. > > The reviewer hasn't tested this, but others have. > > This package is APPROVED! Thanks! > /me looks for fedora-review flag :) According to: http://rpmfusion.org/Contributors#head-8354059095aca6ccb5494f9dac8815b732cfe2fd the equivalent is to switch the blocker bug from bug #2 to bug #4 (RF_ACCEPT). Package CVS request ====================== Package Name: xbmc Short Description: Media center Owners: alexlan Branches: F-12 F-13 InitialCC: ---------------------- License tag: free (Note: initially creating in the free-repo, for the moment. If I get requests from users to enable the faac functionality, I will switch this to the nonfree repo) These patches are ready to push upstream AFAIK, but I think upstream may not appreciate the fact that it isn't integrated in automake/autoconf. It's just a straightforward way to make XBMC build on F12 using external hdhomerun. I trust upstream improves the patches after being pushed, though the may also just reject them. (In reply to comment #162) > > New hdhomerun patch to force usage of system hdhomerun > Rolf, are these patches ready to push upstream? I'd like to submit them if > possible, at least the parts that are ready for prime-time. I guess this would > also include the part of the CFLAGS patch that is specific to hdhomerun (the > rest of that patch has already been applied in upstream SVN). (In reply to comment #166) > These patches are ready to push upstream AFAIK, but I think upstream may not > appreciate the fact that it isn't integrated in automake/autoconf. It's just a > straightforward way to make XBMC build on F12 using external hdhomerun. > > I trust upstream improves the patches after being pushed, though the may also > just reject them. Ah, yes, looking at them again, it seems that your patch doesn't detect the presence of the hdhomerun headers using autoconf. Ideally it would detect the headers and use them if present and fallback to using the internal one if not present, at the moment you simply remove the ability to use the internal one. This would necessitate adding corresponding --enable-hdhomerun in configure.in, I wonder if you'd be willing to emulate how it is done for other libraries like ffmpeg. A patch like stands a better chance of being accepted upstream because it wouldn't affect the default behaviour. Also now that I look at them, it seems that a couple of your other patches could be modified in a similar way, specifically xbmc-9.11-b1-dvdlibs-external.patch and xbmc-9.11-remlibass.patch, since they both remove internal functionality rather than conditionally detect it. OK, CVS import was successful and initial builds have successfully completed: rawhide: http://buildsys.rpmfusion.org/build-status/job.psp?uid=6673 F-12: http://buildsys.rpmfusion.org/build-status/job.psp?uid=6646 These are now in the needsign queue, so closing bug. (Please note that all F-13 builds for all rpmfusion packages are currently stalled in the buildsys, but will hopefully be restarted soon). Thanks to everyone who helped with this monster review! For any further issues, please open up new bugs against the newly-created "xbmc" component. I may open up a few soon to track some of the ongoing issues. (In reply to comment #167) > I wonder if you'd be willing to emulate how it is done for other libraries like > ffmpeg. A patch like stands a better chance of being accepted upstream > because it wouldn't affect the default behaviour. If I'd understand what I was doing, I'd be more than willing to create a better upstream patch. However I don't know anything of automake and autoconf, and learning these is beyond my goal to help get XBMC in RPMfusion. So for the moment I'd hope that the upstream guys pick it up. If not, I may try to do so in the future if I somehow find the time.... Hello. I've got a little bug with xbmc 9.11-18.fc12 installed from the repo rpmfusion testing in fedora 12 : when I play original video dvd with subtitle, the first subtitle of each new chapter disappear or is not at the good time... There's no problems with subtitles for avi files. Thanks in advance for the solution. (In reply to comment #170) > Hello. > I've got a little bug with xbmc 9.11-18.fc12 installed from the repo rpmfusion > testing in fedora 12 : when I play original video dvd with subtitle, the first > subtitle of each new chapter disappear or is not at the good time... There's no > problems with subtitles for avi files. > Thanks in advance for the solution. Please open up a bug against the actual "xbmc" component now on bugzilla.rpmfusion.org. This bug relates to the packaging effort only and is now closed. In addition, unless you know that this issue is Fedora/RPM Fusion-specific, it's best to also report these kinds of issues with XBMC upstream (and post a link to that upstream bug on the RPM Fusion bug) as I'm unlikely to be able fix things in the core code, but may be able to fix issues that relate to packaging errors. Package Change Request ====================== Package Name: xbmc New Branches: EL-6 Updated RPMFusion Owners: alexlan,ktdreyer Updated EPEL Owners: alexlan,ktdreyer Updated EPEL CC: alexlan,ktdreyer As discussed in Bug 2339 I'll be co-maintaining the EL-6 branch. Package Change Request ====================== Package Name: xbmc New Branches: Updated RPMFusion Owners: alexlan,ktdreyer,maci Updated EPEL Owners: alexlan,ktdreyer,maci Updated EPEL CC: alexlan,ktdreyer,maci Package Change Request ====================== Package Name: xbmc New Branches: Updated RPMFusion Owners: alexlan,ktdreyer,maci,mooninite Updated EPEL Owners: alexlan,ktdreyer,maci Updated EPEL CC: alexlan,ktdreyer,maci As discussed in bug 3088 I would like to maintain the Fedora package. I am a Fedora packager (FAS name: mooninite). Package Change Request ====================== Package Name: xbmc New Branches: Updated RPMFusion Owners: mooninite,ktdreyer,maci,alexlan Updated EPEL Owners: maci,ktdreyer,alexlan Updated EPEL CC: maci,ktdreyer,alexlan As mentioned on Bug 3231 I would like mooninite (Mike Cronenworth) to be moved up to primary maintainer for RPM Fusion. |