Bug 249892

Summary: Review Request: bouml-doc - Documentation for the BOUML tool
Product: [Fedora] Fedora Reporter: Debarshi Ray <debarshir>
Component: Package ReviewAssignee: Mamoru TASAKA <mtasaka>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: fedora-package-review, mtasaka, notting
Target Milestone: ---Flags: mtasaka: fedora-review+
kevin: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2007-08-16 18:04:50 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On: 247417    
Bug Blocks:    

Description Debarshi Ray 2007-07-27 18:05:42 UTC
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 18:09:56 UTC
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 14:08:05 UTC
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 15:34:41 UTC
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 15:36:40 UTC
Ah.. 0-0.X.date20070520 is better, where X is incremented as
1, 2, 3, .....

Comment 5 Debarshi Ray 2007-07-31 03:32:49 UTC
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-31 03:44:58 UTC
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 06:09:09 UTC
(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 06:20:51 UTC
(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 06:22:31 UTC
(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 16:38:17 UTC
(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 17:26:38 UTC
(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 17:33:59 UTC
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 17:37:56 UTC
(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 17:55:15 UTC
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 18:49:08 UTC
Newest spec file is okay,
however this review request is blocked by bouml.

Comment 17 Mamoru TASAKA 2007-08-06 14:50:44 UTC
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 18:21:25 UTC
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 04:19:30 UTC
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 17:12:21 UTC
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 09:29:34 UTC
SRPM: http://rishi.fedorapeople.org/bouml-doc-2.30-1.fc8.src.rpm

Comment 22 Mamoru TASAKA 2007-08-11 15:25:40 UTC
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 21:34:18 UTC
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
Branches: FC-6, F-7
InitialCC: 
Commits by cvsextras: no


Comment 24 Kevin Fenzi 2007-08-11 22:25:30 UTC
cvs done.

Comment 25 Debarshi Ray 2007-08-12 07:50:43 UTC
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> - 2.30-3
- Added 'mkdir -p $RPM_BUILD_ROOT' in install stanza.

Comment 26 Mamoru TASAKA 2007-08-12 15:49:43 UTC
(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> - 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 14:43:52 UTC
Once you rebuilt this, please close this bug.