Skip to content

Add OS suffix support for all tool properties - #1761

Merged
sbc100 merged 1 commit into
mainfrom
generic-platform-attrs
Aug 4, 2026
Merged

Add OS suffix support for all tool properties#1761
sbc100 merged 1 commit into
mainfrom
generic-platform-attrs

Conversation

@sbc100

@sbc100 sbc100 commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

We had specific hardcoded support for OS-specific URLs. This change makes that generic and allows _windows / _linux / _mac suffixes to work with any key.

This change is really a no-op that makes way to using a windows specific activated_path for now (allowing us to use the upstream windows node archive, rather than re-packaging/hacking it in (scripts/update_node.py). See #1760.

Also, remove support for os: "all".. this is not used and doesn't seem useful (since you can just omit it).

@sbc100
sbc100 force-pushed the generic-platform-attrs branch 2 times, most recently from 36da296 to 3ea90f9 Compare August 3, 2026 05:57
@sbc100
sbc100 requested a review from dschuff August 3, 2026 05:57
@sbc100
sbc100 force-pushed the generic-platform-attrs branch 3 times, most recently from cbf2ac8 to 702725d Compare August 3, 2026 06:24
@sbc100
sbc100 requested a review from kripken August 3, 2026 16:20
@sbc100

sbc100 commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator Author

If you like I could split out the removal of the unix suffix?

@sbc100
sbc100 force-pushed the generic-platform-attrs branch from 702725d to 4bdf971 Compare August 3, 2026 17:01
@sbc100
sbc100 changed the base branch from main to remove_unix_suffix August 3, 2026 17:08
We had specific hardcoded support for OS-specific URLs.  This change makes that generic and allows `_windows` / `_linux` / `_mac` suffixes to work with any key.

This change is really a no-op that makes way to using a windows specific activated_path for now (allowing us to use the upstream windows node archive, rather than re-packaging/hacking it in (scripts/update_node.py).  See #1760.

Also:

- Remove support for `os: "all"`.. this is not used and doesn't seem useful (since you can just omit it).
- Remove support the `unix` urls.  This complexity (i.e. both unix and linux being valid suffixes) is not needed, since we can get the same behaviour by specifying `_windows` along with a generic url.
sbc100 added a commit that referenced this pull request Aug 4, 2026
Instead, we can just have `window_url` and the use the default
`url` as the fallback.

This simplification is part of the larger change to how we do
OS-specific properties.  See #1761
@sbc100
sbc100 force-pushed the remove_unix_suffix branch from e1bf31f to 99649d7 Compare August 4, 2026 00:52
@sbc100
sbc100 changed the base branch from remove_unix_suffix to main August 4, 2026 00:52
@sbc100
sbc100 force-pushed the generic-platform-attrs branch from 4bdf971 to f4ebea7 Compare August 4, 2026 00:53
@sbc100
sbc100 enabled auto-merge (squash) August 4, 2026 00:53
@sbc100
sbc100 merged commit 9fcdf59 into main Aug 4, 2026
9 checks passed
@sbc100
sbc100 deleted the generic-platform-attrs branch August 4, 2026 01:16
sbc100 added a commit that referenced this pull request Aug 4, 2026
Update the version of Node.js shipped with emsdk to the latest LTS
(v24.18.1).

One additional change here is that since #1761 we can now have a
different activated_path for windows to other platforms. This allows us
to use the upstream node archives without modifying the windows versions
(they ship node.exe at the top level and not under `bin/`).

Fixes: #1758
sbc100 added a commit that referenced this pull request Aug 4, 2026
Since #1761 we now have os-specific keys so we don't need separate
entries in these cases anymore.
sbc100 added a commit that referenced this pull request Aug 5, 2026
Since #1761 we now have os-specific keys so we don't need separate
entries in these cases anymore.
sbc100 added a commit that referenced this pull request Aug 5, 2026
Since #1761 we now have os-specific keys so we don't need separate
entries in these cases anymore.
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.

2 participants