-
Notifications
You must be signed in to change notification settings - Fork 48
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
Moved winetricks to submodule #125
Merged
+9
−20,404
Merged
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,6 +27,7 @@ exclude = [ | |
| "site-packages", | ||
| "venv", | ||
| "protonfixes_test.py", | ||
| "subprojects", | ||
| ] | ||
|
|
||
| # Same as Black. | ||
|
|
||
Submodule winetricks
added at
72b934
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have to check in about this -- whether we want to always use the latest winetricks. I think we do though. However, I think it would be better if we fork winetricks instead. That way, we wouldn't be fully dependent on them when things break, especially when they are relatively trivial to fix (e.g., digest mismatches).
Once we create our own fork, we'd also have to update the Makefile so it will be added to the
distdirectory.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Winetricks is moving pretty slow and the releases are getting quite stale. We want to use the latest, I think.
The submodules are actually fixed on a specific commit though, so we don't need to use the latest. There's really no difference from the current solution.
I'm working on a reimplementation of Winetricks in Python. I can't give an ETA at the moment, but I have some groundwork and I'm working on an importer that converts the verbs into a maintainable file format.
Winetricks is huge though... and slow. It has a lot of functionality, so it will take some time to implement all of it.
I hope to finish work on it, but don't hold your breath. I'm not sure if I'm motivated enough to finish this little side project.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can actually override verbs for small fixes by adding them to the local verbs. So we don't need to fork.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great. Yes, I have been deliberating about a winetricks rewrite for a bit and had kinda planned for this, actually. Like the rewrite would be tailored to Proton, only containing the most useful verbs and metadata of them would be separated from the main program to separate files (e.g., toml, json, etc.). And while I imagine a rewrite in Python would speed up its runtime execution, be simpler, and better designed, I believe only part of its execution would be sped up unless we do not create subprocesses to wine binaries like the original script or perhaps if the registration of DLLs is possible to do in a thread-safe manner (which I haven't really explored yet).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be modules that can be imported with a CLI that imports the modules as well. Since the verbs would be in individual files, it doesn't really matter if there are only the most useful verbs or all of them. I will implement the most used functions of the verbs first, which should port most of the verbs pretty fast.
This is an issue, but I think some verbs are overkill.. like installing a complete software suite for having a dll.. but to refactor and test those verbs, we would need way more manpower. Not realistic atm.. but registering dlls or changing the registry might be viable.