unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#46641: process-tests assume network connection
@ 2021-02-19 17:59 Glenn Morris
  2021-02-21 11:11 ` Robert Pluim
  0 siblings, 1 reply; 11+ messages in thread
From: Glenn Morris @ 2021-02-19 17:59 UTC (permalink / raw)
  To: 46641

Package: emacs
Version: 27.1

Some process-tests fail if the system has no network connection.
I don't know what the appropriate skip-unless condition to test for
network access is.

Ref: https://bugs.debian.org/982969





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#46641: process-tests assume network connection
  2021-02-19 17:59 bug#46641: process-tests assume network connection Glenn Morris
@ 2021-02-21 11:11 ` Robert Pluim
  2021-02-21 13:41   ` Lars Ingebrigtsen
  2021-02-21 14:37   ` Philipp
  0 siblings, 2 replies; 11+ messages in thread
From: Robert Pluim @ 2021-02-21 11:11 UTC (permalink / raw)
  To: Glenn Morris; +Cc: 46641

>>>>> On Fri, 19 Feb 2021 12:59:13 -0500, Glenn Morris <rgm@gnu.org> said:

    Glenn> Package: emacs
    Glenn> Version: 27.1

    Glenn> Some process-tests fail if the system has no network connection.
    Glenn> I don't know what the appropriate skip-unless condition to test for
    Glenn> network access is.

    Glenn> Ref: https://bugs.debian.org/982969

So Debian deliberately cripple their test environment, run the network
tests for an editor which can do network access, and we have to adapt
our tests? I am not amused.

I guess we could wrap them all in

(skip-unless (dns-query "google.com"))

instead of the getenv "EMACS_HYDRA_CI" stuff, assuming Debian donʼt
complain about that as well.

Robert

PS And since when did 'does not pass all tests' become 'fails to build
from source'?





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#46641: process-tests assume network connection
  2021-02-21 11:11 ` Robert Pluim
@ 2021-02-21 13:41   ` Lars Ingebrigtsen
  2021-02-21 14:40     ` Philipp
  2021-02-21 16:19     ` Robert Pluim
  2021-02-21 14:37   ` Philipp
  1 sibling, 2 replies; 11+ messages in thread
From: Lars Ingebrigtsen @ 2021-02-21 13:41 UTC (permalink / raw)
  To: Robert Pluim; +Cc: Glenn Morris, 46641

Robert Pluim <rpluim@gmail.com> writes:

>     Glenn> Some process-tests fail if the system has no network connection.
>     Glenn> I don't know what the appropriate skip-unless condition to test for
>     Glenn> network access is.
>
>     Glenn> Ref: https://bugs.debian.org/982969
>
> So Debian deliberately cripple their test environment, run the network
> tests for an editor which can do network access, and we have to adapt
> our tests? I am not amused.
>
> I guess we could wrap them all in
>
> (skip-unless (dns-query "google.com"))

It'd be nice if Emacs did have a predicate for "is there any network
here?"  But I don't know what that would look like.  That is, there's a
difference between having a local network (i.e., 127.0.0.1), and being
on the Internet.  

But if any of our tests require Emacs to be on a functioning internet
connection, they should indeed be guarded by something like the
`skip-unless' you propose.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#46641: process-tests assume network connection
  2021-02-21 11:11 ` Robert Pluim
  2021-02-21 13:41   ` Lars Ingebrigtsen
@ 2021-02-21 14:37   ` Philipp
  2021-02-21 16:21     ` Robert Pluim
  1 sibling, 1 reply; 11+ messages in thread
From: Philipp @ 2021-02-21 14:37 UTC (permalink / raw)
  To: Robert Pluim; +Cc: Glenn Morris, 46641



> Am 21.02.2021 um 12:11 schrieb Robert Pluim <rpluim@gmail.com>:
> 
>>>>>> On Fri, 19 Feb 2021 12:59:13 -0500, Glenn Morris <rgm@gnu.org> said:
> 
>    Glenn> Package: emacs
>    Glenn> Version: 27.1
> 
>    Glenn> Some process-tests fail if the system has no network connection.
>    Glenn> I don't know what the appropriate skip-unless condition to test for
>    Glenn> network access is.
> 
>    Glenn> Ref: https://bugs.debian.org/982969
> 
> So Debian deliberately cripple their test environment, run the network
> tests for an editor which can do network access, and we have to adapt
> our tests? I am not amused.

This is pretty common for CI systems.  Accessing the network is a security risk, and in addition tends to make tests unreproducible.




^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#46641: process-tests assume network connection
  2021-02-21 13:41   ` Lars Ingebrigtsen
@ 2021-02-21 14:40     ` Philipp
  2021-02-21 16:19     ` Robert Pluim
  1 sibling, 0 replies; 11+ messages in thread
From: Philipp @ 2021-02-21 14:40 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: Glenn Morris, Robert Pluim, 46641



> Am 21.02.2021 um 14:41 schrieb Lars Ingebrigtsen <larsi@gnus.org>:
> 
> Robert Pluim <rpluim@gmail.com> writes:
> 
>>    Glenn> Some process-tests fail if the system has no network connection.
>>    Glenn> I don't know what the appropriate skip-unless condition to test for
>>    Glenn> network access is.
>> 
>>    Glenn> Ref: https://bugs.debian.org/982969
>> 
>> So Debian deliberately cripple their test environment, run the network
>> tests for an editor which can do network access, and we have to adapt
>> our tests? I am not amused.
>> 
>> I guess we could wrap them all in
>> 
>> (skip-unless (dns-query "google.com"))
> 
> It'd be nice if Emacs did have a predicate for "is there any network
> here?"

This isn’t a yes/no question.  For example, it’s often a good idea to put tests into a network namespace that only has a loopback interface.  In that case, there’s some network (the loopback interface), but that still doesn’t allow Internet access.






^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#46641: process-tests assume network connection
  2021-02-21 13:41   ` Lars Ingebrigtsen
  2021-02-21 14:40     ` Philipp
@ 2021-02-21 16:19     ` Robert Pluim
  2021-02-21 16:45       ` Robert Pluim
  1 sibling, 1 reply; 11+ messages in thread
From: Robert Pluim @ 2021-02-21 16:19 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: Glenn Morris, 46641

>>>>> On Sun, 21 Feb 2021 14:41:11 +0100, Lars Ingebrigtsen <larsi@gnus.org> said:

    >> I guess we could wrap them all in
    >> 
    >> (skip-unless (dns-query "google.com"))

    Lars> It'd be nice if Emacs did have a predicate for "is there any network
    Lars> here?"  But I don't know what that would look like.  That is, there's a
    Lars> difference between having a local network (i.e., 127.0.0.1), and being
    Lars> on the Internet.  

    Lars> But if any of our tests require Emacs to be on a functioning internet
    Lars> connection, they should indeed be guarded by something like the
    Lars> `skip-unless' you propose.

Well, the tests in question are asking 'does emacs have a correctly
functioning internet connection', so making that a prerequisite for
the test seems kind of redundant, but we can do it. But first:
disabling my network connection causes dns-query to hang, so something
like this is needed, I think (we can skip the first hunk if you want):

diff --git a/lisp/net/dns.el b/lisp/net/dns.el
index 2045d4dfca..598ceebab8 100644
--- a/lisp/net/dns.el
+++ b/lisp/net/dns.el
@@ -332,7 +332,7 @@ dns-set-servers
 	  (setq dns-servers (nreverse dns-servers))))
       (when (executable-find "nslookup")
 	(with-temp-buffer
-	  (call-process "nslookup" nil t nil "localhost")
+	  (call-process "nslookup" nil t nil "-retry=0" "-timeout=2" "localhost")
 	  (goto-char (point-min))
           (when (re-search-forward
 	   "^Address:[ \t]*\\([0-9]+\\.[0-9]+\\.[0-9]+\\.[0-9]+\\|[[:xdigit:]:]*\\)" nil t)
@@ -496,15 +496,17 @@ dns-query
   "Query a DNS server for NAME of TYPE.
 If FULL, return the entire record returned.
 If REVERSE, look up an IP address."
-  (let ((result nil))
-    (dns-query-asynchronous
-     name
-     (lambda (response)
-       (setq result (list response)))
-     type full reverse)
-    ;; Loop until we get the callback.
-    (while (not result)
-      (sleep-for 0.01))
+  (let ((result nil)
+        (query-started
+         (dns-query-asynchronous
+          name
+          (lambda (response)
+            (setq result (list response)))
+          type full reverse)))
+    (if query-started
+        ;; Loop until we get the callback.
+        (while (not result)
+          (sleep-for 0.01)))
     (car result)))
 
 (provide 'dns)





^ permalink raw reply related	[flat|nested] 11+ messages in thread

* bug#46641: process-tests assume network connection
  2021-02-21 14:37   ` Philipp
@ 2021-02-21 16:21     ` Robert Pluim
  2021-02-21 19:40       ` Philipp
  0 siblings, 1 reply; 11+ messages in thread
From: Robert Pluim @ 2021-02-21 16:21 UTC (permalink / raw)
  To: Philipp; +Cc: Glenn Morris, 46641

>>>>> On Sun, 21 Feb 2021 15:37:27 +0100, Philipp <p.stephani2@gmail.com> said:

    Philipp> This is pretty common for CI systems.  Accessing the network is a
    Philipp> security risk, and in addition tends to make tests unreproducible.

I can give you the second one, but in what way is eg doing a DNS lookup a
'security risk'? Weʼre not talking about setting up a listening server
on a public IP here.

Robert





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#46641: process-tests assume network connection
  2021-02-21 16:19     ` Robert Pluim
@ 2021-02-21 16:45       ` Robert Pluim
  2021-02-21 18:32         ` Lars Ingebrigtsen
  0 siblings, 1 reply; 11+ messages in thread
From: Robert Pluim @ 2021-02-21 16:45 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: Glenn Morris, 46641

>>>>> On Sun, 21 Feb 2021 17:19:12 +0100, Robert Pluim <rpluim@gmail.com> said:

    Robert> Well, the tests in question are asking 'does emacs have a correctly
    Robert> functioning internet connection', so making that a prerequisite for
    Robert> the test seems kind of redundant, but we can do it. But first:
    Robert> disabling my network connection causes dns-query to hang, so something
    Robert> like this is needed, I think (we can skip the first hunk if you want):

Ah, the wonders of running diff when you haven't tested the result
from a clean emacs. This one actually works.

diff --git a/lisp/net/dns.el b/lisp/net/dns.el
index 2045d4dfca..3ae7469798 100644
--- a/lisp/net/dns.el
+++ b/lisp/net/dns.el
@@ -332,7 +332,7 @@ dns-set-servers
 	  (setq dns-servers (nreverse dns-servers))))
       (when (executable-find "nslookup")
 	(with-temp-buffer
-	  (call-process "nslookup" nil t nil "localhost")
+	  (call-process "nslookup" nil t nil "-retry=0" "-timeout=2" "localhost")
 	  (goto-char (point-min))
           (when (re-search-forward
 	   "^Address:[ \t]*\\([0-9]+\\.[0-9]+\\.[0-9]+\\.[0-9]+\\|[[:xdigit:]:]*\\)" nil t)
@@ -496,15 +496,17 @@ dns-query
   "Query a DNS server for NAME of TYPE.
 If FULL, return the entire record returned.
 If REVERSE, look up an IP address."
-  (let ((result nil))
-    (dns-query-asynchronous
-     name
-     (lambda (response)
-       (setq result (list response)))
-     type full reverse)
-    ;; Loop until we get the callback.
-    (while (not result)
-      (sleep-for 0.01))
+  (let* ((result nil)
+         (query-started
+          (dns-query-asynchronous
+           name
+           (lambda (response)
+             (setq result (list response)))
+           type full reverse)))
+    (if query-started
+        ;; Loop until we get the callback.
+        (while (not result)
+          (sleep-for 0.01)))
     (car result)))
 
 (provide 'dns)





^ permalink raw reply related	[flat|nested] 11+ messages in thread

* bug#46641: process-tests assume network connection
  2021-02-21 16:45       ` Robert Pluim
@ 2021-02-21 18:32         ` Lars Ingebrigtsen
  2021-02-22 14:49           ` Robert Pluim
  0 siblings, 1 reply; 11+ messages in thread
From: Lars Ingebrigtsen @ 2021-02-21 18:32 UTC (permalink / raw)
  To: Robert Pluim; +Cc: Glenn Morris, 46641

Robert Pluim <rpluim@gmail.com> writes:

> Ah, the wonders of running diff when you haven't tested the result
> from a clean emacs. This one actually works.

Haven't tested the patch, but it makes sense to me.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#46641: process-tests assume network connection
  2021-02-21 16:21     ` Robert Pluim
@ 2021-02-21 19:40       ` Philipp
  0 siblings, 0 replies; 11+ messages in thread
From: Philipp @ 2021-02-21 19:40 UTC (permalink / raw)
  To: Robert Pluim; +Cc: Glenn Morris, 46641



> Am 21.02.2021 um 17:21 schrieb Robert Pluim <rpluim@gmail.com>:
> 
>>>>>> On Sun, 21 Feb 2021 15:37:27 +0100, Philipp <p.stephani2@gmail.com> said:
> 
>    Philipp> This is pretty common for CI systems.  Accessing the network is a
>    Philipp> security risk, and in addition tends to make tests unreproducible.
> 
> I can give you the second one, but in what way is eg doing a DNS lookup a
> 'security risk'? Weʼre not talking about setting up a listening server
> on a public IP here.

A CI system will typically run arbitrary code that’s not under the control of the CI system itself.  Therefore, the CI system needs to prevent any malicious behavior of the system under test.  Since the code being tested is opaque, the CI system can’t really decide whether it’s malicious or not, so it has to conservatively assume that any network access is malicious.  While it might be possible to prevent more specific behavior (like creating a listening socket), that tends to be more complex, so the simpler and safer „no network at all“ tends to be a reasonable choice.




^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#46641: process-tests assume network connection
  2021-02-21 18:32         ` Lars Ingebrigtsen
@ 2021-02-22 14:49           ` Robert Pluim
  0 siblings, 0 replies; 11+ messages in thread
From: Robert Pluim @ 2021-02-22 14:49 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: Glenn Morris, 46641

tags 46641 fixed 
close 46641 28.1
quit

>>>>> On Sun, 21 Feb 2021 19:32:25 +0100, Lars Ingebrigtsen <larsi@gnus.org> said:

    Lars> Robert Pluim <rpluim@gmail.com> writes:
    >> Ah, the wonders of running diff when you haven't tested the result
    >> from a clean emacs. This one actually works.

    Lars> Haven't tested the patch, but it makes sense to me.

Pushed as 934dcc2157

And the requisite changes to the test suite I pushed as a728135a2b

Robert





^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2021-02-22 14:49 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-19 17:59 bug#46641: process-tests assume network connection Glenn Morris
2021-02-21 11:11 ` Robert Pluim
2021-02-21 13:41   ` Lars Ingebrigtsen
2021-02-21 14:40     ` Philipp
2021-02-21 16:19     ` Robert Pluim
2021-02-21 16:45       ` Robert Pluim
2021-02-21 18:32         ` Lars Ingebrigtsen
2021-02-22 14:49           ` Robert Pluim
2021-02-21 14:37   ` Philipp
2021-02-21 16:21     ` Robert Pluim
2021-02-21 19:40       ` Philipp

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).