Skip to content

Conversation

@bmhan12
Copy link
Contributor

@bmhan12 bmhan12 commented Jan 15, 2026

This PR:

  • Removes unused CMake variable AXOM_ENABLE_CORE

This variable was noticed by @johnbowen42 while testing Axom builds.
The variable is only defined here and not used anywhere else in the CMake build.
The variable is only mentioned here and not anywhere else in the CMake build.

@bmhan12 bmhan12 added the Build system Issues related to Axom's build system label Jan 15, 2026
@white238
Copy link
Member

white238 commented Jan 15, 2026

The point of that was to error if someone tried to turn off Core. It wasnt creating the variable.

@adayton1 adayton1 self-requested a review January 15, 2026 21:48
@johnbowen42
Copy link

The point of that was to error if someone tried to turn off Core. It wasnt creating the variable.

I tried to manually enable core and hit this error. Maybe we could keep the variable but only error if somebody actually turns it off?

@white238
Copy link
Member

Here is where it is not created:

if(NOT "${COMPONENT_NAME_LOWERED}" STREQUAL "core")
option(AXOM_ENABLE_${COMPONENT_NAME_CAPITALIZED}
"Enables ${arg_COMPONENT_NAME}"
${arg_DEFAULT_STATE})
endif()

We could change the check to see if it is defined and set to OFF, but we should not removed the error check.

@johnbowen42
Copy link

Here is where it is not created:

if(NOT "${COMPONENT_NAME_LOWERED}" STREQUAL "core")
option(AXOM_ENABLE_${COMPONENT_NAME_CAPITALIZED}
"Enables ${arg_COMPONENT_NAME}"
${arg_DEFAULT_STATE})
endif()

We could change the check to see if it is defined and set to OFF, but we should not removed the error check.

that's what I was thinking

Copy link
Member

@white238 white238 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update if to include allowing users to set it to ON, though mildly pointless. Dont remove helpful error message.

@rhornung67
Copy link
Member

Update if to include allowing users to set it to ON, though mildly pointless. Dont remove helpful error message.

Pointless, yes. But some people may find it cathartic.... 😄

@bmhan12
Copy link
Contributor Author

bmhan12 commented Jan 15, 2026

Update if to include allowing users to set it to ON, though mildly pointless. Dont remove helpful error message.

Gotcha, adjusted.

Copy link
Member

@kennyweiss kennyweiss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like a reasonable change.
I'll echo @white238's comments that core cannot be disabled, but I suppose it's harmless to manually enable it

@white238 white238 merged commit df4d8ab into develop Jan 17, 2026
15 checks passed
@white238 white238 deleted the bugfix/han12/axom_enable_core branch January 17, 2026 04:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Build system Issues related to Axom's build system

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants