fix(clustalo/align): wait for compression and propagate pigz failures

[dnncha]

Problem

The compressed output expression uses clustalo ... -o >(pigz ... > result.aln.gz). Bash does not wait for that process substitution as part of the foreground command, and the compressor's status is not included in the Clustal Omega exit status, even with set -euo pipefail.

This can report task success before compression finishes, or after a compressor failure when the producer has already successfully written its buffered output. The .aln.gz file may exist but be incomplete or invalid.

Fix

Append && wait $! to the compressed-output command, with the dollar sign escaped in the Groovy string. The producer's failure is preserved by &&; after producer success, Bash waits for the compressor and returns its failure status. This keeps streaming, avoids an intermediate uncompressed file, and preserves the separation of alignment data from verbose stdout. The uncompressed branch and existing nf-test snapshots are unchanged.

Reproduction and tests

bash -euo pipefail -c 'printf alignment > >(cat >/dev/null; exit 42)'
# exit 0 before
bash -euo pipefail -c 'printf alignment > >(cat >/dev/null; exit 42) && wait $!'
# exit 42 after

Added a dependency-free regression test that extracts the actual module output expression and runs it in Bash with stand-in executables:

python modules/nf-core/clustalo/align/tests/test_compression.py -v

Five cases cover delayed compression completion, compressor failure after draining all input (no SIGPIPE assumption), producer failure, uncompressed output, and separation of stdout progress messages from the compressed alignment. Executed against the pinned original and patched source: 2 failures before; all 5 pass after.

These are shell lifecycle tests, not an alignment-engine or full Nextflow validation. Nextflow/nf-test and the native tools are unavailable in this execution environment; the existing native integration tests still need CI. The standalone regression is run with the command above and is not claimed to be automatically wired into nf-test.

Prepared with AI assistance. Based on upstream commit 781f2625386fe0bf2f337c7b99f56e618c59fc99.

[SPPearce]

This feels complicated.
Why does this tool not just make the output file and then gzip it?

[dnncha]

That would be simpler, and seems a reasonable approach. I kept the existing streaming setup to make the correction small and avoid writing an intermediate uncompressed alignment. The problem is that the foreground command can finish successfully while compression is still running, or after the compressor has failed.

Writing the alignment first and then running pigz would also address that, provided either failure makes the task fail. I’m happy to revise in that direction if that is your preference. I would retain the failure checks and verify the revised module through Nextflow/nf-test; the current five checks use shell stand-ins, so they do not establish native workflow integration.

[SPPearce]

@lrauschning , @erikrikarddaniel , do you have any input on this? I don't know why it is being done in this way.

[erikrikarddaniel]

@lrauschning , @erikrikarddaniel , do you have any input on this? I don't know why it is being done in this way.

No idea. I have no recollection of implementing this and have nothing against doing it the simple way, i.e. gzip after creation of the alignment file. That won't leave an intermediate which would be my only concern in general.

[SPPearce]

Ok, let's do that, make the file and then gzip it if required, as we generally do. Much less error prone.
Don't need that python test at that point.

[lrauschning]

Hi @SPPearce,

I added this to the clustalo (and if memory serves, also to other MSA modules that don't support writing to stdout), as uncompressed MSA output can get very large due to gaps -- essentially, they scale with O(n^2) for diverse sequences. Writing uncompressed output to disk was both using a lot of scratch space and becoming a performance bottlenecks in some tests I ran.
I can't comment on the pytest as I've never seen an nfcore module use anything like it, but the && wait $! solution seems alright to me.

Cheers,
Leon

[dnncha]

Thanks Leon, that explains the streaming choice. Given the scratch-space cost you’ve seen, @SPPearce, do you still prefer writing the alignment first, or keeping streaming with the explicit wait? I’m happy with either approach provided completion and compressor failures are handled. I’ll follow the module’s usual test setup.

[SPPearce]

Ok, if there is a significant reason not to write the intermediate file that is fine.
I don't think we need the python test here though (or even have a way to run it at the moment).
Can you please join the nf-core community via the github-invitations channel on the nf-core slack, to run the tests.

[erikrikarddaniel]

Corroboration from the other end, in case it is useful: we hit this on real data in nf-core/phyloplace while turning clustalo/align's compress option on, before finding this PR.

• Every .aln.gz above roughly 120 KB came out truncated — gzip -t → unexpected end of file. The ~8 KB outputs won the race, which is exactly why the minimal test profiles stayed green and nothing looked wrong.
• The consequence downstream is worse than a mis-reported exit status. EPA-NG hangs on a short gzip stream rather than failing, so a task sat for its full one-hour time limit and was killed with exit 143. That reads like a stalled executor, not corrupt data — it took reading .exitcode and the file's gzip -t to find the real cause.

So this is a silent-corruption bug with a downstream reader that blocks indefinitely, not only an error-propagation one. That seems worth having in the PR description.

I also checked && wait $! in this module's own container (bash 5.2.15, mulled-v2-4cefc38542f86c17596c29b35a059de10387c6a7):

$ printf alignment > >(cat >/dev/null; exit 42)
exit=0
$ printf alignment > >(cat >/dev/null; exit 42) && wait $!
exit=42

and with a deliberately slow writer, wait does block until the file is complete — so it fixes the truncation as well as the status. Note it relies on bash setting $! for a process substitution, which is bash 5.1+; fine for the pinned container, possibly worth a thought for the conda profile on older systems.

@lrauschning's scratch-space point convinced me, for what it is worth — I had independently gone the write-then-compress route in #12991 and am dropping or converting that part to follow whatever lands here.

One thing that may be out of scope for this PR: the same >(pigz ...) idiom is in kalign/align, magus/align and muscle5/super5, so all three have the same latent truncation. Happy to apply whichever form is settled on here to those three in a follow-up, rather than guessing now.

Investigated with AI assistance (Claude Code); the truncation, the container bash behaviour and the EPA-NG hang were all measured rather than inferred.

[SPPearce]

@dnncha , can you please join the organisation via the github-invitations channel on the nf-core slack, so your tests run.

[SPPearce]

I'm happy to merge this pattern without the python script.