Bug 225709

Summary: Merge Review: doxygen
Product: [Fedora] Fedora Reporter: Nobody's working on this, feel free to take it <nobody>
Component: Package ReviewAssignee: Thomas Spura <tomspur>
Status: CLOSED RAWHIDE QA Contact: Fedora Package Reviews List <fedora-package-review>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: christoph.wickert, redhat-bugzilla, than, tomspur
Target Milestone: ---Flags: tomspur: fedora-review+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: 1.6.1-4.fc13 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2009-12-19 17:27:47 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:

Description Nobody's working on this, feel free to take it 2007-01-31 18:30:09 UTC
Fedora Merge Review: doxygen

http://cvs.fedora.redhat.com/viewcvs/devel/doxygen/
Initial Owner: than

Comment 1 Robert Scheck 2009-01-13 22:22:50 UTC
doxygen.i386: W: file-not-utf8 /usr/share/doc/doxygen-1.5.7.1/LANGUAGE.HOWTO
doxygen.i386: W: summary-ended-with-dot A documentation system for C/C++.
doxygen.i386: W: no-version-in-last-changelog
doxygen.i386: E: tag-not-utf8 %changelog
doxygen.src: E: non-utf8-spec-file /tmp/doxygen-1.5.7.1-1.src.rpm.6705/doxygen.spec
doxygen.src:71: W: configure-without-libdir-spec
doxygen.src: W: summary-ended-with-dot A documentation system for C/C++.
doxygen.src: W: no-version-in-last-changelog
doxygen.src: E: tag-not-utf8 %changelog

Comment 2 Thomas Spura 2009-10-23 14:30:25 UTC
Some new/different rpmlint output (too long to copy&paste, instead a summary):

- configure-without-libdir-spec
  is ok, because the configure script doesn't support that.

- summary-ended-with-dot A documentation system for C/C++.
  summary-ended-with-dot A GUI for creating and editing configuration files.

- incoherent-version-in-changelog 1.6.1-1 ['1:1.6.1-1.fc13', '1:1.6.1-1']
  epoch is missing in changelog

- spurious-executable-perm /usr/share/doc/doxygen-1.6.1/examples/page/html/*
- doc-file-dependency /usr/share/doc/doxygen-1.6.1/examples/include/html/*
  "find examples -type f | xargs chmod -x" at the end of %build fixes this

- file-not-utf8 /usr/share/doc/doxygen-1.6.1/LANGUAGE.HOWTO
  iconv --from=ISO-8859-1 --to=UTF-8 LANGUAGE.HOWTO > LANGUAGE.HOWTO.new && \
  touch -r LANGUAGE.HOWTO LANGUAGE.HOWTO.new && \
  mv LANGUAGE.HOWTO.new LANGUAGE.HOWTO


- wrong-script-interpreter /usr/share/doc/doxygen-1.6.1/examples/tag/html/installdox perl


Not in rpmlint:

It would be better, if you split it examples and put them into a doc subpackage, because the most space of this package is consumed by examples, which the less people will watch. With splitting it's up to them to install them and other people to save space.

Comment 3 Than Ngo 2009-12-11 14:25:17 UTC
(In reply to comment #2)
> Some new/different rpmlint output (too long to copy&paste, instead a summary):
> 
> - configure-without-libdir-spec
>   is ok, because the configure script doesn't support that.
> 
> - summary-ended-with-dot A documentation system for C/C++.
>   summary-ended-with-dot A GUI for creating and editing configuration files.
> 
it's fixed

> - incoherent-version-in-changelog 1.6.1-1 ['1:1.6.1-1.fc13', '1:1.6.1-1']
>   epoch is missing in changelog
> 

it's fixed

> - spurious-executable-perm /usr/share/doc/doxygen-1.6.1/examples/page/html/*
> - doc-file-dependency /usr/share/doc/doxygen-1.6.1/examples/include/html/*
>   "find examples -type f | xargs chmod -x" at the end of %build fixes this
>

it's fixed

> - file-not-utf8 /usr/share/doc/doxygen-1.6.1/LANGUAGE.HOWTO
>   iconv --from=ISO-8859-1 --to=UTF-8 LANGUAGE.HOWTO > LANGUAGE.HOWTO.new && \
>   touch -r LANGUAGE.HOWTO LANGUAGE.HOWTO.new && \
>   mv LANGUAGE.HOWTO.new LANGUAGE.HOWTO
> 
> 
it's fixed

> - wrong-script-interpreter
> /usr/share/doc/doxygen-1.6.1/examples/tag/html/installdox perl
> 
>
it's fixed

> Not in rpmlint:
> 
> It would be better, if you split it examples and put them into a doc
> subpackage, because the most space of this package is consumed by examples,
> which the less people will watch. With splitting it's up to them to install
> them and other people to save space.  

no, the package is small enough, it doesn't make sense to split it.

doxygen-1.6.1-2 includes the fix for all above issues

Comment 4 Thomas Spura 2009-12-11 17:49:58 UTC
(In reply to comment #3)
> (In reply to comment #2)
> > Some new/different rpmlint output (too long to copy&paste, instead a summary):
> > 
> > - configure-without-libdir-spec
> >   is ok, because the configure script doesn't support that.
> > 
> > - summary-ended-with-dot A documentation system for C/C++.
> >   summary-ended-with-dot A GUI for creating and editing configuration files.
> > 
> it's fixed
Yes, now the same for subpackage:

oxygen-doxywizard.x86_64: W: summary-ended-with-dot A GUI for creating and editing configuration files.

> > - file-not-utf8 /usr/share/doc/doxygen-1.6.1/LANGUAGE.HOWTO
> >   iconv --from=ISO-8859-1 --to=UTF-8 LANGUAGE.HOWTO > LANGUAGE.HOWTO.new && \
> >   touch -r LANGUAGE.HOWTO LANGUAGE.HOWTO.new && \
> >   mv LANGUAGE.HOWTO.new LANGUAGE.HOWTO
> > 
> > 
> it's fixed

Not yet, you wrote 'touch -r LANGUAGE.HOWTO.new', but you need to write
'touch -r LANGUAGE.HOWTO LANGUAGE.HOWTO.new'

> 
> > - wrong-script-interpreter
> > /usr/share/doc/doxygen-1.6.1/examples/tag/html/installdox perl
> > 
> >
> it's fixed

Not here... Are you sure?


- https://fedoraproject.org/wiki/Packaging/Guidelines#.25global_preferred_over_.25define



By the way.
I don't know where the 'with_qt' flag is coming from, but maybe it would be more usefull to use bcond_with.

See bug #540328 comment #1 as example.

Comment 5 Than Ngo 2009-12-18 13:32:03 UTC
(In reply to comment #4)
> (In reply to comment #3)
> > (In reply to comment #2)
> > > Some new/different rpmlint output (too long to copy&paste, instead a summary):
> > > 
> > > - configure-without-libdir-spec
> > >   is ok, because the configure script doesn't support that.
> > > 
> > > - summary-ended-with-dot A documentation system for C/C++.
> > >   summary-ended-with-dot A GUI for creating and editing configuration files.
> > > 
> > it's fixed
> Yes, now the same for subpackage:
> 
> oxygen-doxywizard.x86_64: W: summary-ended-with-dot A GUI for creating and
> editing configuration files.
> 

it's fixed in 1:1.6.1-3

> > > - file-not-utf8 /usr/share/doc/doxygen-1.6.1/LANGUAGE.HOWTO
> > >   iconv --from=ISO-8859-1 --to=UTF-8 LANGUAGE.HOWTO > LANGUAGE.HOWTO.new && \
> > >   touch -r LANGUAGE.HOWTO LANGUAGE.HOWTO.new && \
> > >   mv LANGUAGE.HOWTO.new LANGUAGE.HOWTO
> > > 
> > > 
> > it's fixed
> 
> Not yet, you wrote 'touch -r LANGUAGE.HOWTO.new', but you need to write
> 'touch -r LANGUAGE.HOWTO LANGUAGE.HOWTO.new'
> 

t's fixed in 1:1.6.1-3

> > 
> > > - wrong-script-interpreter
> > > /usr/share/doc/doxygen-1.6.1/examples/tag/html/installdox perl
> > > 
> > >
> > it's fixed
> 
> Not here... Are you sure?
> 
> 
> -
> https://fedoraproject.org/wiki/Packaging/Guidelines#.25global_preferred_over_.25define
> 
> 
> 

t's fixed in 1:1.6.1-3

> By the way.
> I don't know where the 'with_qt' flag is coming from, but maybe it would be
> more usefull to use bcond_with.
> 
it's not needed, i dropped the conditinal

Comment 6 Thomas Spura 2009-12-18 13:57:42 UTC
FYI: BR: tex(latex) is more general than texlive-latex

Please use %global instead %define (I'm unsure, if this is new or I didn't see this last time).


##########################

This was my first merge review, so I don't know the exact procedure...

I'll approve this, when it's in devel, too.

Comment 7 Than Ngo 2009-12-18 14:23:46 UTC
(In reply to comment #6)
> FYI: BR: tex(latex) is more general than texlive-latex
> 
> Please use %global instead %define (I'm unsure, if this is new or I didn't see
> this last time).
> 
> 

it's not new and i don't see why we cannot use %define.


> ##########################
> 
> This was my first merge review, so I don't know the exact procedure...
> 
> I'll approve this, when it's in devel, too.  

it's built in devel

Comment 8 Thomas Spura 2009-12-18 14:56:20 UTC
That's what https://fedoraproject.org/wiki/Packaging/Guidelines#.25global_preferred_over_.25define says:

"""Use %global instead of %define, unless you really need only locally defined submacros within other macro definitions (a very rare case). """

In this case, it won't be a problem to use %define and not %global, but if FESCo decides to ust %global everywhere and not %define, we should simply get used to it or feel free to complain at https://fedorahosted.org/fesco/ 


If you use some %ifs or %{!?foo: ... } constructs and %define something in it, this could be gone, if you are out of the %if again, but still need it. Sometimes rpm removes the %define again, sometimes not...

That's the main reason, why not using %define.

Comment 9 Thomas Spura 2009-12-18 14:58:27 UTC
Sorry, setting the review flag to early...

%define is a blocker...

Comment 10 Christoph Wickert 2009-12-18 16:16:14 UTC
(In reply to comment #9)

> %define is a blocker...  

No it's not, since
a) it is only a recommendation to use %global and
b) it's only a draft that has never passed a FESCo vote.

Comment 11 Than Ngo 2009-12-18 16:19:07 UTC
the %define is not needed, and dropped in doxygen-1.6.1-4.

Comment 12 Thomas Spura 2009-12-19 17:27:47 UTC
(In reply to comment #10)
> (In reply to comment #9)
> 
> > %define is a blocker...  
> 
> No it's not, since
> a) it is only a recommendation to use %global and
> b) it's only a draft that has never passed a FESCo vote.  

Sorry then, in irc the consensus was it's a blocker...

################


APPROVED