Skip to content
This repository was archived by the owner on Feb 20, 2024. It is now read-only.

Allow hosted application for all targets. #22

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 16 additions & 19 deletions src/plugins/compilergcc/compilergcc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2019,42 +2019,39 @@ int CompilerGCC::Run(ProjectBuildTarget* target)
}

if ( target->GetTargetType() == ttDynamicLib
|| target->GetTargetType() == ttStaticLib )
|| target->GetTargetType() == ttStaticLib
|| target->GetTargetType() == ttCommandsOnly )
Copy link
Owner

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.

{
// 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;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Place this on new line please.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean the body of the if, like this?

if (commandIsQuoted)
     command << strQUOTE;

Copy link
Owner

Choose a reason for hiding this comment

The 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);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bluehazzard recently removed this call. Why do you need it back?

Copy link
Author

Choose a reason for hiding this comment

The 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.

Copy link
Owner

Choose a reason for hiding this comment

The 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.

Copy link
Author

Choose a reason for hiding this comment

The 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)
Expand Down