Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support non-default python layouts #123

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 35 additions & 25 deletions rosdoc2/verbs/build/builders/sphinx_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -385,24 +385,33 @@ def build(self, *, doc_build_folder, output_staging_directory):

package_xml_directory = os.path.dirname(self.build_context.package.filename)
# If 'python_source' is specified, construct 'python_src_directory' from it
python_src_directory = None
if self.build_context.python_source is not None:
python_src_directory = \
os.path.abspath(
os.path.join(
package_xml_directory,
self.build_context.python_source))
# If not provided, try to find the python source directory
else:
package_list = setuptools.find_packages(where=package_xml_directory)
if self.build_context.package.name in package_list:
python_src_directory = \
os.path.abspath(
os.path.join(
package_xml_directory,
self.build_context.package.name))
else:
if not os.path.isdir(python_src_directory):
logger.warning(f'python_source specified as {self.build_context.python_source} '
'is not a directory')
python_src_directory = None

# If not provided or invalid, try to find the python source directory
if not python_src_directory:
search_dirs = ['.', 'src']
for search_dir in search_dirs:
where = os.path.abspath(os.path.join(package_xml_directory, search_dir))
package_list = setuptools.find_packages(where=where)
if self.build_context.package.name in package_list:
python_src_directory = \
os.path.abspath(
os.path.join(
where,
self.build_context.package.name))
if python_src_directory:
break

# We will ultimately run the sphinx project from a wrapped directory. Create it now,
# so that we can put generated items there.
wrapped_sphinx_directory = os.path.abspath(
Expand Down Expand Up @@ -493,25 +502,26 @@ def build(self, *, doc_build_folder, output_staging_directory):
# If the package has python code, then invoke `sphinx-apidoc` before building
if has_python:
if not python_src_directory or not os.path.isdir(python_src_directory):
raise RuntimeError(
logger.warning(
'Could not locate source directory to invoke sphinx-apidoc in. '
'If this is package does not have a standard Python package layout, '
"please specify the Python source in 'rosdoc2.yaml'.")
cmd = [
'sphinx-apidoc',
'-o', wrapped_sphinx_directory,
'-e', # Document each module in its own page.
python_src_directory,
]
logger.info(
f"Running sphinx-apidoc: '{' '.join(cmd)}' in '{wrapped_sphinx_directory}'"
)
completed_process = subprocess.run(cmd, cwd=wrapped_sphinx_directory)
msg = f"sphinx-apidoc exited with return code '{completed_process.returncode}'"
if completed_process.returncode == 0:
logger.debug(msg)
else:
raise RuntimeError(msg)
cmd = [
'sphinx-apidoc',
'-o', wrapped_sphinx_directory,
'-e', # Document each module in its own page.
python_src_directory,
]
logger.info(
f"Running sphinx-apidoc: '{' '.join(cmd)}' in '{wrapped_sphinx_directory}'"
)
completed_process = subprocess.run(cmd, cwd=wrapped_sphinx_directory)
msg = f"sphinx-apidoc exited with return code '{completed_process.returncode}'"
if completed_process.returncode == 0:
logger.debug(msg)
else:
logger.warning(msg)

# Invoke Sphinx-build.
sphinx_output_dir = os.path.abspath(
Expand Down
2 changes: 1 addition & 1 deletion rosdoc2/verbs/build/inspect_package_for_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
## be assumed for 'sphinx-apidoc' invocation. The user can provide the path
## (relative to the 'package.xml' file) where the Python modules defined by this
## package are located.
python_source: '{package_name}'
# python_source: '{package_name}'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can the above cases not be covered by setting this non-standard location in their config files?

Implementing custom discovery logic seem fragile and like it will end up with a lot of maintenance in the longer term versus setting the non-standard location in the configuration for those handful of packages. And we can make sure that we provide a useful error/warning such that developers can know to set this setting in their config file.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to be clear, the reason for the change in the line that you quoted is because {package_name} is the default. I would expect that a sample config file shows what the default is, but does not set it unless setting it is required. It is not.


## This setting, if true, attempts to run `doxygen` and the `breathe`/`exhale`
## extensions to `sphinx` regardless of build type. This is most useful if the
Expand Down
1 change: 1 addition & 0 deletions test/packages/basic_cpp/package.xml
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,6 @@

<export>
<build_type>ament_cmake</build_type>
<rosdoc2>rosdoc2.yaml</rosdoc2>
</export>
</package>
2 changes: 1 addition & 1 deletion test/packages/basic_cpp/rosdoc2.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ settings:
## be assumed for 'sphinx-apidoc' invocation. The user can provide the path
## (relative to the 'package.xml' file) where the Python modules defined by this
## package are located.
python_source: 'basic_cpp'
# python_source: 'basic_cpp'

## This setting, if true, attempts to run `doxygen` and the `breathe`/`exhale`
## extensions to `sphinx` regardless of build type. This is most useful if the
Expand Down
12 changes: 12 additions & 0 deletions test/packages/false_python/package.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<?xml version="1.0"?>
<?xml-model href="http://download.ros.org/schema/package_format3.xsd" schematypens="http://www.w3.org/2001/XMLSchema"?>
<package format="3">
<name>false_python</name>
<version>0.0.0</version>
<description>I say I am python, but no actual python</description>
<maintainer email="[email protected]">Ye ol' Python Pro</maintainer>
<license>Apache-2.0</license>
<export>
<build_type>ament_python</build_type>
</export>
</package>
16 changes: 16 additions & 0 deletions test/packages/invalid_python_source/package.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<?xml version="1.0"?>
<?xml-model href="http://download.ros.org/schema/package_format3.xsd" schematypens="http://www.w3.org/2001/XMLSchema"?>
<package format="3">
<name>invalid_python_source</name>
<version>0.0.0</version>
<description>This packages incorrectly specifies python source</description>
<maintainer email="[email protected]">ros2 user</maintainer>
<license>Apache 2.0</license>

<buildtool_depend>ament_cmake</buildtool_depend>
<buildtool_depend>ament_cmake_python</buildtool_depend>
<export>
<build_type>ament_cmake</build_type>
<rosdoc2>rosdoc2.yaml</rosdoc2>
</export>
</package>
68 changes: 68 additions & 0 deletions test/packages/invalid_python_source/rosdoc2.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
## Default configuration, generated by rosdoc2.

## This 'attic section' self-documents this file's type and version.
type: 'rosdoc2 config'
version: 1

---

settings:
## If this is true, a standard index page is generated in the output directory.
## It uses the package information from the 'package.xml' to show details
## about the package, creates a table of contents for the various builders
## that were run, and may contain links to things like build farm jobs for
## this package or links to other versions of this package.

## If false, you can still include content that would have been in the index
## into one of your '.rst' files from your Sphinx project, using the
## '.. include::' directive in Sphinx.
## For example, you could include it in a custom 'index.rst' so you can have
## the standard information followed by custom content.

## TODO(wjwwood): provide a concrete example of this (relative path?)

## If this is not specified explicitly, it defaults to 'true'.
generate_package_index: true

## This setting is relevant mostly if the standard Python package layout cannot
## be assumed for 'sphinx-apidoc' invocation. The user can provide the path
## (relative to the 'package.xml' file) where the Python modules defined by this
## package are located.
python_source: 'i_do_not_exist'

## This setting, if true, attempts to run `doxygen` and the `breathe`/`exhale`
## extensions to `sphinx` regardless of build type. This is most useful if the
## user would like to generate C/C++ API documentation for a package that is not
## of the `ament_cmake/cmake` build type.
always_run_doxygen: false

## This setting, if true, attempts to run `sphinx-apidoc` regardless of build
## type. This is most useful if the user would like to generate Python API
## documentation for a package that is not of the `ament_python` build type.
always_run_sphinx_apidoc: false

# This setting, if provided, will override the build_type of this package
# for documentation purposes only. If not provided, documentation will be
# generated assuming the build_type in package.xml.
# override_build_type: 'ament_python'
builders:
## Each stanza represents a separate build step, performed by a specific 'builder'.
## The key of each stanza is the builder to use; this must be one of the
## available builders.
## The value of each stanza is a dictionary of settings for the builder that
## outputs to that directory.
## Required keys in the settings dictionary are:
## * 'output_dir' - determines output subdirectory for builder instance
## relative to --output-directory
## * 'name' - used when referencing the built docs from the index.

- doxygen: {
name: 'basic_cpp Public C/C++ API',
output_dir: 'generated/doxygen'
}
- sphinx: {
name: 'basic_cpp',
## This path is relative to output staging.
doxygen_xml_directory: 'generated/doxygen/xml',
output_dir: ''
}
1 change: 1 addition & 0 deletions test/packages/src_alt_python/launch/dummy.launch.py
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
print('This is a dummy launch file')
12 changes: 12 additions & 0 deletions test/packages/src_alt_python/package.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<?xml version="1.0"?>
<?xml-model href="http://download.ros.org/schema/package_format3.xsd" schematypens="http://www.w3.org/2001/XMLSchema"?>
<package format="3">
<name>src_alt_python</name>
<version>0.0.0</version>
<description>Python with python module name different from ros name</description>
<maintainer email="[email protected]">Ye ol' Python Pro</maintainer>
<license>Apache-2.0</license>
<export>
<build_type>ament_python</build_type>
</export>
</package>
Empty file.
6 changes: 6 additions & 0 deletions test/packages/src_alt_python/src/other_name/python_node.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
def main():
print('Hello, world.')


if __name__ == '__main__':
main()
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# This package should not be the python source because it is a sub package
12 changes: 12 additions & 0 deletions test/packages/src_python/package.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<?xml version="1.0"?>
<?xml-model href="http://download.ros.org/schema/package_format3.xsd" schematypens="http://www.w3.org/2001/XMLSchema"?>
<package format="3">
<name>src_python</name>
<version>0.0.0</version>
<description>Basic Python node with source layout</description>
<maintainer email="[email protected]">Ye ol' Python Pro</maintainer>
<license>Apache-2.0</license>
<export>
<build_type>ament_python</build_type>
</export>
</package>
Empty file.
6 changes: 6 additions & 0 deletions test/packages/src_python/src/src_python/python_node.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
def main():
print('Hello, world.')


if __name__ == '__main__':
main()
15 changes: 15 additions & 0 deletions test/packages/too_many_python_packages/package.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<?xml version="1.0"?>
<?xml-model href="http://download.ros.org/schema/package_format3.xsd" schematypens="http://www.w3.org/2001/XMLSchema"?>
<package format="3">
<name>too_many_python_packages</name>
<version>0.0.0</version>
<description>Too many unspecified python packages</description>
<maintainer email="[email protected]">ros2 user</maintainer>
<license>Apache 2.0</license>

<buildtool_depend>ament_cmake</buildtool_depend>
<buildtool_depend>ament_cmake_python</buildtool_depend>
<export>
<build_type>ament_cmake</build_type>
</export>
</package>
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
def main():
print('Hello, world.')


if __name__ == '__main__':
main()
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
def main():
print('Hello, world.')


if __name__ == '__main__':
main()
50 changes: 50 additions & 0 deletions test/test_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,56 @@ def test_only_python(session_dir):
links_exist=links_exist)


def test_src_python(session_dir):
PKG_NAME = 'src_python'
do_build_package(DATAPATH / PKG_NAME, session_dir)

includes = ['src_python package']
links_exist = ['src_python.html']

do_build_package(DATAPATH / PKG_NAME, session_dir)

do_test_package(PKG_NAME, session_dir,
includes=includes,
links_exist=links_exist)


def test_false_python(session_dir):
PKG_NAME = 'false_python'
do_build_package(DATAPATH / PKG_NAME, session_dir)

excludes = ['python api']
includes = ['I say I am python, but no actual python']

do_test_package(PKG_NAME, session_dir,
includes=includes,
excludes=excludes)


def test_invalid_python_source(session_dir):
PKG_NAME = 'invalid_python_source'
do_build_package(DATAPATH / PKG_NAME, session_dir)

excludes = ['python api']
includes = ['This packages incorrectly specifies python source']

do_test_package(PKG_NAME, session_dir,
includes=includes,
excludes=excludes)


def test_too_many_python_packages(session_dir):
PKG_NAME = 'too_many_python_packages'
do_build_package(DATAPATH / PKG_NAME, session_dir)

excludes = ['python api']
includes = ['Too many unspecified python packages']

do_test_package(PKG_NAME, session_dir,
includes=includes,
excludes=excludes)


def test_only_messages(session_dir):
"""Test a package only containing messages."""
PKG_NAME = 'only_messages'
Expand Down