Bug 709125

Summary: Review Request: obdgpslogger - OBDII and GPS data logger for your car
Product: [Fedora] Fedora Reporter: Gary Briggs <chunky>
Component: Package ReviewAssignee: Nobody's working on this, feel free to take it <nobody>
Status: CLOSED NOTABUG QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: unspecified    
Version: rawhideCC: fedora-package-review, martin.gieseking, notting, volker27
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
URL: http://icculus.org/obdgpslogger/
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2012-01-08 02:40:04 EST Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Bug Depends On:    
Bug Blocks: 201449    

Description Gary Briggs 2011-05-30 14:00:19 EDT
Spec URL: http://icculus.org/obdgpslogger/downloads/fedorapackaging/obdgpslogger.spec
SRPM URL: http://icculus.org/obdgpslogger/downloads/fedorapackaging/obdgpslogger-0.16-1.fc15.src.rpm
Description:

obdgpslogger has been my pet project for a few years, now. It's a tool to log OBDII and GPS data from your car, and convert it into useful formats such as GPX or KML.

It's entirely scratching an itch; there's almost no software out there for working with OBDII as well as GPS, even less that's open source, and absolutely none that's made it into any distributions yet.

It also contains a simulator I've written, that's used more extensively than the logger itself; most of the OBDII software developers I know of, even the closed source ones, are now using my sim.

I've been building this on a Fedora 15 x86_64 VM. At time of writing, rpmlint(1) has no errors, and one warning: A spelling error, "obdsim", which I feel is excusable since it's the name of the sim. I see no errors or warnings from mock(1).


If you wish to test the software itself or see that it executes without problems, use obdsim with the "-o" flag. That will launch the sim [which creates a PTY on Linux], and connect obdgpslogger to the slave end of the PTY with verbose output turned on; if you see spam to stdout, that means that both the sim and the logger are functioning correctly. The warning, "close(9) on netlib_connectsock()", is created by the gpsd library, not my software.
Comment 1 Gary Briggs 2011-05-30 14:02:03 EDT
Adding FE-NEEDSPONSOR to blocking bugs, this will hopefully be my first package.
Comment 2 Volker Fröhlich 2011-06-03 19:24:07 EDT
Please remove the Requires. RPM is smart enough to figure out these two. Please put each BuildRequires on a separate line, as it is more legible.

If you don't plan to maintain your package in EPEL 5 or older, you can remove the buildroot definition and the buildroot cleaning. You can drop %defattr as well.

The files section could be a bit more specific -- Especially for the bindir.

Feel free to add the TODO file to %doc, if you find it useful.

make and rm macros are no longer used -- just use plain make and rm. Despite that, please use %cmake instead of cmake.

Please adapt your Cmake files to use the system's sqlite and also add it to the BRs. Therefore, also delete the tarball's libs directory in the prep section.

The package description should end in a full stop.
Comment 3 Volker Fröhlich 2011-06-03 19:33:33 EDT
Oh, didn't notice OBD_SQLITE_INCLUDED_LIB before.
Comment 4 Gary Briggs 2011-06-03 20:04:30 EDT
I would like to maintain the spec for as many distributions as possible. I dream of this making it into currently supported distribution versions as well as upcoming ones, so I'm inclined to leave the clean stuff there until distributions that require it are EOL'd.

The requires I left in there are ones that aren't libraries that get linked to; it's useful that users have the binaries that come in those packages, rather than just the library packages that rpm automatically figures out.

Is that OK, or will they just be inherited as "Suggests"-type packages from the libraries that *are* set as requirements?

[In case that was incoherent, an example: RPM will figure out that it requires libgpsd, but the gpsd daemon itself comes in a separate package that cannot be inferred from ldd]

I'll fix everything else tonight and push new versions of those files. Thank-you for the review
Comment 5 Gary Briggs 2011-06-04 00:52:44 EDT
OK, I have made all of the changes suggested. I have uploaded the files to the same location as before:

Spec URL:
http://icculus.org/obdgpslogger/downloads/fedorapackaging/obdgpslogger.spec
SRPM URL:
http://icculus.org/obdgpslogger/downloads/fedorapackaging/obdgpslogger-0.16-1.fc15.src.rpm
Comment 6 Volker Fröhlich 2011-06-06 18:12:56 EDT
There is a spelling mistake in "Eearth". The last sentence of the description also seems a bit strange to me.

The make invocation should be:
make %{?_smp_mflags}

Please leave an explaining comment near where you delete the libs directory.

Whenever you publish an altered Spec file, please bump the release number. Don't upload a different file using the same release number, as it makes the reviewer's work harder. Besides that, you mustn't build two packages with the same release number afterwards, so it's good practice.

All relevant changes must also be noted in the changelog section of the spec file.

I didn't want to talk you out of maintaining your package in EPEL 5. Just wanted to let you know that some things are unnecessary if you don't want to. EPEL has a different policy on updating though: http://fedoraproject.org/wiki/EPEL/GuidelinesAndPolicies#Some_examples_of_what_package_updates_that_are_fine_or_not

RPM will install all packages that provide, what your package requires, as well as everything that is required therefore in turn. RPM does not have "suggests", as Debian has it. You can either require something or not.

You're absolutely right about these libraries not being linked. In the case of gpsd it works nevertheless:

$ rpm -qp --requires /home/makerpm/rpmbuild/RPMS/x86_64/obdgpslogger-0.16-1.fc14.x86_64.rpm
...
libgps.so.19()(64bit)
...


$ locate libgps.so.19
...
/usr/lib64/libgps.so.19
...

$ rpm -qf /usr/lib64/libgps.so.19
gpsd-2.95-2.fc14.x86_64

... which also provides libgpsd.
Comment 7 Gary Briggs 2011-06-07 00:45:45 EDT
I think I shall hold off on EPEL packaging; I feel there's more value in immediately targeting Fedora, and possibly worrying about backporting into EPEL later.

The new spec file is in the same place, the new SRPM is tagged with the new version suffix:

Spec URL: http://icculus.org/obdgpslogger/downloads/fedorapackaging/obdgpslogger.spec
SRPM URL: http://icculus.org/obdgpslogger/downloads/fedorapackaging/obdgpslogger-0.16-2.fc15.src.rpm
Comment 8 Gary Briggs 2011-06-13 23:42:58 EDT
I asked in IRC about followups, they suggested another ping by appending a comment on this bug. So, erm, here it is.
Comment 9 Martin Gieseking 2011-07-21 09:30:45 EDT
Gary, the package is almost ready. Just add a .desktop file for "obdgui" as described here:
https://fedoraproject.org/wiki/Packaging/Guidelines#desktop


Have you already done some informal reviews? This is usually necessary to show that your familiar with the Fedora packaging guidelines. Since you're allowed to formally review and to approve other packager's submissions once you're sponsored into the packager group, you should know how to do that correctly. :)
I'm a potential sponsor, BTW.

Here are some more information about the sponsoring process and the guidelines:
http://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group
https://fedoraproject.org/wiki/Packaging:ReviewGuidelines
https://fedoraproject.org/wiki/Packaging/Guidelines

In order to do a first informal review, please choose a yet uncommented review request not blocked by FE-NEEDSPONSOR, check all MUST and SHOULD items mentioned in the review guidelines (see link above), and post your results into the ticket.
Comment 10 Volker Fröhlich 2011-12-13 14:33:44 EST
Any news, Gary?
Comment 11 Volker Fröhlich 2012-01-08 02:40:04 EST
I'll close this ticket. If you re-gain interest, feel free to open it again.

http://fedoraproject.org/wiki/Policy_for_stalled_package_reviews#Submitter_not_responding