Re: [PATCHv2 1/2] gdb/guile: perform tilde expansion when sourcing guile scripts

[Andrew Burgess <andrew.burgess@embecosm.com>]

* Simon Marchi <simon.marchi@polymtl.ca> [2021-05-05 22:23:11 -0400]:

> 
> 
> On 2021-05-05 5:01 p.m., Andrew Burgess wrote:
> > Before this patch:
> > 
> >   (gdb) source ~/script.scm
> >   ERROR: In procedure apply-smob/1:
> >   ERROR: In procedure primitive-load-path: Unable to find file "~/script.scm" in load path
> >   Error while executing Scheme code.
> >   (gdb)
> > 
> > This is because the path is not tilde expanded.  In contrast, when
> > sourcing a .py or .gdb script the path is tilde expanded.
> > 
> > This commit fixes this oversight, and allows the above source command
> > to work as expected.
> > 
> > The tilde expansion is done in guile/scm-safe-call.c rather than at a
> > higher level, for example, it might be tempting to add the tilde
> > expansion in cli/cli-cmds.c:source_script_from_stream, however, not
> > every extension language wants to see a tilde expanded file path.  For
> > example, consider Python.  Currently we pass in an unexpanded path and
> > we see this behaviour:
> > 
> >   (gdb) source ~/tmp/xxx.py
> >   Traceback (most recent call last):
> >     File "~/tmp/xxx.py", line 1, in <module>
> >   NameError: name 'undefined' is not defined
> > 
> > If we performed tilde expansion prior to calling the Python sourcer
> > function (gdbpy_source_script), we would instead see:
> > 
> >   (gdb) source ~/tmp/xxx.py
> >   Traceback (most recent call last):
> >     File "/home/andrew/tmp/xxx.py", line 1, in <module>
> >       undefined
> >   NameError: name 'undefined' is not defined
> > 
> > Notice the path expansion in the second line of the error message, I
> > believe this is a change for the worse.
> 
> I really don't have a strong opinion, but I don't think this would be a
> problematic change.  The tilde-expansion thing is really a shell thing
> (and we replicate it in GDB).  But once the paths are processed by
> programs such as Python, the tilde usually doesn't have a special
> meaning.
> 
> If I do:
> 
>     $ python3 ~/foo.py
> 
> I get:
> 
>     Traceback (most recent call last):
>       File "/home/simark/foo.py", line 1, in <module>
> 	blah
>     NameError: name 'blah' is not defined
> 
> Because the shell did the tilde-expansion before passing the file name
> to Python.  If I do:
> 
>     $ python3 '~/foo.py'
> 
> then I get:
> 
>     python3: can't open file '/home/simark/build/binutils-gdb-one-target/gdb/~/foo.py': [Errno 2] No such file or directory
> 
> Python shows the tilde version of the path in our error messages just
> becore we tell it that's the name of the file, Python doesn't use that
> name otherwise (I think).  We could tell it the filename is "potato",
> and it would say that the error is in file "potato".
> 
> So in a sense, passing the tilde-expanded version would make it look
> more like the "regular" Python interpreter.
> 
> But like I said, I don't have a strong opinion: in the end this is shown
> to the user, they will understand it either way.
> 
> Otherwise, the patch LGTM.

I think you convinced me.

How's the patch below?

Thanks,
Andrew

---

commit 25d175146367939dca8b45a155fd85fb0cc28092
Author: Andrew Burgess <andrew.burgess@embecosm.com>
Date:   Wed May 5 16:50:17 2021 +0100

    gdb/guile: perform tilde expansion when sourcing guile scripts
    
    Before this patch:
    
      (gdb) source ~/script.scm
      ERROR: In procedure apply-smob/1:
      ERROR: In procedure primitive-load-path: Unable to find file "~/script.scm" in load path
      Error while executing Scheme code.
      (gdb)
    
    This is because the path is not tilde expanded.  In contrast, when
    sourcing a .py or .gdb script the path is tilde expanded.
    
    This commit fixes this oversight, and allows the above source command
    to work as expected.
    
    The tilde expansion is done in the generic GDB code before we call the
    sourcer function for any particular extension language.
    
    gdb/ChangeLog:
    
            * cli/cli-cmds.c: Add 'gdbsupport/gdb_tilde_expand.h'
            include.
            (source_script_with_search): Perform tilde expansion.
    
    gdb/testsuite/ChangeLog:
    
            * gdb.guile/guile.exp: Add an extra test.

diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index 68ef92eca4c..c641b9db13d 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -54,6 +54,7 @@
 
 #include "extension.h"
 #include "gdbsupport/pathstuff.h"
+#include "gdbsupport/gdb_tilde_expand.h"
 
 #ifdef TUI
 #include "tui/tui.h"	/* For tui_active et.al.  */
@@ -737,8 +738,10 @@ source_script_with_search (const char *file, int from_tty, int search_path)
      anyway so that error messages show the actual file used.  But only do
      this if we (may have) used search_path, as printing the full path in
      errors for the non-search case can be more noise than signal.  */
+  gdb::unique_xmalloc_ptr<char> file_to_open = gdb_tilde_expand_up (file);
   source_script_from_stream (opened->stream.get (), file,
-			     search_path ? opened->full_path.get () : file);
+			     search_path ? opened->full_path.get ()
+			     : file_to_open.get ());
 }
 
 /* Wrapper around source_script_with_search to export it to main.c
diff --git a/gdb/testsuite/gdb.guile/guile.exp b/gdb/testsuite/gdb.guile/guile.exp
index 6e464cc0e77..0fb82284f46 100644
--- a/gdb/testsuite/gdb.guile/guile.exp
+++ b/gdb/testsuite/gdb.guile/guile.exp
@@ -82,3 +82,10 @@ gdb_test_no_output "guile (define a (execute \"help\" #:to-string #t))" \
 
 gdb_test "guile (print a)" "= .*aliases -- User-defined aliases of other commands.*" \
     "verify help to uiout"
+
+# Verify that we can source a guile script using ~ for the HOME directory.
+save_vars { env(HOME) } {
+    set env(HOME) $srcdir/$subdir
+    clean_restart
+    gdb_test "source ~/source2.scm" "yes"
+}