From 3c290b1f6d2e582797152334872a2d8acadac77b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Wed, 13 Jul 2022 12:45:55 +0200 Subject: [PATCH] Manager: ensure the `{jobs}` implicit variable uses forward slashes Since the variable expansion is unaware of path semantics, using forward slashes is the safest way to go about things in a platform-indepdent way. --- internal/manager/api_impl/varrepl_test.go | 19 ++++++++++++++----- internal/manager/config/config.go | 3 ++- internal/manager/config/settings_test.go | 9 +++++++-- 3 files changed, 23 insertions(+), 8 deletions(-) diff --git a/internal/manager/api_impl/varrepl_test.go b/internal/manager/api_impl/varrepl_test.go index f52999cb..ce2023af 100644 --- a/internal/manager/api_impl/varrepl_test.go +++ b/internal/manager/api_impl/varrepl_test.go @@ -3,7 +3,7 @@ package api_impl // SPDX-License-Identifier: GPL-3.0-or-later import ( - "path" + "runtime" "testing" "github.com/stretchr/testify/assert" @@ -11,6 +11,7 @@ import ( "git.blender.org/flamenco/internal/manager/config" "git.blender.org/flamenco/internal/manager/persistence" "git.blender.org/flamenco/pkg/api" + "git.blender.org/flamenco/pkg/crosspath" ) func varreplTestTask() api.AssignedTask { @@ -106,25 +107,33 @@ func TestReplaceJobsVariable(t *testing.T) { // An implicit variable "{jobs}" should be created, regardless of whether // Shaman is enabled or not. + var storagePath string + switch runtime.GOOS { + case "windows": + storagePath = `C:\path\to\flamenco-storage` + default: + storagePath = "/path/to/flamenco-storage" + } + { // Test with Shaman enabled. conf := config.GetTestConfig(func(c *config.Conf) { - c.SharedStoragePath = "/path/to/flamenco-storage" + c.SharedStoragePath = storagePath c.Shaman.Enabled = true }) replacedTask := replaceTaskVariables(&conf, task, worker) - expectPath := path.Join(conf.Shaman.CheckoutPath(), "path/in/storage.blend") + expectPath := crosspath.Join(crosspath.ToSlash(conf.Shaman.CheckoutPath()), "path/in/storage.blend") assert.Equal(t, expectPath, replacedTask.Commands[2].Parameters["filepath"]) } { // Test without Shaman. conf := config.GetTestConfig(func(c *config.Conf) { - c.SharedStoragePath = "/path/to/flamenco-storage" + c.SharedStoragePath = storagePath c.Shaman.Enabled = false }) replacedTask := replaceTaskVariables(&conf, task, worker) - expectPath := "/path/to/flamenco-storage/jobs/path/in/storage.blend" + expectPath := crosspath.Join(storagePath, "jobs", "path/in/storage.blend") assert.Equal(t, expectPath, replacedTask.Commands[2].Parameters["filepath"]) } } diff --git a/internal/manager/config/config.go b/internal/manager/config/config.go index 7395fa33..89ccef84 100644 --- a/internal/manager/config/config.go +++ b/internal/manager/config/config.go @@ -19,6 +19,7 @@ import ( yaml "gopkg.in/yaml.v2" "git.blender.org/flamenco/internal/appinfo" + "git.blender.org/flamenco/pkg/crosspath" shaman_config "git.blender.org/flamenco/pkg/shaman/config" ) @@ -271,7 +272,7 @@ func (c *Conf) addImplicitVariables() { { Audience: VariableAudienceAll, Platform: VariablePlatformAll, - Value: c.EffectiveStoragePath(), + Value: crosspath.ToSlash(c.EffectiveStoragePath()), }, }, } diff --git a/internal/manager/config/settings_test.go b/internal/manager/config/settings_test.go index 4920ce33..ca62b0d1 100644 --- a/internal/manager/config/settings_test.go +++ b/internal/manager/config/settings_test.go @@ -5,6 +5,7 @@ package config import ( "testing" + "git.blender.org/flamenco/pkg/crosspath" "github.com/stretchr/testify/assert" ) @@ -66,7 +67,9 @@ func TestStorageImplicitVariablesWithShaman(t *testing.T) { t.FailNow() } assert.False(t, c.implicitVariables["jobs"].IsTwoWay) - assert.Equal(t, c.Shaman.CheckoutPath(), c.implicitVariables["jobs"].Values[0].Value) + assert.Equal(t, + crosspath.ToSlash(c.Shaman.CheckoutPath()), + c.implicitVariables["jobs"].Values[0].Value) } func TestStorageImplicitVariablesWithoutShaman(t *testing.T) { @@ -92,5 +95,7 @@ func TestStorageImplicitVariablesWithoutShaman(t *testing.T) { t.FailNow() } assert.False(t, c.implicitVariables["jobs"].IsTwoWay) - assert.Equal(t, c.SharedStoragePath, c.implicitVariables["jobs"].Values[0].Value) + assert.Equal(t, + crosspath.ToSlash(c.SharedStoragePath), + c.implicitVariables["jobs"].Values[0].Value) }