Feature/update git bash string resource editor

[fourpastmidnight]

@dscho: Could you please look this over?

• Move Makefile and edit-git-bash.c to build-extra since both InnoSetup and MSI installers require this to edit git-bash.exe string resources.
• Ensure that edit-git-bash.{dll,exe} update string resources using the original git-bash.exe locale of LCID 1033 (0x0409, a/k/a en-US).

[fourpastmidnight]

It appears some commit messages could use a bit of "fine tuning" to be a bit more accurate.

[dscho]

Would you mind signing off on the patches?

I am not quite sure we would want to specify an output directory, but rather move the .dll as well, adjusting installer/install.iss and .gitignore for the new path.

Other than that, I really like this PR!

[fourpastmidnight]

Thanks! I'll make the suggested changes and sign off on everything.

[fourpastmidnight]

@dscho: Since you want me to change .gitignore, are you implying that edit-git-bash.{dll,exe} should be compiled as part of the release process when creating the installers?

[dscho]

Since you want me to change .gitignore, are you implying that edit-git-bash.{dll,exe} should be compiled as part of the release process when creating the installers?

I think we can keep the .dll precompiled (it must be 32-bit because InnoSetup itself is 32-bit and could not load a 64-bit .dll in-process if it wanted to). If you use the .exe in the MSI-based installer, I would like to compile it on the fly (via the Makefile, i.e. avoiding unnecessary recompiles).

But now that I come to think of it: it is really dirty to use a precompiled .dll. Since we already require the cross-compilation toolchain to build MINGW packages (both 32-bit and 64-bit packages are built in the 64-bit SDK if you use the please.sh helper), we could easily (cross-)compile the .dll. Or just go ahead and use the .exe in the InnoSetup-based installer, too...

What do you think?

[fourpastmidnight]

I think I agree that both the DLL and EXE should be compiled as part of the release process. When changes were made to edit-git-bash.c in the past, the DLL has forgotten to be recompiled (it's an easy enough thing to forget!). Having it compiled as part of the release process makes this much less likely.

I was having trouble making it cross-compile. The Makefile isn't set up for that (it doesn't work on my machine, anyway), and I'm even having trouble trying to do it manually. I didn't check the please.sh script; I'll take a look at that script. That may solve the problem.

I don't think it makes much of a difference whether we use the EXE or the DLL in the InnoSetup installer. If we use the EXE, could we retain the DontCopy flag and still be able to execute it? Something tells me no; it doesn't work that way in the MSI either. The EXE will need to be copied to the disk, executed, and optionally deleted, much like we do with post-install.bat. For MSI, I don't have much choice; for InnoSetup, if the EXE would work the way I think it would, I'd rather keep using the DLL.

[fourpastmidnight]

The please.sh script won't solve my problem of being able to cross-compile edit-git-bash.c as a 32-bit DLL for the InnoSetup installer from the 64-bit SDK. I must not have something setup correctly in my SDK environment (though, I see a lot of people online having trouble with MinGW and cross-compiling--but they're all on Linux boxes and I'm on a Windows box in a specialized MinGW environment for building Git, so I'm having trouble relating the solutions to my environment).

[dscho]

I was having trouble making it cross-compile. The Makefile isn't set up for tha

This seems to work for me:

diff --git a/installer/Makefile b/installer/Makefile
index 73737e1..89de752 100644
--- a/installer/Makefile
+++ b/installer/Makefile
@@ -4,7 +4,8 @@ DLLINKFLAGS=-Wl,--kill-at -static-libgcc -shared
 all: edit-git-bash.dll

 edit-git-bash.dll: edit-git-bash.c
-   gcc $(CFLAGS) $(DLLINKFLAGS) -o $@ $^
+   PATH=/mingw32/bin:$$PATH \
+   i686-w64-mingw32-gcc -march=i686 $(CFLAGS) $(DLLINKFLAGS) -o $@ $^

 edit-git-bash.exe: edit-git-bash.c
    gcc -DSTANDALONE_EXE $(CFLAGS) -o $@ $^

(I have not tested the .dll in an installer, though)

If we use the EXE, could we retain the DontCopy flag and still be able to execute it?

I think we should be able to use it; We use the .dll, after all, so why not the .exe (with an absolute path)?

[fourpastmidnight]

Thanks, I'll check it out.

...why not the .exe (with an absolute path)?

The DLL is never copied to disk, it's loaded into memory by the installer process and the method within is exported. To my knowledge, there's nothing preventing a EXE from exporting functions, so it might work.

I'll give it a try. The worst that could happen is it doesn't work.

---

Well, my knowledge was (mostly) wrong. I think I'm going to go with the path of least resistance: keep using edit-git-bash.dll in InnoSetup and use the EXE in the MSI.

[fourpastmidnight]

Thanks @dscho, it compiled. I should've known it was that simple. I'll do a few tests to make sure everything works ok.

[fourpastmidnight]

@dscho: Could I borrow your eyes once more? 😄

Just a note: I plan on making more changes to Makefile, but since that's MSI-related for the TERMINAL option, I'm going to do that on another branch I have for implementing that option in the MSI to keep the set of changes for that MSI option all together.

[dscho]

InnoSetup in only 32-bit "aware"

s/in/is/

[dscho]

Good work!

As I said in the comments, I'd prefer to hardcode the (cross-)compiler in the .dll rule, as it makes things obvious, and it works in the intended setup (which is really all I care about).

After that, I think this PR is ready to go!

[fourpastmidnight]

@dscho: OK, fixed that commit message typo and reverted to the last Makefile implementation with the hard-coded 32-bit gcc.

[dscho]

Excellent! Thank you so much!