Bug 975310 - Review Request: odb - C++ Object-Relational Mapping
Review Request: odb - C++ Object-Relational Mapping
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
unspecified Severity medium
: ---
: ---
Assigned To: Peter Lemenkov
Fedora Extras Quality Assurance
:
Depends On: 975309
Blocks:
  Show dependency treegraph
 
Reported: 2013-06-18 00:51 EDT by Dave Johansen
Modified: 2013-11-10 01:12 EST (History)
4 users (show)

See Also:
Fixed In Version: odb-2.2.2-1.fc20
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2013-10-26 23:59:37 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
lemenkov: fedora‑review+
limburgher: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Dave Johansen 2013-06-18 00:51:30 EDT
Spec URL: https://docs.google.com/file/d/0B9bQ41XLJ8QuakFLa0NDUE5NTXc/edit?usp=sharing
SRPM URL: https://docs.google.com/file/d/0B9bQ41XLJ8QuWWFFSk9ZSFRuMjQ/edit?usp=sharing
Description: ODB is an open-source, cross-platform, and cross-database object-relational mapping (ORM) system for C++. It allows you to persist C++ objects to a relational database without having to deal with tables, columns, or SQL and without manually writing any mapping code. ODB supports MySQL, SQLite, PostgreSQL, Oracle, and Microsoft SQL Server relational databases as well as C++98/03 and C++11 language standards. It also comes with optional profiles for Boost and Qt which allow you to seamlessly usevalue types, containers, and smart pointers from these libraries in your persistent C++ classes.
Fedora Account System Username: daveisfera

NOTE: This is my first package and I'm seeking a sponsor.
Comment 1 Dave Johansen 2013-06-18 00:53:17 EDT
I forgot to mention that I didn't do a koji build because it depends on libcutl ( see the build for it here: http://koji.fedoraproject.org/koji/taskinfo?taskID=5515092 ) and so it wouldn't work, if I had tried.
Comment 2 Peter Lemenkov 2013-06-18 10:47:14 EDT
Unblocking FE-NEEDSPONSOR - I've just sponsored Dave.
Comment 3 Dave Johansen 2013-07-13 01:09:04 EDT
The updated files can be found at:
Spec URL: http://daveisfera.fedorapeople.org/odb_2.2/specs/odb.spec
SRPM URL: http://daveisfera.fedorapeople.org/odb_2.2/SRPMS/odb-2.2.2-1.fc19.src.rpm
Comment 4 Dave Johansen 2013-07-24 00:47:18 EDT
rpmlint outputs that the file odb-manual.ps is not UTF-8. Should I exclude the .ps since same content is available in a .pdf as well? Or should I just leave it in?
Comment 5 Boris Kolpackov 2013-07-27 10:42:07 EDT
Dave, a couple of comments and suggestions about this spec file:

1. You exclude i386 saying that "It won't build on i386 because it needs the __exchange_and_add and __atomic_fetch_add_4 built-ins". That's strange since ODB code doesn't (directly) use those. Can you elaborate what needs them?

2. You specify --disable-static to configure (two places). This is the default and is not necessary. I would suggest that you remove it in order not to suggest that something special is going on here.

3. I suggest that you add the --with-options-file=../../etc/odb/default.options
to configure and add the default.options file to the package. This file can be used to specify installation-wide options to the ODB compiler (e.g., -I, -D, etc). It can be really handy sometimes. Here is the contents of this file that we include with our make-shift .rpm package:

