Bug 657577

I assume, the hack is useless without SDL installed. Please require SDL.

You may consider to build and install explicitly, since it requires about as many lines as the sed approach does.

Writing "./build" and "./install" should be fine as well.

Please give a rationale, why you choose to create an own directory for the library to install to.

You can simplify the files section to:

%{_bindir}/*
%{_libdir}/%{name}
%{_mandir}/man1/*

I don't know if this package makes sense for EPEL. If you don't want to introduce it there, you can drop the buildroot, the clean section and the rm for buildroot in the install section.

(In reply to comment #1)
> I assume, the hack is useless without SDL installed. Please require SDL.
> 
> You may consider to build and install explicitly, since it requires about as
> many lines as the sed approach does.
> 
> Writing "./build" and "./install" should be fine as well.
> 

- You are right.

> Please give a rationale, why you choose to create an own directory for the
> library to install to.
> 

- since the library isn't intended to be used by any other program, it should go in a private dir.

> You can simplify the files section to:
> 
> %{_bindir}/*
> %{_libdir}/%{name}
> %{_mandir}/man1/*
> 
> I don't know if this package makes sense for EPEL. If you don't want to
> introduce it there, you can drop the buildroot, the clean section and the rm
> for buildroot in the install section.

- I thinking keeping the buildroot and clean is always a good idea, no harm in keeping these.

(In reply to comment #1)

> 
> You can simplify the files section to:
> 
> %{_bindir}/*
> %{_libdir}/%{name}
> %{_mandir}/man1/*
> 

I don't where the simplification is.

A second thought about Requiring SDL.

The program is basically overriding some calls in SDL, so I don't see why it should require it.

Yes, keeping buildroot and clean doesn't harm.

Please add the rationale as a comment to the spec file. Maybe let upstream know.

The simplification is just a little more general syntax. It doesn't change the behaviour. No need to change it.

What would be the use to override something that isn't there?

Or is it useful all on its own?

(In reply to comment #6)
> Or is it useful all on its own?

The application that we will be forced to minimize should be using SDL. So SDL is brought through the application.

The answer to your question is : sdlhack is not useful on its own, it becomes useful when you have an SDL fullscreen app that you want to force to minimize.

> SDLHack is a wrapper for SDL which lets you force full-screen games to
> minimize.

s/to minimize/into windowed mode/

"minimize" means to have the app shown only in the taskbar.

Hmmm, nevermind, actually the description is correct. (The readme introduces it first as "This is a simple LD_PRELOAD wrapper for SDL which enables switching away from a fullscreen game.", but if I read it more carefully, I see that it does this by minimizing entirely (which sucks, but that's not the package's problem).)

OK, forget about the Requires.

Please add a comment, when you change your spec-file and also increment the release number and add an entry to the changelog. Otherwise the reviewer can't easily see, he's looking at an updated version.

Your current spec doesn't work.

(In reply to comment #10)
> OK, forget about the Requires.
> 
> Please add a comment, when you change your spec-file and also increment the
> release number and add an entry to the changelog. Otherwise the reviewer can't
> easily see, he's looking at an updated version.
> 
> Your current spec doesn't work.

What do you mean by "doesn't work" ? I test if it builds before submitting any changes

+ install -Dpm 755 libsdlhack-i386.so libsdlhack-x86_64.so /home/makerpm/rpmbuild/BUILDROOT/sdlhack-1.1-1.fc13.x86_64/usr/lib64/sdlhack/libsdlhack.so
install: target `/home/makerpm/rpmbuild/BUILDROOT/sdlhack-1.1-1.fc13.x86_64/usr/lib64/sdlhack/libsdlhack.so' is not a directory
error: Bad exit status from /var/tmp/rpm-tmp.mfzHwm (%install)

(In reply to comment #12)
> + install -Dpm 755 libsdlhack-i386.so libsdlhack-x86_64.so
> /home/makerpm/rpmbuild/BUILDROOT/sdlhack-1.1-1.fc13.x86_64/usr/lib64/sdlhack/libsdlhack.so
> install: target
> `/home/makerpm/rpmbuild/BUILDROOT/sdlhack-1.1-1.fc13.x86_64/usr/lib64/sdlhack/libsdlhack.so'
> is not a directory
> error: Bad exit status from /var/tmp/rpm-tmp.mfzHwm (%install)

Fixed :

http://koji.fedoraproject.org/koji/taskinfo?taskID=2676567


About not bumping release number, I don't think it is worth it since we are dealing with trivial issues.

Package approved.

Rpmlint:
3 packages and 0 specfiles checked; 0 errors, 0 warnings.

Source files match upstream:
sha256sum: 785104cb314447be4f3ab2fe13087cc1d6e6ca3bf171bbb57c85b099e977a534

BRs are listed.

Compiles and builds on at least x86_64 (EPEL6, F13, 14 and Rawhide) and ppc64 (EPEL6).

Package meets naming and versioning guidelines.

Specfile is properly named, is cleanly written and uses macros consistently.

License text included in package and open source-compatible;

Build root is correct.

Package doesn't own files or directories already owned by other packages.

%docs are not necessary for the proper functioning of the package. documentation is small, so no -docs subpackage is necessary.

No duplicates in %files.

Package contains code.

No shared libraries are added to the regular linker search paths.

Final provides and requires are sane:
/bin/bash
libc.so.6()(64bit)
libc.so.6(GLIBC_2.2.5)(64bit)
libc.so.6(GLIBC_2.3.4)(64bit)
libdl.so.2()(64bit)
libdl.so.2(GLIBC_2.2.5)(64bit)
rpmlib(CompressedFileNames) <= 3.0.4-1
rpmlib(FileDigests) <= 4.6.0-1
rpmlib(PayloadFilesHavePrefix) <= 4.0-1
rtld(GNU_HASH)
rpmlib(PayloadIsXz) <= 5.2-1

Forgot the provides:

libsdlhack.so((64bit)
sdlhack = 1.1-1.fc13
sdlhack(x86-64) = 1.1-1.fc13

thanks for reviewing this package

New Package SCM Request
=======================
Package Name: sdlhack
Short Description: Force full-screen games to minimize
Owners: hicham
Branches: f13 f14
InitialCC: hicham

> About not bumping release number, I don't think it is worth it since we are
> dealing with trivial issues.

It doesn't matter anymore for this particular package since it already passed review, but you're supposed to always bump Release and add a changelog entry for any change made after the initial review submission, no matter how trivial.

Git done (by process-git-requests).

sdlhack-1.1-1.fc14 has been submitted as an update for Fedora 14.
https://admin.fedoraproject.org/updates/sdlhack-1.1-1.fc14

sdlhack-1.1-1.fc13 has been submitted as an update for Fedora 13.
https://admin.fedoraproject.org/updates/sdlhack-1.1-1.fc13

sdlhack-1.1-1.fc13 has been pushed to the Fedora 13 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update sdlhack'.  You can provide feedback for this update here: https://admin.fedoraproject.org/updates/sdlhack-1.1-1.fc13

sdlhack-1.1-1.fc13 has been pushed to the Fedora 13 stable repository.  If problems still persist, please make note of it in this bug report.

sdlhack-1.1-1.fc14 has been pushed to the Fedora 14 stable repository.  If problems still persist, please make note of it in this bug report.