[Bug 1352408] New: Review Request: lasem - A library for rendering SVG and Mathml, implementing a DOM like API

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1352408

Bug ID: 1352408
           Summary: Review Request: lasem - A library for rendering SVG
                    and Mathml, implementing a DOM like API
           Product: Fedora
           Version: rawhide
         Component: Package Review
          Severity: medium
          Priority: medium
          Assignee: [e-mail]
          Reporter: [e-mail]
        QA Contact: [e-mail]
                CC: [e-mail]

Spec URL: https://belegdol.fedorapeople.org/lasem/lasem.spec
SRPM URL: https://belegdol.fedorapeople.org/lasem/lasem-0.4.3-1.fc22.src.rpm
Description: Lasem is a library for rendering SVG and Mathml, implementing a
DOM like API.
It's based on GObject and use Pango and Cairo for the rendering.
Fedora Account System Username: belegdol

[Bug 1352408] Review Request: lasem - A library for rendering SVG and Mathml, implementing a DOM like API

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1352408

Igor Gnatenko  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
                 CC|                            |[e-mail]
           Assignee|[e-mail]    |[e-mail]

[Bug 1352408] Review Request: lasem - A library for rendering SVG and Mathml, implementing a DOM like API

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1352408

Igor Gnatenko  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
              Alias|                            |lasem

[Bug 1352408] Review Request: lasem - A library for rendering SVG and Mathml, implementing a DOM like API

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1352408

