Bug 2245348

Copr build:
https://copr.fedorainfracloud.org/coprs/build/6552107
(failed)

Build log:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2245348-janet/srpm-builds/06552107/builder-live.log.gz

Please make sure the package builds successfully at least for Fedora Rawhide.

- If the build failed for unrelated reasons (e.g. temporary network
  unavailability), please ignore it.
- If the build failed because of missing BuildRequires, please make sure they
  are listed in the "Depends On" field


---
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.

[fedora-review-service-build]

This is an unofficial review as I am not accepted into the packager group yet.

> janet.x86_64: E: version-control-internal-file /usr/share/doc/janet/examples/numarray/.gitignore

This file should be removed.

> janet-static.x86_64: E: static-library-without-debuginfo /usr/lib64/libjanet.a

Static library does not contain debug information so please use --disable-static configure option or remove the file manually.

Thanks for the initial review. Updated.
spec: https://download.copr.fedorainfracloud.org/results/fed500/janet/fedora-rawhide-x86_64/06599510-janet/janet.spec
srpm: https://download.copr.fedorainfracloud.org/results/fed500/janet/fedora-rawhide-x86_64/06599510-janet/janet-1.32.1-1.fc40.src.rpm

Everything looks pretty nice and clean… Some questions and suggestions below.

It's nicer when %description is wrapped to <=80 columns (currently it's rather narrower).

> %package static
> Requires:  %{name}-devel%{?_isa} = %{version}-%{release}
I don't think the Requires is necessary. Nothing in the static library
uses files from -devel.

> %description static
The description should end in a dot.

> %{_libdir}/janet/janet.c
> %{_libdir}/libjanet.so.1.*
> %{_libdir}/libjanet.a
So the library part is delivered as a shared libary, a static library,
and an embeddable source file… Is anything except a shared library
really needed? How do you plan to use this? If it's as a dependency for
other fedora packages, then I think the effort should go into everybody
using the shared library.

$ ls -l /usr/bin/janet /usr/lib64/libjanet.so.1.32.1 
-rwxr-xr-x 1 root root 788360 Oct 20 02:00 /usr/bin/janet
-rwxr-xr-x 1 root root 811744 Oct 20 02:00 /usr/lib64/libjanet.so.1.32.1
Please link /usr/bin/janet to the shared library. This will shave off ~50% percent
of the package size.

Thanks.
Wrapped description.
Removed the static package.
Used dynamic linking to the library for the executable.

Have left the amalgamated C file, use case is different than
static library.  Can place it in a separate subpackage, though
it is a development file. Documentation on embedding:
https://janet-lang.org/capi/embedding.html

Koji build:
https://koji.fedoraproject.org/koji/taskinfo?taskID=108669568

spec: https://download.copr.fedorainfracloud.org/results/fed500/janet/fedora-rawhide-x86_64/06604151-janet/janet.spec
srpm: https://download.copr.fedorainfracloud.org/results/fed500/janet/fedora-rawhide-x86_64/06604151-janet/janet-1.32.1-1.fc40.src.rpm

$ ls -lL /usr/bin/janet /lib64/libjanet.so.1.32
-rwxr-xr-x 1 root root  32720 Nov  6 01:00 /usr/bin/janet
-rwxr-xr-x 1 root root 810696 Nov  6 01:00 /lib64/libjanet.so.1.32

That is a nice saving.

> Have left the amalgamated C file, use case is different than static library.
Sounds reasonable.

+ package name is OK
+ latest version
+ license is acceptable for Fedora (MIT)
+ license is specified correctly
+ builds and installs OK
+ P/R/BR are reasonable

Package is APPROVED.

Are the plans to package the whole ecosystem? https://janet-lang.org/ lists a bunch
of modules…

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

Thanks. Do not currently plan to package the whole ecosystem. There are
other packages would like to finish first. Can then come back to add
additional packages.

FEDORA-2023-fcae98bae2 has been submitted as an update to Fedora 39. https://bodhi.fedoraproject.org/updates/FEDORA-2023-fcae98bae2

FEDORA-2023-45d80bb730 has been submitted as an update to Fedora 38. https://bodhi.fedoraproject.org/updates/FEDORA-2023-45d80bb730

FEDORA-2023-2e50c0d37e has been submitted as an update to Fedora 37. https://bodhi.fedoraproject.org/updates/FEDORA-2023-2e50c0d37e

FEDORA-2023-45d80bb730 has been pushed to the Fedora 38 testing repository.
Soon you'll be able to install the update with the following command:
`sudo dnf install --enablerepo=updates-testing --refresh --advisory=FEDORA-2023-45d80bb730 \*`
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2023-45d80bb730

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

FEDORA-2023-fcae98bae2 has been pushed to the Fedora 39 testing repository.
Soon you'll be able to install the update with the following command:
`sudo dnf install --enablerepo=updates-testing --refresh --advisory=FEDORA-2023-fcae98bae2 \*`
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2023-fcae98bae2

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

FEDORA-2023-2e50c0d37e has been pushed to the Fedora 37 testing repository.
Soon you'll be able to install the update with the following command:
`sudo dnf install --enablerepo=updates-testing --refresh --advisory=FEDORA-2023-2e50c0d37e \*`
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2023-2e50c0d37e

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

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

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

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