Bug 1992931

Taking for a review.

Rpmlint is complaining about FSF's address in the GPL license file; it seems upstream might have an old license file?

~~~
$ rpmlint ./srpm-unpacked/rubygem-image_size.spec srpm/rubygem-image_size-2.1.1-1.fc34.src.rpm results/rubygem-image_size-2.1.1-1.fc35.noarch.rpm
== rpmlint session starts ==
rpmlint: 2.0.0
configuration:
    /usr/lib/python3.10/site-packages/rpmlint/configdefaults.toml
    /etc/xdg/rpmlint/fedora.toml
    /etc/xdg/rpmlint/licenses.toml
    /etc/xdg/rpmlint/scoring.toml
    /etc/xdg/rpmlint/users-groups.toml
    /etc/xdg/rpmlint/warn-on-functions.toml
checks: 31, packages: 3

rubygem-image_size.noarch: E: incorrect-fsf-address /usr/share/gems/gems/image_size-2.1.1/GPL
~~~

* `export RUBY_LIB=$(pwd)/lib` in %check does not do anything (or shouldn't)
  - Ruby's manpage specifies that the correct variable name is `RUBYLIB`
  - it does not seem to be required to run the test suite (I tested it)

* `%{nil}` is used. I am not really sure what it does? Why is it being supplied to the rm -rf?

Small nits (not blockers):
* Doc subpackage could be noarch.

* You do not have to move the gemspec file in %prep.
  The path a level above can be supplied as well; like so: `gem build ../%{gem_name}-%{version}.gemspec`.

* Consider using `gem2rpm`, which does most of the work for you in the case of RubyGems in a standardized way.

* JFTR, the upstream's gemspec, Gemfile, and `spec/` test suite can be included in the *-doc subpackage as it can be used for a reference.

Thank you for review, I will reply and update the spec in tomorrow (perhaps).

Just some notes:

* %{nil} convention
  This is useful when I want to add additional lines after "spec \" line.
  When not using %{nil}, I have to write "spec" (ends here), and when I want to add additional
  line, I have to change the line to "spec \", while using %nil as "sentinel", this can
  make diff a bit smaller.

* Doc subpackage could be noarch.
  Actually I wonder why gem2rpm writes "noarch" to subpackage... When the main package is
  marked as "BuildArch: noarch", all subpackages becomes noarch automatically.
  (Conversely: we cannot make subpackage arch-dependent when writing main package as
   "BuildArch: noarch")

* You do not have to move the gemspec file in %prep.
  This is to clean up build dir after rebuild.
  When you do "$ rpmbuild --rebuild *.src.rpm" or "$ fedpkg --release f34 local -- --clean",
  you can see:
-------------------------------------------------------
Checking for unpackaged file(s): /usr/lib/rpm/check-files /home/tasaka1/rpmbuild/INSTROOT/rubygem-image_size-2.1.1-1.fc34-foo-tasaka1
Wrote: /home/tasaka1/rpmbuild/fedora-specific/RUBYGEMS/rubygem-image_size/Review/rubygem-image_size-2.1.1-1.fc34.src.rpm
Wrote: /home/tasaka1/rpmbuild/fedora-specific/RUBYGEMS/rubygem-image_size/Review/noarch/rubygem-image_size-2.1.1-1.fc34.noarch.rpm
Wrote: /home/tasaka1/rpmbuild/fedora-specific/RUBYGEMS/rubygem-image_size/Review/noarch/rubygem-image_size-doc-2.1.1-1.fc34.noarch.rpm
Executing(%clean): /bin/sh -e /home/tasaka1/rpmbuild/INSTROOT/rpm-tmp.9WXLyW
+ umask 022
+ cd /home/tasaka1/rpmbuild/fedora-specific/RUBYGEMS/rubygem-image_size/Review
+ cd image_size-2.1.1
+ /usr/bin/rm -rf /home/tasaka1/rpmbuild/INSTROOT/rubygem-image_size-2.1.1-1.fc34-foo-tasaka1
+ RPM_EC=0
++ jobs -p
+ exit 0
Executing(--clean): /bin/sh -e /home/tasaka1/rpmbuild/INSTROOT/rpm-tmp.ENZqEC
+ umask 022
+ cd /home/tasaka1/rpmbuild/fedora-specific/RUBYGEMS/rubygem-image_size/Review
+ rm -rf image_size-2.1.1 <========================================
+ RPM_EC=0
++ jobs -p
+ exit 0
-------------------------------------------------------
  Without moving every unpackaged files under the directory specfied by "%setup -n" option,
  files are not cleaned up after rpmbuild.
  Actually I oppose to use directories outside the one specified by "%setup -n".

* I will ask the upstream to modify FSF address later, however for now I will leave this
  as it is.

Updated:

* For FSF address issue, I will ask the upstream later
* For spec Gemfile and so on, I will keep removing them.

https://mtasaka.fedorapeople.org/Review_request/gem-related/rubygem-image_size.spec
https://mtasaka.fedorapeople.org/Review_request/gem-related/rubygem-image_size-2.1.1-2.fc34.src.rpm

* Wed Aug 18 2021 Mamoru TASAKA <mtasaka> - 2.1.1-2
- Remove unneeded environment setting
- Disable failing test on s390x

* if ( uname -m | grep -q s390x )
  - use the ifarch conditional instead `%ifarch s390x` (improves readability IMO)
  - The issue should be reported upstream if it is not the fault of the Fedora environment.

Other than that the package is LGTM.

* if ( uname -m | grep -q s390x )
1.
%ifarch s390x won't work because the srpm is **noarch** . 
Actually build environment is of course on some target, so "uname -m" 
returns some arch-dependent result, but in this case %ifarch matches "noarch".

2. 
s390x issue needs debugging, currently I have not figure out what component
is fault, I want to debug later, however I don't want to think this is a
blocker.

LGTM.

Package approved.

Thank you!!

By the way, FYI I debugged out s390x issue:
https://github.com/toy/image_size/pull/16 : I will include this patch.

https://pagure.io/releng/fedora-scm-requests/issue/36436
https://pagure.io/releng/fedora-scm-requests/issue/36437
https://pagure.io/releng/fedora-scm-requests/issue/36438
https://pagure.io/releng/fedora-scm-requests/issue/36439

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

Successfully built on rawhide: https://koji.fedoraproject.org/koji/buildinfo?buildID=1823091

Thank you for review! Closing.