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

cmd/go: add -C flag to change directory #50332

Closed
thockin opened this issue Dec 24, 2021 · 18 comments
Closed

cmd/go: add -C flag to change directory #50332

thockin opened this issue Dec 24, 2021 · 18 comments

Comments

@thockin
Copy link

thockin commented Dec 24, 2021

I'm not sure if this rises to a proposal or not.

Many tools (e.g. make, git) have a universal -C <dir> flag which lets you do things in other directories without pushd/popd silliness. Given how go uses CWD for important things like modules, it would be nice to have something like this in go. For example go -C path/to/dir list gives me the package-path. Otherwise it's things like pushd path/to/dir >/dev/null; go list; popd >/dev/null.

It's really just a convenience thing. Has it already been discussed? I couldn't find it.

@gopherbot gopherbot added this to the Proposal milestone Dec 24, 2021
@seankhliao seankhliao changed the title proposal: Add a -Cflag to go - change dir proposal: cmd/go: Add a -Cflag to go - change dir Dec 24, 2021
@ianlancetaylor ianlancetaylor added this to Incoming in Proposals (old) Dec 26, 2021
@mvdan
Copy link
Member

mvdan commented Dec 29, 2021

I imagine this would be a flag to follow go <cmd>, such as go build -C dir or go test -C dir ./.... I don't think the root go command has ever taken flags directly, so it would be odd to break that rule - unless we deem this flag special enough. Note that we already have common flags between some commands, such as -modfile or -x. I also imagine some commands may not need this flag, such as go version or go bug, but perhaps I'm wrong.

As for the logic itself: would it effectively replace the use of the current directory everywhere? For example, if I do go build -C /new/dir -o cmdbin foo.com/cmd@latest, would the binary show up in /new/dir/cmdbin or in ${PWD}/cmdbin? I would imagine the former for the sake of consistency; just like what a real pushd/popd pair would do. Note there are plenty of other flags that accept relative paths, such as -modfile or -overlay, so we need to think about them all together.

I think I'm neutral on this proposal. On one hand, it seems like a harmless addition, and there is indeed precedent in other popular tools. On the other hand, it's hard to judge how necessary the flag really is, and it does seem like a pretty far-reaching flag compared to the other common flags we have right now.

What particular use case are you thinking about? What is the particular problem with pushd/popd, the verbosity of it?

@thockin
Copy link
Author

thockin commented Dec 29, 2021 via email

@zikaeroh
Copy link
Contributor

zikaeroh commented Dec 29, 2021

The main use case is to avoid the constant refrain of pushd path/to/foo >/dev/null; to do whatever; popd >/dev/null in scripts (even worse if I need the exit code from go) and to make it more like the other tools.

You could use a subshell, which is a lot shorter than pushd/popd (or even the quietier cd foo; cd -) and do give you back the exit codes.

$ (cd foo; pwd; true); echo $?
foo
0
$ (cd foo; pwd; false); echo $?
foo
1

I most commonly do this with my Go checkout to run make.bash from my repo root: (cd src; GOGC=off ./make.bash).

@rsc
Copy link
Contributor

rsc commented Jan 12, 2022

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@rsc rsc moved this from Incoming to Active in Proposals (old) Jan 12, 2022
@rsc
Copy link
Contributor

rsc commented Jan 19, 2022

If -C is really "do a chdir before anything else", then that's easy. But what about

go build -C bar -o x.exe 

or worse

go build -o x.exe -C bar

Does that x.exe get written to bar/x.exe? What behavior will people expect?

It sounds like the suggestion here is that this command writes to bar/x.exe.
Will that be too confusing for people?

/cc @matloob @bcmills

@thockin
Copy link
Author

thockin commented Jan 19, 2022 via email

@rsc
Copy link
Contributor

rsc commented Jan 26, 2022

@matloob @bcmills, any thoughts on adding -C meaning exactly

go cmd -C dir ...
==
(cd dir && go cmd ...)

where the parens make the shell switch back when go exits.

?

@fsouza
Copy link
Contributor

fsouza commented Jan 27, 2022

Being nitpicky here, but I assume we'd want to make it equivalent to (cd dir && go cmd ...)? like if dir doesn't exist, go should bail with a non-0 code.

@mmwtsn
Copy link

mmwtsn commented Jan 27, 2022

This would be a nice, small change that is consistent with other tools like git and would help simplify build scripts.

I worked on some build automation last year on a previous team that had most of their code in a monorepo. All of the build instructions and scripts were littered with pushd/popd or the equivalent cd dir && go … && cd .. pattern. This added a bit of visual noise and in some cases was error-prone when directory structures were refactored as the relative paths in cd .. needed updating. In those same scripts, git repositories outside of the main monorepo were interacted with using git -C.

I've seen some Makefiles in Go projects that followed patterns similar to this that would have also benefitted:

GOCMD=go
GOBUILD=$(GOCMD) build
BINARY_NAME=app

build: 
  cd dir; $(GOBUILD) -o $(BINARY_NAME) -v

The directory traversal was typically handled with something like that or $(MAKE) -C dir but having the Go tooling handle the directory change without needing to worry about using a subshell or invoking Make is a little easier to work with. For both shell scripts and Makefiles, I often see counter arguments raised that "you could just do x!" which are often technically correct but less friendly to developers who may not be as familiar with some rough edges in both areas. In that sense, a go -C flag would likely be easier to use for a wider range of backgrounds and experience levels, which I think is a good thing.

@rsc
Copy link
Contributor

rsc commented Feb 2, 2022

Any objections to adding this?

@dmitshur
Copy link
Contributor

dmitshur commented Feb 2, 2022

I tried to think of whether any existing cmd/go behaviors might intersect poorly with this, and did not spot a glaring serious conflict.

For example, go build takes -modfile that allows selecting an alternative go.mod file, which is okay to do at the same time as using another working directory.

Presumably, it'd also be possible to set -C via cmd/go's GOFLAGS environment variable, possibly configured in environment or via go env -w. Since the go command doesn't use the current working directory while looking up go env GOENV, it doesn't conflict.

Edit: Also, go test and go doc have boolean -c (lower case) flags. They do not conflict since this proposal is about -C (upper case).

@mmwtsn
Copy link

mmwtsn commented Feb 2, 2022

I'd be interested in working on this if there are no objections to the proposal and no one else has picked it up yet.

@rsc rsc moved this from Active to Likely Accept in Proposals (old) Feb 9, 2022
@rsc
Copy link
Contributor

rsc commented Feb 9, 2022

Based on the discussion above, this proposal seems like a likely accept.
— rsc for the proposal review group

@pohly
Copy link

pohly commented Feb 16, 2022

You could use a subshell, which is a lot shorter than pushd/popd (or even the quietier cd foo; cd -) and do give you back the exit codes.

I also reached for that solution in a GitHub action and then found that it failed on Windows: it would accept the syntax, but then ran the following commands in the directory of the subshell.

So +1 from me for the proposal.

@rsc rsc moved this from Likely Accept to Accepted in Proposals (old) Feb 16, 2022
@rsc
Copy link
Contributor

rsc commented Feb 16, 2022

No change in consensus, so accepted. 🎉
This issue now tracks the work of implementing the proposal.
— rsc for the proposal review group

@rsc rsc changed the title proposal: cmd/go: Add a -Cflag to go - change dir cmd/go: Add a -Cflag to go - change dir Feb 16, 2022
@rsc rsc modified the milestones: Proposal, Backlog Feb 16, 2022
@gopherbot
Copy link

Change https://go.dev/cl/421436 mentions this issue: cmd/go: add -C flag

@rsc rsc changed the title cmd/go: Add a -Cflag to go - change dir cmd/go: add -C flag to change directory Aug 10, 2022
@dmitshur dmitshur modified the milestones: Backlog, Go1.20 Nov 3, 2022
@thockin
Copy link
Author

thockin commented Nov 3, 2022

YAY! Thanks all.

romaindoumenc pushed a commit to TroutSoftware/go that referenced this issue Nov 3, 2022
The -C flag is like tar -C or make -C: it changes to the named directory
early in command startup, before anything else happens.

Fixes golang#50332.

Change-Id: I8e4546f69044cb3a028d4d26dfba482b08cb845d
Reviewed-on: https://go-review.googlesource.com/c/go/+/421436
Reviewed-by: Bryan Mills <bcmills@google.com>
Auto-Submit: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
@gopherbot
Copy link

Change https://go.dev/cl/449075 mentions this issue: doc/go1.20: add release notes for cmd/go changes

gopherbot pushed a commit that referenced this issue Nov 14, 2022
Updates #41696.
Updates #50332.
Updates #41583.

Change-Id: I99e96a2996f14da262570a5cb5273dcdce45df2b
Reviewed-on: https://go-review.googlesource.com/c/go/+/449075
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
@seankhliao seankhliao added the GoCommand cmd/go label May 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

No branches or pull requests

10 participants