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

Bump jmp@0.7.0 #3

Merged
merged 3 commits into from
Nov 7, 2016
Merged

Bump jmp@0.7.0 #3

merged 3 commits into from
Nov 7, 2016

Conversation

lgeiger
Copy link
Contributor

@lgeiger lgeiger commented Oct 31, 2016

@n-riesco
Copy link
Owner

@lgeiger Could we have 32bit binaries first, please? Otherwise this change is going to cause me some inconvenience (since most of IJavascript development happens in a 32bit machine).

Also, since this is your first contribution to jp-kernel, the AUTHORS file needs updating. You know the drill:

Pull requests will be distributed under the terms in the LICENSE file. Hence, before accepting any pull requests, it is important that the copyright holder of a pull request acknowledges their consent. To express this consent, please, ensure the AUTHORS file has been updated accordingly.

@lgeiger
Copy link
Contributor Author

lgeiger commented Oct 31, 2016

Also, since this is your first contribution to jp-kernel, the AUTHORS file needs updating.

@n-riesco Sorry I was working from the github UI and commited to the wrong branch.

Could we have 32bit binaries first, please? Otherwise this change is going to cause me some inconvenience (since most of IJavascript development happens in a 32bit machine).

It will build from source on 32-bit just like it did before. With the only difference being that it uses the bundled ØMQ 4.1.5 instead of the one installed on your system. You don't need any extra dependencies https://github.com/zeromq/zeromq.js#installation---contributors-and-development

In theory we could ship 32-bit binaries right now but we need a 32-bit build machine (or a docker image).
Since Travis and Circle CI only have 64-bit VMs we cannot automate this easily (We would need to somehow use docker on Travis). Do you know any build services using 32-bit VMs?
If you really want 32-bit prebuilts, you can manually build them and upload them to the latest github release.

@n-riesco
Copy link
Owner

On 31/10/16 17:31, Lukas Geiger wrote:

Could we have 32bit binaries first, please? Otherwise this change is going to cause me some inconvenience (since most of IJavascript development happens in a 32bit machine).

It will build from source on 32-bit just like it did before. With the only difference being that it uses the bundled ØMQ 4.1.5 instead of the one installed on your system. You don't need any extra dependencies https://github.com/zeromq/zeromq.js#installation---contributors-and-development

I know. I've checked that works. The problem is that it renders my notebook unusable until the compilation of ZMQ completes. I haven't timed it but I reckon that's more than 10'.

I can't deal with this today. Please, don't waste your time on building the 32bit binaries. I'll come back to you on this.

@n-riesco
Copy link
Owner

n-riesco commented Nov 1, 2016

@lgeiger I'm now using yarn to cache the installation of zeromq (and this's helped a great deal to alleviate my pain).

I'm still concerned that the use of zeromq in jp-kernel will make me lose most of the 32bit users.

Here are some ideas that would help ease the pain for 32bit users:

  • (for 32bit users only) do not compile tweetnacl in parallel (my feeling is that this is the point where my notebook freezes).
  • (for 32bit linux users only) use the native zmq library if present

@lgeiger
Copy link
Contributor Author

lgeiger commented Nov 1, 2016

I haven't timed it but I reckon that's more than 10'.

Ouch 10 minutes? Thats way too long.
I timed the libzmq build script on my notebook (OS X) using different settings:

command time
make -j/ make -j 4 24 s
make -j 2 26 s
make 36 s

I have no idea why it takes so long on your machine. You could try to use ccache, maybe it helps a bit.

I'm now using yarn to cache the installation of zeromq (and this's helped a great deal to alleviate my pain).

That's great! But we can not recommend yarn for other platforms right now since it will allays build from source instead of using the prebuilts: prebuild/prebuild#132

(for 32bit users only) do not compile tweetnacl in parallel (my feeling is that this is the point where my notebook freezes).

We can definitely limit the build to 2 cores since the improvement over a full parallel build (4 virtual cores on my machine) is marginal.

(for 32bit linux users only) use the native zmq library if present

I'm against that since it would introduce the installation problems we had with https://github.com/JustinTulloss/zeromq.node again. If we would dynamically link libzmq we couldn't use zeromq inside a bundled application like nteract.

@lgeiger
Copy link
Contributor Author

lgeiger commented Nov 7, 2016

zeromq v3.1.1 is now available.

Should I make a PR to jmp to require to ^3.1.1 explicitly?

@n-riesco
Copy link
Owner

n-riesco commented Nov 7, 2016

On 07/11/16 02:39, Lukas Geiger wrote:

|zeromq v3.1.1| is now available.

Should I make a PR to |jmp| to require to |^3.1.1| explicitly?

I haven't come up with anything yet to stop the repeat compilations in 32bit machines?

Any progress on your side?

@lgeiger
Copy link
Contributor Author

lgeiger commented Nov 7, 2016

I haven't come up with anything yet to stop the repeat compilations in 32bit machines?

I thought speeding up the build resolved your problem. I'll take a look at this 👍

@n-riesco
Copy link
Owner

n-riesco commented Nov 7, 2016

@lgeiger I've just published jmp@0.7.2. Please, could you update this PR to use ^0.7.2?

@lgeiger
Copy link
Contributor Author

lgeiger commented Nov 7, 2016

Thanks for releasing jmp 0.7.2 so fast. I updated this PR.

@n-riesco
Copy link
Owner

n-riesco commented Nov 7, 2016

@lgeiger Were you reading over my shoulder? 😝

@n-riesco n-riesco merged commit d0ee620 into n-riesco:master Nov 7, 2016
@lgeiger lgeiger deleted the patch-1 branch November 7, 2016 15:40
@lgeiger
Copy link
Contributor Author

lgeiger commented Nov 7, 2016

😂 GitHub for the win
bildschirmfoto 2016-11-07 um 16 41 03

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.

2 participants