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

core.getInput treats YAML booleans as strings #361

Closed
andrewdhazlett opened this issue Feb 24, 2020 · 6 comments
Closed

core.getInput treats YAML booleans as strings #361

andrewdhazlett opened this issue Feb 24, 2020 · 6 comments
Labels
bug Something isn't working

Comments

@andrewdhazlett
Copy link

Describe the bug
When I try to use a boolean value in my action.yaml file or workflow definition and retrieve it using core.getInput the result is a string.

To Reproduce
Steps to reproduce the behavior:

  1. Go to 'https://github.com/andrewdhazlett/gh-action-boolean-input-repro'
  2. Click on 'Actions > gh-action-boolean-input-repro > Bug repro'
  3. Scroll down to Run /./
  4. See error

Expected behavior
No error is thrown, the input is interpreted as a boolean

@ericsciple
Copy link
Contributor

getInput always returns a string by design. The inputs are passed as env var across process.

You can use a pattern like getInput('my-input').toUpper() === 'true' to convert to boolean.

Workflow uses yaml 1.2 so all the yaml 1.1 boolean values are intentionally not supported.

@andrewdhazlett
Copy link
Author

@ericsciple please also see my response on the PR I submitted.

I don't see why the implementation detail of passing env vars should be of concern to the consuming code. The values are defined in a format that supports non-string scalar types, so I expect those types to be available in the consuming code. Is there anywhere this design decision is documented?

@likern
Copy link

likern commented Aug 3, 2020

@ericsciple please also see my response on the PR I submitted.

I don't see why the implementation detail of passing env vars should be of concern to the consuming code. The values are defined in a format that supports non-string scalar types, so I expect those types to be available in the consuming code. Is there anywhere this design decision is documented?

The whole API is very-very badly written and designed. I would highly recommend to find out Typescript / Javascript expert in your team (I think @JasonEtco is that guy) to do that from scratch. Otherwise you have to spend years to fix up GitHub Actions on top of that errorneous foundation.

I started writing my own action and struggle with that decision every minute. The whole idea of GitHub Actions is awesome, but developing experience not so much.

I wanted to write action to setup React Native on top of @actions/setup-java action. And what I found out.

Actions are not composable

I can't call other action from my own action. React Native requires Java to be already installed.
In normal scenario user would expect to do something like this:

jobs:
  build:
    name: Build library
    runs-on: ubuntu-20.04
    steps:
      - name: Checkout repository
        uses: actions/checkout@v2

      - name: Setup Java environment
        uses: actions/setup-java@v1
        with:
          java-version: ${{ env.JAVA_VERSION }}

      - name: Setup React Native environment
        uses: actions/setup-react-native@v1
        with:
          version: 0.63.2

But it's not possible in actions/setup-react-native to know whether Java was installed or not (in safe and reliable way). I have to check whether actions/setup-java was called and if it's not call it by myself. Also I can't check what that actions/setup-java returned as output.

Either I have to add other input parameter java_installation_path so that users can provide JAVA_HOME to my action:

jobs:
  build:
    name: Build library
    runs-on: ubuntu-20.04
    steps:
      - name: Checkout repository
        uses: actions/checkout@v2

      - id: set-java
        name: Setup Java environment
        uses: actions/setup-java@v1
        with:
          java-version: ${{ env.JAVA_VERSION }}

      - name: Setup React Native environment
        uses: actions/setup-react-native@v1
        with:
          java_installation_path: ${{ steps.set-java.outputs.path }} // Provide manually

Or I have to guess about what actions/setup-java has changed in global state or know it's internal implementation - know that JAVA_HOME variable was set (or might be JAVA_8_HOME?)

Questions:

  • How can I check that before my actions/setup-react-native another actions/setup-java was called? (not possible)
  • How can I check with what input parameters actions/setup-java was called? (for Android SDK to install we need sdkmanager which doesn't work with Java >= 9, only Java 8 is supported) (not possible)

Action's API is not type-safe

Questions:

  • How can I differentiate between input parameter version was not set or was set to emtpy string? (not possible)
  • How to iterate over all input parameters? (to inform user that mutually exclusive parameters were provided ) (not possible)

Not using all advancements of Javascript ecosystem of 2020

Questions:

  • Why do you provide own (possible buggy and low-maintained) implementation of @actions/http-client instead of relying on axios / superagent?
  • Why don't you support publishing actions as npm packages?
  • Why don't you export important utility functions from actions (installing / extracting / patching java)?
  • Why action's code can't be used as function in other action?
  • Why at all do you provide your own implementations of common libraries (which are already written by Javascript community) instead of relying on them?
  • Why do we need to compile javascript using ncc instead of publishing packages?

You coud use these libraries:

You could use https://pnpm.js.org package manager which stores only one package for whole filesystem (and use could cache this directory, making package installations instant). Yes, using symlinks it installs only one version of package in ~/.pnpm-store and makes symlinks to these packages. This greatly reduces required space and improves installation speed.

Now actions are just a bunch of scripts exactly like bash, but using javascript. Every function mutates global state (for example in @actions/setup-java implementation), it's not possible to call them safely without side-effects. I can't reuse your code in @actions/setup-java because every call of every functon changes something in virtual machine (set up new environment variable / modifies PATH / do something with filesystem)

Law and License violations

According to MIT License (and Apache License has similar clause too) it's prohibited to provide / publish code without LICENSE / NOTICE file. It's a direct license violation.

Excerpt from MIT License:

The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software.

Whenever you (and actions's developers too) use any npm package (under MIT / Apache license) license of that package shall be provided into execution environment (if node_modules/ are used there is no problems - they all already contain that LICENSE and NOTICE file). But compiling all npm packages into one file with ncc utility you do not provide licenses for all of these dependencies.

State of the Art of Docker and GitHub Action

When it comes to current situation there is even problems when one action executed multiple times (due to global state modification) actions/setup-java#44. All these problems will grow as snpwball.

But at the same time you use advanced technologies (not utilizing full their power) like containers / layered filesystem https://docs.docker.com/storage/storagedriver

Solutions

Javascript Ecosystem:

  • Use standard npm packages for action distribution and it's dependencies. Do not compile them into one file.
  • Use pnpm for package dependencies installation
  • Provide for base Docker containers prebuild pnpm cache ~/.pnpm-store
  • Install into that cache most used dependencies

GitHub Actions:

  • Every step should be considered as absolutely isolated. Any changes into container should not be preserved (file downloading / environment modifications / file modifications)
  • Every step shoud create own filesystem layer in Docker https://docs.docker.com/storage/storagedriver/
  • Every step should have commit phase where every anounced changes (file or environment variables) of step can be inspected. And every change can be applied or rejected.
  • preserve: boolean should be provided. If preserve=false (default) running container is effectively stopped so every step starts from standard set of processes and step's processes (and other shared resources) do not intermix. If preserve=true container continues running on next step saving it's state (filesystem, env, process, other shared resouces). Rarely, but that might be required for very exceptional or complex cases.
  • Provide set of command for modification of step announced changes.

Real Use Cases

Le't look at real bug and try to solve them with new architecture.

actions/setup-java#44 action could look like this

jobs:
  build:
    name: Install multiple JDKs
    runs-on: ubuntu-20.04
    steps:
      - name: Setup Java 8
       // Setup JAVA_HOME and installs java into /usr/share/java
        uses: actions/setup-java@v1 
        with:
          java-version: 8
       // Allow to save modifications into filesystem
       commit: true
      - name: Setup Java 9
       // **Same action** setup JAVA_HOME and installs java into /usr/share/java
        uses: actions/setup-java@v1 
        with:
          java-version: 9
       // Inspect **announced** changes; add is a special command
       // set value on JAVA_HOME into JAVA_9_HOME
       // usual Javascript function call
       commit: setEnvironmentVariable('JAVA_9_HOME', after.step.env.changed['JAVA_HOME'])
       // rename and move installed and unziped Java package into another location
       // if we don't know changed paths we can always inspect them beforehand
       commit: moveToDirectory('/usr/share/java9', after.step.path.changed['/usr/share/java'])
       // append to PATH changes made in step
       commit: appenToPath(extractPathDifference(before.step.env, after.step.env.changed['PATH']))

All hevy lifting job was done by action actions/setup-java@v1 - it knows location from where to download bundle, downloaded them, extracted them, setup environment variable, put bundle at defauilt location.

What we did is just some tweaks to action - changed PATH, and renamed bundle location, put action's default JAVA_HOME under JAVA_8_HOME.

Only these changes will be applied. If action did anything else (saved downloaded file, downloaded another files, changed other environment variables) - all these changes will be lost.

@bryanmacfarlane
Copy link
Member

bryanmacfarlane commented Aug 3, 2020

We should focus this issue and scope discussions to the boolean question at hand. Much of the other work mentioned here like using packages to distribute, composing actions is on the services backlog and beyond the scope of just the toolkit.

Happy to discuss and / or route other issues raised as separate issues.

ink-pot-monkey added a commit to NewChromantics/PopAction_BuildApple that referenced this issue Aug 20, 2020
notlmn added a commit to fregante/release-with-changelog that referenced this issue Sep 11, 2020
GitHub Actions does not support boolean inputs in yaml files yet.
See: actions/toolkit#361
simoneb added a commit to fastify/github-action-merge-dependabot that referenced this issue Mar 4, 2021
all inputs are strings even though their yaml definition suggests that
they may of other types too. see actions/toolkit#361
@thboop
Copy link
Collaborator

thboop commented Apr 29, 2021

You can now retrieve boolean values via getBooleanInput 🚀
#725

This will be released shortly

@migounette
Copy link

getInput('my-input').toLower() === 'true' : )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants