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

No working directory for sh.Exec #213

Open
corebreaker opened this issue Dec 23, 2018 · 9 comments
Open

No working directory for sh.Exec #213

corebreaker opened this issue Dec 23, 2018 · 9 comments

Comments

@corebreaker
Copy link

corebreaker commented Dec 23, 2018

Hello,

os/exec.Command allow a working directory, but sometimes, we need it in sh.Exec too.

The easy way is to use the exec.Command function, but i don't like it.

I propose to pass a special variable in env parameter, like this:

Exec(map[string]string { "#CWD": "/home/theUser/mydir" }, nil, nil, "my-command")
@natefinch
Copy link
Member

I think this is a valid concern and I think there is a way to address it. But the linked PR was not the right solution. I think we can come up with something better and more intuitive.

@corebreaker
Copy link
Author

corebreaker commented Dec 31, 2018

ok, so i'll propose another PR.

@philipsahli
Copy link

philipsahli commented Jun 7, 2019

If have added the PR #246, which should solve this issue. I need to run commands with mage/sh in the working directory, so I pass the variable #CWD with the value from os.Getenv("PWD") to sh.RunWith.

@corebreaker
Copy link
Author

Great !

@sluedecke
Copy link

I currently use this function as a workaround:

func runCommand(cwd string, command string, args ...string) (ran bool, err error) {
	wd, _ := os.Getwd()
	err = os.Chdir(cwd)
	if err != nil {
		return false, err
	}
	ran, err = sh.Exec(map[string]string{"#CWD": cwd}, os.Stdout, os.Stderr, command, args...)
	os.Chdir(wd) // can return an error, but should never happen.  Smart coders might want to handle it anyways
	return ran, err
}

@waj334
Copy link

waj334 commented Oct 29, 2021

I currently use this function as a workaround:

func runCommand(cwd string, command string, args ...string) (ran bool, err error) {
	wd, _ := os.Getwd()
	err = os.Chdir(cwd)
	if err != nil {
		return false, err
	}
	ran, err = sh.Exec(map[string]string{"#CWD": cwd}, os.Stdout, os.Stderr, command, args...)
	os.Chdir(wd) // can return an error, but should never happen.  Smart coders might want to handle it anyways
	return ran, err
}

This could cause some commands that run in parallel to fail

@jcchavezs
Copy link

I think sh.Exec should have a way to pass the directory. For example, in Coraza we run go test ./... in /my-folder and then go test ./... in /my-folder/my-subfolder (see https://github.com/corazawaf/coraza/pull/422/files#diff-520109d035a196d29d0a86e9c4e6c98e98abf056141b84df68c0d48167b87b25R83) and there is no way to pass a global working directory.
cc @anuraaga as he usually have good ideas on how to overcome API challenges.

hamzaelsaawy added a commit to hamzaelsaawy/mage that referenced this issue Sep 18, 2022
This PR is an attempt to solve magefile#213

Added `sh.Command` struct to mirror `exec.Cmd` and allow configuring
`sh.Exec` options, instead of adding a new function that
can change the working directory.

A single configuration struct was chosen instead of options since
the struct aggregates all configuration options together.

Added current `sh.Exec` parameters to `sh.Command` as fields, and
mimicked current behavior.

Moved `sh.run` functionality to `sh.(*Command).run`, and updated
`sh.Exec` to use `sh.Command.Exec`.

Added `WorkingDir` field to change the command's working
directory.

Signed-off-by: Hamza El-Saawy <hamza.elsaawy@gmail.com>
hamzaelsaawy added a commit to hamzaelsaawy/mage that referenced this issue Sep 18, 2022
This PR is an attempt to solve magefile#213

Added `sh.Command` struct to mirror `exec.Cmd` and allow configuring
`sh.Exec` options, instead of adding a new function that
can change the working directory.

A single configuration struct was chosen instead of options since
the struct aggregates all configuration options together.

Added current `sh.Exec` parameters to `sh.Command` as fields, and
mimicked current behavior.

Moved `sh.run` functionality to `sh.(*Command).run`, and updated
`sh.Exec` to use `sh.Command.Exec`.

Added `WorkingDir` field to change the command's working
directory.

Signed-off-by: Hamza El-Saawy <hamza.elsaawy@gmail.com>
hamzaelsaawy added a commit to hamzaelsaawy/mage that referenced this issue Sep 18, 2022
This PR is an attempt to solve magefile#213

Added `sh.Command` struct to mirror `exec.Cmd` and allow configuring
`sh.Exec` options, instead of adding a new function that
can change the working directory.

A single configuration struct was chosen instead of options since
the struct aggregates all configuration options together.

Added current `sh.Exec` parameters to `sh.Command` as fields, and
mimicked current behavior.

Moved `sh.run` functionality to `sh.(*Command).run`, and updated
`sh.Exec` to use `sh.Command.Exec`.

Added `WorkingDir` field to change the command's working
directory.

Signed-off-by: Hamza El-Saawy <hamza.elsaawy@gmail.com>
@hamzaelsaawy
Copy link

Is there any consensus on this?
I created #444 to create a struct to allow setting the different options in a struct, but I can update it to use options instead, as mentioned in #307.

@ivanduka
Copy link

I currently use this function as a workaround:

func runCommand(cwd string, command string, args ...string) (ran bool, err error) {
	wd, _ := os.Getwd()
	err = os.Chdir(cwd)
	if err != nil {
		return false, err
	}
	ran, err = sh.Exec(map[string]string{"#CWD": cwd}, os.Stdout, os.Stderr, command, args...)
	os.Chdir(wd) // can return an error, but should never happen.  Smart coders might want to handle it anyways
	return ran, err
}

This could cause some commands that run in parallel to fail

This is a great workaround, I added a mutex call like this in front of the function and everything works fine except now everything is sequential in execution:

var mu sync.Mutex

func runInFolder(cwd string, command string, args ...string) error {
	mu.Lock()
	defer mu.Unlock()

	fmt.Printf("Running %q with %#v in %q\n", command, args, cwd)

	wd, err := os.Getwd()
	if err != nil {
		return err
	}

	err = os.Chdir(cwd)
	if err != nil {
		return err
	}

	output, err := sh.Output(command, args...)
	if err != nil {
		return err
	}

	fmt.Println(output)

	err = os.Chdir(wd)
	if err != nil {
		return err
	}

	return nil
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants