Windows builds via GitHub Actions by jkbonfield · Pull Request #1796 · samtools/htslib

jkbonfield · 2024-06-26T09:20:25Z

The AppVeyor builds have become slower to launch and they're also a bit slow on execution. GitHub Actions runs in about half the time.

Note this needs samtools/htscodecs#123 merging first, but for purposes of testing I just incorporated it in this PR via an extra commit to change the submodule commit. If the htscodecs PR gets modified in any way, changing the hash, then I'll update this appropriately.

Also, if you wish to see these tests in-situ here, we could copy this PR branch to samtools/htslib from jkbonfield/htslib as otherwise it won't enable the workflow. (As I did for htscodecs)

jmarshall · 2024-06-26T21:49:26Z

I was initially going to suggest a git config core.autocrlf-core.eol-etc step before the checkout step (on the basis that it would be better to affect only the action runner rather than all Windows users as adding to .gitattributes does), but after reading through actions/checkout#135 I have changed my mind. Fixing this for the test files for all Windows users is actually the right thing to do!

That's a long thread, but actions/checkout#135 (comment) makes a very good point that just doing a blanket ** -text that would also affect *.c files would be the wrong thing to do. So I'm glad to see this PR just does ‑text more specifically for a bunch of test file extensions.

jkbonfield · 2024-06-27T08:16:03Z

