-
Notifications
You must be signed in to change notification settings - Fork 63
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
Add reactor-cpp as a submodule and reorganize the C++ build process #971
Conversation
While working on this, I noticed that many methods in |
This sounds like a big improvement! Cloning the Git repository has definitely been a slow point in the language server. From your description it sounds like it probably will break |
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.
With the changes I just pushed, the language server seems to work -- I checked by merging this into the lsp
branch locally. LSP tests also pass for me locally, so hopefully there are no more blockers on that front.
Thanks for your fixes! I will clean things up then and get it to work in Epoch. |
As far as I am concerned, this is ready to be merged. The copy mechanism now also works from Epoch and was (manually) tested. It would be great if we could include this PR in the 0.1.0 release. I would only need an approval ;) |
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.
Look very good! Would the directory name reactor-cpp-default
maybe be better than reactor-cpp-lfbuiltin
? The latter looks a bit odd and difficult to read, IMO.
Yes, probably default is better |
This PR brings the following changes:
The new C++ build process
Consider this LF project structure:
Where
A.lf
andB.lf
both contain a main reactor.Previously, compiling
A.lf
would create a completely isolated cmake project insrc-gen/A/
. With this PR, this structure is created instead:The internals of
src-gen/A/
are similar to before. However, we get a new top-level CMakeLists.txt that creates a top-level cmake project for the entire LF project.A/
is automatically included as a subproject. The directoryreactor-cpp-lfbuiltin
is automatically copied from thereactor-cpp
submodule. The-lfbuiltin
suffix indicates that it is the built-in reactor-cpp version of LF.Let's assume we now compile
B.lf
and it sets this target property:runtime-version: foo
. This will modify the src-gen directory as follows:The
reactor-cpp-foo
directory will be automatically fetched via git. The sub-projectB
is configured to depend onreactor-cpp-foo
, whileA
still depends onreactor-cpp-lfbuiltin
.Overall, this change makes the build-process a lot more robust. CMake is aware of all the files within a project and knows exatly what needs to be compiled when.
I am not sure if these changes might interfere with the LSP linting. @petervdonovan could you check?