Spec URL: https://github.com/fedora-sail/lem/blob/master/lem.spec SRPM URL: https://github.com/fedora-sail/lem Description: Lem is a tool for lightweight executable mathematics, for writing, managing, and publishing large-scale portable semantic definitions, with export to LaTeX, executable code (currently OCaml) and interactive theorem provers (currently Coq, HOL4, and Isabelle/HOL, though the generated Coq is not necessarily idiomatic). It is also intended as an intermediate language for generating definitions from domain-specific tools, and for porting definitions between interactive theorem proving systems. Fedora Account System Username: U2FsdGVkX1
Cannot find any valid SRPM URL for this ticket. Common causes are: - You didn't specify `SRPM URL: ...` in the ticket description or any of your comments - The URL schema isn't HTTP or HTTPS - The SRPM package linked in your URL doesn't match the package name specified in the ticket summary --- 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.
Spec URL: https://raw.githubusercontent.com/fedora-sail/lem/master/lem.spec SRPM URL: https://github.com/fedora-sail/lem/raw/master/lem-2022.12.10-1.fc41.src.rpm
Copr build: https://copr.fedorainfracloud.org/coprs/build/7639462 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2291284-lem/fedora-rawhide-x86_64/07639462-lem/fedora-review/review.txt Found issues: - No gcc, gcc-c++ or clang found in BuildRequires Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/C_and_C++/ - Package has .a files: lem-devel. Does not provide -static: lem-devel. Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/#packaging-static-libraries 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.
The warning from fedora-review-service can be safely ignored. 1. Upstream has released the new 2025 version. Please update accordingly. 2. Non-readable files in /usr/share/lem/ were detected. The file mode 600 is unusual and should be verified. (rpmlint) 3. Duplicates of files have been found. Please assess if these duplicate files are indeed required. (rpmlint) 4. According to the packaging guidelines for OCaml (https://docs.fedoraproject.org/en-US/packaging-guidelines/OCaml), files with extensions *.a, *.cmxa, *.cmx, and *.mli must be excluded from the main package and moved into the -devel package. 5. Your description lines should not exceed 80 characters. (rpmlint) If a line is exceeding this number, cut it to fit in two lines.
(In reply to Nianqing Yao from comment #4) > The warning from fedora-review-service can be safely ignored. > > 1. Upstream has released the new 2025 version. Please update accordingly. > > 2. Non-readable files in /usr/share/lem/ were detected. The file mode > 600 is unusual and should be verified. (rpmlint) > > 3. Duplicates of files have been found. Please assess if these duplicate > files are indeed required. (rpmlint) > > 4. According to the packaging guidelines for OCaml > (https://docs.fedoraproject.org/en-US/packaging-guidelines/OCaml), > files with extensions *.a, *.cmxa, *.cmx, and *.mli must be excluded > from the main package and moved into the -devel package. > > 5. Your description lines should not exceed 80 characters. (rpmlint) > If a line is exceeding this number, cut it to fit in two lines. 1. I have updated to the new version. 2. I have fixed this issue. 3. This is expected, or rather, these are prepared for different backends. 4. I have fixed this issue. 5. I have fixed this issue. Thank you for your review! If possible, could you please check the new version again?
Ok, let's call a rebuild. Spec URL: https://github.com/fedora-sail/lem/raw/refs/heads/master/lem.spec SRPM URL: https://github.com/fedora-sail/lem/raw/refs/heads/master/lem-2025.03.13-1.fc43.src.rpm
Eh, let's try again. [fedora-review-service-build]
Created attachment 2088076 [details] The .spec file difference from Copr build 7639462 to 8988511
Copr build: https://copr.fedorainfracloud.org/coprs/build/8988511 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2291284-lem/fedora-rawhide-x86_64/08988511-lem/fedora-review/review.txt Found issues: - No gcc, gcc-c++ or clang found in BuildRequires Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/C_and_C++/ - Package has .a files: lem-devel. Does not provide -static: lem-devel. Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/#packaging-static-libraries 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.
LGTM
Hello @U2FsdGVkX1, since this is your first Fedora package, you need to get sponsored by a package sponsor before it can be accepted. A sponsor is an experienced package maintainer who will guide you through the processes that you will follow and the tools that you will use as a future maintainer. A sponsor will also be there to answer your questions related to packaging. You can find all active sponsors here: https://docs.pagure.org/fedora-sponsors/ I created a sponsorship request for you: https://pagure.io/packager-sponsors/issue/717 Please take a look and make sure the information is correct. Thank you, and best of luck on your packaging journey. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service
I'm supposed to be Songsong's sponsor.
The Pagure repository was created at https://src.fedoraproject.org/rpms/lem
For the purpose of educating both the submitter and the reviewer, here are a few things to look for in future reviews. It is hard to convey tone in electronic communications, so I want to stress that I am attempting to educate, not criticize. First, "%global debug_package %{nil}" is both unnecessary and wrong. See https://docs.fedoraproject.org/en-US/packaging-guidelines/Debuginfo/. That line can and should be removed, since the -g flag is passed to both ocamlc and ocamlopt during the build. Second, the spec contains "BuildRequires: ocaml-rpm-macros", but none of the RPM macros defined in that package are actually used. If there is reason not to use them, then this dependency should be removed. Third, the use of %attr in %files seems dodgy. Why do the files have wrong permissions in the first place? It appears that the lem tool itself is setting the incorrect permission bits. If I remove the built files in the isabelle-lib directory and run this command: ../lem -isa -outdir ../isabelle-lib -wl ign -wl_auto_import err -wl_rename err bool.lem basic_classes.lem function.lem maybe.lem num.lem tuple.lem list.lem either.lem set_helpers.lem set.lem map.lem relation.lem sorting.lem function_extra.lem assert_extra.lem list_extra.lem string.lem num_extra.lem map_extra.lem set_extra.lem maybe_extra.lem string_extra.lem word.lem show.lem show_extra.lem machine_word.lem pervasives.lem pervasives_extra.lem debug.lem -auxiliary_level none -only_changed_output then the files are generated with the incorrect permissions. In src/process_file.ml, lines 90-92 read: let open_output_with_check dir file_name = let (temp_file_name, o) = Filename.open_temp_file "lem_temp" "" in (o, (o, temp_file_name, Filename.concat dir file_name)) and the documentation of open_temp_file (https://ocaml.org/manual/5.3/api/Filename.html) says: "The file is created with permissions perms (defaults to readable and writable only by the file owner, 0o600)." So this could be fixed by passing "~perms:0o666" to open_temp_file. This should probably be discussed with lem upstream. Finally, I see several instances of this warning in the build log: Alert ocaml_deprecated_auto_include: OCaml's lib directory layout changed in 5.0. The str subdirectory has been automatically added to the search path, but you should add -I +str to the command-line to silence this alert (e.g. by adding str to the list of libraries in your dune file, or adding use_str to your _tags file for ocamlbuild, or using -package str for ocamlfind). Please tell upstream that "use_str" needs to be added to src/_tags or the package will stop building in some future OCaml release. I missed this package when rebuilding all Fedora OCaml packages this weekend, so I will take care of #1 and #2 when I rebuild.
(In reply to Jerry James from comment #14) > For the purpose of educating both the submitter and the reviewer, here are a > few things to look for in future reviews. It is hard to convey tone in > electronic communications, so I want to stress that I am attempting to > educate, not criticize. > > First, "%global debug_package %{nil}" is both unnecessary and wrong. See > https://docs.fedoraproject.org/en-US/packaging-guidelines/Debuginfo/. That > line can and should be removed, since the -g flag is passed to both ocamlc > and ocamlopt during the build. > > Second, the spec contains "BuildRequires: ocaml-rpm-macros", but none of the > RPM macros defined in that package are actually used. If there is reason > not to use them, then this dependency should be removed. > > Third, the use of %attr in %files seems dodgy. Why do the files have wrong > permissions in the first place? It appears that the lem tool itself is > setting the incorrect permission bits. If I remove the built files in the > isabelle-lib directory and run this command: > > ../lem -isa -outdir ../isabelle-lib -wl ign -wl_auto_import err -wl_rename > err bool.lem basic_classes.lem function.lem maybe.lem num.lem tuple.lem > list.lem either.lem set_helpers.lem set.lem map.lem relation.lem sorting.lem > function_extra.lem assert_extra.lem list_extra.lem string.lem num_extra.lem > map_extra.lem set_extra.lem maybe_extra.lem string_extra.lem word.lem > show.lem show_extra.lem machine_word.lem pervasives.lem pervasives_extra.lem > debug.lem -auxiliary_level none -only_changed_output > > then the files are generated with the incorrect permissions. In > src/process_file.ml, lines 90-92 read: > > let open_output_with_check dir file_name = > let (temp_file_name, o) = Filename.open_temp_file "lem_temp" "" in > (o, (o, temp_file_name, Filename.concat dir file_name)) > > and the documentation of open_temp_file > (https://ocaml.org/manual/5.3/api/Filename.html) says: "The file is created > with permissions perms (defaults to readable and writable only by the file > owner, 0o600)." So this could be fixed by passing "~perms:0o666" to > open_temp_file. This should probably be discussed with lem upstream. > > Finally, I see several instances of this warning in the build log: > > Alert ocaml_deprecated_auto_include: > OCaml's lib directory layout changed in 5.0. The str subdirectory has been > automatically added to the search path, but you should add -I +str to the > command-line to silence this alert (e.g. by adding str to the list of > libraries in your dune file, or adding use_str to your _tags file for > ocamlbuild, or using -package str for ocamlfind). > > Please tell upstream that "use_str" needs to be added to src/_tags or the > package will stop building in some future OCaml release. > > I missed this package when rebuilding all Fedora OCaml packages this > weekend, so I will take care of #1 and #2 when I rebuild. Thank you for your guidance, and apologies for the late reply. I’ll look into how to address these issues.