Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Passing environment variables to CmdAction is broken #171

Closed
onnodb opened this issue Apr 15, 2017 · 2 comments
Closed

Passing environment variables to CmdAction is broken #171

onnodb opened this issue Apr 15, 2017 · 2 comments
Labels
Milestone

Comments

@onnodb
Copy link
Contributor

onnodb commented Apr 15, 2017

It seems that d58a73 broke the ability to pass environment variables into CmdAction: if I now pass the env kwarg into the CmdAction constructor, this leads to a traceback:

File "/Users/onno/anaconda/envs/p3/lib/python3.5/site-packages/doit/action.py", line 204, in execute **self.pkwargs) TypeError: type object got multiple values for keyword argument 'env'

A minimal dodo.py to reproduce:

from doit.action import CmdAction

def task_env_call():
    return {'actions': [CmdAction('echo Hi', env={'ENV_VAR', 'test'})]}

I guess that a workaround might be to prepend the environment variables to be set to the shell command string.

@schettino72
Copy link
Member

Yes. broken 😞

I guess that a workaround might be to prepend the environment variables to be set to the shell command string.

I would rather manipulate the env variable if it was passed. Something like:

kwargs = self.pkwargs.copy()
if self.buffering:
    if 'env' not in kwargs:
        kwargs['env'] = os.environ.copy() 
    kwargs['env']['PYTHONUNBUFFERED'] = '1'

        # spawn task process
        process = subprocess.Popen(
            action,
            shell=self.shell,
            #bufsize=2, # ??? no effect use PYTHONUNBUFFERED instead
            stdout=subprocess.PIPE, stderr=subprocess.PIPE,
            **kwargs)

Could please provide a patch including unit-tests?

@onnodb
Copy link
Contributor Author

onnodb commented Apr 19, 2017

I guess that a workaround might be to prepend the environment variables to be set to the shell command string.

I actually meant a temporary workaround for people using the broken version of Pydoit ;-)

As for actually fixing it: I'd cetainly be happy to provide a pull request. It's not something I have time for right away, but I'll see if there's some time on the weekend.

Thanks for the quick response!

onnodb added a commit to onnodb/doit that referenced this issue Apr 22, 2017
onnodb added a commit to onnodb/doit that referenced this issue Apr 22, 2017
schettino72 pushed a commit that referenced this issue May 29, 2017
schettino72 pushed a commit that referenced this issue May 29, 2017
@schettino72 schettino72 modified the milestone: 0.31 May 29, 2017
gabc pushed a commit to M32Media/doit that referenced this issue Dec 5, 2017
gabc pushed a commit to M32Media/doit that referenced this issue Dec 5, 2017
gabc pushed a commit to M32Media/doit that referenced this issue Dec 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants