This service will be undergoing maintenance at 00:00 UTC, 2016-08-01. It is expected to last about 1 hours

Bug 610934

Summary: Review Request: go - The Google Go Programming Language
Product: [Fedora] Fedora Reporter: Brandon <brandon>
Component: Package ReviewAssignee: Michel Alexandre Salim <michel>
Status: CLOSED DUPLICATE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: low    
Version: rawhideCC: alex.combas, bloch, esm, fedora-package-review, jochen, jvillalo, konrad, michel, mike, notting, supercyper1
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2010-11-13 18:31:47 EST Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:

Description Brandon 2010-07-02 15:21:34 EDT
Spec URL: http://www.pwnage.ca/dist/SPEC/go.spec
SRPM URL: http://www.pwnage.ca/dist/SRPMS/go-0-0.20100621hg.fc13.src.rpm
Description: The Google Go Programming Language
Comment 1 Brandon 2010-07-02 15:22:50 EDT
I forgot to mention this is my first package and I need a sponsor.
Comment 2 Fabian Affolter 2010-07-03 19:04:09 EDT
*** Bug 557366 has been marked as a duplicate of this bug. ***
Comment 3 Michel Alexandre Salim 2010-07-04 10:57:51 EDT
(In reply to comment #1)
> I forgot to mention this is my first package and I need a sponsor.    

no problem on that, I can sponsor. I'll take a look and start providing review feedback.
Comment 4 Jochen Schmitt 2010-07-08 15:59:58 EDT
Regarding of the packaging guidlines the emacs and vim subpackages should be named as emacs-go and vim-go.
Comment 5 Brandon 2010-07-08 16:32:34 EDT
I'll fix those. There was a new release of Go on the 2nd of July, I'll have a new spec soon regardless.
Comment 6 Michel Alexandre Salim 2010-07-11 09:08:34 EDT
Brandon,

When updating for the new release, please do the following:

- for the clone instruction, instead of -r release, please use -r release.2010-07-01 . This makes sure that the instruction would fetch the correct source even after subsequent releases

(aside: kind of curious how the release.2010-07-01 tag does *not* appear in the output of 'hg tags' until after I pull in some changes newer than the tag)

- use hg archive instead of invoking tar yourself. That would strip out the .hg metadata, and for those who track the Go upstream repo (as you, me as the reviewer, and any future co-maintainers likely do), it would avoid having to clone a new copy every time a new release come out because the working tree has been used for compilation. 

- if you're targeting only recent Fedora releases, consider using xz instead of bzip2 for the archive compression.

hg archive -t tar ../go-20100701.tar
cd ..
xz -9e -k go-*.tar # -k only used so the .tar is kept for re-compressing
bzip2 --best -k go-*.tar
ls -l go*.tar*

$ ls -l go*.tar*
-rw-rw-r--. 1 michel michel 20541440 Jul 11 14:54 go-20100701.tar
-rw-rw-r--. 1 michel michel  4700747 Jul 11 14:54 go-20100701.tar.bz2
-rw-rw-r--. 1 michel michel  3762540 Jul 11 14:54 go-20100701.tar.xz

- Here's the Emacs packaging guideline. You'd want to byte-compile the *.el files if possible, and only package the .elc files in the main Emacs subpackage. The source *.el should be in a separate emacs-go-el subpackage.

- there seems to be a bug in the files section. Since 8* is generated on ix86, and 6* on x86_64, you probably want

%ifarch %ix86
%{_bindir}/8*
%else
%{_bindir}/6*
%endif
... common filenames here ...

- you need to own %{_sysconfdir}/bash_completion.d 

- for Fedora 12 and above, BuildRoot no longer needs to be defined. For Fedora 13 and above, %clean is also no longer needed.
http://fedoraproject.org/wiki/Packaging/Guidelines#BuildRoot_tag

So depending on which distributions you want to target, you can remove those. Keeping them is fine too.
Comment 7 Conrad Meyer 2010-07-11 12:44:08 EDT
(In reply to comment #6)
> - there seems to be a bug in the files section. Since 8* is generated on ix86,
> and 6* on x86_64, you probably want
> 
> %ifarch %ix86
> %{_bindir}/8*
> %else
> %{_bindir}/6*
> %endif
> ... common filenames here ...

Nah, some of the binaries are generated as 6* regardless of what architecture you're on.
Comment 8 Michel Alexandre Salim 2010-11-02 12:35:13 EDT
Hi Conrad,

There's been several releases -- the last on October 27th. Is there an updated spec I can review? The pointed-out problems with checkout instruction and subpackages are still present in the spec file listed in the request.

Thanks!
Comment 9 Conrad Meyer 2010-11-02 13:28:07 EDT
It's Brandon's review, not mine.
Comment 10 Michel Alexandre Salim 2010-11-11 07:25:36 EST
Oops, yes, I mean Brandon.

Will close as FE-DEADREVIEW if there is no response by next week.
Comment 11 W. Michael Petullo 2010-11-12 13:05:07 EST
I would be happy to take over the maintenance of this package. I am presently working with several other people on a research OS that uses Go extensively. A Go package would be beneficial to our work. I may have some time this weekend to look at revising the RPM specification if we don't hear from Brandon.
Comment 12 Michel Alexandre Salim 2010-11-12 16:56:14 EST
Hi Michael,

That'd be great. If we don't hear by say Sunday morning, please create a new bug entry and close this one as a duplicate. I'll start the review then.

Thanks! Oh, and can I find out more about this research OS? seems interesting. You write the kernel in Go, or just the userspace?
Comment 13 W. Michael Petullo 2010-11-13 18:31:47 EST

*** This bug has been marked as a duplicate of bug 652987 ***
Comment 14 Brandon 2010-11-14 09:19:58 EST
Take over, I explain why in his new bug.