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
clojupyter: init at 0.3.2 #103560
clojupyter: init at 0.3.2 #103560
Conversation
pkgs/applications/editors/jupyter-kernels/clojupyter/default.nix
Outdated
Show resolved
Hide resolved
pkgs/applications/editors/jupyter-kernels/clojupyter/default.nix
Outdated
Show resolved
Hide resolved
cp "$uberjar" $out/share/clojupyter | ||
''; | ||
|
||
# doCheck = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do the tests work?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure! I haven't tried because we need to prove we can get past the classpath and reproducibility issues first.
pkgs/applications/editors/jupyter-kernels/clojupyter/default.nix
Outdated
Show resolved
Hide resolved
pkgs/applications/editors/jupyter-kernels/clojupyter/default.nix
Outdated
Show resolved
Hide resolved
Thanks for the review @SuperSandro2000 ! |
I marked this as stale due to inactivity. → More info |
Sorry to be late to the party. I've rebased this and taken it for a spin: https://github.com/bendlas/nixpkgs/tree/clojupyter First thing I noticed was that the build seemed to access my user's So after applying the I then changed the build to unpack the standalone jar into the target directory. The jvm is just as happy to run bytecode from a directory of In addition to verifying this fix, I'd also like to discuss the options for getting rid of fixed output: dwn: thanks for bringing it up. the way i conceptualized it, it's very much intended to solve packaging like this. unfortunately, it has gotten too unfocused to be useful, and it's not very modular. I'm addressing these issues by building it up straight on mvn2nix: do you have a link to the issue, you mentioned, @thomasjm? as far as I'm aware, it should work fine together with https://github.com/talios/clojure-maven-plugin and its main problem is just the sheer bloat of its generated files (due to internal duplication) clj2nix: cool, I hadn't seen that. In theory, it should be pretty easy to just convert (a subset of) a |
Thanks @bendlas , I'll try your branch now. This looks like the issue I mentioned: fzakaria/mvn2nix#30. It doesn't look like a major blocker though so I'm not sure what I was talking about, maybe I just didn't get very far with mvn2nix. |
going with clj2nix seems most painless, for now bendlas@62fa08e |
pkgs/applications/editors/jupyter-kernels/clojupyter/default.nix
Outdated
Show resolved
Hide resolved
oops, sorry, the fixed hash masked this ..
That's strange, because if you look into In any case, the commit you mentioned also has That would have the advantage of sharing artifact downloads with mvn2nix, and in fact having any download caching at all. |
You're right, I was just guessing. It is unpacked I ran it on two machines and got seemingly nondeterministic output. There are about 20k files in the result and differences in 195 of them (diff output: diff.log). I'll try playing with the |
I think the { pkgs, jre, writeShellScript }:
let cljdeps = import ./deps.nix { inherit pkgs; };
classp = cljdeps.makeClasspaths {};
in
writeShellScript "clojupyter" ''
${jre}/bin/java -cp ${classp} clojupyter.kernel.core $*
'' |
if i'm reading this correctly, it looks like you need to pass a connection file argument, and that you do that from the kernel descriptor json via placeholder: https://github.com/clojupyter/clojupyter/blob/b54b30b5efa115937b7a85e708a7402bd9efa0ab/src/clojupyter/util.clj#L138 does the nixos jupyter package support generating kernel.json? |
Yes, I'll put a kernel definition in this PR for the final version. I was just slightly concerned because I was running the script with random arguments and it always just exited silently. I'll try it with a real connection file though. |
Okay, I think it's working! I added a snippet in the main file showing how to launch Jupyter Notebook with it. Many thanks @bendlas. @SuperSandro2000 could you take another look? |
pkgs/applications/editors/jupyter-kernels/clojupyter/default.nix
Outdated
Show resolved
Hide resolved
pkgs/applications/editors/jupyter-kernels/clojupyter/default.nix
Outdated
Show resolved
Hide resolved
# $ nix-prefetch-github hlolli clj2nix | ||
|
||
nix-shell --run "clj2nix deps.edn deps.nix" -E ' | ||
with import <nixpkgs> { }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use a relative path.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I don't quite understand (this code was kindly provided by @bendlas) -- relative path to what?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the idea is smth like ../../../../default.nix
, instead of <nixpkgs>
. The reason I left <nixpkgs>
was, that the script could be called from another working dir, and I didn't want to mess with bash-isms to get the script directory.
EDIT Actually scratch that first point, the script has a dependency on its WD anyway, for finding deps.edn
and deps.nix
.
Also, I thought building the updater from whatever <nixpkgs>
is current, may even be more appropriate, than building the current system to build the updater in this case. The intended audience for this script are only devs, hacking on nixpkgs.
OTOH, I only hacked this up as a proposal. No qualms about having it another way. I was actually thinking, that clj2nix
should probably be packaged with nixpkgs, if we were to do this approach.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should use the current checkout because it is not guaranteed that the nixpkgs channel exists.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just changed it to a relative path and confirmed it works, although I tend to agree with @bendlas that it makes more sense to use <nixpkgs>
since this update script is only for maintainers, and this relative path makes it more fragile to moving the file around etc.
Either way, this PR is ready for another look!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this relative path makes it more fragile to moving the file around etc.
The file usually does not move around and when you use the nix-channel you can easily bypass local changes and wonder why they are not used in the updater.
Please clenaup the git history and squash all the package related commits. |
Aah sorry for the reviewer noise, squashed and rebased on master now. |
Motivation for this change
This PR attempts to address #72464 by packaging
clojupyter
.It almost works, but there is some kind of Java issue making the final JAR file unable to run. It seems to be a classpath issue, and it doesn't happen when building outside of Nix. When you try to run the JAR file inside the
buildPhase
right after building it, you getjava.lang.NoClassDefFoundError: io/simplect/compose$?
. As far as I can tell, it's trying to load a class that doesn't exist (and has a name that's a single question mark?). In a weird way this is a bit like issue #75011 due to the extraneous question mark.I'm hoping someone familiar with Clojure in Nix can help push this over the finish line. A few pings:
dwn
would be suitable for packaging this I'd be happy to switch to it as well.Why does this use a fixed-output derivation?
I tried to find a more Nix-y way to do this, but building Clojure applications in Nix is a little tricky at the moment as discussed here: https://discourse.nixos.org/t/maintained-usable-tooling-for-building-clojure-projects-in-nix/1556/9.
mvn2nix
, but hit some blockers and filed an issue thereclj2nix
, but had trouble figuring out how to use it with Leiningendeps.edn
and thelein
build (combined with the use ofstrip-nondeterminism
) should therefore be reproducible. However in practice I'm seeing it get different hashes on each build, which raises another question about whetherlein uberjar
can be made reproducible.Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)