Skip to content

os/exec: Cmd can only be executed once, but that isn't documented #10305

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.

Sign up for GitHub

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

Jump to bottom
Closed
AlexFinkel opened this issue Apr 1, 2015 · 4 comments
Closed

os/exec: Cmd can only be executed once, but that isn't documented #10305

AlexFinkel opened this issue Apr 1, 2015 · 4 comments
Labels
Documentation Issues describing a change to documentation. FrozenDueToAge
Milestone
Go1.5

Comments

@AlexFinkel
Copy link

Once an exec.Cmd has been Run (e.g. through .Run, .Output or .CombinedOutput) calling one of these run methods again will result in an error.

  1. This fact isn't documented.
  2. The resulting error messages don't clearly identify what caused the problem.
  3. This behaviour probably isn't necessary; for many commands it makes perfectly good sense to run them more than once.

I'm not talking about parallel execution here, this is running, checking the output, then running again.

Example error message from calling CombinedOutput more than once:
"exec: Stdout already set."
http://play.golang.org/p/Aaj3Gbva_S
@ianlancetaylor ianlancetaylor changed the title exec.Cmd type can only be executed once, but that isn't documented. os/exec: Cmd can only be executed once, but that isn't documented Apr 1, 2015
@minux minux added repo-main Documentation Issues describing a change to documentation. labels Apr 1, 2015
@minux minux added this to the Go1.5 milestone Apr 1, 2015
@cznic
Copy link
Contributor

cznic commented Apr 1, 2015

bytes.Buffer is not reusable, in the general case, without issuing a Reset either. IOW, if a complex thing with private fields is not explicitly stated as reusable, it is not. Documenting that for only one thing, exec.Cmd, is not very consistent. Documenting that for every thing in the sdtlib seems like an overkill.

Same approach is used for documenting concurrent access safety everywhere. Unless marked safe, it is implicitly not concurrent safe.

@alexf101
Copy link

alexf101 commented Apr 1, 2015

@cznic I don't really understand your response. The API documented in os/exec doesn't talk about the fact that it's one use, and the cause of errors seem implementation-specific.

Calling a function multiple times safely is the assumed behaviour of most APIs - I wouldn't expect documentation to clarify that it's safe to call fmt.Println multiple times.

If it helps, try to imagine if you were implementing an alternative implementation of the os/exec module. Would it be clear to you (without looking at the code) whether or not Run should be reusable?

@rsc rsc removed the repo-main label Apr 14, 2015
@gopherbot
Copy link
Contributor

CL https://golang.org/cl/9440 mentions this issue.

minux pushed a commit that referenced this issue Apr 29, 2015
@bsiegert @minux
os/exec: Document the fact that Cmd cannot be reused.
1203420 
Update #10305

Change-Id: Iea04758bc200038a1c64457a68100dcdd7f75212
Reviewed-on: https://go-review.googlesource.com/9440
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@bradfitz
Copy link
Contributor

This has been documented. I don't believe there was more to do, but let me know if I'm wrong.

@golang golang locked and limited conversation to collaborators Jun 25, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
Documentation Issues describing a change to documentation. FrozenDueToAge
Projects
None yet
Milestone
Go1.5
Development

No branches or pull requests
7 participants
@bradfitz @rsc @minux @cznic @alexf101 @AlexFinkel @gopherbot