----cut here----
# Default ODB options file. This file is automatically loaded by the ODB                                                                                        
# compiler and can be used for installation-wide customizations, such as                                                                                        
# adding an include search path for a commonly used library. For example:                                                                                       
#                                                                                                                                                               
# -I /opt/boost_1_45_0                                                                                                                                          
#   
----cut here---
Comment 6 Peter Lemenkov 2013-07-29 09:33:03 EDT
(In reply to Dave Johansen from comment #3)
> The updated files can be found at:
> Spec URL: http://daveisfera.fedorapeople.org/odb_2.2/specs/odb.spec
> SRPM URL:
> http://daveisfera.fedorapeople.org/odb_2.2/SRPMS/odb-2.2.2-1.fc19.src.rpm

Dave, is this the latest package?
Comment 7 Dave Johansen 2013-07-29 10:15:24 EDT
I just updated that link to the latest one, but I haven't yet addressed the comments from Boris (the upstream maintainer).
Comment 8 Dave Johansen 2013-08-30 01:11:20 EDT
Ok, sorry this took so long, but the .spec and source .rpm with the updates based on the upstream maintainers comments are available at:
http://daveisfera.fedorapeople.org/odb_2.2/specs/odb.spec
http://daveisfera.fedorapeople.org/odb_2.2/SRPMS/odb-2.2.2-1.el6.src.rpm

I just realized that there's a potential issue with this package. For RHEL 5/6, this package requires devtoolset-1.1 for gcc plugin support (only available in gcc 4.5 and higher). Is there a way to use the devtoolset in the EPEL build process?


Here's the responses to the questions in comment #5:

> 1. You exclude i386 saying that "It won't build on i386 because it needs the __exchange_and_add and __atomic_fetch_add_4 built-ins". That's strange since ODB code doesn't (directly) use those. Can you elaborate what needs them?

I looked at this, and it's actually the code in devtoolset-1.1 that is using this. So I'm going to interpret the statements made here ( https://fedoraproject.org/wiki/Architectures#ExcludeArch_.26_ExclusiveArch ) as indicating that I shouldn't include the ExcludeArch because it's not explicitly this package that is having the issue with the i386 architecture. Please let me know if that's an incorrect interpretation.


> 2. You specify --disable-static to configure (two places). This is the default and is not necessary. I would suggest that you remove it in order not to suggest that something special is going on here.

This was just the way that the template that I based .spec file on was and it's what I've seen in other .spec files, so I believe it's "common pratice", but please let me know if it should be removed.


> 3. I suggest that you add the --with-options-file=../../etc/odb/default.options
to configure and add the default.options file to the package. This file can be used to specify installation-wide options to the ODB compiler (e.g., -I, -D, etc). It can be really handy sometimes.

I added /etc/odb.conf as a config file for the package and populated it with the Boost 1.41 stuff on RHEL 5.
Comment 9 Boris Kolpackov 2013-08-30 08:56:38 EDT
> I added /etc/odb.conf as a config file for the package

I would strongly suggest that you use the /etc/odb/default.options variant for the following reasons:

1. Other packages already use this naming so it will be more consistent from the user' perspective (i.e., someone switching from Ubuntu won't have to "know" that the default options file has a different name and location on RH).

2. This file is an "options file" in the ODB sense. That is, it has the same format as files that can be passed to the ODB compiler with the --options-file option. The .options extension is used throughout the documentation so calling it odb.options instead of odb.conf will again be more consistent.

3. In the future we may want to add some other "config" file for ODB. Having a sub-directory in /etc from the start makes this easy.
Comment 10 Dave Johansen 2013-08-30 10:45:59 EDT
Sounds good. That's an easy thing to change and so I can definitely put the file in /etc/odb/default.options
Comment 11 Dave Johansen 2013-08-31 11:37:52 EDT
I updated the default options file based on the feedback from upstream and they updated files are available at the previously posted links.
Comment 12 Peter Lemenkov 2013-09-15 11:46:16 EDT
Successful Koji build for F-19:

* http://koji.fedoraproject.org/koji/taskinfo?taskID=5937080

REVIEW:

Legend: + = PASSED, - = FAILED, 0 = Not Applicable

+ rpmlint is silent

Auriga ~: rpmlint Desktop/odb-
odb-2.2.2-1.fc19.src.rpm               odb-2.2.2-1.fc19.x86_64.rpm            odb-debuginfo-2.2.2-1.fc19.x86_64.rpm  
Auriga ~: rpmlint Desktop/odb-*
odb.src:69: W: macro-in-comment %{_sysconfdir}
odb.src:69: W: macro-in-comment %{name}
odb.src:78: W: macro-in-comment %{_sysconfdir}
odb.src:78: W: macro-in-comment %{name}

^^^ these are harmless, however in the future please escape macro substitution when unnecessary by putting additional "%" sign at the beginning. E.g. %%{_sysconfdir}.

odb.x86_64: W: conffile-without-noreplace-flag /etc/odb.conf

^^^ Are you shure you don't want to set "noreplace" here? Also as Boris stated above it's better just to use /etc/odb/default.options instead.

odb.x86_64: W: file-not-utf8 /usr/share/doc/odb-2.2.2/odb-manual.ps

^^^ Looks harmless.

3 packages and 0 specfiles checked; 0 errors, 6 warnings.
Auriga ~: 

+ The package is named according to the  Package Naming Guidelines.
+ The spec file name matches the base package %{name}, in the format %{name}.spec.
+ The package meets the Packaging Guidelines.
+ The package is licensed with a Fedora approved license and meets the Licensing Guidelines.
+ The License field in the package spec file matches the actual license (GPLv3 only).
+ The file, containing the text of the license(s) for the package, is included in %doc.
+ The spec file is written in American English.
+ The spec file for the package is legible.
+ The sources used to build the package, match the upstream source, as provided in the spec URL.

sulaco ~/rpmbuild/SOURCES: sha256sum odb-2.2.2.tar.bz2*
37e1ee6979a83e0de1080126e26396544a924b5a99d079e3477abdb2e35c993e  odb-2.2.2.tar.bz2
37e1ee6979a83e0de1080126e26396544a924b5a99d079e3477abdb2e35c993e  odb-2.2.2.tar.bz2.1
sulaco ~/rpmbuild/SOURCES: 

+ The package successfully compiles and builds into binary rpms on at least one primary architecture. See Koji link above.

+/- All build dependencies MUST be listed in BuildRequires. Well, I really don't like the situation with EPEL5 and EPEL5 (devtoolset-related stuff which isn't available in Koji yet AFAIK). However I won't insist on improving this ASAP. I propose requesting only Fedora branches after the approval and add EL-branches as soon as it will be possible to build ODB cleanly for EL5 and EL6 within Koji.

