Patchwork [RFA] Fix gdb snapshots

login
register
mail settings
Submitter Tom Tromey
Date Nov. 29, 2017, 4:31 p.m.
Message ID <20171129163158.18968-1-tom@tromey.com>
Download mbox | patch
Permalink /patch/24599/
State New
Headers show

Comments

Tom Tromey - Nov. 29, 2017, 4:31 p.m.
Joel pointed out that gdb snapshots were broken by my Makefile patch
series.  The bug is that rmdir in distclean was failing, because the
directories in question did not exist.  The simplest fix was to just use
"rm -rf", which won't fail if the directory is missing.

Tested using "src-release.sh gdb".

2017-11-29  Tom Tromey  <tom@tromey.com>

	* Makefile.in (distclean): Use "rm -rf", not "rmdir".
---
 gdb/ChangeLog   | 4 ++++
 gdb/Makefile.in | 4 +++-
 2 files changed, 7 insertions(+), 1 deletion(-)
Simon Marchi - Nov. 29, 2017, 4:41 p.m.
On 2017-11-29 11:31, Tom Tromey wrote:
> Joel pointed out that gdb snapshots were broken by my Makefile patch
> series.  The bug is that rmdir in distclean was failing, because the
> directories in question did not exist.  The simplest fix was to just 
> use
> "rm -rf", which won't fail if the directory is missing.
> 
> Tested using "src-release.sh gdb".
> 
> 2017-11-29  Tom Tromey  <tom@tromey.com>
> 
> 	* Makefile.in (distclean): Use "rm -rf", not "rmdir".
> ---
>  gdb/ChangeLog   | 4 ++++
>  gdb/Makefile.in | 4 +++-
>  2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index ebb969998c..7532016499 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,7 @@
> +2017-11-29  Tom Tromey  <tom@tromey.com>
> +
> +	* Makefile.in (distclean): Use "rm -rf", not "rmdir".
> +
>  2017-11-27  Tom Tromey  <tom@tromey.com>
> 
>  	* Makefile.in (REMOTE_OBS): Remove.
> diff --git a/gdb/Makefile.in b/gdb/Makefile.in
> index 6e16bc6682..39f90bad9f 100644
> --- a/gdb/Makefile.in
> +++ b/gdb/Makefile.in
> @@ -1995,7 +1995,9 @@ distclean: clean
>  	rm -f Makefile
>  	rm -rf $(DEPDIR)
>  	for i in $(CONFIG_SRC_SUBDIR); do \
> -		rmdir $$i/$(DEPDIR); \
> +		# Use rm -rf, not rmdir, to avoid errors when the \
> +		# directory does not exist. \
> +		rm -rf $$i/$(DEPDIR); \
>  	done
> 
>  maintainer-clean: local-maintainer-clean do-maintainer-clean distclean

As always, I am really not comfortable with using rm -rf in scripts.

Ref: https://github.com/MrMEEE/bumblebee-Old-and-abbandoned/issues/123

Since we know that the .deps directories will only contain files, can we 
do something like this instead (not tested)?

rm -f $$i/$(DEPDIR)/*
rmdir $$i/$(DEPDIR)

Simon
Tom Tromey - Nov. 29, 2017, 5 p.m.
>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:

Simon> As always, I am really not comfortable with using rm -rf in scripts.

You'll be disappointed by the current Makefile then :)

Simon> Since we know that the .deps directories will only contain files, can
Simon> we do something like this instead (not tested)?
Simon> rm -f $$i/$(DEPDIR)/*
Simon> rmdir $$i/$(DEPDIR)

The issue is that this rmdir will fail because, in this situation, the
directory does not exist at all.

Perhaps rmdir||true will be more to your liking.

Tom
Simon Marchi - Nov. 29, 2017, 5:05 p.m.
On 2017-11-29 12:00, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:
> 
> Simon> As always, I am really not comfortable with using rm -rf in 
> scripts.
> 
> You'll be disappointed by the current Makefile then :)
> 
> Simon> Since we know that the .deps directories will only contain 
> files, can
> Simon> we do something like this instead (not tested)?
> Simon> rm -f $$i/$(DEPDIR)/*
> Simon> rmdir $$i/$(DEPDIR)
> 
> The issue is that this rmdir will fail because, in this situation, the
> directory does not exist at all.
> 
> Perhaps rmdir||true will be more to your liking.

It's too bad rmdir does not have a -f switch like rm...  To avoid 
printing an error message (No such file or directory) when the directory 
does not exist, you could do:

[ -d $$i/$(DEPDIR) ] && rmdir $$i/$(DEPDIR)

or

test -d $$i/$(DEPDIR) && $$i/$(DEPDIR)

Simon

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index ebb969998c..7532016499 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,7 @@ 
+2017-11-29  Tom Tromey  <tom@tromey.com>
+
+	* Makefile.in (distclean): Use "rm -rf", not "rmdir".
+
 2017-11-27  Tom Tromey  <tom@tromey.com>
 
 	* Makefile.in (REMOTE_OBS): Remove.
diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 6e16bc6682..39f90bad9f 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -1995,7 +1995,9 @@  distclean: clean
 	rm -f Makefile
 	rm -rf $(DEPDIR)
 	for i in $(CONFIG_SRC_SUBDIR); do \
-		rmdir $$i/$(DEPDIR); \
+		# Use rm -rf, not rmdir, to avoid errors when the \
+		# directory does not exist. \
+		rm -rf $$i/$(DEPDIR); \
 	done
 
 maintainer-clean: local-maintainer-clean do-maintainer-clean distclean