New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
HPCC-18308 eclcmd should pass eclcc options as debug values #10443
HPCC-18308 eclcmd should pass eclcc options as debug values #10443
Conversation
https://track.hpccsystems.com/browse/HPCC-18308 |
@ghalliday Please sanity check this |
ecl/eclcmd/eclcmd_core.cpp
Outdated
@@ -148,6 +148,8 @@ bool doDeploy(EclCmdWithEclTarget &cmd, IClientWsWorkunits *client, const char * | |||
expandDefintionsAsDebugValues(cmd.definitions, cmd.debugValues); | |||
if (cmd.debugValues.length()) | |||
{ | |||
IArrayOf<IEspNamedValue> debugValues; |
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.
trivial: not used.
@@ -254,7 +254,8 @@ class EclccCompileThread : implements IPooledThread, implements IErrorReporter, | |||
{ | |||
//Allow eclcc-xx-<n> so that multiple values can be passed through for the same named debug symbol | |||
const char * start = option + (*option=='-' ? 1 : 6); | |||
const char * dash = strrchr(start, '-'); // position of second dash, if present | |||
const char * finger = (*start=='-') ? start+1 : start; //support leading double dash |
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.
I don't understand this change - can you elaborate?
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.
@ghalliday It's slightly confusing, but the key is that when the option is appended to the eclcc command line the stripped '-' is restored. I was trying not to refactor the code more than I had to as this is a last minute PR.
eclccCmd.appendf(" -%s=%s", optName.get(), value);
Some eclcc options actually have two dashes. "--nostdinc" for example. So this change preserves the second dash in the option name "-nostdinc" and then both dashes end up on the command line.
@afishbeck I may be misunderstanding, but it looks like an option -f-x is converted to eclcc-x, but would then be matched in eclccserver as option x. I would have expected it to be converted to eclcc--x (then the double minus code would make sense.) |
"ecl run <target> <file> -f-<option>" Allows arbitrary eclcc options to be passed to the eclcc command line. Those options should also be passed as workunit debug values formatted as "eclcc-<option>". Signed-off-by: Anthony Fishbeck <anthony.fishbeck@lexisnexis.com>
b0158dc
to
8e3fb66
Compare
@ghalliday Removed unused variable. Responded to your other comment. |
Automated Smoketest: ✅
Install hpccsystems-platform-community_6.4.2-rc3.el7.x86_64.rpm Unit tests result:
Regression test result:
HPCC Stop: OK |
@richardkchapman looks good. I'll let you decide which build it goes in. |
I have a concern that --fastparse and --nostdinc should be being applied to the creation of the archive when ecl command calls out to a local eclcc, but don't necessarily apply to the parsing at the remote end - so while it's ok to fix the -f- processing as you have here, we really need --fastparse and --nostdinc to be also handled explicitly by all ecl commands. I also suspect we may need to push this out to 6.4.4. There is a workaround of creating the archive independently I think. |
@richardkchapman I could easily add those as explicit eclcmd options. My guess would have been that --nostdinc had no effect on the backend since the depencies were already gathered into the archive at the client side? I can't seem to find --fastparse. |
As mentioned in the meeting, --fastparse was a typo, it's --fastsyntax. |
"ecl run -f-"
Allows arbitrary eclcc options to be passed to the eclcc command line.
Those options should also be passed as workunit debug values formatted
as "eclcc-".
Signed-off-by: Anthony Fishbeck anthony.fishbeck@lexisnexis.com
Type of change:
Checklist:
Testing: