From 6cfdd40cad056875829606de9eb552a03da1c6d7 Mon Sep 17 00:00:00 2001
From: Kefu Chai <kchai@redhat.com>
Date: Thu, 19 Aug 2021 17:24:32 +0800
Subject: [PATCH 1/2] common/options: validate see-also

y2c.py is like a compiler which translates .yaml to .cc and .h files,
it does not have access to all .yaml files. to validate the dangling
see-also issue, we need to do this with a "linker".

in this change, validate-options.py is introduced to check if any of
option name included by the see-also property is valid.

Fixes: https://tracker.ceph.com/issues/51483
Signed-off-by: Kefu Chai <kchai@redhat.com>
---
 cmake/modules/AddCephTest.cmake        |  3 +-
 src/common/options/CMakeLists.txt      |  7 ++++
 src/common/options/validate-options.py | 49 ++++++++++++++++++++++++++
 3 files changed, 58 insertions(+), 1 deletion(-)
 create mode 100755 src/common/options/validate-options.py

diff --git a/cmake/modules/AddCephTest.cmake b/cmake/modules/AddCephTest.cmake
index af54e8a9c1a33..0df7125b50833 100644
--- a/cmake/modules/AddCephTest.cmake
+++ b/cmake/modules/AddCephTest.cmake
@@ -2,7 +2,8 @@
 
 #adds makes target/script into a test, test to check target, sets necessary environment variables
 function(add_ceph_test test_name test_path)
-  add_test(NAME ${test_name} COMMAND ${test_path} ${ARGN})
+  add_test(NAME ${test_name} COMMAND ${test_path} ${ARGN}
+    COMMAND_EXPAND_LISTS)
   if(TARGET ${test_name})
     add_dependencies(tests ${test_name})
     set_property(TARGET ${test_name}
diff --git a/src/common/options/CMakeLists.txt b/src/common/options/CMakeLists.txt
index 86de2e7972c80..b2b50716466ca 100644
--- a/src/common/options/CMakeLists.txt
+++ b/src/common/options/CMakeLists.txt
@@ -1,5 +1,6 @@
 set(common_options_srcs build_options.cc)
 set(legacy_options_headers)
+set(options_yamls)
 
 # to mimic the behavior of file(CONFIGURE ...)
 file(GENERATE OUTPUT configure_file.cmake
@@ -26,6 +27,8 @@ function(add_options name)
   set(yaml_file ${CMAKE_CURRENT_BINARY_DIR}/${name}.yaml)
   file_configure("${yaml_in_file}"
     "${yaml_file}" @ONLY)
+  list(APPEND options_yamls ${yaml_file})
+  set(options_yamls ${options_yamls} PARENT_SCOPE)
   set(cc_file "${name}_options.cc")
   set(h_file "${PROJECT_BINARY_DIR}/include/${name}_legacy_options.h")
   add_custom_command(PRE_BUILD
@@ -102,3 +105,7 @@ add_library(common-options-objs OBJECT
   ${common_options_srcs})
 add_custom_target(legacy-option-headers
   DEPENDS ${legacy_options_headers})
+
+include(AddCephTest)
+add_ceph_test(validate-options
+  ${Python3_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/validate-options.py ${options_yamls})
diff --git a/src/common/options/validate-options.py b/src/common/options/validate-options.py
new file mode 100755
index 0000000000000..5bc5d4d467363
--- /dev/null
+++ b/src/common/options/validate-options.py
@@ -0,0 +1,49 @@
+#!/usr/bin/env python3
+
+import argparse
+import fileinput
+import sys
+import yaml
+from typing import Any, Dict
+
+
+class ValidationError(Exception):
+    pass
+
+
+OptionType = Dict[str, Any]
+
+
+def validate_see_also(opt: OptionType, opts: Dict[str, OptionType]) -> None:
+    see_also = opt.get('see_also')
+    if see_also is None:
+        return
+    for ref in see_also:
+        if ref not in opts:
+            msg = f'see_also contains "{ref}". But it is not found.'
+            raise ValidationError(msg)
+
+
+def main() -> None:
+    parser = argparse.ArgumentParser(
+        formatter_class=argparse.ArgumentDefaultsHelpFormatter)
+    parser.add_argument('yamls', nargs='*')
+    opts = parser.parse_args()
+    options = {}
+    for yaml_file in opts.yamls:
+        with open(yaml_file) as f:
+            yml = yaml.load(f, yaml.SafeLoader)
+            options.update({opt['name']: opt for opt in yml['options']})
+    for name, opt in options.items():
+        try:
+            validate_see_also(opt, options)
+        except ValidationError as e:
+            raise Exception(f'failed to validate "{name}": {e}')
+
+
+if __name__ == '__main__':
+    try:
+        main()
+    except Exception as e:
+        print(e, file=sys.stderr)
+        sys.exit(1)

From a66daecbea3ab85af64729db2935c846f417e534 Mon Sep 17 00:00:00 2001
From: Kefu Chai <kchai@redhat.com>
Date: Thu, 19 Aug 2021 18:01:03 +0800
Subject: [PATCH 2/2] cmake: s/Python_EXECUTABLE/Python3_EXECUTABLE/

as FindPython3.cmake only sets Python3_EXECUTABLE for us, we should
stick with Python3_EXECUTABLE instead of Python_EXECUTABLE.

Signed-off-by: Kefu Chai <kchai@redhat.com>
---
 src/common/options/CMakeLists.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/common/options/CMakeLists.txt b/src/common/options/CMakeLists.txt
index b2b50716466ca..d2104a0dad267 100644
--- a/src/common/options/CMakeLists.txt
+++ b/src/common/options/CMakeLists.txt
@@ -33,7 +33,7 @@ function(add_options name)
   set(h_file "${PROJECT_BINARY_DIR}/include/${name}_legacy_options.h")
   add_custom_command(PRE_BUILD
     OUTPUT ${cc_file} ${h_file}
-    COMMAND ${Python_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/y2c.py
+    COMMAND ${Python3_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/y2c.py
       --input ${yaml_file}
       --output ${cc_file}
       --legacy ${h_file}