drsybren/Flamenco
1
0
Fork 0
Code Issues 1 Pull Requests Actions Packages Projects Releases Wiki Activity

Manager: only do pathsep localisation on two-way variables

Browse Source 
By accident the Manager was performing slash localisation on all
command parameters, causing some math expressions for FFmpeg to fail.
This commit is contained in:
Sybren A. Stüvel 2022-08-25 15:02:56 +02:00
parent 3d2bdddffa
commit 0c91fe93d0
2 changed files with 19 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
internal/manager/api_impl/varrepl_test.go
View File

@ -87,14 +87,14 @@ func TestReplacePathsWindows(t *testing.T) {
replacedTask := replaceTaskVariables(&conf, task, worker)
assert.Equal(t,
`s:\flamenco\jobs\sybren\2017-06-08-181223.625800-sybren-flamenco-test.flamenco\flamenco-test.flamenco.blend`,
`s:/flamenco/jobs/sybren/2017-06-08-181223.625800-sybren-flamenco-test.flamenco/flamenco-test.flamenco.blend`,
replacedTask.Commands[2].Parameters["filepath"],
)
assert.Equal(t,
[]string{"--render-out", `s:\flamenco\render\long\sybren\blender-cloud-addon\flamenco-test__intermediate\render-smpl-0001-0084-frm-######`},
[]string{"--render-out", `s:/flamenco/render/long/sybren/blender-cloud-addon/flamenco-test__intermediate/render-smpl-0001-0084-frm-######`},
replacedTask.Commands[2].Parameters["args"],
)
assert.Equal(t, `{hey}\haha`, replacedTask.Commands[2].Parameters["otherpath"])
assert.Equal(t, `{hey}/haha`, replacedTask.Commands[2].Parameters["otherpath"])
}
func TestReplacePathsUnknownOS(t *testing.T) {

22
internal/manager/config/config.go
View File

@ -435,6 +435,7 @@ func (c *Conf) ExpandVariables(inputChannel <-chan string, outputChannel chan<-
//
// Practically, this replaces "value for the Manager platform" with "value
// for the target platform".
isPathValue := false
for varname, managerValue := range twoWayVars {
targetValue, ok := varsForPlatform[varname]
if !ok {
@ -444,12 +445,18 @@ func (c *Conf) ExpandVariables(inputChannel <-chan string, outputChannel chan<-
continue
}
expanded = targetValue + expanded[len(managerValue):]
// Since two-way variables are meant for path replacement, we know this
// should be a path.
if c.isTwoWay(varname) {
isPathValue = true
}
}
// Since two-way variables are meant for path replacement, we know this
// should be a path. For added bonus, translate it to the target platform's
// path separators.
return crosspath.ToPlatform(expanded, string(platform))
if isPathValue {
expanded = crosspath.ToPlatform(expanded, string(platform))
}
return expanded
}
for valueToExpand := range inputChannel {
@ -505,6 +512,10 @@ func (c *Conf) getVariables(audience VariableAudience, platform VariablePlatform
return varsForPlatform
}
func (c *Conf) isTwoWay(varname string) bool {
return c.implicitVariables[varname].IsTwoWay || c.Variables[varname].IsTwoWay
}
// GetTwoWayVariables returns the two-way variable values for this (audience,
// platform) combination. If no variables are found, just returns an empty map.
// If a value is defined for both the "all" platform and specifically the given
@ -515,8 +526,7 @@ func (c *Conf) GetTwoWayVariables(audience VariableAudience, platform VariablePl
// Only keep the two-way variables.
twoWayVars := map[string]string{}
for varname, value := range varsForPlatform {
isTwoWay := c.implicitVariables[varname].IsTwoWay || c.Variables[varname].IsTwoWay
if isTwoWay {
if c.isTwoWay(varname) {
twoWayVars[varname] = value
}
}