Close terminal after LaunchInTerminalTestCase test

[ki.stfu]

Summary

The test_launch_in_terminal test leaves a running terminal.
This patch adds "exit" after debugging with eLaunchFlagLaunchInTTY flag.

[vharron]

I'm not sure that this is desirable. I don't like having a bunch of stale
terminals left around but its worse when you need to see the output in the
terminal and the terminal disappears

[ki.stfu]

Ok, but:

1. You can set "don't close after exit" flag in settings
2. When we run this test on buildbot we get open terminal each time when it's executed.

Maybe we can fix this test but I don't know how to close terminal without its pid.

[clayborg]

We could add a new "settings set" setting that allows us to control if terminals close down after executing and we could set this from the test suite permanently. Search for "thread-format" and add a new bool setting.

[vharron]

You can set "don't close after exit" flag in settings

Awesome

FWIW, I prefer flags being expressed in positive terms. I think it
significantly improves understanding. (e.g. "close after exit" is better
than "don't close after exit") I would also default to "close after exit"

false

UI presentation:

• Launch in new TTY
  • Close TTY after process exit

[zturner]

+1 to expressing flags in positive terms.

[jingham]

A long-standing bug in the lldb command help system is that it doesn't print what the default value of boolean flags is. So for instance you get:

(lldb) help process launch

Launch the executable in the debugger.

Syntax: process launch <cmd-options> [<run-args>]

Command Options Usage:

process launch [-s] [-A <boolean>] [-p <plugin>] [-w <directory>] [-a <arch>] [-v <none>] [-c[<filename>]] [-i <filename>] [-o <filename>] [-e <filename>] [<run-args>]
process launch [-st] [-A <boolean>] [-p <plugin>] [-w <directory>] [-a <arch>] [-v <none>] [-c[<filename>]] [<run-args>]
process launch [-ns] [-A <boolean>] [-p <plugin>] [-w <directory>] [-a <arch>] [-v <none>] [-c[<filename>]] [<run-args>]

     -A <boolean> ( --disable-aslr <boolean> )
          Set whether to disable address space layout randomization when launching a process.

You can't tell what you are going to get if don't specify -A. So I try to write the option so that it indicates what the default value is. In this case, if you say nothing we'll disable ASLR. This is obviously just a hack to work around an omission in the help system. And in a couple of places (like --one-shot in the "breakpoint set" command where I don't do this because it would have made the name really non-obvious.)

That's a pretty weak reason for the naming, and we really should include the default value - at least for booleans - in the command table and then use it in the help system.

Jim

[zturner]

This sounds like an easy patch. Is it subtly more complicated than it
sounds? If it's actually easy as it sounds, I'll just do it.

[jingham]

It's actually a bit of work.

Right now, all the default values are set in the OptionParsingStarting method of the Options subclass in the CommandObject subclass that implements the command. So the only entity that knows the default value of an option is the state of the associated ivar in the Option object right after OptionParsingStarting. And the relation between option flag and the variable that stores the flag value in the Options (and thus has the default value) is some arbitrary choice made by the Option class. There isn't even a general way to ask: "what ivar stores the option value for command option 'a'?" since it's up to Options::SetOptionValue to stuff values in ivars, and only the parse function knows which ivar went with which option flag.

You'd have to come up with some way to encode default values in the options tables, and then change over OptionParsingStarting to read the default values from the tables. You'd also have to deal with the OptionGroups which do things differently. I think once you started into a task like this, you'd end up rewriting a substantial portion of the option parsing machinery. That in itself would not be a bad thing, there are some crusty aspects of the command parser in general that we should revise at some point. But it ain't a Friday afternoon hack.

Jim

[ki.stfu]

Add close-after-exit debugger's option; Add eLaunchFlagCloseTTYOnExit; Fix test

[ki.stfu]

Fix test in Diff 2

[ki.stfu]

I had faced with problem how to get debugger's property in Host.mm file. I decided to pass ePropertyCloseAfterExit through launch_info using the eLaunchFlagCloseTTYOnExit flag. It looks good for me so I wanna remove close-after-exit debugger's property and use only launch flags. I'll make patch later.

[ki.stfu]

Remove close-after-exit debugger's option (use eLaunchFlagCLoseTTYOnExit instead); Update test

include/lldb/lldb-enumerations.h

typedef enum LaunchFlags
         eLaunchFlagLaunchInTTY  = (1u << 5),       ///< Launch the process in a new TTY if supported by the host
         eLaunchFlagLaunchInShell= (1u << 6),       ///< Launch the process inside a shell to get shell expansion
         eLaunchFlagLaunchInSeparateProcessGroup = (1u << 7), ///< Launch the process in a separate process group
         eLaunchFlagDontSetExitStatus = (1u << 8), ///< If you are going to hand the process off (e.g. to debugserver)
                                                    ///< set this flag so lldb & the handee don't race to set its exit status.
         eLaunchFlagDetachOnError = (1u << 9),     ///< If set, then the client stub should detach rather than killing the debugee
                                                    ///< if it loses connection with lldb.
         eLaunchFlagGlobArguments  = (1u << 10),       ///< Glob arguments without going through a shell
+        eLaunchFlagCloseTTYOnExit = (1u << 11),    ///< Close the open TTY on exit
     } LaunchFlags;
         
     //----------------------------------------------------------------------
     // Thread Run Modes
     //----------------------------------------------------------------------
     typedef enum RunMode {
         eOnlyThisThread,
         eAllThreads,

source/Host/macosx/Host.mm

if (argv)
                 command.Printf(" '%s'", argv[i]);
         }
     }
     else
     {
         command.Printf(" '%s'", exe_path);
     }
     command.PutCString (" ; echo Process exited with status $?");
+    if (launch_info.GetFlags().Test(lldb::eLaunchFlagCloseTTYOnExit))
+        command.PutCString (" ; exit");
     
     StreamString applescript_source;
 
     const char *tty_command = command.GetString().c_str();
 //    if (tty_name && tty_name[0])
 //    {
 //        applescript_source.Printf (applscript_in_existing_tty, 
 //                                   tty_command,

test/functionalities/tty/TestTerminal.py

class LaunchInTerminalTestCase(TestBase):
     # Do we need to disable this test if the testsuite is being run on a rmeote system?
     # This env var is only defined when the shell is running in a local mac terminal window
     # @unittest2.skipUnless(os.environ.has_key('TERM_PROGRAM'), "test must be run on local system")
 
     def test_launch_in_terminal (self):
         exe = "/bin/ls"
         target = self.dbg.CreateTarget(exe)
         launch_info = lldb.SBLaunchInfo(["-lAF", "/tmp/"])
-        launch_info.SetLaunchFlags(lldb.eLaunchFlagLaunchInTTY)
+        launch_info.SetLaunchFlags(lldb.eLaunchFlagLaunchInTTY | lldb.eLaunchFlagCloseTTYOnExit)
         error = lldb.SBError()
         process = target.Launch (launch_info, error)
         self.assertTrue(error.Success(), "Make sure launch happened successfully in a terminal window")
         # Running in synchronous mode our process should have run and already exited by the time target.Launch() returns
         self.assertTrue(process.GetState() == lldb.eStateExited)
 
 if __name__ == '__main__':
     import atexit
     lldb.SBDebugger.Initialize()
     atexit.register(lambda: lldb.SBDebugger.Terminate())
     unittest2.main()