Support GUIX_DISABLE_NETWORK_TESTS environment variable

Vagrant Cascadian wrote on 6 Nov 2020 22:30

Recipients:(address . bug-guix@gnu.org)

Message-ID:87k0uy6um8.fsf@yucca

The following patch adds a GUIX_DISABLE_NETWORK_TESTS environmentvariable and disables tests that require network access when it is set.
This is needed for packaging GNU Guix in Debian, where packaging policiesprohibit network access during builds, but may not technically block networkaccess during builds.
If this could be considered for the upcoming 1.2 release, that would beappreciated, though I can also carry the patches in Debian...

live well, vagrant

From 36516e767f68dbc2bd3dc186f956c0b0fd7de9f1 Mon Sep 17 00:00:00 2001From: Vagrant Cascadian <vagrant@debian.org>Date: Thu, 5 Nov 2020 17:35:52 -0800Subject: [PATCH] tests: Support disabling network tests.
This is needed for packaging GNU Guix in Debian, where packaging policiesprohibit network access during builds, but may not technically block networkaccess during builds.
* guix/tests.scm (network-reachable): Return #f when  GUIX_DISABLE_NETWORK_TESTS is set.* tests/common.sh: New file.* tests/guix-build-branch.sh, tests/guix-environment.sh,  tests/guix-pack.sh, tests/guix-package-net.sh: Use  network_reachable function from common.sh.--- guix/tests.scm             | 7 +++++-- tests/common.sh            | 8 ++++++++ tests/guix-build-branch.sh | 8 ++------ tests/guix-environment.sh  | 5 ++--- tests/guix-pack.sh         | 5 ++--- tests/guix-package-net.sh  | 9 ++------- 6 files changed, 21 insertions(+), 21 deletions(-) create mode 100644 tests/common.sh

Toggle diff (110 lines)diff --git a/guix/tests.scm b/guix/tests.scmindex fc3d521163..9f98cef33f 100644--- a/guix/tests.scm+++ b/guix/tests.scm@@ -204,8 +204,11 @@ too expensive to build entirely in the test store."              (zero? (logand #o222 (stat:mode st)))))))  (define (network-reachable?)-  "Return true if we can reach the Internet."-  (false-if-exception (getaddrinfo "www.gnu.org" "80" AI_NUMERICSERV)))+  "Return true if we can reach the Internet and GUIX_DISABLE_NETWORK_TESTS+is not set."+  (if (getenv "GUIX_DISABLE_NETWORK_TESTS")+      #f+      (false-if-exception (getaddrinfo "www.gnu.org" "80" AI_NUMERICSERV))))  (define-syntax-rule (mock (module proc replacement) body ...)   "Within BODY, replace the definition of PROC from MODULE with the definitiondiff --git a/tests/common.sh b/tests/common.shnew file mode 100644index 0000000000..b91c0bdcd4--- /dev/null+++ b/tests/common.sh@@ -0,0 +1,8 @@+network_reachable() {+    if [ -n "$GUIX_DISABLE_NETWORK_TESTS" ]; then+		exit 77+    fi+    if ! guile -c '(getaddrinfo "www.gnu.org" "80" AI_NUMERICSERV)' 2> /dev/null; then+        exit 77+    fi+}diff --git a/tests/guix-build-branch.sh b/tests/guix-build-branch.shindex 79aa06a58f..55f8f388ab 100644--- a/tests/guix-build-branch.sh+++ b/tests/guix-build-branch.sh@@ -24,12 +24,8 @@ guix build --version  # 'guix build --with-branch' requires access to the network to clone the # Git repository below.--if ! guile -c '(getaddrinfo "www.gnu.org" "80" AI_NUMERICSERV)' 2> /dev/null-then-    # Skipping.-    exit 77-fi+. $(dirname $0)/common.sh+network_reachable  orig_drv="`guix build guile-gcrypt -d`" latest_drv="`guix build guile-gcrypt --with-branch=guile-gcrypt=master -d`"diff --git a/tests/guix-environment.sh b/tests/guix-environment.shindex f8be48f0c0..d140566aef 100644--- a/tests/guix-environment.sh+++ b/tests/guix-environment.sh@@ -174,9 +174,9 @@ case "$transformed_drv" in     *)           false;; esac +. $(dirname $0)/common.sh+network_reachable -if guile -c '(getaddrinfo "www.gnu.org" "80" AI_NUMERICSERV)' 2> /dev/null-then     # Compute the build environment for the initial GNU Make.     guix environment --bootstrap --no-substitutes --search-paths --pure \          -e '(@ (guix tests) gnu-make-for-tests)' > "$tmpdir/a"@@ -244,4 +244,3 @@ then     do 	guix gc --references "$profile" | grep "$dep"     done-fidiff --git a/tests/guix-pack.sh b/tests/guix-pack.shindex 0339221ac2..bc902c7e90 100644--- a/tests/guix-pack.sh+++ b/tests/guix-pack.sh@@ -23,9 +23,8 @@  # A network connection is required to build %bootstrap-coreutils&co, # which is required to run these tests with the --bootstrap option.-if ! guile -c '(getaddrinfo "www.gnu.org" "80" AI_NUMERICSERV)' 2> /dev/null; then-    exit 77-fi+. $(dirname $0)/common.sh+network_reachable  guix pack --version diff --git a/tests/guix-package-net.sh b/tests/guix-package-net.shindex 6d21c6cff6..ec7952f63d 100644--- a/tests/guix-package-net.sh+++ b/tests/guix-package-net.sh@@ -38,13 +38,8 @@ shebang_too_long () 	 -ge 128 } -if ! guile -c '(getaddrinfo "www.gnu.org" "80" AI_NUMERICSERV)' 2> /dev/null \-	|| shebang_too_long-then-    # Skipping.-    exit 77-fi-+. $(dirname $0)/common.sh+network_reachable  profile="t-profile-$$" profile_alt="t-profile-alt-$$"-- 2.20.1

