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

Windows support #32

Closed
kmalakoff opened this issue Apr 25, 2020 · 9 comments
Closed

Windows support #32

kmalakoff opened this issue Apr 25, 2020 · 9 comments
Labels
backlog Identified as a backlog item, often combined with low-priority and help-wanted labels discuss ignore_sla windows

Comments

@kmalakoff
Copy link

Orb version

2.0.3

What happened

Version check fails on windows

Expected behavior

Script should work on all platforms including windows

This line fails on windows when trying to produce a cross-platform matrix script.

Here's my config (adapted from one of the CircleCI blogposts):

version: 2.1

executors:
  windows: win/default
  node: node/default

orbs:
  node: circleci/node@2.0.3
  win: circleci/windows@2.2.0

jobs:
  test:
    parameters:
      os:
        type: executor
      node-version:
        type: string
    executor: << parameters.os >>
    steps:
      - checkout
      - node/install:
          node-version: '<< parameters.node-version >>'
          install-yarn: false
          install-npm: false
      - run: npm install
      - run: npm test

workflows:
  all-tests:
    jobs:
      - test:
          matrix:
            parameters:
              os: [node, windows]
              node-version: ['0.10.48', '0.12.18', '6.17.1', '8.17.0', '10.20.1', 'current', 'latest']

Error messages:

Missing '(' after 'if' in if statement.
At line:13 char:7
+   if [[ latest == "" || latest == "latest" || latest == "current" ]]; ...
+       ~
Missing type name after '['.
At line:13 char:22
+   if [[ latest == "" || latest == "latest" || latest == "current" ]]; ...
+                      ~~

I'm not sure if I'm doing this wrong or if I have unrealistic expectations! Let me know!

@KyleTryon
Copy link
Contributor

Hello @kmalakoff,

This orb in its current state would not work on Windows, unless perhaps you were looking to install Node in the WSL portion of Windows.

This orb in its current form expects that it will be running in a terminal shell (not powershell), which allows it to work on MacOS and Linux. We could implement a windows specific node orb or Windows versions of the commands in this orb.

This is an incredibly cool and efficient config setup you have here however, I would love to get to the point of supporting this.

In this configuration, is not node attempting to install itself on the node docker image too?

Between Linux and Mac I think the configuration you have will work, I am not sure if it will be possible to get something As simple when windows is involved, we may at minimum need to separate those jobs with separate commands.

@KyleTryon
Copy link
Contributor

I believe the new advanced conditionals may allow us to bake in Windows support automatically now. Looking into this.

https://discuss.circleci.com/t/advanced-logic-in-config/36011

@kmalakoff
Copy link
Author

Thank you for the updates Kyle.

I spent some time going down the rabbit hole of running Node.js in a cross-platform way (nave, nvm, nvm-windows, etc) and decided to write cross-platform scripts directly in JavaScript given your base images come pre-installed with Node.js.

I released a couple of modules:

  1. install Node.js: https://github.com/kmalakoff/node-install-release.

$ nri [version string]

  1. run a specific version of Node.js: https://github.com/kmalakoff/node-version-use

$ nvu [version string] [command and arguments]

If you can assume Node.js is already installed in the base image, it might be easier to write in JavaScript.

@brandonros
Copy link

brandonros commented Nov 10, 2020

version: 2.1
orbs:
  win: circleci/windows@2.2.0
  node: circleci/node@4.1.0
jobs:
  build:
    executor:
      name: win/default
      shell: bash.exe
    working_directory: ~/repo/electron
    steps:
      - checkout
      - node/install
      - run:
          name: Update NPM
          command: "npm.exe install -g npm"
      - restore_cache:
          key: dependency-cache-{{ checksum "package.json" }}
      - run:
          name: Install Dependencies
          command: "npm.exe install"
      - save_cache:
          key: dependency-cache-{{ checksum "package.json" }}
          paths:
            - ./node_modules
      - run:
          name: Generate Builds
          command: "npm.exe run build:windows"
      - store_artifacts:
          path: ~/repo/out/make

so, this doesn't work? :(

Latest version of NPM is 6.14.8
Something went wrong; the specified version of NPM could not be installed

@Kurt-von-Laven
Copy link

I believe the new advanced conditionals may allow us to bake in Windows support automatically now. Looking into this.

https://discuss.circleci.com/t/advanced-logic-in-config/36011

I may be missing something here, but could one set a variable to the default shell option on Linux and MacOS, but bash.exe on Windows, and then slap shell: on every run statement? Lack of Windows support is a problem with almost all orbs (even the Slack orb) presently if I understand correctly though. Rather than support Windows in each orb one-by-one, I would suggest addressing this issue more structurally by, for instance, allowing the default shell to be changed or offering some other sort of cross-platform mode.

@duncdrum
Copy link

duncdrum commented Aug 18, 2021

this is particularly unfortunate since the example is actually part of the official documentation.
see circleci/circleci-docs#5611

@Jaryt
Copy link
Contributor

Jaryt commented Dec 13, 2021

Hi All! We're reevaluating this and are looking for some more specific use cases regarding using this orb with windows. I might expect the installation command to be replaced with something like chocolately.

As @Kurt-von-Laven said, it's more about a interoperability problem with orbs that we feel may be necessary to be focused on before implementing a patch solution for this orb specifically.

@Kurt-von-Laven
Copy link

We no longer use CircleCI, so I'm not sure how helpful my input is to you, but one reason I could think of might be to run a static analysis tool like cspell or jscpd that ships as an npm package. We use MegaLinter instead, but I know some projects may prefer a lighter-weight approach of installing only one or a few linters manually.

@Jaryt Jaryt added the backlog Identified as a backlog item, often combined with low-priority and help-wanted labels label Feb 23, 2022
@Jaryt
Copy link
Contributor

Jaryt commented Mar 17, 2022

Wanted to double check to ensure that this issue is still valid, and after some testing I found that Windows works perfectly fine with the Node orb as long as the executor's shell is set to bash :)

Closing out!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlog Identified as a backlog item, often combined with low-priority and help-wanted labels discuss ignore_sla windows
Projects
None yet
Development

No branches or pull requests

6 participants