Skip to content

Commit ce7b062

Browse files
authored
[skip changelog] Use appropriate name for field that stores library folder name (#1900)
Each Arduino library has a name, which is used as its sole identifier by the user in `arduino-cli lib` commands, and by Arduino CLI when referring to the library in messages displayed to the user. The name is defined by: - "1.5 format" libraries: `name` field in the library.properties metadata file - "1.0 format" (AKA "legacy") libraries: installation folder name The name is resolved when loading the library and stored in the `Name` field of the `github.com/arduino/arduino-cli/arduino/libraries.Library` struct. The name of the library's installation folder is used by Arduino CLI in several other ways, most notably for determining "folder name priority" for use in library dependency resolution. For this reason, the folder name is also stored in the struct when loading the library. Arduino CLI and arduino-builder have been plagued by problems caused by the inappropriate use of this folder name as the identifier for the library instead of the sole correct identifier (which is only the folder name in the case of "1.0 format" libraries. The design of the `github.com/arduino/arduino-cli/arduino/libraries.Library` struct may have been a contributing factor in those bugs, since at the time of their occurrence the folder name was stored in the `Name` field, the metadata-defined name in a `RealName`. In addition to the fact that no one field could be used as a source of the name in all cases, I suspect the ambiguous field names themselves caused confusion to developers. This situation was improved by providing the library identifier via a single field for all library formats. The name provided by this field is the "canonical" name of the library. Inexplicably, at that time the field containing the folder name was renamed "CanonicalName". The string contained by this field is in no way a "canonical" name for the library, so the field name is bound to cause more of the same bugs and confusion the redesign of the struct was intended to prevent. The inappropriately named `github.com/arduino/arduino-cli/arduino/libraries.Library.CanonicalName` field is hereby renamed to the accurate `DirName`.
1 parent 02e8c77 commit ce7b062

File tree

9 files changed

+26
-26
lines changed

9 files changed

+26
-26
lines changed

Diff for: arduino/libraries/libraries.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ type Library struct {
6363
Types []string `json:"types,omitempty"`
6464

6565
InstallDir *paths.Path
66-
CanonicalName string
66+
DirName string
6767
SourceDir *paths.Path
6868
UtilityDir *paths.Path
6969
Location LibraryLocation

Diff for: arduino/libraries/librariesresolver/cpp.go

+7-7
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ func computePriority(lib *libraries.Library, header, arch string) int {
168168
header = strings.TrimSuffix(header, filepath.Ext(header))
169169
header = simplify(header)
170170
name := simplify(lib.Name)
171-
canonicalName := simplify(lib.CanonicalName)
171+
dirName := simplify(lib.DirName)
172172

173173
priority := 0
174174

@@ -185,17 +185,17 @@ func computePriority(lib *libraries.Library, header, arch string) int {
185185
priority += 0
186186
}
187187

188-
if name == header && canonicalName == header {
188+
if name == header && dirName == header {
189189
priority += 600
190-
} else if name == header || canonicalName == header {
190+
} else if name == header || dirName == header {
191191
priority += 500
192-
} else if name == header+"-master" || canonicalName == header+"-master" {
192+
} else if name == header+"-master" || dirName == header+"-master" {
193193
priority += 400
194-
} else if strings.HasPrefix(name, header) || strings.HasPrefix(canonicalName, header) {
194+
} else if strings.HasPrefix(name, header) || strings.HasPrefix(dirName, header) {
195195
priority += 300
196-
} else if strings.HasSuffix(name, header) || strings.HasSuffix(canonicalName, header) {
196+
} else if strings.HasSuffix(name, header) || strings.HasSuffix(dirName, header) {
197197
priority += 200
198-
} else if strings.Contains(name, header) || strings.Contains(canonicalName, header) {
198+
} else if strings.Contains(name, header) || strings.Contains(dirName, header) {
199199
priority += 100
200200
}
201201

Diff for: arduino/libraries/librariesresolver/cpp_test.go

+6-6
Original file line numberDiff line numberDiff line change
@@ -147,14 +147,14 @@ func TestCppHeaderResolver(t *testing.T) {
147147
func TestCppHeaderResolverWithLibrariesInStrangeDirectoryNames(t *testing.T) {
148148
resolver := NewCppResolver()
149149
librarylist := libraries.List{}
150-
librarylist.Add(&libraries.Library{CanonicalName: "onewire_2_3_4", Name: "OneWire", Architectures: []string{"*"}})
151-
librarylist.Add(&libraries.Library{CanonicalName: "onewireng_2_3_4", Name: "OneWireNg", Architectures: []string{"avr"}})
150+
librarylist.Add(&libraries.Library{DirName: "onewire_2_3_4", Name: "OneWire", Architectures: []string{"*"}})
151+
librarylist.Add(&libraries.Library{DirName: "onewireng_2_3_4", Name: "OneWireNg", Architectures: []string{"avr"}})
152152
resolver.headers["OneWire.h"] = librarylist
153-
require.Equal(t, "onewire_2_3_4", resolver.ResolveFor("OneWire.h", "avr").CanonicalName)
153+
require.Equal(t, "onewire_2_3_4", resolver.ResolveFor("OneWire.h", "avr").DirName)
154154

155155
librarylist2 := libraries.List{}
156-
librarylist2.Add(&libraries.Library{CanonicalName: "OneWire", Name: "OneWire", Architectures: []string{"*"}})
157-
librarylist2.Add(&libraries.Library{CanonicalName: "onewire_2_3_4", Name: "OneWire", Architectures: []string{"avr"}})
156+
librarylist2.Add(&libraries.Library{DirName: "OneWire", Name: "OneWire", Architectures: []string{"*"}})
157+
librarylist2.Add(&libraries.Library{DirName: "onewire_2_3_4", Name: "OneWire", Architectures: []string{"avr"}})
158158
resolver.headers["OneWire.h"] = librarylist2
159-
require.Equal(t, "OneWire", resolver.ResolveFor("OneWire.h", "avr").CanonicalName)
159+
require.Equal(t, "OneWire", resolver.ResolveFor("OneWire.h", "avr").DirName)
160160
}

Diff for: arduino/libraries/loader.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ func makeNewLibrary(libraryDir *paths.Path, location LibraryLocation) (*Library,
108108
if err := addExamples(library); err != nil {
109109
return nil, errors.Errorf(tr("scanning examples: %s"), err)
110110
}
111-
library.CanonicalName = libraryDir.Base()
111+
library.DirName = libraryDir.Base()
112112
library.Name = strings.TrimSpace(libProperties.Get("name"))
113113
library.Author = strings.TrimSpace(libProperties.Get("author"))
114114
library.Maintainer = strings.TrimSpace(libProperties.Get("maintainer"))
@@ -132,7 +132,7 @@ func makeLegacyLibrary(path *paths.Path, location LibraryLocation) (*Library, er
132132
SourceDir: path,
133133
Layout: FlatLayout,
134134
Name: path.Base(),
135-
CanonicalName: path.Base(),
135+
DirName: path.Base(),
136136
Architectures: []string{"*"},
137137
IsLegacy: true,
138138
Version: semver.MustParse(""),

Diff for: docs/UPGRADING.md

+5-5
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,12 @@ Here you can find a list of migration guides to handle breaking changes between
99
In the structure `github.com/arduino/arduino-cli/arduino/libraries.Library` the field:
1010

1111
- `RealName` has been renamed to `Name`
12-
- `Name` has been renamed to `CanonicalName`
12+
- `Name` has been renamed to `DirName`
1313

14-
Now `Name` is the name of the library as it appears in the `library.properties` file and `CanonicalName` it's the name
15-
of the directory containing the library. The `CanonicalName` is usually the name of the library with non-alphanumeric
16-
characters converted to underscore, but it could be actually anything since the directory where the library is installed
17-
can be freely renamed.
14+
Now `Name` is the name of the library as it appears in the `library.properties` file and `DirName` it's the name of the
15+
directory containing the library. The `DirName` is usually the name of the library with non-alphanumeric characters
16+
converted to underscore, but it could be actually anything since the directory where the library is installed can be
17+
freely renamed.
1818

1919
This change improves the overall code base naming coherence since all the structures involving libraries have the `Name`
2020
field that refers to the library name as it appears in the `library.properties` file.

Diff for: internal/integrationtest/lib/lib_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ func TestLibUpgradeCommand(t *testing.T) {
6565
requirejson.Query(t, stdOut, `.[].library | select(.name=="Servo") | .version`, servoVersion)
6666
}
6767

68-
func TestLibCommandsUsingNameInsteadOfCanonicalName(t *testing.T) {
68+
func TestLibCommandsUsingNameInsteadOfDirName(t *testing.T) {
6969
env, cli := integrationtest.CreateArduinoCLIWithEnvironment(t)
7070
defer env.CleanUp()
7171

Diff for: legacy/builder/create_cmake_rule.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ func (s *ExportProjectCMake) Run(ctx *types.Context) error {
7373
dynamicLibsFromPkgConfig := map[string]bool{}
7474
for _, library := range ctx.ImportedLibraries {
7575
// Copy used libraries in the correct folder
76-
libDir := libBaseFolder.Join(library.CanonicalName)
76+
libDir := libBaseFolder.Join(library.DirName)
7777
mcu := ctx.BuildProperties.Get(constants.BUILD_PROPERTIES_BUILD_MCU)
7878
utils.CopyDir(library.InstallDir.String(), libDir.String(), validExportExtensions)
7979

Diff for: legacy/builder/phases/libraries_builder.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ func compileLibrary(ctx *types.Context, library *libraries.Library, buildPath *p
131131
if ctx.Verbose {
132132
ctx.Info(tr(`Compiling library "%[1]s"`, library.Name))
133133
}
134-
libraryBuildPath := buildPath.Join(library.CanonicalName)
134+
libraryBuildPath := buildPath.Join(library.DirName)
135135

136136
if err := libraryBuildPath.MkdirAll(); err != nil {
137137
return nil, errors.WithStack(err)
@@ -189,7 +189,7 @@ func compileLibrary(ctx *types.Context, library *libraries.Library, buildPath *p
189189
return nil, errors.WithStack(err)
190190
}
191191
if library.DotALinkage {
192-
archiveFile, err := builder_utils.ArchiveCompiledFiles(ctx, libraryBuildPath, paths.New(library.CanonicalName+".a"), libObjectFiles, buildProperties)
192+
archiveFile, err := builder_utils.ArchiveCompiledFiles(ctx, libraryBuildPath, paths.New(library.DirName+".a"), libObjectFiles, buildProperties)
193193
if err != nil {
194194
return nil, errors.WithStack(err)
195195
}

Diff for: legacy/builder/types/types.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ func buildRoot(ctx *Context, origin interface{}) *paths.Path {
5353
case *sketch.Sketch:
5454
return ctx.SketchBuildPath
5555
case *libraries.Library:
56-
return ctx.LibrariesBuildPath.Join(o.CanonicalName)
56+
return ctx.LibrariesBuildPath.Join(o.DirName)
5757
default:
5858
panic("Unexpected origin for SourceFile: " + fmt.Sprint(origin))
5959
}

0 commit comments

Comments
 (0)