Bug 2314358 - Review Request: idris2 - Purely functional programming language with first class types
Summary: Review Request: idris2 - Purely functional programming language with first cl...
Keywords:
Status: ASSIGNED
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Zbigniew Jędrzejewski-Szmek
QA Contact: Fedora Extras Quality Assurance
URL: https://www.idris-lang.org
Whiteboard:
Depends On:
Blocks: 2346153
TreeView+ depends on / blocked
 
Reported: 2024-09-24 02:43 UTC by Jens Petersen
Modified: 2025-02-18 08:52 UTC (History)
2 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed:
Type: ---
Embargoed:


Attachments (Terms of Use)
The .spec file difference from Copr build 8064021 to 8545411 (1.06 KB, patch)
2025-01-19 11:11 UTC, Fedora Review Service
no flags Details | Diff

Description Jens Petersen 2024-09-24 02:43:29 UTC
Spec URL: https://petersen.fedorapeople.org/reviews/idris2/idris2.spec
SRPM URL: https://petersen.fedorapeople.org/reviews/idris2/idris2-0.7.0-1.fc42.src.rpm

Description:
Idris is a programming language designed to encourage Type-Driven Development.

Fedora Account System Username: petersen


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

Comment 1 Fedora Review Service 2024-09-24 03:01:45 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/8064021
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2314358-idris2/fedora-rawhide-x86_64/08064021-idris2/fedora-review/review.txt

Found issues:

- idris2 : /usr/lib64/idris2-0.7.0/support/c/getline.h idris2 : /usr/lib64/idris2-0.7.0/support/c/idris_directory.h idris2 : /usr/lib64/idris2-0.7.0/support/c/idris_file.h idris2 : /usr/lib64/idris2-0.7.0/support/c/idris_memory.h idris2 : /usr/lib64/idris2-0.7.0/support/c/idris_net.h idris2 : /usr/lib64/idris2-0.7.0/support/c/idris_signal.h idris2 : /usr/lib64/idris2-0.7.0/support/c/idris_support.h idris2 : /usr/lib64/idris2-0.7.0/support/c/idris_system.h idris2 : /usr/lib64/idris2-0.7.0/support/c/idris_term.h idris2 : /usr/lib64/idris2-0.7.0/support/c/idris_util.h idris2 : /usr/lib64/idris2-0.7.0/support/refc/_datatypes.h idris2 : /usr/lib64/idris2-0.7.0/support/refc/buffer.h idris2 : /usr/lib64/idris2-0.7.0/support/refc/cBackend.h idris2 : /usr/lib64/idris2-0.7.0/support/refc/casts.h idris2 : /usr/lib64/idris2-0.7.0/support/refc/clock.h idris2 : /usr/lib64/idris2-0.7.0/support/refc/conCaseHelper.h idris2 : /usr/lib64/idris2-0.7.0/support/refc/mathFunctions.h idris2 : /usr/lib64/idris2-0.7.0/support/refc/memoryManagement.h idris2 : /usr/lib64/idris2-0.7.0/support/refc/prim.h idris2 : /usr/lib64/idris2-0.7.0/support/refc/refc_util.h idris2 : /usr/lib64/idris2-0.7.0/support/refc/runtime.h idris2 : /usr/lib64/idris2-0.7.0/support/refc/stringOps.h idris2 : /usr/lib64/idris2-0.7.0/support/refc/threads.h idris2-docs : /usr/share/doc/idris2-docs/samples/FFI-readline/readline_glue/idris_readline.h 
  Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_devel_packages
- Unversioned so-files directly in %_libdir.
  Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_devel_packages
- Package has .a files: idris2. Illegal package name: idris2. Does not provide -static: idris2.
  Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/#packaging-static-libraries
- Documentation size is 3032712 bytes in 188 files. 
  Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_documentation

Please know that there can be false-positives.

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

Comment 2 Zbigniew Jędrzejewski-Szmek 2025-01-16 11:27:37 UTC
Please don't use the confusingly-named %bconf_with/without.

> # always bootstrap: otherwise rebuild fails
> %bcond_without boot
%bcond bootstrap 1

> %bcond_without docs
%bcond docs 1

> # requires network?
> %bcond_with test
%bcond test 0

> # no chez-scheme for s390x
> # ppc64le and i686 give linking error:
> # - Exception: (while loading libc.so) /lib/libc.so: invalid ELF header
> %ifarch ppc64le s390x %{ix86}
> %bcond_without racket
> %else
> %bcond_with racket
> %endif
Hmm. Is the with/without reversed?
https://github.com/rpm-software-management/rpm/issues/3165
is still unresolved. So I guess something like this is the best we
can do right now:

%ifarch ppc64le s390x %{ix86}
%bcond racket 0
%else
%bcond racket 1
%endif

Comment 3 Jens Petersen 2025-01-19 09:50:58 UTC
Thanks, Zbigniew

Comment 5 Fedora Review Service 2025-01-19 11:11:45 UTC
Created attachment 2066676 [details]
The .spec file difference from Copr build 8064021 to 8545411

Comment 6 Fedora Review Service 2025-01-19 11:11:47 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/8545411
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2314358-idris2/fedora-rawhide-x86_64/08545411-idris2/fedora-review/review.txt

Found issues:

- idris2 : /usr/lib64/idris2-0.7.0/support/c/getline.h idris2 : /usr/lib64/idris2-0.7.0/support/c/idris_directory.h idris2 : /usr/lib64/idris2-0.7.0/support/c/idris_file.h idris2 : /usr/lib64/idris2-0.7.0/support/c/idris_memory.h idris2 : /usr/lib64/idris2-0.7.0/support/c/idris_net.h idris2 : /usr/lib64/idris2-0.7.0/support/c/idris_signal.h idris2 : /usr/lib64/idris2-0.7.0/support/c/idris_support.h idris2 : /usr/lib64/idris2-0.7.0/support/c/idris_system.h idris2 : /usr/lib64/idris2-0.7.0/support/c/idris_term.h idris2 : /usr/lib64/idris2-0.7.0/support/c/idris_util.h idris2 : /usr/lib64/idris2-0.7.0/support/refc/_datatypes.h idris2 : /usr/lib64/idris2-0.7.0/support/refc/buffer.h idris2 : /usr/lib64/idris2-0.7.0/support/refc/cBackend.h idris2 : /usr/lib64/idris2-0.7.0/support/refc/casts.h idris2 : /usr/lib64/idris2-0.7.0/support/refc/clock.h idris2 : /usr/lib64/idris2-0.7.0/support/refc/conCaseHelper.h idris2 : /usr/lib64/idris2-0.7.0/support/refc/mathFunctions.h idris2 : /usr/lib64/idris2-0.7.0/support/refc/memoryManagement.h idris2 : /usr/lib64/idris2-0.7.0/support/refc/prim.h idris2 : /usr/lib64/idris2-0.7.0/support/refc/refc_util.h idris2 : /usr/lib64/idris2-0.7.0/support/refc/runtime.h idris2 : /usr/lib64/idris2-0.7.0/support/refc/stringOps.h idris2 : /usr/lib64/idris2-0.7.0/support/refc/threads.h idris2-docs : /usr/share/doc/idris2-docs/samples/FFI-readline/readline_glue/idris_readline.h 
  Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_devel_packages
- Unversioned so-files directly in %_libdir.
  Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_devel_packages
- Package has .a files: idris2. Illegal package name: idris2. Does not provide -static: idris2.
  Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/#packaging-static-libraries
- Documentation size is 3035172 bytes in 189 files. 
  Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_documentation

Please know that there can be false-positives.

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

Comment 7 Zbigniew Jędrzejewski-Szmek 2025-01-20 09:18:11 UTC
Suggestion 1:
> https://github.com/idris-lang/Idris2/archive/refs/tags/v0.7.0.tar.gz#/%{name}-%{version}.tar.gz
Source0:        https://github.com/idris-lang/Idris2/archive/v%{version_no_tilde}/%{name}-%{version_no_tilde}.tar.gz

Suggestion 2: rpmautospec :)

rpmlint:
idris2-docs.x86_64: E: version-control-internal-file /usr/share/doc/idris2-docs/samples/FFI-readline/readline_glue/.gitignore
idris2-docs.x86_64: W: hidden-file-or-dir /usr/share/doc/idris2-docs/html/.buildinfo
idris2-docs.x86_64: W: hidden-file-or-dir /usr/share/doc/idris2-docs/samples/FFI-readline/readline_glue/.gitignore
Please drop.

idris2-lib.x86_64: W: summary-not-capitalized idris2 runtime support library
False positive.

idris2.x86_64: E: static-library-without-debuginfo /usr/lib64/idris2-0.7.0/support/refc/libidris2_refc.a
Hmm, is this needed? If yes, please add a comment in the spec file.

idris2-lib.x86_64: W: no-soname /usr/lib64/libidris2_support.so
This seems to be an upstream problem. What is the indended use of the library?

idris2.x86_64: W: no-manual-page-for-binary idris2
Unfortunately that is common.

idris2-lib.x86_64: W: no-documentation
This is OK.

idris2-docs.x86_64: E: no-binary
Please make the subpackage noarch.

idris2.x86_64: W: files-duplicate /usr/lib64/idris2-0.7.0/linear-0.7.0/2023090800/Data/Linear/List/LQuantifiers.so /usr/lib64/idris2-0.7.0/contrib-0.7.0/2023090800/Data/Order.so
idris2-docs.x86_64: W: files-duplicate /usr/share/doc/idris2-docs/samples/ffi/dummy.ipkg /usr/share/doc/idris2-docs/samples/dummy.ipkg
Those are all tiny, so this doesn't matter. I guess hardlinking could be done at the end of %install. This would have the benefit of suppressing the warning from rpmlint.
(BuildRequires: hardlink; hardlink --reflink=never -v %{buildroot}%{_usr})

