https://bugzilla.redhat.com/show_bug.cgi?id=2314358 --- Comment #8 from Jens Petersen <petersen@xxxxxxxxxx> --- Thank you for taking this review :-) I hadn't forgotten about it... Sorry for the very long delay: I wanted to respond in full, but lacked time. TBH this package was one of the hardest to create in my long packaging history. Part of the problem is that upstream doesn't really care that much about distro packaging. Nevertheless v2 is fairly mature by now, and I feel including it in Fedora is overdue, since v1 is long deprecated. I guess what I wanted to say is I don't know how much further the packaging can be improved easily... (For better or worse it was fairly trivial to get v1 to still build with ghc 9.8 in rawhide: the old Haskell codebase seems surprisingly robust against aging.) (In reply to Zbigniew Jędrzejewski-Szmek from comment #7) > 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 Okay > Suggestion 2: rpmautospec :) Sure - I typically do this when importing. :) > 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. Sure > 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. Probably I think so - I would have to ask upstream or check the code. I might have asked in the past. > idris2-lib.x86_64: W: no-soname /usr/lib64/libidris2_support.so > This seems to be an upstream problem. What is the intended use of the > library? Good question - I think it is needed for the runtime. I can ask upstream or try to dig up old discussions from when I did the initial packaging work. > idris2-docs.x86_64: E: no-binary > Please make the subpackage noarch. Okay - I am renaming it to -doc too. > 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 Yeah: slightly surprising > 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}) Okay will try to look into it more. > 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. I/we should probably clarify what we mean by "runtime". (In other part of this comment by "runtime" I meant not compiler runtime but for running output of the compiler, ie RTS.) Yes, I think they are needed at compiler runtime for the C backend. Though I can check with upstream I guess. > > # no chez-scheme for s390x So actually we have chez-scheme for s390x now in the meantime, though I haven't tried to build with it yet. (Also there is a new chez-scheme minor release out, so I will try to build that soon in rawhide) > > # 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. Okay Also ppc64le and ix86 are currently disabled. Unfortunately upstream only supports ppc64le via vm bytecodes. > 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.) Okay moving it. The idea is that the lib subpackage would be used for runtime support: not compilation runtime, but running generated binaries. > + 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 standardized on -doc spelling. The package is currently called -docs. > Please rename. Sure > + %doc files are not used at runtime > - static files in -static subpackage > See above. > - development files in -devel subpackage > Also see above. Perhaps some could be split out - not very sure. But initially might be easier just to keep things together perhaps. If we consider idris2 a compiler then the main distinction is really compile-time vs runtime. > + .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 Well you can try some basic things from here: https://idris2.readthedocs.io/en/latest/tutorial/starting.html#checking-installation > + 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. Will try to follow up with any info I can find. -- You are receiving this mail because: You are always notified about changes to this product and component You are on the CC list for the bug. https://bugzilla.redhat.com/show_bug.cgi?id=2314358 Report this comment as SPAM: https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=report-spam&short_desc=Report%20of%20Bug%202314358%23c8 -- _______________________________________________ package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@xxxxxxxxxxxxxxxxxxxxxxx Do not reply to spam, report it: https://pagure.io/fedora-infrastructure/new_issue
