Bug 440597

Well, olpc2 seems to have only i386, however rebuild fails on
dist-f9 x86_64 and I think this is worth fixing.

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

Well, from a very quick glance:
* Please consider to use %?dist tag:
  http://fedoraproject.org/wiki/Packaging/DistTag
* License tag "LGPL" is invalid for current Fedora license policy:
  http://fedoraproject.org/wiki/Packaging/LicensingGuidelines
  http://fedoraproject.org/wiki/Licensing
* "Requires: liblo libsndfile" is redundant and should be removed.
  These types of library dependency are automatically detected and
  added to rebuilt binary rpms.
* Don't specify %python_site_dir in the way you write now and
  please refer to
  http://fedoraproject.org/wiki/Packaging/Python
* Remove long Authors entry from %description. If you want this,
  write them in a file and include it as %doc.
* Fedora specific compilation flags are not honored (you can check
  what flags are used by "$rpm --eval %optflags")
* Please make it sure that all directories created when installing a
  rpm are owned by the rpm. For example,
  %{_libdir}/csound/ is not owned by any package.

(In reply to comment #1)
> Well, olpc2 seems to have only i386, however rebuild fails on
> dist-f9 x86_64 and I think this is worth fixing.
> http://koji.fedoraproject.org/koji/taskinfo?taskID=569171

I am not sure this olpc version of Csound can be built on
x86_64. I don't think anything can be done about that. Is it
a problem?

Fedora rawhide csound srpm has a patch named
csound-5.03.0-64-bit-plugin-path.patch so please investigate the
patch.

(In reply to comment #4)
> Fedora rawhide csound srpm has a patch named
> csound-5.03.0-64-bit-plugin-path.patch so please investigate the
> patch.

Ummm.. It seems that on 64 bit csound does some tricky method
to install plugins corretly. As olpc2 seems to be supporting i386
only, I guess we can ignore it for now. So please fix the rest issues.

I will do. Thanks for the review.

I have now addressed all the points raised in the comments above. 
Could someone take another look? I would like to continue the review 
process, as the Csound developers for OLPC are depending on this.
I also need sponsoring...

Spec URL: http://music.nuim.ie/vlazzarini/tmp/olpcsound.spec
SRPM URL: http://music.nuim.ie/vlazzarini/tmp/olpcsound-5.08.91-0.fc7.src.rpm

The source rpm name has changed due the use of the dist tag.

From next time please change the EVR (Epoch-Version-Release)
of your spec/srpm every time you modify your spec/srpm to
avoid confusion.

For (2nd) 5.08.91-0:
* License
-------------------------------------------------------
Whole:		LGPLv2+

OOps/random.c			BSD
Opcodes/Loris/lorisgens5.C	GPLv2+
Opcodes/Loris/lorisgens5.h	GPLv2+
Opcodes/Loris/morphdemo.py	GPLv2+
Opcodes/py/pycall-gen.py	GPLv2+
Opcodes/scansyn.c		NON-FREE
Opcodes/scansyn.h		NON-FREE
Opcodes/scansynx.c		NON-FREE
SDIF/sdif-mem.c			MIT
SDIF/sdif-mem.h			MIT
SDIF/sdif.c			MIT
SDIF/sdif.h			MIT
examples/cscore/		GPLv2+
frontends/CsoundX/AudioCode/	NON-FREE
util/*.{c,h}			GPLv2+
util/sortex/			GPLv2+
-------------------------------------------------------
  - To follow http://fedoraproject.org/wiki/Packaging/LicensingGuidelines :
    * libscansyn.so is non-free and cannot be allowed for Fedora
      so please remove this.
    * libstdutil.so is under GPLv2+. So the license tag of olpcsound
      rpm should be "LGPLv2+ and GPLv2+". Also some explanation is
      needed on spec file. Please refer to the section
      "Multiple Licensing Scenarios" of "LicensingGuidelines" wiki.

* Requires
  - Requires for -devel subpackage is wrong for now as:
-------------------------------------------------------
$ rpm -qp --requires olpcsound-devel-5.08.91-0.olpc2.i386.rpm 
libcsnd.so.5.1  
libcsound.so.5.1  
olpcsound=5.08.91-0.olpc2  
rpmlib(CompressedFileNames) <= 3.0.4-1
rpmlib(PayloadFilesHavePrefix) <= 4.0-1
-------------------------------------------------------
    * This shows that -devel subpackage now Requies the rpm
      named "olpcsound=5.08.91-0.olpc2", not "olpcsound" with
      EVR 5.08.91-0.olpc2.

* Optflags
  - Would you explain why you want "-ffast-math"? This option
    changes (reduces) precision and may render debugging difficult.

* Macros
  - Use macros properly. For example /usr/bin should be %_bindir.
    (by the way why do you want to call scons by full path?)

* Directory ownership issue
  - Again please make it sure all directories created when installing
    a rpm are owned by the rpm.
    * For example, %_libdir/csound is not owned by any package.

* Build working directoryy issue
---------------------------------------------------------
%files -f %{_builddir}/%{name}-%{version}/csound5.lang
---------------------------------------------------------
  - %{_builddir}/%{name}-%{version}/ part is redundant because
    at this stage the working directory is the directory.

* debuginfo rpm issue
  - build.log says:
---------------------------------------------------------
   808  + /usr/lib/rpm/find-debuginfo.sh /builddir/build/BUILD/olpcsound-5.08.91
   809  extracting debug info from
/var/tmp/olpcsound-5.08.91-0.olpc2-root-mockbuild/usr/lib/libcsound.so.5.1
   810  extracting debug info from
/var/tmp/olpcsound-5.08.91-0.olpc2-root-mockbuild/usr/lib/libcsnd.so.5.1
   811  extracting debug info from
/var/tmp/olpcsound-5.08.91-0.olpc2-root-mockbuild/usr/lib/python2.5/site-packages/_csnd.so
   812  0 blocks
---------------------------------------------------------
    This means that these binaries are stripped before %install
    stage ends. Make it sure that these binaries are _not_ stripped
    to create debuginfo rpm properly.

* Documents
  - Please add the following files to %doc.
---------------------------------------------------------
AUTHORS
---------------------------------------------------------
  - The file "INSTALL" is for people who want to build and install
    a software by themselves and is not needed for people who use
    rpm system.

* rpmlint issue
---------------------------------------------------------
olpcsound.i386: W: file-not-utf8 /usr/share/doc/csound/readme-csound5.txt
olpcsound.i386: E: description-line-too-long olpcsound .....
olpcsound.i386: W: no-version-in-last-changelog
---------------------------------------------------------
  - Change the encoding of %_docdir/%name/readme-csound5.txt to UTF-8.
  - Make it sure that all lines in %desctiption should have less than
    80 characters.
  - Add EVR info to %changelog

I can take care of everything bar this one:

*debuginfo rpm issue: the install script strips the binaries. 
I can change that, but then where do I do the stripping? Should
I do it explicitely in the rpm somewhere?
Besides, the csound spec (approved by Fedora) does seem to do the
same thing, so if it's wrong here it is wrong there too.

(That goes also for the ownership of directories. In the approved
csound spec, no one owns the %_libdir/csound directory, just the
%_libdir/csound/plugins. So it's wrong there too)

(In reply to comment #9)
> I can take care of everything bar this one:
> 
> *debuginfo rpm issue: the install script strips the binaries. 
> I can change that, but then where do I do the stripping? Should
> I do it explicitely in the rpm somewhere?

You should _not_ strip those binaries. rpmbuild automatically
strips those binaries properly at last.
Some explanation is written in
http://fedoraproject.org/wiki/Packaging/Debuginfo

> Besides, the csound spec (approved by Fedora) does seem to do the
> same thing, so if it's wrong here it is wrong there too.
I have not checked csound spec file, from your comment csound
spec file is also wrong.

> (That goes also for the ownership of directories. In the approved
> csound spec, no one owns the %_libdir/csound directory, just the
> %_libdir/csound/plugins. So it's wrong there too)
Same above. 

Ok that's easy, just take the strip out of the installer 
script. Where does it say that rpmbuild does the stripping?
I remember seeing that the binary package built without the 
stripping was about 5 times bigger than the one using the
stripping installer (which tells me it was not stripped).
But if you say it's done automatically, I am OK with it
(but I will check).

Someone ought to look at the fedora project csound.spec. I could
do it once I get this done and am sponsored.

Thanks.

(In reply to comment #11)
> Ok that's easy, just take the strip out of the installer 
> script. Where does it say that rpmbuild does the stripping?

I said this on my comment 8. Or you can check what __spec_install_post
does.

ping?

sorry had no time to make the needed changes. I am hoping to
have time early next week, from my university work. There is
one morning's worth of work there, checking everything, so
I should have it by Monday afternoon. 

I think I have addressed everything now. Here is the
latest src rpm and spec:

Spec URL: http://music.nuim.ie/vlazzarini/tmp/olpcsound.spec
SRPM URL: http://music.nuim.ie/vlazzarini/tmp/olpcsound-5.08.92-0.fc7.src.rpm

Thanks again

$ LANG=C rpm -K olpcsound-5.08.92-0.fc7.src.rpm 
error: olpcsound-5.08.92-0.fc7.src.rpm: headerRead failed

It seems that your srpm itself is broken...

sorry, something went wrong in the upload. It seems to be fixed now,
same links as above.

For 5.08.92-0:

* License
------------------------------------------------
Opcodes/Loris/		GPLv2+
examples/cscore/	GPLv2+
------------------------------------------------
  - Now libstdutil.so is also under LGPLv2+
    (also checked 2008-05-08 on ChangeLog)
    License tag can be "LGPLv2+" now.

* Compiler flags
------------------------------------------------
   164  gcc -o Engine/auxfd.os -c -O3 -mtune=k6 -DGNU_GETTEXT -DUSE_LRINT -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 -g -fomit-frame-pointer -DLINUX -DPIPES
-fvisibility=hidden -fPIC -DHAVE_LIBSNDFILE=1016 -DBETA
-DENABLE_OPCODEDIR_WARNINGS=0 -DOLPC -DHAVE_SOCKETS -DHAVE_FCNTL_H
-DHAVE_UNISTD_H -DHAVE_STDINT_H -DHAVE_SYS_TIME_H -DHAVE_SYS_TYPES_H
-DHAVE_TERMIOS_H -DHAVE_SOCKETS -DHAVE_DIRENT_H -D__BUILDING_LIBCSOUND -I. -IH
-I/usr/include/fltk-1.1 -I/usr/local/include -I/usr/include -I/usr/include
-I/usr/X11R6/include Engine/auxfd.c
------------------------------------------------
  - -fomit-frame-pointer is strictly forbidden for Fedora as
    this makes debugging almost impossible.

* Directory ownership issue
  - %_includedir/csound is not owned by any packages.

* rpmlint issue
------------------------------------------------
olpcsound.i386: E: description-line-too-long olpcsound is a subset of the Csound
sound and music synthesis system, tailored specifically for  XO platform.
------------------------------------------------
  - Please check $ rpmlint -I description-line-too-long

Then:
-------------------------------------------------------------
NOTE: Before being sponsored:

This package will be accepted with another few work. 
But before I accept this package, someone (I am a candidate) 
must sponsor you.

Once you are sponsored, you have the right to review other 
submitters' review requests and approve the packages formally. 
For this reason, the person who want to be sponsored (like you) 
are required to "show that you have an understanding 
of the process and of the packaging guidelines" as is described
on :
http://fedoraproject.org/wiki/PackageMaintainers/HowToGetSponsored

Usually there are two ways to show this.
A. submit other review requests with enough quality.
B. Do a "pre-review" of other person's review request
   (at the time you are not sponsored, you cannot do
   a formal review)

When you have submitted a new review request or have pre-reviewed other 
person's review request, please write the bug number on this bug report 
so that I can check your comments or review request.

Fedora package collection review requests which are waiting for someone to
review can be checked on:
http://fedoraproject.org/PackageReviewStatus/NEW.html
(NOTE: please don't choose "Merge Review")


Review guidelines are described mainly on:
http://fedoraproject.org/wiki/Packaging/ReviewGuidelines
http://fedoraproject.org/wiki/Packaging/Guidelines
http://fedoraproject.org/wiki/Packaging/ScriptletSnippets
------------------------------------------------------------

OK these are the fixes

Spec URL: http://music.nuim.ie/vlazzarini/tmp/olpcsound.spec
SRPM URL: http://music.nuim.ie/vlazzarini/tmp/olpcsound-5.08.92-1.fc7.src.rpm

Did you say you are waiting yourself to be sponsored? Does this mean I
need to get someone else to do an official review?
Thanks

No, what I am saying is that you must
- sumbit another review request
- or do a pre-review of other person's review request
so that I or another sponsor can sponsor you (I am sponsor members).

Ok. Sorry I misunderstood you. I will try and submit another
package. This might take a few days to prepare. Thanks

I have submitted a new review request, here is the bug number: 448702
Thanks.

* Source file
----------------------------------------------------
5687217 2008-05-19 21:03 olpcsound-5.08.92-0.fc7/olpcsound-5.08.92.tar.bz2
5660600 2008-05-20 18:29 olpcsound-5.08.92-1.fc7/olpcsound-5.08.92.tar.bz2
  - What happened?
----------------------------------------------------

* rpmlint
  - Also now rpmlint complains about spurious-executable-perm
    on -debuginfo rpm:
----------------------------------------------------
olpcsound-debuginfo.i386: W: spurious-executable-perm
/usr/src/debug/olpcsound-5.08.92/util/mixer.c
olpcsound-debuginfo.i386: W: spurious-executable-perm
/usr/src/debug/olpcsound-5.08.92/Opcodes/hrtfopcodes.c
----------------------------------------------------

* Changelog
----------------------------------------------------
olpcsound-debuginfo.i386: W: no-version-in-last-changelog
----------------------------------------------------
  - Please write %changelog in the format like:
----------------------------------------------------
* Mon May 19 2008 Victor.Lazzarini <vlazzarini>  - 5.08.92-1
  - fixed license
  - removed -fomit-frame-pointer from SConstruct
  - fixed description
----------------------------------------------------

Thanks, I realised only today about the changelog format
as I was doing the othe package. 
As for the source code, I have removed some files that were
not being built, so the source is smaller now.
Not sure about the spurious exec perm, will check and fix.
Thanks again for your help.

(In reply to comment #24)
> As for the source code, I have removed some files that were
> not being built, so the source is smaller now.

Please don't modify the tarball used as the source of srpm by yourself
but use the upstream released tarball as it is (except for some special
case).
Or are you one of the upstream developer and 5.08.92 tarball is
not a formally released one?

> Not sure about the spurious exec perm, will check and fix.
Usually the permissions of the files in source tarball are wrong.

> Or are you one of the upstream developer and 5.08.92 tarball is
not a formally released one?

I am one of the upstream developers and the 5.08.92 is a formally
released one, which has been updated. Is that a problem?

The problem is that the tarball downloaded from the URL
and the tarball in your srpm differ, which is very confusing.

We don't allow to modify the upstream tarball _itself_ except
for some special case (of course I am not saying that we should not
apply any patches or so, just saying that the tarball itself
must coincide with what is on upstream URL).
If you are the upstream and want to modify the tarball _itself_,
please release the new version of tarball.

yes, that is my plan: the tarball that goes in the srpm should
be the same one release in the URL. In my hurry to submit fixes
I might have forgot to update the release. But when I submit the
fixes tomorrow, I will also release a matching tarball. Would
that be OK?
Thanks

Yes :)

I think the problems above are now corrected:

Spec URL: http://music.nuim.ie/vlazzarini/tmp/olpcsound.spec
SRPM URL: http://music.nuim.ie/vlazzarini/tmp/olpcsound-5.08.92-2.fc7.src.rpm

I have also updated the sourceforge release olpcsound tarball

Thanks

This time your srpm does not build.
http://koji.fedoraproject.org/koji/taskinfo?taskID=635125

By the way;
(In reply to comment #30)
> I have also updated the sourceforge release olpcsound tarball

Please don't! Please do not change the released tarball itself
without changing the version.

5661154 2008-05-30 01:35 new/olpcsound-5.08.92.tar.bz2
5687217 2008-05-19 21:19 old/olpcsound-5.08.92.tar.bz2

(In reply to comment #27)
> If you are the upstream and want to modify the tarball _itself_,
> please release the *new version* of tarball.

build log says

Checking for C header file stdio.h... 
no
 *** Failed to compile a simple test program. The compiler is
 *** possibly not set up correctly, or is used with invalid flags.
 *** Check config.log to find out more about the error.

This seems to indicate that the compiler is not set up properly,
as stdio.h is not found.
I have seen this happen in fc7 systems using ccache, which causes
problems for scons. There is nothing I can do, because the same
code builds elsewhere. Could you suggest anything?
Thanks 

(In reply to comment #31)
> This time your srpm does not build.
> http://koji.fedoraproject.org/koji/taskinfo?taskID=635125

Sorry I built this not for the target olpc-3 but for the target
dist-f10. Actually on i386 (i.e. on olpc-3) build succeeds.
Will check later. 

Okay, good

----------------------------------------------------------------------
    This package (olpcsound) is APPROVED by me
----------------------------------------------------------------------

Please follow the procedure written on:
http://fedoraproject.org/wiki/PackageMaintainers/Join
from "Get a Fedora Account".
At a point a mail should be sent to sponsor members which notifies
that you need a sponsor. At the stage, please also write on
this bug for confirmation that you requested for sponsorship and
your FAS (Fedora Account System) name. Then I will sponsor you.

Thanks! 
My FAS name is veplaini

Now I should be sponsoring you. Please follow "Join" wiki
again.

New Package CVS Request
=======================
Package Name: olpcsound
Short Description: olpc subset of csound 5
Owners: veplaini
Branches: olpc-3
InitialCC: veplaini
Cvsextras Commits: yes

cvs done.

Please try to build on koji.

I will. I just have been unable to work on this, as I have been
busy with University stuff admin and marking. Thanks.

I finally was able to come back to it. Two horrid weeks
of admin work at my Uni. Anyway, all seemed to have gone
well: http://koji.fedoraproject.org/koji/buildinfo?buildID=53372

One question: At work, I am behind a firewall and the only way
to get out is through the proxy. I use a socks proxy with
cvs and curl, but I could not figure out how to make koji connect
(I tried tsocks which is what I use with cvs, to no avail). Do
you have any suggestions? As it is I am building from home, which
is not ideal.

Should I close this ticket?

Thanks for all your help, it really made a difference.

Package Change Request
======================
Package Name: olpcsound
New Branches: olpc-2

(In reply to comment #41)
> One question: At work, I am behind a firewall and the only way
> to get out is through the proxy. I use a socks proxy with
> cvs and curl, but I could not figure out how to make koji connect
> (I tried tsocks which is what I use with cvs, to no avail). Do
> you have any suggestions? As it is I am building from home, which
> is not ideal.

Well, I am not a expert of koji and currently I cannot tell you
how to deal with that soon. Maybe it is better that you ask on
fedora-devel-list.
 
> Should I close this ticket?

Yes, now I am closing.

cvs done.