Spec URL: http://astokes.fedorapeople.org/mingw32-cxxtest.spec SRPM URL: http://astokes.fedorapeople.org/mingw32-cxxtest-3.10.1-4.fc15.src.rpm Description: Provided under mingw32 compilation : CxxTest is a JUnit/CppUnit/xUnit-like framework for C++. Its advantages over existing alternatives are that it: - doesn't require RTTI - doesn't require member template functions - doesn't require exception handling - doesn't require any external libraries (including memory management, file/console I/O, graphics libraries)
Review done. Everything seems to be ok.
New Package SCM Request ======================= Package Name: mingw32-cxxtest Short Description: CxxTest ported to mingw32 Owners: astokes Branches: f15 devel InitialCC: abeekhof pmyers
InitialCC needs to contain FAS account names, not email addresses.
InitialCC: beekhof rrakus
Git done (by process-git-requests).
mingw32-cxxtest-3.10.1-4.fc15 has been submitted as an update for Fedora 15. https://admin.fedoraproject.org/updates/mingw32-cxxtest-3.10.1-4.fc15
mingw32-cxxtest-3.10.1-4.fc15 has been pushed to the Fedora 15 testing repository.
mingw32-cxxtest-3.10.1-4.fc15 has been pushed to the Fedora 15 stable repository.
I'm re-opening this review ticket as I don't agree with the 'review' which was done here. The .spec file which is attached here doesn't even build in mock! The .spec file which was imported in rawhide is in a bit better shape, but still not compliant with the general Fedora packaging guidelines and the MinGW-specific packaging guidelines: http://fedoraproject.org/wiki/Packaging:MinGW I'll do a proper review now based on what's now in rawhide (3.10.1-4.fc16). The Source0 and Source1 URL's are invalid: $ spectool -g mingw32-cxxtest.spec Getting http://cxxtest.tigris.org/files/documents/6421/43281/mingw32-cxxtest-3.10.1.tar.gz to ./mingw32-cxxtest-3.10.1.tar.gz % Total % Received % Xferd Average Speed Time Time Time Current Dload Upload Total Spent Left Speed 0 0 0 0 0 0 0 0 --:--:-- --:--:-- --:--:-- 0 curl: (22) The requested URL returned error: 404 Getting http://cxxtest.tigris.org/files/documents/6421/43284/mingw32-cxxtest-guide-3.10.1.pdf to ./mingw32-cxxtest-guide-3.10.1.pdf % Total % Received % Xferd Average Speed Time Time Time Current Dload Upload Total Spent Left Speed 0 0 0 0 0 0 0 0 --:--:-- --:--:-- --:--:-- 0 curl: (22) The requested URL returned error: 404 Please use working URLs or add a comment how the .tar.gz can be regenerated As your only targeting F-15 and rawhide, several things can be dropped from the .spec file like the BuildRoot tag, the 'rm -rf $RPM_BUILD_ROOT' from the %install phase, the entire %clean section and the %defattr lines from both subpackages. The conditionals for fedora < 11 and rhel can also be dropped as they're unneeded when you only target F-15 and rawhide. Why are you bundling the python pieces with this package? We don't have python support in the MinGW toolchain in Fedora so the python pieces are kinda useless. The python code and the -doc subpackage are also bundled with the native Fedora cxxtest package so they can both the dropped from the mingw package. Is it correct that this package only provides some C++ header files? If that's the case then the two %global overrides can be dropped as they only apply to mingw binaries. Do note however that when this package starts to bundle binaries that several overrides need to be added (for dependency and debuginfo extraction). See the Fedora MinGW packaging guidelines for an example. If you aren't bundling any mingw binaries then you need add to a Requires: mingw32-filesystem manually. Why was this package imported as mingw32-cxxtest? The current Fedora MinGW guidelines strongly suggest to name new packages mingw-, so that in the future it would be easier to build mingw64- binary packages. Now that you've used mingw32- source package naming, you'll have to retire mingw32-cxxtest and re-review mingw-cxxtest once the mingw64 compiler is ready, probably in the F17 timeframe
Created attachment 514160 [details] Patch to address issues pointed out by Erik Adam and Roman: To help you guys out, I am attaching a patch which should hopefully address all the issues pointed out by Erik.
(In reply to comment #10) > Created attachment 514160 [details] > Patch to address issues pointed out by Erik > > Adam and Roman: To help you guys out, I am attaching a patch which should > hopefully address all the issues pointed out by Erik. Very awesome, thank you so much. I'll run some quick tests and post an updated package. Thanks again
(In reply to comment #9) > I'm re-opening this review ticket as I don't agree with the 'review' which was > done here. The .spec file which is attached here doesn't even build in mock! > The .spec file which was imported in rawhide is in a bit better shape, but > still not compliant with the general Fedora packaging guidelines and the > MinGW-specific packaging guidelines: > http://fedoraproject.org/wiki/Packaging:MinGW > > I'll do a proper review now based on what's now in rawhide (3.10.1-4.fc16). > > The Source0 and Source1 URL's are invalid: > $ spectool -g mingw32-cxxtest.spec > Getting > http://cxxtest.tigris.org/files/documents/6421/43281/mingw32-cxxtest-3.10.1.tar.gz > to ./mingw32-cxxtest-3.10.1.tar.gz > % Total % Received % Xferd Average Speed Time Time Time Current > Dload Upload Total Spent Left Speed > 0 0 0 0 0 0 0 0 --:--:-- --:--:-- --:--:-- 0 > curl: (22) The requested URL returned error: 404 > Getting > http://cxxtest.tigris.org/files/documents/6421/43284/mingw32-cxxtest-guide-3.10.1.pdf > to ./mingw32-cxxtest-guide-3.10.1.pdf > % Total % Received % Xferd Average Speed Time Time Time Current > Dload Upload Total Spent Left Speed > 0 0 0 0 0 0 0 0 --:--:-- --:--:-- --:--:-- 0 > curl: (22) The requested URL returned error: 404 > > Please use working URLs or add a comment how the .tar.gz can be regenerated > > As your only targeting F-15 and rawhide, several things can be dropped from the > .spec file like the BuildRoot tag, the 'rm -rf $RPM_BUILD_ROOT' from the > %install phase, the entire %clean section and the %defattr lines from both > subpackages. The conditionals for fedora < 11 and rhel can also be dropped as > they're unneeded when you only target F-15 and rawhide. > > Why are you bundling the python pieces with this package? We don't have python > support in the MinGW toolchain in Fedora so the python pieces are kinda > useless. The python code and the -doc subpackage are also bundled with the > native Fedora cxxtest package so they can both the dropped from the mingw > package. > > Is it correct that this package only provides some C++ header files? If that's > the case then the two %global overrides can be dropped as they only apply to > mingw binaries. Do note however that when this package starts to bundle > binaries that several overrides need to be added (for dependency and debuginfo > extraction). See the Fedora MinGW packaging guidelines for an example. > > If you aren't bundling any mingw binaries then you need add to a Requires: > mingw32-filesystem manually. > > Why was this package imported as mingw32-cxxtest? The current Fedora MinGW > guidelines strongly suggest to name new packages mingw-, so that in the future > it would be easier to build mingw64- binary packages. Now that you've used > mingw32- source package naming, you'll have to retire mingw32-cxxtest and > re-review mingw-cxxtest once the mingw64 compiler is ready, probably in the F17 > timeframe Just for my benefit what happens to the existing mingw32 packages that are already in the distro? Will we be doing a mass rename of these when the compiler is ready? Thanks
mingw32-cxxtest-3.10.1-5.fc15 has been submitted as an update for Fedora 15. https://admin.fedoraproject.org/updates/mingw32-cxxtest-3.10.1-5.fc15
Package mingw32-cxxtest-3.10.1-5.fc15: * should fix your issue, * was pushed to the Fedora 15 testing repository, * should be available at your local mirror within two days. Update it with: # su -c 'yum update --enablerepo=updates-testing mingw32-cxxtest-3.10.1-5.fc15' as soon as you are able to. Please go to the following url: https://admin.fedoraproject.org/updates/mingw32-cxxtest-3.10.1-5.fc15 then log in and leave karma (feedback).
Sorry for the late response. Thanks for processing the feedback and updating the package. Just one little nitpick, you forgot to add a new %changelog entry at http://pkgs.fedoraproject.org/gitweb/?p=mingw32-cxxtest.git;a=commitdiff;h=10cd67fbff548076f5521cdba69afdd53aa4fef9 Regarding your question about a mass-rename. At the moment we can't give you a real answer to that question. Once the mingw-w64 toolchain is in Fedora (which is expected in the F-17 release cycle) all current mingw32-* packages have to be renamed to mingw-* and updated to be compliant with the new mingw packaging guidelines in order to add mingw-w64 support. As there are already about 100 different mingw32-* packages in Fedora it doesn't seem practical to start the package rename process (which includes a full review) for all these packages. So we're currently thinking about asking FeSCO for an exception for a mass rename, but we'll have to wait for the mingw-w64 toolchain to enter Fedora first before moving on.
mingw32-cxxtest-3.10.1-5.fc15 has been pushed to the Fedora 15 stable repository. If problems still persist, please make note of it in this bug report.