Bug 429221 - Review Request: dzcomm - Dzcomm a RS-232 API/lib
Review Request: dzcomm - Dzcomm a RS-232 API/lib
Status: CLOSED WONTFIX
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
low Severity medium
: ---
: ---
Assigned To: Nobody's working on this, feel free to take it
Fedora Extras Quality Assurance
:
Depends On:
Blocks: 429178
  Show dependency treegraph
 
Reported: 2008-01-17 19:12 EST by Ian Hands
Modified: 2009-05-22 12:56 EDT (History)
5 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-05-05 16:51:19 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Ian Hands 2008-01-17 19:12:09 EST
Spec URL: http://ian.ahands.org/fedora/dzcomm.spec
SRPM URL: http://ian.ahands.org/fedora/dzcomm-0.9.9i-1.fc8.src.rpm
Description: Dzcomm is a RS-232 API for as many OSes / platforms as we can achieve Originally designed to work on MS-DOS alongside the Allegro games programming library, Dzcomm can now work alongside the Allegro library as well as independently.  Dzcomm works on some *nixes as well.

This is my second package submission. I posted a package named scantool_net and this is a required dependency of that package. I guess I still need a sponsor(?).
Comment 1 Ian Hands 2008-01-17 19:17:31 EST
Doh I forgot to edit the title.
Comment 2 Mamoru TASAKA 2008-01-23 10:47:40 EST
! Note
  For general packaging guidelines you can refer to
  http://fedoraproject.org/wiki/Packaging/Guidelines
  http://fedoraproject.org/wiki/Packaging/ReviewGuidelines

I just did a quick glance at your spec file and just tried to rebuild
your srpm.

* Rebuild failed.
  http://koji.fedoraproject.org/koji/taskinfo?taskID=362863
  It seems that at least texinfo is missing from BuildRequires.
