Bug 1519590 - Review Request: module-build-service-copr - Copr plugin for the Module Build Service
Summary: Review Request: module-build-service-copr - Copr plugin for the Module Build...
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Robert-André Mauchin 🐧
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2017-12-01 00:46 UTC by Jakub Kadlčík
Modified: 2018-01-10 17:13 UTC (History)
5 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2018-01-10 17:13:52 UTC
Type: ---
Embargoed:
zebob.m: fedora-review+


Attachments (Terms of Use)

Description Jakub Kadlčík 2017-12-01 00:46:00 UTC
Spec URL: https://copr-be.cloud.fedoraproject.org/results/@copr/copr-dev/fedora-27-x86_64/00682745-python-module-build-service-copr/module-build-service-copr.spec
SRPM URL: https://copr-be.cloud.fedoraproject.org/results/@copr/copr-dev/fedora-27-x86_64/00682745-python-module-build-service-copr/python-module-build-service-copr-0.1-1.git.14.0eec290.fc27.src.rpm
Description:

This is a plugin for MBS containing Copr related extensions such as
CoprModuleBuilder and CoprResolver.


The Module Build Service (MBS) coordinates module builds and is responsible
for a number of tasks:

- Providing an interface for module client-side tooling via which module build
  submission and build state queries are possible.
- Verifying the input data (modulemd, RPM SPEC files and others) is available
  and correct.
- Preparing the build environment in the supported build systems, such as koji.
- Scheduling and building of the module components and tracking the build
  state.
- Emitting bus messages about all state changes so that other infrastructure
  services can pick up the work.



Fedora Account System Username: frostyx

Comment 2 Igor Gnatenko 2017-12-01 10:04:55 UTC
> %if 0%{?fedora}
> %global with_python3 1
> %else
> %global with_python3 0
> %endif

%if 0%{?rhel} && 0%{?rhel} <= 7
%bcond_with python3
%else
%bcond_without python3
%endif

* Move Requires/BuildRequires under appropriate subpackages

> %{python3_sitelib}/*

Be more specific here.

> * Tue Nov 28 2017 Jakub KadlÄík <frostyx> 0.1-1

Version is wrong, encoding is wrong.

Comment 3 Robert-André Mauchin 🐧 2017-12-03 15:03:14 UTC
 - URL is wrong. It should be:

URL:            https://pagure.io/copr/%{srcname}

 - Explain in a comment how to generate the Source0 tgz

 - If you package a snapshot, the Release tag should be in the format:

Release:        0.1.20171201git0eec290%{?dist}

See: https://fedoraproject.org/wiki/Packaging:Versioning#Snapshots

Comment 5 Jakub Kadlčík 2017-12-03 21:04:58 UTC
Thank you guys for the feedback. No way I would expect it so fast. I've addressed your comments except for 

> > %{python3_sitelib}/*
>
> Be more specific here.

When I narrow it to the %{python3_sitelib}/module_build_service_copr it complains about unpackaged egg files. I've looked into the docs https://fedoraproject.org/wiki/Packaging:Python_Eggs#Providing_Egg_Metadata_Using_Setuptools and it is done the same way as I have it. I have no philosophical problem with changing the line, but I don't really know how.

Comment 6 Robert-André Mauchin 🐧 2017-12-03 21:19:03 UTC
Try something like:

%{python3_sitelib}/%{srcname}
%{python3_sitelib}/srcname-%{version}-*.egg-info

Comment 8 Robert-André Mauchin 🐧 2017-12-03 22:49:04 UTC
The way you update Version/Release is wrong. Version should reflect the version of the app you are packaging, it should not change if upstream version hasn't changed. Release is corresponding to the revision of the SPEC file, i.e. it is incremented when you make change to the SPEC file and reset to 1 when a new upstream is published.
It seems you bump the upstream version each time you update the SPEC, this doesn't seem right, you should have a version specific to the module_build_service_copr module that doesn't depend on the SPEC.

Comment 9 Miroslav Suchý 2017-12-04 11:15:05 UTC
@Robert He is an upstream. So he can do it. It does not make sense to maintain two spec file for upstream and downstream if he is maintainer of both upstream and downstream.

Comment 10 Robert-André Mauchin 🐧 2017-12-04 17:23:49 UTC
Ok then the package is sound and accepted.


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