Spec URL: https://xor.cryptomilk.org/rpm/lua-luv/lua-luv.spec SRPM URL: https://xor.cryptomilk.org/rpm/lua-luv/lua-luv-1.30.1-1.fc30.src.rpm Description: Lua bindings for libluv Fedora Account System Username: asn
This is required for neovim >= 0.4.0 Aron, could you please review the package?
You should not use compat- in your naming of subpackages for old versions of Lua, so it's probably best not to name your variables with compat either. This also doesn't build, but the output is rather weird, so I'm not sure if it's a bug in Rawhide at the moment: -- The C compiler identification is unknown -- The ASM compiler identification is unknown -- Didn't find assembler CMake Error at CMakeLists.txt:7 (project): No CMAKE_C_COMPILER could be found. Tell CMake where to find the compiler by setting either the environment variable "CC" or the CMake cache entry CMAKE_C_COMPILER to the full path to the compiler, or to the compiler name if it is in the PATH. CMake Error at CMakeLists.txt:7 (project): No CMAKE_ASM_COMPILER could be found. Tell CMake where to find the compiler by setting either the environment variable "ASM" or the CMake cache entry CMAKE_ASM_COMPILER to the full path to the compiler, or to the compiler name if it is in the PATH. -- Warning: Did not find file Compiler/-ASM -- Configuring incomplete, errors occurred! See also "/builddir/build/BUILD/luv-1.30.1-0/obj/CMakeFiles/CMakeOutput.log". See also "/builddir/build/BUILD/luv-1.30.1-0/obj/CMakeFiles/CMakeError.log".
a) I do not see that subpackages do have compat in the name. b) lua packaging is a huge mess. There should be marcos provided for lua packaging and also lua 5.3 schould use 5.3 directories for installing libs headers etc. c) I will fix the compiler issue
Andreas, I tried to review this but it's been a painful number of hours and I still don't have anything working. Sure it builds into an rpm, but does that rpm work for building neovim? Not that I've been able to accomplish. When neovim builds, it looks for the pkgconfig file which isn't included in this rpm. Building the libluv.pc file depends on -DBUILD_SHARED_LIBS=ON which is mutually exclusive with -DBUILD_MODULE=ON. However even if you build the pc file, everything inside it will be wrong, so that needs to be patched/replaced also. So anyway, here are my notes, in case you want to take another run at it: 1. -DBUILD_MODULE=ON and -DBUILD_SHARED_LIBS=ON are mutually exclusive. The former builds bare luv.so and the latter builds a versioned libluv.so with all the symlinks and the pkgconfig file. However the only place BUILD_SHARED_LIBS is referenced is in an "else" section for BUILD_MODULE, so if you want that, you have to use -DBUILD_MODULE=OFF -DBUILD_SHARED_LIBS=ON 2. On Debian, they completely ignore the CMakeLists.txt and build the entire thing with their own debhelper-powered formula. They also generate their own pkgconfig file without referring to the one included in the source. The upside is that everything gets installed consistently. One thing to note is that they effectively build the equivalent of -DBUILD_SHARED_LIBS (versioned dir) and then they symlink from that to luv.so for Lua to load. 3. I'm not sure, but it seems like this lib might link directly to the underlying interpreter library as well, which means that the lua-5.1 version of the build might NOT be the same as the luajit version. I wish I had better news for you :-(
I realized I'm overthinking this. The OpenSUSE builds seem to work fine with the -DBUILD_MODULE=ON approach. On Tumbleweed I see (leaving out the uninteresting parts) # rpm -ql lua51-luv-1.30.1-1.1.x86_64 /usr/lib64/lua/5.1/luv.so # rpm -ql lua51-luv-devel /usr/include/lua5.1/luv/lhandle.h /usr/include/lua5.1/luv/lreq.h /usr/include/lua5.1/luv/lthreadpool.h /usr/include/lua5.1/luv/luv.h /usr/include/lua5.1/luv/util.h and there is no pkgconfig file. So we shouldn't need one on Fedora either, and that means your rpm is fine. Sorry for the fuss, Andreas. 🤦 For the neovim 0.4.2 build, it seems we just need to indicate where to find libluv. This works in my experiments: %if %{with luajit} %global luaver 5.1 %else %global luaver 5.3 %endif -DLIBLUV_INCLUDE_DIR=%{_includedir}/lua-%{luaver} \ -DLIBLUV_LIBRARY=%{_libdir}/lua/%{luaver}/luv.so \ That assumes that the proper version of libvterm is available.
Regarding the review, it appears I'm not able to do the formal review because I'm not a member of the group at https://admin.fedoraproject.org/accounts/group/members/packager/* I tried to proceed anyway, but the fedora-review tool immediately asks for a password without any further explanation or context, and I don't know what it needs. Unfortunately it would probably be better to find another person to do the formal review, as I'm too busy at work to invest the time necessary right now.
(In reply to Andreas Schneider from comment #3) > a) I do not see that subpackages do have compat in the name. Yes, I didn't say you had compat in the subpackage names, but in the macro names.
Spec URL: https://xor.cryptomilk.org/rpm/lua-luv/lua-luv.spec SRPM URL: https://xor.cryptomilk.org/rpm/lua-luv/lua-luv-1.30.1-2.fc32.src.rpm Rawhide build: https://koji.fedoraproject.org/koji/taskinfo?taskID=37962558
Tomas, could you do the review?
I gave this a quick look, but I don't have any system with fedora-review set up and I don't have the time to get it working right now. I also ran into the following issue: warning: line 16: Possible unexpanded macro in: BuildRequires: lua >= %{lua_version} warning: line 17: Possible unexpanded macro in: BuildRequires: lua-devel >= %{lua_version}
Spec URL: https://xor.cryptomilk.org/rpm/lua-luv/lua-luv.spec SRPM URL: https://xor.cryptomilk.org/rpm/lua-luv/lua-luv-1.30.1-3.fc32.src.rpm Thanks. I've fixed it.
- Valid shorthand for Apache 2:0 is ASL 2.0: License: ASL 2.0 - Typo: required: in %description devel and %description -n lua5.1-luv-devel %description devel Files required for lua-luv development - Typo: Hopefully in %description and %description -n lua5.1-luv The best docs currently are the libuv docs themselves. Hopefully soon we'll have a copy locally tailored for lua. - Source is 404: Getting https://github.com/luvit/luv/archive/1.30.1/luv-1.30.1-0.tar.gz to ./luv-1.30.1-0.tar.gz % Total % Received % Xferd Average Speed Time Time Time Current Dload Upload Total Spent Left Speed 100 130 0 130 0 0 185 0 --:--:-- --:--:-- --:--:-- 184 0 0 0 0 0 0 0 0 --:--:-- 0:00:01 --:--:-- 0 curl: (22) The requested URL returned error: 404 Not Found Seems you should include an extraver variable for the version number after the - in https://github.com/luvit/luv/releases %global extraver 1 […] Name: lua-luv Version: 1.30.1 Release: 3.%{extraver}%{?dist} […] Source0: https://github.com/luvit/luv/archive/%{version}-%{extraver}/luv-%{version}-%{extraver}.tar.gz […] %autosetup -p1 -n luv-%{version}-%{extraver} […] %changelog * Mon Sep 30 2019 Andreas Schneider <asn> - 1.30.1-3.1 - Fixed BR for lua 5.3 - Devel subpackages should probably requires their main packages counterpart and drop the doc/license from %files %package devel Summary: Development files for lua-luv Requires: %{name}%{?_isa} = %{version}-%{release} […] %package -n lua5.1-luv-devel Summary: Development files for lua5.1-luv Requires: lua5.1-luv%{?_isa} = %{version}-%{release} […] %files devel %dir %{lua_53_incdir}/luv/ %{lua_53_incdir}/luv/lhandle.h %{lua_53_incdir}/luv/lreq.h %{lua_53_incdir}/luv/luv.h %{lua_53_incdir}/luv/util.h […] %files -n lua5.1-luv-devel %dir %{lua_51_incdir}/luv/ %{lua_51_incdir}/luv/lhandle.h %{lua_51_incdir}/luv/lreq.h %{lua_51_incdir}/luv/luv.h %{lua_51_incdir}/luv/util.h - Patch application fails on latest version: + cd luv-1.30.1-1 + /usr/bin/chmod -Rf a+rX,u+w,g-w,o-w . + /usr/bin/cat /builddir/build/SOURCES/luv-1.30-include_lua_header.patch + /usr/bin/patch -p1 -s --fuzz=0 --no-backup-if-mismatch 1 out of 1 hunk FAILED -- saving rejects to file src/luv.c.rej error: Bad exit status from /var/tmp/rpm-tmp.zcFESt (%prep) It is already included and thus needs to be dropped. - missing -m flag: install -d -m 0755 %{buildroot}%{lua_53_incdir}/luv - Use "install -p" to keep timestamp # lua-5.3 install -d -m 0755 %{buildroot}%{lua_53_libdir} install -pm 0755 %{lua_53_builddir}/luv.so %{buildroot}%{lua_53_libdir}/luv.so install -d -m 0755 %{buildroot}%{lua_53_incdir}/luv for f in lhandle.h lreq.h luv.h util.h; do install -pm 0644 src/$f %{buildroot}%{lua_53_incdir}/luv/$f done # lua-5.1 install -d -m 0755 %{buildroot}%{lua_51_libdir} install -pm 0755 %{lua_51_builddir}/luv.so %{buildroot}%{lua_51_libdir}/luv.so install -d 0755 %{buildroot}%{lua_51_incdir}/luv for f in lhandle.h lreq.h luv.h util.h; do install -pm 0644 src/$f %{buildroot}%{lua_51_incdir}/luv/$f done Package Review ============== Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated [ ] = Manual review needed ===== MUST items ===== C/C++: [x]: Package does not contain kernel modules. [x]: Package contains no static executables. [x]: Development (unversioned) .so files in -devel subpackage, if present. Note: Unversioned so-files in private %_libdir subdirectory (see attachment). Verify they are not in ld path. [x]: If your application is a C or C++ application you must list a BuildRequires against gcc, gcc-c++ or clang. [x]: Header files in -devel subpackage, if present. [x]: Package does not contain any libtool archives (.la) [x]: Rpath absent or only used for internal libs. Generic: [x]: Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines. [!]: License field in the package spec file matches the actual license. Note: Checking patched sources after %prep for licenses. Licenses found: "Unknown or generated", "*No copyright* Apache License (v2.0)", "Apache License (v2.0)". 61 files have unknown license. Detailed output of licensecheck in /home/bob/packaging/review/lua-luv/review- lua-luv/licensecheck.txt [x]: License file installed when any subpackage combination is installed. [!]: Package requires other packages for directories it uses. Note: No known owner of /usr/include/lua-5.3 [x]: %build honors applicable compiler flags or justifies otherwise. [x]: Package contains no bundled libraries without FPC exception. [x]: Changelog in prescribed format. [x]: Sources contain only permissible code or content. [-]: Package contains desktop file if it is a GUI application. [x]: Development files must be in a -devel package [x]: Package uses nothing in %doc for runtime. [x]: Package consistently uses macros (instead of hard-coded directory names). [x]: Package is named according to the Package Naming Guidelines. [x]: Package does not generate any conflict. [x]: Package obeys FHS, except libexecdir and /usr/target. [-]: If the package is a rename of another package, proper Obsoletes and Provides are present. [x]: Requires correct, justified where necessary. [x]: Spec file is legible and written in American English. [-]: Package contains systemd file(s) if in need. [x]: Useful -debuginfo package or justification otherwise. [x]: Package is not known to require an ExcludeArch tag. [-]: Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files. Note: Documentation size is 20480 bytes in 2 files. [x]: Package complies to the Packaging Guidelines [x]: Package successfully compiles and builds into binary rpms on at least one supported primary architecture. [x]: Package installs properly. [x]: Rpmlint is run on all rpms the build produces. Note: There are rpmlint messages (see attachment). [x]: If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package is included in %license. [x]: Package does not own files or directories owned by other packages. [x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT [x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the beginning of %install. [x]: Macros in Summary, %description expandable at SRPM build time. [x]: Dist tag is present. [x]: Package does not contain duplicates in %files. [x]: Permissions on files are set properly. [x]: Package use %makeinstall only when make install DESTDIR=... doesn't work. [x]: Package is named using only allowed ASCII characters. [x]: Package does not use a name that already exists. [x]: Package is not relocatable. [x]: Sources used to build the package match the upstream source, as provided in the spec URL. [x]: Spec file name must match the spec package %{name}, in the format %{name}.spec. [x]: File names are valid UTF-8. [x]: Packages must not store files under /srv, /opt or /usr/local ===== SHOULD items ===== Generic: [-]: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it. [x]: Final provides and requires are sane (see attachments). [!]: Fully versioned dependency in subpackages if applicable. Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in lua5.1-luv , lua5.1-luv-devel [?]: Package functions as described. [!]: Latest version is packaged. [x]: Package does not include license text files separate from upstream. [-]: Sources are verified with gpgverify first in %prep if upstream publishes signatures. Note: gpgverify is not used. [-]: Description and summary sections in the package spec file contains translations for supported Non-English languages, if available. [x]: Package should compile and build into binary rpms on all supported architectures. [x]: %check is present and all tests pass. [!]: Packages should try to preserve timestamps of original installed files. [x]: Reviewer should test that the package builds in mock. [x]: Buildroot is not present [x]: Package has no %clean section with rm -rf %{buildroot} (or $RPM_BUILD_ROOT) [x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin. [x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file [x]: Sources can be downloaded from URI in Source: tag [x]: SourceX is a working URL. [x]: Spec use %global instead of %define unless justified. ===== EXTRA items ===== Generic: [x]: Rpmlint is run on debuginfo package(s). Note: There are rpmlint messages (see attachment). [x]: Rpmlint is run on all installed packages. Note: There are rpmlint messages (see attachment). [x]: Large data in /usr/share should live in a noarch subpackage if package is arched. [x]: Spec file according to URL is the same as in SRPM. Rpmlint ------- Checking: lua-luv-1.30.1-3.1.fc32.x86_64.rpm lua-luv-devel-1.30.1-3.1.fc32.x86_64.rpm lua5.1-luv-1.30.1-3.1.fc32.x86_64.rpm lua5.1-luv-devel-1.30.1-3.1.fc32.x86_64.rpm lua-luv-debuginfo-1.30.1-3.1.fc32.x86_64.rpm lua-luv-debugsource-1.30.1-3.1.fc32.x86_64.rpm lua-luv-1.30.1-3.1.fc32.src.rpm lua-luv.x86_64: W: spelling-error Summary(en_US) libuv -> lib lua-luv.x86_64: W: spelling-error %description -l en_US libuv -> lib lua-luv.x86_64: W: spelling-error %description -l en_US luvit -> fluvial lua-luv.x86_64: W: spelling-error %description -l en_US uv -> UV, iv, u lua-luv.x86_64: W: spelling-error %description -l en_US Hopfully -> Hopefully, Hop fully, Hop-fully lua-luv.x86_64: W: invalid-license Apache-2.0 lua-luv.x86_64: E: missing-call-to-setgroups-before-setuid /usr/lib64/lua/5.3/luv.so lua-luv-devel.x86_64: W: invalid-license Apache-2.0 lua-luv-devel.x86_64: W: no-documentation lua5.1-luv.x86_64: W: spelling-error Summary(en_US) libuv -> lib lua5.1-luv.x86_64: W: spelling-error %description -l en_US libuv -> lib lua5.1-luv.x86_64: W: spelling-error %description -l en_US luvit -> fluvial lua5.1-luv.x86_64: W: spelling-error %description -l en_US uv -> UV, iv, u lua5.1-luv.x86_64: W: invalid-license Apache-2.0 lua5.1-luv.x86_64: E: missing-call-to-setgroups-before-setuid /usr/lib64/lua/5.1/luv.so lua5.1-luv-devel.x86_64: W: invalid-license Apache-2.0 lua5.1-luv-devel.x86_64: W: no-documentation lua-luv-debuginfo.x86_64: W: invalid-license Apache-2.0 lua-luv-debugsource.x86_64: W: invalid-license Apache-2.0 lua-luv.src: W: spelling-error Summary(en_US) libuv -> lib lua-luv.src: W: spelling-error %description -l en_US libuv -> lib lua-luv.src: W: spelling-error %description -l en_US luvit -> fluvial lua-luv.src: W: spelling-error %description -l en_US uv -> UV, iv, u lua-luv.src: W: spelling-error %description -l en_US Hopfully -> Hopefully, Hop fully, Hop-fully lua-luv.src: W: invalid-license Apache-2.0 7 packages and 0 specfiles checked; 2 errors, 23 warnings.
Spec URL: https://xor.cryptomilk.org/rpm/lua-luv/lua-luv.spec SRPM URL: https://xor.cryptomilk.org/rpm/lua-luv/lua-luv-1.30.1-4.1.fc32.src.rpm Robert-André, thank you very much for the review and ideas. I hope I've addressed all issues you've pointed out.
extra_version is part of upstream's version, isn't it? So it should be part of Version, but replacing the dash with a period. https://docs.fedoraproject.org/en-US/packaging-guidelines/Versioning/#_upstream_uses_invalid_characters_in_the_version
Spec URL: https://xor.cryptomilk.org/rpm/lua-luv/lua-luv.spec SRPM URL: https://xor.cryptomilk.org/rpm/lua-luv/lua-luv-1.30.1.1-5.fc32.src.rpm
(In reply to Andreas Schneider from comment #15) > Spec URL: https://xor.cryptomilk.org/rpm/lua-luv/lua-luv.spec > SRPM URL: > https://xor.cryptomilk.org/rpm/lua-luv/lua-luv-1.30.1.1-5.fc32.src.rpm - You've forgotten the %{?_isa} in devel subpackages Requires: %package devel Summary: Development files for lua-luv Requires: lua-luv%{?_isa} = %{version}-%{release} […] %package -n lua5.1-luv-devel Summary: Development files for lua5.1-luv Requires: lua5.1-luv%{?_isa} = %{version}-%{release} Package approved, please fix the aforementioned issue before import.
(fedscm-admin): The Pagure repository was created at https://src.fedoraproject.org/rpms/lua-luv
FEDORA-2019-f868ba36c9 has been submitted as an update to Fedora 31. https://bodhi.fedoraproject.org/updates/FEDORA-2019-f868ba36c9
libvterm-0.1.1-2.fc31, lua-luv-1.30.1.1-5.fc31, neovim-0.4.2-1.fc31 has been pushed to the Fedora 31 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2019-f868ba36c9
libvterm-0.1.1-2.fc31, lua-luv-1.30.1.1-5.fc31, neovim-0.4.2-1.fc31 has been pushed to the Fedora 31 stable repository. If problems still persist, please make note of it in this bug report.
Still broken for me. $ nvim nvim: symbol lookup error: /usr/lib64/lua/5.1/luv.so: undefined symbol: uv_os_environ neovim-0.4.3-1.fc31.x86_64 compat-lua-libs-5.1.5-15.fc31.x86_64 lua5.1-luv-1.32.0.0-0.fc31.x86_64 luajit-2.1.0-0.16beta3.fc31.x86_64 unibilium-2.0.0-5.fc31.x86_64 libtermkey-0.20-7.fc31.x86_64 msgpack-3.1.0-3.fc31.x86_64 libvterm-0.1.1-2.fc31.x86_64 The only workaround I have found is to downgrade to lua5.1-luv-1.30.1.1-5.fc31.x86_64
(In reply to Javier Garcia from comment #21) > Still broken for me. > > $ nvim > nvim: symbol lookup error: /usr/lib64/lua/5.1/luv.so: undefined symbol: > uv_os_environ > > neovim-0.4.3-1.fc31.x86_64 > compat-lua-libs-5.1.5-15.fc31.x86_64 > lua5.1-luv-1.32.0.0-0.fc31.x86_64 > luajit-2.1.0-0.16beta3.fc31.x86_64 > unibilium-2.0.0-5.fc31.x86_64 > libtermkey-0.20-7.fc31.x86_64 > msgpack-3.1.0-3.fc31.x86_64 > libvterm-0.1.1-2.fc31.x86_64 > > The only workaround I have found is to downgrade to > lua5.1-luv-1.30.1.1-5.fc31.x86_64 Please report the issue following this link: https://src.fedoraproject.org/rpms/lua-luv So that the maintainer can see it and act accordingly.