Ludovic Courtès wrote on 8 Nov 2020 18:46

Recipients:(name . Vagrant Cascadian)(address . vagrant@debian.org)(address . 44491@debbugs.gnu.org)

Message-ID:87y2jbu4gx.fsf@gnu.org

Hi Vagrant,
Vagrant Cascadian <vagrant@debian.org> skribis:

Toggle quote (3 lines)

> If this could be considered for the upcoming 1.2 release, that would be> appreciated, though I can also carry the patches in Debian...

Yay! It should be doable, let’s see.

Toggle quote (16 lines)> From 36516e767f68dbc2bd3dc186f956c0b0fd7de9f1 Mon Sep 17 00:00:00 2001> From: Vagrant Cascadian <vagrant@debian.org>> Date: Thu, 5 Nov 2020 17:35:52 -0800> Subject: [PATCH] tests: Support disabling network tests.>> This is needed for packaging GNU Guix in Debian, where packaging policies> prohibit network access during builds, but may not technically block network> access during builds.>> * guix/tests.scm (network-reachable): Return #f when>   GUIX_DISABLE_NETWORK_TESTS is set.> * tests/common.sh: New file.> * tests/guix-build-branch.sh, tests/guix-environment.sh,>   tests/guix-pack.sh, tests/guix-package-net.sh: Use>   network_reachable function from common.sh.

[...]

Toggle quote (12 lines)> --- /dev/null> +++ b/tests/common.sh> @@ -0,0 +1,8 @@> +network_reachable() {> +    if [ -n "$GUIX_DISABLE_NETWORK_TESTS" ]; then> +		exit 77> +    fi> +    if ! guile -c '(getaddrinfo "www.gnu.org" "80" AI_NUMERICSERV)' 2> /dev/null; then> +        exit 77> +    fi> +}

Could you add the usual copyright/license header? Also please add thisfile to ‘EXTRA_DIST’ in Makefile.am.
Looking at the tests you modified, we need two things:
• a ‘network_reachable’ function that returns true or false, without exiting;
• a ‘skip_if_network_is_unreachable’ function that does “exit 77” when network is “unreachable”.

Toggle quote (14 lines)> --- a/tests/guix-environment.sh> +++ b/tests/guix-environment.sh> @@ -174,9 +174,9 @@ case "$transformed_drv" in>      *)           false;;>  esac>  > +. $(dirname $0)/common.sh> +network_reachable>  > -if guile -c '(getaddrinfo "www.gnu.org" "80" AI_NUMERICSERV)' 2> /dev/null> -then>      # Compute the build environment for the initial GNU Make.>      guix environment --bootstrap --no-substitutes --search-paths --pure \

I think this is the only place where you’d write “if network_reachable”instead of “skip_if_network_is_unreachable”.
WDYT?
Thanks!
Ludo’.

Vagrant Cascadian wrote on 10 Nov 2020 19:13

Recipients:(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 44491@debbugs.gnu.org)

Message-ID:87k0utt6zt.fsf@yucca

On 2020-11-08, Ludovic Courtès wrote:

Toggle quote (7 lines)

> Vagrant Cascadian <vagrant@debian.org> skribis:>>> If this could be considered for the upcoming 1.2 release, that would be>> appreciated, though I can also carry the patches in Debian...>> Yay! It should be doable, let’s see.

It seems like a simpler workaround is to pass RES_OPTIONS=attempts:0,which should disable name resolution, and thus the network checks willfail.
With the RES_OPTIONS workaround, the changes to guix/tests.scmnetwork-reachable are no longer needed ... i think. :)
Might still be worth refactoring some of *.sh tests to use commonfunctions, since the code is basically copied and pasted in severaldifferent places.
I suspect there are some additional tests that should check for network,but that is probably a separate bug once they have been identified.

live well, vagrant

Ludovic Courtès wrote on 10 Nov 2020 22:26

Recipients:(name . Vagrant Cascadian)(address . vagrant@debian.org)(address . 44491@debbugs.gnu.org)

Message-ID:87wnysnbs6.fsf@gnu.org

Vagrant Cascadian <vagrant@debian.org> skribis:

Toggle quote (15 lines)> On 2020-11-08, Ludovic Courtès wrote:>> Vagrant Cascadian <vagrant@debian.org> skribis:>>>>> If this could be considered for the upcoming 1.2 release, that would be>>> appreciated, though I can also carry the patches in Debian...>>>> Yay!  It should be doable, let’s see.>> It seems like a simpler workaround is to pass RES_OPTIONS=attempts:0,> which should disable name resolution, and thus the network checks will> fail.>> With the RES_OPTIONS workaround, the changes to guix/tests.scm> network-reachable are no longer needed ... i think. :)

Oooh nice, the wonders of glibc!

Toggle quote (4 lines)

> Might still be worth refactoring some of *.sh tests to use common> functions, since the code is basically copied and pasted in several> different places.

Yes, that’s still a good idea. Would you like to adjust your patchaccordingly?

Toggle quote (3 lines)

> I suspect there are some additional tests that should check for network,> but that is probably a separate bug once they have been identified.

OK!
Thanks,Ludo’.

Vagrant Cascadian wrote on 11 Nov 2020 04:39

Recipients:(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 44491@debbugs.gnu.org)

Message-ID:87h7pwtvde.fsf@yucca

On 2020-11-10, Ludovic Courtès wrote:

Toggle quote (25 lines)> Vagrant Cascadian <vagrant@debian.org> skribis:>> On 2020-11-08, Ludovic Courtès wrote:>>> Vagrant Cascadian <vagrant@debian.org> skribis:>>>>>>> If this could be considered for the upcoming 1.2 release, that would be>>>> appreciated, though I can also carry the patches in Debian...>>>>>> Yay!  It should be doable, let’s see.>>>> It seems like a simpler workaround is to pass RES_OPTIONS=attempts:0,>> which should disable name resolution, and thus the network checks will>> fail.>>>> With the RES_OPTIONS workaround, the changes to guix/tests.scm>> network-reachable are no longer needed ... i think. :)>> Oooh nice, the wonders of glibc!>>> Might still be worth refactoring some of *.sh tests to use common>> functions, since the code is basically copied and pasted in several>> different places.>> Yes, that’s still a good idea.  Would you like to adjust your patch> accordingly?

Thanks for the review!
Updated patch attached, with changes:
* Copyright header added to common.sh.* New function skip_if_network_unreachable in common.sh* Dropped GUIX_DISABLE_NETWORK_TESTS in favor of using RES_OPTIONS=attempts:0.* Updated tests to use skip_if_network_unreachable or network_reachable.

live well, vagrant

From 3fd93fec957491450639c647e05a5f72f1e787fd Mon Sep 17 00:00:00 2001From: Vagrant Cascadian <vagrant@debian.org>Date: Tue, 10 Nov 2020 19:26:04 -0800Subject: [PATCH] tests: Add common functions for to check for network reachability.
* tests/common.sh: New file.* tests/guix-build-branch.sh, tests/guix-pack.sh,  tests/guix-package-net.sh: Use skip_if_network_unreachable function  from common.sh.* tests/guix-environment.sh: Use network_reachable function from  common.sh.--- Makefile.am                |  1 + tests/common.sh            | 30 ++++++++++++++++++++++++++++++ tests/guix-build-branch.sh |  8 ++------ tests/guix-environment.sh  |  5 ++--- tests/guix-pack.sh         |  5 ++--- tests/guix-package-net.sh  |  9 ++------- 6 files changed, 39 insertions(+), 19 deletions(-) create mode 100644 tests/common.sh

