Enable CI platforms to build Linux, Windows and OS X releases #63
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,6 +19,10 @@ workflows: | |
| - test-node-v6 | ||
| - test-node-v8 | ||
| - docker-build | ||
| - electron-pack-and-release: | ||
| requires: | ||
| - test-node-v6 | ||
| - test-node-v8 | ||
|
|
||
| version: 2 | ||
| jobs: | ||
|
|
@@ -29,7 +33,7 @@ jobs: | |
| - run: | ||
| name: List versions | ||
| command: | | ||
| echo "yarn: $(yarn --version)" | ||
|
There was a problem hiding this comment. I'd prefer sticking to There was a problem hiding this comment. ... though adding a There was a problem hiding this comment. @JamesCropcho BTW, There was a problem hiding this comment. Hello @etpinard, I switched to Yarn because Falcon uses Yarn. (You will notice this is a recurring pattern.) So, you would only want Yarn if there are clear upsides, but you also list one clear upside to Yarn. So, would you the program to use Yarn or NPM? I am happy to do it either way; let me know!
On the off chance it's not just a tiny oversight, However, I would be happy to amend the existing Dockerfile. There was a problem hiding this comment.
The upside being a package lock file? In that case, that's not an upside, npm has one now. Voting for |
||
| echo "node: $(node --version)" | ||
| - checkout | ||
|
There was a problem hiding this comment. In Falcon, we also keep a cache for There was a problem hiding this comment. Okay, @n-riesco, how about now?: https://circleci.com/gh/plotly/image-exporter/1175#config/containers/0 |
||
| - restore_cache: | ||
|
|
@@ -39,40 +43,39 @@ jobs: | |
| - run: | ||
| name: Install deps | ||
| command: | | ||
| yarn install | ||
| sudo apt-get install poppler-utils | ||
| - run: | ||
| name: List deps | ||
| command: | | ||
| yarn list | ||
| pdftops -v | ||
| - save_cache: | ||
| paths: | ||
| - node_modules | ||
| key: v{{ .Environment.CIRCLE_CACHE_VERSION }}-deps-{{ .Branch }}-{{ .Environment.CIRCLE_JOB }}-{{ checksum "package.json" }} | ||
|
There was a problem hiding this comment. If I'm not mistaken using |
||
| - run: | ||
| name: Test | ||
| command: yarn run test | ||
| - run: | ||
| name: Coverage | ||
| command: yarn run coverage | ||
| - store_artifacts: | ||
| path: build | ||
|
|
||
| test-node-v6: | ||
| <<: *base | ||
| docker: | ||
| - image: circleci/node:6-browsers | ||
|
|
||
| test-node-v8: | ||
| <<: *base | ||
| docker: | ||
| - image: circleci/node:8-browsers | ||
|
There was a problem hiding this comment. @etpinard My rationale, is that this software is made to run on many people's personal computers, with diverse versioning profiles, rather than servers for which Plotly has complete control to use only one very precise version. I am happy to revert this change if Plotly wishes me to do so, otherwise I vote to keep this change. There was a problem hiding this comment. I prefer locking down the image versions. I don't think this project is likely to break from one minor release of node.js to another. There was a problem hiding this comment. Okay, I have made this change. How's it look? |
||
|
|
||
| docker-build: | ||
| docker: | ||
| - image: circleci/node:8-browsers | ||
| steps: | ||
| - setup_remote_docker: | ||
| reusable: true | ||
|
|
@@ -90,10 +93,9 @@ jobs: | |
| docker run -d -p 9091:9091/tcp --name imageserver quay.io/plotly/image-exporter:$CIRCLE_SHA1 | ||
| docker run --network container:imageserver quay.io/plotly/wget wget --retry-connrefused --waitretry=1 -t 60 -O /dev/null --progress=dot http://localhost:9091/ping | ||
|
|
||
| docker-push: | ||
| docker: | ||
| - image: circleci/node:8-browsers | ||
|
|
||
| steps: | ||
| - setup_remote_docker: | ||
|
|
@@ -106,3 +108,17 @@ jobs: | |
| docker tag quay.io/plotly/image-exporter:$CIRCLE_SHA1 quay.io/plotly/image-exporter:$CIRCLE_BRANCH | ||
| docker push quay.io/plotly/image-exporter:$CIRCLE_SHA1 | ||
| docker push quay.io/plotly/image-exporter:$CIRCLE_BRANCH | ||
|
|
||
| electron-pack-and-release: | ||
| docker: | ||
| - image: circleci/node:8-browsers | ||
| steps: | ||
|
There was a problem hiding this comment. I'd add the same steps to save and restore caches for There was a problem hiding this comment. @n-riesco good call. Okay, I have added these caches: https://circleci.com/gh/plotly/image-exporter/1201#config/containers/0. I ran the builds twice after this change, to ensure it worked with and without a primed cache—all good. What do you think? There was a problem hiding this comment. @JamesCropcho In Falcon we use 2 caches: There was a problem hiding this comment. You are correct, @n-riesco . I have changed the CircleCI caching strategy as per your recommendations, and tested with fresh and primed caches. |
||
| - checkout | ||
| - run: | ||
| name: Install deps | ||
| command: yarn install | ||
| - run: | ||
| name: Electron-Builder pack | ||
| command: yarn run pack | ||
| - store_artifacts: | ||
| path: release | ||
|
There was a problem hiding this comment. We don't really want to include There was a problem hiding this comment. Okay @n-riesco, I believe you'll like it now: https://circleci.com/gh/plotly/image-exporter/1172#artifacts/containers/0. What do you think? |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,6 +5,8 @@ coverage | |
| .nyc_output | ||
|
|
||
| /build | ||
| /release | ||
|
|
||
| deployment/*.retry | ||
|
|
||
| /yarn-error.log | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| matrix: | ||
| include: | ||
| - os: osx | ||
| osx_image: xcode9.0 | ||
| language: node_js | ||
| node_js: "8" | ||
| env: | ||
| - ELECTRON_CACHE=$HOME/.cache/electron | ||
| - ELECTRON_BUILDER_CACHE=$HOME/.cache/electron-builder | ||
|
|
||
| cache: | ||
| directories: | ||
| - node_modules | ||
| - $HOME/.cache/electron | ||
| - $HOME/.cache/electron-builder | ||
| yarn: true | ||
|
|
||
| install: | ||
| - yarn install | ||
|
|
||
| script: | ||
| - yarn run pack | ||
| - zip mac-release.zip release/Plotly\ Graph\ Exporter* | ||
|
|
||
| before_cache: | ||
| - rm -rf $HOME/.cache/electron-builder/wine | ||
|
|
||
| addons: | ||
| artifacts: | ||
| s3_region: us-east-1 | ||
| paths: mac-release.zip | ||
| debug: true |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| version: 0.1.{build} | ||
|
|
||
| environment: | ||
| matrix: | ||
| - nodejs_version: 8 | ||
|
|
||
| platform: | ||
| - x64 | ||
|
|
||
| #services: | ||
|
|
||
| cache: | ||
| - '%USERPROFILE%\.electron' | ||
| - C:\Users\appveyor\AppData\Local\electron-builder\cache | ||
| - C:\projects\image-exporter-vm18n\node_modules | ||
|
There was a problem hiding this comment. @JamesCropcho I don't know why these paths are listed in the cache. Have you found any documentation about this? I've noticed There was a problem hiding this comment. @n-riesco I am unsure why I chose those paths, however I feel most confident copying from Electron-Builder, so I have replaced my cache entries with theirs. AppVeyor has not been running builds for the past fifteen minutes, so I am unable to verify a lack of errors at present. |
||
|
|
||
| init: | ||
| - git config --global core.autocrlf input | ||
|
|
||
| install: | ||
| - ps: Install-Product node $env:nodejs_version $env:platform | ||
| - yarn install | ||
|
|
||
| build_script: | ||
| - yarn run pack | ||
|
|
||
| after_build: | ||
| - 7z a release.zip release/latest.yml release/*.exe | ||
|
|
||
| artifacts: | ||
| - path: release.zip | ||
| name: release | ||
|
|
||
| test: off | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,27 +13,43 @@ | |
| "test:lint": "standard | snazzy", | ||
| "test:unit": "tap test/unit/*_test.js", | ||
| "test:integration": "tap test/integration/*_test.js", | ||
| "test": "yarn run test:lint && yarn run test:unit && yarn run test:integration", | ||
| "coverage": "yarn run test:unit -- --cov", | ||
| "lint": "standard --fix", | ||
| "pack": "cross-env NODE_ENV=production electron-builder --publish=never" | ||
|
There was a problem hiding this comment. Thanks! It certainly seems to, both locally and across all three CI environments. |
||
| }, | ||
| "build": { | ||
| "appId": "com.plotly.image-exporter.graph-exporter", | ||
| "productName": "Plotly Graph Exporter", | ||
| "files": [ | ||
| "bin", | ||
| "src" | ||
| ], | ||
| "appImage": { | ||
| "systemIntegration": "doNotAsk" | ||
| }, | ||
| "linux": { | ||
| "category": "Utility", | ||
| "executableName": "plotly-graph-exporter", | ||
| "maintainer": "enquiries@nicolasriesco.net", | ||
|
There was a problem hiding this comment. @etpinard I guess that here should go your email. There was a problem hiding this comment. @n-riesco Please excuse this error or mine! I am sure I copied this from Falcon. @etpinard @jackparmer @chriddyp Would you like me to put one of my email address in here? I suspect you'd prefer something which looks like There was a problem hiding this comment. you can put There was a problem hiding this comment. I have done so. |
||
| "target": [ | ||
|
There was a problem hiding this comment. @JamesCropcho I guess you want to add There was a problem hiding this comment. @n-riesco I'm probably missing something silly here, but isn't There was a problem hiding this comment. @JamesCropcho https://github.com/plotly/image-exporter/blob/cc257b7cc36b8d3c4353c177efb1d4c8cc34fd2b/package.json#L28-L30 only configures the builder. We still need to include There was a problem hiding this comment. I have done this. |
||
| "tar.gz" | ||
| ] | ||
| }, | ||
| "win": { | ||
| "target": [ | ||
| "nsis" | ||
| ] | ||
| }, | ||
| "mac": { | ||
| "category": "public.app-category.tools", | ||
| "target": [ | ||
| "dmg" | ||
| ] | ||
| }, | ||
| "directories": { | ||
| "output": "release" | ||
| } | ||
| }, | ||
| "author": "Plotly, Inc.", | ||
| "keywords": [ | ||
|
|
@@ -44,6 +60,7 @@ | |
| ], | ||
| "dependencies": { | ||
| "body": "^5.1.0", | ||
| "cross-env": "^5.1.3", | ||
|
There was a problem hiding this comment. If I'm not mistaken, There was a problem hiding this comment. @n-riesco I am inclined to keep cross-env in The thing is, this software is going to be executed by end-users in the command line (if only as a subprocess in a Pip package, or maybe more directly) on Windows and OS X environments. I worry about consistency across all environments with regards to environment variables. @n-riesco et al, what do you think? There was a problem hiding this comment. @JamesCropcho That's not my understanding. We want to distribute If we include There was a problem hiding this comment. I have moved cross-env to |
||
| "fast-isnumeric": "^1.1.1", | ||
| "file-type": "^7.2.0", | ||
| "get-stdin": "^5.0.1", | ||
|
|
@@ -63,6 +80,7 @@ | |
| "devDependencies": { | ||
| "devtron": "^1.4.0", | ||
| "electron": "^1.7.9", | ||
| "electron-builder": "^20.2.0", | ||
| "electron-debug": "^1.4.0", | ||
| "image-size": "^0.6.1", | ||
| "sinon": "^3.2.0", | ||
|
|
||
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 job didn't run:
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.
@n-riesco It appears to me that "electron-pack-and-release" is running: https://circleci.com/gh/plotly/image-exporter/tree/test-builds-across-travis-circle-appveyor
(See #1124 for example.)
Please pardon, am I missing something simple?
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.
@JamesCropcho For some reason, github doesn't show the build (as seen on the above screenshot). Since circleCI runs the job, then this isn't an issue.