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

Static build to xeus-octave #33

Closed
marimeireles opened this issue Jun 8, 2021 · 25 comments
Closed

Static build to xeus-octave #33

marimeireles opened this issue Jun 8, 2021 · 25 comments
Labels
what/build 🔨 Build system related

Comments

@marimeireles
Copy link
Member

Hey @rapgenic,
I was wondering if you're interested in having the cmake file refactored so it allow us to have xeus-octave built statically :) (maybe it helps in #9?)
All xeus kernels tend to follow this pattern: https://github.com/jupyter-xeus/xeus-sql/blob/master/CMakeLists.txt and I was thinking about implementing it here.

@rapgenic rapgenic added the type/question ❓ Further information is requested label Jun 8, 2021
@rapgenic
Copy link
Collaborator

rapgenic commented Jun 8, 2021

Hi, I don't really understand what you mean: Xeus and related libraries are actually downloaded by cmake and statically compiled in the xeus-octave binary. Would you like to build xeus-octave as a static library itself? If so why, is there any need for it to be a library?

@marimeireles
Copy link
Member Author

Would you like to build xeus-octave as a static library itself?

Yes, the main motivation is so it's easy for users to import xeus-octave to their projects, if they want to expand on it, also might be useful to people who just want to have xeus-octave without having to worry about getting its dependencies.

@rapgenic
Copy link
Collaborator

rapgenic commented Jun 9, 2021

I must say I am not really sure about this... I don't really understand why someone would want to link to a Jupyter kernel, when it exposes a communication channel whose purpose is to avoid the need to link it as a library...

If you have in mind any example of using xeus-octave as a library, I'm open to doing it, but if not, as of now I'd rather not complicate the cmake file too much, also because I need to do a few refactorings (#37 #36)

#9 Would not involve any linking, my idea was to build the xeus-octave binary and install the kernel description files directly from octave, actually I'm trying to avoid linking as much as possible 😄

@SylvainCorlay
Copy link
Member

Hey @rapgenic the use case for providing means to statically link with xeus-octave is for embedding. This has been done with the xeus-python kernel for e.g. the slicerjupyter project.

More generally I think it would be cool to homogenize the cmake (to address such corner cases easily and have homogeneous options and target names)!

@rapgenic
Copy link
Collaborator

I agree on the standardisation across similar projects issue, actually, and I'm beginning to see why there may be the need to embed a kernel into another.

At the moment I have little time and little familiarity with cmake features related to exporting targets, so I'll accept a PR on this. Of course I'm always available for helping.

There's mainly one thing, however, of the current build system that I need to keep (that I haven't seen in other xeus based kernels), which is the possibility to have cmake download and build statically xeus and related libraries. This is quite vital to me, as I use xeus-octave daily and my distro does not include them.

@rapgenic rapgenic added what/build 🔨 Build system related type/planning 📑 Internal organization issues and removed type/question ❓ Further information is requested labels Jun 10, 2021
@SylvainCorlay
Copy link
Member

Great! We should definitely support that use case. This is kinda what we do for the xeus-python-wheel cmake that statically builds the whole thing.

@marimeireles
Copy link
Member Author

Hey @rapgenic this new include(FindOrFetch) thing with these find_or_fetch_package functions are not working for me. Am I missing something?
Thanks!

@rapgenic
Copy link
Collaborator

Sorry to hear that. I just cloned a clean copy from the repo and I got it to build as before. Do you have any log about the error?

@marimeireles
Copy link
Member Author

I do:

(octave)  ~/Development/xeus-octave/build  ➦ 8ba770e ●  cmake -DCMAKE_INSTALL_PREFIX=$CONDA_PREFIX ..
CMake Error at CMakeLists.txt:38 (include):
  include could not find requested file:

    FindOrFetch


CMake Error at CMakeLists.txt:40 (find_or_fetch_package):
  Unknown CMake command "find_or_fetch_package".


-- Configuring incomplete, errors occurred!
See also "/Users/mariana/Development/xeus-octave/build/CMakeFiles/CMakeOutput.log".
See also "/Users/mariana/Development/xeus-octave/build/CMakeFiles/CMakeError.log".

It used to work before, I'm trying to remember if I did something different but this same environment used to build xeus-octave with no issues.

@rapgenic
Copy link
Collaborator

That's quite strange... Just to be clear it used to work even with the new modifications to the build system (FindOrFetch)?

Try replacing the include(FindOrFetch) in the CMakeLists.txt with include(cmake/Modules/FindOrFetch.cmake)

@marimeireles
Copy link
Member Author

No, this for example bd646a0 still works, this is the 0.0.1 tag, so I think the mesa refactoring you did here 2eb09f8 changed something.

@marimeireles
Copy link
Member Author

Try replacing the include(FindOrFetch) in the CMakeLists.txt with include(cmake/Modules/FindOrFetch.cmake)

Doesn't work unfortunately... But I'll have a look on what changed from this tag to now :)

@marimeireles
Copy link
Member Author

I think the problem is using this new function find or fetch, when I change it to FetchContent_Declare, then the problem is gone.
But then, at least for my mac it's still trying to link with openGL.

-- Checking for module 'octinterp'
--   Found octinterp, version 6.2.0
-- Configuring done
CMake Error at CMakeLists.txt:103 (add_executable):
  Target "xeus-octave" links to target "OpenGL::OpenGL" but the target was
  not found.  Perhaps a find_package() call is missing for an IMPORTED
  target, or an ALIAS target is missing?


-- Generating done
CMake Generate step failed.  Build files cannot be regenerated correctly.

@rapgenic
Copy link
Collaborator

I think the problem is using this new function find or fetch, when I change it to FetchContent_Declare, then the problem is gone.

Actually, from your first log message it seems that it doesn't find the file where this function is defined (which is in the folder cmake/Modules. Might it be related to how paths are searched in macos? On linux it works without any problem.

I'll put the content of the FindOrFetch.cmake file in the main cmakelists and push it on a temporary branch for you to try.

But then, at least for my mac it's still trying to link with openGL.

Have you set GLFW3_OSMESA_BACKEND=true in the cmake cache?

@rapgenic
Copy link
Collaborator

the branch is temp/33/cmakefix if you want to try

@marimeireles
Copy link
Member Author

Sorry 🙈 I had somehow erased my cmake dir, I restored it and it's good.

Now I'm having issues with finding osmesa on macos.

-- Found glfw3.
-- Checking for module 'osmesa'
--   No package 'osmesa' found
CMake Error at /usr/local/Cellar/cmake/3.20.1/share/cmake/Modules/FindPkgConfig.cmake:556 (message):
  A required package was not found
Call Stack (most recent call first):
  /usr/local/Cellar/cmake/3.20.1/share/cmake/Modules/FindPkgConfig.cmake:778 (_pkg_check_modules_internal)
  CMakeLists.txt:117 (pkg_check_modules)

brew doesn't seem to offer anything, I tried this package called quartz no success... Maybe it only works for linux.

@marimeireles
Copy link
Member Author

I think this thing might actually do the trick, but it's kind of a long way to go.

@rapgenic rapgenic mentioned this issue Jun 11, 2021
@marimeireles
Copy link
Member Author

ok, I think I found a solution!
Might push a solution soon.

@SylvainCorlay
Copy link
Member

Hey @rapgenic. Just a quick heads up about something loosely connected to this conversation:

@DerThorsten published a "xeus-cookiecutter" which produces a basic kernel with the cmake template that we got, and could potentially be applied here for xeus-octave.

@SylvainCorlay
Copy link
Member

By the way, would there be any interest on your side about make this project part of jupyter-xeus, and announce it a bit more broadly to the community, e.g. with a post on blog.jupyter.org?

@rapgenic
Copy link
Collaborator

rapgenic commented Jan 9, 2022

@DerThorsten published a "xeus-cookiecutter" which produces a basic kernel with the cmake template that we got

Hi @SylvainCorlay, that's a great suggestion, thank you! I've spent a few days adapting the kernel to that template and I must say that it's quite good, I got it up and running in not that much time (I also had the opportunity to become a little more comfortable with conda et al., which is nice as well). The biggest thing for me was having CI for many platforms, as I develop only on Linux.

I got the static build to work (even though untested, as some CMake code is still obscure to me, I'm more relaxed however knowing that's from a template). WIP is available on branch temp/33/template if you're interested.

By the way, would there be any interest on your side about make this project part of jupyter-xeus, and announce it a bit more broadly to the community, e.g. with a post on blog.jupyter.org?

First of all thank you very much for your offer and general interest in this project! I'd surely be interested in making it part of the broader Xeus ecosystem, and in a blog article as well!

There are a couple things that stop me from saying "yes" right away:

  • First of all (and the main problem IMO): maintenance... I am quite busy with many other things, and even though I use this kernel regularly (which means that I'm interested in keeping it alive) I do not have the resources to be a full time maintainer (as you might see from the commit history), which is something people might expect from an "official" jupyter-xeus project
  • The code is quite hacky here and there (a mix of prototype code and real hacks to make octave work), and the GL support is quite a mess. Things are better now that I'm integrating CI, testing and generally cleaning up, but still not optimal, and again I don't have much time for support.

So if given this premises you're still interested in making this part of jupyter-xeus, I am totally in favour! If not, I'd totally understand anyway!

Just to be clear, however, my idea would be to wait some more time to adapt the code to the new template and make a new release in conda, and if possible to get the kernel to work in binder, especially for a blog post.

@SylvainCorlay
Copy link
Member

Hi @SylvainCorlay, that's a great suggestion, thank you! I've spent a few days adapting the kernel to that template and I must say that it's quite good, I got it up and running in not that much time (I also had the opportunity to become a little more comfortable with conda et al., which is nice as well). The biggest thing for me was having CI for many platforms, as I develop only on Linux.

Awesome ! This is a great validation of the cookiecutter approach. We plan on moving it to the jupyter-xeus organization. If you found any issue with it, please report back!

By the way, would there be any interest on your side about make this project part of jupyter-xeus, and announce it a bit more broadly to the community, e.g. with a post on blog.jupyter.org?

First of all thank you very much for your offer and general interest in this project! I'd surely be interested in making it part of the broader Xeus ecosystem, and in a blog article as well!

Great! Regarding your concerns with respect to the level of maturity of the code base, it may just be a matter of signaling it to the end user, in the readme, and even in the console banner... Moving it to jupyter-xeus may actually help with recruiting contributors as it shows the intent to make it more than a that it is more than a personal project.

@SylvainCorlay
Copy link
Member

Hey @rapgenic - would you be ok with me moving forward with this?

I am especially interested in making a build of xeus-octave for WebAssembly and make it usable in JupyterLite.

@AntoinePrv AntoinePrv removed the type/planning 📑 Internal organization issues label Sep 21, 2022
@SylvainCorlay
Copy link
Member

It seems that we should be able to close this issue as xeus-octave follows the same patter as other xeus-based kernels.

@SylvainCorlay
Copy link
Member

Closing as fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
what/build 🔨 Build system related
Projects
None yet
Development

No branches or pull requests

4 participants