Make File option flags consistent for Python API

[fjricci]

Summary

Fixes SBCommandReturnObject::SetImmediateOutputFile() and
SBCommandReturnObject::SetImmediateOutputFile() for files opened
with "a" or "a+" by resolving inconsistencies between File and
our Python parsing of file objects.

[clayborg]

source/Host/common/File.cpp

48

This change isn't needed if "File::eOpenOptionWrite" is specified in the "a+" spec below...

source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp

1258

Docs for this say:

``a+''  Open for reading and writing.  The file is created if it does not exist.  The stream is posi-tioned positioned
        tioned at the end of the file.  Subsequent writes to the file will always end up at the then
        current end of file, irrespective of any intervening fseek(3) or similar.

Why is the File:: eOpenOptionWrite not included for "a+"?

[fjricci]

So this was definitely a decision that I was debating. But I assume that the Append flag must imply the Write flag, since you can't open a file for Append without also opening it for write ("a" and "a+" both include write privileges). So I figured that having both "Append" and "Write" was redundant. And since "a" was previously using "Append" while "a+" was previously using "Write", I think that one of them needed to be changed to be consistent with the other. I can do it the other way around though if that's preferable.

[clayborg]

Just to be clear in the code we should set the write flag and not make any assumptions. So then the first diff isn't needed and only PythonDataObjects.cpp needs to change.

[fjricci]

That makes sense. Will do.

[fjricci]

Always use write flag, even in append mode

[clayborg]

Looks great, thanks for catching and fixing this. Can we add a test for this so we don't regress?

[fjricci]

I want to add this testing to functionalities/command_script_immediate_output/TestCommandScriptImmediateOutput.py, since that test is already very basic coverage of the same functionality.

However, it appears that TestCommandScriptImmediateOutput is an expected fail on FreeBSD, Linux, and Windows. And when I run it on the master branch on OSX, fails as ERROR. So it might be that the test doesn't actually pass on any platform.

When I run locally on Linux, the test succeeds, so I extended the test and made sure it passes Linux locally. But I'm not sure how useful that is, since the test is currently disabled on Linux.

I'll upload the test I wrote and we can go from there.

[fjricci]

Added unit test for python file api

[fjricci]

Friendly ping.

[sas]

Committed as r264351.

source/Host/common/File.cpp

GetStreamOpenModeFromOptions (uint32_t options)
     {
         if (options & File::eOpenOptionRead)
         {
             if (options & File::eOpenOptionCanCreateNewOnly)
                 return "a+x";
             else
                 return "a+";
         }
-        else if (options & File::eOpenOptionWrite)
+        else
         {
             if (options & File::eOpenOptionCanCreateNewOnly)
                 return "ax";
             else
                 return "a";
         }
     }
     else if (options & File::eOpenOptionRead && options & File::eOpenOptionWrite)

source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp

if (mode.empty())
         return 0;
 
     return llvm::StringSwitch<uint32_t>(mode.str().c_str())
     .Case("r",   File::eOpenOptionRead)
     .Case("w",   File::eOpenOptionWrite)
     .Case("a",   File::eOpenOptionAppend|File::eOpenOptionCanCreate)
     .Case("r+",  File::eOpenOptionRead|File::eOpenOptionWrite)
     .Case("w+",  File::eOpenOptionRead|File::eOpenOptionWrite|File::eOpenOptionCanCreate|File::eOpenOptionTruncate)
-    .Case("a+",  File::eOpenOptionRead|File::eOpenOptionWrite|File::eOpenOptionCanCreate)
+    .Case("a+",  File::eOpenOptionAppend|File::eOpenOptionRead|File::eOpenOptionCanCreate)
     .Default(0);
 }
 
 bool
 PythonFile::GetUnderlyingFile(File &file) const
 {
     if (!IsValid())
         return false;