obsproject / obs-plugintemplate

GNU General Public License v2.0
285 stars 133 forks source link

cmake: Fix potential configure error when setting up buildnumber #122

Closed WarmUpTill closed 4 months ago

WarmUpTill commented 4 months ago

Description

If the CI environment variable is defined but GITHUB_RUN_ID is not this will result in a broken if statement. This can apparently be the case for some Debian build pipelines.

This problem was originally reported here, while packaging obs-advanced-scene-switcher: https://github.com/WarmUpTill/SceneSwitcher/issues/1090

Motivation and Context

Fixes potential configure error.

How Has This Been Tested?

Tested by manually defining only the CI variable.

Types of changes

Checklist:

PatTheMav commented 4 months ago

If it breaks because GITHUB_RUN_ID is not set, then the code (as it's currently implemented) works exactly as it should. Wrapping it in double quotes (to make it empty strings) defeats the purpose.

I think it would be better to first only check for the CI environment variable and then add a check for GITHUB_RUN_ID or GITLAB_RUN_ID and then use whatever is available to generate a build ID.

Because the code right now makes the assumption that CI would only ever be GitHub Actions, but I don't think it necessarily needs to be limited to that in the CMake files.

WarmUpTill commented 4 months ago

Makes sense. I have updated the PR.

PatTheMav commented 4 months ago

Could you update the commit title to make it more explicit that this adds support for using GitLab run IDs as build numbers (because that's the precise scope of the change now)?

WarmUpTill commented 4 months ago

Done :)