--- Comment #3 from Igor Gnatenko  ---
(In reply to Julian Sikorski from comment #2)
> (In reply to Igor Gnatenko from comment #1)
> Thank you for taking the review! Please find my feedback below:
> > Missing BuildRequires: gcc
>
> Has something changed? I thought gcc was part of minimal buildroot and does
> not need to be required explicitly. In any case, something is pulling it in
> as the build is working.

Something changed. Since (~ f23) it's required to put all BRs like gcc.
> > > BuildRequires:  intltool
> >
> > I would add also BuildRequires: gettext
>
> Intltool pulls gettext by requiring gettext-devel, is an explicit
> requirement necessary?

No, hopefully at some point upstream will stop using intltool and we will
switch to gettext.
> > > Requires:       pkg-config
> >
> > Drop it
>
> OK
> > > make %{?_smp_mflags}
> >
> > could be replaced with %make_build
>
> OK, it was not in the template created by rpmdev-newspec on f22.

rpmdev-newspec is completely outdated.
> > > rm -rf $RPM_BUILD_ROOT
> >
> > Drop it
>
> Why? rpmdev-newspec still put it in as of f24

It's not needed since ~ EL6.
> > > find $RPM_BUILD_ROOT -name '*.la' -exec rm -f {} ';'
> >
> > find %{buildroot}%{_libdir} -type f -name '*.la' -delete -print
>
> Why is this needed? The version I have now is what rpmdev-newspec as of f24
> enters.

Not all packages maintained well. When you use -delete, it doesn't spawn any
command, it just uses unlinkat().
> > > %find_lang %{name} --all-name
> >
> > I think --all-name is not needed, but not sure.
>
> I replaced it with %{name}-%{apiver}
> > > %{_datadir}/gtk-doc/html/%{name}-0.4
> >
> > %doc %{_datadir}/gtk-doc/html/%{name}-0.4
>
> OK
> %dir %{_datadir}/gtk-doc
> %dir %{_datadir}/gtk-doc/html

you don't need to do this.
> > > $RPM_BUILD_ROOT
> >
> > %{buildroot}
>
> I prefer the variable format and as per current guidelines [1] both are
> acceptable.

hopefully it will be deprecated at some point. it's better to use %{buildroot}.
but as you noted, it's up to you.
> > also would be great to do: %global apiver 0.4 and use it in %files.
>
> OK
>
> New releases:
> Spec URL: https://belegdol.fedorapeople.org/lasem/lasem.spec
> SRPM URL: https://belegdol.fedorapeople.org/lasem/lasem-0.4.3-2.fc24.src.rpm
>
> [1]
> https://fedoraproject.org/wiki/Packaging:Guidelines#Using_.25.7Bbuildroot.
> 7D_and_.25.7Boptflags.7D_vs_.24RPM_BUILD_ROOT_and_.24RPM_OPT_FLAGS

Resolution: ALMOST GOOD

[Bug 1352408] Review Request: lasem - A library for rendering SVG and Mathml, implementing a DOM like API

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1352408

--- Comment #4 from Julian Sikorski  ---
(In reply to Igor Gnatenko from comment #3)
> (In reply to Julian Sikorski from comment #2)
> > (In reply to Igor Gnatenko from comment #1)
> > Thank you for taking the review! Please find my feedback below:
> > > Missing BuildRequires: gcc
> >
> > Has something changed? I thought gcc was part of minimal buildroot and does
> > not need to be required explicitly. In any case, something is pulling it in
> > as the build is working.
>
> Something changed. Since (~ f23) it's required to put all BRs like gcc.

Thank you! Will add it.
> > > > BuildRequires:  intltool
> > >
> > > I would add also BuildRequires: gettext
> >
> > Intltool pulls gettext by requiring gettext-devel, is an explicit
> > requirement necessary?
>
> No, hopefully at some point upstream will stop using intltool and we will
> switch to gettext.

If/when it happens BR adjustment will be necessary anyway, so I prefer to keep
things as they are.
> > > > Requires:       pkg-config
> > >
> > > Drop it
> >
> > OK
> > > > make %{?_smp_mflags}
> > >
> > > could be replaced with %make_build
> >
> > OK, it was not in the template created by rpmdev-newspec on f22.
>
> rpmdev-newspec is completely outdated.
> > > > rm -rf $RPM_BUILD_ROOT
> > >
> > > Drop it
> >
> > Why? rpmdev-newspec still put it in as of f24
>
> It's not needed since ~ EL6.

Didn't know that, thank you! Will change it.
> > > > find $RPM_BUILD_ROOT -name '*.la' -exec rm -f {} ';'
> > >
> > > find %{buildroot}%{_libdir} -type f -name '*.la' -delete -print
> >
> > Why is this needed? The version I have now is what rpmdev-newspec as of f24
> > enters.
>
> Not all packages maintained well. When you use -delete, it doesn't spawn any
> command, it just uses unlinkat().

Didn't know that either, thanks! Will change it too!
> > > > %find_lang %{name} --all-name
> > >
> > > I think --all-name is not needed, but not sure.
> >
> > I replaced it with %{name}-%{apiver}
> > > > %{_datadir}/gtk-doc/html/%{name}-0.4
> > >
> > > %doc %{_datadir}/gtk-doc/html/%{name}-0.4
> >
> > OK
> > %dir %{_datadir}/gtk-doc
> > %dir %{_datadir}/gtk-doc/html
>
> you don't need to do this.

I think I do - otherwise the directory ownership guideline [2] might be
violated I think. lasem puts files in %{_datadir}/gtk-doc but does not depend
on it, so it needs to own it. %doc only owns the lasem-0.4 folder.
> > > > $RPM_BUILD_ROOT
> > >
> > > %{buildroot}
> >
> > I prefer the variable format and as per current guidelines [1] both are
> > acceptable.
>
> hopefully it will be deprecated at some point. it's better to use
> %{buildroot}. but as you noted, it's up to you.

Let's stay with variable version then.
> > > also would be great to do: %global apiver 0.4 and use it in %files.
> >
> > OK
> >
> > New releases:
> > Spec URL: https://belegdol.fedorapeople.org/lasem/lasem.spec
> > SRPM URL: https://belegdol.fedorapeople.org/lasem/lasem-0.4.3-2.fc24.src.rpm
> >
> > [1]
> > https://fedoraproject.org/wiki/Packaging:Guidelines#Using_.25.7Bbuildroot.
> > 7D_and_.25.7Boptflags.7D_vs_.24RPM_BUILD_ROOT_and_.24RPM_OPT_FLAGS
>
> Resolution: ALMOST GOOD

New releases:
Spec URL: https://belegdol.fedorapeople.org/lasem/lasem.spec
SRPM URL: https://belegdol.fedorapeople.org/lasem/lasem-0.4.3-3.fc24.src.rpm

[2]
https://fedoraproject.org/wiki/Packaging:Guidelines#The_directory_is_owned_b...

[Bug 1352408] Review Request: lasem - A library for rendering SVG and Mathml, implementing a DOM like API

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1352408

--- Comment #5 from Igor Gnatenko  ---
> lasem.x86_64: W: shared-lib-calls-exit /usr/lib64/liblasem-0.4.so.4.0.3 exit@GLIBC_2.2.5

probably can be filled to upstream
> lasem.x86_64: E: incorrect-fsf-address /usr/share/licenses/lasem/COPYING

MUST be filled bug to upstream. Every file in src/ has incorrect FSF address
> I think I do - otherwise the directory ownership guideline [2] might be violated I think. lasem puts files in %{_datadir}/gtk-doc but does not depend on it, so it needs to own it. %doc only owns the lasem-0.4 folder.

I think with %doc it's not needed.

There is bundled itex2mml in sources, add following:
Licese: LGPLv2+ and GPLv2+
Provides: bundled(itex2mml) = 1.4.5
%license itex2mml/COPYING.itex2mml

[Bug 1352408] Review Request: lasem - A library for rendering SVG and Mathml, implementing a DOM like API

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1352408

--- Comment #6 from Julian Sikorski  ---
(In reply to Igor Gnatenko from comment #5)
> > lasem.x86_64: W: shared-lib-calls-exit /usr/lib64/liblasem-0.4.so.4.0.3 exit@GLIBC_2.2.5
>
> probably can be filled to upstream

https://bugzilla.gnome.org/show_bug.cgi?id=768591
> > lasem.x86_64: E: incorrect-fsf-address /usr/share/licenses/lasem/COPYING
>
> MUST be filled bug to upstream. Every file in src/ has incorrect FSF address

https://bugzilla.gnome.org/show_bug.cgi?id=768590
> > I think I do - otherwise the directory ownership guideline [2] might be violated I think. lasem puts files in %{_datadir}/gtk-doc but does not depend on it, so it needs to own it. %doc only owns the lasem-0.4 folder.
>
> I think with %doc it's not needed.

Why? %doc or no %doc, files are getting placed in a folder which could be left
dangling if package is uninstalled.
> There is bundled itex2mml in sources, add following:
> Licese: LGPLv2+ and GPLv2+

Not sure about this one... As per COPYING.itex2mml, itex2mml is tri-licensed:
GPL, MPL and LGPL, with no version specified. On the other hand, licensecheck
says it's GPLv3+
> Provides: bundled(itex2mml) = 1.4.5
> %license itex2mml/COPYING.itex2mml

Agreed

[Bug 1352408] Review Request: lasem - A library for rendering SVG and Mathml, implementing a DOM like API

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1352408

--- Comment #7 from Julian Sikorski  ---
New releases:
Spec URL: https://belegdol.fedorapeople.org/lasem/lasem.spec
SRPM URL: https://belegdol.fedorapeople.org/lasem/lasem-0.4.3-4.fc24.src.rpm

I added itex2mml elements. Is there anything else?

[Bug 1352408] Review Request: lasem - A library for rendering SVG and Mathml, implementing a DOM like API

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1352408

Igor Gnatenko  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |POST
              Flags|fedora-review?              |fedora-review+

--- Comment #8 from Igor Gnatenko  ---
* $RPM_BUILD_ROOT -> %{buildroot}

[Bug 1352408] Review Request: lasem - A library for rendering SVG and Mathml, implementing a DOM like API

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1352408

--- Comment #9 from Jon Ciesla  ---
Package request has been approved:
https://admin.fedoraproject.org/pkgdb/package/rpms/lasem

[Bug 1352408] Review Request: lasem - A library for rendering SVG and Mathml, implementing a DOM like API

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1352408

--- Comment #10 from Fedora Update System  ---
lasem-0.4.3-4.fc25 has been submitted as an update to Fedora 25.
https://bodhi.fedoraproject.org/updates/FEDORA-2016-0580653585

[Bug 1352408] Review Request: lasem - A library for rendering SVG and Mathml, implementing a DOM like API

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1352408

Fedora Update System  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|POST                        |MODIFIED

[Bug 1352408] Review Request: lasem - A library for rendering SVG and Mathml, implementing a DOM like API

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1352408

--- Comment #11 from Fedora Update System  ---
lasem-0.4.3-4.fc24 has been submitted as an update to Fedora 24.
https://bodhi.fedoraproject.org/updates/FEDORA-2016-406d38c5ef

[Bug 1352408] Review Request: lasem - A library for rendering SVG and Mathml, implementing a DOM like API

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1352408

--- Comment #12 from Fedora Update System  ---
lasem-0.4.3-4.fc23 has been submitted as an update to Fedora 23.
https://bodhi.fedoraproject.org/updates/FEDORA-2016-4060940fc7

[Bug 1352408] Review Request: lasem - A library for rendering SVG and Mathml, implementing a DOM like API

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1352408

Fedora Update System  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|MODIFIED                    |ON_QA

--- Comment #13 from Fedora Update System  ---
lasem-0.4.3-4.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-2016-0580653585

[Bug 1352408] Review Request: lasem - A library for rendering SVG and Mathml, implementing a DOM like API

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1352408

--- Comment #14 from Fedora Update System  ---
lasem-0.4.3-4.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-2016-406d38c5ef

[Bug 1352408] Review Request: lasem - A library for rendering SVG and Mathml, implementing a DOM like API

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1352408

--- Comment #15 from Fedora Update System  ---
lasem-0.4.3-4.fc23 has been pushed to the Fedora 23 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-2016-4060940fc7

[Bug 1352408] Review Request: lasem - A library for rendering SVG and Mathml, implementing a DOM like API

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1352408

--- Comment #16 from Fedora Update System  ---
lasem-0.4.3-4.fc25 has been pushed to the Fedora 25 stable repository. If
problems still persist, please make note of it in this bug report.

[Bug 1352408] Review Request: lasem - A library for rendering SVG and Mathml, implementing a DOM like API

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1352408

--- Comment #17 from Fedora Update System  ---
lasem-0.4.3-4.fc23 has been pushed to the Fedora 23 stable repository. If
problems still persist, please make note of it in this bug report.

[Bug 1352408] Review Request: lasem - A library for rendering SVG and Mathml, implementing a DOM like API

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1352408

Fedora Update System  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ON_QA                       |CLOSED
         Resolution|---                         |ERRATA
        Last Closed|                            |2016-09-09 12:53:59

[Bug 1352408] Review Request: lasem - A library for rendering SVG and Mathml, implementing a DOM like API

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1352408

--- Comment #18 from Fedora Update System  ---
lasem-0.4.3-4.fc24 has been pushed to the Fedora 24 stable repository. If
problems still persist, please make note of it in this bug report.