From 806b16c2c860249feec26181e839243e237aa9ac Mon Sep 17 00:00:00 2001 From: Li Jie Date: Sun, 7 Sep 2025 15:13:58 +0800 Subject: [PATCH] refine: reduce duplicated env expand funcs --- internal/crosscompile/crosscompile.go | 63 +---------- internal/crosscompile/crosscompile_test.go | 123 --------------------- internal/env/utils.go | 63 +++++++++++ internal/env/utils_test.go | 84 ++++++++++++++ internal/flash/flash.go | 22 +--- 5 files changed, 150 insertions(+), 205 deletions(-) create mode 100644 internal/env/utils.go create mode 100644 internal/env/utils_test.go diff --git a/internal/crosscompile/crosscompile.go b/internal/crosscompile/crosscompile.go index d22624e5..48a5d9f5 100644 --- a/internal/crosscompile/crosscompile.go +++ b/internal/crosscompile/crosscompile.go @@ -59,65 +59,6 @@ func cacheDir() string { return filepath.Join(cacheRoot(), "crosscompile") } -// expandEnv expands template variables in a string -// Supports variables like {port}, {hex}, {bin}, {root}, {tmpDir}, etc. -// Special case: {} expands to the first available file variable (hex, bin, img, zip) -func expandEnv(template string, envs map[string]string) string { - return expandEnvWithDefault(template, envs) -} - -// expandEnvWithDefault expands template variables with optional default for {} -func expandEnvWithDefault(template string, envs map[string]string, defaultValue ...string) string { - if template == "" { - return "" - } - - result := template - - // Handle special case of {} - use provided default or first available file variable - if strings.Contains(result, "{}") { - defaultVal := "" - if len(defaultValue) > 0 && defaultValue[0] != "" { - defaultVal = defaultValue[0] - } else { - // Priority order: hex, bin, img, zip - for _, key := range []string{"hex", "bin", "img", "zip"} { - if value, exists := envs[key]; exists && value != "" { - defaultVal = value - break - } - } - } - result = strings.ReplaceAll(result, "{}", defaultVal) - } - - // Replace named variables - for key, value := range envs { - if key != "" { // Skip empty key used for {} default - result = strings.ReplaceAll(result, "{"+key+"}", value) - } - } - return result -} - -// expandEnvSlice expands template variables in a slice of strings -func expandEnvSlice(templates []string, envs map[string]string) []string { - return expandEnvSliceWithDefault(templates, envs) -} - -// expandEnvSliceWithDefault expands template variables in a slice with optional default for {} -func expandEnvSliceWithDefault(templates []string, envs map[string]string, defaultValue ...string) []string { - if len(templates) == 0 { - return templates - } - - result := make([]string, len(templates)) - for i, template := range templates { - result[i] = expandEnvWithDefault(template, envs, defaultValue...) - } - return result -} - // buildEnvMap creates a map of template variables for the current context func buildEnvMap(llgoRoot string) map[string]string { envs := make(map[string]string) @@ -539,7 +480,7 @@ func UseTarget(targetName string) (export Export, err error) { ccflags = append(ccflags, "--target="+config.LLVMTarget) } // Expand template variables in cflags - expandedCFlags := expandEnvSlice(config.CFlags, envs) + expandedCFlags := env.ExpandEnvSlice(config.CFlags, envs) cflags = append(cflags, expandedCFlags...) // The following parameters are inspired by tinygo/builder/library.go @@ -685,7 +626,7 @@ func UseTarget(targetName string) (export Export, err error) { // Combine with config flags and expand template variables export.CFLAGS = cflags export.CCFLAGS = ccflags - expandedLDFlags := expandEnvSlice(config.LDFlags, envs) + expandedLDFlags := env.ExpandEnvSlice(config.LDFlags, envs) export.LDFLAGS = append(ldflags, expandedLDFlags...) return export, nil diff --git a/internal/crosscompile/crosscompile_test.go b/internal/crosscompile/crosscompile_test.go index 42152a58..b779dc6d 100644 --- a/internal/crosscompile/crosscompile_test.go +++ b/internal/crosscompile/crosscompile_test.go @@ -302,126 +302,3 @@ func TestUseWithTarget(t *testing.T) { t.Error("Expected LDFLAGS to be set for native build") } } - -func TestExpandEnv(t *testing.T) { - envs := map[string]string{ - "port": "/dev/ttyUSB0", - "hex": "firmware.hex", - "bin": "firmware.bin", - "root": "/usr/local/llgo", - } - - tests := []struct { - template string - expected string - }{ - { - "avrdude -c arduino -p atmega328p -P {port} -U flash:w:{hex}:i", - "avrdude -c arduino -p atmega328p -P /dev/ttyUSB0 -U flash:w:firmware.hex:i", - }, - { - "simavr -m atmega328p -f 16000000 {}", - "simavr -m atmega328p -f 16000000 firmware.hex", // {} expands to hex (first priority) - }, - { - "-I{root}/lib/CMSIS/CMSIS/Include", - "-I/usr/local/llgo/lib/CMSIS/CMSIS/Include", - }, - { - "no variables here", - "no variables here", - }, - { - "", - "", - }, - } - - for _, test := range tests { - result := expandEnv(test.template, envs) - if result != test.expected { - t.Errorf("expandEnv(%q) = %q, want %q", test.template, result, test.expected) - } - } -} - -func TestExpandEnvSlice(t *testing.T) { - envs := map[string]string{ - "root": "/usr/local/llgo", - "port": "/dev/ttyUSB0", - } - - input := []string{ - "-I{root}/include", - "-DPORT={port}", - "static-flag", - } - - expected := []string{ - "-I/usr/local/llgo/include", - "-DPORT=/dev/ttyUSB0", - "static-flag", - } - - result := expandEnvSlice(input, envs) - - if len(result) != len(expected) { - t.Fatalf("expandEnvSlice length mismatch: got %d, want %d", len(result), len(expected)) - } - - for i, exp := range expected { - if result[i] != exp { - t.Errorf("expandEnvSlice[%d] = %q, want %q", i, result[i], exp) - } - } -} - -func TestExpandEnvWithDefault(t *testing.T) { - envs := map[string]string{ - "port": "/dev/ttyUSB0", - "hex": "firmware.hex", - "bin": "firmware.bin", - "img": "image.img", - } - - tests := []struct { - template string - defaultValue string - expected string - }{ - { - "simavr {}", - "", // No default - should use hex (priority) - "simavr firmware.hex", - }, - { - "simavr {}", - "custom.elf", // Explicit default - "simavr custom.elf", - }, - { - "qemu -kernel {}", - "vmlinux", // Custom kernel - "qemu -kernel vmlinux", - }, - { - "no braces here", - "ignored", - "no braces here", - }, - } - - for i, test := range tests { - var result string - if test.defaultValue == "" { - result = expandEnvWithDefault(test.template, envs) - } else { - result = expandEnvWithDefault(test.template, envs, test.defaultValue) - } - - if result != test.expected { - t.Errorf("Test %d: expandEnvWithDefault(%q, envs, %q) = %q, want %q", - i, test.template, test.defaultValue, result, test.expected) - } - } -} diff --git a/internal/env/utils.go b/internal/env/utils.go new file mode 100644 index 00000000..ec3d4f3e --- /dev/null +++ b/internal/env/utils.go @@ -0,0 +1,63 @@ +/* + * Copyright (c) 2024 The GoPlus Authors (goplus.org). All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package env + +import "strings" + +// ExpandEnvWithDefault expands template variables with optional default for {} +func ExpandEnvWithDefault(template string, envs map[string]string, defaultValue ...string) string { + if template == "" { + return "" + } + + result := template + + // Handle special case of {} - use provided default or first available file variable + if strings.Contains(result, "{}") { + defaultVal := "" + if len(defaultValue) > 0 && defaultValue[0] != "" { + defaultVal = defaultValue[0] + } + result = strings.ReplaceAll(result, "{}", defaultVal) + } + + // Replace named variables + for key, value := range envs { + if key != "" { // Skip empty key used for {} default + result = strings.ReplaceAll(result, "{"+key+"}", value) + } + } + return result +} + +// ExpandEnvSlice expands template variables in a slice of strings +func ExpandEnvSlice(templates []string, envs map[string]string) []string { + return ExpandEnvSliceWithDefault(templates, envs) +} + +// ExpandEnvSliceWithDefault expands template variables in a slice with optional default for {} +func ExpandEnvSliceWithDefault(templates []string, envs map[string]string, defaultValue ...string) []string { + if len(templates) == 0 { + return templates + } + + result := make([]string, len(templates)) + for i, template := range templates { + result[i] = ExpandEnvWithDefault(template, envs, defaultValue...) + } + return result +} diff --git a/internal/env/utils_test.go b/internal/env/utils_test.go new file mode 100644 index 00000000..e12297f2 --- /dev/null +++ b/internal/env/utils_test.go @@ -0,0 +1,84 @@ +package env + +import "testing" + +func TestExpandEnvSlice(t *testing.T) { + envs := map[string]string{ + "root": "/usr/local/llgo", + "port": "/dev/ttyUSB0", + } + + input := []string{ + "-I{root}/include", + "-DPORT={port}", + "static-flag", + } + + expected := []string{ + "-I/usr/local/llgo/include", + "-DPORT=/dev/ttyUSB0", + "static-flag", + } + + result := ExpandEnvSlice(input, envs) + + if len(result) != len(expected) { + t.Fatalf("expandEnvSlice length mismatch: got %d, want %d", len(result), len(expected)) + } + + for i, exp := range expected { + if result[i] != exp { + t.Errorf("expandEnvSlice[%d] = %q, want %q", i, result[i], exp) + } + } +} + +func TestExpandEnvWithDefault(t *testing.T) { + envs := map[string]string{ + "port": "/dev/ttyUSB0", + "hex": "firmware.hex", + "bin": "firmware.bin", + "img": "image.img", + } + + tests := []struct { + template string + defaultValue string + expected string + }{ + { + "simavr {}", + "firmware.hex", + "simavr firmware.hex", + }, + { + "simavr {}", + "custom.elf", // Explicit default + "simavr custom.elf", + }, + { + "qemu -kernel {}", + "vmlinux", // Custom kernel + "qemu -kernel vmlinux", + }, + { + "no braces here", + "ignored", + "no braces here", + }, + } + + for i, test := range tests { + var result string + if test.defaultValue == "" { + result = ExpandEnvWithDefault(test.template, envs) + } else { + result = ExpandEnvWithDefault(test.template, envs, test.defaultValue) + } + + if result != test.expected { + t.Errorf("Test %d: expandEnvWithDefault(%q, envs, %q) = %q, want %q", + i, test.template, test.defaultValue, result, test.expected) + } + } +} diff --git a/internal/flash/flash.go b/internal/flash/flash.go index b4e790f0..d5ac65b0 100644 --- a/internal/flash/flash.go +++ b/internal/flash/flash.go @@ -248,7 +248,7 @@ func flashCommand(flash Flash, envMap map[string]string, port string, verbose bo envs := buildFlashEnvMap(envMap, port) // Expand template variables in command - expandedCommand := expandEnv(flash.Command, envs) + expandedCommand := env.ExpandEnvWithDefault(flash.Command, envs) if verbose { fmt.Fprintf(os.Stderr, "Flash command: %s\n", expandedCommand) @@ -417,26 +417,6 @@ func buildFlashEnvMap(envMap map[string]string, port string) map[string]string { return envs } -// expandEnv expands template variables in a string -// Supports variables like {port}, {hex}, {bin}, {root}, {tmpDir}, etc. -func expandEnv(template string, envs map[string]string) string { - fmt.Fprintf(os.Stderr, "Expanding template: %s with envs: %v\n", template, envs) - if template == "" { - return "" - } - - result := template - - // Replace named variables - for key, value := range envs { - if key != "" { - result = strings.ReplaceAll(result, "{"+key+"}", value) - } - } - - return result -} - // copyFile copies a file from src to dst func copyFile(src, dst string) error { sourceFile, err := os.Open(src)