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

Add Libqrcodegencpp on Linux and add its finder #8943

Merged
merged 2 commits into from
Jul 14, 2023

Conversation

tytan652
Copy link
Collaborator

@tytan652 tytan652 commented May 21, 2023

Description

Related to:

obs-websockets PR requires:

  • Drop Ubuntu 20.04, related CI will failed because of an API break from Libqrcodegen between 1.5.0 (20.04) and 1.7.0 (22.04) (header file name change) with obs-websocket PRs.

Dependency of:

Add Libqrcodegencpp to Ubuntu script and Flatpak and its finder since not all distribution of the library comes with a CMake package.

Please, review the finder on the cmake-finders repo PR.

Motivation and Context

Reduce to zero the number of submodule in obs-websocket repo.

How Has This Been Tested?

Generated a QR code with obs-websocket with obs-websocket build with the library and not the submodule on Linux.

Test binaries available on a PR on my fork: tytan652#13

Types of changes

  • CI and CMake additions

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
Copy link
Member

RytoEX commented Jul 11, 2023

This has a merge conflict. Can this be merged independently of obsproject/obs-deps#182?

@tytan652
Copy link
Collaborator Author

This has a merge conflict.

Fixed

Can this be merged independently of obsproject/obs-deps#182?

Yes, only the obs-websocket PR does not.

@tytan652 tytan652 force-pushed the last_ws_dep_standing_pr branch 2 times, most recently from d224f7a to 8753601 Compare July 12, 2023 05:45
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.

Looks okay at a glance.

@GeorgesStavracas Flatpak parts look okay?
@PatTheMav CMake finders look okay?

Copy link
Member

@GeorgesStavracas GeorgesStavracas left a comment

Choose a reason for hiding this comment

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

Flatpak change is fine 👍

@PatTheMav
Copy link
Member

Did I understand it right that we need this finder despite manually adding a CMake project for the library at obs-deps? Does the separate CMake project repo add everything but proper CMake project generation?

@tytan652
Copy link
Collaborator Author

tytan652 commented Jul 13, 2023

Did I understand it right that we need this finder despite manually adding a CMake project for the library at obs-deps? Does the separate CMake project repo add everything but proper CMake project generation?

I quote myself from the PR description:

…and its finder since not all distribution of the library comes with a CMake package

Edit: So blame Ubuntu and/or Debian.

Copy link
Member

@PatTheMav PatTheMav left a comment

Choose a reason for hiding this comment

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

Should be fine even with Windows and macOS variants shipped as CMake packages (local FindModules take precedence). Once Linux is updated to the CMake path, the finder probably needs to be moved to the Linux-specific subdirectory (just like the Windows-specific Python finder) as it will not be needed by the other platforms.

@RytoEX
Copy link
Member

RytoEX commented Jul 14, 2023

It seems that #8327 introduced a minor merge conflict.

@tytan652
Copy link
Collaborator Author

Conflict fixed.

@RytoEX RytoEX merged commit ad243f2 into obsproject:master Jul 14, 2023
9 checks passed
@RytoEX RytoEX added this to the OBS Studio (Next Release) milestone Jul 14, 2023
@tytan652 tytan652 deleted the last_ws_dep_standing_pr branch August 6, 2023 00:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
New Feature New feature or plugin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants