search

mymindstorm / setup-emsdk

Setup Emscripten for use with GitHub actions
MIT License
94 stars 26 forks source link

Replace update-tags (which has no effect) with update #22

Closed kofigumbs closed 2 years ago

kofigumbs commented 3 years ago

I encountered the following error in my action whenever I set an explicit version:

/home/runner/work/_temp/8da50650-0692-4b2a-b2ce-6bb33e5d99ea/emsdk-master/emsdk update-tags
`update-tags` is not longer needed.  To install the latest tot release just run `install tot`
/home/runner/work/_temp/8da50650-0692-4b2a-b2ce-6bb33e5d99ea/emsdk-master/emsdk install 2.0.29
Error: No tool or SDK found by name '2.0.29'.
Error: The process '/home/runner/work/_temp/8da50650-0692-4b2a-b2ce-6bb33e5d99ea/emsdk-master/emsdk' failed with exit code 1
My workflow config 

    - uses: mymindstorm/setup-emsdk@v9
      with:
        version: '2.0.29'
        update-tags: true
        actions-cache-folder: 'emsdk-cache'

Changing emsdk update-tag to emsdk update fixed it for me, but I'm not 100% sure if that's the right solution. It does seem like update-tags is effectively gone though: https://github.com/emscripten-core/emsdk/pull/738.

If this switch to update is the way you want to go, I'd be happy to do the work to get this PR merge-ready. I only had to change the implementation to workaround the issue, but I imagine we'd at least want to update the docs too. One open question is whether changing the name in the action input would cause any compatibility concerns.

cschreib commented 2 years ago

I confirm this fixes the problem for me as well. Thanks! Using your fork in the meantime...

mymindstorm commented 2 years ago

I apologize for missing this, I've been very busy. It does seem that update is the correct way to go. I'll release a new version with updated docs. I'll have the old update-tags configuration act as a fallback if update is not defined for compatibility.

mymindstorm commented 2 years ago

I confirm this fixes the problem for me as well. Thanks! Using your fork in the meantime...

Please be aware that @kofigumbs 's fork has not updated dist/index.js, meaning that his changes are not actually being executed when using the action.

cschreib commented 2 years ago

Thanks for the quick turn around!

Please be aware that @kofigumbs 's fork has not updated dist/index.js, meaning that his changes are not actually being executed when using the action.

Oh, strange that it worked then. I'll switch back to your repo to be on the safe side, thank you for the warning.

mymindstorm commented 2 years ago

No problem. A tag for v11 has been created.