From aec5ee49e067aa81395a60fcb30af2e27335d41b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Thu, 14 Jul 2022 12:22:56 +0200 Subject: [PATCH] First-Time Wizard: allow selecting Blender executables The wizard now finds Blender in various ways, and lets the user select which one to use. Doesn't save anything yet, though. --- internal/find_blender/find_blender.go | 95 +++++++++++++++- internal/find_blender/find_blender_test.go | 38 +++++++ internal/find_blender/nonwindows.go | 4 +- internal/find_blender/windows.go | 10 +- internal/find_blender/windows_test.go | 10 +- internal/manager/api_impl/interfaces.go | 4 + internal/manager/api_impl/meta.go | 85 ++++++++++++++ .../api_impl/mocks/api_impl_mock.gen.go | 12 ++ internal/worker/command_blender.go | 6 +- internal/worker/mocks/client.gen.go | 60 ++++++++++ web/app/src/assets/base.css | 16 +++ web/app/src/views/FirstTimeWizardView.vue | 104 +++++++++++++++++- 12 files changed, 422 insertions(+), 22 deletions(-) create mode 100644 internal/find_blender/find_blender_test.go diff --git a/internal/find_blender/find_blender.go b/internal/find_blender/find_blender.go index fbe790a2..13ccb107 100644 --- a/internal/find_blender/find_blender.go +++ b/internal/find_blender/find_blender.go @@ -2,13 +2,96 @@ package find_blender // SPDX-License-Identifier: GPL-3.0-or-later -import "errors" +import ( + "context" + "errors" + "os/exec" + "strings" + + "git.blender.org/flamenco/pkg/api" + "git.blender.org/flamenco/pkg/crosspath" + "github.com/rs/zerolog/log" +) var ErrNotAvailable = errors.New("not available on this platform") -// FindBlender returns the full path of a Blender executable. -// `ErrNotAvailable` is returned if no "blender finder" is available for the current platform. -func FindBlender() (string, error) { - // findBlender() is implemented in one of the platform-dependent files. - return findBlender() +type CheckBlenderResult struct { + Input string // What was the original 'exename' CheckBlender was told to find. + FoundLocation string + BlenderVersion string + Source api.BlenderPathSource +} + +// FileAssociation returns the full path of a Blender executable, by inspecting file association with .blend files. +// `ErrNotAvailable` is returned if no "blender finder" is available for the current platform. +func FileAssociation() (string, error) { + // findBlender() is implemented in one of the platform-dependent files. + return fileAssociation() +} + +func CheckBlender(ctx context.Context, exename string) (CheckBlenderResult, error) { + if exename == "" { + // exename is not given, see if we can use .blend file association. + fullPath, err := fileAssociation() + switch { + case err == ErrNotAvailable: + // Association finder not available, act as if "blender" was given as exename. + return CheckBlender(ctx, "blender") + case err != nil: + // Some other error occurred, better to report it. + return CheckBlenderResult{}, err + default: + // The full path was found, report the Blender version. + return getResultWithVersion(ctx, exename, fullPath, api.BlenderPathSourceFileAssociation) + } + } + + if crosspath.Dir(exename) != "." { + // exename is some form of path, see if it works directly as executable. + return getResultWithVersion(ctx, exename, exename, api.BlenderPathSourceInputPath) + } + + // Try to find exename on $PATH + fullPath, err := exec.LookPath(exename) + if err != nil { + return CheckBlenderResult{}, err + } + return getResultWithVersion(ctx, exename, fullPath, api.BlenderPathSourcePathEnvvar) +} + +// getResultWithVersion tries to run the command to get Blender's version. +// The result is returned as a `CheckBlenderResult` struct. +func getResultWithVersion( + ctx context.Context, + input, + commandline string, + source api.BlenderPathSource, +) (CheckBlenderResult, error) { + result := CheckBlenderResult{ + Input: input, + FoundLocation: commandline, + Source: source, + } + + version, err := getBlenderVersion(ctx, commandline) + if err != nil { + return result, err + } + + result.BlenderVersion = version + return result, nil +} + +func getBlenderVersion(ctx context.Context, commandline string) (string, error) { + logger := log.With().Str("commandline", commandline).Logger() + + cmd := exec.CommandContext(ctx, commandline, "--version") + stdoutStderr, err := cmd.CombinedOutput() + if err != nil { + logger.Info().Err(err).Str("output", string(stdoutStderr)).Msg("error running command") + return "", err + } + + version := strings.TrimSpace(string(stdoutStderr)) + return version, nil } diff --git a/internal/find_blender/find_blender_test.go b/internal/find_blender/find_blender_test.go new file mode 100644 index 00000000..ded05a0c --- /dev/null +++ b/internal/find_blender/find_blender_test.go @@ -0,0 +1,38 @@ +package find_blender + +// SPDX-License-Identifier: GPL-3.0-or-later + +import ( + "context" + "flag" + "os/exec" + "testing" + + "github.com/stretchr/testify/assert" +) + +var withBlender = flag.Bool("withBlender", false, "run test that requires Blender to be installed") + +func TestGetBlenderVersion(t *testing.T) { + if !*withBlender { + t.Skip("skipping test, -withBlender arg not passed") + } + + path, err := exec.LookPath("blender") + if !assert.NoError(t, err) { + t.Fatal("running with -withBlender requires having a `blender` command on $PATH") + } + + ctx := context.Background() + + // Try finding version from "/path/to/blender": + version, err := getBlenderVersion(ctx, path) + if assert.NoError(t, err) { + assert.Contains(t, version, "Blender") + } + + // Try non-existing executable: + version, err = getBlenderVersion(ctx, "This-Blender-Executable-Does-Not-Exist") + assert.ErrorIs(t, err, exec.ErrNotFound) + assert.Empty(t, version) +} diff --git a/internal/find_blender/nonwindows.go b/internal/find_blender/nonwindows.go index 2de0d0f3..2c6b59b9 100644 --- a/internal/find_blender/nonwindows.go +++ b/internal/find_blender/nonwindows.go @@ -4,7 +4,7 @@ package find_blender // SPDX-License-Identifier: GPL-3.0-or-later -// findBlender returns ErrNotAvailable, as the file association lookup is only available on Windows. -func findBlender() (string, error) { +// fileAssociation isn't implemented on non-Windows platforms. +func fileAssociation() (string, error) { return "", ErrNotAvailable } diff --git a/internal/find_blender/windows.go b/internal/find_blender/windows.go index e821d7cb..0fede474 100644 --- a/internal/find_blender/windows.go +++ b/internal/find_blender/windows.go @@ -14,9 +14,9 @@ import ( "unsafe" ) -// findBlender returns the full path of `blender.exe` associated with ".blend" files. -func findBlender() (string, error) { - exe, err := fileAssociation(".blend") +// fileAssociation returns the full path of `blender.exe` associated with ".blend" files. +func fileAssociation() (string, error) { + exe, err := getFileAssociation(".blend") if err != nil { return "", err } @@ -40,9 +40,9 @@ func findBlender() (string, error) { return blenderPath, nil } -// fileAssociation finds the executable associated with the given extension. +// getFileAssociation finds the executable associated with the given extension. // The extension must be a string like ".blend". -func fileAssociation(extension string) (string, error) { +func getFileAssociation(extension string) (string, error) { // Load library. libname := "shlwapi.dll" libshlwapi, err := syscall.LoadLibrary(libname) diff --git a/internal/find_blender/windows_test.go b/internal/find_blender/windows_test.go index 129de070..ef7f78df 100644 --- a/internal/find_blender/windows_test.go +++ b/internal/find_blender/windows_test.go @@ -11,17 +11,17 @@ import ( "github.com/stretchr/testify/assert" ) -var withBlender = flag.Bool("withBlender", false, "run test that requires Blender to be installed") - -// TestFindBlender is a "weak" test, which actually accepts both happy and unhappy flows. +// TestFileAssociation is a "weak" test, which actually accepts both happy and unhappy flows. // It would be too fragile to always require a file association to be set up with Blender. -func TestFindBlender(t *testing.T) { - exe, err := findBlender() +func TestFileAssociation(t *testing.T) { + exe, err := fileAssociation() if err != nil { assert.Empty(t, exe) if *withBlender { t.Fatalf("unexpected error: %v", err) + } else { + t.Skip("skipping test, -withBlender arg not passed") } return } diff --git a/internal/manager/api_impl/interfaces.go b/internal/manager/api_impl/interfaces.go index 1d5e3935..4e7c1c4d 100644 --- a/internal/manager/api_impl/interfaces.go +++ b/internal/manager/api_impl/interfaces.go @@ -161,6 +161,10 @@ type ConfigService interface { // IsFirstRun returns true if this is likely to be the first run of Flamenco. IsFirstRun() (bool, error) + + // ForceFirstRun forces IsFirstRun() to return true. This is used to force the + // first-time wizard on a configured system. + ForceFirstRun() } type Shaman interface { diff --git a/internal/manager/api_impl/meta.go b/internal/manager/api_impl/meta.go index 0f987039..25b5bbaa 100644 --- a/internal/manager/api_impl/meta.go +++ b/internal/manager/api_impl/meta.go @@ -9,9 +9,11 @@ import ( "io/fs" "net/http" "os" + "os/exec" "path/filepath" "git.blender.org/flamenco/internal/appinfo" + "git.blender.org/flamenco/internal/find_blender" "git.blender.org/flamenco/internal/manager/config" "git.blender.org/flamenco/pkg/api" "github.com/labstack/echo/v4" @@ -144,6 +146,89 @@ func (f *Flamenco) CheckSharedStoragePath(e echo.Context) error { }) } +func (f *Flamenco) FindBlenderExePath(e echo.Context) error { + logger := requestLogger(e) + ctx := e.Request().Context() + + response := api.BlenderPathFindResult{} + + // TODO: the code below is a bit too coupled with the innards of find_blender.CheckBlender(). + + // Find by file association, falling back to just finding "blender" on the + // path if not available. This uses find_blender.CheckBlender() instead of + // find_blender.FindBlender() because the former also tries to run the found + // executable and reports on the version of Blender. + result, err := find_blender.CheckBlender(ctx, "") + switch { + case errors.Is(err, fs.ErrNotExist): + logger.Info().Msg("Blender could not be found") + case err != nil: + logger.Warn().Err(err).Msg("there was an error finding Blender") + return sendAPIError(e, http.StatusInternalServerError, "there was an error finding Blender: %v", err) + default: + response = append(response, api.BlenderPathCheckResult{ + IsUsable: true, + Path: result.FoundLocation, + Cause: result.BlenderVersion, + Source: result.Source, + }) + } + + if result.Source == api.BlenderPathSourceFileAssociation { + // There could be another Blender found on $PATH. + result, err := find_blender.CheckBlender(ctx, "blender") + switch { + case errors.Is(err, fs.ErrNotExist): + logger.Info().Msg("Blender could not be found as 'blender' on $PATH") + case err != nil: + logger.Warn().Err(err).Msg("there was an error finding Blender as 'blender' on $PATH") + return sendAPIError(e, http.StatusInternalServerError, "there was an error finding Blender: %v", err) + default: + response = append(response, api.BlenderPathCheckResult{ + IsUsable: true, + Path: result.FoundLocation, + Cause: result.BlenderVersion, + Source: result.Source, + }) + } + } + + return e.JSON(http.StatusOK, response) +} + +func (f *Flamenco) CheckBlenderExePath(e echo.Context) error { + logger := requestLogger(e) + + var toCheck api.CheckSharedStoragePathJSONBody + if err := e.Bind(&toCheck); err != nil { + logger.Warn().Err(err).Msg("bad request received") + return sendAPIError(e, http.StatusBadRequest, "invalid format") + } + + command := toCheck.Path + logger = logger.With().Str("command", command).Logger() + logger.Info().Msg("checking whether this command leads to Blender") + + ctx := e.Request().Context() + checkResult, err := find_blender.CheckBlender(ctx, command) + response := api.BlenderPathCheckResult{ + Source: checkResult.Source, + } + + switch { + case errors.Is(err, exec.ErrNotFound): + response.Cause = "Blender could not be found" + case err != nil: + response.Cause = fmt.Sprintf("There was an error running the command: %v", err) + default: + response.IsUsable = true + response.Path = checkResult.FoundLocation + response.Cause = fmt.Sprintf("Found %v", checkResult.BlenderVersion) + } + + return e.JSON(http.StatusOK, response) +} + func flamencoManagerDir() (string, error) { exename, err := os.Executable() if err != nil { diff --git a/internal/manager/api_impl/mocks/api_impl_mock.gen.go b/internal/manager/api_impl/mocks/api_impl_mock.gen.go index 4fb68b24..d898ee0f 100644 --- a/internal/manager/api_impl/mocks/api_impl_mock.gen.go +++ b/internal/manager/api_impl/mocks/api_impl_mock.gen.go @@ -699,6 +699,18 @@ func (mr *MockConfigServiceMockRecorder) ExpandVariables(arg0, arg1, arg2 interf return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ExpandVariables", reflect.TypeOf((*MockConfigService)(nil).ExpandVariables), arg0, arg1, arg2) } +// ForceFirstRun mocks base method. +func (m *MockConfigService) ForceFirstRun() { + m.ctrl.T.Helper() + m.ctrl.Call(m, "ForceFirstRun") +} + +// ForceFirstRun indicates an expected call of ForceFirstRun. +func (mr *MockConfigServiceMockRecorder) ForceFirstRun() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ForceFirstRun", reflect.TypeOf((*MockConfigService)(nil).ForceFirstRun)) +} + // Get mocks base method. func (m *MockConfigService) Get() *config.Conf { m.ctrl.T.Helper() diff --git a/internal/worker/command_blender.go b/internal/worker/command_blender.go index bd8025f6..b91a203f 100644 --- a/internal/worker/command_blender.go +++ b/internal/worker/command_blender.go @@ -10,7 +10,6 @@ import ( "fmt" "io" "os/exec" - "path/filepath" "regexp" "github.com/google/shlex" @@ -18,6 +17,7 @@ import ( "git.blender.org/flamenco/internal/find_blender" "git.blender.org/flamenco/pkg/api" + "git.blender.org/flamenco/pkg/crosspath" ) // The buffer size used to read stdout/stderr output from Blender. @@ -116,14 +116,14 @@ func (ce *CommandExecutor) cmdBlenderRenderCommand( return nil, err } - if filepath.Dir(parameters.exe) == "." { + if crosspath.Dir(parameters.exe) == "." { // No directory path given. Check that the executable can be found on the // path. if _, err := exec.LookPath(parameters.exe); err != nil { // Attempt a platform-specific way to find which Blender executable to // use. If Blender cannot not be found, just use the configured command // and let the OS produce the errors. - path, err := find_blender.FindBlender() + path, err := find_blender.FileAssociation() if err == nil { logger.Info().Str("path", path).Msg("found Blender") parameters.exe = path diff --git a/internal/worker/mocks/client.gen.go b/internal/worker/mocks/client.gen.go index 48c887ce..487c091d 100644 --- a/internal/worker/mocks/client.gen.go +++ b/internal/worker/mocks/client.gen.go @@ -36,6 +36,46 @@ func (m *MockFlamencoClient) EXPECT() *MockFlamencoClientMockRecorder { return m.recorder } +// CheckBlenderExePathWithBodyWithResponse mocks base method. +func (m *MockFlamencoClient) CheckBlenderExePathWithBodyWithResponse(arg0 context.Context, arg1 string, arg2 io.Reader, arg3 ...api.RequestEditorFn) (*api.CheckBlenderExePathResponse, error) { + m.ctrl.T.Helper() + varargs := []interface{}{arg0, arg1, arg2} + for _, a := range arg3 { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "CheckBlenderExePathWithBodyWithResponse", varargs...) + ret0, _ := ret[0].(*api.CheckBlenderExePathResponse) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// CheckBlenderExePathWithBodyWithResponse indicates an expected call of CheckBlenderExePathWithBodyWithResponse. +func (mr *MockFlamencoClientMockRecorder) CheckBlenderExePathWithBodyWithResponse(arg0, arg1, arg2 interface{}, arg3 ...interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + varargs := append([]interface{}{arg0, arg1, arg2}, arg3...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CheckBlenderExePathWithBodyWithResponse", reflect.TypeOf((*MockFlamencoClient)(nil).CheckBlenderExePathWithBodyWithResponse), varargs...) +} + +// CheckBlenderExePathWithResponse mocks base method. +func (m *MockFlamencoClient) CheckBlenderExePathWithResponse(arg0 context.Context, arg1 api.CheckBlenderExePathJSONRequestBody, arg2 ...api.RequestEditorFn) (*api.CheckBlenderExePathResponse, error) { + m.ctrl.T.Helper() + varargs := []interface{}{arg0, arg1} + for _, a := range arg2 { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "CheckBlenderExePathWithResponse", varargs...) + ret0, _ := ret[0].(*api.CheckBlenderExePathResponse) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// CheckBlenderExePathWithResponse indicates an expected call of CheckBlenderExePathWithResponse. +func (mr *MockFlamencoClientMockRecorder) CheckBlenderExePathWithResponse(arg0, arg1 interface{}, arg2 ...interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + varargs := append([]interface{}{arg0, arg1}, arg2...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CheckBlenderExePathWithResponse", reflect.TypeOf((*MockFlamencoClient)(nil).CheckBlenderExePathWithResponse), varargs...) +} + // CheckSharedStoragePathWithBodyWithResponse mocks base method. func (m *MockFlamencoClient) CheckSharedStoragePathWithBodyWithResponse(arg0 context.Context, arg1 string, arg2 io.Reader, arg3 ...api.RequestEditorFn) (*api.CheckSharedStoragePathResponse, error) { m.ctrl.T.Helper() @@ -256,6 +296,26 @@ func (mr *MockFlamencoClientMockRecorder) FetchWorkersWithResponse(arg0 interfac return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "FetchWorkersWithResponse", reflect.TypeOf((*MockFlamencoClient)(nil).FetchWorkersWithResponse), varargs...) } +// FindBlenderExePathWithResponse mocks base method. +func (m *MockFlamencoClient) FindBlenderExePathWithResponse(arg0 context.Context, arg1 ...api.RequestEditorFn) (*api.FindBlenderExePathResponse, error) { + m.ctrl.T.Helper() + varargs := []interface{}{arg0} + for _, a := range arg1 { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "FindBlenderExePathWithResponse", varargs...) + ret0, _ := ret[0].(*api.FindBlenderExePathResponse) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// FindBlenderExePathWithResponse indicates an expected call of FindBlenderExePathWithResponse. +func (mr *MockFlamencoClientMockRecorder) FindBlenderExePathWithResponse(arg0 interface{}, arg1 ...interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + varargs := append([]interface{}{arg0}, arg1...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "FindBlenderExePathWithResponse", reflect.TypeOf((*MockFlamencoClient)(nil).FindBlenderExePathWithResponse), varargs...) +} + // GetConfigurationFileWithResponse mocks base method. func (m *MockFlamencoClient) GetConfigurationFileWithResponse(arg0 context.Context, arg1 ...api.RequestEditorFn) (*api.GetConfigurationFileResponse, error) { m.ctrl.T.Helper() diff --git a/web/app/src/assets/base.css b/web/app/src/assets/base.css index e6f44198..c902dd08 100644 --- a/web/app/src/assets/base.css +++ b/web/app/src/assets/base.css @@ -586,4 +586,20 @@ span.state-transition-arrow.lazy { content: "❌ "; } +.first-time-wizard .blender-selector { + padding: 0.5em; + outline: thin solid var(--color-border); + border-radius: var(--border-radius); +} + +.first-time-wizard .blender-selector.selected-blender { + color: var(--color-accent-text); + background-color: var(--color-accent-background); + outline-width: var(--border-width); +} +.first-time-wizard .blender-selector button { + display: block; + margin-left: auto; +} + /* ------------ /First Time Wizard ------------ */ diff --git a/web/app/src/views/FirstTimeWizardView.vue b/web/app/src/views/FirstTimeWizardView.vue index b7d2b1e4..4b0df4f8 100644 --- a/web/app/src/views/FirstTimeWizardView.vue +++ b/web/app/src/views/FirstTimeWizardView.vue @@ -30,6 +30,36 @@

Which Blender?

+ +

Choose which Blender to use below:

+ +

... finding Blenders ...

+
+
+
Version
+
{{ blender.cause }}
+ +
Path
+
{{ blender.path }}
+ +
Source
+
{{ sourceLabels[blender.source] }}
+
+ +
+ +

Or provide an alternative command to try:

+ +
+ + +
+

... checking ...

+

+ {{ blenderExeCheckResult.cause }} +

@@ -54,13 +84,34 @@ export default { }, data: () => ({ sharedStoragePath: "", - sharedStorageCheckResult: null, + sharedStorageCheckResult: null, // api.PathCheckResult metaAPI: new MetaApi(apiClient), + + allBlenders: [], // combination of autoFoundBlenders and blenderExeCheckResult. + + autoFoundBlenders: [], // list of api.BlenderPathCheckResult + blenderExeFinding: false, + selectedBlender: null, // the chosen api.BlenderPathCheckResult + + customBlenderExe: "", + blenderExeChecking: false, + blenderExeCheckResult: null, // api.BlenderPathCheckResult + sourceLabels: { + file_association: "This Blender runs when you double-click a .blend file.", + path_envvar: "This Blender was found on the $PATH environment.", + input_path: "You pointed Flamenco to this executable.", + } }), computed: { cleanSharedStoragePath() { return this.sharedStoragePath.trim(); }, + cleanCustomBlenderExe() { + return this.customBlenderExe.trim(); + }, + }, + mounted() { + this.findBlenderExePath(); }, methods: { // SocketIO connection event handlers: @@ -69,6 +120,7 @@ export default { onSIODisconnected(reason) { }, + // TODO: add a Refresh button that calls this again. checkSharedStoragePath() { const pathCheck = new PathCheckInput(this.cleanSharedStoragePath); console.log("requesting path check:", pathCheck); @@ -81,6 +133,56 @@ export default { console.log("Error checking storage path:", error); }) }, + + findBlenderExePath() { + this.blenderExeFinding = true; + this.autoFoundBlenders = []; + + console.log("Finding Blender"); + this.metaAPI.findBlenderExePath() + .then((result) => { + console.log("Result of finding Blender:", result); + this.autoFoundBlenders = result; + this._refreshAllBlenders(); + }) + .catch((error) => { + console.log("Error finding Blender:", error); + }) + .finally(() => { + this.blenderExeFinding = false; + }) + }, + + checkBlenderExePath() { + this.blenderExeChecking = true; + this.blenderExeCheckResult = null; + + const pathCheck = new PathCheckInput(this.cleanCustomBlenderExe); + console.log("requesting path check:", pathCheck); + this.metaAPI.checkBlenderExePath({ pathCheckInput: pathCheck }) + .then((result) => { + console.log("Blender exe path check result:", result); + this.blenderExeCheckResult = result; + if (result.is_usable) { + this.selectedBlender = result; + } + this._refreshAllBlenders(); + }) + .catch((error) => { + console.log("Error checking storage path:", error); + }) + .finally(() => { + this.blenderExeChecking = false; + }) + }, + + _refreshAllBlenders() { + if (this.blenderExeCheckResult == null || !this.blenderExeCheckResult.is_usable) { + this.allBlenders = this.autoFoundBlenders; + } else { + this.allBlenders = this.autoFoundBlenders.concat([this.blenderExeCheckResult]); + } + }, }, }