[Bug 1282893] Review Request: quantum-espresso - A suite for electronic-structure calculations and materials modeling

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



https://bugzilla.redhat.com/show_bug.cgi?id=1282893



--- Comment #19 from marcindulak <Marcin.Dulak@xxxxxxxxx> ---
(In reply to Zbigniew Jędrzejewski-Szmek from comment #11)
> Thanks for tackling this. It seems to be a really complicated package and I
> can see that a lot of work has gone into putting it in a decent shape.
> 
> Some issues:
> 
> openblas is ExclusiveArch x86_64 %{ix86} armv7hl ppc64le, you should
> probably match that.

fixed

> 
> License tag is wrong. The License tag specifies the *effective* license of
> the *binary* package [1, 2]. So anything that is not part of the binary rpm
> (like install/install-sh) doesn't matter at all. If you combine GPLv2+ with
> BSD or LGPL or MIT, the effective license is GPLv2+. So, if a file in the
> binary rpm has at least one GPL licensed file, that file is GPL. If a file
> only had sources with more permissive licenses, than that file would have
> some other license. So you need to determine the effective license of all
> files in the binary rpm and put the result in License. I'd guess that the
> result is going to be License:GPLv2+.
> 

fixed

> [1]
> https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#License:_field
> [2]
> https://fedoraproject.org/wiki/Licensing:FAQ?rd=Licensing/FAQ#What_is_.
> 22effective_license.22_and_do_I_need_to_know_that_for_the_License:_tag.3F
> 
> Requires:openmpi and Requires:mpich in the q-e-openmpi and q-e-mpich look
> wrong to me. Normally the automatically generated dependency would be
> enough. E.g. q-e-mpich.i686 requires
>   libmpi.so.12(mpich-i386)
>   libmpifort.so.12(mpich-i386)
> and this should be enough.

The builds need to work also on el6/el7. I keep those Requires for now.

> 
> OTOH, q-e-mpich-devel should probably R: mpich-devel, and q-e-openmpi-devel
> should probably R: openmpi-devel.

The BuildRquires and Requires are correct for q-e-*-devel subpackages with the
current packaging of openmpi and mpich.
They *Requires need to be mpich and openmpi due to the paths to directories
containing fortran mod files (%{_fmoddir}/openmpi%{?_cc_name_suffix}/) being
owned by mpich and openmpi.

> 
> Actually, I'd suggest moving all the BuildRequires to the top. Right now it
> is hard to get an overview of what will be installed in the build root, and
> some BR are repeated.

I'll get more confused by moving the dependencies to one place.

> 
> Why export OMP_NUM_THREADS=1 in %check?

removed now.

> 
> Maybe you could use '%{lua: for i=20,41 do print("%{SOURCE" .. i .. "} ")
> end}'
> instead of listing all the SOURCExx explicitly in cp?

how can this be done?
One the command line the following looks OK:

$ rpm --eval '%{lua:for i=20,41 do print(string.format("cp -p %%{SOURCE%u}
pseudo\n", i)) end}'

but with rpmbuild I get additional quotes:

...
+ cp -p '%{SOURCE20}' pseudo
cp: cannot stat `%{SOURCE20}': No such file or directory
...

> 
> The comments from Dave are also all relevant:
> (In reply to Dave Love from comment #10)
> > Created attachment 1107142 [details]
> > fix requires
> This looks correct.

fixed

> 
> > * It won't install because it requires an arch-specific -common package;
> > patch attached.
> Right.
>  
> > * I'd have thought iotk should be unbundled, but I don't know if it's of
> > more general use.
> Hm, good question. Is it used anywhere else?

iotk is required by http://www.yambo-code.org/
Unbundling iotk would require some work, and I'm not willing to do this
without the support from quantum-espresso developers.
Yambo is currently hosted on qe-forge
http://qe-forge.org/gf/project/yambo/frs/?action=FrsReleaseBrowse&frs_package_id=40
which means that it's probably related to quantum-espresso and when packaging
yambo one could just BuildRequires: quantum-espresso-{openmpi,static}-devel and
quantum-espresso-{openmpi,static}-static

> 
> > * Shouldn't this build against atlas or lapack/blas on non-x86?  (ppc64le
> > seems a plausible architecture to run it on.)  I know that makes it
> > particularly suffer the BLAS mess in Fedora, but scalapack links against
> > reference blas anyhow.
> openblas is available on ppc64le. So maybe building everywhere that
> openblas-devel
> is avaialable would be enough...

I prefer openblas. Had problems with atlas in the past with various codes, and
openblas just worked.

> 
> > * It's not using the default compilation and linking flags (not that I agree
> > with that requirement for computational programs).

%{optflags} are used now

> > 
> > * Will smp make not work?  There's no comment, and the build takes a while.
> Yeah, the build takes forever. More threads would be great.

make %{?_smp_mflags} failed for me

>  
> > * Why not use elpa?  (I haven't tried with this version and the Fedora elpa.)

I tried to use Fedora's ELPA but several makefiles have hardcoded ELPA.
I didn't want to spend my time on this and decided to simply remove the bundled
ELPA.

> > 
> > * Shouldn't the doc be installed?

I believe nobody reads the docs installed on the system nowadays.

> > 
> > * Would the GUI be useful?  (I don't know.)

Let's leave it for later. pwgui looks to me as a separate package.

Spec URL:
https://marcindulak.fedorapeople.org/packages/quantum-espresso/r03/quantum-espresso.spec
SRPM URL:
https://marcindulak.fedorapeople.org/packages/quantum-espresso/r03/quantum-espresso-5.2.1-3.el6.src.rpm

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review




[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]