0 No need to handle locales.
0 No shared library files in some of the dynamic linker's default paths.
+ The package does NOT bundle copies of system libraries.
0 The package is not designed to be relocatable.
+ The package owns all directories that it creates.
+ The package does not list a file more than once in the spec file's %files listings.
+ Permissions on files are set properly.
+ The package has a %clean section, which contains rm -rf %{buildroot} (or $RPM_BUILD_ROOT).
+ The package consistently uses macros.
+ The package contains code, or permissible content.
0 No extremely large documentation files.
+ Anything, the package includes as %doc, does not affect the runtime of the application.
0 No C/C++ header files.
0 No static libraries.
0 No pkgconfig(.pc) files.
0 The package doesn't contain library files without a suffix (e.g. libfoo.so) in some of the dynamic linker's default paths.
0 No devel sub-package.
+ The package does NOT contain any .la libtool archives.
0 Not a GUI application.
+ The package does not own files or directories already owned by other packages.
+ At the beginning of %install, the package runs rm -rf %{buildroot} (or $RPM_BUILD_ROOT).
+ All filenames in rpm packages are valid UTF-8.

Ok, considering that 

* you will continue working on it to support EL5/EL6/(maybe EL7) branches properly, 
* you'll address Boris' concern about the config file default code location,


this package is

APPROVED.
Comment 13 Boris Kolpackov 2013-09-16 04:27:26 EDT
> odb.x86_64: W: conffile-without-noreplace-flag /etc/odb.conf
> 
> ^^^ Are you shure you don't want to set "noreplace" here?

Agree, we most likely don't want to replace this file on package upgrades.
Comment 14 Dave Johansen 2013-09-17 00:44:07 EDT
> Auriga ~: rpmlint Desktop/odb-*
> odb.src:69: W: macro-in-comment %{_sysconfdir}
> odb.src:69: W: macro-in-comment %{name}
> odb.src:78: W: macro-in-comment %{_sysconfdir}
> odb.src:78: W: macro-in-comment %{name}
> 
> ^^^ these are harmless, however in the future please escape macro substitution when unnecessary by putting additional "%" sign at the beginning. E.g. %%{_sysconfdir}.

This was an easy fix, so I fixed it.

> odb.x86_64: W: conffile-without-noreplace-flag /etc/odb.conf
> 
> ^^^ Are you shure you don't want to set "noreplace" here? Also as Boris stated above it's better just to use /etc/odb/default.options instead.

Yes, I did want "noreplace". I'm just still fairly new to writing spec files and forgot to add it.

> odb.x86_64: W: file-not-utf8 /usr/share/doc/odb-2.2.2/odb-manual.ps
> 
> ^^^ Looks harmless.

This is also available as a .pdf, so I could remove it, if desired, but it sounds like it's ok to leave it in, so I will.

> +/- All build dependencies MUST be listed in BuildRequires. Well, I really don't like the situation with EPEL5 and EPEL5 (devtoolset-related stuff which isn't available in Koji yet AFAIK). However I won't insist on improving this ASAP. I propose requesting only Fedora branches after the approval and add EL-branches as soon as it will be possible to build ODB cleanly for EL5 and EL6 within Koji.

This package requires gcc 4.5 or greater for plugin support and such a version of gcc is available in EL 5/6 through the devtoolset ( https://access.redhat.com/site/documentation/Red_Hat_Developer_Toolset/ ) but this requires a developer subscription and is not currently available on Koji.

There is currently discussion about support for SCLs in Fedora packaging ( https://lists.fedoraproject.org/pipermail/packaging/2013-September/009525.html ) and I will participate in that and see if the use of the devtoolset can be integrated into the EPEL to allow for packaging of this package in the EPEL for EL 5/6.


> Ok, considering that 
> 
> * you will continue working on it to support EL5/EL6/(maybe EL7) branches properly, 

EL7 support should work out of the box, but see above note about hopefully using devtoolset in the EPEL.

> * you'll address Boris' concern about the config file default code location,

I changed the default.conf location to match upstream's standard configuration.
Comment 15 Dave Johansen 2013-09-17 01:04:20 EDT
New Package SCM Request
=======================
Package Name: odb
Short Description: Object-relational mapping (ORM) system for C++
Owners: daveisfera
Branches: f18 f19 f20 
InitialCC: peter
Comment 16 Gwyn Ciesla 2013-09-17 08:38:46 EDT
Git done (by process-git-requests).
Comment 17 Fedora Update System 2013-10-17 10:06:09 EDT
odb-2.2.2-1.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/odb-2.2.2-1.fc18
Comment 18 Fedora Update System 2013-10-17 10:06:21 EDT
odb-2.2.2-1.fc19 has been submitted as an update for Fedora 19.
https://admin.fedoraproject.org/updates/odb-2.2.2-1.fc19
Comment 19 Fedora Update System 2013-10-17 10:06:32 EDT
odb-2.2.2-1.fc20 has been submitted as an update for Fedora 20.
https://admin.fedoraproject.org/updates/odb-2.2.2-1.fc20
Comment 20 Peter Lemenkov 2013-10-17 10:09:59 EDT
Dave, you forgot to file a Bodhi requests for pushing these builds to updates-testing. But don't worry - I've just did it for you :)

* https://fedoraproject.org/wiki/Bodhi

I prefer submitting package via web-interface rather than from commandline:

* https://admin.fedoraproject.org/updates/new/
Comment 21 Fedora Update System 2013-10-17 16:19:58 EDT
odb-2.2.2-1.fc20 has been pushed to the Fedora 20 testing repository.
Comment 22 Fedora Update System 2013-10-26 23:59:37 EDT
odb-2.2.2-1.fc18 has been pushed to the Fedora 18 stable repository.
Comment 23 Fedora Update System 2013-10-27 01:34:56 EDT
odb-2.2.2-1.fc19 has been pushed to the Fedora 19 stable repository.
Comment 24 Fedora Update System 2013-10-27 01:35:25 EDT
odb-2.2.2-1.fc18 has been pushed to the Fedora 18 stable repository.
Comment 25 Fedora Update System 2013-11-10 01:12:32 EST
odb-2.2.2-1.fc20 has been pushed to the Fedora 20 stable repository.

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