From 53bb95eeaa9246bf6fe0852c1933518df4020b84 Mon Sep 17 00:00:00 2001 From: Crypt Keeper <64215+codefromthecrypt@users.noreply.github.com> Date: Thu, 23 Mar 2023 08:53:14 +0100 Subject: [PATCH] Allow ModuleConfig.WithName("") to clear the module name (#1277) We currently allow clearing other config with nil, such as FSConfig. However, we missed a spot as internally we couldn't differentiate between name never set, or explicitly set to empty. Now, when someone sets the module name to empty, the name in the binary section is ignored. Signed-off-by: Adrian Cole --- config.go | 4 +++- config_test.go | 34 ++++++++++++++++++++++++++++------ runtime.go | 2 +- runtime_test.go | 6 +++++- 4 files changed, 37 insertions(+), 9 deletions(-) diff --git a/config.go b/config.go index e66c0b85..e97d0bbc 100644 --- a/config.go +++ b/config.go @@ -455,7 +455,7 @@ type ModuleConfig interface { WithFSConfig(FSConfig) ModuleConfig // WithName configures the module name. Defaults to what was decoded from - // the name section. + // the name section. Empty string ("") clears any name. WithName(string) ModuleConfig // WithStartFunctions configures the functions to call after the module is @@ -597,6 +597,7 @@ type ModuleConfig interface { type moduleConfig struct { name string + nameSet bool startFunctions []string stdin io.Reader stdout io.Writer @@ -685,6 +686,7 @@ func (c *moduleConfig) WithFSConfig(config FSConfig) ModuleConfig { // WithName implements ModuleConfig.WithName func (c *moduleConfig) WithName(name string) ModuleConfig { ret := c.clone() + ret.nameSet = true ret.name = name return ret } diff --git a/config_test.go b/config_test.go index 1174669f..c0824a41 100644 --- a/config_test.go +++ b/config_test.go @@ -100,29 +100,50 @@ func TestRuntimeConfig(t *testing.T) { func TestModuleConfig(t *testing.T) { tests := []struct { - name string - with func(ModuleConfig) ModuleConfig - expected string + name string + with func(ModuleConfig) ModuleConfig + expectNameSet bool + expectedName string }{ + { + name: "WithName default", + with: func(c ModuleConfig) ModuleConfig { + return c + }, + expectNameSet: false, + expectedName: "", + }, { name: "WithName", with: func(c ModuleConfig) ModuleConfig { return c.WithName("wazero") }, - expected: "wazero", + expectNameSet: true, + expectedName: "wazero", }, { name: "WithName empty", with: func(c ModuleConfig) ModuleConfig { return c.WithName("") }, + expectNameSet: true, + expectedName: "", }, { name: "WithName twice", with: func(c ModuleConfig) ModuleConfig { return c.WithName("wazero").WithName("wa0") }, - expected: "wa0", + expectNameSet: true, + expectedName: "wa0", + }, + { + name: "WithName can clear", + with: func(c ModuleConfig) ModuleConfig { + return c.WithName("wazero").WithName("") + }, + expectNameSet: true, + expectedName: "", }, } for _, tt := range tests { @@ -131,7 +152,8 @@ func TestModuleConfig(t *testing.T) { t.Run(tc.name, func(t *testing.T) { input := NewModuleConfig() rc := tc.with(input) - require.Equal(t, tc.expected, rc.(*moduleConfig).name) + require.Equal(t, tc.expectNameSet, rc.(*moduleConfig).nameSet) + require.Equal(t, tc.expectedName, rc.(*moduleConfig).name) // The source wasn't modified require.Equal(t, NewModuleConfig(), input) }) diff --git a/runtime.go b/runtime.go index 6c5501e6..2d8f9461 100644 --- a/runtime.go +++ b/runtime.go @@ -283,7 +283,7 @@ func (r *runtime) InstantiateModule( } name := config.name - if name == "" && code.module.NameSection != nil && code.module.NameSection.ModuleName != "" { + if !config.nameSet && code.module.NameSection != nil && code.module.NameSection.ModuleName != "" { name = code.module.NameSection.ModuleName } diff --git a/runtime_test.go b/runtime_test.go index 420cfce9..1b5dfe92 100644 --- a/runtime_test.go +++ b/runtime_test.go @@ -446,19 +446,23 @@ func TestRuntime_InstantiateModule_WithName(t *testing.T) { internal := r.(*runtime) m1, err := r.InstantiateModule(testCtx, base, NewModuleConfig().WithName("1")) require.NoError(t, err) + require.Equal(t, "1", m1.Name()) require.Nil(t, internal.Module("0")) require.Equal(t, internal.Module("1"), m1) m2, err := r.InstantiateModule(testCtx, base, NewModuleConfig().WithName("2")) require.NoError(t, err) + require.Equal(t, "2", m2.Name()) require.Nil(t, internal.Module("0")) require.Equal(t, internal.Module("2"), m2) // Empty name module shouldn't be returned via Module() for future optimization. - _, err = r.InstantiateModule(testCtx, base, NewModuleConfig().WithName("")) + m3, err := r.InstantiateModule(testCtx, base, NewModuleConfig().WithName("")) require.NoError(t, err) + require.Equal(t, "", m3.Name()) + ret := internal.Module("") require.Nil(t, ret) }