Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
please don't override cmake's defaults
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.
Can you give a reason? Installing to /usr/local is a nuisance to many, I have met somebody who couldn't find SC after installing ;) So what are the risks?
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.
you are overriding both the cmake defaults and the command line arguments passed to cmake: if a user sets the install prefix, you simply overwrite what the user requests.
fwiw, on osx there is no reason to use "install" anyway, as app bundles are self contained and osx isn't really fhs-compliant, anyway
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.
Not from what I found. -DCMAKE_INSTALL_PREFIX at configure time still overrides this setting. There is no "force" involved.
Many roads lead to Rome... I don't see a problem in adding a convenience for some that does not conflict with the workflow you propose.
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.
afaik cmake's set(...) command would need the FORCE argument to override -D params - this should be okay. And on OSX, install is required for any real use, as the executables in their build locations aren't really self-contained.
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.
ok ... not having self-contained app bundles is a shortcoming of sc's cmake implementation, though.
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.
@scztt, sorry for extending this thread a bit: running sc in Debug mode from the IDE doesn't seem exactly self-explanatory. Is it the best way to "build" the "SuperCollider" Scheme rather than the all_builds target?
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.
I was still wondering about this PR. IMO should not have merged so fast.
so as it is this is now installing to {project dir}/Install ?
and a user would either use it in place or copy it to somewhere.
On Sun, Jun 28, 2015 at 11:56 AM Rainer Schütz notifications@github.com
wrote:
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.
sorry! I had assumed it was a minor detail. But we can undo it easily if a different solution is found.
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.
It installs by default to a subdirectory "Install" of the build directory, wherever you chose that to be. This is how it used to be and the Readme still reflects that. You can chose any install location by adding the parameter -DCMAKE_INSTALL_PREFIX as cmake argument, and you can also move the SuperCollider folder wherever you want after the "install".
Without this change cmake would "install" to /usr/local which simply is an awkward place for an application bundle, you can't even access it from the Finder. People who wanted the install in some visible, easily accessible place, had to make the "install_prefix" explicit.
If you don't "install" but just work with the bundles in .../editors/sc-ide/Debug or Release this change doesn't affect you, and if you want to "install" to another location, you'd have to use the cmake argument anyways. So I agree with J. that it's a minor detail, just a convenience for people who expect a install folder with exactly the same content as the application installed via a disk-image, and are used to the buildfolder/Install location.