Toggle diff (121 lines)diff --git a/Makefile.am b/Makefile.amindex e7053ee4f4..7dbe41201c 100644--- a/Makefile.am+++ b/Makefile.am@@ -599,6 +599,7 @@ EXTRA_DIST +=						\   tests/test.drv					\   tests/signing-key.pub					\   tests/signing-key.sec					\+  tests/common.sh					\   tests/cve-sample.json					\   tests/civodul.key					\   tests/rsa.key						\diff --git a/tests/common.sh b/tests/common.shnew file mode 100644index 0000000000..f9dd3c2c59--- /dev/null+++ b/tests/common.sh@@ -0,0 +1,30 @@+# GNU Guix --- Functional package management for GNU+# Copyright © 2018, 2019, 2020 Ludovic Courtès <ludo@gnu.org>+# Copyright © 2020 Vagrant Cascadian <vagrant@debian.org>+#+# This file is part of GNU Guix.+#+# GNU Guix is free software; you can redistribute it and/or modify it+# under the terms of the GNU General Public License as published by+# the Free Software Foundation; either version 3 of the License, or (at+# your option) any later version.+#+# GNU Guix is distributed in the hope that it will be useful, but+# WITHOUT ANY WARRANTY; without even the implied warranty of+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the+# GNU General Public License for more details.+#+# You should have received a copy of the GNU General Public License+# along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.++network_reachable() {+    if ! guile -c '(getaddrinfo "www.gnu.org" "80" AI_NUMERICSERV)' 2> /dev/null; then+        return 0+    fi+}++skip_if_network_unreachable() {+	if ! network_reachable ; then+		exit 77+	fi+}diff --git a/tests/guix-build-branch.sh b/tests/guix-build-branch.shindex 79aa06a58f..89f7ea4a08 100644--- a/tests/guix-build-branch.sh+++ b/tests/guix-build-branch.sh@@ -24,12 +24,8 @@ guix build --version  # 'guix build --with-branch' requires access to the network to clone the # Git repository below.--if ! guile -c '(getaddrinfo "www.gnu.org" "80" AI_NUMERICSERV)' 2> /dev/null-then-    # Skipping.-    exit 77-fi+. $(dirname $0)/common.sh+skip_if_network_unreachable  orig_drv="`guix build guile-gcrypt -d`" latest_drv="`guix build guile-gcrypt --with-branch=guile-gcrypt=master -d`"diff --git a/tests/guix-environment.sh b/tests/guix-environment.shindex f8be48f0c0..ce01ac04be 100644--- a/tests/guix-environment.sh+++ b/tests/guix-environment.sh@@ -174,9 +174,8 @@ case "$transformed_drv" in     *)           false;; esac --if guile -c '(getaddrinfo "www.gnu.org" "80" AI_NUMERICSERV)' 2> /dev/null-then+. $(dirname $0)/common.sh+if network_reachable ; then     # Compute the build environment for the initial GNU Make.     guix environment --bootstrap --no-substitutes --search-paths --pure \          -e '(@ (guix tests) gnu-make-for-tests)' > "$tmpdir/a"diff --git a/tests/guix-pack.sh b/tests/guix-pack.shindex 0339221ac2..822a67b157 100644--- a/tests/guix-pack.sh+++ b/tests/guix-pack.sh@@ -23,9 +23,8 @@  # A network connection is required to build %bootstrap-coreutils&co, # which is required to run these tests with the --bootstrap option.-if ! guile -c '(getaddrinfo "www.gnu.org" "80" AI_NUMERICSERV)' 2> /dev/null; then-    exit 77-fi+. $(dirname $0)/common.sh+skip_if_network_unreachable  guix pack --version diff --git a/tests/guix-package-net.sh b/tests/guix-package-net.shindex 6d21c6cff6..f682d09245 100644--- a/tests/guix-package-net.sh+++ b/tests/guix-package-net.sh@@ -38,13 +38,8 @@ shebang_too_long () 	 -ge 128 } -if ! guile -c '(getaddrinfo "www.gnu.org" "80" AI_NUMERICSERV)' 2> /dev/null \-	|| shebang_too_long-then-    # Skipping.-    exit 77-fi-+. $(dirname $0)/common.sh+skip_if_network_unreachable  profile="t-profile-$$" profile_alt="t-profile-alt-$$"-- 2.20.1

Vagrant Cascadian wrote on 11 Nov 2020 07:06

Recipients:(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 44491@debbugs.gnu.org)

Message-ID:87eel0tok1.fsf@yucca

On 2020-11-10, Vagrant Cascadian wrote:

Toggle quote (36 lines)> On 2020-11-10, Ludovic Courtès wrote:>> Vagrant Cascadian <vagrant@debian.org> skribis:>>> On 2020-11-08, Ludovic Courtès wrote:>>>> Vagrant Cascadian <vagrant@debian.org> skribis:>>>>>>>>> If this could be considered for the upcoming 1.2 release, that would be>>>>> appreciated, though I can also carry the patches in Debian...>>>>>>>> Yay!  It should be doable, let’s see.>>>>>> It seems like a simpler workaround is to pass RES_OPTIONS=attempts:0,>>> which should disable name resolution, and thus the network checks will>>> fail.>>>>>> With the RES_OPTIONS workaround, the changes to guix/tests.scm>>> network-reachable are no longer needed ... i think. :)>>>> Oooh nice, the wonders of glibc!>>>>> Might still be worth refactoring some of *.sh tests to use common>>> functions, since the code is basically copied and pasted in several>>> different places.>>>> Yes, that’s still a good idea.  Would you like to adjust your patch>> accordingly?>> Thanks for the review!>> Updated patch attached, with changes:>> * Copyright header added to common.sh.> * New function skip_if_network_unreachable in common.sh> * Dropped GUIX_DISABLE_NETWORK_TESTS in favor of using>   RES_OPTIONS=attempts:0.> * Updated tests to use skip_if_network_unreachable or network_reachable.

...

Toggle quote (5 lines)> diff --git a/tests/common.sh b/tests/common.sh> new file mode 100644> index 0000000000..f9dd3c2c59> --- /dev/null> +++ b/tests/common.sh

...

Toggle quote (6 lines)

> +network_reachable() {> + if ! guile -c '(getaddrinfo "www.gnu.org" "80" AI_NUMERICSERV)' 2> /dev/null; then> + return 0> + fi> +}

Ooops. I inverted that check... probably "if guile -c ..." and probablyshould return 1 or something if it isn't... or maybe 77?

anyways... testing again.

live well, vagrant

Ludovic Courtès wrote on 3 Dec 2020 18:09

Recipients:(name . Vagrant Cascadian)(address . vagrant@debian.org)(address . 44491@debbugs.gnu.org)

Message-ID:87sg8mbyuz.fsf@gnu.org

Hi Vagrant,
Should we close this issue now that you found the RES_OPTIONS=attempts:0trick, or do you think we should still keep the refactoring bits?
Ludo’.

Vagrant Cascadian wrote on 3 Dec 2020 20:13

Recipients:(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 44491@debbugs.gnu.org)

Message-ID:87eek6n1of.fsf@yucca

On 2020-12-03, Ludovic Courtès wrote:

Toggle quote (3 lines)

> Should we close this issue now that you found the RES_OPTIONS=attempts:0> trick, or do you think we should still keep the refactoring bits?

Well, it's three cases of copy-paste code, and one nearly identical butinverted. Someone once suggested to me to refactor on the third instanceof copy-pasted code...
Having common tests makes it a little easier to add to new tests in thefuture with the same code, and if there's ever a change in theunderlying code, you fix it in once place. It also opens the door toadding other common functions, if it comes up.
So I'd say it's worth applying, though also would be ok with leaving asis and just taking advantage of the RES_OPTIONS=attempts:0 workaround.

live well, vagrant

Ludovic Courtès wrote on 7 Dec 2020 08:55

Recipients:(name . Vagrant Cascadian)(address . vagrant@debian.org)(address . 44491@debbugs.gnu.org)

Message-ID:87im9e2gpz.fsf@gnu.org

Hi!
Vagrant Cascadian <vagrant@debian.org> skribis:

Toggle quote (16 lines)> On 2020-12-03, Ludovic Courtès wrote:>> Should we close this issue now that you found the RES_OPTIONS=attempts:0>> trick, or do you think we should still keep the refactoring bits?>> Well, it's three cases of copy-paste code, and one nearly identical but> inverted. Someone once suggested to me to refactor on the third instance> of copy-pasted code...>> Having common tests makes it a little easier to add to new tests in the> future with the same code, and if there's ever a change in the> underlying code, you fix it in once place. It also opens the door to> adding other common functions, if it comes up.>> So I'd say it's worth applying, though also would be ok with leaving as> is and just taking advantage of the RES_OPTIONS=attempts:0 workaround.

Makes sense to me. I’ll let you push it or let me know if you prefer meto do it.
Thanks!
Ludo’.

is:open	open issues
is:done	closed issues
submitter:<who>	search issue submitter
author:<who>	search by message author
date:yesterday..now	search by issue date
mdate:3m..2d	search by message date