This is an archive of the discontinued LLVM Phabricator instance.

[lldb] build.py: fix behavior when passing --compiler=/path/to/compiler
ClosedPublic

Authored by jgorbe on May 8 2019, 7:22 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

jgorbe created this revision.May 8 2019, 7:22 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 8 2019, 7:22 PM
Herald added a subscriber: teemperor. · View Herald Transcript
labath accepted this revision.May 9 2019, 4:11 AM

Sounds reasonable.

This revision is now accepted and ready to land.May 9 2019, 4:11 AM
labath added a comment.May 9 2019, 4:48 AM

BTW, there's some existing tests for build.py in lit/BuildScript. Could you check if it's feasible to make an analogous test for the fix you're making here?

jgorbe updated this revision to Diff 198853.May 9 2019, 9:36 AM

Added a new test that checks that, when a full path to a compiler is specified:

  • That compiler is used in build commands
  • The flag style used in build commands matches the toolchain type autodetected from the compiler executable name
labath accepted this revision.May 9 2019, 9:39 AM

Awesome, thanks!

jgorbe updated this revision to Diff 198858.May 9 2019, 9:42 AM

Removed trailing blank lines from test case, will commit now. Thanks for the review!

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMay 9 2019, 9:46 AM

I think we can just add --mode=link to the test to avoid it trying to mess with the linker (since that is not important here anyway).

I think we can just add --mode=link to the test to avoid it trying to mess with the linker (since that is not important here anyway).

Did you mean --mode=compile? I can confirm it fixes the test. Should I commit it?

I think we can just add --mode=link to the test to avoid it trying to mess with the linker (since that is not important here anyway).

Did you mean --mode=compile? I can confirm it fixes the test. Should I commit it?

Yes, that's what I meant. :) Feel free to commit.