Improve windows build.

[jkbonfield]

Specifically we create the extra files needed for MSVC linkage, and document the MSYS2/MINGW setup process.

Also added a win-dist target which attempts to produce a directory structure suitable for binary distribution. This isn't executed by default, but is a good aide-memoire and to simplify testing compatibility with things like MSVC.

Ideally we'd have a similar mechanism for all platforms to permit easy creation of binary distributions (see #533).

[jmarshall]

And a general comment: Did you mean to affect the submodule?

[jmarshall]

Alphabetise. (These aren't used in implicit rules, so you don't really need to add them to .SUFFIXES anyway.)

[jmarshall]

Could list all three of .dll / .lib / .def if they are end products used by users.

[jkbonfield]

They are all end products, but I'm not sure what benefit there is to explicitly specifying everything in the dependency chain rather than the final item.

If for some inexplicable reason you decide you want hts-3.dll only and not the .def or .lib, you can still do make hts-3.dll and it'll build it. If you want to just let it build a shared libary then make lib-shared will do the right thing.

[jmarshall]

Documentation essentially — it's a bit confusing if lib-shared lists a .lib but not .dll.

[jmarshall]

dist-windows or dist-msys or so IMHO.

[jmarshall]

Hardcoded hts-3?

[jkbonfield]

Oops, yeah that's a mistake. Thanks (for this and other comments).

[jmarshall]

--out-implib in the DLL rule is intended to produce a suitable import lib. Is this just a filename issue, or are there other reasons why the existing hts.dll.a does not suffice? Either way, the rules should just produce one or the other, not both.

[jkbonfield]

I did notice the --out-implib option, but for whatever reason is simply doesn't work. I don't get the .def or .lib files being built. Maybe it's because it builds a msys2 specific import library instead?

I don't see the problem with building more than one target here. We're trying to produce a set of targets which can be used by as wide a set of compilers as possible. That means also building things that work for MSVC.

[daviesrob]

Is there a reason why you're using llvm-dlltool here instead of binutil's dlltool? Having to download the llvm toolchain adds quite a lot of installed package just to get a single binary...

A test build that gets the .lib file with dlltool can be seen here. Unfortunately I can't easily check if the resulting file works, but it should be easy enough for you to verify.

[jkbonfield]

I did try the official mingw solution to this, but I forget if that was dlltool or another program. Regardless, it didn't work with MSVC :(.

The suggestion to use llvm-dlltool came from various Stack overflow posts. I can test again to see what the problem was.

[jkbonfield]

I tried the dlltool that comes with mingw64 and it does appear to work, but produces rather different output. I don't know enough about this stuff to know whether it's significant or not.

Sizes are wildly different:

-rw-r--r-- 1 jkbon jkbon 403882 Jan 26 15:21 hts-3.b.lib
-rw-r--r-- 1 jkbon jkbon 84430 Jan 26 15:21 hts-3.lib

The .b.lib is the mingw64 dlltool version.

These are ar archives. The number of entries are very similar (554 vs 553), but each object is very different:

$ ar tv hts-3.b.lib|head
rw-rw-rw- 0/0    582 Jan 26 15:21 2022 dgnvt.o
rw-rw-rw- 0/0    640 Jan 26 15:21 2022 dgnvh.o
rw-rw-rw- 0/0    626 Jan 26 15:21 2022 dgnvs00550.o
rw-rw-rw- 0/0    608 Jan 26 15:21 2022 dgnvs00549.o
rw-rw-rw- 0/0    597 Jan 26 15:21 2022 dgnvs00548.o
rw-rw-rw- 0/0    608 Jan 26 15:21 2022 dgnvs00547.o
rw-rw-rw- 0/0    620 Jan 26 15:21 2022 dgnvs00546.o
rw-rw-rw- 0/0    620 Jan 26 15:21 2022 dgnvs00545.o
rw-rw-rw- 0/0    618 Jan 26 15:21 2022 dgnvs00544.o
rw-rw-rw- 0/0    614 Jan 26 15:21 2022 dgnvs00543.o

$ ar tv hts-3.lib|head
rw-r--r-- 0/0    364 Jan 01 00:00 1970 hts-3.dll
rw-r--r-- 0/0    127 Jan 01 00:00 1970 hts-3.dll
rw-r--r-- 0/0    161 Jan 01 00:00 1970 hts-3.dll
rw-r--r-- 0/0     40 Jan 01 00:00 1970 hts-3.dll
rw-r--r-- 0/0     40 Jan 01 00:00 1970 hts-3.dll
rw-r--r-- 0/0     40 Jan 01 00:00 1970 hts-3.dll
rw-r--r-- 0/0     40 Jan 01 00:00 1970 hts-3.dll
rw-r--r-- 0/0     41 Jan 01 00:00 1970 hts-3.dll
rw-r--r-- 0/0     41 Jan 01 00:00 1970 hts-3.dll
rw-r--r-- 0/0     43 Jan 01 00:00 1970 hts-3.dll

