On Tuesday, 12 March 2013 at 21:39:47 UTC, Steven Schveighoffer wrote:
> On Tue, 12 Mar 2013 17:18:43 -0400, Lars T. Kyllingstad <public@kyllingen.net> wrote:
>
>> On Tuesday, 12 March 2013 at 14:10:34 UTC, Steven Schveighoffer wrote:
>
> I think all are in agreement at this point that closing the files between fork and exec is a good solution.  Whether or not to ALSO set F_CLOEXEC wherever Phobos opens a file descriptor is an additional matter.

Yeah, I've read through the discussion now, and I think I agree. I dislike the idea of looping over an unknown number of file descriptors, but it is both the safest and least disruptive strategy.


> As a fallback to standard unix behavior, we can have a Config option that says "don't do the close thing".

Config.dontDoTheCloseThing it is. ;)

Lars

On Tuesday, 12 March 2013 at 21:39:47 UTC, Steven Schveighoffer wrote:
>> I'd be very interested to hear if you have a suggestion for a better way to do it, keeping in mind that there needs to be *some* way to clear the environment too.
>
> Sadly, no I don't.  I had hoped [] would allocate an empty AA, but it fails to compile.
>
> Note that you can "hack" it by setting a single environment variable which nobody will ever use.
>
> i.e. spawnProcess("blah.exe", ["_____":"_____"]);
>
> But that is really, really ugly.

How about this:

@property string[string] emptyEnvironment()
{
    string[string] result;
    result["a"] = "a";
    result.remove("a");
    assert(result && result.length == 0);
    return result;
}

(can be cached to avoid allocating each time)

On Wed, Mar 13, 2013 at 04:27:21AM +0100, Vladimir Panteleev wrote:
> On Tuesday, 12 March 2013 at 21:39:47 UTC, Steven Schveighoffer wrote:
> >>I'd be very interested to hear if you have a suggestion for a better way to do it, keeping in mind that there needs to be *some* way to clear the environment too.
> >
> >Sadly, no I don't.  I had hoped [] would allocate an empty AA, but it fails to compile.
> >
> >Note that you can "hack" it by setting a single environment variable which nobody will ever use.
> >
> >i.e. spawnProcess("blah.exe", ["_____":"_____"]);
> >
> >But that is really, really ugly.
> 
> How about this:
> 
> @property string[string] emptyEnvironment()
> {
>     string[string] result;
>     result["a"] = "a";
>     result.remove("a");
>     assert(result && result.length == 0);
>     return result;
> }
> 
> (can be cached to avoid allocating each time)

I like this idea.


T

-- 
It won't be covered in the book. The source code has to be useful for something, after all. -- Larry Wall

On Wednesday, 13 March 2013 at 03:27:25 UTC, Vladimir Panteleev wrote:
> On Tuesday, 12 March 2013 at 21:39:47 UTC, Steven Schveighoffer wrote:
>>> I'd be very interested to hear if you have a suggestion for a better way to do it, keeping in mind that there needs to be *some* way to clear the environment too.
>>
>> Sadly, no I don't.  I had hoped [] would allocate an empty AA, but it fails to compile.
>>
>> Note that you can "hack" it by setting a single environment variable which nobody will ever use.
>>
>> i.e. spawnProcess("blah.exe", ["_____":"_____"]);
>>
>> But that is really, really ugly.
>
> How about this:
>
> @property string[string] emptyEnvironment()
> {
>     string[string] result;
>     result["a"] = "a";
>     result.remove("a");
>     assert(result && result.length == 0);
>     return result;
> }
>
> (can be cached to avoid allocating each time)

That's a lot better than ["____":"_____"], at least. :)

But still, the difference between a null AA and an empty AA is still very subtle, and I am hesitant to design an API that depends on it.  We'd have to explain to the users that "ok, so there are two kinds of empty AAs: the ones you've done nothing with, and the ones you've added and removed a value from..."

Furthermore, the language spec doesn't seem to mention "null" in relation to AAs.  Shouldn't the difference between null and empty then be treated as an implementation detail?  Can we even be sure that "aa is null" will work in two years?

Lars

Am 13.03.2013 07:31, schrieb Lars T. Kyllingstad:
> On Wednesday, 13 March 2013 at 03:27:25 UTC, Vladimir Panteleev
> wrote:
>> On Tuesday, 12 March 2013 at 21:39:47 UTC, Steven Schveighoffer
>> wrote:
>>>> I'd be very interested to hear if you have a suggestion for a
>>>> better way to do it, keeping in mind that there needs to be
>>>> *some* way to clear the environment too.
>>>
>>> Sadly, no I don't.  I had hoped [] would allocate an empty AA,
>>> but it fails to compile.
>>>
>>> Note that you can "hack" it by setting a single environment
>>> variable which nobody will ever use.
>>>
>>> i.e. spawnProcess("blah.exe", ["_____":"_____"]);
>>>
>>> But that is really, really ugly.
>>
>> How about this:
>>
>> @property string[string] emptyEnvironment()
>> {
>>     string[string] result;
>>     result["a"] = "a";
>>     result.remove("a");
>>     assert(result && result.length == 0);
>>     return result;
>> }
>>
>> (can be cached to avoid allocating each time)
>
> That's a lot better than ["____":"_____"], at least. :)
>
> But still, the difference between a null AA and an empty AA is
> still very subtle, and I am hesitant to design an API that
> depends on it.  We'd have to explain to the users that "ok, so
> there are two kinds of empty AAs: the ones you've done nothing
> with, and the ones you've added and removed a value from..."
>
> Furthermore, the language spec doesn't seem to mention "null" in
> relation to AAs.  Shouldn't the difference between null and empty
> then be treated as an implementation detail?  Can we even be sure
> that "aa is null" will work in two years?
>
> Lars
>

why not differentiate on callsite?

like

environment_usage =
{
  PARENT_ENVIRONMENT,
  NONE_ENVIRONMENT, // which differs from empty given environment
  GIVEN_ENVIRONMENT
}

spawnProcess(process,parameter,environment_usage = PARENT_ENVIRONMENT, environemnt = null)

it feels very wrong to put the environment "usage" type in any way into the environment-abstraction itself (by occupying null or empty...)

+some nice helpers

