[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`.

Author: per1234

arduino/libraries/libraries.go

@@ -63,7 +63,7 @@ type Library struct {
 	Types []string `json:"types,omitempty"`
 
 	InstallDir             *paths.Path
-	CanonicalName          string
+	DirName                string
 	SourceDir              *paths.Path
 	UtilityDir             *paths.Path
 	Location               LibraryLocation

arduino/libraries/librariesresolver/cpp.go

@@ -168,7 +168,7 @@ func computePriority(lib *libraries.Library, header, arch string) int {
 	header = strings.TrimSuffix(header, filepath.Ext(header))
 	header = simplify(header)
 	name := simplify(lib.Name)
-	canonicalName := simplify(lib.CanonicalName)
+	dirName := simplify(lib.DirName)
 
 	priority := 0
 
@@ -185,17 +185,17 @@ func computePriority(lib *libraries.Library, header, arch string) int {
 		priority += 0
 	}
 
-	if name == header && canonicalName == header {
+	if name == header && dirName == header {
 		priority += 600
-	} else if name == header || canonicalName == header {
+	} else if name == header || dirName == header {
 		priority += 500
-	} else if name == header+"-master" || canonicalName == header+"-master" {
+	} else if name == header+"-master" || dirName == header+"-master" {
 		priority += 400
-	} else if strings.HasPrefix(name, header) || strings.HasPrefix(canonicalName, header) {
+	} else if strings.HasPrefix(name, header) || strings.HasPrefix(dirName, header) {
 		priority += 300
-	} else if strings.HasSuffix(name, header) || strings.HasSuffix(canonicalName, header) {
+	} else if strings.HasSuffix(name, header) || strings.HasSuffix(dirName, header) {
 		priority += 200
-	} else if strings.Contains(name, header) || strings.Contains(canonicalName, header) {
+	} else if strings.Contains(name, header) || strings.Contains(dirName, header) {
 		priority += 100
 	}
 

arduino/libraries/librariesresolver/cpp_test.go

@@ -147,14 +147,14 @@ func TestCppHeaderResolver(t *testing.T) {
 func TestCppHeaderResolverWithLibrariesInStrangeDirectoryNames(t *testing.T) {
 	resolver := NewCppResolver()
 	librarylist := libraries.List{}
-	librarylist.Add(&libraries.Library{CanonicalName: "onewire_2_3_4", Name: "OneWire", Architectures: []string{"*"}})
-	librarylist.Add(&libraries.Library{CanonicalName: "onewireng_2_3_4", Name: "OneWireNg", Architectures: []string{"avr"}})
+	librarylist.Add(&libraries.Library{DirName: "onewire_2_3_4", Name: "OneWire", Architectures: []string{"*"}})
+	librarylist.Add(&libraries.Library{DirName: "onewireng_2_3_4", Name: "OneWireNg", Architectures: []string{"avr"}})
 	resolver.headers["OneWire.h"] = librarylist
-	require.Equal(t, "onewire_2_3_4", resolver.ResolveFor("OneWire.h", "avr").CanonicalName)
+	require.Equal(t, "onewire_2_3_4", resolver.ResolveFor("OneWire.h", "avr").DirName)
 
 	librarylist2 := libraries.List{}
-	librarylist2.Add(&libraries.Library{CanonicalName: "OneWire", Name: "OneWire", Architectures: []string{"*"}})
-	librarylist2.Add(&libraries.Library{CanonicalName: "onewire_2_3_4", Name: "OneWire", Architectures: []string{"avr"}})
+	librarylist2.Add(&libraries.Library{DirName: "OneWire", Name: "OneWire", Architectures: []string{"*"}})
+	librarylist2.Add(&libraries.Library{DirName: "onewire_2_3_4", Name: "OneWire", Architectures: []string{"avr"}})
 	resolver.headers["OneWire.h"] = librarylist2
-	require.Equal(t, "OneWire", resolver.ResolveFor("OneWire.h", "avr").CanonicalName)
+	require.Equal(t, "OneWire", resolver.ResolveFor("OneWire.h", "avr").DirName)
 }

arduino/libraries/loader.go

@@ -108,7 +108,7 @@ func makeNewLibrary(libraryDir *paths.Path, location LibraryLocation) (*Library,
 	if err := addExamples(library); err != nil {
 		return nil, errors.Errorf(tr("scanning examples: %s"), err)
 	}
-	library.CanonicalName = libraryDir.Base()
+	library.DirName = libraryDir.Base()
 	library.Name = strings.TrimSpace(libProperties.Get("name"))
 	library.Author = strings.TrimSpace(libProperties.Get("author"))
 	library.Maintainer = strings.TrimSpace(libProperties.Get("maintainer"))
@@ -132,7 +132,7 @@ func makeLegacyLibrary(path *paths.Path, location LibraryLocation) (*Library, er
 		SourceDir:     path,
 		Layout:        FlatLayout,
 		Name:          path.Base(),
-		CanonicalName: path.Base(),
+		DirName:       path.Base(),
 		Architectures: []string{"*"},
 		IsLegacy:      true,
 		Version:       semver.MustParse(""),

docs/UPGRADING.md

@@ -9,12 +9,12 @@ Here you can find a list of migration guides to handle breaking changes between
 In the structure `github.com/arduino/arduino-cli/arduino/libraries.Library` the field:
 
 - `RealName` has been renamed to `Name`
-- `Name` has been renamed to `CanonicalName`
+- `Name` has been renamed to `DirName`
 
-Now `Name` is the name of the library as it appears in the `library.properties` file and `CanonicalName` it's the name
-of the directory containing the library. The `CanonicalName` is usually the name of the library with non-alphanumeric
-characters converted to underscore, but it could be actually anything since the directory where the library is installed
-can be freely renamed.
+Now `Name` is the name of the library as it appears in the `library.properties` file and `DirName` it's the name of the
+directory containing the library. The `DirName` is usually the name of the library with non-alphanumeric characters
+converted to underscore, but it could be actually anything since the directory where the library is installed can be
+freely renamed.
 
 This change improves the overall code base naming coherence since all the structures involving libraries have the `Name`
 field that refers to the library name as it appears in the `library.properties` file.

internal/integrationtest/lib/lib_test.go

@@ -65,7 +65,7 @@ func TestLibUpgradeCommand(t *testing.T) {
 	requirejson.Query(t, stdOut, `.[].library | select(.name=="Servo") | .version`, servoVersion)
 }
 
-func TestLibCommandsUsingNameInsteadOfCanonicalName(t *testing.T) {
+func TestLibCommandsUsingNameInsteadOfDirName(t *testing.T) {
 	env, cli := integrationtest.CreateArduinoCLIWithEnvironment(t)
 	defer env.CleanUp()
 

legacy/builder/create_cmake_rule.go

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

legacy/builder/phases/libraries_builder.go

@@ -131,7 +131,7 @@ func compileLibrary(ctx *types.Context, library *libraries.Library, buildPath *p
 	if ctx.Verbose {
 		ctx.Info(tr(`Compiling library "%[1]s"`, library.Name))
 	}
-	libraryBuildPath := buildPath.Join(library.CanonicalName)
+	libraryBuildPath := buildPath.Join(library.DirName)
 
 	if err := libraryBuildPath.MkdirAll(); err != nil {
 		return nil, errors.WithStack(err)
@@ -189,7 +189,7 @@ func compileLibrary(ctx *types.Context, library *libraries.Library, buildPath *p
 			return nil, errors.WithStack(err)
 		}
 		if library.DotALinkage {
-			archiveFile, err := builder_utils.ArchiveCompiledFiles(ctx, libraryBuildPath, paths.New(library.CanonicalName+".a"), libObjectFiles, buildProperties)
+			archiveFile, err := builder_utils.ArchiveCompiledFiles(ctx, libraryBuildPath, paths.New(library.DirName+".a"), libObjectFiles, buildProperties)
 			if err != nil {
 				return nil, errors.WithStack(err)
 			}

legacy/builder/types/types.go

@@ -53,7 +53,7 @@ func buildRoot(ctx *Context, origin interface{}) *paths.Path {
 	case *sketch.Sketch:
 		return ctx.SketchBuildPath
 	case *libraries.Library:
-		return ctx.LibrariesBuildPath.Join(o.CanonicalName)
+		return ctx.LibrariesBuildPath.Join(o.DirName)
 	default:
 		panic("Unexpected origin for SourceFile: " + fmt.Sprint(origin))
 	}