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

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

Merged
merged 1 commit into from
Nov 21, 2021
Merged

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

merged 1 commit into from
Nov 21, 2021

Conversation

kofigumbs
Copy link
Contributor

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: emscripten-core/emsdk#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
Copy link

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

@mymindstorm
Copy link
Owner

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 mymindstorm merged commit c50714c into mymindstorm:master Nov 21, 2021
@mymindstorm
Copy link
Owner

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
Copy link

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
Copy link
Owner

No problem. A tag for v11 has been created.

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 this pull request may close these issues.

3 participants