* Fix SourceURL (see "Sourceforge.net" of
  http://fedoraproject.org/wiki/Packaging/SourceURL )
* Don't use Epoch unless needed.
* BuildRoot does not match Fedora guidelines.
* Please make it sure that fedora specific compilation flags are
  honored correctly ("Compiler flags" of
  http://fedoraproject.org/wiki/Packaging/Guidelines )
* Compilation flag "-O3" is not allowed for Fedora because this
  makes debugging difficult.
* Also "-fomit-frame-pointer" is forbidden
* And please consider to remove "-ffast-math" as this relaxes
  calculation precision and may cause debugging difficult.
* We now recommend %defattr(-,root,root,-)
* Please write the %files entry more verbosely so that we can grasp
  what files are installed easily.
Comment 3 Mamoru TASAKA 2008-02-02 13:07:33 EST
ping?
Comment 4 Ian Hands 2008-02-02 17:08:01 EST
(In reply to comment #2)
> ! Note
>   For general packaging guidelines you can refer to
>   http://fedoraproject.org/wiki/Packaging/Guidelines
>   http://fedoraproject.org/wiki/Packaging/ReviewGuidelines
> 
> I just did a quick glance at your spec file and just tried to rebuild
> your srpm.
> 

> * Rebuild failed.
>   http://koji.fedoraproject.org/koji/taskinfo?taskID=362863
>   It seems that at least texinfo is missing from BuildRequires.
Added texinfo

> * Fix SourceURL (see "Sourceforge.net" of
>   http://fedoraproject.org/wiki/Packaging/SourceURL )
Changed dl.sf.net to downloads.sourceforge.net

> * Don't use Epoch unless needed.
Removed Epoch: 1

> * BuildRoot does not match Fedora guidelines.
The package has its own root dir in the zipfile... (I don't know the procedure
with this)

> * Please make it sure that fedora specific compilation flags are
>   honored correctly ("Compiler flags" of
>   http://fedoraproject.org/wiki/Packaging/Guidelines )
> * Compilation flag "-O3" is not allowed for Fedora because this
>   makes debugging difficult.
> * Also "-fomit-frame-pointer" is forbidden
> * And please consider to remove "-ffast-math" as this relaxes
>   calculation precision and may cause debugging difficult.
"make %{?_smp_mflags}" changed to "make %{optflags} %{?_smp_mflags}"
Does this cover the -O3 flag? Was that being used as the default? (I am not sure
how -O3 was making it in there).
Removal of -ffast-math? should the %{optflags} take care of this also?
Just looked at the configure script... Should I just remove the CFLAGS= line
then gcc would default to whatever the environment is set to (wich should be
%{optflags})? If so I can patch the configure file.

> * We now recommend %defattr(-,root,root,-)
changed to %defattr(-,root,root,-)

> * Please write the %files entry more verbosely so that we can grasp
>   what files are installed easily.
This one I am at a loss with.... it is just one .a and a bunch of .h files. rpm
-ql on the installed packages shows them all. (help please)

Sorry for the delay (busy at home and work) and the lack of knowledge on my part.

Thanks for the help,
-Ian
Comment 5 Mamoru TASAKA 2008-02-03 06:11:34 EST
First where is your new spec/srpm? (please change the release number
every time you modify your spec file to avoid confusion)

(In reply to comment #4)
> (In reply to comment #2)
> > * Please make it sure that fedora specific compilation flags are
> >   honored correctly ("Compiler flags" of
> >   http://fedoraproject.org/wiki/Packaging/Guidelines )
> > * Compilation flag "-O3" is not allowed for Fedora because this
> >   makes debugging difficult.
> > * Also "-fomit-frame-pointer" is forbidden
> > * And please consider to remove "-ffast-math" as this relaxes
> >   calculation precision and may cause debugging difficult.
> "make %{?_smp_mflags}" changed to "make %{optflags} %{?_smp_mflags}"
I guess "make %optflags %{?_smp_mflags}" won't work.

> Does this cover the -O3 flag? Was that being used as the default? (I am not sure
> how -O3 was making it in there).
No. Fedora specific compilation flags can be checked by
"rpm --eval %optflags". On rawhide i386 system, this returns:
----------------------------------------------------------------------------
[root@localhost ~]# rpm --eval %optflags
-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
--param=ssp-buffer-size=4 -m32 -march=i386 -mtune=generic
-fasynchronous-unwind-tables
----------------------------------------------------------------------------

> Removal of -ffast-math? should the %{optflags} take care of this also?
> Just looked at the configure script... Should I just remove the CFLAGS= line
> then gcc would default to whatever the environment is set to (wich should be
> %{optflags})? If so I can patch the configure file.
It seems that some patch is needed against configure.
Comment 6 Mamoru TASAKA 2008-02-12 07:02:55 EST
ping?
Comment 7 Ian Hands 2008-02-12 21:04:34 EST
Added a patch for configure... dzcomm-0.9.9i-2.fc8.src.rpm. All good?

Thanks,
-Ian
Comment 8 Mamoru TASAKA 2008-02-13 10:54:03 EST
Still not
* Actually this does not build.
  http://koji.fedoraproject.org/koji/taskinfo?taskID=422215
* Fedora specific compilation flags are yet not honored.
  Please recheck the section "Compiler flags" of
  http://fedoraproject.org/wiki/Packaging/Guidelines
  You can check what flags are used for Fedora now by
  `rpm --eval %optflags`.
* The installation directory is perhaps wrong.
--------------------------------------------------------------------
Installing lib/unix/libdzcom.a to /var/tmp/dz0.9.9i-2.fc9-root-mockbuild/lib
--------------------------------------------------------------------
  Shared libraries must be installed into %_libdir or /%_lib (in
  some special cases). On 64 bits this is /usr/lib64, not /usr/lib.
  Perhaps you want to use %configure macro

(In reply to comment #4)
> > * BuildRoot does not match Fedora guidelines.
> The package has its own root dir in the zipfile... (I don't know the procedure
> with this)
  What do you mean by this?
Comment 9 Ian Hands 2008-02-17 14:54:45 EST
(In reply to comment #8)
> Still not
> * Actually this does not build.
>   http://koji.fedoraproject.org/koji/taskinfo?taskID=422215
> * Fedora specific compilation flags are yet not honored.
>   Please recheck the section "Compiler flags" of
>   http://fedoraproject.org/wiki/Packaging/Guidelines
>   You can check what flags are used for Fedora now by
>   `rpm --eval %optflags`.
> * The installation directory is perhaps wrong.
> --------------------------------------------------------------------
> Installing lib/unix/libdzcom.a to /var/tmp/dz0.9.9i-2.fc9-root-mockbuild/lib
> --------------------------------------------------------------------
>   Shared libraries must be installed into %_libdir or /%_lib (in
>   some special cases). On 64 bits this is /usr/lib64, not /usr/lib.
>   Perhaps you want to use %configure macro
> 
> (In reply to comment #4)
> > > * BuildRoot does not match Fedora guidelines.
> > The package has its own root dir in the zipfile... (I don't know the procedure
> > with this)
>   What do you mean by this?

Okay the CFLAGS should be honored now.
I changed the BuildRoot to match what is suggested in the guidelines.
I can not figure out why the %configure macro doesn't work. When I enable it the
build fails right after (or during?) the find-debuginfo.sh... It seems the
script is looking for the directory "/var/tmp/$RPM_BUILD_ROOT" which is not there.

Any hints?

Thanks,
-Ian
Comment 10 Mamoru TASAKA 2008-02-18 12:34:40 EST
Well, first of all your srpm does not build.

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

I guess you are rebuilding srpm as root? Please don't do it,
i.e. please make it sure that your srpm builds as non-root users.
As rebuild fails before executing find-debuginfo.sh currently
I cannot figure out how find-debuginfo doesn't work for you.
Comment 11 Mamoru TASAKA 2008-02-26 11:26:44 EST
ping?
Comment 12 Mamoru TASAKA 2008-03-15 11:18:51 EDT
ping again?
Comment 13 Mamoru TASAKA 2008-04-03 12:02:51 EDT
ping again?
Comment 14 Mamoru TASAKA 2008-04-16 14:22:20 EDT
This review request will be closed if no response from the reporter
is received with in ONE WEEK.
Comment 15 Ian Hands 2008-04-16 19:53:27 EDT
Please excuse my absence... I have recently switched email addresses and have
been busy. I do want to continue working on this as I feel this project will be
used by some and it is an easy way for me to contribute to a project that gives
the open source (one that I use, and depend on) alot.

I wonder though; if because review requests are filed as bugs, it would be
better for this to actually be closed and I can resubmit it after I have done
some more work (and have a more time to dedicate to it). Is it bad to have one
open while waiting on a sporadic contributer? Either way I don't mind.

That being said I need to be pointed in the (general) direction of problem or
documentation. I think I really need to go in and mess with the Makefile via the
configure script so that it accepts rpm specific environment variables.

How is the $RPM_BUILD_ROOT passed from rpmbuild to ./configure or make does it
set BASH environment variable? I think the only reason the spec build was
working for me is because I was root and thus stuff was getting installed on the
system instead of in the proper temporary build root.

Sorry if some of my rambling is hard to follow.

Thanks,
-Ian
Comment 16 Mamoru TASAKA 2008-04-17 02:57:32 EDT
(In reply to comment #15)
> I wonder though; if because review requests are filed as bugs, it would be
> better for this to actually be closed and I can resubmit it after I have done
> some more work (and have a more time to dedicate to it). Is it bad to have one
> open while waiting on a sporadic contributer? Either way I don't mind.

Usually if you don't have time to continue a review request, the bug
should be closed so that someone else can open a new review request.

> That being said I need to be pointed in the (general) direction of problem or
> documentation. I think I really need to go in and mess with the Makefile via the
> configure script so that it accepts rpm specific environment variables.

When you try "rpmbuild --rebuild XXXX.src.rpm" build.log says:
--------------------------------------------------------------------------
Executing(%install): /bin/sh -e /var/tmp/rpm-tmp.35898
--------------------------------------------------------------------------
You can check how rpm specific variables are passed by seeing
the shell script

> I think the only reason the spec build was
> working for me is because I was root 

Yes.

> and thus stuff was getting installed on the
> system instead of in the proper temporary build root.

But actually the failing point is not that (according to the
build.log:)
http://koji.fedoraproject.org/koji/getfile?taskID=570216&name=build.log
-------------------------------------------------------------------------------
Installing lib/unix/libdzcom.a to /var/tmp/dzcomm-0.9.9i-3.fc9-uaWW0k/lib
/sbin/ldconfig /var/tmp/dzcomm-0.9.9i-3.fc9-uaWW0k/lib
/sbin/ldconfig: Can't create temporary cache file /etc/ld.so.cache~
: Permission denied
make: *** [install-lib] Error 1
error: Bad exit status from /var/tmp/rpm-tmp.35898 (%install)
-------------------------------------------------------------------------------
Comment 17 Hans de Goede 2008-05-03 05:58:40 EDT
Hi Ian,

Short intro: I'm a long time Fedora contributer and a sponsor. I haven't been
sponsoring anyone in a while as I've been rather busy with other stuff. But
getting new people into Fedora is important so I've decided to make some time
for sponsering again. As such I would like to sponsor you. The first step here
would be to finish up the packaging of dzcomm and scantool_net so that they are
in a state where they can be approved. The next step then is for you to either
package one or two more packages, for example from these lists:
http://fedoraproject.org/wiki/PackageMaintainers/WishList
http://fedoraproject.org/wiki/SIGs/Games/WishList

Or you can do some reviews of other people packages, when you do this please add
a note that it is not an official review as you aren't a contributer yet.

The purpose here is for you to show a good understanding of the packaging
guidelines. Once you've done a couple of good reviews and / or submitted one or
two more good packages, then you can apply for cvsextras membership in the
account system and I'll sponsor you.

---

For starters I would like to give you a hand with fixing some of the issues with
this package (dzcomm), as you're running into some issues which can be somewhat
hard to fix for someone new to packaging, and on top of that this package should
really be building a shared library not a static library as static libraries are
evil, which will require some further patching of the Makefiles.

However before I start spending time on this I would first like to get a firm
commitment from your side, are you willing to make time for this to make this
work, if there will be a silence of 2 weeks between comments all the time, then
this is not going to work.
Comment 18 Ian Hands 2008-05-05 16:44:28 EDT
Hello Hans,

First off thank you for your interest. I am a GNU/Linux user, and while Fedora
is not my main distribution of choice, I do feel a strong urge to give back to a
community that has given (in my eyes) desktop GNU/Linux so much.

As stated above I would rather have the bug temporarily put on hold (closed?)
until I can dedicate my time to it. There are many things happening from May -
June (July maybe) that will prevent me from working on this on a regular basis
(moving,  work changes, family vacation, etc.). I don't necessarily mind leaving
the bug open as is, but if it will inconvenience anyone (I guess it is not
"standard operations") please do with it what you must.

That being said I have thought of coding a project that will replace Scantool
GUI and thus the somewhat outdated Allegro and dzcomm dependencies. I know there
is a CLI only project called freediag that I believe is much better maintained,
and writing a GUI front end in SWT or Python should not be very difficult...
Hopefully I can port a few features from Scantool_net to freediag in the process
(It has been a while since I used freediag... maybe it is now feature rich(??)).

Either way I do hope in the near future (July?) I can take you up on that offer,
and deliver a firm commitment; whether it be on scantool_net, or a new project
of the same nature.

Thanks,
-Ian
Comment 19 Hans de Goede 2008-05-05 16:51:19 EDT
Ian,

In that case I'll close this and the other review request as won't fix. Please
open up a new request for any packages you want to submit when you've got more
time. Also drop me a private mail, or put me in the CC of those reviews, and
then we'll pick things up from there.

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