[Bug 1859627] New: Review Request: arm-none-eabi-gdb - GDB for (remote) debugging ARM bare-metal targets
https://bugzilla.redhat.com/show_bug.cgi?id=1859627
Bug ID: 1859627 Summary: Review Request: arm-none-eabi-gdb - GDB for (remote) debugging ARM bare-metal targets Product: Fedora Version: rawhide Hardware: All OS: Linux Status: NEW Component: Package Review Severity: medium Assignee: nobody@fedoraproject.org Reporter: austin880625@gmail.com QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org Target Milestone: --- Classification: Fedora
Spec URL: https://people.cs.nctu.edu.tw/~austin/rpmbuild/arm-none-eabi-gdb.spec SRPM URL: https://people.cs.nctu.edu.tw/~austin/rpmbuild/arm-none-eabi-gdb-9.2-7.fc32.... Description: This is a version of GDB, the GNU Project debugger, for (remote) debugging arm-none-eabi binaries. GDB allows you to see and modify what is going on inside another program while it is executing. Fedora Account System Username: austin880625
https://bugzilla.redhat.com/show_bug.cgi?id=1859627
Austin Chang austin880625@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |177841 (FE-NEEDSPONSOR)
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=177841 [Bug 177841] Tracker: Review requests from new Fedora packagers who need a sponsor
https://bugzilla.redhat.com/show_bug.cgi?id=1859627
Austin Chang austin880625@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Comment|0 |updated
--- Comment #0 has been edited ---
Spec URL: https://people.cs.nctu.edu.tw/~austin/rpmbuild/arm-none-eabi-gdb.spec SRPM URL: https://people.cs.nctu.edu.tw/~austin/rpmbuild/arm-none-eabi-gdb-9.2-7.fc32.... Description: This is a version of GDB, the GNU Project debugger, for (remote) debugging arm-none-eabi binaries. GDB allows you to see and modify what is going on inside another program while it is executing. Fedora Account System Username: austin880625
https://bugzilla.redhat.com/show_bug.cgi?id=1859627
Andy Mender andymenderunix@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |andymenderunix@gmail.com Assignee|nobody@fedoraproject.org |andymenderunix@gmail.com Doc Type|--- |If docs needed, set a value
https://bugzilla.redhat.com/show_bug.cgi?id=1859627
Andy Mender andymenderunix@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED
https://bugzilla.redhat.com/show_bug.cgi?id=1859627
--- Comment #1 from Andy Mender andymenderunix@gmail.com ---
This is my first package so I need a sponsor. If I did anything wrong please point them out.
If you need sponsorship, you need to explicitly indicate it by blocking the FE-NEEDSPONSOR bug report: https://bugzilla.redhat.com/show_bug.cgi?id=FE-NEEDSPONSOR Also, check these docs: - https://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group - https://fedoraproject.org/wiki/Join_the_package_collection_maintainers?rd=Pa...
And regarding general packaging guidelines: https://docs.fedoraproject.org/en-US/packaging-guidelines/
As for the spec file:
%define target arm-none-eabi %define gdb_datarootdir %{_datarootdir}/gdb-%{target}-%{version}
I see the %{_datarootdir} macro works, but /usr/share is referred to as %{_datadir}.
Name: %{target}-gdb Version: 9.2 Release: 1%{?dist} Summary: GDB for (remote) debugging ARM bare-metal targets Group: Development/Debuggers License: GPLv3+ URL: http://sources.redhat.com/gdb/
- It's a bit safer to use spaces as a separator to avoid tabs being converted to different numbers of spaces. - "Group:" blocks are deprecated - The URL redirects me to: http://www.sourceware.org/gdb/. Should that be the case?
BuildRoot: %(mktemp -ud %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX) BuildRequires: texinfo BuildRequires: ncurses-devel BuildRequires: python-devel BuildRequires: texinfo-tex BuildRequires: expat-devel
- Explicitly specifying a buildroot via the "BuildRoot:" block is not preferred anymore. - Dependencies on -devel packages should be specified as "pkgconfig(foo)" like so: BuildRequires: pkgconfig(ncurses) - Python should never be referred to without specifying the version and Python2 is End-of-Life already.
%package devel Summary: GDB for (remote) debugging ARM targets Group: Development/Debuggers Requires: %{name} = %{version}-%{release}
Here, it's better to use the more explicit form: Requires: %{name}%{?_isa} = %{version}-%{release}
# Set datarootdir to have target and version in so that we can exist # side-by-side with other gdb installations of different versions CFLAGS="$RPM_OPT_FLAGS" ../gdb-%{version}/configure --prefix=%{_prefix} \ --libdir=%{_libdir} --mandir=%{_mandir} --infodir=%{_infodir} \ --datarootdir=%{gdb_datarootdir} --disable-rpath \ --target=%{target} --disable-nls --disable-werror --with-python --without-doc --with-xml --with-expat make %{?_smp_mflags}
You should use %make_build instead of the "make %{?_smp_mflags}" form.
%install rm -rf $RPM_BUILD_ROOT cd build make install DESTDIR=$RPM_BUILD_ROOT
- Cleaning the buildroot prior is no longer needed. - Use the %make_install macro instead.
%files %defattr(-,root,root,-) %doc gdb-%{version}/{COPYING?,COPYING?.LIB}
The %defattr macro is no longer used.
%{_bindir}/%{target}-* %{_mandir}/man1/%{target}-*.1.gz %{_mandir}/man5/%{target}-*.5.gz %dir %{_datarootdir}/gdb-%{target}-%{version} %{_datarootdir}/gdb-%{target}-%{version}/*
- Since you're packaging only GDB, if someone adds GCC for your target later, your wildcards might hijack its files. - Use "%{_datadir}/gdb-%{target}-%{version}/" to properly own the dir instead of the last 2 lines.
Currently, your package fails to build in koji and in a mock environment locally. A successful build in a mock environment is needed to run the full review matrix. Please, have a look why the build is failing via mock :).
https://bugzilla.redhat.com/show_bug.cgi?id=1859627
--- Comment #2 from Andy Mender andymenderunix@gmail.com --- Sorry, just realized I missed some stuff:
%clean rm -rf $RPM_BUILD_ROOT
This is no longer needed as cleaning is done by default.
%files %defattr(-,root,root,-) %doc gdb-%{version}/{COPYING?,COPYING?.LIB}
- The COPYING* files contain the license so they should be marked with the %license macro. - %doc should be applied to README files, which are not listed here at all.
https://bugzilla.redhat.com/show_bug.cgi?id=1859627
--- Comment #3 from Austin Chang austin880625@gmail.com --- Hi, thanks for the review. As I am new to packaging and just want to bring this retired package back originally, I simply modified the spec file based on one of the version before it has been retired, and only built on my own local machine. But I didn't noticed that there are so many improper/outdated manners in this file. I'll fix them later today or tomorrow, thank you.
https://bugzilla.redhat.com/show_bug.cgi?id=1859627
--- Comment #4 from Austin Chang austin880625@gmail.com --- Hi, I fixed the problem you have mentioned. Here is the modified spec file and source package:
Spec URL: https://people.cs.nctu.edu.tw/~austin/rpmbuild/202007241716/arm-none-eabi-gd... SRPM URL: https://people.cs.nctu.edu.tw/~austin/rpmbuild/202007241716/arm-none-eabi-gd...
I also tested the build in mock with `mock -r fedora-32-x86_64 --rebuild arm-none-eabi-gdb-9.2-1.fc32.src.rpm` and in koji with `koji build --scratch rawhide ~/rpmbuild/SRPMS/arm-none-eabi-gdb-9.2-1.fc32.src.rpm` Both of them didn't run into apparent errors.
You may check if I miss some other things.
https://bugzilla.redhat.com/show_bug.cgi?id=1859627
--- Comment #5 from Andy Mender andymenderunix@gmail.com --- After some fiddling, I managed to run `fedora-review` from the COPR build, thanks!
BuildRequires: gmp-devel BuildRequires: libmpc-devel %if 0%{?fedora} >= 32 BuildRequires: pkgconfig(mpfr) %else BuildRequires: mpfr-devel %endif BuildRequires: pkgconfig(ncurses) BuildRequires: sed BuildRequires: texinfo BuildRequires: pkgconfig(zlib)
I would check whether it's possible to replace the "*-devel" lines with "pkgconfig(foo)" like you did in the other cases.
cd binutils %make_install cd -
cd gcc PATH=$PWD/../bin:$PATH %make_install # Reset the path PATH=%{base_path} cd -
cd gdb %make_install cd -
Add the "-p" flag to %make_install to preserve timestamps.
Quite a bit of clean-up needs to be done still, sorry :(. There is a load of header files, libtools and static objects which shouldn't be there. The full review matrix is below (I included the full review + rpmlint in attached file):
Package Review ==============
Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated [ ] = Manual review needed
Issues: ======= - Header files in -devel subpackage, if present. **Lots of leftover header files in subdirs of /usr/msp430-elf** See: https://docs.fedoraproject.org/en-US/packaging- guidelines/#_devel_packages - Package does not contain any libtool archives (.la) Note: msp430-elf-gcc : /usr/msp430-elf/libexec/gcc/msp430-elf/9.2.0/liblto_plugin.la See: https://docs.fedoraproject.org/en-US/packaging- guidelines/#packaging-static-libraries - Package uses either %{buildroot} or $RPM_BUILD_ROOT Note: Using both %{buildroot} and $RPM_BUILD_ROOT See: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_macros - 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. Note: License file copying.c is not marked as %license See: https://docs.fedoraproject.org/en-US/packaging- guidelines/LicensingGuidelines/#_license_text - Static libraries in -static or -devel subpackage, providing -devel if present. Note: Package has .a files: msp430-elf-gcc, msp430-elf-gcc-c++. Illegal package name: msp430-elf-gcc, msp430-elf-gcc-c++. Does not provide -static: msp430-elf-gcc, msp430-elf-gcc-c++. See: https://docs.fedoraproject.org/en-US/packaging- guidelines/#packaging-static-libraries
===== MUST items =====
C/C++: [x]: Package does not contain kernel modules. [?]: Package contains no static executables. [!]: 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]: Provides: bundled(gnulib) in place as required. [x]: If your application is a C or C++ application you must list a BuildRequires against gcc, gcc-c++ or clang. [x]: Rpath absent or only used for internal libs.
Generic: [x]: Package successfully compiles and builds into binary rpms on at least one supported primary architecture. Note: Using prebuilt packages [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: Cannot run licensecheck: Command 'licensecheck -r /var/lib/mock/fedora- rawhide-x86_64/root/builddir/build/BUILD/msp430-elf-toolchain' returned non-zero exit status 2. Review: Ran licensecheck manually. A lot of files are BSD licensed! This should be included in the "License:" block as "GPL and BSD" [!]: License file installed when any subpackage combination is installed. Review: all main packages should include the license file. [x]: Package requires other packages for directories it uses. Note: No known owner of /usr/msp430-elf/libexec/gcc/msp430-elf/9.2.0, /usr/msp430-elf/lib/gcc/msp430-elf/9.2.0, /usr/msp430-elf/lib, /usr/msp430-elf/libexec/gcc/msp430-elf, /usr/msp430-elf/lib/gcc/msp430-elf, /usr/msp430-elf/libexec/gcc, /usr/msp430-elf/msp430-elf/lib, /usr/msp430-elf/bin, /usr/msp430-elf/lib/gcc, /usr/msp430-elf/msp430-elf, /usr/msp430-elf, /usr/msp430-elf/libexec, /usr/msp430-elf/msp430-elf/include [!]: Package must own all directories that it creates. Note: Directories without known owners: /usr/msp430-elf/msp430-elf/lib/large/full-memory-range, /usr/msp430-elf/libexec/gcc/msp430-elf, /usr/msp430-elf/libexec/gcc, /usr/msp430-elf/msp430-elf/lib/large/exceptions, /usr/msp430-elf/lib/gcc, /usr/msp430-elf, /usr/msp430-elf/msp430-elf/lib/exceptions, /usr/msp430-elf/msp430-elf/lib/large/full-memory-range/exceptions, /usr/msp430-elf/msp430-elf/include, /usr/msp430-elf/msp430-elf/lib/430/exceptions, /usr/msp430-elf/libexec/gcc/msp430-elf/9.2.0, /usr/msp430-elf/msp430-elf/lib/430, /usr/msp430-elf/lib/gcc/msp430-elf/9.2.0, /usr/msp430-elf/lib, /usr/msp430-elf/lib/gcc/msp430-elf, /usr/msp430-elf/bin, /usr/msp430-elf/msp430-elf/lib, /usr/msp430-elf/msp430-elf/lib/large, /usr/msp430-elf/msp430-elf, /usr/msp430-elf/libexec [x]: %build honors applicable compiler flags or justifies otherwise. [x]: Package contains no bundled libraries without FPC exception. [!]: Changelog in prescribed format. Macros in changelog are not allowed. [x]: Sources contain only permissible code or content. [-]: Package contains desktop file if it is a GUI application. [-]: Development files must be in a -devel package [?]: 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. [?]: 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. [?]: 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 10240 bytes in 1 files. [!]: Package complies to the Packaging Guidelines [x]: Package installs properly. [x]: Rpmlint is run on all rpms the build produces. Note: There are rpmlint messages (see attachment). [x]: Package does not own files or directories owned by other packages. [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 must not depend on deprecated() packages. [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: [x]: Reviewer should test that the package builds in mock. [!]: Sources can be downloaded from URI in Source: tag Note: Could not download Source0: http://software-
dl.ti.com/msp430/msp430_public_sw/mcu/msp430/MSPGCC/9_2_0_0/export/msp430-gcc-9.2.0.50-source- full.tar.bz2 See: https://docs.fedoraproject.org/en-US/packaging- guidelines/SourceURL/ NOt [-]: 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 msp430-elf-gcc , msp430-elf-gcc-c++ , msp430-elf-gdb , msp430-elf- binutils Review: it's a good idea to include this. [?]: Package functions as described. [x]: Latest version is packaged. [x]: Package does not include license text files separate from upstream. [x]: SourceX tarball generation or download is documented. Note: Package contains tarball without URL, check comments [-]: 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. [-]: %check is present and all tests pass. [!]: Packages should try to preserve timestamps of original installed files. [x]: Spec use %global instead of %define unless justified. Note: %define requiring justification: %define __os_install_post . ./os_install_post [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]: SourceX is a working URL.
===== 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: No rpmlint messages. [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.
**Full rpmlint in attached file**
Unversioned so-files -------------------- msp430-elf-gcc: /usr/msp430-elf/lib/gcc/msp430-elf/9.2.0/plugin/libcc1plugin.so msp430-elf-gcc: /usr/msp430-elf/lib/gcc/msp430-elf/9.2.0/plugin/libcp1plugin.so msp430-elf-gcc: /usr/msp430-elf/libexec/gcc/msp430-elf/9.2.0/liblto_plugin.so
Requires -------- msp430-elf-gcc (rpmlib, GLIBC filtered): libc.so.6()(64bit) libcc1plugin.so.0()(64bit) libcp1plugin.so.0()(64bit) libdl.so.2()(64bit) libgcc_s.so.1()(64bit) libgcc_s.so.1(GCC_3.0)(64bit) libgcc_s.so.1(GCC_3.3)(64bit) libgcc_s.so.1(GCC_3.4)(64bit) libgcc_s.so.1(GCC_4.2.0)(64bit) libgmp.so.10()(64bit) liblto_plugin.so.0()(64bit) libm.so.6()(64bit) libmpc.so.3()(64bit) libmpfr.so.6()(64bit) libstdc++.so.6()(64bit) libstdc++.so.6(CXXABI_1.3)(64bit) libstdc++.so.6(CXXABI_1.3.8)(64bit) libstdc++.so.6(CXXABI_1.3.9)(64bit) rtld(GNU_HASH)
msp430-elf-gcc-c++ (rpmlib, GLIBC filtered): libc.so.6()(64bit) libgcc_s.so.1()(64bit) libgcc_s.so.1(GCC_3.3)(64bit) libgcc_s.so.1(GCC_4.2.0)(64bit) libm.so.6()(64bit) libstdc++.so.6()(64bit) libstdc++.so.6(CXXABI_1.3)(64bit) libstdc++.so.6(CXXABI_1.3.9)(64bit) rtld(GNU_HASH)
msp430-elf-gdb (rpmlib, GLIBC filtered): /usr/bin/sh libc.so.6()(64bit) libdl.so.2()(64bit) libexpat.so.1()(64bit) libgcc_s.so.1()(64bit) libgcc_s.so.1(GCC_3.0)(64bit) libm.so.6()(64bit) libncursesw.so.6()(64bit) libpthread.so.0()(64bit) libstdc++.so.6()(64bit) libstdc++.so.6(CXXABI_1.3)(64bit) libstdc++.so.6(CXXABI_1.3.11)(64bit) libstdc++.so.6(CXXABI_1.3.2)(64bit) libstdc++.so.6(CXXABI_1.3.3)(64bit) libstdc++.so.6(CXXABI_1.3.5)(64bit) libstdc++.so.6(CXXABI_1.3.8)(64bit) libstdc++.so.6(CXXABI_1.3.9)(64bit) libtinfo.so.6()(64bit) rtld(GNU_HASH)
msp430-elf-binutils (rpmlib, GLIBC filtered): libc.so.6()(64bit) libdl.so.2()(64bit) libgcc_s.so.1()(64bit) libgcc_s.so.1(GCC_3.3)(64bit) libgcc_s.so.1(GCC_4.2.0)(64bit) libm.so.6()(64bit) libstdc++.so.6()(64bit) libstdc++.so.6(CXXABI_1.3)(64bit) libstdc++.so.6(CXXABI_1.3.9)(64bit) rtld(GNU_HASH)
msp430-elf-toolchain-debuginfo (rpmlib, GLIBC filtered):
msp430-elf-toolchain-debugsource (rpmlib, GLIBC filtered):
Provides -------- msp430-elf-gcc: bundled(gnulib) libcc1plugin.so.0()(64bit) libcp1plugin.so.0()(64bit) liblto_plugin.so.0()(64bit) msp430-elf-gcc msp430-elf-gcc(x86-64)
msp430-elf-gcc-c++: msp430-elf-gcc-c++ msp430-elf-gcc-c++(x86-64)
msp430-elf-gdb: msp430-elf-gdb msp430-elf-gdb(x86-64)
msp430-elf-binutils: msp430-elf-binutils msp430-elf-binutils(x86-64)
msp430-elf-toolchain-debuginfo: msp430-elf-toolchain-debuginfo msp430-elf-toolchain-debuginfo(x86-64)
msp430-elf-toolchain-debugsource: msp430-elf-toolchain-debugsource msp430-elf-toolchain-debugsource(x86-64)
https://bugzilla.redhat.com/show_bug.cgi?id=1859627
--- Comment #6 from Andy Mender andymenderunix@gmail.com --- Ah, I'm terribly sorry! This is a review for another package. I will tackle yours soon.
https://bugzilla.redhat.com/show_bug.cgi?id=1859627
--- Comment #7 from Andy Mender andymenderunix@gmail.com --- I built the package in a Fedora 33/Rawhide x86-64 mock environment locally and it builds cleanly, however `fedora-review` struggles with creating its build environment.
COPR build from your SRPM: https://copr.fedorainfracloud.org/coprs/andymenderunix/arm-none-eabi/build/1... COPR build with the "pkgconfig(mpfr)" workaround: https://copr.fedorainfracloud.org/coprs/andymenderunix/arm-none-eabi/build/1...
%define target arm-none-eabi %define gdb_datarootdir %{_datadir}/gdb-%{target}-%{version}
Use the %global macro instead.
License: GPLv3+
Some source files from libiberty are BSD licensed: gdb-9.2/libiberty/bsearch.c: BSD 3-clause "New" or "Revised" License gdb-9.2/libiberty/random.c: BSD 3-clause "New" or "Revised" License gdb-9.2/libiberty/strtol.c: BSD 3-clause "New" or "Revised" License gdb-9.2/libiberty/strtoll.c: BSD 3-clause "New" or "Revised" License gdb-9.2/libiberty/strtoul.c: BSD 3-clause "New" or "Revised" License gdb-9.2/libiberty/strtoull.c: BSD 3-clause "New" or "Revised" License
The "License:" block should contain "GPLv3+ and BSD".
BuildRequires: texinfo BuildRequires: texinfo-tex BuildRequires: make BuildRequires: gcc-c++ BuildRequires: autoconf BuildRequires: pkgconfig(mpfr) BuildRequires: pkgconfig(ncurses) BuildRequires: pkgconfig(expat)
If you could sort the BuildRequires alphabetically and add gcc like so: BuildRequires: gcc
It's pulled in implicitly, but it's always better to be explicit :).
I also realized that Fedora 31 doesn't support "pkgconfig(mpfr)", so if you want this package to go into Fedora 31, you need this workaround: %if 0%{?fedora} >= 32 BuildRequires: pkgconfig(mpfr) %else BuildRequires: mpfr-devel %endif
%prep %setup -q -c -n %{name} chmod 644 %{SOURCE0}
Is this needed to avoid permission issues during the build?
CFLAGS="$RPM_OPT_FLAGS" ../gdb-%{version}/configure --prefix=%{_prefix} \ --libdir=%{_libdir} --mandir=%{_mandir} --infodir=%{_infodir} \ --datarootdir=%{gdb_datarootdir} --disable-rpath \ --target=%{target} --disable-nls --disable-werror --without-python --without-doc --with-xml --with-expat
- You can replace $RPM_OPT_FLAGS with the %{optflags} macro. - I'm not sure about the current "--prefix" setting, since gdb is theoretically a part of GCC and the prefix should include the target as well ("--prefix=%{_prefix}/%{target}"). The point here is to avoid invading directories of the main on-target GCC package.
%install cd build %make_install
Add the "-p" flag to %make_install to preserve timestamps.
# we don't want these as this is a cross-compiler rm -rf $RPM_BUILD_ROOT%{_infodir} rm -f $RPM_BUILD_ROOT%{_libdir}/libiberty.a
# Get rid of the shared lib rm -f $RPM_BUILD_ROOT%{_libdir}/lib%{target}-sim.a
# Non-linux targets don't have syscalls rm -rf $RPM_BUILD_ROOT%{_prefix}/share/gdb/syscalls
I would probably replace $RPM_BUILD_ROOT with the %{buildroot} macro. This is not a strong requirement, because the package consistently uses $RPM_BUILD_ROOT everywhere.
%{_bindir}/%{target}-*
This should be more specific to your binaries. Based on the mandir entries, you should have lines like these: %{_bindir}/%{target}-gdb %{_bindir}/%{target}-gdbserver %{_bindir}/%{target}-gdb-add-index %{_bindir}/%{target}-gdbinit
%dir %{_datadir}/gdb-%{target}-%{version} %{_datadir}/gdb-%{target}-%{version}/*
Replace these with: %{_datadir}/gdb-%{target}-%{version}/
%files devel %{_includedir}/gdb/jit-reader.h
This is quite risky, because the regular gdb package also installs this header file. Not sure how/whether they differ, but you would need to at least make your package own the /usr/include/gdb dir. To me that doesn't sound like a good idea.
%changelog
- Wed Jul 22 2020 Austin Chang austin880625@gmail.com - 9.2
- Rebuilt the package for newer version.
The version in this %changelog entry should contain a revision number, so "9.2-1" instead of "9.2".
The full review matrix based on COPR build 1575928 (the one with the "pkgconfig(mpfr)" fix):
Package Review ==============
Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated [ ] = Manual review needed
Issues: ======= - Provides: bundled(gnulib) in place as required. Note: Bundled gnulib but no Provides: bundled(gnulib) See: https://fedoraproject.org/wiki/Bundled_Libraries#Requirement_if_you_bundle - 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. Note: License file copying.c is not marked as %license See: https://docs.fedoraproject.org/en-US/packaging- guidelines/LicensingGuidelines/#_license_text - Package does not use a name that already exists. Note: A package with this name already exists. Please check https://src.fedoraproject.org/rpms/arm-none-eabi-gdb See: https://docs.fedoraproject.org/en-US/packaging- guidelines/Naming/#_conflicting_package_names
===== MUST items =====
C/C++: [x]: Package does not contain kernel modules. [x]: Package contains no static executables. [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 successfully compiles and builds into binary rpms on at least one supported primary architecture. Note: Using prebuilt packages [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: "GNU Lesser General Public License", "Unknown or generated", "GPL (v3 or later)", "Expat License", "FSF Unlimited License (with Retention) GPL (v2 or later)", "FSF Unlimited License (with Retention)", "FSF All Permissive License", "GPL (v2 or later)", "GNU General Public License", "*No copyright* GPL (v3 or later)", "GNU Lesser General Public License (v2 or later)", "GPL (v3.1)", "GNU Lesser General Public License (v2.1 or later)", "*No copyright* Public domain", "BSD 3-clause "New" or "Revised" License", "GNU Free Documentation License (v1.3 or later)", "NTP License", "zlib/libpng license", "GNU Free Documentation License (v1.3)", "GPL (v3 or later) GNU Lesser General Public License (v2.1 or later)", "Public domain", "ISC License GPL (v3 or later)", "GNU General Public License (v3)", "*No copyright* GPL (v3)", "Public domain GPL (v3 or later)", "GPL (v3 or later) (with incorrect FSF address)", "GNU Lesser General Public License (v3 or later)", "GPL (v2 or later) (with incorrect FSF address)", "GPL (with incorrect FSF address)", "*No copyright* Boost Software License 1.0", "Boost Software License 1.0", "*No copyright* zlib/libpng license". 5211 files have unknown license. Detailed output of licensecheck in /home/amender/rpmbuild/SPECS/arm-none-eabi- gdb/copr-build-1575928/review-arm-none-eabi-gdb/licensecheck.txt [x]: License file installed when any subpackage combination is installed. [!]: Package must own all directories that it creates. Note: Directories without known owners: /usr/include/gdb [x]: %build honors applicable compiler flags or justifies otherwise. [x]: Package contains no bundled libraries without FPC exception. [!]: 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 [?]: 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. [!]: 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. [!]: Requires correct, justified where necessary. [x]: Spec file is legible and written in American English. [-]: Package contains systemd file(s) if in need. [?]: Useful -debuginfo package or justification otherwise. [x]: Package is not known to require an ExcludeArch tag. [!]: Package complies to the Packaging Guidelines [x]: Package installs properly. [x]: Rpmlint is run on all rpms the build produces. Note: There are rpmlint messages (see attachment). [x]: Package requires other packages for directories it uses. [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 must not depend on deprecated() packages. [x]: Package use %makeinstall only when make install DESTDIR=... doesn't work. [x]: Package is named using only allowed ASCII characters. [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]: Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files. Note: Documentation size is 0 bytes in 0 files. [x]: Packages must not store files under /srv, /opt or /usr/local
===== SHOULD items =====
Generic: [x]: Reviewer should test that the package builds in mock. [-]: 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). [?]: Package functions as described. [x]: 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. [-]: %check is present and all tests pass. [!]: Packages should try to preserve timestamps of original installed files. [!]: Spec use %global instead of %define unless justified. Note: %define requiring justification: %define target arm-none-eabi, %define gdb_datarootdir %{_datadir}/gdb-%{target}-%{version} [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]: Fully versioned dependency in subpackages if applicable. [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.
===== EXTRA items =====
Generic: [x]: Rpmlint is run on debuginfo package(s). Note: No rpmlint messages. [x]: Rpmlint is run on all installed packages. Note: No rpmlint messages. [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: arm-none-eabi-gdb-9.2-1.fc33.x86_64.rpm arm-none-eabi-gdb-devel-9.2-1.fc33.x86_64.rpm arm-none-eabi-gdb-debuginfo-9.2-1.fc33.x86_64.rpm arm-none-eabi-gdb-debugsource-9.2-1.fc33.x86_64.rpm arm-none-eabi-gdb-9.2-1.fc33.src.rpm arm-none-eabi-gdb.x86_64: W: incoherent-version-in-changelog 9.2 ['9.2-1.fc33', '9.2-1'] arm-none-eabi-gdb.x86_64: W: no-manual-page-for-binary arm-none-eabi-run arm-none-eabi-gdb-devel.x86_64: W: no-documentation arm-none-eabi-gdb.src:18: W: mixed-use-of-spaces-and-tabs (spaces: line 4, tab: line 18) 5 packages and 0 specfiles checked; 0 errors, 4 warnings.
Rpmlint (debuginfo) ------------------- Checking: arm-none-eabi-gdb-debuginfo-9.2-1.fc33.x86_64.rpm 1 packages and 0 specfiles checked; 0 errors, 0 warnings.
Rpmlint (installed packages) ---------------------------- (none): E: no installed packages by name arm-none-eabi-gdb-debugsource (none): E: no installed packages by name arm-none-eabi-gdb (none): E: no installed packages by name arm-none-eabi-gdb-devel (none): E: no installed packages by name arm-none-eabi-gdb-debuginfo 0 packages and 0 specfiles checked; 0 errors, 0 warnings.
Source checksums ---------------- http://ftp.gnu.org/gnu/gdb/gdb-9.2.tar.xz : CHECKSUM(SHA256) this package : 360cd7ae79b776988e89d8f9a01c985d0b1fa21c767a4295e5f88cb49175c555 CHECKSUM(SHA256) upstream package : 360cd7ae79b776988e89d8f9a01c985d0b1fa21c767a4295e5f88cb49175c555
Requires -------- arm-none-eabi-gdb (rpmlib, GLIBC filtered): /usr/bin/sh libc.so.6()(64bit) libdl.so.2()(64bit) libexpat.so.1()(64bit) libgcc_s.so.1()(64bit) libgcc_s.so.1(GCC_3.0)(64bit) libgcc_s.so.1(GCC_3.4)(64bit) libgmp.so.10()(64bit) libm.so.6()(64bit) libmpfr.so.6()(64bit) libncursesw.so.6()(64bit) libpthread.so.0()(64bit) libstdc++.so.6()(64bit) libstdc++.so.6(CXXABI_1.3)(64bit) libstdc++.so.6(CXXABI_1.3.11)(64bit) libstdc++.so.6(CXXABI_1.3.2)(64bit) libstdc++.so.6(CXXABI_1.3.3)(64bit) libstdc++.so.6(CXXABI_1.3.5)(64bit) libstdc++.so.6(CXXABI_1.3.8)(64bit) libstdc++.so.6(CXXABI_1.3.9)(64bit) libtinfo.so.6()(64bit) rtld(GNU_HASH)
arm-none-eabi-gdb-devel (rpmlib, GLIBC filtered): arm-none-eabi-gdb(x86-64)
arm-none-eabi-gdb-debuginfo (rpmlib, GLIBC filtered):
arm-none-eabi-gdb-debugsource (rpmlib, GLIBC filtered):
Provides -------- arm-none-eabi-gdb: arm-none-eabi-gdb arm-none-eabi-gdb(x86-64)
arm-none-eabi-gdb-devel: arm-none-eabi-gdb-devel arm-none-eabi-gdb-devel(x86-64)
arm-none-eabi-gdb-debuginfo: arm-none-eabi-gdb-debuginfo arm-none-eabi-gdb-debuginfo(x86-64) debuginfo(build-id)
arm-none-eabi-gdb-debugsource: arm-none-eabi-gdb-debugsource arm-none-eabi-gdb-debugsource(x86-64)
https://bugzilla.redhat.com/show_bug.cgi?id=1859627
--- Comment #8 from Austin Chang austin880625@gmail.com --- (In reply to Andy Mender from comment #7)
Sorry for the late response, but I fixed some of the mentioned issues, and still need to check several things:
%prep %setup -q -c -n %{name} chmod 644 %{SOURCE0}
Is this needed to avoid permission issues during the build?
rpmlint shows "weird permission" warning at that time and I fixed by this, but it disappeared in later builds. I have deleted now.
CFLAGS="$RPM_OPT_FLAGS" ../gdb-%{version}/configure --prefix=%{_prefix} \ --libdir=%{_libdir} --mandir=%{_mandir} --infodir=%{_infodir} \ --datarootdir=%{gdb_datarootdir} --disable-rpath \ --target=%{target} --disable-nls --disable-werror --without-python --without-doc --with-xml --with-expat
- You can replace $RPM_OPT_FLAGS with the %{optflags} macro.
- I'm not sure about the current "--prefix" setting, since gdb is
theoretically a part of GCC and the prefix should include the target as well ("--prefix=%{_prefix}/%{target}"). The point here is to avoid invading directories of the main on-target GCC package.
In this way won't I need to copy the needed file from the specified prefix to placed like %{_bindir} in the %install section for it to reside in correct path? If the files to be installed has been listed explicitly in the %files section, how would this option "invade" other normal GCC packages?
%{_bindir}/%{target}-*
This should be more specific to your binaries. Based on the mandir entries, you should have lines like these: %{_bindir}/%{target}-gdb %{_bindir}/%{target}-gdbserver %{_bindir}/%{target}-gdb-add-index %{_bindir}/%{target}-gdbinit
I have changed this into the explicit list, the actual binaries are:
%{_bindir}/%{target}-gdb %{_bindir}/%{target}-gdb-add-index %{_bindir}/%{target}-run
%files devel %{_includedir}/gdb/jit-reader.h
This is quite risky, because the regular gdb package also installs this header file. Not sure how/whether they differ, but you would need to at least make your package own the /usr/include/gdb dir. To me that doesn't sound like a good idea.
I'm not sure about the right way to deal with it. Is there a standard(or distribution-independent) path to place arch-specific header files when the package itself doesn't have a safe default path?
I have only seen similar things in /usr/lib like placing object files in directories like x86_64-linux-gnu. But they still create possibly conflicting symlinks in /usr/lib . I also haven't learned about how to do this correctly for header files(maybe there is an config options like --libdir ?)
As I personally don't use it, is it OK not to install it and remove the whole devel subpackage(which includes only this file)?
https://bugzilla.redhat.com/show_bug.cgi?id=1859627
--- Comment #9 from Andy Mender andymenderunix@gmail.com ---
In this way won't I need to copy the needed file from the specified prefix to placed like %{_bindir} in the %install section for it to reside in correct path?
If the files to be installed has been listed explicitly in the %files section, how would this option "invade" other normal GCC packages?
Honestly, I'm a little torn regarding this. I'm reviewing another package, a GCC version for a non-standard target: https://bugzilla.redhat.com/show_bug.cgi?id=1350884 (relevant section of the Packaging guidelines: https://fedoraproject.org/wiki/Packaging_Cross_Compiling_Toolchains#Cross-co...) In that case it seems like all of the extra trickery is needed. However, for instance avr-gdb, a package extremely similar to yours, doesn't do any of that (a snippet from its SPEC file):
%prep %setup -q -c cp %{SOURCE1} .
%build mkdir -p build pushd build CFLAGS="$RPM_OPT_FLAGS -D_GNU_SOURCE" \ ../gdb-%{version}/configure --prefix=%{_prefix} \ --libdir=%{_libdir} --mandir=%{_mandir} --infodir=%{_infodir} \ --target=%{target} --disable-nls --disable-werror \ --with-system-zlib make %{?_smp_mflags} popd
%install pushd build make install DESTDIR=$RPM_BUILD_ROOT popd
# we don't want these as we are a cross version rm -r $RPM_BUILD_ROOT%{_infodir} rm -r $RPM_BUILD_ROOT%{_datadir}/gdb # Should not be installed rm $RPM_BUILD_ROOT%{_libdir}/libavr-sim.a
# no need for devel files rm -rf $RPM_BUILD_ROOT%{_includedir}
%files %doc gdb-%{version}/COPYING* gdb-%{version}/README* %{_bindir}/%{name}* %{_bindir}/avr-run %{_mandir}/man1/avr-* %{_mandir}/man5/avr-*
The %doc line is not correct, because it includes also COPYING* license files. However, both avr-gdb and your arm-none-eabi-gdb are not full compiler toolchains so perhaps many of the general cross-compiler guidelines can be relaxed.
I'm not sure about the right way to deal with it. Is there a standard(or distribution-independent) path to
place arch-specific header files when the package itself doesn't have a safe default path?
Not that I'm aware of. Usually stuff goes into "/usr/include" or "/usr/local/include" and then into subdirs like "/usr/include/%{name}".
I have only seen similar things in /usr/lib like placing object files in directories like x86_64-linux-gnu.
But they still create possibly conflicting symlinks in /usr/lib . I also haven't learned about how to do this correctly for header files(maybe there is an config options like --libdir ?)
I had a look at the gdb's "configure" script and there is an option for header files called "--includedir".
As I personally don't use it, is it OK not to install it and remove the whole devel subpackage(which includes only this file)?
I'd say you probably can remove the -devel subpackage. Headers if provided should sit in -devel subpackages, but I haven't seen a rule which would say that you always have to distribute them, unless they're required by the binaries to function. Notice that the avr-gdb package doesn't have a -devel subpackage either.
https://bugzilla.redhat.com/show_bug.cgi?id=1859627
Andy Mender andymenderunix@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |fedora-review?
https://bugzilla.redhat.com/show_bug.cgi?id=1859627
Andy Mender andymenderunix@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |needinfo?(austin880625@gmai | |l.com)
--- Comment #10 from Andy Mender andymenderunix@gmail.com --- Hello Austin, any updates on this?
https://bugzilla.redhat.com/show_bug.cgi?id=1859627
Austin Chang austin880625@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(austin880625@gmai | |l.com) |
--- Comment #11 from Austin Chang austin880625@gmail.com --- Ah, I removed devel package as I intended to. I will do a test with mock and koji again then post a current version for review soon.
https://bugzilla.redhat.com/show_bug.cgi?id=1859627
--- Comment #12 from Austin Chang austin880625@gmail.com --- I found that I haven't finish the part of the prefix for binaries yet. Below is the current version, but I will keep working on the one with correct prefix if that is really needed.
SPEC: https://people.cs.nctu.edu.tw/~austin/rpmbuild/202010040006/arm-none-eabi-gd... SRPM: https://people.cs.nctu.edu.tw/~austin/rpmbuild/202010040006/arm-none-eabi-gd...
https://bugzilla.redhat.com/show_bug.cgi?id=1859627
--- Comment #13 from Andy Mender andymenderunix@gmail.com --- Koji build from latest SRPM: https://koji.fedoraproject.org/koji/taskinfo?taskID=52819915
%if 0%{?fedora} >= 32 BuildRequires: pkgconfig(mpfr) %else BuildRequires: mpfr-devel %endif
Since Fedora 33 is on its way, Fedora 34 is the new Rawhide, I think this if-else can be simplified to:
BuildRequires: pkgconfig(mpfr)
I re-ran fedora-review just in case and it found a couple of outstanding points, mostly bundling of "gnulib" and the License tag. The rest looks good:
Package Review ==============
Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated [ ] = Manual review needed
Issues: ======= - Provides: bundled(gnulib) in place as required. Note: Bundled gnulib but no Provides: bundled(gnulib) See: https://fedoraproject.org/wiki/Bundled_Libraries#Requirement_if_you_bundle Review: yes, this needs to be added to the SPEC file, for instance below the list of BuildRequires. - 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. Note: License file copying.c is not marked as %license See: https://docs.fedoraproject.org/en-US/packaging- guidelines/LicensingGuidelines/#_license_text Review: Please, ignore this one. copying.c is not a license file. - Package does not use a name that already exists. Note: A package with this name already exists. Please check https://src.fedoraproject.org/rpms/arm-none-eabi-gdb See: https://docs.fedoraproject.org/en-US/packaging- guidelines/Naming/#_conflicting_package_names Review: correct, since it's an unretirement request.
===== MUST items =====
C/C++: [x]: Package does not contain kernel modules. [x]: Package contains no static executables. [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 successfully compiles and builds into binary rpms on at least one supported primary architecture. Note: Using prebuilt packages Review: Tested in Koji. [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: "GNU Lesser General Public License", "Unknown or generated", "GPL (v3 or later)", "Expat License", "FSF Unlimited License (with Retention) GPL (v2 or later)", "FSF Unlimited License (with Retention)", "FSF All Permissive License", "GPL (v2 or later)", "GNU General Public License", "*No copyright* GPL (v3 or later)", "GNU Lesser General Public License (v2 or later)", "GPL (v3.1)", "GNU Lesser General Public License (v2.1 or later)", "*No copyright* Public domain", "BSD 3-clause "New" or "Revised" License", "GNU Free Documentation License (v1.3 or later)", "NTP License", "zlib/libpng license", "GNU Free Documentation License (v1.3)", "GPL (v3 or later) GNU Lesser General Public License (v2.1 or later)", "Public domain", "ISC License GPL (v3 or later)", "GNU General Public License (v3)", "*No copyright* GPL (v3)", "Public domain GPL (v3 or later)", "GPL (v3 or later) (with incorrect FSF address)", "GNU Lesser General Public License (v3 or later)", "GPL (v2 or later) (with incorrect FSF address)", "GPL (with incorrect FSF address)", "*No copyright* Boost Software License 1.0", "Boost Software License 1.0", "*No copyright* zlib/libpng license". 5211 files have unknown license. Detailed output of licensecheck in /home/amender/rpmbuild/SPECS/arm-none-eabi-gdb/arm- none-eabi-gdb/licensecheck.txt Review: Quite a bit of extra licenses listed here. Have a look at the main gdb package as an example: https://src.fedoraproject.org/rpms/gdb/blob/master/f/gdb.spec#_42 [x]: License file installed when any subpackage combination is installed. [!]: If the package is under multiple licenses, the licensing breakdown must be documented in the spec. Review: you don't have to go overboard with this, of course. Just mention which modules have which license. I'll attach a licensecheck.txt output for details. [x]: %build honors applicable compiler flags or justifies otherwise. [x]: Package contains no bundled libraries without FPC exception. Review. Only gnulib was marked as bundled by the automatic checks. [x]: Changelog in prescribed format. [x]: Sources contain only permissible code or content. [-]: Package contains desktop file if it is a GUI application. [-]: Development files must be in a -devel package [?]: 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. [x]: Package complies to the Packaging Guidelines Review: Yes, but see above comments. [x]: Package installs properly. [x]: Rpmlint is run on all rpms the build produces. Note: There are rpmlint messages (see attachment). [x]: Package requires other packages for directories it uses. [x]: Package must own all directories that it creates. [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 must not depend on deprecated() packages. [x]: Package use %makeinstall only when make install DESTDIR=... doesn't work. [x]: Package is named using only allowed ASCII characters. [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]: Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files. Note: Documentation size is 0 bytes in 0 files. [x]: Packages must not store files under /srv, /opt or /usr/local
===== SHOULD items =====
Generic: [x]: Reviewer should test that the package builds in mock. [-]: 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). [?]: Package functions as described. [x]: 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. [-]: %check is present and all tests pass. Review: not provided by upstream. [x]: Packages should try to preserve timestamps of original installed files. [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]: Fully versioned dependency in subpackages if applicable. [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: No rpmlint messages. [x]: Rpmlint is run on all installed packages. Note: No rpmlint messages. [x]: Large data in /usr/share should live in a noarch subpackage if package is arched.
Rpmlint ------- Checking: arm-none-eabi-gdb-9.2-1.fc34.x86_64.rpm arm-none-eabi-gdb-debuginfo-9.2-1.fc34.x86_64.rpm arm-none-eabi-gdb-debugsource-9.2-1.fc34.x86_64.rpm arm-none-eabi-gdb-9.2-1.fc32.src.rpm arm-none-eabi-gdb.x86_64: W: no-manual-page-for-binary arm-none-eabi-run 4 packages and 0 specfiles checked; 0 errors, 1 warnings.
Rpmlint (debuginfo) ------------------- Checking: arm-none-eabi-gdb-debuginfo-9.2-1.fc34.x86_64.rpm 1 packages and 0 specfiles checked; 0 errors, 0 warnings.
Rpmlint (installed packages) ---------------------------- (none): E: no installed packages by name arm-none-eabi-gdb (none): E: no installed packages by name arm-none-eabi-gdb-debuginfo (none): E: no installed packages by name arm-none-eabi-gdb-debugsource 0 packages and 0 specfiles checked; 0 errors, 0 warnings.
Source checksums ---------------- http://ftp.gnu.org/gnu/gdb/gdb-9.2.tar.xz : CHECKSUM(SHA256) this package : 360cd7ae79b776988e89d8f9a01c985d0b1fa21c767a4295e5f88cb49175c555 CHECKSUM(SHA256) upstream package : 360cd7ae79b776988e89d8f9a01c985d0b1fa21c767a4295e5f88cb49175c555
Requires -------- arm-none-eabi-gdb (rpmlib, GLIBC filtered): /usr/bin/sh libc.so.6()(64bit) libdl.so.2()(64bit) libexpat.so.1()(64bit) libgcc_s.so.1()(64bit) libgcc_s.so.1(GCC_3.0)(64bit) libgcc_s.so.1(GCC_3.4)(64bit) libgmp.so.10()(64bit) libm.so.6()(64bit) libmpfr.so.6()(64bit) libncursesw.so.6()(64bit) libpthread.so.0()(64bit) libstdc++.so.6()(64bit) libstdc++.so.6(CXXABI_1.3)(64bit) libstdc++.so.6(CXXABI_1.3.11)(64bit) libstdc++.so.6(CXXABI_1.3.2)(64bit) libstdc++.so.6(CXXABI_1.3.3)(64bit) libstdc++.so.6(CXXABI_1.3.5)(64bit) libstdc++.so.6(CXXABI_1.3.8)(64bit) libstdc++.so.6(CXXABI_1.3.9)(64bit) libtinfo.so.6()(64bit) rtld(GNU_HASH)
arm-none-eabi-gdb-debuginfo (rpmlib, GLIBC filtered):
arm-none-eabi-gdb-debugsource (rpmlib, GLIBC filtered):
Provides -------- arm-none-eabi-gdb: arm-none-eabi-gdb arm-none-eabi-gdb(x86-64)
arm-none-eabi-gdb-debuginfo: arm-none-eabi-gdb-debuginfo arm-none-eabi-gdb-debuginfo(x86-64) debuginfo(build-id)
arm-none-eabi-gdb-debugsource: arm-none-eabi-gdb-debugsource arm-none-eabi-gdb-debugsource(x86-64)
https://bugzilla.redhat.com/show_bug.cgi?id=1859627
--- Comment #14 from Andy Mender andymenderunix@gmail.com --- Created attachment 1719142 --> https://bugzilla.redhat.com/attachment.cgi?id=1719142&action=edit licensecheck.txt
Output of licensecheck, generated during fedora-review
https://bugzilla.redhat.com/show_bug.cgi?id=1859627
--- Comment #15 from Austin Chang austin880625@gmail.com --- After reading license guide in https://fedoraproject.org/wiki/Licensing:Main?rd=Licensing , I got several problems:
1. How should I check whether a license can have "GPLx with exception"? It is listed in gdb package but I didn't see any license listed in licensecheck.txt have such short name in the license guide. 2. If license like "NTP" is not listed in the license guide, can it simply be listed as "NTP"? 3. What does "XXX GPL" mean(e.g. ISC License GPL)? How does it related with GPL and the license itself? And how should I specify them in License field?
Spec file and SRPM below contains most licenses I can infer from licensecheck:
https://people.cs.nctu.edu.tw/~austin/rpmbuild/202010091905/arm-none-eabi-gd... https://people.cs.nctu.edu.tw/~austin/rpmbuild/202010091905/arm-none-eabi-gd...
https://bugzilla.redhat.com/show_bug.cgi?id=1859627
--- Comment #16 from Andy Mender andymenderunix@gmail.com ---
- How should I check whether a license can have "GPLx with exception"? It is listed in gdb package but I didn't see any license listed in licensecheck.txt have such short name in the license guide.
I think you need to have a look at specific files listed in licensecheck.txt and see whether the license header in them contains clauses which could be considered exceptions from the general text of a particular GPL license. The doc you mentioned earlier (https://fedoraproject.org/wiki/Licensing:Main?rd=Licensing#SoftwareLicenses) gives examples of licenses which are GPL-like, but contain such clauses (for instance, covering font embedding).
- If license like "NTP" is not listed in the license guide, can it simply be listed as "NTP"?
No, licenses which are not listed in the license guide need to go through Fedora Legal (legal@lists.fedoraproject.org) for verification. NTP is actually a misnomer for one of the MIT licenses: https://fedoraproject.org/wiki/Licensing:MIT When you compare the text of the NTP license to one of the "old style" variants of the MIT license, it's exactly the same. I thought it was supposed to be added to the license matrix in the license guide, but wasn't yet it seems.
- What does "XXX GPL" mean(e.g. ISC License GPL)? How does it related with GPL and the license itself? And how should I specify them in License field?
This I don't know, since ISC and GPL are different licenses. Have a look at the file with the license header and compare to both the ISC license and the GPL3 license to see which one's more likely. If it's absolutely unclear, run it through Fedora Legal.
I think your package should also contain the "Boost" license tag, even though the original "gdb" package doesn't mention it.
https://bugzilla.redhat.com/show_bug.cgi?id=1859627
--- Comment #17 from Austin Chang austin880625@gmail.com --- Some findings after looking into licenses in the source files:
1. files labeled as "NTP license": arm-none-eabi-gdb/gdb-9.2/libiberty/strcasecmp.c, arm-none-eabi-gdb/gdb-9.2/libiberty/strncasecmp.c : these two files contain a statement I cannot really identify what license it is (something from UC Berkeley?) arm-none-eabi-gdb/gdb-9.2/readline/readline/support/install.sh : This looks like an old style MIT license arm-none-eabi-gdb/gdb-9.2/zlib/contrib/iostream2/zstream.h : This looks like CMR license?
2. file labeled as "ISC License GPL (v3 or later)": arm-none-eabi-gdb/gdb-9.2/gnulib/import/inet_ntop.c : Yes this file really contains both GPL and ISC license text.
3. file labeled as "FSF Unlimited License (with Retention) GPL (v2 or later)": arm-none-eabi-gdb/gdb-9.2/libtool.m4 : This is FSFULLR licensed, with a m4 macro defined at the beginning to be expanded as GPL license text, which would be used elsewhere.
4. About "Boost" license tag: I think it is already contained in my spec file(between BSD ans zlib).
Now I should at least remove the "NTP" tag in spec file. You may point out what to do with above issues or other problems so that I can deal with them at once.
https://bugzilla.redhat.com/show_bug.cgi?id=1859627
Andy Mender andymenderunix@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |POST Flags|fedora-review? |fedora-review+
--- Comment #18 from Andy Mender andymenderunix@gmail.com ---
arm-none-eabi-gdb/gdb-9.2/libiberty/strcasecmp.c, arm-none-eabi-gdb/gdb-9.2/libiberty/strncasecmp.c : these two files contain a statement I cannot really identify what license it is (something from UC Berkeley?)
This looks like a legacy BSD license, with added attribution like here: https://fedoraproject.org/wiki/Licensing/BSD_with_Attribution And a very very limited disclaimer.
More examples of BSD licenses: https://en.wikipedia.org/wiki/BSD_licenses Notice that it's very similar to the "Previous license" and even the dates roughly align.
arm-none-eabi-gdb/gdb-9.2/readline/readline/support/install.sh : This looks like an old style MIT license
Correct. This is a MIT license alright.
arm-none-eabi-gdb/gdb-9.2/zlib/contrib/iostream2/zstream.h : This looks like CMR license?
There is no CMR license in the license table, but it looks like a MIT variant.
- file labeled as "ISC License GPL (v3 or later)":
arm-none-eabi-gdb/gdb-9.2/gnulib/import/inet_ntop.c : Yes this file really contains both GPL and ISC license text.
Absolutely correct! ISC is compatible with GPL, thankfully.
- About "Boost" license tag:
I think it is already contained in my spec file(between BSD ans zlib).
Yes, sorry, I missed that.
Now I should at least remove the "NTP" tag in spec file. You may point out what to do with above issues or other problems so that I can deal with them at once.
From my side everything looks okay now. However, please indicate roughly which bits use which licenses, since there is quite a lot of them. Regarding arm-none-eabi-gdb/gdb-9.2/libiberty/strcasecmp.c and arm-none-eabi-gdb/gdb-9.2/libiberty/strncasecmp.c, if in doubt, contact Fedora Legal to make sure it's clear from their side as well. Later, that license text can be added to the BSD license subpage as an extra example. The key point here is that all of these licenses need to be compatible with each other.
Package approved.
https://bugzilla.redhat.com/show_bug.cgi?id=1859627
Andy Mender andymenderunix@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |needinfo?(austin880625@gmai | |l.com)
--- Comment #19 from Andy Mender andymenderunix@gmail.com --- Hello Austin :). All good or do you need some help with the repository and initial import?
https://bugzilla.redhat.com/show_bug.cgi?id=1859627
Austin Chang austin880625@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(austin880625@gmai | |l.com) |
--- Comment #20 from Austin Chang austin880625@gmail.com --- I didn't notice the approved notification. Now the scm request is created(https://pagure.io/releng/fedora-scm-requests/issue/31183).
https://bugzilla.redhat.com/show_bug.cgi?id=1859627
Andy Mender andymenderunix@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|POST |ASSIGNED Flags| |needinfo?(austin880625@gmai | |l.com)
--- Comment #21 from Andy Mender andymenderunix@gmail.com --- I saw the Pagure new-repo request was rejected as you're not an official packager yet. Could you mark this bug request as blocking the FE-NEEDSPONSOR tracking bug report? https://bugzilla.redhat.com/show_bug.cgi?id=FE-NEEDSPONSOR
https://bugzilla.redhat.com/show_bug.cgi?id=1859627
Austin Chang austin880625@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks|177841 (FE-NEEDSPONSOR) | Depends On| |177841 (FE-NEEDSPONSOR)
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=177841 [Bug 177841] Tracker: Review requests from new Fedora packagers who need a sponsor
https://bugzilla.redhat.com/show_bug.cgi?id=1859627
Austin Chang austin880625@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(austin880625@gmai | |l.com) |
--- Comment #22 from Austin Chang austin880625@gmail.com --- Err, I moved the "FE-NEEDSPONSOR" from "Blocks" to "Depends On" field. Am I doing it right(Sorry for being not familiar with the process)?
https://bugzilla.redhat.com/show_bug.cgi?id=1859627
Jaroslav Kysela jkysela@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |jkysela@redhat.com Flags| |needinfo?(austin880625@gmai | |l.com)
--- Comment #23 from Jaroslav Kysela jkysela@redhat.com --- Austin - a quick question, what does not work for you in the standard gdb package? I'm just trying to debug the Raspberry Pi Pico (arm-none-eabi compiler) and it seems working:
$ gdb -ex "target extended-remote localhost:3333" blink.elf GNU gdb (GDB) Fedora 10.1-4.fc33 Copyright (C) 2020 Free Software Foundation, Inc. License GPLv3+: GNU GPL version 3 or later http://gnu.org/licenses/gpl.html This is free software: you are free to change and redistribute it. There is NO WARRANTY, to the extent permitted by law. Type "show copying" and "show warranty" for details. This GDB was configured as "x86_64-redhat-linux-gnu". Type "show configuration" for configuration details. For bug reporting instructions, please see: https://www.gnu.org/software/gdb/bugs/. Find the GDB manual and other documentation resources online at: http://www.gnu.org/software/gdb/documentation/.
For help, type "help". Type "apropos word" to search for commands related to "word"... Reading symbols from blink.elf... Remote debugging using localhost:3333 warning: Remote gdbserver does not support determining executable automatically. RHEL <=6.8 and <=7.2 versions of gdbserver do not support such automatic executable detection. The following versions of gdbserver support it: - Upstream version of gdbserver (unsupported) 7.10 or later - Red Hat Developer Toolset (DTS) version of gdbserver from DTS 4.0 or later (only on x86_64) - RHEL-7.3 versions of gdbserver (on any architecture) warning: multi-threaded target stopped without sending a thread-id, using first non-exited thread to_us_since_boot (t=...) at /opt/pico-sdk/src/common/pico_base/include/pico/types.h:46 46 return t._private_us_since_boot; (gdb) bt #0 to_us_since_boot (t=...) at /opt/pico-sdk/src/common/pico_base/include/pico/types.h:46 #1 time_reached (t=...) at /opt/pico-sdk/src/rp2_common/hardware_timer/include/hardware/timer.h:108 #2 sleep_until (t=...) at /opt/pico-sdk/src/common/pico_time/time.c:343 #3 0x10000d94 in sleep_us (us=<optimized out>) at /opt/pico-sdk/src/common/pico_time/include/pico/time.h:102 #4 0x10000dce in sleep_ms (ms=ms@entry=250) at /opt/pico-sdk/src/common/pico_time/time.c:367 #5 0x10000386 in main () at /home/perex/mysrc/rpi/pico/pico-examples/blink/blink.c:20 (gdb) break sleep_ms Breakpoint 1 at 0x10000dbc: file /opt/pico-sdk/src/common/pico_time/time.c, line 366. Note: automatically using hardware breakpoints for read-only addresses. (gdb) c Continuing. target halted due to debug-request, current mode: Thread xPSR: 0x01000000 pc: 0x00000178 msp: 0x20041f00
Thread 1 hit Breakpoint 1, sleep_ms (ms=ms@entry=250) at /opt/pico-sdk/src/common/pico_time/time.c:366 366 void sleep_ms(uint32_t ms) {
https://bugzilla.redhat.com/show_bug.cgi?id=1859627
--- Comment #24 from Jaroslav Kysela jkysela@redhat.com --- I built arm-none-eabi-gdb in copr (updated to gdb 10.2) for tests: https://copr.fedorainfracloud.org/coprs/perex/raspberry-pi-pico/package/arm-... .
But I don't see a difference.
https://bugzilla.redhat.com/show_bug.cgi?id=1859627
Federic feddy@piments.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |feddy@piments.com
--- Comment #25 from Federic feddy@piments.com --- That's good point since gdb is running locally not on the remote hardware. You may have just saved me some time wondering why I could not find "cross" gdb.
https://bugzilla.redhat.com/show_bug.cgi?id=1859627
Austin Chang austin880625@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(austin880625@gmai | |l.com) |
--- Comment #26 from Austin Chang austin880625@gmail.com --- Sorry for the late response. I have also tested my debugging environment on my STM32L4 board and it seems to run without any problem. So maybe this package is not needed anymore. The original board I tested on was a STM32F4 compatible board produced by an unknown factory, and it's not in my hand anymore. I believe the problem may be on the board itself or hardware debugger, etc.
The reason I made this package was that I used to have arm-none-eabi-gdb before Fedora 31 as I remembered. But it disappeared in Fedora 32 without any technical discussion I can find(seems it just retired because of a long time of unmaintainment?). There are also a bunch of tutorials using 'arm-none-eabi-gdb' instead of plain 'gdb', and in Arch Linux or Ubuntu world, there is an exactly same package or something like gdb-multiarch. The GNU Toolchain distributed by ARM(https://developer.arm.com/tools-and-software/open-source-software/developer-...) also provides 'arm-none-eabi-gdb'. So I believed and thought it was a need.
Indeed I thought in the same way as previous comments, gdb runs locally and should not have anything to do with the remote hardware. Maybe it need to detect something like calling convention when using commands like 'call' but can be set by command 'set arch'. I'm happy that 'gdb' works well, but I still glad to know what the '--target' option in GNU docs(https://sourceware.org/gdb/current/onlinedocs/gdb/Configure-Options.html) really means and how it affects the gdb build, which may not be proper to discuss here.
https://bugzilla.redhat.com/show_bug.cgi?id=1859627
seb@k-7.ch changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |seb@k-7.ch
--- Comment #27 from seb@k-7.ch --- Hello, any news?
https://bugzilla.redhat.com/show_bug.cgi?id=1859627 Bug 1859627 depends on bug 177841, which changed state.
Bug 177841 Summary: Tracker: Review requests from new Fedora packagers who need a sponsor https://bugzilla.redhat.com/show_bug.cgi?id=177841
What |Removed |Added ---------------------------------------------------------------------------- Status|ON_QA |CLOSED Resolution|--- |ERRATA
https://bugzilla.redhat.com/show_bug.cgi?id=1859627
Austin Chang austin880625@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |CLOSED Resolution|--- |NOTABUG Flags|needinfo?(austin880625@gmai | |l.com) | Last Closed| |2022-12-17 09:43:26
--- Comment #29 from Austin Chang austin880625@gmail.com --- sorry for late reply. As I don't need this now and others commented the current status of gdb should work fine in my use case, I'll close it right away
Product: Fedora Version: rawhide Component: Package Review
Austin Chang austin880625@gmail.com has canceled Package Review package-review@lists.fedoraproject.org's request for Austin Chang austin880625@gmail.com's needinfo: Bug 1859627: Review Request: arm-none-eabi-gdb - GDB for (remote) debugging ARM bare-metal targets https://bugzilla.redhat.com/show_bug.cgi?id=1859627
--- Comment #29 from Austin Chang austin880625@gmail.com --- sorry for late reply. As I don't need this now and others commented the current status of gdb should work fine in my use case, I'll close it right away
package-review@lists.fedoraproject.org
-
bugzilla＠redhat.com