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

Rename Bazel @nodejs repo to @nodejs_host #1020

Closed
attilaolah opened this issue Apr 1, 2022 · 8 comments · Fixed by #1035
Closed

Rename Bazel @nodejs repo to @nodejs_host #1020

attilaolah opened this issue Apr 1, 2022 · 8 comments · Fixed by #1035

Comments

@attilaolah
Copy link
Contributor

Upstream bazel_nodejs rules have removed the @nodejs repo, now the repo is named @nodejs_host to access the files for the host architecture, or e.g. @nodejs_linux_amd64 to target specific architectures (this was the case earlier too).

Since the toolchain is typically run on the host, we should simply replace @nodejs// with @nodejs_host// in BUILD.bazel files. Note however that this is a breaking change, and will also require users to update rules_nodejs.

However, without this change, users cannot upgrade rules_nodejs without patching the Emscripten SDK.

@kripken
Copy link
Member

kripken commented Apr 4, 2022

cc @walkingeyerobot

@walkingeyerobot
Copy link
Collaborator

I attempted to make this change but I can't quite get it to a state where it's working. I wasn't able to actually get bazel to find the repo named @nodejs_host. Based on bazel-contrib/rules_nodejs#3375, it looks like this has just been proposed and not yet implemented. @attilaolah if you have more information, or even better if you want to put together a PR, I'd appreciate it!

@jfirebaugh
Copy link
Contributor

It seems like this has been implemented in rules_nodejs already. Or at least, current versions of rules_nodejs and emsdk do not work. Here's a minimal repro: https://github.com/jfirebaugh/bazel_emscripten_test. If you run bazel build ... in this workspace, you'll get:

ERROR: [...]/external/emsdk/emscripten_toolchain/BUILD.bazel:26:10: no such package '@nodejs//': The repository '@nodejs' could not be resolved: Repository '@nodejs' is not defined and referenced by '@emsdk//emscripten_toolchain:link-emscripten'

jfirebaugh added a commit to figma/emsdk that referenced this issue Apr 14, 2022
walkingeyerobot pushed a commit that referenced this issue Apr 14, 2022
@trungdinhth
Copy link

Hi, I'm also seeing the same issue, trying to used emscripten and nodejs rule in the same workspace.
If specifying node js rule before emscripten, I got error:

no such package '@nodejs//': The repository '@nodejs' could not be resolved: Repository '@nodejs' is not defined and referenced by '@emsdk//emscripten_toolchain:common_files'

If specifying emscripten rule before node js, I got error:

ERROR: error loading package '': cannot load '@build_bazel_rules_nodejs//:repositories.bzl': no such file

In waiting for a real fix, do you know any possible workaround or where to patch?

@attilaolah
Copy link
Contributor Author

I use this patch.

@rb-ditto
Copy link

rb-ditto commented Dec 5, 2022

this is still an issue. i am attempting to build a project with Bazel and ran into the no such package '@nodejs//' issue. I was able to run the build using a patch similar to what @attilaolah described above -- i think this issue should be re-opened since the linked PR was not a fix after all.

@alexeagle
Copy link

Sorry, I just forgot about bazel-contrib/rules_nodejs#3375 and didn't realize there's still suffering here. I'll fix that upstream.

jfirebaugh added a commit to figma/emsdk that referenced this issue Dec 16, 2022
@jfirebaugh
Copy link
Contributor

The upstream fix was released in rules_nodejs 5.8.0. So the solution for emsdk Bazel users is to update rules_nodejs to >= 5.8.0. I've opened #1150 to upgrade the internal dependency.

@sbc100 sbc100 closed this as completed in 5b80c10 Dec 20, 2022
lewing pushed a commit to dotnet/emsdk that referenced this issue Feb 7, 2023
* [bazel] Set CLOSURE_COMPILER to workaround RBE+symlinks issue (#1037)

* [bazel] Set CLOSURE_COMPILER to workaround RBE+symlinks issue

* space

* specify node_js

* 3.1.10 (#1046)

* Optimize sandbox performance (#1045)

* Optimize sandbox performance

Link just the files needed to compile, link, or archive, rather than the entire directory tree. This drastically improves build times on macOS, where the sandbox is known to be slow (bazelbuild/bazel#8230).

* Linux wants clang to link

* all_files not needed?

* Linux wants llc

* And llvm-ar

* Templated build_file_content

* include node modules glob with linker files. also some minor formatting fixes. (#1052)

* Using bazelisk on macOS CI (#1049)

* Explicit outputs for wasm_cc_binary (#1047)

* Explicit outputs for wasm_cc_binary

* Backwards compatibility

* data_runfiles restore

* restore test_bazel.sh

* Using wrong path on accident

* two separate rules for legacy support

* Added name attribute to wasm_cc_binary rule

* 3.1.11 (#1053)

* 3.1.12 (#1054)

* 3.1.13 (#1055)

* [bazel] Add additional files necessary for building with closure and on RBE (#1057)

* 3.1.14 (#1061)

* 3.1.15 (#1066)

* Pin `latest` to a specific version for arm64-linux (#1065)

Fixes: #1040

* 3.1.16 (emscripten-core#1071)

* 3.1.17 (emscripten-core#1076)

* Exclude msys from path fix function. (emscripten-core#1078)

Fixes: #911

* 3.1.18 (emscripten-core#1081)

* 3.1.18

* Update LLVM include path in Bazel files

* Version 3.1.18-2 (emscripten-core#1083)

3.1.18 had a bad release binary on ARM64 Mac so push an updated version of the release.

* 3.1.19 (emscripten-core#1090)

* Add EMSDK_QUIET to make emsdk_env less chatting (emscripten-core#1091)

Without this the recommended way to silence emsdk_env was to pipe its
stderr to /dev/null.. but then you also loose potentially useful error
message.

Fixes: #946

* 3.1.20 (emscripten-core#1095)

* Add double-quotes to allow spaces in path (emscripten-core#1097)

* 3.1.21 (emscripten-core#1101)

* Update latest-arm64-linux to 3.1.21 (emscripten-core#1102)

* Update XCode version on CircleCI (emscripten-core#1103)

12.2 is being deprecated

* 3.1.22 (emscripten-core#1107)

* 3.1.23 (emscripten-core#1111)

* Avoid exporting EM_CONFIG for modern SDK versions (emscripten-core#1110)

Newer versions of emscipten, starting all the way back in 1.39.13, can
automatically locate the `.emscripten` config file that emsdk creates so
there is no need for the explicit EM_CONFIG environment variable.  Its
redundant and adds unnessary noisce/complexity.

Really, adding emcc to the PATH is all the is needed these days.

One nice thing about this change is that it allows folks to run
whichever emcc they want to and have it just work, even if they have
configured emsdk.   Without this change, if I activate emsdk and I run
`some/other/emcc` then emsdk's `EM_CONFIG` will still be present and
override the configuration embedded in `some/other/emcc`.

e.g. in the same shell, with emsdk activated, I can run both these
commands and have them both just work as expected.

```
$ emcc --version
$ /path/to/my/emcc --version
```

* Use x64 version for Windows on Arm (emscripten-core#1115)

* 3.1.24 (emscripten-core#1122)

* 3.1.25 (emscripten-core#1130)

* [bazel] Switch to platforms-based toolchain resolution (#1036)

* remove "name" attribute from bazel rules (emscripten-core#1131)

* 3.1.26 (emscripten-core#1134)

* Update remote_docker version in CircleCI config (emscripten-core#1117)

20.10.17 is the current default.

* docker image: Change base to Ubuntu 22.04 LTS (jammy) (emscripten-core#1135)

Done to upgrade from CMake 3.16.3 to 3.22.1. CMake 3.21 or newer is needed to build the Qt 6.4.1 sources with emscripten.

Also update to libidn12 to resolve an "Unable to locate package libidn11" error.

* 3.1.27 (emscripten-core#1139)

* Use constants for fixed paths. NFC (emscripten-core#1140)

* Add standalone_wasm feature to bazel emscripten_toolchain (emscripten-core#1145)

* 3.1.28 (emscripten-core#1149)

* Upgrade to rules_nodejs 5.8.0 (emscripten-core#1150)

Fixes emscripten-core#1020

* 3.1.29 (emscripten-core#1160)

* Pin Windows CI to Bazel 5.4.0 (emscripten-core#1163)

* Remove reference to fastcomp-latest. NFC (emscripten-core#1164)

fastcomp can only be install using explicit versions names so this name
doesn't work.

* Remove fastcomp SDK and fastcomp build rules. NFC (emscripten-core#1165)

Folks that want to work with fastcomp will now need to use an older
checkout of emsdk.

* 3.1.30 (emscripten-core#1167)

* Bump emscripten to 3.1.30

* Bump clang version

* Do not include test directory

* Update eng/emsdk.proj

Co-authored-by: Alexander Köplinger <[email protected]>

---------

Co-authored-by: Kevin Lubick <[email protected]>
Co-authored-by: Sam Clegg <[email protected]>
Co-authored-by: John Firebaugh <[email protected]>
Co-authored-by: walkingeyerobot <[email protected]>
Co-authored-by: Ezekiel Warren <[email protected]>
Co-authored-by: Heejin Ahn <[email protected]>
Co-authored-by: Tim Ebbeke <[email protected]>
Co-authored-by: Derek Schuff <[email protected]>
Co-authored-by: Joel Van Eenwyk <[email protected]>
Co-authored-by: Pierrick Bouvier <[email protected]>
Co-authored-by: Trevor Hickey <[email protected]>
Co-authored-by: Fredrik Orderud <[email protected]>
Co-authored-by: Robbert van Ginkel <[email protected]>
Co-authored-by: Alexander Köplinger <[email protected]>
shlomif pushed a commit to shlomif/emsdk that referenced this issue Sep 29, 2023
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 a pull request may close this issue.

7 participants