Hi,
i tried to fix https://sourceforge.net/p/codeblocks/tickets/328/ (the fix is already in the code below)
and found some, what i think is, code duplication sdk:
if ( target->GetTargetType() == ttDynamicLib
|| target->GetTargetType() == ttStaticLib )
{
// check for hostapp
if (target->GetHostApplication().IsEmpty())
{
cbMessageBox(_("You must select a host application to \"run\" a library..."));
m_pProject->SetCurrentlyCompilingTarget(0);
return -1;
}
command << hostapStr << strSPACE;
command << target->GetExecutionParameters();
Manager::Get()->GetMacrosManager()->ReplaceMacros(command);
}
else if (target->GetTargetType() != ttCommandsOnly)
{
command << execStr << strSPACE;
command << target->GetExecutionParameters();
// each shell execution must be enclosed to "":
// xterm -T X -e /bin/sh -c "/usr/bin/cb_console_runner X"
if (commandIsQuoted)
command << strQUOTE;
Manager::Get()->GetMacrosManager()->ReplaceMacros(command, target);
Manager::Get()->GetMacrosManager()->ReplaceEnvVars(command);
}
else
{
// commands-only target?
if (target->GetHostApplication().IsEmpty())
{
cbMessageBox(_("You must select a host application to \"run\" a commands-only target..."));
m_pProject->SetCurrentlyCompilingTarget(0);
return -1;
}
command << hostapStr << strSPACE;
command << target->GetExecutionParameters();
Manager::Get()->GetMacrosManager()->ReplaceMacros(command, target);
Manager::Get()->GetMacrosManager()->ReplaceEnvVars(command);
}
if we look at the declaration of GetMacrosManager()::ReplaceEnvVars (macrosmanager.h:32)
void ReplaceEnvVars(wxString& buffer) { ReplaceMacros(buffer); } /* misnomer, should be ReplaceVariables */;
So this is only a ReplaceMacros. I think we can remove all this calls in the above functions
any thoughts?
So if you remove the second call are you going to preserve current behaviour?
I am not 100% sure, i will test, but i am not 100% sure how...
I think that the current behavior is broken:
if we look at this two lines of code:
Manager::Get()->GetMacrosManager()->ReplaceMacros(command, target);
Manager::Get()->GetMacrosManager()->ReplaceEnvVars(command);
this will translate to
Manager::Get()->GetMacrosManager()->ReplaceMacros(command, target); // (1)
Manager::Get()->GetMacrosManager()->ReplaceMacros(command, nullptr); // (2)
what will replace all variables related to the current build target (1) and then with the current active target (2)[1]. If the current build target is different from the active target this is wrong behavior. I currently do not know if this can happen (i have in my mind a bug report that reported this error, but i can not find it)
[1]
in MacrosManager::ReplaceMacros() (macrosmanager.cpp:538):
const cbProject* project = target
? target->GetParentProject()
: Manager::Get()->GetProjectManager()->GetActiveProject();
Ok, i tested it (somehow*) extensively
diff --git a/src/plugins/compilergcc/compilergcc.cpp b/src/plugins/compilergcc/compilergcc.cpp
index 28f3dd1c0..30540db5c 100644
--- a/src/plugins/compilergcc/compilergcc.cpp
+++ b/src/plugins/compilergcc/compilergcc.cpp
@@ -2015,7 +2015,7 @@ int CompilerGCC::Run(ProjectBuildTarget* target)
m_pProject->SetCurrentlyCompilingTarget(0);
return -1;
}
-
+ Manager::Get()->GetMacrosManager()->ReplaceEnvVars(hostapStr);
command << hostapStr << strSPACE;
command << target->GetExecutionParameters();
}
@@ -2028,6 +2028,8 @@ int CompilerGCC::Run(ProjectBuildTarget* target)
// here is last \"
if (commandIsQuoted)
command << strQUOTE;
+ Manager::Get()->GetMacrosManager()->ReplaceMacros(command, target);
+ Manager::Get()->GetMacrosManager()->ReplaceEnvVars(command);
}
else
{
@@ -2040,8 +2042,10 @@ 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);
+
wxString script = command;
if (platform::macosx)
this patch removes code/ execution duplication and fixes the ticket by enabling macro expansion for shared libraries
i could not trigger any bug i had in mind, i also performed a forum search, but could not find any report about this...
if there are no objections i would apply this patch in the next days
* it is quite difficult to think about any possibility this complex code is called. I have tried all normal ways i could think of to trigger the related code portions. To find out if this breaks some obscure workflow we probably have to wait for complains after the patch is applied... obligatory xkcd: https://xkcd.com/1172/