-
Notifications
You must be signed in to change notification settings - Fork 115
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
Compiling with ant. Moved non compilable outdated files to /legacy #247
base: master
Are you sure you want to change the base?
Conversation
…lder so that current important files compile properly.
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.
overall this LGTM. I like that we reorganized the java files into separate directories, teasing out those that are used by the "main" benchmarks from the other one-off tests that are not typically used, but I don't know if the word "legacy" is appropriate for those? Maybe "misc"?
I'm curious what the "out of box" experience is for users with this change. What happens if I check out a fresh working folder and run python src/python/setup.py
and then python src/python/localrun.py
-- will python code run this ant/ivy build? Or ... does it still use its own internal build system?
README.md
Outdated
### Preparing the benchmark candidates | ||
|
||
The benchmark compares a baseline version of Lucene to a patched one. Therefore we need two checkouts of Lucene, for example: | ||
The benchmark compares a baseline version of Lucene to a patched one. Therefor we need two checkouts of Lucene, for example: |
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.
"Therefore" is correct
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.
Thank you for the work! I'm still a bit want to move to full gradle instead of ant for several reasons:
- It makes IDE setup easier
- It is considered to be more "morden" (in exchange of speed maybe LOL) -- it is sometimes important, for example, one of my (newly graduated) colleague doesn't even know what is ant.
- Lucene uses gradle as well, so more or less we can suppose Lucene people all know gradle. (This probably is not a reason, but just want to say gradle shouldn't be considered a "hard to use" thing at all)
To me I think 1 is really important for letting new people participate in this repo, even it is most likely a one time thing but it set up a high mental barrier for people who are not that familiar with all those things.
I know @mikemccand is probably against that so just tagging him here :)
### (Optional, for development) set up IntelliJ | ||
You can open the project as usual in IntelliJ. | ||
|
||
Select src/main folder as Source Root. `Right Click src/main --> Mark Directory As --> Source Root` | ||
|
||
Select src/python folder as source Root. `Right Click src/python --> Mark Directory As --> Source Root` | ||
|
||
For configuring the right jar files you will have to do the following. | ||
The jars that you need are present in at 3 places. | ||
1. Apache Lucene jars. Which are present in your ${lucene.checkout}/lucene folder. | ||
2. Some jars will be downloaded by Ant-Ivy build system of this project into the `./libs` folder of this package. | ||
3. Some jars are shipped with this project which are present in the `./lib` folder. | ||
|
||
IntelliJ Idea does not automatically detect these jars in the classpath. So you will have to do following steps to | ||
include them. Also, IntelliJ Idea by default do not recursively include all the jars in sub folders, so follow the below | ||
instructions to enable recursive search in folder for both the above jar paths. | ||
|
||
`` | ||
IntelliJ IDEA --> File --> Project Structure --> Modules --> luceneutil-root --> Dependencies --> + | ||
--> Jars And Directories --> Select the path to ${lucene.checkout}/lucene --> Open --> Choose Roots (Unselect All) --> | ||
OK --> Choose Categories of Selected Files --> Jar Directory --> OK --> Apply --> OK | ||
`` | ||
|
||
After this you will have to update the file in .idea folder manually to enable 'recursive' jar discovery. Open your | ||
project's `.iml` file and change the `<jarDirectory>` xml tag's recursive attribute to `true` | ||
` vim $luceneutil.root/.idea/luceneutil.iml ` | ||
|
||
Change line like below from recursive="false" to recursive="true" | ||
``` xml | ||
<library> | ||
... | ||
<jarDirectory url="file://$MODULE_DIR$/../../lucene/lucene" recursive="false" /> | ||
</library> | ||
``` | ||
|
||
This will make idea search jar files recursively in lucene project. | ||
Alternatively, you can configure idea with the source code of Lucene as a library, by not doing "unselecting all" in the `Choose Roots` step above. | ||
|
||
Repeat the same process for lib and libs folder in $luceneutil.root folder. | ||
|
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 think gradle could help skip all those steps, altho it is admittedly the slowest build system but I really think an easy and fast IDE setup is very important, especially for newcomer.
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.
Agree, IDEs understand Gradle much more than Ant. But I also felt that there is a learning curve for gradle, especiallly because of language dependence. Also, Although there is java toolkit support in Gradle now, but there can be some difficulties in supporting newer or custom JDK for testing. But such does not seem to be an issue with Ant as it does not come in between execution of code.
With this small initial setup we can work with both lucene classes or lucene jars. Moreover if all the lucene jars are put in one folder in its build then we don't need to manually add recursive in *.iml file.
I think it will still use "Mike build"? Because that build is written in |
@msokolov : As I have not updated any classes or files and Ant does not come in way of execution, all the exiting practices and commands should work like before. To test I checked out the this PR in a new folder and ran the following commands which worked correctly.
May be one by one we can make the 'Mike build' to merge with Ant system ( shouldn't be difficult but preferable separate PR ). Currently I have renamed the existing build.xml to data-build.xml and "imported" it into the build.xml. So even the existing build commands like 'ant build' and 'ant vectors300-tasks' work as is. Now we can update the lucene.checkout in the build.properties file. |
I'm having trouble evaluating the two options here :) I do agree, simplifying the onboarding of new devs using IDEs that are NOT emacs, heh, is important. Can someone compare the onboarding process of Gradle (as is now in luceneutil, but also as it could be if we further improved it) with ant? I agree that modifying the benchy runner to also use this new build tooling can/should come later. It's somewhat complex since a single benchy may need to compile against different Lucene clones (baseline and competitor), possibly using different JDKs each time as well. Sometimes the Lucene level changes being tested require code changes to luceneutil sources as well, making things extra hairy :) |
This change