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

Conditional operator or function for expression syntax #409

Closed
StephenCleary opened this issue Apr 6, 2020 · 26 comments
Closed

Conditional operator or function for expression syntax #409

StephenCleary opened this issue Apr 6, 2020 · 26 comments

Comments

@StephenCleary
Copy link

Describe the enhancement

I'd like some kind of conditional operation added to expression syntax. This can be an actual ternary operator (? :) or a built-in function (e.g., if(<condition>, <true-value>, <false-value>)).

Additional information

There is a workaround: using shell scripts to evaluate the condition, setting step outputs, and having other steps reference those outputs. But it would be much cleaner to have some kind of ternary / if/then/else / conditional branching built in to the expression syntax.

@StephenCleary StephenCleary added the enhancement New feature or request label Apr 6, 2020
@StephenCleary StephenCleary changed the title Conditional operator or function Conditional operator or function for expression syntax Apr 6, 2020
@TingluoHuang TingluoHuang added runner Actions Feature Feature requires both runner, pipelines service and launch changes and removed enhancement New feature or request runner labels Jun 8, 2020
@ylemkimon
Copy link

Another alternative syntax could be:

${{ if(condition) }} evaluateIfTrue ${{ else }} evaluateIfFalse ${{ endif }}

like CircleCI.

@ylemkimon
Copy link

ylemkimon commented Nov 15, 2020

It seems, although not identical, <condition> && <true-value> || <false-value> works in most cases.

@molszanski
Copy link

molszanski commented Dec 30, 2020

Syntax: ${{ x && 'ifTrue' || 'ifFalse' }}

fake ternary example

    steps:
      - name: stuff
        env:
          PR_NUMBER_OR_MASTER: ${{ github.event.number == 0 && 'master' ||  format('pr-{0}', github.event.number)  }}

on condition is that your first <true-value> must be true for this to work

ageorgou added a commit to Trovemaster/TROVE that referenced this issue Jun 17, 2021
I don't like this but it seems there's no way to conditionally
set the environment variable. See also
actions/runner#409
Will possibly change to conditional steps later.
ageorgou added a commit to Trovemaster/TROVE that referenced this issue Jun 29, 2021
* Start running with Intel compiler

* Use correct compiler command

I don't like this but it seems there's no way to conditionally
set the environment variable. See also
actions/runner#409
Will possibly change to conditional steps later.

* Remember Intel-related parameters

The scripts set the PATH (and maybe other variables?) but those are
not preserved across steps. Since the build step is becoming more
distinct between the two compilers, let's try splitting it in two
conditional ones.

* Try caching Intel installation

* Look for Intel libraries before tests

* Try to cache whole directory

* Allow running tests with MPI

* Fix syntax

* Install MPI compilers when needed

* Try to force use of ifort with MPI

By default, the Intel version of mpif90 uses gfortran. We can
either switch to using mpiifort instead, or try to control the
underlying compiler this way. See also, for example:
https://www.hpc.cineca.it/center_news/important-use-intel-mpi-wrappers-mpif90-mpicc-mpicxx

* Make sure cache names don't clash with/without MPI

* Update variables so MKL and BLACS can be found

* Fix MKL installation

* Show some more info and build faster on CI

* Only use one MPI process on CI for debugging

See if the error still happens when testing.

* Avoid reading from stdin

* Build faster on CI with gfortran

* Avoid segmentation fault with quick gfortran build

This reverts commit b8413c6.

* Replicate execution on cluster temporarily

Adding the mpiio option may be required for now but will eventually
be removed.

* Don't test file_intensity with MPI

It's currently failing when run with MPI and > 1 processes
(not just on GitHub Actions, also on CSD3).

* Simplify USE_MPI in Makefile and CI

Now behaving similar to the other Makefile variables. This also
lets us make the GitHub Actions job a bit simpler.

* Don't install MKL twice when using MPI, clean up
@TWiStErRob
Copy link

Example use case https://github.com/TWiStErRob/net.twisterrob.gradle/pull/136/files
Related StackOverflow: https://stackoverflow.com/a/68422069/253468

@LaPeste
Copy link

LaPeste commented Aug 26, 2021

The fact that in a year no one said a word about this, means that the team isn't interested in following up on this one?

@nmattia
Copy link

nmattia commented Jan 14, 2022

A note about the workaround ${{ x && <yes> || <false> }}:

This only works if <yes> isn't the empty string. If <yes> == '' then '' is considered as false, which then evaluates the right hand side of the ||. Just lost ten minutes over this, hope it helps!

@islishude
Copy link

Seems that there is already an approach

https://docs.github.com/en/actions/learn-github-actions/expressions#example

env:
  MY_ENV_VAR: ${{ github.ref == 'refs/heads/main' && 'value_for_main_branch' || 'value_for_other_branches' }}

I think this issue can be closed.

@mayank99
Copy link

mayank99 commented Aug 31, 2023

@islishude you can't shouldn't just comment on an issue that hundreds of people have subscribed to, just to say "this can be closed" when your suggested approach (and its limitation) has already been mentioned. see #409 (comment)

@islishude
Copy link

Interesting, you saying I can't comment, is the comment illegal?

you can't just comment on an issue that hundreds of people have subscribed to

@TWiStErRob
Copy link

TWiStErRob commented Aug 31, 2023

It's good to see GitHub embracing the workaround as a stop-gap, they even mention its limitation in the docs @islishude linked.

On one hand it's good they added it, because at least now it's easier to discover, but on the other hand I do hope it doesn't mean they consider ternary "supported".

It would be great to get some official response from GitHub staff. 🤔

@igor-pinchuk
Copy link

It's ridiculous that we do not have ternary operator and this issue is 3+ y.o.
Github actions is so silly engineered, will avoid it for any future projects by all means.

@ameknite
Copy link

Correct answer for booleans
${{ (condition && 'ifTrue') || (!condition && 'ifFalse') }}

@socketbox
Copy link

socketbox commented Oct 5, 2023

It's good to see GitHub embracing the workaround as a stop-gap, they even mention its limitation in the docs @islishude linked.

The docs state, "In this example, we're using a ternary operator to set the value of the MY_ENV_VAR environment variable," which is laughable, because there is no ternary operator, just a logical kludge that relies on short-circuit boolean evaluation:

env:
  MY_ENV_VAR: ${{ github.ref == 'refs/heads/main' && 'value_for_main_branch' || 'value_for_other_branches' }}

Great exposition on just why this is a poor solution: https://7tonshark.com/posts/github-actions-ternary-operator/

Copy link
Contributor

Thank you for your interest in the runner application and taking the time to provide your valuable feedback. We kindly ask you to redirect this feedback to the GitHub Community Support Forum which our team actively monitors and would be a better place to start a discussion for new feature requests in GitHub Actions. For more information on this policy please read our contribution guidelines. 😃

@patrickelectric
Copy link

Looks more as unsolved than "completed"

@enumag
Copy link

enumag commented Nov 18, 2023

Couldn't find an existing thread there so I created one: https://github.com/orgs/community/discussions/75928

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests