Spec URL: http://developer.postgresql.org/~devrim/rpms/other/orafce/postgresql-orafce.spec SRPM URL: http://developer.postgresql.org/~devrim/rpms/other/orafce/postgresql-orafce-2.1.1-1.fc7.src.rpm Description: The goal of this project is implementation some functions from Oracle database. Some date functions (next_day, last_day, trunc, round, ...) are implemented now. Functionality was verified on Oracle 10g and module is useful for production work.
Created attachment 172420 [details] postgresql-orafce build log The following comments are based on the Package Review Guidelines and the Packaging Guidelines. But I'm not a sponsor of the Fedora project, so these are not official review comments. - You should post the results of rpmlint command. - You should apply the common compiler flags defined as $RPM_OPT_FLAGS or %{optflags} . - You should use -p option with 'install' command to preserve timestamps. - You should improve Makefile to enable to build with non-privilleged user. See the attachment. "make install" tries to make a directory of "/usr/share/pgsql/contrib", and %buildroot is not used. I could not build your package because of this problem. - In addition, "/usr/share/pgsql/contrib" should be owned this package, if you intend to create the directory actually. It is not owned by the postgresql package. [Package Review Guidelines] http://fedoraproject.org/wiki/Packaging/ReviewGuidelines [The Packaging Guidelines] http://fedoraproject.org/wiki/Packaging/Guidelines)
Hi Devrim, I'd love to do a full review, can you update the package according to comment #1?
Hi, (In reply to comment #1) > - You should post the results of rpmlint command. > - You should apply the common compiler flags defined as $RPM_OPT_FLAGS or > %{optflags} . Added. > - You should use -p option with 'install' command to preserve timestamps. Added. > - You should improve Makefile to enable to build with non-privilleged user. > See the attachment. "make install" tries to make a directory of > "/usr/share/pgsql/contrib", and %buildroot is not used. > I could not build your package because of this problem. Ok, asked upstream for a solution for this. > - In addition, "/usr/share/pgsql/contrib" should be owned this package, if you > intend to create the directory actually. It is not owned by the postgresql > package. Done. Thanks for the review. Will submit the new SRPM after upstream comes with a fix for the problem you reported. Regards, Devrim
All reported issues fixed (the remaining issue is absolutely was an issue in spec file btw...): New spec: http://developer.postgresql.org/~devrim/rpms/other/orafce/postgresql-orafce.spec New SRPM: http://developer.postgresql.org/~devrim/rpms/other/orafce/postgresql-orafce-2.1.3-2.fc8.src.rpm (Updated package to 2.1.3 to work with PostgreSQL 8.3, which is in rawhide now). $ md5sum orafce-2.1.3.tar.gz dca37c424635bb1a97f26eb223ad6aab orafce-2.1.3.tar.gz rpmlint does not report any errors. Regards, Devrim
I'm not sure what happened here; looks like Ruben missed the vact that you updated the package. This did not build for me in mock: bison -y -d -p orafce_sql_yy sqlparse.y make: execvp: bison: Permission denied which is a bit weird, but bison is indeed not installed. I added a build dependency and things fail due to flex not being installed. So I added that and things still fail: error: openssl/ssl.h: No such file or directory error: openssl/err.h: No such file or directory error: gssapi/gssapi.h: No such file or directory I added a build dependency on openssl-devel and things finally build, although I'm a bit confused because it doesn't supply a gssapi.h Can you try doing a mock or koji build and ensure that there's nothing else that needs to be added to get this to build properly?
Devrim, can you have a look at Jason's comments?
(In reply to comment #6) > Devrim, can you have a look at Jason's comments? I cannot do mock build at home (bw issues). Let me check other options.
You can do as many scratch builds in koji as you like, though. You don't actually need to have a local mock installation at home.
Sratch build in Koji failed: http://koji.fedoraproject.org/koji/taskinfo?taskID=879804
It's been quite some time since there was any movement here. This ticket will be closed soon if there is no response.
Please hold this bug open a bit more.
OK, so it's been four more months and there doesn't seem to have been any progress. Since you want to hold this open even though there's no reviewable package, I'm going to indicate that it's not ready for review. Please clear the whiteboard when you're ready. However, I'll still go ahead and close this eventually if there's no progress.
Ok, package is now built cleanly in mock: http://koji.fedoraproject.org/koji/taskinfo?taskID=1258906 I think we can approve this package now.
you can merge your install commands in 2 lines using install -Dp -m your package seems to be good for me.
Would you mind posting updated spec and srpm. That one I can download from your scratch build do not work for me: $ rpm -Uvh orafce-2.1.4-1.fc11.src.rpm 1:orafce warning: user mockbuild does not exist - using root warning: group mockbuild does not exist - using root ########################################### [100%] error: unpacking of archive failed on file /home/msuchy/rpmbuild/SOURCES/orafce-2.1.4.tar.gz;49d0fee4: cpio: MD5 sum mismatch
Sure. Spec file: http://www.gunduz.org/temp/orafce.spec SRPM:http://www.gunduz.org/temp/orafce-2.1.4-1.f9.src.rpm
============== Key: - = N/A x = Check ! = Problem ? = Not evaluated === REQUIRED ITEMS === [x] Package is named according to the Package Naming Guidelines. [!] Spec file name must match the base package %{name}, in the format %{name}.spec. [x] Package meets the Packaging Guidelines including the Perl specific items [x] Package successfully compiles and builds into binary rpms on at least one supported architecture. http://koji.fedoraproject.org/koji/taskinfo?taskID=1269956 [x] Rpmlint output: empty [x] Package is not relocatable. [x] Buildroot is correct %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) [x] Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines. [!] License field in the package spec file matches the actual license. [!] If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package is included in %doc. [x] Spec file is legible and written in American English. [x] Sources used to build the package matches the upstream source, as provided in the spec URL. SHA1SUM of package: 2a091014fe63e10dc4aa4661453f4b07 orafce-2.1.4.tar.gz [x] Package is not known to require ExcludeArch [x] All build dependencies are listed in BuildRequires, except for any that are listed in the exceptions section of Packaging Guidelines. [-] The spec file handles locales properly. [-] ldconfig called in %post and %postun if required. [x] Package must own all directories that it creates. [x] Package requires other packages for directories it uses. [x] Package does not contain duplicates in %files. [x] Permissions on files are set properly. [x] Package has a %clean section, which contains rm -fR $RPM_BUILD_ROOT. [x] Package consistently uses macros. [x] Package contains code, or permissable content. [-] Large documentation files are in a -doc subpackage, if required. [x] Package uses nothing in %doc for runtime. [-] Header files in -devel subpackage, if present. [-] Static libraries in -devel subpackage, if present. [-] Package requires pkgconfig, if .pc files are present. [-] Development .so files in -devel subpackage, if present. [-] Fully versioned dependency in subpackages, if present. [x] Package does not contain any libtool archives (.la). [-] Package contains a properly installed %{name}.desktop file if it is a GUI application. [x] Package does not own files or directories owned by other packages. === SUGGESTED ITEMS === [x] Latest version is packaged. [!] Package does not include license text files separate from upstream. [-] Description and summary sections in the package spec file contains translations for supported Non-English languages, if available. [x] Reviewer should test that the package builds in mock. Tested on: koji scratch build [x] Package should compile and build into binary rpms on all supported architectures. Tested on:koji scratch build [?] Package functions as described. [-] Scriptlets must be sane, if used. [-] The placement of pkgconfig(.pc) files is correct. [-] File based requires are sane. [-] %check is present and the tests pass ToDo: So do you plan to name the package postgresql-orafce or just orafce. In first case you should rename your spec file and Name, in second you should change summary of this BZ. I like the first option as we can have in feature something like mysql-orafce, firebird-orafce... The license is stated BSD, but the text in COPYRIGHT.orafunc differ from BSD licence as I know it (http://en.wikipedia.org/wiki/BSD_license). Can you clarify it? %{_docdir}/pgsql/contrib/*.orafunc That should be %doc COPYRIGHT.orafunc INSTALL.orafunc README.orafunc Do you really want to requires postgresql, I suppose that correct should be be postgresql-server, but IMHO.
3.0 version is about to be released, and I will update the package then.
3.0.1 has been release on August 21, 2009 Do you want to continue with review or I can close this BZ?
Here is the new spec: https://projects.commandprompt.com/public/pgcore/browser/rpm/redhat/8.5/orafce/F-11/orafce.spec Downloadable version: https://projects.commandprompt.com/public/pgcore/repo/rpm/redhat/8.5/orafce/F-11/orafce.spec SRPM: http://yum.pgsqlrpms.org/srpms/8.4/fedora/fedora-11-i386/orafce-3.0.1-1.f11.src.rpm No rpmlint errors detected.
BTW, I changed package name to orafce, instead of postgresql-orafce.
I was wondering whether this package should be a requirement for spacewalk.
>http://yum.pgsqlrpms.org/srpms/8.4/fedora/fedora-11-i386/orafce-3.0.1-1.f11.src.rpm 404 Not Found Anyway, from quick look on spec itself, I still see, that two of my previous comments has not been addressed: > %{_docdir}/pgsql/contrib/*.orafunc > That should be > %doc COPYRIGHT.orafunc INSTALL.orafunc README.orafunc And: >Do you really want to requires postgresql, I suppose that correct should be be >postgresql-server, but IMHO.
Spec URL: http://jfsaucier.fedorapeople.org/packages/postgresql-orafce.spec SRPM URL: http://jfsaucier.fedorapeople.org/packages/postgresql-orafce-3.0.1-2.fc12.src.rpm Hi, Since this is a long time with no post and that I need this software for a project of mine, I have packaged it based on the SPEC Devrim did. I renamed the package back to postgresql-orafce because I think that's what is best and consistent with other postgresql extensions. I think that I have included all the modifications asked here. It build fine in koji for rawhide, F-13, F-12 and F-11. Here is the scratch build : - http://koji.fedoraproject.org/koji/taskinfo?taskID=2127126 - http://koji.fedoraproject.org/koji/taskinfo?taskID=2127141 - http://koji.fedoraproject.org/koji/taskinfo?taskID=2127148 - http://koji.fedoraproject.org/koji/taskinfo?taskID=2127162 rpmlint give no warning/error on spec, srpm and rpm. Tested and work fine here. Thank you.
============== Key: - = N/A x = Check ! = Problem ? = Not evaluated === REQUIRED ITEMS === [x] Package is named according to the Package Naming Guidelines. [x] Spec file name must match the base package %{name}, in the format %{name}.spec. [x] Package meets the Packaging Guidelines including the Java specific items [x] Package successfully compiles and builds into binary rpms on at least one supported architecture. tested in: devel/koji http://koji.fedoraproject.org/koji/taskinfo?taskID=2134029 [x] Rpmlint output: postgresql-orafce.src: W: spelling-error %description -l en_US trunc -> trunk, truncate, truncheon trunc is name of function [x] Package is not relocatable. [x] Buildroot is correct %{_tmppath}/%{name}-%{version}-%{release}-buildroot may need to change, see notes on bottom [x] Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines. [x] License field in the package spec file matches the actual license. License type: BSD [x] If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package is included in %doc. source package do not have license text [x] Spec file is legible and written in American English. [x] Sources used to build the package matches the upstream source, as provided in the spec URL. 40c6759b79255b705a80692006a45cd4cd6b18eea4629de26d056bc559404f64 orafce-3.0.1.tar.gz 40c6759b79255b705a80692006a45cd4cd6b18eea4629de26d056bc559404f64 ../SOURCES/orafce-3.0.1.tar.gz [x] Package is not known to require ExcludeArch [x] All build dependencies are listed in BuildRequires, except for any that are listed in the exceptions section of Packaging Guidelines. [-] The spec file handles locales properly. [-] ldconfig called in %post and %postun if required. [x] Package must own all directories that it creates. [x] Package requires other packages for directories it uses. [x] Package does not contain duplicates in %files. [x] Permissions on files are set properly. [x] Package has a %clean section, which contains rm -fR $RPM_BUILD_ROOT. [x] Package consistently uses macros. [x] Package contains code, or permissable content. [-] Large documentation files are in a -doc subpackage, if required. [x] Package uses nothing in %doc for runtime. [x] Header files in -devel subpackage, if present. [-] Static libraries in -devel subpackage, if present. [-] Package requires pkgconfig, if .pc files are present. [x] Development .so files in -devel subpackage, if present. [-] Fully versioned dependency in subpackages, if present. [x] Package does not contain any libtool archives (.la). [-] Package contains a properly installed %{name}.desktop file if it is a GUI application. [x] Package does not own files or directories owned by other packages. === SUGGESTED ITEMS === [x] Latest version is packaged. 3.0.1 [x] Package does not include license text files separate from upstream. [-] Description and summary sections in the package spec file contains translations for supported Non-English languages, if available. [x] Reviewer should test that the package builds in mock. Tested on: koji scratch build [x] Package should compile and build into binary rpms on all supported architectures. Tested on:koji scratch build [?] Package functions as described. [-] Scriptlets must be sane, if used. [-] The placement of pkgconfig(.pc) files is correct. [-] File based requires are sane. [-] %check is present and the tests pass ================ *** APPROVED *** ================
Folks, May I ask a name change? orafce is the upstream name. I'm trying to get of postgresql- prefixes in rawhide, so please let's get rid of this prefix in orafce package, too. i.e., postgresql-pgpool-II looks terribly long and stupid for me.
... and Jean-Francois, thanks a lot for the work.
I have nothing against either name. For the moment, everything is called postgresql-something. If you say that it will change soon in Fedora, it could be a good idea to rename it to orafce now so that it won't change down the line. But if the naming stay as it is, postgresql-orafce is the way to go. Maybe we can wait a little bit before importing the package to see what people think about this issue?
After discussion on devel list and points raised by the maintainer of PostgreSQL in Fedora/RHEL, I made the decision to match the name of this package with upstream. This seems to be the direction of future releases of Fedora and I think to name this package orafce now will ease the migration in the future. Thank you for the review! New Package CVS Request ======================= Package Name: orafce Short Description: Implementation of some Oracle functions into PostgreSQL Owners: jfsaucier Branches: F-11 F-12 F-13 InitialCC:
CVS done (by process-cvs-requests.py).
orafce-3.0.1-3.fc13 has been submitted as an update for Fedora 13. http://admin.fedoraproject.org/updates/orafce-3.0.1-3.fc13
orafce-3.0.1-3.fc12 has been submitted as an update for Fedora 12. http://admin.fedoraproject.org/updates/orafce-3.0.1-3.fc12
orafce-3.0.1-3.fc11 has been submitted as an update for Fedora 11. http://admin.fedoraproject.org/updates/orafce-3.0.1-3.fc11
orafce-3.0.1-3.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 orafce'. You can provide feedback for this update here: http://admin.fedoraproject.org/updates/orafce-3.0.1-3.fc13
orafce-3.0.1-3.fc12 has been pushed to the Fedora 12 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 orafce'. You can provide feedback for this update here: http://admin.fedoraproject.org/updates/orafce-3.0.1-3.fc12
orafce-3.0.1-3.fc11 has been pushed to the Fedora 11 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 orafce'. You can provide feedback for this update here: http://admin.fedoraproject.org/updates/orafce-3.0.1-3.fc11
orafce-3.0.1-3.fc13 has been pushed to the Fedora 13 stable repository. If problems still persist, please make note of it in this bug report.
orafce-3.0.1-3.fc11 has been pushed to the Fedora 11 stable repository. If problems still persist, please make note of it in this bug report.
orafce-3.0.1-3.fc12 has been pushed to the Fedora 12 stable repository. If problems still persist, please make note of it in this bug report.