Bug 757657

Summary: Review Request: perl-Wx-Scintilla - Scintilla source code editing component for wxWidgets
Product: [Fedora] Fedora Reporter: Marcela Mašláňová <mmaslano>
Component: Package ReviewAssignee: Petr Pisar <ppisar>
Status: CLOSED NOTABUG QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: rawhideCC: i, package-review, ppisar, robinlee.sysu
Target Milestone: ---   
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2020-07-11 00:46:32 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: 201449, 754091, 784252, 815713    

Comment 1 Petr Pisar 2011-11-28 12:14:42 UTC
Source tar ball is original. Ok.
Summary verified from lib/Wx/Scintilla.pm. Ok.

FIX: License tag does not cover bundled Scintilla sources (wx-scintilla/src/scintilla/).
FIX: Do not bundle Scintilla (wx-scintilla/src/scintilla/).

TODO: Augment description with other paragraphs from lib/Wx/Scintilla.pm POD. The first one does not define what this package delivers.

FIX: Build-require perl(Exporter) (lib/Wx/Scintilla/Constant.pm:12)
FIX: Build-require perl(Caro) (lib/Wx/Scintilla.pm:6)
FIX: Build-require perl(XSLoader) (lib/Wx/Scintilla.pm:8)

FIX: Build-require perl(base) (t/lib/Tests_Helper.pm:197)
FIX: Build-require perl(Wx::App) (t/lib/Tests_Helper.pm:197)
FIX: Build-require perl(Wx::Frame) (t/lib/Tests_Helper.pm:218)
FIX: Build-require perl(Wx::Event) (t/03_editor.t:27)
FIX: Build-require perl(lib) (t/02_inheritance.t:10)

TODO: Do not %doc-package META.json (as you do not package META.yml)

All tests pass. Ok.

$ rpmlint  perl-Wx-Scintilla.spec  ../SRPMS/perl-Wx-Scintilla-0.34-1.fc17.src.rpm ../RPMS/x86_64/perl-Wx-Scintilla-*
perl-Wx-Scintilla.spec:22: W: comparison-operator-in-deptoken font(:lang=en)
perl-Wx-Scintilla.src: W: spelling-error Summary(en_US) wxWidgets -> caskets, gaskets, casket's
perl-Wx-Scintilla.src: W: spelling-error %description -l en_US StyledTextCtrl
perl-Wx-Scintilla.src: W: spelling-error %description -l en_US wxWidgets -> caskets, gaskets, casket's
perl-Wx-Scintilla.src: W: spelling-error %description -l en_US timeline -> time line, time-line, timberline
perl-Wx-Scintilla.src:22: W: comparison-operator-in-deptoken font(:lang=en)
perl-Wx-Scintilla.x86_64: W: spelling-error Summary(en_US) wxWidgets -> caskets, gaskets, casket's
perl-Wx-Scintilla.x86_64: W: spelling-error %description -l en_US StyledTextCtrl
perl-Wx-Scintilla.x86_64: W: spelling-error %description -l en_US wxWidgets -> caskets, gaskets, casket's
perl-Wx-Scintilla.x86_64: W: spelling-error %description -l en_US timeline -> time line, time-line, timberline
perl-Wx-Scintilla.x86_64: W: private-shared-object-provides /usr/lib64/perl5/vendor_perl/auto/Wx/Scintilla/Scintilla.so Scintilla.so()(64bit)
perl-Wx-Scintilla.x86_64: W: private-shared-object-provides /usr/lib64/perl5/vendor_perl/auto/Wx/Scintilla/libwx_gtk2u_scintilla-2.8.so libwx_gtk2u_scintilla-2.8.so()(64bit)
perl-Wx-Scintilla.x86_64: W: private-shared-object-provides /usr/lib64/perl5/vendor_perl/auto/Wx/Scintilla/Scintilla.so Scintilla.so()(64bit)
perl-Wx-Scintilla.x86_64: W: private-shared-object-provides /usr/lib64/perl5/vendor_perl/auto/Wx/Scintilla/libwx_gtk2u_scintilla-2.8.so libwx_gtk2u_scintilla-2.8.so()(64bit)
perl-Wx-Scintilla.x86_64: E: binary-or-shlib-defines-rpath /usr/lib64/perl5/vendor_perl/auto/Wx/Scintilla/  ['$ORIGIN']
perl-Wx-Scintilla-debuginfo.x86_64: E: debuginfo-without-sources
3 packages and 1 specfiles checked; 2 errors, 14 warnings.

FIX: Do not provide private libraries (unbundle or use %perl_default_filter).
FIX: Do not set RPATH in Scintilla.so (unbundle or use LDFLAGS+=--enable-new-dtags to convert RPATH to RUN_PATH).

