From 74dab89be91bdca8a5c32727eddd71a8d4bd5164 Mon Sep 17 00:00:00 2001 From: Marek Kubica Date: Wed, 7 Aug 2024 11:32:39 +0200 Subject: [PATCH] Replace oneshot_webserver with fake curl (#10808) This solves a lot of the waiting leftover HTTP processes etc. The new fake curl will read the list of files to serve from `fake-curls` and map each line to a port. To match the behavior of the oneshot-webserver it will only serve each port once, subsequent requests for the same port will be answered by 404 (so not exactly the same but similar enough), the ports that were served are recorded into an `already-served` file. The same file can be served multiple times, as long as it is present multiple times in `fake-curls`. Signed-off-by: Marek Kubica --- src/dune_pkg/fetch.ml | 2 +- test/blackbox-tests/test-cases/dune | 2 +- .../pkg/compute-checksums-when-missing.t | 82 +++++++----------- test/blackbox-tests/test-cases/pkg/dune | 3 +- test/blackbox-tests/test-cases/pkg/e2e.t | 10 +-- .../test-cases/pkg/extra-sources.t | 25 +++--- .../test-cases/pkg/source-caching.t | 7 +- test/blackbox-tests/test-cases/pkg/tarball.t | 18 ++-- .../pkg/unavailable-package-source.t | 15 ++-- test/blackbox-tests/utils/curl | 83 +++++++++++++++++++ test/blackbox-tests/utils/dune | 5 -- .../blackbox-tests/utils/webserver_oneshot.ml | 63 -------------- 12 files changed, 147 insertions(+), 168 deletions(-) create mode 100755 test/blackbox-tests/utils/curl delete mode 100644 test/blackbox-tests/utils/webserver_oneshot.ml diff --git a/src/dune_pkg/fetch.ml b/src/dune_pkg/fetch.ml index 4601aacf50c..28f7a45e245 100644 --- a/src/dune_pkg/fetch.ml +++ b/src/dune_pkg/fetch.ml @@ -176,7 +176,7 @@ let fetch_curl ~unpack:unpack_flag ~checksum ~target (url : OpamUrl.t) = let exn = User_message.make [ Pp.textf - "failed to unpackage archive downloaded from %s" + "failed to unpack archive downloaded from %s" (OpamUrl.to_string url) ; Pp.text "reason:" ; msg diff --git a/test/blackbox-tests/test-cases/dune b/test/blackbox-tests/test-cases/dune index 0934807c370..630343865a7 100644 --- a/test/blackbox-tests/test-cases/dune +++ b/test/blackbox-tests/test-cases/dune @@ -8,7 +8,7 @@ ../utils/dunepp.exe ../utils/melc_stdlib_prefix.exe ../utils/refmt.exe - ../utils/webserver_oneshot.exe + ../utils/curl ../utils/sherlodoc.exe ../utils/ocaml_index.exe))) diff --git a/test/blackbox-tests/test-cases/pkg/compute-checksums-when-missing.t b/test/blackbox-tests/test-cases/pkg/compute-checksums-when-missing.t index 7e97eb582e9..21216c5492e 100644 --- a/test/blackbox-tests/test-cases/pkg/compute-checksums-when-missing.t +++ b/test/blackbox-tests/test-cases/pkg/compute-checksums-when-missing.t @@ -5,23 +5,11 @@ downloaded from non-local sources. $ . ./helpers.sh $ mkrepo - $ strip_transient() { - > sed -e "s#$PWD##" | \ - > # strip the entire address as the port number may also appear in the hash - > sed -e "s#http://.*$PORT##" - > } - A file that will comprise the package source: - $ echo "Hello, World!" > foo.txt -Run the server in the background: - -We have some really slow tests here - $ export DUNE_WEBSERVER_TIMEOUT=10 - - $ webserver_oneshot --content-file foo.txt --port-file port.txt & - $ until test -f port.txt; do sleep 0.1; done - $ PORT=$(cat port.txt) + $ echo "Hello, World!" > foo.txt + $ echo foo.txt > fake-curls + $ PORT=1 $ mkpkg foo < url { @@ -29,59 +17,52 @@ We have some really slow tests here > } > EOF - $ solve foo | strip_transient - Solution for dune.lock: - - foo.0.0.1 + $ solve foo Package "foo" has source archive which lacks a checksum. - The source archive will be downloaded from: + The source archive will be downloaded from: http://0.0.0.0:1 Dune will compute its own checksum for this source archive. - - $ wait + Solution for dune.lock: + - foo.0.0.1 Replace the path in the lockfile as it would otherwise include the sandbox path. - $ cat dune.lock/foo.pkg | strip_transient + $ cat dune.lock/foo.pkg (version 0.0.1) (source (fetch - (url ) + (url http://0.0.0.0:1) (checksum md5=bea8252ff4e80f41719ea13cdf007273))) (dev) Now make sure we can gracefully handle the case when the archive is missing. +Recreate the foo package with a fake port number to signal that the file will +404: - $ rm port.txt - $ webserver_oneshot --content-file foo.txt --port-file port.txt --simulate-not-found & - $ until test -f port.txt; do sleep 0.1; done - $ PORT=$(cat port.txt) - -Recreate the foo package as the port number will have changed: + $ PORT=9000 $ mkpkg foo < url { > src: "http://0.0.0.0:$PORT" > } > EOF - $ solve foo 2>&1 | strip_transient + $ solve foo 2>&1 Package "foo" has source archive which lacks a checksum. - The source archive will be downloaded from: + The source archive will be downloaded from: http://0.0.0.0:9000 Dune will compute its own checksum for this source archive. Warning: download failed with code 404 Solution for dune.lock: - foo.0.0.1 - $ cat dune.lock/foo.pkg | strip_transient + $ cat dune.lock/foo.pkg (version 0.0.1) (source (fetch - (url ))) + (url http://0.0.0.0:9000))) (dev) - $ wait - Check that no checksum is computed for a local source file: $ mkpkg foo < src: "$PWD/foo.txt" > } > EOF - $ solve foo 2>&1 | strip_transient + $ solve foo 2>&1 Solution for dune.lock: - foo.0.0.1 @@ -101,7 +82,7 @@ Check that no checksum is computed for a local source directory: > src: "$PWD/src" > } > EOF - $ solve foo 2>&1 | strip_transient + $ solve foo 2>&1 Solution for dune.lock: - foo.0.0.1 @@ -109,11 +90,11 @@ Check that no checksum is computed for a local source directory: Create 3 packages that all share the same source url with no checksum. Dune will need to download each package's source archive to compute their hashes. Test that dune only downloads the file a single time since the source url is -identical among the packgaes. The fact that the download only occurs once is +identical among the packages. The fact that the download only occurs once is asserted by the fact that the webserver will only serve the file a single time. - $ webserver_oneshot --content-file foo.txt --port-file port.txt & - $ until test -f port.txt; do sleep 0.1; done - $ PORT=$(cat port.txt) + + $ echo foo.txt >> fake-curls + $ PORT=2 $ mkpkg foo < url { @@ -133,20 +114,17 @@ asserted by the fact that the webserver will only serve the file a single time. > } > EOF - $ solve foo bar baz | strip_transient - Solution for dune.lock: - - bar.0.0.1 - - baz.0.0.1 - - foo.0.0.1 + $ solve foo bar baz Package "bar" has source archive which lacks a checksum. - The source archive will be downloaded from: + The source archive will be downloaded from: http://0.0.0.0:2 Dune will compute its own checksum for this source archive. Package "baz" has source archive which lacks a checksum. - The source archive will be downloaded from: + The source archive will be downloaded from: http://0.0.0.0:2 Dune will compute its own checksum for this source archive. Package "foo" has source archive which lacks a checksum. - The source archive will be downloaded from: + The source archive will be downloaded from: http://0.0.0.0:2 Dune will compute its own checksum for this source archive. - - $ wait - + Solution for dune.lock: + - bar.0.0.1 + - baz.0.0.1 + - foo.0.0.1 diff --git a/test/blackbox-tests/test-cases/pkg/dune b/test/blackbox-tests/test-cases/pkg/dune index 3c590a80eb0..d901e655d3d 100644 --- a/test/blackbox-tests/test-cases/pkg/dune +++ b/test/blackbox-tests/test-cases/pkg/dune @@ -38,8 +38,9 @@ (applies_to pkg-deps)) (cram - (deps %{bin:webserver_oneshot}) + (deps %{bin:curl}) (applies_to + unavailable-source-package compute-checksums-when-missing e2e source-caching diff --git a/test/blackbox-tests/test-cases/pkg/e2e.t b/test/blackbox-tests/test-cases/pkg/e2e.t index 8c023316555..4ad6ab4c00c 100644 --- a/test/blackbox-tests/test-cases/pkg/e2e.t +++ b/test/blackbox-tests/test-cases/pkg/e2e.t @@ -22,10 +22,10 @@ Make a library: $ tar -czf foo.tar.gz foo $ rm -rf foo -Start a oneshot webserver so dune can download the packgae with http: - $ webserver_oneshot --content-file foo.tar.gz --port-file port.txt & - $ until test -f port.txt; do sleep 0.1; done - $ PORT=$(cat port.txt) +Configure our fake curl to serve the tarball + + $ echo foo.tar.gz >> fake-curls + $ PORT=1 Make a package for the library: $ mkpkg foo < fake-curls + $ SRC_PORT=1 + $ echo required.patch >> fake-curls + $ REQUIRED_PATCH_PORT=2 We now have the checksums as well as the port numbers, so we can define the package. @@ -124,15 +122,12 @@ correct, patched, message: Set up a new version of the package which has multiple `extra-sources`, the application order of them mattering: - $ webserver_oneshot --content-file needs-patch.tar --port-file tarball-port.txt & - $ until test -f tarball-port.txt ; do sleep 0.1; done - $ SRC_PORT=$(cat tarball-port.txt) - $ webserver_oneshot --content-file required.patch --port-file required-patch-port.txt & - $ until test -f required-patch-port.txt ; do sleep 0.1; done - $ REQUIRED_PATCH_PORT=$(cat required-patch-port.txt) - $ webserver_oneshot --content-file additional.patch --port-file additional-patch-port.txt & - $ until test -f additional-patch-port.txt ; do sleep 0.1; done - $ ADDITIONAL_PATCH_PORT=$(cat additional-patch-port.txt) + $ echo needs-patch.tar >> fake-curls + $ SRC_PORT=3 + $ echo required.patch >> fake-curls + $ REQUIRED_PATCH_PORT=4 + $ echo additional.patch >> fake-curls + $ ADDITIONAL_PATCH_PORT=5 $ mkpkg needs-patch 0.0.2 < build: ["dune" "build" "-p" name "-j" jobs] diff --git a/test/blackbox-tests/test-cases/pkg/source-caching.t b/test/blackbox-tests/test-cases/pkg/source-caching.t index f2ed6e11a1d..b76119654d9 100644 --- a/test/blackbox-tests/test-cases/pkg/source-caching.t +++ b/test/blackbox-tests/test-cases/pkg/source-caching.t @@ -1,4 +1,4 @@ -This test demonstratest that fetching package sources should be cached +This test demonstrates that fetching package sources should be cached $ . ./helpers.sh @@ -9,9 +9,8 @@ This test demonstratest that fetching package sources should be cached $ mkdir $sources; touch $sources/dummy $ tar -czf $tarball $sources $ checksum=$(md5sum $tarball | awk '{ print $1 }') - $ webserver_oneshot --content-file $tarball --port-file port.txt & - $ until test -f port.txt ; do sleep 0.1; done - $ port=$(cat port.txt) + $ echo $tarball > fake-curls + $ port=1 $ makepkg() { > make_lockpkg $1 < --content-file tarball1.tar.gz \ - > --content-file tarball2.tar.gz & - $ until test -f port.txt; do sleep 0.1; done - $ port=$(cat port.txt) + $ echo tarball1.tar.gz > fake-curls + $ echo tarball2.tar.gz >> fake-curls $ runtest() { > make_lockpkg foo < (version 0.1.0) - > (source (fetch (url http://0.0.0.0:$port))) + > (source (fetch (url http://0.0.0.0:$1))) > (build (run sh -c "find . | sort")) > EOF > build_pkg foo > rm -rf _build > } - $ runtest tarball1.tar.gz + $ runtest 1 . ./foo - $ runtest tarball2.tar.gz + $ runtest 2 . ./foo - - $ wait diff --git a/test/blackbox-tests/test-cases/pkg/unavailable-package-source.t b/test/blackbox-tests/test-cases/pkg/unavailable-package-source.t index 7ce0ca8bb8e..fd55f9d1a01 100644 --- a/test/blackbox-tests/test-cases/pkg/unavailable-package-source.t +++ b/test/blackbox-tests/test-cases/pkg/unavailable-package-source.t @@ -34,10 +34,11 @@ Git Hint: Check that this Git URL in the project configuration is correct: "file://PWD/dummy" -Http -A bit annoying that this test can pass by accident if there's a server running -on 35000 - $ runtest "(fetch (url \"https://0.0.0.0:35000\"))" 2>&1 | sed -ne '/Error:/,$ p' - Error: curl returned an invalid error code 7 - - +HTTP + + $ runtest "(fetch (url \"https://0.0.0.0:35000\"))" 2>&1 | sed -ne '/Error:/,$ p' | sed -e 's/".*"/""/' + Error: + failed to unpack archive downloaded from https://0.0.0.0:35000 + reason: + unable to extract "" + diff --git a/test/blackbox-tests/utils/curl b/test/blackbox-tests/utils/curl new file mode 100755 index 00000000000..c9d2fd04b7f --- /dev/null +++ b/test/blackbox-tests/utils/curl @@ -0,0 +1,83 @@ +#!/usr/bin/env bash + +# our tiny drop in fake version of curl +# needs to support +# curl -V +# curl -L -s --user-agent foo --write-out "%{http_code}" -o -- uri + +SOURCE_FILE=fake-curls +REPEAT_FILE=already-served + +POSITIONAL_ARGS=() + +while [[ $# -gt 0 ]]; do + case $1 in + -V) + VERSIONINFO=YES + shift # past argument + ;; + -L|-s|--) + shift # past argument + ;; + -o|--extension) + OUTPUT="$2" + shift # past argument + shift # past value + ;; + --user-agent|--write-out) + shift # past argument + shift # past value + ;; + -*|--*) + echo "Unknown option $1" + exit 1 + ;; + *) + POSITIONAL_ARGS+=("$1") # save positional arg + shift # past argument + ;; + esac +done + +set -- "${POSITIONAL_ARGS[@]}" # restore positional parameters + +if [ "$VERSIONINFO" = "YES" ]; then + echo "curl 0.0.0 (dune-fake)" + exit 0 +fi + +# read the offset from the URL +PORT_OFFSET=$(echo "$POSITIONAL_ARGS" | grep -Eo ':[0-9]+' | grep -o '[0-9]*') + +POSSIBLE_FILES=$(wc -l < "$SOURCE_FILE") + +write_out() { + # --write_out "${http_code}" does not write a newline + echo -n \"$1\" +} + +>&2 echo '$POSSIBLE_FILES' \""$POSSIBLE_FILES"\" '$PORT_OFFSET' \""$PORT_OFFSET"\" + +if [ ! -f "$REPEAT_FILE" ]; then + file_already_served=false +else + if grep "^$PORT_OFFSET\$" "$REPEAT_FILE" 2>/dev/null 1>&2 ; then + file_already_served=true + else + file_already_served=false + fi +fi + +>&2 echo File already served: $file_already_served + +if [ $PORT_OFFSET -gt $POSSIBLE_FILES ] || [ $file_already_served = true ]; then + >&2 echo "Failing the download" + write_out 404 +else + >&2 echo "Succeeding the download" + # read the line signified by the port from the file + INPUT=$(head -n "$PORT_OFFSET" "$SOURCE_FILE" | tail -n 1) + echo "$PORT_OFFSET" >> "$REPEAT_FILE" + cp "$INPUT" "$OUTPUT" + write_out 200 +fi diff --git a/test/blackbox-tests/utils/dune b/test/blackbox-tests/utils/dune index 09f3e38f714..8ea5c63a38f 100644 --- a/test/blackbox-tests/utils/dune +++ b/test/blackbox-tests/utils/dune @@ -24,11 +24,6 @@ (name refmt) (modules refmt)) -(executable - (name webserver_oneshot) - (modules webserver_oneshot) - (libraries unix http stdune)) - (executable (name sherlodoc) (modules sherlodoc) diff --git a/test/blackbox-tests/utils/webserver_oneshot.ml b/test/blackbox-tests/utils/webserver_oneshot.ml deleted file mode 100644 index 6e9eefa80e8..00000000000 --- a/test/blackbox-tests/utils/webserver_oneshot.ml +++ /dev/null @@ -1,63 +0,0 @@ -(* An http server which serves the contents of a given file a single time and - then terminates *) - -open Stdune - -module Args = struct - type t = - { content_files : string list - ; port_file : string - ; simulate_not_found : bool - } - - let parse () = - let content_files = ref [] in - let port_file = ref "" in - let simulate_not_found = ref false in - let specs = - [ ( "--content-file" - , Arg.String (fun file -> content_files := file :: !content_files) - , "File to serve" ) - ; ( "--port-file" - , Arg.Set_string port_file - , "The server will write its port number to this file" ) - ; "--simulate-not-found", Arg.Set simulate_not_found, "Return a 404 page" - ] - in - Arg.parse - specs - (fun _anon_arg -> failwith "unexpected anonymous argument") - "Run a webserver on a random port which serves the contents of a single file a \ - single time, then terminates."; - { content_files = List.rev !content_files - ; port_file = !port_file - ; simulate_not_found = !simulate_not_found - } - ;; -end - -let main content_files port_file ~simulate_not_found = - let host = Unix.inet_addr_loopback in - let addr = Unix.ADDR_INET (host, 0) in - let server = Http.Server.make addr in - Http.Server.start server; - let port = Http.Server.port server in - (* Create the port file immediately before starting the server. This way - clients can use the existence of the port file to know roughly when the - server is ready to accept connections. Note that there is technically a - small delay between creating the port file and the server being ready - which we can remove if it ends up causing us problems. *) - Out_channel.with_open_text port_file (fun out_channel -> - Out_channel.output_string out_channel (Printf.sprintf "%d\n" port)); - List.iter content_files ~f:(fun content_file -> - Http.Server.accept server ~f:(fun session -> - let () = Http.Server.accept_request session in - if simulate_not_found - then Http.Server.respond session ~status:`Not_found ~content:"" - else Http.Server.respond_file session ~file:content_file)) -;; - -let () = - let { Args.content_files; port_file; simulate_not_found } = Args.parse () in - main content_files port_file ~simulate_not_found -;;