gh-obsproject/obs-browser

Watch

Fork 0

cmake: Fix CEF minimum version #458

Merged

RytoEX merged 1 commit from fix-cef-minimum-version into master

2024-10-14 06:01:58 -05:00

RytoEX commented

2024-10-11 20:47:04 -05:00

(Migrated from github.com)

Description

Apparently, the find_package signature is incorrect. The [version] component must come before the [REQUIRED] component. I mistakenly placed them in the reverse order when originally adding the minimum version.

https://cmake.org/cmake/help/v3.28/command/find_package.html#basic-signature

Motivation and Context

Noticed this while reviewing another PR. Want to fix my past mistakes.

How Has This Been Tested?

Tested locally on Windows 11.
Before:

-- Found CEF: C:/dev/obsproject/obs-studio/.deps/cef_binary_6533_windows_x64/Release/libcef.dll (found version "127.3.4")

After:

-- Found CEF: C:/dev/obsproject/obs-studio/.deps/cef_binary_6533_windows_x64/Release/libcef.dll (found suitable version "127.3.4", minimum required is "95")

Types of changes

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

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.

### Description   Apparently, the find_package signature is incorrect. The [version] component must come before the [REQUIRED] component. I mistakenly placed them in the reverse order when originally adding the minimum version. https://cmake.org/cmake/help/v3.28/command/find_package.html#basic-signature ### Motivation and Context    Noticed this while reviewing another PR. Want to fix my past mistakes. ### How Has This Been Tested?    Tested locally on Windows 11. Before: ``` -- Found CEF: C:/dev/obsproject/obs-studio/.deps/cef_binary_6533_windows_x64/Release/libcef.dll (found version "127.3.4") ``` After: ``` -- Found CEF: C:/dev/obsproject/obs-studio/.deps/cef_binary_6533_windows_x64/Release/libcef.dll (found suitable version "127.3.4", minimum required is "95") ``` ### Types of changes  - Bug fix (non-breaking change which fixes an issue)    - Code cleanup (non-breaking change which makes code smaller or more readable)   ### Checklist:   - [x] My code has been run through [clang-format](https://github.com/obsproject/obs-studio/blob/master/.clang-format). - [x] I have read the [**contributing** document](https://github.com/obsproject/obs-studio/blob/master/CONTRIBUTING.rst). - [x] My code is not on the master branch. - [x] The code has been tested. - [x] All commit messages are properly formatted and commits squashed where appropriate. - [x] I have included updates to all appropriate documentation.

WizardCM (Migrated from github.com) approved these changes

2024-10-11 20:51:09 -05:00

PatTheMav (Migrated from github.com) reviewed

2024-10-12 09:25:06 -05:00

PatTheMav (Migrated from github.com) left a comment

Does the module even compile with a Chromium 95 build?

Because this instruction says it does, but if that wasn't the case then it's not just the order of arguments that is wrong.

Does the module even compile with a Chromium 95 build? Because this instruction says it does, but if that wasn't the case then it's not just the order of arguments that is wrong.

RytoEX commented

2024-10-12 11:34:19 -05:00

(Migrated from github.com)

I have no reason to believe it wouldn't build, since we still have the ifdefs in place. I haven't tried yet though.

PatTheMav commented

2024-10-12 12:44:43 -05:00

(Migrated from github.com)

I have no reason to believe it wouldn't build, since we still have the ifdefs in place. I haven't tried yet though.

I wouldn't have expected it to either, but if we use a version guard, it should also use the correct version and not allow a solution/project be generated when a severely outdated CEF version is provided.

> I have no reason to believe it wouldn't build, since we still have the ifdefs in place. I haven't tried yet though. I wouldn't have expected it to either, but if we use a version guard, it should also use the correct version and not allow a solution/project be generated when a severely outdated CEF version is provided.

RytoEX commented

2024-10-12 15:43:37 -05:00

(Migrated from github.com)

I have no reason to believe it wouldn't build, since we still have the ifdefs in place. I haven't tried yet though.

I wouldn't have expected it to either, but if we use a version guard, it should also use the correct version and not allow a solution/project be generated when a severely outdated CEF version is provided.

I used a double negative. To clarify, I would expect it to build, because we still have ifdefs in place down to that version.

> > I have no reason to believe it wouldn't build, since we still have the ifdefs in place. I haven't tried yet though. > > I wouldn't have expected it to either, but if we use a version guard, it should also use the correct version and not allow a solution/project be generated when a severely outdated CEF version is provided. I used a double negative. To clarify, I would expect it to build, because we still have ifdefs in place down to that version.

WizardCM commented

2024-10-14 06:01:49 -05:00

(Migrated from github.com)

Yes, currently OBS is supposed to still build with Chromium 95 if the user chooses to provide the necessary CEF build.

If we successfully release with Chromium 127, the intent is to drop Chromium 95 and below in favour of 103 & 127.

I have verified current master does build with 95.

Yes, currently OBS is supposed to still build with Chromium 95 if the user chooses to provide the necessary CEF build. If we successfully release with Chromium 127, the intent is to drop Chromium 95 and below in favour of 103 & 127. I have verified current master does build with 95.

No reviewers

WizardCM

No milestone

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

gh-obsproject/obs-browser!458

No description provided.

Rows
Columns