Bug 1004306

I really doubt if you've read the guidelines?

https://fedoraproject.org/wiki/Packaging:Guidelines

Let me quote all your spec:

------------------------------------------------------

=================
%define name		opvdm 
%define release		1
%define version 	2.0
=================

Why do you waste 3 lines but not use

Name: 			opvdm
Version: 		2.0
Release: 		1%{?dist}

????????? Besides, you've missed dist tag.

------------------------------------------------------
=================
%define buildroot %{_tmppath}/%{name}-%{version}-root
=================

We don't need it. Now is 2013.

------------------------------------------------------
=================
BuildRequires: suitesparse-devel, zlib-devel, openssl-devel, gsl-devel, blas-devel, libcurl-devel, 

requires: gnuplot >= 4.6, python >= 2.7.5, suitesparse >= 4.0.2, blas >= 3.4.2
=================

Hi, can you move such things below basic information? It's abrupt.

And why did you write requires but not Requires?

------------------------------------------------------
=================
BuildRoot:	%{buildroot}
Summary: 		Organic solar cell device model (OPVDM)
License: 		GNU V2.0 (Copyright Roderick MacKenzie 2013)
=================

Have you _read_ guidelines? ?? ???

GNU V2.0 (Copyright Roderick MacKenzie 2013) is invalid, see
https://fedoraproject.org/wiki/Licensing:Main?rd=Licensing

------------------------------------------------------
=================
Source: 		opvdm.tar
Prefix: 		/usr
Group: 			Development/Tools
AutoReqProv: yes
=================

Where does source come from?
Why did you define prefix but not using %{_prefix}
Why do you define AutoReqProv eq yes? In another way why do we have need to write it?

------------------------------------------------------
=================
# I've not included a desktop file in this spec file because:
# The main program (opvdm_core) is command line line/text file driven.
# Although I have bundled a very simple GUI, it's a very early
# alpha version and not mature enough to want people to use it
# as the main way in which to interact with the program.  The gui
# also requires you to prepare the directory structure with the
# command line before it is run.
=================

OK, but please make a better GUI when you release X+1 version or whatsoever.

------------------------------------------------------
=================
%description
An organic solar cell simulator (opvdm)
=================

Too short.
------------------------------------------------------

%prep
%setup -q

------------------------------------------------------
=================
%build
make
=================

Where is the smp flag?

https://fedoraproject.org/wiki/Packaging:Guidelines#Parallel_make
------------------------------------------------------
=================
%install
make  DEST_DIR=%{buildroot} install
=================

------------------------------------------------------
=================

