This revision was automatically updated to reflect the committed changes.
Closed by commit rL321355: debugserver: Propagate environment in launch-mode
(pr35671) (authored by labath, committed by ).
Repository:
rL LLVM
https://reviews.llvm.org/D41352
Files:
lldb/trunk/tools/debugserver/sourc
clayborg accepted this revision.
clayborg added a comment.
This revision is now accepted and ready to land.
Thanks for all the revisions. Looks good.
https://reviews.llvm.org/D41352
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://li
labath updated this revision to Diff 127898.
labath added a comment.
Add PushEnvironmentIfNeeded to avoid creating duplicate entries in the
environment.
https://reviews.llvm.org/D41352
Files:
tools/debugserver/source/RNBContext.cpp
tools/debugserver/source/RNBContext.h
tools/debugserver/
labath added inline comments.
Comment at: tools/debugserver/source/debugserver.cpp:1426
+for (i = 0; (env_entry = host_env[i]) != NULL; ++i)
+ remote->Context().PushEnvironment(env_entry);
+ }
clayborg wrote:
> We need to check if the env var is already
clayborg added inline comments.
Comment at: tools/debugserver/source/debugserver.cpp:1426
+for (i = 0; (env_entry = host_env[i]) != NULL; ++i)
+ remote->Context().PushEnvironment(env_entry);
+ }
We need to check if the env var is already in the environm
labath updated this revision to Diff 127681.
labath added a comment.
Herald added a subscriber: mgorny.
New version: Make sure we respect variables set by --env and that they are not
overridden by --forward-env (the last part relies on the fact that in the
presence of multiply-defined environment
clayborg added a comment.
This is exposing a bug where if we have options like:
% debugserver --env USER=hello --forward-env -- /bin/ls -lAF
We would set USER to hello, then "--forward-env" would clobber the setting...
https://reviews.llvm.org/D41352
__
clayborg added a comment.
Environment vars might be set by using --env, so those need to go into
"inferior_envp" first. If we are launching, we will copy only the host
environment vars that haven't been already set manually (they don't already
exist in the inferior_envp).
https://reviews.llvm
clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.
So we can also specify extra environment variable using the "--env" option with
debugserver and your current fix will break that.
https://reviews.llvm.org/D41352
clayborg added a comment.
The existing code for the "--forward-env" does this:
case 'F':
// Pass the current environment down to the process that gets launched
{
char **host_env = *_NSGetEnviron();
char *env_entry;
size_t i;
for (i = 0; (env_entry = host_env[i])
jasonmolenda accepted this revision.
jasonmolenda added a comment.
This revision is now accepted and ready to land.
Thanks for the background Pavel. I'm fine with changing the default behavior.
I don't see this having any impact on users of debugserver, and if it does, we
can add a flag like G
clayborg added a comment.
So the main question is what do we expect to happen by default. I kind of agree
that if we launch the inferior directly when launching I would expect the
environment to be passed along. Jason: since we always just launch debugserver
for Xcode in the mode that doesn't e
labath added a comment.
In https://reviews.llvm.org/D41352#959051, @jasonmolenda wrote:
> I guess I don't have an opinion on this one. The correct way to pass
> environment variables to the inferior is through
> SBLaunchInfo::SetEnvironmentEntries or in cmd line lldb, process launch -v
> ENV=
jasonmolenda added a comment.
I guess I don't have an opinion on this one. The correct way to pass
environment variables to the inferior is through
SBLaunchInfo::SetEnvironmentEntries or in cmd line lldb, process launch -v
ENV=val. A test that assumes an environment variable set in lldb will
clayborg added a comment.
I am ok either way, but I do want to make sure Jason gets input before we do
anything. He might be out for a few weeks over the break, so there might be a
delay.
https://reviews.llvm.org/D41352
___
lldb-commits mailing li
labath added a comment.
In https://reviews.llvm.org/D41352#958549, @clayborg wrote:
> We already have an option for this named "--forward-env". It might be better
> to fix this by adding the "--forward-env" argument to the debugserver launch
> during testing? When we launch through LLDB, it for
clayborg added a comment.
We already have an option for this named "--forward-env". It might be better to
fix this by adding the "--forward-env" argument to the debugserver launch
during testing? When we launch through LLDB, it forwards all environment
variables manually. Maybe we add a "--forw
labath updated this revision to Diff 127358.
labath added a comment.
re-clang-format the patch
https://reviews.llvm.org/D41352
Files:
tools/debugserver/source/debugserver.cpp
unittests/tools/lldb-server/tests/LLGSTest.cpp
Index: unittests/tools/lldb-server/tests/LLGSTest.cpp
=
labath created this revision.
labath added reviewers: jasonmolenda, clayborg.
Herald added a subscriber: JDevlieghere.
Make sure we propagate environment when starting debugserver with a pre-loaded
inferior. AFAIK, RNBRunLoopLaunchInferior is only called in pre-loaded inferior
scenario, so we can
19 matches
Mail list logo
