Switch to UPX instead of strip

[AranVink]

This creates smaller binaries, and seems to be a better option than strip for DJGPP binaries.
See https://www.delorie.com/djgpp/v2faq/faq8_14.html end of the page. Seems a special DJP executable compressor was developed at some point, but development has since shifted to UPX.

[Malvineous]

Since UPX isn't a standard binary, I wonder whether you could tweak it so that if the UPX command fails, the Makefile continues to run anyway? Should be a trivial change and would mean installing UPX is optional.

[binarymaster]

Question: Wouldn't this make debugging it more complicated?

[Malvineous]

strip already removes all the debugging symbols so it's very hard to debug after that anyway. Normally when you're debugging you are running a local build, rather than the make binarydist optimised output which runs strip/upx/etc.

[mywave82]

Does UPX strip the binary, if not strip should be ran before applying upx self decompression.

(sorry for being slow lately, got a baby at home)

[AranVink]

Question: Wouldn't this make debugging it more complicated?

As far as I could tell from the code, Adplay for DOS already has it's own DEBUG flag, which makes log commands output to files, since outputting to console would mess up the main interface. This should just compress the binary and not alter its inner workings.
I agree that adding a live debugger might be more complicated, but you can then simply build in a different way, since this is just for the binarydist make target.

Does UPX strip the binary, if not strip should be ran before applying upx self decompression.

(sorry for being slow lately, got a baby at home)

Good point, I'll do a test to verify if it makes any difference in binary size. (And congratulations on the baby 👶)

[AranVink]

Did some testing/research, here are the results:

no compression: 2318433 bytes
strip:          1228288 bytes
upx:             484776 bytes
upx + strip:     484660 bytes

So it does not make much difference overall if strip is used before UPX.

UPX manpage actually recommends against using strip, and also sheds some light on why the difference is so small:

   NOTES FOR DJGPP2/COFF
       First of all, it is recommended to use UPX *instead* of strip. strip has the very bad
       habit of replacing your stub with its own (outdated) version.  Additionally UPX corrects a
       bug/feature in strip v2.8.x: it will fix the 4 KiB alignment of the stub.

       UPX includes the full functionality of stubify. This means it will automatically stubify
       your COFF files. Use the option --coff to disable this functionality (see below).

Source

[AranVink]

@Malvineous @mywave82
Please let me know if you require any additional information/research before merging.

[Malvineous]

Hoping you can address the first reply (above) to avoid a hard dependency on UPX, then I'm happy to merge

[AranVink]

Updated ✅
I've used bash/which command in the Makefile to detect the presence of strip/upx. Typically I would use configure/autotools to do these things, but since it's a smaller project, and has just one compiler/target arch I didn't bother with it.

Sidenote:
I'm thinking of pushing for a new release, which also requires a new release of Adplug, and some regression/integration testing in both Adplay and Adplug.
If you have any other suggestions/code smells you want me to look at in Adplay for DOS let me know, happy to help where I can 😄

[mywave82]

Question: Wouldn't this make debugging it more complicated?

Makefile already has some minor logic for DEBUG variable, which could be extended to prohibit stripping/compressing the binary if it is set.