diff --git a/internal/plugin/plugin.go b/internal/plugin/plugin.go index cf2834559e8..b5d127dccab 100644 --- a/internal/plugin/plugin.go +++ b/internal/plugin/plugin.go @@ -7,6 +7,7 @@ import ( "bytes" "cmp" "encoding/json" + "fmt" "io/fs" "log/slog" "os" @@ -17,6 +18,7 @@ import ( "github.com/pkg/errors" "github.com/tailscale/hujson" + "go.jetify.com/devbox/internal/cachehash" "go.jetify.com/devbox/internal/devconfig/configfile" "go.jetify.com/devbox/internal/devpkg" "go.jetify.com/devbox/internal/lock" @@ -54,6 +56,66 @@ type PluginOnlyData struct { Source Includable } +// Hash returns a hash of the plugin's config that, for local (path:) plugins, +// also incorporates the contents of the files referenced by create_files. The +// embedded ConfigFile.Hash only covers the plugin.json itself, so without this +// a change to a create_files source file leaves the project's state hash +// unchanged and the file is never re-created in the virtenv. This matters for +// local plugins under active development, whose source files can change without +// any edit to devbox.json or the plugin.json. +// See https://github.com/jetify-com/devbox/issues/2755 +// +// Only local plugins are handled: for built-in/git/github plugins the source +// content is stable for a given plugin version, and reading it here would risk +// network I/O (HTTP fetch / git clone) on the otherwise-fast "is up to date" +// path, making `devbox shell` slow or fail offline. +func (c *Config) Hash() (string, error) { + h, err := c.ConfigFile.Hash() + if err != nil { + return "", err + } + local, ok := c.Source.(*LocalPlugin) + if !ok || len(c.CreateFiles) == 0 { + return h, nil + } + + buf := bytes.Buffer{} + buf.WriteString(h) + + // Iterate deterministically so the hash is stable across runs. + filePaths := make([]string, 0, len(c.CreateFiles)) + for filePath := range c.CreateFiles { + filePaths = append(filePaths, filePath) + } + slices.Sort(filePaths) + + for _, filePath := range filePaths { + contentPath := c.CreateFiles[filePath] + + // Fold in a fixed-size hash of the source content rather than the raw + // bytes, so the buffer stays small even for large source files. A + // missing or unreadable source file should not hard-fail the shell, so + // it just contributes an empty content hash. + contentHash := "" + if contentPath != "" { + if content, err := local.FileContent(contentPath); err == nil { + contentHash = cachehash.Bytes(content) + } + } + + // Length-prefix every field (netstring-style) so the concatenation is + // unambiguous: no two distinct (filePath, contentPath, content) sets + // can produce the same byte stream. + fmt.Fprintf(&buf, "%d:%s%d:%s%d:%s", + len(filePath), filePath, + len(contentPath), contentPath, + len(contentHash), contentHash, + ) + } + + return cachehash.Bytes(buf.Bytes()), nil +} + func (c *Config) ProcessComposeYaml() (string, string) { for file, contentPath := range c.CreateFiles { if strings.HasSuffix(file, "process-compose.yaml") || strings.HasSuffix(file, "process-compose.yml") { diff --git a/internal/plugin/plugin_test.go b/internal/plugin/plugin_test.go new file mode 100644 index 00000000000..cb424b12a55 --- /dev/null +++ b/internal/plugin/plugin_test.go @@ -0,0 +1,65 @@ +// Copyright 2024 Jetify Inc. and contributors. All rights reserved. +// Use of this source code is governed by the license in the LICENSE file. + +package plugin + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.jetify.com/devbox/nix/flake" +) + +// TestConfigHashIncludesCreateFilesContent verifies that a plugin's hash +// changes when the content of a create_files source file changes. This is what +// makes local plugins under active development re-create their virtenv files +// when their source changes (https://github.com/jetify-com/devbox/issues/2755). +func TestConfigHashIncludesCreateFilesContent(t *testing.T) { + pluginDir := t.TempDir() + projectDir := t.TempDir() + + pluginJSON := `{ + "name": "testplugin", + "version": "0.0.1", + "create_files": { + "{{ .Virtenv }}/test.txt": "test.txt" + } + }` + require.NoError(t, os.WriteFile( + filepath.Join(pluginDir, "plugin.json"), []byte(pluginJSON), 0o644)) + srcFile := filepath.Join(pluginDir, "test.txt") + require.NoError(t, os.WriteFile(srcFile, []byte("123"), 0o644)) + + cfg := localPluginConfigForTest(t, pluginDir, projectDir) + + hash1, err := cfg.Hash() + require.NoError(t, err) + + // Re-hashing without any change must be stable. + hash1Again, err := cfg.Hash() + require.NoError(t, err) + assert.Equal(t, hash1, hash1Again, "hash should be stable when nothing changes") + + // Changing the create_files source content must change the hash so that the + // file gets re-created in the virtenv on the next shell. + require.NoError(t, os.WriteFile(srcFile, []byte("456"), 0o644)) + hash2, err := cfg.Hash() + require.NoError(t, err) + assert.NotEqual(t, hash1, hash2, + "hash should change when create_files source content changes") +} + +func localPluginConfigForTest(t *testing.T, pluginDir, projectDir string) *Config { + t.Helper() + ref, err := flake.ParseRef("path:" + pluginDir) + require.NoError(t, err) + localPlugin, err := newLocalPlugin(ref, projectDir) + require.NoError(t, err) + cfg, err := getConfigIfAny(localPlugin, projectDir) + require.NoError(t, err) + require.NotNil(t, cfg) + return cfg +}