[PATCH] testsuite: Cleanup some temp dirs with gdb-index files

[PATCH] testsuite: Cleanup some temp dirs with gdb-index files

[Bernd Edlinger]

[-- Attachment #1: Type: text/plain, Size: 611 bytes --]

Hi,

I'd like to cleanup some files/directories which
remain after the gdb testsute runs.

I want to avoid using "rm -rf ..." since that can be dangerous.
Therefore I remove the *.gdb-index files, if any, and use
rmdir instead.  I am not sure if there is a better way,
instead of using "remote_exec host sh -c" to do the globbing.

Tested on x86_64-pc-linux-gnu.
Is it OK for trunk?


gdb/testsuite:
2021-05-14  Bernd Edlinger  <bernd.edlinger@hotmail.de>

        * gdb.base/index-cache.exp: Cleanup $cache_dir/*.gdb-index and
        remove the directory.
        * gdb.dwarf2/per-bfd-sharing.exp: Likewise.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-testsuite-Cleanup-some-temp-dirs-with-gdb-index-file.patch --]
[-- Type: text/x-patch; name="0001-testsuite-Cleanup-some-temp-dirs-with-gdb-index-file.patch", Size: 1624 bytes --]

From 044dafce200d87e5b0546111714f723e99a59594 Mon Sep 17 00:00:00 2001
From: Bernd Edlinger <bernd.edlinger@hotmail.de>
Date: Fri, 14 May 2021 14:53:19 +0200
Subject: [PATCH] testsuite: Cleanup some temp dirs with gdb-index files

After the gdb test-suite runs there are some files
left in /tmp/tmp*/*.gdb-index, remove those files
and the directory at the end of the test case.

gdb/testsuite:
2021-05-14  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* gdb.base/index-cache.exp: Cleanup $cache_dir/*.gdb-index and
	remove the directory.
	* gdb.dwarf2/per-bfd-sharing.exp: Likewise.
---
 gdb/testsuite/gdb.base/index-cache.exp       | 2 ++
 gdb/testsuite/gdb.dwarf2/per-bfd-sharing.exp | 3 +++
 2 files changed, 5 insertions(+)

diff --git a/gdb/testsuite/gdb.base/index-cache.exp b/gdb/testsuite/gdb.base/index-cache.exp
index 13d5532..1ae8252 100644
--- a/gdb/testsuite/gdb.base/index-cache.exp
+++ b/gdb/testsuite/gdb.base/index-cache.exp
@@ -226,3 +226,5 @@ test_cache_enabled_hit $cache_dir
 # Test again with the cache disabled, now that it is populated.
 test_cache_disabled $cache_dir "after populate"
 
+remote_exec host sh "-c \"rm -f $cache_dir/*.gdb-index\""
+remote_exec host rmdir "$cache_dir"
diff --git a/gdb/testsuite/gdb.dwarf2/per-bfd-sharing.exp b/gdb/testsuite/gdb.dwarf2/per-bfd-sharing.exp
index 22ab91f..bc0d2eb 100644
--- a/gdb/testsuite/gdb.dwarf2/per-bfd-sharing.exp
+++ b/gdb/testsuite/gdb.dwarf2/per-bfd-sharing.exp
@@ -91,3 +91,6 @@ foreach_with_prefix first $methods {
 	}
     }
 }
+
+remote_exec host sh "-c \"rm -f $cache_dir/*.gdb-index\""
+remote_exec host rmdir "$cache_dir"
-- 
1.9.1

Re: [PP?] [PATCH] testsuite: Cleanup some temp dirs with gdb-index files

[Simon Marchi]

On 2021-05-14 9:20 a.m., Bernd Edlinger wrote:
> Hi,
> 
> I'd like to cleanup some files/directories which
> remain after the gdb testsute runs.
> 
> I want to avoid using "rm -rf ..." since that can be dangerous.
> Therefore I remove the *.gdb-index files, if any, and use
> rmdir instead.  I am not sure if there is a better way,
> instead of using "remote_exec host sh -c" to do the globbing.
> 
> Tested on x86_64-pc-linux-gnu.
> Is it OK for trunk?
> 
> 
> gdb/testsuite:
> 2021-05-14  Bernd Edlinger  <bernd.edlinger@hotmail.de>
> 
>         * gdb.base/index-cache.exp: Cleanup $cache_dir/*.gdb-index and
>         remove the directory.
>         * gdb.dwarf2/per-bfd-sharing.exp: Likewise.
> 

LGTM, but I have one question: if the rm or rmdir ever fails (because
there are additional files to delete, for example), would we know about
it or would it silently fail?  I think it would be nice if the error
was noisy so we could fix it.

Simon

Re: [PP?] [PATCH] testsuite: Cleanup some temp dirs with gdb-index files

[Bernd Edlinger]

[-- Attachment #1: Type: text/plain, Size: 1116 bytes --]

On 5/14/21 3:26 PM, Simon Marchi wrote:
> On 2021-05-14 9:20 a.m., Bernd Edlinger wrote:
>> Hi,
>>
>> I'd like to cleanup some files/directories which
>> remain after the gdb testsute runs.
>>
>> I want to avoid using "rm -rf ..." since that can be dangerous.
>> Therefore I remove the *.gdb-index files, if any, and use
>> rmdir instead.  I am not sure if there is a better way,
>> instead of using "remote_exec host sh -c" to do the globbing.
>>
>> Tested on x86_64-pc-linux-gnu.
>> Is it OK for trunk?
>>
>>
>> gdb/testsuite:
>> 2021-05-14  Bernd Edlinger  <bernd.edlinger@hotmail.de>
>>
>>         * gdb.base/index-cache.exp: Cleanup $cache_dir/*.gdb-index and
>>         remove the directory.
>>         * gdb.dwarf2/per-bfd-sharing.exp: Likewise.
>>
> 
> LGTM, but I have one question: if the rm or rmdir ever fails (because
> there are additional files to delete, for example), would we know about
> it or would it silently fail?  I think it would be nice if the error
> was noisy so we could fix it.
> 

Ah Yes, good point, that would be as silent as before.

So, how about this new version?


Thanks
Bernd.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-testsuite-Cleanup-some-temp-dirs-with-gdb-index-file.patch --]
[-- Type: text/x-patch; name="0001-testsuite-Cleanup-some-temp-dirs-with-gdb-index-file.patch", Size: 1828 bytes --]

From 04569ed1ff0f2ee09352f3aaee7ce517766b2aec Mon Sep 17 00:00:00 2001
From: Bernd Edlinger <bernd.edlinger@hotmail.de>
Date: Fri, 14 May 2021 14:53:19 +0200
Subject: [PATCH] testsuite: Cleanup some temp dirs with gdb-index files

After the gdb test-suite runs there are some files
left in /tmp/tmp*/*.gdb-index, remove those files
and the directory at the end of the test case.

gdb/testsuite:
2021-05-14  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* gdb.base/index-cache.exp: Cleanup $cache_dir/*.gdb-index and
	remove the directory.
	* gdb.dwarf2/per-bfd-sharing.exp: Likewise.
---
 gdb/testsuite/gdb.base/index-cache.exp       | 6 ++++++
 gdb/testsuite/gdb.dwarf2/per-bfd-sharing.exp | 7 +++++++
 2 files changed, 13 insertions(+)

diff --git a/gdb/testsuite/gdb.base/index-cache.exp b/gdb/testsuite/gdb.base/index-cache.exp
index 13d5532..de581f7 100644
--- a/gdb/testsuite/gdb.base/index-cache.exp
+++ b/gdb/testsuite/gdb.base/index-cache.exp
@@ -226,3 +226,9 @@ test_cache_enabled_hit $cache_dir
 # Test again with the cache disabled, now that it is populated.
 test_cache_disabled $cache_dir "after populate"
 
+remote_exec host sh "-c \"rm -f $cache_dir/*.gdb-index\""
+lassign [remote_exec host rmdir "$cache_dir"] ret
+if { $ret != 0 } {
+    fail "couldn't remove temporary cache dir"
+    return
+}
diff --git a/gdb/testsuite/gdb.dwarf2/per-bfd-sharing.exp b/gdb/testsuite/gdb.dwarf2/per-bfd-sharing.exp
index 22ab91f..3717825 100644
--- a/gdb/testsuite/gdb.dwarf2/per-bfd-sharing.exp
+++ b/gdb/testsuite/gdb.dwarf2/per-bfd-sharing.exp
@@ -91,3 +91,10 @@ foreach_with_prefix first $methods {
 	}
     }
 }
+
+remote_exec host sh "-c \"rm -f $cache_dir/*.gdb-index\""
+lassign [remote_exec host rmdir "$cache_dir"] ret
+if { $ret != 0 } {
+    fail "couldn't remove temporary cache dir"
+    return
+}
-- 
1.9.1

Re: [PP?] [PATCH] testsuite: Cleanup some temp dirs with gdb-index files

[Simon Marchi]

On 2021-05-14 11:03 a.m., Bernd Edlinger wrote:
> On 5/14/21 3:26 PM, Simon Marchi wrote:
>> On 2021-05-14 9:20 a.m., Bernd Edlinger wrote:
>>> Hi,
>>>
>>> I'd like to cleanup some files/directories which
>>> remain after the gdb testsute runs.
>>>
>>> I want to avoid using "rm -rf ..." since that can be dangerous.
>>> Therefore I remove the *.gdb-index files, if any, and use
>>> rmdir instead.  I am not sure if there is a better way,
>>> instead of using "remote_exec host sh -c" to do the globbing.
>>>
>>> Tested on x86_64-pc-linux-gnu.
>>> Is it OK for trunk?
>>>
>>>
>>> gdb/testsuite:
>>> 2021-05-14  Bernd Edlinger  <bernd.edlinger@hotmail.de>
>>>
>>>         * gdb.base/index-cache.exp: Cleanup $cache_dir/*.gdb-index and
>>>         remove the directory.
>>>         * gdb.dwarf2/per-bfd-sharing.exp: Likewise.
>>>
>>
>> LGTM, but I have one question: if the rm or rmdir ever fails (because
>> there are additional files to delete, for example), would we know about
>> it or would it silently fail?  I think it would be nice if the error
>> was noisy so we could fix it.
>>
> 
> Ah Yes, good point, that would be as silent as before.
> 
> So, how about this new version?

LGTM, but while at it let's check the return value of both commands.

Simon

Re: [PP?] [PATCH] testsuite: Cleanup some temp dirs with gdb-index files

[Bernd Edlinger]

[-- Attachment #1: Type: text/plain, Size: 1629 bytes --]

On 5/14/21 5:23 PM, Simon Marchi wrote:
> 
> 
> On 2021-05-14 11:03 a.m., Bernd Edlinger wrote:
>> On 5/14/21 3:26 PM, Simon Marchi wrote:
>>> On 2021-05-14 9:20 a.m., Bernd Edlinger wrote:
>>>> Hi,
>>>>
>>>> I'd like to cleanup some files/directories which
>>>> remain after the gdb testsute runs.
>>>>
>>>> I want to avoid using "rm -rf ..." since that can be dangerous.
>>>> Therefore I remove the *.gdb-index files, if any, and use
>>>> rmdir instead.  I am not sure if there is a better way,
>>>> instead of using "remote_exec host sh -c" to do the globbing.
>>>>
>>>> Tested on x86_64-pc-linux-gnu.
>>>> Is it OK for trunk?
>>>>
>>>>
>>>> gdb/testsuite:
>>>> 2021-05-14  Bernd Edlinger  <bernd.edlinger@hotmail.de>
>>>>
>>>>         * gdb.base/index-cache.exp: Cleanup $cache_dir/*.gdb-index and
>>>>         remove the directory.
>>>>         * gdb.dwarf2/per-bfd-sharing.exp: Likewise.
>>>>
>>>
>>> LGTM, but I have one question: if the rm or rmdir ever fails (because
>>> there are additional files to delete, for example), would we know about
>>> it or would it silently fail?  I think it would be nice if the error
>>> was noisy so we could fix it.
>>>
>>
>> Ah Yes, good point, that would be as silent as before.
>>
>> So, how about this new version?
> 
> LGTM, but while at it let's check the return value of both commands.
> 

Yeah Okay. I think then I should also remove the -f switch from the rm command,
so it has something to complain about when there is nothing to there to delete,
since there should always be one index file at the end of the tests which have
just completed. 


So like this?


Thanks
Bernd.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-testsuite-Cleanup-some-temp-dirs-with-gdb-index-file.patch --]
[-- Type: text/x-patch; name="0001-testsuite-Cleanup-some-temp-dirs-with-gdb-index-file.patch", Size: 2051 bytes --]

From 7301662ef97336f2eb333e81c71d2181b2dd953f Mon Sep 17 00:00:00 2001
From: Bernd Edlinger <bernd.edlinger@hotmail.de>
Date: Fri, 14 May 2021 14:53:19 +0200
Subject: [PATCH] testsuite: Cleanup some temp dirs with gdb-index files

After the gdb test-suite runs there are some files
left in /tmp/tmp*/*.gdb-index, remove those files
and the directory at the end of the test case.

gdb/testsuite:
2021-05-14  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* gdb.base/index-cache.exp: Cleanup $cache_dir/*.gdb-index and
	remove the directory.
	* gdb.dwarf2/per-bfd-sharing.exp: Likewise.
---
 gdb/testsuite/gdb.base/index-cache.exp       | 11 +++++++++++
 gdb/testsuite/gdb.dwarf2/per-bfd-sharing.exp | 12 ++++++++++++
 2 files changed, 23 insertions(+)

diff --git a/gdb/testsuite/gdb.base/index-cache.exp b/gdb/testsuite/gdb.base/index-cache.exp
index 13d5532..0daf7bb 100644
--- a/gdb/testsuite/gdb.base/index-cache.exp
+++ b/gdb/testsuite/gdb.base/index-cache.exp
@@ -226,3 +226,14 @@ test_cache_enabled_hit $cache_dir
 # Test again with the cache disabled, now that it is populated.
 test_cache_disabled $cache_dir "after populate"
 
+lassign [remote_exec host sh "-c \"rm $cache_dir/*.gdb-index\""] ret
+if { $ret != 0 } {
+    fail "couldn't remove files in temporary cache dir"
+    return
+}
+
+lassign [remote_exec host rmdir "$cache_dir"] ret
+if { $ret != 0 } {
+    fail "couldn't remove temporary cache dir"
+    return
+}
diff --git a/gdb/testsuite/gdb.dwarf2/per-bfd-sharing.exp b/gdb/testsuite/gdb.dwarf2/per-bfd-sharing.exp
index 22ab91f..081088a 100644
--- a/gdb/testsuite/gdb.dwarf2/per-bfd-sharing.exp
+++ b/gdb/testsuite/gdb.dwarf2/per-bfd-sharing.exp
@@ -91,3 +91,15 @@ foreach_with_prefix first $methods {
 	}
     }
 }
+
+lassign [remote_exec host sh "-c \"rm $cache_dir/*.gdb-index\""] ret
+if { $ret != 0 } {
+    fail "couldn't remove files in temporary cache dir"
+    return
+}
+
+lassign [remote_exec host rmdir "$cache_dir"] ret
+if { $ret != 0 } {
+    fail "couldn't remove temporary cache dir"
+    return
+}
-- 
1.9.1

Re: [PP?] [PATCH] testsuite: Cleanup some temp dirs with gdb-index files

[Simon Marchi]

On 2021-05-14 11:39 a.m., Bernd Edlinger wrote:
> On 5/14/21 5:23 PM, Simon Marchi wrote:
>>
>>
>> On 2021-05-14 11:03 a.m., Bernd Edlinger wrote:
>>> On 5/14/21 3:26 PM, Simon Marchi wrote:
>>>> On 2021-05-14 9:20 a.m., Bernd Edlinger wrote:
>>>>> Hi,
>>>>>
>>>>> I'd like to cleanup some files/directories which
>>>>> remain after the gdb testsute runs.
>>>>>
>>>>> I want to avoid using "rm -rf ..." since that can be dangerous.
>>>>> Therefore I remove the *.gdb-index files, if any, and use
>>>>> rmdir instead.  I am not sure if there is a better way,
>>>>> instead of using "remote_exec host sh -c" to do the globbing.
>>>>>
>>>>> Tested on x86_64-pc-linux-gnu.
>>>>> Is it OK for trunk?
>>>>>
>>>>>
>>>>> gdb/testsuite:
>>>>> 2021-05-14  Bernd Edlinger  <bernd.edlinger@hotmail.de>
>>>>>
>>>>>         * gdb.base/index-cache.exp: Cleanup $cache_dir/*.gdb-index and
>>>>>         remove the directory.
>>>>>         * gdb.dwarf2/per-bfd-sharing.exp: Likewise.
>>>>>
>>>>
>>>> LGTM, but I have one question: if the rm or rmdir ever fails (because
>>>> there are additional files to delete, for example), would we know about
>>>> it or would it silently fail?  I think it would be nice if the error
>>>> was noisy so we could fix it.
>>>>
>>>
>>> Ah Yes, good point, that would be as silent as before.
>>>
>>> So, how about this new version?
>>
>> LGTM, but while at it let's check the return value of both commands.
>>
> 
> Yeah Okay. I think then I should also remove the -f switch from the rm command,
> so it has something to complain about when there is nothing to there to delete,
> since there should always be one index file at the end of the tests which have
> just completed. 
> 
> 
> So like this?
> 
> 
> Thanks
> Bernd.
> 

That LGTM, thanks!

Simon