Skip to content

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

Merged
mymindstorm merged 1 commit intoemscripten-core:masterfrom
kofigumbs:master
Nov 21, 2021
Merged

Replace update-tags (which has no effect) with update#22
mymindstorm merged 1 commit intoemscripten-core:masterfrom
kofigumbs:master

Conversation

@kofigumbs
Copy link
Copy Markdown
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
Copy Markdown

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

@mymindstorm
Copy link
Copy Markdown
Collaborator

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 emscripten-core:master Nov 21, 2021
@mymindstorm
Copy link
Copy Markdown
Collaborator

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

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
Copy Markdown
Collaborator

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