Fix packaging and tests across platforms

[LiberalArtist]

Not ready to merge. Finally ready to merge!

I am trying to clean up the packaging of libgit2 and get the test suite to run across platforms. This isn't done, but I thought I should open a pull request early to ask for input, especially if anyone is currently using this package successfully. (I have had some conversations about using this package and heard others on the mailing list, but I'm not sure how far anyone's gotten.)

The first issue seems to me to be packaging the native libraries, as @jbclements started to do with libgit2-x86_64-linux-natipkg. Until this point there has been no native package for Mac, and Windows DLLs for i386 and x86_64 are checked into this repository, rather than being packaged in platform-specific dependencies with the copy-foreign-libs and install-platform keys for raco setup in their info.rkt files. (This repository also does not include a license or source code for upstream libgit2, which seems like a GPL problem.)

So far, I have packaged the binary for x86_64 Mac in libgit2-x86_64-macosx. Doing this exposed some further complications. The currently-checked-in Windows DLLs are for v0.26.0. When I tried to build that version, the corresponding test suite failed. On further investigation, I saw that libgit2 is about to release version v0.28.0 (as in, they've written the changelog and updated the version on HEAD). [Update: libgit2 v0.28.0 has now been released.] They plan to release v1.0 around late March.

To the extent we need to pick a version to develop against, my inclination is to target v1.0 (v0.28.0 for now) in getting this package working. There are a few breaking changes since v0.26.0, but they seem to be fairly few and well-documented. More significantly, there seem to have been several security updates since v0.26.0. I am optimistic that there shouldn't be too many changes needed in this package to support v1.0, and it seems like it would give a nice, stable target going forward.

A few other things that I hope to address:

• The test suite still doesn't pass on Mac. At least one failure seems to have to with assuming Windows-style paths.
• Tests are also failing on the package build server.
• The documentation claims to build, but is actually broken as discussed in Fix link to docs in pkgs.racket-lang.org #3
• Updated Windows binaries should be packaged in the normal way.
• I'm not dead set on this, but I think it may be worth packaging the native library for GNU/Linux (not just natipkg), rather than relying on a system version, because it sounds like system versions are often not up-to-date. Julia appears to take this approach. (I guess they use libgit2 for their core package manager.)

[spdegabrielle]

@LiberalArtist This is great! Thank you so much.
Cc: @bbusching @jbclements

[jbclements]

I'm delighted that you're working on this. Let me know when you think it's ready.

[LiberalArtist]

Here's a bit of a progress report.

I think the main blocker to this PR leaving things in better shape than I found them is the state of the platform-specific native packages. I set up a repository at https://github.com/LiberalArtist/native-libgit2-pkgs to try to build packages consistently across platforms, and it is working now for Mac OS and x86_64 GNU/Linux. (In addition to the considerations I mentioned above, I decided I'd like to package the Linux .so because there are a number of ways to build libgit2, and it would be nice to be able to expect one built with a certain set of features—for example, while I haven't implemented this yet, it would be nice to take advantage of the iconv already distributed with Racket. Also, selfishly, I would personally need to build the .so for x86_64 GNU/Linux anyway, and I would rather manage it through the Racket package system.)

As I mentioned on the mailing list, I'm currently blocked on getting the Windows DLL to load: it builds, and I can open it from Python, but I can't open it from Racket. Hopefully this is something silly I'm doing wrong because I'm not a big Windows person, rather than a more serious issue. I'm going to try to borrow a Windows machine to do more debugging.

I haven't addressed i386 packages yet. These aren't a big priority for me personally, but they'd be nice to have, especially since this repo does currently have an i386 Windows DLL checked in. The complication is that it doesn't look like either Travis or AppVeyor have i386 VMs for either Windows or GNU/Linux. (AppVeyor looked like it did, but apparently its "x86" platform is just an alias for its "x64" platform, confusingly.) I believe it should be possible to cross-compile the i386 versions from the x86_64 VMs, but I haven't tried this yet, and I'm slightly hesitant about packaging the binaries without a better story for testing that they actually work, especially given my current struggles with x86_64 Windows. @spdegabrielle and @jbclements, I'm very open to your thoughts on this: AFAIK you are the only other two people who have been trying to use or work on this package. (Also, @jbclements, since I'm now trying to package the x86_64 GNU/Linux .so with the same build recipe as for Mac and Windows, I would propose to use the same package for the package build server as well, rather than your libgit2-x86_64-linux-natipkg: you'll see I've made that change in the latest commit to this PR, which means the Travis build now passes.)