TODO: You can package some tools/*.

$ rpm -q -lv -p ../RPMS/x86_64/perl-Wx-Scintilla-0.34-1.fc17.x86_64.rpm 
drwxr-xr-x    2 root    root                        0 Nov 28 12:06 /usr/lib64/perl5/vendor_perl/Wx
drwxr-xr-x    2 root    root                        0 Nov 28 12:06 /usr/lib64/perl5/vendor_perl/Wx/Scintilla
-rw-r--r--    1 root    root                    14987 Nov 28 12:06 /usr/lib64/perl5/vendor_perl/Wx/Scintilla.pm
-rw-r--r--    1 root    root                   160579 Nov 28 12:06 /usr/lib64/perl5/vendor_perl/Wx/Scintilla/Constant.pm
-rw-r--r--    1 root    root                      517 Nov 28 12:06 /usr/lib64/perl5/vendor_perl/Wx/Scintilla/Manual.pod
-rw-r--r--    1 root    root                      139 Nov 28 12:06 /usr/lib64/perl5/vendor_perl/Wx/Scintilla/TextCtrl.pm
-rw-r--r--    1 root    root                      141 Nov 28 12:06 /usr/lib64/perl5/vendor_perl/Wx/Scintilla/TextEvent.pm
drwxr-xr-x    2 root    root                        0 Nov 28 12:06 /usr/lib64/perl5/vendor_perl/auto/Wx
drwxr-xr-x    2 root    root                        0 Nov 28 12:06 /usr/lib64/perl5/vendor_perl/auto/Wx/Scintilla
-rwxr-xr-x    1 root    root                   432240 Nov 28 12:06 /usr/lib64/perl5/vendor_perl/auto/Wx/Scintilla/Scintilla.so
-rwxr-xr-x    1 root    root                  2276232 Nov 28 12:06 /usr/lib64/perl5/vendor_perl/auto/Wx/Scintilla/libwx_gtk2u_scintilla-2.8.so
drwxr-xr-x    2 root    root                        0 Nov 28 12:06 /usr/share/doc/perl-Wx-Scintilla-0.34
-rw-r--r--    1 root    root                    10463 Nov  2 12:51 /usr/share/doc/perl-Wx-Scintilla-0.34/Changes
-rw-r--r--    1 root    root                     2012 Nov  2 12:51 /usr/share/doc/perl-Wx-Scintilla-0.34/META.json
-rw-r--r--    1 root    root                     1557 Nov  2 12:51 /usr/share/doc/perl-Wx-Scintilla-0.34/README
-rw-r--r--    1 root    root                     2496 Nov  2 12:51 /usr/share/doc/perl-Wx-Scintilla-0.34/Scintilla_README
-rw-r--r--    1 root    root                     5186 Nov 28 12:06 /usr/share/man/man3/Wx::Scintilla.3pm.gz
-rw-r--r--    1 root    root                    16800 Nov 28 12:06 /usr/share/man/man3/Wx::Scintilla::Constant.3pm.gz
-rw-r--r--    1 root    root                     1931 Nov 28 12:06 /usr/share/man/man3/Wx::Scintilla::Manual.3pm.gz

File layout an permission Ok.

$ rpm -q --requires -p ../RPMS/x86_64/perl-Wx-Scintilla-0.[test@fedora-17 SPECS]$ rpm -q --requires -p ../RPMS/x86_64/perl-Wx-Scintilla-0.34-1.fc17.x86_64.rpm |sort |uniq -c
      1 libatk-1.0.so.0()(64bit)
      1 libcrypt.so.1()(64bit)
      1 libc.so.6()(64bit)
      1 libc.so.6(GLIBC_2.14)(64bit)
      1 libc.so.6(GLIBC_2.2.5)(64bit)
      1 libc.so.6(GLIBC_2.3)(64bit)
      1 libdl.so.2()(64bit)
      1 libfontconfig.so.1()(64bit)
      1 libfreetype.so.6()(64bit)
      1 libgcc_s.so.1()(64bit)
      1 libgcc_s.so.1(GCC_3.0)(64bit)
      1 libgdk_pixbuf-2.0.so.0()(64bit)
      1 libgdk-x11-2.0.so.0()(64bit)
      1 libglib-2.0.so.0()(64bit)
      1 libgmodule-2.0.so.0()(64bit)
      1 libgobject-2.0.so.0()(64bit)
      1 libgthread-2.0.so.0()(64bit)
      1 libgtk-x11-2.0.so.0()(64bit)
      1 libm.so.6()(64bit)
      1 libnsl.so.1()(64bit)
      1 libpango-1.0.so.0()(64bit)
      1 libpangoft2-1.0.so.0()(64bit)
      1 libpng15.so.15()(64bit)
      1 libpthread.so.0()(64bit)
      1 libpthread.so.0(GLIBC_2.2.5)(64bit)
      1 libresolv.so.2()(64bit)
      1 librt.so.1()(64bit)
      1 libstdc++.so.6()(64bit)
      1 libstdc++.so.6(CXXABI_1.3)(64bit)
      1 libstdc++.so.6(GLIBCXX_3.4)(64bit)
      1 libutil.so.1()(64bit)
      1 libwx_baseu-2.8.so.0()(64bit)
      1 libwx_baseu-2.8.so.0(WXU_2.8)(64bit)
      1 libwx_gtk2u_core-2.8.so.0()(64bit)
      1 libwx_gtk2u_core-2.8.so.0(WXU_2.8)(64bit)
      1 libwx_gtk2u_scintilla-2.8.so()(64bit)
      1 libz.so.1()(64bit)
      1 perl >= 0:5.008
      1 perl(Carp)
      1 perl(constant)
      1 perl(Exporter)
      1 perl(:MODULE_COMPAT_5.14.2)
      1 perl(strict)
      1 perl(warnings)
      1 perl(Wx)
      1 perl(Wx::Scintilla::Constant)
      1 perl(Wx::Scintilla::TextCtrl)
      1 perl(Wx::Scintilla::TextEvent)
      1 perl(XSLoader)
      1 rpmlib(CompressedFileNames) <= 3.0.4-1
      1 rpmlib(FileDigests) <= 4.6.0-1
      1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1
      1 rpmlib(PayloadIsXz) <= 5.2-1
      1 rtld(GNU_HASH)
Binary requires Ok.

$ rpm -q --provides -p ../RPMS/x86_64/perl-Wx-Scintilla-0.34-1.fc17.x86_64.rpm |sort |uniq -c
      1 libwx_gtk2u_scintilla-2.8.so()(64bit)  
      1 perl(Wx::Scintilla) = 0.34
      1 perl-Wx-Scintilla = 0.34-1.fc17
      1 perl(Wx::Scintilla::Constant) = 0.34
      1 perl(Wx::Scintilla::Manual)  
      1 perl(Wx::Scintilla::TextCtrl) = 0.34
      1 perl(Wx::ScintillaTextCtrl) = 0.34
      1 perl(Wx::Scintilla::TextEvent) = 0.34
      1 perl(Wx::ScintillaTextEvent) = 0.34
      1 perl-Wx-Scintilla(x86-64) = 0.34-1.fc17
      1 Scintilla.so()(64bit)  
FIX: Do not export private libwx_gtk2u_scintilla-2.8.so()(64bit).

$ resolvedeps rawhide ../RPMS/x86_64/perl-Wx-Scintilla-0.34-1.fc17.x86_64.rpm 
Binary dependencies resolvable. Ok.

Package builds in F17 (http://koji.fedoraproject.org/koji/taskinfo?taskID=3546038). Ok.

Package is in line with Perl packaging guidelines.
Package does not conform to Fedora packaging guidelines: Bundles library.

Correct all `FIX' prefixed issues, consider fixing `TODO' items and provide new spec file.

Resolution: Package NOT approved.

Comment 2 Marcela Mašláňová 2012-01-02 16:06:11 UTC
(In reply to comment #1)
New version of the specfile.
http://mmaslano.fedorapeople.org/review/perl-Wx-Scintilla.spec

> Source tar ball is original. Ok.
> Summary verified from lib/Wx/Scintilla.pm. Ok.
> 
> FIX: License tag does not cover bundled Scintilla sources
> (wx-scintilla/src/scintilla/).
I can attach an email from the upstream if comments in the specfile are not enough. 

> FIX: Do not bundle Scintilla (wx-scintilla/src/scintilla/).
Scintilla must be bundled. According to upstream development of the main source is slow and it doesn't have some features, which they need. So, they've also changed some files for specific usage in Padre. qtscintilla made the same thing, but for their own use.
Another reason is the portability.
> 
> TODO: Augment description with other paragraphs from lib/Wx/Scintilla.pm POD.
> The first one does not define what this package delivers.
Fixed.
> 
> FIX: Build-require perl(Exporter) (lib/Wx/Scintilla/Constant.pm:12)
> FIX: Build-require perl(Caro) (lib/Wx/Scintilla.pm:6)
> FIX: Build-require perl(XSLoader) (lib/Wx/Scintilla.pm:8)
> 
> FIX: Build-require perl(base) (t/lib/Tests_Helper.pm:197)
> FIX: Build-require perl(Wx::App) (t/lib/Tests_Helper.pm:197)
> FIX: Build-require perl(Wx::Frame) (t/lib/Tests_Helper.pm:218)
> FIX: Build-require perl(Wx::Event) (t/03_editor.t:27)
> FIX: Build-require perl(lib) (t/02_inheritance.t:10)
> 
Fixed.

> TODO: Do not %doc-package META.json (as you do not package META.yml)
> 
Fixed.
> All tests pass. Ok.
> 
> $ rpmlint  perl-Wx-Scintilla.spec 
> ../SRPMS/perl-Wx-Scintilla-0.34-1.fc17.src.rpm
> ../RPMS/x86_64/perl-Wx-Scintilla-*
> perl-Wx-Scintilla.spec:22: W: comparison-operator-in-deptoken font(:lang=en)
> perl-Wx-Scintilla.src: W: spelling-error Summary(en_US) wxWidgets -> caskets,
> gaskets, casket's
> perl-Wx-Scintilla.src: W: spelling-error %description -l en_US StyledTextCtrl
> perl-Wx-Scintilla.src: W: spelling-error %description -l en_US wxWidgets ->
> caskets, gaskets, casket's
> perl-Wx-Scintilla.src: W: spelling-error %description -l en_US timeline -> time
> line, time-line, timberline
> perl-Wx-Scintilla.src:22: W: comparison-operator-in-deptoken font(:lang=en)
> perl-Wx-Scintilla.x86_64: W: spelling-error Summary(en_US) wxWidgets ->
> caskets, gaskets, casket's
> perl-Wx-Scintilla.x86_64: W: spelling-error %description -l en_US
> StyledTextCtrl
> perl-Wx-Scintilla.x86_64: W: spelling-error %description -l en_US wxWidgets ->
> caskets, gaskets, casket's
> perl-Wx-Scintilla.x86_64: W: spelling-error %description -l en_US timeline ->
> time line, time-line, timberline
> perl-Wx-Scintilla.x86_64: W: private-shared-object-provides
> /usr/lib64/perl5/vendor_perl/auto/Wx/Scintilla/Scintilla.so
> Scintilla.so()(64bit)
> perl-Wx-Scintilla.x86_64: W: private-shared-object-provides
> /usr/lib64/perl5/vendor_perl/auto/Wx/Scintilla/libwx_gtk2u_scintilla-2.8.so
> libwx_gtk2u_scintilla-2.8.so()(64bit)
> perl-Wx-Scintilla.x86_64: W: private-shared-object-provides
> /usr/lib64/perl5/vendor_perl/auto/Wx/Scintilla/Scintilla.so
> Scintilla.so()(64bit)
> perl-Wx-Scintilla.x86_64: W: private-shared-object-provides
> /usr/lib64/perl5/vendor_perl/auto/Wx/Scintilla/libwx_gtk2u_scintilla-2.8.so
> libwx_gtk2u_scintilla-2.8.so()(64bit)
> perl-Wx-Scintilla.x86_64: E: binary-or-shlib-defines-rpath
> /usr/lib64/perl5/vendor_perl/auto/Wx/Scintilla/  ['$ORIGIN']
> perl-Wx-Scintilla-debuginfo.x86_64: E: debuginfo-without-sources
> 3 packages and 1 specfiles checked; 2 errors, 14 warnings.
> 
> FIX: Do not provide private libraries (unbundle or use %perl_default_filter).
Filter is used.

> FIX: Do not set RPATH in Scintilla.so (unbundle or use
> LDFLAGS+=--enable-new-dtags to convert RPATH to RUN_PATH).
I guess RPATH is not a problem here. They are using the bundled version, so they want to use this specific Scintilla.so.

> 
> TODO: You can package some tools/*.
Not sure if they are useful.

Comment 3 Petr Pisar 2012-01-03 16:51:42 UTC
Spec file changes:
--- perl-Wx-Scintilla.spec	2011-11-28 11:00:50.000000000 +0100
+++ perl-Wx-Scintilla.spec.1	2012-01-02 16:58:24.000000000 +0100
@@ -4,17 +4,33 @@
 Version:        0.34
 Release:        1%{?dist}
 Summary:        Scintilla source code editing component for wxWidgets
-License:        GPL+ or Artistic
+# details of licenses:
+# wx-scintilla is bundle of scintilla under wxwindows
+# wx-scintilla/src/scintilla/src is copy of files from scintilla/{lexers,lexlib,src}
+# wx-scintilla/src/scintilla/include contains upstream scintilla/include without *.py file
+# wx-scintilla/src/scintilla/License.txt is MIT
+# wx-scintilla/include files are marked as wxWindows license
+# According to email with upstream the bundle is necessary, because every project needs
+# modify scintilla for their own use.
+License:        (GPL+ or Artistic) and MIT and wxWidgets
 Group:          Development/Libraries
 URL:            http://search.cpan.org/dist/Wx-Scintilla/
 Source0:        http://www.cpan.org/authors/id/A/AZ/AZAWAWI/Wx-Scintilla-%{version}.tar.gz
 BuildRequires:  perl(Alien::wxWidgets)
+BuildRequires:  perl(Carp)
+BuildRequires:  perl(Exporter)
 BuildRequires:  perl(ExtUtils::XSpp) >= 0.1602
 BuildRequires:  perl(Module::Build)
 BuildRequires:  perl(Test::More)
 BuildRequires:  perl(Wx)
+BuildRequires:  perl(XSLoader)
 # tests BR
 BuildRequires:  wxGTK-devel, gtk2-devel
+BuildRequires:  perl(base)
+BuildRequires:  perl(Wx::App)
+BuildRequires:  perl(Wx::Frame)
+BuildRequires:  perl(Wx::Event)
+BuildRequires:  perl(lib)
 %if %{use_x11_tests}
 # X11 tests:
 BuildRequires:  xorg-x11-server-Xvfb
@@ -26,18 +42,26 @@
 
 %description
 While we already have a good scintilla editor component support via
-Wx::StyledTextCtrl in Perl, we already suffer from an older scintilla
-package and thus lagging Perl support in the popular Wx Scintilla
-component. wxWidgets http://wxwidgets.org has a *very slow* release
-timeline. Scintilla is a contributed project which means it will not be the
-latest by the time a new wxWidgets distribution is released. And on the
-scintilla front, the Perl 5 lexer is not 100% bug free even and we do not
-have any kind of Perl 6 support in Scintilla.
+Wx::StyledTextCtrl in Perl, we already suffer from an older scintilla package
+and thus lagging Perl support in the popular Wx Scintilla component. wxWidgets
+L<http://wxwidgets.org> has a *very slow* release timeline. Scintilla is a
+contributed project which means it will not be the latest by the time a new
+wxWidgets distribution is released. And on the scintilla front, the Perl 5 lexer
+is not 100% bug free even and we do not have any kind of Perl 6 support in
+Scintilla.
+
+The ambitious goal of this project is to provide fresh Perl 5 and maybe 6
+support in L<Wx> while preserving compatibility with Wx::StyledTextCtrl
+and continually contribute it back to Scintilla project.
+
+%{?perl_default_filter}
 
 %prep
 %setup -q -n Wx-Scintilla-%{version}
+rm -rf ./Wx-Scintilla-%{version}/wx-scintilla
 
 %build
+export LDFLAGS=--enable-new-dtags
 %{__perl} Build.PL installdirs=vendor optimize="$RPM_OPT_FLAGS"
 ./Build
 
@@ -59,7 +83,7 @@
 %endif
 
 %files
-%doc Changes META.json README Scintilla_README 
+%doc Changes README Scintilla_README 
 %{perl_vendorarch}/auto/*
 %{perl_vendorarch}/Wx*
 %{_mandir}/man3/*


> FIX: License tag does not cover bundled Scintilla sources
> (wx-scintilla/src/scintilla/).
-License:        GPL+ or Artistic
+# details of licenses:
+# wx-scintilla is bundle of scintilla under wxwindows
+# wx-scintilla/src/scintilla/src is copy of files from scintilla/{lexers,lexlib,src}
+# wx-scintilla/src/scintilla/include contains upstream scintilla/include without *.py file
+# wx-scintilla/src/scintilla/License.txt is MIT
+# wx-scintilla/include files are marked as wxWindows license
+# According to email with upstream the bundle is necessary, because every project needs
+# modify scintilla for their own use.
+License:        (GPL+ or Artistic) and MIT and wxWidgets

FIX: wx-scintilla/src/scintilla/License.txt looks like MIT but it is not exact copy of any wordings from <https://fedoraproject.org/wiki/Licensing/MIT>. Please get confirmation by Fedora legal department. Scintiall variant should appear on the web page.

> > FIX: Do not bundle Scintilla (wx-scintilla/src/scintilla/).
> Scintilla must be bundled.
FIX: Then you need exception from Fedora Packaging Committee (https://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries#Exceptions).

> TODO: Augment description with other paragraphs from lib/Wx/Scintilla.pm POD.
> The first one does not define what this package delivers.
 %description
 While we already have a good scintilla editor component support via
-Wx::StyledTextCtrl in Perl, we already suffer from an older scintilla
-package and thus lagging Perl support in the popular Wx Scintilla
-component. wxWidgets http://wxwidgets.org has a *very slow* release
-timeline. Scintilla is a contributed project which means it will not be the
-latest by the time a new wxWidgets distribution is released. And on the
-scintilla front, the Perl 5 lexer is not 100% bug free even and we do not
-have any kind of Perl 6 support in Scintilla.
+Wx::StyledTextCtrl in Perl, we already suffer from an older scintilla package
+and thus lagging Perl support in the popular Wx Scintilla component. wxWidgets
+L<http://wxwidgets.org> has a *very slow* release timeline. Scintilla is a
+contributed project which means it will not be the latest by the time a new
+wxWidgets distribution is released. And on the scintilla front, the Perl 5 lexer
+is not 100% bug free even and we do not have any kind of Perl 6 support in
+Scintilla.
+
+The ambitious goal of this project is to provide fresh Perl 5 and maybe 6
+support in L<Wx> while preserving compatibility with Wx::StyledTextCtrl
+and continually contribute it back to Scintilla project.

FIX: Remove POD mark-up from Description text.

> FIX: Build-require perl(Exporter) (lib/Wx/Scintilla/Constant.pm:12)
+BuildRequires:  perl(Exporter)
Ok.

> FIX: Build-require perl(Caro) (lib/Wx/Scintilla.pm:6)
+BuildRequires:  perl(Carp)
Ok.

> FIX: Build-require perl(XSLoader) (lib/Wx/Scintilla.pm:8)
+BuildRequires:  perl(XSLoader)
Ok.

> FIX: Build-require perl(base) (t/lib/Tests_Helper.pm:197)
+BuildRequires:  perl(base)
Ok.

> FIX: Build-require perl(Wx::App) (t/lib/Tests_Helper.pm:197)
+BuildRequires:  perl(Wx::App)
Ok.

> FIX: Build-require perl(Wx::Frame) (t/lib/Tests_Helper.pm:218)
+BuildRequires:  perl(Wx::Frame)
Ok.

> FIX: Build-require perl(Wx::Event) (t/03_editor.t:27)
+BuildRequires:  perl(Wx::Event)
Ok.

> FIX: Build-require perl(lib) (t/02_inheritance.t:10)
+BuildRequires:  perl(lib)
Ok.

> TODO: Do not %doc-package META.json (as you do not package META.yml)
 %files
-%doc Changes META.json README Scintilla_README 
+%doc Changes README Scintilla_README 
Ok.


$ rpmlint perl-Wx-Scintilla.spec ../SRPMS/perl-Wx-Scintilla-0.34-1.fc17.src.rpm ../RPMS/x86_64/perl-Wx-Scintilla-*
perl-Wx-Scintilla.spec:38: W: comparison-operator-in-deptoken font(:lang=en)
perl-Wx-Scintilla.src: W: spelling-error Summary(en_US) wxWidgets -> caskets, gaskets, casket's
perl-Wx-Scintilla.src: W: spelling-error %description -l en_US StyledTextCtrl
perl-Wx-Scintilla.src: W: spelling-error %description -l en_US wxWidgets -> caskets, gaskets, casket's
perl-Wx-Scintilla.src: W: spelling-error %description -l en_US http -> HTTP, HP, HT
perl-Wx-Scintilla.src: W: spelling-error %description -l en_US wxwidgets -> caskets, gaskets, casket's
perl-Wx-Scintilla.src: W: spelling-error %description -l en_US timeline -> time line, time-line, timberline
perl-Wx-Scintilla.src:38: W: comparison-operator-in-deptoken font(:lang=en)
perl-Wx-Scintilla.x86_64: W: spelling-error Summary(en_US) wxWidgets -> caskets, gaskets, casket's
perl-Wx-Scintilla.x86_64: W: spelling-error %description -l en_US StyledTextCtrl
perl-Wx-Scintilla.x86_64: W: spelling-error %description -l en_US wxWidgets -> caskets, gaskets, casket's
perl-Wx-Scintilla.x86_64: W: spelling-error %description -l en_US http -> HTTP, HP, HT
perl-Wx-Scintilla.x86_64: W: spelling-error %description -l en_US wxwidgets -> caskets, gaskets, casket's
perl-Wx-Scintilla.x86_64: W: spelling-error %description -l en_US timeline -> time line, time-line, timberline
perl-Wx-Scintilla.x86_64: E: binary-or-shlib-defines-rpath /usr/lib64/perl5/vendor_perl/auto/Wx/Scintilla/Scintilla.so ['$ORIGIN']
perl-Wx-Scintilla-debuginfo.x86_64: E: debuginfo-without-sources
3 packages and 1 specfiles checked; 2 errors, 14 warnings.


> FIX: Do not provide private libraries (unbundle or use %perl_default_filter).
+%{?perl_default_filter}
Ok.

> > FIX: Do not set RPATH in Scintilla.so (unbundle or use
> > LDFLAGS+=--enable-new-dtags to convert RPATH to RUN_PATH).
> I guess RPATH is not a problem here. They are using the bundled version, so
> they want to use this specific Scintilla.so.
RPATH cannot be overridden by environment variables (if requested by user) in contrast to RUN_PATH. Thus converting RPATH to RUN_PATH is good. Apparently the LDFLAGS value did not pass to linker. However this is not blocker, especially if the second library is in private directory. So you can remove the useless variable export.

$ rpm -q --provides -p ../RPMS/x86_64/perl-Wx-Scintilla-0.34-1.fc17.x86_64.rpm
perl(Wx::Scintilla) = 0.34
perl(Wx::Scintilla::Constant) = 0.34
perl(Wx::Scintilla::Manual)  
perl(Wx::Scintilla::TextCtrl) = 0.34
perl(Wx::Scintilla::TextEvent) = 0.34
perl(Wx::ScintillaTextCtrl) = 0.34
perl(Wx::ScintillaTextEvent) = 0.34
perl-Wx-Scintilla = 0.34-1.fc17
perl-Wx-Scintilla(x86-64) = 0.34-1.fc17

Binary provides are Ok.


Please correct all `FIX' prefixed issues, get exception from FPC and provide new spec file.

Comment 4 Marcela Mašláňová 2012-01-04 11:04:36 UTC
(In reply to comment #3)
> Spec file changes:
> FIX: wx-scintilla/src/scintilla/License.txt looks like MIT but it is not exact
> copy of any wordings from <https://fedoraproject.org/wiki/Licensing/MIT>.
> Please get confirmation by Fedora legal department. Scintiall variant should
> appear on the web page.
I've already sent email to legal. Spot answered it's MIT and my wording of license is correct.

> > > FIX: Do not bundle Scintilla (wx-scintilla/src/scintilla/).
> > Scintilla must be bundled.
> FIX: Then you need exception from Fedora Packaging Committee
Will do.

> > TODO: Augment description with other paragraphs from lib/Wx/Scintilla.pm > FIX: Remove POD mark-up from Description text.
Done.

> > > FIX: Do not set RPATH in Scintilla.so (unbundle or use
> > > LDFLAGS+=--enable-new-dtags to convert RPATH to RUN_PATH).
> > I guess RPATH is not a problem here. They are using the bundled version, so
> > they want to use this specific Scintilla.so.
> RPATH cannot be overridden by environment variables (if requested by user) in
> contrast to RUN_PATH. Thus converting RPATH to RUN_PATH is good. Apparently the
> LDFLAGS value did not pass to linker. However this is not blocker, especially
> if the second library is in private directory. So you can remove the useless
> variable export.
> 
Done.

New specfile http://mmaslano.fedorapeople.org/review/perl-Wx-Scintilla.spec
FPC ticket: https://fedorahosted.org/fpc/ticket/126

Comment 5 Petr Pisar 2012-01-04 14:41:00 UTC
Spec file changes:

--- perl-Wx-Scintilla.spec      2012-01-02 16:58:24.000000000 +0100
+++ perl-Wx-Scintilla.spec.1    2012-01-04 11:57:07.000000000 +0100
@@ -44,7 +44,7 @@
 While we already have a good scintilla editor component support via
 Wx::StyledTextCtrl in Perl, we already suffer from an older scintilla package
 and thus lagging Perl support in the popular Wx Scintilla component. wxWidgets
-L<http://wxwidgets.org> has a *very slow* release timeline. Scintilla is a
+http://wxwidgets.org has a *very slow* release timeline. Scintilla is a
 contributed project which means it will not be the latest by the time a new
 wxWidgets distribution is released. And on the scintilla front, the Perl 5 lexer
 is not 100% bug free even and we do not have any kind of Perl 6 support in
@@ -61,7 +61,6 @@
 rm -rf ./Wx-Scintilla-%{version}/wx-scintilla
 
 %build
-export LDFLAGS=--enable-new-dtags
 %{__perl} Build.PL installdirs=vendor optimize="$RPM_OPT_FLAGS"
 ./Build

> > FIX: wx-scintilla/src/scintilla/License.txt looks like MIT but it is not
> > exact copy of any wordings from
> > <https://fedoraproject.org/wiki/Licensing/MIT>. Please get confirmation
> > by Fedora legal department. Scintiall variant should appear on the web
> > page.
> I've already sent email to legal. Spot answered it's MIT
<http://lists.fedoraproject.org/pipermail/legal/2011-December/001767.html>
Ok.

> and my wording of license is correct.
I didn't thought your wording. I thought copy of Scintilla MIT variant would appear on the web page. I thought that each unique  wording recognized as a variant should appear there. Maybe my assumption is wrong.

> FIX: Remove POD mark-up from Description text.
-L<http://wxwidgets.org> has a *very slow* release timeline. Scintilla is a
+http://wxwidgets.org has a *very slow* release timeline. Scintilla is a

This is not everything. I can still see L<http://wxwidgets.org> and L<Wx>.
FIX: Clean all the marks

> FPC ticket: https://fedorahosted.org/fpc/ticket/126
Once you obtain exception, drop a notice here and refer to new spec file with cleaned description.

Comment 6 Petr Pisar 2012-01-04 14:42:57 UTC
> This is not everything. I can still see L<http://wxwidgets.org> and L<Wx>.
> FIX: Clean all the marks

Oops. I read old spec file. The latest one still contains L<Wx>.

Comment 7 Petr Pisar 2015-07-24 14:09:13 UTC
I compared Wx-Scintilla-0.39 with scintilla-3.0.4 and there are very small differences:

(1) scintilla's lexers, lexlib and src directories content is merged into Wc-Scintilla's src directory.

(2) Wx-Scintilla's "include" directory is missing Face.py and HFacer.py files. These are used only for generating headers from Scintilla.iface file.

(3) Wx-Scintilla's "include" directory extends SciLexer.h because Wx-Scintilla adds support for Perl6 lexer.

--- /tmp/scintilla-3.0.4/include/SciLexer.h     2012-03-08 03:59:38.000000000 +0100
+++ include/SciLexer.h  2012-09-04 12:20:47.000000000 +0200
@@ -118,6 +118,7 @@
 #define SCLEX_TCMD 103
 #define SCLEX_AVS 104
 #define SCLEX_ECL 105
+#define SCLEX_PERL6 199
 #define SCLEX_AUTOMATIC 1000
 #define SCE_P_DEFAULT 0
 #define SCE_P_COMMENTLINE 1
@@ -359,6 +360,9 @@
 #define SCE_PL_STRING_QQ_VAR 64
 #define SCE_PL_STRING_QX_VAR 65
 #define SCE_PL_STRING_QR_VAR 66
+#define SCE_P6_DEFAULT  0
+#define SCE_P6_COMMENT  1
+#define SCE_P6_STRING   2
 #define SCE_RB_DEFAULT 0
 #define SCE_RB_ERROR 1
 #define SCE_RB_COMMENTLINE 2

(4) Wx-Scintilla's "lexers" directory adds LexPerl6.cxx file with the Perl6 lexer. It does not bring any new dependencies or incompatibilities.

(5) Wx-Scintilla patches PropSetSimple.cxx and WordList.cxx in liblex directory because of Visual C++ compiler. It also patches SparseState.h for unknown reason:

--- /tmp/scintilla-3.0.4/lexlib/SparseState.h   2011-09-16 11:13:20.000000000 +0
200
+++ lexlib/SparseState.h        2012-09-04 12:20:47.000000000 +0200
@@ -92,7 +92,7 @@
                                changed = true;
                        }
                        typename stateVector::const_iterator startOther = other.
states.begin();
-                       if (!states.empty() && !other.states.empty() && states.b
ack().value == startOther->value)
+                       if (!states.empty() && states.back().value == startOther
->value)
                                ++startOther;
                        if (startOther != other.states.end()) {
                                states.insert(states.end(), startOther, other.st
ates.end());

(6) Wx-Scintilla removed LexGen.py and SciTE.properties from "src" directory. The SciTE.properties is for SciTE editor which is now a standalone project and the LexGen.py regenerates sources.

These are the only differences.

Other question is if it possible to build scintilla without an implicit binding to GTK as is done in the upstream.

Comment 8 Petr Pisar 2017-03-13 10:52:37 UTC
A year ago I tried to build Scintilla as a shared library (despite upstream does not support it). It worked, but current upstream version is incompatible with the old one bundled in the Wx-Scintilla. (Oh the irony. Wx-Scintilla blaming Scintilla to be to slow itself became out-dated.) Current Fedora guidelines allow bundling in this case.

If you provide a spec file updated to current Perl packaging guidelines (build-requiring perl-generators etc.), I will continue with this review.

Comment 9 Petr Pisar 2020-04-22 14:28:02 UTC
Do you want to continue in the review? If you don't say yeas, I will close this review as a death one.

Comment 10 Package Review 2020-07-11 00:46:32 UTC
This is an automatic action taken by review-stats script.

The ticket submitter failed to clear the NEEDINFO flag in a month.
As per https://fedoraproject.org/wiki/Policy_for_stalled_package_reviews
we consider this ticket as DEADREVIEW and proceed to close it.