Bug 460600 (msp430-binutils)

Summary: Review Request: msp430-binutils - Cross compiling binutils targeted at the msp430
Product: [Fedora] Fedora Reporter: Rob Gilton <rob>
Component: Package ReviewAssignee: Steve Whitehouse <swhiteho>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: fedora-package-review, itamar, jan.kratochvil, notting, susi.lehtola, swhiteho, tcallawa
Target Milestone: ---Flags: swhiteho: fedora-review+
kevin: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2009-01-29 23:02:31 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description Rob Gilton 2008-08-28 22:14:16 UTC
Spec URL: http://users.ecs.soton.ac.uk/rds204/rpms/mspgcc/msp430-binutils.spec
SRPM URL: http://users.ecs.soton.ac.uk/rds204/rpms/mspgcc/msp430-binutils-2.18-1.fc9.src.rpm
Description: A cross compiling version of binutils configured to assemble and link binaries for the MSP430 microcontrollers.  Patches from the mspgcc project (http://mspgcc.sf.net).

I based the specfile on the avr-binutils specfile (reviewed in #234750).

rpmlint gives one warning complaining that the package creates /usr/msp430 ("W: non-standard-dir-in-usr msp430"), but I believe that this isn't a problem, as the avr-libc package has a similar warning associated with it about /usr/avr.

Also, I have included the '--disable-nls' configure flag (see the discussion about this in the avr-binutils bug mentioned above) since the native binutils provides the translation files.

This is my first package, and so I need a sponsor :-)

Comment 1 Rob Gilton 2008-12-13 00:09:18 UTC
I've rebuilt this for Fedora 10.  The specfile stayed the same.

Spec URL: http://users.ecs.soton.ac.uk/rds/rpm/mspgcc/msp430-binutils.spec
SRPM URL: http://users.ecs.soton.ac.uk/rds/rpm/mspgcc/msp430-binutils-2.18-1.fc10.src.rpm

Is there something that I haven't done?  It seems to be taking quite a while for this to get reviewed.

Comment 2 Jan Kratochvil 2008-12-23 18:47:11 UTC
(In reply to comment #0)
> This is my first package, and so I need a sponsor :-)

Please check the list of sponsors as described on:
http://fedoraproject.org/wiki/PackageMaintainers/HowToGetSponsored
You may also want to mail <fedora-devel-list>.

With the .spec file I find there these issues:

# Don't want the source
rm -rf $RPM_BUILD_ROOT/usr/src
(1) It has no effect as /usr/src/debug is created _after_ all the %install
    commands.
(2) /usr/src/debug should be present for valid debuginfo package, I do not know
    why you would like to delete it at all.

This argument has any effect? Could you make a comment there, please?
MAKEINFO=`which makeinfo`

IMO this your change was unnecessary, the single line does the some:
-%{_prefix}/%{target}
+%dir %{_prefix}/%{target}
+%dir %{_prefix}/%{target}/lib
+%dir %{_prefix}/%{target}/lib/ldscripts
+%dir %{_prefix}/%{target}/bin
+
+%{_prefix}/%{target}/lib/ldscripts/*
+%{_prefix}/%{target}/bin/*

This is missing there:
# these are for win targets only
rm $RPM_BUILD_ROOT%{_mandir}/man1/%{target}-{dlltool,nlmconv,windres}.1

`rm' commands there miss `-f' which is IMO recommended in general.

Comment 3 Steve Whitehouse 2009-01-13 16:47:41 UTC
*** Bug 479530 has been marked as a duplicate of this bug. ***

Comment 4 Steve Whitehouse 2009-01-13 17:00:15 UTC
Robert, the spec file looks good, except as per Jan's comments. Could you update it and let us know?

It might also be a good plan to consider updating to bintuils 2.19 now, as I think that means you can drop the patches. I am keen to see this package make it into Fedora, so I'll do my best to help move things along.

Comment 5 Steve Whitehouse 2009-01-13 17:04:23 UTC
Robert,

Btw, are you intending to also submit packages for msp430-gdb & msp430-gcc as well? That would be a good set to have.

Comment 6 Rob Gilton 2009-01-13 17:06:31 UTC
Hi Steve,

Sure, I should be able to sort out this RPM later today.

I also have specs queued up for msp430-gcc, msp430-libc and msp430-gdb -- I was waiting on this one before I submitted them.

Rob

Comment 7 Steve Whitehouse 2009-01-13 17:35:02 UTC
Ok, sounds good. Be sure to Cc me (or assign the bz to me if you can) when you submit the review requests for the other packages and I'll take a look at them as soon as I can.

Comment 8 Rob Gilton 2009-01-14 03:41:49 UTC
Hello.

Thanks for the analysis Jan :-)

I've bumped up to binutils 2.19 and I believe I've fixed all the issues that Jan mentioned.

Spec: http://users.ecs.soton.ac.uk/rds/rpm/mspgcc/msp430-binutils.spec
SRPM: http://users.ecs.soton.ac.uk/rds/rpm/mspgcc/msp430-binutils-2.19-1.fc10.src.rpm

The MAKEINFO variable was set because the configure script in binutils 2.18 had a bug that meant makeinfo detection would fail.  Setting MAKEINFO bypassed this test.  Binutils 2.19 has fixed this problem.

I think that I should explain where the patches come from slightly more.  The mspgcc project has maintained these patches against binutils for at least two years now.  They add support for a variety of MSP430 variants.

For 2.19, these patches have been merged into one.  These patches are available in the mspgcc CVS:
http://mspgcc.cvs.sourceforge.net/viewvc/mspgcc/packaging/patches/

I'm not completely sure why these are not in binutils itself.  However, I think that keeping this patch out would just irritate users.  I will post to the mspgcc mailing list about this and try to get that patch into binutils itself.  Not now though, it's very late/early here.

Cheers,

Rob

p.s. Apologies if I sound irritated in this post.  Xorg hung on me and the Firefox session saving thing doesn't save form element contents... so I ended up writing this twice!

Comment 9 Steve Whitehouse 2009-01-14 09:27:24 UTC
The URL you've given still points at the old spec file, but I have installed the SRPM which does have the new spec file in it, and that does look much better now.

I don't think the patch is an issue which should hold up review. Obviously its much better if you can get the binutils maintainers to take it since it reduces the future work that needs to be done, but its fine for now.

Comment 10 Rob Gilton 2009-01-14 12:39:45 UTC
Hi Steve,

Thanks.  I did change the specfile on the server.  I guess it could be a cache issue.

R

Comment 11 Steve Whitehouse 2009-01-14 13:28:50 UTC
Yes, I see now, sorry about that, anyway it looks good to me. I've set the + flag so that you should now be able to submit the request for CVS access. The only thing that I've not done yet is to sponsor you. Its the first time I've done that, so I'm currently enquiring as to the correct way to go about it and I'll try and get the process underway as soon as I can.

Btw, if you are able, it would be helpful if you could also submit the other packages for review now too.

Comment 12 Tom "spot" Callaway 2009-01-14 15:44:37 UTC
Robert: Nice package! Please try to get upstream binutils to take that patch (and put a comment in the spec file showing the mailing list thread or bug submission).

I will sponsor you, since Steve can't. :)

Please pick up the process at this step:
https://fedoraproject.org/wiki/PackageMaintainers/Join#Get_Sponsored

Comment 13 Rob Gilton 2009-01-15 13:36:53 UTC
Hi.

I've mailed the binutils mailing list.  I've already got a response.  The copyright for the patch needs to be assigned to the FSF.  I will attempt to get on top of this soon -- by mailing the mspgcc list.

I've updated the specfile to contain a link to my post.  Available at the same url.  SRPM here: 
http://users.ecs.soton.ac.uk/rds/rpm/mspgcc/msp430-binutils-2.19-2.fc10.src.rpm

Tom: Just to make sure, am I right in thinking that I am now required to demonstrate my ability to follow the Fedora procedures?

Comment 14 Tom "spot" Callaway 2009-01-15 15:50:59 UTC
Well, at least for this package, yes. :) I'm willing to sponsor you based on the cleanliness of the package and the adherence to the Fedora Packaging Guidelines (https://fedoraproject.org/wiki/Packaging/Guidelines). Please read them over and be sure you understand them, if you have not done so.

Then, go ahead and create your fedora account, sign the CLA, and request to be added to the packager group. All of that is documented in the link I posted in the previous comment. I'll sponsor you, then you can continue on to request CVS for this package, import the SRPM, and throw builds at the koji buildsystem.

If you have any questions along the way, feel free to ask them here or email me directly.

Comment 15 Rob Gilton 2009-01-15 21:27:05 UTC
Apologies for the previous confusion.  I had already applied for membership to the packager group, and have completed the CLA.

I have read through the Guidelines previously.  I had another look just now to verify that I'm familiar with them.

Cheers,

Rob

Comment 16 Steve Whitehouse 2009-01-22 13:08:59 UTC
This seems to have stalled again. Robert, do you have everything you need to submit the CVS request now?

Comment 17 Rob Gilton 2009-01-22 13:41:58 UTC
Steve: I'm waiting for my application to the packager group to be approved.  

The guidelines are a little hazy on whether I can submit the CVS request when I'm not yet a member of the packager group.

Comment 18 Steve Whitehouse 2009-01-22 13:49:15 UTC
Tom, can you approve Robert's application now? Then I think we can stop bothering you :-)

Comment 19 Tom "spot" Callaway 2009-01-22 15:08:41 UTC
Sorry, I didn't see Robert's request come across, my spam trap must have caught it. He's sponsored now.

Comment 20 Rob Gilton 2009-01-22 15:23:38 UTC
New Package CVS Request
=======================
Package Name: msp430-binutils
Short Description: Cross Compiling GNU binutils targeted at msp430
Owners: rspanton
Branches: F-10
InitialCC:

Comment 21 Kevin Fenzi 2009-01-23 23:31:46 UTC
cvs done.

Comment 22 Fedora Update System 2009-01-27 01:56:23 UTC
msp430-binutils-2.19-2.fc10 has been pushed to the Fedora 10 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update msp430-binutils'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F10/FEDORA-2009-1052

Comment 23 Steve Whitehouse 2009-01-28 09:45:24 UTC
Robert, you ought to be able to give your F-10 updates package a +1 from yourself. I've given it a +1 and there is another too, so that should be enough to get it released properly.

If you are looking for a second maintainer, then I'm happy to do that. For now though I've only applied for the two "watch" permissions on the package.

Comment 24 Rob Gilton 2009-01-28 14:02:13 UTC
Steve, thanks -- I wasn't sure whether it was "the done thing" to add karma to one's own package.

If you want to be a second maintainer on it, I've not problem with that :-)

I should get around to submitting the msp430-gcc, msp430-libc and msp430-gdb packages for review in the next couple of days.

Comment 25 Steve Whitehouse 2009-01-28 14:16:53 UTC
Yes, I wondered about that too, but I've tended to find that with the packages that I've been working on (with a fairly low user base compared with the core packages) if I don't do that and then specifically ask other people to look at the package, then it can just sit around in limbo forever. I think provided you've checked to see that it works, there should be no problem really.

I'll send the requests through shortly for the other permissions in that case.

It will be really good to get the other packages in too. I guess the one bit we can't do is the gdb proxy bit as it has a funny license, which is a pity I think as it would have made a nice set. Even so, getting the core bits in is really good.

Comment 26 Fedora Update System 2009-01-29 23:02:26 UTC
msp430-binutils-2.19-2.fc10 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 27 Rob Gilton 2009-06-12 14:44:05 UTC
Package Change Request
======================
Package Name: msp430-binutils
New Branches: F-11
Owners: rspanton

Comment 28 Kevin Fenzi 2009-06-14 18:52:14 UTC
There is already a F-11 branch here... 

make sure you do 'cvs update -d' to get new directories...