Bring back lib deduping/caching by name

Make explicit setting of library names stricter, obviating the need to read
manifests of overridden libraries.  Slightly reduce build time as a side effect.

prepareLib(): include an extensive commentary of explicit library naming with
rationales for allowing or forbidding every case of library name setting.
Provide unit tests for all 9 cases commented, plus a test for the obviated
reading of an unreadable but overridden library.

The primary driver for this change is the use case of overriding a library
where the library source being overridden is inaccessible during building.  This
valuable use case can't be supported if determining the library name requires
reading the manifest of the library being overridden.

cli/manifest_parser/libs_by_loc_map.go → cli/manifest_parser/libs_by_name_map.go

@@ -22,31 +22,31 @@ import (
 	"github.com/mongoose-os/mos/cli/build"
 )
 
-type libByLoc struct {
+type libByName struct {
 	Lib *build.SWModule
 	mtx sync.Mutex
 }
 
-type libByLocMap struct {
-	m   map[string]*libByLoc
+type libByNameMap struct {
+	m   map[string]*libByName
 	mtx sync.Mutex
 }
 
-func newLibByLocMap() *libByLocMap {
-	return &libByLocMap{m: map[string]*libByLoc{}}
+func newLibByNameMap() *libByNameMap {
+	return &libByNameMap{m: map[string]*libByName{}}
 }
 
-// AddOrFetchAndLock() tries to add a new location key to the set.  If
-// successful, the new entry (Lib: nil) is locked and returned; otherwise (the
-// location key already exists) the pre-existing entry is locked and returned.
-func (lm *libByLocMap) AddOrFetchAndLock(loc string) *libByLoc {
+// AddOrFetchAndLock() tries to add a new name key to the set.  If successful,
+// the new entry (Lib: nil) is locked and returned; otherwise (the name key
+// already exists) the pre-existing entry is locked and returned.
+func (lm *libByNameMap) AddOrFetchAndLock(name string) *libByName {
 	lm.mtx.Lock()
 	defer lm.mtx.Unlock()
 
-	ls, ok := lm.m[loc]
+	ls, ok := lm.m[name]
 	if !ok {
-		ls = &libByLoc{}
-		lm.m[loc] = ls
+		ls = &libByName{}
+		lm.m[name] = ls
 	}
 
 	ls.mtx.Lock()

cli/manifest_parser/manifest_parser.go

@@ -528,8 +528,8 @@ type manifestParseContext struct {
 
 	prepareLibs []*prepareLibsEntry
 
-	mtx       *sync.Mutex
-	libsByLoc *libByLocMap
+	mtx        *sync.Mutex
+	libsByName *libByNameMap
 }
 
 // readManifestWithLibs reads manifest from the provided dir, "expands" all
@@ -568,8 +568,8 @@ func readManifestWithLibs(
 
 		cbs: cbs,
 
-		mtx:       &sync.Mutex{},
-		libsByLoc: newLibByLocMap(),
+		mtx:        &sync.Mutex{},
+		libsByName: newLibByNameMap(),
 	}
 
 	manifest, mtime, err := readManifestWithLibs2(dir, pc)
@@ -909,14 +909,14 @@ func prepareLib(
 		return
 	}
 
-	ls := pc.libsByLoc.AddOrFetchAndLock(m.Location)
+	ls := pc.libsByName.AddOrFetchAndLock(m.Name)
 	defer ls.mtx.Unlock()
 	if ls.Lib != nil {
 		prepareLibReencounter(parentNodeName, manifest, pc, ls.Lib, m)
 		return
 	}
 
-	ourutil.Freportf(pc.logWriter, "Reading lib at %q...", m.Location)
+	ourutil.Freportf(pc.logWriter, "Reading lib %q at %q...", m.Name, m.Location)
 
 	libLocalDir, err := pc.cbs.ComponentProvider.GetLibLocalPath(
 		m, pc.rootAppDir, pc.appManifest.LibsVersion, manifest.Platform,
@@ -947,22 +947,73 @@ func prepareLib(
 		return
 	}
 
-	// The library can explicitly set its name in its manifest.  Also its
-	// name can be explicitly set in the referring manifest.  Either
-	// separately is fine.  None is fine too (the name defaults to the
-	// location basename).  However if both are used, treat a mismatch as an
-	// error (building without one of the two names in place is guaranteed
-	// to fail, and is likely with both too).
+	// The name of a library can be explicitly set in its manifest and/or in the
+	// referring manifest.  Barring that, the name defaults to the location
+	// basename.
+	//
+	// The library code is hardwired to the right name via at least its
+	// mgos_*_init() symbol.  The code using the library may also be via, e.g.,
+	// HAVE_* variables.
+	//
+	// The building process uses the library name for library deduplication and
+	// overriding.
+	//
+	// The location being `.../foo', the library name in use is:
+	//
+	// (#)  lib mos.yml  ref mos.yml  name in use
+	// --------------------------------------
+	// (1)  (none)       (none)       foo
+	// (2)  (none)       foo          foo
+	// (3)  (none)       bar          bar
+	// (4)  foo          (none)       foo
+	// (5)  foo          foo          foo
+	// (6)  foo          bar          (ERROR)
+	// (7)  bar          (none)       (ERROR)
+	// (8)  bar          foo          (ERROR)
+	// (9)  bar          bar          bar
+	//
+	// Rationales per (#) case:
+	// - (1) The most typical use case.
+	// - (2, 4, 5) Effectively no new information atop the location basename.
+	// - (3) The handy use case of, e.g., .../foo-test1, .../foo-test2 copies
+	//   adjacent to one another and/or the original .../foo.
+	// - (6) If `foo' were correct here, the same problem as in (7) would apply.
+	//   If `bar' were correct, then `foo' in lib mos.yml would produce erroneous
+	//   results via any automation/content generation applied to individual
+	//   libraries outside their usages.
+	// - (7) The name set via the library manifest must be replicated in the
+	//   referring manifest.  Otherwise, if this library is overridden while this
+	//   particular location isn't accessible from the building environment,
+	//   library deduplication/overriding by name can't obviate the need to read
+	//   the inaccessible manifest.
+	// - (8) This is (6) with the names swapped around.
+	// - (9) A location-independently named library.  E.g., Github repositories
+	//   .../mgos-foo, .../mgos-bar with libraries next to repositories unrelated
+	//   to Mongoose OS.
+	//
+	// At this point, libRefName == `ref mos.yml' name above; libManifest.name ==
+	// `lib mos.yml' name; m.Name == libRefName || library location basename.
+	// After validation, m.Name will be the library `name to use' above.
 	if libManifest.Name != "" {
-		if libRefName != "" && libRefName != libManifest.Name {
+		if libRefName != "" && libRefName != libManifest.Name {  // (6, 8) above
 			lpres <- libPrepareResult{
 				err: fmt.Errorf("Library %q at %q is referred to as %q from %q",
 					libManifest.Name, m.Location,
 					libRefName, manifest.Origin),
 			}
 			return
 		}
-		m.Name = libManifest.Name
+		if libRefName == "" && m.Name != libManifest.Name {  // (7) above
+			lpres <- libPrepareResult{
+				err: fmt.Errorf("Library %q at %q must be referred to as %q from %q",
+					libManifest.Name, m.Location,
+					libManifest.Name, manifest.Origin),
+			}
+			return
+		}
+	}
+	if libRefName != "" && m.Name != libRefName {  // (3, 9) above
+		m.Name = libRefName
 	}
 	name, err := m.GetName()
 	if err != nil {

cli/manifest_parser/test_manifests/testset_10_lib_name_settings/common/mylib1-lib-name/mos.yml

@@ -0,0 +1,4 @@
+type: lib
+name: mylib1
+sources: [src]
+manifest_version: 2020-08-02

cli/manifest_parser/test_manifests/testset_10_lib_name_settings/common/mylib1-lib-name/src

@@ -0,0 +1 @@
+../lib-blank/src

cli/manifest_parser/test_manifests/testset_10_lib_name_settings/test_01_no_name/app/mos.yml

@@ -0,0 +1,8 @@
+manifest_version: 2020-08-02
+
+libs:
+  - location: libs/mylib1
+
+# prepareLib() commentary case (1): a library in use has no explicit `name'
+# settings.  Check that the generated MGOS_HAVE_* and mgos_*_init() identifiers
+# are proper.

cli/manifest_parser/test_manifests/testset_10_lib_name_settings/test_01_no_name/expected/esp32/mgos_deps_init.c

@@ -0,0 +1,80 @@
+/* This file is auto-generated by mos build, do not edit! */
+
+#include <stdbool.h>
+#include <stdio.h>
+
+#include "common/cs_dbg.h"
+
+#include "mgos_app.h"
+
+extern bool mgos_core_init(void);
+extern bool mgos_mylib1_init(void);
+
+
+#ifndef MGOS_LIB_INFO_VERSION
+struct mgos_lib_info {
+  const char *name;
+  const char *version;
+  const char *repo_version;
+  const char *binary_libs;
+  bool (*init)(void);
+};
+#define MGOS_LIB_INFO_VERSION 2
+#endif
+
+#ifndef MGOS_MODULE_INFO_VERSION
+struct mgos_module_info {
+  const char *name;
+  const char *repo_version;
+};
+#define MGOS_MODULE_INFO_VERSION 1
+#endif
+
+const struct mgos_lib_info mgos_libs_info[] = {
+
+    // "core". deps: [ ]
+#if MGOS_LIB_INFO_VERSION == 1
+    {.name = "core", .version = NULL, .init = mgos_core_init},
+#else
+    {.name = "core", .version = NULL, .repo_version = NULL, .binary_libs = NULL, .init = mgos_core_init},
+#endif
+
+    // "mylib1". deps: [ "core" ]
+#if MGOS_LIB_INFO_VERSION == 1
+    {.name = "mylib1", .version = NULL, .init = mgos_mylib1_init},
+#else
+    {.name = "mylib1", .version = NULL, .repo_version = NULL, .binary_libs = NULL, .init = mgos_mylib1_init},
+#endif
+
+    // Last entry.
+    {.name = NULL},
+};
+
+const struct mgos_module_info mgos_modules_info[] = {
+
+    {.name = "mongoose-os", .repo_version = NULL},
+
+    // Last entry.
+    {.name = NULL},
+};
+
+bool mgos_deps_init(void) {
+  for (const struct mgos_lib_info *l = mgos_libs_info; l->name != NULL; l++) {
+#if MGOS_LIB_INFO_VERSION == 1
+    LOG(LL_DEBUG, ("Init %s %s...", l->name, (l->version ? l->version : "")));
+#else
+    LOG(LL_DEBUG, ("Init %s %s (%s)...",
+          l->name,
+          (l->version ? l->version : ""),
+          (l->repo_version ? l->repo_version : "")));
+#endif
+    if (l->init != NULL && !l->init()) {
+      LOG(LL_ERROR, ("%s init failed", l->name));
+      return false;
+    }
+  }
+  for (const struct mgos_module_info *m = mgos_modules_info; m->name != NULL; m++) {
+    LOG(LL_DEBUG, ("Module %s %s", m->name, (m->repo_version ? m->repo_version : "")));
+  }
+  return true;
+}

cli/manifest_parser/test_manifests/testset_10_lib_name_settings/test_01_no_name/expected/esp32/mgos_deps_manifest.yml

@@ -0,0 +1,13 @@
+app_name: app
+libs:
+- name: core
+  location: https://github.com/mongoose-os-libs/core
+  version: "0.01"
+- name: mylib1
+  location: libs/mylib1
+  version: "0.01"
+modules:
+- name: mongoose-os
+  location: https://github.com/cesanta/mongoose-os
+  version: "0.01"
+manifest_version: "2021-03-26"

cli/manifest_parser/test_manifests/testset_10_lib_name_settings/test_01_no_name/expected/esp32/mos_final.yml

@@ -0,0 +1,48 @@
+name: app
+type: app
+platform: esp32
+platforms:
+__ALL_PLATFORMS__
+sources:
+- __APP_ROOT__/libs/core/src/lib.c
+- __APP_ROOT__/libs/mylib1/src/lib.c
+- __APP_ROOT__/app/build/gen/mgos_deps_init.c
+modules:
+- name: mongoose-os
+  location: https://github.com/cesanta/mongoose-os
+  version: "0.01"
+build_vars:
+  BOARD: ""
+  ESP_IDF_EXTRA_COMPONENTS: ""
+  ESP_IDF_SDKCONFIG_OPTS: ""
+  MGOS: "1"
+  MGOS_HAVE_CORE: "1"
+  MGOS_HAVE_MYLIB1: "1"
+cdefs:
+  MGOS: "1"
+  MGOS_HAVE_CORE: "1"
+  MGOS_HAVE_MYLIB1: "1"
+libs_version: "0.01"
+modules_version: "0.01"
+mongoose_os_version: "0.01"
+manifest_version: "2020-08-02"
+libs_handled:
+- lib:
+    name: core
+    location: https://github.com/mongoose-os-libs/core
+  path: __APP_ROOT__/libs/core
+  sources:
+  - __APP_ROOT__/libs/core/src/lib.c
+  version: "0.01"
+- lib:
+    name: mylib1
+    location: libs/mylib1
+  path: __APP_ROOT__/libs/mylib1
+  init_deps:
+  - core
+  sources:
+  - __APP_ROOT__/libs/mylib1/src/lib.c
+  version: "0.01"
+init_deps:
+- core
+- mylib1

cli/manifest_parser/test_manifests/testset_10_lib_name_settings/test_02_ref_name_same/app/mos.yml

@@ -0,0 +1,9 @@
+manifest_version: 2020-08-02
+
+libs:
+  - location: libs/mylib1
+    name: mylib1
+
+# prepareLib() commentary case (2): a library in use has `name' set in referring
+# manifest that corresponds to the basename of its location.  Check that the
+# generated MGOS_HAVE_* and mgos_*_init() identifiers are unaffected.

cli/manifest_parser/test_manifests/testset_10_lib_name_settings/test_02_ref_name_same/expected

@@ -0,0 +1 @@
+../test_01_no_name/expected

cli/manifest_parser/test_manifests/testset_10_lib_name_settings/test_02_ref_name_same/libs

@@ -0,0 +1 @@
+../test_01_no_name/libs

cli/manifest_parser/test_manifests/testset_10_lib_name_settings/test_03_ref_name/app/mos.yml

@@ -0,0 +1,9 @@
+manifest_version: 2020-08-02
+
+libs:
+  - location: libs/mylib1
+    name: lib1
+
+# prepareLib() commentary case (3): a library in use has `name' set in referring
+# manifest that is distinct from its location basename.  Check that the
+# generated MGOS_HAVE_* and mgos_*_init() identifiers are properly altered.

cli/manifest_parser/test_manifests/testset_10_lib_name_settings/test_03_ref_name/libs

@@ -0,0 +1 @@
+../test_01_no_name/libs