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:
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
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
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
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.
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).
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
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
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
>
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
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
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
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
16 matches
Mail list logo
