[Bug 1350884] New: Review Request: mspgcc - Rebase of GCC for the MSP430 to TI / Red Hat upstream

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1350884

Bug ID: 1350884
           Summary: Review Request: mspgcc - Rebase of GCC for the MSP430
                    to TI / Red Hat upstream
           Product: Fedora
           Version: rawhide
         Component: Package Review
          Severity: medium
          Assignee: [e-mail]
          Reporter: [e-mail]
        QA Contact: [e-mail]
                CC: [e-mail]

Spec URL:
https://bitbucket.org/nielsenb/mspgcc-fedora/raw/809a95b0ab9a952f2972efccf45...
SRPM URL:
https://bitbucket.org/nielsenb/mspgcc-fedora/downloads/mspgcc-3.5.0.0-3.src....
Description:

The version of GCC for the TI MSP430 family of microcontrollers currently
packaged by Fedora has been deprecated by upstream [1]. This is an updated
package based on the new TI / Red Hat sources [2].

This package also serves as a workaround for bug 1175942. It does not fix the
issue with GNU strip that seems to be the cause of that bug as indicated by bug
1175942, comment 6. This package is not affected by the issue.

[1] - https://sourceforge.net/projects/mspgcc/
[2] - http://www.ti.com/tool/msp430-gcc-opensource

Fedora Account System Username: nielsenb

[Bug 1350884] Review Request: mspgcc - Rebase of GCC for the MSP430 to TI / Red Hat upstream

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1350884

Brandon Nielsen  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

[Bug 1350884] Review Request: mspgcc - Rebase of GCC for the MSP430 to TI / Red Hat upstream

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1350884

--- Comment #1 from Brandon Nielsen  ---
This is my first package, so I am in need of a sponsor. As you can see by the
fact this isn't in my Description, I have already screwed up this process, I
apologize.

This package has successfully built on Koji:
http://koji.fedoraproject.org/koji/taskinfo?taskID=14673869

[Bug 1350884] Review Request: mspgcc - Rebase of GCC for the MSP430 to TI / Red Hat upstream

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1350884

--- Comment #2 from Dominik 'Rathann' Mierzejewski  ---
Some quick drive-by comments:

I think the package name should be at least msp430-gcc if not msp430-elf-gcc.
If you insist on having gcc as a subpackage then maybe name the source package
msp430-elf-toolchain, but I think that's redundant.

%clean section is redundant.

rm -rf %{buildroot} is redundant at the beginning of %install, too.

How are these sources different from what's in Fedora? I can see they are based
on gcc-4.9.1, so it would be nice to see the diff between gcc-4.9.1 and the
included sources.

A lot of code is bundled, some of which is also packaged in Fedora already
(binutils, dejagnu, elfcpp, gas, gcc, gdb, gmp, gold, gprof, itcl, libmpc,
mpfr, tcl, tk, zlib and all the lib* directories in mspgcc/sources/tools).

Those that are not used during build should be deleted in %prep and those that
can't be unbundled should be declared using Provides: bundled(foo) = version.

[Bug 1350884] Review Request: mspgcc - Rebase of GCC for the MSP430 to TI / Red Hat upstream

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1350884

--- Comment #3 from Brandon Nielsen  ---
New spec URL:
https://bitbucket.org/nielsenb/mspgcc-fedora/raw/3b5371a0a86ce831c5a97deca05...

New SRPM URL:
https://bitbucket.org/nielsenb/mspgcc-fedora/downloads/msp430-elf-toolchain-...

