Bug 1576879

Looking at https://fedoraproject.org/wiki/PackagingDrafts/Go

and docker.spec, it looks like we should do:

BuildRequires: %{?go_compiler:compiler(go-compiler)}%{!?go_compiler:golang >= 1.6.2}

Maybe?  There's also:

ExclusiveArch:  %{go_arches}

Which it looks like docker is carrying a workaround that we could probably drop.

Other than that, LGTM.

Thanks Colin. I'll update, test, and upload changes to my Fedora People space.

Updated and uploaded both spec and SRPM rebuild to https://smilner.fedorapeople.org/ignition/.



[steve@qward SPECS]$ rpmlint  /home/steve/rpmbuild/RPMS/x86_64/ignition-0.23.0-0.4.fc26.x86_64.rpm                          
# not an ssue
ignition.x86_64: W: spelling-error %description -l en_US systemd -> systems, system, system d
# Not an issue
ignition.x86_64: W: spelling-error %description -l en_US networkd -> networks, network, networked
# Expected
ignition.x86_64: W: unstripped-binary-or-object /usr/bin/ignition
# Can raise with upstream if this is an issue, but I don't think this would stop inclusion
ignition.x86_64: E: missing-call-to-chdir-with-chroot /usr/bin/ignition
# Expected
ignition.x86_64: W: unstripped-binary-or-object /usr/bin/ignition-validate
# Expected
ignition.x86_64: W: no-manual-page-for-binary ignition
# Expected
ignition.x86_64: W: no-manual-page-for-binary ignition-validate
1 packages and 0 specfiles checked; 1 errors, 6 warnings.


[steve@qward SPECS]$ rpmlint ignition.spec 
0 packages and 1 specfiles checked; 0 errors, 0 warnings.

I'm not sure of what's typical here so forgive my ignorance, but Ignition isn't particularly useful without an appropriate dracut configuration. Would the corresponding dracut module be something that should be included in the Ignition RPM, or would that typically be in a separate RPM?

Derek,

Most likely it would be seperate. It's possible that the ignition package would eventually require said dracut module/config.

I think I agree with derek here. seems like ignition isn't that useful without the dracut modules so why not include them?

In this case so that we can make the stream pull from dist-git to rebuild. If we need to make more modifications to the spec then I'll suggest we move towards direct building for the time being in the stream.

I have a couple of questions here:

1. Why are we disabling debuginfo? That has been working with Go for several releases now.

2. My understanding is that ignition uses dracut modules and has dracut module configuration too. I don't see it in the ignition source tree, though. Where is it, and what will provide it? I would expect a dracut module subpackage, like how other packages do it (e.g. kiwi).

I also think the changes that Colin requested in comment 1 need to be done as well.

You also have an empty message entry in your %changelog. Please fix. :)

 - 0.24 is published.

 - Unbundle vendor, that might require lots of work packaging the missing deps. Use gofed to package these.

%prep
%autosetup -n %{name}-%{version}
rm -rf vendor

Discovering package dependencies
        Class: github.com/ajeddeloh/go-json (golang-github-ajeddeloh-go-json) PkgDB=False
        Class: github.com/aws/aws-sdk-go (golang-github-aws-aws-sdk-go) PkgDB=True
        Class: github.com/coreos/go-semver (golang-github-coreos-go-semver) PkgDB=True
        Class: github.com/coreos/go-systemd (golang-github-coreos-go-systemd) PkgDB=True
        Class: github.com/pin/tftp (golang-github-pin-tftp) PkgDB=False
        Class: github.com/sigma/vmw-guestinfo (golang-github-sigma-vmw-guestinfo) PkgDB=False
        Class: github.com/vincent-petithory/dataurl (golang-github-vincent-petithory-dataurl) PkgDB=False
        Class: github.com/vmware/vmw-ovflib (golang-github-vmware-vmw-ovflib) PkgDB=False

Discovering test dependencies
        Class: github.com/stretchr/testify (golang-github-stretchr-testify) PkgDB=True


 - Consider using https://fedoraproject.org/wiki/More_Go_packaging . See examples: https://eclipseo.fedorapeople.org/golang/

 - Thus I would not use the build script but a standard %build section like this:

%build 
%gobuildroot
%gobuild -o _bin/ignition %{goipath}/internal
%gobuild -o _bin/ignition-validate" %{goipath}/validate

 - Unbundle vendor, that might require lots of work packaging the missing deps. Use gofed to package these.

