Bug 1859627 - Review Request: arm-none-eabi-gdb - GDB for (remote) debugging ARM bare-metal targets
Summary: Review Request: arm-none-eabi-gdb - GDB for (remote) debugging ARM bare-metal...
Keywords:
Status: CLOSED NOTABUG
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
unspecified
medium
Target Milestone: ---
Assignee: Nobody's working on this, feel free to take it
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: FE-NEEDSPONSOR
TreeView+ depends on / blocked
 
Reported: 2020-07-22 15:37 UTC by Austin Chang
Modified: 2022-12-17 09:43 UTC (History)
8 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2022-12-17 09:43:26 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)
licensecheck.txt (721.30 KB, text/plain)
2020-10-05 19:00 UTC, Andy Mender
no flags Details

Description Austin Chang 2020-07-22 15:37:13 UTC
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.src.rpm
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.

This is my first package so I need a sponsor. If I did anything wrong please point them out.
Fedora Account System Username: austin880625

Comment 1 Andy Mender 2020-07-22 20:15:10 UTC
> 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=PackageMaintainers/Join

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 :).

Comment 2 Andy Mender 2020-07-22 20:32:44 UTC
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.

Comment 3 Austin Chang 2020-07-22 22:08:42 UTC
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.

Comment 4 Austin Chang 2020-07-24 09:40:53 UTC
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-gdb.spec
SRPM URL: https://people.cs.nctu.edu.tw/~austin/rpmbuild/202007241716/arm-none-eabi-gdb-9.2-1.fc32.src.rpm

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.

Comment 5 Andy Mender 2020-07-25 22:34:37 UTC
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)

Comment 6 Andy Mender 2020-07-25 22:43:41 UTC
Ah, I'm terribly sorry! This is a review for another package. I will tackle yours soon.

Comment 7 Andy Mender 2020-07-26 10:13:05 UTC
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/1575927/
COPR build with the "pkgconfig(mpfr)" workaround: https://copr.fedorainfracloud.org/coprs/andymenderunix/arm-none-eabi/build/1575928/

> %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> - 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)

Comment 8 Austin Chang 2020-08-01 09:09:58 UTC
(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)?

Comment 9 Andy Mender 2020-08-01 15:07:58 UTC
> 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-compiling_GCC_tool-chains)
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.

Comment 10 Andy Mender 2020-10-03 14:50:04 UTC
Hello Austin, any updates on this?

Comment 11 Austin Chang 2020-10-03 15:13:05 UTC
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.

Comment 12 Austin Chang 2020-10-03 16:36:38 UTC
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-gdb.spec
SRPM: https://people.cs.nctu.edu.tw/~austin/rpmbuild/202010040006/arm-none-eabi-gdb-9.2-1.fc32.src.rpm

Comment 13 Andy Mender 2020-10-05 18:59:39 UTC
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)

Comment 14 Andy Mender 2020-10-05 19:00:42 UTC
Created attachment 1719142 [details]
licensecheck.txt

Output of licensecheck, generated during fedora-review

Comment 15 Austin Chang 2020-10-09 11:19:11 UTC
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-gdb.spec
https://people.cs.nctu.edu.tw/~austin/rpmbuild/202010091905/arm-none-eabi-gdb-9.2-1.fc32.src.rpm

Comment 16 Andy Mender 2020-10-10 11:23:26 UTC
> 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.

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).

> 2. 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.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.

> 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?

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.

Comment 17 Austin Chang 2020-11-02 01:22:03 UTC
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.

Comment 18 Andy Mender 2020-11-10 20:20:21 UTC
> 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.

> 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.

Absolutely correct! ISC is compatible with GPL, thankfully.

> 4. 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.

Comment 19 Andy Mender 2020-12-05 12:35:22 UTC
Hello Austin :). All good or do you need some help with the repository and initial import?

Comment 20 Austin Chang 2020-12-05 13:18:00 UTC
I didn't notice the approved notification. Now the scm request is created(https://pagure.io/releng/fedora-scm-requests/issue/31183).

Comment 21 Andy Mender 2020-12-28 11:07:00 UTC
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

Comment 22 Austin Chang 2021-04-13 13:25:05 UTC
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)?

Comment 23 Jaroslav Kysela 2021-04-26 15:16:14 UTC
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) {

Comment 24 Jaroslav Kysela 2021-04-26 15:33:14 UTC
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-none-eabi-gdb/ .

But I don't see a difference.

Comment 25 Federic 2021-05-30 22:08:54 UTC
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.

Comment 26 Austin Chang 2021-07-07 02:45:22 UTC
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-tools/gnu-toolchain/gnu-rm/downloads) 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.

Comment 27 seb 2021-11-24 18:04:39 UTC
Hello, any news?

Comment 28 Package Review 2022-12-10 16:46:53 UTC
The un-retirement process was never finalized, resetting bug status. Austin, are you still interested in this?

Comment 29 Austin Chang 2022-12-17 09:43:26 UTC
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


Note You need to log in before you can comment on or make changes to this bug.