catalyst / catalyst-moodle-workflows

4 stars 7 forks source link

Failing to deploy to Moodle Plugin Directory #116

Open djarran opened 1 week ago

djarran commented 1 week ago

Description: We are experiencing the following error in the GitHub Action for mod_scormremote.

Warning: Unable to locate the previous sha: 37eed5f223c5d48a8a34566aef4faa4a254a212e
Warning: You seem to be missing 'fetch-depth: 0' or 'fetch-depth: 2'. See https://github.com/tj-actions/changed-files#usage
Error: Process completed with exit code 1.

The issue appears to be occurring due to this step/line of CI.yml here

- name: Check out plugin code
      uses: actions/checkout@v3
      with:
        # Needed for 'changed-files' actions (alternatively could be a fixed
        # large number but may cause issues if limited).
---->   fetch-depth: ${{ ((contains(github.event_name, 'push') && steps.check-branch.outputs.publishable == 'true')) && 0 || 2 }}
        path: plugin
        submodules: true

Currently, the fetch-depth conditional is not being evaluated correctly. It uses a ternary operator and so if both contains(github.event_name, 'push') and steps.check-branch.outputs.publishable == 'true' are true, it should return 0. However, it is returning 2 as seen in the debug logs below:

##[debug]Evaluating: ((contains(github.event_name, 'push') && (steps.check-branch.outputs.publishable == 'true') && 0) || 2)
##[debug]Evaluating Or:
##[debug]..Evaluating And:
##[debug]....Evaluating contains:
##[debug]......Evaluating Index:
##[debug]........Evaluating github:
##[debug]........=> Object
##[debug]........Evaluating String:
##[debug]........=> 'event_name'
##[debug]......=> 'push'
##[debug]......Evaluating String:
##[debug]......=> 'push'
##[debug]....=> true
##[debug]....Evaluating Equal:
##[debug]......Evaluating Index:
##[debug]........Evaluating Index:
##[debug]..........Evaluating Index:
##[debug]............Evaluating steps:
##[debug]............=> Object
##[debug]............Evaluating String:
##[debug]............=> 'check-branch'
##[debug]..........=> Object
##[debug]..........Evaluating String:
##[debug]..........=> 'outputs'
##[debug]........=> Object
##[debug]........Evaluating String:
##[debug]........=> 'publishable'
##[debug]......=> 'true'
##[debug]......Evaluating String:
##[debug]......=> 'true'
##[debug]....=> true
##[debug]....Evaluating Number:
##[debug]....=> 0
##[debug]..=> 0
##[debug]..Evaluating Number:
##[debug]..=> 2
##[debug]=> 2
##[debug]Expanded: ((contains('push', 'push') && ('true' == 'true') && 0) || 2)
##[debug]Result: 2

It is altering the conditional by moving the closing brace to surround the 0 as well so that the only possible result to return is 2. This means that this step is only fetching the latest two commits. In the case of mod_scormremote, fetching only the last two commits is not enough to find previous SHA 37eed5f image

Note that setting fetch-depth to 0 is recommended here

djarran commented 1 week ago

We attempted to fix this issue with PR #117 (contains explanation of changes). This was tested with nekos/act to run the Action locally to ensure that the ternary was evaluated correctly. It seems like nekos/act is not 100% representation of Github Action execution as it was still not evaluating correctly.

The latest change as part of #118 uses a fixed number for fetch-depth