CMake Chem and Chem+KPP Build #2018
Conversation
|
The Jenkins test results: |
… value This now mirrors the make build and allows the make build to supply the WRF_CHEM and WRF_KPP option via defines/command line similar to the make build. Likewise, while there are files that use or or any variation of this, these were never being used by the make build. This may be a bug, but the priority of the cmake build is to acheive parity with the make build in compile flags and overall effect.
There was a problem hiding this comment.
While replacing the call to system(cp_command) with a safer alternative would be a good improvement, this system call was not introduced in this PR. Addressing it might be better suited for a future PR.
|
|
||
| system(cp_command); | ||
| #ifndef NO_COPY | ||
| system(cp_command); |
There was a problem hiding this comment.
Calling system() with a command-line-provided argument introduces significant security risks, as it can allow command injection if the input is not carefully sanitized. Could we replace system(cp_command) with a safer alternative that avoids directly invoking the shell?
For example, the following implementation securely copies files using fork() and execlp():
#include <unistd.h>
#include <sys/wait.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
void copy_file(const char *source, const char *destination) {
pid_t pid = fork();
if (pid == 0) {
// Child process: Securely execute the `cp` command
execlp("cp", "cp", source, destination, NULL);
perror("execlp failed"); // If exec fails
exit(EXIT_FAILURE);
} else if (pid > 0) {
// Parent process: Wait for the child to complete
int status;
waitpid(pid, &status, 0);
if (WIFEXITED(status)) {
printf("Copy completed with exit code %d\n", WEXITSTATUS(status));
} else {
fprintf(stderr, "Copy process terminated abnormally\n");
}
} else {
perror("fork failed");
exit(EXIT_FAILURE);
}
}
int main(int argc, char *argv[]) {
if (argc != 2) {
fprintf(stderr, "Usage: %s \"<source_file> <destination_file>\"\n", argv[0]);
return EXIT_FAILURE;
}
// Parse the cp_command argument into source and destination
char *cp_command = argv[1];
char *source = strtok(cp_command, " ");
char *destination = strtok(NULL, " ");
if (!source || !destination) {
fprintf(stderr, "Error: Invalid command. Provide source and destination files.\n");
return EXIT_FAILURE;
}
// Call the copy_file function
printf("Copying '%s' to '%s'...\n", source, destination);
copy_file(source, destination);
return EXIT_SUCCESS;
}
|
I'm getting a compilation error that's probably due to something I'm doing wrong. On Derecho, starting with the default modules I first switch to the GNU compilers with |
|
@mgduda I think I've isolated the issue to the version drop in #2132. Configuration should have looked something like Even after fixing these issues, the I've tested reverting back to Note: the reason this wasn't caught in normal testing for the version requirement drop is the default for normal compilation is to use BASIC nesting which does not use the |
Other parts of the cmake build rely on language overrides being fed into the compiler. While this could be adjusted to maintain the lower version number, these adjustments to the source code are outside the scope of these changes. Coupled with the only-recent version drop, the more appropriate approach is to revert the version back to the higher working version and evaluate the necessary code adjustments for cmake 3.19 to be viable at a later time in a separate feature.
|
@amstokely @mgduda Update pushed to (unfortunately) revert back to v3.20. I looked into converting |
TYPE: enhancement
KEYWORDS: cmake, chem, kpp
SOURCE: internal
DESCRIPTION OF CHANGES:
Problem:
Current CMake build does not build chem or chem+kpp configurations
Solution:
Build kpp and associated tools, and cmake commands to facilitate simplified logic of the
configure_wkcandcompile_wkcscripts. As with all CMake builds, all auto-generated source code is placed in the out-of-source build directory.Notable differences to make build :
-j Nparallel jobs to generate KPP sources up to a limitmodule_kpp_*tuv_kppa directory to locate where include file is to be generated, and allow control of file IO mode*registryuses-DWRF_CHEMand-DWRF_KPPdefines passed at command line instead ofgetenv()to match all other options**Affects make build in subtle ways but do not change user instructions
LIST OF MODIFIED FILES:
M CMakeLists.txt
M chem/CMakeLists.txt
A chem/KPP/CMakeLists.txt
M chem/KPP/compile_wkc
A chem/KPP/kpp/kpp-2.1/CMakeLists.txt
A chem/KPP/util/wkc/CMakeLists.txt
M chem/KPP/util/wkc/gen_kpp.c
M chem/KPP/util/wkc/protos_kpp.h
M chem/KPP/util/wkc/tuv_kpp.c
A chem/KPP/util/write_decomp/CMakeLists.txt
M chem/KPP/util/write_decomp/Makefile
M chem/KPP/util/write_decomp/integr_edit.c
M chem/chem_driver.F
M tools/CMakeLists.txt
M tools/data.h
M tools/registry.c
TESTS CONDUCTED:
RELEASE NOTE:
CMake Chem and Chem+KPP Build