-
Notifications
You must be signed in to change notification settings - Fork 77
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
Fix to allow building pljava with Microsoft Visual C #29
Conversation
Ken Olson caught this a year ago in his pull request tada#29, but it was my oversight; when rearranging the sqlj relations back in tada#10 to accommodate more than one deployment descriptor, I updated the sql in install_jar and remove_jar (and deployInstall and deployRemove), but I just totally overlooked replace_jar. Oops.
This PR has been overlooked. Sorry about that. It needs a rebase before it can go in. |
Code changes to allow compilation and linking with Microsoft Visual C. Maven build process conditionalized to to detect Visual C and adjust options appropriately. See msvc-build-notes.txt for full details. Property names updated for clarity
fdccfd2
to
dbf2bdc
Compare
This pull request has been rebased and is ready to be merged, if you are comfortable with it. It builds fine with postgresql versions through 9.3. Beginning with 9.4, the postgres.exe Windows executable no longer exports all symbols and therefore the fields log_min_messages, client_min_messages and session_timezone that are referenced by pl/java are undefined. I've created a stub file that supplies defaults for log_min_messages and client_min_messages and sets session_timezone to NULL - this works for my application but is not a general solution. I'll create a separate pull request for this stub file since it is independent of the Visual C build changes. |
Looks good. I had just a minor remark regarding the naming of non mvcs specific properties. |
pom.xml updated to remove the msvc prefix except for the one msvc setting for rint definitions. |
Fix to allow building pljava with Microsoft Visual C
Hello Ken (I hope you'll still get mail on this closed PR), I have just been working on the build process some more, and I discovered I have questions about your patch that didn't come to me until now:
Thank you, |
Creating the MS VC build was my first experience with Maven and the learning curve was “interesting”. Also this work was done some time back and only merged recently after a number of other PRs that added support for building with Java 7 (PR #35) which I didn’t subsequently take into account. So, with that background let’s see about your questions (numbering does not match).
I have no vested interest in any of the naming conventions and am happy for you to reorganize any of this to make it clearer or more consistent. I’d be happy to pull down your branch and test any changes. I can usually turn this around with a few days. Ken |
Ok, that's encouraging. I think I can streamline some of that. Completely agree about the Maven learning curve. I might have something later in the week. -Chap |
Hi Ken, There's a branch in my repo where I think I have a way to do this that relies more on the profile and less on injecting properties into things. I am not feeling cocky enough to make it a pull request yet, but I will be eager to hear how close (or not close) it comes to building in your environment. A possible issue I can foresee is that it is adding the MS-specific include paths late in the list. If it's necessary for them to be earlier, I did find a way to do that too, it's just (even) more verbose. It will also mention libjvm twice (with a right path, I hope, and a wrong one), but as long as that won't hurt anything, it's probably not worth the further added verbosity to do anything about it. I am hoping that Do you happen to know what makes the -Chap |
I was able to test compilation and linking with your branch - there is one change needed in the pom.xml file. The directory reference for the jvm.lib file needed to use JAVA_HOME rather than java.home. With this change the link worked fine (got undefined references to JNI methods with the original). I like the streamlined build - much cleaner! I haven't had a chance to look at the extra libraries you mention - no experience with them. A search across sources implies that Backend.c may use pq library in versions prior to 9.3 but I haven't tested this. |
That's great, I was expecting it to need more tweaking. I am still curious about the I'm suspecting that they might refer to slightly different places in the tree so the relative paths following them have to be slightly different. I find myself with a mild preference for relying on the property set by the Java runtime itself rather than the one found in the environment (you don't know where that's been!), if we can make it work by putting the right relative path after it. About the three libraries, I'd be curious what happens for you if you just delete those 15 lines right out of the POM for experiment's sake. For me, nothing at all seems to miss them. I would expect them to make a difference if we were building a main module that had to pull those other things in when invoked, but I guess the commit that put them in was named "Updated to Java7 and clang for Mac OS X Mavericks" so maybe that's my big hint. :) Maybe they can go in a separate OS X profile. I guess they don't really hurt anything, they just add that special kind of befuddlement you get when trying to understand a file and wondering "am I crazy for thinking these shouldn't be needed here?" -Chap |
... output from |
Wait, hold the phone. I've updated my branch. Turns out there's a nar plugin config option that's supposed to automatically add the right No more worrying about sun.misc.boot.library.path java.home JAVA_HOME .... |
Your latest change with the nar option to add the spec works for me also - compiles and builds the windows dll just fine. I found some notes from a colleague who built pljava for mac (in Septemeber 2014 from tada/pljava/master) in which he says "By default the output of the native build is to create a "shared" library. On Linux platforms this usually means a ".so", on Windows this will likely mean a ".so" file. On Mac by default this creates a ".dylib" file which is not what we want. Instead we need to change the output type to "plugin" which will create a ".bundle" file which we can rename to a ".so" later." So this seems to confirm your hypothesis that these entries may be specific to the mac os build. |
Hi Ken, Would you have any time to do a test MSVC build with another branch in my repo? I'm trying to simplify the pom and build.xml a bit more. ... let me know if it becomes a chore, but for right now without a proper buildfarm, it's very helpful for me to know there's someone with that environment to try out a build. Thanks! |
I should be able to get to it sometime this week, and will update you with what I find. Ken From: Chapman Flack [mailto:notifications@github.com] Hi Ken, Would you have any time to do a test MSVC build with another branch in my repohttps://github.com/jcflack/pljava/tree/simplify/master/libdirmacro? I'm trying to simplify the pom and build.xml a bit more. ... let me know if it becomes a chore, but for right now without a proper buildfarm, it's very helpful for me to know there's someone with that environment to try out a build. Thanks! — |
Code changes to allow compilation and linking with Microsoft
Visual C. Maven build process conditionalized to to detect Visual C
and adjust options appropriately. See msvc-build-notes.txt for
full details.