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

Patches to get cmake to work with Solaris; continuation of #842 and #844 #851

Closed
wants to merge 1 commit into from

Conversation

whorfin
Copy link
Contributor

@whorfin whorfin commented May 26, 2015

Continuation of #842 and #844
Also some cleanliness changes to .gitignore for cmake that aren't
specific to Solaris.
Most of the time was spent beating my head against cmake until i realized the OS definitions
were happening AFTER the custom cmake modules were being loaded.
D'oh!

This is by no means fully tested as I've spent all my time trying to get clean builds with the new cmake system.
...which i like and much prefer, so not complaining, but man...

Thanks again to @knnniggett
Cheers

Also some cleanliness changes to .gitignore for cmake that aren't
specific to Solaris
@whorfin whorfin mentioned this pull request May 26, 2015
@connortechnology
Copy link
Member

Yeah... the OS detection should probably go earlier... but the rest.. now so good as is. I'm not entirely sure I get what is going on with the sendfile stuff.

Since I'm the one who added it, but got it from elsewhere I'd like to know more. I assume the sendfile detection just doesn't work on solaris?

@knight-of-ni
Copy link
Member

My bad. I put my original response in the wrong spot. Moving here...

You should undo all the changes to .gitignore. The files you added are needed by other distros. It does not hurt anything to have those files lying around.

Did you make the changes to the cmake modules yourself, or did you get them from some other source?
The modules under cmake/Modules are canned, and they should be kept that way. Troubleshooting custom changes to the modules would make things more difficult than it should be.

We need to accomplish what you are trying to do within the CmakeList.txt files, or adding additional modules (or replacing them entirely is ok), as long are they come from a reputable source that has tested the modules.

For example, does your system have access to a Solaris version of FindPolkit.cmake module? We could check for Solaris in CMakeLists.txt and then call that module instead of the one already there .....Or, if we can verify the canned module is cross-platform then we could use that.

@knight-of-ni
Copy link
Member

@connortechnology I actually added the OS detection stuff to CMake, which follows what you had previously done in autotools. Thanks to @whorfin for discovering that it matters where we put it inside the CmakeFile.

@whorfin
Copy link
Contributor Author

whorfin commented May 26, 2015

@connortechnology - The sendfile .cmake rules had two problems. First, "-lsendfile" is needed
in order for the test compile to succeed. Second, the second rule [where i put the comment, originally line 22] can not work given the internals of check_library_exists. These check routines check
for the existence of the variable, not its value, and since the only way to hit that line of code is for a failure with HAVE_SENDFILE4_SUPPORT, that variable is defined and set to 0. Than means
check_library_exists takes an early exit and never does what the author [seems to have] intended.

I pass in
-DZM_EXTRA_LIBS="socket;nsl;sendfile"
to cmake, but these aren't passed to the cmake rules, hence the requirement for another -lsendfile
if you set up the cmake test environment properly from the ZM_EXTRA_LIBS, the tests
would succeed and the sendfile test would succeed. From an OmniOS perspective, those
libraries are just plain needed, and otherwise don't seem to get defined in the build system.

@knnniggett - just trying to help with the .gitignore; most of this has nothing to do with Solaris, these are mostly emphemeral or generated files being generated by cmake. As such, should they not be ignored by git? It is the same spirit as ignoring .o and .a - seems like good practice.
A build should not create files thought by version control to be "authored", IMHO, that results in confusion between what is managed programming intent, and what is the dependent result of some process of compilation.

As for the cmake files, OK. It seemed more helpful to correct the modules then to hack around them in the master CMakeLists.txt - I did not realize you were trying to reference previously existing cmake modules. That seems a bit dangerous as they are constantly evolving and quite installation specific; the openssl module just broke recently with the OpenSSL security fixes, for example.
Would you rather I add kludges in the master?
There are no policykit .cmake modules on OmniOS, at least not with the JoyEnt cmake version which I use. My rules simply search in the correct place on OmniOS, rather than the wrong place [there is no polkit-gobject-1, the pkg-config name is "polkit", which is what my fix corrects]
OmniOS does not use cmake, it uses pkg-config to talk to build systems.
cmake is thus not core-os, but added as a layer [via JoyEnt], and not fully Solarized yet.

@knight-of-ni
Copy link
Member

I am leaving for vacation for two weeks and plan to pick back up on this after return.

What I plan to do is figure out the best way to incorporate the proposed changes without modifying the existing modules under the cmake modules folder. If a newer module is ever released, we need to be able to easily drop it into the module folder, without having to worry about merging custom changes to it.

It is disappointing that something called Cross-Platform Make is not so cross-platform after all. Anyway, I need to do some research on this.

@whorfin
Copy link
Contributor Author

whorfin commented May 27, 2015

That makes sense

On May 27, 2015, at 6:52 AM, Andrew Bauer [email protected] wrote:

I am leaving for vacation for two weeks and plan to pick back up on this after return.

What I plan to do is figure out the best way to incorporate the proposed changes without modifying the existing modules under the cmake modules folder. If a newer module is ever released, we need to be able to easily drop it into the module folder, without having to worry about merging custom changes to it.

It is disappointing that something called Cross-Platform Make is not so cross-platform after all. Anyway, I need to do some research on this.


Reply to this email directly or view it on GitHub.

@knight-of-ni
Copy link
Member

Superseded by #906
Discussion is in #905

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants