Skip to content

obs-studio: patch to fix libobs.po on arm64#275221

Merged
fpletz merged 2 commits intoNixOS:masterfrom
hufman:patch-obs-arm64
Jan 23, 2024
Merged

obs-studio: patch to fix libobs.po on arm64#275221
fpletz merged 2 commits intoNixOS:masterfrom
hufman:patch-obs-arm64

Conversation

@hufman
Copy link
Contributor

@hufman hufman commented Dec 18, 2023

Description of changes

The current OBS Studio has a bug that generates an incorrect libobs pkgconfig file on ARM systems, preventing plugins from being compiled against it. This PR packages an Arch patch to hardcode the relevant option, another approach is to render the CMake expression before writing the pkgconfig file. OBS is instead pursuing a different approach for the next release, but this patch will help until that version is released and packaged.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@hufman
Copy link
Contributor Author

hufman commented Dec 18, 2023

This should fix #270050

@ofborg ofborg bot requested review from MP2E, deviant, jb55 and materusPL December 18, 2023 18:06
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. labels Dec 18, 2023
@fpletz
Copy link
Member

fpletz commented Jan 18, 2024

After reading the linked issues PR, this looks good. I'm wondering however why the second hunk from https://git.alpinelinux.org/aports/tree/community/obs-studio/broken-config.patch?id=a92887564dcc65e07b6be8a6224fda730259ae2b is not needed.

@fpletz
Copy link
Member

fpletz commented Jan 18, 2024

Rebased and fixed the merge conflict due to the OBS upgrade.

@ofborg ofborg bot requested review from eclairevoyant and fpletz January 18, 2024 15:34
Copy link
Contributor

@eclairevoyant eclairevoyant left a comment

Choose a reason for hiding this comment

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

Fetching the patch is preferred, any reason why that's not being done here?

@hufman
Copy link
Contributor Author

hufman commented Jan 18, 2024

Good morning!
The second hunk of the patch didn't seem to be needed in my setup, so to reduce possible future conflicts I opted to not include it.
Instead of fetching directly, I needed to adjust the patch slightly to work with the slightly different version included in Nixos (the line number changed by one, so the patch application warns about being slightly dirty). I also didn't want to add load to Alpine's hosting. It still worked with the warning, so I could easily adjust this PR to fetch it if desired.

@eclairevoyant
Copy link
Contributor

eclairevoyant commented Jan 18, 2024

Conflicts shouldn't be an issue if upstream's fixing it on the next release. As mentioned in https://github.com/NixOS/nixpkgs/blob/master/pkgs/README.md#patches:

Patches available online should be retrieved using fetchpatch.

I'd prefer to adhere to the guidelines here.

@hufman
Copy link
Contributor Author

hufman commented Jan 20, 2024

Turns out the Alpine patch does not apply cleanly after rebasing to the new 30.0.2 version (which no longer has a cmake/external directory that the second hunk targets). Using the current PR diff still works and OBS 30.0.2 successfully launches and offers the selected plugins.

@eclairevoyant
Copy link
Contributor

Then the second hunk can be excluded, fetchpatch takes other args

@hufman
Copy link
Contributor Author

hufman commented Jan 20, 2024

Neat, that fixed it, thanks!

@hufman
Copy link
Contributor Author

hufman commented Jan 21, 2024

Can you trigger an ofborg rebuild? It looks like the builder had a disk space issue, hopefully not caused by this PR!

@eclairevoyant
Copy link
Contributor

@ofborg eval

@ofborg ofborg bot requested a review from eclairevoyant January 21, 2024 15:49
@fpletz fpletz merged commit 7faa0f1 into NixOS:master Jan 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants