bugTransport Sample Protocol - Bugs: bug #20919, RPM build fails if doxygen is not...

 
 

bug #20919: RPM build fails if doxygen is not found

Submitted by:  Frederik Deweerdt <deweerdt>
Submitted on:  Tue 28 Aug 2007 08:00:40 AM UTC  
 
Category:  Build System Severity:  3 - Normal
Status:  In Progress Privacy:  Public
Assigned to:  Frederik Deweerdt <deweerdt> Open/Closed:  Open
Release:  CVS Fixed Release:  NextRelease

Add a New Comment (Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

( Jump to the original submission)

Wed 07 Nov 2007 01:04:23 PM UTC, comment #9: 

Ok thank you very much Robert.
I was kind of busy, sorry for not answering
you previous post.

I'll give this a try tomorrow
but I think we have converged to an OK solution.

Eric NOULARD <erk>
Project Administrator
Tue 06 Nov 2007 08:52:23 PM UTC, comment #8: 

A small experiment shows that it is not necessary to cache the DOXYGEN_PACKAGE DOXYGEN_DOT_PACKAGE and the RPM_MAKE_APIDOC variables.
Eric, I have checked in your patch plus the changes to remove the above mentioned variables from the cache.
This brings make/FindDoxygen.cmake almost back to what it was in revision 1.4 (minus a logic bug for setting the DOXYGEN_DOT_PACKAGE variable and minus the DOXYGEN_DOT_PATH variable).

Robert de Vries <rhdv>
Project Member
Tue 06 Nov 2007 11:01:54 AM UTC, comment #7: 

The fix looks fine.
You can see who actually knows cmake and who is just hacking around a bit. :-)
Anyway I have another question about the need for CACHE.
As I see it now there is no need to cache the RPM_BUILD_APIDOC variable. Maybe the other DOXYGEN_PACKAGE/DOXYGEN_DOT_PACKAGE variables also no longer need to be cached too.
Time for a little expirement (either you or me, but I will only have time this evening).

Robert de Vries <rhdv>
Project Member
Tue 06 Nov 2007 09:19:50 AM UTC, comment #6: 

Hi Robert,

Thanks for the fix.
Some remark about CMake usage and
possible second pass fix.

You shouldn't
INCLUDE(FindDoxygen)
but use
FIND_PACKAGE(Doxygen)

there are several mechanisms involved
in FIND_PACKAGE which are not handled
when using INCLUDE.
(see cmake --help-command FIND_PACKAGE
  or cmake --help-command INCLUDE)

Concerning the the MAKE_APIDOC add
I think it shouldn't be in FindDoxygen.cmake
but in our main CMakeLists.txt
In fact FindDoxygen.cmake included in tsp source
is a (modified) derivative taken from CMake distros.
I did submit an update to CMake team and will be pleased
to remove the TSP one as soon as CMake embbed
the new one, this would make 1 file less to maintain :=)

Moreover in our CMakeLists.txt there is
the
OPTION(BUILD_API_DOC)
which indicates whether if current build should
build doxygen doc.

Thus, I propose to rename MAKE_APIDOC
to RPM_MAKE_APIDOC.
This would be more obvious that this var will
be used by generated RPM spec file.

This way you may have build tree for
which BUILD_API_DOC is false but
when you
make tsp_rpm
the APIDOC rpm will be generated if doxygen+dot
were found.

I have attached my proposed patch.
I may commit I you are Ok with my proposal.

(file #14326)

Eric NOULARD <erk>
Project Administrator
Mon 05 Nov 2007 09:33:52 PM UTC, comment #5: 

Fixed in the following files:
CMakeLists.txt
make/FindDoxygen.cmake
src/scripts/tsp.spec.in

Include FindDoxygen in top level CMakeLists.txt file to ensure that
the variables are propagated to the RPMTools_ADD_RPM_TARGETS macro.
Clean up logic of finding packages
Remove unused variable
Add extra variable that indicates that the apidoc package can be made.
Add %if statements to spec file to conditionally create apidoc package
if both doxygen and dot are found.

Robert de Vries <rhdv>
Project Member
Sat 03 Nov 2007 12:26:01 PM UTC, comment #4: 

The trouble was that
DOXYGEN_PACKAGE and DOXYGEN_DOT_PACKAGE
were not define as CACHE CMake variables in
make/FindDoxygen.cmake.

thus when the RPMTools_ADD_RPM_TARGETS is called
[ macro defined in make/UseRPMTools.cmake
  and used in main CMakeLists.txt          ]

the variables weren't defined anymore because
FindDoxygen.cmake was called in a subdir (src/doc)
and not in the main CMakeLists.txt.

In CMakeLists.txt when you recurse in subdir
you inherit the var definition from the parent
but parent does not inherit var definition from
subdir unless you set those vars as CACHE var.

I did replace
SET(DOXYGEN_PACKAGE doxygen)
with
SET(DOXYGEN_PACKAGE doxygen CACHE STRING "The rpm package name containing doxygen")

in make/FindDoxygen.cmake

[same for DOXYGEN_DOT_PACKAGE]

I've checked in the file.
You may update, it should work now.
You'll have to explicitely rerun cmake in order to
make thoses vars appears in CMakeCache.txt.

I agree with your solution of
%if statements around the %package apidoc

Eric NOULARD <erk>
Project Administrator
Fri 02 Nov 2007 10:18:09 PM UTC, comment #3: 

I noticed that even though doxygen and graphviz have been detected, their package names are not substituted in the package file.
Does anyone with more cmake knowledge have any idea what might cause this behaviour?

Robert de Vries <rhdv>
Project Member
Fri 02 Nov 2007 07:56:33 AM UTC, comment #2: 

Ah it is already a build dependency, but a conditional one.
So the fix is to add a couple of %if statements around the %package apidoc bit.
Let me see what I can do here.

Robert de Vries <rhdv>
Project Member
Fri 02 Nov 2007 07:46:27 AM UTC, comment #1: 

Or you could make doxygen a build dependency.
Otherwise you must disable the apidoc part of the RPM spec file depending on the availability of doxygen.
I prefer that people creating RPM packages do not have half way installed development systems.
What you might get is different contents of RPM packages (or not all packages) depending on the build system.

Robert de Vries <rhdv>
Project Member
Tue 28 Aug 2007 08:00:40 AM UTC, original submission:  

If doxygen is not found, the RPM build fails because we issue make apidoc anyway.
Suggested fix is to disable apidoc building if doxygen was not found.

Frederik Deweerdt <deweerdt>
Project AdministratorIn charge of this item.

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #14326:  patch-bug20919-erk-1.patch added by erk (2KiB - text/x-patch - One more fix proposal for doxygen+RPM)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by erk (Posted a comment)
  • -email is unavailable- added by rhdv (Posted a comment)
  • -email is unavailable- added by deweerdt (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

     

     

    Follows 1 latest change.

    Date Changed by Updated Field Previous Value => Replaced by
    2007-11-06 erk Attached File- => Added patch-bug20919-erk-1.patch, #14326

    Back to the top


    Powered by Savane 3.5