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

Add nodejs toolchain #222

Merged
merged 7 commits into from
Oct 6, 2022
Merged

Add nodejs toolchain #222

merged 7 commits into from
Oct 6, 2022

Conversation

betaboon
Copy link
Contributor

No description provided.

@betaboon
Copy link
Contributor Author

I'm still working on this.

currently i'm stuck at it running into mismatching values: support_nix.
i bet there is just a minor thing missing somewhere. maybe someone has a clue.
i would be greatful for any hints ;)

@betaboon
Copy link
Contributor Author

thanks to @chancila i figured out that only a setting in .bazelrc was missing:
build:nix --extra_execution_platforms=@io_tweag_rules_nixpkgs//nixpkgs/platforms:host

@avdv
Copy link
Member

avdv commented May 2, 2022

Hi @betaboon , thank you for your contribution!

Are you still working on it or do you need some assistance?

@betaboon
Copy link
Contributor Author

betaboon commented May 2, 2022

@avdv I'm still working on this. planning to get the documentation ready in the next couple days.

i could need some assistance on the issue of the required --extra_execution_platforms, if that is something that we would want.

if there are any other pointers to get this in a ready state I'd gladly integrate them

@avdv avdv force-pushed the node-configure branch from 786c80d to 6d4f3e2 Compare June 1, 2022 13:38
@avdv
Copy link
Member

avdv commented Jun 1, 2022

Hi @betaboon , sorry for the long delay.

I had a look at this again. Also, I rebased onto master and added a missing dependency.

An example for the toolchain would be nice to have.

Thank you!

@betaboon betaboon force-pushed the node-configure branch 3 times, most recently from 070f566 to 2f0be41 Compare August 8, 2022 14:40
@betaboon betaboon marked this pull request as ready for review August 8, 2022 14:40
@betaboon
Copy link
Contributor Author

betaboon commented Aug 8, 2022

@avdv sorry for the long silence.

i just rebased against master, included your changes, updated the nodejs and nixpkgs rules, added an example.

i think this is now fine and ready :)

@avdv
Copy link
Member

avdv commented Sep 16, 2022

Oh, I missed this update over the course of my holidays... I'm taking a look now.

@betaboon
Copy link
Contributor Author

i just rebased against master

toolchains/nodejs/tests/BUILD.bazel Outdated Show resolved Hide resolved
core/.bazelrc Outdated
@@ -1,4 +1,5 @@
build --host_platform=@rules_nixpkgs_core//platforms:host
build --extra_execution_platforms=@io_tweag_rules_nixpkgs//nixpkgs/platforms:host
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain why this is necessary? I commented it out and everything seemed to be fine.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, after reverting the commit, CI was all green: https://github.com/tweag/rules_nixpkgs/actions/runs/3095774582

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

iirc back then it failed to pick up the toolchain. i had some longer discussion in slack with someone that lead me to find this was required. maybe things changed since then ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, so let's skip this setting for now, we can add it back later if we see it's required.

avdv added 5 commits October 6, 2022 11:05
```
ERROR: /private/var/tmp/_bazel_runner/a4a3182e50c9460fa52b8c6c1e6f916f/sandbox/processwrapper-sandbox/9/execroot/io_tweag_rules_nixpkgs/bazel-out/darwin-fastbuild/bin/tests/run-test-invalid-nixpkgs-package.runfiles/io_tweag_rules_nixpkgs/WORKSPACE:20:27: fetching local_repository rule //external:rules_nixpkgs_nodejs: java.io.IOException: The repository's path is "./toolchains/nodejs" (absolute: "/private/var/tmp/_bazel_runner/a4a3182e50c9460fa52b8c6c1e6f916f/sandbox/processwrapper-sandbox/9/execroot/io_tweag_rules_nixpkgs/bazel-out/darwin-fastbuild/bin/tests/run-test-invalid-nixpkgs-package.runfiles/io_tweag_rules_nixpkgs/toolchains/nodejs") but it does not exist or is not a directory.
ERROR: no such package '@rules_nixpkgs_nodejs//': The repository's path is
"./toolchains/nodejs" (absolute:
"/private/var/tmp/_bazel_runner/a4a3182e50c9460fa52b8c6c1e6f916f/sandbox/processwrapper-sandbox/9/execroot/io_tweag_rules_nixpkgs/bazel-out/darwin-fastbuild/bin/tests/run-test-invalid-nixpkgs-package.runfiles/io_tweag_rules_nixpkgs/toolchains/nodejs")
but it does not exist or is not a directory.
```
@avdv avdv added the merge-queue merge on green CI label Oct 6, 2022
@avdv
Copy link
Member

avdv commented Oct 6, 2022

Thank you, again! @betaboon

@mergify mergify bot merged commit ea5c0bb into tweag:master Oct 6, 2022
@mergify mergify bot removed the merge-queue merge on green CI label Oct 6, 2022
@betaboon betaboon deleted the node-configure branch October 7, 2022 07:33
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.

4 participants