llnl / llnl/blt

Modify behavior of `blt_list_append` ?

Open
#654 0 comments 0 reactions 0 assignees View on GitHub
design question User Request
Dominant language
C++
Stars
296
Forks
66
Avg merge
1h 3m
Merged PRs (30d)
1

Description

The Axom project recently fixed a bug related to `ENV` variables that were not correctly setup -- https://github.com/LLNL/axom/pull/1168

It revealed that `blt_list_append` generates a `FATAL_ERROR` when attempting to append empty strings to a list.
https://github.com/LLNL/blt/blob/d57f7995ff8299e4612e74ec94e35eacde93cf40/cmake/BLTInstallableMacros.cmake#L40-L42

Is this the desired behavior? Or would it be better if appending an empty string to a list was a no-op?

Ben Liu suggested some additional options:
* `blt_list_append` should emit a warning in this case since this is often unexpected behavior
* `blt_list_append` should take an (optional) additional parameter regarding how to treat this case, e.g. `FATAL_ERROR`, warning or no-op. If we pursue this option, we'd also need to decide on the default behavior.

@white238 noted that CMake's `(list(APPEND )` is a no-op

Contributor guide

Open the contributing guide

Research direction

Start by reviewing cmake/BLTInstallableMacros.cmake at lines 40-42 and the linked Axom pull request. Resolve whether empty-string appends should be fatal, a warning, configurable, or a no-op, using CMake's list(APPEND) behavior as a reference. Done means the chosen behavior and default are implemented and verified.

Written by the indexing model from the issue text.

Assessment

Tech stack
cmake
Domain
build-system
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.