Skip to content

cmake: add COMMAND_ERROR_IS_FATAL ANY to configure-time execute_process calls#438

Merged
spe-ciellt merged 1 commit intogerbv:developfrom
SourceParts:fix/execute-process-fatal-error
Mar 18, 2026
Merged

cmake: add COMMAND_ERROR_IS_FATAL ANY to configure-time execute_process calls#438
spe-ciellt merged 1 commit intogerbv:developfrom
SourceParts:fix/execute-process-fatal-error

Conversation

@rampageservices
Copy link
Copy Markdown
Contributor

Closes #403.

Problem

The three execute_process calls that run at configure time have no failure handling. If any of them exit non-zero, CMake silently continues and the failure surfaces later as a confusing downstream error — the specific example in the issue is an xgettext error caused by the header-generation step failing to produce bugs.c/authors.c.

Change

Add COMMAND_ERROR_IS_FATAL ANY to:

  • config/CMakeLists.txt — the generate_headers.cmake script invocation
  • cmake/Gettext_helpers.cmake — the xgettext .pot generation
  • cmake/Gettext_helpers.cmake — the msginit .po initialisation

cmake/GetGitVersion.cmake is left unchanged; those calls already use RESULT_VARIABLE and are intentionally tolerant of git not being installed.

COMMAND_ERROR_IS_FATAL requires CMake ≥ 3.19; the project's cmake_minimum_required is 3.28, so there is no compatibility concern.

…ss calls

Without COMMAND_ERROR_IS_FATAL, a failure in execute_process silently
continues and manifests later as a confusing downstream error (e.g.
xgettext failing on missing generated files).

Add COMMAND_ERROR_IS_FATAL ANY to the three configure-time
execute_process calls:
- config/CMakeLists.txt: generate_headers.cmake script
- cmake/Gettext_helpers.cmake: xgettext .pot generation
- cmake/Gettext_helpers.cmake: msginit .po initialisation

GetGitVersion.cmake is intentionally left unchanged; those calls use
RESULT_VARIABLE and tolerate git not being present.

Closes gerbv#403.
@spe-ciellt
Copy link
Copy Markdown
Contributor

Thanks for adding the flags. It exposed a very obvious problem, which I haven't noticed. I do not understand why this is happening in the pipeline, since it not happening locally. Needs examination.

I also noticed that when the build in the pipeline uses shallow copies of the repository and the version strings got all messy. So now I added #443 , so hopefully that solves the problem.

@eyal0
Copy link
Copy Markdown
Collaborator

eyal0 commented Mar 17, 2026

Thanks for adding the flags. It exposed a very obvious problem, which I haven't noticed. I do not understand why this is happening in the pipeline, since it not happening locally. Needs examination.

I also noticed that when the build in the pipeline uses shallow copies of the repository and the version strings got all messy. So now I added #443 , so hopefully that solves the problem.

This is another dependency issue, yes? authors.c is a generated file and the build has a parallelism level of sixteen. If you want to reproduce it on your computer, you must remember to use an argument like -j 16.

I sent a PR with a similar fix perhaps for a different target.

@spe-ciellt
Copy link
Copy Markdown
Contributor

Which PR?

@eyal0
Copy link
Copy Markdown
Collaborator

eyal0 commented Mar 17, 2026

I think #365

It forces authors.c to be made before the .pot files. I think that is the error in the ci build.

@eyal0
Copy link
Copy Markdown
Collaborator

eyal0 commented Mar 17, 2026

https://github.com/gerbv/gerbv/actions/runs/23174090248/job/67332350422?pr=438

Is that job failing to make the .pot file because authors.c is missing? I can see that in the environment variable, the parallelism is set to 16.

@spe-ciellt
Copy link
Copy Markdown
Contributor

The fix is easy hopefully. Swap the lines according to #444 . @rampageservices if you add the fix from #444 to this PR, and the compilation passes, I will happily merge it . I will kill of the #444, just keeping it for reference until the fix is added.

@spe-ciellt
Copy link
Copy Markdown
Contributor

OK, I couldn't rebase as I thought. I will take a gamble to just merge this branch. I have tested it locally in all sorts of ways, so hopefully I will not break anything.

@spe-ciellt spe-ciellt merged commit cc3fe52 into gerbv:develop Mar 18, 2026
1 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The execute_process should add COMMAND_ERROR_IS_FATAL ANY

3 participants