This is an automated email from the ASF dual-hosted git repository. sjanc pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/mynewt-newt.git
commit 2ed032d118548b2d43f091c3ba7c846dce4cf0b9 Author: Szymon Janc <[email protected]> AuthorDate: Mon Oct 19 18:01:31 2026 +0200 newt/toolchain: Keep options with separate arguments together Compiler and linker flags were passed through util.SortFields(), which sorts all white space separated tokens and removes duplicates. Options taking a separate argument were torn apart from it, e.g. "-include a.h -include b.h" became "-include a.h b.h", making the compiler treat b.h as an input file. In addition, when such an option was given as a separate list element, addFlags() discarded it as a conflict if another package already used the same option, leaving its argument behind. Add SortFlags(), which keeps options like -include, -imacros, -isystem or -Xlinker together with their argument, in their original order and without deduplication, and sorts and deduplicates all other flags as before. Use it for the compiler command lines and the CMake output, and let addFlags() add such option pairs as a unit. Assisted-by: Claude:claude-opus-5-5 --- newt/builder/cmake.go | 10 ++--- newt/toolchain/compiler.go | 23 ++++++++--- newt/toolchain/flags.go | 93 ++++++++++++++++++++++++++++++++++++++++++++ newt/toolchain/flags_test.go | 37 ++++++++++++++++++ 4 files changed, 152 insertions(+), 11 deletions(-) diff --git a/newt/builder/cmake.go b/newt/builder/cmake.go index 876d090c..ca3038c9 100644 --- a/newt/builder/cmake.go +++ b/newt/builder/cmake.go @@ -141,7 +141,7 @@ func CmakeSourceObjectWrite(w io.Writer, cj toolchain.CompilerJob, cj.Filename = trimProjectPath(cj.Filename) // Sort and remove duplicate flags - otherFlags = util.SortFields(otherFlags...) + otherFlags = toolchain.SortFlags(otherFlags...) fmt.Fprintf(w, `set_property(SOURCE %s APPEND_STRING @@ -271,7 +271,7 @@ func (b *Builder) CMakeTargetWrite(w io.Writer, targetCompiler *toolchain.Compil compileFlags = append(compileFlags, c.GetLocalCompilerInfo().Cflags...) compileFlags = append(compileFlags, c.GetCompilerInfo().CXXflags...) compileFlags = append(compileFlags, c.GetLocalCompilerInfo().CXXflags...) - compileFlags = util.SortFields(compileFlags...) + compileFlags = toolchain.SortFlags(compileFlags...) fmt.Fprintf(w, `set_property(TARGET %s APPEND_STRING @@ -284,7 +284,7 @@ func (b *Builder) CMakeTargetWrite(w io.Writer, targetCompiler *toolchain.Compil lFlags := append(c.GetCompilerInfo().Lflags, c.GetLocalCompilerInfo().Lflags...) lFlags = append(lFlags, linkFlags...) - lFlags = util.SortFields(lFlags...) + lFlags = toolchain.SortFlags(lFlags...) for _, ld := range c.LinkerScripts { lFlags = append(lFlags, "-T"+ld) @@ -295,13 +295,13 @@ func (b *Builder) CMakeTargetWrite(w io.Writer, targetCompiler *toolchain.Compil var cFlags []string cFlags = append(cFlags, c.GetCompilerInfo().Cflags...) cFlags = append(cFlags, c.GetLocalCompilerInfo().Cflags...) - cFlags = util.SortFields(cFlags...) + cFlags = toolchain.SortFlags(cFlags...) lFlags = append(lFlags, cFlags...) var cxxFlags []string cxxFlags = append(cxxFlags, c.GetCompilerInfo().CXXflags...) cxxFlags = append(cxxFlags, c.GetLocalCompilerInfo().CXXflags...) - cxxFlags = util.SortFields(cxxFlags...) + cxxFlags = toolchain.SortFlags(cxxFlags...) lFlags = append(lFlags, cxxFlags...) fmt.Fprintf(w, diff --git a/newt/toolchain/compiler.go b/newt/toolchain/compiler.go index f1537ac2..306818b1 100644 --- a/newt/toolchain/compiler.go +++ b/newt/toolchain/compiler.go @@ -298,7 +298,18 @@ func addFlags(flagType string, orig []string, new []string) []string { origMap := flagsMap(orig) combined := orig - for _, c := range new { + for i := 0; i < len(new); i++ { + c := new[i] + + // An option taking a separate argument (e.g., "-include", "a.h") is + // added together with its argument; the option alone does not + // identify a conflict. + if flagTakesArg(new, i) { + combined = append(combined, c, new[i+1]) + i++ + continue + } + newBase := flagsBase(c) origVal := origMap[newBase] if origVal == "" { @@ -575,9 +586,9 @@ func (c *Compiler) includesStrings() []string { } func (c *Compiler) cflagsStrings() []string { - cflags := util.SortFields(c.info.Cflags...) + cflags := SortFlags(c.info.Cflags...) - for _, lclinfo_flag := range util.SortFields(c.lclInfo.Cflags...) { + for _, lclinfo_flag := range SortFlags(c.lclInfo.Cflags...) { for _, info_flag := range cflags { if lclinfo_flag == info_flag { continue @@ -589,17 +600,17 @@ func (c *Compiler) cflagsStrings() []string { } func (c *Compiler) cxxflagsStrings() []string { - cxxflags := util.SortFields(c.info.CXXflags...) + cxxflags := SortFlags(c.info.CXXflags...) return cxxflags } func (c *Compiler) aflagsStrings() []string { - aflags := util.SortFields(c.info.Aflags...) + aflags := SortFlags(c.info.Aflags...) return aflags } func (c *Compiler) lflagsStrings() []string { - lflags := util.SortFields(c.info.Lflags...) + lflags := SortFlags(c.info.Lflags...) return lflags } diff --git a/newt/toolchain/flags.go b/newt/toolchain/flags.go new file mode 100644 index 00000000..fa3460f9 --- /dev/null +++ b/newt/toolchain/flags.go @@ -0,0 +1,93 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 toolchain + +import ( + "sort" + "strings" + + "mynewt.apache.org/newt/util" +) + +// Options which take their argument as a separate command line argument +// (e.g., "-include file.h"). +var flagsWithArg = map[string]bool{ + "--param": true, + "-D": true, + "-I": true, + "-L": true, + "-MF": true, + "-MQ": true, + "-MT": true, + "-T": true, + "-U": true, + "-Xassembler": true, + "-Xlinker": true, + "-Xpreprocessor": true, + "-idirafter": true, + "-imacros": true, + "-imultilib": true, + "-include": true, + "-iprefix": true, + "-iquote": true, + "-isysroot": true, + "-isystem": true, + "-iwithprefix": true, + "-iwithprefixbefore": true, + "-o": true, + "-u": true, + "-x": true, + "-z": true, +} + +// flagTakesArg indicates whether flags[i] is an option followed by its +// separate argument. +func flagTakesArg(flags []string, i int) bool { + return flagsWithArg[flags[i]] && i+1 < len(flags) +} + +// SortFlags splits the specified strings on white space, sorts the resulting +// flags and removes duplicates. Options taking a separate argument are kept +// together with their argument; these pairs are placed after all other flags +// in their original order and are not deduplicated, because their order and +// count may be significant (e.g., "-include a.h -include b.h" or +// "-Xlinker -z -Xlinker relro"). +func SortFlags(flags ...string) []string { + tokens := []string{} + for _, f := range flags { + tokens = append(tokens, strings.Fields(f)...) + } + + singles := []string{} + pairs := []string{} + for i := 0; i < len(tokens); i++ { + if flagTakesArg(tokens, i) { + pairs = append(pairs, tokens[i], tokens[i+1]) + i++ + } else { + singles = append(singles, tokens[i]) + } + } + + singles = util.UniqueStrings(singles) + sort.Strings(singles) + + return append(singles, pairs...) +} diff --git a/newt/toolchain/flags_test.go b/newt/toolchain/flags_test.go index a93e511f..fb25da16 100644 --- a/newt/toolchain/flags_test.go +++ b/newt/toolchain/flags_test.go @@ -72,3 +72,40 @@ func TestCommandString(t *testing.T) { t.Errorf("unexpected command string: %q", s) } } + +func TestSortFlags(t *testing.T) { + got := SortFlags("-Wall -O2", "-include a.h", "-Wall", + "-include", "b.h", "-Xlinker -z -Xlinker relro", + "-Xlinker -z -Xlinker noexecstack", "-DFOO=1") + + exp := []string{ + "-DFOO=1", "-O2", "-Wall", + "-include", "a.h", "-include", "b.h", + "-Xlinker", "-z", "-Xlinker", "relro", + "-Xlinker", "-z", "-Xlinker", "noexecstack", + } + if !slices.Equal(got, exp) { + t.Errorf("unexpected flags:\nexpected %q\ngot %q", exp, got) + } + + // An option without argument at the end is kept. + got = SortFlags("-b", "-a", "-include") + exp = []string{"-a", "-b", "-include"} + if !slices.Equal(got, exp) { + t.Errorf("unexpected flags:\nexpected %q\ngot %q", exp, got) + } +} + +func TestCflagsStringsPairs(t *testing.T) { + c := newTestCompiler() + + // Flags from two packages, both using forced includes as separate list + // elements. + c.AddInfo(&CompilerInfo{Cflags: []string{"-Wall", "-include", "a.h"}}) + c.AddInfo(&CompilerInfo{Cflags: []string{"-include", "b.h", "-Wall"}}) + + exp := []string{"-Wall", "-include", "a.h", "-include", "b.h"} + if got := c.cflagsStrings(); !slices.Equal(got, exp) { + t.Errorf("unexpected flags:\nexpected %q\ngot %q", exp, got) + } +}
