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.

Sign up for GitHub

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

Jump to bottom

CMake: update required enet version #12343

Merged
merged 1 commit into from Dec 9, 2023
Merged

CMake: update required enet version #12343

merged 1 commit into from Dec 9, 2023

Conversation

Tilka
Copy link
Member

@Tilka Tilka commented Dec 1, 2023

Dolphin now relies on ENET_SOCKOPT_TTL which was merged upstream but not released yet. This fix assumes that enet continues its current versioning scheme.

@Tilka
CMake: update required enet version
6a0b17f 
Dolphin now relies on ENET_SOCKOPT_TTL which was merged upstream but not
released yet. This fix assumes that enet continues its current
versioning scheme.
@xiota
Copy link

xiota commented Dec 2, 2023 •
edited

  • This PR would prevent building dolphin with system enet, even if enet is built from git checkout, because it still identifies itself as 1.3.17. See enet/configure.ac.
  • There hasn't been an enet release in over 3 years.
  • Why isn't enet listed in .gitmodules?

@Tilka
Copy link
Member Author

Tilka commented Dec 2, 2023

Why isn't enet listed in .gitmodules?

Because not all of Dolphin's dependencies are submodules yet. Would you like to convert it? :)

@xiota
Copy link

xiota commented Dec 2, 2023

I've opened an issue to request a new enet release: lsalzman/enet#232

Because not all of Dolphin's dependencies are submodules yet. Would you like to convert it? :)

I don't know how. Will do some research.

@V0rt3x667 V0rt3x667 mentioned this pull request Dec 7, 2023
Closed
@AdmiralCurtiss
Copy link
Contributor

IMO this is better than the current situation where it can never succeed building if a release build of enet gets picked instead of the included Externals. But yeah we should probably convert it to a submodule as well.

@AdmiralCurtiss AdmiralCurtiss merged commit 78d22d8 into dolphin-emu:master Dec 9, 2023
11 checks passed
@Tilka Tilka deleted the cmake branch December 9, 2023 17:35
@AdmiralCurtiss
Copy link
Contributor

Submodule conversion is over at #12368

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@OatmealDome OatmealDome OatmealDome approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
4 participants
@Tilka @xiota @AdmiralCurtiss @OatmealDome