Page MenuHomePhabricator

[Driver] Add command line option to allow loading local lldbinit file
ClosedPublic

Authored by JDevlieghere on May 5 2019, 7:22 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

JDevlieghere created this revision.May 5 2019, 7:22 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 5 2019, 7:22 PM
labath added inline comments.May 6 2019, 12:27 AM
lldb/tools/driver/Options.td
94–100 ↗(On Diff #198214)

What's the interaction between these two flags?

JDevlieghere marked an inline comment as done.May 6 2019, 9:10 AM
JDevlieghere added inline comments.
lldb/tools/driver/Options.td
94–100 ↗(On Diff #198214)

Good question: the no-lldbinit flag supersedes the allow-lldbinit flag. I'll update the description to make that clear.

Just wanted to verify that we can put a:

settings set target.load-cwd-lldbinit true

in our ~/.lldbinit file and it will then load the local init file in the current working directory?

Just wanted to verify that we can put a:

settings set target.load-cwd-lldbinit true

in our ~/.lldbinit file and it will then load the local init file in the current working directory?

Yep, that behavior remains unchanged.

Make it clear that --no-lldbinit supersedes the new flag.

labath accepted this revision.May 7 2019, 7:13 AM

Just wanted to verify that we can put a:

settings set target.load-cwd-lldbinit true

in our ~/.lldbinit file and it will then load the local init file in the current working directory?

Yep, that behavior remains unchanged.

Unfortunately, I guess that also means that if somebody does that (set this setting in the global config file), this test will fail for him. I think that means that one day we will have to add a flag to specifically disable reading of the global config file, or find some way to mock its contents.

lldb/tools/driver/Options.td
94–100 ↗(On Diff #198214)

Ok, thanks for clarifying.

This revision is now accepted and ready to land.May 7 2019, 7:13 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMay 7 2019, 9:54 AM

Just wanted to verify that we can put a:

settings set target.load-cwd-lldbinit true

in our ~/.lldbinit file and it will then load the local init file in the current working directory?

Yep, that behavior remains unchanged.

Unfortunately, I guess that also means that if somebody does that (set this setting in the global config file), this test will fail for him. I think that means that one day we will have to add a flag to specifically disable reading of the global config file, or find some way to mock its contents.

The dotest.py tests all disable reading the global .lldbinit file. Do the lit tests not do that as well? For tests that actually test reading the user's .lldbinit file we will need to do something fancy (can we change the HOME environment variable to some other place so that it gets read from there?)

The dotest.py tests all disable reading the global .lldbinit file. Do the lit tests not do that as well? For tests that actually test reading the user's .lldbinit file we will need to do something fancy (can we change the HOME environment variable to some other place so that it gets read from there?)

Yes, they do. However, this test specifically disables that behavior because otherwise we wouldn't even read the CWD lldbinit. Changing the HOME variable might be enough to address this though (and I see Jonas already did that).

The dotest.py tests all disable reading the global .lldbinit file. Do the lit tests not do that as well? For tests that actually test reading the user's .lldbinit file we will need to do something fancy (can we change the HOME environment variable to some other place so that it gets read from there?)

Yes, they do. However, this test specifically disables that behavior because otherwise we wouldn't even read the CWD lldbinit. Changing the HOME variable might be enough to address this though (and I see Jonas already did that).

Ah, excellent. That's probably good enough.

The notion of being able to specify to the driver where you store the global .lldbinit file is not 100% crazy - for instance if you had several different configurations you wanted to run lldb in, you could just switch global lldb dirs (e.g. using various shell aliases for the different global locations). So if this does cause problems adding that wouldn't be a totally silly way to fix the tests. But I'm not sure it's worth adding on its own.