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

Add NO_X11 build option for headless plugins compilation #3738

Merged
merged 4 commits into from
Jun 21, 2018

Conversation

orbsmiv
Copy link
Contributor

@orbsmiv orbsmiv commented May 23, 2018

When compiling for a headless system I'm passing the flags -DSC_IDE=OFF -DSC_QT=OFF but the compiler automatically builds the UIUgens plugin, which requires libx11. I've therefore defined a further flag NO_UIUGENS (taking the form similar to NO_AVAHI) which skips all of the UIUgens-related code.
Perhaps a further option might be to define a higher-level NO_UI flag that wraps all of the UI-related flags into one?

Copy link
Contributor

@muellmusik muellmusik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks okay to me, but have you tested with a supernova build?

@orbsmiv
Copy link
Contributor Author

orbsmiv commented May 23, 2018

I have now! Builds and runs successfully both with and without supernova.

@muellmusik
Copy link
Contributor

Great. I'll approve my review then but leave it for the moment in case anyone else wants to check.

@nhthn nhthn added the comp: build CMake build system label May 26, 2018
@nhthn nhthn changed the title Topic/no uiugens Add NO_UIUGENS build option for headless plugins compilation May 26, 2018
@nhthn nhthn added this to the 3.10 milestone May 26, 2018
Copy link
Contributor

@nhthn nhthn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks great, thanks. the UIUGen class files will still be installed in sclang and people will get "UGen not installed" errors if they try to use the classes, but it's part of a broader problem with sclang that selective class import is really annoying to do.

think the best solution here would be to add some info to the class file docs for each UGen in the UIUGens plugin. "If you get 'UGen MouseX not installed errors,' the server was probably compiled with UIUGens disabled. This is done on headless systems where there is no X server."

once this is merged i'll open a ticket to make these documentation changes.

@mossheim
Copy link
Contributor

mossheim commented May 26, 2018

looks great, thanks. the UIUGen class files will still be installed in sclang and people will get "UGen not installed" errors if they try to use the classes, but it's part of a broader problem with sclang that selective class import is really annoying to do.

Couldn't we rather not copy those files while building, in the same way GUI files are not copied?

@nhthn
Copy link
Contributor

nhthn commented Jun 8, 2018

i would prefer NO_X11 over NO_UIUGENS, in case there are other future dependencies on X11 that don't involve UGens.

Couldn't we rather not copy those files while building, in the same way GUI files are not copied?

hmmmm i'm not wild about reorganizing the directory structure in the help files and the class library just to separate out these dudes, but i guess that's what we gotta do in this wonky language.

so now we have to account for the possible absence of a UIUGen on any given SC installation. fortunately, there's only one use in the class lib: MouseX is used in Filter.scopeResponse in SCClassLibrary/Common/GUI/PlusGUI/Control. however if you have Qt you definitely have X, so not an issue. (also, I may add, Filter:scopeResponse, and the fact that it uses MouseX for some reason, is pretty exemplary of the bloat we have in the class lib)

@mossheim
Copy link
Contributor

mossheim commented Jun 9, 2018

i would prefer NO_X11 over NO_UIUGENS, in case there are other future dependencies on X11 that don't involve UGens.

That's a good point.

hmmmm i'm not wild about reorganizing the directory structure in the help files and the class library just to separate out these dudes, but i guess that's what we gotta do in this wonky language.

You could also build a regex by listing the names of the files you explicitly want to ignore, but grouping them by functionality would also be OK. Either solution is fine with me.

@mossheim
Copy link
Contributor

mossheim commented Jun 9, 2018

@orbsmiv do you mind rebasing this and fixing the merge conflict?

Copy link
Contributor

@mossheim mossheim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

marking this as 'needs changes' until concerns have been addressed

@orbsmiv orbsmiv force-pushed the topic/no-uiugens branch from ff831f5 to 136b142 Compare June 11, 2018 15:02
@orbsmiv
Copy link
Contributor Author

orbsmiv commented Jun 11, 2018

Rebase done.
Is the consensus that NO_X11 is the preferred flag name? If so I'll go ahead any make these changes too.

@mossheim
Copy link
Contributor

Thanks @orbsmiv! I think that NO_X11 makes more sense as a flag name, thanks for the suggestion @snappizz.

@orbsmiv orbsmiv changed the title Add NO_UIUGENS build option for headless plugins compilation Add NO_X11 build option for headless plugins compilation Jun 12, 2018
@orbsmiv
Copy link
Contributor Author

orbsmiv commented Jun 12, 2018

Okay, flag name and description updated.

Copy link
Contributor

@mossheim mossheim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!

@nhthn nhthn merged commit 3c49a4f into supercollider:develop Jun 21, 2018
@orbsmiv orbsmiv deleted the topic/no-uiugens branch June 22, 2018 11:45
@nhthn nhthn mentioned this pull request Aug 23, 2018
4 tasks
@klingtnet
Copy link

I am trying to compile headless supercollider on a Raspberry PI 3b+ on the latest raspbian but the cmake call fails because X11 is not present even though I disabled it with NO_X11=ON:

$ git clone --recursive https://github.com/supercollider/supercollider && mkdir supercollider/build && cd supercollider/build
$ cmake -DSC_IDE=OFF -DSC_QT=OFF -NO_X11=ON ..
# ...
CMake Error at /usr/share/cmake-3.7/Modules/FindX11.cmake:429 (message):
  Could not find X11
# ...

@mossheim
Copy link
Contributor

@klingtnet You are missing the D in -D NO_X11=ON...

@klingtnet
Copy link

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp: build CMake build system enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants