https://bugzilla.redhat.com/show_bug.cgi?id=2217273 Robert Scheck <redhat-bugzilla@xxxxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |redhat-bugzilla@linuxnetz.d | |e --- Comment #1 from Robert Scheck <redhat-bugzilla@xxxxxxxxxxxx> --- I am sorry for being slow. These are my thoughts while reading the spec file: You can avoid "%global luaver 5.4" by using the more modern "%{lua_version}" instead. See also: https://fedoraproject.org/wiki/PackagingDrafts/Lua You can avoid "%global lualibdir %{_libdir}/lua/%{luaver}" by using the more modern "%{lua_libdir}" instead. See also: https://fedoraproject.org/wiki/PackagingDrafts/Lua As "%{luapkgdir}" isn't used, you could simply remove "%global luapkgdir %{_datadir}/lua/%{luaver}". You could use "https://github.com/lualdap/lualdap/archive/v%{version}/lualdap-%{version}.tar.gz"; instead of "https://github.com/lualdap/lualdap/archive/refs/tags/v1.3.1.tar.gz";, which creates a more nice tarball name and doesn't hardcode the version. See also: https://docs.fedoraproject.org/en-US/packaging-guidelines/SourceURL/ I would recommend "BuildRequires: lua-devel >= %{lua_version}" instead of only "BuildRequires: lua-devel". You should add "BuildRequires: make" when using "make" in "%build" etc. In Fedora, "make" is no longer part of the default buildroot, see also: https://fedoraproject.org/wiki/Changes/Remove_make_from_BuildRoot Using "%make_build" instead of "make %{?_smp_mflags}" is recommented. See also: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_parallel_make Using "%make_install" instead of "make install DESTDIR=%{buildroot}" is recommented. See also https://docs.fedoraproject.org/en-US/packaging-guidelines/#_why_the_makeinstall_macro_should_not_be_used Rather than changing the 'config' file during "%setup", you could also simply append 'CFLAGS="%{optflags}" LDFLAGS="%{?__global_ldflags} -fPIC" LUA_LIBDIR=%{_libdir} LUA_INCDIR=%{_includedir}' to "%make_build" (note I that I used "%{optflags}" instead of "$RPM_OPT_FLAGS", because you used the macro style instead of environment variables at other places, too). And appending 'INST_LIBDIR=%{lua_libdir}' to "%make_install" might ease things as well. See also: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_using_buildroot_and_optflags_vs_rpm_build_root_and_rpm_opt_flags While it is not necessary, a "%check" section could help to ensure that the Lua module can be actually loaded by the Lua interpreter (sometimes Lua modules break during a Lua update and isn't catched by a simple rebuild, but during run-time). My suggestion here is: --- 8< --- %check lua -e \ 'package.cpath="%{buildroot}%{lua_libdir}/?.so;"..package.cpath; local lualdap = require("lualdap"); print("Hello from "..lualdap._VERSION.."!");' --- 8< --- However, this requires additionally (and latter only if you target RHEL < 9): --- 8< --- BuildRequires: lua >= %{lua_version} %if 0%{?rhel} && 0%{?rhel} < 9 Requires: lua(abi) = %{lua_version} %endif --- 8< --- And if you target RHEL 7 as well, you need also this: --- 8< --- %if 0%{?rhel} == 7 BuildRequires: lua-rpm-macros %endif --- 8< --- You could add "%license docs/license.md" to "%files". I would tune "%docs" to "%doc README.md docs/[cmn]*.md" to avoid picking up the logo (which is unused/unreferenced inside the docs), while the "index.md" is more or less similar to "README.md". Just let me know if you have any questions. -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component https://bugzilla.redhat.com/show_bug.cgi?id=2217273 Report this comment as SPAM: https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=report-spam&short_desc=Report%20of%20Bug%202217273%23c1 _______________________________________________ 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
