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

internal/devconfig: move project directory search into devbox.Find #2172

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

gcurtis
Copy link
Collaborator

@gcurtis gcurtis commented Jun 25, 2024

Instead of first searching for a project directory and then calling devbox.Open on that path, add a devbox.Find method that performs the search and open as a single step.

  • Keeping Find a distinct function from Open makes it clearer when we're recursively searching for a config vs. not.
  • Find can just try reading a potential config file path instead of performing a separate os.Stat. This halves the number of system calls at startup and eliminates any potential races between the call to os.Stat and os.Open.
// Open loads a Devbox config from a file or project directory.
// For use with `devbox -c some/dir <subcmd>`.
func Open(path string) (*Config, error)

// Find is like [Open] except it recursively searches up the directory tree.
// For use with `devbox <subcmd>` (without a `-c` flag).
func Find(path string) (*Config, error)

Changes to user error messages:

 $ devbox add go

-Error: No devbox.json found in this directory, or any parent directories. Did you run `devbox init` yet?
+Error: no devbox.json found in the current directory (or any parent directories). Did you run `devbox init` yet?

 $ devbox -c badpath add go
-Error: stat /var/folders/79/1yc1ywp10w9f2xnr_rpp_ff00000gn/T/tmp.bU25JVWovO/badpath: no such file or directory
+
+Error: the devbox config path "badpath" does not exist.

 $ mkdir child
 $ devbox -c child add go

-Error: No devbox.json found in child. Did you run `devbox init` yet?
+Error: no devbox.json found in "child". Did you run `devbox init` yet?

Instead of first searching for a project directory and then calling
`devbox.Open` on that path, add a `devbox.Find` method that performs the search
and open as a single step.

- Keeping `Find` a distinct function from `Open` makes it clearer when we're
  recursively searching for a config vs. not.
- `Find` can just try reading a potential config file path instead of performing
  a separate `os.Stat`. This halves the number of system calls at startup and
  eliminates any potential races between the call to `os.Stat` and `os.Open`.

```go
// Open loads a Devbox config from a file or project directory.
// For use with `devbox -c some/dir <subcmd>`.
func Open(path string) (*Config, error)

// Find is like [Open] except it recursively searches up the directory tree.
// For use with `devbox <subcmd>` (without a `-c` flag).
func Find(path string) (*Config, error)
```

Changes to user error messages:

```diff
 $ devbox add go

-Error: No devbox.json found in this directory, or any parent directories. Did you run `devbox init` yet?
+Error: no devbox.json found in the current directory (or any parent directories). Did you run `devbox init` yet?

 $ devbox -c badpath add go
-Error: stat /var/folders/79/1yc1ywp10w9f2xnr_rpp_ff00000gn/T/tmp.bU25JVWovO/badpath: no such file or directory
+
+Error: the devbox config path "badpath" does not exist.

 $ mkdir child
 $ devbox -c child add go

-Error: No devbox.json found in child. Did you run `devbox init` yet?
+Error: no devbox.json found in "child". Did you run `devbox init` yet?
```
@gcurtis gcurtis marked this pull request as ready for review June 26, 2024 18:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant