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

nbb: init at 0.5.103 #173452

Closed
wants to merge 33 commits into from
Closed

nbb: init at 0.5.103 #173452

wants to merge 33 commits into from

Conversation

PlumpMath
Copy link

@PlumpMath PlumpMath commented May 18, 2022

Description of changes

Add the nbb for clojurescript (babashka) interpreting in node.js
https://github.com/babashka/nbb

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 22.05 Release Notes (or backporting 21.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@PlumpMath PlumpMath mentioned this pull request May 19, 2022
@PlumpMath
Copy link
Author

PlumpMath commented May 19, 2022

There is a disadvantage in the nix ecosystem that modules are called only after entering the external node_path. I only know how to use node2nix
about it...

svanderburg/node2nix#294

Why did you exclude node2nix from the nix node package official documentation? I wish I could go back to it now.

https://github.com/NixOS/nixpkgs/blob/master/doc/languages-frameworks/javascript.section.md

Is nix a problem with nodejs over the past few months? There was and now the node2nix packages are installed well. However, it is not imported even in node (of course in nbb). If there is a solution to that, someone would appreciate it.

Of course, if I can solve it, I'll change the code here.

Copy link
Author

@PlumpMath PlumpMath left a comment

Choose a reason for hiding this comment

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

conversations reviews.
hmm
https://github.com/NixOS/nixpkgs/search?q=NODE_PATH

I don't know how to load the installed nodePackages if I use something in particular. If you install nix as a user (including node2nix), everything is gathered under ~/.nix-profile/lib/node_modules.

Anyway, I’ll use NODE_PATH and overrideAttr after uploaded this pull request.

@PlumpMath
Copy link
Author

@SuperSandro2000
Now what should I do?

@SuperSandro2000
Copy link
Member

Also please squash the package related commits into on and follow the contributing guide when naming them.

@SuperSandro2000
Copy link
Member

I don't know how to load the installed nodePackages if I use something in particular. If you install nix as a user (including node2nix), everything is gathered under ~/.nix-profile/lib/node_modules.

WDYM? Does the program require some node packages to run on itself or do you want to load some node_modules for your workload?

@PlumpMath
Copy link
Author

PlumpMath commented Jun 1, 2022

I don't know how to load the installed nodePackages if I use something in particular. If you install nix as a user (including node2nix), everything is gathered under ~/.nix-profile/lib/node_modules.

WDYM? Does the program require some node packages to run on itself or do you want to load some node_modules for your workload?

It's nothing. I was using node2nix with c dependencies like zeromq (node-gyp?) and didn't know much, so I was just frustrated. Its fixed now, but i dont know why. And of course, it seems to have nothing to do with nbb.

For now, I just want to upload other things as soon as this nbb goes up.

@PlumpMath
Copy link
Author

PlumpMath commented Jun 2, 2022

Also please squash the package related commits into on and follow the contributing guide when naming them.

hmmm; its not too easy to me.

https://youtu.be/fvj8H5yUKu8

I just followed this and I'm sorry, but I've never done the next step squash commit, so I don't know what to do. I am afraid of making a mistake.

If there is a blog post, document, or video that can be a reference for the git command, you might be able to see it and solve it right away. In the current situation, I have to learn merge, rebase, and squash completely, but it seems to be progressing I'm sorry;;

If I can get past this one, I want to upload some other packages too. It's a bit frustrating. My ignorance makes me angry too.
And It's a bit strange that there is no merge resolve commit on github.

@SuperSandro2000
Copy link
Member

Maybe https://www.youtube.com/watch?v=OR8Q-4PLalc can help you?

@PlumpMath
Copy link
Author

PlumpMath commented Jun 3, 2022

Maybe https://www.youtube.com/watch?v=OR8Q-4PLalc can help you?

cause mistake I did a fixed merge...
so I would tried to squash merge it again to back, but it failed because I didn't know how to go back to that point.
sorry;

I'll try again.

@PlumpMath
Copy link
Author

PlumpMath commented Jun 3, 2022

@SuperSandro2000 Looks like you've even done a squash now. thank you for telling me. y,.g; Thanks.
It was difficult, but I think I can do better next time.
Now, when will it be uploaded to nixpkgs/master? Is there only one thing left to do?...
As you pointed out, I'll have to read the contribution manual more carefully.
Thanks @SuperSandro2000

@kubukoz
Copy link
Member

kubukoz commented Jun 3, 2022

@PlumpMath as far as I can see this PR hasn't been properly squashed, to match the desired commit convention you'll need something like this:

image

here's a working branch: https://github.com/NixOS/nixpkgs/compare/master...kubukoz:nbb?expand=1 - we can try to force push to your branch to get these commits, or I can reopen the PR for you.

@PlumpMath
Copy link
Author

PlumpMath commented Jun 3, 2022

@kubukoz I want to force push my branch because I want to put some extra gift-like code next time. I'm sorry I don't know too much about collaboration. Because I also want to start contributing, if not as much as the long-standing contributions of others I have received. What should I do next? How to push upstream(nixpkgs/master) I want to know the standard method after squash. Etc I'd appreciate it if you could let me know. Sorry to bother us. Thanks.

@kubukoz
Copy link
Member

kubukoz commented Jun 4, 2022

@PlumpMath the contributing guidelines for this repo are in https://github.com/NixOS/nixpkgs/blob/master/CONTRIBUTING.md, generally someone else who has merge rights will merge your PR. In this instance I can do that but the branch currently doesn't follow the guidelines.

@kubukoz
Copy link
Member

kubukoz commented Jun 4, 2022

Pushed my commits here, just make sure to squash properly in the future - I recommend playing around with rebasing in a new repository that you won't be afraid to mess up (maybe even another clone of nixpkgs) so that it's easier next time ;)

Copy link
Author

@PlumpMath PlumpMath left a comment

Choose a reason for hiding this comment

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

fix whitespace deps.edn file.

@PlumpMath
Copy link
Author

Except for NODE_PATH, my nbb settings are the same. If that doesn't work, I don't know how to solve it.
I hope the test succeeds.

@PlumpMath
Copy link
Author

clojure-emacs/cider#2359

add GIT_SSL_CAINFO SSL_CERT_FILE
Copy link
Author

@PlumpMath PlumpMath left a comment

Choose a reason for hiding this comment

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

add GIT_SSL_CAINFO SSL_CERT_FILE

Copy link
Author

@PlumpMath PlumpMath left a comment

Choose a reason for hiding this comment

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

change jdk to graalvm11-ce & export JAVA_HOME

change DEPS_CLJ_TOOLS_DIR
Copy link
Author

@PlumpMath PlumpMath left a comment

Choose a reason for hiding this comment

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

change DEPS_CLJ_TOOLS_DIR


{
:deps {
com.github.seancorfield/honeysql {:git/tag "v2.2.891" :git/sha "796c734"}
Copy link
Member

Choose a reason for hiding this comment

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

If clojure CLI is invoked (I supposed the bb release command is invoking it), clojure will make a network request to resolve the tag. To avoid that, remove :git/tag and use the full sha

Copy link
Contributor

Choose a reason for hiding this comment

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

This is a bug and tracked upstream at https://clojure.atlassian.net/browse/TDEPS-223.

In case that makes you feel any better about doing the workaround :)

@jlesquembre
Copy link
Member

@PlumpMath Thanks for your effort to add nbb to nixpkgs.

Since nbb is distributed as an npm package, I'd go with a pragmatic approach and just add the npm package to the list node packages in nix. If you are not familiar with it, here are the docs: https://github.com/NixOS/nixpkgs/blob/master/doc/languages-frameworks/javascript.section.md#adding-and-updating-javascript-packages-in-nixpkgs

I tried that, and I was able to run the nbb repl.

The main issue I see with building it from source is that Clojure is not well supported in nixpkgs. There are some efforts to improve the situation, like clj2nix, clojure-nix-locker or clj-nix (I'm the author of the last one), but none of those are on nixpkgs yet. I'd go with the node package for now, and if in the future one of those gets into nixpkgs, we could consider trying to build nbb from the source.

@PlumpMath
Copy link
Author

PlumpMath commented Jun 13, 2022

@PlumpMath Thanks for your effort to add nbb to nixpkgs.

Since nbb is distributed as an npm package, I'd go with a pragmatic approach and just add the npm package to the list node packages in nix. If you are not familiar with it, here are the docs: https://github.com/NixOS/nixpkgs/blob/master/doc/languages-frameworks/javascript.section.md#adding-and-updating-javascript-packages-in-nixpkgs

I tried that, and I was able to run the nbb repl.

The main issue I see with building it from source is that Clojure is not well supported in nixpkgs. There are some efforts to improve the situation, like clj2nix, clojure-nix-locker or clj-nix (I'm the author of the last one), but none of those are on nixpkgs yet. I'd go with the node package for now, and if in the future one of those gets into nixpkgs, we could consider trying to build nbb from the source.

A few years ago, when I was learning nixos for first time.
After learning how to install nodejs package for the first time with the link you mentioned.
Then I moved to node2nix right away.
I didn't think of that. good hint thanks.

Before going to bed this morning, I took setup of clj-nix hello, and put everything in my config nix (flake nix-darwin & nixos & wsl nixos). It's very fast and good (thanks for making clj-nix, it's fantastic @jlesquembre ).

So I'm talking about
I was thinking about you and clj-nix.

borkdude did mention you.
(I didn't know until this morning, until I asked the nbb question)

anyway like think
Because when release bb If I wind up with as clj-nix to install bb and build nbb, Assume will the build succeed?
that's what I thought in the morning. That's just my imagination, but for clj-nix to evolve, I plan to experiment more with clj-nix anyway.
.
Anyway, I'm going to try it by installing the nodejs nbb package as mentioned above, and if it doesn't work, I'm going to experiment with this thread from time to time.

I'm not good at English, so what you mentioned above is the current clojure error, right? @whentze (sandbox false right! Thanks)
and
Please understand even if it can be frustrating because I don't understand the words well.
If there's anything else I've missed, please reply. I'll do it right away.

good day. Thanks all.

@whentze
Copy link
Contributor

whentze commented Jun 13, 2022

@PlumpMath

No problem, my description wasn't very clear. Here's my view:

Your current error ("Build Error building classpath. Failed to read artifact descriptor for org.clojure:clojure:jar") is caused by the nix sandbox. What happens there is the clojure dependency management library tools.deps tries to pull a dependency from maven.org. It fails, because the sandbox bans all network access.
In the specific error you're seeing, the dependency it's trying to fetch is clojure itself.
However, even if you fixed that, all the other clojure and Java dedendencies of nbb would fail to get fetched for the same reason.

There are ways to stop tools.deps from pulling dependencies from the network.
When using clj2nix or clj-nix, the way is to prefetch all the dependencies into a /nix/store path and then give that classpath to clojure in your derivation whenever you run it.
Your PR already does the first part (prefetch) but not the second part.
That is, you call bb release without anything to tell it about this precomputed classpath.
For regular clojure or clj invocations, the flag you need to pass is -Scp ${someClasspath}.
I do not know how to do it for bb, but the documentation can help you here.

The issue I linked to (https://clojure.atlassian.net/browse/TDEPS-223) is not directly related to this, but it is the cause of another error that you might run into later due to the way you have written your deps.edn. Currently, if you have both a :git/tag and :git/sha in there, it will always go to the network to fetch the tag. The sandbox will also stop this. You can fix this by removing :git/tag and putting in a full-length :git/sha instead of an abbreviated one.

@PlumpMath
Copy link
Author

@PlumpMath

No problem, my description wasn't very clear. Here's my view:

Your current error ("Build Error building classpath. Failed to read artifact descriptor for org.clojure:clojure:jar") is caused by the nix sandbox. What happens there is the clojure dependency management library tools.deps tries to pull a dependency from maven.org. It fails, because the sandbox bans all network access. In the specific error you're seeing, the dependency it's trying to fetch is clojure itself. However, even if you fixed that, all the other clojure and Java dedendencies of nbb would fail to get fetched for the same reason.

There are ways to stop tools.deps from pulling dependencies from the network. When using clj2nix or clj-nix, the way is to prefetch all the dependencies into a /nix/store path and then give that classpath to clojure in your derivation whenever you run it. Your PR already does the first part (prefetch) but not the second part. That is, you call bb release without anything to tell it about this precomputed classpath. For regular clojure or clj invocations, the flag you need to pass is -Scp ${someClasspath}. I do not know how to do it for bb, but the documentation can help you here.

The issue I linked to (https://clojure.atlassian.net/browse/TDEPS-223) is not directly related to this, but it is the cause of another error that you might run into later due to the way you have written your deps.edn. Currently, if you have both a :git/tag and :git/sha in there, it will always go to the network to fetch the tag. The sandbox will also stop this. You can fix this by removing :git/tag and putting in a full-length :git/sha instead of an abbreviated one.

clearly i got it. Thanks.

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants