https://bugzilla.redhat.com/show_bug.cgi?id=1004306 --- Comment #2 from Roderick MacKenzie <r.c.i.mackenzie@xxxxxxxxxxxxxx> --- 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? -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=2HTEQTIGNb&a=cc_unsubscribe _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review