* Mon Feb 21 2011 Jeff Moe <moe> - 2011.02.20git-2
- Add .pdf docs, TODO.txt

* Mon Feb 21 2011 Jeff Moe <moe> - 2011.02.20git-1
- Minor cleanup of .spec

==========================
Currently I am building by grabbing the latest git:
https://github.com/openscad/openscad.git

Then I run this to generate a tarball:
git archive --format=tar --prefix=openscad-`date +%Y.%m.%d`git/ HEAD | bzip2 > ../openscad-`date +%Y.%m.%d`git.tar.bz2

The current Source: line doesn't have the full URL, as I'm not sure how it would be done in this case as it is a git archive.
Source0:        %{name}-%{version}.tar.bz2

I am not certain the version numbering is ideal, but it reflects upstream practice, AFAICT.

==========================

Latest versions here:
http://repos.fedorapeople.org/repos/jebba/reprap/

This can be added as a repo, instructions here:
http://repos.fedorapeople.org/repos/jebba/reprap/readme.html

* Needs icon

* Needs openscad.desktop

* Mon Feb 21 2011 Jeff Moe <moe> - 2011.02.20git-3
- Add .desktop file
- Add icons

Hi Jebba - I have put together an informal review for you to help this on its way. Comments of note are denoted with ***

regards,

Brendan

+ OK
- N/A
! Problem
? Not evaluated

Required
=========
[+] rpmlint must be run on the source rpm and all binary rpms the build produces
[!] named according to the Package Naming Guidelines 
	*** 20110220git is a more appropriate version number or an abbreviated git hash. See: http://fedoraproject.org/wiki/Packaging/NamingGuidelines
[+] The spec file name must match the base package %{name}, in the format %{name}.spec 
[+] Meet the Packaging Guidelines
[+] Be licensed with a Fedora approved license and meet the Licensing Guidelines 
[!] The License field in the package spec file must match the actual license
	*** GPLv2+ rather than GPLv2
[+] License file must be included in %doc
[+] The spec file must be written in American English
[+] The spec file for the package MUST be legible
[+] The sources used to build the package must match the upstream source
[+] Successfully compile and build into binary rpms on at least one primary architecture
[+] Proper use of ExcludeArch 
[+] All build dependencies must be listed in BuildRequires
[+] The spec file MUST handle locales properly
[+] Shared library files (not just symlinks) in any of the dynamic linker's default paths, must call ldconfig in %post and %postun
[+] Packages must NOT bundle copies of system libraries
[-] If the package is designed to be relocatable, the packager must state this fact in the request for review, along with the rationalization for relocation of that specific package
[+] A package must own all directories that it creates
[+] A Fedora package must not list a file more than once in the spec file's %files listings
[+] Permissions on files must be set properly. Every %files section must include a %defattr(...) line
[+] Each package must consistently use macros
[+] The package must contain code, or permissable content
[+] Large documentation files must go in a -doc subpackage
[+] If a package includes something as %doc, it must not affect the runtime of the application
[+] Header files must be in a -devel package
[-] Static libraries must be in a -static package
[+] library files that end in .so (without suffix) must go in a -devel package
[-] devel packages must require the base package using a fully versioned dependency
[+] Packages must NOT contain any .la libtool archives
[!] GUI apps must include a %{name}.desktop file, properly installed with desktop-file-install in the %install section 
	*** no desktop file or icon included - query upstream or create as additional sources
	
[+] Packages must not own files or directories already owned by other packages
[+] All filenames in rpm packages must be valid UTF-8

Should Items
============
[-] the packager SHOULD query upstream for any missing license text filesto include it
[?] Non-English language support for description and summary sections in the package spec if available
[+] The reviewer should test that the package builds in mock
[+] The package should compile and build into binary rpms on all supported architectures
[+] The reviewer should test that the package functions as described
[-] If scriptlets are used, those scriptlets must be sane
[-] Usually, subpackages other than devel should require the base package using a fully versioned dependency
[-] The placement of pkgconfig(.pc) should usually be placed in a -devel pkg
[-] If the package has file dependencies outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin consider requiring the package which provides the file instead of the file itself
[!] Should contain man pages for binaries/scripts
	*** however, PDF docs are supplied



-- 
Fedora Bugzappers volunteer triage team
https://fedoraproject.org/wiki/BugZappers

(In reply to comment #4)
> Required
> =========
> [!] named according to the Package Naming Guidelines 
>  *** 20110220git is a more appropriate version number or an abbreviated git
> hash. See: http://fedoraproject.org/wiki/Packaging/NamingGuidelines

Upstream gives their latest release names like 2010.05, so I wanted to stay in line with that. Also, when you go to Help-->About in the program it gives a version number like 2011.02.21.   I agree it is a bit ugly, but it is also tracking upstream releases.

> [!] The License field in the package spec file must match the actual license
>  *** GPLv2+ rather than GPLv2

I think it is actually "GPLv2 with exceptions". GPLv2+ is GPLv2 or any later versions. This one has exceptions to allow it to play ok with CGAL.

> [!] GUI apps must include a %{name}.desktop file, properly installed with
> desktop-file-install in the %install section 
>  *** no desktop file or icon included - query upstream or create as additional
> sources

I have added them.
 
> Should Items
> ============
> [!] Should contain man pages for binaries/scripts
>  *** however, PDF docs are supplied

No man pages available.

Thanks for the review!

Here are some additional comments:

- please always post links to the updated spec and srpm files

- the Release count should start with 1 (not with 0)

- 2011.02.20git is not a valid version number. Replace it with 2011.02.20 as
  this seems to be the version number chosen by upstream. 

- change the Release field according to 
  http://fedoraproject.org/wiki/Packaging/NamingGuidelines#Snapshot_packages
  e.g. Release: 0.%{X}.20110222git%{REV}%{?dist}
  where %{X} is the revision number to increase, and %{REV} is the Git 
  changeset hash used to build the package

- add a comment above Source0 with the commands required to recreate the 
  tarball with the same content

- According to the source file headers, the license is GPLv2+. I recommend to 
  ask the experts on the legal mailing list whether it's necessary to declare  
  the mentioned exception.

- the package doesn't build in mock because of missing BR: glew-devel

- you should drop Requires: opencsg as the dependency is detected automatically

- also drop Requires: desktop-file-utils

- add Requires: hicolor-icon-theme

- remove the --vendor switch from desktop-file-install 

- adapt the icon cache scriptlets according to 
  http://fedoraproject.org/wiki/Packaging/ScriptletSnippets#Icon_Cache

- replace %{_datadir}/%{name}/* with %{_datadir}/%{name}/
  for proper directory ownership.

(In reply to comment #6)
> Here are some additional comments:

Wow, thanks for all your suggestions. I have implemented them, but have a question about the Version/Release.

> - 2011.02.20git is not a valid version number. Replace it with 2011.02.20 as
>   this seems to be the version number chosen by upstream. 
> 
> - change the Release field according to 
>   http://fedoraproject.org/wiki/Packaging/NamingGuidelines#Snapshot_packages
>   e.g. Release: 0.%{X}.20110222git%{REV}%{?dist}
>   where %{X} is the revision number to increase, and %{REV} is the Git 
>   changeset hash used to build the package

Doing the above, if I understood correctly, results in quite a long package name, with the date in there twice:

openscad-2011.02.22-0.1.20110222git420c36603b.fc14.src.rpm

Is this OK/correct? Maybe this (still long) would be better:

openscad-2011.02.22-0.1.git420c36603b.fc14.src.rpm

Thanks again!

(In reply to comment #7)
> Doing the above, if I understood correctly, results in quite a long package
> name, with the date in there twice:
> 
> openscad-2011.02.22-0.1.20110222git420c36603b.fc14.src.rpm
> 
> Is this OK/correct? Maybe this (still long) would be better:
> 
> openscad-2011.02.22-0.1.git420c36603b.fc14.src.rpm

Yes, the filenames get pretty long indeed. However, you have to distinguish between the upstream version number (2011.02.22) and the date you checked out the sources (not the date of the changeset but actually the date you cloned or pulled the sources). This way you ensure that the release tag of future packages with newer snapshots but identical version number is greater than the current one, e.g. 2011.02.22-0.1.20110222gitFOO < 2011.02.22-0.1.20110225gitBAR. Otherwise, rpm wouldn't be able to determine what package is the newer one.

The hash value after "git" is optional though, and you can drop it if you want.

Jeff(?): Please input your full name in Bugzilla.

(In reply to comment #9)
> Jeff(?): Please input your full name in Bugzilla.

Jeff Moe. Done.

* Sat Feb 26 2011 Jeff Moe <moe> - 2011.02.22-1.20110222.git
- Use latest 420c36603b commit.
- Drop git hash from Version number.
- Fix Release.
- Change license to GPLv2+.
- Drop Requires desktop-file-utils and opencsg.
- Add Require hicolor-icon-theme
- No --vendor for desktop-file-util
- Update icon cache
- Fix /usr/share/openscad ownership in %%files


Latest files:
http://repos.fedorapeople.org/repos/jebba/reprap/openscad.spec

http://repos.fedorapeople.org/repos/jebba/reprap/14/SRPMS/openscad-2011.02.22-1.20110222.git.fc14.src.rpm

Hi Jeff

just  a few suggestions on your desktop file:
* Add a "GenericName:". Remember that Gnome uses "Comment" whereas KDE uses
"GenericName". 
* do you need to add MimeType= for the file mime types that you want registered with openscad? If so you will need to add the desktop-database scriplet to your spec as well http://fedoraproject.org/wiki/Packaging/ScriptletSnippets#desktop-database

regards
Brendan

I have added to the repo x86_64 builds for Fedora-15 dev branch.

Hi,

 I'm interested in all three packages. I cannot review, I do maintain one package (libcacard), and I can move this forward by doing review changes if the original author is in limbo (Jeff?).

 bug 678980 (this) - openscad
 bug 679273 repsnapper
 bug 678955 (dependency of openscad) - opencsg

Alon

Alon,

Yes, please pick this up, if you can!

The process got wedged because of "W: undefined-non-weak-symbol" in OpenCSG. I simply don't know how to solve that one, so things never progressed past that.

Repsnapper can be completely dropped, unless for some reason you want to package it. If not, I'll close that bug. Repsnapper is obsolete by now. Pronterface (Printrun on github by Kliment) has completely replaced it.

That said, we use OpenSCAD on Fedora without problems. Sorry the process got stopped, I really would like to see this in Fedora!

Thanks. :)

I recently had to fix a "W: undefined-non-weak-symbol" in bug 751466. It may be of help.

Thanks. I'm resigning from this. Sorry I didn't get it completed. Feel free to take it over, I'm closing out this bug.

I would like to take this over, but I have no right to change the status, can anyone do it?

I wrote the spec file from scratch.

Spec URL: https://raw.github.com/hroncok/SPECS/master/openscad.spec

SRPM URL: http://repo.hroncok.cz/SRPMS/openscad-2012.10-1.fc17.src.rpm

You'll need opencsg: https://bugzilla.redhat.com/show_bug.cgi?id=825489

Thanks for picking this up!

I can change the status from Closed to Assigned, at which point you may be able to take it over.

Miro, are you already a member of the Fedora packager group (I can't find your email address in FAS)? If not, please see here how to join us: https://fedoraproject.org/wiki/PackageMaintainers/Join

Open a new ticket for your review request and add a link to this one (bug 678980). The status of this ticket should then be changed to CLOSED DUPLICATE. If you can't access the status field yet, someone else (e.g. your sponsor or a commenter) can do it.

Thanks, done, it's bug 864187

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