Bug 975266 - Review Request: cmap-japan1-6 - Japanese character mapping resources from Adobe's cmap
Review Request: cmap-japan1-6 - Japanese character mapping resources from Ado...
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
unspecified Severity medium
: ---
: ---
Assigned To: Lubomir Rintel
Fedora Extras Quality Assurance
:
Depends On:
Blocks: 859246
  Show dependency treegraph
 
Reported: 2013-06-17 21:09 EDT by Ben Rosser
Modified: 2014-08-15 20:33 EDT (History)
5 users (show)

See Also:
Fixed In Version: cmap-japan1-6-2012.08.14-4.fc20
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2014-08-15 20:25:13 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
lkundrak: fedora‑review+
limburgher: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Ben Rosser 2013-06-17 21:09:13 EDT
Spec URL: http://venus.arosser.com/fedora/cmap/cmap-japan.spec
SRPM URL: http://venus.arosser.com/fedora/cmap/cmap-japan-1.6-0.fc18.src.rpm
Description: CMap (Character Map) resources are used to unidirectionally
map character codes, such as a Unicode encoding form, to CIDs
(Characters IDs, meaning glyphs) of a CIDFont resource.

This package contains the Japanese cmap resources.

Fedora Account System Username: tc01

--

The only real question I have left is where the package should be installed- should it be under a cmap subdirectory in /usr/share (as it is at the moment)? If yes, which package (there will be several cmap-blah packages) should own it? Can multiple packages own a directory, so it gets removed if they're all gone but exists as long as some remain on the system?

Or should cmap-japan-1.6 (%{name}-%{version}) just be in /usr/share?
Comment 1 Jason Tibbitts 2013-06-19 20:07:48 EDT
It is OK for /usr/share/cmap to be multiply owned in this case.  http://fedoraproject.org/wiki/Packaging:Guidelines#File_and_Directory_Ownership
Or I guess it wouldn't hurt anything if the cmap-japan-1.6 directory moved one level up, though the current setup arguably looks cleaner.  The only real consideration there, I think, is whether one is easier for the packages which will consume these files.

Since I promised, I'll go ahead and take this ticket for review.  I can't see anything other than the /usr/share/cmap ownership issue which would be a problem, though.
Comment 2 Ben Rosser 2013-06-20 15:45:13 EDT
Alright, then I've fixed that issue- cmap-japan now owns /usr/share/cmap, and the other packages will as well.

Spec URL: http://venus.arosser.com/fedora/cmap/cmap-japan.spec
SRPM URL: http://venus.arosser.com/fedora/cmap/cmap-japan-1.6-1.fc18.src.rpm
Comment 3 Jason Tibbitts 2013-06-20 15:52:28 EDT
Awesome, thanks.  I will be away for a long weekend but I'll try to carve out some time this evening to give this a full review.
Comment 4 Orion Poplawski 2013-10-20 22:01:52 EDT
Ping?
Comment 5 Till Maas 2013-10-21 15:59:28 EDT
According to the other bugs, Ben is not yet sponsored.

Btw. Ping Tibbs, you intended to review this one.
Comment 6 Lubomir Rintel 2014-07-28 11:40:12 EDT
Ben is sponsored now, removing FE_NEEDSPONSOR blocker.
Comment 7 Ben Rosser 2014-07-28 20:31:41 EDT
These are now the correct spec/srpm links (sorry about that!):

Spec URL: http://mars.arosser.com/fedora/cmap/cmap-japan.spec
SRPM URL: http://mars.arosser.com/fedora/cmap/cmap-japan-1.6-1.fc18.src.rpm
Comment 8 Lubomir Rintel 2014-07-29 08:29:28 EDT
Stealing this from Jason since he does not respond.

0.) The package naming and versioning

As you pointed out, the number in the file name is not a version. The package should be named cmap-japan1-6; the question what to take for the version remains open (as only the mappings themselves seem to be versioned separately).

I suggest you either contact upstream and ask for their advice on packaging/versioning, or use the modification date, such as 20120814 or 2012.08.14 for version.

1.) I'm not sure you need to include the two documentation files

Licensing information is embedded in the files themselves and these two files seem to change upstream; we'd need to figure out how to package them then.

