Skip to content

Commit ea502d4

Browse files
danielbotrosDaniel Botros
andcommitted
[APP-8439] Ensure VIAM_MODULE_ROOT is set for local modules (viamrobotics#5246)
Co-authored-by: Daniel Botros <[email protected]>
1 parent 82466e6 commit ea502d4

File tree

5 files changed

+26
-14
lines changed

5 files changed

+26
-14
lines changed

config/module.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -180,8 +180,8 @@ func (m Module) SyntheticPackage() (PackageConfig, error) {
180180
return PackageConfig{Name: name, Package: name, Type: PackageTypeModule, Version: m.LocalVersion}, nil
181181
}
182182

183-
// exeDir returns the parent directory for the unpacked module.
184-
func (m Module) exeDir(packagesDir string) (string, error) {
183+
// ExeDir returns the parent directory for the unpacked module.
184+
func (m Module) ExeDir(packagesDir string) (string, error) {
185185
if !m.NeedsSyntheticPackage() {
186186
return filepath.Dir(m.ExePath), nil
187187
}
@@ -218,7 +218,7 @@ func (m Module) EvaluateExePath(packagesDir string) (string, error) {
218218
if !filepath.IsAbs(path) {
219219
return "", fmt.Errorf("expected ExePath to be absolute path, got %s", path)
220220
}
221-
exeDir, err := m.exeDir(packagesDir)
221+
exeDir, err := m.ExeDir(packagesDir)
222222
if err != nil {
223223
return "", err
224224
}
@@ -270,7 +270,7 @@ func (m *Module) FirstRun(
270270
) error {
271271
logger = logger.Sublogger("first_run").WithFields("module", m.Name)
272272

273-
unpackedModDir, err := m.exeDir(localPackagesDir)
273+
unpackedModDir, err := m.ExeDir(localPackagesDir)
274274
if err != nil {
275275
return err
276276
}

config/module_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ func TestInternalMeta(t *testing.T) {
4040
})
4141

4242
t.Run("meta.json present", func(t *testing.T) {
43-
exeDir, err := mod.exeDir(packagesDir)
43+
exeDir, err := mod.ExeDir(packagesDir)
4444
test.That(t, err, test.ShouldBeNil)
4545
manifest := JSONManifest{Entrypoint: "entry"}
4646
testWriteJSON(t, filepath.Join(exeDir, "meta.json"), manifest)
@@ -91,7 +91,7 @@ func TestSyntheticModule(t *testing.T) {
9191
})
9292

9393
t.Run("syntheticPackageExeDir", func(t *testing.T) {
94-
dir, err := modNeedsSynthetic.exeDir(tmp)
94+
dir, err := modNeedsSynthetic.ExeDir(tmp)
9595
test.That(t, err, test.ShouldBeNil)
9696
test.That(t, dir, test.ShouldEqual, filepath.Join(tmp, "data/module/synthetic--"))
9797
})

module/modmanager/manager.go

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1024,13 +1024,14 @@ func (mgr *Manager) FirstRun(ctx context.Context, conf config.Module) error {
10241024
return err
10251025
}
10261026
}
1027-
env := getFullEnvironment(conf, dataDir, mgr.viamHomeDir)
1027+
env := getFullEnvironment(conf, pkgsDir, dataDir, mgr.viamHomeDir)
10281028

10291029
return conf.FirstRun(ctx, pkgsDir, dataDir, env, mgr.logger)
10301030
}
10311031

10321032
func getFullEnvironment(
10331033
cfg config.Module,
1034+
packagesDir string,
10341035
dataDir string,
10351036
viamHomeDir string,
10361037
) map[string]string {
@@ -1042,8 +1043,18 @@ func getFullEnvironment(
10421043
if cfg.Type == config.ModuleTypeRegistry {
10431044
environment["VIAM_MODULE_ID"] = cfg.ModuleID
10441045
}
1045-
// Overwrite the base environment variables with the module's environment variables (if specified)
1046+
1047+
// For local modules, we set VIAM_MODULE_ROOT to the parent directory of the unpacked module.
10461048
// VIAM_MODULE_ROOT is filled out by app.viam.com in cloud robots.
1049+
if cfg.Type == config.ModuleTypeLocal {
1050+
moduleRoot, err := cfg.ExeDir(packagesDir)
1051+
// err should never not be nil since we are working with local modules
1052+
if err == nil {
1053+
environment["VIAM_MODULE_ROOT"] = moduleRoot
1054+
}
1055+
}
1056+
1057+
// Overwrite the base environment variables with the module's environment variables (if specified)
10471058
for key, value := range cfg.Environment {
10481059
environment[key] = value
10491060
}

module/modmanager/manager_test.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -160,17 +160,18 @@ func TestModManagerFunctions(t *testing.T) {
160160

161161
test.That(t, mod.process.Stop(), test.ShouldBeNil)
162162

163-
modEnv := mod.getFullEnvironment(viamHomeTemp)
163+
modEnv := mod.getFullEnvironment(viamHomeTemp, filepath.Join(viamHomeTemp, "packages"))
164164
test.That(t, modEnv["VIAM_HOME"], test.ShouldEqual, viamHomeTemp)
165165
test.That(t, modEnv["VIAM_MODULE_DATA"], test.ShouldEqual, "module-data-dir")
166166
test.That(t, modEnv["VIAM_MODULE_ID"], test.ShouldEqual, "new:york")
167167
test.That(t, modEnv["SMART"], test.ShouldEqual, "MACHINES")
168168

169-
// Test that VIAM_MODULE_ID is unset for local modules
169+
// Test that VIAM_MODULE_ID is unset and VIAM_MODULE_ROOT is set correctly for local modules
170170
mod.cfg.Type = config.ModuleTypeLocal
171-
modEnv = mod.getFullEnvironment(viamHomeTemp)
171+
modEnv = mod.getFullEnvironment(viamHomeTemp, filepath.Join(viamHomeTemp, "packages"))
172172
_, ok = modEnv["VIAM_MODULE_ID"]
173173
test.That(t, ok, test.ShouldBeFalse)
174+
test.That(t, modEnv["VIAM_MODULE_ROOT"], test.ShouldNotBeEmpty)
174175

175176
// Make a copy of addr and client to test that connections are properly remade
176177
oldAddr := mod.addr

module/modmanager/module.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ func (m *module) startProcess(
190190
if err != nil {
191191
return err
192192
}
193-
moduleEnvironment := m.getFullEnvironment(viamHomeDir)
193+
moduleEnvironment := m.getFullEnvironment(viamHomeDir, packagesDir)
194194
// Prefer VIAM_MODULE_ROOT as the current working directory if present but fallback to the directory of the exepath
195195
moduleWorkingDirectory, ok := moduleEnvironment["VIAM_MODULE_ROOT"]
196196
if !ok {
@@ -399,8 +399,8 @@ func (m *module) cleanupAfterCrash(mgr *Manager) {
399399
}
400400
}
401401

402-
func (m *module) getFullEnvironment(viamHomeDir string) map[string]string {
403-
return getFullEnvironment(m.cfg, m.dataDir, viamHomeDir)
402+
func (m *module) getFullEnvironment(viamHomeDir, packagesDir string) map[string]string {
403+
return getFullEnvironment(m.cfg, packagesDir, m.dataDir, viamHomeDir)
404404
}
405405

406406
func (m *module) getFTDCName() string {

0 commit comments

Comments
 (0)