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 Windows Implementation Libraries as an external library #8427

Merged
merged 1 commit into from
Mar 15, 2020

Conversation

CookiePLMonster
Copy link
Contributor

Previously a part of #8316 but I would really like to use WIL in other PR's to get RAII wrappers around WinAPI resources.

Quoting the relevant part of this PR:

As a preparation, WIL has been added, since it's very useful for ensuring proper RAII-based destruction for several WinAPI types - in my case it was proper scoped calls to CoUninitialize and proper destruction of previously leaking (in some cases) PROPVARIANT.

WIL also includes proper unique_ptr-like wrappers around eg. module handles, file handles, registry handles, CoTaskMem memory... etc.

@@ -71,6 +71,11 @@ if(WIN32)

WASAPIStream.cpp
WASAPIStream.h
target_include_directories(audiocommon PRIVATE

Choose a reason for hiding this comment

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

Maybe this part should remain with audiocommon or be moved somewhere more generic?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know if we have a "more generic" place for this, so I just removed it from that PR and will make it part of WASAPI PR.

@CookiePLMonster
Copy link
Contributor Author

As per the discussion on IRC I'll modify cmake to make WIL a dependency library so it's easier to use than what it is now. Will do it around Saturday.

@CookiePLMonster
Copy link
Contributor Author

Yesterday I spent way more time than I'd like trying to fix cmake project on Windows and looks like it's just fundamentally broken - PCH seems to be set up incorrectly and I failed to fix it fully.

Because of this I am unable to test and come up with a reasonable way to integrate WIL there. Any ideas how to proceed?

Tagging @lioncash since they're usually involved in cmake discussions.

@CookiePLMonster
Copy link
Contributor Author

As per discussion on IRC earlier, I now added WIL as a regular directory and not a submodule.

I did notice one problem with that however - now when it's not a submodule, it's not obvious what exact revision it contains...

@CookiePLMonster CookiePLMonster changed the title Add Windows Implementation Libraries as a submodule Add Windows Implementation Libraries as an external library Oct 31, 2019
@leoetlino leoetlino added the RFC Request for comments label Nov 8, 2019
@shuffle2
Copy link
Contributor

shuffle2 commented Dec 15, 2019

I did notice one problem with that however - now when it's not a submodule, it's not obvious what exact revision it contains...

In the past dolphin would work around this by leaving a .txt file a level above, stating the version of the external resource, if changes were made, etc. Mainly to simplify process of updating the resource in the future.

@CookiePLMonster
Copy link
Contributor Author

Rebased on latest master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RFC Request for comments
Development

Successfully merging this pull request may close these issues.

5 participants