feat: auto-apply features marked default = true in the manifest#1302
Open
krystophny wants to merge 8 commits into
Open
feat: auto-apply features marked default = true in the manifest#1302krystophny wants to merge 8 commits into
krystophny wants to merge 8 commits into
Conversation
Extend the argv-based Fortran compile spawn to Windows so it no longer falls back to the shell there. Tokenize with the OS-native lexer (ms_split on Windows, sh_split on Unix) so backslash paths and quoting survive, route run_argv through c_run_argv on both platforms, and add a CreateProcessA path with per-process stdout/stderr redirect. The shell run remains as a fallback when the spawn returns a negative status. Windows std handles are only overridden when redirecting, to avoid stripping the child console; out-of-range child exit codes are clamped positive so they are not mistaken for the spawn-failure sentinel.
Replace the shell run() in link_executable/link_shared/make_archive with an argv spawn through run_argv (posix_spawn on Unix, CreateProcess on Windows), which is fork-safe under OpenMP, and drop the critical(run_command) that previously serialized them. Link and archive steps now run in parallel; the rare shell fallback (tokenization failure) and run_argv's own shell fallback stay serialized under critical(run_command). make_archive reproduces 'self%ar // output' exactly: the archive name glues onto the last flag token for MSVC 'lib /OUT:' and is a separate token for GNU 'ar -rs '. Promote split_append/clean_shlex_token to module scope so link/archive reuse the compile path tokenization.
Apply the argv spawn (run_argv: posix_spawn on Unix, CreateProcess on Windows, both fork-safe) to compile_c and compile_cpp, matching compile_fortran. Serialize the rare shell fallbacks (tokenization failure) in compile_c/compile_cpp/compile_fortran under critical(run_command). No unguarded concurrent shell fork() then remains in the compile, link, or archive build paths.
Under OpenMP the build loop runs compile_fortran/compile_c/compile_cpp on many threads. Each tokenizes its flags through fortran-shlex (split_append) and then registers a compile_commands entry (cct_register), which tokenizes again. The lexer returns a deferred-length result array that was read back into the argv list outside the critical region, so a concurrent re-entry of the lexer corrupted the in-flight tokens: the compiler received mangled flags (-fPIC truncated to -fP, several flags merged into one), aborting the build intermittently on CI. Hold the fpm_shlex_split lock across both the split and the readback in split_append, and lock cct_register's tokenize plus result consumption under the same name so the two sites never enter the lexer concurrently.
build_target created the output directory inside an unnamed `!$omp critical`. mkdir shells out twice (is_dir runs `test -d`, then `mkdir -p`), and both fork. The unnamed critical is a different lock than `critical(run_command)` that guards every other shell fork (the compile/link/archive fallbacks), so a mkdir fork on one thread could run concurrently with a run_command fork on another - the exact concurrent-fork hazard the run_command lock exists to prevent. Put the directory creation under `critical(run_command)` so all shell forks share one lock.
krystophny
force-pushed
the
feat/openmp-default-parallel-build
branch
from
June 4, 2026 07:18
091126d to
441f782
Compare
Add a `default` field to feature_collection_t. When a feature declares `default = true` in fpm.toml and the build does not pass an explicit `--features` list, apply that feature automatically (alongside the existing default-profile mechanism). An explicit `--features` list opts out. This is a generic mechanism: any feature can be made default, not just openmp. fpm's own fpm.toml does not set `default = true` for openmp because libgomp+--static segfaults at atexit on gfortran 10-13 (known issue, see acdaf19 and fortran-lang#962). Users opt in per-feature in their own projects. Changes: - feature_collection_t gains `is_default` logical, parsed from the `default` key in the feature subtable - export_config iterates features with is_default instead of looking up "openmp" by name - `default` added to the allowed-key whitelist in feature validation - traverse_feature_table skips the `default` key during traversal - Serialization (dump/load/same) updated for the new field
krystophny
force-pushed
the
feat/openmp-default-parallel-build
branch
from
June 4, 2026 07:26
441f782 to
55329ed
Compare
This was referenced Jun 4, 2026
When --features is specified but empty, allocate a zero-size features array so present(features) is true in export_config. This suppresses auto-default features, giving CI and users an explicit opt-out for configurations where a default feature is unsafe (e.g. --static + libgomp on gcc 10-13). Without this, --features "" was treated the same as not specifying --features at all, and auto-defaults would still apply.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
default = truefield to feature definitions in fpm.toml--featureslist is given, features withdefault = trueare automatically applied (alongside the existing default-profile mechanism)
--features ""(explicit empty) opts out of auto-defaults--features "openmp,mpi"(explicit list) applies only listed featuresContext
After the argv spawn chain (#1299, #1300, #1301) and the mkdir fork-safety
fix (#1298), the parallel build backend is fully fork-safe. This PR lets
packages opt into default features automatically via the manifest.
A package that wants OpenMP by default declares:
CI or users on compilers where a default feature is unsafe (e.g.
libgomp+--static on gcc 10-13, see #962) opt out with
--features "".Bootstrap note: fpm's own
fpm.tomlcannot setdefault = trueyetbecause the bootstrap fpm (v0.13.0) rejects unknown keys in feature tables.
Once this change lands in a release and the bootstrap is bumped, a follow-up
adds
default = trueto fpm.toml and updates CI to use--features ""onthe affected compilers.
Change
feature_collection_tgainsis_defaultlogical, parsed fromdefaultkey in the feature subtable, serialized in dump/load/same
export_configiterates features withis_default = .true.defaultadded to allowed-key whitelist in feature table validationtraverse_feature_tableskips thedefaultkey during traversal--features ""allocates a zero-size features array sopresent(features)is true and auto-defaults are suppressedMerge order
This PR is stacked on the full fork-safety chain. Merge in order:
Until #1299-#1301 and #1298 merge, the diff includes their commits.
Also in this series:
Refs: #962
Verification
CI: gcc 10-13 pass (the mechanism is available but fpm.toml does not yet
set
default = truedue to the bootstrap constraint).