Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

> agoric install beta does not update dapp workspace #4170

Closed
mhofman opened this issue Dec 11, 2021 · 3 comments · Fixed by #4236
Closed

> agoric install beta does not update dapp workspace #4170

mhofman opened this issue Dec 11, 2021 · 3 comments · Fixed by #4236
Assignees
Milestone

Comments

@mhofman
Copy link
Member

mhofman commented Dec 11, 2021

agoric install beta will update the workspace's yarn.lock with the beta dependencies. From that point on, installing the dapp only needs yarn install (though agoric install will work, too).

This actually doesn't work for me. The yarn.lock file of my existing dapp doesn't seem to be updated. The preinstall script is also left in place, which means yarn install is not possible.

Originally posted by @mhofman in #4129 (comment)

@mhofman
Copy link
Member Author

mhofman commented Dec 11, 2021

After digging through why yarn global add agoric didn't work, and manually patching my way through (see #4169), I managed to use agoric install beta without ever doing a cli-link from a locally installed sdk. However I still ran into problems. The yarn.lock file was respected, even though beta is way ahead and should have forced new versions of all the @agoric packages.

I'd also expect the various package.json which have * (or in the future outdated beta) dependencies to be patched to the current beta version.

@mhofman
Copy link
Member Author

mhofman commented Dec 11, 2021

I'm thinking this is because this was on a pristine checkout where there were no existing node_modules.

I guess the current updateSdkVersion logic relies on exisiting symlinks to do its work. A bit of a catch-22

@mhofman
Copy link
Member Author

mhofman commented Dec 11, 2021

Alright, so with a linked agoric cli, it seems to work closer to as expected.

agoric-local --no-sdk install beta installed and patched my package.json.

I encountered another problem: I wasn't able to switch back because it seems the agoric-sdk node_modules got corrupted in the process!

$ agoric-local --sdk install
node:internal/process/esm_loader:94
    internalBinding('errors').triggerUncaughtException(
                              ^

Error [ERR_MODULE_NOT_FOUND]: Cannot find package '/home/node/workspace/agoric/agoric-sdk/packages/agoric-cli/node_modules/ws/' imported from /home/node/workspace/agoric/agoric-sdk/packages/agoric-cli/src/entrypoint.js
Did you mean to import ws/index.js?
    at new NodeError (node:internal/errors:371:5)
    at legacyMainResolve (node:internal/modules/esm/resolve:335:9)
    at packageResolve (node:internal/modules/esm/resolve:877:14)
    at moduleResolve (node:internal/modules/esm/resolve:929:18)
    at defaultResolve (node:internal/modules/esm/resolve:1044:11)
    at ESMLoader.resolve (node:internal/modules/esm/loader:422:30)
    at ESMLoader.getModuleJob (node:internal/modules/esm/loader:222:40)
    at ModuleWrap.<anonymous> (node:internal/modules/esm/module_job:76:40)
    at link (node:internal/modules/esm/module_job:75:36) {
  code: 'ERR_MODULE_NOT_FOUND'
}

After resetting the local SDK node_modules, I was able to switch back. I see that switching back to a local SDK doesn't switch the versions back from beta to * in package.json. I actually think we should move away from automatically patching package.json, so happy with that. We might still need such a mechanism to manually/explicitly update the @agoric deps to the latest, but right need it seems to require the @agoric packages to be linked and have a uniform version?

Also I'm happy to report that switching to an explicit dist-tag resulted in the same yarn.lock updates that what I ended up at by manually patching it to force an update when using a linked sdk! If we can fix the non linked cli issues, I believe this will let us stop relying on a linked sdk.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants