From d26ef0ecd9faf47fe3ac7dc49bbf080209d7cfb1 Mon Sep 17 00:00:00 2001 From: EduardGomezEscandell Date: Mon, 6 Mar 2023 14:58:26 +0100 Subject: [PATCH] Moved WslFlags functionality and tests to internal package This avoids circular dependencies between Flags, Configuration, and Configuration-related wslApi.dll calls. --- distro.go | 69 ++++----------------------------- internal/flags/flags.go | 9 ++--- internal/flags/flags_test.go | 74 +++++++++++++++++++++++++++++++++++ internal/flags/packed.go | 65 +++++++++++++++++++++++++++++++ internal_test.go | 75 ------------------------------------ 5 files changed, 151 insertions(+), 141 deletions(-) create mode 100644 internal/flags/flags_test.go create mode 100644 internal/flags/packed.go delete mode 100644 internal_test.go diff --git a/distro.go b/distro.go index 1ddec52f..7d00658a 100644 --- a/distro.go +++ b/distro.go @@ -114,12 +114,9 @@ func DefaultDistro(ctx context.Context) (d Distro, err error) { // Configuration is the configuration of the distro. type Configuration struct { - Version uint8 // Type of filesystem used (lxfs vs. wslfs, relevant only to WSL1) - DefaultUID uint32 // User ID of default user - InteropEnabled bool // Whether interop with windows is enabled - PathAppended bool // Whether Windows paths are appended - DriveMountingEnabled bool // Whether drive mounting is enabled - undocumentedWSLVersion uint8 // Undocumented variable. WSL1 vs. WSL2. + Version uint8 // Type of filesystem used (lxfs vs. wslfs, relevant only to WSL1) + DefaultUID uint32 // User ID of default user + flags.Unpacked DefaultEnvironmentVariables map[string]string // Environment variables passed to the distro by default } @@ -181,21 +178,21 @@ func (d Distro) GetConfiguration() (c Configuration, err error) { defer decorate.OnError(&err, "could not access configuration for %s", d.name) var conf Configuration - var flags flags.WslFlags + var f flags.WslFlags err = selectBackend(d.ctx).WslGetDistributionConfiguration( d.Name(), &conf.Version, &conf.DefaultUID, - &flags, + &f, &conf.DefaultEnvironmentVariables, ) if err != nil { return conf, err } + conf.Unpacked = flags.Unpack(f) - conf.unpackFlags(flags) return conf, nil } @@ -254,7 +251,7 @@ func (d Distro) configToString() string { - DriveMountingEnabled: %t - undocumentedWSLVersion: %d - DefaultEnvironmentVariables:%s`, c.Version, c.DefaultUID, c.InteropEnabled, c.PathAppended, - c.DriveMountingEnabled, c.undocumentedWSLVersion, fmtEnvs) + c.DriveMountingEnabled, c.UndocumentedWSLVersion, fmtEnvs) } // configure is a wrapper around Win32's WslConfigureDistribution. @@ -264,60 +261,10 @@ func (d Distro) configToString() string { // - PathAppended // - DriveMountingEnabled func (d *Distro) configure(config Configuration) error { - flags, err := config.packFlags() + flags, err := config.Pack() if err != nil { return err } return selectBackend(d.ctx).WslConfigureDistribution(d.Name(), config.DefaultUID, flags) } - -// unpackFlags examines a winWslFlags object and stores its findings in the Configuration. -func (conf *Configuration) unpackFlags(f flags.WslFlags) { - conf.InteropEnabled = false - if flags.ENABLE_INTEROP != 0 { - conf.InteropEnabled = true - } - - conf.PathAppended = false - if f&flags.APPEND_NT_PATH != 0 { - conf.PathAppended = true - } - - conf.DriveMountingEnabled = false - if f&flags.ENABLE_DRIVE_MOUNTING != 0 { - conf.DriveMountingEnabled = true - } - - conf.undocumentedWSLVersion = 1 - if f&flags.Undocumented_WSL_VERSION != 0 { - conf.undocumentedWSLVersion = 2 - } -} - -// packFlags generates a winWslFlags object from the Configuration. -func (conf Configuration) packFlags() (flags.WslFlags, error) { - f := flags.NONE - - if conf.InteropEnabled { - f = f | flags.ENABLE_INTEROP - } - - if conf.PathAppended { - f = f | flags.APPEND_NT_PATH - } - - if conf.DriveMountingEnabled { - f = f | flags.ENABLE_DRIVE_MOUNTING - } - - switch conf.undocumentedWSLVersion { - case 1: - case 2: - f = f | flags.Undocumented_WSL_VERSION - default: - return f, fmt.Errorf("unknown WSL version %d", conf.undocumentedWSLVersion) - } - - return f, nil -} diff --git a/internal/flags/flags.go b/internal/flags/flags.go index a4278e91..e4191708 100644 --- a/internal/flags/flags.go +++ b/internal/flags/flags.go @@ -12,13 +12,12 @@ type WslFlags int32 const ( // All nolints are regarding the use of UPPPER_CASE. - NONE WslFlags = 0x0 - ENABLE_INTEROP WslFlags = 0x1 //nolint: revive - APPEND_NT_PATH WslFlags = 0x2 //nolint: revive - ENABLE_DRIVE_MOUNTING WslFlags = 0x4 //nolint: revive + flag_ENABLE_INTEROP WslFlags = 0x1 //nolint:revive + flag_APPEND_NT_PATH WslFlags = 0x2 //nolint:revive + flag_ENABLE_DRIVE_MOUNTING WslFlags = 0x4 //nolint:revive // Per the conversation at https://github.com/microsoft/WSL-DistroLauncher/issues/96 // the information about version 1 or 2 is on the 4th bit of the flags, which is // currently referenced neither by the API nor the documentation. - Undocumented_WSL_VERSION WslFlags = 0x8 //nolint: revive + flag_undocumented_WSL_VERSION WslFlags = 0x8 //nolint:revive ) diff --git a/internal/flags/flags_test.go b/internal/flags/flags_test.go new file mode 100644 index 00000000..f43e1dc4 --- /dev/null +++ b/internal/flags/flags_test.go @@ -0,0 +1,74 @@ +package flags_test + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/ubuntu/gowsl/internal/flags" +) + +func TestUnpackFlags(t *testing.T) { + t.Parallel() + + tests := []struct { + wants flags.Unpacked + input flags.WslFlags + }{ + {input: flags.WslFlags(0x0), wants: flags.Unpacked{InteropEnabled: false, PathAppended: false, DriveMountingEnabled: false}}, + {input: flags.WslFlags(0x1), wants: flags.Unpacked{InteropEnabled: true, PathAppended: false, DriveMountingEnabled: false}}, + {input: flags.WslFlags(0x2), wants: flags.Unpacked{InteropEnabled: false, PathAppended: true, DriveMountingEnabled: false}}, + {input: flags.WslFlags(0x3), wants: flags.Unpacked{InteropEnabled: true, PathAppended: true, DriveMountingEnabled: false}}, + {input: flags.WslFlags(0x4), wants: flags.Unpacked{InteropEnabled: false, PathAppended: false, DriveMountingEnabled: true}}, + {input: flags.WslFlags(0x5), wants: flags.Unpacked{InteropEnabled: true, PathAppended: false, DriveMountingEnabled: true}}, + {input: flags.WslFlags(0x6), wants: flags.Unpacked{InteropEnabled: false, PathAppended: true, DriveMountingEnabled: true}}, + {input: flags.WslFlags(0x7), wants: flags.Unpacked{InteropEnabled: true, PathAppended: true, DriveMountingEnabled: true}}, + // The following may be encountered due to an undocumented fourth flagcd + {input: flags.WslFlags(0x8), wants: flags.Unpacked{InteropEnabled: false, PathAppended: false, DriveMountingEnabled: false}}, + {input: flags.WslFlags(0x9), wants: flags.Unpacked{InteropEnabled: true, PathAppended: false, DriveMountingEnabled: false}}, + {input: flags.WslFlags(0xa), wants: flags.Unpacked{InteropEnabled: false, PathAppended: true, DriveMountingEnabled: false}}, + {input: flags.WslFlags(0xb), wants: flags.Unpacked{InteropEnabled: true, PathAppended: true, DriveMountingEnabled: false}}, + {input: flags.WslFlags(0xc), wants: flags.Unpacked{InteropEnabled: false, PathAppended: false, DriveMountingEnabled: true}}, + {input: flags.WslFlags(0xd), wants: flags.Unpacked{InteropEnabled: true, PathAppended: false, DriveMountingEnabled: true}}, + {input: flags.WslFlags(0xe), wants: flags.Unpacked{InteropEnabled: false, PathAppended: true, DriveMountingEnabled: true}}, + {input: flags.WslFlags(0xf), wants: flags.Unpacked{InteropEnabled: true, PathAppended: true, DriveMountingEnabled: true}}, + } + + for _, tc := range tests { + tc := tc + t.Run(fmt.Sprintf("input_0x%x", int(tc.input)), func(t *testing.T) { + t.Parallel() + got := flags.Unpack(tc.input) + assert.Equal(t, tc.wants.InteropEnabled, got.InteropEnabled, "InteropEnabled does not match the expected value") + assert.Equal(t, tc.wants.PathAppended, got.PathAppended, "PathAppended does not match the expected value") + assert.Equal(t, tc.wants.DriveMountingEnabled, got.DriveMountingEnabled, "DriveMountingEnabled does not match the expected value") + }) + } +} + +func TestPackFlags(t *testing.T) { + t.Parallel() + tests := []struct { + input flags.Unpacked + wants flags.WslFlags + }{ + {wants: flags.WslFlags(0x0), input: flags.Unpacked{InteropEnabled: false, PathAppended: false, DriveMountingEnabled: false}}, + {wants: flags.WslFlags(0x1), input: flags.Unpacked{InteropEnabled: true, PathAppended: false, DriveMountingEnabled: false}}, + {wants: flags.WslFlags(0x2), input: flags.Unpacked{InteropEnabled: false, PathAppended: true, DriveMountingEnabled: false}}, + {wants: flags.WslFlags(0x3), input: flags.Unpacked{InteropEnabled: true, PathAppended: true, DriveMountingEnabled: false}}, + {wants: flags.WslFlags(0x4), input: flags.Unpacked{InteropEnabled: false, PathAppended: false, DriveMountingEnabled: true}}, + {wants: flags.WslFlags(0x5), input: flags.Unpacked{InteropEnabled: true, PathAppended: false, DriveMountingEnabled: true}}, + {wants: flags.WslFlags(0x6), input: flags.Unpacked{InteropEnabled: false, PathAppended: true, DriveMountingEnabled: true}}, + {wants: flags.WslFlags(0x7), input: flags.Unpacked{InteropEnabled: true, PathAppended: true, DriveMountingEnabled: true}}, + } + + for _, tc := range tests { + tc := tc + t.Run(fmt.Sprintf("expects_0x%x", int(tc.wants)), func(t *testing.T) { + t.Parallel() + got, _ := tc.input.Pack() + require.Equal(t, tc.wants, got) + }) + } +} diff --git a/internal/flags/packed.go b/internal/flags/packed.go new file mode 100644 index 00000000..b9de23d1 --- /dev/null +++ b/internal/flags/packed.go @@ -0,0 +1,65 @@ +package flags + +import "fmt" + +// Unpacked contains the same information as WslFlags but in a struct instead of an integer. +type Unpacked struct { + InteropEnabled bool // Whether interop with windows is enabled + PathAppended bool // Whether Windows paths are appended + DriveMountingEnabled bool // Whether drive mounting is enabled + UndocumentedWSLVersion uint8 // Undocumented variable. WSL1 vs. WSL2. +} + +// Unpack examines a WslFlags object and stores its data in a Unpacked flags struct. +func Unpack(f WslFlags) Unpacked { + var up Unpacked + + up.InteropEnabled = false + if f&flag_ENABLE_INTEROP != 0 { + up.InteropEnabled = true + } + + up.PathAppended = false + if f&flag_APPEND_NT_PATH != 0 { + up.PathAppended = true + } + + up.DriveMountingEnabled = false + if f&flag_ENABLE_DRIVE_MOUNTING != 0 { + up.DriveMountingEnabled = true + } + + up.UndocumentedWSLVersion = 1 + if f&flag_undocumented_WSL_VERSION != 0 { + up.UndocumentedWSLVersion = 2 + } + + return up +} + +// Pack generates a WslFlags object from the Unpacked struct. +func (conf Unpacked) Pack() (WslFlags, error) { + var f WslFlags + + if conf.InteropEnabled { + f = f | flag_ENABLE_INTEROP + } + + if conf.PathAppended { + f = f | flag_APPEND_NT_PATH + } + + if conf.DriveMountingEnabled { + f = f | flag_ENABLE_DRIVE_MOUNTING + } + + switch conf.UndocumentedWSLVersion { + case 1: + case 2: + f = f | flag_undocumented_WSL_VERSION + default: + return f, fmt.Errorf("unknown WSL version %d", conf.UndocumentedWSLVersion) + } + + return f, nil +} diff --git a/internal_test.go b/internal_test.go deleted file mode 100644 index 46fcb88a..00000000 --- a/internal_test.go +++ /dev/null @@ -1,75 +0,0 @@ -package gowsl - -import ( - "fmt" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - "github.com/ubuntu/gowsl/internal/flags" -) - -func TestUnpackFlags(t *testing.T) { - t.Parallel() - - tests := []struct { - wants Configuration - input flags.WslFlags - }{ - {input: flags.WslFlags(0x0), wants: Configuration{InteropEnabled: false, PathAppended: false, DriveMountingEnabled: false}}, - {input: flags.WslFlags(0x1), wants: Configuration{InteropEnabled: true, PathAppended: false, DriveMountingEnabled: false}}, - {input: flags.WslFlags(0x2), wants: Configuration{InteropEnabled: false, PathAppended: true, DriveMountingEnabled: false}}, - {input: flags.WslFlags(0x3), wants: Configuration{InteropEnabled: true, PathAppended: true, DriveMountingEnabled: false}}, - {input: flags.WslFlags(0x4), wants: Configuration{InteropEnabled: false, PathAppended: false, DriveMountingEnabled: true}}, - {input: flags.WslFlags(0x5), wants: Configuration{InteropEnabled: true, PathAppended: false, DriveMountingEnabled: true}}, - {input: flags.WslFlags(0x6), wants: Configuration{InteropEnabled: false, PathAppended: true, DriveMountingEnabled: true}}, - {input: flags.WslFlags(0x7), wants: Configuration{InteropEnabled: true, PathAppended: true, DriveMountingEnabled: true}}, - // The following may be encountered due to an undocumented fourth flag - {input: flags.WslFlags(0x8), wants: Configuration{InteropEnabled: false, PathAppended: false, DriveMountingEnabled: false}}, - {input: flags.WslFlags(0x9), wants: Configuration{InteropEnabled: true, PathAppended: false, DriveMountingEnabled: false}}, - {input: flags.WslFlags(0xa), wants: Configuration{InteropEnabled: false, PathAppended: true, DriveMountingEnabled: false}}, - {input: flags.WslFlags(0xb), wants: Configuration{InteropEnabled: true, PathAppended: true, DriveMountingEnabled: false}}, - {input: flags.WslFlags(0xc), wants: Configuration{InteropEnabled: false, PathAppended: false, DriveMountingEnabled: true}}, - {input: flags.WslFlags(0xd), wants: Configuration{InteropEnabled: true, PathAppended: false, DriveMountingEnabled: true}}, - {input: flags.WslFlags(0xe), wants: Configuration{InteropEnabled: false, PathAppended: true, DriveMountingEnabled: true}}, - {input: flags.WslFlags(0xf), wants: Configuration{InteropEnabled: true, PathAppended: true, DriveMountingEnabled: true}}, - } - - for _, tc := range tests { - tc := tc - t.Run(fmt.Sprintf("input_0x%x", int(tc.input)), func(t *testing.T) { - t.Parallel() - got := Configuration{InteropEnabled: false, PathAppended: false, DriveMountingEnabled: false} - got.unpackFlags(tc.input) - assert.Equal(t, tc.wants.InteropEnabled, got.InteropEnabled) - assert.Equal(t, tc.wants.PathAppended, got.PathAppended) - assert.Equal(t, tc.wants.DriveMountingEnabled, got.DriveMountingEnabled) - }) - } -} - -func TestPackFlags(t *testing.T) { - t.Parallel() - tests := []struct { - input Configuration - wants flags.WslFlags - }{ - {wants: flags.WslFlags(0x0), input: Configuration{InteropEnabled: false, PathAppended: false, DriveMountingEnabled: false}}, - {wants: flags.WslFlags(0x1), input: Configuration{InteropEnabled: true, PathAppended: false, DriveMountingEnabled: false}}, - {wants: flags.WslFlags(0x2), input: Configuration{InteropEnabled: false, PathAppended: true, DriveMountingEnabled: false}}, - {wants: flags.WslFlags(0x3), input: Configuration{InteropEnabled: true, PathAppended: true, DriveMountingEnabled: false}}, - {wants: flags.WslFlags(0x4), input: Configuration{InteropEnabled: false, PathAppended: false, DriveMountingEnabled: true}}, - {wants: flags.WslFlags(0x5), input: Configuration{InteropEnabled: true, PathAppended: false, DriveMountingEnabled: true}}, - {wants: flags.WslFlags(0x6), input: Configuration{InteropEnabled: false, PathAppended: true, DriveMountingEnabled: true}}, - {wants: flags.WslFlags(0x7), input: Configuration{InteropEnabled: true, PathAppended: true, DriveMountingEnabled: true}}, - } - - for _, tc := range tests { - tc := tc - t.Run(fmt.Sprintf("expects_0x%x", int(tc.wants)), func(t *testing.T) { - t.Parallel() - got, _ := tc.input.packFlags() - require.Equal(t, tc.wants, got) - }) - } -}