This is my first package contribution to Fedora and I will need a sponsor.

(In reply to Steven Jay Munroe from comment #0)
> Spec URL: https://github.com/open-power-sdk/pveclib/blob/master/pveclib.spec
> SRPM URL:
> https://copr.fedorainfracloud.org/coprs/munroesj52/pveclib/packages/

Could we please have the complete link to the source rpm also? (fedora-review will not work otherwise.)

https://src.fedoraproject.org/rpms/fedora-review

> Could we please have the complete link to the source rpm also? (fedora-review will not work otherwise.)

Ok I am learning:

https://copr-be.cloud.fedoraproject.org/results/munroesj52/pveclib/fedora-rawhide-ppc64le/00955639-pveclib/pveclib-1.0.2-7.fc31.src.rpm

No worries, also need the "raw" link for the spec so fedora-review can download it:

https://raw.githubusercontent.com/open-power-sdk/pveclib/master/pveclib.spec

I can review this, and I'm also a sponsor so I can sponsor you when the time comes :)

Would you be able to do a few informal reviews yourself while we go through this ticket? That'll help you pick up the process and the tools quicker:
https://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group#Show_Your_Expertise_by_Commenting_on_other_Review_Requests

(When you comment with your review, please mention that it's an informal review because you're still waiting to be sponsored).

- Use built-in macros as much as possible (https://docs.fedoraproject.org/en-US/packaging-guidelines/#_macros):

'%setup -q -n %{name}-%{version}' --> %autosetup

'make %{?_smp_mflags}' --> %make_build

'make install DESTDIR=$RPM_BUILD_ROOT' --> %make_install

'rm -rf %{buildroot}/usr/share/doc/libpvec/ChangeLog.md' --> rm -rf %{buildroot}%{_datadir}/doc/libpvec/ChangeLog.md

- Move %check section after the %install one

- Is it needed to re-run %configure in %check?

- The static libraries MUST be placed in a *-static sub-package.
  (https://docs.fedoraproject.org/en-US/packaging-guidelines/#_packaging_static_libraries)

- The lines

%dir %{_includedir}/pveclib
%{_includedir}/pveclib/vec_common_ppc.h
%{_includedir}/pveclib/vec_f128_ppc.h
%{_includedir}/pveclib/vec_f64_ppc.h
%{_includedir}/pveclib/vec_f32_ppc.h
%{_includedir}/pveclib/vec_int128_ppc.h
%{_includedir}/pveclib/vec_int64_ppc.h
%{_includedir}/pveclib/vec_int32_ppc.h
%{_includedir}/pveclib/vec_int16_ppc.h
%{_includedir}/pveclib/vec_char_ppc.h
%{_includedir}/pveclib/vec_bcd_ppc.h

can be condensed with

%{_includedir}/pveclib/

- The line '%{!?_licensedir:%global license %%doc}' is useless

Oops!

Sorry Ankur.

Ha, no worries Antonio, please feel free to provide comments too :)

Here are some preliminary notes too. The scratch build succeeds: https://koji.fedoraproject.org/koji/taskinfo?taskID=36021962

rpmlint picks up a few important things that need looking into. `rpmlint -i` would give more verbose results:

/home/asinha/rpmbuild/SPECS/pveclib.spec:1: W: mixed-use-of-spaces-and-tabs (spaces: line 1, tab: line 1)
^
Please use either spaces or tabs, it's just good to be consistent.


pveclib.src: W: spelling-error %description -l en_US intrinsics -> intrinsic, intrinsic s, extrinsic
pveclib.src: W: spelling-error %description -l en_US altivec -> elective

^ Can be ignored.


pveclib.src: W: invalid-license APACHE
^
The License is OK, but the right name needs to be picked for the specfile. Please have a look here:
https://fedoraproject.org/wiki/Licensing:Main?rd=Licensing#Good_Licenses

pveclib.ppc64le: W: incoherent-version-in-changelog 1.0.2 ['1.0.2-7.fc31', '1.0.2-7']
^ 
The changelog must reflect the version/release correctly.
https://docs.fedoraproject.org/en-US/packaging-guidelines/#changelogs

You can start with the Release as 1 here. That only needs to be bumped when the spec is changed and a new build is to be made.

pveclib.ppc64le: E: shared-lib-without-dependency-information /usr/lib64/libpvec.so.0.0.0
^
Will have to look deeper to see what this one is about.

pveclib.ppc64le: W: dangling-relative-symlink /usr/share/licenses/pveclib/COPYING LICENSE
^
Please fix this.

> pveclib.src: W: invalid-license APACHE

So you want this spelled out as "Apache Software License 2.0" or the short form "ASL 2.0" ?

> pveclib.ppc64le: E: shared-lib-without-dependency-information /usr/lib64/libpvec.so.0.0.0

The current build only uses the static libraries for some unit tests. As is these libraries only contains arrays of large numeric constants. But this might change in later pveclib version are more complex operations are added.

> pveclib.ppc64le: W: dangling-relative-symlink /usr/share/licenses/pveclib/COPYING LICENSE

looks like I am caught on the horns of the "LICENSE" vs "COPYING" controversy: 

https://stackoverflow.com/questions/5678462/should-i-provide-a-license-txt-or-copying-txt-file-in-my-project

The symlink avoids duplicating the file. While preserving COPYING for lint checks that insist on that file is there.

Not sure why it is "dangling" because it is used in Makefile.am

+dist_license_DATA = COPYING

+dist_doc_DATA = CONTRIBUTING.md README.md ChangeLog.md

Based on  https://bugzilla.redhat.com/show_bug.cgi?id=1536209 I assumed this is what you (Fedora) wanted.

I am working on the other issues.

(In reply to Steven Jay Munroe from comment #9)
> > pveclib.src: W: invalid-license APACHE
> 
> So you want this spelled out as "Apache Software License 2.0" or the short
> form "ASL 2.0" ?

should be the short name from https://fedoraproject.org/wiki/Licensing:Main?rd=Licensing#Good_Licenses
 
> > pveclib.ppc64le: E: shared-lib-without-dependency-information /usr/lib64/libpvec.so.0.0.0
> 
> The current build only uses the static libraries for some unit tests. As is
> these libraries only contains arrays of large numeric constants. But this
> might change in later pveclib version are more complex operations are added.
> 
> > pveclib.ppc64le: W: dangling-relative-symlink /usr/share/licenses/pveclib/COPYING LICENSE
> 
> looks like I am caught on the horns of the "LICENSE" vs "COPYING"
> controversy: 
> 
> https://stackoverflow.com/questions/5678462/should-i-provide-a-license-txt-
> or-copying-txt-file-in-my-project
> 
> The symlink avoids duplicating the file. While preserving COPYING for lint
> checks that insist on that file is there.
> 
> Not sure why it is "dangling" because it is used in Makefile.am
> 
> +dist_license_DATA = COPYING
> 
> +dist_doc_DATA = CONTRIBUTING.md README.md ChangeLog.md
> 
> Based on  https://bugzilla.redhat.com/show_bug.cgi?id=1536209 I assumed this
> is what you (Fedora) wanted.

AFAIK it's OK to have the license text part of the autotools "docs" and then the rpmbuild will care about the details in the background when %license and %doc tags are used in the spec.

making some progress:

> > pveclib.ppc64le: W: dangling-relative-symlink /usr/share/licenses/pveclib/COPYING LICENSE

rpmbuild/rpmlint just can't handle the symlink. I can copy LICENSE over COPYING (to check licensing separate from doc) or rename LICENSE to COPYING (and use COPYING for both).

Either eliminated this warning.

> > pveclib.ppc64le: W: dangling-relative-symlink /usr/share/licenses/pveclib/COPYING LICENSE

This library (currently) has not dependencies (all data).

I can make rpmlint happy by adding the fake dependency: libpvec_la_LIBADD = -lc

> > pveclib-devel.ppc64le: W: no-documentation

Not sure what this is: I have 200 pages of documentation our on github: https://munroesj52.github.io/
But not the king of thing you should put in a man file.

The ChangeLog.md  CONTRIBUTING.md  COPYING  README.md files are installed in /usr/share/doc/pveclib/
and pveclib-1.0.2-1.fc30.ppc64le.rpm but not pveclib-devel-1.0.2-1.fc30.ppc64le.rpm

not sure what I am missing here.

Making progress on clean up of rpmlint issues:

Latest changes in pull request https://github.com/open-power-sdk/pveclib/pull/79 for review.

Current results:
$ rpmlint ./rpmbuild/
pveclib.ppc64le: W: spelling-error %description -l en_US altivec -> elective
pveclib-devel.ppc64le: W: no-documentation
pveclib.src: W: spelling-error %description -l en_US altivec -> elective
5 packages and 2 specfiles checked; 0 errors, 3 warnings.

Not sure is the no-documentation is a show stopper. Not sure what do as man files would be useless and generating PDF would add a lot more dependencies to the build.

Antonio:

I merged most of your suggestions except for:
- The static libraries MUST be placed in a *-static sub-package.

The referenced doc tells me what but not how. I will need examples.

(In reply to Steven Jay Munroe from comment #12)
> Making progress on clean up of rpmlint issues:
> 
> Latest changes in pull request
> https://github.com/open-power-sdk/pveclib/pull/79 for review.
> 
> Current results:
> $ rpmlint ./rpmbuild/
> pveclib.ppc64le: W: spelling-error %description -l en_US altivec -> elective
> pveclib-devel.ppc64le: W: no-documentation
> pveclib.src: W: spelling-error %description -l en_US altivec -> elective
> 5 packages and 2 specfiles checked; 0 errors, 3 warnings.
> 
> Not sure is the no-documentation is a show stopper. Not sure what do as man
> files would be useless and generating PDF would add a lot more dependencies
> to the build.

If the man files are part of the source, they may be included. This isn't a blocker.

> 
> Antonio:
> 
> I merged most of your suggestions except for:
> - The static libraries MUST be placed in a *-static sub-package.
> 
> The referenced doc tells me what but not how. I will need examples.

You define a new sub-package, like the devel package is defined, and add the related files to the new files section. Here is an example package:

https://src.fedoraproject.org/rpms/klt/blob/master/f/klt.spec#_43

Could you please provide links to the new spec/srpm when you've made changes? That way, we always run checks on the newest iterations and don't end up pointing out issues that have already been fixed.

Antonio

>Could you please provide links to the new spec/srpm when you've made changes?

Pull request including spec changes are here: https://github.com/open-power-sdk/pveclib/pull/79

waiting for feed back on the Debian side. Then I will commit and and run a build on COPR

>  You define a new sub-package,...

So ... add?

%package static
	
%description static

%files static

(In reply to Steven Jay Munroe from comment #14)
> Antonio
> 
> >Could you please provide links to the new spec/srpm when you've made changes?
> 
> Pull request including spec changes are here:
> https://github.com/open-power-sdk/pveclib/pull/79
> 
> waiting for feed back on the Debian side. Then I will commit and and run a
> build on COPR

Okay. Great. Please drop a comment with the spec/sprm URLs and I'll go through them again.

> 
> >  You define a new sub-package,...
> 
> So ... add?
> 
> %package static
> 	
> %description static
> 
> %files static

Yes, that'll do it. 
This can be done to add any arbitrary sub-packages. The commonest ones, of course, are -doc, -devel, -static.

So now I am getting:
   pveclib-static.ppc64le: W: no-documentation
in addition to:
   pveclib-devel.ppc64le: W: no-documentation

But by adding:
   %doc README.md
to "%files devel" and "%files static" those warnings go away.

and now I only get:
   $ rpmlint ./rpmbuild/
   pveclib.ppc64le: W: spelling-error %description -l en_US altivec -> elective
   pveclib.src: W: spelling-error %description -l en_US altivec -> elective
   6 packages and 2 specfiles checked; 0 errors, 2 warnings.

Is this acceptable?

Ok I have new copr build

https://copr-be.cloud.fedoraproject.org/results/munroesj52/pveclib/fedora-30-ppc64le/00968950-pveclib/

SRPM: https://copr-be.cloud.fedoraproject.org/results/munroesj52/pveclib/fedora-30-ppc64le/00968950-pveclib/pveclib-1.0.2-1.fc30.src.rpm
SPEC: https://copr-be.cloud.fedoraproject.org/results/munroesj52/pveclib/fedora-30-ppc64le/00968950-pveclib/pveclib.spec

Looks good. A few tweaks are all that are needed now.

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

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


Issues:
=======
- Package installs properly.
^
I'm unable to check this.

- If your application is a C or C++ application you must list a
  BuildRequires against gcc, gcc-c++ or clang.
  Note: No gcc, gcc-c++ or clang found in BuildRequires
  See: https://docs.fedoraproject.org/en-US/packaging-guidelines/C_and_C++/

- ldconfig not called in %post and %postun for Fedora 28 and later.
  Note: /sbin/ldconfig called in pveclib
  See: https://fedoraproject.org/wiki/Changes/Removing_ldconfig_scriptlets
^
No need for them now so they can be removed.


- 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.
  Note: License file COPYING is not marked as %license
  See: https://docs.fedoraproject.org/en-US/packaging-
  guidelines/LicensingGuidelines/#_license_text
^
Is COPYING the same as LICENSE? It should be in %license nevertheless.

- Sources used to build the package match the upstream source, as provided
  in the spec URL.
  Note: Upstream MD5sum check error, diff is in /home/asinha/dump/fedora-
  review/pveclib/review-pveclib/diff.txt
  See: https://docs.fedoraproject.org/en-US/packaging-guidelines/SourceURL/
^
Please check this.

- %configure is called twice---is that intentional?


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

C/C++:
[?]: Provides: bundled(gnulib) in place as required.
     Note: Sources not installed
^
fedora-review picked this up. Not entirely sure if this is needed.

[x]: Package does not contain kernel modules.
[x]: Package contains no static executables.
[x]: Header files in -devel subpackage, if present.
[x]: Package does not contain any libtool archives (.la)
[x]: Rpath absent or only used for internal libs.
[x]: Development (unversioned) .so files in -devel subpackage, if present.

Generic:
[x]: Package successfully compiles and builds into binary rpms on at least
     one supported primary architecture.
     Note: Using prebuilt packages
^
Ran a scratch build:
https://koji.fedoraproject.org/koji/taskinfo?taskID=36361206

[x]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.

[x]: License field in the package spec file matches the actual license.

[x]: License file installed when any subpackage combination is installed.

[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.
[x]: 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.
[x]: Useful -debuginfo package or justification otherwise.
[x]: Package is not known to require an ExcludeArch tag.
[-]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 61440 bytes in 6 files.
[x]: Package complies to the Packaging Guidelines
[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 must own all directories that it creates.
[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 must not depend on deprecated() packages.
[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]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[x]: Static libraries in -static or -devel subpackage, providing -devel if
     present.
     Note: Package has .a files: pveclib-static.
[x]: File names are valid UTF-8.
[x]: Packages must not store files under /srv, /opt or /usr/local

===== SHOULD items =====

Generic:
[x]: Reviewer should test that the package builds in mock.
^
Verified in a scratch build

[-]: 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]: Fully versioned dependency in subpackages if applicable.
[?]: Package functions as described.
^
You'll have to check this

[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[x]: Scriptlets must be sane, if used.
[-]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[x]: Package should compile and build into binary rpms on all supported
     architectures.
[x]: %check is present and all tests pass.
[x]: Packages should try to preserve timestamps of original installed
     files.
[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]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: Sources can be downloaded from URI in Source: tag
[x]: SourceX is a working URL.
[x]: Spec use %global instead of %define unless justified.

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

Generic:
[!]: Rpmlint is run on all installed packages.
^
Couldn't run it on installed packages.

[!]: Package should not use obsolete m4 macros
     Note: Some obsoleted macros found, see the attachment.
     See: https://fedorahosted.org/FedoraReview/wiki/AutoTools

[x]: Large data in /usr/share should live in a noarch subpackage if package
     is arched.


Rpmlint
-------
Checking: pveclib-1.0.2-1.fc31.ppc64le.rpm
          pveclib-devel-1.0.2-1.fc31.ppc64le.rpm
          pveclib-static-1.0.2-1.fc31.ppc64le.rpm
          pveclib-debuginfo-1.0.2-1.fc31.ppc64le.rpm
          pveclib-debugsource-1.0.2-1.fc31.ppc64le.rpm
          pveclib-1.0.2-1.fc31.src.rpm
pveclib.ppc64le: W: spelling-error %description -l en_US altivec -> elective
pveclib.src: W: spelling-error %description -l en_US altivec -> elective
6 packages and 0 specfiles checked; 0 errors, 2 warnings.




Source checksums
----------------
https://github.com/open-power-sdk/pveclib/archive/v1.0.2.tar.gz :
  CHECKSUM(SHA256) this package     : 9fe2ec2a0a16fce087e46f5eb70187f6f86f84ba522f1a371c046d0ef6ba80a5
  CHECKSUM(SHA256) upstream package : a7d063bd6615bf75ac1b6c1bb3e6aa49044d602e84907140b41519792077199c
diff -r also reports differences

^
Please check this


Requires
--------
pveclib (rpmlib, GLIBC filtered):
    /sbin/ldconfig
    libc.so.6()(64bit)
    rtld(GNU_HASH)

pveclib-devel (rpmlib, GLIBC filtered):
    libpvec.so.0()(64bit)
    pveclib(ppc-64)

pveclib-static (rpmlib, GLIBC filtered):
    pveclib(ppc-64)

pveclib-debuginfo (rpmlib, GLIBC filtered):

pveclib-debugsource (rpmlib, GLIBC filtered):



Provides
--------
pveclib:
    libpvec.so.0()(64bit)
    pveclib
    pveclib(ppc-64)

pveclib-devel:
    pveclib-devel
    pveclib-devel(ppc-64)

pveclib-static:
    pveclib-static
    pveclib-static(ppc-64)

pveclib-debuginfo:
    debuginfo(build-id)
    pveclib-debuginfo
    pveclib-debuginfo(ppc-64)

pveclib-debugsource:
    pveclib-debugsource
    pveclib-debugsource(ppc-64)



AutoTools: Obsoleted m4s found
------------------------------
  AC_PROG_LIBTOOL found in: /home/asinha/dump/fedora-review/pveclib/review-
  pveclib/upstream-unpacked/Source0/pveclib-1.0.2/configure.ac:15

^
Also worth checking and reporting upstream.


Generated by fedora-review 0.7.2 (65d36bb) last change: 2019-04-09
Command line :/usr/bin/fedora-review -n pveclib -p
Buildroot used: fedora-rawhide-x86_64
Active plugins: C/C++, Shell-api, Generic
Disabled plugins: Ocaml, SugarActivity, fonts, Python, Java, Haskell, R, Perl, PHP
Disabled flags: EPEL6, EPEL7, DISTTAG, BATCH, EXARCH

- If your application is a C or C++ application you must list a
  BuildRequires against gcc, gcc-c++ or clang.
  Note: No gcc, gcc-c++ or clang found in BuildRequires
  See: https://docs.fedoraproject.org/en-US/packaging-guidelines/C_and_C++/

* pveclib.spec [BuildRequires]: Add gcc-c++.


- ldconfig not called in %post and %postun for Fedora 28 and later.
  Note: /sbin/ldconfig called in pveclib
  See: https://fedoraproject.org/wiki/Changes/Removing_ldconfig_scriptlets
^
No need for them now so they can be removed.

[%post, %postun] Depreacted, removed


Is COPYING the same as LICENSE? It should be in %license nevertheless.

Yes but some older distros require/expect it and rpmlint did not like the symlink. So I duplicated LICENSE to COPYING.

I have multiple masters here and trying keep them all happy.

[%license]: Add COPYING.
[%doc]: COPYING here too.

- Sources used to build the package match the upstream source, as provided
  in the spec URL.

not there yet working on it. Trying not the burn another version tag until we have this ready to go.


- %configure is called twice---is that intentional?

Noop, removed the dup.

[?]: Package functions as described.
^
You'll have to check this

Make check verifies this. Tested for -mcpu=power7/8/9 using gcc 6/7/8/9. Fedora tests are GCC9 -mcpu=power8

AutoTools: Obsoleted m4s found
------------------------------
  AC_PROG_LIBTOOL found in: /home/asinha/dump/fedora-review/pveclib/review-
  pveclib/upstream-unpacked/Source0/pveclib-1.0.2/configure.ac:15

^
Also worth checking and reporting upstream.

I found not indication that  is Obsoleted. Found and corrected 2 other cases.

* configure.ac [AC_INIT] Bump version to 1.0.3
AC_CANONICAL_SYSTEM is deprecated,
Replace with AC_CANONICAL_TARGET.
AM_PROG_CC_C_O is deprecated, remove.

Ankur you provided the link to https://pagure.io/FedoraReview above. I assumed that you intended that I used this tool to find issues and address them. 

But When I followed the install instructions and tried "python3 try-fedora-review --help" the tool ask for a password.

It is not what the password is for and the obvious choices (local sudo and Fedora login) did not work.

Some additional clues are needed.

I have a successful build on COPR with using the latest master with a temporary tag v1.0.2x 

https://copr-be.cloud.fedoraproject.org/results/munroesj52/pveclib/fedora-30-ppc64le/00977765-pveclib/
https://copr-be.cloud.fedoraproject.org/results/munroesj52/pveclib/fedora-rawhide-ppc64le/00977765-pveclib/

If this if acceptable I will tag the master as v1.0.3 and update the spec-file to match.

So here is a cleaner build on COPR using the latest master with a temporary tag v1.0.2y.

https://copr-be.cloud.fedoraproject.org/results/munroesj52/pveclib/fedora-30-ppc64le/00978483-pveclib/
https://copr-be.cloud.fedoraproject.org/results/munroesj52/pveclib/fedora-rawhide-ppc64le/00978483-pveclib/

Turns out the build process is really picked about all the version #s matching. And having pveclib.spec in the source tree is a mistake. 

Now the rpmlint is clean expect the spelling of altivec.h:

$ rpmlint ./rpmbuild/
pveclib.ppc64le: W: spelling-error %description -l en_US altivec -> elective
pveclib.src: W: spelling-error %description -l en_US altivec -> elective
6 packages and 1 specfiles checked; 0 errors, 2 warnings.

one weird thing is that the RPMS built by COPR have bad signatures

Error checking signature of /home/sjmunroe/rpmtest/pveclib-debuginfo-1.0.2y-1.fc30.ppc64le.rpm: /home/sjmunroe/rpmtest/pveclib-debuginfo-1.0.2y-1.fc30.ppc64le.rpm: digests SIGNATURES NOT OK

rpm -ql complains: Header V3 RSA/SHA1 Signature, key ID 81dc7181: NOKEY

But the ones I build on Fedora 30 P8 does NOT have these errors. 

I have no idea what this is oe how to deal with this.

(In reply to Steven Jay Munroe from comment #20)
> Ankur you provided the link to https://pagure.io/FedoraReview above. I
> assumed that you intended that I used this tool to find issues and address
> them. 
> 
> But When I followed the install instructions and tried "python3
> try-fedora-review --help" the tool ask for a password.
> 
> It is not what the password is for and the obvious choices (local sudo and
> Fedora login) did not work.
> 
> Some additional clues are needed.

`sudo dnf -y install fedora-review` is sufficient---you needn't fetch if from git. Lots of options though, so please look at man fedora-review to see how to use it. The general usage is:

fedora-review -b review-ticket-number

(In reply to Steven Jay Munroe from comment #19)
> <snip>
> 
> I have multiple masters here and trying keep them all happy.

Ugh, this will not be easy. While they all use RPM, the guidelines for packaging can be quite different. You can either keep different spec files for them or use conditionals if you'd like to keep a single spec.

https://docs.fedoraproject.org/en-US/packaging-guidelines/DistTag/#_conditionals

(I do not know about other distributions, sorry).

(In reply to Steven Jay Munroe from comment #22)
> So here is a cleaner build on COPR using the latest master with a temporary
> tag v1.0.2y.
> 
> https://copr-be.cloud.fedoraproject.org/results/munroesj52/pveclib/fedora-30-
> ppc64le/00978483-pveclib/
> https://copr-be.cloud.fedoraproject.org/results/munroesj52/pveclib/fedora-
> rawhide-ppc64le/00978483-pveclib/
> 
> Turns out the build process is really picked about all the version #s
> matching. And having pveclib.spec in the source tree is a mistake. 

Yes, while the versions of rpms and upstream must match, the rpms can carry tweaks and changes and the release tag incremented each time, accordingly.


> Now the rpmlint is clean expect the spelling of altivec.h:
> 
> $ rpmlint ./rpmbuild/
> pveclib.ppc64le: W: spelling-error %description -l en_US altivec -> elective
> pveclib.src: W: spelling-error %description -l en_US altivec -> elective
> 6 packages and 1 specfiles checked; 0 errors, 2 warnings.

^ That's fine.

> 
> one weird thing is that the RPMS built by COPR have bad signatures
> 
> Error checking signature of
> /home/sjmunroe/rpmtest/pveclib-debuginfo-1.0.2y-1.fc30.ppc64le.rpm:
> /home/sjmunroe/rpmtest/pveclib-debuginfo-1.0.2y-1.fc30.ppc64le.rpm: digests
> SIGNATURES NOT OK
> 
> rpm -ql complains: Header V3 RSA/SHA1 Signature, key ID 81dc7181: NOKEY
> 
> But the ones I build on Fedora 30 P8 does NOT have these errors. 
> 
> I have no idea what this is oe how to deal with this.

I wouldn't worry about it. When built using koji, these should not occur.


The spec looks pretty good now. One last nitpick, but you can change that before you import into SCM:

Please explicitly version the shared objects


%{_libdir}/libpvec.so.0
%{_libdir}/libpvec.so.0.0.0

This ensures that any soname bumps will not slip through.


XXX APPROVED XXXX


I've now sponsored you to the packager group! Welcome!!

Please import the package and build it following the process here:

https://fedoraproject.org/wiki/Join_the_package_collection_maintainers#Add_Package_to_Source_Code_Management_.28SCM.29_system_and_Set_Owner

> Please explicitly version the shared objects

OK

> XXX APPROVED XXXX

Thanks.

I bumped the pveclib tag to v1.0.3 and updated the pveclib.spec to reflect that:
https://github.com/open-power-sdk/fedora/blob/master/pveclib.spec

Another round of tests on to verify I did not miss something then on to fedora-review

> Another round of tests on to verify I did not miss something then on to
> fedora-review

It is not required to run fedora-review on your spec/srpm now since the review here is complete, but please feel free to use fedora-review as a helper tool to run checks and review other's submissions.

So my first attempt to request "fedpkg request-repo" was rejected because I (no one) was not assigned to this bugzilla.

But when tried to click on the assignee link gnome mail crashed and now all I get is "nobody" email popup.

Is this how this works (send email to nobody)? I was expecting more automation.

(In reply to Steven Jay Munroe from comment #27)
> So my first attempt to request "fedpkg request-repo" was rejected because I
> (no one) was not assigned to this bugzilla.

Should work now. Somehow I forgot to assign the bug to myself.

> 
> But when tried to click on the assignee link gnome mail crashed and now all
> I get is "nobody" email popup.
> 
> Is this how this works (send email to nobody)? I was
> expecting more automation.


I reckon you clicked the e-mail address which was a "mailto" link, and so it tried to open in your e-mail client.

(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/pveclib

ok I have been able to:
fedpkg --user munroesj52 clone pveclib

set up and run a mock build:

mock -r default --rebuild pveclib-1.0.3-1.fc30.src.rpm

then import the srpm built by mock

fedpkg --user munroesj52 import  /var/lib/mock/fedora-rawhide-ppc64le/result/pveclib-1.0.3-1.fc31.src.rpm

I had to manually git commit/push (perhaps because I had not set up kerberos/Koji yet).

git add v1.0.3.tar.gz 
git commit -m "Initial import (#1725924)."
git push

Then Koji build against the repo:

fedpkg --user munroesj52 build

with good status:

36741637 build (rawhide, /rpms/pveclib.git:e49d0062cea3bbc86e8542357175abad416ee19b) completed successfully

Note I use --user munroesj52 because my login for the local system is different. Does setting up ~/.fedora.upn remove this issue?

Also Koji claimed to build for: armv7-06, s390x-18, in addition to ppc64le-24. I had assumed that the "ExclusiveArch: ppc %{power64}" would have prevented that?

So what is the next step?

From Fedora updates:

This update was automatically created an hour ago

This update's test gating status has been changed to 'waiting'. an hour ago

This update's test gating status has been changed to 'ignored'. an hour ago

This update can be pushed to stable now if the maintainer wishes an hour ago

This update has been submitted for stable by bodhi. an hour ago

I assume this is normal

> NEW: Fedora-Rawhide-20190802.n.1
...
> ===== ADDED PACKAGES =====
...
> Package: pveclib-1.0.3-1.fc31
> Summary: Library for simplified access to PowerISA vector operations
> RPMs:    pveclib pveclib-devel pveclib-static
> Size:    157.71 KiB

Thanks for all your help.

Built and included for a long time, so let's close the review.