Skip to content

Commit 2780fef

Browse files
HydrochargedHydrocharged
committed
PR Feedback
1 parent d62892c commit 2780fef

File tree

8 files changed

+126
-63
lines changed

8 files changed

+126
-63
lines changed

‎.github/workflows/ci-staticcheck.yaml

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,5 @@ jobs:
2222
working-directory: ./postgres/parser
2323
shell: bash
2424
- name: Run check
25-
run: |
26-
go install honnef.co/go/tools/cmd/[email protected]
27-
staticcheck ./...
25+
run: ./run_staticcheck.sh
26+
working-directory: ./scripts

‎CONTRIBUTING.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,8 @@ Remember to recompile the executable on your PATH whenever you want to re-test a
3030
9. **Change the data directory**: This is optional but recommended.
3131
By default, we create databases within the `~/doltgres/databases` directory.
3232
For developmental purposes, you may want to change this behavior. You have two options:
33-
1. Set the `DOLTGRES_DATA_DIR_CWD` environment variable to `true`. Any value besides `true` will be interpreted as `false`. This causes DoltgreSQL to use the current directory as the data directory, so you can have multiple data directories simply by running the program in different directories. This behavior is more consistent with [Dolt's](https://github.com/dolthub/dolt) behavior. This is the recommended option for development. If this variable is set to `true`, then it overrides all other environment variables.
34-
2. Specify a new directory in the `DOLTGRES_DATA_DIR` environment variable. When this is empty, it uses the default directory.
33+
1. Set the `DOLTGRES_DATA_DIR` environment variable to a different directory. A value of `.` causes DoltgreSQL to use the current directory as the data directory, so you can have multiple data directories simply by running the program in different directories. This behavior is more consistent with [Dolt's](https://github.com/dolthub/dolt) behavior. This is the recommended option for development.
34+
2. Specify the directory in the `--data-dir` argument. This overrides the environment variable if it is present.
3535

3636
### Note for Windows Users
3737

‎README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ If you are interested in using Doltgres now or in the future, please:
4343

4444
2. Put `doltgres` on your `PATH`
4545

46-
3. Run `doltgres`. This will create a `doltgres` user and a `doltgres` database in `~/doltgres/databases` (change the `DOLTGRES_DATA_DIR` environment variable to use a different directory).
46+
3. Run `doltgres`. This will create a `doltgres` user and a `doltgres` database in `~/doltgres/databases` (add the `--data-dir` argument or change the `DOLTGRES_DATA_DIR` environment variable to use a different directory).
4747
```bash
4848
$ doltgres
4949
Successfully initialized dolt data repository.

‎main.go

Lines changed: 105 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -84,42 +84,10 @@ func main() {
8484
warnIfMaxFilesTooLow()
8585

8686
var fs filesys.Filesys
87-
if (len(args) >= 1 && args[0] == "init") || (strings.ToLower(os.Getenv(server.DOLTGRES_DATA_DIR_CWD)) == "true") {
88-
// If "init" is passed as the command, or DOLTGRES_DATA_DIR_CWD is set to "true", then we use the current directory
89-
fs = filesys.LocalFS
90-
} else {
91-
// We should use the directory as pointed to by "DOLTGRES_DATA_DIR", if has been set, otherwise we'll use the default
92-
var dbDir string
93-
if envDir := os.Getenv(server.DOLTGRES_DATA_DIR); len(envDir) > 0 {
94-
dbDir = envDir
95-
} else {
96-
homeDir, err := env.GetCurrentUserHomeDir()
97-
if err != nil {
98-
cli.PrintErrln(err.Error())
99-
os.Exit(1)
100-
}
101-
dbDir = filepath.Join(homeDir, server.DOLTGRES_DATA_DIR_DEFAULT)
102-
fileInfo, err := os.Stat(dbDir)
103-
if os.IsNotExist(err) {
104-
if err = os.MkdirAll(dbDir, 0755); err != nil {
105-
cli.PrintErrln(err.Error())
106-
os.Exit(1)
107-
}
108-
} else if err != nil {
109-
cli.PrintErrln(err.Error())
110-
os.Exit(1)
111-
} else if !fileInfo.IsDir() {
112-
cli.PrintErrln(fmt.Sprintf("Attempted to use the directory `%s` as the DoltgreSQL database directory, "+
113-
"however the preceding is a file and not a directory. Please change the environment variable `%s` so "+
114-
"that is points to a directory.", dbDir, server.DOLTGRES_DATA_DIR))
115-
os.Exit(1)
116-
}
117-
}
118-
fs, err = filesys.LocalFilesysWithWorkingDir(dbDir)
119-
if err != nil {
120-
cli.PrintErrln(err.Error())
121-
os.Exit(1)
122-
}
87+
fs, args, err = loadFileSystem(args)
88+
if err != nil {
89+
cli.PrintErrln(err.Error())
90+
os.Exit(1)
12391
}
12492
dEnv := env.Load(ctx, env.GetCurrentUserHomeDir, fs, doltdb.LocalDirDoltDB, server.Version)
12593

@@ -158,6 +126,107 @@ func main() {
158126
os.Exit(exitCode)
159127
}
160128

129+
// loadFileSystem loads the file system from a combination of the given arguments and environment variables.
130+
func loadFileSystem(args []string) (fs filesys.Filesys, outArgs []string, err error) {
131+
// We can't use the argument parser yet since it relies on the environment, so we'll handle the data directory
132+
// argument here. This will also remove it from the args, so that the Dolt layer doesn't try to move the directory
133+
// again (in the case of relative paths).
134+
var dataDir string
135+
var hasDataDirArgument bool
136+
for i, arg := range args {
137+
arg = strings.ToLower(arg)
138+
if arg == "--data-dir" {
139+
if len(args) <= i+1 {
140+
return nil, args, fmt.Errorf("--data-dir is missing the directory")
141+
}
142+
dataDir = args[i+1]
143+
hasDataDirArgument = true
144+
// os.Args is read from Dolt, so we have to update it, else we'll apply the move twice
145+
newArgs := make([]string, len(args)-2)
146+
copy(newArgs, args[:i])
147+
copy(newArgs[i:], args[i+2:])
148+
args = newArgs
149+
os.Args = os.Args[:len(args)+1]
150+
copy(os.Args[1:], args)
151+
break
152+
} else if strings.HasPrefix(arg, "--data-dir=") {
153+
dataDir = arg[11:]
154+
hasDataDirArgument = true
155+
// os.Args is read from Dolt, so we have to update it, else we'll apply the move twice
156+
newArgs := make([]string, len(args)-1)
157+
copy(newArgs, args[:i])
158+
copy(newArgs[i:], args[i+1:])
159+
args = newArgs
160+
os.Args = os.Args[:len(args)+1]
161+
copy(os.Args[1:], args)
162+
}
163+
}
164+
165+
if len(args) >= 1 && args[0] == "init" {
166+
// If "init" is passed as the command, then we use the current directory
167+
fs = filesys.LocalFS
168+
} else if hasDataDirArgument {
169+
// If the --data-dir argument was used, then we'll use the directory that it points to
170+
fileInfo, err := os.Stat(dataDir)
171+
if os.IsNotExist(err) {
172+
if err = os.MkdirAll(dataDir, 0755); err != nil {
173+
return nil, args, err
174+
}
175+
} else if err != nil {
176+
return nil, args, err
177+
} else if !fileInfo.IsDir() {
178+
return nil, args, fmt.Errorf("Attempted to use the directory `%s` as the DoltgreSQL database directory, "+
179+
"however the preceding is a file and not a directory. Please change the `--data-dir` argument so "+
180+
"that it points to a directory.", dataDir)
181+
}
182+
fs, err = filesys.LocalFilesysWithWorkingDir(dataDir)
183+
if err != nil {
184+
return nil, args, err
185+
}
186+
} else {
187+
// We should use the directory as pointed to by "DOLTGRES_DATA_DIR", if has been set, otherwise we'll use the default
188+
var dbDir string
189+
if envDir := os.Getenv(server.DOLTGRES_DATA_DIR); len(envDir) > 0 {
190+
dbDir = envDir
191+
fileInfo, err := os.Stat(dbDir)
192+
if os.IsNotExist(err) {
193+
if err = os.MkdirAll(dbDir, 0755); err != nil {
194+
return nil, args, err
195+
}
196+
} else if err != nil {
197+
return nil, args, err
198+
} else if !fileInfo.IsDir() {
199+
return nil, args, fmt.Errorf("Attempted to use the directory `%s` as the DoltgreSQL database directory, "+
200+
"however the preceding is a file and not a directory. Please change the environment variable `%s` so "+
201+
"that it points to a directory.", dbDir, server.DOLTGRES_DATA_DIR)
202+
}
203+
} else {
204+
homeDir, err := env.GetCurrentUserHomeDir()
205+
if err != nil {
206+
return nil, args, err
207+
}
208+
dbDir = filepath.Join(homeDir, server.DOLTGRES_DATA_DIR_DEFAULT)
209+
fileInfo, err := os.Stat(dbDir)
210+
if os.IsNotExist(err) {
211+
if err = os.MkdirAll(dbDir, 0755); err != nil {
212+
return nil, args, err
213+
}
214+
} else if err != nil {
215+
return nil, args, err
216+
} else if !fileInfo.IsDir() {
217+
return nil, args, fmt.Errorf("Attempted to use the directory `%s` as the DoltgreSQL database directory, "+
218+
"however the preceding is a file and not a directory. Please change the environment variable `%s` so "+
219+
"that it points to a directory.", dbDir, server.DOLTGRES_DATA_DIR)
220+
}
221+
}
222+
fs, err = filesys.LocalFilesysWithWorkingDir(dbDir)
223+
if err != nil {
224+
return nil, args, err
225+
}
226+
}
227+
return fs, args, nil
228+
}
229+
161230
func configureCliCtx(subcommand string, apr *argparser.ArgParseResults, fs filesys.Filesys, dEnv *env.DoltEnv, ctx context.Context) (cli.CliContext, error) {
162231
dataDir, hasDataDir := apr.GetValue(commands.DataDirFlag)
163232
if hasDataDir {

‎scripts/run_staticcheck.sh

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
#!/bin/bash
2+
3+
# Set the working directory to the directory of the script's location
4+
cd "$(cd -P -- "$(dirname -- "$0")" && pwd -P)"
5+
cd ..
6+
7+
go run honnef.co/go/tools/cmd/[email protected] ./...

‎server/server.go

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -43,10 +43,6 @@ const (
4343
DOLTGRES_DATA_DIR = "DOLTGRES_DATA_DIR"
4444
// DOLTGRES_DATA_DIR_DEFAULT is the portion to append to the user's home directory if DOLTGRES_DATA_DIR has not been specified
4545
DOLTGRES_DATA_DIR_DEFAULT = "doltgres/databases"
46-
// DOLTGRES_DATA_DIR_CWD is an environment variable that causes DoltgreSQL to use the current directory for the
47-
// location of DoltgreSQL databases, rather than the DOLTGRES_DATA_DIR. This means that it has priority over
48-
// DOLTGRES_DATA_DIR.
49-
DOLTGRES_DATA_DIR_CWD = "DOLTGRES_DATA_DIR_CWD"
5046
)
5147

5248
var sqlServerDocs = cli.CommandDocumentationContent{
@@ -61,7 +57,7 @@ var sqlServerDocs = cli.CommandDocumentationContent{
6157
indentLines(sqlserver.ServerConfigAsYAMLConfig(sqlserver.DefaultServerConfig()).String()) + "\n\n" + `
6258
SUPPORTED CONFIG FILE FIELDS:
6359
64-
{{.EmphasisLeft}}data_dir{{.EmphasisRight}}: A directory where the server will load dolt databases to serve, and create new ones. Defaults to the current directory.
60+
{{.EmphasisLeft}}data_dir{{.EmphasisRight}}: A directory where the server will load dolt databases to serve, and create new ones. Defaults to the DOLTGRES_DATA_DIR environment variable, or {{.EmphasisLeft}}~/doltgres/databases{{.EmphasisRight}}.
6561
6662
{{.EmphasisLeft}}cfg_dir{{.EmphasisRight}}: A directory where the server will load and store non-database configuration data, such as permission information. Defaults {{.EmphasisLeft}}$data_dir/.doltcfg{{.EmphasisRight}}.
6763
@@ -220,8 +216,8 @@ func runServer(ctx context.Context, args []string, dEnv *env.DoltEnv) (*svcs.Con
220216
// The else branch means that there's a Doltgres item, so we need to error if it's a file since we
221217
// enforce the creation of a Doltgres database/directory, which would create a name conflict with the file
222218
return nil, fmt.Errorf("Attempted to create the default `doltgres` database at `%s`, but a file with "+
223-
"the same name was found. Either remove the file, or change the environment variable `%s` so that it "+
224-
"points to a different directory.", workingDir, DOLTGRES_DATA_DIR)
219+
"the same name was found. Either remove the file, change the directory using the `--data-dir` argument, "+
220+
"or change the environment variable `%s` so that it points to a different directory.", workingDir, DOLTGRES_DATA_DIR)
225221
}
226222
}
227223

‎testing/bats/setup/common.bash

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,7 @@ setup_common() {
5656
# multiple tests can be run in parallel on the same machine
5757
mkdir "dolt-repo-$$"
5858
cd "dolt-repo-$$"
59-
nativevar DOLTGRES_DATA_DIR "$(pwd)" /p # This has to be set in every function that calls doltgresql
60-
nativevar DOLTGRES_DATA_DIR_CWD "" /w
59+
nativevar DOLTGRES_DATA_DIR "$(pwd)" /p
6160

6261
mkdir "postgres"
6362
cd "postgres"

‎testing/bats/setup/query-server-common.bash

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -30,15 +30,13 @@ start_sql_server() {
3030
DEFAULT_DB="$1"
3131
DEFAULT_DB="${DEFAULT_DB:=postgres}"
3232
nativevar DEFAULT_DB "$DEFAULT_DB" /w
33-
nativevar DOLTGRES_DATA_DIR "$(pwd)" /p # This has to be set in every function that calls doltgresql
34-
nativevar DOLTGRES_DATA_DIR_CWD "" /w
3533
logFile="$2"
3634
PORT=$( definePORT )
3735
if [[ $logFile ]]
3836
then
39-
doltgresql --host 0.0.0.0 --port=$PORT --user "${SQL_USER:-postgres}" > $logFile 2>&1 &
37+
doltgresql --host 0.0.0.0 --port=$PORT --data-dir=. --user "${SQL_USER:-postgres}" > $logFile 2>&1 &
4038
else
41-
doltgresql --host 0.0.0.0 --port=$PORT --user "${SQL_USER:-postgres}" &
39+
doltgresql --host 0.0.0.0 --port=$PORT --data-dir=. --user "${SQL_USER:-postgres}" &
4240
fi
4341
SERVER_PID=$!
4442
wait_for_connection $PORT 7500
@@ -50,8 +48,7 @@ start_sql_server() {
5048
start_sql_server_with_args() {
5149
DEFAULT_DB=""
5250
nativevar DEFAULT_DB "$DEFAULT_DB" /w
53-
nativevar DOLTGRES_DATA_DIR "$(pwd)" /p # This has to be set in every function that calls doltgresql
54-
nativevar DOLTGRES_DATA_DIR_CWD "" /w
51+
nativevar DOLTGRES_DATA_DIR "$(pwd)" /p
5552
PORT=$( definePORT )
5653
doltgresql "$@" --port=$PORT &
5754
SERVER_PID=$!
@@ -62,8 +59,7 @@ start_sql_server_with_config() {
6259
DEFAULT_DB="$1"
6360
DEFAULT_DB="${DEFAULT_DB:=postgres}"
6461
nativevar DEFAULT_DB "$DEFAULT_DB" /w
65-
nativevar DOLTGRES_DATA_DIR "$(pwd)" /p # This has to be set in every function that calls doltgresql
66-
nativevar DOLTGRES_DATA_DIR_CWD "" /w
62+
nativevar DOLTGRES_DATA_DIR "$(pwd)" /p
6763
PORT=$( definePORT )
6864
echo "
6965
log_level: debug
@@ -89,8 +85,7 @@ start_sql_multi_user_server() {
8985
DEFAULT_DB="$1"
9086
DEFAULT_DB="${DEFAULT_DB:=postgres}"
9187
nativevar DEFAULT_DB "$DEFAULT_DB" /w
92-
nativevar DOLTGRES_DATA_DIR "$(pwd)" /p # This has to be set in every function that calls doltgresql
93-
nativevar DOLTGRES_DATA_DIR_CWD "" /w
88+
nativevar DOLTGRES_DATA_DIR "$(pwd)" /p
9489
PORT=$( definePORT )
9590
echo "
9691
log_level: debug
@@ -115,8 +110,6 @@ start_multi_db_server() {
115110
DEFAULT_DB="$1"
116111
DEFAULT_DB="${DEFAULT_DB:=postgres}"
117112
nativevar DEFAULT_DB "$DEFAULT_DB" /w
118-
nativevar DOLTGRES_DATA_DIR "$(pwd)" /p # This has to be set in every function that calls doltgresql
119-
nativevar DOLTGRES_DATA_DIR_CWD "" /w
120113
PORT=$( definePORT )
121114
doltgresql --host 0.0.0.0 --port=$PORT --user postgres --data-dir ./ &
122115
SERVER_PID=$!

0 commit comments

Comments
 (0)