Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ci: add Windows builds + tests #249

Merged
merged 1 commit into from
Nov 14, 2023
Merged

ci: add Windows builds + tests #249

merged 1 commit into from
Nov 14, 2023

Conversation

vszakats
Copy link
Member

@vszakats vszakats commented Nov 9, 2023

Add automated Windows builds and tests.

Use curl-for-win with llvm + mingw64, and a minimal libcurl build with
no external dependencies to build x64, ARM64 and x86 trurl.exe.

Boost build performance by not building the curl tool [EXPERIMENTAL].
Use a customized curl-for-win build with disabled TLS to further reduce
footprint and build time.

Non-UNITY libcurl builds can make turl binaries about 120KB smaller, but
they require 2x build times (4m vs. 2m), so opted not to use those here.

Also enable tests and fix issues along the way:

  • libcurl with IDN support cannot be used because trurl itself lacks
    UNICODE support and thus fails to accept non-ASCII strings via the
    command-line.
    expected:
    'https://xn--rksmrgs-5wao1o.se/\n'
    got:
    ''
    104: failed 'https://räksmörgås.se' -g '{puny:host}'
    
    Ref: https://github.com/curl/trurl/actions/runs/6863796328/job/18664263891#step:3:4406
  • add test.py support for a runner like wine.
    Via --runner=<bin>
    option. This disables valgrind tests.
  • add test.py to override the default trurl binary to test.
    Via --trurl=<bin> option.
  • skip stderr tests when using a runner.
    (wine does trash stderr output)
  • fix to enable punycode2idn only when libcurl has IDN support.
  • delete line-ending spaces from test.json.
  • add --keep-port to 6 tests to avoid relying on libcurl builds with
    specific protocols enabled, such as HTTPS or FTP.
  • add a new test with default-port using http/80.
  • update 4 tests to use http/imap instead of https/imaps to make them
    work with no-TLS libcurl.
  • build libcurl with IMAP to make 'options' URL field extraction work in
    tests.

Fixes #109
Closes #249

@vszakats vszakats marked this pull request as draft November 9, 2023 16:12
@vszakats vszakats added the enhancement New feature or request label Nov 9, 2023
@vszakats
Copy link
Member Author

vszakats commented Nov 9, 2023

First successful build:
https://github.com/curl/trurl/actions/runs/6814620035/job/18531898994#step:3:5308
Also for x86 and arm64.

@vszakats
Copy link
Member Author

vszakats commented Nov 9, 2023

Second run (3m11s), with downloadable artifacts:
https://github.com/curl/trurl/actions/runs/6814704059

@vszakats vszakats changed the title ci: try adding windows builds ci: add windows builds Nov 9, 2023
@vszakats vszakats marked this pull request as ready for review November 9, 2023 16:43
@vszakats vszakats mentioned this pull request Nov 9, 2023
2 tasks
@vszakats vszakats changed the title ci: add windows builds ci: add Windows builds Nov 9, 2023
@bagder
Copy link
Member

bagder commented Nov 9, 2023

Nice!

There is of course a lot of stuff in that downloadable, but we can potentially polish on that later.

@vszakats
Copy link
Member Author

vszakats commented Nov 9, 2023

