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

Some structural refactoring #37

Open
3 of 5 tasks
cboulay opened this issue Oct 9, 2019 · 6 comments
Open
3 of 5 tasks

Some structural refactoring #37

cboulay opened this issue Oct 9, 2019 · 6 comments

Comments

@cboulay
Copy link
Collaborator

cboulay commented Oct 9, 2019

At IEEE SMC in Bari, there was a meeting including Christian, Tim, David, and myself. One of the action items that came out of the meeting were some changes to improve upon small points of dissatisfaction with the current organization of the repos.

  • The language-specific example code should sit next to the LSL source code for that language. In other words, the C/C++ examples should be moved into a folder within the liblsl repository.
  • There are a mix of build instructions. The proposed solution is to put the build instructions only in a BUILD.md doc and remove any other build instructions, including the Wiki. Language-specific build instructions (java, Matlab) should sit with the language binding code.
  • There is some redundancy between the main README.md and the Wiki. Any information that needs to be front and centre should go into the README.md and be removed from the Wiki.
  • The Wiki landing page (HOME) should be nothing more than a table of contents.
  • Having both a within-tree and out-of-tree build system is confusing and prone to break. Out-of-tree is necessary so we will abandon the within-tree build system.*

There are some consequences to abandoning the within-tree build system:

  • Each app will have to have its own copy of the cmake code required to find liblsl.
  • The default way to find liblsl should be to download the latest version of the compiled library from the GitHub release pages. The second way would be to search in known folders.
  • For liblsl developer friendliness, the test applications can either move within liblsl, or if they stay separate then its cmake file can default to finding liblsl locally.
@tstenner
Copy link
Collaborator

tstenner commented Oct 9, 2019

The language-specific example code should sit next to the LSL source code for that language. In other words, the C/C++ examples should be moved into a folder within the liblsl repository.

I'll prepare a PR in the next days.

There are a mix of build instructions. The proposed solution is to put the build instructions only in a BUILD.md doc and remove any other build instructions, including the Wiki.

Agreed. I'd put all the documentation in a separate repo and push the built documentation to readthedocs (PR, example), so there's one place instead of several wikis and fragments in the repositories.

Language-specific build instructions (java, Matlab) should sit with the language binding code.
Makes sense.

Having both a within-tree and out-of-tree build system is confusing and prone to break. Out-of-tree is necessary so we will abandon the within-tree build system.*

The within-tree build system comes practically for free (i.e., a single if(NOT TARGET LSL::lsl) at the top of the code to search for liblsl), so I'd leave it in there. The main problem is that the out-of-tree builds have some rough edges, so instead I'd promote that to the default option.

Each app will have to have its own copy of the cmake code required to find liblsl.
I already started that (e.g. here, here and here).

The default way to find liblsl should be to download the latest version of the compiled library from the GitHub release pages. The second way would be to search in known folders.

liblsl has no external dependencies, builds with compilers more than a decade old and takes less than two minutes to compile on a consumer laptop. I wrote the code to download a precompiled liblsl for the pylsl package and it's not something I'd do unless it's really necessary.

The CI builds download liblsl, but they are constrained to one or two OSes. Doing it from within CMake isn't something I'd do.

For liblsl developer friendliness, the test applications can either move within liblsl, or if they stay separate then its cmake file can default to finding liblsl locally.

Which test applications? The unit tests should stay in the liblsl repo for the CI system no matter what.

@cboulay
Copy link
Collaborator Author

cboulay commented Oct 9, 2019

I'd put all the documentation in a separate repo and push the built documentation to readthedocs

There was some agreement that having the documentation with the source code was desirable. If you pull the repository or get the zip from somewhere, you should be able to read and follow the BUILD.md instructions while offline. Obviously if you're missing dependencies then you need to be online or get them from somewhere, but let's pretend theoretical offline developer has all required dependencies.

The main problem is that the out-of-tree builds have some rough edges, so instead I'd promote that to the default option.

Yes that's good. I would also say that we shouldn't bother documenting the within-tree builds, except maybe in a hard-to-find section in the wiki where the core developers can paste their cmake command lines. (I often refer back to here to find a good cmake command)

Doing it from within CMake isn't something I'd do.

Let's pretend that we have a MSVC developer without too much experience, as improbable as that may be. Is there a way that we can get them to use MSVC's integrated cmake build system and have the downloading of liblsl be transparent to them?

@tstenner
Copy link
Collaborator

There was some agreement that having the documentation with the source code was desirable.

Which parts? All of it is way too much so there'd be at least two separate docs to maintain.

or get the zip from somewhere, you should be able to read and follow the BUILD.md instructions while offline.

Especially for the (less hypothetical than I'd wish for) end-user who got a zip file from somewhere I'd want an up to date build documentation before I get another inquiry why the pirated VS2015 Enterprise doesn't work.

Obviously if you're missing dependencies then you need to be online or get them from somewhere, but let's pretend theoretical offline developer has all required dependencies.

Ok, I try to imagine there's this huge group of end-users who

  • have no internet connection
  • no experience with C++ and CMake
  • can't use a prebuilt binary
  • have a working C++ compiler and CMake installed

As a counter suggestion, I'd put a simple build script:

echo Read the build documentation at http://foo.bar/liblsl/build if the below commands don't work
cmake -S $current_path -B $current_path/build
cmake --build $current_path/build --target install
explorer.exe $current_path/build/install

and a short paragraph ("Building: install the latest Visual Studio (link) and CMake (another link), download this zip file and double click on build.bat") in the repository (and therefore this zip file from who knows where).

Then, the extensive build documentation could be online or in (two steps with Sphinx) a single, downloadable PDF for those who want something to read on the train or in the wilderness.

Is there a way that we can get them to use MSVC's integrated cmake build system and have the downloading of liblsl be transparent to them?

Sure, but it's a lot of magic that's hard to understand and debug. It's nice when it works, but when it doesn't it's a huge headache.

The Findliblsl.cmake already looks for the liblsl source tree and prebuilt packages in several predefined paths, so we could include ${CMAKE_CURRENT_SOURCE_DIR}\liblsl in there and add a note what to download and extract to the documentation.

I would also say that we shouldn't bother documenting the within-tree builds, except maybe in a hard-to-find section in the wiki.

Agreed.

@cboulay
Copy link
Collaborator Author

cboulay commented Oct 10, 2019

Which parts? All of it is way too much so there'd be at least two separate docs to maintain.

Just the README.md and BUILD.md.

Ok, I try to imagine there's this huge group of end-users who

  • have no internet connection
  • no experience with C++ and CMake
  • can't use a prebuilt binary
  • have a working C++ compiler and CMake installed

I think you just described most scientists who use open source software and are in a hotel conference room with awful wifi trying to follow along in a workshop.

@tstenner
Copy link
Collaborator

Agreed, except for the "working C++ compiler and CMake installed"-part, but the solution for that is different. A portable SSD costs around $50 and even though it's hard it's possible to have an offline MSVC installer so with a double digit price tag a workshop could distribute the source code, development environment, documentation, test files (the XDF reader will be covered) and marketing materials in 1-2 hours.

@tstenner
Copy link
Collaborator

Github also renders rst files, so a copy of the build guide could be kept in the main repository, but somebody would need to keep them in sync.

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

No branches or pull requests

2 participants