I don't think we have an interest in doing this right now.  We have higher priority things to do: for example, fixing Ignition to work more nicely with SELinux.

Colin,

Agreed.

(In reply to Colin Walters from comment #10)
>  - Unbundle vendor, that might require lots of work packaging the missing
> deps. Use gofed to package these.
> 
> I don't think we have an interest in doing this right now.  We have higher
> priority things to do: for example, fixing Ignition to work more nicely with
> SELinux.

If you're not going to unbundle (as I agree you should), then you need to do the right thing and document _all_ your bundled dependencies, per the policy: https://fedoraproject.org/wiki/Bundled_Libraries#Requirement_if_you_bundle

Also, please properly post the Spec and Source RPM links every time you update the package. No one can properly review your package if you don't.

yeah. will document bundled dependencies using gofed like I did for the kompose rpm when I originally created it: 

https://src.fedoraproject.org/rpms/kompose/c/abdc98fbde943502c205f3d410991f7081225baa?branch=master

ok updated. ran it through gofed and tried to take notes about my steps.

Here are my notes [1], the original spec generated by gofed [2], and the modified spec I edited [3]. I created a scratch build [4] and an srpm from that [5] can be accessed.  

[1] https://raw.githubusercontent.com/dustymabe/ignition-rpm/master/notes.txt
[2] https://raw.githubusercontent.com/dustymabe/ignition-rpm/cd20621c8cfa1027f7afd76bac7af11899a453a2/ignition.spec.orig
[3] https://raw.githubusercontent.com/dustymabe/ignition-rpm/cd20621c8cfa1027f7afd76bac7af11899a453a2/ignition.spec
[4] https://koji.fedoraproject.org/koji/taskinfo?taskID=27790594
[5] https://kojipkgs.fedoraproject.org//work/tasks/595/27790595/ignition-0.26.0-0.1.git7610725.fc29.src.rpm

Taking this review.

it was requested that I add ignition-dracut as a subpackage. While I originally didn't want to do this because of spec file complexity I gave in (you're welcome Neal :)). Here is updated information spec [1] SRPM [2] scratch build [3]

[1] https://raw.githubusercontent.com/dustymabe/ignition-rpm/3d83b5d89ce2613647a4eb6b21c1a6bc66b52ca7/ignition.spec
[2] https://kojipkgs.fedoraproject.org//work/tasks/2449/27862449/ignition-0.26.0-0.1.git7610725.fc29.src.rpm
[3] https://koji.fedoraproject.org/koji/taskinfo?taskID=27862448

and to make fedora-review tool happy:

Spec URL: https://raw.githubusercontent.com/dustymabe/ignition-rpm/3d83b5d89ce2613647a4eb6b21c1a6bc66b52ca7/ignition.spec
SRPM URL: https://kojipkgs.fedoraproject.org//work/tasks/2449/27862449/ignition-0.26.0-0.1.git7610725.fc29.src.rpm

> %global dracutprovider_prefix %{provider}.%{provider_tld}/%{project}/%{repo}
> %global dracutimport_path     %{provider_prefix}

fedora-review failed to validate the sources because the URL produced for Source1 was wrong. That was cuased by these two not being set to the correct macros. Please fix.

fixed.

Spec URL: https://raw.githubusercontent.com/dustymabe/ignition-rpm/e60050652e3bf43c79ee0b69cfa969b824220131/ignition.spec
SRPM URL: https://kojipkgs.fedoraproject.org//work/tasks/3005/27863005/ignition-0.26.0-0.1.git7610725.fc29.src.rpm

*** Bug 1594378 has been marked as a duplicate of this bug. ***

Package review notes:

[x] Package naming follows guidelines
[x] Bundled components are correctly specified
[x] Licensing is correctly documented
[x] Macros are used consistently
[x] No serious issues from rpmlint that aren't already being worked on
[!] ignition-dracut does not do "Requires: %{name} = %{version}-%{release}"
[!] Unnecessary "%defattr(-,root,root,0755)" in dracut subpackage

Please fix the following issues noted above.

updated:

Spec URL: https://raw.githubusercontent.com/dustymabe/ignition-rpm/a789cdea4942165f80d04e2916d363605a39caa1/ignition.spec

Looks good to me.

PACKAGE APPROVED.

Per request, I officially hand off the package in this review to Dusty Mabe.

(fedrepo-req-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/ignition

(fedrepo-req-admin):  Processing with --force at the request of @mohanboddu