Thanks for this, Anil. 

I'll look at it in the next few days.

Have you created a Fedora Account System (FAS) account yet? It's what gives you access to the necessary infrastructure. You can get one here: https://admin.fedoraproject.org/accounts/
Please use the same e-mail address as you are using here on Bugzilla, since that's how Bugzilla syncs with the rest of the infrastructure set up.

Cheers,
Ankur

Hi Ankur I have FAS account it seems.
My username is "anilbey".

Best,
Anıl

Thanks. 

We use fedora-review to automate some of the tests/checks. `sudo dnf install fedora-review`, and https://pagure.io/FedoraReview

I just tried to run fedora-review to get things started. The srpm link isn't the raw link, so fedora-review couldn't pick it up. 

Could you please make another comment here with the SPEC and SRPM URLs, but with the raw url for the SRPM also. That'll allow fedora-review to fetch both and run the necessary preliminary tests.

I built it using the rpmbuild -ba python-bluepyopt.spec command.
What do I need to do to get the raw SRPM?

No, the srpm is fine. Only the github link needs to be updated to the "raw" link, like the one for the Spec:

https://github.com/anilbey/bluepyopt-rpm/raw/master/python-bluepyopt-1.9.48-1.fc32.src.rpm

(the link of the "download" button on Github)

So just a comment with this link for the srpm, and the raw one for the spec is sufficient for fedora-review to pick it up (it takes the latest comment).

The reason is that with this link:

https://github.com/anilbey/bluepyopt-rpm/blob/master/python-bluepyopt-1.9.48-1.fc32.src.rpm

one gets an HTML page, which fedora-review correctly detects as "not an rpm".

So, this link to the src rpm file should work:

https://github.com/anilbey/bluepyopt-rpm/raw/master/python-bluepyopt-1.9.48-1.fc32.src.rpm

I've downloaded the files manually and passed them on to fedora-review for the initial review. 

After each round of comments, you (the package submitter) update the spec/srpm and post new links. Then, I (the reviewer) will re-run fedora-review to verify the new versions---until we pass all checks and the package is approved. So, having the links that fedora-review can directly pull from just quickens the process a little. I can run it as soon as I get an e-mail notification from Bugzilla with the new links, for example.

I'll post the review as soon as I've finished going through the checklist.

Some preliminary comments:


Issues:
=======
- Package installs properly.
  Note: Installation errors (see attachment)
  See: https://docs.fedoraproject.org/en-US/packaging-guidelines/
^
We'll need to look into this.

- Package contains BR: python2-devel or python3-devel
^
All packages must include BuildRequires: python3-devel

(see https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/ -
currently inaccessible due to the infra move)

- Package is not relocatable.
  Note: Package has a "Prefix:" tag
  See: https://docs.fedoraproject.org/en-US/packaging-
  guidelines/#_relocatable_packages

Please remove the following deprecated tags:

- Prefix
- Vendor
- Group


You do not need to define version and release macros. The tags themselves become
macros, so you can use %{release} %{version} %{SOURCE0} etc. in the spec.

Take a look at the python spec template:
https://pagure.io/neuro-sig/NeuroFedora/blob/master/f/spec-templates/python.spec

(This template is also available on the currently inaccessible Python packaging guidelines)


- You can use the %{pypi_source} macro for SOURCE0. It takes three optional arguments:

# default without arguments
$ rpm -E %{pypi_source}
https://files.pythonhosted.org/packages/source/%/%name/%name-%version.tar.gz

# with positional arguments
$ rpm -E "%{pypi_source pkgname version xz}"
https://files.pythonhosted.org/packages/source/p/pkgname/pkgname-version.xz


- You can replace the setup macro line with the autosetup macro
https://rpm.org/user_doc/autosetup.html


- In the build section, please use %{py3_build} (rpm -E %... to see the macro
  expansion). It applies the necessary compilation flags.

- In the install section, please use %{py3_install}

- clean section isn't needed anymore so can be removed.


- We dont' use the INSTALLED_FILES method. We explicitly mention the files so
  that if the package is updated and the file list changes, the build fails to
  make the maintainer aware of this. Please see the template linked above to
  see how to list the installed files.

- The spec is missing a changelog: this is required.

Hi Ankur thanks for the info.

comment5

Ok I updated the link.

comment8

I applied the changes here. One of the macros is trying to install everything listed on setup.py therefore causing this error.

Error: 
 Problem: conflicting requests
  - nothing provides python3.8dist(scoop) >= 0.7 needed by python-bluepyopt-1.9.48-1.fc32.noarch
(try to add '--skip-broken' to skip uninstallable packages)

Is there a way of ignoring the scoop requirement? 

Also I am curious which flags py3_build and py3_install are using? - I couldn't find them in the documentation.

Best,
Anil

(In reply to Anil Tuncel from comment #9)
> Hi Ankur thanks for the info.
> 
> comment5
> 
> Ok I updated the link.
> 
> comment8
> 
> I applied the changes here. One of the macros is trying to install
> everything listed on setup.py therefore causing this error.
> 
> Error: 
>  Problem: conflicting requests
>   - nothing provides python3.8dist(scoop) >= 0.7 needed by
> python-bluepyopt-1.9.48-1.fc32.noarch
> (try to add '--skip-broken' to skip uninstallable packages)
> 
> Is there a way of ignoring the scoop requirement? 

Ah, this is being picked up from the setup.py file then by the automatic dependency generator:
https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/#_automatically_generated_dependencies

https://github.com/BlueBrain/BluePyOpt/blob/master/setup.py#L35

A simple way to resolve this is to remove this requirement from the setup.py file in the %prep section:

%prep
...

sed -i '/scoop/ d' setup.py

Also note that you do not list the Requires: manually now.


> 
> Also I am curious which flags py3_build and py3_install are using? - I
> couldn't find them in the documentation.

All builds in Fedora use the same flags, so these are also defined for python packages (but don't make a difference for pure python packages)

The guidelines will have this info here:
https://docs.fedoraproject.org/en-US/packaging-guidelines/#_compiler_flags

and here:
https://docs.fedoraproject.org/en-US/packaging-guidelines/RPMMacros/#_macros_providing_compiler_and_linker_flags

$ rpm -E "%optflags"
-O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection

The python macros are documented here here:
https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/#_macros


I'll look at the spec again now.

It is ready on the master branch with this commit d15b9ac.
Now with `BuildRequires:  python3-neuron`, BluePyOpt examples can be run as expected.

Spec URL: https://raw.githubusercontent.com/anilbey/bluepyopt-rpm/master/python-bluepyopt.spec
SRPM URL: https://github.com/anilbey/bluepyopt-rpm/raw/master/python-bluepyopt-1.9.48-1.fc32.src.rpm

(In reply to Anil Tuncel from comment #11)
> It is ready on the master branch with this commit d15b9ac.
> Now with `BuildRequires:  python3-neuron`, BluePyOpt examples can be run as
> expected.

I've suggested some more changes on Github to enable tests etc. If you could please have a look, we'll continue the review.

Cheers,

Hi Anil,

I've submitted an updated review here. Once approved, I'll add you as a co-maintainer so you can maintain the package along with the neuro-sig. 

Hope that works,

Cheers,
Ankur

*** This bug has been marked as a duplicate of bug 1948298 ***

Hi Ankur,

Thanks a lot for the change. 
This is the spec, right? 
https://ankursinha.fedorapeople.org/python-bluepyopt/python-bluepyopt.spec

Best
Anil

Hi Anil,

Yes, that's the current version that I've submitted for review. I'm also keeping these changes in a fork of your GitHub repo, but it's just easier to use fedorapeople to host the files so that they are easily accessible by the fedora-review tool.

https://github.com/sanjayankur31/bluepyopt-rpm

Cheers,
Ankur