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

Minimallib NPM release fixes - wrong node bin command in Dockerfile and prepublish npm script command replacement #5349

Merged

Conversation

MichelML
Copy link
Contributor

@MichelML MichelML commented Jun 3, 2022

Reference Issue

ref #5346 (comment)

What does this implement/fix? Explain your changes.

Any other comments?

check context in issue ref #5346 (comment)

cc @greglandrum @ptosco


TLDR;

  1. the npm prepublish script which runs automatically when you run npm publish has been renamed to prepublishOnly in recent npm versions
  2. the node bin command that gets installed is now named node and not nodejs

@MichelML
Copy link
Contributor Author

MichelML commented Jun 3, 2022

Stacktrace regarding nodejs (using the nodejs command):

 => ERROR [build-stage 23/23] RUN nodejs tests.js                                                                                                      1.1s
------
 > [build-stage 23/23] RUN nodejs tests.js:
#26 0.589 Adding directories to PATH:
#26 0.589 PATH += /opt/emsdk
#26 0.589 PATH += /opt/emsdk/upstream/emscripten
#26 0.589 PATH += /opt/emsdk/node/14.18.2_64bit/bin
#26 0.589 
#26 0.589 Setting environment variables:
#26 0.589 PATH = /opt/emsdk:/opt/emsdk/upstream/emscripten:/opt/emsdk/node/14.18.2_64bit/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
#26 0.589 EMSDK = /opt/emsdk
#26 0.589 EM_CONFIG = /opt/emsdk/.emscripten
#26 0.589 EMSDK_NODE = /opt/emsdk/node/14.18.2_64bit/bin/node
#26 1.106 failed to asynchronously prepare wasm: CompileError: AsyncCompile: Wasm decoding failed: unexpected section: Code @+19456
#26 1.107 Aborted(CompileError: AsyncCompile: Wasm decoding failed: unexpected section: Code @+19456)
#26 1.110 /src/rdkit/Code/MinimalLib/demo/RDKit_minimal.js:9
#26 1.110 var Module=typeof initRDKitModule!="undefined"?initRDKitModule:{};var readyPromiseResolve,readyPromiseReject;Module["ready"]=new Promise(function(resolve,reject){readyPromiseResolve=resolve;readyPromiseReject=reject});var moduleOverrides=Object.assign({},Module);var arguments_=[];var thisProgram="./this.program";var quit_=(status,toThrow)=>{throw toThrow};var ENVIRONMENT_IS_WEB=typeof window=="object";var ENVIRONMENT_IS_WORKER=typeof importScripts=="function";var ENVIRONMENT_IS_NODE=typeof process=="object"&&typeof process.versions=="object"&&typeof process.versions.node=="string";var scriptDirectory="";function locateFile(path){if(Module["locateFile"]){return Module["locateFile"](path,scriptDirectory)}return scriptDirectory+path}var read_,readAsync,readBinary,setWindowTitle;function logExceptionOnExit(e){if(e instanceof ExitStatus)return;let toLog=e;err("exiting due to exception: "+toLog)}var fs;var nodePath;var requireNodeFS;if(ENVIRONMENT_IS_NODE){if(ENV
#26 1.110 
#26 1.110 RuntimeError: Aborted(CompileError: AsyncCompile: Wasm decoding failed: unexpected section: Code @+19456). Build with -sASSERTIONS for more info.
#26 1.110     at abort (/src/rdkit/Code/MinimalLib/demo/RDKit_minimal.js:9:10301)
#26 1.110     at /src/rdkit/Code/MinimalLib/demo/RDKit_minimal.js:9:12242
------
executor failed running [/bin/bash -c -l nodejs tests.js]: exit code: 7

and successful example with the node command:

 => [build-stage 23/23] RUN node tests.js                                                                                                              2.7s
 => [export-stage 1/2] COPY --from=build-stage /src/rdkit/Code/MinimalLib/demo /                                                                       0.0s
 => [export-stage 2/2] COPY --from=build-stage /src/rdkit/Code/MinimalLib/docs /                                                                       0.0s
 => exporting to client                                                                                                                                0.0s
 => => copying files 5.20MB                                                                                                                            0.0s
Build completed
MinimalLib distribution files are at Code/MinimalLib/dist

@MichelML
Copy link
Contributor Author

MichelML commented Jun 6, 2022

A bit out of scope: I took the time to build an Azure pipeline for semi-automated npm release of new rdkit versions

Available here: https://github.com/MichelML/rdkit-js/blob/master/azure-pipelines.yml
Proof that it works: https://dev.azure.com/michmoreaul/rdkit-js/_build/results?buildId=15&view=logs&j=12f1170f-54f2-53f3-20dd-22fc7dff55f9
Automated release: https://www.npmjs.com/package/@rdkit/rdkit-temporary-testing

I could port this to the main rdkit repo, or when I'm finally done with rdkit-js v1 (only writing the readme's is left), we'll trigger the npm releases from there.

Copy link
Member

@greglandrum greglandrum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@greglandrum greglandrum merged commit c7abf20 into rdkit:master Jun 6, 2022
@greglandrum greglandrum added Cleanup Code cleanup and refactoring infrastructure build infrastructure and the like labels Jun 6, 2022
@greglandrum greglandrum added this to the 2022_03_4 milestone Jun 6, 2022
@greglandrum
Copy link
Member

I could port this to the main rdkit repo, or when I'm finally done with rdkit-js v1 (only writing the readme's is left), we'll trigger the npm releases from there.

Very cool. Thanks for doing that!
I don't think this needs to be in the main repo (which is focused on the code itself and the CI for that) but I'm not opposed either. A middle ground, if you were interested, would be to transfer the rdkit-js repo to the RDKit organization; that way it's "close" to the main code base. Really it's your call.

greglandrum pushed a commit that referenced this pull request Jul 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Cleanup Code cleanup and refactoring infrastructure build infrastructure and the like
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants