Bug 1361883

Summary: Review Request: libmypaint - Library for making brush strokes
Product: [Fedora] Fedora Reporter: Nils Philippsen <nphilipp>
Component: Package ReviewAssignee: Igor Gnatenko <ignatenko>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: jridky, lukas+fedora, nphilipp, package-review
Target Milestone: ---Flags: ignatenko: fedora-review+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2017-02-15 20:51:10 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:

Description Nils Philippsen 2016-07-31 13:37:30 UTC
Spec URL: http://copr-dist-git.fedorainfracloud.org/cgit/nphilipp/gimp-unstable/libmypaint.git/plain/libmypaint.spec
SRPM URL: https://copr-be.cloud.fedoraproject.org/results/nphilipp/gimp-unstable/fedora-rawhide-x86_64/00429428-libmypaint/libmypaint-1.3.0-0.1.beta.1.fc26.src.rpm
Description: This is a self-contained library containing the MyPaint brush engine.
Fedora Account System Username: nphilipp

Comment 1 Nils Philippsen 2016-07-31 13:48:03 UTC
Josef, I've added you to CC because this package is a prerequisite for the next GIMP stable release 2.10 (whenever that one is ready ;) and of course the unstable releases up to that -- I'm currently working on getting 2.9.4 built for the gimp-unstable COPR.

Comment 2 Igor Gnatenko 2016-07-31 14:22:40 UTC
will review very soon.

Comment 3 Igor Gnatenko 2016-07-31 15:46:13 UTC
> Requires: pkgconfig
not needed

> Summary: libmypaint development files
summary should start from upper-case

> make %{?_smp_mflags}
should be changed to %make_build

> make DESTDIR=%{buildroot} install
should be changed to %make_install

> rm -f %{buildroot}%{_libdir}/*.la
personally I prefer to do: find %{buildroot}%{_libdir} -name '*.la' -delete -print

> %{_libdir}/girepository-1.0
%dir %{_libdir}/girepository-1.0
%{_libdir}/girepository-1.0/... specify here file

same for gir directory

> # files missing from tarball
why not to use git-tag tarball instead of make-dist tarball?

> No known owner of /usr/share/locale/ar_AR/LC_MESSAGES, /usr/share/locale/ar_AR
That's interesting... Can you check if it's correct locale name "ar_AR" and if it is, please open bug against package "filesystem" which doesn't know anything about that lang.

> libmypaint-devel.x86_64: W: spelling-error %description -l en_US constains -> contains, constrains, con stains
fix it please.

> libmypaint.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libmypaint-1.3.so.0.0.0 /lib64/libglib-2.0.so.0
> libmypaint.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmypaint-gegl.so.0.0.0 gegl_buffer_get
> libmypaint.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmypaint-gegl.so.0.0.0 g_object_unref
> libmypaint.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmypaint-gegl.so.0.0.0 mypaint_tiled_surface_init
> libmypaint.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmypaint-gegl.so.0.0.0 gegl_buffer_set
> libmypaint.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmypaint-gegl.so.0.0.0 gegl_buffer_iterator_next
> libmypaint.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmypaint-gegl.so.0.0.0 g_assertion_message_expr
> libmypaint.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmypaint-gegl.so.0.0.0 g_return_if_fail_warning
> libmypaint.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmypaint-gegl.so.0.0.0 gegl_node_new_child
> libmypaint.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmypaint-gegl.so.0.0.0 g_object_new
> libmypaint.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmypaint-gegl.so.0.0.0 gegl_buffer_get_format
> libmypaint.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmypaint-gegl.so.0.0.0 gegl_node_new
> libmypaint.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmypaint-gegl.so.0.0.0 gegl_malloc
> libmypaint.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmypaint-gegl.so.0.0.0 g_object_ref
> libmypaint.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmypaint-gegl.so.0.0.0 gegl_free
> libmypaint.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmypaint-gegl.so.0.0.0 babl_format_get_bytes_per_pixel
> libmypaint.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmypaint-gegl.so.0.0.0 babl_type
> libmypaint.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmypaint-gegl.so.0.0.0 babl_format_new
> libmypaint.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmypaint-gegl.so.0.0.0 mypaint_surface_ref
> libmypaint.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmypaint-gegl.so.0.0.0 g_log
> libmypaint.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmypaint-gegl.so.0.0.0 gegl_buffer_iterator_new
> libmypaint.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmypaint-gegl.so.0.0.0 gegl_buffer_get_type
> libmypaint.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmypaint-gegl.so.0.0.0 gegl_node_link
> libmypaint.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmypaint-gegl.so.0.0.0 gegl_rectangle_bounding_box
> libmypaint.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmypaint-gegl.so.0.0.0 g_type_check_instance_is_a
> libmypaint.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmypaint-gegl.so.0.0.0 babl_component
> libmypaint.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmypaint-gegl.so.0.0.0 g_type_check_instance_cast
> libmypaint.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmypaint-gegl.so.0.0.0 gegl_buffer_set_extent
> libmypaint.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmypaint-gegl.so.0.0.0 g_boxed_type_register_static
> libmypaint.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmypaint-gegl.so.0.0.0 babl_model
> libmypaint.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmypaint-gegl.so.0.0.0 g_object_get
> libmypaint.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmypaint-gegl.so.0.0.0 mypaint_surface_unref
> libmypaint.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmypaint-gegl.so.0.0.0 mypaint_tiled_surface_destroy
> libmypaint.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmypaint-gegl.so.0.0.0 gegl_node_process
> libmypaint.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmypaint-gegl.so.0.0.0 gegl_rectangle_set
> libmypaint.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libmypaint-gegl.so.0.0.0 /lib64/libm.so.6
> libmypaint.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libmypaint-gegl.so.0.0.0 /lib64/libpthread.so.0
report bug upstream, please

> BuildRequires: python
> BuildRequires: python-devel
I guess you need none of them

> BuildRequires: python-sphinx
libmypaint explicitly requires sphinx-build-2, su change to python2-sphinx.

* Missing BR: gcc
* Looks like you build docs, but they're not getting installed

Comment 4 Nils Philippsen 2016-07-31 21:36:06 UTC
Thanks for taking the review!

(In reply to Igor Gnatenko from comment #3)
> > Requires: pkgconfig
> not needed

The pkgconfig package contains the %{_libdir}/pkgconfig directory and is normally used for building with the library.

> > Summary: libmypaint development files
> summary should start from upper-case

fixed

> > make %{?_smp_mflags}
> should be changed to %make_build
> 
> > make DESTDIR=%{buildroot} install
> should be changed to %make_install
> 
> > rm -f %{buildroot}%{_libdir}/*.la
> personally I prefer to do: find %{buildroot}%{_libdir} -name '*.la' -delete
> -print

thanks for the suggestions, fixed

> > %{_libdir}/girepository-1.0
> %dir %{_libdir}/girepository-1.0
> %{_libdir}/girepository-1.0/... specify here file
> 
> same for gir directory

Why? Listing the directory and the files separately, or just the directory recursively doesn't make a difference in the result.
 
> > # files missing from tarball
> why not to use git-tag tarball instead of make-dist tarball?

Because git-tagged tarballs don't include all files contained in a make-dist tarball, and we're ultimately targeting official releases.

> > No known owner of /usr/share/locale/ar_AR/LC_MESSAGES, /usr/share/locale/ar_AR
> That's interesting... Can you check if it's correct locale name "ar_AR" and
> if it is, please open bug against package "filesystem" which doesn't know
> anything about that lang.

I'm pretty sure it isn't a correct locale name, I've submitted a fix/pull request to rename the file:

https://github.com/mypaint/libmypaint/pull/63

> > libmypaint-devel.x86_64: W: spelling-error %description -l en_US constains -> contains, constrains, con stains
> fix it please.

fixed

> > libmypaint.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libmypaint-1.3.so.0.0.0 /lib64/libglib-2.0.so.0
> > libmypaint.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmypaint-gegl.so.0.0.0 gegl_buffer_get
> > libmypaint.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmypaint-gegl.so.0.0.0 g_object_unref
> > libmypaint.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmypaint-gegl.so.0.0.0 mypaint_tiled_surface_init
> > libmypaint.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmypaint-gegl.so.0.0.0 gegl_buffer_set
> > libmypaint.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmypaint-gegl.so.0.0.0 gegl_buffer_iterator_next
> > libmypaint.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmypaint-gegl.so.0.0.0 g_assertion_message_expr
> > libmypaint.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmypaint-gegl.so.0.0.0 g_return_if_fail_warning
> > libmypaint.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmypaint-gegl.so.0.0.0 gegl_node_new_child
> > libmypaint.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmypaint-gegl.so.0.0.0 g_object_new
> > libmypaint.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmypaint-gegl.so.0.0.0 gegl_buffer_get_format
> > libmypaint.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmypaint-gegl.so.0.0.0 gegl_node_new
> > libmypaint.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmypaint-gegl.so.0.0.0 gegl_malloc
> > libmypaint.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmypaint-gegl.so.0.0.0 g_object_ref
> > libmypaint.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmypaint-gegl.so.0.0.0 gegl_free
> > libmypaint.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmypaint-gegl.so.0.0.0 babl_format_get_bytes_per_pixel
> > libmypaint.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmypaint-gegl.so.0.0.0 babl_type
> > libmypaint.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmypaint-gegl.so.0.0.0 babl_format_new
> > libmypaint.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmypaint-gegl.so.0.0.0 mypaint_surface_ref
> > libmypaint.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmypaint-gegl.so.0.0.0 g_log
> > libmypaint.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmypaint-gegl.so.0.0.0 gegl_buffer_iterator_new
> > libmypaint.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmypaint-gegl.so.0.0.0 gegl_buffer_get_type
> > libmypaint.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmypaint-gegl.so.0.0.0 gegl_node_link
> > libmypaint.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmypaint-gegl.so.0.0.0 gegl_rectangle_bounding_box
> > libmypaint.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmypaint-gegl.so.0.0.0 g_type_check_instance_is_a
> > libmypaint.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmypaint-gegl.so.0.0.0 babl_component
> > libmypaint.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmypaint-gegl.so.0.0.0 g_type_check_instance_cast
> > libmypaint.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmypaint-gegl.so.0.0.0 gegl_buffer_set_extent
> > libmypaint.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmypaint-gegl.so.0.0.0 g_boxed_type_register_static
> > libmypaint.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmypaint-gegl.so.0.0.0 babl_model
> > libmypaint.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmypaint-gegl.so.0.0.0 g_object_get
> > libmypaint.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmypaint-gegl.so.0.0.0 mypaint_surface_unref
> > libmypaint.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmypaint-gegl.so.0.0.0 mypaint_tiled_surface_destroy
> > libmypaint.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmypaint-gegl.so.0.0.0 gegl_node_process
> > libmypaint.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmypaint-gegl.so.0.0.0 gegl_rectangle_set
> > libmypaint.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libmypaint-gegl.so.0.0.0 /lib64/libm.so.6
> > libmypaint.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libmypaint-gegl.so.0.0.0 /lib64/libpthread.so.0
> report bug upstream, please

https://github.com/mypaint/libmypaint/issues/64

> > BuildRequires: python
> > BuildRequires: python-devel
> I guess you need none of them

removed

> > BuildRequires: python-sphinx
> libmypaint explicitly requires sphinx-build-2, su change to python2-sphinx.

Fedora 23 doesn't know about python2-sphinx, I've fixed that for Fedora 24 and later.

> * Missing BR: gcc

added

> * Looks like you build docs, but they're not getting installed

included as %doc in the -devel subpackage

Spec File: http://copr-dist-git.fedorainfracloud.org/cgit/nphilipp/gimp-unstable/libmypaint.git/plain/libmypaint.spec?id=6fcf4039c256ef290703cb9f84b5cb91c94c5822
SRPM: https://copr-be.cloud.fedoraproject.org/results/nphilipp/gimp-unstable/fedora-rawhide-x86_64/00430631-libmypaint/libmypaint-1.3.0-0.2.beta.1.fc26.src.rpm

Comment 5 Nils Philippsen 2016-08-11 16:16:07 UTC
Igor, ping?

Comment 6 Igor Gnatenko 2016-08-13 09:50:10 UTC
(In reply to Nils Philippsen from comment #4)
> Thanks for taking the review!
> 
> (In reply to Igor Gnatenko from comment #3)
> > > Requires: pkgconfig
> > not needed
> 
> The pkgconfig package contains the %{_libdir}/pkgconfig directory and is
> normally used for building with the library.
There is automatic requirement for pkgconfig in RPM.

> > > %{_libdir}/girepository-1.0
> > %dir %{_libdir}/girepository-1.0
> > %{_libdir}/girepository-1.0/... specify here file
> > 
> > same for gir directory
> 
> Why? Listing the directory and the files separately, or just the directory
> recursively doesn't make a difference in the result.
It depends. For example, if they will add Gio-99.typelib into girepository you will not notice by using '*'.

Comment 7 Lukas Middendorf 2016-11-10 12:19:19 UTC
I tried to install this on F24 and noticed that there are file conflicts with mypaint-1.1.0

I think this package needs 
Conflicts: mypaint < 1.3.0

Comment 8 Igor Gnatenko 2016-11-14 12:49:23 UTC
ping?

Comment 9 Nils Philippsen 2017-01-27 13:05:18 UTC
Oh, hello 2017! ;)

Sorry for the long wait.

(In reply to Igor Gnatenko from comment #6)
> (In reply to Nils Philippsen from comment #4)
> > Thanks for taking the review!
> > 
> > (In reply to Igor Gnatenko from comment #3)
> > > > Requires: pkgconfig
> > > not needed
> > 
> > The pkgconfig package contains the %{_libdir}/pkgconfig directory and is
> > normally used for building with the library.
> There is automatic requirement for pkgconfig in RPM.

Actually, rpm-build requires pkgconfig but we can't rely on that to be available -- people might want to build software against libmypaint without packaging it.

> > > > %{_libdir}/girepository-1.0
> > > %dir %{_libdir}/girepository-1.0
> > > %{_libdir}/girepository-1.0/... specify here file
> > > 
> > > same for gir directory
> > 
> > Why? Listing the directory and the files separately, or just the directory
> > recursively doesn't make a difference in the result.
> It depends. For example, if they will add Gio-99.typelib into girepository
> you will not notice by using '*'.

Understood, I've added each file in these directories explicitly.

Spec File: http://copr-dist-git.fedorainfracloud.org/cgit/nphilipp/gimp-unstable/libmypaint.git/plain/libmypaint.spec?h=f26&id=7f7d5d921449c4acbf33bc4c6f9ec071b84175d7
SRPM: https://copr-be.cloud.fedoraproject.org/results/nphilipp/gimp-unstable/fedora-26-x86_64/00504439-libmypaint/libmypaint-1.3.0-0.4.fc26.src.rpm

Comment 10 Gwyn Ciesla 2017-01-31 17:31:49 UTC
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/libmypaint

Comment 11 Fedora Update System 2017-02-03 13:27:24 UTC
libmypaint-1.3.0-1.fc24 has been submitted as an update to Fedora 24. https://bodhi.fedoraproject.org/updates/FEDORA-2017-a80caa430d

Comment 12 Fedora Update System 2017-02-03 13:27:32 UTC
libmypaint-1.3.0-1.fc25 has been submitted as an update to Fedora 25. https://bodhi.fedoraproject.org/updates/FEDORA-2017-6f2d59c60f

Comment 13 Fedora Update System 2017-02-03 22:55:07 UTC
libmypaint-1.3.0-1.fc24 has been pushed to the Fedora 24 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2017-a80caa430d

Comment 14 Fedora Update System 2017-02-03 23:50:34 UTC
libmypaint-1.3.0-1.fc25 has been pushed to the Fedora 25 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2017-6f2d59c60f

Comment 15 Fedora Update System 2017-02-15 20:51:10 UTC
libmypaint-1.3.0-1.fc24 has been pushed to the Fedora 24 stable repository. If problems still persist, please make note of it in this bug report.

Comment 16 Fedora Update System 2017-02-15 20:53:23 UTC
libmypaint-1.3.0-1.fc25 has been pushed to the Fedora 25 stable repository. If problems still persist, please make note of it in this bug report.