From patchwork Mon Feb 1 14:04:16 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: M X-Patchwork-Id: 26808 Return-Path: X-Original-To: patchwork@mira.cbaines.net Delivered-To: patchwork@mira.cbaines.net Received: by mira.cbaines.net (Postfix, from userid 113) id 9C8EB27BC24; Mon, 1 Feb 2021 14:06:28 +0000 (GMT) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on mira.cbaines.net X-Spam-Level: X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED, FREEMAIL_FROM,MAILING_LIST_MULTI,RCVD_IN_MSPIKE_H4,RCVD_IN_MSPIKE_WL, SPF_HELO_PASS,T_DKIM_INVALID,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.2 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) by mira.cbaines.net (Postfix) with ESMTPS id CCC7227BC26 for ; Mon, 1 Feb 2021 14:06:26 +0000 (GMT) Received: from localhost ([::1]:42560 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1l6ZqT-0001T8-S7 for patchwork@mira.cbaines.net; Mon, 01 Feb 2021 09:06:25 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]:49854) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1l6Zq6-0001Sx-Ue for guix-patches@gnu.org; Mon, 01 Feb 2021 09:06:04 -0500 Received: from debbugs.gnu.org ([209.51.188.43]:47867) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1l6Zq6-0003Ts-0x for guix-patches@gnu.org; Mon, 01 Feb 2021 09:06:02 -0500 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1l6Zq5-0004k2-Sl for guix-patches@gnu.org; Mon, 01 Feb 2021 09:06:01 -0500 X-Loop: help-debbugs@gnu.org Subject: [bug#46216] [PATCH] Remove duplication in tests/publish.scm Resent-From: Maxime Devos Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Mon, 01 Feb 2021 14:06:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 46216 X-GNU-PR-Package: guix-patches X-GNU-PR-Keywords: patch To: 46216@debbugs.gnu.org Received: via spool by 46216-submit@debbugs.gnu.org id=B46216.161218833618185 (code B ref 46216); Mon, 01 Feb 2021 14:06:01 +0000 Received: (at 46216) by debbugs.gnu.org; 1 Feb 2021 14:05:36 +0000 Received: from localhost ([127.0.0.1]:59413 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1l6Zpf-0004jD-KU for submit@debbugs.gnu.org; Mon, 01 Feb 2021 09:05:36 -0500 Received: from baptiste.telenet-ops.be ([195.130.132.51]:45028) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1l6Zpc-0004j2-3T for 46216@debbugs.gnu.org; Mon, 01 Feb 2021 09:05:33 -0500 Received: from ptr-bvsjgyjmffd7q9timvx.18120a2.ip6.access.telenet.be ([IPv6:2a02:1811:8c09:9d00:aaf1:9810:a0b8:a55d]) by baptiste.telenet-ops.be with bizsmtp id Pq5W2400A0mfAB401q5WbT; Mon, 01 Feb 2021 15:05:30 +0100 Message-ID: <92135424cd296e2e02f4f58a638da5cc6dcc1667.camel@telenet.be> From: Maxime Devos Date: Mon, 01 Feb 2021 15:04:16 +0100 In-Reply-To: References: User-Agent: Evolution 3.34.2 MIME-Version: 1.0 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=telenet.be; s=r21; t=1612188330; bh=VEPHxdttsi5zZlN/yzlN4Lmxx3+HcL0yahP9IZsn6Ts=; h=Subject:From:Reply-To:To:Date:In-Reply-To:References; b=FczjSY/iYV8Im5cj68WHAcS4GfwAJ4/IXNFAcUzQof2YqZYelybwGN9dd726LDrxD 0O6WDVbO7pgNhp/M3xrCoNGnvpj4rOqTl+ToIvY7HrXCPtBO0T/hlkih5Jaw/lx2gt gxWX3FQnCQRAFyJVb/eQU93fL/njxpwN2sOzhJZiIL3mv5JgHzxgDnxKUi9GHwEQnA 8M3erU23MYJMcRQ1QLWOSnlbHydnud+tvBBAnnqWHbLl1mcyBmpjSotd4cwwzpN3XM jOdVx27EYdsObYMemlSiGggqZEaL8aGTjpfgNKk0piMX7mixEEM+uNr+zQtZbApsGp lGT6x6w+R5ezA== X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: guix-patches@gnu.org List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: 46216@debbugs.gnu.org Errors-To: guix-patches-bounces+patchwork=mira.cbaines.net@gnu.org Sender: "Guix-patches" X-getmail-retrieved-from-mailbox: Patches This is the fourth patch in the series. It removes any explicit port numbers in the test. This may be useful for preventing some potential future problems with parallel "make check". Description from the commit message: > Subject: [PATCH 4/4] tests: publish: don't bind the test server to a port. > > This way, multiple instances of 'make check TESTS=tests/publish.scm' > can be run in parallel. Also, there's no risk of the ports used > in this test conflicting with ports assigned to system services > anymore. This also prevents any potential future conflicts > with ports used by other tests that would lead to nondeterministic > test failures when parallel tests are enabled. From e88c86ac6ccfffcc6fa5f3cbd2d5ec5178421763 Mon Sep 17 00:00:00 2001 From: Maxime Devos Date: Mon, 1 Feb 2021 14:28:03 +0100 Subject: [PATCH 4/4] tests: publish: don't bind the test server to a port. This way, multiple instances of 'make check TESTS=tests/publish.scm' can be run in parallel. Also, there's no risk of the ports used in this test conflicting with ports assigned to system services anymore. This also prevents any potential future conflicts with ports used by other tests that would lead to nondeterministic test failures when parallel tests are enabled. * guix/scripts/publish.scm (when-bound): new parameter for communicating with tests/publish.scm. (guix-publish): inform tests/publish.scm about the port the server socket was bound to via 'when-bound'. Also correctly log to which port the server was bound. * tests/publish.scm (spawn-guix-publish): remove 'port' argument, ask "guix publish" not to explicitely bind the server socket to a port and add the port it was implicitely bound to, to the return values. (call-with-guix-publish): adjust call to 'spawn-guix-publish'. Also adjust the code to spawn the first server and 'publish-uri' to the new semantics of 'spawn-guix-publish'. --- guix/scripts/publish.scm | 34 +++++++++++++++++++---- tests/publish.scm | 58 +++++++++++++++++++++++----------------- 2 files changed, 63 insertions(+), 29 deletions(-) diff --git a/guix/scripts/publish.scm b/guix/scripts/publish.scm index fa85088ed0..43233a4fd0 100644 --- a/guix/scripts/publish.scm +++ b/guix/scripts/publish.scm @@ -3,6 +3,7 @@ ;;; Copyright © 2020 by Amar M. Singh ;;; Copyright © 2015, 2016, 2017, 2018, 2019, 2020 Ludovic Courtès ;;; Copyright © 2020 Maxim Cournoyer +;;; Copyright © 2021 Maxime Devos ;;; ;;; This file is part of GNU Guix. ;;; @@ -75,7 +76,9 @@ open-server-socket publish-service-type run-publish-server - guix-publish)) + guix-publish + + when-bound)) (define (show-help) (format #t (G_ "Usage: guix publish [OPTION]... @@ -116,6 +119,19 @@ Publish ~a over HTTP.\n") %store-directory) (newline) (show-bug-report-information)) +;; When testing, ideally the server isn't explicitly bound to +;; any particular port, to avoid conflicts with other software, +;; and to be able "make check" multiple Guix checkouts in parallel. +;; +;; While these conflicts won't appear in the build container, +;; they can still be annoying when a developer is testing +;; something *outside* a network container. +;; +;; When this parameter's value is not false, it is a procedure +;; accepting the port number the server was (implicitly) bound +;; to (by the kernel). +(define when-bound (make-parameter #f)) + (define (getaddrinfo* host) "Like 'getaddrinfo', but properly report errors." (catch 'getaddrinfo-error @@ -1125,7 +1141,7 @@ methods, return the applicable compression." %default-options)) (advertise? (assoc-ref opts 'advertise?)) (user (assoc-ref opts 'user)) - (port (assoc-ref opts 'port)) + (requested-port (assoc-ref opts 'port)) (ttl (assoc-ref opts 'narinfo-ttl)) (compressions (match (filter-map (match-lambda (('compression . compression) @@ -1139,8 +1155,13 @@ methods, return the applicable compression." (address (let ((addr (assoc-ref opts 'address))) (make-socket-address (sockaddr:fam addr) (sockaddr:addr addr) - port))) + requested-port))) (socket (open-server-socket address)) + ;; If requested-port = 0, then the kernel + ;; will automatically assign a free port number. + (port (if (= 0 requested-port) + (sockaddr:port (getsockname socket)) + requested-port)) (nar-path (assoc-ref opts 'nar-path)) (repl-port (assoc-ref opts 'repl)) (cache (assoc-ref opts 'cache)) @@ -1151,6 +1172,10 @@ methods, return the applicable compression." (public-key (read-file-sexp (assoc-ref opts 'public-key-file))) (private-key (read-file-sexp (assoc-ref opts 'private-key-file)))) + ;; Inform tests/publish.scm about the port number used. + (let ((proc (when-bound))) + (when proc (proc port))) + (when user ;; Now that we've read the key material and opened the socket, we can ;; drop privileges. @@ -1167,8 +1192,7 @@ consider using the '--user' option!~%"))) (cache-bypass-threshold)))) (info (G_ "publishing ~a on ~a, port ~d~%") %store-directory - (inet-ntop (sockaddr:fam address) (sockaddr:addr address)) - (sockaddr:port address)) + (inet-ntop (sockaddr:fam address) (sockaddr:addr address)) port) (for-each (lambda (compression) (info (G_ "using '~a' compression method, level ~a~%") diff --git a/tests/publish.scm b/tests/publish.scm index 0a132dfe04..00509c7e82 100644 --- a/tests/publish.scm +++ b/tests/publish.scm @@ -46,6 +46,7 @@ #:use-module (rnrs bytevectors) #:use-module (ice-9 binary-ports) #:use-module (srfi srfi-1) + #:use-module (srfi srfi-8) #:use-module (srfi srfi-26) #:use-module (srfi srfi-64) #:use-module (ice-9 threads) @@ -80,9 +81,6 @@ ;; (PORT might be a custom binary input port). port)))) -(define (publish-uri route) - (string-append "http://localhost:6789" route)) - (define-syntax-rule (with-separate-output-ports exp ...) ;; Since ports aren't thread-safe in Guile 2.0, duplicate the output and ;; error ports to make sure the two threads don't end up stepping on each @@ -94,29 +92,41 @@ exp ...))))) ;; Run local publishing servers in a separate thread. -;; Attempt to avoid port collision by choosing ports -;; unlikely to be used in the wild (6789 and higher) -(define (spawn-guix-publish port . extra-arguments) +(define (spawn-guix-publish . extra-arguments) "Run a local publishing server in a separate thread. -The server will listen at PORT. EXTRA-ARGUMENTS are -passed as-is as extra command-line arguments. -The resulting thread is returned." - (with-separate-output-ports - (call-with-new-thread - (lambda () - (apply guix-publish (format #f "--port=~a" port) extra-arguments))))) - -;; Keep track of port numbers, to avoid multiple -;; servers listening at the same port. -(define *latest-port* 6789) +The port number will automatically be assigned. +EXTRA-ARGUMENTS are passed as-is as extra command-line +arguments. The resulting port number and thread is returned." + (let ((*port* #f) ; protected by port-mutex + (port-mutex (make-mutex)) + ;; Condition variable for signalling / checking whether + ;; *port* is set / can be read. + (port-bound (make-condition-variable))) + (define (when-bound-proc port) + (with-mutex port-mutex (set! *port* port)) + (signal-condition-variable port-bound)) + (let ((thread + (parameterize ((when-bound when-bound-proc)) + (with-separate-output-ports + (call-with-new-thread + (lambda () + ;; --port=0: automatically assign a port + (apply guix-publish "--port=0" extra-arguments))))))) + ;; A loop is required in case of spurious wakeups. + (with-mutex port-mutex + (let loop () + (if *port* + (values *port* thread) + (begin + (wait-condition-variable port-bound port-mutex) + (loop)))))))) (define (call-with-guix-publish extra-arguments proc) "Call PROC in an environment where a local publishing service is running in a separate thread, passing the port listened at. EXTRA-ARGUMENTS are passed as-is as extra command-line arguments." - (let* ((port (1+ *latest-port*)) - (thread (apply spawn-guix-publish port extra-arguments))) - (set! *latest-port* port) + (receive (port thread) + (apply spawn-guix-publish extra-arguments) (wait-until-ready port) (proc port))) @@ -124,7 +134,10 @@ EXTRA-ARGUMENTS are passed as-is as extra command-line arguments." (call-with-guix-publish extra-arguments (lambda (port) exp ...))) -(spawn-guix-publish 6789 "-C0") +(define first-server (spawn-guix-publish "-C0")) + +(define (publish-uri route) + (format #f "http://localhost:~a~a" first-server route)) (define (wait-until-ready port) ;; Wait until the server is accepting connections. @@ -150,9 +163,6 @@ EXTRA-ARGUMENTS are passed as-is as extra command-line arguments." ;; Magic bytes of gzip file. #vu8(#x1f #x8b)) -;; Wait until the two servers are ready. -(wait-until-ready 6789) - ;; Initialize the public/private key SRFI-39 parameters. (%public-key (read-file-sexp %public-key-file)) (%private-key (read-file-sexp %private-key-file)) -- 2.30.0