Bug 2224783

Copr build:
https://copr.fedorainfracloud.org/coprs/build/6204266
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2224783-dragonbox/fedora-rawhide-x86_64/06204266-dragonbox/fedora-review/review.txt

Please take a look if any issues were found.

---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.

Taking this.

I only see an issue: static library should be avoided as possible. The remaining things LGTM.

(In reply to Felix Wang from comment #3)
> I only see an issue: static library should be avoided as possible. The
> remaining things LGTM.

This is a header only library, so adheres to https://docs.fedoraproject.org/en-US/packaging-guidelines/#packaging-static-libraries

BTW, if you're willing to make a formal review, you should set the ticket status to ASSIGNED and set yourself as the assignee.
Thanks.

So, do I need to adjust anything, or can the package be approved?

Hmm, so what is in the .a file? If it is not needed for anything, maybe drop it and make the -devel
package noarch?


> BTW, if you're willing to make a formal review, you should set the ticket status to ASSIGNED and set yourself as the assignee.

I see that Felix didn't actually assign this to himself, and it was assigned by Petr instead.
Felix, do you still want to continue the review?

(In reply to Zbigniew Jędrzejewski-Szmek from comment #6)
> Hmm, so what is in the .a file? If it is not needed for anything, maybe drop
> it and make the -devel
> package noarch?
> 

I'm not really expert of C, but here are an excerpt from the README:
"""
Although Dragonbox is intended for float-to-string conversion routines, the actual string generation is not officially a part of the algorithm. Dragonbox just outputs two integers (the decimal significand/exponent) that can be consumed by a string generation procedure. The header file include/dragonbox/dragonbox.h includes everything needed for this (it is header-only). Nevertheless, a string generation procedure is included in the library. There are two additional files needed for that: include/dragonbox/dragonbox_to_chars.h and source/dragonbox_to_chars.cpp.
"""

If you think it isn't worth to distribute the .a file, the instructions say I can use '-DDRAGONBOX_INSTALL_TO_CHARS=OFF' to disable building the library. I'll have to check if LibreOffice build just requires the headers, though, since this is the only reason to package dragonbox.

> I see that Felix didn't actually assign this to himself, and it was assigned by Petr instead.
> Felix, do you still want to continue the review?

Sorry for the late response. I have been very busy with my graduation thesis this month and do not have time and energy to spare on this, so I withdraw from the package review assignment.

Spec URL: https://mattia.fedorapeople.org/dragonbox/dragonbox.spec
SRPM URL: https://mattia.fedorapeople.org/dragonbox/dragonbox-1.1.3-1.fc40.src.rpm

So, LibreOffice only needs the headers, therefore I've disabled the build of the static library and made the package noarch.
Note that Fedora Packaging Guidelines says that only the subpackage should be noarch, while the main package must be arch'ed so that tests are run. But this package has no tests, so I see no point in wasting resources to build it multiple times on all arches... I dropped a note in the specfile, if in future tests are added (unlikely) I will move the noarch to the subpackage only.

With the latest changes, the review is trivial:

+ package name is OK
+ latest version
+ license is acceptable for Fedora (Apache-2.0 and change)
- license is specified correctly
  (see below)
+ builds and installs OK
+ BR/R/P look reasonable

> License: Apache-2.0 WITH LLVM-exception AND BSL-1.0

Shouldn't this be "OR" instead? Both files that are in the binary package
have the same header that says "Alternatively, the contents of this file may be used under
the terms of the BSL".

Please add a comment how the license is derived.

I don't think this makes any difference in practice, so the
package is APPROVED.

(In reply to Zbigniew Jędrzejewski-Szmek from comment #10)
> 
> Shouldn't this be "OR" instead? 

Yeah, right. I'll change upon import.
Thanks.

The Pagure repository was created at https://src.fedoraproject.org/rpms/dragonbox

FEDORA-2023-8d303b9ee8 has been submitted as an update to Fedora 40. https://bodhi.fedoraproject.org/updates/FEDORA-2023-8d303b9ee8

FEDORA-2023-8d303b9ee8 has been pushed to the Fedora 40 stable repository.
If problem still persists, please make note of it in this bug report.