Host changes to FNA and SDL as patch files
flibitijibibo opened this issue · 8 comments
As of writing, wine-mono has its own copies of SDL and FNA. While SDL is currently vanilla 2.0.9, FNA does have a couple unique changes just for XNA Win32 compatibility:
FNA-XNA/FNA@2f3a309
madewokherd/FNA@18f78a2
These changes tend to be very very small, but are also unique to the problem that wine-mono is trying to solve. At the same time, maintaining unique branches can be a fair amount of busy work, vs just using upstream and having patches that go on top, similar to wine-staging:
https://github.com/wine-staging/wine-staging
I'd like to throw around the idea of adding a patches folder that hosts the patches currently used as well as potential patches in the future (for example, I'm considering making a unique SDL patch that disables DirectInput, for XNA Win32 accuracy). It would make updates easier in exchange for adding a build step that applies the patches before compiling the final libraries.
The only questions in mind:
- How should the patches be generated?
git diff/hg diffor plaindiffoutput? - How do we prevent re-applying patches when building 2+ times? (Basically, how do we reset the repos when starting over?)
I can do the work of generating fresh patches, but I dunno if I'm the best person to mess with the build process...
I'm not convinced. Maintaining separate patches is the same amount of work, if not more, as doing merges.
The advantage of the system wine-staging uses is that the patches are all separated and documented. This is useful for upstreaming. It allows some patches to be easily disabled at build time.
Rebases may be easier when patches are upstreamed - stop applying those patches, and you get fewer conflicts from other changes in the same areas.
Our diff is growing, but I don't think it's close to reaching a point where that matters.
But I'm looking at this as an outsider. I'd invite @zfigura or @alesliehughes to comment.
In the case of disabling patches, one example I can think of is the SDL2_FNAPlatform Form patch, where the Form doesn't properly respect GraphicsDeviceManager requests and doesn't handle window style changes yet, and is only needed for some games. Being able to disable that for testing games that just need the HWND for the WndProc would be good to have, and would be good for testing a WIP Form patch to replace the current one that actually does the Form work properly, vs having branches in various places that have to be managed separately.
The SDL patch is what actually made me think about this in the first place - we would either need to be absolutely sure not to overwrite that one patch (and other patches?) for SDL updates, or move to a Git mirror (and then fork that mirror, so a mirror of a mirror) that we can use as a submodule, or we'd host the original Hg repo as-is in a submodule somehow... basically all of those paths are pretty messy.
We don't have any SDL patches at the moment, but I have a plan for patching it and doing merges if needed.
For completion's sake, here's the WIP patch in question:
# HG changeset patch
# User Ethan Lee <flibitijibibo@flibitijibibo.com>
# Date 1565375250 14400
# Fri Aug 09 14:27:30 2019 -0400
# Node ID 851b5a55eac8b5341643ebb080880852de7b04fe
# Parent 11955981a7062557081cfda48e6e9b78535d2369
wine-mono HACK: Disable dinput by force
diff -r 11955981a706 -r 851b5a55eac8 configure
--- a/configure Tue Aug 06 23:08:01 2019 -0400
+++ b/configure Fri Aug 09 14:27:30 2019 -0400
@@ -23583,6 +23583,10 @@
have_dinput=false
;;
esac
+
+ # wine-mono: XNA only used XInput, and games are known to reference
+ # DirectInput explicitly, so disable this to prevent device duplication
+ have_dinput=false
fi
ac_fn_c_check_header_mongrel "$LINENO" "mmdeviceapi.h" "ac_cv_header_mmdeviceapi_h" "$ac_includes_default"
diff -r 11955981a706 -r 851b5a55eac8 configure.ac
--- a/configure.ac Tue Aug 06 23:08:01 2019 -0400
+++ b/configure.ac Fri Aug 09 14:27:30 2019 -0400
@@ -3003,6 +3003,10 @@
have_dinput=false
;;
esac
+
+ # wine-mono: XNA only used XInput, and games are known to reference
+ # DirectInput explicitly, so disable this to prevent device duplication
+ have_dinput=false
fi
AC_CHECK_HEADER(mmdeviceapi.h, have_wasapi=yes)
I don't particularly want to claim that the system wine-staging uses is really efficient; in fact I think it can be quite annoying. I'm not sure that there's really a good way to improve it, mind; I might prefer to maintain it as a branch that sees regular rebases, but this makes reverting to earlier versions much harder. (I don't think using merges instead would work at all; I find them a lot messier to work with in general and we wouldn't be able to get a form of patch X that applies to master, which is pretty much a hard requirement as long as we're trying to encourage people to upstream things. For wine-mono this is probably different as long as mono has no interest in accepting our diffs upstream).
What I'd prefer to see, and I think would result in the least work for everyone, is if upstream FNA were to just keep the relevant changes in its master branch, but guarded by e.g. preprocessor directives or whatever else is appropriate (conditional logic in configure scripts, as in the above patch). Failing that, for as few patches as we have—and ideally our diff won't grow considerably—it probably makes the most sense to just keep them in a separate branch and do rebases (or merges, I guess).
For a debian point of view we prefer that you work with upstream in order to minimize distrib patching.
Rebased branch is nicer than merge
I'm not going to add a patching system to the build process.
For SDL: That patch can probably be more easily accomplished as a change to the default configure settings.
For FNA: Merges are less work for me, and there's no clear advantage to moving the diff into a patch file. Having the ability to make our customizations as a configuration change instead of patching would be preferable.
For Mono: It'd be better organized, but patching or rebasing is too much of a pain to compared to merging, for the minor advantage it would have.