idris2.x86_64: W: devel-file-in-non-devel-package /usr/lib64/idris2-0.7.0/support/c/getline.h
idris2.x86_64: W: devel-file-in-non-devel-package /usr/lib64/idris2-0.7.0/support/c/idris_directory.h
idris2.x86_64: W: devel-file-in-non-devel-package /usr/lib64/idris2-0.7.0/support/c/idris_file.h
idris2.x86_64: W: devel-file-in-non-devel-package /usr/lib64/idris2-0.7.0/support/c/idris_memory.h
idris2.x86_64: W: devel-file-in-non-devel-package /usr/lib64/idris2-0.7.0/support/c/idris_net.h
idris2.x86_64: W: devel-file-in-non-devel-package /usr/lib64/idris2-0.7.0/support/c/idris_signal.h
idris2.x86_64: W: devel-file-in-non-devel-package /usr/lib64/idris2-0.7.0/support/c/idris_support.h
idris2.x86_64: W: devel-file-in-non-devel-package /usr/lib64/idris2-0.7.0/support/c/idris_system.h
idris2.x86_64: W: devel-file-in-non-devel-package /usr/lib64/idris2-0.7.0/support/c/idris_term.h
idris2.x86_64: W: devel-file-in-non-devel-package /usr/lib64/idris2-0.7.0/support/c/idris_util.h
idris2.x86_64: W: devel-file-in-non-devel-package /usr/lib64/idris2-0.7.0/support/refc/_datatypes.h
idris2.x86_64: W: devel-file-in-non-devel-package /usr/lib64/idris2-0.7.0/support/refc/buffer.h
idris2.x86_64: W: devel-file-in-non-devel-package /usr/lib64/idris2-0.7.0/support/refc/cBackend.h
idris2.x86_64: W: devel-file-in-non-devel-package /usr/lib64/idris2-0.7.0/support/refc/casts.h
idris2.x86_64: W: devel-file-in-non-devel-package /usr/lib64/idris2-0.7.0/support/refc/clock.h
idris2.x86_64: W: devel-file-in-non-devel-package /usr/lib64/idris2-0.7.0/support/refc/conCaseHelper.h
idris2.x86_64: W: devel-file-in-non-devel-package /usr/lib64/idris2-0.7.0/support/refc/libidris2_refc.a
idris2.x86_64: W: devel-file-in-non-devel-package /usr/lib64/idris2-0.7.0/support/refc/mathFunctions.h
idris2.x86_64: W: devel-file-in-non-devel-package /usr/lib64/idris2-0.7.0/support/refc/memoryManagement.h
idris2.x86_64: W: devel-file-in-non-devel-package /usr/lib64/idris2-0.7.0/support/refc/prim.h
idris2.x86_64: W: devel-file-in-non-devel-package /usr/lib64/idris2-0.7.0/support/refc/refc_util.h
idris2.x86_64: W: devel-file-in-non-devel-package /usr/lib64/idris2-0.7.0/support/refc/runtime.h
idris2.x86_64: W: devel-file-in-non-devel-package /usr/lib64/idris2-0.7.0/support/refc/stringOps.h
idris2.x86_64: W: devel-file-in-non-devel-package /usr/lib64/idris2-0.7.0/support/refc/threads.h
Are those needed at runtime? If yes, please add a comment.

============ 6 packages and 0 specfiles checked; 3 errors, 32 warnings, 27 filtered, 3 badness; has taken 1.8 s ============


> # no chez-scheme for s390x
> # ppc64le and i686 give linking error:
> # - Exception: (while loading libc.so) /lib/libc.so: invalid ELF header
> %ifarch ppc64le s390x %{ix86}
> %bcond racket 1
So the %bcond_without _was_ correct. But I was confused by this comment. It'd be good to expand it a bit so that a casual reader is not confused.

This package is fairly complex and unusual, so I'll go by the official list in Things To Check On Review:
- rpmlint: some things to fix
+ package name is OK
+ spec file name is OK
+ license is acceptable for Fedora (BSD-3-Clause)
+ license is specified correctly
- license file is installed
  The license file should be in -lib, so that it is always installed.
  (Or alternatively, -lib subpackage should be merged into the main subpackage. I'm not sure if the library is useful on its own.)
+ spec file is in English and is legible
+ Source URL is OK, but see suggestion above
+ package builds OK
+ BR/R/P look OK
+ locales don't seem to be supported
+ files seem to be listed correctly
+ macros are used as appropriate
+ package contains code
- -doc subpackage has been split out
We've standarized on -doc spelling. The package is currently called -docs. Please rename.
+ %doc files are not used at runtime
- static files in -static subpackage
See above.
- development files in -devel subpackage
Also see above.
+ .desktop file for GUI applications
Not applicable.
+ directory ownership looks OK
+ file names as all ASCII
+ no deprecated packages are referenced
+ license text already included
+ the package seems be build fine in mock
+ the package has provisions to build on all architectures
+ the binary runs, I have no idea how to make it do useful things
+ no scriptlets are needed
+ a versioned dependency is defined from the main package to -lib
+ no pkgconfig file is present
+ no file dependencies are defined
+ man pages are missing, but that is not required

Some things to fix. Please explain why the .a and .h and .so files are handled as they are.


Note You need to log in before you can comment on or make changes to this bug.