From a08b48adaa2ca4c828037da6579720069ec52da5 Mon Sep 17 00:00:00 2001 From: Nick Craig-Wood Date: Sat, 11 Apr 2026 12:18:35 +0100 Subject: [PATCH] gui: don't run fetch-gui on make - Fail gracefully if `make fetch-gui` hasn't been run - Return errors instead of panic or fatal errrors - Don't run `make fetch-gui` on every make since we have it in the workflow --- .gitignore | 2 -- Makefile | 2 +- bin/fetch-gui-dist.sh | 4 ++++ cmd/gui/dist/.gitignore | 4 ++++ cmd/gui/dist/README.md | 1 + cmd/gui/gui.go | 45 +++++++++++++++++++++++------------------ cmd/gui/gui_test.go | 19 +++++++++++++---- 7 files changed, 50 insertions(+), 27 deletions(-) create mode 100644 cmd/gui/dist/.gitignore create mode 100644 cmd/gui/dist/README.md diff --git a/.gitignore b/.gitignore index 728b6832a..70efa8580 100644 --- a/.gitignore +++ b/.gitignore @@ -20,5 +20,3 @@ __pycache__ .DS_Store resource_windows_*.syso .devcontainer -# GUI dist is fetched at build time via `make fetch-gui` -cmd/gui/dist/ \ No newline at end of file diff --git a/Makefile b/Makefile index 3541f2461..9ab411ea6 100644 --- a/Makefile +++ b/Makefile @@ -47,7 +47,7 @@ LDFLAGS=--ldflags "-s -X github.com/rclone/rclone/fs.Version=$(TAG)" .PHONY: rclone test_all vars version fetch-gui -rclone: fetch-gui +rclone: ifeq ($(GO_OS),windows) go run bin/resource_windows.go -version $(TAG) -syso resource_windows_`go env GOARCH`.syso endif diff --git a/bin/fetch-gui-dist.sh b/bin/fetch-gui-dist.sh index bf37bbebb..e15a3fe94 100755 --- a/bin/fetch-gui-dist.sh +++ b/bin/fetch-gui-dist.sh @@ -71,6 +71,10 @@ rm -rf "${DEST}" mkdir -p "${DEST}" unzip -q "${TMPFILE}" -d "${DEST}" +# Restore marker files +git checkout "${DEST}"/.gitignore +git checkout "${DEST}"/README.md + # Write tag for cache comparison echo -n "${TAG}" > "${TAG_FILE}" diff --git a/cmd/gui/dist/.gitignore b/cmd/gui/dist/.gitignore new file mode 100644 index 000000000..58bbeb139 --- /dev/null +++ b/cmd/gui/dist/.gitignore @@ -0,0 +1,4 @@ +# This directory gets the web gui release which we ignore +* +# Except this file which we use to keep the directory available +!.gitignore diff --git a/cmd/gui/dist/README.md b/cmd/gui/dist/README.md new file mode 100644 index 000000000..45a48533b --- /dev/null +++ b/cmd/gui/dist/README.md @@ -0,0 +1 @@ +Use `make fetch-gui` to populate this directory. diff --git a/cmd/gui/gui.go b/cmd/gui/gui.go index 17eb83c78..4e9a56b3b 100644 --- a/cmd/gui/gui.go +++ b/cmd/gui/gui.go @@ -80,11 +80,11 @@ Use --no-auth to disable authentication entirely: "versionIntroduced": "v1.74", "groups": "RC", }, - Run: func(command *cobra.Command, args []string) { + RunE: func(command *cobra.Command, args []string) error { cmd.CheckArgs(0, 0, command, args) ctx := context.Background() - // --- 1. Create the GUI server (binds port eagerly, before Serve) --- + // Create the GUI server (binds port eagerly, before Serve) guiCfg := libhttp.DefaultCfg() if command.Flags().Changed("addr") { guiCfg.ListenAddr = guiAddr @@ -93,13 +93,13 @@ Use --no-auth to disable authentication entirely: } guiServer, err := libhttp.NewServer(ctx, libhttp.WithConfig(guiCfg)) if err != nil { - fs.Fatalf(nil, "Failed to create GUI server: %v", err) + return fmt.Errorf("failed to create GUI server: %w", err) } // Read the GUI origin from the bound address (available before Serve). guiOrigin := originFromURL(guiServer.URLs()[0]) - // --- 2. Configure the RC API server --- + // Configure the RC API server opt := rc.Opt // copy global defaults opt.Enabled = true opt.WebUI = false @@ -110,7 +110,7 @@ Use --no-auth to disable authentication entirely: } else { port, err := freePort() if err != nil { - fs.Fatalf(nil, "Failed to find a free port for RC: %v", err) + return fmt.Errorf("failed to find a free port for RC: %w", err) } opt.HTTP.ListenAddr = []string{fmt.Sprintf("localhost:%d", port)} } @@ -123,7 +123,7 @@ Use --no-auth to disable authentication entirely: opt.EnableMetrics = enableMetrics } - // --- 3. Generate credentials if needed --- + // Generate credentials if needed if command.Flags().Changed("user") { opt.Auth.BasicUser = user } @@ -142,28 +142,28 @@ Use --no-auth to disable authentication entirely: if opt.Auth.BasicPass == "" { randomPass, err := random.Password(128) if err != nil { - fs.Fatalf(nil, "Failed to make password: %v", err) + return fmt.Errorf("failed to make password: %w", err) } opt.Auth.BasicPass = randomPass fs.Infof(nil, "No password specified. Using random password: %s", randomPass) } } - // --- 4. Start the RC server (unchanged rcserver.Start) --- + // Start the RC server (unchanged rcserver.Start) rcServer, err := rcserver.Start(ctx, &opt) - if err != nil { - fs.Fatalf(nil, "Failed to start RC server: %v", err) - } - if rcServer == nil { - fs.Fatal(nil, "RC server not configured") + if err != nil || rcServer == nil { + return fmt.Errorf("failed to start RC server: %w", err) } // Build the RC URL from the address we configured (rcserver.Server // does not expose URLs, and we know the address we passed in). rcURL := "http://" + opt.HTTP.ListenAddr[0] + "/" - // --- 5. Mount the embedded GUI handler and start serving --- - spaHandler := guiHandler() + // Mount the embedded GUI handler and start serving + spaHandler, err := guiHandler() + if err != nil || spaHandler == nil { + return fmt.Errorf("failed to start GUI handler: %w", err) + } guiServer.Router().Get("/*", spaHandler.ServeHTTP) guiServer.Router().Head("/*", spaHandler.ServeHTTP) guiServer.Serve() @@ -171,7 +171,7 @@ Use --no-auth to disable authentication entirely: guiURL := guiServer.URLs()[0] fs.Logf(nil, "Serving GUI on %s", guiURL) - // --- 6. Open browser --- + // Open browser loginURL := buildLoginURL(guiURL, rcURL, opt.Auth.BasicUser, opt.Auth.BasicPass, opt.NoAuth) fs.Logf(nil, "GUI available at %s", loginURL) @@ -181,7 +181,7 @@ Use --no-auth to disable authentication entirely: } } - // --- 7. Wait for either server to exit, then shut both down --- + // Wait for either server to exit, then shut both down defer systemd.Notify()() done := make(chan struct{}, 2) go func() { rcServer.Wait(); done <- struct{}{} }() @@ -189,6 +189,7 @@ Use --no-auth to disable authentication entirely: <-done _ = rcServer.Shutdown() _ = guiServer.Shutdown() + return nil }, } @@ -214,10 +215,14 @@ func originFromURL(rawURL string) string { // guiHandler returns an http.Handler that serves the embedded GUI bundle // with SPA fallback: paths that don't match a real file return index.html. -func guiHandler() http.Handler { +func guiHandler() (http.Handler, error) { sub, err := iofs.Sub(assets, "dist") if err != nil { - panic("gui: embedded dist missing: " + err.Error()) + return nil, fmt.Errorf("embedded GUI dir not found: was `make fetch-gui` run before building?: %w", err) + } + _, err = iofs.Stat(sub, "index.html") + if err != nil { + return nil, fmt.Errorf("embedded GUI not found: was `make fetch-gui` run before building?: %w", err) } fileServer := http.FileServer(http.FS(sub)) return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -233,7 +238,7 @@ func guiHandler() http.Handler { // client-side routing (e.g. /login) works. r.URL.Path = "/" fileServer.ServeHTTP(w, r) - }) + }), nil } // guiBaseURL is the GUI server's URL. rcURL is the RC API server's URL. diff --git a/cmd/gui/gui_test.go b/cmd/gui/gui_test.go index 2d99704e0..6fc562aa0 100644 --- a/cmd/gui/gui_test.go +++ b/cmd/gui/gui_test.go @@ -94,8 +94,19 @@ func TestFreePort(t *testing.T) { assert.Less(t, port, 65536) } +// newTestHandler returns a guiHandler, or skips the test if the embedded +// GUI bundle is not present (i.e. `make fetch-gui` has not been run). +func newTestHandler(t *testing.T) http.Handler { + t.Helper() + h, err := guiHandler() + if err != nil { + t.Skipf("skipping: GUI dist not embedded (run `make fetch-gui`): %v", err) + } + return h +} + func TestHandlerServesIndexHTML(t *testing.T) { - h := guiHandler() + h := newTestHandler(t) req := httptest.NewRequest("GET", "/", nil) w := httptest.NewRecorder() h.ServeHTTP(w, req) @@ -109,7 +120,7 @@ func TestHandlerServesIndexHTML(t *testing.T) { } func TestHandlerServesStaticAssets(t *testing.T) { - h := guiHandler() + h := newTestHandler(t) req := httptest.NewRequest("GET", "/icon.svg", nil) w := httptest.NewRecorder() h.ServeHTTP(w, req) @@ -123,7 +134,7 @@ func TestHandlerServesStaticAssets(t *testing.T) { } func TestHandlerSPAFallback(t *testing.T) { - h := guiHandler() + h := newTestHandler(t) // /login is not a real file — it should fall back to index.html req := httptest.NewRequest("GET", "/login", nil) @@ -140,7 +151,7 @@ func TestHandlerSPAFallback(t *testing.T) { } func TestHandlerSPAFallbackDeepPath(t *testing.T) { - h := guiHandler() + h := newTestHandler(t) req := httptest.NewRequest("GET", "/some/deep/route", nil) w := httptest.NewRecorder()