(In reply to Dominik 'Rathann' Mierzejewski from comment #2)
> Some quick drive-by comments:
>
> I think the package name should be at least msp430-gcc if not
> msp430-elf-gcc. If you insist on having gcc as a subpackage then maybe name
> the source package msp430-elf-toolchain, but I think that's redundant.

I went with msp430-elf-toolchain for now, since the TI upstream ships gdb with
the gcc source.
> %clean section is redundant.

Fixed.
> rm -rf %{buildroot} is redundant at the beginning of %install, too.

Fixed.
> How are these sources different from what's in Fedora? I can see they are
> based on gcc-4.9.1, so it would be nice to see the diff between gcc-4.9.1
> and the included sources.

I'll look into this more. It's a little hard to easily get a diff that makes
sense since TI bundles so much code.
> A lot of code is bundled, some of which is also packaged in Fedora already
> (binutils, dejagnu, elfcpp, gas, gcc, gdb, gmp, gold, gprof, itcl, libmpc,
> mpfr, tcl, tk, zlib and all the lib* directories in mspgcc/sources/tools).
>
> Those that are not used during build should be deleted in %prep and those
> that can't be unbundled should be declared using Provides: bundled(foo) =
> version.

I now remove everything that isn't required for building as Fedora provides the
necessary headers. I also remove everything that I'm pretty sure isn't commonly
used when doing MSP430 development.

I'm a little confused about adding 'Provides'. Would I do Provides:
bundled(msp430-elf-foo) = version? The version of foo I'd provide isn't general
use, the object files would be specific to msp430 build targets.

Would I instead be better served to add an msp430-elf-foo subpackage? For
example, look at how libgcc is handled in the current gcc specfile.

[Bug 1350884] Review Request: mspgcc - Rebase of GCC for the MSP430 to TI / Red Hat upstream

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1350884

Till Maas  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #5 from Till Maas  ---
Hi Brandon, would you please consider unofficially reviewing other review
requests to show that you know the packaging guidelines and link the reviews
here?

[Bug 1350884] Review Request: mspgcc - Rebase of GCC for the MSP430 to TI / Red Hat upstream

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1350884

--- Comment #6 from Brandon Nielsen  ---
Did a few informal reviews:

https://bugzilla.redhat.com/show_bug.cgi?id=1387927
https://bugzilla.redhat.com/show_bug.cgi?id=1378095
https://bugzilla.redhat.com/show_bug.cgi?id=1382850

[Bug 1350884] Review Request: mspgcc - Rebase of GCC for the MSP430 to TI / Red Hat upstream

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1350884

--- Comment #7 from Brandon Nielsen  ---
New spec URL:
https://bitbucket.org/nielsenb/mspgcc-fedora/raw/02c17fe25211ee2243391d14664...

New SRPM URL:
https://bitbucket.org/nielsenb/mspgcc-fedora/downloads/msp430-elf-toolchain-...

No longer assume manpages will be gzipped, as per packaging guidelines. Fixes
an improper library strip.

[Bug 1350884] Review Request: mspgcc - Rebase of GCC for the MSP430 to TI / Red Hat upstream

https://bugzilla.redhat.com/show_bug.cgi?id=1350884

Elliott Sales de Andrade  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #8 from Elliott Sales de Andrade  ---
Since the old packages still exist, would it be helpful to mark these as
Obsolete-ing the others?

[Bug 1350884] Review Request: mspgcc - Rebase of GCC for the MSP430 to TI / Red Hat upstream

https://bugzilla.redhat.com/show_bug.cgi?id=1350884

--- Comment #9 from Brandon Nielsen  ---
(In reply to Elliott Sales de Andrade from comment #8)
> Since the old packages still exist, would it be helpful to mark these as
> Obsolete-ing the others?

That's definitely a good idea.

Upstream changed how the project is distributed and built back in January and
I'm only just getting around to changing my specfile to work with the new
versions.

I'll hopefully have an updated specfile and SRPM ready to go soon.

[Bug 1350884] Review Request: mspgcc - Rebase of GCC for the MSP430 to TI / Red Hat upstream

https://bugzilla.redhat.com/show_bug.cgi?id=1350884

--- Comment #10 from Brandon Nielsen  ---
New spec URL:
https://bitbucket.org/nielsenb/mspgcc-fedora/raw/91abb9d0f9654defd6c5538cf19...

New SRPM URL:
https://bitbucket.org/nielsenb/mspgcc-fedora/downloads/msp430-elf-toolchain-...

msp430-elf-gcc now obsoletes msp430-gcc. Rebased to the TI / Somnium upstream.

[Bug 1350884] Review Request: mspgcc - Rebase of GCC for the MSP430 to TI / Red Hat upstream

https://bugzilla.redhat.com/show_bug.cgi?id=1350884

--- Comment #11 from Elliott Sales de Andrade  ---
Bit weird that it's version 5.0.0, but 6.2.1 of gcc (and presumably some other
version of the other tools). Not sure what should be done about that.

Minor things out of the spec: Group tag and %defattr are no longer needed; g++
stuff should be its own subpackage, no?

Some issues I see out of fedora-review:

- Provides: bundled(gnulib) in place as required.
  Note: Bundled gnulib but no Provides: bundled(gnulib)
  See:

http://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries#Requirement_if_...

- Package does not contain any libtool archives (.la)
  Note: msp430-elf-gcc :
  /usr/lib64/gcc/msp430-elf/6.2.1/plugin/libcc1plugin.la msp430-elf-gcc :
  /usr/libexec/gcc/msp430-elf/6.2.1/liblto_plugin.la msp430-elf-gcc :
  /usr/msp430-elf/lib/430/libssp.la msp430-elf-gcc :
  /usr/msp430-elf/lib/430/libssp_nonshared.la msp430-elf-gcc :
  /usr/msp430-elf/lib/430/libstdc++.la msp430-elf-gcc :
  /usr/msp430-elf/lib/430/libsupc++.la msp430-elf-gcc :
  /usr/msp430-elf/lib/large/libssp.la msp430-elf-gcc :
  /usr/msp430-elf/lib/large/libssp_nonshared.la msp430-elf-gcc :
  /usr/msp430-elf/lib/large/libstdc++.la msp430-elf-gcc :
  /usr/msp430-elf/lib/large/libsupc++.la msp430-elf-gcc :
  /usr/msp430-elf/lib/libssp.la msp430-elf-gcc :
  /usr/msp430-elf/lib/libssp_nonshared.la msp430-elf-gcc :
  /usr/msp430-elf/lib/libstdc++.la msp430-elf-gcc :
  /usr/msp430-elf/lib/libsupc++.la
  See: http://fedoraproject.org/wiki/Packaging/Guidelines#StaticLibraries

- SHOULD items: Generic:
[!]: Uses parallel make %{?_smp_mflags} macro.
[!]: 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/latest/exports/msp430-gcc-6.2.1
     .16_source-full.tar.bz2
     See: http://fedoraproject.org/wiki/Packaging:Guidelines#Tags

These match gcc, so probably fine, but I did not check every file:
- Header files in -devel subpackage, if present.
- Static libraries in -static or -devel subpackage, providing -devel if
  present.
  Note: Package has .a files: msp430-elf-gcc. Illegal package name:
msp430-elf-gcc. Does not provide -static: msp430-elf-gcc.
  See: http://fedoraproject.org/wiki/Packaging/Guidelines#StaticLibraries

Possibly relevant things from rpmlint:
msp430-elf-toolchain-debuginfo.x86_64: E: debuginfo-without-sources
msp430-elf-gcc.x86_64: W: obsolete-not-provided msp430-gcc
msp430-elf-toolchain.src:42: W: unversioned-explicit-obsoletes msp430-gcc

[Bug 1350884] Review Request: mspgcc - Rebase of GCC for the MSP430 to TI / Red Hat upstream

https://bugzilla.redhat.com/show_bug.cgi?id=1350884

--- Comment #12 from Brandon Nielsen  ---
New spec URL:
https://bitbucket.org/nielsenb/mspgcc-fedora/raw/d0f1b3ccb1611c8adf5aacf0f63...

New SRPM URL:
https://bitbucket.org/nielsenb/mspgcc-fedora/downloads/msp430-elf-toolchain-...

(In reply to Elliott Sales de Andrade from comment #11)
> Bit weird that it's version 5.0.0, but 6.2.1 of gcc (and presumably some
> other
> version of the other tools). Not sure what should be done about that.

I agree, their versioning scheme is incredibly confusing. Doubly so since they
changed from the Red Hat developed SDK to the new SOMNIUM one, some version
numbers have actually gone backwards. Since we haven't had the packages in
Fedora, it hasn't been an issue, but it did break my Copr for awhile until I
noticed. I'm not really positive which upstream we should use version numbers
for, upstream's upstream (GCC), or just upstream (SOMNIUM).
> Minor things out of the spec: Group tag and %defattr are no longer needed;
> g++ stuff should be its own subpackage, no?

Got rid of the group tag and %defattr. g++ stuff was included in the now dead
msp430-gcc, I started out trying to match that as much as possible. I'm open to
changing it. Do you know of an example I could look at?
> Some issues I see out of fedora-review:
> Provides: bundled(gnulib) in place as required.
> Note: Bundled gnulib but no Provides: bundled(gnulib)
> See:
>
> http://fedoraproject.org/wiki/Packaging:
> No_Bundled_Libraries#Requirement_if_you_bundle

Fixed (I think). The bundled(gnulib) needs to be versioned, but which version
do I specify?
> Package does not contain any libtool archives (.la)
> Note: msp430-elf-gcc :
> /usr/lib64/gcc/msp430-elf/6.2.1/plugin/libcc1plugin.la msp430-elf-gcc :
> /usr/libexec/gcc/msp430-elf/6.2.1/liblto_plugin.la msp430-elf-gcc :
> /usr/msp430-elf/lib/430/libssp.la msp430-elf-gcc :
> /usr/msp430-elf/lib/430/libssp_nonshared.la msp430-elf-gcc :
> /usr/msp430-elf/lib/430/libstdc++.la msp430-elf-gcc :
> /usr/msp430-elf/lib/430/libsupc++.la msp430-elf-gcc :
> /usr/msp430-elf/lib/large/libssp.la msp430-elf-gcc :
> /usr/msp430-elf/lib/large/libssp_nonshared.la msp430-elf-gcc :
> /usr/msp430-elf/lib/large/libstdc++.la msp430-elf-gcc :
> /usr/msp430-elf/lib/large/libsupc++.la msp430-elf-gcc :
> /usr/msp430-elf/lib/libssp.la msp430-elf-gcc :
> /usr/msp430-elf/lib/libssp_nonshared.la msp430-elf-gcc :
> /usr/msp430-elf/lib/libstdc++.la msp430-elf-gcc :
> /usr/msp430-elf/lib/libsupc++.la
> See: http://fedoraproject.org/wiki/Packaging/Guidelines#StaticLibraries

Fixed.
> SHOULD items: Generic:
>
> [!]: Uses parallel make %{?_smp_mflags} macro.
> [!]: 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/latest/exports/msp430-
> gcc-6.2.1
>      .16_source-full.tar.bz2
>      See: http://fedoraproject.org/wiki/Packaging:Guidelines#Tags

I now use parallel make, the download link works for me, so not sure what to
make of that.
> These match gcc, so probably fine, but I did not check every file:
> Header files in -devel subpackage, if present.
> Static libraries in -static or -devel subpackage, providing -devel if
> present.
> Note: Package has .a files: msp430-elf-gcc. Illegal package name:
>
> msp430-elf-gcc. Does not provide -static: msp430-elf-gcc.
>   See: http://fedoraproject.org/wiki/Packaging/Guidelines#StaticLibraries

Anything specific I should do to address these? I'm particular worried about
the illegal package name, especially since we've already looked at naming once.
> Possibly relevant things from rpmlint:
> msp430-elf-toolchain-debuginfo.x86_64: E: debuginfo-without-sources
> msp430-elf-gcc.x86_64: W: obsolete-not-provided msp430-gcc
> msp430-elf-toolchain.src:42: W: unversioned-explicit-obsoletes msp430-gcc

I don't see the debuginfo-without-sources warning on my system.

msp430-gcc isn't packaged in Fedora 26, and I used an unversioned obsolete
since this supersedes msp430-gcc in every way.

[Bug 1350884] Review Request: mspgcc - Rebase of GCC for the MSP430 to TI / Red Hat upstream

https://bugzilla.redhat.com/show_bug.cgi?id=1350884

--- Comment #13 from Elliott Sales de Andrade  ---
For g++ subpackage, I'd check original gcc, maybe.  Same for gnulib, assuming
it has also been bundling it. The illegal package name has to do with including
static files in a package name without -static suffix, which is likely not
relevant here.

I'd say pinging someone from the Embedded SIG might be useful:
https://fedoraproject.org/wiki/Embedded

[Bug 1350884] Review Request: mspgcc - Rebase of GCC for the MSP430 to TI / Red Hat upstream

https://bugzilla.redhat.com/show_bug.cgi?id=1350884

--- Comment #14 from Brandon Nielsen  ---
(In reply to Elliott Sales de Andrade from comment #13)
> For g++ subpackage, I'd check original gcc, maybe.  Same for gnulib,
> assuming it has also been bundling it. The illegal package name has to do
> with including static files in a package name without -static suffix, which
> is likely not relevant here.
>
> I'd say pinging someone from the Embedded SIG might be useful:
> https://fedoraproject.org/wiki/Embedded

New spec URL:
https://bitbucket.org/nielsenb/mspgcc-fedora/raw/e291164cb85b8d82603e5e4f2dc...

New SRPM URL:
https://bitbucket.org/nielsenb/mspgcc-fedora/downloads/msp430-elf-toolchain-...

I split out the C++ stuff. Original gcc doesn't bundle gnulib, but the bundled
libiberty is not versioned.

I'll check out the Embedded SIG when I get a chance.

[Bug 1350884] Review Request: mspgcc - Rebase of GCC for the MSP430 to TI / Red Hat upstream

https://bugzilla.redhat.com/show_bug.cgi?id=1350884

--- Comment #15 from Elliott Sales de Andrade  ---
So a couple things:
* the URL in the spec does not work anymore,
* the build fails due to missing debugsource files; this is probably related to
the debuginfo/debugsource split and not too difficult to fix.

Any chance to ping the Embedded SIG?

[Bug 1350884] Review Request: mspgcc - Rebase of GCC for the MSP430 to TI / Red Hat upstream

https://bugzilla.redhat.com/show_bug.cgi?id=1350884

--- Comment #16 from Brandon Nielsen  ---
(In reply to Elliott Sales de Andrade from comment #15)
> So a couple things:
> the URL in the spec does not work anymore,
> the build fails due to missing debugsource files; this is probably related
>
> to the debuginfo/debugsource split and not too difficult to fix.
>
> Any chance to ping the Embedded SIG?

Upstream changed things around again, it's being maintained by Mitto Systems
Limited now, the earlier version was maintained by Somnium. The URLs got
shuffled around in the changeover.

I've only just got my head wrapped around the changes and started updating the
specfile. I'll ping the Embedded SIG for their thoughts once I'm done.

[Bug 1350884] Review Request: mspgcc - Rebase of GCC for the MSP430 to TI / Red Hat upstream

https://bugzilla.redhat.com/show_bug.cgi?id=1350884

--- Comment #17 from Brandon Nielsen  ---
New spec URL:
https://bitbucket.org/nielsenb/mspgcc-fedora/raw/c1bb3dd343d496c17d7adf11c8d...

New SRPM URL:
https://bitbucket.org/nielsenb/mspgcc-fedora/downloads/msp430-elf-toolchain-...

Rpmlint still shows the download link as broken, but I believe that's because
it doesn't follow redirects? TI appears to redirect twice before the actual
download.

I see the following during the build:

extracting debug info from
/home/nielsenb/rpmbuild/BUILDROOT/msp430-elf-toolchain-6.0.1.0-1.x86_64/usr/lib64/gcc/msp430-elf/7.3.1/plugin/libcp1plugin.so.0.0.0
/usr/lib/rpm/sepdebugcrcfix: Updated 35 CRC32s, 0 CRC32s did match.
cpio: msp430-gcc-7.3.1.24-source-full/build/binutils/binutils/arlex.c: Cannot
stat: No such file or directory
cpio: msp430-gcc-7.3.1.24-source-full/build/binutils/binutils/arlex.l: Cannot
stat: No such file or directory
cpio: msp430-gcc-7.3.1.24-source-full/build/binutils/binutils/arparse.c: Cannot
stat: No such file or directory
cpio: msp430-gcc-7.3.1.24-source-full/build/binutils/binutils/arparse.h: Cannot
stat: No such file or directory
cpio: msp430-gcc-7.3.1.24-source-full/build/binutils/binutils/arparse.y: Cannot
stat: No such file or directory
cpio: msp430-gcc-7.3.1.24-source-full/build/binutils/ld/ldgram.c: Cannot stat:
No such file or directory
cpio: msp430-gcc-7.3.1.24-source-full/build/binutils/ld/ldgram.h: Cannot stat:
No such file or directory
cpio: msp430-gcc-7.3.1.24-source-full/build/binutils/ld/ldgram.y: Cannot stat:
No such file or directory
cpio: msp430-gcc-7.3.1.24-source-full/build/binutils/ld/ldlex.c: Cannot stat:
No such file or directory
cpio: msp430-gcc-7.3.1.24-source-full/build/binutils/ld/ldlex.l: Cannot stat:
No such file or directory
cpio: msp430-gcc-7.3.1.24-source-full/build/gcc/gcc/cfns.gperf: Cannot stat: No
such file or directory
cpio: msp430-gcc-7.3.1.24-source-full/build/gdb/opcodes/msp430-decode.opc:
Cannot stat: No such file or directory
149256 blocks

The resulting compiler seems to work, and it's not clear to me what step of the
build is generating the errors.

Finally, poked my head into the Embedded SIG IRC channel, but it was just me
and ChanServ. I'll check in again when I have some free time.