After discussion with Jakub, compile.sh removed, definition of gobuild and gotest moved to macros. macros.go-compilers now contains:

%gobuild() \
function gobuild { \
local LDFLAGS="${LDFLAGS:-} -B 0x$(head -c20 /dev/urandom|od -An -tx1|tr -d ' \\n')" \
go build -compiler gc    -ldflags "${LDFLAGS:-}" -a -v -x "$@"; \
} \
gobuild %{*}

%gotest() \
function gotest { go test -compiler gc    -ldflags "${LDFLAGS:-}" "$@"; } \
gotest %{*}

Use:
%{gobuild} -o bin/go-md2man %{import_path}
%{gotest} %{import_path}

%{gobuild} can be used multiple times as gobuild function is only redefined. The same holds for %{gotest}. At the moment, the package ships only macros.go-compilers file.

Spec URL: https://jchaloup.fedorapeople.org/reviews/go-compilers/go-compilers.spec

SRPM URL: https://jchaloup.fedorapeople.org/reviews/go-compilers/go-compilers-1-1.fc20.src.rpm

koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=10894598
ppc-koji: http://ppc.koji.fedoraproject.org/koji/taskinfo?taskID=2726756
s390-koji: http://s390.koji.fedoraproject.org/koji/taskinfo?taskID=1939913

Updated definition of gobuild and gotest
- added 'undef -f gobuild' and 'undef -f gotest' so both functions are not redefined if %gobuild/%gotest macros are used multi times.

Besides, the correct use of both macros is without braces:
%gobuild -o bin/go-md2man %{import_path}.

With braces macro's parameters are skipped, so they have to be put inside:
%{gobuild -o bin/go-md2man %{import_path}}

The latter option looks ugly.

Correction: it is 'unset -f', not 'undef -f'

Plus gobuild and gotest macros takes only -o option. For other options rpm reports an error but the building phase continue without any problems. At this point only -o option is relevant as it is common for both golang and gcc-go compiler. Did not make a survey on which options are common. However, I don't want to rely on common options. Let's make the macro as simple and general as possible. For specific compiler options user can create his own %ifelse branches to cover his/her usecase.

As it stands, go-srpm-macros can not be removed from minimal buildroot if %{go_arches} macro is about to be used with ExclusiveArch. Reasoning:

Building of rpm has three phases:
1) generate srpm on a random machine as noarch
2) choose architectures and regenerate noarch srpm as arch specific srpm
3) from arch specific srpm build rpm

At phase 1 and 2, 'rpmbuild -bs --target noarch/arch --nodeps ...' is used with --nodeps options. So when srpm is generated no package tagged as BuildRequires is installed. Srpm is generated in minimal buildroot. So the only way how to get go-srpm-macros into the buildroot where srpm is generated is via redhat-rpm-config and have it installed in the minimal buildroot.

I will update go-compilers and suggest new solution. After discussion with vondruch, one of possible solutions is to remove ExclusiveArch tag a thus no need for go_arches macro. At the same time assuming all architectures has a golang language compiler. Will checkout what is an impact of this decision.

Looking into the .spec file, wouldn't it be better to reverse the virtual provides, e.g.

compiler(go-compiler) -> go(compiler)
compiler(gcc-go) -> go(gcc-go)
compiler(golang) -> go(golang)

Or possibly go(comp9ler_gcc-go), go(compiler_golang)?

As far as I understand these are Go specific macros, so they should be in go() format similarly to every other language specific macros.

And why are you creating platform specific subpackages? They are never created both on single platform if I understand it correctly, so what is the point then?

I would also suggest to include the "macros.go-compilers" files as a SOURCE instead creating them on the fly. One could easier compare them with current state for example and you can save some escaping ...

(In reply to Vít Ondruch from comment #8)
The downside is that you have to replace/expand the %{golang_build}, %{gcc_go_build}, %{golang_test} and %{gcc_go_test} macros in the template, but that is still probably better

> And why are you creating platform specific subpackages? They are never created
> both on single platform if I understand it correctly, so what is the point
> then?

So I can provide compiler(gcc-go) and compiler(golang) among others. Which could be on the other hand ifarched in one package. Good point. It is more like to have package named gcc-go for gcc-go compiler and golang for golang compiler. To have two semantically distinct packages.

Note: ifarch of macros.* files, compiler(gcc-go/golang), Requires and summary will work but it does not look so good as two distinct packages.

> I would also suggest to include the "macros.go-compilers" files as a SOURCE
> instead creating them on the fly. One could easier compare them with current
> state for example and you can save some escaping ...

Point taken. Created macros.golang-compiler and macros.gcc-go-compiler. At the same time I have moved some macros from go-srpm-macros to macros.*-compiler and reduced unnecessary lines in macros definition.

> The downside is that you have to replace/expand the %{golang_build},
> %{gcc_go_build}, %{golang_test} and %{gcc_go_test} macros in the template, but
> that is still probably better

Not needed anymore with the change above.

> Looking into the .spec file, wouldn't it be better to reverse the virtual
> provides, e.g.
>
> compiler(go-compiler) -> go(compiler)
> compiler(gcc-go) -> go(gcc-go)
> compiler(golang) -> go(golang)
> 
> Or possibly go(comp9ler_gcc-go), go(compiler_golang)?
> 
> As far as I understand these are Go specific macros, so they should be in go()
> format similarly to every other language specific macros.>

This is another way of looking at it. I don't mine any of them. What about to provide both ways?

compiler(...) is general and can be provided by any package with a compiler. To get a list of all compiler provided by distribution, you can run 'dnf/yum provides "compiler(*)"' or similar command.

On the other hand go(...) is specific and meant only for golang. Which is more suitable for spec file. However, it is harder to search for it.

I have not found any mention of compiler(...) or similar that would be used in general. At least not for gcc not java.

(In reply to Jan Chaloupka from comment #11)
> compiler(...) is general and can be provided by any package with a compiler.
> To get a list of all compiler provided by distribution, you can run 'dnf/yum
> provides "compiler(*)"' or similar command.

While I agree with this, I don't see any immediate benefit. If that was useful, it should be probably discussed with wider audience. This review is not probably the best place to decide.

What would the use of go-compilers and go-srpm-macros macros?

1)
%if 0%{?go_arches:1}
ExclusiveArch:  %{go_arches}
%else
ExclusiveArch:  %{ix86} x86_64 %{arm}
%endif

If not being optimistic, this will work without go-srpm-macros package as well. So no need for it.

2)
Unit-test subpackage and main package (if necessary) will use:
%if 0%{?fedora}
BuildRequires: compiler(golang)
%endif
...

%if ! 0%{?gobuild:1}
%global gobuild(o) go build %{?**};
%endif

And use would be:
%gobuild -o bin/go-md2man %{import_path}

So again if there is no go-srpm-macros package => no macros => gobuild get defined locally. Of course this functionality is valid only for non-Fedora distribution. As from f22 and higher, golang package will no longer provide its macros (so no gopath macro as well). However, it has to be made sure the macro is present on non-Fedora distros.

This is not the final use case. However, it does not depend on go-compilers package so it can be discussed independently. Just mentioning it.

At the same time there will be no support for debug info with the current definition of gobuild. Of course, the definition can be updated and generate debuginfo no matter what (as defined and mentions in macros.golang-compiler). 

Once go-srpm-macros and go-compilers get into other distribution, the spec file can be further extended/updated. Which is not the case at the moment.

Proposed form and use of ExclusiveArchs and BuildRequires for various architectures and distributions with or without go-srpm-macros packaged.

https://github.com/ingvagabund/gofed/issues/29

Again, minimized number of macros in macros.golang/gcc-go-compilers and redefined gobuild/gotest:

macros.golang-compiler:
# Define commands for building
# BUILD_ID can be generated for golang build no matter of debuginfo
%gobuild(o:) go build -compiler gc -ldflags "${LDFLAGS:-} -B 0x$(head -c20 /dev/urandom|od -An -tx1|tr -d ' \\n')" -a -v -x %{?**};

# Define commands for testing
%gotest() go test -compiler gc -ldflags "${LDFLAGS:-}" %{?**};

macros.gcc-go-compiler is analogous.

Added new macro go_compiler which if set to 1 means that virtual provide compiler(go-compiler) is available. So distributions where there is no such provide can BuildRequire implicit golang compiler.

Spec file is ready for review.

Spec URL: https://jchaloup.fedorapeople.org/reviews/go-compilers/go-compilers.spec

SRPM URL: https://jchaloup.fedorapeople.org/reviews/go-compilers/go-compilers-1-1.fc20.src.rpm

Package spec file seems fine and packages seems to work as intended, only issue is reported by fedora-review (coreutils BR) and it seems minor and easy to fix before/during import.

------

Package Review
==============

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated


Issues:
=======
- All build dependencies are listed in BuildRequires, except for any that
  are listed in the exceptions section of Packaging Guidelines.
  Note: These BR are not needed: coreutils
  See: http://fedoraproject.org/wiki/Packaging/Guidelines#Exceptions_2


===== MUST items =====

Generic:
[x]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[-]: 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.
[x]: License field in the package spec file matches the actual license.
[x]: Package must own all directories that it creates.
     Note: Directories without known owners: /usr/lib/rpm/macros.d,
     /usr/lib/rpm
[x]: %build honors applicable compiler flags or justifies otherwise.
[x]: Package contains no bundled libraries without FPC exception.
[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
[x]: 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.
[-]: Useful -debuginfo package or justification otherwise.
[x]: Package is not known to require an ExcludeArch tag.
[x]: Package complies to the Packaging Guidelines
[x]: Package successfully compiles and builds into binary rpms on at least
     one supported primary architecture.
[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 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]: 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:
[-]: 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).
[x]: Package functions as described.
[-]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[-]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[-]: %check is present and all tests pass.
[x]: Packages should try to preserve timestamps of original installed
     files.
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: Reviewer should test that the package builds in mock.
[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]: SourceX is a working URL.
[x]: Package should compile and build into binary rpms on all supported
     architectures.
[x]: Spec use %global instead of %define unless justified.

===== EXTRA items =====

Generic:
[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).
[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: go-compilers-golang-compiler-1-1.fc22.x86_64.rpm
          go-compilers-1-1.fc22.src.rpm
go-compilers-golang-compiler.x86_64: W: summary-not-capitalized C compiler for golang
go-compilers-golang-compiler.x86_64: W: no-url-tag
go-compilers-golang-compiler.x86_64: W: only-non-binary-in-usr-lib
go-compilers-golang-compiler.x86_64: W: no-documentation
go-compilers.src: W: spelling-error %description -l en_US golang -> Angolan, Golan, Angola
go-compilers.src: W: no-url-tag
go-compilers.src:27: W: unversioned-explicit-provides compiler(go-compiler)
go-compilers.src:28: W: unversioned-explicit-provides compiler(golang)
go-compilers.src:40: W: unversioned-explicit-provides compiler(go-compiler)
go-compilers.src:41: W: unversioned-explicit-provides compiler(gcc-go)
2 packages and 0 specfiles checked; 0 errors, 10 warnings.




Rpmlint (installed packages)
----------------------------
go-compilers-golang-compiler.x86_64: W: summary-not-capitalized C compiler for golang
go-compilers-golang-compiler.x86_64: W: no-url-tag
go-compilers-golang-compiler.x86_64: W: only-non-binary-in-usr-lib
go-compilers-golang-compiler.x86_64: W: no-documentation
1 packages and 0 specfiles checked; 0 errors, 4 warnings.



Requires
--------
go-compilers-golang-compiler (rpmlib, GLIBC filtered):
    golang



Provides
--------
go-compilers-golang-compiler:
    compiler(go-compiler)
    compiler(golang)
    go-compilers-golang-compiler
    go-compilers-golang-compiler(x86-64)

Thanks Jakub.

These could be fixed:
go-compilers.src:27: W: unversioned-explicit-provides compiler(go-compiler)
go-compilers.src:28: W: unversioned-explicit-provides compiler(golang)
go-compilers.src:40: W: unversioned-explicit-provides compiler(go-compiler)
go-compilers.src:41: W: unversioned-explicit-provides compiler(gcc-go)

Will update the spec file in the repository.

New Package SCM Request
=======================
Package Name: go-compilers
Short Description: Go language compilers for various architectures
Upstream URL: 
Owners: jchaloup jcajka fpokorny
Branches: f23 f22 f21 el6
InitialCC: golang-sig

Git done (by process-git-requests).

go-compilers-1-1.fc22 has been submitted as an update to Fedora 22. https://bodhi.fedoraproject.org/updates/FEDORA-2015-15464

go-compilers-1-1.fc23 has been submitted as an update to Fedora 23. https://bodhi.fedoraproject.org/updates/FEDORA-2015-15466

go-compilers-1-1.fc22 has been pushed to the Fedora 22 testing repository. If problems still persist, please make note of it in this bug report.\nIf you want to test the update, you can install it with \n su -c 'yum --enablerepo=updates-testing update go-compilers'. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2015-15464

go-compilers-1-1.fc23 has been pushed to the Fedora 23 testing repository. If problems still persist, please make note of it in this bug report.\nIf you want to test the update, you can install it with \n su -c 'yum --enablerepo=updates-testing update go-compilers'. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2015-15466

go-compilers-1-2.fc23 has been submitted as an update to Fedora 23. https://bodhi.fedoraproject.org/updates/FEDORA-2015-15532

go-compilers-1-2.fc22 has been submitted as an update to Fedora 22. https://bodhi.fedoraproject.org/updates/FEDORA-2015-15533

go-compilers-1-2.fc23 has been pushed to the Fedora 23 testing repository. If problems still persist, please make note of it in this bug report.\nIf you want to test the update, you can install it with \n su -c 'yum --enablerepo=updates-testing update go-compilers'. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2015-15532

go-compilers-1-2.fc22 has been pushed to the Fedora 22 testing repository. If problems still persist, please make note of it in this bug report.\nIf you want to test the update, you can install it with \n su -c 'yum --enablerepo=updates-testing update go-compilers'. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2015-15533

go-compilers-1-2.fc23 has been pushed to the Fedora 23 stable repository. If problems still persist, please make note of it in this bug report.

go-compilers-1-2.fc22 has been pushed to the Fedora 22 stable repository. If problems still persist, please make note of it in this bug report.

go-compilers-1-2.fc21 has been submitted as an update to Fedora 21. https://bodhi.fedoraproject.org/updates/FEDORA-2015-b9da8834ba

go-compilers-1-2.fc21 has been pushed to the Fedora 21 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 'dnf --enablerepo=updates-testing update go-compilers'
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2015-b9da8834ba

jchaloup's scratch build of go-memdb-9ea975b.tar.gz for cli-build/1454938967.533693.wYGVMBzT/go-memdb-9ea975b.tar.gz and f22-candidate failed http://koji.fedoraproject.org/koji/taskinfo?taskID=12902531

The package is in Fedora, I believe this should be closed.