Bug 249892 - Review Request: bouml-doc - Documentation for the BOUML tool
Review Request: bouml-doc - Documentation for the BOUML tool
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Mamoru TASAKA
Fedora Extras Quality Assurance
:
Depends On: 247417
Blocks:
  Show dependency treegraph
 
Reported: 2007-07-27 14:05 EDT by Debarshi Ray
Modified: 2007-11-30 17:12 EST (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-08-16 14:04:50 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
mtasaka: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Debarshi Ray 2007-07-27 14:05:42 EDT
Spec URL: http://rishi.fedorapeople.org/bouml-doc.spec
SRPM URL: http://rishi.fedorapeople.org/bouml-doc-2.29.1-1.fc8.src.rpm

Description:

Documentation of the BOUML tool provided in HTML and PDF formats.
Comment 1 Debarshi Ray 2007-07-27 14:09:56 EDT
I have submitted a review request for the bouml package here:
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=247417
Comment 2 Debarshi Ray 2007-07-30 10:08:05 EDT
There has been a new upstream release for BOUML-- version 2.30. However the
documentation was last revised on May 20th 2007 and is compatible with BOUML
releases since 2.26.2. See http://bouml.free.fr/download.html

How do I handle the value for the Version tag?
Comment 3 Mamoru TASAKA 2007-07-30 11:34:41 EDT
IMO it is better that the EVR is 0-0.date20070520, for example

http://fedoraproject.org/wiki/Packaging/NamingGuidelines#NonNumericRelease
Comment 4 Mamoru TASAKA 2007-07-30 11:36:40 EDT
Ah.. 0-0.X.date20070520 is better, where X is incremented as
1, 2, 3, .....
Comment 5 Debarshi Ray 2007-07-30 23:32:49 EDT
That results in a strange looking /usr/share/doc/bouml-doc-0 as the
documentation directory. Is that fine?
Comment 6 Mamoru TASAKA 2007-07-30 23:44:58 EDT
Um, then:

--------------------------------------------------
%install
<snip>
# Reorganizing PDF.
cp -p %{SOURCE1} ./pdf
cp -p %{SOURCE2} ./pdf

mkdir $RPM_BUILD_ROOT%_datadir/doc/%name
cp -pr html pdf $RPM_BUILD_ROOT%_datadir/doc/%name

%files
%defattr(-,root,root,-)
%_datadir/doc/%name
----------------------------------------------------
is admitted.

Just note: files under %_datadir/doc is automatically marked as
           %doc.
* Please use "cp -p" to keep timestamp
Comment 7 Debarshi Ray 2007-08-01 02:09:09 EDT
(In reply to comment #6)
 
> mkdir $RPM_BUILD_ROOT%_datadir/doc/%name
> cp -pr html pdf $RPM_BUILD_ROOT%_datadir/doc/%name

Just for the sake of consistency, would it be a problem if I used 'install' instead?
Comment 8 Mamoru TASAKA 2007-08-01 02:20:51 EDT
(In reply to comment #7)
> (In reply to comment #6)
>  
> > mkdir $RPM_BUILD_ROOT%_datadir/doc/%name
> > cp -pr html pdf $RPM_BUILD_ROOT%_datadir/doc/%name
> 
> Just for the sake of consistency, would it be a problem if I used 'install'
instead?
> 

My recognition is that "install" is to copy "files" (and set 
attributes), while "cp" is to copy "files _and_ directories (if needed,
recursively)".

For for this case, "install" command cannot be used (if I am not wrong).
Comment 9 Mamoru TASAKA 2007-08-01 02:22:31 EDT
(Well, "install" can make directory, however I don't know how to
 copy files recursively using "install" command)
Comment 11 Mamoru TASAKA 2007-08-01 12:38:17 EDT
(Maybe you have received the following comment, however
 I rewrite)

Well, why don't you write just as following?
----------------------------------------------------
%files
%defattr(-,root,root,-)
%_datadir/doc/%name
-----------------------------------------------------

The file entry
-----------------------------------------------------
%files
%defattr(-,root,root,-)
foo/
-----------------------------------------------------
(where foo/ is a directory) means the directory foo/ itself and
all files/directories under foo/.

Currently %{_datadir}/doc/%{name}/html/ and %{_datadir}/doc/%{name}/pdf/
are not owned by any packages.
Comment 12 Debarshi Ray 2007-08-01 13:26:38 EDT
(In reply to comment #11)

> (Maybe you have received the following comment, however
>  I rewrite)

Actually you are the first one to state it. :-)

> Well, why don't you write just as following?
> ----------------------------------------------------
> %files
> %defattr(-,root,root,-)
> %_datadir/doc/%name

I tend to use the more verbose form for the sake of readability. A quick glance
at the %files stanza reminds me that a directory (in this case
%{_datadir}/doc/%{name}) has been created by the package, and a short listing of
what it contains. I find this necessary since the %install stanza becomes too
'dirty' to realise what files are present with just a quick look at it.

However I don't prefer to list out every file and sub-directory if the list is
too long because that would defeat the initial idea.

%dir %{_datadir}/doc/%{name}
%{_datadir}/doc/%{name}/html/*
%{_datadir}/doc/%{name}/pdf/*
...is just enough to remind me that the documentation directory, named
%{_datadir}/doc/%{name}, contains the both the HTML and PDF representations.

Another example where I have followed this style is in gengetopt.spec
(https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=243607):
%dir %{_datadir}/%{name}
%{_datadir}/%{name}/getopt.c
%{_datadir}/%{name}/getopt1.c
%{_datadir}/%{name}/gnugetopt.h
Since there were only 3 files, I listed them explicitly.

If this does not violate the packaging guidelines and if you are compfortable
with it, then I would be happy to keep them this way. :-)
Comment 13 Mamoru TASAKA 2007-08-01 13:33:59 EDT
Well what is the real problem is that currently the directories
%{_datadir}/doc/%{name}/html/ and %{_datadir}/doc/%{name}/pdf/
*themselves* are not owned by any package.

In your way you must write:
------------------------------------------------------
%dir %_datadir/doc/%name
%dir %_datadir/doc/%name/html
%datadir/doc/%name/html/*
...........
------------------------------------------------------
or
------------------------------------------------------
%dir %_datadir/doc/%name
%_datadir/doc/%name/html/
.......
------------------------------------------------------
Comment 14 Mamoru TASAKA 2007-08-01 13:37:56 EDT
(Well, the reason I comment my way is because directory
 ownership issue becomes difficult to check and is frequently
 overlooked in verbose list way...)
Comment 15 Debarshi Ray 2007-08-01 13:55:15 EDT
Spec: http://rishi.fedorapeople.org/bouml-doc.spec

I took the middle path.
%files
%defattr(-,root,root,-)
%dir %{_datadir}/doc/%{name}
%{_datadir}/doc/%{name}/html
%{_datadir}/doc/%{name}/pdf

I am unable to upload the SRPM since I do not have the bandwidth to upload a
26MB file. If you want I can upload it tomorrow.
Comment 16 Mamoru TASAKA 2007-08-01 14:49:08 EDT
Newest spec file is okay,
however this review request is blocked by bouml.
Comment 17 Mamoru TASAKA 2007-08-06 10:50:44 EDT
Now I can see that the manual files are numbered, so please
update your spec/srpm as such.
Comment 18 Debarshi Ray 2007-08-06 14:21:25 EDT
What should the version-release values be? Although the files are numbered, it
still says; "Last revision August 4th 2007, up to date, new  compatible with
Bouml releases since 2.30 "?

I think I should also put in:
Requires: bouml >= 2.30
Comment 19 Mamoru TASAKA 2007-08-09 00:19:30 EDT
Sorry for reply..

(In reply to comment #18)
> What should the version-release values be?
The latest doc files seem to have the name 2.30 explicitly,
so it is reasonable that the rpm version is 2.30.
Comment 20 Debarshi Ray 2007-08-09 13:12:21 EDT
Spec: http://rishi.fedorapeople.org/bouml-doc.spec

Will be uploading the SRPM tomorrow. Don't have sufficient bandwidth now.
Comment 21 Debarshi Ray 2007-08-10 05:29:34 EDT
SRPM: http://rishi.fedorapeople.org/bouml-doc-2.30-1.fc8.src.rpm
Comment 22 Mamoru TASAKA 2007-08-11 11:25:40 EDT
Actually almost nothing to be reviewed, however:

* rpmlint complaints:
--------------------------------------------------
W: bouml-doc invalid-license GPL
W: bouml-doc no-%build-section
--------------------------------------------------
  - Change the license to GPLv2+
  - Add %build section, even if it is empty
--------------------------------------------------
$ rpmlint -I no-%build-section
no-%build-section :
The spec file does not contain a %build section.  Even if some packages
don't directly need it, section markers may be overridden in rpm's
configuration to provide additional "under the hood" functionality, such as
injection of automatic -debuginfo subpackages.  Add the section, even if
empty.
--------------------------------------------------
  Other thing okay. Please fix the issue above before
  committing to CVS.


--------------------------------------------------
   This package (bouml-doc) is APPROVED by me
--------------------------------------------------

Note: the template of CVSAdminProcedure changed so
      please re-read
      http://fedoraproject.org/wiki/PackageMaintainers/CVSAdminProcedure
Comment 23 Debarshi Ray 2007-08-11 17:34:18 EDT
Spec: http://rishi.fedorapeople.org/bouml-doc.spec

New Package CVS Request
=======================
Package Name: bouml-doc
Short Description: Documentation for the BOUML tool.
Owners: debarshi.ray@gmail.com
Branches: FC-6, F-7
InitialCC: 
Commits by cvsextras: no
Comment 24 Kevin Fenzi 2007-08-11 18:25:30 EDT
cvs done.
Comment 25 Debarshi Ray 2007-08-12 03:50:43 EDT
Spec: http://rishi.fedorapeople.org/bouml-doc.spec

I had to update the Spec to fix this build
issue:http://koji.fedoraproject.org/koji/getfile?taskID=98683&name=build.log

Last %changelog entry:

%changelog
* Sun Aug 12 2007 Debarshi Ray <rishi@fedoraproject.org> - 2.30-3
- Added 'mkdir -p $RPM_BUILD_ROOT' in install stanza.
Comment 26 Mamoru TASAKA 2007-08-12 11:49:43 EDT
(In reply to comment #25)
> Spec: http://rishi.fedorapeople.org/bouml-doc.spec
> 
> I had to update the Spec to fix this build
> issue:http://koji.fedoraproject.org/koji/getfile?taskID=98683&name=build.log
> 
> Last %changelog entry:
> 
> %changelog
> * Sun Aug 12 2007 Debarshi Ray <rishi@fedoraproject.org> - 2.30-3
> - Added 'mkdir -p $RPM_BUILD_ROOT' in install stanza.

Maybe you can (or should) file against "rpm". I guess this is due to
recent changes in /usr/lib/rpm/find-debuginfo.sh (in rpm-build).

Anyway when rebuild is done, please close this bug.

Comment 27 Mamoru TASAKA 2007-08-16 10:43:52 EDT
Once you rebuilt this, please close this bug.

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