search

microsoft / vscode-jupyter

VS Code Jupyter extension
https://marketplace.visualstudio.com/items?itemName=ms-toolsai.jupyter
MIT License
1.29k stars 292 forks source link

Onboard onto the latest zeromq-prebuilt changes #15970

Closed rzhao271 closed 1 month ago

rzhao271 commented 2 months ago

After https://github.com/microsoft/vscode-zeromq/issues/30 is completed, I would like the Jupyter extension to ship with only the latest @vscode/zeromq if possible in order for the extension to become compliant with internal Microsoft policies.

DonJayamanne commented 1 month ago

@rzhao271 Looks like the build outputs have changed. In the past we used to get node.napi.glibc.node, now the build generates zeromq.glibc.node Have a look at the contents in the artifacts of the zip files in the last two relases. The latest release ....16.11 is built from your changes, and the previous ...16.10 is the old stable build.

rzhao271 commented 1 month ago

I confirm that we renamed some of the output files while bumping to 16.11, ref PR microsoft/zeromq-prebuilt#32.

Adding Deepak to this issue.

@DonJayamanne @deepak1556 Do we want to keep the new file names (such as zeromq.glibc.node) or revert them (so they return to node.napi.glibc.node)?

DonJayamanne commented 1 month ago

Do we want to keep the new file names (such as zeromq.glibc.node) or revert them (so they return to node.napi.glibc.node)?

IMHO I think we will have to revert, as 3rd party packages (that we use) rely on the older names. Else it required changes upstream to packages like https://www.npmjs.com/package/@aminya/node-gyp-build & https://github.com/zeromq/zeromq.js/

Basically this name is imposed by the packages we depend on,

Here's the code where node-gyp looks for the node file name prefix https://github.com/aminya/node-gyp-build/blob/master/node-gyp-build.js#L132

DonJayamanne commented 1 month ago

@rzhao271 thanks for identifying the problem, didn't realize it was a change made a while ago.

deepak1556 commented 1 month ago

The change comes from the prebuildify version that upstream uses which ends up deciding the final name of the binaries, specifically https://github.com/prebuild/prebuildify/commit/7b6dcbd0860a82db8804079ba55663affd9ae555

I don't think this warrants a change in upstream, we can preserve the older names by having a rename action in vscode-zeromq when downloading the prebuilts. zeromq.* => node.napi.*

DonJayamanne commented 1 month ago

Got it, thanks for the details @deepak1556 Will make these changes during debt week.

rzhao271 commented 1 month ago

@DonJayamanne I published @vscode/zeromq@0.2.3, which preserves the older names. Could you try onboarding again?

DonJayamanne commented 1 month ago

Changes have been merged, smoke tests pass and tested locally as well.