Bug 2161

Some sporadic remarks after a quick glance:

One of the good things with svn is it's unique version nr. Wouldn't it might make sense to include it in the release tag .(20120128svn73976) and  in the commented svn checkout: 

#svn checkout -r 73976 https://pcsxr.svn.codeplex.com/svn/pcsxr

Also, the source comment is sort of incomplete: checking out the svn repo  doesn't give you the zip archive...

Am I wrong thinking you need "Requires(postun): gtk2" and "Requires(postrans): gtk2"?

BTW,33 errors is a bit clumsy even if they could be ignored. And, in ~/bin lives fsf-fix.sh, which never, ever will be shown to anyone. As a result, there is http://pastebin.com/Z9yCQUct, a patch which fixes everything besides COPYING. Use it as you like, possibly forget or trash it. 

There are similar problems in the .po files, but rmplint doen't care about them and they thus  not fixed.

[while doing other things]
The license is not just GPLv2. Using 'licensecheck' I find no GPLv2, but instead at least GPLv2+, GPLv3+, BSD, LGPLv2,1+, public domain, a possible MIT-variant...

Many (most?) of these seems to be in the various macos/win32 plugins - these can just be removed from the source I guess(?) But even so, there are more than GPLv2, possibly requiring a license breakdown IMHO.

Note the file debian-upstream/copyright which explicitly states the license to be  GPLv2+, not GPLv2. However, I guess at least the GPLv3+ files means a need for a clever solution. 

While removing things from the source tarball, you might consider taking away debian-upstream as well; it's not really useful here...

(In reply to comment #1)
> Some sporadic remarks after a quick glance:
> 
> One of the good things with svn is it's unique version nr. Wouldn't it might
> make sense to include it in the release tag .(20120128svn73976) and  in the
> commented svn checkout: 

Good point, I will make this change; Fedora's version guidelines are a little vague, hence why I omitted the revision number.

> #svn checkout -r 73976 https://pcsxr.svn.codeplex.com/svn/pcsxr
> 
> Also, the source comment is sort of incomplete: checking out the svn repo 
> doesn't give you the zip archive...

I was under the impression it was assumed it would then need to be zipped. If you can provide me an example used in a current RPMFusion or Fedora package, I will be fully willing to change it. I will clarify it does not give a zip file else wise or for the time being.

> Am I wrong thinking you need "Requires(postun): gtk2" and "Requires(postrans):
> gtk2"?

I don't think it is needed, but I will check to make sure when I have a chance.

(In reply to comment #2)
> BTW,33 errors is a bit clumsy even if they could be ignored. And, in ~/bin
> lives fsf-fix.sh, which never, ever will be shown to anyone. As a result, there
> is http://pastebin.com/Z9yCQUct, a patch which fixes everything besides
> COPYING. Use it as you like, possibly forget or trash it. 
> 
> There are similar problems in the .po files, but rmplint doen't care about them
> and they thus  not fixed.

I've been strongly advised not to try and fix source code unless it causes it not to build. As well, I don't have interest to fix bugs, I will report this upstream and as soon as it's fixed, update the source to that revision.
Also can you elaborate on the issues with the .po files? The only license I see in the *.po files are "This file is distributed under the same license as the pcsxr package."

(In reply to comment #3)
> [while doing other things]
> The license is not just GPLv2. Using 'licensecheck' I find no GPLv2, but
> instead at least GPLv2+, GPLv3+, BSD, LGPLv2,1+, public domain, a possible
> MIT-variant...

My apologies, I meant to put GPLv2+, not GPLv2. I seem to have been a little too hasty when posted this spec. As well, I seem to have forgotten to run licensecheck to make sure the debian-upstream/copyright was correct

> Many (most?) of these seems to be in the various macos/win32 plugins - these
> can just be removed from the source I guess(?) But even so, there are more than
> GPLv2, possibly requiring a license breakdown IMHO.

> Note the file debian-upstream/copyright which explicitly states the license to
> be  GPLv2+, not GPLv2. However, I guess at least the GPLv3+ files means a need
> for a clever solution. 

Worst case scenario is that I ask upstream to fix this issue, or ask permission to distribute it all under GPLv3+ or all conflicting licenses under a version of LGPL. As well, I agree, I should have a look through the source and remove all unnecessary files in order to simplify things, including a custom spin of the source.

(In reply to comment #4)
> (In reply to comment #1)
> > #svn checkout -r 73976 https://pcsxr.svn.codeplex.com/svn/pcsxr
> > 
> > Also, the source comment is sort of incomplete: checking out the svn repo 
> > doesn't give you the zip archive...
> 
> I was under the impression it was assumed it would then need to be zipped. If
> you can provide me an example used in a current RPMFusion or Fedora package, I
> will be fully willing to change it. I will clarify it does not give a zip file
> else wise or for the time being.

Basically what we / other maintainers need is the full set of commands that you ran to arrive at a zip file. For example:

# export SVNVERSION=73976
# svn checkout -r $SVNVERSION https://pcsxr.svn.codeplex.com/svn/pcsxr
# zip pcsxr-$SVNVERSION.zip pcsxr/trunk/

...or whatever you used.

(In reply to comment #4)
> (In reply to comment #1)
> 
> > #svn checkout -r 73976 https://pcsxr.svn.codeplex.com/svn/pcsxr
> > 
> > Also, the source comment is sort of incomplete: checking out the svn repo 
> > doesn't give you the zip archive...
> 
> I was under the impression it was assumed it would then need to be zipped. If
> you can provide me an example used in a current RPMFusion or Fedora package, I
> will be fully willing to change it. I will clarify it does not give a zip file
> else wise or for the time being.
If you look into http://fedoraproject.org/wiki/Packaging:SourceURL#Using_Revision_Control there's an example. I would just add something like
# zip --exclude \*.svn -r %{source} pcsxr

> 
> (In reply to comment #2)
> > BTW,33 errors is a bit clumsy even if they could be ignored. And, in ~/bin
> > lives fsf-fix.sh, which never, ever will be shown to anyone. As a result, there
> > is http://pastebin.com/Z9yCQUct, a patch which fixes everything besides
> > COPYING. Use it as you like, possibly forget or trash it. 
> > 
> > There are similar problems in the .po files, but rmplint doen't care about them
> > and they thus  not fixed.
> 
> I've been strongly advised not to try and fix source code unless it causes it
> not to build.
By whom? Packaging people? Upstream? From packaging point of view I think patching is a perfectly sane thing to do, besides COPYING. But this is anyway  a minor issue; it's just a little easier to handle rpmlint w/o 33 errors :)

> As well, I don't have interest to fix bugs, I will report this
> upstream and as soon as it's fixed, update the source to that revision.
Then send them the patch, it might speed up things.

> Also can you elaborate on the issues with the .po files? The only license I see
It's the AboutDlg key, containing the complete license, with wrong address (ZIP).

> 
> (In reply to comment #3)
> > [while doing other things]
> > The license is not just GPLv2. Using 'licensecheck' I find no GPLv2, but
> > instead at least GPLv2+, GPLv3+, BSD, LGPLv2,1+, public domain, a possible
> > MIT-variant...
> 
> My apologies, I meant to put GPLv2+, not GPLv2. I seem to have been a little
> too hasty when posted this spec. As well, I seem to have forgotten to run
> licensecheck to make sure the debian-upstream/copyright was correct

No apologies, were just trying to make a job together. Deal?

[cut]
> Worst case scenario is that I ask upstream to fix this issue, or ask permission
> to distribute it all under GPLv3+ or all conflicting licenses under a version
> of LGPL. As well, I agree, I should have a look through the source and remove
> all unnecessary files in order to simplify things, including a custom spin of
> the source.

Licenses: there's also the option to ship conflicting licenses using sub-packages

Licenses: Since the bulk of the code is GPLv2+, you don't need a upstream permission to distribute the complete package under GPLv3, I guess. But the question is if the other licenses are GPL3-compatible...

(In reply to comment #6)
> 
> 
> If you look into
> http://fedoraproject.org/wiki/Packaging:SourceURL#Using_Revision_Control
> there's an example. I would just add something like
> # zip --exclude \*.svn -r %{source} pcsxr

Fair enough, I'll include the command.

> By whom? Packaging people? Upstream? From packaging point of view I think
> patching is a perfectly sane thing to do, besides COPYING. But this is anyway 
> a minor issue; it's just a little easier to handle rpmlint w/o 33 errors :)

Actually by a Fedora Sponsor, I was told to try to get it fixed upstream and if it didn't take, resort to patches in the RPM.

> It's the AboutDlg key, containing the complete license, with wrong address
> (ZIP).

Okay, I'll include this in another bug report upstream. I was a little confused when I couldn't see it digging through the *.po files ;)
 
> No apologies, were just trying to make a job together. Deal?

Course :)
 
> Licenses: there's also the option to ship conflicting licenses using
> sub-packages

Well as I said, worst case scenario, I'll have to dig through the source to figure out what needs what and act accordingly. Best case scenario is isolating all the messy licenses in separated packages of course. Plugins would be the easiest to split up.

(In reply to comment #8)
> Licenses: Since the bulk of the code is GPLv2+, you don't need a upstream
> permission to distribute the complete package under GPLv3, I guess. But the
> question is if the other licenses are GPL3-compatible...

Yep, I'll take a look when I get a chance; it's a bit of a tedious job and I haven't had a lot of time to do it.

(In reply to comment #1)
> Am I wrong thinking you need "Requires(postun): gtk2" and "Requires(postrans):
> gtk2"?

I'm sure I saw something about this somewhere, as I read it a while ago but I found it after a quick search; according to fedora's guidelines, it's unnecessary:
http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Icon_Cache


Also I looked all the way through the source code and after removing the win32 and macosx source, I only

found GPLv2+, GPLv3+, Modified BSD License and Public Domain

All of which is compatible with each other if the GPLv2+ parts are distributed with GPLv3+.

Only SOURCE/plugins/gxvideo/* is GPLv3+, so if distributing the GPLv2+ code as GPLv3+ is an issue (although I don't why it would be) this plugin can be packaged separately.

I have updated the package with the suggested changes, but the FSF ADDRESS issue still exists, I will report it upstream. If I don't get a response or they don't plan to fix it, I will implement the a patch in this package.

SPEC:
http://dl.dropbox.com/u/42480493/pcsxr.spec
SPRM:
http://dl.dropbox.com/u/42480493/pcsxr-1.9.92-1.20120128svn.fc16.src.rpmi

Why are you creating a *zip?

*.zips are quite unusual/obscure on Linux and are 
inefficient in comparision to modern compressions:

# ls -s1 pcsxr*xz pcsxr-*zip
1196 pcsxr-73976.tar.xz
2220 pcsxr-73976.zip

Compare also the "Smallest Compressed Archive"
https://fedoraproject.org/wiki/Packaging:SourceURL#Referencing_Source

Provided you are manually building the source-packages, feel encouraged to use tar.xz.

The commments on how to retrieve the source: since the License tag (and related conflicts) only applies to the binary RPM:s, I think you should move the code cleanup to the %prep section. Something like

%prep
%setup -q
chmod 755 plugins/dfinput/util.*
rm -rf macosx win32 debian-upstream

This way manual source generation becomes simpler, and when upstream releases a usable version which can be stated as a %source url it will "Just Work"(tm).

The patch/not patch discussion: simple misunderstanding. You send report/patches upstream, and chooses to patch or not later. Agreed. We need the upstream reference before this is over.

License breakdown: done in comments, which basically is fine. However, you miss some of the information in debian/copyright, notably the note on psemu_plugin_defs. Personally, I think I would have created a new file like
LICENSES based on the debian stuff, but I guess either way is OK.

Appolgies for my late response, as I've been extremely busy.

(In reply to comment #13)
> Why are you creating a *zip?
> 
> *.zips are quite unusual/obscure on Linux and are 
> inefficient in comparision to modern compressions:
> 
> # ls -s1 pcsxr*xz pcsxr-*zip
> 1196 pcsxr-73976.tar.xz
> 2220 pcsxr-73976.zip
> 
> Compare also the "Smallest Compressed Archive"
> https://fedoraproject.org/wiki/Packaging:SourceURL#Referencing_Source
> 
> Provided you are manually building the source-packages, feel encouraged to use
> tar.xz.

Good point, I'll used xz instead.

(In reply to comment #14)
> The commments on how to retrieve the source: since the License tag (and related
> conflicts) only applies to the binary RPM:s, I think you should move the code
> cleanup to the %prep section. Something like
> 
> %prep
> %setup -q
> chmod 755 plugins/dfinput/util.*
> rm -rf macosx win32 debian-upstream
>
> This way manual source generation becomes simpler, and when upstream releases a
> usable version which can be stated as a %source url it will "Just Work"(tm).

I can do that, but I'll have to note in the comments that the source will be in zip instead of tar.xz. I think that rc040203@freenet.de has a point and I should be using a better compression method, although if it would be more consistent to use zip, I can do that too.
 
> The patch/not patch discussion: simple misunderstanding. You send
> report/patches upstream, and chooses to patch or not later. Agreed. We need the
> upstream reference before this is over.

Okay, duly noted; I have requested it to be fixed, along with the list of files and the changes that need to be made. I haven't made a patch though.

> License breakdown: done in comments, which basically is fine. However, you miss
> some of the information in debian/copyright, notably the note on
> psemu_plugin_defs. Personally, I think I would have created a new file like
> LICENSES based on the debian stuff, but I guess either way is OK.

Oh sorry bout that, I seem to have omitted that by accident, I'll change it right away. I don't see it as all that necessary for a LICENSES file and I'd rather avoid adding anything that would be unnecessary. Although if a breakdown in the SPEC isn't enough, I'll be sure to include such file. Given that the plugin defs file no longer has a header, this doesn't seem like it will create confusion.

Here's the updated files:
SPEC:
http://dl.dropbox.com/u/42480493/pcsxr.spec
SRPM:
http://dl.dropbox.com/u/42480493/pcsxr-1.9.92-2.20120128svn73976.fc16.src.rpm

Looks fine to me, without doing a complete review. Open issues are about comments:

- Regression: the commands used to create the source are missing again. When you add them, make sure that remove the .svn directories from source; either by svn -export or by using --exclude in the tar command. See http://fedoraproject.org/wiki/Common_Rpmlint_issues#version-control-internal-file
- The source situation is perhaps better explained by a commented, correct Source: line IMHO.

If you want, I can make a more complete review. And I would be extremely happy if you could review "my" bug 2140 later, but there are updates pending  for that right now.

(In reply to comment #16)
> Looks fine to me, without doing a complete review. Open issues are about
> comments:
> 
> - Regression: the commands used to create the source are missing again.

I removed the source svn command because I thought the download link would suffice. If you think both would be better, I can re-add it.

> - The source situation is perhaps better explained by a commented, correct
> Source: line IMHO.

What would you suggest? I'm a little confused on what would make things more clear.

> If you want, I can make a more complete review. And I would be extremely happy
> if you could review "my" bug 2140 later, but there are updates pending  for
> that right now.

Sure, I can do a review when it updates if you would review this.

OK, misunderstood a little. I would do

#The source can be downloaded here
#http://pcsxr.codeplex.com/SourceControl/changeset/changes/73976
Source:         %{name}-73976.zip

i. e., let the Source: match the realities.  We wish it was something else, but this is how it is.

I'll make a full review later. Let this remark wait until it's done to save some work :)

Thanks for your help.

Here you go :)
SPEC:
http://dl.dropbox.com/u/42480493/pcsxr.spec
SRPM:
http://dl.dropbox.com/u/42480493/pcsxr-1.9.92-3.20120128svn73976.fc16.src.rpm

Sorry for delay, too much work, too little time...

rpmlint:
pcsxr.spec: W: invalid-url Source0: pcsxr-73976.zip
pcsxr.i686: W: executable-stack /usr/lib/games/psemu/libDFXVideo.so
pcsxr.i686: W: executable-stack /usr/lib/games/psemu/libGXVideo.so
pcsxr.i686: E: incorrect-fsf-address /usr/share/doc/pcsxr-1.9.92/COPYING
pcsxr-debuginfo.i686: E: incorrect-fsf-address /usr/src/debug/pcsxr/plugins/gxvideo/draw.c
[Repeated ~28 times]

[OK] Package meets naming and packaging guidelines
[OK] Spec file matches base package name.
[E ] Spec has consistant macro usage.
[OK] Meets Packaging Guidelines.
[OK] License
[OK] License field in spec matches
[OK] License file included in package
[OK] Spec in American English
[OK] Spec is legible.
[- ] Sources match upstream md5sum:
        NA, svn gives different sums for different checkouts.
        diff -r is OK.
[OK] Package needs ExcludeArch
[OK] BuildRequires correct
[OK] Spec handles locales/find_lang
[- ] Package is relocatable and has a reason to be.
[OK] Package is code or permissible content.
[- ] Doc subpackage needed/used.
[OK ]Packages %doc files don't affect runtime.

[OK] Package is a GUI app and has a .desktop file

[OK ]Package compiles and builds on at least one arch.
[OK ] Package has no duplicate files in %files.
[OK] Package doesn't own any directories other packages own.
[OK]Package owns all the directories it creates.
[E ] No rpmlint output.
- final provides and requires are sane:
    ~/rpmbuild/RPMS/i686/pcsxr-debuginfo-1.9.92-3.20120128svn73976.fc15.i686.rpm
    pcsxr-debuginfo = 1.9.92-3.20120128svn73976.fc15
    pcsxr-debuginfo(x86-32) = 1.9.92-3.20120128svn73976.fc15
    =

    ~/rpmbuild/RPMS/i686/pcsxr-1.9.92-3.20120128svn73976.fc15.i686.rpm
    libBladeSio1.so
    libDFCdrom.so
    libDFInput.so
    libDFNet.so
    libDFSound.so
    libDFXVideo.so
    libGXVideo.so
    libpeopsxgl.so
    pcsxr = 1.9.92-3.20120128svn73976.fc15
    pcsxr(x86-32) = 1.9.92-3.20120128svn73976.fc15
    =
    /bin/sh
    libGL.so.1
    libSDL-1.2.so.0
    libX11.so.6
    libXext.so.6
    libXtst.so.6
    libXv.so.1
    libXxf86vm.so.1
    libatk-1.0.so.0
    libc.so.6
    libcairo.so.2
    libcdio.so.12
    libcdio.so.12(CDIO_12)
    libdl.so.2
    libfontconfig.so.1
    libfreetype.so.6
    libgdk-x11-2.0.so.0
    libgdk_pixbuf-2.0.so.0
    libgio-2.0.so.0
    libglade-2.0.so.0
    libglib-2.0.so.0
    libgmodule-2.0.so.0
    libgobject-2.0.so.0
                                                                                                                        
SHOULD Items:

[OK] Should build in mock.
      http://koji.fedoraproject.org/koji/taskinfo?taskID=3790345 (f17)
[-] Should build on all supported archs
      Not tested
[OK] Should function as described.
       Starts, the menus are functional, but no more tests.
[OK] Should have sane scriptlets.
[- ] Should have subpackages require base package with fully versioned depend.
[OK] Should have dist tag
[OK] Should package latest version 

ISSUES
1 Rpmlint: The executable-stack warning is strange and requires at 
  least an explanation. Emulator asm code?  Other output can be ignored.
2 If possible, the note about issues w OpenGL 64-bit support should have a 
  link to bugtracker or so, in order to know when it's solved.
3 The svn version is used in several places, better handled with a macro.

ISSUES (cont)
4 In the License: comments you refer to debian-upstream/copyright, which is not part of the package.

(In reply to comment #20)
> rpmlint:
> pcsxr.spec: W: invalid-url Source0: pcsxr-73976.zip
> pcsxr.i686: W: executable-stack /usr/lib/games/psemu/libDFXVideo.so
> pcsxr.i686: W: executable-stack /usr/lib/games/psemu/libGXVideo.so
> pcsxr.i686: E: incorrect-fsf-address /usr/share/doc/pcsxr-1.9.92/COPYING
> pcsxr-debuginfo.i686: E: incorrect-fsf-address
> /usr/src/debug/pcsxr/plugins/gxvideo/draw.c
> [Repeated ~28 times]
> 
> [OK] Package meets naming and packaging guidelines
> [OK] Spec file matches base package name.
> [E ] Spec has consistant macro usage.
> [OK] Meets Packaging Guidelines.
> [OK] License
> [OK] License field in spec matches
> [OK] License file included in package
> [OK] Spec in American English
> [OK] Spec is legible.
> [- ] Sources match upstream md5sum:
>         NA, svn gives different sums for different checkouts.
>         diff -r is OK.
> [OK] Package needs ExcludeArch
> [OK] BuildRequires correct
> [OK] Spec handles locales/find_lang
> [- ] Package is relocatable and has a reason to be.
> [OK] Package is code or permissible content.
> [- ] Doc subpackage needed/used.
> [OK ]Packages %doc files don't affect runtime.
> 
> [OK] Package is a GUI app and has a .desktop file
> 
> [OK ]Package compiles and builds on at least one arch.
> [OK ] Package has no duplicate files in %files.
> [OK] Package doesn't own any directories other packages own.
> [OK]Package owns all the directories it creates.
> [E ] No rpmlint output.
> - final provides and requires are sane:
>    
> ~/rpmbuild/RPMS/i686/pcsxr-debuginfo-1.9.92-3.20120128svn73976.fc15.i686.rpm
>     pcsxr-debuginfo = 1.9.92-3.20120128svn73976.fc15
>     pcsxr-debuginfo(x86-32) = 1.9.92-3.20120128svn73976.fc15
>     =
> 
>     ~/rpmbuild/RPMS/i686/pcsxr-1.9.92-3.20120128svn73976.fc15.i686.rpm
>     libBladeSio1.so
>     libDFCdrom.so
>     libDFInput.so
>     libDFNet.so
>     libDFSound.so
>     libDFXVideo.so
>     libGXVideo.so
>     libpeopsxgl.so
>     pcsxr = 1.9.92-3.20120128svn73976.fc15
>     pcsxr(x86-32) = 1.9.92-3.20120128svn73976.fc15
>     =
>     /bin/sh
>     libGL.so.1
>     libSDL-1.2.so.0
>     libX11.so.6
>     libXext.so.6
>     libXtst.so.6
>     libXv.so.1
>     libXxf86vm.so.1
>     libatk-1.0.so.0
>     libc.so.6
>     libcairo.so.2
>     libcdio.so.12
>     libcdio.so.12(CDIO_12)
>     libdl.so.2
>     libfontconfig.so.1
>     libfreetype.so.6
>     libgdk-x11-2.0.so.0
>     libgdk_pixbuf-2.0.so.0
>     libgio-2.0.so.0
>     libglade-2.0.so.0
>     libglib-2.0.so.0
>     libgmodule-2.0.so.0
>     libgobject-2.0.so.0
> 
> SHOULD Items:
> 
> [OK] Should build in mock.
>       http://koji.fedoraproject.org/koji/taskinfo?taskID=3790345 (f17)
> [-] Should build on all supported archs
>       Not tested
> [OK] Should function as described.
>        Starts, the menus are functional, but no more tests.
> [OK] Should have sane scriptlets.
> [- ] Should have subpackages require base package with fully versioned depend.
> [OK] Should have dist tag
> [OK] Should package latest version 
> 
> ISSUES
> 1 Rpmlint: The executable-stack warning is strange and requires at 
>   least an explanation. Emulator asm code?  Other output can be ignored.

I'm almost certain this is the case, but I will look into it to make sure.

> 2 If possible, the note about issues w OpenGL 64-bit support should have a 
>   link to bugtracker or so, in order to know when it's solved.

I've been looking for the report upstream but I'll be sure to include it.

> 3 The svn version is used in several places, better handled with a macro.

Good point, it would be useful indeed.

(In reply to comment #21)
> ISSUES (cont)
> 4 In the License: comments you refer to debian-upstream/copyright, which is not
> part of the package.

Yeah I guess I didn't think that through, to be honest it's straightforward enough that I think it's fairly redundant to even have the reference in there.


On a side note, I got an email from a dev, the FSF issue should be fixed in the latest svn, so I'll post a new package when I have some time.


(In reply to comment #20)
> Sorry for delay, too much work, too little time...
> 

Yeah I know what you mean; I've been horribly busy, so I haven't had time to review any of you're packages yet. I'll be sure to review it this week though.

Nit-pickinng, indeed. Since my point is that the copyright contains something valuable  I'll do:

%prep
cp debian-upstream/copyright .
rm -rf debian-ujpstream

%files
%doc copyright

Sorry if I'm to obvious, as is certainly the case. And take your time to find a opportunity to review :)

(In reply to comment #23)
> Nit-pickinng, indeed. Since my point is that the copyright contains something
> valuable  I'll do:
> 
> %prep
> cp debian-upstream/copyright .
> rm -rf debian-ujpstream
> 
> %files
> %doc copyright

Actually I'm a little confused as of why I put that note in there in the first place, as the copyright file is a little inapplicable. It's mostly for debian purposes, a rehash of the AUTHORS file, and to help clear up the confusion of why psemu_plugin_defs.h is public domain despite it's licensing header. It seems that the pcsxr team removed this header a long time ago, so adding the copyright file may make things even more confusion haha.
Anyway, I think it would be best just not to include any reference to this file, as the COPYING file, the AUTHORS file and the license breakdown in the spec provide all the useful information in a more applicable fashion.


Anyway, here's the new spec and SRPM:
SPEC:
http://dl.dropbox.com/u/42480493/pcsxr.spec
SPRM:
http://dl.dropbox.com/u/42480493/pcsxr-1.9.92-1.20120219svn75156.fc16.src.rpm

Notes:
* All but 2 FSF-address warnings have been fixed, I have re-opened my bug report to let the dev's know that they missed these two files.

* As well compiling natively and on mock, i686 and x86-64, does not give me the executable-stack warning what so ever, so I can't say what is causing this. I would think it could be ASM as pcsxr does include a small amount of ASM code, but I don't even know what file is giving the error.

* As for the opengl issue, it seems I was wrong about that. After doing some testing myself, the issue is only a minor resolution bug that occurs on both architectures. I have reported the bug upstream and enabled opengl on both arch's.

Also, Alec are you sponsored in Fedora or RPMFusion at the moment?
I believe if you do not have either yet, this review is unofficial, although I still really appreciate the help :).
I can contact my sponsor to see if I can make it official. I would still recommend making an account if you haven't done so already:
http://rpmfusion.org/Contributors#head-643ba68e754c44d2db673ad9aea6a843ad0b7fb3
Note that I'm fairly certain you can't join the cvsextras group until you are sponsored in either Fedora or RPMFusion.

Since 2140 is blocked by RH#790628, I'll add myself to the CC and wait for that bug to be cleared first. If you have any other packages on RPMFusion, I can review them as well, as I am looking for someone to review my dolphin-emu package (Bug 2098).

Upstream seemed to have responded very quickly to my bug report for the FSF address issues, so I updated the spec to the latest SVN:

SPEC:
http://dl.dropbox.com/u/42480493/pcsxr.spec
SPRM:
http://dl.dropbox.com/u/42480493/pcsxr-1.9.92-1.20120219svn75200.fc16.src.rpm

No, I'm not. And I'm terribly sorry I didn't say so in my very first remark. I really should have, and I normally do. I just slipped, while thinking you knew.  What's even worse is that I thought we were in the same situation, both requiring a formal review later without checking that as well.

Bottom line: this review is informal.

And because of that, the "deal" between us (review for review) is not fair. I would of course appreciate if you could handle it later, but feel free to unassign it if you don't have the time or interest.

OTOH: it might take some time before ASL is completed :)

OTOH, I just got sponsored in Fedora. So I should be able to approve this once the administrative processes clears me for rpmfusion as well. Stay tuned :)

That's great news, congrats! :)

The %define (first line) should be replaced with %global (see guidelines, macros section if in doubt). With this change:

***APPROVED

You forgot to assign yourself ;)

But thanks! Much appreciated, I'm still waiting on your request, but feel free send me an email if you need a review in the future :)

Package CVS request
======================
Package Name: pcsxr
Short Description: A plugin based PlayStation (PSX) emulator with high compatibility
Owners: jem256 (Jeremy Newton)
Branches: F-15, F-16, F-17, devel
InitialCC:
----------------------
License tag: free

Both stable Builds have succeed, so I'm closing this. :)

(In reply to comment #30)
> The %define (first line) should be replaced with %global (see guidelines,
> macros section if in doubt). With this change:
> 

I'll make this change in the next update as it's not dire, thanks!