2.) This is unnecessary:

mkdir -p %{buildroot}%{_docdir}/cmap/%{name}-%{version}

3.) These belong in %prep (unless you decide to drop them as I suggested):

mkdir _tmpdoc
install -p -m0644 %SOURCE1 %SOURCE2 _tmpdoc

Also, "rm -rf _tmpdoc" is unnecessary.
Comment 9 Ben Rosser 2014-07-29 14:24:45 EDT
I have gotten rid of all the documentation, as suggested.

I've changed the version to 2012.08.14; that seems reasonable enough. I also changed the package name to cmap-japan1-6.

Should I retroactively update the changelog to use the right versioning? I did, but I'm not completely confident it was the right thing to do.

Spec URL: http://mars.arosser.com/fedora/cmap/cmap-japan1-6.spec
SRPM URL: http://mars.arosser.com/fedora/cmap/cmap-japan1-6-2012.08.14-4.fc20.src.rpm
Comment 10 Lubomir Rintel 2014-08-04 05:02:39 EDT
(In reply to Ben Rosser from comment #9)
> I have gotten rid of all the documentation, as suggested.
> 
> I've changed the version to 2012.08.14; that seems reasonable enough. I also
> changed the package name to cmap-japan1-6.

Thank you.

> Should I retroactively update the changelog to use the right versioning? I
> did, but I'm not completely confident it was the right thing to do.

I don't really have a strong opinion on this and I don't believe it matters as the package was not really released. Do whatever seems fine to you.

I'm finishing the review now:

* Package is properly named
* The version is correct
* The license tag is correct
* License good for Fedora
* Full license text included (in each of the files)
* SPEC file clean and legible
* Filelist sane
* Requires/provides make sense
* Package owns all directories it should
* Builds fine in mock
* Rpmlint reasonably happy

The package is APPROVED now.
Please go ahead and file a SCM request: http://fedoraproject.org/wiki/Package_SCM_admin_requests
Comment 11 Ben Rosser 2014-08-04 13:35:54 EDT
New Package SCM Request
=======================
Package Name: cmap-japan1-6
Short Description: The Japanese character resource mappings from the Adobe cmap project. 
Upstream URL: http://sourceforge.net/projects/cmap.adobe/
Owners: tc01
Branches: f19 f20 f21
InitialCC:
Comment 12 Gwyn Ciesla 2014-08-04 14:30:19 EDT
WARNING: Requested package name cmap-japan1-6 doesn't match bug summary
cmap-japan
Comment 13 Ben Rosser 2014-08-04 14:54:58 EDT
Oops, I guess that happens when the package name changes during the review.

Can we try that again?

New Package SCM Request
=======================
Package Name: cmap-japan1-6
Short Description: The Japanese character resource mappings from the Adobe cmap project. 
Upstream URL: http://sourceforge.net/projects/cmap.adobe/
Owners: tc01
Branches: f19 f20 f21
InitialCC:
Comment 14 Gwyn Ciesla 2014-08-04 14:58:19 EDT
Git done (by process-git-requests).
Comment 15 Fedora Update System 2014-08-04 17:06:57 EDT
cmap-japan1-6-2012.08.14-4.fc20 has been submitted as an update for Fedora 20.
https://admin.fedoraproject.org/updates/cmap-japan1-6-2012.08.14-4.fc20
Comment 16 Fedora Update System 2014-08-04 17:22:12 EDT
cmap-japan1-6-2012.08.14-4.fc19 has been submitted as an update for Fedora 19.
https://admin.fedoraproject.org/updates/cmap-japan1-6-2012.08.14-4.fc19
Comment 17 Fedora Update System 2014-08-07 11:36:17 EDT
cmap-japan1-6-2012.08.14-4.fc19 has been pushed to the Fedora 19 testing repository.
Comment 18 Fedora Update System 2014-08-15 20:25:13 EDT
cmap-japan1-6-2012.08.14-4.fc20 has been pushed to the Fedora 20 stable repository.
Comment 19 Fedora Update System 2014-08-15 20:33:21 EDT
cmap-japan1-6-2012.08.14-4.fc19 has been pushed to the Fedora 19 stable repository.

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