spawnProcessWithParentEnvironment((process,parameter)
spawnProcessWithoutEnvironment((process,parameter)
spawnProcessWithEnvironment((process,parameter,environment=...)

woulnd't that be much clearer?

the other way could be an

spawnProcess(process,parameter,environment=use_parent_environment());
with parent-environment selector

spawnProcess(process,parameter,environment=given_environment(environment));

On Wednesday, 13 March 2013 at 06:31:57 UTC, Lars T. Kyllingstad wrote:
> That's a lot better than ["____":"_____"], at least. :)
>
> But still, the difference between a null AA and an empty AA is still very subtle, and I am hesitant to design an API that depends on it.  We'd have to explain to the users that "ok, so there are two kinds of empty AAs: the ones you've done nothing with, and the ones you've added and removed a value from..."
>
> Furthermore, the language spec doesn't seem to mention "null" in relation to AAs.  Shouldn't the difference between null and empty then be treated as an implementation detail?  Can we even be sure that "aa is null" will work in two years?

It doesn't need to be part of the API. Just treat emptyEnvironment as a magic value that means to pass an empty environment. If the language's AA semantics change, the implementation can return an arbitrary AA literal from emptyEnvironment, and spawnProcess would do a "if (environment is emptyEnvironment)" check.

Making use of the current distinction between empty and null AAs is useful as it simplifies the implementation, and I think is marginally better than a magic (non-empty) value.

On Wed, 13 Mar 2013 02:45:57 -0400, dennis luehring <dl.soluz@gmx.net>
wrote:

> Am 13.03.2013 07:31, schrieb Lars T. Kyllingstad:
>> On Wednesday, 13 March 2013 at 03:27:25 UTC, Vladimir Panteleev
>> wrote:
>>> On Tuesday, 12 March 2013 at 21:39:47 UTC, Steven Schveighoffer
>>> wrote:
>>>>> I'd be very interested to hear if you have a suggestion for a
>>>>> better way to do it, keeping in mind that there needs to be
>>>>> *some* way to clear the environment too.
>>>>
>>>> Sadly, no I don't.  I had hoped [] would allocate an empty AA,
>>>> but it fails to compile.
>>>>
>>>> Note that you can "hack" it by setting a single environment
>>>> variable which nobody will ever use.
>>>>
>>>> i.e. spawnProcess("blah.exe", ["_____":"_____"]);
>>>>
>>>> But that is really, really ugly.
>>>
>>> How about this:
>>>
>>> @property string[string] emptyEnvironment()
>>> {
>>>     string[string] result;
>>>     result["a"] = "a";
>>>     result.remove("a");
>>>     assert(result && result.length == 0);
>>>     return result;
>>> }
>>>
>>> (can be cached to avoid allocating each time)
>>
>> That's a lot better than ["____":"_____"], at least. :)
>>
>> But still, the difference between a null AA and an empty AA is
>> still very subtle, and I am hesitant to design an API that
>> depends on it.  We'd have to explain to the users that "ok, so
>> there are two kinds of empty AAs: the ones you've done nothing
>> with, and the ones you've added and removed a value from..."
>>
>> Furthermore, the language spec doesn't seem to mention "null" in
>> relation to AAs.  Shouldn't the difference between null and empty
>> then be treated as an implementation detail?  Can we even be sure
>> that "aa is null" will work in two years?
>>
>> Lars
>>
>
> why not differentiate on callsite?
>
> like
>
> environment_usage =
> {
>    PARENT_ENVIRONMENT,
>    NONE_ENVIRONMENT, // which differs from empty given environment
>    GIVEN_ENVIRONMENT
> }
>
> spawnProcess(process,parameter,environment_usage = PARENT_ENVIRONMENT, environemnt = null)
>
> it feels very wrong to put the environment "usage" type in any way into the environment-abstraction itself (by occupying null or empty...)
>
> +some nice helpers
>
> spawnProcessWithParentEnvironment((process,parameter)
> spawnProcessWithoutEnvironment((process,parameter)
> spawnProcessWithEnvironment((process,parameter,environment=...)
>
> woulnd't that be much clearer?
>
> the other way could be an
>
> spawnProcess(process,parameter,environment=use_parent_environment());
> with parent-environment selector
>
> spawnProcess(process,parameter,environment=given_environment(environment));

Hm.. I think I actually like this.

I hate to have feature creep at this point, but one kind of annoying thing
is, if you want to *add* to the current environment, it is a multi-step
process:

auto curenv = environment.toAA;
curenv["x"] = "y";
spawnProcess("helloworld", curenv);

But with something similar to Dennis' idea, we have a possible way to do
that without making a copy of the current environment into an AA and
adding:

struct EnvironmentArg
{
     this(string[string] env, bool useParent=false) { this.env = env;
this.useParent = useParent;}
     this(bool useParent) {this.useParent = useParent;}
     string[string] env;
     bool useParent;
}

spawnProcess("helloworld", EnvironmentArg(["x":"y"], true)); // use parent
environment, add x=y
spawnProcess("helloworld", EnvironmentArg(["x":"y"])); // replace
environment with x=y
spawnProcess("helloworld", EnvironmentArg(false)); // use empty environment
spawnProcess("helloworld", EnvironmentArg(true)); // use parent
environment exactly

EnvironmentArg should probably have better name, and I would recommend
some global functions that make common things, like:

EnvironmentArg emptyEnvironment() { return EnvironmentArg(null, false);}
EnvironmentArg parentEnvironment() { return EnvironmentArg(null, true);}

Like? Hate?

-Steve

On Wed, 13 Mar 2013 02:45:57 -0400, dennis luehring <dl.soluz@gmx.net>
wrote:

> why not differentiate on callsite?
>
> like
>
> environment_usage =
> {
>    PARENT_ENVIRONMENT,
>    NONE_ENVIRONMENT, // which differs from empty given environment
>    GIVEN_ENVIRONMENT
> }
>
> spawnProcess(process,parameter,environment_usage = PARENT_ENVIRONMENT, environemnt = null)
>
> it feels very wrong to put the environment "usage" type in any way into the environment-abstraction itself (by occupying null or empty...)
>
> +some nice helpers
>
> spawnProcessWithParentEnvironment((process,parameter)
> spawnProcessWithoutEnvironment((process,parameter)
> spawnProcessWithEnvironment((process,parameter,environment=...)
>
> woulnd't that be much clearer?
>
> the other way could be an
>
> spawnProcess(process,parameter,environment=use_parent_environment());
> with parent-environment selector
>
> spawnProcess(process,parameter,environment=given_environment(environment));

Hm.. I think I actually like this.

I hate to have feature creep at this point, but one kind of annoying thing
is, if you want to *add* to the current environment, it is a multi-step
process:

auto curenv = environment.toAA;
curenv["x"] = "y";
spawnProcess("helloworld", curenv);

But with something similar to Dennis' idea, we have a possible way to do
that without making a copy of the current environment into an AA and
adding:

struct EnvironmentArg
{
     this(string[string] env, bool useParent=false) { this.env = env;
this.useParent = useParent;}
     this(bool useParent) {this.useParent = useParent;}
     string[string] env;
     bool useParent;
}

spawnProcess("helloworld", EnvironmentArg(["x":"y"], true)); // use parent environment, add x=y
spawnProcess("helloworld", EnvironmentArg(["x":"y"])); // replace environment with x=y
spawnProcess("helloworld", EnvironmentArg(false)); // use empty environment
spawnProcess("helloworld", EnvironmentArg(true)); // use parent environment exactly

EnvironmentArg should probably have better name, and I would recommend
some global functions that make common things, like:

EnvironmentArg emptyEnvironment() { return EnvironmentArg(null, false);}
EnvironmentArg parentEnvironment() { return EnvironmentArg(null, true);}

Like? Hate?

-Steve

On Wednesday, 13 March 2013 at 20:26:44 UTC, Steven Schveighoffer wrote:
> I hate to have feature creep at this point, but one kind of annoying thing
> is, if you want to *add* to the current environment, it is a multi-step
> process:
>
> auto curenv = environment.toAA;
> curenv["x"] = "y";
> spawnProcess("helloworld", curenv);
>
> But with something similar to Dennis' idea, we have a possible way to do
> that without making a copy of the current environment into an AA and
> adding:
>
> struct EnvironmentArg
> {
>      this(string[string] env, bool useParent=false) { this.env = env;
> this.useParent = useParent;}
>      this(bool useParent) {this.useParent = useParent;}
>      string[string] env;
>      bool useParent;
> }
>
> spawnProcess("helloworld", EnvironmentArg(["x":"y"], true)); // use parent
> environment, add x=y
> spawnProcess("helloworld", EnvironmentArg(["x":"y"])); // replace
> environment with x=y
> spawnProcess("helloworld", EnvironmentArg(false)); // use empty environment
> spawnProcess("helloworld", EnvironmentArg(true)); // use parent
> environment exactly
>
> EnvironmentArg should probably have better name, and I would recommend
> some global functions that make common things, like:
>
> EnvironmentArg emptyEnvironment() { return EnvironmentArg(null, false);}
> EnvironmentArg parentEnvironment() { return EnvironmentArg(null, true);}
>
> Like? Hate?

Hmm.. what if spawnProcess() takes a normal string[string] like it does now, but we add a flag to Config that determines whether it is merged with the parent's environment or not?

string[string] myEnv = [ "foo" : "bar" ];
spawnProcess("helloworld", null);     // Parent's env
spawnProcess("helloworld", myEnv);    // Parent's env + myEnv
spawnProcess("helloworld", null, ..., Config.clearEnv);  // Empty env
spawnProcess("helloworld", myEnv, ..., Config.clearEnv); // Only myEnv

Lars

On Tuesday, 12 March 2013 at 21:39:47 UTC, Steven Schveighoffer wrote:
> I think all are in agreement at this point that closing the files between fork and exec is a good solution.  Whether or not to ALSO set F_CLOEXEC wherever Phobos opens a file descriptor is an additional matter.

I have a created a pull request for a sys/resource.h translation now.  It contains getrlimit(), which we can use to get the max. number of file descriptors.

https://github.com/D-Programming-Language/druntime/pull/445

Lars