[Bug gdb/31420] New: Use std::filesystem::remove_all in compile.c

[Bug gdb/31420] New: Use std::filesystem::remove_all in compile.c

[tromey at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=31420

            Bug ID: 31420
           Summary: Use std::filesystem::remove_all in compile.c
           Product: gdb
           Version: HEAD
            Status: NEW
          Severity: normal
          Priority: P2
         Component: gdb
          Assignee: unassigned at sourceware dot org
          Reporter: tromey at sourceware dot org
  Target Milestone: ---

compile.c uses system() to remove a temporary directory.
This could use std::filesystem::remove_all instead.

There may be other places std::filesystem could be used.
Auditing filestuff.cc may be worthwhile.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/31420] Use std::filesystem::remove_all in compile.c

[tromey at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=31420

Tom Tromey <tromey at sourceware dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |31419


Referenced Bugs:

https://sourceware.org/bugzilla/show_bug.cgi?id=31419
[Bug 31419] [meta] Convert GDB to C++17
-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/31420] Use std::filesystem::remove_all in compile.c

[tromey at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=31420

--- Comment #1 from Tom Tromey <tromey at sourceware dot org> ---
If this is done, perhaps final cleanups need to
decide what to do on exception.  See

https://sourceware.org/pipermail/gdb-patches/2024-February/206862.html

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/31420] Use std::filesystem::remove_all in compile.c

[cvs-commit at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=31420

--- Comment #2 from Sourceware Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Lancelot SIX <lsix@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=7bba0ad08576309763e3f41193eaa93025e10b8b

commit 7bba0ad08576309763e3f41193eaa93025e10b8b
Author: Lancelot SIX <lancelot.six@amd.com>
Date:   Sun Mar 3 16:47:56 2024 +0000

    gdb/compile: Use std::filesystem::remove_all in cleanup

    In a previous review, I noticed that some code in gdb/compile/compile.c
    could use c++17's `std::filesystem::remove_all` instead of using some
    `system ("rm -rf ...");`.

    This patch implements this.

    Note that I use the noexcept overload of std::filesystem::remove_all and
    explicitly check for an error code.  This means that this code called
    during the cleanup procedure cannot throw, and does not risk preventing
    other cleanup functions to be called.

    Tested on x86_64-linux.

    Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31420
    Change-Id: If5668bf3e15e66c020e5c3b4fa999f861690e4cf
    Approved-By: Tom Tromey <tom@tromey.com>

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/31420] Use std::filesystem::remove_all in compile.c

[cvs-commit at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=31420

--- Comment #3 from Sourceware Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Lancelot SIX <lsix@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=f89ae595dd1f5195dd6e8e57bc2217463c436888

commit f89ae595dd1f5195dd6e8e57bc2217463c436888
Author: Lancelot SIX <lancelot.six@amd.com>
Date:   Wed Apr 3 15:39:10 2024 +0100

    Revert "gdb/compile: Use std::filesystem::remove_all in cleanup"

    This reverts commit 7bba0ad08576309763e3f41193eaa93025e10b8b.

    Tom de Vries reported that 7bba0ad0857 (gdb/compile: Use
    std::filesystem::remove_all in cleanup) broke builds with gcc-7.5.0
    which mostly supports c++17, but not std::filesystem[1].  As this change
    is not critical, revert it to maintain compatibility.

    [1]
https://inbox.sourceware.org/gdb-patches/a06e6483-aa2e-4b8a-854f-e369a1e961ea@suse.de/

    Change-Id: I58150bd27600c95052bdf1bbbd6b44718a5a0bbf
    Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31420
    Approved-By: Tom Tromey <tom@tromey.com>

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/31420] Use std::filesystem::remove_all in compile.c

[lsix at lancelotsix dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=31420

Lancelot SIX <lsix at lancelotsix dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |lsix at lancelotsix dot com

--- Comment #4 from Lancelot SIX <lsix at lancelotsix dot com> ---
I had a look a t various compiler versions.

- gcc-7.5 does not have std::filesystem support available, so if we want to
keep supporting this compiler, we can't move to remove_all.
- gcc-8 has std::filesystem, but using it require using "-lstdc++fs". 
Supporting this compiler would require an extra configure step to check if this
flag is required.
- starting gcc-9, std::filesytem is available without extra compiler flags.

For now, I think we will try to maintain gcc-7.5 compatibility as it is still
used.  This will need to be revisited at some point.

-- 
You are receiving this mail because:
You are on the CC list for the bug.