Spec URL: http://dl.dropbox.com/u/2682668/discount.spec SRPM URL: http://dl.dropbox.com/u/2682668/fedora-remix/15/source/packages/discount-2.1.1.3-1.fc15.src.rpm Hi, I would like to suggest Discount for inclusion in Fedora. I am completely new to .rpm packaging and somewhat new to Fedora, so apologies if I've messed something up. Short summaries: Discount is a fast C implementation of the Markdown language with a few additional extras, such as smartypants substitutions. More information at: http://www.pell.portland.or.us/~orc/Code/discount Markdown is a simple "plain-text formatting syntax" that can be easily converted to HTML. More information at: http://daringfireball.net/projects/markdown/
Well, it seems I did mess quite a few things up. I've fixed some of them but neglected to notice that I was statically compiling a library that should probably be compiled --shared and packaged separately. I will fix it soon and post an update here.
I hadn't anticipated changing the spec much before getting some feedback but I'm running into various issues, so I thought I may as well put it under version control to make it easier to review. I thought being as it's only a single file, I would use a use a GitHub gist. Their web interface isn't too helpful but you can clone and log/diff if required. https://gist.github.com/1240217/
Hi! Someone else will need to sponsor you, but here's some comments: - You should always upload a SRPM file somewhere when updating the spec file and post a link here. It will make it easier for reviewers. Once you upload one, I'll take a closer look. - You seem to have the subpackage idea right: discount will be the command line utility, libmarkdown the library itself and libmarkdown-devel will contain the headers. However, you need to fix the following: 1. Explicit requirements between the packages. Since the command line utility depends on the libmarkdown library, you should add an explicit require on the base package for libmarkdown. (Following the syntax here: http://fedoraproject.org/wiki/Packaging:Guidelines#Requiring_Base_Package) 2. In addition, if a user wants to develop software using libmarkdown, they will need libmarkdown to use the development files, so add a requirement for libmarkdown to the devel subpackage too. 3. Non-versioned shared libraries (libmarkdown.so) must be packaged in the development subpackage (http://fedoraproject.org/wiki/Packaging:Guidelines#Devel_Packages). (You should also remove the noarch parameter from -devel.) - You should select files in the %files section more verbosely: for example: 1. For the binary file, just do %{_bindir}/markdown 2. For the manual files, do something like %{_mandir}/manx/*.x* (where x is the number) 3. For the library files, you can select the versioned files with libmarkdown.so.* and the non-versioned file with just libmarkdown.so. - Needless to say, the description and summary for libmarkdown should contain something. - If you don't plan on building this package for EPEL5, you can remove %clean. Otherwise, you need to add a BuildRoot tag and do rm -rf %{buildroot} in the beginning of %install too. (http://fedoraproject.org/wiki/EPEL/GuidelinesAndPolicies#BuildRoot_tag)
(In reply to comment #3) > Hi! Someone else will need to sponsor you, but here's some comments: > > - You should always upload a SRPM file somewhere when updating the spec file > and post a link here. It will make it easier for reviewers. Once you upload > one, I'll take a closer look. Sorry I forgot to make the SRPM available when I bumped the spec. I will keep SRPMs here from now: http://dl.dropbox.com/u/2682668/discount-2.1.1.3-4.fc15.src.rpm > - You seem to have the subpackage idea right: discount will be the command line > utility, libmarkdown the library itself and libmarkdown-devel will contain the > headers. However, you need to fix the following: > > 1. Explicit requirements between the packages. Since the command line utility > depends on the libmarkdown library, you should add an explicit require on the > base package for libmarkdown. (Following the syntax here: > http://fedoraproject.org/wiki/Packaging:Guidelines#Requiring_Base_Package) I did this and rpmlint gave an error (E: explicit-lib-dependency). I researched it a little and it seems that if the base package depends on a sub-package library, RPM handles the dependency automatically. Correct me if I'm wrong. I have left this one out for now. > 2. In addition, if a user wants to develop software using libmarkdown, they > will need libmarkdown to use the development files, so add a requirement for > libmarkdown to the devel subpackage too. Fixed. > 3. Non-versioned shared libraries (libmarkdown.so) must be packaged in the > development subpackage > (http://fedoraproject.org/wiki/Packaging:Guidelines#Devel_Packages). (You > should also remove the noarch parameter from -devel.) Fixed. > - You should select files in the %files section more verbosely: for example: > > 1. For the binary file, just do %{_bindir}/markdown > > 2. For the manual files, do something like %{_mandir}/manx/*.x* (where x is the > number) > > 3. For the library files, you can select the versioned files with > libmarkdown.so.* and the non-versioned file with just libmarkdown.so. Fixed (I think). Did everything you suggested here but more feedback appreciated. > - Needless to say, the description and summary for libmarkdown should contain > something. Done. > - If you don't plan on building this package for EPEL5, you can remove %clean. > Otherwise, you need to add a BuildRoot tag and do rm -rf %{buildroot} in the > beginning of %install too. > (http://fedoraproject.org/wiki/EPEL/GuidelinesAndPolicies#BuildRoot_tag) Yeah, I had just blindly copied that part, not really understanding what it was for. I've moved the -rf %{buildroot} back to the top of install. Hopefully that's correct. feedback appreciated. Thanks a lot for the feedback. It helped to clear up a few confusions I had and to fix most of the rpmlint warnings. There's just 1 remaining warning about libmarkdown-devel not having any docs. Also, I have just noticed that the old description about "developer-oriented man pages" is no longer accurate. Will fix and bump shortly.
Sorry about the line wrapping there. I thought Bugzilla allowed manual wrapping. I just thought I'd explain the patch I added too, before I forget what it was for. The reason I added it is, when the "--shared" flag is passed to configure.sh, it causes "make install" to run a script bundled by the author called "librarian.sh". Apparently this is just to "clean up the Makefile" but it also tries to call ldconfig. I guess this is a legitimate thing to do for people compiling it themselves but when executed via rpmbuild it gives a fatal permission error: > "/sbin/ldconfig: Can't create temporary cache file /etc/ld.so.cache~: Permission denied" The patch simply removes the call to ldconfig. I have added %post and %postun sections to call ldconfig, as detailed in the packaging guidelines. This fix appears to work, although I'm not 100% sure that this is the correct way to fix it. I also read in the packaging guidelines that every patch should be reported upstream. I can report it but I will wait for some feedback here first about whether or not the patch is actually the correct thing to do.
Forgot to mention. The updated spec is on GitHub as before. The patch is there too. SPEC and patch: https://gist.github.com/1240217/eced423a96efef119a590d08390f1c8393a88c09 SRPM: http://dl.dropbox.com/u/2682668/discount-2.1.1.3-4.fc15.src.rpm
Bumped to release 5: - Moved man3 pages from libmarkdown to libmarkdown-devel. - Added documentation to libmarkdown (same ones as in the base package) This fixes the last rpmlint warning (W: no-documentation). SPEC: https://gist.github.com/1240217/53d2db710eabb5ab9c03ca4e82199762ed2af457 SRPM: http://dl.dropbox.com/u/2682668/discount-2.1.1.3-5.fc15.src.rpm Hope this is correct.
> I did this and rpmlint gave an error (E: explicit-lib-dependency). I researched it a little and it seems that if the base package depends on a sub-package library, RPM handles the dependency automatically. Correct me if I'm wrong. I have left this one out for now. Are you sure? I just tried adding "Requires: libmarkdown%{?_isa} = %{version}-%{release}" below the BuildRequires and I don't get any rpmlint errors. (I can also think of two existing packages that do this: curl and ufraw). > I have added %post and %postun sections to call ldconfig, as detailed in the packaging guidelines. Since the discount package doesn't have any shared libraries, you don't need to call ldconfig (so remove the %post and %postun lines that aren't for libmarkdown).
(In reply to comment #8) > > I did this and rpmlint gave an error (E: explicit-lib-dependency). I researched it a little and it seems that if the base package depends on a sub-package library, RPM handles the dependency automatically. Correct me if I'm wrong. I have left this one out for now. > > Are you sure? I just tried adding "Requires: libmarkdown%{?_isa} = > %{version}-%{release}" below the BuildRequires and I don't get any rpmlint > errors. (I can also think of two existing packages that do this: curl and > ufraw). Yeah, you're right. I just added the dependency and built again and rpmlint passes without complaint. I think I must have been specifying the wrong version to get that error before. > > I have added %post and %postun sections to call ldconfig, as detailed in the packaging guidelines. > > Since the discount package doesn't have any shared libraries, you don't need to > call ldconfig (so remove the %post and %postun lines that aren't for > libmarkdown). Removed, thanks. I have updated the spec in the same place as before and added the new SRPM. It might be worth noting that I also made 1 other revision between my last update here, which I have detailed in the changelog. Spec: https://gist.github.com/1240217/5fd79f8bf2cb991794e0cdc43dbc23df5b7acf90 SRPM: http://dl.dropbox.com/u/2682668/discount-2.1.2-2.fc15.src.rpm One problem that I can still see is that 2 of the binaries probably have overly generic names. "markdown" is used by at least 1 other package ("python-markdown") and the sample program "theme" is very generic. I would be inclined to rename "markdown" to "discount" and "theme" to something like "discount-theme" but I'm not really sure how naming conflicts should be resolved. I'm just reading through the guidelines now but I thought I would mention this now anyway. Thanks.
Craig, are you still interested in this package? If so, please upload the SRPM again. After having a quick look at the spec file, you'll get a name clash for /usr/bin/makepage. Package python-markdown already provides this file. Also, the other filenames (makepage and theme) are a bit too generic. You should ask upstream to rename them or add a prefix. Also see http://fedoraproject.org/wiki/Packaging:Conflicts#Binary_Name_Conflicts
Sorry, I didn't read your latest comment completely and didn't notice that you already found the filename clash. If upstream doesn't agree to rename the files, just add a prefix to them.
Hi Martin. Yes, I'm still interested in the package. I had been maintaining the spec file in another git repository and had forgot about this one. The other repository is rather cluttered with other things, so I kept this GitHub Gist for the review, so as to keep the noise out of the commit log. I have just pushed one old commit I made in December and one more I made just now. The most recent one fixes the filename clashes, along with various other things (see changelog and specfile comments). Hope this is ok. rpmlint output: 5 packages and 1 specfiles checked; 0 errors, 0 warnings. The spec is still at: https://gist.github.com/1240217 Latest SRPM is at: http://dl.dropbox.com/u/2682668/fedora-remix/16/source/packages/discount-2.1.3-1.fc16.src.rpm
Hmm, I didn't really do any research or forward thinking with this renaming thing. I'm starting to think renaming the binary from "markdown" to "discount" might not be the best aproach. Case in point: Discount: * Ships with a full set of man pages * Has no external runtime dependencies * Is fast * Passes the markdown test suite 100% * Is already is Debian and OpenSUSE repositories without any file renaming v.s. python-markdown: * Requires a full Python distribution at runtime * Has "a few known issues" not conforming with the test suite * Is comparatively slow * Ships without any man pages * and most notably: recently had it's "markdown" binary renamed in Fedora to markdown_py[1] [1]: http://pkgs.fedoraproject.org/gitweb/?p=python-markdown.git;a=commitdiff;h=2ff5e4af8eaae64f8d73a686cc913c9208d0e52f In my mind, this seems to make a pretty strong case for using discount to provive "/usr/bin/markdown". I'm just not so sure about the sample programs. Any suggestions? OpenSuse and Debian seem to package them as-is, without any renames, although they still seem far too generic to me.
Updated spec: https://gist.github.com/1240217/bb5e0727a035d4597123f84587610187da1c8886 ...and SRPM: http://dl.dropbox.com/u/2682668/fedora-remix/16/source/packages/discount-2.1.3-2.fc16.src.rpm
The renaming of /usr/bin/markdown to /usr/bin/markdown_py simplifies things a bit. However, python-markdown 2.1.0 was only pushed to rawhide (F17). The current release branches still provide 2.0.3. Thus, if you want to build discount for Fedora < 17 as well, you might want to ask the maintainer of python-markdown whether it's possible to safely rename the binary. Since python-markdown is required by a couple of critical packages (e.g. bodhi-server, transifex) it's probably not a good idea to update to 2.1.0 in F16. Concerning the demo programs, I think the prefixed names discount-* are fine. I'll have a closer look at your SRPM in the next couple of days. Currently, I'm too busy. Just a quick note: You should not add files more than once, so drop the %doc files from the base package. They are installed by its dependency libmarkdown. If you don't have a sponsor yet, I can sponsor you. But I would like you to do some informal reviews to show a basic understanding of the packaging and review guidelines. Just pick a yet uncommented review request (one that's not blocked by FE-NEEDSPONSOR) from the review queue [1], check the items from list [2], and post your comments into the bug ticket. [1] http://fedoraproject.org/PackageReviewStatus/NEW.html [2] https://fedoraproject.org/wiki/Packaging:ReviewGuidelines
Updates: * Tue Jan 24 2012 Craig Barnes <cr> - 2.1.3-3 - Remove duplicate docs from base package (already included in libmarkdown) - Add --enable-all-features flag to "turn on all stable, optional features" - Specify single include file (mkdio.h) instead of using glob matching - Make man3 and man7 file matching more accurate (specify the "mkd" prefix) Spec: https://gist.github.com/1240217/5fb9f9fea692bef24362706abd4c69efbc114b8c SRPM: http://dl.dropbox.com/u/2682668/fedora-remix/16/source/packages/discount-2.1.3-3.fc16.src.rpm
SRPM link above is invalid. The spec itself looks pretty clean. You can remove the first line of %install as it is unnecessary. Not having the srpm, I have to ask instead of looking: is the configure.sh file some kind of autoconf-generated configure script? If so, why not use the %configure macro and get the proper compiler flags set up for free? If not, why is there a build dependency on autoconf?
Thanks for the feedback. It appears that autoconf has never been a requirement of discount - not sure why I added that in there. I have removed this and also the unnecessary buildroot clean-up in the latest release. I should probably also note that I am maintaining the spec file in a different place now and that there has been one other release (2.1.3-4) between the latest (2.1.3-5) and the last one that I linked to above (2.1.3-3). I have retired the old link to avoid any confusion and will try to avoid moving things around in future. The spec is now at: https://raw.github.com/craigbarnes/packages/master/discount.spec and the SRPM: https://github.com/downloads/craigbarnes/packages/discount-2.1.3-5.fc17.src.rpm Also, in retrospect, the changes I made in 2.1.3-4 now seem a little hacky. Any feedback on that is appreciated.
Forget to mention, the patch is now at: https://raw.github.com/craigbarnes/packages/master/sources/discount/discount-ldconfig.patch
I've just cleaned up the changes made in 2.1.3-4. I have no remaining doubts about this spec now. The latest revision passes rpmlint with no warnings. https://raw.github.com/craigbarnes/packages/master/discount.spec https://github.com/downloads/craigbarnes/packages/discount-2.1.3-6.fc17.src.rpm
I am triaging old review tickets. I apologize that it has been so long since anyone looked at this ticket, but there are more packages submitted now than the pool of reviewers can handle, and some tickets fall through the cracks. In order to keep the queue manageable, we need to occasionally find tickets which are not reviewable so as to not waste what reviewer time is available. Accordingly, I'm pinging this ticket and setting NEEDINFO. If you are still interested in having your package reviewed, please do the following: * Make sure your package still reflects the current status of its upstream. * Check that your package still builds on current Fedora releases. * Audit your package versus the current status of the packaging guidelines, current rpmlint and current fedora-review tools. And, finally, reply, making sure that the NEEDINFO flag gets cleared so that this ticket reappears in the review queue. I can't promise a review if you reply, but by closing out the stale tickets we can devote extra attention to the ones which aren't stale.
I'm still maintaining the package and building/testing for each new Fedora stable release. The the spec was updated to the latest Discount release (2.1.6) last month. Spec: https://raw.github.com/craigbarnes/packages/master/discount.spec F18 SRPM: https://github.com/craigbarnes/packages/raw/gh-pages/18/source/discount-2.1.6-1.fc18.src.rpm I'm subscribed to the Fedora announcement lists and I've kept up with changes to the packaging guidelines, although this package in particular hasn't required any changes. I'm also maintaining a couple dozen other packages[1] but haven't opened a packaging request for any of them yet, since I require sponsorship (if I remember correctly) and assumed if this one has been open for 18 months, that I was unlikely to get far with the others. [1]: https://github.com/craigbarnes/packages/
Thanks for replying. Actually for new packagers it's actually better if you can put up two or three packages so that sponsors can get some idea of your packaging abilities. Otherwise you're asking us to judge based on one really simple package. In any case, though, you're obviously dedicated to getting this package in, and while I don't have a whole lot of spare time, it doesn't appear that anyone else is going to jump in and take care of this, so I'll commit to doing this. If for some reason I don't respond quickly enough or let this go idle for even a couple of days, please ping me.
Looks like there wasn't much for me to do; this package is clean. Sorry it took so long for you to receive a review. I would go ahead and sponsor you now and get this done, but I don't believe you've mentioned your FAS account anywhere in this ticket, there is more than one "Craig Barnes" in FAS and neither of them seems to share an email address with the address you're using for bugzilla or any address in the spec changelog. So if you'll just give me that bit of info before we can proceed. * source files match upstream. sha256sum: 702bb29e17e387f82e40fae062d5e4939bc6fb22dcf53e6109982a5faa110796 discount-2.1.6.tar.bz2 * package meets naming and versioning guidelines. * specfile is properly named, is cleanly written and uses macros consistently. * summary is OK. * description is OK. * dist tag is present. * license field matches the actual license. * license is open source-compatible. * license text included in package. * latest version is being packaged. * BuildRequires are proper (none needed) * compiler flags are appropriate. * package builds in mock (rawhide, x86_64). * package installs properly. * debuginfo package looks complete. * rpmlint is silent. * final provides and requires are sane: discount-2.1.6-1.fc20.x86_64.rpm discount = 2.1.6-1.fc20 discount(x86-64) = 2.1.6-1.fc20 = libmarkdown(x86-64) = 2.1.6-1.fc20 libmarkdown.so.2()(64bit) libmarkdown-2.1.6-1.fc20.x86_64.rpm libmarkdown = 2.1.6-1.fc20 libmarkdown(x86-64) = 2.1.6-1.fc20 libmarkdown.so.2()(64bit) = /sbin/ldconfig libmarkdown-devel-2.1.6-1.fc20.x86_64.rpm libmarkdown-devel = 2.1.6-1.fc20 libmarkdown-devel(x86-64) = 2.1.6-1.fc20 = libmarkdown(x86-64) = 2.1.6-1.fc20 libmarkdown.so.2()(64bit) * %check is present and all tests pass. * no bundled libraries (that I can find) * shared libraries present: ldconfig called properly unversioned .so files are in the -devel package. * owns the directories it creates. * doesn't own any directories it shouldn't. * no duplicates in %files. * file permissions are appropriate. * no generically named files. * scriptlets are OK (ldconfig). * code, not content. * documentation is small, so no -doc subpackage is necessary. * %docs are not necessary for the proper functioning of the package. * headers in the -devel package. * no static libraries. * no libtool .la files.
Thanks. My FAS account name is "cbb". I'll update my email address on BugZilla to make the link clear.
I have sponsored you and approved this package. Please let me know if you need any assistance with getting your package imported and built. You should be approximately at this step: https://fedoraproject.org/wiki/Join_the_package_collection_maintainers#Add_Package_to_Source_Code_Management_.28SCM.29_system_and_Set_Owner but it may take an hour for the new permissions to propagate.
Thanks for the pointer. The wiki says the next step in the procedure is to make an "SCM admin request", which requires that I set the "fedora-cvs" flag to "?" on BugZilla. I can't seem to find that flag anywhere. The wiki also says: "If you are newly sponsored member of the Fedora Packager group you might have to wait an hour before you will get the permission to set the flag in Bugzilla as the sync is done hourly" ...but my BugZilla account still has "There are no permission bits set on your account" in the Permissions section. Am I missing something or should I just wait a little longer for the permissions to appear?
Disregard that last comment. I spoke to the folks in #fedora-admin and apparently it was a problem with a cron job not syncing FAS/BZ permissions correctly. It's fixed now.
New Package SCM Request ======================= Package Name: discount Short Description: An implementation of the Markdown language in C Owners: cbb Branches: f17 f18 f19 InitialCC:
Git done (by process-git-requests).
discount-2.1.6-1.fc18 has been submitted as an update for Fedora 18. https://admin.fedoraproject.org/updates/discount-2.1.6-1.fc18
discount-2.1.6-1.fc19 has been submitted as an update for Fedora 19. https://admin.fedoraproject.org/updates/discount-2.1.6-1.fc19
discount-2.1.6-1.fc19 has been pushed to the Fedora 19 testing repository.
discount-2.1.6-1.fc18 has been pushed to the Fedora 18 stable repository.
discount-2.1.6-1.fc19 has been pushed to the Fedora 19 stable repository.