More details about these builds:

  • trurl binaries are built against libcurl DLL. IOW not standalone static builds. Building the latter is technically doable, but:

    1. it's cumbersome to implement (like all static builds), so in curl-for-win I did not plan to do it.
      [UPDATE-3: I special cased it for -zero builds, which we use here. Still cumbersome, but less.]
    2. it'd result in trurl binaries as large as the curl tool. Meaning 700KB–6MB+ depending on options. ~~This could probably be improved in curl, so that linking the functions necessary for trurl wouldn't pull in almost all curl code. This would be generally a good idea for all curl dependents but also a sisyphean work.
      [UPDATE-2: This isn't an issue with the trurl CI artifact, because curl tool isn't distributed and trurl + libcurl.dll is the same size as trurl static.]

    UPDATE-1: This'd also need tweaking curl's unity build, to keep functions/sources required for trurl in a separate unity block. That is, after making these sources modular enough to not pull in unnecessary libcurl parts. For that, a slimmed-down curl_global_init()/-cleanup() is required at the minimum, then dealing with alloc/free, slist, curl_version_info(), curl_*printf(), curl_easy_[un]escape(), then curl_url_get().

  • They do not run make test. This is something to implement locally (not in curl-for-win). Switching to a Windows native CI host might make this easier, and it will need Python. Overall probably not a huge issue given that other CI jobs already run this and there isn't much (any?) platform dependent logic in it.
    UPDATE: It runs tests now.

  • They can be trivially extended to other platforms curl-for-win supports: Linux MUSL, Linux glibc and macOS.

  • They depend on curl-for-win Git repository head. Which may be temporarly broken. This happens rarely and it's covered by curl-for-win daily builds.

  • They also depend on curl Git repository master. Same caveat applies, but in practice it also builds fine almost always. Switching to use the latest stable curl is a trivial configuration option: -dev-test in CW_CONFIG.

@vszakats
Copy link
Member Author

vszakats commented Nov 9, 2023

@bagder: Yes, the artifact was tailored for curl. For trurl this can be re-done locally by picking out bin/turl.exe and bin/libcurl*.dll from the artifact. I'm thinking of an option to leave the necessary directories on disk (DONE → curl/curl-for-win@67a04d1). (I was also thinking doing this for the standalone curl binaries for easier access that doesn't require any unpacking, though GHA will zip it anyway, so didn't pursue this just yet.)

@vszakats
Copy link
Member Author

vszakats commented Nov 9, 2023

Improved artifacts with nothing else than necessary for trurl:
https://github.com/curl/trurl/actions/runs/6815583239

curl-8.5.0-DEV_4fc402b9f2cefa597f58b9703293af2992988d68-win32-mingw-pico/bin/libcurl.dll
curl-8.5.0-DEV_4fc402b9f2cefa597f58b9703293af2992988d68-win32-mingw-pico/bin/trurl.exe
curl-8.5.0-DEV_4fc402b9f2cefa597f58b9703293af2992988d68-win64-mingw-pico/bin/libcurl-x64.dll
curl-8.5.0-DEV_4fc402b9f2cefa597f58b9703293af2992988d68-win64-mingw-pico/bin/trurl.exe
curl-8.5.0-DEV_4fc402b9f2cefa597f58b9703293af2992988d68-win64a-mingw-pico/bin/libcurl-arm64.dll
curl-8.5.0-DEV_4fc402b9f2cefa597f58b9703293af2992988d68-win64a-mingw-pico/bin/trurl.exe
urls.txt

@vszakats vszakats force-pushed the ci-windoz branch 2 times, most recently from 8a21a68 to 05af44a Compare November 9, 2023 19:31
vszakats added a commit to vszakats/trurl that referenced this pull request Nov 9, 2023
Add automated Windows builds.

Use curl-for-win with llvm + mingw64, and a minimal curl build with
no external dependencies to build x64, ARM64 and x86 `trurl.exe`.

Ref: curl#109
Closes curl#249
vszakats added a commit to vszakats/trurl that referenced this pull request Nov 9, 2023
Add automated Windows builds.

Use curl-for-win with llvm + mingw64, and a minimal curl build with
no external dependencies to build x64, ARM64 and x86 `trurl.exe`.

Ref: curl#109
Closes curl#249
@vszakats vszakats force-pushed the ci-windoz branch 2 times, most recently from 4d8eb72 to 278ee48 Compare November 10, 2023 12:06
vszakats added a commit to vszakats/trurl that referenced this pull request Nov 10, 2023
Add automated Windows builds.

Use curl-for-win with llvm + mingw64, and a minimal curl build with
no external dependencies to build x64, ARM64 and x86 `trurl.exe`.