The difference in sizes for these objects within the archives aren't just debugging information as stripping them gets from 600ish to 350ish bytes, so still an order of magnitude larger. objdump isn't really explaining how they differ much.

The binaries I get linking against either .lib appears to work fine though and be largely the same size, so I'm assuming this is just an implementation thing and it doesn't actually matter. (Both have the same bug which is that hts_verbose symbol isn't accessable externally, which I assume is a decl_spec missing, but with that commented out I can get test_view to compile and run.)

I'll switch therefore. (Edit: according to CI timings it shaved off a whopping 4% of the build & test time. Lol well "every little helps".)

[jmarshall]

I wonder if this could be done by depending on install as a prerequisite and documenting conventions around using DESTDIR instead. Calling $(MAKE) is a bit unfortunate and suggests this would go better as a contrib/* script IMHO. Or if it stays as recursive make, the recipe line should have a + prefix.

[jkbonfield]

Valid point on the "+" prefix, but I don't see the issue with a recursive make. We already do this elsewhere infact (destdir).

Note I'm not actually expecting this target to be used much by users. As commented it's mainly as a tool to aid the maintainers in building binary distributions for testing against other compilers, as possibly as a precursor to making binary distributions.

Personally I don't like moving these sorts of things into external scripts either. It's not how the GNU tools work either. The general wisdom there is with "make dist" and "make distcheck" targets, done in-situ within the Makefile. Given the simplicity of this and the limited audience for now I think it's appropriate as is.

[jkbonfield]

Ah I think I see your meaning about DESTDIR. So something like:

dist-windows: DESTDIR=win-dist
dist-windows: install
        # stuff

It's possible a target-specific variable may work, but I'd need to read up on how portible it is. Hmm actually, maybe not... our only supported windows environment is MSYS/MINGW which is probably guaranteed to be GNU make.

(There is the caveat that if anyone attempted "make dist-windows install" then they only get the former being built and is different to "make dist-windows; make install", but that's a somewhat esoteric corner case.)

[jmarshall]

I was imagining making you type make DESTDIR=disttemp dist-windows rather than the target-specific variable, but that (target-specific variable) is probably the best approach even though they're a bit fragile.

Fair enough re in situ rather than a separate script. (That $(MAKE) in distdir is there because it's needed to cooperate with automake. Also Ed made me do it 😄)

[jkbonfield]

And a general comment: Did you mean to affect the submodule?

No. I've yet to get out of the habit of after a fresh develop pull and checkout -b to make a fresh PR, doing "commit -a" once I've made my changes. Sadly I occasionally get nonsensical htscodecs commits being added then too. (I don't understand it as my local win-build has the exact same htscodecs submodule revision as develop and status tells me it's fine.)

[jkbonfield]

I think more important though is addressing the binaries issues.

Linux and MacOSX users can just use BioConda. Windows however is by far the most challenging environment to build on, while simulatenously also being the hardest to find prebuilt binaries. Not that I'm specifically trying to promote it as a platform(!) it does probably imply a binary distribution would be useful.

It's perhaps a little messier given all the extra gubbins you need to bundle, which means copies of all their licenses etc. (I found it a bit hard to work out which of the myriad of dlls used by curl are needed and have a requirement to bundle a license, but I figured if I couldn't find one in the mingw directories then it's probably not a requirement.)

I can look at creating artifacts in appveyor if you wish.

Of course, being a set of command line tools only, it begs the question over how people would interact with samtools/bcftools? No one in their right mind is using a DOS command line these days, although maybe some use PowerShell? Most I'd bet just use WSL, msys, mingw, or similar. More interesting is the library though as that's almost certainly used within Windows GUI applications (indeed we know of some). Having a standard distribution would also make it more likely that when it gets bundled into other applications they replicate the necessary licenses and dependencies.

[whitwham]

make win-dist has no target.

[jkbonfield]

Oops I forgot to change that. I should perhaps not have this bit in there at all though (see below).

[whitwham]

make dist-windows brought up the following error with MINGW64.

cp: cannot stat '/mingw64/share/licenses/libdeflate': No such file or directory
make: *** [Makefile:349: dist-windows] Error 1

Do we require libdeflate for the Windows build?

[jkbonfield]

Require - no. However it's useful. The purpose really for dist-windows is for maintainers (eg us) producing decent windows binaries. My thought was we may want this to be part of a CI build artifact.

So I'm not really expecting end-users to have this as a requirement for building, but for us wanting to produce binaries (and Windows is really the only system where there aren't easy alternatives available to people), then it's easier done as a Makefile rule than a completely separate repository as we use right now for the distribution code.

I suspect we should, for now, just not document the existance of the dist-windows target.