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

Corrade and Magnum on Linux #3611

Merged
merged 4 commits into from
Jul 4, 2018
Merged

Corrade and Magnum on Linux #3611

merged 4 commits into from
Jul 4, 2018

Conversation

lecopivo
Copy link
Contributor

To get corrade and magnum working on Linux I had to do a minor change.

  1. Extension ".exe" was assumed for executable.
  2. Removed default feature magnum[windowlesswglapplication] which is Windows only

I have tested it on few examples and it worked without a problem. However, a more through examination to test all possible plugins and extensions is probably necessary.

Removed ".exe" extension for executables on Linux.
Removed default feature magnum[windowlesswglapplication] which works
only on Windows.
@msftclas
Copy link

msftclas commented May 29, 2018

CLA assistant check
All CLA requirements met.

@mosra
Copy link

mosra commented May 29, 2018

@lecopivo ohoo, thanks! This was on my roadmap (I even have a dirty WIP patch lying around), but you were faster :)

Just one note: there's a builtin CMAKE_EXECUTABLE_SUFFIX variable that you can use instead of creating your own.

Removing the windows-specific default feature makes sense for now, however in the long run I actually wanted to have system-specific Default-Features (so picking one enabled-by-default application for every system, but -- a question for vcpkg developers: is there a possibility to specify Default-Features or a list of Features differently for each system (Windows, Linux, macOS)?

@alexkaratarakis alexkaratarakis self-assigned this May 30, 2018
@mosra
Copy link

mosra commented Jun 4, 2018

Oh, um, just in case I wasn't clear and it's blocking the merge (sorry): from my side it's completely fine to merge the changes in their current form.

Copy link
Contributor

@Squareys Squareys left a comment

Choose a reason for hiding this comment

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

Looks good to me, too!

@mosra
Copy link

mosra commented Jun 17, 2018

@alexkaratarakis @ras0219-msft any chance to get this merged? :) Thanks a lot!

@ras0219-msft ras0219-msft merged commit 7c4e262 into microsoft:master Jul 4, 2018
@ras0219-msft
Copy link
Contributor

Sorry for taking so long on this and thanks for the PR!

Unfortunately, the portfile isn't a "full" CMake context, so some settings like CMAKE_EXECUTABLE_SUFFIX aren't available :(

@mosra
Copy link

mosra commented Jul 4, 2018

Ah, I see.

Thanks a lot for the merge! 👍

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

Successfully merging this pull request may close these issues.

6 participants