Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
Summary: Review Request: mingw32-physfs - MinGW Windows port of the PhysicsFS library
https://bugzilla.redhat.com/show_bug.cgi?id=489633
Summary: Review Request: mingw32-physfs - MinGW Windows port of the PhysicsFS library Product: Fedora Version: rawhide Platform: All OS/Version: Linux Status: NEW Severity: medium Priority: low Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: ploujj@gmail.com QAContact: extras-qa@fedoraproject.org CC: notting@redhat.com, fedora-package-review@redhat.com Estimated Hours: 0.0 Classification: Fedora
Spec URL: http://plouj.com/rpmbuild/SPECS/mingw32-physfs.spec SRPM URL: http://plouj.com/rpmbuild/SRPMS/mingw32-physfs-1.0.1-11.fc10.src.rpm
Hi! I just finished packaging physfs for the Fedora/mingw32 project (http://fedoraproject.org/wiki/SIGs/MinGW). This is my first package and I need a sponsor.
Description: PhysicsFS is a library to provide abstract access to various archives. It is intended for use in video games, and the design was somewhat inspired by Quake 3's file subsystem. The programmer defines a "write directory" on the physical filesystem. No file writing done through the PhysicsFS API can leave that write directory, for security. For example, an embedded scripting language cannot write outside of this path if it uses PhysFS for all of its I/O, which means that untrusted scripts can run more safely. Symbolic links can be disabled as well, for added safety. For file reading, the programmer lists directories and archives that form a "search path". Once the search path is defined, it becomes a single, transparent hierarchical filesystem. This makes for easy access to ZIP files in the same way as you access a file directly on the disk, and it makes it easy to ship a new archive that will override a previous archive on a per-file basis. Finally, PhysicsFS gives you platform-abstracted means to determine if CD-ROMs are available, the user's home directory, where in the real filesystem your program is running, etc.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=489633
Michael Ploujnikov ploujj@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |177841(FE-NEEDSPONSOR)
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=489633
Richard W.M. Jones rjones@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |fedora-mingw@lists.fedorapr | |oject.org, | |rjones@redhat.com
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=489633
--- Comment #1 from Richard W.M. Jones rjones@redhat.com 2009-03-13 11:54:55 EDT --- Koji scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=1240160
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=489633
Richard W.M. Jones rjones@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |fedora-review?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=489633
--- Comment #2 from Richard W.M. Jones rjones@redhat.com 2009-03-13 12:05:38 EDT --- The %{_mingw32_configure} line needs to be moved down so it is in the %build section.
Also you don't need %pre and %post, because ldconfig only cares about native Fedora binaries, and won't know what to do with *.DLLs. Just remove the %pre and %post sections completely.
I will continue the review assuming that you have done the above.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=489633
--- Comment #3 from Richard W.M. Jones rjones@redhat.com 2009-03-13 12:18:32 EDT --- auto-buildrequires suggests the following extra BuildRequires lines:
BuildRequires: mingw32-dlfcn BuildRequires: mingw32-readline BuildRequires: mingw32-zlib
(A build might work without those, but configure might disable features if those packages aren't present. On the other hand, maybe those are false dependencies since auto-buildrequires isn't perfect).
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=489633
--- Comment #4 from Richard W.M. Jones rjones@redhat.com 2009-03-13 12:20:48 EDT --- rpmlint output:
mingw32-physfs.src:66: E: files-attr-not-set mingw32-physfs.src:67: E: files-attr-not-set mingw32-physfs.src:68: E: files-attr-not-set mingw32-physfs.src:69: E: files-attr-not-set mingw32-physfs.src:70: E: files-attr-not-set
You are missing a %defattr line in the specfile
mingw32-physfs.x86_64: W: devel-file-in-non-devel-package /usr/i686-pc-mingw32/sys-root/mingw/lib/libphysfs.dll.a mingw32-physfs.x86_64: W: spurious-executable-perm /usr/i686-pc-mingw32/sys-root/mingw/lib/libphysfs.dll.a mingw32-physfs.x86_64: W: devel-file-in-non-devel-package /usr/i686-pc-mingw32/sys-root/mingw/include/physfs.h mingw32-physfs.x86_64: W: binaryinfo-readelf-failed readelf: Error: Not an ELF file - it has the wrong magic bytes at the start mingw32-physfs.x86_64: W: non-standard-dir-in-usr i686-pc-mingw32
You can ignore these, as per http://fedoraproject.org/wiki/MinGW/Rpmlint
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=489633
--- Comment #5 from Richard W.M. Jones rjones@redhat.com 2009-03-13 12:30:04 EDT --- - rpmlint output see comment 4 + package name satisfies the packaging naming guidelines + specfile name matches the package base name - package should satisfy packaging guidelines problems, see comment 2, comment 3, comment 4 + license meets guidelines and is acceptable to Fedora zlib + license matches the actual package license + %doc includes license file + spec file written in American English + spec file is legible ? upstream sources match sources in the srpm 9bab1ed6383958b8bc7db792c6989b01 663342 (upstream website was being very slow - I will check this later) + package successfully builds on at least one architecture n/a ExcludeArch bugs filed ? BuildRequires list all build dependencies see comment 3 n/a %find_lang instead of %{_datadir}/locale/* + binary RPM with shared library files must call ldconfig in %post and %postun this doesn't apply for mingw packages, see comment 2 + does not use Prefix: /usr + package owns all directories it creates + no duplicate files in %files - %defattr line see comment 4 + %clean contains rm -rf $RPM_BUILD_ROOT + consistent use of macros + package must contain code or permissible content n/a large documentation files should go in -doc subpackage + files marked %doc should not affect package n/a header files should be in -devel doesn't apply to mingw packages n/a static libraries should be in -static could consider creating a -static subpackage, but not vital n/a packages containing pkgconfig (.pc) files need 'Requires: pkgconfig' n/a libfoo.so must go in -devel n/a -devel must require the fully versioned base n/a packages should not contain libtool .la files n/a packages containing GUI apps must include %{name}.desktop file n/a packages must not own files or directories owned by other packages + %install must start with rm -rf %{buildroot} etc. + filenames must be valid UTF-8
Optional:
n/a if there is no license file, packager should query upstream n/a translations of description and summary for non-English languages, if available + reviewer should build the package in mock + the package should build into binary RPMs on all supported architectures - review should test the package functions as described n/a scriptlets should be sane n/a pkgconfig files should go in -devel n/a shouldn't have file dependencies outside /etc /bin /sbin /usr/bin or /usr/sbin
------------------------
Please see comment 2, comment 3, comment 4.
Also why are we not building the latest upstream version, which is 1.1.1?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=489633
--- Comment #6 from Michael Ploujnikov ploujj@gmail.com 2009-03-14 13:18:05 EDT --- (In reply to comment #2)
The %{_mingw32_configure} line needs to be moved down so it is in the %build section.
Also you don't need %pre and %post, because ldconfig only cares about native Fedora binaries, and won't know what to do with *.DLLs. Just remove the %pre and %post sections completely.
I will continue the review assuming that you have done the above.
Done (In reply to comment #3)
auto-buildrequires suggests the following extra BuildRequires lines:
BuildRequires: mingw32-dlfcn
This is definitely needed since physfs can use dlopen().
BuildRequires: mingw32-readline
This is only required for the test program
BuildRequires: mingw32-zlib
Physfs can use an internal zlib if this isn't available. I suspect it would be better for security updates to have physfs use the system's zlib rather than its own. (In reply to comment #4)
rpmlint output:
mingw32-physfs.src:66: E: files-attr-not-set mingw32-physfs.src:67: E: files-attr-not-set mingw32-physfs.src:68: E: files-attr-not-set mingw32-physfs.src:69: E: files-attr-not-set mingw32-physfs.src:70: E: files-attr-not-set
You are missing a %defattr line in the specfile
Fixed, although my rpmlint (0.85) didn't mention this.
(In reply to comment #5)
Also why are we not building the latest upstream version, which is 1.1.1?
Version 1.1.1 uses cmake, which I haven't figured out how to use with mingw32 yet. Version 1.1.0 still uses autotools so I'll try creating a .spec file for that one, later. The reasons I chose 1.0.1 originally is because the latest Fedora development version still uses 1.0.1: http://cvs.fedoraproject.org/viewvc/devel/physfs/physfs.spec?revision=1.9&am... .
I've updated http://plouj.com/rpmbuild/SPECS/mingw32-physfs.spec and http://plouj.com/rpmbuild/SRPMS/mingw32-physfs-1.0.1-11.fc10.src.rpm with all of the above fixes.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=489633
Richard W.M. Jones rjones@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-review? |fedora-review+
--- Comment #7 from Richard W.M. Jones rjones@redhat.com 2009-03-16 08:01:51 EDT --- Issues in comment 2 are FIXED.
Issues in comment 3 are FIXED.
Issue in comment 4 is FIXED.
--------------------------------- This package is APPROVED by rjones ---------------------------------
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=489633
--- Comment #8 from Richard W.M. Jones rjones@redhat.com 2009-03-16 08:03:23 EDT --- Plouj, you should continue the process with http://fedoraproject.org/wiki/CVSAdminProcedure
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=489633
Itamar Reis Peixoto itamar@ispbrasil.com.br changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |itamar@ispbrasil.com.br Flag| |needinfo?(ploujj@gmail.com)
--- Comment #9 from Itamar Reis Peixoto itamar@ispbrasil.com.br 2009-03-16 08:19:59 EDT --- (In reply to comment #8) Richard.
have you sponsored the guy ?
---------------------------------------------------
Michael
Do you have a fedora account ? what's your FAS username ?
if you don't have one please get one at the flowing link
https://fedoraproject.org/wiki/PackageMaintainers/Join#Get_a_Fedora_Account
after you get a FAS account please join the fedora packager group and post your username here, so Richard will sponsor you :-)
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=489633
--- Comment #10 from Richard W.M. Jones rjones@redhat.com 2009-03-16 08:41:17 EDT --- I'm quite happy with sponsoring him. TBH I have no idea how the sponsorship process works though ...
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=489633
Michael Ploujnikov ploujj@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|needinfo?(ploujj@gmail.com) |
--- Comment #11 from Michael Ploujnikov ploujj@gmail.com 2009-03-16 08:49:09 EDT --- (In reply to comment #9)
Michael
Do you have a fedora account ? what's your FAS username ?
"plouj"
after you get a FAS account please join the fedora packager group and post your username here, so Richard will sponsor you :-)
I've just applied for the Fedora Packager CVS Commit Group (packager).
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=489633
Tom "spot" Callaway tcallawa@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |tcallawa@redhat.com
--- Comment #12 from Tom "spot" Callaway tcallawa@redhat.com 2009-03-16 10:39:02 EDT --- (In reply to comment #11)
I've just applied for the Fedora Packager CVS Commit Group (packager).
Michael, I've sponsored you based on Richard's recommendation. If you have any questions about the process or future packages, please do not hesitate to contact me.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=489633
--- Comment #13 from Tom "spot" Callaway tcallawa@redhat.com 2009-03-16 10:41:49 EDT --- Also, as the physfs maintainer, we haven't bumped to the higher level releases because it is an API break and none of the dependent programs have moved to the newer physfs API yet. That said, if you need the newer one for your mingw work, I have no problem with it.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=489633
Itamar Reis Peixoto itamar@ispbrasil.com.br changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |needinfo?(ploujj@gmail.com)
--- Comment #14 from Itamar Reis Peixoto itamar@ispbrasil.com.br 2009-03-16 10:53:57 EDT --- Michael you should continue the process with
https://fedoraproject.org/wiki/PackageMaintainers/CVSAdminProcedure#New_Pack...
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=489633
Michael Ploujnikov ploujj@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|needinfo?(ploujj@gmail.com) |
--- Comment #15 from Michael Ploujnikov ploujj@gmail.com 2009-03-16 13:28:06 EDT --- New Package CVS Request ======================= Package Name: mingw32-physfs Short Description: MinGW Windows port of the PhysicsFS library Owners: plouj rjones Branches: F-10 InitialCC:
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=489633
Michael Ploujnikov ploujj@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |fedora-cvs?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=489633
Kevin Fenzi kevin@tummy.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED AssignedTo|nobody@fedoraproject.org |rjones@redhat.com Flag|fedora-cvs? |fedora-cvs+
--- Comment #16 from Kevin Fenzi kevin@tummy.com 2009-03-17 01:10:29 EDT --- cvs done.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=489633
Richard W.M. Jones rjones@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |needinfo?(ploujj@gmail.com)
--- Comment #17 from Richard W.M. Jones rjones@redhat.com 2009-03-17 04:19:54 EDT --- Plouj, next step is to import the package into Fedora's CVS, both the devel and F-10 branches.
https://fedoraproject.org/wiki/PackageMaintainers/Join#Install_the_Client_To...
Roughly speaking the commands are something like this. Please check them carefully.
cd /tmp
fedora-cvs mingw32-physfs cd mingw32-physfs/devel ../common/cvs-import name-of-the-SRPM cvs up make build
cd ../F-10 ../common/cvs-import name-of-the-SRPM cvs up make build make update
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=489633
Mamoru Tasaka mtasaka@ioa.s.u-tokyo.ac.jp changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks|177841(FE-NEEDSPONSOR) |
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=489633
--- Comment #18 from Fedora Update System updates@fedoraproject.org 2009-03-17 22:34:38 EDT --- mingw32-physfs-1.0.1-11.fc10 has been submitted as an update for Fedora 10. http://admin.fedoraproject.org/updates/mingw32-physfs-1.0.1-11.fc10
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=489633
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |ON_QA
--- Comment #19 from Fedora Update System updates@fedoraproject.org 2009-03-18 15:13:11 EDT --- mingw32-physfs-1.0.1-11.fc10 has been pushed to the Fedora 10 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 mingw32-physfs'. You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F10/FEDORA-2009-2840
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=489633
Kevin Kofler kevin@tigcc.ticalc.org changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |kevin@tigcc.ticalc.org
--- Comment #20 from Kevin Kofler kevin@tigcc.ticalc.org 2009-03-18 16:50:27 EDT --- For CMake issues (i.e. for the latest version), these should help: http://www.cmake.org/Wiki/CMake_Cross_Compiling http://techbase.kde.org/Getting_Started/Build/KDE4/Windows/Cross-Compiling
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=489633
--- Comment #21 from Fedora Update System updates@fedoraproject.org 2009-03-23 21:07:57 EDT --- mingw32-physfs-1.0.1-12.fc10 has been submitted as an update for Fedora 10. http://admin.fedoraproject.org/updates/mingw32-physfs-1.0.1-12.fc10
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=489633
--- Comment #22 from Fedora Update System updates@fedoraproject.org 2009-04-21 20:59:03 EDT --- mingw32-physfs-1.0.1-12.fc10 has been pushed to the Fedora 10 stable repository. If problems still persist, please make note of it in this bug report.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=489633
Richard W.M. Jones rjones@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|needinfo?(ploujj@gmail.com) |
--- Comment #23 from Richard W.M. Jones rjones@redhat.com 2009-06-06 11:26:41 EDT --- This bug doesn't need to be in NEEDINFO any more.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=489633
Peter Lemenkov lemenkov@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ON_QA |CLOSED CC| |lemenkov@gmail.com Resolution| |NEXTRELEASE
--- Comment #24 from Peter Lemenkov lemenkov@gmail.com 2009-06-16 03:57:34 EDT --- It seems that we should close this ticket (mingw32-physfs was built and pushed to updates).
package-review@lists.fedoraproject.org