Once this runs on Windows, I think it's at least arguable that this PR would be ready to merge, but there are some other things I'd like to do to this library. I'll leave it up to you whether any of these things should go in this PR or in (a) subsequent pull request(s):

• Probably what I'd be most inclined to bundle with this PR is adding a note to the docs on the status of this library. If you agree with what I wrote above, I would be inclined to say that we are going to try to track the API of libgit2 v1.0.0 (which, as I said above, should be out next month—they have in fact released v0.28.0 since I opened this PR), in which case this library should be regarded as not totally stable until we get there. It sounds like the upstream team's priority for 1.0 is finalizing a nice, consistent API that they intend to support for a long time, and that's certainly what I'd like to be developing my Racket-level code against. The next bullet point is another reason I'd prefer not to commit to remaining 100% backward-compatible with the Racket API as it exists right now. On a practical level, upstream libgit2's timeline for getting to 1.0 tracks nicely with the time-frame in which I'm hoping to start using this library in some of my tools for Digital Ricoeur, so I very much hope to get any potential breaking changes ironed out in about that time.
• I think this package takes the right approach in general in sticking pretty close to the C API for libgit2 and leaving it to other libraries to experiment with higher-level APIs. There are a few areas, though, where I'd like to do slightly more to protect Racketeers from C-level details. The most significant point, in my view, is that the Racket API currently requires direct use of malloc and various _free functions in some cases, mostly when the C API expects to be passed a pointer to a block of memory and to fill it in for you. I would rather have this library consistently take care of memory management via ffi/unsafe's allocator/deallocator, which it already does in many (most?) cases. There are also some more minor points that I've noticed, such as the Racket API requiring you to convert path arguments using path->string, which is incorrect in some edge cases: I'd rather change this library to use the _path ctype and handle the boilerplate regarding path->complete-path recommended in the docs. There may be other, similar points I haven't noticed yet.
• Obviously, I want to investigate what's going on with the parts of the test suite that are currently commented out, and probably expand the current test suite in general. I already know there are a few minor issues, e.g. it doesn't clean up after itself properly in the tmp directory. (It should be wrapped in a dynamic-wind.)
• I haven't really done much with the documentation yet. At a minimum, I know there's some housekeeping to do in terms of getting the for-label requires right, making sure that any material drawn from the upstream documentation is clearly attributed and that any differences from the C API are noted, etc. It would also be nice to add examples and such. I'm inclined to address this gradually as I work through the above (and learn more of the API myself).

[jbclements]

Many thanks for the progress report. I'm inclined to defer to your judgment on this package's decisions and progress; I think you may be the only active user at this point, so anything that makes it better for you is probably the right thing.

Actually, I'm curious about your use of CI instances to build Windows DLLs... the RSound DLLs are way out of date, and one of the obstacles for me is bringing my Windows machine back from the dead.

[guygastineau]

@LiberalArtist are you still working on this?

I think you have some really good thoughts about how this library should work. I brought up a cheeky Issue regarding naming conventions. To be clear, I don't mind the naming convention adhering strictly to the C API, but the few cases where the conventions are mixed concerns me. I would prefer this package remain the thinnest wrapper possible if that is its goal.

I am willing to put in quite a bit of work. I am inexperienced with the FFI facilities in Racket, but I am familiar with C and Racket, so I don't expect to get confused. I do like the idea of having some racket tools automate the memory management, but I don't mind wrapping everything manually if necessary.

If you are still interested in this project I would love to get your perspective now (more than a full year after your last comments that I can find).

Thanks

[jbclements]

As @guygastineau says, I'd also like to find out whether we can merge this PR. I would hate to see this work go to waste. Guy, how does this pull request relate to yours? Would accepting your pull request make it harder to accept this one? (It seems likely to me.)

[guygastineau]

@jbclements The two PRs I opened yesterday are minor, and even if they cause conflicts in this PR it is just tweaks in the .gitignore and the test suite. As for the further work I am interested in doing for this library: I don't have a clear vision of it yet, and I was hoping to reach out a bit in order to get feedback from folks who are more familiar with both this project and libgit2 (I am mostly just familiar with the command line tool git).

Even though I use Linux, my motivations are actually to use this package to automate git workflows for my team (who are all using enterprise Windows at a school). I can just do it in PowerShell, but I don't like PowerShell; I figured it was a good excuse to work on something with a lot of vertical integration. The team all has racket installed too even though they aren't very familiar with it.

Anyway, I suppose that is to say that having this package cleanly and easily install libgit2 on Windows with a simple raco pkg install libgit2 is VERY attractive to me (I am unsure whether it currently does that or not).

Thank you for jumping back into the conversation here, jbclements.

PS. My PR #8 that fixes the tests for me might break one test for Windows. If raco pkg install libgit2 will install libgit2 on Windows, then I can easily test it on windows too. Or if you know another way to install libgit2 on Windows... ? I basically just couldn't find it with chocolatey, so I gave up. If only Nix or Guix package managers worked on Windows 🤣

[LiberalArtist]

This is very much still on my radar, and I'm hoping one of my nascent projects for which I intend to use it will finally be ready this fall (a year and a half after I'd hoped …), so my attention had been turning back to this in the last few weeks anyway.

I'll try to look over this in more detail in the next few days, but IIRC the only blocker to this being strictly an improvement over the status quo is figuring out the proper incantation to build libgit2 for Windows in a way that works with Racket's ffi-lib: then we can put it in a proper platform-specific Racket package with raco pkg. @guygastineau, if you have any insight on this, that would be great! My familiarity with compiling/linking Windows software is very shallow: I linked further up this discussion to issues I was encountering. Otherwise, I have easier access to a Windows install now, so I can give it another try.

Some of the other changes I discussed above would be backwards-incompatible, but, if we can get the packaging set up properly, I think it would make sense to merge this and potentially address those in further PRs. It sounds like there's broad agreement on what we want from this package: a fairly low-level API but with correct ctypes and some Racket conveniences around memory safety etc. I still think it makes sense to consider this package unstable for now and come to consensus on a stable API consistent with upstream v1.0, which was released this spring (incidentally, also about a year after they'd intended).

[LiberalArtist]

It has taken years rather than weeks, but the commits I've just pushed fixed the final item on my "do no harm" checklist:

• Updated Windows binaries should be packaged in the normal way.

so I say this is ready to merge!

I've effectively cherry-picked @guygastineau's repairs from #6 and #8 (specifically, 65a16b3), as well.

I have gone through—um, maybe 4?—approaches to building the native libraries over at https://github.com/LiberalArtist/native-libgit2-pkgs before finding one that works reasonably, using a combination of Nix and Guix. The resulting packages support:

• x86_64-macosx and aarch64-macosx, with the oldest supported version determined by the SDK in Nixpkgs 21.11: see Status of macOS NixOS/nixpkgs#116341. (I'm a little unclear on if that's still 10.12 or something newer. I will try to test on an old machine eventually.)
• x86_64-win32 and i386-win32, probably a long way back in history. These currently use Windows-native SSL/TLS: I think it would be much better to figure out how to use the OpenSSL distributed with Racket, so that Racketeers can rely on using openssl to configure settings on all platforms.
• x86_64-linux for distributions that have at least Glibc 2.31, which is in Ubuntu 20.04 (Focal) and Debian 11 (Bullseye). Racket itself builds against Glibc 2.24, which corresponds to Debian 9 (Stretch): I plan to seek advice from the Guix community about how to do likewise.

As far as:

• I'm not dead set on this, but I think it may be worth packaging the native library for GNU/Linux (not just natipkg), rather than relying on a system version, because it sounds like system versions are often not up-to-date. Julia appears to take this approach. (I guess they use libgit2 for their core package manager.)

I'm still taking this approach, at least for now. I think I should look again at what other distributors are doing, now that libgit2 itself has stabilized a bit more, but, regardless, I intend to keep building libgit2 so that it can be installed by non-natipkg Racket. The question is just whether it should be by default.

There's still work to be done as far as:

It sounds like there's broad agreement on what we want from this package: a fairly low-level API but with correct ctypes and some Racket conveniences around memory safety etc. I still think it makes sense to consider this package unstable for now and come to consensus on a stable API consistent with upstream v1.0, which was released this spring (incidentally, also about a year after they'd intended).

The next thing I plan to address on that front is asking for some clarification from upstream on what language bindings should do about Unicode and what should be left to application programmers—in Racket terms, basically, where we should be using string?s vs. bytes?. In particular, libgit2/libgit2#4300 (comment) sounded to me like maybe we should be passing path?s on Windows through (bytes-open-converter "WTF-16" "WTF-8").

But none of that need delay this PR any longer!

[jbclements]

OH my goodness. Many thanks to @kengruven for bumping this PR. I will be honest with you, I have never been more than a caretaker on this project, which was written many years ago with a student of mine. I'm therefore going to merge this without anything that would really approximate a "review"; it does seem possible that I'll be a client of this package going forward, so perhaps I will have more of an opinion after I've actually tried it out. Many many thanks for all the work here!