Ref: curl#109
Closes curl#249
@vszakats vszakats force-pushed the ci-windoz branch 2 times, most recently from 21c027f to 2f80800 Compare November 10, 2023 13:17
@vszakats
Copy link
Member Author

vszakats commented Nov 10, 2023

Now with a 'flat' package artifact:

curl-8.5.0-DEV_8197af4364d7c8402bc3a36f809801508ad2e96c-win32-mingw-zero/libcurl.dll
curl-8.5.0-DEV_8197af4364d7c8402bc3a36f809801508ad2e96c-win32-mingw-zero/trurl.exe
curl-8.5.0-DEV_8197af4364d7c8402bc3a36f809801508ad2e96c-win64-mingw-zero/libcurl-x64.dll
curl-8.5.0-DEV_8197af4364d7c8402bc3a36f809801508ad2e96c-win64-mingw-zero/trurl.exe
curl-8.5.0-DEV_8197af4364d7c8402bc3a36f809801508ad2e96c-win64a-mingw-zero/libcurl-arm64.dll
curl-8.5.0-DEV_8197af4364d7c8402bc3a36f809801508ad2e96c-win64a-mingw-zero/trurl.exe

vszakats added a commit to vszakats/trurl that referenced this pull request Nov 10, 2023
Add automated Windows builds.

Use curl-for-win with llvm + mingw64, and a minimal curl build with
no external dependencies to build x64, ARM64 and x86 `trurl.exe`.

Ref: curl#109
Closes curl#249
@vszakats vszakats marked this pull request as draft November 10, 2023 17:35
vszakats added a commit to curl/curl-for-win that referenced this pull request Nov 10, 2023
Use a solution that doesn't need `find -execdir`, because that is not
supported by busybox (alpine):
```
find: unrecognized: -execdir
BusyBox v1.36.1 (2023-07-27 17:12:24 UTC) multi-call binary.
```
https://github.com/curl/curl-for-win/actions/runs/6827484086/job/18569701123#step:3:5661

Also useful for trurl CI job:
curl/trurl#249
vszakats added a commit to vszakats/trurl that referenced this pull request Nov 10, 2023
Add automated Windows builds.

Use curl-for-win with llvm + mingw64, and a minimal curl build with
no external dependencies to build x64, ARM64 and x86 `trurl.exe`.

Ref: curl#109
Closes curl#249
vszakats added a commit to vszakats/trurl that referenced this pull request Nov 11, 2023
Add automated Windows builds.

Use curl-for-win with llvm + mingw64, and a minimal curl build with
no external dependencies to build x64, ARM64 and x86 `trurl.exe`.

Ref: curl#109
Closes curl#249
vszakats added a commit to vszakats/trurl that referenced this pull request Nov 11, 2023
Add automated Windows builds.

Use curl-for-win with llvm + mingw64, and a minimal curl build with
no external dependencies to build x64, ARM64 and x86 `trurl.exe`.

Ref: curl#109
Closes curl#249
vszakats added a commit to vszakats/trurl that referenced this pull request Nov 13, 2023
Add automated Windows builds.

Use curl-for-win with llvm + mingw64, and a minimal curl build with
no external dependencies to build x64, ARM64 and x86 `trurl.exe`.

Disable UNITY builds in curl to make `trurl` binaries about 120KB
smaller each.

Ref: curl#109
Closes curl#249
vszakats added a commit to vszakats/trurl that referenced this pull request Nov 14, 2023
Add automated Windows builds.

Use curl-for-win with llvm + mingw64, and a minimal curl build with
no external dependencies to build x64, ARM64 and x86 `trurl.exe`.

Disable UNITY builds in curl to make `trurl` binaries about 120KB
smaller each. Regain some of the lost build performance by not building
the curl tool [EXPERIMENTAL]. Use a customized curl-for-win build with
disabled TLS to further reduce footprint.

Ref: curl#109
Closes curl#249
vszakats added a commit to vszakats/trurl that referenced this pull request Nov 14, 2023
Add automated Windows builds.

Use curl-for-win with llvm + mingw64, and a minimal curl build with
no external dependencies to build x64, ARM64 and x86 `trurl.exe`.

Disable UNITY builds in curl to make `trurl` binaries about 120KB
smaller each. Regain some of the lost build performance by not building
the curl tool [EXPERIMENTAL]. Use a customized curl-for-win build with
disabled TLS to further reduce footprint.

Ref: curl#109
Closes curl#249
vszakats added a commit to vszakats/trurl that referenced this pull request Nov 14, 2023
Add automated Windows builds.

Use curl-for-win with llvm + mingw64, and a minimal curl build with
no external dependencies to build x64, ARM64 and x86 `trurl.exe`.

Disable UNITY builds in curl to make `trurl` binaries about 120KB
smaller each. Regain some of the lost build performance by not building
the curl tool [EXPERIMENTAL]. Use a customized curl-for-win build with
disabled TLS to further reduce footprint.

Ref: curl#109
Closes curl#249
vszakats added a commit to vszakats/trurl that referenced this pull request Nov 14, 2023
Add automated Windows builds.

Use curl-for-win with llvm + mingw64, and a minimal curl build with no
external dependencies to build x64, ARM64 and x86 `trurl.exe`.

Boost build performance by not building the curl tool [EXPERIMENTAL].
Use a customized curl-for-win build with disabled TLS to further reduce
footprint and build time.

Non-UNITY libcurl builds can make turl binaries about 120KB smaller, but
they require 2x build times (4m vs. 2m), so opted not to use those here.

Ref: curl#109
Closes curl#249
vszakats added a commit to vszakats/trurl that referenced this pull request Nov 14, 2023
Add automated Windows builds.

Use curl-for-win with llvm + mingw64, and a minimal curl build with no
external dependencies to build x64, ARM64 and x86 `trurl.exe`.

Boost build performance by not building the curl tool [EXPERIMENTAL].
Use a customized curl-for-win build with disabled TLS to further reduce
footprint and build time.

Non-UNITY libcurl builds can make turl binaries about 120KB smaller, but
they require 2x build times (4m vs. 2m), so opted not to use those here.

Ref: curl#109
Closes curl#249
@vszakats vszakats force-pushed the ci-windoz branch 2 times, most recently from f82c5fc to 38c249a Compare November 14, 2023 10:05
@vszakats vszakats changed the title ci: add Windows builds ci: add Windows builds + tests Nov 14, 2023
@vszakats
Copy link
Member Author

vszakats commented Nov 14, 2023

When trying to run tests (after fixing a bunch of roadblocks), there are 10 tests still failing:
https://github.com/curl/trurl/actions/runs/6864110037/job/18665177457#step:3:4226

These are test tests failing in the Windows libcurl + wine configuration:

20: passed	https://curl.se:22/ -s port=443
21: passed	https://curl.se:22/ -s port=443 --get '{url}'
23: passed	--default-port --url https://curl.se/we/are.html --get '{port}'
27: passed	--url 'imap://hello:secret;[email protected]/we/are.html' --get '{options}'
35: passed	--url https://curl.se/we/are.html -g '{default:port}'
59: passed	https://hello:443/foo
60: passed	ftp://hello:21/foo
62: passed	ftp://hello:443/foo -s scheme=https
84: passed	https://curl.se --iterate 'port=80 81 443'
105: passed	'imaps://user:password;crazy@[ff00::1234%hello]:1234/path?a=b&c=d#fragment' --json

E.g.:

59: failed	https://hello:443/foo
--- stdout --- 
expected:
'https://hello/foo\n'
got:
'https://hello:443/foo\n'

Does anybody have an idea where the problem might be?

