Re: bug#57129: 29.0.50; Improve behavior of conditionals in Eshell

[Paul Eggert]

On 8/16/22 12:59, Bruno Haible wrote:

Since this contradicts what you debugged on mingw, I ran the test 1
times on native Windows. Result:
   - on 32-bit mingw, no failure.
   - on 64-bit mingw, around 30 failures (or around 10 failures with Paul's
 newest patch). That is, a probability of ca. 0.3% of getting the same
 file name as on the previous call.


That's odd, for two reasons. First, 64-bit and 32-bit mingw shouldn't 
differ, as they both use uint_fast64_t which should be the same width on 
both platforms. Second, I could not reproduce the problem on 64-bit 
Ubuntu x86-64 (after altering tempname.c to always define 
HAS_CLOCK_ENTROPY to false) test-tempname always succeeded in 1 tries.


Could you investigate further why mingw 64-bit fails?

Re: bug#57129: 29.0.50; Improve behavior of conditionals in Eshell

[Bruno Haible]

Paul Eggert wrote:
> Emacs 'configure' deliberately suppresses MinGW's clock_gettime.

Ah, this explains why Eli saw the problem in Emacs in a deterministic
manner, whereas I see it only with 0.3% probability in my tests on 64-bit
mingw.

> I installed 
> into Gnulib the attached patch, which I hope fixes the Emacs problem 
> without changing glibc's generated code (once this gets migrated back 
> into glibc).

In 1 runs on 64-bit mingw, your patch went from 27 test failures
to 11 test failures. So, we're still at ca. 0.1% probability (*), far
above the 62^-6 or 10^-10 probability that one would have hoped for.

(*) I wouldn't consider these measurements reliable. Maybe the probability
did not change, and what I'm seeing is merely a measurement fluke.

Bruno

Re: bug#57129: 29.0.50; Improve behavior of conditionals in Eshell

[Bruno Haible]

Eli Zaretskii wrote:
> Emacs has a with-temp-file macro, which generates a temporary file
> name, executes the body of the macro with a variable bound to the file
> name, then deletes the file.  Very handy for writing test suites.

Since, even with Paul's newest patch, gen_tempname on 64-bit mingw
produces the same file name as on the previous call with a probability
of about 0.1%, you still need to be prepared to see the original problem
occasionally.

Bruno

Re: bug#57129: 29.0.50; Improve behavior of conditionals in Eshell

[Bruno Haible]

Eli Zaretskii wrote:
> Looking at your test program, I see that you generate the seconds file
> name without deleting the first one.  When a file by the first
> generated name already exists, gen_tempname will indeed generate a
> different name.  But in the scenario I described, Emacs creates one
> temporary file, uses it, then deletes it, and only after that creates
> another file.

I'm adding this scenario to the unit test. Still, the unit test succeeds
when run once on:
  Linux, FreeBSD, NetBSD, OpenBSD, macOS, Solaris, Cygwin, native Windows.

Since this contradicts what you debugged on mingw, I ran the test 1
times on native Windows. Result:
  - on 32-bit mingw, no failure.
  - on 64-bit mingw, around 30 failures (or around 10 failures with Paul's
newest patch). That is, a probability of ca. 0.3% of getting the same
file name as on the previous call.

Bruno
>From aa52cadc36fb1af0509dc3a4bce4ce73197ece68 Mon Sep 17 00:00:00 2001
From: Bruno Haible 
Date: Tue, 16 Aug 2022 21:50:11 +0200
Subject: [PATCH] tempname: Add more tests.

Based on scenario described by Eli Zaretskii in
.

* tests/test-tempname.c (main): Add another test.
* modules/tempname-tests (Status): Mark the test as unportable.
---
 ChangeLog  |  8 ++
 modules/tempname-tests |  3 ++
 tests/test-tempname.c  | 65 ++
 3 files changed, 58 insertions(+), 18 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index eb96281591..de113ce081 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2022-08-16  Bruno Haible  
+
+	tempname: Add more tests.
+	Based on scenario described by Eli Zaretskii in
+	.
+	* tests/test-tempname.c (main): Add another test.
+	* modules/tempname-tests (Status): Mark the test as unportable.
+
 2022-08-16  Paul Eggert  
 
 	tempname: generate better names for MinGW Emacs
diff --git a/modules/tempname-tests b/modules/tempname-tests
index 384f98707b..adccd0d8e9 100644
--- a/modules/tempname-tests
+++ b/modules/tempname-tests
@@ -2,6 +2,9 @@ Files:
 tests/test-tempname.c
 tests/macros.h
 
+Status:
+unportable-test
+
 Depends-on:
 unlink
 
diff --git a/tests/test-tempname.c b/tests/test-tempname.c
index d463eec2b4..4a0ca65f2c 100644
--- a/tests/test-tempname.c
+++ b/tests/test-tempname.c
@@ -34,24 +34,53 @@ main ()
   char filename1[18 + 1];
   char filename2[18 + 1];
 
-  strcpy (filename1, templ18);
-  int fd1 = gen_tempname (filename1, strlen (".xyz"), 0, GT_FILE);
-  ASSERT (fd1 >= 0);
-
-  strcpy (filename2, templ18);
-  int fd2 = gen_tempname (filename2, strlen (".xyz"), 0, GT_FILE);
-  ASSERT (fd2 >= 0);
-
-  /* With 6 'X' and a good pseudo-random number generator behind the scenes,
- the probability of getting the same file name twice in a row should be
- 1/62^6 < 1/10^10.  */
-  ASSERT (strcmp (filename1, filename2) != 0);
-
-  /* Clean up.  */
-  close (fd1);
-  close (fd2);
-  unlink (filename1);
-  unlink (filename2);
+  /* Case 1: The first file still exists while gen_tempname is called a second
+ time.  */
+  {
+strcpy (filename1, templ18);
+int fd1 = gen_tempname (filename1, strlen (".xyz"), 0, GT_FILE);
+ASSERT (fd1 >= 0);
+
+strcpy (filename2, templ18);
+int fd2 = gen_tempname (filename2, strlen (".xyz"), 0, GT_FILE);
+ASSERT (fd2 >= 0);
+
+/* gen_tempname arranges (via O_EXCL) to not return the name of an existing
+   file.  */
+ASSERT (strcmp (filename1, filename2) != 0);
+
+/* Clean up.  */
+close (fd1);
+close (fd2);
+unlink (filename1);
+unlink (filename2);
+  }
+
+  /* Case 2: The first file is deleted before gen_tempname is called a second
+ time.  */
+  {
+strcpy (filename1, templ18);
+int fd1 = gen_tempname (filename1, strlen (".xyz"), 0, GT_FILE);
+ASSERT (fd1 >= 0);
+
+/* Clean up.  */
+close (fd1);
+unlink (filename1);
+
+strcpy (filename2, templ18);
+int fd2 = gen_tempname (filename2, strlen (".xyz"), 0, GT_FILE);
+ASSERT (fd2 >= 0);
+
+/* With 6 'X' and a good pseudo-random number generator behind the scenes,
+   the probability of getting the same file name twice in a row should be
+   1/62^6 < 1/10^10.
+   But on 64-bit native Windows, this probability is ca. 0.1% to 0.3%.  */
+ASSERT (strcmp (filename1, filename2) != 0);
+
+/* Clean up.  */
+close (fd2);
+unlink (filename2);
+  }
 
   return 0;
 }
-- 
2.34.1

Re: bug#57129: 29.0.50; Improve behavior of conditionals in Eshell

[Paul Eggert]

On 8/16/22 10:47, Eli Zaretskii wrote:


(Why are you talking about MS-DOS?)


I mistakenly thought it was an MS-DOS problem because tempname.c 
ordinarily would use clock_gettime on MinGW. I didn't know Emacs 
'configure' deliberately suppresses MinGW's clock_gettime.




Thanks, but why not use 'random' instead?  Emacs does have it on all
platforms, including MS-Windows.  AFAIU, it's better than 'rand'.


If the code used 'random' then the Gnulib 'tempname' module would need 
to add a dependency on the Gnulib 'random' module, which would in turn 
add a cascading dependency on Gnulib's 'random_r' module. It's better to 
avoid this dependency if we can easily do so.


Come to think of it, we don't need to use 'rand' either, since 
tempname.c already has a good-enough pseudorandom generator. I installed 
into Gnulib the attached patch, which I hope fixes the Emacs problem 
without changing glibc's generated code (once this gets migrated back 
into glibc).




If I understand things correctly this should make the names random
enough on MS-DOS, though Emacs itself still needs a patch as I mentioned
a few minutes ago.


Why would Emacs need that patch?


In another part of this thread you rejected that patch, on the grounds 
that fixing the unlikely Emacs bug is more trouble than it's worth. So 
I'll drop that suggestion.From 512e44adaebb3096ddd1bf564e679d06e0301616 Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Tue, 16 Aug 2022 12:06:48 -0700
Subject: [PATCH] tempname: generate better names for MinGW Emacs
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

On MinGW, GNU Emacs disables clock_gettime, which reliably breaks
some of gen_tempname’s optimistic callers.  Work around the
problem by making the generated names less predictable.  We don’t
need cryptographic randomness here, just enough unpredictability
to keep Emacs happy most of the time.
* lib/tempname.c (HAS_CLOCK_ENTROPY): New macro.
(random_bits): Use it.
(try_tempname_len): On systems lacking clock entropy, maintain
state so that gen_filename generates less-predictable names on
successive successful calls.
---
 ChangeLog  | 14 ++
 lib/tempname.c | 18 +-
 2 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/ChangeLog b/ChangeLog
index b639d1709d..eb96281591 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,17 @@
+2022-08-16  Paul Eggert  
+
+	tempname: generate better names for MinGW Emacs
+	On MinGW, GNU Emacs disables clock_gettime, which reliably breaks
+	some of gen_tempname’s optimistic callers.  Work around the
+	problem by making the generated names less predictable.  We don’t
+	need cryptographic randomness here, just enough unpredictability
+	to keep Emacs happy most of the time.
+	* lib/tempname.c (HAS_CLOCK_ENTROPY): New macro.
+	(random_bits): Use it.
+	(try_tempname_len): On systems lacking clock entropy, maintain
+	state so that gen_filename generates less-predictable names on
+	successive successful calls.
+
 2022-08-16  Simon Josefsson  
 
 	maintainer-makefile: Check for incorrect DISTCHECK_CONFIGURE_FLAGS
diff --git a/lib/tempname.c b/lib/tempname.c
index e6520191d7..5adfe629a8 100644
--- a/lib/tempname.c
+++ b/lib/tempname.c
@@ -77,6 +77,12 @@ typedef uint_fast64_t random_value;
 #define BASE_62_DIGITS 10 /* 62**10 < UINT_FAST64_MAX */
 #define BASE_62_POWER (62LL * 62 * 62 * 62 * 62 * 62 * 62 * 62 * 62 * 62)
 
+#if _LIBC || (defined CLOCK_MONOTONIC && HAVE_CLOCK_GETTIME)
+# define HAS_CLOCK_ENTROPY true
+#else
+# define HAS_CLOCK_ENTROPY false
+#endif
+
 static random_value
 random_bits (random_value var, bool use_getrandom)
 {
@@ -84,7 +90,7 @@ random_bits (random_value var, bool use_getrandom)
   /* Without GRND_NONBLOCK it can be blocked for minutes on some systems.  */
   if (use_getrandom && __getrandom (, sizeof r, GRND_NONBLOCK) == sizeof r)
 return r;
-#if _LIBC || (defined CLOCK_MONOTONIC && HAVE_CLOCK_GETTIME)
+#if HAS_CLOCK_ENTROPY
   /* Add entropy if getrandom did not work.  */
   struct __timespec64 tv;
   __clock_gettime64 (CLOCK_MONOTONIC, );
@@ -267,6 +273,13 @@ try_tempname_len (char *tmpl, int suffixlen, void *args,
  alignment.  */
   random_value v = ((uintptr_t) ) / alignof (max_align_t);
 
+#if !HAS_CLOCK_ENTROPY
+  /* Arrange gen_tempname to return less predictable file names on
+ systems lacking clock entropy .  */
+  static random_value prev_v;
+  v ^= prev_v;
+#endif
+
   /* How many random base-62 digits can currently be extracted from V.  */
   int vdigits = 0;
 
@@ -318,6 +331,9 @@ try_tempname_len (char *tmpl, int suffixlen, void *args,
   if (fd >= 0)
 {
   __set_errno (save_errno);
+#if !HAS_CLOCK_ENTROPY
+  prev_v = v;
+#endif
   return fd;
 }
   else if (errno != EEXIST)
-- 
2.34.1

Re: bug#57129: 29.0.50; Improve behavior of conditionals in Eshell

[Eli Zaretskii]

> From: Bruno Haible 
> Cc: egg...@cs.ucla.edu, bug-gnulib@gnu.org, la...@gnus.org, 
> 57...@debbugs.gnu.org, jporterb...@gmail.com
> Date: Tue, 16 Aug 2022 19:29:33 +0200
> 
> > The buffer which visited that file still exists, and still records the
> > name of the file, yes.  And then, when the program writes to another
> > file created by another call to make-temp-file, it actually writes to
> > a file that some buffer still visits, and in Emacs that triggers a
> > prompt to the user to refresh the buffer.
> 
> That is a reasonable behaviour for a text editor — but only for file
> names that are explicitly controlled by some program or by the user,
> not for temporary files.

Why not for temporary files?

Emacs has a with-temp-file macro, which generates a temporary file
name, executes the body of the macro with a variable bound to the file
name, then deletes the file.  Very handy for writing test suites.  We
don't usually bother deleting the buffers where the file was processed
in those tests, because the test suite runs in batch mode, and Emacs
exits after running the tests, thus cleaning up.

Having to carefully make sure no such buffers were left from the
previous test is a nuisance.

> > The programmer didn't
> > expect that because it is natural to expect each call to a function
> > that creates a temporary file to create a file under a new name, not
> > reuse the same name.
> 
> This is an incorrect assumption. Just like socket numbers are randomly
> generated in some situations, temp file names are random, and you can't
> make assumptions about the resulting file name.

People get their ideas from other similar APIs, and functions like
tmpfile in C do generate a different name each call.

> How about storing, as an attribute of the buffer, a boolean that tells
> whether the underlying file name is randomly generated (i.e. a temp file),
> and query this boolean before prompting to the user to refresh the buffer?

That's a terrible complications, especially since the solution is so
close at hand, and it makes gen_tempname behave on Windows as it does
on GNU/Linux, and as it behaved just a year or two ago.

Re: bug#57129: 29.0.50; Improve behavior of conditionals in Eshell

[Eli Zaretskii]

> Date: Tue, 16 Aug 2022 10:30:54 -0700
> Cc: bug-gnulib@gnu.org, la...@gnus.org, 57...@debbugs.gnu.org,
>  jporterb...@gmail.com, br...@clisp.org
> From: Paul Eggert 
> 
> On 8/16/22 10:04, Eli Zaretskii wrote:
> > That "finite number" of 62^6 sounds pretty close to infinity to me!
> 
> It's only 57 billion or so. That's not infinity

Yeah, right.

> Since we have the patch to prevent it, why not install it?

Because it isn't needed, and is a weird thing to do.  It also slows
down make-temp-file, especially when a session has a lot of buffers
and with remote file names.  So thanks, but no, thanks.

Re: bug#57129: 29.0.50; Improve behavior of conditionals in Eshell

[Eli Zaretskii]

> Date: Tue, 16 Aug 2022 10:25:57 -0700
> Cc: bug-gnulib@gnu.org, la...@gnus.org, 57...@debbugs.gnu.org,
>  jporterb...@gmail.com
> From: Paul Eggert 
> 
> On 8/16/22 09:25, Eli Zaretskii wrote:
> > why does this work differently on
> > different platforms?
> 
> Platforms that lack clock_gettime and CLOCK_MONOTONIC fall back on a 
> deterministic algorithm. Presumably MS-DOS one of the few such 
> platforms, which is why the problem is observed only on MS-DOS.

(Why are you talking about MS-DOS?)

MinGW does have clock_gettime, but we don't want to use it, because
one of the MinGW flavors (MinGW64) implements that function in
libwinpthreads, and that makes Emacs depend on libwinpthreads DLL,
which we want to avoid.

> How about something like the attached patch to Gnulib's lib/tempname.c? 

Thanks, but why not use 'random' instead?  Emacs does have it on all
platforms, including MS-Windows.  AFAIU, it's better than 'rand'.

> If I understand things correctly this should make the names random 
> enough on MS-DOS, though Emacs itself still needs a patch as I mentioned 
> a few minutes ago.

Why would Emacs need that patch?

Re: bug#57129: 29.0.50; Improve behavior of conditionals in Eshell

[Eli Zaretskii]

> Cc: jporterb...@gmail.com, la...@gnus.org, bug-gnulib@gnu.org,
>  57...@debbugs.gnu.org, br...@clisp.org
> Date: Tue, 16 Aug 2022 20:04:49 +0300
> From: Eli Zaretskii 
> 
> Therefore, if there's no intention to fix this in Gnulib, I'm going to
> update the documentation of make-temp-file so that Emacs users and
> programmers will be informed about that and will be careful enough to
> side-step these issues in all the situations.  (Not that I understand
> why won't Gnulib provide consistent behavior on all platforms, but I
> guess it's above my pay grade.)

And btw, looking closer, I see that this is a regression in Emacs 28,
which was caused by a change in Gnulib: the versions of tempname.c
that we used up to and including Emacs 27.2 used gettimeofday and the
PID to generate the file name, so it was more random than the code in
current Gnulib, and in particular the sequence of

   generate file-name1
   delete file-name1
   generate file-name2

would produce file-name2 that is different from file-name1.  With the
current code in Gnulib, something similar happens only on glibc
systems.  So I hope the code in Gnulib's tempname.c will be amended to
call clock_gettime or something similar, so that the names on
non-glibc platforms could also be random.  Or, failing that, at least
that gen_tempname in glibc would behave identically to other systems,
i.e. start from a fixed "random" value.

Re: bug#57129: 29.0.50; Improve behavior of conditionals in Eshell

[Paul Eggert]

On 8/16/22 10:04, Eli Zaretskii wrote:

That "finite number" of 62^6 sounds pretty close to infinity to me!


It's only 57 billion or so. That's not infinity, considering all the 
Emacs sessions out there. If Emacs's success grows, almost surely 
someone will run into this unlikely bug. Since we have the patch to 
prevent it, why not install it?

Re: bug#57129: 29.0.50; Improve behavior of conditionals in Eshell

[Bruno Haible]

Thanks for the explanations, Eli.

Eli Zaretskii wrote:
> > Maybe the problem is that the file gets deleted too early, when some parts
> > of Emacs still reference it?
> 
> The buffer which visited that file still exists, and still records the
> name of the file, yes.  And then, when the program writes to another
> file created by another call to make-temp-file, it actually writes to
> a file that some buffer still visits, and in Emacs that triggers a
> prompt to the user to refresh the buffer.

That is a reasonable behaviour for a text editor — but only for file
names that are explicitly controlled by some program or by the user,
not for temporary files.

> The programmer didn't
> expect that because it is natural to expect each call to a function
> that creates a temporary file to create a file under a new name, not
> reuse the same name.

This is an incorrect assumption. Just like socket numbers are randomly
generated in some situations, temp file names are random, and you can't
make assumptions about the resulting file name.

How about storing, as an attribute of the buffer, a boolean that tells
whether the underlying file name is randomly generated (i.e. a temp file),
and query this boolean before prompting to the user to refresh the buffer?

Bruno

Re: bug#57129: 29.0.50; Improve behavior of conditionals in Eshell

[Paul Eggert]

On 8/16/22 09:25, Eli Zaretskii wrote:

why does this work differently on
different platforms?


Platforms that lack clock_gettime and CLOCK_MONOTONIC fall back on a 
deterministic algorithm. Presumably MS-DOS one of the few such 
platforms, which is why the problem is observed only on MS-DOS.


How about something like the attached patch to Gnulib's lib/tempname.c? 
If I understand things correctly this should make the names random 
enough on MS-DOS, though Emacs itself still needs a patch as I mentioned 
a few minutes ago.


If this patch isn't good enough for MS-DOS, what sort of random bits are 
easily available on that platform? We don't need anything 
cryptographically secure; a higher-res clock should suffice.diff --git a/lib/tempname.c b/lib/tempname.c
index e6520191d7..e3d09d963d 100644
--- a/lib/tempname.c
+++ b/lib/tempname.c
@@ -71,7 +71,8 @@
 
 /* Use getrandom if it works, falling back on a 64-bit linear
congruential generator that starts with Var's value
-   mixed in with a clock's low-order bits if available.  */
+   mixed in with a clock's low-order bits (or with some other
+   pseudorandom number) if available.  */
 typedef uint_fast64_t random_value;
 #define RANDOM_VALUE_MAX UINT_FAST64_MAX
 #define BASE_62_DIGITS 10 /* 62**10 < UINT_FAST64_MAX */
@@ -89,6 +90,9 @@ random_bits (random_value var, bool use_getrandom)
   struct __timespec64 tv;
   __clock_gettime64 (CLOCK_MONOTONIC, );
   var ^= tv.tv_nsec;
+#else
+  /* Fall back on pseudorandom number .  */
+  var ^= rand ();
 #endif
   return 2862933555777941757 * var + 3037000493;
 }

Re: bug#57129: 29.0.50; Improve behavior of conditionals in Eshell

[Eli Zaretskii]

> Date: Tue, 16 Aug 2022 09:54:25 -0700
> Cc: bug-gnulib@gnu.org, la...@gnus.org, 57...@debbugs.gnu.org,
>  jporterb...@gmail.com, Bruno Haible 
> From: Paul Eggert 
> 
> On 8/16/22 09:25, Eli Zaretskii wrote:
> > The programmer didn't
> > expect that because it is natural to expect each call to a function
> > that creates a temporary file to create a file under a new name, not
> > reuse the same name.
> 
> Regardless of whether the programmer expects a random name or a 
> predictable-but-unique name, there are only a finite number of names to 
> choose from so the programmer must take into account the possibility 
> that the chosen name clashes with names that the programmer would prefer 
> not to be chosen.

Then what is this comment and the following assertion in Bruno's code
about?

  /* With 6 'X' and a good pseudo-random number generator behind the scenes,
 the probability of getting the same file name twice in a row should be
 1/62^6 < 1/10^10.  */
  ASSERT (strcmp (filename1, filename2) != 0);

That "finite number" of 62^6 sounds pretty close to infinity to me!

> This means an Emacs patch such as [1] is needed regardless of whether 
> Gnulib's gen_tempname is changed to be more random than it is. Although 
> it's true that the bug fixed by [1] is less likely if gen_tempname 
> generates more-random names, the bug can occur no matter what we do 
> about gen_tempname.

No, sorry.  I object to this patch, because it hides the problem under
the carpet.  That there's a file-visiting buffer that records the name
of a temporary file is just one case where this issue gets in the way.
The general problem is still there.  And it isn't an Emacs problem, so
Emacs is not the place to solve it.

Therefore, if there's no intention to fix this in Gnulib, I'm going to
update the documentation of make-temp-file so that Emacs users and
programmers will be informed about that and will be careful enough to
side-step these issues in all the situations.  (Not that I understand
why won't Gnulib provide consistent behavior on all platforms, but I
guess it's above my pay grade.)

Re: bug#57129: 29.0.50; Improve behavior of conditionals in Eshell

[Paul Eggert]

On 8/16/22 09:25, Eli Zaretskii wrote:

The programmer didn't
expect that because it is natural to expect each call to a function
that creates a temporary file to create a file under a new name, not
reuse the same name.


Regardless of whether the programmer expects a random name or a 
predictable-but-unique name, there are only a finite number of names to 
choose from so the programmer must take into account the possibility 
that the chosen name clashes with names that the programmer would prefer 
not to be chosen.


This means an Emacs patch such as [1] is needed regardless of whether 
Gnulib's gen_tempname is changed to be more random than it is. Although 
it's true that the bug fixed by [1] is less likely if gen_tempname 
generates more-random names, the bug can occur no matter what we do 
about gen_tempname.


[1] 
https://debbugs.gnu.org/cgi/bugreport.cgi?filename=0001-Don-t-create-temp-file-with-same-name-as-visited.patch;bug=57129;msg=59;att=2

Re: bug#57129: 29.0.50; Improve behavior of conditionals in Eshell

[Eli Zaretskii]

> From: Bruno Haible 
> Cc: egg...@cs.ucla.edu, bug-gnulib@gnu.org, la...@gnus.org, 
> 57...@debbugs.gnu.org, jporterb...@gmail.com
> Date: Tue, 16 Aug 2022 16:40:16 +0200
> 
> Eli Zaretskii wrote:
> > Looking at your test program, I see that you generate the seconds file
> > name without deleting the first one.  When a file by the first
> > generated name already exists, gen_tempname will indeed generate a
> > different name.  But in the scenario I described, Emacs creates one
> > temporary file, uses it, then deletes it, and only after that creates
> > another file.
> 
> Why would it be important for the second temporary file to bear a different
> name, once the first one is gone? I mean, process IDs get reused over time;
> socket numbers get reused over time; what is wrong with reusing a file name,
> once the older file is gone?

Because the Emacs Lisp program expected that, based on what
make-temp-file in Emacs promises (see the "random characters" part),
and because that's how it behaves on GNU/Linux.  Then the same program
behaved differently on MS-Windows, and the programmer was stumped.

Sure, it's possible to write the same program somewhat differently,
carefully working around this issue, but my point is that such
functions should ideally present the same behavior traits on all
systems, otherwise the portability is hampered.

> Maybe the problem is that the file gets deleted too early, when some parts
> of Emacs still reference it?

The buffer which visited that file still exists, and still records the
name of the file, yes.  And then, when the program writes to another
file created by another call to make-temp-file, it actually writes to
a file that some buffer still visits, and in Emacs that triggers a
prompt to the user to refresh the buffer.  The programmer didn't
expect that because it is natural to expect each call to a function
that creates a temporary file to create a file under a new name, not
reuse the same name.  E.g., similar facilities in the Standard C
library exhibit that behavior.  So the program was written assuming
that the second write was to an entirely different and unrelated file.

And again, my main point is: why does this work differently on
different platforms?  If you consider it OK to return the same file
name each time, provided that no such file exists, then why doesn't
that function do it on GNU/Linux?

[PATCH] maintainer-makefile: Check for incorrect DISTCHECK_CONFIGURE_FLAGS usage.

[Simon Josefsson via Gnulib discussion list]

I discovered use of DISTCHECK_CONFIGURE_FLAGS in Makefile.am for several
projects, but that's a user-variable and AM_DISTCHECK_CONFIGURE_FLAGS
should be used.

/Simon
From dec7194206fc1ec7db0a94472d8ece58025040c6 Mon Sep 17 00:00:00 2001
From: Simon Josefsson 
Date: Tue, 16 Aug 2022 17:26:56 +0200
Subject: [PATCH] maintainer-makefile: Check for incorrect
 DISTCHECK_CONFIGURE_FLAGS usage.

* top/maint.mk (sc_makefile_DISTCHECK_CONFIGURE_FLAGS): Add.
---
 ChangeLog| 6 ++
 top/maint.mk | 6 ++
 2 files changed, 12 insertions(+)

diff --git a/ChangeLog b/ChangeLog
index c38798745d..b639d1709d 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2022-08-16  Simon Josefsson  
+
+	maintainer-makefile: Check for incorrect DISTCHECK_CONFIGURE_FLAGS
+	usage.
+	* top/maint.mk (sc_makefile_DISTCHECK_CONFIGURE_FLAGS): Add.
+
 2022-08-16  Bruno Haible  
 
 	tempname: Add tests.
diff --git a/top/maint.mk b/top/maint.mk
index c1fdf9ca2c..5745d5831d 100644
--- a/top/maint.mk
+++ b/top/maint.mk
@@ -1256,6 +1256,12 @@ sc_makefile_path_separator_check:
 	halt=$(msg)			\
 	  $(_sc_search_regexp)
 
+sc_makefile_DISTCHECK_CONFIGURE_FLAGS:
+	@prohibit='^DISTCHECK_CONFIGURE_FLAGS'\
+	in_vc_files='akefile|\.mk$$'	\
+	halt="use AM_DISTCHECK_CONFIGURE_FLAGS"\
+	  $(_sc_search_regexp)
+
 # Check that 'make alpha' will not fail at the end of the process,
 # i.e., when pkg-M.N.tar.xz already exists (either in "." or in ../release)
 # and is read-only.
-- 
2.30.2



signature.asc
Description: PGP signature

Re: bug#57129: 29.0.50; Improve behavior of conditionals in Eshell

[Bruno Haible]

Eli Zaretskii wrote:
> Looking at your test program, I see that you generate the seconds file
> name without deleting the first one.  When a file by the first
> generated name already exists, gen_tempname will indeed generate a
> different name.  But in the scenario I described, Emacs creates one
> temporary file, uses it, then deletes it, and only after that creates
> another file.

Why would it be important for the second temporary file to bear a different
name, once the first one is gone? I mean, process IDs get reused over time;
socket numbers get reused over time; what is wrong with reusing a file name,
once the older file is gone?

Maybe the problem is that the file gets deleted too early, when some parts
of Emacs still reference it?

Bruno

Re: support shallow gnulib submodule checkouts

[Simon Josefsson via Gnulib discussion list]

Updated patch below.

Meanwhile, I'll disable the public-submodule-commit test in the projects
I work on -- I don't think it is appropriate to run it on every 'make
check' invocation.  Disable it by putting this in cfg.mk:

submodule-checks =
gl_public_submodule_commit =

/Simon

diff --git a/top/maint.mk b/top/maint.mk
index c1fdf9ca2c..1982833e91 100644
--- a/top/maint.mk
+++ b/top/maint.mk
@@ -1493,10 +1493,13 @@ submodule-checks ?= no-submodule-changes 
public-submodule-commit
 # Ensure that each sub-module commit we're using is public.
 # Without this, it is too easy to tag and release code that
 # cannot be built from a fresh clone.
+gl_seed ?= d146c864e8d8cc82e96d722337253dd5a3a803b8
 .PHONY: public-submodule-commit
 public-submodule-commit:
$(AM_V_GEN)if test -d $(srcdir)/.git\
-   && git --version >/dev/null 2>&1; then  \
+   && git --version >/dev/null 2>&1\
+   && { cd $(gnulib_dir) &&\
+   git cat-file -e $(gl_seed); }; then \
  cd $(srcdir) &&   \
  git submodule --quiet foreach \
  'test "$$(git rev-parse "$$sha1")"\


signature.asc
Description: PGP signature

Re: bug#57129: 29.0.50; Improve behavior of conditionals in Eshell

[Eli Zaretskii]

> From: Bruno Haible 
> Cc: la...@gnus.org, 57...@debbugs.gnu.org, jporterb...@gmail.com
> Date: Tue, 16 Aug 2022 15:35:17 +0200
> 
> Eli Zaretskii wrote:
> > The problem is that callers of
> > make-temp-file reasonably expect it to return a new name every call.
> > And evidently, it does that on GNU/Linux (not sure how, given that the
> > tempname.c code is supposed to be in glibc?).  So if there's no
> > intention to change gen_tempname on non-glibc platforms so that it
> > doesn't repeat the same "random characters" upon each call, I think
> > Emacs should call a different function to generate "random" names of
> > temporary files, for consistency across platforms if not for other
> > reasons.
> 
> You are making incorrect assumptions or hypotheses.

They are neither assumptions nor hypotheses.  They are what I actually
saw by stepping with a debugger into the code.

> I am adding the unit test below. It proves that (at least) on Linux,
> FreeBSD 11, NetBSD 7, OpenBSD 6.0, macOS, AIX 7.1, Solaris 10,
> Cygwin, and native Windows, gen_tempname *does* return a new file
> name on each call, with a very high probability.
> 
> So, gen_tempname does *not* repeat the same "random characters" upon each 
> call.

Well, it does here, when called from Emacs in the particular scenario
of the unit test I was looking into.

Looking at your test program, I see that you generate the seconds file
name without deleting the first one.  When a file by the first
generated name already exists, gen_tempname will indeed generate a
different name.  But in the scenario I described, Emacs creates one
temporary file, uses it, then deletes it, and only after that creates
another file.  In terms of your test program, you need to move the
first unlink call to before the second call to gen_tempname.  Then
your test program will faithfully reproduce what Emacs does in the
case in point.

Re: bug#57129: 29.0.50; Improve behavior of conditionals in Eshell

[Bruno Haible]

Eli Zaretskii wrote:
> The problem is that callers of
> make-temp-file reasonably expect it to return a new name every call.
> And evidently, it does that on GNU/Linux (not sure how, given that the
> tempname.c code is supposed to be in glibc?).  So if there's no
> intention to change gen_tempname on non-glibc platforms so that it
> doesn't repeat the same "random characters" upon each call, I think
> Emacs should call a different function to generate "random" names of
> temporary files, for consistency across platforms if not for other
> reasons.

You are making incorrect assumptions or hypotheses. I am adding the unit
test below. It proves that (at least) on Linux, FreeBSD 11, NetBSD 7,
OpenBSD 6.0, macOS, AIX 7.1, Solaris 10, Cygwin, and native Windows,
gen_tempname *does* return a new file name on each call, with a very high
probability.

So, gen_tempname does *not* repeat the same "random characters" upon each call.


2022-08-16  Bruno Haible  

tempname: Add tests.
* tests/test-tempname.c: New file.
* modules/tempname-tests: New file.

=== tests/test-tempname.c ==
/* Test of creating a temporary file.
   Copyright (C) 2022 Free Software Foundation, Inc.

   This program 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.

   This program 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 this program.  If not, see .  */

/* Written by Bruno Haible , 2022.  */

#include 

#include "tempname.h"

#include 
#include 

#include "macros.h"

int
main ()
{
  /* Verify that two consecutive calls to gen_tempname return two different
 file names, with a high probability.  */
  const char *templ18 = "gl-temp-XX.xyz";
  char filename1[18 + 1];
  char filename2[18 + 1];

  strcpy (filename1, templ18);
  int fd1 = gen_tempname (filename1, strlen (".xyz"), 0, GT_FILE);
  ASSERT (fd1 >= 0);

  strcpy (filename2, templ18);
  int fd2 = gen_tempname (filename2, strlen (".xyz"), 0, GT_FILE);
  ASSERT (fd2 >= 0);

  /* With 6 'X' and a good pseudo-random number generator behind the scenes,
 the probability of getting the same file name twice in a row should be
 1/62^6 < 1/10^10.  */
  ASSERT (strcmp (filename1, filename2) != 0);

  /* Clean up.  */
  close (fd1);
  close (fd2);
  unlink (filename1);
  unlink (filename2);

  return 0;
}
=== modules/tempname-tests =
Files:
tests/test-tempname.c
tests/macros.h

Depends-on:
unlink

configure.ac:

Makefile.am:
TESTS += test-tempname
check_PROGRAMS += test-tempname
test_tempname_LDADD = $(LDADD) $(LIB_GETRANDOM) $(LIB_CLOCK_GETTIME)

Re: bug#57129: 29.0.50; Improve behavior of conditionals in Eshell

[Eli Zaretskii]

> Date: Mon, 15 Aug 2022 13:55:35 -0700
> Cc: la...@gnus.org, 57...@debbugs.gnu.org, Jim Porter
>  , Gnulib bugs 
> From: Paul Eggert 
> 
> On 8/15/22 11:58, Eli Zaretskii wrote:
> 
> > Ah, okay.  It's a (mis)feature of Gnulib's gen_tempname function
> > (which is the guts of make-temp-file) in its implementation for
> > MS-Windows (and maybe other platforms?): it always begins from the
> > same "random" characters in the file name, and only generates other
> > random characters if there's already a file by that name.
> 
> Not sure I'd call it a misfeature

I didn't say "misfeature".  Evidently, for you and perhaps others it's
a feature.

> as gen_tempname is generating a uniquely-named file that is
> exclusive to Emacs, which is all it's supposed to do.

Then perhaps calling it from make-temp-file in Emacs is a mistake,
because that function's doc string says

  Create a temporary file.
  The returned file name (created by appending some random characters at the end
  of PREFIX, and expanding against ‘temporary-file-directory’ if necessary),
  is guaranteed to point to a newly created file.

When you tell someone that the file name will include "some random
characters", they don't expect that they will be the same "random
characters" every call, just like saying that some function generates
random numbers doesn't imply it will be the same number every call.

> I'm not sure I'm entirely understanding the Emacs problem, but it 
> appears to be that Emacs has its own set of filenames that it thinks it 
> knows about, and Emacs wants the new temporary file's name to not be a 
> member of that set. If I'm right, does the second attached patch (this 
> patch is to Emacs) address the problem? I haven't tested or installed it.

I don't think that's the problem, no.  The problem is that callers of
make-temp-file reasonably expect it to return a new name every call.
And evidently, it does that on GNU/Linux (not sure how, given that the
tempname.c code is supposed to be in glibc?).  So if there's no
intention to change gen_tempname on non-glibc platforms so that it
doesn't repeat the same "random characters" upon each call, I think
Emacs should call a different function to generate "random" names of
temporary files, for consistency across platforms if not for other
reasons.

support shallow gnulib submodule checkouts

[Simon Josefsson via Gnulib discussion list]

Hi

Fetching the gnulib submodule takes quite some time, so I'd like to use
a shallow checkout instead.  However I get an error:

jas@latte:~/src/gsasl-small$ make check
  GEN  public-submodule-commit
fatal: run_command returned non-zero status for gnulib
.
maint.mk: found non-public submodule commit
make: *** [maint.mk:1498: public-submodule-commit] Fel 1
jas@latte:~/src/gsasl-small$ 

See code snippet from maint.mk below.  The test fails but the situation
the documentation says it is intended to protect against is not
occuring.

How about this patch?  It will only run the test if the first gnulib
commit is present.  I'm not sure it is possible to catch the problem the
situation is trying to detect in a shallow checkout?

diff --git a/top/maint.mk b/top/maint.mk
index c1fdf9ca2c..0b3208a158 100644
--- a/top/maint.mk
+++ b/top/maint.mk
@@ -1496,7 +1496,8 @@ submodule-checks ?= no-submodule-changes 
public-submodule-commit
 .PHONY: public-submodule-commit
 public-submodule-commit:
$(AM_V_GEN)if test -d $(srcdir)/.git\
-   && git --version >/dev/null 2>&1; then  \
+   && git --version >/dev/null 2>&1\
+   && git cat-file -e d146c864e8d8cc82e96d722337253dd5a3a803b8; 
then   \
  cd $(srcdir) &&   \
  git submodule --quiet foreach \
  'test "$$(git rev-parse "$$sha1")"\

/Simon

# Ensure that each sub-module commit we're using is public.
# Without this, it is too easy to tag and release code that
# cannot be built from a fresh clone.
.PHONY: public-submodule-commit
public-submodule-commit:
$(AM_V_GEN)if test -d $(srcdir)/.git\
&& git --version >/dev/null 2>&1; then  \
  cd $(srcdir) &&   \
  git submodule --quiet foreach \
  'test "$$(git rev-parse "$$sha1")"\
  = "$$(git merge-base origin "$$sha1")"'   \
|| { echo '$(ME): found non-public submodule commit' >&2;   \
 exit 1; }; \
else\
  : ;   \
fi
# This rule has a high enough utility/cost ratio that it should be a
# dependent of "check" by default.  However, some of us do occasionally
# commit a temporary change that deliberately points to a non-public
# submodule commit, and want to be able to use rules like "make check".
# In that case, run e.g., "make check gl_public_submodule_commit="
# to disable this test.
gl_public_submodule_commit ?= public-submodule-commit
check: $(gl_public_submodule_commit)


signature.asc
Description: PGP signature