%files
%defattr(0444,root,root)
%attr(755, -, -) /bin/opvdm
%attr(755, -, -) /bin/opvdm_core
%attr(0444, -, -) /usr/opvdm/*.inp
%attr(0444, -, -) /usr/opvdm/image.jpg
%attr(0755, -, -) /bin/*.sh
%attr(0755, -, -) /usr/opvdm/plot
%attr(0755, -, -) /usr/opvdm/plot/*
=================

1. We don't need %defattr().

2. Have you used Fedora? /bin?

http://fedoraproject.org/wiki/Features/UsrMove

3. Can you ensure their permissions when install them but not using attr()?

You should drop all attr().

4. /usr/opvdm is not a good dir.

---> /usr/share/opvdm

5. /bin/*.sh for what?

6. 644 should be the best but not 444 as it's not standard.

7. You should use macro to list files but not hardcode them.

https://fedoraproject.org/wiki/Packaging:Guidelines#Macros
------------------------------------------------------

=================
#%doc %attr(0444,root,root) /usr/local/share/man/man1/wget.1
=================

Are you coming from Debian?

Hi Christopher,
  Thanks for your comments and going though the spec file.  I've addressed them and commented below to indicate what has been updated.  You can find the updated spec file and SRPM at:
Spec URL: www.roderickmackenzie.eu/opvdm.spec
SRPM URL: www.roderickmackenzie.eu/opvdm-2.0-1.src.rpm
I've also tested it again wiht koji
https://koji.fedoraproject.org/koji/taskinfo?taskID=5893983

Best wishes,
Rod

(In reply to Christopher Meng from comment #1)
> I really doubt if you've read the guidelines?
> 
> https://fedoraproject.org/wiki/Packaging:Guidelines
> 
> Let me quote all your spec:
> 
> ------------------------------------------------------
> 
> =================
> %define name		opvdm 
> %define release		1
> %define version 	2.0
> =================
> 
> Why do you waste 3 lines but not use
I've now removed these lines.- thanks for pointing this out.
> 
> Name: 			opvdm
> Version: 		2.0
> Release: 		1%{?dist}
> 
> ????????? Besides, you've missed dist tag.
> 
> ------------------------------------------------------
> =================
> %define buildroot %{_tmppath}/%{name}-%{version}-root
> =================
> 
> We don't need it. Now is 2013.
I've taken this out - thanks.
> 
> ------------------------------------------------------
> =================
> BuildRequires: suitesparse-devel, zlib-devel, openssl-devel, gsl-devel,
> blas-devel, libcurl-devel, 
> 
> requires: gnuplot >= 4.6, python >= 2.7.5, suitesparse >= 4.0.2, blas >=
> 3.4.2
> =================
> 
> Hi, can you move such things below basic information? It's abrupt.
I have moved these lines, they are now below the basic information.
> 
> And why did you write requires but not Requires?
This was a typo – I've changed it to Requires.
> 
> ------------------------------------------------------
> =================
> BuildRoot:	%{buildroot}
> Summary: 		Organic solar cell device model (OPVDM)
> License: 		GNU V2.0 (Copyright Roderick MacKenzie 2013)
> =================
> 
> Have you _read_ guidelines? ?? ???
> 
> GNU V2.0 (Copyright Roderick MacKenzie 2013) is invalid, see
> https://fedoraproject.org/wiki/Licensing:Main?rd=Licensing

Thanks for pointing this out, I've now changed the license to read “GPLv2”
> 
> ------------------------------------------------------
> =================
> Source: 		opvdm.tar
> Prefix: 		/usr
> Group: 			Development/Tools
> AutoReqProv: yes
> =================
> 
> Where does source come from?
I've replaced opvdm.tar with www.roderickmackenzie.eu/opvdm.tar
> Why did you define prefix but not using %{_prefix}
I've deleted this.
> Why do you define AutoReqProv eq yes? In another way why do we have need to
> write it?
I've deleted this now.
> 
> ------------------------------------------------------
> =================
> # I've not included a desktop file in this spec file because:
> # The main program (opvdm_core) is command line line/text file driven.
> # Although I have bundled a very simple GUI, it's a very early
> # alpha version and not mature enough to want people to use it
> # as the main way in which to interact with the program.  The gui
> # also requires you to prepare the directory structure with the
> # command line before it is run.
> =================
> 
> OK, but please make a better GUI when you release X+1 version or whatsoever.
I hope to set a student project doing this, this coming year.
> 
> ------------------------------------------------------
> =================
> %description
> An organic solar cell simulator (opvdm)
> =================
> 
> Too short.
I've made it longer – please see new spec file.
> ------------------------------------------------------
> 
> %prep
> %setup -q
> 
> ------------------------------------------------------
> =================
> %build
> make %{?_smp_mflags}
> =================
> 
> Where is the smp flag?
I've added the smp flags as per the link - please see the new spec file.
> 
> https://fedoraproject.org/wiki/Packaging:Guidelines#Parallel_make
> ------------------------------------------------------
> =================
> %install
> make  DEST_DIR=%{buildroot} install
> =================
> 
> ------------------------------------------------------
> =================
> 
> %files
> %defattr(0444,root,root)
> %attr(755, -, -) /bin/opvdm
> %attr(755, -, -) /bin/opvdm_core
> %attr(0444, -, -) /usr/opvdm/*.inp
> %attr(0444, -, -) /usr/opvdm/image.jpg
> %attr(0755, -, -) /bin/*.sh
> %attr(0755, -, -) /usr/opvdm/plot
> %attr(0755, -, -) /usr/opvdm/plot/*
> =================
> 
> 1. We don't need %defattr().
I've deleted it - please see the new spec file.
> 
> 2. Have you used Fedora? /bin?
> 
> http://fedoraproject.org/wiki/Features/UsrMove
I've moved the binaries to /usr/bin/
> 
> 3. Can you ensure their permissions when install them but not using attr()?
> 
> You should drop all attr().
I've dropped all the attr lines.
> 
> 4. /usr/opvdm is not a good dir.
> 
> ---> /usr/share/opvdm
Done.
> 
> 5. /bin/*.sh for what?
Sorry, that was a mistake – deleted the line.
> 
> 6. 644 should be the best but not 444 as it's not standard.
Done – changed the make install.
> 
> 7. You should use macro to list files but not hardcode them.
> 
> https://fedoraproject.org/wiki/Packaging:Guidelines#Macros
> ------------------------------------------------------
I've rewritten this section to remove attr and use macros.
> 
> =================
> #%doc %attr(0444,root,root) /usr/local/share/man/man1/wget.1
> =================
> 
> Are you coming from Debian?

Hi,
  I've just realized the name of the src.rpm has changed due to the changes in the spec file.  New link:
http://www.roderickmackenzie.eu/opvdm-2.0.fc19-1.src.rpm
best wishes,
Rod

You should also add link to your spec file as we need to first make your spec readable.

Besides please find a sponsor, I can't sponsor you, sorry.

See: http://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group

Hi Christopher,
  Thanks for your reply.  Here is a link to the spec file and the srpm.
http://www.roderickmackenzie.eu/opvdm.spec
http://www.roderickmackenzie.eu/opvdm-2.0.fc19-1.src.rpm
The spec file looks quite different from before - if you can spot anything that still does not comply to the guide lines I would be happy to change it.  If you think it looks reasonable I will go and look for a sponsor.
best wishes,
Rod

Ah yes still have problem.

Version: 		2.0%{dist}
Release: 		1

Oh, you should put dist in %release like this:

Version: 		2.0
Release: 		1%{dist}

When you edit spec everrtime, remember to bump 1 to 2 to 3 to 4... And write %changelog. I can't find %changelog section in the spec.

=======


%{_bindir}/opvdm
%{_bindir}/opvdm_core
%{_bindir}/opvdm_clone.sh
%{_bindir}/opvdm_import.sh
%{_bindir}/opvdm_dump_tab.sh
%{_bindir}/opvdm_load.sh

I hope you can remove all .sh suffix.

Also, %{_datarootdir} can be %{_datadir}

Suggestion:

Release your software in tarball named : %{name}-%{version}, but not only %{name}.

Anyway, you are getting better and better! So let's start finding a sponsor, I can only help you here.

If you add a bugzilla comment with two valid lines "Spec URL:" and "SRPM URL:", it becomes possible to point the fedora-review tool at this ticket (fedora-review -b 1004306). Let's give it a try:

Spec URL: http://www.roderickmackenzie.eu/opvdm.spec
SRPM URL: http://www.roderickmackenzie.eu/opvdm-2.0.fc19-1.src.rpm



> Also, %{_datarootdir} can be %{_datadir}

This is for Christopher: It would be more helpful to give the rationale or link specific guidelines. %_datarootdir has been okay and unchanged for a long time, it's just that %_datadir is more common and saves some typing, too.

  $ rpm -E %_datarootdir 
  /usr/share


Also, especially NEEDSPONSOR reviews should always start with mentioning rpmlint. It's the first item on the https://fedoraproject.org/wiki/Packaging:ReviewGuidelines list.

Run rpmlint (or rpmlint -i for more helpful output) on the src.rpm and all
built rpms. Feel free to ignore obvious false positives in the report, but fix
anything else. Preferably add a comment here about whether/when you think what
rpmlint reports is correct or incorrect.


> Requires: gnuplot >= 4.6, python >= 2.7.5, suitesparse >= 4.0.2, blas >= 3.4.2

https://fedoraproject.org/wiki/Packaging:Guidelines#Explicit_Requires


> http://kojipkgs.fedoraproject.org//work/tasks/3984/5893984/build.log

https://fedoraproject.org/wiki/Packaging:Guidelines#Compiler_flags


> %{_datarootdir}/opvdm/*.inp
> %{_datarootdir}/opvdm/image.jpg
> %{_datarootdir}/opvdm/plot
> %{_datarootdir}/opvdm/plot/*

$ rpmls -p opvdm-2.0.fc19-1.x86_64.rpm |grep ^d
drwxr-xr-x  /usr/share/opvdm/plot

The opvdm directory is not included yet:
https://fedoraproject.org/wiki/Packaging:UnownedDirectories


And the plot/* entry is redundant, because

  %{_datadir}/opvdm/plot

includes the directory and anything in it. For increased readability, many packagers add a trailing slash to be more explicit that it's a dir and not a normal file:

  %{_datadir}/opvdm/plot/

Also take a look at the file list of the built packages, e.g. with "rpmls -p ...". Currently, there's a backup file included.

Dear Michael and Christopher,
  Thanks for your last two posts.  It took me slightly longer to make the changes this time. See below.  Is this OK?
best wishes,
Rod

Spec URL: http://www.roderickmackenzie.eu/opvdm.spec
SRPM URL: http://www.roderickmackenzie.eu/opvdm-2.0-2.fc19.src.rpm

koji build link: https://koji.fedoraproject.org/koji/taskinfo?taskID=5909155

output of “rpmlint -i opvdm-2.0-2.fc19.src.rpm” :
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

Output of “rpmlint -i opvdm-2.0-2.fc19.x86_64.rpm” :
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

output of “fedora-review -rn opvdm-2.0-2.fc19.src.rpm“
Package Review
==============

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
[ ] = Manual review needed



===== MUST items =====

C/C++:
[ ]: Package does not contain kernel modules.
[ ]: Package contains no static executables.
[x]: Package does not contain any libtool archives (.la)
[x]: Rpath absent or only used for internal libs.

Generic:
[ ]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[ ]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses found:
     "GPL (v2 or later)". Detailed output of licensecheck in
     /home/rod/rpmbuild/SRPMS/opvdm/licensecheck.txt
[ ]: %build honors applicable compiler flags or justifies otherwise.
[ ]: Package contains no bundled libraries without FPC exception.
[ ]: Changelog in prescribed format.
[ ]: Sources contain only permissible code or content.
[ ]: Package contains desktop file if it is a GUI application.
[ ]: Development files must be in a -devel package
[ ]: Package uses nothing in %doc for runtime.
[ ]: Package consistently uses macros (instead of hard-coded directory names).
[ ]: Package is named according to the Package Naming Guidelines.
[ ]: Package does not generate any conflict.
[ ]: Package obeys FHS, except libexecdir and /usr/target.
[ ]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[ ]: Requires correct, justified where necessary.
[ ]: Spec file is legible and written in American English.
[ ]: Package contains systemd file(s) if in need.
[ ]: Useful -debuginfo package or justification otherwise.
[ ]: Package is not known to require an ExcludeArch tag.
[ ]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 20480 bytes in 2 files.
[ ]: Package complies to the Packaging Guidelines
[x]: Package successfully compiles and builds into binary rpms on at least one
     supported primary architecture.
[x]: Package installs properly.
[x]: Rpmlint is run on all rpms the build produces.
     Note: No rpmlint messages.
[x]: If (and only if) the source package includes the text of the license(s)
     in its own file, then that file, containing the text of the license(s)
     for the package is included in %doc.
[x]: Package requires other packages for directories it uses.
[x]: Package must own all directories that it creates.
[x]: Package does not own files or directories owned by other packages.
[x]: All build dependencies are listed in BuildRequires, except for any that
     are listed in the exceptions section of Packaging Guidelines.
[x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
[x]: Each %files section contains %defattr if rpm < 4.4
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Package use %makeinstall only when make install' ' DESTDIR=... doesn't
     work.
[x]: Package is named using only allowed ASCII characters.
[x]: Package do not use a name that already exist
[x]: Package is not relocatable.
[x]: Sources used to build the package match the upstream source, as provided
     in the spec URL.
[x]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[x]: File names are valid UTF-8.
[x]: Packages must not store files under /srv, /opt or /usr/local

===== SHOULD items =====

Generic:
[!]: Dist tag is present (not strictly required in GL).
[ ]: If the source package does not include license text(s) as a separate file
     from upstream, the packager SHOULD query upstream to include it.
[ ]: Final provides and requires are sane (see attachments).
[ ]: Package functions as described.
[ ]: Latest version is packaged.
[ ]: Package does not include license text files separate from upstream.
[ ]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[ ]: Package should compile and build into binary rpms on all supported
     architectures.
[ ]: %check is present and all tests pass.
[ ]: Packages should try to preserve timestamps of original installed files.
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: Sources can be downloaded from URI in Source: tag
[x]: Reviewer should test that the package builds in mock.
[x]: Buildroot is not present
[x]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: Fully versioned dependency in subpackages if applicable.
[x]: Uses parallel make %{?_smp_mflags} macro.
[x]: SourceX tarball generation or download is documented.
[x]: SourceX is a working URL.
[x]: Spec use %global instead of %define unless justified.

===== EXTRA items =====

Generic:
[x]: Rpmlint is run on all installed packages.
     Note: No rpmlint messages.
[x]: Large data in /usr/share should live in a noarch subpackage if package is
     arched.


Rpmlint
-------
Checking: opvdm-2.0-2.fc19.x86_64.rpm
          opvdm-2.0-2.fc19.src.rpm
2 packages and 0 specfiles checked; 0 errors, 0 warnings.




Rpmlint (installed packages)
----------------------------
# rpmlint opvdm
1 packages and 0 specfiles checked; 0 errors, 0 warnings.
# echo 'rpmlint-done:'



Requires
--------
opvdm (rpmlib, GLIBC filtered):
    /bin/bash
    /usr/bin/env
    gnuplot
    libamd.so.2()(64bit)
    libblas.so.3()(64bit)
    libc.so.6()(64bit)
    libcrypto.so.10()(64bit)
    libcrypto.so.10(libcrypto.so.10)(64bit)
    libcurl.so.4()(64bit)
    libdl.so.2()(64bit)
    libgsl.so.0()(64bit)
    libgslcblas.so.0()(64bit)
    libm.so.6()(64bit)
    libumfpack.so.5()(64bit)
    libz.so.1()(64bit)
    rtld(GNU_HASH)



Provides
--------
opvdm:
    opvdm
    opvdm(x86-64)



Source checksums
----------------
http://www.roderickmackenzie.eu/opvdm-2.0.tar :
  CHECKSUM(SHA256) this package     : 43ad6b17a6f7edab516e2c862f0b964d8670f75ae0919a0ac22e96bb8e56d3b8
  CHECKSUM(SHA256) upstream package : 43ad6b17a6f7edab516e2c862f0b964d8670f75ae0919a0ac22e96bb8e56d3b8


Generated by fedora-review 0.5.0 (920221d) last change: 2013-08-30
Command line :/usr/bin/fedora-review -rn opvdm-2.0-2.fc19.src.rpm
Buildroot used: fedora-19-x86_64
Active plugins: Generic, Shell-api, C/C++
Disabled plugins: Java, Python, SugarActivity, Perl, R, PHP, Ruby
Disabled flags: EPEL5, EXARCH, DISTTAG

Running "fedora-review -b 1004306" fails with 404 Not Found error.

Please paste 2.1 SRPM here again.

BTW I hope you can find a sponsor.

No SRPM provided for long time.
Closing due long inactivity. Feel free to reopen if you want to continue.