I confess for the Samtools one I gave up worrying about every little extension and just went for test/**, but I do accept there are some C files in there. I wonder if we can then do text/*.c +text or similar to reenable it as an exception to the blanket rule? I doubt it matters though tbh as I'm not convinced we'll be getting many, if any, PRs from windows developers, and even if so the code tools typically all support nl anyway.

daviesrob · 2024-07-04T08:39:05Z

+      run: |
+        export PATH=/mingw64/bin:$PATH
+        export MSYSTEM=MINGW64
+        make check


Compared with the old script, we've lost make test-shlib-exports. Assuming it does actually work on Windows, it's probably useful to have.

Good spot. I'm assuming it does work as it was in the appveyor config. I'll put it back and see what happens.

jkbonfield · 2024-07-04T09:26:52Z

Hmm, disappointingly adding back the test-shlib-exports now fails:

Checking shared library exports
kh_begin
kh_clear_##name
kh_del_##name
kh_destroy_##name
kh_get_##name
kh_init_##name
kh_put_##name
kh_resize_##name
ks_destroy
ks_getuntil2
ks_init
kseq_destroy
kseq_init
kseq_read
Error: Found unexported symbols (listed above)

I initially thought they're all erroneous things from the templates-in-C style of klib, but kseq_read is more normal, although it's still done as a macro with a SCOPE define. I'll need to dig deeper into what it's attempting to do and why we're no longer passing this script.

Also amend .gitattributes file for more Windows text-mode removal as for some reason the GitHub Actions git is much more likely to use CR-LF line endings. Added libdeflate to the windows build. Corrected test/header_syms.pl to work on windows line endings.

jkbonfield · 2024-07-04T11:27:53Z

Fixing the header checking perl script to cope with windows line endings worked. So I've patched that and done some rebasing and squashing of commits. Assuming it now passes this is ready for re-review.

daviesrob · 2024-07-04T13:24:55Z

As foretold, the htscodecs submodule commit needs to be adjusted. It also might be good to leave the AppVeyor configuration in place for now, for a less messy transition. Once the couple of recent pull requests that straddle the change have been committed, (#1797, #1799) we can turn it off in the settings and push a commit to remove the configuration.

jkbonfield · 2024-07-04T13:35:18Z

Ah yes I meant to update the htscodecs submodule and forgot.

I was assuming we'd just turn off the appveyor, rerun tests, and if it passed merge it. But we can do it in two separate PRs if really needed. It's minor fluff tbh.

Completes the migration of Windows testing to GitHub actions. GitHub actions tests were added in commit 624e95b (PR samtools#1796).

Completes the migration of Windows testing to GitHub actions. GitHub actions tests were added in commit 624e95b (PR #1796).

Notice: this is the last SAMtools / HTSlib release where CRAM 3.0 will be the default CRAM version. From the next we will change to CRAM 3.1 unless the version is explicitly specified, for example using "samtools view -O cram,version=3.0". Updates ------- * Extend annot-tsv with several new command line options. --delim permits use of other delimiters. --headers for selection of other header formats. --no-header-idx to suppress column index numbers in header. Also removed -h as it is now short for --headers. Note --help still works. (PR samtools#1779) * Allow annot-tsv -a to rename annotations. (PR samtools#1709) * Extend annot-tsv --overlap to be able to specify the overlap fraction separately for source and target. (PR samtools#1811) * Added new APIs to facilitate low-level CRAM container manipulations, used by the new "samtools cat" region filtering code. Functions are: cram_container_get_coords() cram_filter_container() cram_index_extents() cram_container_num2offset() cram_container_offset2num() cram_num_containers() cram_num_containers_between() Also improved cram_index_query() to cope with HTS_IDX_NOCOOR regions. (PR samtools#1771) * Bgzip now retains file modification and access times when compressing and decompressing. (PR samtools#1727, fixes samtools#1718. Requested by Gert Hulselmans.) * Use FNV1a for string hashing in khash. The old algorithm was particularly weak with base-64 style strings and lead to a large number of collisions. (PR samtools#1806. Fixes samtools/samtools#2066, reported by Hans-Joachim Ruscheweyh) * Improve the speed of the nibble2base() function on Intel (PR samtools#1667, PR samtools#1764, PR samtools#1786, PR samtools#1802, thanks to Ruben Vorderman) and ARM (PR samtools#1795, thanks to John Marshall). * bgzf_getline() will now warn if it encounters UTF-16 data. (PR samtools#1487, thanks to John Marshall) * Speed up bgzf_read(). While this does not reduce CPU significantly, it does increase the maximum parallelism available permitting 10-15% faster decoding. (PR samtools#1772, PR samtools#1800, Issue samtools#1798) * Speed up faidx by use of better isgraph methods (PR samtools#1797) and whole-line reading (PR samtools#1799, thanks to John Marshall). * Speed up kputll() function, speeding up BAM -> SAM conversion by about 5% and also samtools depth. (PR samtools#1805) * Added more example code, covering fasta/fastq indexing, tabix indexing and use of the thread pool. (PR samtools#1666) Build Changes ------------- * Code warning fixes for pedantic compilers (PR samtools#1777) and avoid some undefined behaviour (PR samtools#1810, PR samtools#1816, PR samtools#1828). * Windows based CI has been migrated from AppVeyor to GitHub Actions. (PR samtools#1796, PR samtools#1803, PR samtools#1808) * Miscellaneous minor build infrastructure and code fixes. (PR samtools#1807, PR samtools#1829, both thanks to John Marshall) * Updated htscodecs submodule to version 1.6.1 (PR samtools#1828) * Fixed an awk script in the Makefile that only worked with gawk. (PR samtools#1831) Bug fixes --------- * Fix small OSS-Fuzz reported issues with CRAM encoding and long CIGARS and/or illegal positions. (PR samtools#1775, PR samtools#1801, PR samtools#1817) * Fix issues with on-the-fly indexing of VCF/BCF (bcftools --write-index) when not using multiple threads. (PR samtools#1837. Fixes samtools/bcftools#2267, reported by Giulio Genovese) * Stricter limits on POS / MPOS / TLEN in sam_parse1(). This fixes a signed overflow reported by OSS-Fuzz and should help prevent other as-yet undetected bugs. (PR samtools#1812) * Check that the underlying file open worked for preload: URLs. Fixes a NULL pointer dereference reported by OSS-Fuzz. (PR samtools#1821) * Fix an infinite loop in hts_itr_query() when given extremely large positions which cause integer overflow. Also adds hts_bin_maxpos() and hts_idx_maxpos() functions. (PR samtools#1774, thanks to John Marshall and reported by Jesus Alberto Munoz Mesa) * Fix an out of bounds read in hts_itr_multi_next() when switching chromosomes. This bug is present in releases 1.11 to 1.20. (PR samtools#1788. Fixes samtools/samtools#2063, reported by acorvelo) * Work around parsing problems with colons in CHROM names. Fixes samtools/bcftools#2139. (PR samtools#1781, John Marshall / James Bonfield) * Correct the CPU detection for Mac OS X 10.7. cpuid is used by htscodecs (see samtools/htscodecs#116), and the corresponding changes in htslib are PR samtools#1785. Reported by Ryan Carsten Schmidt. * Make BAM zero-length intervals work the same as CRAM; permitted and returning overlapping records. (PR samtools#1787. Fixes samtools/samtools#2060, reported by acorvelo) * Replace assert() with abort() in BCF synced reader. This is not an ideal solution, but it gives consistent behaviour when compiling with or without NDEBUG. (PR samtools#1791, thanks to Martin Pollard) * Fixed failure to change the write block size on compressed SAM or VCF files due to an internal type confusion. (PR samtools#1826) * Fixed an out-of-bounds read in cram_codec_iter_next() (PR samtools#1832)

htslib release 1.21: The primary user-visible changes in this release are updates to the annot-tsv tool and some speed improvements. Full details of other changes and bugs fixed are below. Notice: this is the last SAMtools / HTSlib release where CRAM 3.0 will be the default CRAM version. From the next we will change to CRAM 3.1 unless the version is explicitly specified, for example using "samtools view -O cram,version=3.0". Updates ------- * Extend annot-tsv with several new command line options. --delim permits use of other delimiters. --headers for selection of other header formats. --no-header-idx to suppress column index numbers in header. Also removed -h as it is now short for --headers. Note --help still works. (PR samtools#1779) * Allow annot-tsv -a to rename annotations. (PR samtools#1709) * Extend annot-tsv --overlap to be able to specify the overlap fraction separately for source and target. (PR samtools#1811) * Added new APIs to facilitate low-level CRAM container manipulations, used by the new "samtools cat" region filtering code. Functions are: cram_container_get_coords() cram_filter_container() cram_index_extents() cram_container_num2offset() cram_container_offset2num() cram_num_containers() cram_num_containers_between() Also improved cram_index_query() to cope with HTS_IDX_NOCOOR regions. (PR samtools#1771) * Bgzip now retains file modification and access times when compressing and decompressing. (PR samtools#1727, fixes samtools#1718. Requested by Gert Hulselmans.) * Use FNV1a for string hashing in khash. The old algorithm was particularly weak with base-64 style strings and lead to a large number of collisions. (PR samtools#1806. Fixes samtools/samtools#2066, reported by Hans-Joachim Ruscheweyh) * Improve the speed of the nibble2base() function on Intel (PR samtools#1667, PR samtools#1764, PR samtools#1786, PR samtools#1802, thanks to Ruben Vorderman) and ARM (PR samtools#1795, thanks to John Marshall). * bgzf_getline() will now warn if it encounters UTF-16 data. (PR samtools#1487, thanks to John Marshall) * Speed up bgzf_read(). While this does not reduce CPU significantly, it does increase the maximum parallelism available permitting 10-15% faster decoding. (PR samtools#1772, PR samtools#1800, Issue samtools#1798) * Speed up faidx by use of better isgraph methods (PR samtools#1797) and whole-line reading (PR samtools#1799, thanks to John Marshall). * Speed up kputll() function, speeding up BAM -> SAM conversion by about 5% and also samtools depth. (PR samtools#1805) * Added more example code, covering fasta/fastq indexing, tabix indexing and use of the thread pool. (PR samtools#1666) Build Changes ------------- * Code warning fixes for pedantic compilers (PR samtools#1777) and avoid some undefined behaviour (PR samtools#1810, PR samtools#1816, PR samtools#1828). * Windows based CI has been migrated from AppVeyor to GitHub Actions. (PR samtools#1796, PR samtools#1803, PR samtools#1808) * Miscellaneous minor build infrastructure and code fixes. (PR samtools#1807, PR samtools#1829, both thanks to John Marshall) * Updated htscodecs submodule to version 1.6.1 (PR samtools#1828) * Fixed an awk script in the Makefile that only worked with gawk. (PR samtools#1831) Bug fixes --------- * Fix small OSS-Fuzz reported issues with CRAM encoding and long CIGARS and/or illegal positions. (PR samtools#1775, PR samtools#1801, PR samtools#1817) * Fix issues with on-the-fly indexing of VCF/BCF (bcftools --write-index) when not using multiple threads. (PR samtools#1837. Fixes samtools/bcftools#2267, reported by Giulio Genovese) * Stricter limits on POS / MPOS / TLEN in sam_parse1(). This fixes a signed overflow reported by OSS-Fuzz and should help prevent other as-yet undetected bugs. (PR samtools#1812) * Check that the underlying file open worked for preload: URLs. Fixes a NULL pointer dereference reported by OSS-Fuzz. (PR samtools#1821) * Fix an infinite loop in hts_itr_query() when given extremely large positions which cause integer overflow. Also adds hts_bin_maxpos() and hts_idx_maxpos() functions. (PR samtools#1774, thanks to John Marshall and reported by Jesus Alberto Munoz Mesa) * Fix an out of bounds read in hts_itr_multi_next() when switching chromosomes. This bug is present in releases 1.11 to 1.20. (PR samtools#1788. Fixes samtools/samtools#2063, reported by acorvelo) * Work around parsing problems with colons in CHROM names. Fixes samtools/bcftools#2139. (PR samtools#1781, John Marshall / James Bonfield) * Correct the CPU detection for Mac OS X 10.7. cpuid is used by htscodecs (see samtools/htscodecs#116), and the corresponding changes in htslib are PR samtools#1785. Reported by Ryan Carsten Schmidt. * Make BAM zero-length intervals work the same as CRAM; permitted and returning overlapping records. (PR samtools#1787. Fixes samtools/samtools#2060, reported by acorvelo) * Replace assert() with abort() in BCF synced reader. This is not an ideal solution, but it gives consistent behaviour when compiling with or without NDEBUG. (PR samtools#1791, thanks to Martin Pollard) * Fixed failure to change the write block size on compressed SAM or VCF files due to an internal type confusion. (PR samtools#1826) * Fixed an out-of-bounds read in cram_codec_iter_next() (PR samtools#1832) # -----BEGIN PGP SIGNATURE----- # # iQJJBAABCgAzFiEEaGn1wQ1nqxSLDC2XHsvluhXGTMwFAmbjBlMVHHJtZCtnaXRA # c2FuZ2VyLmFjLnVrAAoJEB7L5boVxkzMgb0P/3AAxkJNLGK8Q1pBBo+in7KzwBOc # /nRfvReki47r4+i1ev98abEF/+XZoYSS8ZkiJH8JMkLCclb1ch2fDQTIL/sjJwrU # MfhYx6oHhzLUOjpG24m390wlvlkEsg5S69jbgImWxLQ1n4ZcSuZGumiwPx5274w5 # 3R/K9vIXtuSF9DTD/jrTgoZeBiN+B4XefAJqHLSo4/XzDo7DNMSZryNGFpHdyKcB # oEfEXX45vJLgzA3Fxo3LqNpC8UTDO5C9u9Uup+hCLN7RTBjV5Fu0UxcwWTSPqfiC # Jroi2FhxNPlDkRL3jraI8LrB3Cqb0G/31OU+cryasrqYnfJrl0e/MyXiRSa0sg25 # uHHWVyyC4JR7uBfUCppTVf76iajbkIVpHgHLZS1heGwM4PCL+AalvsIgGcvLXcBs # g/AwZLoXzao+pbzg8hzN1JuLB5hHcrHp6TsQ6JbTTrWaLeAL3s/+eQ4+d+Np2iUn # v8TMg6onu1HiNgVMQcFXSLC2Q51rB+lD6AA3eusKjcQsaxyk+U7SLCx3OhbvxGsd # SjxQWP1W+WW//e9nm5f4CTSSf1PHoDMyR/kVEc3qwEWJslri68RyB69HrDIVzrWT # 3gKAKJYafi5ssFHsKTsCngESpDsTg3aQV1AGOQPLN/TIjyYDTUOL+/HylRs3k9XN # KOGO5yggx+yqeL3w # =lHVl # -----END PGP SIGNATURE----- # gpg: Signature made Thu 12 Sep 2024 08:18:43 AM PDT # gpg: using RSA key 6869F5C10D67AB148B0C2D971ECBE5BA15C64CCC # gpg: issuer "rmd+git@sanger.ac.uk" # gpg: Can't check signature: No public key

daviesrob self-assigned this Jun 27, 2024

jkbonfield force-pushed the windows-actions branch from 355a4bc to aa9c5c0 Compare June 27, 2024 11:18

jkbonfield marked this pull request as ready for review June 27, 2024 13:34

daviesrob reviewed Jul 4, 2024

View reviewed changes

jkbonfield force-pushed the windows-actions branch 2 times, most recently from b5f0997 to 8ca61a7 Compare July 4, 2024 11:09

jkbonfield force-pushed the windows-actions branch from 8ca61a7 to 1955431 Compare July 4, 2024 11:27

Update htscodecs to include github actions branch

a3403fa

jkbonfield force-pushed the windows-actions branch from 1955431 to a3403fa Compare July 4, 2024 13:36

daviesrob merged commit b8145e6 into samtools:develop Jul 4, 2024

jkbonfield mentioned this pull request Jul 4, 2024

Windows build samtools/samtools#2064

Closed

daviesrob added a commit to daviesrob/htslib that referenced this pull request Jul 8, 2024

Remove AppVeyor configuration

2b453ee

Completes the migration of Windows testing to GitHub actions. GitHub actions tests were added in commit 624e95b (PR samtools#1796).

daviesrob mentioned this pull request Jul 8, 2024

Remove AppVeyor configuration #1803

Merged

daviesrob added a commit that referenced this pull request Jul 8, 2024

Remove AppVeyor configuration

1187fa8

Completes the migration of Windows testing to GitHub actions. GitHub actions tests were added in commit 624e95b (PR #1796).

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Windows builds via GitHub Actions#1796

Windows builds via GitHub Actions#1796
daviesrob merged 2 commits into
samtools:developfrom
jkbonfield:windows-actions

jkbonfield commented Jun 26, 2024

Uh oh!

jmarshall commented Jun 26, 2024 •

edited

Loading

Uh oh!

jkbonfield commented Jun 27, 2024

Uh oh!

daviesrob Jul 4, 2024

Uh oh!

jkbonfield Jul 4, 2024

Uh oh!

jkbonfield commented Jul 4, 2024

Uh oh!

jkbonfield commented Jul 4, 2024

Uh oh!

daviesrob commented Jul 4, 2024

Uh oh!

jkbonfield commented Jul 4, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

jkbonfield commented Jun 26, 2024

Uh oh!

jmarshall commented Jun 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jkbonfield commented Jun 27, 2024

Uh oh!

daviesrob Jul 4, 2024

Choose a reason for hiding this comment

Uh oh!

jkbonfield Jul 4, 2024

Choose a reason for hiding this comment

Uh oh!

jkbonfield commented Jul 4, 2024

Uh oh!

jkbonfield commented Jul 4, 2024

Uh oh!

daviesrob commented Jul 4, 2024

Uh oh!

jkbonfield commented Jul 4, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jmarshall commented Jun 26, 2024 •

edited

Loading