Release problems

[kspalaiologos]

CC @alerque

The first problem with releases is that Windows binaries require a pthread DLL, which we don't supply, and the binaries supposedly don't run on Windows 7 and XP. I might look into it in a while myself. Also the libsais maintainer refuses to fix the UB, so I will have to do it.

The second problem is that we don't test bzip3 on other architectures, which could theoretically be resolved using https://github.com/uraimo/run-on-arch-action.

The third problem that I'm not sure how to fix (hence asking you) is the bug that my Gentoo maintainer friend noticed that is pretty much the same as https://bugs.gentoo.org/843560 - something related to libtool making the packagers having to rerun eautoreconf?

[alerque]

1. I'm way out of my depth on this one. Perhaps it would be best to build the Windows version --without-pthread? Or is the problem that we need to force static linking it for those targets?

2. Yes, that is something we should probably test. No I don't think that particular action will work since all the supported platforms are *nix. I think we'd need to run something on the Windows platform natively, or perhaps in a vbox VM like we are doing for FreeBSD at the moment.

3. I saw the Gentoo build just now (I was trying to figure out if it used clang or not). The issue with needing to rerun autoreconf is because they used the wrong sources. They used the git archive generated download not the source package we posted. Those archives do not have the full autoconf sources bundled. Some projects sidestep this by checking in a vendored copy of all the autogenerated autoconf stuff, but that isn't the way it was meant to work and I think that's messy. The solution is to download /releases/download/<tag>/bzip3-<tag>.tar.gz instead of /archive/<tag>.tar.gz. That way they can skip everything they did in src_prepare() including bothe the eautoreconf and needing to stuff the version in.

[kspalaiologos]

Oh, got it. I'll tweak 1) and I started working on 2) on the workflows-arch branch. I'm going to contact the Gentoo maintainer if necessary.

Regarding the Windows thing, I think that we could just bundle the required DLL with the distribution and I could figure out why it doesn't work on WinXP... The DLL in question is libwinpthread-1.dll

[alerque]

You might ask about the clang thing while you are at it, I don't know what the default build environment is but I noticed this bit the Alpine packager too (and even myself packaging for Arch Linux). Distro packaging environments are frequently carefully controlled chroots and it doesn't do any good for our configure script to prefer clang if the dev environment doesn't provide it by default.

[alerque]

If we need to bundle the DLL I suppose the binary artifacts we attach should probably be a zip file with the dll and exe (and whatever else Windows users need). I can help make that happen but it looks like that DLL file would be coming from a Winodws host system/dev environment not something our build generates, is that right?

[kspalaiologos]

Yeah, I could extract it from my MinGW installation and we could just bundle it alongside the .exes?

[kspalaiologos]

https://palaiologos.rocks/doc/libpthread.tar.bz3 should do it

[kspalaiologos]

Speaking of 1), the containers fail with the following:

  config.status: creating Makefile
  config.status: executing depfiles commands
  config.status: error: in `/bzip3':
  config.status: error: Something went wrong bootstrapping makefile fragments
      for automatic dependency tracking.  Try re-running configure with the
      '--disable-dependency-tracking' option to at least be able to build
      the package (albeit without support for automatic dependency tracking).
  See `config.log' for more details

[alerque]

I think the config error you posted is actually about the warning at the very top and that I commented on here

The requested image's platform (linux/arm/v6) does not match the detected host platform (linux/amd64) and no specific platform was requested

There needs to be some target specified at configure time if you aren't targeting the builder itself, so ./configure --host <something>. I'm not sure what the target triplet or whatever you need is, but you need to pass some value out of that matrix I think.

Note that the flag is confusing because --host is actually the target platform not the builder:

$ /configure --help | grep host
  --host=HOST       cross-compile to build programs to run on HOST [BUILD]

[kspalaiologos]

But we're not crosscompiling, right? The warning actually comes from Docker before any of our code is ran. Why would the --host value matter?

[alerque]

