add support for environment files

[mpldr]

At times, it is necessary or at least easier to set environment variables using a file.

Add a function that augments the environment of the spawned process with the provided variables.

Tested to work on Linux. Not sure on how to write a test for it without either passing in the content as a reader or basically testing stdlib IO functions.

[mpldr]

On Sun 02 Jun 2024 05:11:53, ccoVeille wrote:

+func (e Engine) modifyEnvironment(c exec.Cmd) {

By bare return, I meant using return ceverywhere and not return

That is completely missing the point of a bare return, I think. If the function signature does not specify any returns, of course a bare return is the way to… well… return. Bare returns, at least as I understand it are referring to:

// bare return, i. e. bad
func (e *Engine) modifyEnvironment(c *exec.Cmd) (err error) {
    err = errors.New("I feel bare")
    return
}

// non-bare return, i. e. better
func (e *Engine) modifyEnvironment(c *exec.Cmd) error {
    return errors.New("I feel bare")
}

To be fair, not the best example, but it gets the point across. Since it wasn't using a bare return this doesn't apply. I am still happy to report that the added return is not bare :)

-- Moritz Poldrack https://moritz.sh

Not intended to diagnose, treat, cure or prevent any disease.

[mpldr]

OS handling of environment variables and Go stdlib.

-- Moritz Poldrack https://moritz.sh

No alcohol, dogs or horses.

[xiantang]

why do u just pass env var like this:

A=B air

[ccoVeille]

why do u just pass env var like this:

A=B air

@xiantang I was also wondering when I read the PR for the first time.

For me the environment variables are what they are supposed to be something outside the binary that exists prior the binary is launched, they are added by the shell.

I was surprised by what @mpldr added, but I was like, never mind 🤷, but as you raise the point 🤨 too. I feel like my first thought was right.

I wouldn't expect this to be added to air

It's usually the role of:

• a runner tool that will import the variables, then launch air
• a shell script
• a global environment variable added to the shell
• any virtualenv tool
• a command-line scoped environment variable as you spotted @xiantang
• the equivalent of what is implemented should be a part of the config file of air

[mpldr]

why do u just pass env var like this:

A=B air

mainly two reasons:

• No way to change the environment between restart, which at times can be necessary (for example when changing parsing logic or moving on to a different task)
• This gets ridiculously long for some docker services, where you have to set a dozen or more environment variables.

they are supposed to be something outside the binary that exists prior the binary is launched

Since air acts as the launcher (it calls sh), this seems like an appropriate place to do it, to me.

[ccoVeille]

What about using the air config file to set the env variables, so not the location of an env file?

[mpldr]

That was an approach I was considering, but ultimately abandoned it because .env files or some variation of them are usually already present. And having two places to maintain environment variables might be a tough sell :)

[ccoVeille]

Thanks for your reply.

It won't change the points I already raised, it's surprising to see such implementation.

https://github.com/air-verse/air/pull/598#issuecomment-2144833248

For me, it's over the role of an app to do that

I look at what supervisord was doing and it was providing such a feature and the reply and arguments were very similar

https://github.com/Supervisor/supervisor/issues/771

But anyway, why not… but I would discourage you doing it.

[ccoVeille]

I'm still not convinced, but it's not a problem 😅. I'm a random voice among others. I can live with this feature being in the app, but I won't use it.

But if you want to keep the feature please look at https://github.com/air-verse/air/pull/598#discussion_r1625432195