Bug 193787 - Review Request: scite - Scintilla based text editor
Review Request: scite - Scintilla based text editor
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Jason Tibbitts
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-06-01 12:17 EDT by cq92j9y+rlkr0w
Modified: 2007-11-30 17:11 EST (History)
1 user (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-06-06 01:47:22 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)

  None (edit)
Description cq92j9y+rlkr0w 2006-06-01 12:17:28 EDT
Spec URL: http://calle255.org/scite/scite.spec
SRPM URL: http://calle255.org/scite/scite-1.69-1.src.rpm
Description: SciTE is a SCIntilla based Text Editor. Originally built to demonstrate Scintilla, it has grown to be a generally useful editor with facilities for building and running programs.

This is my first package so I'd appreciate a sponsor :)
Comment 1 Parag AN(पराग) 2006-06-02 00:28:05 EDT
Not a Review but some comments:-
rpmlint on source package gives
W: scite invalid-license Historical Permission Notice and Disclaimer
Comment 2 Jason Tibbitts 2006-06-02 10:38:12 EDT
The license seems to me to be equivalent to the MIT license
(http://www.opensource.org/licenses/mit-license.php); that's what I'd use in the
License: field.

Some other comments:

You don't seem to use %{optflags}; the package is compiled with -Os instead of
the usual Fedora set (which includes FORTIFY_SOURCE and -g so that proper
debuginfo packages can be generated).

It seems that scintilla is built separately and then statically linked in.  Is
it reasonable at all to build in a separate package and then dynamically link it
in?  (Keep in mind that I know nothing about scintilla.  It does seem that most
of the packages that use scintilla seem to just include a copy of the source,
which may be the best way to handle it.)

Finally, I just wanted to make sure you understand that sponsorship is generally
granted only after you've demonstrated familiarity with the packaging
guidelines; generally you do this by commenting on other packages up for review.
 I personally am reluctant to sponsor someone after looking at just a single
submitted package. 
Comment 3 cq92j9y+rlkr0w 2006-06-02 22:10:11 EDT
Hi Jason,

I've updated the spec file and SRPM:

Spec URL: http://calle255.org/scite/scite.spec
SRPM URL: http://calle255.org/scite/scite-1.69-2.src.rpm

(In reply to comment #2)
> The license seems to me to be equivalent to the MIT license
> (http://www.opensource.org/licenses/mit-license.php); that's what I'd use in
the License: field.

I've changed the license to MIT. Like you said both licenses are functionally
equivalent (it is also what Wikipedia says). Now rpmlint doesn't give any warnings.

> You don't seem to use %{optflags};
Fixed.

> 
> It seems that scintilla is built separately and then statically linked in.  Is
> it reasonable at all to build in a separate package and then dynamically link
it in?

It can surely be done, but it would require a lot of work and I don't think it's
worth it. Upstream isn't very interested on making scintilla a shared-library
and their recommendation has always been to static-link software using it.
Also, scite and scintilla are released together so I'm pretty sure using
different versions of scite/scintilla together will break things. The only
distro I know of that made scite/scintilla independent packages (making scite
dynamically linked) is PLD and it seems they are not doing it anymore.
Nevertheless, if you feel this is a major issue I'll try to fix it ;)

> Finally, I just wanted to make sure you understand that sponsorship is generally
> granted only after you've demonstrated familiarity with the packaging
> guidelines; generally you do this by commenting on other packages up for review.
>  I personally am reluctant to sponsor someone after looking at just a single
> submitted package. 

I see your point, but I'm very sorry to hear this. Though I haven't made any
comments on other RRs I've been following the discussions for a while
(fedora-extras-list included).

This isn't my first RPM but it's my first package for Extras so I'm doing my
best to get it right. If you think I'm not following any of the
PackagingGuidelines please let me know.

Jorge.
Comment 4 Jason Tibbitts 2006-06-02 23:10:06 EDT
(In reply to comment #3)

(dynamically linking scintilla) 
> It can surely be done, but it would require a lot of work and I don't think it's
> worth it.

I can understand that; I was merely curious as to its feasibility since it seems
cleaner.  But if it's not intended to work that way then there's not much point.

> I see your point, but I'm very sorry to hear this. Though I haven't made any
> comments on other RRs I've been following the discussions for a while
> (fedora-extras-list included).

Well, you can always go ahead and make a review comments.  (You can't assign
bugs to yourself or change their status but you can make comments.)

We're really just trying to establish two things: that you understand the
packaging guidelines and you're willing to commit to maintaining your packages
in the long term.  We can guage the former by looking at review comments and the
quality of packages you've submitted, but it's hard to guage the latter so we
have to look at depth of community involvement.  Your quick response to comments
is encouraging, and if you have anything you'd like to point me at that would
help to reassure me, I'd be happy to take a look.

The package itself is fine, although I'd still have to do a full review.
Comment 5 cq92j9y+rlkr0w 2006-06-02 23:43:27 EDT
For what it's worth: I'm currently a Fedora Ambassador for Colombia.
I know Extras and Ambassadors are separate projects, but I became a Fedora
Ambassador precisely because I want to be involved with Fedora and help in
anyway possible.

Anyway, I'll try yo get more involved in RRs discussions :)
Comment 6 Jason Tibbitts 2006-06-03 20:30:36 EDT
I was not aware of your community involvement.  I think folks who need sponsors
should be encouraged to tell us these kinds of things.  I will be happy to
sponsor you; go ahead and request cvsextras membership.

Note: desktop-file-install logs the following warning:
/var/tmp/scite-1.69-2.fc6-root-mockbuild/usr/share/applications/fedora-SciTE.desktop:
warning: non-standard key "MultipleArgs" lacks the "X-" prefix

I believe this should be removed.  It seems to be something that was in use some
time ago but no longer.

Review:
* package meets naming and packaging guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* dist tag is present.
* build root is correct.
* license field matches the actual license.
* license is open source-compatible.  License text included in package.
* source files match upstream:
   3f84986922ccf9c21a1481ba85153be6  scite169.tgz
   3f84986922ccf9c21a1481ba85153be6  scite169.tgz-srpm
* latest version is being packaged.
* BuildRequires are proper.
* package builds in mock (development, x86_64).
* rpmlint is silent.
* final provides and requires are sane:
   scite = 1.69-2.fc6
  -
   libatk-1.0.so.0()(64bit)
   libcairo.so.2()(64bit)
   libgdk-x11-2.0.so.0()(64bit)
   libgdk_pixbuf-2.0.so.0()(64bit)
   libglib-2.0.so.0()(64bit)
   libgmodule-2.0.so.0()(64bit)
   libgobject-2.0.so.0()(64bit)
   libgthread-2.0.so.0()(64bit)
   libgtk-x11-2.0.so.0()(64bit)
   libpango-1.0.so.0()(64bit)
   libpangocairo-1.0.so.0()(64bit)
* no shared libraries are present.
* package is not relocatable.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* %clean is present.
* %check is not present; no test suite upstream.
* no scriptlets present.
* code, not content.
* documentation is small, so no -docs subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
* no headers.
* no pkgconfig files.
* no libtool .la droppings.
X GUI app, with desktop file.  But desktop-file-install complains as above.
Comment 7 cq92j9y+rlkr0w 2006-06-04 12:19:18 EDT
Hi,
I removed the 'MultipleArgs' key and now desktop-file-install doesn't complain.

Updated spec & SRPM:

Spec URL: http://calle255.org/scite/scite.spec
SRPM URL: http://calle255.org/scite/scite-1.69-3.src.rpm
Comment 8 Iago Rubio 2006-06-05 02:58:49 EDT
(In reply to comment #3)
> (In reply to comment #2)
> The license seems to me to be equivalent to the MIT license
> (http://www.opensource.org/licenses/mit-license.php); that's what I'd use in
> the License: field.

That's wrong. I've got some involvement with the scintilla project and I'm
almost sure both scintilla and scite have ever used the Python license.

http://sourceforge.net/projects/scintilla/
License: Python License (CNRI Python License).

http://www.opensource.org/licenses/pythonpl.php

I can request an authoritative answer upstream if you wish, as I've been
participating on scintilla's mailing list for some years now.
Comment 9 cq92j9y+rlkr0w 2006-06-05 08:27:02 EDT
SciTE's License.txt file contains a copy of the "Historical Permission Notice
and Disclaimer" license instead of the Python license.

License.txt (scintilla/SciTE)
http://scintilla.cvs.sourceforge.net/scintilla/scite/License.txt?view=markup

Historical Permission Notice and Disclaimer
http://www.opensource.org/licenses/historical.php

According to Wikipedia [1]: "It [SciTE] is licensed under a minimal version of
the Historical Permission Notice and Disclaimer".

[1] http://en.wikipedia.org/wiki/Scite
Comment 10 Jason Tibbitts 2006-06-05 18:53:11 EDT
The new package looks good; the only issue I had is fixed.

APPROVED

Let me know if you need help checking in.
Comment 11 Iago Rubio 2006-06-06 01:51:51 EDT
(In reply to comment #9)
> SciTE's License.txt file contains a copy of the "Historical Permission Notice
> and Disclaimer" license instead of the Python license.

Sorry for the delay, but on doubt I asked upstream.

http://aspn.activestate.com/ASPN/Mail/Message/scintilla-interest/3153640

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