You kind of are — just because you are in a Docker container with a bunch of other platform header files and emulation going on doesn't mean autoconf isn't going to see that there is a miss match under the hood. Docker isn't a full VM (like the VirturalBox image running our FreeBSD build is) it is a lightweight shim between the host...and evidence of that does bleed through. I think you need to treat builds in Docker containers as cross-compiles in that sense even if the "host system" default header files and such are a batch for the target.

[kspalaiologos]

I figured out the problem already and it was missing make.

[kspalaiologos]

@alerque make dist doesn't include etc/shakespeare.txt which means that make roundtrip fails.

[alerque]

Yes, I believe commented on that in an earlier PR ;-) We could include it in Makefile.am with:

EXTRA_DIST += etc/shakespeare.txt

BUT I would suggest a different test though so that doesn't need to be distributed. You can just use one of the source code files instead as a rountrip test, the shakespeare test is more of a benchmark than a self check anyway.

[kspalaiologos]

I decided to use configure.

[alerque]

Also note at one point I had check: rountrip to automatically trigger the round trip test, but that broke make check for cross-compiles since the host couldn't run them. The roundtrip test actually executing the generated binaries should be a separate optional step not a required one.

[alerque]

I decided to use configure.

Allow me to suggest the README or something that is not otherwise part of the build. Running it on configure will bump the timestamp of the file which will trigger other things to be rebuilt the next time make is run.

If you really want source code try the build-aux/git-version-gen script. It is in the distdir and touching its timestamp is one of the few things that won't trip up autoconf into thinking more rebuilds are needed.

---

...or just don't extract it back to configure but to some other file location, then use cmp to see if it actually matches the original!

[kspalaiologos]

https://github.com/kspalaiologos/bzip3/runs/6434386258?check_suite_focus=true#step:4:1136 - something is also wrong with the FreeBSD build.

[alerque]

kspalaiologos/bzip3/runs/6434386258?check_suite_focus=true#step:4:1136 - something is also wrong with the FreeBSD build.

FreeBSD doesn't have time for you ;-)

time: ./: Permission denied

I suggest not mixing benchmarky things like time with the self check. Don't time the operation, just check the exit codes and make sure it worked.

[kspalaiologos]

That's not the problem. The binary name is not put in the correct place. I think that it used to work before...

[kspalaiologos]

https://github.com/kspalaiologos/bzip3/runs/6434451970?check_suite_focus=true#step:4:1175

[alerque]

Hmm, that might be a bmake handling difference here, it makes fewer passes setting variables along the way. Try this on for size (git am < x.patch):

From bc3b5a35c59311c43702383ef9e0cd9cf9d53228 Mon Sep 17 00:00:00 2001
From: Caleb Maclennan <caleb@alerque.com>
Date: Sat, 14 May 2022 15:10:51 +0300
Subject: [PATCH] Fixup rountrip test

Signed-off-by: Caleb Maclennan <caleb@alerque.com>
---
 .gitignore  | 1 +
 Makefile.am | 5 +++--
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/.gitignore b/.gitignore
index 12f364c..959beaa 100644
--- a/.gitignore
+++ b/.gitignore
@@ -16,6 +16,7 @@ tags
 bzip3
 bzip3-*
 .version
+LICENSE2

 # Autotools
 .deps/
diff --git a/Makefile.am b/Makefile.am
index 3cc70cb..9b96fdf 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -65,9 +65,10 @@ format: $(bzip3_SOURCES) $(include_HEADERS) $(noinst_HEADERS)
 cloc: $(bzip3_SOURCES) $(include_HEADERS) $(noinst_HEADERS)
    cloc $^

+CLEANFILES += LICENSE2
 .PHONY: roundtrip
-roundtrip: $(bin_PROGRAMS)
+roundtrip: bzip3$(EXEEXT)
+   rm -f LICENSE2
    ./$< -e -b 6 LICENSE
    ./$< -d LICENSE.bz3 LICENSE2
    cmp LICENSE LICENSE2
-   rm -rf LICENSE2
-- 
2.36.1

