Bug 630754 - Review Request: mscgen - Message Sequence Chart Rendering tool
Review Request: mscgen - Message Sequence Chart Rendering tool
Status: CLOSED DUPLICATE of bug 842064
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
All Linux
low Severity medium
: ---
: ---
Assigned To: Nobody's working on this, feel free to take it
Fedora Extras Quality Assurance
Depends On:
  Show dependency treegraph
Reported: 2010-09-06 15:18 EDT by Michael McTernan
Modified: 2013-10-19 10:42 EDT (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Last Closed: 2011-03-05 10:05:00 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---

Attachments (Terms of Use)

  None (edit)
Description Michael McTernan 2010-09-06 15:18:17 EDT
Spec URL: http://www.mcternan.me.uk/mscgen/fc12/mscgen.spec
SRPM URL: http://www.mcternan.me.uk/mscgen/fc12/mscgen-0.18-1.fc12.src.rpm
Description: Mscgen is a small program that parses Message Sequence Chart descriptions and produces PNG, SVG, EPS or server side image maps as the output. It can be used standalone or with Doxygen to help document call flows and is licensed under the GPL.

Examples and more information are available at http://www.mcternan.me.uk/mscgen/
Comment 1 Michael McTernan 2010-09-06 15:23:31 EDT
Note: This is my first Fedora package.  I will need a sponsor.
Comment 2 Martin Gieseking 2010-09-07 04:18:38 EDT
Hi Michael,

here are some initial comments:

- according to the source file headers, the license seems to be GPLv2+ 
 (because of the addition "or (at your option) any later version")
  This must be reflected in the License field

- you can drop Requires: gd since it's picked up automatically as a dependency

- I suggest to run the tests coming with the tarball
  (add a %check section containing "make check")

- please use macro %{_defaultdocdir} instead of %{_docdir}

- in the %files section, replace the four %doc lines with 

- replace %{_mandir}/man1/mscgen.1.gz with %{_mandir}/man1/mscgen.1*
  (because the compression method applied by rpmbuild might change)

- In the %changelog section, you should only list changes made to the
  package/spec. Don't add the program's (upstream) changelog.
Comment 3 Michael McTernan 2010-09-07 15:08:57 EDT
Thanks for the quick feedback, and apologies for my errors.

I've updated the spec file with your comments and made a new src package.  Please see the following locations:

Spec URL: http://www.mcternan.me.uk/mscgen/fc12/mscgen.spec
SRPM URL: http://www.mcternan.me.uk/mscgen/fc12/mscgen-0.18-2.fc12.src.rpm
Comment 4 Martin Gieseking 2010-09-07 15:50:01 EDT
There's no need to apologize. Actually, the purpose of the reviewing process is to find packaging issues and to fix them. It would be boring here if nobody made mistakes any longer. :)

An additional note:
Please remove the "%doc" prefix from %doc %{_defaultdocdir}/%{name}-%{version}/
For further details see
Comment 5 Michael McTernan 2010-09-07 17:25:57 EDT
Okay - cool.  I've fixed the %doc part:

Spec URL: http://www.mcternan.me.uk/mscgen/fc12/mscgen.spec
SRPM URL: http://www.mcternan.me.uk/mscgen/fc12/mscgen-0.18-3.fc12.src.rpm

I read the packaging tricks page again, and *think* that's the only one from there...
Comment 6 Martin Gieseking 2010-09-08 03:41:07 EDT
OK, the spec file looks fine now. However, the source tarball in your SRPM doesn't match the upstream tarball:

$ md5sum mscgen-src-0.18.tar.gz*
2b6cfbabbc8bbc25eb52684410705510  mscgen-src-0.18.tar.gz
0922258a7fb86612bb623c1101260fd0  mscgen-src-0.18.tar.gz.1

You must use the untouched upstream archive to build the package. If it's necessary to modify parts of the sources, add patches to your package.
Comment 7 Michael McTernan 2010-09-12 16:28:31 EDT
There's a short delay here while I fix upstream.  Then I'll make a new package (likely 0.19-1) with matching MD5s and retry.
Comment 8 Martin Gieseking 2010-09-13 07:42:35 EDT
OK, thanks for the feedback. No need to hurry.
Comment 9 Michael Schwendt 2010-12-30 05:32:24 EST
> %files
> %defattr(-,root,root,-)
> %{_defaultdocdir}/%{name}-%{version}/

As %files sections like this keep popping up lately, note that explicitly including files in %_defaultdocdir conflicts with using %doc. It's advised to add a comment about that. For example,

 | %files
 | %defattr(-,root,root,-)
 | %{_defaultdocdir}/%{name}-%{version}/
 | %doc TODO

with Fedora 14 would give an error, whereas with older dists it might empty the contents of %_defaultdocdir and replace them with the %doc files without any warning.

# due to this entry, %doc must not be used to add any other files
Comment 10 Michael McTernan 2011-01-03 15:54:32 EST
Version 0.19 of mscgen is now out.  I've updated the spec file and added a warning comment as per Michael Schwendt's comment.  I also checked that the MD5 of the source package matches upstream.

New versions are as follows:

Spec URL: http://www.mcternan.me.uk/mscgen/fc13/mscgen.spec
SRPM URL: http://www.mcternan.me.uk/mscgen/fc13/mscgen-0.19-2.fc13.src.rpm
Comment 11 Martin Gieseking 2011-01-24 15:03:18 EST
Michael, have you already done some informal reviews of other packager's submissions? If not, I'd encourage you to do so in order to show your understanding of the packaging guidelines. This is an important part of the sponsoring process. For further information have a look at
Comment 12 Michael McTernan 2011-02-06 16:45:10 EST
I've looked at other packages to see how they fit together, particularly gd2.

I take your point though; I'll see if I can usefully comment on some other packages up for review.
Comment 13 Michael McTernan 2011-03-05 10:05:00 EST
Thanks to all that contributed to this review but I've decided not to pursue this any more.

Instead I've created my own repo for mscgen on Fedora and EPEL: 


If someone wished to package mscgen for Fedora directly I would be happy and support them, but using my own repo is easier for me.
Comment 14 Damian Wrobel 2012-07-21 10:57:17 EDT

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

Note You need to log in before you can comment on or make changes to this bug.