Bug 1615816 - Review Request: R-units - Measurement Units for R Vectors
Summary: Review Request: R-units - Measurement Units for R Vectors
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: José Matos
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2018-08-14 10:09 UTC by Iñaki Ucar
Modified: 2018-08-23 10:32 UTC (History)
2 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2018-08-23 10:32:34 UTC
Type: ---
Embargoed:
jamatos: fedora-review+


Attachments (Terms of Use)

Description Iñaki Ucar 2018-08-14 10:09:30 UTC
Spec URL: https://iucar.fedorapeople.org/pkgs/R-units.spec
SRPM URL: https://iucar.fedorapeople.org/pkgs/R-units-0.6.0-1.fc28.src.rpm

koji scratch build: https://koji.fedoraproject.org/koji/taskinfo?taskID=29062210

Description:
Support for measurement units in R vectors, matrices and arrays: automatic
propagation, conversion, derivation and simplification of units; raising
errors in case of unit incompatibility. Compatible with the POSIXct, Date
and difftime classes. Uses the UNIDATA udunits library and unit database
for unit compatibility checking and conversion.

Fedora Account System Username: iucar

Comment 1 José Matos 2018-08-14 11:01:30 UTC
Hi Iñaki,
  another option for dealing with missing dependencies for the check stage is to run R with the --no-install option:

Here this was taken from R-car:

%check
# Use --no-install until R-leaps is available
%{_bindir}/R CMD check --no-install %{packname}

The advantage of this method is that no patch is required. :-)

I did not look in the patch with detail, so I am leaving here this note for reference. :-)

Comment 2 Iñaki Ucar 2018-08-14 11:15:57 UTC
Indeed, thanks! :) The problem here was that some examples and tests using Suggests were not properly guarded with 'requireNamespace' or 'skip_if_not_installed', so I would have needed to add --no-tests and --no-examples too. I preferred to apply the patch because I sent these changes upstream, and they will be present in a future release.

Comment 3 José Matos 2018-08-14 13:51:16 UTC
Regarding comment 2: that is fair. :-)

BTW the line with
Requires:       R-core R-Rcpp udunits2

is not necessary.

Running fedora-review (in F28 and x86_64) and analyzing the corresponding rpms we can see why:

$ rpm -q --requires results/R-units-0.6.0-1.fc28.x86_64.rpm
R-Rcpp
R-core
libR.so()(64bit)
libc.so.6()(64bit)
libc.so.6(GLIBC_2.14)(64bit)
libc.so.6(GLIBC_2.2.5)(64bit)
libc.so.6(GLIBC_2.4)(64bit)
libexpat.so.1()(64bit)
libgcc_s.so.1()(64bit)
libgcc_s.so.1(GCC_3.0)(64bit)
libm.so.6()(64bit)
libstdc++.so.6()(64bit)
libstdc++.so.6(CXXABI_1.3)(64bit)
libstdc++.so.6(CXXABI_1.3.9)(64bit)
libstdc++.so.6(GLIBCXX_3.4)(64bit)
libstdc++.so.6(GLIBCXX_3.4.11)(64bit)
libstdc++.so.6(GLIBCXX_3.4.21)(64bit)
libstdc++.so.6(GLIBCXX_3.4.9)(64bit)
libudunits2.so.0()(64bit)
rpmlib(CompressedFileNames) <= 3.0.4-1
rpmlib(FileDigests) <= 4.6.0-1
rpmlib(PayloadFilesHavePrefix) <= 4.0-1
rpmlib(PayloadIsXz) <= 5.2-1
rtld(GNU_HASH)
udunits2

not that all the dependencies are already there:

libR.so()(64bit) -> R.core
libudunits2.so.0()(64bit) -> udunits2

Comment 4 Iñaki Ucar 2018-08-14 14:25:15 UTC
You're right, thanks. I've removed R-core and udunits2 from Requires in the SPEC and updated the SRPM accordingly (same links above). Rcpp is needed though, because 'importFrom(Rcpp,evalCpp)' is in the NAMESPACE.

Comment 5 José Matos 2018-08-14 15:24:09 UTC
(In reply to Iñaki Ucar from comment #4)
> You're right, thanks. I've removed R-core and udunits2 from Requires in the
> SPEC and updated the SRPM accordingly (same links above).

The procedure in this case is to bump the release number, add a new changelog indicating the changes and display again the location for the url and srpm.

The advantage of this procedure is that it is always explicit to the possible different reviewers what the version being discussed. While with the online change the version that I have depends on the time I took it.

And yes that has already happened to me (both as the proponent doing the changes in place and as a reviewer having the wrong version). The small inconvenience for you pays off for all, believe me. :-)

If you increase the release number I will formally review and approve the package. :-)

> Rcpp is needed
> though, because 'importFrom(Rcpp,evalCpp)' is in the NAMESPACE.

I spent time cheking why Rcpp the lib is not automatically linked into R-units and it seems that only the headers are used so you are right.

Comment 6 Iñaki Ucar 2018-08-14 15:37:31 UTC
Spec URL: https://iucar.fedorapeople.org/pkgs/R-units.spec
SRPM URL: https://iucar.fedorapeople.org/pkgs/R-units-0.6.0-2.fc28.src.rpm

Changelog updated, new SRPM generated. Thanks!

Comment 7 José Matos 2018-08-14 16:25:50 UTC
(In reply to Iñaki Ucar from comment #6)
> Spec URL: https://iucar.fedorapeople.org/pkgs/R-units.spec
> SRPM URL: https://iucar.fedorapeople.org/pkgs/R-units-0.6.0-2.fc28.src.rpm
> 
> Changelog updated, new SRPM generated. Thanks!

The package is approved.

Comment 8 Gwyn Ciesla 2018-08-14 17:47:11 UTC
(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/R-units

Comment 9 Fedora Update System 2018-08-15 08:51:20 UTC
R-units-0.6.0-2.fc28 has been submitted as an update to Fedora 28. https://bodhi.fedoraproject.org/updates/FEDORA-2018-64fa40b3cb

Comment 10 Fedora Update System 2018-08-15 20:10:59 UTC
R-units-0.6.0-2.fc28 has been pushed to the Fedora 28 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2018-64fa40b3cb

Comment 11 Fedora Update System 2018-08-23 10:32:34 UTC
R-units-0.6.0-2.fc28 has been pushed to the Fedora 28 stable repository. If problems still persist, please make note of it in this bug report.


Note You need to log in before you can comment on or make changes to this bug.