Re: CVS commit: src/external/gpl3

[Kamil Rytarowski <n54%gmx.com@localhost>]

I've found out that this patch in not needed, at least on NetBSD and
likely Linux.

The algorithm to pick tmpdir is as follows:
 - check for memoized tmpdir
 - try env variables
 - try P_tmpdir
 - try /var/tmp, then /usr/tmp, then /tmp
 - fallback to current dir ('.')
 - save memoized tmpdir

P_tmpdir is defined as /tmp on NetBSD for XOPEN/NETBSD SOURCE.

I've verifed that P_tmpdir is always picked and defined (*) in
make-temp-file.c for all of our GNU toolchain programs (gdb, gcc,
binutils) in tools build, in distribution build, in pkgsrc build and in
standalone build.

This means that we return choose_tmpdir() always "/tmp" (**)

I've double checked this with the source code 1 commit before this
change (***) and in the current sources.

So, the question is... is this just a hack for build on Darwin or some
other OS without P_tmpdir? Was this patch ever really needed?

If this is just a hack for one specific OS, I recommend to remove our
local modification and rely on TMPDIR in such setup or discuss with
upstream GCC (libiberty) a patch how to point it to /tmp.


(*) I've checked the claim with:

#ifdef P_tmpdir
      /* We really want a directory name here as if concatenated with
say \dir
         we do not end up with a double \\ which defines an UNC path.  */
      if (strcmp (P_tmpdir, "\\") == 0)
        base = try_dir ("\\.", base);
      else
        base = try_dir (P_tmpdir, base);
#else
#error 1
#endif

(**) To be extra sure, I have verified this with logging to a temporary
file:

       memoized_tmpdir = tmpdir;
+
+      {
+        FILE *fp = fopen("/tmp/log.tmp.txt", "a");
+        fprintf(fp, "PID=%d PROG='%s' TMPDIR='%s'\n", getpid(),
getprogname(), tmpdir);
+        fclose(fp);
+      }


$ LC_ALL=C grep -c /var/tmp /tmp/log.tmp.txt
0

$ LC_ALL=C grep -c tmp /tmp/log.tmp.txt
48745

$ head /tmp/log.tmp.txt
PID=16262 PROG='x86_64--netbsd-gcc' TMPDIR='/tmp//'
PID=20242 PROG='x86_64--netbsd-gcc' TMPDIR='/tmp//'
PID=18851 PROG='x86_64--netbsd-gcc' TMPDIR='/tmp//'
PID=11095 PROG='x86_64--netbsd-gcc' TMPDIR='/tmp//'
PID=8036 PROG='x86_64--netbsd-gcc' TMPDIR='/tmp//'
PID=26914 PROG='x86_64--netbsd-gcc' TMPDIR='/tmp//'
PID=8930 PROG='x86_64--netbsd-gcc' TMPDIR='/tmp//'
PID=21615 PROG='x86_64--netbsd-gcc' TMPDIR='/tmp//'
PID=27201 PROG='x86_64--netbsd-gcc' TMPDIR='/tmp//'
PID=11745 PROG='x86_64--netbsd-gcc' TMPDIR='/tmp//'

(***)
commit 0eba0e605b50d99665de27a77f5ca870f2c8b41d (HEAD -> test)
Author: ozaki-r <ozaki-r%NetBSD.org@localhost>
Date:   Mon Aug 10 09:32:01 2015 +0000

    Fix head and cleanup definitions for cache_expiration tests


On 26.03.2020 01:43, Jonathan A. Kollasch wrote:
> We (to the extent I can assert that as an individual developer) insist
> on keeping it.
>
> 	Jonathan Kollasch
>
> On Thu, Mar 26, 2020 at 01:26:05AM +0100, Kamil Rytarowski wrote:
>> Upstream (GCC) is strongly against this change (even under __NetBSD__
>> ifdef) as /var/tmp is typically larger than /tmp:
>>
>>> I'd strongly recommend against this as-is.
>>>
>>> The whole reason we prefer /var/tmp is because it's often dramatically
>> larger
>>> than a ram-backed /tmp.
>>
>> -- by Jeff Law.
>>
>> Do we insist on this patch? Can we remove it from local sources?
>>
>> The same effect can be achieved with env(1) variables: TMP=/tmp
>> TEMPDIR=/tmp or TEMP=/tmp.
>>
>> On 10.08.2015 17:45, Jonathan A. Kollasch wrote:
>>> Module Name:	src
>>> Committed By:	jakllsch
>>> Date:		Mon Aug 10 15:45:40 UTC 2015
>>>
>>> Modified Files:
>>> 	src/external/gpl3/binutils/dist/libiberty: make-temp-file.c
>>> 	src/external/gpl3/gcc/dist/libiberty: make-temp-file.c
>>> 	src/external/gpl3/gdb/dist/libiberty: make-temp-file.c
>>>
>>> Log Message:
>>> Correct temporary directory preference order in libiberty's choose_tmpdir().
>>>
>>> Because it is intended to be persistent, /var/tmp is about the worst possible
>>> choice for temporary files for most users of libiberty.  /tmp works better,
>>> because the the defined semantics of /tmp allow for a non-persistent tmpfs
>>> to be used.  This should improve performance when /tmp is a tmpfs and it is
>>> difficult or impossible to have an environment variable or command line -pipe
>>> flag passed to every piece of the toolchain.
>>>
>>>
>>> To generate a diff of this commit:
>>> cvs rdiff -u -r1.1.1.3 -r1.2 \
>>>     src/external/gpl3/binutils/dist/libiberty/make-temp-file.c
>>> cvs rdiff -u -r1.1.1.2 -r1.2 \
>>>     src/external/gpl3/gcc/dist/libiberty/make-temp-file.c
>>> cvs rdiff -u -r1.1.1.1 -r1.2 \
>>>     src/external/gpl3/gdb/dist/libiberty/make-temp-file.c
>>>
>>> Please note that diffs are not public domain; they are subject to the
>>> copyright notices on the relevant files.
>>>
>>>
>>> Modified files:
>>>
>>> Index: src/external/gpl3/binutils/dist/libiberty/make-temp-file.c
>>> diff -u src/external/gpl3/binutils/dist/libiberty/make-temp-file.c:1.1.1.3 src/external/gpl3/binutils/dist/libiberty/make-temp-file.c:1.2
>>> --- src/external/gpl3/binutils/dist/libiberty/make-temp-file.c:1.1.1.3	Sun Sep 29 13:46:38 2013
>>> +++ src/external/gpl3/binutils/dist/libiberty/make-temp-file.c	Mon Aug 10 15:45:40 2015
>>> @@ -130,10 +130,10 @@ choose_tmpdir (void)
>>>  	base = try_dir (P_tmpdir, base);
>>>  #endif
>>>
>>> -      /* Try /var/tmp, /usr/tmp, then /tmp.  */
>>> +      /* Try /tmp, /var/tmp, then /usr/tmp.  */
>>> +      base = try_dir (tmp, base);
>>>        base = try_dir (vartmp, base);
>>>        base = try_dir (usrtmp, base);
>>> -      base = try_dir (tmp, base);
>>>
>>>        /* If all else fails, use the current directory!  */
>>>        if (base == 0)
>>>
>>> Index: src/external/gpl3/gcc/dist/libiberty/make-temp-file.c
>>> diff -u src/external/gpl3/gcc/dist/libiberty/make-temp-file.c:1.1.1.2 src/external/gpl3/gcc/dist/libiberty/make-temp-file.c:1.2
>>> --- src/external/gpl3/gcc/dist/libiberty/make-temp-file.c:1.1.1.2	Sat Mar  1 08:41:40 2014
>>> +++ src/external/gpl3/gcc/dist/libiberty/make-temp-file.c	Mon Aug 10 15:45:40 2015
>>> @@ -130,10 +130,10 @@ choose_tmpdir (void)
>>>  	base = try_dir (P_tmpdir, base);
>>>  #endif
>>>
>>> -      /* Try /var/tmp, /usr/tmp, then /tmp.  */
>>> +      /* Try /tmp, /var/tmp, then /usr/tmp.  */
>>> +      base = try_dir (tmp, base);
>>>        base = try_dir (vartmp, base);
>>>        base = try_dir (usrtmp, base);
>>> -      base = try_dir (tmp, base);
>>>
>>>        /* If all else fails, use the current directory!  */
>>>        if (base == 0)
>>>
>>> Index: src/external/gpl3/gdb/dist/libiberty/make-temp-file.c
>>> diff -u src/external/gpl3/gdb/dist/libiberty/make-temp-file.c:1.1.1.1 src/external/gpl3/gdb/dist/libiberty/make-temp-file.c:1.2
>>> --- src/external/gpl3/gdb/dist/libiberty/make-temp-file.c:1.1.1.1	Sat Sep 24 19:49:55 2011
>>> +++ src/external/gpl3/gdb/dist/libiberty/make-temp-file.c	Mon Aug 10 15:45:40 2015
>>> @@ -130,10 +130,10 @@ choose_tmpdir (void)
>>>  	base = try_dir (P_tmpdir, base);
>>>  #endif
>>>
>>> -      /* Try /var/tmp, /usr/tmp, then /tmp.  */
>>> +      /* Try /tmp, /var/tmp, then /usr/tmp.  */
>>> +      base = try_dir (tmp, base);
>>>        base = try_dir (vartmp, base);
>>>        base = try_dir (usrtmp, base);
>>> -      base = try_dir (tmp, base);
>>>
>>>        /* If all else fails, use the current directory!  */
>>>        if (base == 0)
>>>
>>
>>
>
>
>