-
Notifications
You must be signed in to change notification settings - Fork 206
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
9900 elide comments in bundles #9997
Changes from all commits
1439255
68d4fc0
2abea2d
2d410c7
9a9e4fb
d94b404
9e15cb4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,21 +18,38 @@ import { | |
prepareSingleton, | ||
} from '@agoric/vat-data'; | ||
|
||
/** | ||
* @import {VatAdminRootDeviceNode} from '../../devices/vat-admin/device-vat-admin.js'; | ||
* @import {DProxy} from'../plugin-manager.js'; | ||
* @import {Baggage} from '@agoric/vat-data'; | ||
*/ | ||
|
||
const managerTypes = ['local', 'node-subprocess', 'xsnap', 'xs-worker']; // xs-worker is alias | ||
|
||
function producePRR() { | ||
const { promise, resolve, reject } = makePromiseKit(); | ||
return /** @type {const} */ ([promise, { resolve, reject }]); | ||
} | ||
|
||
/** | ||
* Build root object of the bootstrap vat. | ||
* | ||
* @param {VatPowers & { | ||
* D: DProxy; | ||
* }} vatPowers | ||
* @param {never} _vatParameters | ||
* @param {Baggage} baggage | ||
*/ | ||
export function buildRootObject(vatPowers, _vatParameters, baggage) { | ||
const criticalVatKey = prepareSingleton(baggage, 'criticalVatKey', {}); | ||
|
||
const { D } = vatPowers; | ||
const pendingVatCreations = new Map(); // vatID -> { resolve, reject } for promise | ||
const pendingBundles = new Map(); // bundleID -> Promise<BundleCap> | ||
/** @type {Map<BundleID, PromiseKit<BundleCap>>} */ | ||
const pendingBundles = new Map(); | ||
const pendingUpgrades = new Map(); // upgradeID -> Promise<UpgradeResults> | ||
|
||
/** @type {import('../plugin-manager.js').Device<VatAdminRootDeviceNode>} */ | ||
let vatAdminDev; | ||
|
||
const runningVats = new Map(); // vatID -> [doneP, { resolve, reject }] | ||
|
@@ -200,7 +217,7 @@ export function buildRootObject(vatPowers, _vatParameters, baggage) { | |
console.log(`bundle ${bundleID} missing, hoping for reinstall`); | ||
return; | ||
} | ||
pendingBundles.get(bundleID).resolve(bundlecap); | ||
pendingBundles.get(bundleID)?.resolve(bundlecap); | ||
pendingBundles.delete(bundleID); | ||
checkForQuiescence(); | ||
} | ||
|
@@ -418,8 +435,9 @@ export function buildRootObject(vatPowers, _vatParameters, baggage) { | |
if (!pendingBundles.has(bundleID)) { | ||
pendingBundles.set(bundleID, makePromiseKit()); | ||
} | ||
return pendingBundles.get(bundleID).promise; | ||
return pendingBundles.get(bundleID)?.promise; | ||
Comment on lines
435
to
+438
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It’s a little weird to add a runtime check to appease the type system, when the runtime code above guarantees that the promise is defined. const bundlePromise = pendingBundles.get(bundleId);
if (bundlePromise !== undefined) {
return bundlePromise;
}
const bundleKit = makePromiseKit()
pendingBundles.set(bundleId, bundleKit);
return bundleKit.promise; There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree that's better code but are you also asserting it's a better change to make? I opted for the minimum characters changed. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤷♂️ |
||
}, | ||
/** @type {(bundleID: BundleID) => BundleCap} */ | ||
getBundleCap(bundleID) { | ||
const bundlecap = D(vatAdminDev).getBundleCap(bundleID); | ||
if (bundlecap) { | ||
|
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This edit was the only motivation for checking in
update-config.sh
in the first place. We should be able to delete this file and the remove the reference from config.yaml:ebeb975
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noting to make that clearer next time, maybe with a comment like "This file only exists to support this change. If we find this is not needed, please remove this entire file"