Bug 251805 - Review Request: orafce - Implementation of some Oracle functions into PostgreSQL
Review Request: orafce - Implementation of some Oracle functions into PostgreSQL
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Miroslav Suchý
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-08-11 10:43 EDT by Devrim GUNDUZ
Modified: 2016-08-14 12:28 EDT (History)
10 users (show)

See Also:
Fixed In Version: orafce-3.0.1-3.fc12
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2010-05-15 16:20:02 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
msuchy: fedora‑review+
tibbs: fedora‑cvs+


Attachments (Terms of Use)
postgresql-orafce build log (8.53 KB, application/octet-stream)
2007-08-24 10:27 EDT, KaiGai Kohei
no flags Details

  None (edit)
Description Devrim GUNDUZ 2007-08-11 10:43:21 EDT
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.
Comment 1 KaiGai Kohei 2007-08-24 10:27:34 EDT
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)
Comment 2 Ruben Kerkhof 2008-01-20 15:55:08 EST
Hi Devrim,

I'd love to do a full review, can you update the package according to comment #1?
Comment 3 Devrim GUNDUZ 2008-01-20 19:58:00 EST
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
Comment 4 Devrim GUNDUZ 2008-01-26 04:04:41 EST
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
Comment 5 Jason Tibbitts 2008-06-27 18:53:49 EDT
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?
Comment 6 Ruben Kerkhof 2008-08-24 06:19:20 EDT
Devrim, can you have a look at Jason's comments?
Comment 7 Devrim GUNDUZ 2008-08-27 04:23:39 EDT
(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.
Comment 8 Jason Tibbitts 2008-10-03 13:13:48 EDT
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.
Comment 9 Miroslav Suchý 2008-10-14 07:19:35 EDT
Sratch build in Koji failed:
http://koji.fedoraproject.org/koji/taskinfo?taskID=879804
Comment 10 Jason Tibbitts 2008-11-05 10:39:08 EST
It's been quite some time since there was any movement here.  This ticket will be closed soon if there is no response.
Comment 11 Devrim GUNDUZ 2008-11-16 10:50:47 EST
Please hold this bug open a bit more.
Comment 12 Jason Tibbitts 2009-03-07 17:36:25 EST
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.
Comment 13 Devrim GUNDUZ 2009-03-25 19:34:57 EDT
Ok, package is now built cleanly in mock:

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

I think we can approve this package now.
Comment 14 Itamar Reis Peixoto 2009-03-25 20:07:24 EDT
you can merge your install commands in 2 lines using 

install -Dp -m

your package seems to be good for me.
Comment 15 Miroslav Suchý 2009-03-30 13:29:50 EDT
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
Comment 17 Miroslav Suchý 2009-04-01 09:11:05 EDT
==============

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.
Comment 18 Devrim GUNDUZ 2009-06-04 15:43:15 EDT
3.0 version is about to be released, and I will update the package then.
Comment 19 Miroslav Suchý 2009-09-17 03:45:29 EDT
3.0.1 has been release on August 21, 2009
Do you want to continue with review or I can close this BZ?
Comment 21 Devrim GÜNDÜZ 2009-10-02 18:05:16 EDT
BTW, I changed package name to orafce, instead of postgresql-orafce.
Comment 22 Devrim GÜNDÜZ 2009-10-02 18:11:34 EDT
I was wondering whether this package should be a requirement for spacewalk.
Comment 23 Miroslav Suchý 2009-11-17 02:36:40 EST
>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.
Comment 24 Jean-Francois Saucier 2010-04-20 09:18:20 EDT
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.
Comment 25 Miroslav Suchý 2010-04-23 09:16:14 EDT
==============

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 ***
================
Comment 26 Devrim GÜNDÜZ 2010-04-23 09:41:25 EDT
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.
Comment 27 Devrim GÜNDÜZ 2010-04-23 09:42:07 EDT
... and Jean-Francois, thanks a lot for the work.
Comment 28 Jean-Francois Saucier 2010-04-23 10:16:46 EDT
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?
Comment 29 Jean-Francois Saucier 2010-04-26 10:33:05 EDT
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:
Comment 30 Jason Tibbitts 2010-04-28 21:43:44 EDT
CVS done (by process-cvs-requests.py).
Comment 31 Fedora Update System 2010-04-30 09:44:15 EDT
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
Comment 32 Fedora Update System 2010-04-30 09:45:34 EDT
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
Comment 33 Fedora Update System 2010-04-30 09:49:35 EDT
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
Comment 34 Fedora Update System 2010-04-30 19:49:05 EDT
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
Comment 35 Fedora Update System 2010-05-03 12:01:29 EDT
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
Comment 36 Fedora Update System 2010-05-03 12:01:58 EDT
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
Comment 37 Fedora Update System 2010-05-15 16:19:54 EDT
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.
Comment 38 Fedora Update System 2010-05-15 16:28:20 EDT
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.
Comment 39 Fedora Update System 2010-05-15 16:40:36 EDT
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.

Note You need to log in before you can comment on or make changes to this bug.