-
Notifications
You must be signed in to change notification settings - Fork 28
Allow hosted application for all targets. #22
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2019,42 +2019,39 @@ int CompilerGCC::Run(ProjectBuildTarget* target) | |
} | ||
|
||
if ( target->GetTargetType() == ttDynamicLib | ||
|| target->GetTargetType() == ttStaticLib ) | ||
|| target->GetTargetType() == ttStaticLib | ||
|| target->GetTargetType() == ttCommandsOnly ) | ||
{ | ||
// check for hostapp | ||
if (target->GetHostApplication().IsEmpty()) | ||
{ | ||
cbMessageBox(_("You must select a host application to \"run\" a library...")); | ||
cbMessageBox(_("You must select a host application to \"run\" this target...")); | ||
m_pProject->SetCurrentlyCompilingTarget(0); | ||
return -1; | ||
} | ||
|
||
command << hostapStr << strSPACE; | ||
command << target->GetExecutionParameters(); | ||
} | ||
else if (target->GetTargetType() != ttCommandsOnly) | ||
|
||
if (!target->GetHostApplication().IsEmpty()) | ||
{ | ||
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" | ||
// here is last \" | ||
if (commandIsQuoted) | ||
command << strQUOTE; | ||
command << hostapStr << strSPACE; | ||
} | ||
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(); | ||
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" | ||
// here is last \" | ||
if (commandIsQuoted) command << strQUOTE; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Place this on new line please. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You mean the body of the if, like this?
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, this was the way it was written before your change. It makes debugging easier, because you can put a breakpoint on the body of the if. |
||
|
||
Manager::Get()->GetMacrosManager()->ReplaceMacros(command, target); | ||
Manager::Get()->GetMacrosManager()->ReplaceEnvVars(command); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bluehazzard recently removed this call. Why do you need it back? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I use different host applications for testing the same target fairly often - i.e. running the target in an emulator on my build system, sending the target over network to physical hardware or sometimes even passing through a tool that flashes the physical device & then runs the code that way. I find it convenient to set the appropriate host via an environment variable per session and have the hosted application set to, for instance, $HOSTAPP. This also means that different build machines and developers can share the same project file, use the host application most convenient for them and not interfere with each other in source control. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this this new added line necessary? According to @bluehazzard this is not the case and only the first line is enough to get the desired behaviour. He removed this line very recently. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll check, this PR has been hanging around for eons &I only just started playing around with code::blocks again. |
||
wxString script = command; | ||
|
||
if (platform::macosx) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please do not repeat the call to GetTargetType. Store it in a variable.