Bug 1323181

Original review in #1221204.

+Provides:       python-docker-scripts = %{version}-%{release}
+Obsoletes:      python-docker-scripts <= 1.0.0-0.2.rc2
Looks correct.

See https://fedoraproject.org/wiki/Packaging:SourceURL#Git_Hosting_Services for a nicer way to specify Source URL that gives an archive with a better name.

Why do you provide both /usr/bin/docker-squash3 and /usr/bin/docker-squash? Do they provide different functionality? If not, only one should be provided. If both implementations are equivalent, implementation in python3 is preferred [https://fedoraproject.org/wiki/Packaging:Python#Executables_in_.2Fusr.2Fbin].

(In reply to Zbigniew Jędrzejewski-Szmek from comment #1)
> See https://fedoraproject.org/wiki/Packaging:SourceURL#Git_Hosting_Services
> for a nicer way to specify Source URL that gives an archive with a better
> name.

That's cool, although I'll stay with what I have currently :)

> Why do you provide both /usr/bin/docker-squash3 and /usr/bin/docker-squash?
> Do they provide different functionality? If not, only one should be
> provided. If both implementations are equivalent, implementation in python3
> is preferred
> [https://fedoraproject.org/wiki/Packaging:Python#Executables_in_.2Fusr.
> 2Fbin].

Nope, the functionality is the same. For sure I want to maintain the same spec file across all Fedora's and EPEL 7, so I'll see what can I do.

> Nope, the functionality is the same. For sure I want to maintain the same spec file across all Fedora's and EPEL 7, so I'll see what can I do.

You should provide /usr/bin/docker-squash which uses /usr/bin/python3 or /usr/bin/python2 conditionally on the Fedora/EPEL version.

I was looking at the python-pygments spec file to get an idea how to fix it since its mentioned as an example on the wiki, but I still don't get it. Especially this line: http://pkgs.fedoraproject.org/cgit/rpms/python-pygments.git/tree/python-pygments.spec?id=41198a3579e621f6885165e5d19fb76a20937a19#n190 which packages the pygmentize binary in python-pygmentize package (which is still 2 by default).

Additionally python3 version is installed before python2: http://pkgs.fedoraproject.org/cgit/rpms/python-pygments.git/tree/python-pygments.spec?id=41198a3579e621f6885165e5d19fb76a20937a19#n138 (which makes comment on line 136 and 137 true).

So it's effectively python2 version of pygmentize packaged.

Am I missing something obvious here? I think I could get use some help on adopting the change since this is a bit unclear what should I do.

Created attachment 1146671 [details]
updated spec file

Please see the attached spec file. It simplifies stuff quite a bit by using the macros from https://fedoraproject.org/wiki/Packaging:Python#Example_common_spec_file.

I also looked at the effect of 2to3 translation, and it's just one change (iteritems() → items()). It's much nicer to use a single source without 2to3 translation, and py3dir, so I replaced that with a sed invocation. I think you should file that as a patch upstream.

The spec file required one more major change: python2 subpackage must be added according to the new guidelines [https://fedoraproject.org/wiki/Packaging:Python].

(In reply to Zbigniew Jędrzejewski-Szmek from comment #5)
> Please see the attached spec file. It simplifies stuff quite a bit by using
> the macros from
> https://fedoraproject.org/wiki/Packaging:Python#Example_common_spec_file.

That indeed looks nicer, thanks for helping me out! I've build it on all targets and it seems to work, cool.
 
> I also looked at the effect of 2to3 translation, and it's just one change
> (iteritems() → items()). It's much nicer to use a single source without 2to3
> translation, and py3dir, so I replaced that with a sed invocation. I think
> you should file that as a patch upstream.

This basically should be untouched. This is monkey patch for Python 2 only to enable UTF8 names in PAX headers in tar archives. See: https://bugzilla.redhat.com/show_bug.cgi?id=1194473

> The spec file required one more major change: python2 subpackage must be
> added according to the new guidelines
> [https://fedoraproject.org/wiki/Packaging:Python].

Awesome, thanks for doing it!

I additionally upgraded to RC4 and here are new files:

Spec URL: https://goldmann.fedorapeople.org/package_review/python-docker-squash.spec
SRPM URL: https://goldmann.fedorapeople.org/package_review/python-docker-squash-1.0.0-0.5.rc4.fc23.src.rpm

Koji scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=13643225

Thanks for the explanation about xtarfile.

Everything looks OK:
+ latest version
+ name is OK
+ license is acceptable
+ license file is present, %license is used
+ build and installs OK
+ provides/requires look sane
+ obsoletes/provides for the previous version are OK

rpmlint:
python3-docker-squash.noarch: W: no-manual-page-for-binary docker-squash
3 packages and 0 specfiles checked; 0 errors, 1 warnings.

Package is (RE-)APPROVED.

Thank you!

Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/python-docker-squash