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.

Sign up for GitHub

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

Jump to bottom

Use the system version of libyaml when possible #52

Closed
wants to merge 3 commits into from
Closed

Use the system version of libyaml when possible #52

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c0f6a3b
Use the system version of libyaml if possible.
clalancette Oct 26, 2022
79f13c2
Update the Quality declarations.
clalancette Oct 26, 2022
cf63f94
Feedback from review.
clalancette Feb 13, 2023
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
46 changes: 11 additions & 35 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,15 +110,8 @@ macro(build_libyaml)
endmacro()

# Skip building yaml if the expected version is already present in the system
if(yaml_FOUND)
if("${yaml_VERSION}" VERSION_EQUAL 0.2.5)
set(_SKIP_YAML_BUILD 1)
endif()
endif()

if(NOT _SKIP_YAML_BUILD)
if(NOT yaml_FOUND)
build_libyaml()
set(extra_test_dependencies libyaml-0.2.5)
endif()

ament_export_dependencies(yaml)
Expand All @@ -142,37 +135,20 @@ if(BUILD_TESTING)
get_target_property(ament_cmake_cppcheck_ADDITIONAL_INCLUDE_DIRS
performance_test_fixture::performance_test_fixture INTERFACE_INCLUDE_DIRECTORIES)

if(${CMAKE_VERSION} VERSION_LESS "3.13")
link_directories(${yaml_LIBRARY_DIRS})
endif()

ament_add_gtest(test_yaml_reader test/test_reader.cpp
WORKING_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}")
if(TARGET test_yaml_reader)
target_include_directories(test_yaml_reader PRIVATE ${yaml_INCLUDE_DIRS})
target_link_libraries(test_yaml_reader ${yaml_LIBRARIES})
if(${CMAKE_VERSION} VERSION_GREATER_EQUAL "3.13")
target_link_directories(test_yaml_reader PRIVATE ${yaml_LIBRARY_DIRS})
endif()
ament_target_dependencies(test_yaml_reader rcpputils)
if(extra_test_dependencies)
add_dependencies(test_yaml_reader ${extra_test_dependencies})
endif()
endif()
WORKING_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}")
target_link_libraries(test_yaml_reader
rcpputils::rcpputils
yaml
)

add_performance_test(benchmark_reader test/benchmark/benchmark_reader.cpp
WORKING_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}")
ament_target_dependencies(benchmark_reader rcpputils)
if(TARGET benchmark_reader)
target_include_directories(benchmark_reader PRIVATE ${yaml_INCLUDE_DIRS})
target_link_libraries(benchmark_reader ${yaml_LIBRARIES})
if(${CMAKE_VERSION} VERSION_GREATER_EQUAL "3.13")
target_link_directories(benchmark_reader PRIVATE ${yaml_LIBRARY_DIRS})
endif()
if(extra_test_dependencies)
add_dependencies(benchmark_reader ${extra_test_dependencies})
endif()
endif()
target_link_libraries(benchmark_reader
performance_test_fixture::performance_test_fixture
rcpputils::rcpputils
yaml
)
endif()

install(DIRECTORY cmake DESTINATION share/${PROJECT_NAME})
Expand Down
4 changes: 2 additions & 2 deletions QUALITY_DECLARATION.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ This document is a declaration of software quality for the `libyaml_vendor` pack

The package `libyaml_vendor` claims to be in the **Quality Level 1** category.

Below are the rationales, notes, and caveats for this claim, organized by each requirement listed in the [Package Quality Categories in REP-2004](https://www.ros.org/reps/rep-2004.html) of the ROS2 developer guide.
Below are the rationales, notes, and caveats for this claim, organized by each requirement listed in the [Package Quality Categories in REP-2004](https://www.ros.org/reps/rep-2004.html) of the ROS 2 developer guide.

## Version Policy [1]

Expand Down Expand Up @@ -98,7 +98,7 @@ The `libyaml` package is benchmarked and the most recent test results can be fou

### Linters and Static Analysis [4.v]

`libyaml_vendor` uses and passes all the ROS2 standard linters and static analysis tools as described in the [ROS 2 Developer Guide](https://docs.ros.org/en/rolling/Contributing/Developer-Guide.html#linters-and-static-analysis). Passing implies there are no linter/static errors when testing against CI of supported platforms.
`libyaml_vendor` uses and passes all the ROS 2 standard linters and static analysis tools as described in the [ROS 2 Developer Guide](https://docs.ros.org/en/rolling/Contributing/Developer-Guide.html#linters-and-static-analysis). Passing implies there are no linter/static errors when testing against CI of supported platforms.

Currently nightly test results can be seen here:

Expand Down
34 changes: 19 additions & 15 deletions libyaml_q_declaration.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,7 @@ First, a summary discussing how this library is qualified is presented, and then

The `libyaml` meets the basic requirements for a software platform in terms of testing its basic functionality, providing a [valid license](https://github.com/yaml/libyaml/blob/master/License) for the code used and a public Github repository with the changes made to the code over time.

Even though the library does not provide an API/ABI policy targeting the desired use of the library, the imported version of `libyaml` through the `libyaml_vendor` package is fixed to the 0.18 version. This will make the API/ABI stable.

There is no explicit support for any OS platform, however their [Github repository](https://github.com/yaml/libyaml) installation appears to be targeting Linux.
There is no explicit support for any OS platform, however the [Github repository](https://github.com/yaml/libyaml) installation appears to be targeting Linux.
The first version of this library was developed in 2006, and it is used widely.
There is no explicit metric of how much the library is used, but the equivalent library for Python, developed by the same organization is required for at least 150k repositories (According to [Github metrics](https://github.com/yaml/pyyaml/network/dependents?package_id=UGFja2FnZS01MjUyMjEzNQ%3D%3D)).
`libyaml` library is used for some optional fast functionality.
Expand All @@ -22,46 +20,50 @@ It is also used in the [Go-yaml project](https://github.com/go-yaml/yaml), the p
ROS 2 uses this library to load parameters from a file when the arguments are properly used. There are no official tests or code coverage information about this library, but in this case we are only interested in the portion of public API that reads YAML files and we will cover these
tests and coverage in the vendored package. Including specific tests for the `libyaml` version imported by `libyaml_vendor` for the tier 1 platforms listed in [REP-2000](https://www.ros.org/reps/rep-2000.html#support-tiers).

Considering the previously mentioned reasons, we consider this library to be robust and reliable and at Quality Level 1.
Considering the previously mentioned reasons, we consider this library to be reliable and at Quality Level 3.

# Comparison with ROS packages quality standards

## Version policy [1]

### Version Scheme [1.i]

It is not stated if the library supports any kind of version policy. But the version increases with a new release. The imported version of
`libyaml` through the `libyaml_vendor` package is fixed to the 0.18 version.
It is not stated if the library supports any kind of version policy.
But the version increases with a new release.

### Version Stability [1.ii]

Current version of `libyaml` in [its repository](https://github.com/yaml/libyaml) is 0.24, however, for the purposes of ROS2 Quality Level analysis, the imported version of `libyaml` through the `libyaml_vendor` package is fixed to the 0.18 version.
`libyaml` has no releases with a stable version number (>= 1.0.0).
However, given the widespread use of the library major changes to the API seem unlikely.

### Public API Declaration [1.iii]

As a C library, elements available in [yaml.h](https://github.com/yaml/libyaml/blob/master/include/yaml.h) are considered to be the library's public API.

### API Stability Policy [1.iv]

There is no policy for API stability. This is not a problem because the `libyaml_vendor` package importing the `libyaml` dependency is using a fixed version, in this case, the [0.18](https://github.com/yaml/libyaml/tree/release-0.1.8).
There is no stated policy for API stability.
However, major changes to the API are deemed unlikely.

### ABI Stability Policy [1.v]

There is no policy for ABI stability. This is not a problem because the `libyaml_vendor` package importing the `libyaml` dependency is using a fixed version, in this case, the [0.18](https://github.com/yaml/libyaml/tree/release-0.1.8).
There is no stated policy for ABI stability.

### ABI and ABI Stability Within a Released ROS Distribution [1.vi]
### API and ABI Stability Within a Released ROS Distribution [1.vi]

There is not a direct correlation between the `libyaml` releases and the ROS distributions, however this is not a problem because the `libyaml_vendor` package importing the `libyaml` dependency is using a fixed version, in this case, the [0.18](https://github.com/yaml/libyaml/tree/release-0.1.8)
Each ROS distribution uses a specific version of `libyaml`, either via the operating system or through this vendor package.
ROS distributions target specific operating systems at release time, and are maintained on those operating systems throughout their lifetime.
Thus, API and ABI should remain stable throughout the lifetime of a particular ROS distribution.

## Change Control Process [2]

### Change Requests [2.i]

Checking through the commits history, it can be seen is not the case.
Looking throug the commits history, it can be seen that not all changes occur through a change request.

### Contributor Origin [2.ii]

Does not have it (or it does not seem like it’s the case).
It does not seem like `libyaml` has a policy for Contributor Origin.

### Peer Review Policy [2.iii]

Expand All @@ -85,7 +87,8 @@ Not available.

### Feature Documentation [3.i]

Provided [doxygen documentation](https://github.com/yaml/libyaml/tree/master/doc) for the whole project. It is not provided as a document, it has to be built separately when downloading.
Provided [doxygen documentation](https://github.com/yaml/libyaml/tree/master/doc) for the whole project.
It is not provided as a document, it has to be built separately when downloading.

### Public API Documentation [3.ii]

Expand Down Expand Up @@ -153,4 +156,5 @@ Currently nightly results can be seen here:

### Vulnerability Disclosure Policy [7.i]

The `libyaml` library does not have a Vulnerability Disclosure Policy. But for ROS 2's purposes, see the policy defined in the Quality Declaration of the `libyaml_vendor` package.
The `libyaml` library does not have a Vulnerability Disclosure Policy.
But for ROS 2's purposes, see the policy defined in the Quality Declaration of the `libyaml_vendor` package.