rleigh-codelibre commented on a change in pull request #2:
URL: https://github.com/apache/xerces-c/pull/2#discussion_r443149185



##########
File path: CMakeLists.txt
##########
@@ -177,8 +177,14 @@ install(
 # Process subdirectories
 add_subdirectory(doc)
 add_subdirectory(src)
-add_subdirectory(tests)
-add_subdirectory(samples)
+if (NOT (DEFINED XERCES_BUILD_FUZZERS))

Review comment:
       I would prefer that the fuzzer keep its conditionals self-contained if 
possible.  That is, rather than disabling other unrelated features, it should 
only enable or disable the fuzzing functionality.  If you want to disable 
building of the tests and/or samples, I think they should have separate options 
to allow them to be enabled or disabled independently.
   
   You could then do `-Dtests=OFF -Dsamples-OFF -Dfuzzer=ON` to achieve the 
same effect.
   
   Also, the extra compile-time stuff using ExternalProject looks separate from 
building the fuzzer itself.  Maybe always build the fuzzer but have that 
additional stuff gated on the `fuzzer` option.  Or gate all of it.  But I would 
put that conditional stuff *inside* the fuzzer CMakeLists.txt rather than 
having any conditionals around include statements.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: c-dev-unsubscr...@xerces.apache.org
For additional commands, e-mail: c-dev-h...@xerces.apache.org

Reply via email to