[GitHub] [lucene-solr] dweiss commented on pull request #2068: LUCENE-8982: Separate out native code to another module to allow cpp build with gradle
dweiss commented on pull request #2068: URL: https://github.com/apache/lucene-solr/pull/2068#issuecomment-727827305 I've added changes entry and committed it in, thanks @zacharymorn ! This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene-solr] dweiss commented on pull request #2068: LUCENE-8982: Separate out native code to another module to allow cpp build with gradle
dweiss commented on pull request #2068: URL: https://github.com/apache/lucene-solr/pull/2068#issuecomment-726675884 No worries. I've pushed the code the way I think it should work - please take a look, let me know if you don't understand something. I tested on Windows and Linux, runs fine. The 'tests.native' flag is set automatically depending on 'build.native' but is there separately just in case somebody wished to manually enable those tests from IDE level. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene-solr] dweiss commented on pull request #2068: LUCENE-8982: Separate out native code to another module to allow cpp build with gradle
dweiss commented on pull request #2068: URL: https://github.com/apache/lucene-solr/pull/2068#issuecomment-726599397 Thanks Zach. I looked at what you did with tests and I think this can be done in a cleaner way that always works. I'll show you how, give me some time. Also, update your IDE"s formatter settings to the convention used throughout the code (you used 4 spaces indentation). May be helpful in other patches too. I'll get back to you. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene-solr] dweiss commented on pull request #2068: LUCENE-8982: Separate out native code to another module to allow cpp build with gradle
dweiss commented on pull request #2068: URL: https://github.com/apache/lucene-solr/pull/2068#issuecomment-725917917 > On the other hand, I think these tests will break if run from IDEs. Do we need to support that in this PR? Oh, thanks! I'll take a look after I come back from work. I think the PR should be clean in that it doesn't break other people's workflow, so yes - if you added tests they should run or be quietly ignored if they're not supported. I'll take a look. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene-solr] dweiss commented on pull request #2068: LUCENE-8982: Separate out native code to another module to allow cpp build with gradle
dweiss commented on pull request #2068: URL: https://github.com/apache/lucene-solr/pull/2068#issuecomment-725648046 Added the kill switch, Zach. I think you need to merge with master and then update the overview file that Tomoko moved to package-info.java (grep for "build-native-unix" and you'll see old instructions currently in overview.html). This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene-solr] dweiss commented on pull request #2068: LUCENE-8982: Separate out native code to another module to allow cpp build with gradle
dweiss commented on pull request #2068: URL: https://github.com/apache/lucene-solr/pull/2068#issuecomment-724287441 Byt. if we do have to add that explicit 'build.native' option then I'd implement it as a task (graph) exclusion rather than project exclusion. Windows users in particular may complain as the plugin requires visual studio... https://docs.gradle.org/current/userguide/building_cpp_projects.html#sec:cpp_supported_tool_chain So something like this on all native project's tasks (conditionally): https://discuss.gradle.org/t/removing-tasks-from-taskgraph-remove-a-task-dependency/394 or filter them out entirely: This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene-solr] dweiss commented on pull request #2068: LUCENE-8982: Separate out native code to another module to allow cpp build with gradle
dweiss commented on pull request #2068: URL: https://github.com/apache/lucene-solr/pull/2068#issuecomment-724283052 Zach I've cleaned up the native build a bit - moved it under lucene/misc, added Windows build (it does build the native library for me). I didn't check whether it works on a Mac but I suspect it should. I also left the native project included by default in settings (removed the "optional" flag). Gradle's cpp plugin ignores the project on platforms not explicitly mentioned in the targetMachines - I am curious whether we'll blow up something or if it's just going to work. While I don't particularly like having native code in Lucene, I think it's better than it used to be (mixed cpp code with java code, etc.). I allowed myself to commit directly to your fork, hope you don't mind (please test it out!). This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene-solr] dweiss commented on pull request #2068: LUCENE-8982: Separate out native code to another module to allow cpp build with gradle
dweiss commented on pull request #2068: URL: https://github.com/apache/lucene-solr/pull/2068#issuecomment-723842876 bq. The windows equivalentWindowsDirectory.cpp still sits in misc module and hasn't been moved over yet. Ah... I didn't realize this is the case then - sorry. Give me a day or two, I'll try to make those files compile under Windows and maybe we can do a clean patch that moves everything. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
