Integrate manpage with CMake better #10624

matejak · 2023-05-23T15:25:49Z

Pass the build information to the script that generates the manpage from the template to

use CMake paths, and to
leave out info about artifacts that are not built.

Left to do:

Describe the tailoring accordingly.
Update static parts of the template if we know how.

Rationale:

Somewhat addresses https://bugzilla.redhat.com/show_bug.cgi?id=2060028

Review Hints:

The manpage is generated to the top of the build directory. Check it out.

openshift-ci · 2023-05-23T15:25:55Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

github-actions · 2023-05-23T15:55:52Z

Start a new ephemeral environment with changes proposed in this pull request:

Fedora Environment

Oracle Linux 8 Environment

jan-cerny · 2023-06-05T11:39:36Z

CMakeLists.txt

@@ -26,6 +26,7 @@ set(SSG_CONTENT_INSTALL_DIR "${CMAKE_INSTALL_DATADIR}/xml/scap/ssg/content")
 set(SSG_GUIDE_INSTALL_DIR "${CMAKE_INSTALL_DOCDIR}/guides")
 set(SSG_TABLE_INSTALL_DIR "${CMAKE_INSTALL_DOCDIR}/tables")
 set(SSG_ANSIBLE_ROLE_INSTALL_DIR "${CMAKE_INSTALL_DATADIR}/scap-security-guide/ansible")
+set(SSG_ANSIBLE_PER_RULE_PLAYBOOKS_ISNTALL_DIR "${SSG_ANSIBLE_ROLE_INSTALL_DIR}/rule_playbooks")


jan-cerny · 2023-06-05T11:43:18Z

cmake/SSGCommon.cmake

@@ -79,7 +79,7 @@ endmacro()
 macro(ssg_build_man_page)
    add_custom_command(
        OUTPUT "${CMAKE_BINARY_DIR}/scap-security-guide.8"
-        COMMAND env "PYTHONPATH=$ENV{PYTHONPATH}" "${PYTHON_EXECUTABLE}" "${SSG_BUILD_SCRIPTS}/generate_man_page.py" --template "${CMAKE_SOURCE_DIR}/docs/man_page_template.jinja" --input_dir "${CMAKE_BINARY_DIR}" --output "${CMAKE_BINARY_DIR}/scap-security-guide.8"
+	COMMAND env "PYTHONPATH=$ENV{PYTHONPATH}" "${PYTHON_EXECUTABLE}" "${SSG_BUILD_SCRIPTS}/generate_man_page.py" --template "${CMAKE_SOURCE_DIR}/docs/man_page_template.jinja" --input_dir "${CMAKE_BINARY_DIR}" --output "${CMAKE_BINARY_DIR}/scap-security-guide.8" --install-prefix "${CMAKE_INSTALL_PREFIX}" --separate-scap-files "${SSG_SEPARATE_SCAP_FILES_ENABLED}:${SSG_CONTENT_INSTALL_DIR}" --profile-bash "${SSG_BASH_SCRIPTS_ENABLED}:${SSG_BASH_ROLE_INSTALL_DIR}" --profile-ansible "${SSG_ANSIBLE_PLAYBOOKS_ENABLED}:${SSG_ANSIBLE_ROLE_INSTALL_DIR}" --ansible-per-rule "${SSG_ANSIBLE_PLAYBOOKS_PER_RULE_ENABLED}:${SSG_ANSIBLE_PER_RULE_PLAYBOOKS_ISNTALL_DIR}" --kickstarts "ON:${SSG_KICKSTART_INSTALL_DIR}" --tailoring "ON:${SSG_TAILORING_INSTALL_DIR}" --content-path "${SSG_CONTENT_INSTALL_DIR}"


inconsistent indentation

jan-cerny · 2023-06-05T11:47:21Z

docs/man_page_template.jinja

+.RS
+Contains delta tailoring, whatever that means.
+.RE
+{{%- endif %}}


Have you considered dropping the 2 sections about deployment below?

jan-cerny · 2023-06-05T11:48:36Z

docs/man_page_template.jinja

+{{% if tailoring_path -%}}
+.I {{{ install_prefix }}}/{{{ tailoring_path }}}/
+.RS
+Contains delta tailoring, whatever that means.


This isn't a good text for a man page. The man page should provide specific information. I propose:

Contains tailoring files that enable rules that are not covered by third-party SCAP content and disables rules that are covered by the content shipped in scap-security-guide.

jan-cerny · 2023-06-05T12:05:38Z

build-scripts/generate_man_page.py

-    substitution_dicts = {"products": all_products}
+    substitution_dicts = dict(
+        products=all_products,
+        content_path=args.content_path,


Would it be possible to handle also these items by parse_arguments_into_dict as well?

Pass the build information to the script that generates the manpage from the template to - use CMake paths, and to - leave out info about artifacts that are not built.

The text may make sense, but it should not be in an upstream project. Regulations like these should be maintained and supplied by institutions where the project is used, and should not be visible to the upstream general public.

jan-cerny

I have build and reviewed the generated manual page.

I like the proposed changes very much. It's a great improvement!

I have some fresh new ideas:

You can modernize the example in the examples section by showcasing the usage of the --results-arf option instead of the --results and --oval-results.
you can link the project repo instead of the wiki
we sometimes incorrectly use "datastream" or other misspelled variants, we should always use "data stream"

Feel free to apply the ideas or mark the PR as ready to review.

codeclimate · 2023-06-09T17:04:53Z

Code Climate has analyzed commit bc2a32a and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (50% is the threshold).

This pull request will bring the total coverage in the repository to 52.8%.

View more on Code Climate.

jan-cerny

I have seen the generated man page for rhel8.

Thank you.

openshift-ci bot added the do-not-merge/work-in-progress Used by openshift-ci bot. label May 23, 2023

jan-cerny reviewed Jun 5, 2023

View reviewed changes

jan-cerny self-assigned this Jun 5, 2023

matejak force-pushed the manpage branch from b667d5b to 676a1b6 Compare June 8, 2023 11:28

matejak added 4 commits June 8, 2023 14:53

Integrate manpage with CMake better

47dd0e3

Pass the build information to the script that generates the manpage from the template to - use CMake paths, and to - leave out info about artifacts that are not built.

Address reviewer feedback

8139324

Remove the deployment section

eeb7c0b

The text may make sense, but it should not be in an upstream project. Regulations like these should be maintained and supplied by institutions where the project is used, and should not be visible to the upstream general public.

Fix a cmake linting error

d19fbee

matejak force-pushed the manpage branch from 676a1b6 to d19fbee Compare June 8, 2023 12:54

jan-cerny added this to the 0.1.69 milestone Jun 9, 2023

jan-cerny added the Documentation Update in project documentation. label Jun 9, 2023

jan-cerny reviewed Jun 9, 2023

View reviewed changes

Introduce minor text improvements

bc2a32a

matejak marked this pull request as ready for review June 9, 2023 16:21

openshift-ci bot removed the do-not-merge/work-in-progress Used by openshift-ci bot. label Jun 9, 2023

jan-cerny approved these changes Jun 12, 2023

View reviewed changes

jan-cerny merged commit 897eaae into ComplianceAsCode:master Jun 12, 2023
34 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Integrate manpage with CMake better #10624

Integrate manpage with CMake better #10624

matejak commented May 23, 2023 •

edited

openshift-ci bot commented May 23, 2023

github-actions bot commented May 23, 2023

jan-cerny Jun 5, 2023

jan-cerny Jun 5, 2023

jan-cerny Jun 5, 2023

jan-cerny Jun 5, 2023

jan-cerny Jun 5, 2023

jan-cerny left a comment

codeclimate bot commented Jun 9, 2023

jan-cerny left a comment

Integrate manpage with CMake better #10624

Integrate manpage with CMake better #10624

Conversation

matejak commented May 23, 2023 • edited

Rationale:

Review Hints:

openshift-ci bot commented May 23, 2023

github-actions bot commented May 23, 2023

jan-cerny Jun 5, 2023

Choose a reason for hiding this comment

jan-cerny Jun 5, 2023

Choose a reason for hiding this comment

jan-cerny Jun 5, 2023

Choose a reason for hiding this comment

jan-cerny Jun 5, 2023

Choose a reason for hiding this comment

jan-cerny Jun 5, 2023

Choose a reason for hiding this comment

jan-cerny left a comment

Choose a reason for hiding this comment

codeclimate bot commented Jun 9, 2023

jan-cerny left a comment

Choose a reason for hiding this comment

matejak commented May 23, 2023 •

edited