Skip to content

Commit

Permalink
Merge pull request #7016 from dolthub/steph/cli-perf
Browse files Browse the repository at this point in the history
Allow `dolt config` to run in folders without write permissions
  • Loading branch information
stephkyou authored Nov 17, 2023
2 parents f5e0b0f + c6ea972 commit e7180e4
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 23 deletions.
56 changes: 35 additions & 21 deletions go/cmd/dolt/dolt.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,13 @@ var commandsWithoutGlobalArgSupport = []cli.Command{
commands.ConfigCmd{},
}

// commands that do not need write access for the current directory
var commandsWithoutCurrentDirWrites = []cli.Command{
commands.VersionCmd{VersionStr: Version},
commands.ConfigCmd{},
commands.ProfileCmd{},
}

func initCliContext(commandName string) bool {
for _, command := range commandsWithoutCliCtx {
if command.Name() == commandName {
Expand All @@ -185,6 +192,15 @@ func supportsGlobalArgs(commandName string) bool {
return true
}

func needsWriteAccess(commandName string) bool {
for _, command := range commandsWithoutCurrentDirWrites {
if command.Name() == commandName {
return false
}
}
return true
}

var doltCommand = cli.NewSubCommandHandler("dolt", "it's git for data", doltSubCommands)
var globalArgParser = cli.CreateGlobalArgParser("dolt")
var globalDocs = cli.CommandDocsForCommandString("dolt", doc, globalArgParser)
Expand Down Expand Up @@ -432,6 +448,10 @@ func runMain() int {
return 1
}

if dEnv.CfgLoadErr != nil {
cli.PrintErrln(color.RedString("Failed to load the global config. %v", dEnv.CfgLoadErr))
return 1
}
globalConfig, ok := dEnv.Config.GetConfig(env.GlobalConfig)
if !ok {
cli.PrintErrln(color.RedString("Failed to get global config"))
Expand All @@ -450,25 +470,6 @@ func runMain() int {
return 1
}

dataDir, hasDataDir := apr.GetValue(commands.DataDirFlag)
if hasDataDir {
// If a relative path was provided, this ensures we have an absolute path everywhere.
dataDir, err = fs.Abs(dataDir)
if err != nil {
cli.PrintErrln(color.RedString("Failed to get absolute path for %s: %v", dataDir, err))
return 1
}
if ok, dir := fs.Exists(dataDir); !ok || !dir {
cli.Println(color.RedString("Provided data directory does not exist: %s", dataDir))
return 1
}
}

if dEnv.CfgLoadErr != nil {
cli.PrintErrln(color.RedString("Failed to load the global config. %v", dEnv.CfgLoadErr))
return 1
}

emitter := events.NewFileEmitter(root, dbfactory.DoltDir)

defer func() {
Expand Down Expand Up @@ -496,8 +497,7 @@ func runMain() int {
}
}()

// version does not need write permissions
if subcommandName != "version" {
if needsWriteAccess(subcommandName) {
err = reconfigIfTempFileMoveFails(dEnv)

if err != nil {
Expand All @@ -508,6 +508,20 @@ func runMain() int {

defer tempfiles.MovableTempFileProvider.Clean()

dataDir, hasDataDir := apr.GetValue(commands.DataDirFlag)
if hasDataDir {
// If a relative path was provided, this ensures we have an absolute path everywhere.
dataDir, err = fs.Abs(dataDir)
if err != nil {
cli.PrintErrln(color.RedString("Failed to get absolute path for %s: %v", dataDir, err))
return 1
}
if ok, dir := fs.Exists(dataDir); !ok || !dir {
cli.Println(color.RedString("Provided data directory does not exist: %s", dataDir))
return 1
}
}

// Find all database names and add global variables for them. This needs to
// occur before a call to dsess.InitPersistedSystemVars. Otherwise, database
// specific persisted system vars will fail to load.
Expand Down
6 changes: 6 additions & 0 deletions integration-tests/bats/config.bats
Original file line number Diff line number Diff line change
Expand Up @@ -287,3 +287,9 @@ teardown() {
[ "$status" -eq 0 ]
[[ "$output" =~ "* vegan-btw" ]] || false
}

@test "config: config doesn't need write permission in current dir" {
chmod 111 .
dolt config --list
chmod 755 .
}
3 changes: 1 addition & 2 deletions integration-tests/bats/no-repo.bats
Original file line number Diff line number Diff line change
Expand Up @@ -130,8 +130,7 @@ teardown() {

@test "no-repo: dolt version does not need write permissions" {
chmod 111 .
run dolt version
[ "$status" -eq 0 ]
dolt version
chmod 755 .
}

Expand Down
6 changes: 6 additions & 0 deletions integration-tests/bats/profile.bats
Original file line number Diff line number Diff line change
Expand Up @@ -388,3 +388,9 @@ teardown() {
[[ ! "$output" =~ "defaultTable" ]] || false
[[ "$output" =~ "altTable" ]] || false
}

@test "profile: profile doesn't need write permission in current dir" {
chmod 111 .
dolt profile
chmod 755 .
}

0 comments on commit e7180e4

Please sign in to comment.