Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CI: Minor update of build scripts and GitHub Actions workflows and actions #11282

Merged
merged 12 commits into from
Sep 18, 2024

Conversation

PatTheMav
Copy link
Member

Description

Updates the following aspects of the current build scripts and GitHub Actions workflows and actions:

  • Build scripts on Linux require Ubuntu 24.04 and x86_64 - any preliminary/experimental support for aarch64 was removed
  • Qt6 is used by Ubuntu builds by defaults, associated build spec settings are thus superfluous and have been removed
  • Build scripts on macOS and Linux require Zsh 5.9
  • macOS builds use Xcode 15.4 by default
  • Experimental support for CCache on Windows was removed
  • qt-xml-validator now uses the check-changes action to check for and get a list of changed UI files
  • Several complex shell script snippets have been documented in-line
  • XSLT stylesheets used by sparkle-appcast action have been updated with doc comments to explain how they work (XSLT and XPATH are remnants of a past age of Java, SOAP, and XHTML, so not many might be familiar with these)
  • sphinx-publish-action has been updated to use the precise commit hash of version 1.2.0
  • wrangler-action has been updated to commit hash of version 3.7.0

Motivation and Context

Regular maintenance of CI workflows and build scripts.

How Has This Been Tested?

Will need some actual CI runs to test.

Types of changes

  • Code cleanup (non-breaking change which makes code smaller or more readable)

Checklist:

  • My code has been run through clang-format.
  • I have read the contributing document.
  • My code is not on the master branch.
  • The code has been tested.
  • All commit messages are properly formatted and commits squashed where appropriate.
  • I have included updates to all appropriate documentation.

Copy link
Member

@RytoEX RytoEX left a comment

Choose a reason for hiding this comment

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

The changes to pinned third-party actions look fine. The changes to Zsh and Ubuntu version requirements look fine. The documentation updates are largely fine. I don't know if anyone will have opinions about the other changes, but I'll let it sit at least overnight.

.github/actions/sparkle-appcast/action.yaml Outdated Show resolved Hide resolved
.github/workflows/scheduled.yaml Outdated Show resolved Hide resolved
.github/workflows/scheduled.yaml Outdated Show resolved Hide resolved
.github/workflows/scheduled.yaml Outdated Show resolved Hide resolved
.github/workflows/scheduled.yaml Outdated Show resolved Hide resolved
.github/actions/flatpak-builder-lint/action.yaml Outdated Show resolved Hide resolved
@WizardCM WizardCM added the Code Cleanup Non-breaking change which makes code smaller or more readable label Sep 14, 2024
@Vainock
Copy link
Contributor

Vainock commented Sep 16, 2024

If you are on it, could you take a look at why

if: fromJSON(steps.checks.outputs.hasChangedFiles)
causes this failure? https://github.com/obsproject/obs-studio/actions/runs/10875623224/job/30174384982

This line was changed by #11253.
Rather than not triggering the upload action, it now causes this issue.

@PatTheMav
Copy link
Member Author

If you are on it, could you take a look at why

if: fromJSON(steps.checks.outputs.hasChangedFiles)

causes this failure? https://github.com/obsproject/obs-studio/actions/runs/10875623224/job/30174384982
This line was changed by #11253. Rather than not triggering the upload action, it now causes this issue.

That's because the check never ran, so there is no steps.check and thus the expression probably yields null and it cannot be used with fromJSON.

In theory we could use if: steps.checks.outputs.hasChangedFiles && fromJSON(steps.checks.outputs.hasChangedFiles), if the runtime short-circuits the condition:

  • If the step hasn't run, then the value itself should be null, which is interpreted as false and the check short-circuits
  • If the step has run, the value itself will be non-null, so it's "truthy" and the fromJSON check converts only the string "true" into the actual boolean true.

Once upon a time there was the idea to enable use of Ccache for
Windows builds on CI as well, but to achieve this the project would
need to switch to clang-cl.exe as its compiler and use an additional
post-build step to generate PDBs from binaries.

Using a different compiler for continuous integration than for building
release builds defeats the purpose of CI however, so the idea was
dropped.
Also defaults to Qt6 and x86_64 builds, thus no toolchain file is
needed and no additional data in buildspec is needed either.
@RytoEX RytoEX merged commit aa10a7b into obsproject:master Sep 18, 2024
14 checks passed
@PatTheMav PatTheMav deleted the ci-cleanup branch September 18, 2024 20:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code Cleanup Non-breaking change which makes code smaller or more readable Seeking Testers Build artifacts on CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants