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

bloop: 1.2.5 -> 1.3.2 #62812

Merged
merged 4 commits into from Aug 4, 2019
Merged

bloop: 1.2.5 -> 1.3.2 #62812

merged 4 commits into from Aug 4, 2019

Conversation

Tomahna
Copy link
Contributor

@Tomahna Tomahna commented Jun 7, 2019

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@mmahut
Copy link
Member

mmahut commented Jun 7, 2019

I'm trying to test this, however, even with the server running I get:

$ ./blp-client 
Command not found: blp-client

Is this expected?

@Tomahna
Copy link
Contributor Author

Tomahna commented Jun 7, 2019

The correct command binary for the client is ./bloop the blp-client is the unwrapped version. Maybe it should not be in the bin directory. I do not know if there are any best practices around this.

@karolchmist
Copy link
Member

I tried to update it on my side as well, and I have obtained some different sha values...

For example, to get the nailgunCommit, I got 7dfdad8c from https://github.com/scalacenter/bloop/releases/download/v1.3.0/install.py

Maybe it has changed since?

The sha256 for client should change as well, I get 0bchc59dmdxqg5vpdsnhagw0374jr19xg4l5rydcp5ljvv2dp6d4.

For zshCompletion, I get 09qq5888vaqlqan2jbs2qajz2c3ff13zj8r0x2pcxsqmvlqr02hp.

Caveat: I'm not very advanced in Nix, so may I miss something...

PS. Thank you @Tomahna for adding bloop, very useful!

@Tomahna Tomahna changed the title bloop: 1.2.5 -> 1.3.0 bloop: 1.2.5 -> 1.3.2 Jun 9, 2019
@Tomahna
Copy link
Contributor Author

Tomahna commented Jun 9, 2019

Thank you for testing this ! Yes you're perfectly right. As I did not change the sha256, it used the value in my cache so it worked for me, but of course not for you.

It should be fixed.

@Tomahna
Copy link
Contributor Author

Tomahna commented Jun 15, 2019

I've made a few changes. That should prevent both problems from happening again.

As the client has its own derivation it will not appear in the bin directory of the final derivation (only the wrapper will appear), so it should avoid the confusion.

As both the client and the zsh completion have their own derivation, they will have to be rebuilt on version changes and the cache should not be hit.

Please let me know, if there is a better/simpler way to do this.

@svalaskevicius
Copy link
Contributor

svalaskevicius commented Jun 23, 2019

> bloop server    

Defaulting on nailgun port 8212
There is no server running at port 8212
Starting the bloop server... this may take a few seconds
Bloop server could not be located in /nix/store/blp-server.
Pass in the location with `--server-location` before the `server` command.

@svalaskevicius
Copy link
Contributor

 > bloop about
bloop v1.3.2

Using Scala v2.12.8 and Zinc v1.3.0-M4+22-4704d479
Running on Java v1.8.0_212 (/nix/store/i9kl79ch4id866b3xvi3rwr963a4cz9h-openjdk-8u212-ga-jre/lib/openjdk/jre)                                                                
Maintained by the Scala Center (Martin Duhem, Jorge Vicente Cantero)

however:

 > bloop
Command not found: wfid76hlhxc91nk3j37a4sivkbvcg942-bloop-client-9327a60a

@Tomahna
Copy link
Contributor Author

Tomahna commented Jun 24, 2019

If you want to launch the server from the cli you have to pass the location manually, I'm not sure how to fix that and I honestly don't intend to try as I do not use it this way. However you can use the systemd service with services.bloop.install = true or just run the command blp-server.

For the second part, right now I don't know what causes it. It's not really a blocker because running the cli without a command is not really the way you want to use bloop anyway but it is definitely annoying.

@svalaskevicius
Copy link
Contributor

Agreed. Plus I'm not sure how it worked before the upgrade and as both are nonblockers for usage, I'd prefer the new even if somewhat quirky bloop vs nice old one even if these are new issues :)

@Tomahna
Copy link
Contributor Author

Tomahna commented Jun 24, 2019

I checked the old version, it behaves the same. I'd need to check on a non-nix machine to see how it is actually supposed to behave.

@Tomahna Tomahna mentioned this pull request Jun 24, 2019
10 tasks
@Tomahna
Copy link
Contributor Author

Tomahna commented Jun 24, 2019

@FRidh maybe we can remove the work-in-progress tag ?

@svalaskevicius
Copy link
Contributor

btw. I've been using this PR in my manually ported nixpkgs for a while now - not quite sure who could merge it would be great if anyone did before we need to update it again :)

(I've only tested bloop itself, not zsh completions)

@Tomahna
Copy link
Contributor Author

Tomahna commented Jul 18, 2019

I would like that too.

@Tomahna
Copy link
Contributor Author

Tomahna commented Jul 24, 2019

So I've made a few changes to get closer to the way bloop is packaged on other distributions.

This fix a few problems:

  • bloop without argument now work properly
  • bloop server work transparently without having to give it the --server-location
  • it is possible to add java options the way bloop expects them bloop server -J-Xmx2G

Thanks to @svalaskevicius for helping me improve this !

This change prevents accidentally using cached version of dependencies
when updating the version
@svalaskevicius
Copy link
Contributor

just had a chance to test it (except systemd config, as I use my own setup for systemd user unit) - seems like I'm able to pass options to bloop server w/o any issues now :)

@FRidh FRidh merged commit 27e030a into NixOS:master Aug 4, 2019
@svalaskevicius
Copy link
Contributor

Yay! My local nixpkg can now just point to the master :) thanks!

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.

None yet

5 participants