[alerque]

Still no. I'll work on that on my fork...

[kspalaiologos]

Still broken. I think it's the fault of git or nawk not being installed?

[alerque]

Nope, that warning is unrelated (and nawk is installed), but since it is also expected to return non-zero there I'll suppress that warning while I'm at it.

[alerque]

Gah! Apparently $< is a GNU make thing. Bah. At least I can test it locally... stand by.

[alerque]

I don't know what rabbit hole I just fell down. BSD documentation for make suggests that support for $< is "nearly universal". The autotools generated Makefile uses it internally even when using ./configure MAKE=bmake. And yet bmake locally for me fails –not for the autoconf generated rules but for mine– even on a MWE:

.PHONY: foo
foo: bar
    @echo input $<
    @echo output $@

bar:
    @touch $@

$ make
input bar
output foo

$ bmake
input
output foo

[ypsvlq]

On phone so not checking, but I would guess the issue is that $< only works in suffix rules (eg. .c.o:)

[alerque]

Thanks @ypsvlq a little testing suggests you are correct. I'll make accommodation. I already know how it just wasn't nice not to understand why.

[alerque]

@kspalaiologos I think forcing gmake as seen in bc19e13 was a mistake, we should be testing with BSD's native make and we should keep it fully portable so that every platform's default make works out of the box. Why even bother testing on BSD in we just force GNU tooling on it? That will not work out well when more obscure *nixes want to build that make not even have that as an option.

[kspalaiologos]

I presume that the last two problems are fixed, I'll look into libsais in a while. Not sure about the DLL thing.

[alerque]

On the Windows front, I have some unfinished work in the autotools-windows branch on my fork you might be interested in. Using gitbash.exe and clang on the windows-latest platform I got as far as a fully working ./configure and a partially working make (it builds one object successfully then bombs, maybe a linker config issue?). Building natively has the benefit of being able to test it more effectively of course. Not sure that's the way to go or if cross compiling is still better, but it was something I played with.

CI run: https://github.com/alerque/bzip3/runs/6435808722?check_suite_focus=true

[kspalaiologos]

I would just crosscompile, maybe there is a Github Actions CI runner for Windows just to test the result?

[alerque]

Using the Windows host for testing would be pretty easy. Cross compile, post binary artifacts, have another job dependent on the first, download artifacts, test a roundtrip with it.

Also we have a pretty big dichotomy brewing between our test build matrix & the release builds. I can integrate those at some point so the release process uses verified artifacts from the test build matrix, but it is definitely worth getting the tests all setup and working to satisfaction the way we want first and later mixing in the release stuff.

[kspalaiologos]

Using the Windows host for testing would be pretty easy. Cross compile, post binary artifacts, have another job dependent on the first, download artifacts, test a roundtrip with it.

Yeah, I thought of this.

Also we have a pretty big dichotomy brewing between our test build matrix & the release builds. I can integrate those at some point so the release process uses verified artifacts from the test build matrix, but it is definitely worth getting the tests all setup and working to satisfaction the way we want first and later mixing in the release stuff.

I noticed this myself too. Sounds like a good idea.

[synodriver]

I also encountered this problem on windows when I tried to wrap this into a python module. It said can not open "pthread.h": No such file or directory. It only occures on cffi, and I realized that I did not use cython's thread function while cffi wraps it by default. So, maybe the coming python wrapper on windows will not have thread support.

[kspalaiologos]

@synodriver If I remember correctly, cffi doesn't handle pthread well in general. It's rather easy to do threading yourself - parallel compression is just calling bz3_encode_block in a every worker thread.

Additionally, your wrapper seems to notoriously conflate bzip2 and bzip3. bzip3 doesn't have a well-defined the notion of levels (you'd have to pick them manually based on the CLI flags - and even then, a bigger buffer != better compression).

[synodriver]

That is because I want to provide the same interface as the builtin bz2 module, but just found some problem. Maybe some changes is necessary. By the way, it's still developing.

[kspalaiologos]

now discussed in #45.