Ticket #442 - adding support for macros & env variables

[WinterMute]

I've put together a patch to expand macros and environment variables in the program arguments and host application boxes. I've also allowed a host application for the ttNative target.

This is useful for launching emulators to run code built with cross compilers as well as for someone  who wanted to run and debug MPI programs via mpiexec.exe (http://forums.codeblocks.org/index.php/topic,19390.new.html)

https://sourceforge.net/p/codeblocks/tickets/442/

Should I open a new ticket for the second patch or is it OK to add it to the ticket as I've done?

[oBFusCATed]

No need to post second ticket.
But is better to post all the information in a single place (the tickets section or in the forum, but not both).

[oBFusCATed]

I've looked at the patches not long ago and now I find the time to answer, sorry for the delay:

I think the first patch is really dangerous because it applies macro replacement to the whole command in the compiler and in the debugger it applies the replacements to the execution parameters separately. I think we should do this consistently otherwise will create oddly behaving system. Also the patch doesn't change the behaviour in the valgrind plugin, so this will break. Can you apply the change to the compiler in the same/separate fashion as in the debugger? As far as I can see the execution parameters are used everywhere, so you can extract them in a variable before the if/else code and apply the macro expansion there. Also I think we need some helper function that do this expansion automatically in a single place, but I'm not sure how to achieve this yet. Probably we need some execution command generator class with some methods like getFullCmd(), getExePath(), getParameters() and then every call site use either getFullCmd or a combination of getExePath and getParameters. And getFullCmd will call getExePath and getParameters, so the behaviour will be consistent and reliable.

For the second: It should be separated in two patches - one that handles the code reordering and one that introduces the improvement. Also it needs to be expanded to support the debugger and valgrind plugins. But I think we should proceed with it after the comments in the first patch have been resolved.

What do you think?

[WinterMute]

Sorry to take so long getting back to this. Christmas got in the way & then I had some other work that needed finished.

I think this addresses your concerns with the first patch

Code

diff --git a/src/plugins/compilergcc/compilergcc.cpp b/src/plugins/compilergcc/compilergcc.cpp
index a88716d..19065d7 100644
--- a/src/plugins/compilergcc/compilergcc.cpp
+++ b/src/plugins/compilergcc/compilergcc.cpp
@@ -1911,7 +1911,6 @@ int CompilerGCC::Run(ProjectBuildTarget* target)
             m_pProject->SetCurrentlyCompilingTarget(0);
             return -1;
         }
-        Manager::Get()->GetMacrosManager()->ReplaceEnvVars(hostapStr);
         command << hostapStr << strSPACE;
         command << target->GetExecutionParameters();
     }
@@ -1936,10 +1935,11 @@ int CompilerGCC::Run(ProjectBuildTarget* target)
         }
         command << hostapStr << strSPACE;
         command << target->GetExecutionParameters();
-        Manager::Get()->GetMacrosManager()->ReplaceMacros(command, target);
-        Manager::Get()->GetMacrosManager()->ReplaceEnvVars(command);
     }
 
+    Manager::Get()->GetMacrosManager()->ReplaceMacros(command, target);
+    Manager::Get()->GetMacrosManager()->ReplaceEnvVars(command);
+
     wxString script = command;
 
     if (platform::macosx)
diff --git a/src/plugins/contrib/Valgrind/Valgrind.cpp b/src/plugins/contrib/Valgrind/Valgrind.cpp
index cbcec86..d279a13 100644
--- a/src/plugins/contrib/Valgrind/Valgrind.cpp
+++ b/src/plugins/contrib/Valgrind/Valgrind.cpp
@@ -269,7 +269,6 @@ bool CheckRequirements(wxString& ExeTarget, wxString &WorkDir, wxString& Command
        // check the type of the target
        const TargetType TType = Target->GetTargetType();
 
-    MacrosManager* MacrosMgr = Manager::Get()->GetMacrosManager();
        if (TType == ttDynamicLib || TType == ttStaticLib)
        {
            if (Target->GetHostApplication().IsEmpty())
@@ -281,13 +280,11 @@ bool CheckRequirements(wxString& ExeTarget, wxString &WorkDir, wxString& Command
         }
 
         ExeTarget = Project->GetBasePath() + Target->GetHostApplication();
-        MacrosMgr->ReplaceMacros(ExeTarget, Target);
         WorkDir = Target->GetWorkingDir();
        }
        else if (TType == ttExecutable || TType == ttConsoleOnly)
        {
         ExeTarget = Project->GetBasePath() + Target->GetOutputFilename();
-        MacrosMgr->ReplaceMacros(ExeTarget, Target);
         WorkDir = Target->GetWorkingDir();
        }
     else
@@ -307,7 +304,15 @@ bool CheckRequirements(wxString& ExeTarget, wxString &WorkDir, wxString& Command
 //             Manager::Get()->GetLogManager()->DebugLog(msg);
 //             return false;
 //     }
+    MacrosManager* MacrosMgr = Manager::Get()->GetMacrosManager();
+    MacrosMgr->ReplaceMacros(ExeTarget, Target);
+    Manager::Get()->GetMacrosManager()->ReplaceEnvVars(ExeTarget);
+
        CommandLineArguments = Target->GetExecutionParameters();
+
+    MacrosMgr->ReplaceMacros(CommandLineArguments, Target);
+    Manager::Get()->GetMacrosManager()->ReplaceEnvVars(CommandLineArguments);
+
        DynamicLinkerPath = cbGetDynamicLinkerPathForTarget(Project, Target);
        return true;
 }  // end of CheckRequirements
diff --git a/src/plugins/debuggergdb/debuggergdb.cpp b/src/plugins/debuggergdb/debuggergdb.cpp
index 24d007c..1cceee6 100644
--- a/src/plugins/debuggergdb/debuggergdb.cpp
+++ b/src/plugins/debuggergdb/debuggergdb.cpp
@@ -867,6 +867,11 @@ int DebuggerGDB::DoDebug(bool breakOnEntry)
             ShowWindow(windowHandle, SW_HIDE);
     }
     #endif
+
+    // Expand macros in command line
+     Manager::Get()->GetMacrosManager()->ReplaceMacros(cmdline, target);
+     Manager::Get()->GetMacrosManager()->ReplaceEnvVars(cmdline);
+
     // start the gdb process
     wxString wdir = m_State.GetDriver()->GetDebuggersWorkingDirectory();
     if (wdir.empty())

But looking at this now would it be better to do the macro/env expansion in the target GetOutputFilename, GetHostApplication & GetExecutionParameters?