Spec URL: http://www.iammetal.co.uk/jokosher.spec SRPM URL: http://www.iammetal.co.uk/jokosher-0.1-1.src.rpm Description: Jokosher is a multi-track non-linear audio editor with a prime focus on usability. It is developed in Python and is Open Source. The main goals of Jokosher functionality are: * The ability to record from any ALSA sound card, including multi-input cards such as the M-Audio Delta 44. * Non-destructive editing. * Undo/Redo (at least to a reasonable limit, but preferably unlimited). * Be able to edit the volume curve in different parts of the track. * Resizable track views to easily zoom in and out of a waveform. * Support for effects plug-ins, most notably LADSPA. * Be able to apply effects to an entire track or a selected portion of a track. When applying effects, there should be the ability to preview the sound with the effect before it is applied. * It should be able to master to OGG, MP3 and WAV. * It should be able to import OGG, MP3 and WAV. * Most important, it should be easy and intuitive to use.
According to Yum, gnonlin is not in Core, Updates, or Extras, so this package - with regards to Extras - would not be self-supporting in that it would need a dependency external to the default repositories. Perhaps you'd also want to submit Gnonlin for inclusion into Extras as well? Also, you should not use the Packager: tag in your spec. See: http://fedoraproject.org/wiki/Packaging/Guidelines#head-c17fb8c1ce9be40da720a2b25d1e2a241062038f Thirdly, your package would probably break on 64-bit systems, as you hardcode /lib/ into the path of %_pythondir. Please use a macro for this instead, such as %{_lib} (or %{_libdir}, etc. as needed). Forth, you really should be using %{python_sitedir} and following the %files section correctly as mentioned in Packaging/Python on the Wiki. See: http://fedoraproject.org/wiki/Packaging/Python Lastly, in the %changelog, you simply have a user@host identifier. I'm not certain, but I believe that this is supposed to be a full and valid email address (such as "me" or something similar).
(In reply to comment #1) > According to Yum, gnonlin is not in Core, Updates, or Extras, so this package - > with regards to Extras - would not be self-supporting in that it would need a > dependency external to the default repositories. Perhaps you'd also want to > submit Gnonlin for inclusion into Extras as well? Gnonlin is indeed in extras. > > Also, you should not use the Packager: tag in your spec. See: > http://fedoraproject.org/wiki/Packaging/Guidelines#head-c17fb8c1ce9be40da720a2b25d1e2a241062038f > > Thirdly, your package would probably break on 64-bit systems, as you hardcode > /lib/ into the path of %_pythondir. Please use a macro for this instead, such as > %{_lib} (or %{_libdir}, etc. as needed). > > Forth, you really should be using %{python_sitedir} and following the %files > section correctly as mentioned in Packaging/Python on the Wiki. See: > http://fedoraproject.org/wiki/Packaging/Python > > Lastly, in the %changelog, you simply have a user@host identifier. I'm not > certain, but I believe that this is supposed to be a full and valid email > address (such as "me" or something similar). Thank you for these. I shall update and resubmit. Regards Chris
(In reply to comment #1) > Also, you should not use the Packager: tag in your spec. See: > http://fedoraproject.org/wiki/Packaging/Guidelines#head-c17fb8c1ce9be40da720a2b25d1e2a241062038f > Removed. > Thirdly, your package would probably break on 64-bit systems, as you hardcode > /lib/ into the path of %_pythondir. Please use a macro for this instead, such as > %{_lib} (or %{_libdir}, etc. as needed). Added, see below. > > Forth, you really should be using %{python_sitedir} and following the %files > section correctly as mentioned in Packaging/Python on the Wiki. See: > http://fedoraproject.org/wiki/Packaging/Python Done. This would appear to be one and the same with your third point. > > Lastly, in the %changelog, you simply have a user@host identifier. I'm not > certain, but I believe that this is supposed to be a full and valid email > address (such as "me" or something similar). Done. Thanks again for the pointers. Could you please review again and let me know if there are any more blockers. Regards Chris
(In reply to comment #2) > Gnonlin is indeed in extras. Sorry about that. A `yum clean all` followed by a `yum info gnonlin` shows that it is indeed in Extras. On a related note, please bump the release whenever you significantly change the spec during the review or in CVS (once it's APPROVED). Is the resubmission located at the same URIs?
(In reply to comment #4) > Is the resubmission located at the same URIs? Yes, it is. Regards Chris
I have now updated the files following the release of jokosher 0.1 These are at: http://www.iammetal.co.uk/jokosher.spec http://www.iammetal.co.uk/jokosher-0.1-2.src.rpm Regards Chris
I have now read through all steps in the packaging guidelines on the Wiki. I would therefore be grateful if someone could sponsor this package as it is my first attempt at Packaging and Extras submission. Regards Chris
Bad : you need to define BuildArch : noarch rpmlint on the rpm gives E: script-without-shellbang /usr/share/doc/jokosher-0.1/userguide/jokosheruserguide.de.html rpmlint on the debuginfo gives E: empty-debuginfo-package I'm currently running mock over the package. You need to fix the above problems before you go any further as they're blockers.
Fails to build in mock. The problem is in the Source line - you have %{name}-%{version}-%{release}.tar.gz %{release} gets expanded to -2.fc6 Typically, the source tarball (not the SRPM you've produced) will be along the lines of %{name}-%{version}.tar.gz (or sometimes with something after %{version} such as -rc2.tar.gz). This is what Source has to point to. If you've had to create a tarball from the SVN repository, the package name needs to reflect this.
You're missing some requires as well - check http://www.jokosher.org/download for details.
Mid-air collision, so here are some additional comments: * I suggest that upstream do not modify/extend/sublicence the GPL with exceptions, but rather put their distribution licence terms into a separate file. * Run rpmlint on the binary rpm. From the two errors it reports, one is valid. * Directory /usr/lib/python2.4/site-packages/jokosher/ is not included. * Why is it arch-specific and not "BuildArch: noarch"? * Upstream recommends GStreamer 0.10.9 or above and prefers a CVS snapshot. Requires: gstreamer >= 0.10.8 does not reflect that. * Prefer "install -p" over "cp" to preserve time-stamps of files. * Run-time warning: Some functionality will not work correctly or at all. You must have the Python alsaaudio package installed. Please install python-alsaaudio or fetch from http://www.wilstrup.net/pyalsaaudio/. * Crashes reproducibly with below message. Steps to reproduce: 1) Start "jokosher" 2) Enter "Preferences". 3) Click "Close". 4) Click "Create a new project". Starting up /usr/lib/python2.4/site-packages/jokosher/Jokosher.py:1054: GtkDeprecationWarning: gtk.threads_init is deprecated, use gtk.gdk.threads_init instead gtk.threads_init() 6783: assertion failed "allocator->lock == mutex" file "dbus-dataslot.c" line 82 function _dbus_data_slot_allocator_alloc Aborted
The more I look into this package, the more odd it becomes. There is nothing (I can see) on the website as to the license Upstream recommending a cvs snapshot (moving target problems) Upstream not sticking totally to the GPL Sure, it looks a nice package, but as it stands, is it stable (upstream) enough to be in Extras? The functionality issues in #11 give me serious reasons to doubt it's inclusion at this time (though this could be down [again] to upstream or non-inclusion of R's on the rpm itself)
Okay, here goes, First of all thanks for reviewing and for assigning yourself to this one Paul. Most comments understood and have been or are being acted upon. (In reply to comment #8) > Bad : you need to define BuildArch : noarch Done > rpmlint on the rpm gives > > E: script-without-shellbang > /usr/share/doc/jokosher-0.1/userguide/jokosheruserguide.de.html Fixed. > > rpmlint on the debuginfo gives > > E: empty-debuginfo-package Do noarch packages generate debuginfo packages? rpmbuild no longer produces these. Regards Chris
noarch doesn't create a debuginfo package. Can you upload a new spec and src.rpm when you've fixed the other issues please?
(In reply to comment #9) > %{name}-%{version}.tar.gz (or sometimes with something after %{version} such as > -rc2.tar.gz). This is what Source has to point to. Fixed. (In reply to comment #10) > You're missing some requires as well - check http://www.jokosher.org/download > for details. Fixed. Alsaaudio is not a strict requirement for the operation of jokosher and the next version will not require it at all. However I will consider packaging it if necessary. (In reply to comment #11) * I suggest that upstream do not modify/extend/sublicence the GPL with exceptions, but rather put their distribution licence terms into a separate file. I have mentioned this on the development list. * Run rpmlint on the binary rpm. From the two errors it reports, one is valid. Now fixed. * Directory /usr/lib/python2.4/site-packages/jokosher/ is not included. Please could you clarify. * Why is it arch-specific and not "BuildArch: noarch"? Now fixed. * Upstream recommends GStreamer 0.10.9 or above and prefers a CVS snapshot. Requires: gstreamer >= 0.10.8 does not reflect that. Now fixed. * Prefer "install -p" over "cp" to preserve time-stamps of files. Now fixed - yes this is much better, thanks. * Run-time warning: Some functionality will not work correctly or at all. You must have the Python alsaaudio package installed. Please install python-alsaaudio or fetch from http://www.wilstrup.net/pyalsaaudio/. See above. * Crashes reproducibly with below message. Steps to reproduce: 1) Start "jokosher" 2) Enter "Preferences". 3) Click "Close". 4) Click "Create a new project". Starting up /usr/lib/python2.4/site-packages/jokosher/Jokosher.py:1054: GtkDeprecationWarning: gtk.threads_init is deprecated, use gtk.gdk.threads_init instead gtk.threads_init() 6783: assertion failed "allocator->lock == mutex" file "dbus-dataslot.c" line 82 function _dbus_data_slot_allocator_alloc Aborted I cannot reproduce this however I am running the latest gstreamer and gnonlin releases although I fail to see how that would alter things. This has not been reported on the support forums or lists to my knowledge. Perhaps the latest spec file and requires fixes this. py-dbus maybe? (In reply to comment #12) > The more I look into this package, the more odd it becomes. > There is nothing (I can see) on the website as to the license > Upstream recommending a cvs snapshot (moving target problems) Gstreamer have now made a release so no longer dependent on cvs. Gnonlin also anticipated doing the same in the near future however again, current gnonlin will work, just not with full features. > Upstream not sticking totally to the GPL This has been mentioned to the devs. I would appreciate comments on how much of a showstopper this is and how to get around it of there is no altering the stance of how it is licensed upstream. > Sure, it looks a nice package, but as it stands, is it stable (upstream) enough > to be in Extras? I would say yes in the spirit of release early and often. This is not an admittance of buggy software - the two main issues users currently experience are due to current version of gstreamer and gnonlin, both of which Requires indicates. > The functionality issues in #11 give me serious reasons to doubt it's inclusion > at this time (though this could be down [again] to upstream or non-inclusion of > R's on the rpm itself) I hope the above gives you cause to re-think. I would be happy to see it wait until fc6 for inclusion. Thanks again for the advice. Regards Chris
If the way the GPL is messed with adds restrictions, it causes major problems. If it removes restrictions, that's fine. My reading is it adds. The problem with release often and early is that you get releases which haven't had the time to mature and be debugged correctly. I've known of many security problems with that system. "* Directory /usr/lib/python2.4/site-packages/jokosher/ is not included. Please could you clarify." The application creates a directory (via the define) in the python/site-packages/ directory called jokosher. You need in your %files %{python_dir}/site-packages/jokosher "> You're missing some requires as well - check http://www.jokosher.org/download > for details. Fixed. Alsaaudio is not a strict requirement for the operation of jokosher and the next version will not require it at all. However I will consider packaging it if necessary." Depending on the timeframe of this packages possible acceptance into FE, I'd look into importing it. It won't harm. I can also replicate the dbus error on my 64 and 32 bit systems. This will need to be looked into.
(In reply to comment #16) > If the way the GPL is messed with adds restrictions, it causes major problems. > If it removes restrictions, that's fine. My reading is it adds. If the exceptions are placed into a separate file, would this resolve the matter? My reading is that it permits people to use non-GPL compatible plugins and therefore is not adding restrictions but IANAL. > > The problem with release often and early is that you get releases which haven't > had the time to mature and be debugged correctly. I've known of many security > problems with that system. The only comment I can make here is that this version has few known bugs and most of those are outside of its control. One reason the package is being pushed for inclusion in Extras is so that upstream can garner more feedback and improve Q.A. which will surely help with debugging. As for security problems I really cannot comment except to say this is an audio editor written in python using Gtk widgets, not say, a driver with kernel hooks. > > "* Directory /usr/lib/python2.4/site-packages/jokosher/ is not included. > > Please could you clarify." > > The application creates a directory (via the define) in the > python/site-packages/ directory called jokosher. You need in your %files > > %{python_dir}/site-packages/jokosher Fixed. > > "> You're missing some requires as well - check http://www.jokosher.org/download > > for details. > > Fixed. Alsaaudio is not a strict requirement for the operation of jokosher and > the next version will not require it at all. However I will consider packaging > it if necessary." > > Depending on the timeframe of this packages possible acceptance into FE, I'd > look into importing it. It won't harm. A small patch (which only just missed the 0.1 release deadline) has now removed this dependency on alsaaudio. http://jokosher.python-hosting.com/changeset/466 I have updated the package and spec file accordingly and bumped the version. > > I can also replicate the dbus error on my 64 and 32 bit systems. This will need > to be looked into. I have posted on this and will get back to you. Would you please in the meantime avail us of your dbus version which you can get with: $ python -c "import dbus; print dbus.version" Regards Chris
Change the license to GPL with exceptions The dbus version comes back with (0, 51, 0) Can you please upload a new spec and src rpm for review?
(In reply to comment #18) Hello Paul, > Change the license to GPL with exceptions Done! > > The dbus version comes back with > > (0, 51, 0) Hmmm, same here. Can you give the output from rpm -qa | grep -i python - we are continuing to investigate. > > Can you please upload a new spec and src rpm for review? Okay, its at: http://www.iammetal.co.uk/jokosher.spec http://www.iammetal.co.uk/jokosher-0.1-4.src.rpm Regards Chris
the grep python gives you a list of everything with the word python in it! I have python 2.4.3-8.FC5 installed. As there seems to be (currently) a need for pydbus, I'd add it as a Requires
Source: %{tmppath}/SOURCE/%{name}-%{version}-%{release}.tar.gz No! Source: %{name}-%{version}.tar.gz or the URL link from the website (eg http://www.foo.org/package/download/com-1.1.1.tar.gz) remember, the source is the source you've downloaded from the website the package came from (unless you've built the archive yourself, which is not that good an idea, especially if you have patches. You should keep the source unpatched and provide patches which are incorporated via the spec file) Please package again. The src rpm will include the patches, the spec file will apply them in %prep - I can't accept it as is. Can you put the defines right at the start? - it makes things much simpler to follow. Given the specs, it's unlikely this will work in FC5 (unless things are updated in FC5). I'd drop the versions from the Requires. mkdir -p $RPM_BUILD_ROOT%{python_sitelib}/jokosher mkdir -p $RPM_BUILD_ROOT%{python_sitelib}/jokosher/images mkdir -p $RPM_BUILD_ROOT%{python_sitelib}/jokosher/Instruments mkdir -p $RPM_BUILD_ROOT%{python_sitelib}/jokosher/Instruments/images You only need the final line and the images line. -p creates the path requested --mode=644, much easier just to have -m 0644 install -d $RPM_BUILD_ROOT%{python_sitelib}/jokosher/images Not sure these are required as you've already created the directories install -p jokosher $RPM_BUILD_ROOT%{_bindir}/jokosher chmod 755 $RPM_BUILD_ROOT%{_bindir}/jokosher can be written install -p -m 0755 jokosher $RPM_BUILD_ROOT%{_bindir}/jokosher Why have you got BR desktop-file-utils, but never use them. What are they for? Still a way to go, but keep on in there.
"Why have you got BR desktop-file-utils, but never use them. What are they for?" D'oh! I was having a bad time when I wrote that. Apologies.
(In reply to comment #20) > the grep python gives you a list of everything with the word python in it! Err, yep. That was the idea. > As there seems to be (currently) a need for pydbus, I'd add it as a Requires Okay, will do.(In reply to comment #22) > "Why have you got BR desktop-file-utils, but never use them. What are they for?" > > D'oh! I was having a bad time when I wrote that. Apologies. Ah well, one less thing for me to adjust. No problem. I will make the necessary changes and re-submit later today. Regards Chris
Any signs of the update?
(In reply to comment #24) > Any signs of the update? I am waiting for confirmation on a patch and will re-submit once I have it. Should be any day now. Regards Chris
10 days further on... I'm not going to be able to review this week (starting a new job), so if you can get the update in tonight (20th Aug), I'll see what's happening, failing that, it'll be after the 27th.
(In reply to comment #26) > 10 days further on... Tell me about it, doesn't time fly. http://www.iammetal.co.uk/jokosher/jokosher.spec http://www.iammetal.co.uk/jokosher/jokosher-0.1-5.src.rpm The only response rpmlint gives is on the license which I understand is okay as it is. I have now patched the released 0.1 source and these are also available at http://www.iammetal.co.uk/jokosher Regards Chris
FYI, I have run it through mock without problems.
rpmlint is not clean on the srpm - all warnings strange-permission jokosher 0755 3 patches not applied (patch0, 1, 2) invalid-licence (not worried about this one as discussed before - also applies to the rpm) mock does, indeed, build cleanly The URL you have above for the spec file is invalid. Looking at the spec file I have some concerns 1. Why have you got patches in the install section - they should be in prep. You can also make them easier on the eye by using %patch0 -p1 and that's it! 2. You need to read http://fedoraproject.org/wiki/Packaging/Python?highlight=%28python%29 as the python packaging guides have changed. It means %ghost has gone (amongst other things) 3. I can't allow install -p -m 0755 %{SOURCE1} %{buildroot}%{_bindir}. All binaries have to be from the package itself and not just dragged from some random website or other - the possibility for interception and security problems is boundless like that. If you need a file (such as patch) you bundle it with the srpm, this is then installed happily into the SOURCES directory when the srpm is installed. 4. The line mkdir -p %{buildroot}%{python_sitelib}/%{name} is not required. It's automatically created by the next mkdir line down.
Anymore progress on this package?
(In reply to comment #30) > Anymore progress on this package? Yes! It is now ready for further review at www.iammetal.co.uk/jokosher Regards Chris
Why have you got a binary as SOURCE1 which is installed into %{_bindir}? You cannot include anything prebuilt - you *must* use the sources and was it not possible to use sed or perl to fix the __init__.py file rather than install a fresh one?
The SOURCE1 isn't a binary. It's a script. I can see why you're doing that but it is kinda ugly. What's upstream's plans for future releases? Will they include that kind of script? Or make it so Jokosher.py actually can be installed to %{bindir}? Or....? __init__.py might be better as: touch %{buildroot}%{python-sitelib}/%{name}/__init__.py but you have a comment in your changelog about security. Can you explain if touch would be a problem? Putting the png images into %{python-sitelib} isn't a great idea. Is it hard to put them into %{_datadir}/%{name} or something similar? If you use your jokosher script you probably don't want execute permissions on Jokosher.py. I'm not sure about the wisdom of making WaveForm.py exectuable. It loks to me as though directly executing WaveForm.py makes a test case run... so this isn't something someone who installs jokosher is going to want to do. (Correct me if I'm wrong on that.)
Paul, not to be a stick-in-the-mud, but have you noticed this review blocks FE-NEEDSPONSOR? Although I suppose Toshio *is* a sponsor...
(In reply to comment #33) > The SOURCE1 isn't a binary. It's a script. I can see why you're doing that but > it is kinda ugly. What's upstream's plans for future releases? Will they > include that kind of script? Or make it so Jokosher.py actually can be > installed to %{bindir}? Or....? Plans are to make Jokosher.py executable, if not for 0.2 then for 0.3. 0.2 is out end of November. > __init__.py might be better as: > touch %{buildroot}%{python-sitelib}/%{name}/__init__.py fixed. > but you have a comment in your changelog about security. Can you explain if > touch would be a problem? No problem at all, thank you for suggestion. > Putting the png images into %{python-sitelib} isn't a great idea. Is it hard to > put them into %{_datadir}/%{name} or something similar? The instrument files also sit in this directoy however this is changing in 0.2. Can this be accepted for 0.1 or am I better off waiting for 0.2? I am beginning to think the latter as there are a number of things that this process has highlighted that are resolved in 0.2 > If you use your jokosher script you probably don't want execute permissions on > Jokosher.py. fixed > I'm not sure about the wisdom of making WaveForm.py exectuable. It loks to me > as though directly executing WaveForm.py makes a test case run... so this isn't > something someone who installs jokosher is going to want to do. (Correct me if > I'm wrong on that.) Waveform.py is a test case although one class does import it, this has since been fixed. I have fixed this however. As it stands I do not have the know-how to patch the code to meet the requirements wrt the images and instrument files. Unless these can be overlooked along with the script executable I will delay this package until an admittedly more mature app arrives with 0.2. Input appreciated. I have not made a new release until above is clarified.
Personally, I'd hang fire to the 0.2 release as it sounds like it will fix a lot of problems you've encountered. It may be possible if upstream is using svn or cvs to use a snapshot for the review which would possibly have the faults highlighted here as being fixed.
mike orphaned gnonlin in Extras so there's another showstopper for ya.
(In reply to comment #37) > mike orphaned gnonlin in Extras so there's another showstopper for ya. I've gone ahead and taken ownership of this, since I'd like to see Jokosher get into FE as soon as possible.
Has there been anymore progress on this package?
Brian Pepple (above) has agreed to take ownership of this and I imagine he is waiting on the 0.2 release...
I think you'll find that was for gnonlin - or at least that's how I read it.
No, I talked to Christopher about taking ownership of Jokosher also, since he didn't have time to work on it. I've done some initial work on this, but it makes sense to wait for 0.2 to come out since most of the outstanding issues will be fixed there.
0.2 is out
Yeah, 0.2 is out, but it depends on current gstreamer CVS, which make it's a non-starter to go into FE right now.
could we have the package install the one plugin it needs (audiopanorama)? It could change the name so it doesn't conflict with a newer gstreamer when introduced and then depend on later gstreamers when released removing the self installed plugin. Jokosher is a really important app and should at least get into devel for testing to speed up developmentment. If need be I can build the CVS version in Rawhide but it would be nice to have FC6 folk get this so they can play around with it.
(In reply to comment #45) > Jokosher is a really important app and should at least get > into devel for testing to speed up developmentment. > > If need be I can build the CVS version in Rawhide but it would be nice to have > FC6 folk get this so they can play around with it. I agree that we should try our best to get this into Extras as soon as possible, but realistically between FESCo and my other projects, I'm not going to have much free time to work on this. So if someone in the cc' list wants to take the ball and run with it, please feel free to.
Just to update everyone. Gstreamer-plugins-good is still at 0.10.4 in Fedora Development. This week should see the release of a version compliant with Jokosher's requirements. LADSPA is a new requirement in Jokosher 0.2 however it is also packaged in FE. I'm not sure of policy for updating gstreamer in FC6 however I think this will be a showstopper until FC7.
Was there an update for gstreamer to fix this?
gstreamer-plugins-good 0.10.5 was released a few days ago but it hasn't made it into Fedora yet...
can the 0.2 jokosher package at least be put up here for people to test ?
(In reply to comment #50) > can the 0.2 jokosher package at least be put up here for people to test ? There is no package for 0.2 to test. 0.9 is just around the corner. Personally, I intend to run Rawhide from F7 Test 1 and I may create a package when this happens in a week or so. You are of course more than welcome to compile your own however whilst upstream continues to use bleeding edge tools for which there are no packages themselves built, dependencies cannot be resolved so there is little point building them. Fedora 7 will, for the first time, contain all the packages required to run Jokosher in its present form.
I have created a snapshot package from subversion for feedback and testing. Things to note: -I have had to change the permissions on a few files whilst upstream gets them properly set in svn. -I have used sed make a one line change to remove a shebang and to strip dos characters from a userguide file There are still a large number of bugs outstanding at: https://launchpad.net/jokosher/+bugs You can download the rpm, source rpm, spec file and source from: www.iammetal.co.uk/jokosher Regards Chris
Doesn't really build on current Development: Traceback (most recent call last): File "setup.py", line 79, in <module> targeturi = os.path.join(installdir, "share/gnome/", filepath) File "/usr/lib64/python2.5/posixpath.py", line 62, in join elif path == '' or path.endswith('/'): AttributeError: 'NoneType' object has no attribute 'endswith' error: Bad exit status from /var/tmp/rpm-tmp.94847 (%build) RPM build errors: Bad exit status from /var/tmp/rpm-tmp.94847 (%build)
(In reply to comment #53) > Doesn't really build on current Development: Okay, thanks for testing David. I'm currently using FC6, plan to move to F7 when T2 arrives so will take a look at this then. Regards Chris
(In reply to comment #53) > Doesn't really build on current Development: Hi David, I am now running F7T2 and have update to Rawhide 3/3/07. I cannot replicate your above bug. Could you re-test with current tree? Can anyone else try building in mock with rawhide as the target? Regards Chris
Still getting that on todays tree.. 64 bit issue?
(In reply to comment #56) > Still getting that on todays tree.. 64 bit issue? Yes it could well be. I have had someone on #fedora run it through mock on 64 bit with target of rawhide and it has built fine for them however. It would be good for someone else to confirm this before I go chasing it down... Regards Chris
Where are the srpms? I downloaded http://www.iammetal.co.uk/jokosher-0.9-1.20070225svn.src.rpm and was able to build it in mock development on x86_64. But the spec file doesn't have all the fixes reported in previous comments so I'm unsure if I'm building the latest version (that was based off an older revision by mistake) or if i'm building something totally foreign.
To be clear, I am attempting to build the latest src.rpm from http://www.iammetal.co.uk/jokosher/ - namely: http://www.iammetal.co.uk/jokosher/jokosher-0.9-1.20070225svn.src.rpm The traceback happens right after changing mode of build/scripts-2.5/jokosher from 644 to 755
(In reply to comment #58) > Where are the srpms? I downloaded > http://www.iammetal.co.uk/jokosher-0.9-1.20070225svn.src.rpm > and was able to build it in mock development on x86_64. But the spec file > doesn't have all the fixes reported in previous comments so I'm unsure if I'm > building the latest version (that was based off an older revision by mistake) or > if i'm building something totally foreign. The fixes in previous comments have either been applied upstream or are no longer relevant. I'm not sure exactly what you're asking. The build now runs from the setup.py file. The srpms are at the URI you indicated. I am building pre-release packages from svn in readiness for 0.9 final which is due in a few weeks. Good to know it is building okay for you in x86_64. I'm still concerned about David's issue however - any thoughts? What needs to happen now to get this accepted? Regards Chris
The 25/02/2007 rpm installs but wouldn't load because I didn't have the pkg_resources module. Could python-setuptools be added as a dependency?
(In reply to comment #61) > The 25/02/2007 rpm installs but wouldn't load because I didn't have the > pkg_resources module. Could python-setuptools be added as a dependency? I added this as a buildrequires but can quite happily add it as a dependency. I will rebuild and pull updates from svn as well, then re-post. Many thanks for your comments. Regards Chris
(In reply to comment #60) > The fixes in previous comments have either been applied upstream or are no > longer relevant. I'm not sure exactly what you're asking. The build now runs > from the setup.py file. The srpms are at the URI you indicated. I am building > pre-release packages from svn in readiness for 0.9 final which is due in a few > weeks. > There were some fixes to the spec file that have been lost as well. > Good to know it is building okay for you in x86_64. I'm still concerned about > David's issue however - any thoughts? What needs to happen now to get this accepted? I've found David's issue and will attach a patch that you can take upstream. As for getting this in, I don't have time for a complete review but I do have a few comments. Maybe after you fix these and apply the patch, David can continue to do the review:: * Cosmetic: The tarball you've created is really a .tar file, not a .tar.gz. Rpm knows how to handle it, however, and also compresses its payload so it's not strictly necessary to fix this. It would be nice to be accurate when a human extracts the source rpm and tries to look at the sources, though. So having jokosher-0.9.tar or actually gzipping the tarball would be appropriate :-) * A recent addition to the Packaging Guidelines is that for packaging snapshots you need to show how to recreate the snapshot either in a script that you include as another Source line or in a comment. ie:: # This tarball is a snapshot. You can recreate it by doing: # svn co -r 321 http://svn.jokosher.org/trunk jokosher-0.9 # tar -czvf jokosher-20070225.snap.tar.gz jokosher-0.9 This allows reviewers to easily check that the sources are coming from upstream. * The BuildArch: noarch is missing from the spec file * You aren't cleaning the buildroot prior to installing (rpmlint warns about this) * You aren't installing the omf file and registering with scrollkeeper within the %post/%postun in the spec file so the help files won't be found. * You aren't calling update-mime-database or update-desktop-database in the spec file's %post/%postun so jokosher's mimetype and "mailcap" entries aren't being created. * You have a raft of unowned directories. As an example, changing your file entries from this: %{_datadir}/%{name}/pixmaps/*.png into this: %{_datadir}/%{name}/ will own the jokosher directory and all of its subdirectories and files. Where you cannot do this because you don't want all of the files inside the directories you can change from this:: %exclude %{python_sitelib}/Jokosher/Profiler.py %{python_sitelib}/Jokosher/*.py %{python_sitelib}/Jokosher/*.pyo %{python_sitelib}/Jokosher/*.pyc into this: %exclude %{python_sitelib}/Jokosher/Profiler.py %dir %{python_sitelib}/Jokosher %{python_sitelib}/Jokosher/*.py %{python_sitelib}/Jokosher/*.pyo %{python_sitelib}/Jokosher/*.pyc * It looks like you've got the jokosher help in three places: /usr/share/gnome/help/jokosher, /usr/share/doc/jokosher-0.9/userguide, and /usr/share/doc/jokosher-0.9/jokosher It probably only neds to be in /usr/share/gnome/help/ * You need to use the %find_lang macro to include the *.mo files, not just include them in the %files section. The way you've currently got it setup, people won't be able to specify which languages they're interested in when they install this. * David's error is coming from the section of setup.py dealing with installing omf files. However, the whole handling of omf files has issues. Attaching a patch.
Created attachment 149290 [details] Rework omf file installation Patch to setup.py that better handles omf file installation. OMF files are always installed now. But we don't register them with scrollkeeper unless the user is root.
(In reply to comment #63) > I've found David's issue and will attach a patch that you can take upstream. As > for getting this in, I don't have time for a complete review but I do have a few > comments. Maybe after you fix these and apply the patch, David can continue to > do the review:: I have submitted your patch upstream Toshio, thank you for that. > * Cosmetic: The tarball you've created is really a .tar file, not a .tar.gz. > Rpm knows how to handle it, however, and also compresses its payload so it's not > strictly necessary to fix this. It would be nice to be accurate when a human > extracts the source rpm and tries to look at the sources, though. So having > jokosher-0.9.tar or actually gzipping the tarball would be appropriate :-) Done. > * A recent addition to the Packaging Guidelines is that for packaging snapshots > you need to show how to recreate the snapshot either in a script that you > include as another Source line or in a comment. ie:: > # This tarball is a snapshot. You can recreate it by doing: > # svn co -r 321 http://svn.jokosher.org/trunk jokosher-0.9 > # tar -czvf jokosher-20070225.snap.tar.gz jokosher-0.9 > > This allows reviewers to easily check that the sources are coming from upstream. Done. > * The BuildArch: noarch is missing from the spec file Fixed. > * You aren't cleaning the buildroot prior to installing (rpmlint warns about this) Fixed. > * You aren't installing the omf file and registering with scrollkeeper within > the %post/%postun in the spec file so the help files won't be found. Fixed. > * You aren't calling update-mime-database or update-desktop-database in the spec > file's %post/%postun so jokosher's mimetype and "mailcap" entries aren't being > created. Fixed. > * You have a raft of unowned directories. As an example, changing your file > entries from this: > %{_datadir}/%{name}/pixmaps/*.png > into this: > %{_datadir}/%{name}/ > > will own the jokosher directory and all of its subdirectories and files. > > Where you cannot do this because you don't want all of the files inside the > directories you can change from this:: > %exclude %{python_sitelib}/Jokosher/Profiler.py > %{python_sitelib}/Jokosher/*.py > %{python_sitelib}/Jokosher/*.pyo > %{python_sitelib}/Jokosher/*.pyc > into this: > %exclude %{python_sitelib}/Jokosher/Profiler.py > %dir %{python_sitelib}/Jokosher > %{python_sitelib}/Jokosher/*.py > %{python_sitelib}/Jokosher/*.pyo > %{python_sitelib}/Jokosher/*.pyc Fixed. > * It looks like you've got the jokosher help in three places: > /usr/share/gnome/help/jokosher, /usr/share/doc/jokosher-0.9/userguide, and > /usr/share/doc/jokosher-0.9/jokosher > > It probably only neds to be in /usr/share/gnome/help/ Fixed. > * You need to use the %find_lang macro to include the *.mo files, not just > include them in the %files section. The way you've currently got it setup, > people won't be able to specify which languages they're interested in when they > install this. Done. > * David's error is coming from the section of setup.py dealing with installing > omf files. However, the whole handling of omf files has issues. Attaching a patch. Thank you for taking the time to respond Toshio. I have run through rpmlint and receive no errors. It also appears to build cleanly under mock development on i386. I will notify as and when your patch is accepted upstream however for the moment the new packages are in the usual location at: http://www.iammetal.co.uk/jokosher Regards Chris
revision 1337 doesn't fix my problem but 1339 does, also to make this build rpms I had to add the omf file to the %files section. Outside of that jokosher now runs on my Development box and there was much rejoicing.
(In reply to comment #66) > revision 1337 doesn't fix my problem but 1339 does, also to make this build rpms > I had to add the omf file to the %files section. Outside of that jokosher now > runs on my Development box and there was much rejoicing. :) Thanks David, good to know. r1339 includes Toshio's fix so I'm pulling that tonight and rebuilding. Regards Chris
Toshio's patch has now been accepted upstream and Davis reports in #66 that it works for him. Everybody seems happy - please advise what else needs to be done. Regards Chris
I can't sponsor you, but here's a brief unofficial review of your spec/SRPM to help expedite getting this into Fedora because it's such an awesome little app. :] Unfortunately, Mock is also quite broken at the moment, so I cannot post comments about its build-time structure or quality. (However, it seems to run okay through a standard rpmbuild invocation; and rpmlint is silent on that built RPM.) == Unofficial Review of jokosher 0.9-1.20070308svn == ----- BAD: The package does not follow the Naming Guidelines: The Release tag should be "0.%{X}.%{alphatag}" or equivalent as noted in the "Pre-Release packages" section of the naming guidelines (Packaging/NamingGuidelines on the wiki). BAD: You have duplicate BuildRequires listed: python-devel and pycairo-devel are both dependencies of pygtk2-devel. Thus, simply listing pygtk2-devel as a BuildRequires should pull in the other two automagically; and you should remove those two from the BuildRequires list. BAD: %{_datadir}/omf/%{name}/, %{_datadir}/gnome/help/%{name}/, and %{_datadir}/gnome/help/%{name}/C/ should all be owned by the package. Otherwise they are orphaned when the package is removed. We don't like clutter in our filesystems. :] BAD: The package should invoke desktop-file-install to install the .desktop file in your %install section. See the "desktop-file-install usage" section on the wiki's Packaging/Guidelines page for more details. BAD: Your package does not have an appropriate %clean section. It should contain simply "rm -rf $RPM_BUILD_ROOT". See ----- GOOD :The spec file is named "%{name}.spec" in accordance with Naming Guidelines. GOOD: BuildRoot is "%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)", following the packaging guidelines. GOOD: Included documentation (%doc) is OK; and it seems to run well without these files installed. GOOD: Package includes an appropriate .desktop file since it is a graphical application. GOOD: Macros are used instead of hardcoded file names, and usage of these macros (including $RPM_BUILD_ROOT) is consistent throughout the spec file. GOOD: Locale files are handled correctly, using %find_lang appropriately. GOOD: Package is not relocatable. GOOD: Package includes appropriate code and content. GOOD: Package does not own any system files/directories or any files/directories that conflict with another package. GOOD: Package license (GPL) is OK; and a copy of it is included in the package as documentation (%doc COPYING). The License field in the spec file properly reflects this. GOOD: Spec file is legible and in American English. GOOD: No duplicates are listed in the %files section; and its %defattr line is good. GOOD: When installed, the application runs well with no apparent segfaults or major bugs. GOOD: Appropriate scriplets for MIME-type, Scrollkeeper, and .desktop files are used. GOOD: The %files section has a proper %defattr(-,root,root,-) line. ----- N/A: No non-ASCII characters are needed, so encoding is OK. N/A: This is a noarch package, so compiler flags are not needed. N/A: No static libraries or RPATH exclusions are needed. N/A: Package includes no configuration files or data, so %config markings are not needed. N/A: Package uses Python setuptools for building; so using `make %{?_smp_flags}` is not needed. N/A: Package is not a web application. N/A: Package is noarch; thus no ExcludeArch/ExclusiveArch tweaking is required. N/A: Package installs no shared libraries; thus %post/%postun scriplets of /sbin/ldconfig are not needed. N/A: No large (neither in size nor in quantity) documentation is included, thus no -doc subpackage is needed. N/A: No headers, no pkgconfig files, and no static or unsuffixed shared libraries are included. Thus, no -devel subpackage is needed. N/A: Package contains no libtool archives (*.la files) N/A: Package contains no %description or Summary translations. ----- MINOR(?): The source tarball does not match that of upstream (in this case, the upstream tarball is generated from the svn export as you explain in your spec comments): $ md5sum SOURCES/jokosher-0.9* 24ae1fa6adad7893a58fcd32aaec1ff3 jokosher-0.9-srpm.tar.gz 26837769e637a9225fb26afe243488db jokosher-0.9-upstream.tar.gz However, if I grab another SVN snapshot and tar it up, the md5sum changes yet again, so this seems to be a simple false positive; and is probably safe to ignore. MINOR: It's just preference, but instead of listing the directory via %dir and globbing all of the files inside of it, it's probably simpler to simply list the directory itself as a parent. RPM will automagically make it grab all the files and directories in that recursively. E.g., instead of the current: %dir %{python_sitelib}/Jokosher %{python_sitelib}/Jokosher/*.py %{python_sitelib}/Jokosher/*.pyo %{python_sitelib}/Jokosher/*.pyc You could use something like: %{python_sitelib}/Jokosher/ Your %exclude line should still hold, so that's ok.
Hello Peter, (In reply to comment #69) > I can't sponsor you, but here's a brief unofficial review of your spec/SRPM to > help expedite getting this into Fedora because it's such an awesome little app. :] I'm glad you agree - its been a long time coming :P > Unfortunately, Mock is also quite broken at the moment, so I cannot post > comments about its build-time structure or quality. (However, it seems to run > okay through a standard rpmbuild invocation; and rpmlint is silent on that built > RPM.) Mock is working for me and builds fine with the following changes. rpmlint is quiet. > BAD: The package does not follow the Naming Guidelines: The Release tag should > be "0.%{X}.%{alphatag}" or equivalent as noted in the "Pre-Release packages" > section of the naming guidelines (Packaging/NamingGuidelines on the wiki). This should now be fixed as I have started bumping the %{X} section with each new build. > BAD: You have duplicate BuildRequires listed: python-devel and pycairo-devel are > both dependencies of pygtk2-devel. Thus, simply listing pygtk2-devel as a > BuildRequires should pull in the other two automagically; and you should remove > those two from the BuildRequires list. Fixed. > BAD: %{_datadir}/omf/%{name}/, %{_datadir}/gnome/help/%{name}/, and > %{_datadir}/gnome/help/%{name}/C/ should all be owned by the package. Otherwise > they are orphaned when the package is removed. We don't like clutter in our > filesystems. :] Fixed. > BAD: The package should invoke desktop-file-install to install the .desktop file > in your %install section. See the "desktop-file-install usage" section on the > wiki's Packaging/Guidelines page for more details. Fixed. > BAD: Your package does not have an appropriate %clean section. It should contain > simply "rm -rf $RPM_BUILD_ROOT". See Fixed. > MINOR(?): The source tarball does not match that of upstream (in this case, the > upstream tarball is generated from the svn export as you explain in your spec > comments): > $ md5sum SOURCES/jokosher-0.9* > 24ae1fa6adad7893a58fcd32aaec1ff3 jokosher-0.9-srpm.tar.gz > 26837769e637a9225fb26afe243488db jokosher-0.9-upstream.tar.gz > However, if I grab another SVN snapshot and tar it up, the md5sum changes yet > again, so this seems to be a simple false positive; and is probably safe to ignore. Okay, let me know if this becomes a problem. Note I am doing an svn export as opposed to a svn checkout to lose all the usual svn cruft. > MINOR: It's just preference, but instead of listing the directory via %dir and > globbing all of the files inside of it, it's probably simpler to simply list the > directory itself as a parent. RPM will automagically make it grab all the files > and directories in that recursively. E.g., instead of the current: > %dir %{python_sitelib}/Jokosher > %{python_sitelib}/Jokosher/*.py > %{python_sitelib}/Jokosher/*.pyo > %{python_sitelib}/Jokosher/*.pyc > You could use something like: > %{python_sitelib}/Jokosher/ > Your %exclude line should still hold, so that's ok. Fair point. Hopefully it now looks cleaner. http://www.iammetal.co.uk/jokosher/ Cheers Chris
Thanks, Christopher. Toshio Kuratomi emailed me about this review saying that he could sponsor you if I did the formal review. So, I'll re-assign this to myself since Paul seems to have mysteriously vanished. :| I'll formally review this when I get home from work tomorrow. Most of the work's already been done, so it'll be a lot simpler (just testing of build-time functionality/quality as well as ensuring the fixes of the previous blockers which you appear to have fixed up nicely). Thanks.
Mock is currently grabbing some build roots, so since it's going to take severals hours for it all I'm going to let it run overnight, so I'll post the review tomorrow morning. Thanks.
(In reply to comment #70) > > BAD: The package does not follow the Naming Guidelines: The Release tag should > > be "0.%{X}.%{alphatag}" or equivalent as noted in the "Pre-Release packages" > > section of the naming guidelines (Packaging/NamingGuidelines on the wiki). > > This should now be fixed as I have started bumping the %{X} section with each > new build. > That's still not correct. For pre-release snapshots, the final version from upstream will be Release 1, so everything before it must be Release 0.X, and with a suffix of the snapshot date tag too. Everything else looks fixed, with the exception of your dependencies: pycairo is a dependency of pygtk2, so you should remove the former. GOOD: Builds fine in mock (x86_64 and i386, FC-6 and Devel); and rpmlint is silent on the source and built (noarch) packages. > Okay, let me know if this becomes a problem. Note I am doing an svn export as > opposed to a svn checkout to lose all the usual svn cruft. > I just did the svn export again and it came up with a yet different MD5 hash, so that seems to be entirely a false positive. It's a minor issue, too, but for some parts in your %files section, you use the %name macro, yet in others you hardcode "jokosher." Is there a specific reason for this inconsistency? You just need to address these three issues, then it'll be approved. Thanks.
(In reply to comment #73) > (In reply to comment #70) > > > BAD: The package does not follow the Naming Guidelines: The Release tag should > > > be "0.%{X}.%{alphatag}" or equivalent as noted in the "Pre-Release packages" > > > section of the naming guidelines (Packaging/NamingGuidelines on the wiki). > > > > This should now be fixed as I have started bumping the %{X} section with each > > new build. > > > > That's still not correct. For pre-release snapshots, the final version from > upstream will be Release 1, so everything before it must be Release 0.X, and > with a suffix of the snapshot date tag too. I must confess I find the Naming Guidelines a little confusing but hopefully the current iteration meets them. I have tried to match it with the kismet example: kismet-0-0.1.20040110svn (this is a pre-release, svn checkout of kismet) > Everything else looks fixed, with the exception of your dependencies: pycairo is > a dependency of pygtk2, so you should remove the former. Done. > It's a minor issue, too, but for some parts in your %files section, you use the > %name macro, yet in others you hardcode "jokosher." Is there a specific reason > for this inconsistency? Fixed. > You just need to address these three issues, then it'll be approved. Thanks. Looking forward to it. www.iammetal.co.uk/jokosher
The %files and dependencies look good now; thanks for fixing them. The last little bit is that your naming is still off: Your Version tag should still be 0.9, since that is what you are distribution snapshots of. Thus, your full EVR should look something like the following. Also (since this isn't a data-only package) you should probably be using the %{?dist} tag to facilitate release upgrades: Version: 0.9 Release: 0.n.YYYYMMDDsvn%{?dist} (Increment n for each successive snapshot, then use Release: 1 for the final version.) We're almost there. :]
(In reply to comment #75) "... since that is what you are distribution snapshots of." Apologies; that should be "...what you are distributing snapshots of." I blame muscle memory. :o
(In reply to comment #75) > The %files and dependencies look good now; thanks for fixing them. > > The last little bit is that your naming is still off: Your Version tag should > still be 0.9, since that is what you are distribution snapshots of. Thus, your > full EVR should look something like the following. Also (since this isn't a > data-only package) you should probably be using the %{?dist} tag to facilitate > release upgrades: > > Version: 0.9 > Release: 0.n.YYYYMMDDsvn%{?dist} > > (Increment n for each successive snapshot, then use Release: 1 for the final > version.) > > We're almost there. :] > Okay, how is it now?
(In reply to comment #77) > Okay, how is it now? Much better. The only other issue is that you're hardcoding calls to python directly, instead of using the %{__python} macro; but this is not a blocker AFAIK, and is something you can fix if needed after you import it. Everything's fixed up; and this package (jokosher-0.9-0.1.20070325svn) is therefore APPROVED. Go aheard with the account creation requests, and Toshio Kuratomi will take you through the sponsorship process from here and onward; after which you should flag this as fedora-cvs? and request your branches, then import your package, et al.. (Thanks, Toshio!)
Great job, guys! Christopher, if you're in the Fedora Account System already, I need to know what you're user name is. If not, the next step is for you to sign up with the Fedora Account System. Instructions are here: http://tinyurl.com/37vcrk If you need help you can post to this bug or find me on IRC #fedora-devel -- abadger1999.
New Package CVS Request ======================= Package Name: jokosher Short Description: Multi-track non-linear audio editor Owners: snecklifter Branches: InitialCC: