diff mbox series

Rearrange detection of temporary directory for NetBSD

Message ID 20200318192942.22006-1-n54@gmx.com
State New
Headers show
Series Rearrange detection of temporary directory for NetBSD | expand

Commit Message

Kamil Rytarowski March 18, 2020, 7:29 p.m. UTC
Set /tmp first, then /var/tmp. /tmp is volatile on NetBSD and
/var/tmp not. This improves performance in the common use.
The downstream copy of GCC was patched for this preference
since 2015.

Remove occurence of /usr/tmp as it was never valid for NetBSD.
It was already activey disabled in the GCC manual page in 1996 and
in the GCC source code at least in 1998.

This change is not a matter of user-preference but Operating
System defaults that disagree with the libiberty detection plan.

No functional change for other Operataing Systems/environments.

libiberty/ChangeLog:

	* make-temp-file.c (choose_tmpdir): Honor NetBSD specific paths.
---
 libiberty/ChangeLog        | 4 ++++
 libiberty/make-temp-file.c | 8 +++++++-
 2 files changed, 11 insertions(+), 1 deletion(-)

--
2.25.0

Comments

Li, Pan2 via Gcc-patches March 25, 2020, 10:36 p.m. UTC | #1
On Wed, 2020-03-18 at 20:29 +0100, Kamil Rytarowski wrote:
> Set /tmp first, then /var/tmp. /tmp is volatile on NetBSD and
> /var/tmp not. This improves performance in the common use.
> The downstream copy of GCC was patched for this preference
> since 2015.
> 
> Remove occurence of /usr/tmp as it was never valid for NetBSD.
> It was already activey disabled in the GCC manual page in 1996 and
> in the GCC source code at least in 1998.
> 
> This change is not a matter of user-preference but Operating
> System defaults that disagree with the libiberty detection plan.
> 
> No functional change for other Operataing Systems/environments.
> 
> libiberty/ChangeLog:
> 
> 	* make-temp-file.c (choose_tmpdir): Honor NetBSD specific paths.
> ---
>  libiberty/ChangeLog        | 4 ++++
>  libiberty/make-temp-file.c | 8 +++++++-
>  2 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/libiberty/ChangeLog b/libiberty/ChangeLog
> index 106c107e91a..18b9357aaed 100644
> --- a/libiberty/ChangeLog
> +++ b/libiberty/ChangeLog
> @@ -1,3 +1,7 @@
> +2020-03-18  Kamil Rytarowski  <n54@gmx.com>
> +
> +	* make-temp-file.c (choose_tmpdir): Honor NetBSD specific paths.
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.

I wouldn't mind dropping /usr/tmp.  That so antiquated that it'd be non-
controversial.  Can you send that as a separate patch.

Jeff
>
Kamil Rytarowski March 26, 2020, 12:39 a.m. UTC | #2
On 25.03.2020 23:36, Jeff Law wrote:
> On Wed, 2020-03-18 at 20:29 +0100, Kamil Rytarowski wrote:
>> Set /tmp first, then /var/tmp. /tmp is volatile on NetBSD and
>> /var/tmp not. This improves performance in the common use.
>> The downstream copy of GCC was patched for this preference
>> since 2015.
>>
>> Remove occurence of /usr/tmp as it was never valid for NetBSD.
>> It was already activey disabled in the GCC manual page in 1996 and
>> in the GCC source code at least in 1998.
>>
>> This change is not a matter of user-preference but Operating
>> System defaults that disagree with the libiberty detection plan.
>>
>> No functional change for other Operataing Systems/environments.
>>
>> libiberty/ChangeLog:
>>
>> 	* make-temp-file.c (choose_tmpdir): Honor NetBSD specific paths.
>> ---
>>  libiberty/ChangeLog        | 4 ++++
>>  libiberty/make-temp-file.c | 8 +++++++-
>>  2 files changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/libiberty/ChangeLog b/libiberty/ChangeLog
>> index 106c107e91a..18b9357aaed 100644
>> --- a/libiberty/ChangeLog
>> +++ b/libiberty/ChangeLog
>> @@ -1,3 +1,7 @@
>> +2020-03-18  Kamil Rytarowski  <n54@gmx.com>
>> +
>> +	* make-temp-file.c (choose_tmpdir): Honor NetBSD specific paths.
> 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.
>

NetBSD supports swap whenever /tmp is too small. Whenever tmpfs+ram are
too small, users use /tmp that is a regular directory (if I am not
wrong, this is still the default setup from an installer).

In NetBSD we want to use a non-persistent storage for performance
reasons. /var/tmp also affects negatively modern SSD devices with
needless writes.

It would be enough to get these try_dir paths tuned at least from
preprocessor during libiberty build for gdb/gcc/etc.

> I wouldn't mind dropping /usr/tmp.  That so antiquated that it'd be non-
> controversial.  Can you send that as a separate patch.
>

Behavior for !__NetBSD__ is out of interest.

> Jeff
>>
>
Gerald Pfeifer June 28, 2021, 10:45 p.m. UTC | #3
On Thu, 26 Mar 2020, Kamil Rytarowski wrote:
> On 25.03.2020 23:36, Jeff Law wrote:
>> I wouldn't mind dropping /usr/tmp.  That so antiquated that it'd be 
>> non- controversial.  Can you send that as a separate patch.
> Behavior for !__NetBSD__ is out of interest.

This is not a very useful approach in a collaborative project like GCC.

Incremental changes (including cleanups) help and are a good way to get 
engaged, improve the overall code base, and gain support from others 
(who may not have any interest in the __NetBSD__ case, but be willing 
to collaborate).

@Jeff, is the following what you had in mind?  

It passed testing on i686-unknown-freebsd12; okay to push?

Gerald


commit 8365565396cee65aeb6c2e4bfad74e095a3c388c
Author: Gerald Pfeifer <gerald@pfeifer.com>
Date:   Tue Jun 29 00:39:15 2021 +0200

    libiberty: No longer use /usr/tmp
    
    /usr/tmp is antiquated and not present on decently modern systems.
    Remove it from consideration when choosing a directory for temporary
    files.
    
    libiberty:
    
    2021-06-29  Gerald Pfeifer  <gerald@pfeifer.com>
    
            * make-temp-file.c (usrtmp): Remove.
            (choose_tmpdir): Remove use of usrtmp.

diff --git a/libiberty/ChangeLog b/libiberty/ChangeLog
index 1c9138861bd..2f8390cc63a 100644
--- a/libiberty/ChangeLog
+++ b/libiberty/ChangeLog
@@ -1,3 +1,8 @@
+2021-06-13  Gerald Pfeifer  <gerald@pfeifer.com>
+
+	* make-temp-file.c (usrtmp): Remove.
+	(choose_tmpdir): Remove use of usrtmp.
+
 2021-06-05  John David Anglin  <danglin@gcc.gnu.org>
 
 	PR target/100734
diff --git a/libiberty/make-temp-file.c b/libiberty/make-temp-file.c
index 7465cec5ea6..cad0645619e 100644
--- a/libiberty/make-temp-file.c
+++ b/libiberty/make-temp-file.c
@@ -81,8 +81,6 @@ try_dir (const char *dir, const char *base)
 }
 
 static const char tmp[] = { DIR_SEPARATOR, 't', 'm', 'p', 0 };
-static const char usrtmp[] =
-{ DIR_SEPARATOR, 'u', 's', 'r', DIR_SEPARATOR, 't', 'm', 'p', 0 };
 static const char vartmp[] =
 { DIR_SEPARATOR, 'v', 'a', 'r', DIR_SEPARATOR, 't', 'm', 'p', 0 };
 
@@ -131,7 +129,6 @@ choose_tmpdir (void)
 
       /* Try /var/tmp, /usr/tmp, then /tmp.  */
       base = try_dir (vartmp, base);
-      base = try_dir (usrtmp, base);
       base = try_dir (tmp, base);
       
       /* If all else fails, use the current directory!  */
Jeff Law June 29, 2021, 3:15 p.m. UTC | #4
On 6/28/2021 4:45 PM, Gerald Pfeifer wrote:
> On Thu, 26 Mar 2020, Kamil Rytarowski wrote:
>> On 25.03.2020 23:36, Jeff Law wrote:
>>> I wouldn't mind dropping /usr/tmp.  That so antiquated that it'd be
>>> non- controversial.  Can you send that as a separate patch.
>> Behavior for !__NetBSD__ is out of interest.
> This is not a very useful approach in a collaborative project like GCC.
>
> Incremental changes (including cleanups) help and are a good way to get
> engaged, improve the overall code base, and gain support from others
> (who may not have any interest in the __NetBSD__ case, but be willing
> to collaborate).
>
> @Jeff, is the following what you had in mind?
>
> It passed testing on i686-unknown-freebsd12; okay to push?
>
> Gerald
>
>
> commit 8365565396cee65aeb6c2e4bfad74e095a3c388c
> Author: Gerald Pfeifer <gerald@pfeifer.com>
> Date:   Tue Jun 29 00:39:15 2021 +0200
>
>      libiberty: No longer use /usr/tmp
>      
>      /usr/tmp is antiquated and not present on decently modern systems.
>      Remove it from consideration when choosing a directory for temporary
>      files.
>      
>      libiberty:
>      
>      2021-06-29  Gerald Pfeifer  <gerald@pfeifer.com>
>      
>              * make-temp-file.c (usrtmp): Remove.
>              (choose_tmpdir): Remove use of usrtmp.
Yup.  This is fine.  You might consider updating the comment which 
references /usr/tmp in choose_tmpdir along the way.
jeff
Gerald Pfeifer June 30, 2021, 10:03 p.m. UTC | #5
On Tue, 29 Jun 2021, Jeff Law wrote:
>>      2021-06-29  Gerald Pfeifer  <gerald@pfeifer.com>
>>      
>>              * make-temp-file.c (usrtmp): Remove.
>>              (choose_tmpdir): Remove use of usrtmp.
> Yup.  This is fine.  You might consider updating the comment which 
> references /usr/tmp in choose_tmpdir along the way.

You've got sharp eyes - and gave me the opportunity to practice
`git rebase -i`. ;-)

Pushed, thank you.

Gerald
diff mbox series

Patch

diff --git a/libiberty/ChangeLog b/libiberty/ChangeLog
index 106c107e91a..18b9357aaed 100644
--- a/libiberty/ChangeLog
+++ b/libiberty/ChangeLog
@@ -1,3 +1,7 @@ 
+2020-03-18  Kamil Rytarowski  <n54@gmx.com>
+
+	* make-temp-file.c (choose_tmpdir): Honor NetBSD specific paths.
+
 2020-03-05  Egeyar Bagcioglu  <egeyar.bagcioglu@oracle.com>

 	* simple-object.c (handle_lto_debug_sections): Name
diff --git a/libiberty/make-temp-file.c b/libiberty/make-temp-file.c
index cb08c27af6f..674333f042b 100644
--- a/libiberty/make-temp-file.c
+++ b/libiberty/make-temp-file.c
@@ -129,10 +129,16 @@  choose_tmpdir (void)
 	base = try_dir (P_tmpdir, base);
 #endif

-      /* Try /var/tmp, /usr/tmp, then /tmp.  */
+#if defined(__NetBSD__)
+      /* Try /tmp (volatile), then /var/tmp (non-volatile) on NetBSD.  */
+      base = try_dir (tmp, base);
+      base = try_dir (vartmp, base);
+#else
+      /* For others try /var/tmp, /usr/tmp, then /tmp.  */
       base = try_dir (vartmp, base);
       base = try_dir (usrtmp, base);
       base = try_dir (tmp, base);
+#endif

       /* If all else fails, use the current directory!  */
       if (base == 0)