Skip to content

Commit e7d687f

Browse files
committed
cmd/cue: don't use os.Getwd at init time
Users of the Go API at ./cmd/cue/cmd may wish to use os.Chdir before executing the CUE CLI, which is perfectly reasonable. That worked until https://cuelang.org/cl/1195084, where we started deduplicating calls to os.Getwd, but we did not think about the Go API users and os.Chdir. Instead, use a sync.OnceValue so that the first need for a working directory does call os.Getwd, and the value is kept from that point. Note that, to properly support os.Chdir calls between cmd.New calls, the global variable is re-initialized by New. Fixes #3916. Signed-off-by: Daniel Martí <[email protected]> Change-Id: I2b40c5238b351b5feee441fde065b73af3ef2919 Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1214672 TryBot-Result: CUEcueckoo <[email protected]> Reviewed-by: Roger Peppe <[email protected]>
1 parent 7de64e8 commit e7d687f

File tree

13 files changed

+68
-21
lines changed

13 files changed

+68
-21
lines changed

cmd/cue/cmd/cmd.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ Run "cue help commands" for more details on tasks and workflow commands.
147147
sub, err := customCommand(cmd, commandSection, args[0], tools)
148148
if err != nil {
149149
w := cmd.OutOrStderr()
150-
fmt.Fprint(w, errors.Details(err, &errors.Config{Cwd: rootWorkingDir}))
150+
fmt.Fprint(w, errors.Details(err, &errors.Config{Cwd: rootWorkingDir()}))
151151
fmt.Fprintln(w, `Ensure custom commands are defined in a "_tool.cue" file.`)
152152
fmt.Fprintln(w, "Run 'cue help cmd' to list available custom commands.")
153153
return ErrPrintedError

cmd/cue/cmd/fix.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ func runFixAll(cmd *Command, args []string) error {
5757
if len(args) == 0 {
5858
args = []string{"./..."}
5959

60-
dir := rootWorkingDir
60+
dir := rootWorkingDir()
6161
for {
6262
if _, err := os.Stat(filepath.Join(dir, "cue.mod")); err == nil {
6363
args = appendDirs(args, filepath.Join(dir, "cue.mod", "gen"))

cmd/cue/cmd/fmt.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,7 @@ func formatFile(file *build.File, opts []format.Option, doDiff, check bool, cmd
205205
return false, nil
206206
}
207207

208-
path, err := filepath.Rel(rootWorkingDir, file.Filename)
208+
path, err := filepath.Rel(rootWorkingDir(), file.Filename)
209209
if err != nil {
210210
path = file.Filename
211211
}

cmd/cue/cmd/import.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -479,7 +479,7 @@ func handleFile(b *buildPlan, f *ast.File) (err error) {
479479

480480
func writeFile(p *buildPlan, f *ast.File, cueFile string) error {
481481
if flagDryRun.Bool(p.cmd) {
482-
cueFile, err := filepath.Rel(rootWorkingDir, cueFile)
482+
cueFile, err := filepath.Rel(rootWorkingDir(), cueFile)
483483
if err != nil {
484484
return err
485485
}

cmd/cue/cmd/modget.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ func readModuleFile() (string, *modfile.File, []byte, error) {
118118
func findModuleRoot() (string, error) {
119119
// TODO this logic is duplicated in multiple places. We should
120120
// consider deduplicating it.
121-
dir := rootWorkingDir
121+
dir := rootWorkingDir()
122122
for {
123123
if _, err := os.Stat(filepath.Join(dir, "cue.mod")); err == nil {
124124
return dir, nil

cmd/cue/cmd/modinit.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ func runModInit(cmd *Command, args []string) (err error) {
5858
}
5959
}
6060

61-
mod := filepath.Join(rootWorkingDir, "cue.mod")
61+
mod := filepath.Join(rootWorkingDir(), "cue.mod")
6262
if info, err := os.Stat(mod); err == nil {
6363
if !info.IsDir() {
6464
return fmt.Errorf("cue.mod files are no longer supported; use cue.mod/module.cue")

cmd/cue/cmd/modpublish.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -308,7 +308,7 @@ func rootLICENSEFile(ctx context.Context, vcsImpl vcs.VCS) (string, error) {
308308

309309
// relLicenseFile is used in a couple of error situations below
310310
relLicenseFile := func() string {
311-
rel, err := filepath.Rel(rootWorkingDir, licenseFile)
311+
rel, err := filepath.Rel(rootWorkingDir(), licenseFile)
312312
if err == nil {
313313
return rel
314314
}

cmd/cue/cmd/root.go

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"os"
2323
"runtime"
2424
"runtime/pprof"
25+
"sync"
2526
"testing"
2627
"time"
2728

@@ -201,13 +202,28 @@ func mkRunE(c *Command, f runFunction) func(*cobra.Command, []string) error {
201202
}
202203
}
203204

205+
// rootWorkingDir avoids repeated calls to [os.Getwd] in cmd/cue.
206+
// If we can't figure out the current directory, something is very wrong,
207+
// and there's no point in continuing to run a command.
208+
var rootWorkingDir = func() string { panic("must be replaced by a sync.OnceValue") }
209+
204210
// TODO(mvdan): remove this error return at some point.
205211
// The API could also be made clearer if we want to keep cmd public,
206212
// such as not leaking *cobra.Command via embedding.
207213

208214
// New creates the top-level command.
209215
// The returned error is always nil, and is a historical artifact.
210216
func New(args []string) (*Command, error) {
217+
// Each call to New resets rootWorkingDir, to support [os.Chdir] calls in between.
218+
rootWorkingDir = sync.OnceValue(func() string {
219+
wd, err := os.Getwd()
220+
if err != nil {
221+
fmt.Fprintf(os.Stderr, "cannot get current directory: %v\n", err)
222+
os.Exit(1)
223+
}
224+
return wd
225+
})
226+
211227
cmd := &cobra.Command{
212228
Use: "cue",
213229
// TODO: the short help text below seems to refer to `cue cmd`, like helpTemplate.
@@ -273,18 +289,6 @@ func New(args []string) (*Command, error) {
273289
return c, nil
274290
}
275291

276-
// rootWorkingDir avoids repeated calls to [os.Getwd] in cmd/cue.
277-
// If we can't figure out the current directory, something is very wrong,
278-
// and there's no point in continuing to run a command.
279-
var rootWorkingDir = func() string {
280-
wd, err := os.Getwd()
281-
if err != nil {
282-
fmt.Fprintf(os.Stderr, "cannot get current directory: %v\n", err)
283-
os.Exit(1)
284-
}
285-
return wd
286-
}()
287-
288292
// Main runs the cue tool and returns the code for passing to os.Exit.
289293
func Main() int {
290294
start := time.Now()
@@ -302,7 +306,7 @@ func Main() int {
302306
if err := cmd.Run(backgroundContext()); err != nil {
303307
if err != ErrPrintedError {
304308
errors.Print(os.Stderr, err, &errors.Config{
305-
Cwd: rootWorkingDir,
309+
Cwd: rootWorkingDir(),
306310
ToSlash: testing.Testing(),
307311
})
308312
}
@@ -389,7 +393,7 @@ func printError(cmd *Command, err error) {
389393
}
390394
errors.Print(cmd.Stderr(), err, &errors.Config{
391395
Format: format,
392-
Cwd: rootWorkingDir,
396+
Cwd: rootWorkingDir(),
393397
ToSlash: testing.Testing(),
394398
})
395399
}

cmd/cue/cmd/root_test.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"bytes"
1919
"context"
2020
"io"
21+
"os"
2122
"strings"
2223
"testing"
2324

@@ -60,4 +61,31 @@ func TestCommand(t *testing.T) {
6061
qt.Assert(t, qt.IsNil(err))
6162
err = c.Execute()
6263
qt.Assert(t, qt.IsNotNil(err))
64+
65+
// Verify that we can change the current directory via [os.Chdir].
66+
// We test by ensuring we can load the module under testdata/files,
67+
// which uses the working directory as a starting point to find a module root.
68+
t.Run("Chdir", func(t *testing.T) {
69+
// TODO: use [testing.T.Chdir] once we can use Go 1.24 or later.
70+
origDir, err := os.Getwd()
71+
qt.Assert(t, qt.IsNil(err))
72+
73+
qt.Assert(t, qt.IsNil(os.Chdir("testdata/module_broken")))
74+
t.Cleanup(func() {
75+
qt.Assert(t, qt.IsNil(os.Chdir(origDir)))
76+
})
77+
78+
c, err = cmd.New([]string{"mod", "tidy", "--check"})
79+
qt.Assert(t, qt.IsNil(err))
80+
err = c.Execute()
81+
qt.Assert(t, qt.ErrorMatches(err, `^disallowed: field not allowed`))
82+
83+
// Change the directory a second time, to ensure the global state is not sticky.
84+
qt.Assert(t, qt.IsNil(os.Chdir("../module_ok")))
85+
86+
c, err = cmd.New([]string{"mod", "tidy", "--check"})
87+
qt.Assert(t, qt.IsNil(err))
88+
err = c.Execute()
89+
qt.Assert(t, qt.IsNil(err))
90+
})
6391
}
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
// As used by TestCommand/Chdir.
2+
3+
module: "broken.test/root"
4+
language: version: "v0.13.0"
5+
disallowed: "should fail mod tidy"

0 commit comments

Comments
 (0)