ApeWorX / github-action

GitHub Actions CI File for Ape Projects
Apache License 2.0
6 stars 3 forks source link

fix: only default to `-U` if there is not a `version` in the config [APE-1180] #26

Closed antazoey closed 1 year ago

antazoey commented 1 year ago

(finishes) fixes: #16

Before, if you didn't specify any ape-plugins-list input, it would always try -U .. However this causes issues when the ape-config.yaml file contains version specifications. Thus, to fix, we detect if there are versions specified in the yaml. If there are, default to only .. Else, default to -U . as we did before.

So this is not a breaking change because it will still work as it did except in the times when it didn't work at all, now it will work then too, so it is just a bugfix

As a plus, I added tests . Look at the CI!

fubuloubu commented 1 year ago

Note: when this is released, update the major version tag and tag it as a new minor too

antazoey commented 1 year ago

Note: when this is released, update the major version tag and tag it as a new minor too

I was trying to avoid the breaking change because the default value will still be used in all cases where it worked. the only time it would not be used would be in cases where it wasn't working anyway, thus avoiding this breaking something for someone.

fubuloubu commented 1 year ago

Note: when this is released, update the major version tag and tag it as a new minor too

I was trying to avoid the breaking change because the default value will still be used in all cases where it worked. the only time it would not be used would be in cases where it wasn't working anyway, thus avoiding this breaking something for someone.

No I mean update the current v3 tag to point at the new commit

pcaversaccio commented 1 year ago

hey @antazoey @fubuloubu, I'm not 100% sure but I think this PR broke my CI pipeline: https://github.com/pcaversaccio/snekmate/actions/runs/6057836515/job/16439414869#step:6:1219

My ape-config is:

name: snekmate
contracts_folder: src
default_ecosystem: ethereum
plugins:
  - name: vyper
vyper:
  evm_version: shanghai

and the CI setup:

- name: Setup Ape
  uses: ApeWorX/github-action@v2
  with:
    python-version: ${{ matrix.python-version }}

- name: Check Ape compilation
  run: ape compile

Could you maybe have a look at this quickly since it's pretty important for snekmate as it's part of the test smart contracts pipeline and I need to push some updates that won't get tested currently due to this failure.

fubuloubu commented 1 year ago

I reverted the v2 tag before this merged PR since it seems to have an issue in the specific configuration that @pcaversaccio is experiencing

pcaversaccio commented 1 year ago

Thanks a lot @fubuloubu. The bash script seems to throw somehow:

Run if [[ "true"  == "true" ]]; then
  if [[ "true"  == "true" ]]; then
    version_present=$(sed -n '/^plugins:/,/^[^ ]/{/version:/p;}' "ape-config.yaml" | grep -c version)
  else
    version_present=0
  fi

  if [[ "${version_present}" == "1" ]] && [[ -z "" ]]; then
    plugins_value="."
  elif [[ -z "" ]]; then
    plugins_value="--upgrade ."
  else
    plugins_value=""
  fi

  ape plugins install "$plugins_value"
  shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
  env:
    pythonLocation: /opt/hostedtoolcache/Python/3.11.4/x64
    PKG_CONFIG_PATH: /opt/hostedtoolcache/Python/3.11.4/x64/lib/pkgconfig
    Python_ROOT_DIR: /opt/hostedtoolcache/Python/3.11.4/x64
    Python2_ROOT_DIR: /opt/hostedtoolcache/Python/3.11.4/x64
    Python3_ROOT_DIR: /opt/hostedtoolcache/Python/3.11.4/x64
    LD_LIBRARY_PATH: /opt/hostedtoolcache/Python/3.11.4/x64/lib
##[debug]/usr/bin/bash --noprofile --norc -e -o pipefail /home/runner/work/_temp/7b472a5e-f65c-4e38-9823-f9a532a9bbef.sh
Error: Process completed with exit code 1.
antazoey commented 1 year ago

It looks like a bug in "version_present" variable not interpolating and being compared as a literally. I'll fix after the weekend.

antazoey commented 1 year ago

im pretty positive this fixes all the issues: https://github.com/ApeWorX/github-action/pull/29 but if you are able to try it out, let me know!

pcaversaccio commented 1 year ago

FWIW, you can simply fork my repo and run the CI test-contracts to test it.

antazoey commented 1 year ago

FWIW, you can simply fork my repo and run the CI test-contracts to test it.

Ok, I did that here: https://github.com/antazoey/snekmate/actions/runs/6085791567/job/16510699465?pr=1 That run used the changes and the whole action passed. Thanks

pcaversaccio commented 1 year ago

Awesome, thanks for the heads-up!