Spec URL: http://verdurin.fedorapeople.org/reviews/non-sequencer/non-sequencer.spec SRPM URL: http://verdurin.fedorapeople.org/reviews/non-sequencer/non-sequencer-0.0-1.20100131gitba94d2c354145.fc12.src.rpm Description: The Non Sequencer is a powerful real-time, pattern-based MIDI sequencer for Linux-released under the GPL. Filling the void left by countless DAWs, piano-roll editors, and other purely performance based solutions, it is a composition tool-one that transforms MIDI music-making on Linux from a complex nightmare into a pleasurable, efficient, and streamlined process.
Just a couple of quick comments: - according to the source file headers, the license seems to be GPLv2+ - add the following line to the %prep section in order to disable early stripping of debuginfo: sed -i '/^ifneq (\$(USE_DEBUG),yes)/,+4 d' Makefile (alternatively, you can patch the Makefile) - the configure command in the %build section should look like this: %configure --enable-lash - I think, the documentation is extensive enough to put it in a separate doc subpackage - add TODO.mu to %doc $ rpmlint /var/lib/mock/fedora-12-i386/result/*.rpm non-sequencer.src:38: W: configure-without-libdir-spec non-sequencer.src: W: invalid-url Source0: non-sequencer-20100131gitba94d2c354145.tar.bz2 non-sequencer-debuginfo.i686: E: empty-debuginfo-package 3 packages and 0 specfiles checked; 1 errors, 2 warnings.
Thanks for taking a look. I've uploaded a new version that addresses your comments: http://verdurin.fedorapeople.org/reviews/non-sequencer/non-sequencer.spec http://verdurin.fedorapeople.org/reviews/non-sequencer/non-sequencer-1.9.3-2.20100131gitba94d2c354145.fc12.src.rpm $ rpmlint /var/lib/mock/fedora-12-x86_64/result/*.rpm non-sequencer.src: W: invalid-url Source0: non-sequencer-20100131gitba94d2c354145.tar.bz2 4 packages and 0 specfiles checked; 0 errors, 1 warnings. For me it comes up full-screen at startup - not sure whether that's a problem.
Now the doc directory is added twice. You can remove it from the base package. :) I'll have a look at the rest later today.
Oops - fixed in release -3. http://verdurin.fedorapeople.org/reviews/non-sequencer/non-sequencer.spec http://verdurin.fedorapeople.org/reviews/non-sequencer/non-sequencer-1.9.3-3.20100131gitba94d2c354145.fc12.src.rpm
I had a deeper look into the package and have to cancel my previous remark about the doc package. Sorry! The documentation files are expected to be installed with the base package. Otherwise the program's help doesn't work. Nonetheless, installing the doc files is a bit tricky. a) the make statement in %build must look like this: make VERBOSE=1 SYSTEM_PATH=%{_datadir} DOCUMENT_PATH=%{_defaultdocdir}/%{name}-%{version}/doc/ %{?_smp_mflags} It ensures compiling the final doc path into the program. b) the %install section should contain the following lines: sed -i "/\.html/d" Makefile make install DESTDIR=$RPM_BUILD_ROOT c) and finally the %doc line in the %files section: %doc COPYING doc/ Now the help should be available through the help menu of the application. Like you, I wasn't able to start non-sequencer in its own window. It always comes up full-screen. Maybe you should ask upstream whether that's a known issue.
Updated version with those latest changes at: http://verdurin.fedorapeople.org/reviews/non-sequencer/non-sequencer.spec http://verdurin.fedorapeople.org/reviews/non-sequencer/non-sequencer-1.9.3-3.20100131gitba94d2c354145.fc12.src.rpm
Hi, I'll review this package but there are two important issues that need to be fixed first: 1- The build needs to be verbose. We want to see what flags are passed to the compiler. Currently the compiler invocations are silent because of the lines that start with @ in the Makefile. We need to patch or sed these out. I am also not sure whether removing the whole "ifeq ($(USE_DEBUG),yes)" part from the Makefile is right. This also removes -DNDEBUG, and other flags which might be needed by the source, which I didn't check. Simply appending the optflags at the end should be sufficient. Something like sed -i 's|DNDEBUG|DNDEBUG $(CXXFLAGS)|' Makefile Let me know if I am missing something. 2- Since this is a desktop application, we need a .desktop file, an icon and the scriptlets, as usual.
ping? are you still interested in packaging this?
I am interested in packaging it. However, the upstream activity seems rather erratic - no commits since January and there are bugs that have remained unfixed for quite a while. If you think it's worthwhile proceeding with all that in mind, I'll do so.
Put a new version addressing the compilation problems at: http://verdurin.fedorapeople.org/reviews/non-sequencer/non-sequencer.spec http://verdurin.fedorapeople.org/reviews/non-sequencer/non-sequencer-1.9.3-4.20100131gitba94d2c354145.fc13.src.rpm I tried your suggestion regarding CXXFLAGS and that didn't seem to work as then the various include paths were not picked up. The icon is a resized version of the logo file included in the documentation, for want of something better.
Thanks for the update. I will hopefully find time this week to have a look. (In reply to comment #10) > I tried your suggestion regarding CXXFLAGS and that didn't seem to work as then > the various include paths were not picked up. > My suggestion was a guide for you to start-up. It wasn't meant to be _the_ solution. I assume that you fixed the issue with the patch. > The icon is a resized version of the logo file included in the documentation, > for want of something better. BuildRequires: ImageMagick and do the conversion in %build? Or at least some reference would be good that says where you got the image from (so that when someone looks into the specfile, he knows there are no license issues with the image).
Did everything happen here? It looks like oget was going to review, but that was a couple of months ago.
Thanks for the reminder Jason. This package slipped out of my mind. I will look into this by the end of this week. Adam, sorry for the delay.
Sorry for being late. Here is a more thorough review: - rpmlint says non-sequencer.src:61: W: configure-without-libdir-spec non-sequencer.src: W: invalid-url Source0: non-sequencer-20100131gitba94d2c354145.tar.bz2 non-sequencer.x86_64: W: no-manual-page-for-binary non-sequencer These can be ignored. However non-sequencer.x86_64: W: no-documentation needs a fix. You need to put at least the COPYING file in this package, also possibly the TODO file ! Currently, the -doc subpackage does not require the main package. Is this intentional? If this is what you want, the COPYING file must exist both in the main package and the -doc subpackage. * Currently the source tarball contains the .git* stuff that we don't need. Those can be taken out. The tarball instructions tarball tell us to check out the latest git version. What we need is the instructions to generate the exact tarball. In case upstream updates their repo, current instructions will be incorrect. * The %doc package should be noarch * Please explain in the SPEC file where you got the other sources. Do they have copyrights? Also please explain in the SPEC file what the patch is for, and/or its upstream status. * The directory %{_datadir}/%{name}/ must be owned by the main package. See http://fedoraproject.org/wiki/Packaging/Guidelines#File_and_Directory_Ownership * You certainly don't need the BuildRequires: cmake git. I am not sure whether you really need libpng-devel libjpeg-devel. Somehow -lpng and -ljpeg flags are passed to the linking, but what adds them? * Each package must consistently use macros. Please don't use plain /usr. Use %{_prefix} instead. ! Please span %description to 80 columns as much as possible. Currently it is set to 70. * The icon is installed in the wrong place. It should be %{_datadir}/icons/hicolor/128x128/apps/ Note that you leave the scriptlets as is. ? sed -i "/\.html/d" Makefile What is the above for? Can't that be moved to %prep?
Sorry for the delay - will take a look in a couple of days. This was one of my first packages so it's no wonder it needs a lot of work...
Hi Adam, what is going on with this package?
I admit I've neglected this one, as upstream hasn't been active for quite a while. However, I still think it's a useful package, so I'm hoping to have time to work on it at the weekend.
Looks like upstream has pushed out a new release.
Hi Adam, can I close this so someone else can resubmit? This is being tracked by the Fedora Audio spin now. regards, Brendan
Apologies for the delay - I'm in the middle of moving across the country and starting a new job. That said, I do intend to complete the review, so I'm happy to continue unless someone else is desperate to do it. I should have time to update to the new release at the weekend.
There's a new version that includes the recent changes but still needs some work at: http://verdurin.fedorapeople.org/reviews/non-sequencer/non-sequencer.spec http://verdurin.fedorapeople.org/reviews/non-sequencer/non-sequencer-1.9.4-1.20120415git3a7d924.fc16.src.rpm I'll contact upstream about the old FSF address errors.
May we assume this as still alive...? Otherwise, the request should be closed as FE-DEADREVIEW or replaced by a new request from someone else, marking the old one as a duplicate.
No, I haven't given up, just haven't had much time for new packaging work lately. I'll try to make some time over the next week, especially given the project seems to have been a lot more active lately.
Adam, I'll build all of them through the non-daw pacakge. It actually makes sense to do it this way - I would like to dynamically link ntk (upsdtream has forked fltk!) rather than have it statically linked. I believe this is not quite finished upstream but should be soon.
Last comments were years ago. Please provide an update. If there is no response within a week as per policy this will be closed. https://fedoraproject.org/wiki/Policy_for_stalled_package_reviews
This was actually packaged by Brendan as part of non-daw, so this review request can be closed.