(bazel) Set @platforms//os:emscripten for platform_wasm

[trzeciak]

I noticed that @googlewalt added @platforms//os:emscripten LINK, now we can update the platform_wasm.

[trzeciak]

This change require use platforms in version 0.0.9+, in my own project, I set it in MODULE.bazel:

bazel_dep(name = "platforms", version = "0.0.9")

[walkingeyerobot]

Thanks very much!

[sbc100]

CI is reporting ERROR: /root/project/bazel/BUILD:81:9: no such target '@@platforms//os:emscripten': target 'emscripten' not declared in package 'os' defined by /root/.cache/bazel/_bazel_root/04d6701d4e894b3c38d7a891fc3b3ded/external/platforms/os/BUILD and referenced by '//:platform_wasm'

I guess something else needs updating?

[walkingeyerobot]

Likely a bazel version bump is needed somewhere...

[trzeciak]

If I understand the operation of bazel correctly, bazel has a built-in dependency for platforms (and several others), but it can be changed with an entry in the MODULE or WORKSPACE file.
I don't know when [email protected] will become built-in to bazel.
Therefore, I don't know what to do next with this MR so as not to spoil the work of others.
Or maybe it would be possible to add a dependency to a specific version of the platforms, and then it would propagate to the main project?

[walkingeyerobot]

Sadly I'm not entirely sure either. I suspect our bazel on CI is out of date. I think that's controlled approximately here: https://github.com/emscripten-core/emsdk/blob/main/.circleci/config.yml.

It looks like windows is using bazelisk but requesting version 5.4.0, which appears to be pretty old. It looks like mac is also using bazelisk but not requesting a version, so I'd sort of expect that one to work. And then linux... I'm not really sure what's going on with that.

What version of bazel are you running locally? @trzeciak

[fmeum]

Since essentially everything depends on platforms and the first definition in WORKSPACE wins, most WORKSPACE users won't be on a platforms version with this target. However, most users will know that the error they get can be solved by updating their dep on platforms - simply because this breaks all the time with WORKSPACE and even a Bazel update doesn't help if something else brings in an outdated version.

With Bzlmod, bumping the bazel_dep version is all you need.

All in all, I would say go for the update.

[sbc100]

I don't think there is a question as to whether we want to upgrade, but more of question of how to fix the CI tests which are failing. We can't land this until we fix those.

[fmeum]

Could you try adding

maybe(
    http_archive,
    name = "platforms",
    urls = [
        "https://mirror.bazel.build/github.com/bazelbuild/platforms/releases/download/0.0.9/platforms-0.0.9.tar.gz",
        "https://github.com/bazelbuild/platforms/releases/download/0.0.9/platforms-0.0.9.tar.gz",
    ],
    sha256 = "5eda539c841265031c2f82d8ae7a3a6490bd62176e0c038fc469eabf91f6149b",
)

to this macro?

[trzeciak]

Oooo, this looks very promising, I will try it locally first.

[trzeciak]

@fmeum Ok, I test it locally on macOS, and after add your snippet to deps macro, I tested it for few different version of bazel (using bazelisk):

# USE_BAZEL_VERSION=5.4.0  # OK
# USE_BAZEL_VERSION=6.4.0  # OK
# USE_BAZEL_VERSION=6.5.0  # OK
# USE_BAZEL_VERSION=7.0.0  # ERR
# USE_BAZEL_VERSION=7.0.2  # ERR
# USE_BAZEL_VERSION=7.1.1  # ERR

and it seems that not work for 7+ version.

I also followed the circleci logs as @walkingeyerobot mentioned, and now I see that on CI was used latest bazel version (7.1.1) on linux and macOS, and and for some reason 5.4.0 on windows.

Following this line of reasoning, I suppose I can add [email protected] dependency in a new MODULE file in this repo against to WORKSPACE (I will try this way).

By the way, it makes sense that this doesn't work in Bazel 7, because the maybe method is described as adding a dependency if it doesn't exist at all:

def maybe(repo_rule, name, **kwargs):
    """Utility function for only adding a repository if it's not already present.
    …

Going further, it might be nice to update emsdk to fully support bzlmod, but maybe not in this MR.

[fmeum]

Suggested change

	Bazel 7+ additionally requires `platforms` dependencies in the `MODULE.bazel` file.
	```starlark
	bazel_dep(name = "platforms", version = "0.0.9")
	```

This isn't needed: Bzlmod resolves transitive dependencies via Minimum Version Selection, so adding the platforms dep to the emsdk's MODULE.bazel file is sufficient for dependent modules to pick it up.

[trzeciak]

I understand that you are talking about Bazel 7+, but versions 5.4 and/or 6x do not check the MODULE file (by default), so a macro update is needed for older versions.

Should we be concerned about Bazel 5x/6x users? → I have no idea 🤷

[fmeum]

I see, this snippet is meant for users pulling in emsdk via WORKSPACE but who do have --enable_bzlmod set, e.g. because it is the default in Bazel 7+? That makes sense if you don't want to fully set up emsdk for Bzlmod.

If you do, then this wouldn't be necessary and such users could just load emsdk via a bazel_dep in MODULE.bazel instead. But that could be done in a separate PR.

[fmeum]

To properly Bzlmod-ify this so that other Bazel projects can depend on it, you also need to add something like this to the top:

module(
    name = "emsdk",
    version = "<current version>",
)

Instead of manually maintaining <current version>, you can also use the Publish to BCR app to automate the version bump and releases.

[trzeciak]

I hope I understand what you mean xD (I'm still new in bazel).
If it's not a problem, I'll try to do it in the next/separate MR.
Thanks for the tip!

[walkingeyerobot]

Thanks very much! Feel free to send further PRs my way.