Closed RytoEX closed 2 days ago
PatTheMav
commented
6 days ago I'm not sure if the "proper" fix is to remove the tarball entirely - given that only one user has reported that defect (and didn't even notice the entire extent of it), it seems like we already spent too much maintenance effort on a very niche requirement.
tgurr
commented
4 days ago I'm not sure if the "proper" fix is to remove the tarball entirely - given that only one user has reported that defect (and didn't even notice the entire extent of it), it seems like we already spent too much maintenance effort on a very niche requirement.
@PatTheMav The tarball including the whole source is required as it includes submodules (like obs-browser/obs-websocket) which would be missing otherwise. The reason why you're not seeing others complaining is probably because no proper distribution actually bothers with trying to package beta versions, you'll likely see these pop up when a proper release has been done with the things being broken. As I/we were hit by issues previously I got cautious in regards to obs and try to temporarily package beta versions as well so we can try to catch these issues early e.g. before a proper release has been done.
RytoEX
commented
4 days ago Pushed a change that should no longer exclude /buildspec.json or the other files. Do note that it will still exclude /build-aux/, but the current regex was already doing that.
This should exclude:
./build/
./build_x64/
./build_x86/
./build_x86_64/
./build_arm64/
./build_someotherarch/This should include:
./buildspec.json
./cmake/common/buildnumber.cmake
./cmake/common/buildspec_common.cmakeAfter reviewing .github/scripts/.package.zsh, it seems like the only currently valid build directory on CI for the purpose of generating a source package via CPack is build_x86_64.
https://github.com/obsproject/obs-studio/blob/7979421cbf9ba2ece81ef1269b76c2ba022ab712/.github/scripts/.package.zsh#L51-L55
https://github.com/obsproject/obs-studio/blob/7979421cbf9ba2ece81ef1269b76c2ba022ab712/.github/scripts/.package.zsh#L239-L240
We could just do this instead:
set(CPACK_SOURCE_IGNORE_FILES "/.git" "/build_x86_64" "/.ccache" "/.deps")Perhaps it's not as elegant, but it would resolve the issue.
PatTheMav
commented
2 days ago After reviewing
.github/scripts/.package.zsh, it seems like the only currently valid build directory on CI for the purpose of generating a source package via CPack isbuild_x86_64.We could just do this instead:
set(CPACK_SOURCE_IGNORE_FILES "/.git" "/build_x86_64" "/.ccache" "/.deps")Perhaps it's not as elegant, but it would resolve the issue.
The _x64_64 token is dynamic, however you could probably use CMAKE_SYSTEM_PROCESSOR to match it, so /build_${CMAKE_SYSTEM_PROCESSOR} should give you the exact same name dynamically (and thus matches the code/behaviour used by the script to define the build directory).
RytoEX
commented
2 days ago The
_x64_64token is dynamic
In that specific part of the script, it isn't dynamic per se. The .package.zsh script bails out if the target is not in _valid_targets, so the part of the script that creates the sources package on the Ubuntu host can only have x86_64 as the arch at this time. This part of the script only runs on Ubuntu and not on macOS.
That said, /build_${CMAKE_SYSTEM_PROCESSOR} is probably fine, since in this case on Ubuntu it should return x86_64.
Description
The regex was incorrectly excluding any file with build in the name. The intent was to exclude any build directories, so we should be able to restrict this.
Motivation and Context
Fixes #11443.
How Has This Been Tested?
It hasn't.
Types of changes
Checklist: