blender/flamenco
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Separate variables for Blender executable and its arguments.

Browse Source 
Split "executable" from "its arguments" in blender & ffmpeg commands.

Use `{blenderArgs}` variable to hold the default Blender arguments,
instead of having both the executable and its arguments in `{blender}`.

The reason for this is to support backslashes in the Blender executable
path. These were interpreted as escape characters by the shell lexer.
The shell lexer based splitting is now only performed on the default
arguments, with the result that `C:\Program Files\Blender
Foundation\3.3\blender.exe` is now a valid value for `{blender}`.

This does mean that this is backward incompatible change, and that it
requires setting up Flamenco Manager again, and that older jobs will not
be able to be rerun.

It is recommended to remove `flamenco-manager.yaml`, restart Flamenco
Manager, and reconfigure via the setup assistant.
This commit is contained in:
Sybren A. Stüvel 2022-08-30 10:36:04 +02:00
parent db9aca4a37
commit e5a20425c4
11 changed files with 104 additions and 48 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
CHANGELOG.md
View File

@ -6,6 +6,13 @@ bugs in actually-released versions.
## 3.0-beta2 - in development
WARNING: this version is backward incompatible. Any job created with Flamenco
3.0-beta1 will not run with Flamenco 3.0-beta2. Only upgrade after
currently-active jobs have finished, or cancel them.
It is recommended to remove `flamenco-manager.yaml`, restart Flamenco Manager,
and reconfigure via the setup assistant.
- Manager & Add-on: avoid error that could occur when submitting jobs with UDIM files
([44ccc6c3ca70](https://developer.blender.org/rF44ccc6c3ca706fdd268bf310f3e8965d58482449)).
- Manager: don't stop when the Flamenco Setup Assistant cannot start a webbrowser
@ -15,7 +22,11 @@ bugs in actually-released versions.
putting all the files in the root of the tarball).
- Two-way variable replacement now also changes the path separators to the target platform.
- Allow setting priority when submitting a job.
- Separate "blender location" and "blender arguments" into two variables.
- The variable `blender` now should only point at the Blender executable, for
example `D:\Blender_3.2_stable\blender.exe`.
- The variable `blenderArgs` can be used to set the default Blender arguments,
for example `-b -y`.
## 3.0-beta1 - released 2022-08-03

16
internal/manager/api_impl/meta.go
View File

@ -278,16 +278,18 @@ func (f *Flamenco) SaveSetupAssistantConfig(e echo.Context) error {
if commandNeedsQuoting(executable) {
executable = strconv.Quote(executable)
}
blenderCommand := fmt.Sprintf("%s %s", executable, config.DefaultBlenderArguments)
// Use the same command for each platform for now, but put them each in their
// own definition so that they're easier to edit later.
conf.Variables["blender"] = config.Variable{
IsTwoWay: false,
Values: config.VariableValues{
{Platform: "linux", Value: blenderCommand},
{Platform: "windows", Value: blenderCommand},
{Platform: "darwin", Value: blenderCommand},
{Platform: "linux", Value: executable},
{Platform: "windows", Value: executable},
{Platform: "darwin", Value: executable},
},
}
conf.Variables["blenderArgs"] = config.Variable{
Values: config.VariableValues{
{Platform: config.VariablePlatformAll, Value: config.DefaultBlenderArguments},
},
}
@ -314,5 +316,5 @@ func flamencoManagerDir() (string, error) {
}
func commandNeedsQuoting(cmd string) bool {
return strings.ContainsAny(cmd, " \n\t;()")
return strings.ContainsAny(cmd, "\n\t;()")
}

27
internal/manager/api_impl/meta_test.go
View File

@ -126,6 +126,12 @@ func TestSaveSetupAssistantConfig(t *testing.T) {
mf, finish := metaTestFixtures(t)
defer finish()
defaultBlenderArgsVar := config.Variable{
Values: config.VariableValues{
{Platform: config.VariablePlatformAll, Value: config.DefaultBlenderArguments},
},
}
doTest := func(body api.SetupAssistantConfig) config.Conf {
// Always start the test with a clean configuration.
originalConfig := config.DefaultConfig(func(c *config.Conf) {
@ -165,12 +171,13 @@ func TestSaveSetupAssistantConfig(t *testing.T) {
assert.Equal(t, mf.tempdir, savedConfig.SharedStoragePath)
expectBlenderVar := config.Variable{
Values: config.VariableValues{
{Platform: "linux", Value: "blender " + config.DefaultBlenderArguments},
{Platform: "windows", Value: "blender " + config.DefaultBlenderArguments},
{Platform: "darwin", Value: "blender " + config.DefaultBlenderArguments},
{Platform: "linux", Value: "blender"},
{Platform: "windows", Value: "blender"},
{Platform: "darwin", Value: "blender"},
},
}
assert.Equal(t, expectBlenderVar, savedConfig.Variables["blender"])
assert.Equal(t, defaultBlenderArgsVar, savedConfig.Variables["blenderArgs"])
}
// Test situation where the given command could be found on $PATH.
@ -187,12 +194,13 @@ func TestSaveSetupAssistantConfig(t *testing.T) {
assert.Equal(t, mf.tempdir, savedConfig.SharedStoragePath)
expectBlenderVar := config.Variable{
Values: config.VariableValues{
{Platform: "linux", Value: "kitty " + config.DefaultBlenderArguments},
{Platform: "windows", Value: "kitty " + config.DefaultBlenderArguments},
{Platform: "darwin", Value: "kitty " + config.DefaultBlenderArguments},
{Platform: "linux", Value: "kitty"},
{Platform: "windows", Value: "kitty"},
{Platform: "darwin", Value: "kitty"},
},
}
assert.Equal(t, expectBlenderVar, savedConfig.Variables["blender"])
assert.Equal(t, defaultBlenderArgsVar, savedConfig.Variables["blenderArgs"])
}
// Test a custom command given with the full path.
@ -209,12 +217,13 @@ func TestSaveSetupAssistantConfig(t *testing.T) {
assert.Equal(t, mf.tempdir, savedConfig.SharedStoragePath)
expectBlenderVar := config.Variable{
Values: config.VariableValues{
{Platform: "linux", Value: "/bin/cat " + config.DefaultBlenderArguments},
{Platform: "windows", Value: "/bin/cat " + config.DefaultBlenderArguments},
{Platform: "darwin", Value: "/bin/cat " + config.DefaultBlenderArguments},
{Platform: "linux", Value: "/bin/cat"},
{Platform: "windows", Value: "/bin/cat"},
{Platform: "darwin", Value: "/bin/cat"},
},
}
assert.Equal(t, expectBlenderVar, savedConfig.Variables["blender"])
assert.Equal(t, defaultBlenderArgsVar, savedConfig.Variables["blenderArgs"])
}
}

11
internal/manager/config/defaults.go
View File

@ -62,9 +62,14 @@ var defaultConfig = Conf{
// The default commands assume that the executables are available on $PATH.
"blender": {
Values: VariableValues{
VariableValue{Platform: "linux", Value: "blender " + DefaultBlenderArguments},
VariableValue{Platform: "windows", Value: "blender.exe " + DefaultBlenderArguments},
VariableValue{Platform: "darwin", Value: "blender " + DefaultBlenderArguments},
VariableValue{Platform: VariablePlatformLinux, Value: "blender"},
VariableValue{Platform: VariablePlatformWindows, Value: "blender.exe"},
VariableValue{Platform: VariablePlatformDarwin, Value: "blender"},
},
},
"blenderArgs": {
Values: VariableValues{
VariableValue{Platform: VariablePlatformAll, Value: DefaultBlenderArguments},
},
},
// TODO: determine useful defaults for these.

5
internal/manager/config/settings_test.go
View File

@ -20,9 +20,12 @@ func TestDefaultSettings(t *testing.T) {
assert.Equal(t, defaultConfig.DatabaseDSN, config.DatabaseDSN)
assert.Equal(t, false, config.Variables["blender"].IsTwoWay)
assert.Equal(t, "blender "+DefaultBlenderArguments, config.Variables["blender"].Values[0].Value)
assert.Equal(t, "blender", config.Variables["blender"].Values[0].Value)
assert.Equal(t, VariablePlatformLinux, config.Variables["blender"].Values[0].Platform)
assert.Equal(t, DefaultBlenderArguments, config.Variables["blenderArgs"].Values[0].Value)
assert.Equal(t, VariablePlatformAll, config.Variables["blenderArgs"].Values[0].Platform)
assert.Greater(t, config.BlocklistThreshold, 0)
}

3
internal/manager/job_compilers/job_compilers_test.go
View File

@ -103,6 +103,7 @@ func TestSimpleBlenderRenderHappy(t *testing.T) {
assert.Equal(t, "blender-render", t0.Commands[0].Name)
assert.EqualValues(t, AuthoredCommandParameters{
"exe": "{blender}",
"exeArgs": "{blenderArgs}",
"blendfile": settings["blendfile"].(string),
"args": expectCliArgs,
"argsBefore": make([]interface{}, 0),
@ -187,6 +188,7 @@ func TestSimpleBlenderRenderWindowsPaths(t *testing.T) {
assert.Equal(t, "blender-render", t0.Commands[0].Name)
assert.EqualValues(t, AuthoredCommandParameters{
"exe": "{blender}",
"exeArgs": "{blenderArgs}",
"blendfile": "R:\\sf\\jobs\\scene123.blend", // The blendfile parameter is just copied as-is, so keeps using backslash notation.
"args": expectCliArgs,
"argsBefore": make([]interface{}, 0),
@ -240,6 +242,7 @@ func TestSimpleBlenderRenderOutputPathFieldReplacement(t *testing.T) {
}
assert.EqualValues(t, AuthoredCommandParameters{
"exe": "{blender}",
"exeArgs": "{blenderArgs}",
"blendfile": sj.Settings.AdditionalProperties["blendfile"].(string),
"args": expectCliArgs,
"argsBefore": make([]interface{}, 0),

1
internal/manager/job_compilers/scripts/simple_blender_render.js
View File

@ -121,6 +121,7 @@ function authorRenderTasks(settings, renderDir, renderOutput) {
const task = author.Task(`render-${chunk}`, "blender");
const command = author.Command("blender-render", {
exe: settings.blender_cmd,
exeArgs: "{blenderArgs}",
argsBefore: [],
blendfile: settings.blendfile,
args: [

20
internal/worker/command_blender.go
View File

@ -22,7 +22,8 @@ import (
var regexpFileSaved = regexp.MustCompile("Saved: '(.*)'")
type BlenderParameters struct {
exe string // Expansion of `{blender}`: executable path + its CLI parameters defined by the Manager.
exe string // Expansion of `{blender}`: the executable path defined by the Manager.
exeArgs string // Expansion of `{blenderargs}`: its CLI parameters defined by the Manager.
argsBefore []string // Additional CLI arguments defined by the job compiler script, to go before the blend file name.
blendfile string // Path of the file to open.
args []string // Additional CLI arguments defined by the job compiler script, to go after the blend file name.
@ -133,6 +134,10 @@ func cmdBlenderRenderParams(logger zerolog.Logger, cmd api.Command) (BlenderPara
logger.Warn().Interface("command", cmd).Msg("missing 'exe' parameter")
return parameters, NewParameterMissingError("exe", cmd)
}
// Ignore the `ok` return value, as a missing exeArgs key is fine:
parameters.exeArgs, _ = cmdParameter[string](cmd, "exeArgs")
if parameters.argsBefore, ok = cmdParameterAsStrings(cmd, "argsBefore"); !ok {
logger.Warn().Interface("command", cmd).Msg("invalid 'argsBefore' parameter")
return parameters, NewParameterInvalidError("argsBefore", cmd, "cannot convert to list of strings")
@ -146,17 +151,16 @@ func cmdBlenderRenderParams(logger zerolog.Logger, cmd api.Command) (BlenderPara
return parameters, NewParameterInvalidError("args", cmd, "cannot convert to list of strings")
}
// Move any CLI args from 'exe' to 'argsBefore'.
exeArgs, err := shlex.Split(parameters.exe)
// Split the exeArgs string into separate parts, and prepend them to `argsBefore`.
exeArgs, err := shlex.Split(parameters.exeArgs)
if err != nil {
logger.Warn().Err(err).Interface("command", cmd).Msg("error parsing 'exe' parameter with shlex")
return parameters, NewParameterInvalidError("exe", cmd, err.Error())
logger.Warn().Err(err).Interface("command", cmd).Msg("error parsing 'exeArgs' parameter with shlex")
return parameters, NewParameterInvalidError("exeArgs", cmd, err.Error())
}
if len(exeArgs) > 1 {
if len(exeArgs) > 0 {
allArgsBefore := []string{}
allArgsBefore = append(allArgsBefore, exeArgs[1:]...)
allArgsBefore = append(allArgsBefore, exeArgs...)
allArgsBefore = append(allArgsBefore, parameters.argsBefore...)
parameters.exe = exeArgs[0]
parameters.argsBefore = allArgsBefore
}

20
internal/worker/command_blender_test.go
View File

@ -44,11 +44,21 @@ func TestCmdBlenderCliArgsInExeParameter(t *testing.T) {
ce, mocks := testCommandExecutor(t, mockCtrl)
// Just use backslashes even when we're testing on Linux/macOS. Backslashes
// are always causing issues, so it's better to test with those on other
// platforms as well. Even when the resulting command cannot be run, every
// platform should be able to at least make the backslashes survive any
// processing.
exe := `D:\Blender_3.2_stable\blender.exe`
taskID := "1d54c6fe-1242-4c8f-bd63-5a09e358d7b6"
cmd := api.Command{
Name: "blender",
Parameters: map[string]interface{}{
"exe": "/path/to/blender --factory-startup --python-expr \"import bpy; print('hello world')\"",
"exe": exe,
// This intentionally starts with a space (should be ignored) and has
// quoted text within quoted text:
"exeArgs": ` --factory-startup --python-expr "import bpy; print(\"hello world\")"`,
"argsBefore": []string{"-no-audio"},
"blendfile": "file with spaces.blend",
"args": []string{"--debug"},
@ -56,10 +66,10 @@ func TestCmdBlenderCliArgsInExeParameter(t *testing.T) {
}
mocks.cli.EXPECT().CommandContext(gomock.Any(),
"/path/to/blender", // from 'exe'
"--factory-startup", // from 'exe'
"--python-expr", // from 'exe'
"import bpy; print('hello world')", // from 'exe'
exe, // from 'exe'
"--factory-startup", // from 'exeArgs'
"--python-expr", // from 'exeArgs'
`import bpy; print("hello world")`, // from 'exeArgs'
"-no-audio", // from 'argsBefore'
"file with spaces.blend", // from 'blendfile'
"--debug", // from 'args'

20
internal/worker/command_ffmpeg.go
View File

@ -26,7 +26,8 @@ import (
)
type CreateVideoParams struct {
exe string // Executable path + its CLI parameters defined by the Manager.
exe string // Executable path defined by the Manager.
exeArgs string // Its CLI parameters defined by the Manager.
fps float64 // Frames per second of the video file.
inputGlob string // Glob of input files.
outputFile string // File to save the video to.
@ -119,6 +120,10 @@ func cmdFramesToVideoParams(logger zerolog.Logger, cmd api.Command) (CreateVideo
logger.Warn().Interface("command", cmd).Msg("missing 'exe' parameter")
return parameters, NewParameterMissingError("exe", cmd)
}
if parameters.exeArgs, ok = cmdParameter[string](cmd, "exeArgs"); !ok {
logger.Warn().Interface("command", cmd).Msg("invalid 'exeArgs' parameter")
return parameters, NewParameterInvalidError("exeArgs", cmd, "parameter must be string")
}
if parameters.fps, ok = cmdParameter[float64](cmd, "fps"); !ok || parameters.fps == 0.0 {
logger.Warn().Interface("command", cmd).Msg("missing 'fps' parameter")
return parameters, NewParameterMissingError("fps", cmd)
@ -140,17 +145,16 @@ func cmdFramesToVideoParams(logger zerolog.Logger, cmd api.Command) (CreateVideo
return parameters, NewParameterInvalidError("args", cmd, "cannot convert to list of strings")
}
// Move any CLI args from 'exe' to 'argsBefore'.
exeArgs, err := shlex.Split(parameters.exe)
// Split the exeArgs string into separate parts, and prepend them to `argsBefore`.
exeArgs, err := shlex.Split(parameters.exeArgs)
if err != nil {
logger.Warn().Err(err).Interface("command", cmd).Msg("error parsing 'exe' parameter with shlex")
return parameters, NewParameterInvalidError("exe", cmd, err.Error())
logger.Warn().Err(err).Interface("command", cmd).Msg("error parsing 'exeArgs' parameter with shlex")
return parameters, NewParameterInvalidError("exeArgs", cmd, err.Error())
}
if len(exeArgs) > 1 {
if len(exeArgs) > 0 {
allArgsBefore := []string{}
allArgsBefore = append(allArgsBefore, exeArgs[1:]...)
allArgsBefore = append(allArgsBefore, exeArgs...)
allArgsBefore = append(allArgsBefore, parameters.argsBefore...)
parameters.exe = exeArgs[0]
parameters.argsBefore = allArgsBefore
}
parameters.args = append(parameters.args,

16
internal/worker/command_ffmpeg_test.go
View File

@ -24,10 +24,12 @@ func TestCmdFramesToVideoSimplePosix(t *testing.T) {
ce, mocks := testCommandExecutor(t, mockCtrl)
taskID := "1d54c6fe-1242-4c8f-bd63-5a09e358d7b6"
exe := `F:\software\tools\ffmpeg.exe` // Backslashes are tricksy, test with them on all platforms.
cmd := api.Command{
Name: "blender",
Parameters: map[string]interface{}{
"exe": "/path/to/ffmpeg -v quiet",
"exe": exe,
"exeArgs": "-v quiet",
"argsBefore": []string{"-report"},
"inputGlob": "path/to/renders/*.png",
"fps": 10.0,
@ -41,7 +43,7 @@ func TestCmdFramesToVideoSimplePosix(t *testing.T) {
}
cliArgs := []string{
"-v", "quiet", // exe
"-v", "quiet", // exeArgs
"-report", // argsBefore
"-r", "10", // input frame rate
"-pattern_type", "glob", "-i", "path/to/renders/*.png", // inputGlob
@ -49,7 +51,7 @@ func TestCmdFramesToVideoSimplePosix(t *testing.T) {
"-r", "10", // output frame rate
"path/to/renders/preview.mkv", // outputFile
}
mocks.cli.EXPECT().CommandContext(gomock.Any(), "/path/to/ffmpeg", cliArgs).Return(nil)
mocks.cli.EXPECT().CommandContext(gomock.Any(), exe, cliArgs).Return(nil)
err := ce.cmdFramesToVideo(context.Background(), zerolog.Nop(), taskID, cmd)
assert.Equal(t, ErrNoExecCmd, err, "nil *exec.Cmd should result in ErrNoExecCmd")
@ -66,10 +68,12 @@ func TestCmdFramesToVideoSimpleWindows(t *testing.T) {
ce, mocks := testCommandExecutor(t, mockCtrl)
taskID := "1d54c6fe-1242-4c8f-bd63-5a09e358d7b6"
exe := `F:\software\tools\ffmpeg.exe`
cmd := api.Command{
Name: "blender",
Parameters: map[string]interface{}{
"exe": "/path/to/ffmpeg -v quiet",
"exe": exe,
"exeArgs": "-v quiet",
"argsBefore": []string{"-report"},
// NOTE: these files MUST exist, otherwise the glob index file creation will fail.
"inputGlob": "command_ffmpeg_test_files/*.png",
@ -84,7 +88,7 @@ func TestCmdFramesToVideoSimpleWindows(t *testing.T) {
}
cliArgs := []string{
"-v", "quiet", // exe
"-v", "quiet", // exeArgs
"-report", // argsBefore
"-f", "concat", "-safe", "0", "-i", "command_ffmpeg_test_files\\ffmpeg-file-index.txt", // input glob
"-c:v", "hevc", "-crf", "31", "-vf", "pad=ceil(iw/2)*2:ceil(ih/2)*2", // args
@ -92,7 +96,7 @@ func TestCmdFramesToVideoSimpleWindows(t *testing.T) {
"path/to/renders/preview.mkv", // outputFile
}
mocks.cli.EXPECT().
CommandContext(gomock.Any(), "/path/to/ffmpeg", gomock.Any()).
CommandContext(gomock.Any(), exe, gomock.Any()).
DoAndReturn(func(ctx context.Context, name string, arg ...string) error {
assert.EqualValues(t, cliArgs, arg)
return nil // this is the nil *exec.Cmd referenced below