UPDATE-1: This replicates with a local macOS build, using curl 8.5.0-DEV.
UPDATE-2: Also with curl 8.4.0 stable.
UPDATE-3: Also with a local Linux MUSL build, using curl 8.5.0-DEV.

vszakats added a commit to vszakats/trurl that referenced this pull request Nov 14, 2023
Add automated Windows builds.

Use curl-for-win with llvm + mingw64, and a minimal curl build with no
external dependencies to build x64, ARM64 and x86 `trurl.exe`.

Boost build performance by not building the curl tool [EXPERIMENTAL].
Use a customized curl-for-win build with disabled TLS to further reduce
footprint and build time.

Non-UNITY libcurl builds can make turl binaries about 120KB smaller, but
they require 2x build times (4m vs. 2m), so opted not to use those here.

Ref: curl#109
Closes curl#249
@vszakats
Copy link
Member Author

vszakats commented Nov 14, 2023

Right, the solution is that I reduced the libcurl build to the absolute minimum, meaning no TLS, but, this also makes curl not recognize the HTTPS scheme anymore, thus not regarding 443 a default port for HTTPS. And since by default trurl hides default ports, it hides it with a libcurl built with a TLS backend and shows it with a libcurl without a TLS backend.

This extends to all schemes a curl build might recognize, e.g. FTP in above failures (which FWIW are also disabled in the libcurl here).

If anything, tests should not rely on such property of libcurl, so one good fix is to add --keep-port to these.

@vszakats vszakats force-pushed the ci-windoz branch 2 times, most recently from afe21ca to 51954d1 Compare November 14, 2023 15:57
@vszakats
Copy link
Member Author

vszakats commented Nov 14, 2023

Down to:

28: failed	--url '***curl.se/we/are.html' --get '{options}'
106: failed	'***[ff00::1234%hello]:1234/path?a=b&c=d#fragment' --json

UPDATE: Fixed by enabling IMAP in libcurl and moving imaps to imap to work without TLS.

Add automated Windows builds and tests.

Use curl-for-win with llvm + mingw64, and a minimal libcurl build with
no external dependencies to build x64, ARM64 and x86 `trurl.exe`.

Boost build performance by not building the curl tool [EXPERIMENTAL].
Use a customized curl-for-win build with disabled TLS to further reduce
footprint and build time.

Non-UNITY libcurl builds can make turl binaries about 120KB smaller, but
they require 2x build times (4m vs. 2m), so opted not to use those here.

Also enable tests and fix issues along the way:
- libcurl with IDN support cannot be used because trurl itself lacks
  UNICODE support and thus fails to accept non-ASCII strings via the
  command-line.
  ```
  expected:
  'https://xn--rksmrgs-5wao1o.se/\n'
  got:
  ''
  104: failed 'https://räksmörgås.se' -g '{puny:host}'
  ```
  Ref: https://github.com/curl/trurl/actions/runs/6863796328/job/18664263891#step:3:4406
- add `test.py` support for a runner like `wine`.
  Via `--runner=<bin>`
  option. This disables `valgrind` tests.
- add `test.py` to override the default `trurl` binary to test.
  Via `--trurl=<bin>` option.
- skip `stderr` tests when using a runner.
  (`wine` does trash `stderr` output)
- fix to enable `punycode2idn` only when libcurl has IDN support.
- delete line-ending spaces from `test.json`.
- add `--keep-port` to 6 tests to avoid relying on libcurl builds with
  specific protocols enabled, such as HTTPS or FTP.
- add a new test with default-port using http/80.
- update 4 tests to use http/imap instead of https/imaps to make them
  work with no-TLS libcurl.
- build libcurl with IMAP to make 'options' URL field extraction work in
  tests.

Fixes curl#109
Closes curl#249
@vszakats vszakats merged commit bc67bb6 into curl:master Nov 14, 2023
10 checks passed
@vszakats vszakats deleted the ci-windoz branch November 14, 2023 19:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CI job to verify build on Windows
2 participants