CI: Ensure finding no pattern match results in an empty string

[RytoEX]

Description

If no pattern match is found, which we expect for Qt x86 builds, then return an empty string by enabling the nullglob option.

Additionally, add quotes for paths for safety and fix the for loop array syntax.

Motivation and Context

To appease CI.

How Has This Been Tested?

Test locally more thoroughly.

Types of changes

• Bug fix (non-breaking change which fixes an issue)

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.

[RytoEX]

As a separate, but related, matter, there is an additional question about what this pattern's intent is:

files=("${GITHUB_WORKSPACE}"/qt6-windows-"${arch}"-!(*-@(Debug|RelWithDebInfo|Release|MinSizeRel))/*.zip)

As it is, this cannot exclude any of the Qt6 artifacts, and thus will include all of them, because their name pattern is:

qt6-windows-${{ matrix.target }}-RelWithDebInfo-${{ needs.pre-checks.outputs.shortHash }}
qt6-windows-${{ matrix.target }}-Debug-${{ needs.pre-checks.outputs.shortHash }}
qt6-windows-${{ matrix.target }}-${{ needs.pre-checks.outputs.shortHash }}
qt6-windows-${{ matrix.target }}-PDBs-${{ needs.pre-checks.outputs.shortHash }}

or:

qt6-windows-x64-077abdb2f
qt6-windows-x64-Debug-077abdb2f
qt6-windows-x64-RelWithDebInfo-077abdb2f
qt6-windows-x64-PDBs-077abdb2f

Notably, this part of the pattern:

"${arch}"-!(*-@(Debug

Can never match/exclude any such paths, because there are never two - characters adjacent. So "match a string that starts qt6-windows-x64- and continues with anything except a pattern that starts with a -" will match all Qt6 artifacts.

If the intent was to only grab the combined artifact for a release and the PDBs, then it should be:

files=(qt6-windows-"${arch}"-!(Debug-*|RelWithDebInfo-*|Release-*|MinSizeRel-*)/*.zip)

If there's some other intent, I don't recall it. In any case, we've been shipping all four of the Qt artifacts (the one we actually build with, PDBs, Debug, and RelWithDebInfo) with releases since last June, so I suppose there's no urgent need to "fix" the pattern. We can reconsider its purpose some other time.

[PatTheMav]

The correct variant seems to be ${GITHUB_WORKSPACE}/qt6-windows-${arch}-!(@(Debug|RelWithDebInfo|Release|MinSizeRel)-*)/*.zip as the intention was to only match the PDBs and the combined artifact, so exluding the configuration specific files was the goal.

IIRC this might have been an oversight after the order of commit id and config was possibly switched. So we need to exclude any file that contains the config after the architecture and specifically has the commit hash added to the back of that.

[RytoEX]

The correct variant seems to be ${GITHUB_WORKSPACE}/qt6-windows-${arch}-!(@(Debug|RelWithDebInfo|Release|MinSizeRel)-*)/*.zip as the intention was to only match the PDBs and the combined artifact, so exluding the configuration specific files was the goal.

This pattern seems to achieve the same effect as the one I provided, but is a bit clearer in purpose.

IIRC this might have been an oversight after the order of commit id and config was possibly switched. So we need to exclude any file that contains the config after the architecture and specifically has the commit hash added to the back of that.

Correct, though the artifact filename pattern has been that way for over a year (since this glob pattern was introduced), judging by release assets.

[PatTheMav]

The correct variant seems to be ${GITHUB_WORKSPACE}/qt6-windows-${arch}-!(@(Debug|RelWithDebInfo|Release|MinSizeRel)-*)/*.zip as the intention was to only match the PDBs and the combined artifact, so exluding the configuration specific files was the goal.

This pattern seems to achieve the same effect as the one I provided, but is a bit clearer in purpose.

IIRC this might have been an oversight after the order of commit id and config was possibly switched. So we need to exclude any file that contains the config after the architecture and specifically has the commit hash added to the back of that.

Correct, though the artifact filename pattern has been that way for over a year (since this glob pattern was introduced), judging by release assets.

I guess that might be because shipping more files than necessary did not break any builds and just increased the download size.

[RytoEX]

I guess that might be because shipping more files than necessary did not break any builds and just increased the download size.

It didn't increase the download size for what the obs-studio CMake downloads, because they are separate assets. The obs-studio CMake only downloads windows-deps-qt6-2024-08-08-x64.zip. The separate windows-deps-qt6-2024-08-08-x64-PDBs.zip isn't downloaded for builds, it's only meant as a Release/RelWithDebInfo PDB archive (which should be used at some point). The other two release assets are also never downloaded by obs-studio CMake.

In any case, I'll deal with that separately, perhaps today or tomorrow.