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

Add multi-string support for variable values #5

Closed
air3ijai opened this issue Feb 5, 2024 · 5 comments · Fixed by #6
Closed

Add multi-string support for variable values #5

air3ijai opened this issue Feb 5, 2024 · 5 comments · Fixed by #6

Comments

@air3ijai
Copy link

air3ijai commented Feb 5, 2024

We are trying to check how to use that action with include and it fails with our case (shell: bash -e), which is expected as described in the documentation

There is currently no way to pass multiline strings or strings containing colons and/or commas as variable names or values. If you need to have such strings please open an issue.

name: Matrix
on:
  push:
  workflow_dispatch:

jobs:
  # Setup matrix
  setup-matrix:
    runs-on: ubuntu-latest
    outputs:
      matrix: ${{ steps.setup-matrix.outputs.matrix }}
    steps:
      - id: setup-matrix
        uses: druzsan/setup-matrix@v1
        with:
          include: |
            os: linux cpu: amd64 run-on: ubuntu-latest shell: bash -e,
            os: macos cpu: amd64 run-on: macos-latest shell: bash -e,
            os: windows cpu: amd64 run-on: windows-latest shell: msys2

  # Setup python and print version
  build:
    needs: setup-matrix
    strategy:
      matrix: ${{ fromJson(needs.setup-matrix.outputs.matrix) }}
    runs-on: ${{ matrix.run-on }}
    name: '${{ matrix.os }}-${{ matrix.cpu }}-${{ matrix.shell }}'
    steps:
      - name: Checkout sources
        uses: actions/checkout@v4
@druzsan
Copy link
Owner

druzsan commented Feb 5, 2024

Hi @air3ijai, I can confirm this feature is not working at the moment and I couldn't bypass it myself.

Furthermore, it works with plain GitHub strategy matrices, so it seems reasonable for me to implement.

So I am starting to implement this feature.

@druzsan
Copy link
Owner

druzsan commented Feb 7, 2024

Hi @air3ijai

Update: I rewrote the action with Python and it seems to work for virtually any case (special characters/spaces/multilines in both variable name and values) now.

Here is your workflow using the new version and its corresponding run.

Breaking Changes: since had to parse many edge cases I stepped back from my "own" format to classic YAML as a string. Flow and non-flow YAML syntaxes are both supported. Basically, if you had

strategy:
  matrix:
    variable1: [value1, value2]
    variable2: [value1, value2]
    include:
      - variable1: value3
      - variable2: value3 variable3: value1
    exclude:
      - variable1: value1 variable2: value2

now just copy-paste it to the actions inputs:

steps:
  - uses: druzsan/setup-matrix@feature/use-python-dockerfile
    with:
      matrix: |
        variable1: [value1, value2]
        variable2: [value1, value2]
      include: |
        - variable1: value3
        - variable2: value3 variable3: value1
      exclude: |
        - variable1: value1 variable2: value2

WIP: the feature is not released yet, I need some more time to test it out. So you can use the branch specified (uses: druzsan/setup-matrix@feature/use-python-dockerfile) to test it yourself if you wish, but do not fully rely on the unreleased state.

If you have some feedback, feel free to share it.

@air3ijai
Copy link
Author

air3ijai commented Feb 7, 2024

Hi, @druzsan - just did a check and it works as expected for our case and we can pass matrix to reusable workflow.

Thank you for the implementation!

Basically, now we have a copy paste from the existing matrix syntax

Screenshot 2024-02-07 at 13 44 31

@druzsan
Copy link
Owner

druzsan commented Feb 8, 2024

Hi @air3ijai,

I am happy it works for you!

My thoughts were that since there is now no difference in syntax between strategy matrix and usage of action (up to that the latter is a string) it is better to let just one argument matrix and let people copy-paste as you have figured it out yourself.

I am currently preparing some testing and will release a new version soon.

@druzsan
Copy link
Owner

druzsan commented Feb 9, 2024

Hi @air3ijai,

the new version is released, since now you can use action as druzsan/setup-matrix@v2.

@druzsan druzsan closed this as completed Feb 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants