Bug 166915

Summary: Review Request: qscintilla - A Scintilla port to Qt
Product: [Fedora] Fedora Reporter: Konstantin Ryabitsev <icon>
Component: Package ReviewAssignee: Aurelien Bompard <gauret>
Status: CLOSED NEXTRELEASE QA Contact: David Lawrence <dkl>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: fedora-extras-list, rdieter
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
URL: http://www.riverbankcomputing.co.uk/qscintilla/
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2005-09-17 15:49:05 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 163779    
Attachments:
Description Flags
specfile patch for new version + "rm -rf" problem none

Description Konstantin Ryabitsev 2005-08-27 15:59:57 UTC
Spec Name or Url: http://linux.duke.edu/~icon/misc/fe/qscintilla.spec
SRPM Name or Url: http://linux.duke.edu/~icon/misc/fe/qscintilla-1.5.1-1.src.rpm
Description:
QScintilla is a port or Scintilla to the Qt GUI toolkit from Trolltech 
and runs on any operating system supported by Qt (eg. Windows, UNIX/Linux, 
MacOS/X).

Comment 1 Aurelien Bompard 2005-09-11 14:07:31 UTC
* This is not the last version, as a result
http://www.river-bank.demon.co.uk/download/QScintilla/qscintilla-1.62-gpl-1.5.1.tar.gz
is not available any more. Please update
* rm -rf $RPM_BUILD_ROOT should be in %install, not in %build

Comment 2 Konstantin Ryabitsev 2005-09-12 03:16:11 UTC
Hmm... Moving the rm -rf is tricky, since apparently part of the make actually
installs stuff. I'll have to see how to fix this more intelligently.

Comment 3 Aurelien Bompard 2005-09-12 10:07:11 UTC
After looking at the Makefiles, here's what I propose:

--- qscintilla.spec.orig        2005-09-12 11:36:31.000000000 +0200
+++ qscintilla.spec     2005-09-12 12:05:13.000000000 +0200
@@ -52,19 +52,27 @@


 %build
-rm -rf $RPM_BUILD_ROOT
 pushd qt
 qmake qscintilla
-make %{?_smp_mflags}
+echo 'build: $(UICDECLS) $(OBJECTS) $(OBJMOC) $(SUBLIBS) $(OBJCOMP)' >> Makefile
+make %{?_smp_mflags} build
 popd

 pushd designer
 qmake designer
-make %{?_smp_mflags}
+echo 'build: $(UICDECLS) $(OBJECTS) $(OBJMOC) $(SUBLIBS) $(OBJCOMP)' >> Makefile
+make %{?_smp_mflags} build
 popd


 %install
+rm -rf $RPM_BUILD_ROOT
+pushd qt
+make
+popd
+pushd designer
+make
+popd
 mkdir -pm 755 \
     $RPM_BUILD_ROOT%{qtdir}/include \
        $RPM_BUILD_ROOT%{qtdir}/translations


Comment 4 Aurelien Bompard 2005-09-12 10:46:45 UTC
Created attachment 118705 [details]
specfile patch for new version + "rm -rf" problem

This patch will not work with the newer version. Version 1.65-gpl-1.6 supports
INSTALL_ROOT, but still installs files in root during "make". Here's the patch,
adapt it as you wish, I'm not very familiar  with qmake.

Comment 5 Aurelien Bompard 2005-09-12 12:07:49 UTC
And by the way, if you could include the doc/ and example/ subdir in the -devel
package (or make a -docs subpackage), it would be nice.

Comment 6 Konstantin Ryabitsev 2005-09-12 15:07:27 UTC
Aurelien: Thanks for your help! Updated spec file and srpm available from:
http://linux.duke.edu/~icon/misc/fe/qscintilla.spec
http://linux.duke.edu/~icon/misc/fe/qscintilla-1.6-1.fc4.src.rpm

Comment 7 Aurelien Bompard 2005-09-12 17:22:03 UTC
One last thing: the html documentation files in
/usr/share/doc/qscintilla-devel-1.6/Scintilla/* are executable

Comment 9 Aurelien Bompard 2005-09-14 08:52:20 UTC
Review for release 2.fc4:
* RPM name is OK
* Source qscintilla-1.65-gpl-1.6.tar.gz is the same as upstream
* Builds fine in mock
* rpmlint of qscintilla looks OK
* rpmlint of qscintilla-designer looks OK
* rpmlint of qscintilla-devel looks OK
* File list of qscintilla looks OK
* File list of qscintilla-designer looks OK
* File list of qscintilla-devel looks OK


Comment 10 Rex Dieter 2005-09-14 11:28:00 UTC
Small nitpick: it would be better/cleaner if the doc file permission fix were
done in the %prep section, instead of %install.

Comment 11 Konstantin Ryabitsev 2005-09-15 04:26:22 UTC
Yeah, that makes sense. One more small edit to move the permission fixups into
%prep.
http://linux.duke.edu/~icon/misc/fe/qscintilla.spec
http://linux.duke.edu/~icon/misc/fe/qscintilla-1.6-3.fc4.src.rpm

Comment 12 Aurelien Bompard 2005-09-15 06:16:49 UTC
Still looks good (even prettier :) ). APPROVED