Lucene - Core
  1. Lucene - Core
  2. LUCENE-4335

Builds should regenerate all generated sources

    Details

    • Type: Improvement Improvement
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: None
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      We have more and more sources that are generated programmatically (query parsers, fuzzy levN tables from Moman, packed ints specialized decoders, etc.), and it's dangerous because developers may directly edit the generated sources and forget to edit the meta-source. It's happened to me several times ... most recently just after landing the BlockPostingsFormat branch.

      I think we should re-gen all of these in our builds and fail the build if this creates a difference. I know some generators (eg JavaCC) embed timestamps and so always create mods ... we can leave them out of this for starters (or maybe post-process the sources to remove the timestamps) ...

      1. LUCENE-4335.patch
        2 kB
        Michael McCandless
      2. LUCENE-4335.patch
        22 kB
        Michael McCandless
      3. LUCENE-4335.patch
        39 kB
        Robert Muir

        Issue Links

          Activity

          Michael McCandless created issue -
          Hide
          Robert Muir added a comment -

          I think we should use replaceRegexp commands (like that are already there) to remove the various system information (time, paths, etc) that jflex/javacc/etc add from the generated code.

          then we could have an 'ant regenerate' command that regens all sources, and our usual 'svn status' check would ensure nothing changed.

          Show
          Robert Muir added a comment - I think we should use replaceRegexp commands (like that are already there) to remove the various system information (time, paths, etc) that jflex/javacc/etc add from the generated code. then we could have an 'ant regenerate' command that regens all sources, and our usual 'svn status' check would ensure nothing changed.
          Hide
          Uwe Schindler added a comment -

          Thats a good idea, there is one problem with one of the tools, not sure if jflex or javacc. It happens that one of these tools reorders the switch statement's "case XX:" labels and so creating different source. This seems to depend on JDK version used, if you regen again its the same, but often i changed the metafile (like fixing /** to /* for license) and regened, it was different order. The pattern looks like one of these tools use a HashSet/HashMap of "case" statements, where the order is undefined.

          We should check what causes this.

          then we could have an 'ant regenerate' command that regens all sources, and our usual 'svn status' check would ensure nothing changed.

          We have to extend that one to also detect modifications. The current checker task only looks for unversioned files and checks properties. By this you can run it before commit. This one would need to check for mods, too.

          Show
          Uwe Schindler added a comment - Thats a good idea, there is one problem with one of the tools, not sure if jflex or javacc. It happens that one of these tools reorders the switch statement's "case XX:" labels and so creating different source. This seems to depend on JDK version used, if you regen again its the same, but often i changed the metafile (like fixing /** to /* for license) and regened, it was different order. The pattern looks like one of these tools use a HashSet/HashMap of "case" statements, where the order is undefined. We should check what causes this. then we could have an 'ant regenerate' command that regens all sources, and our usual 'svn status' check would ensure nothing changed. We have to extend that one to also detect modifications. The current checker task only looks for unversioned files and checks properties. By this you can run it before commit. This one would need to check for mods, too.
          Hide
          Robert Muir added a comment -

          We should check what causes this.

          I agree, this is always scary when it happens. It makes it harder to tell if something really changed.

          Show
          Robert Muir added a comment - We should check what causes this. I agree, this is always scary when it happens. It makes it harder to tell if something really changed.
          Hide
          Steve Rowe added a comment -

          I'm not sure about Javacc, but I've seen JFlex reorder cases in switch statements, even when there are no .jflex source changes, when run under different JVM versions. I recall seeing this specifically when generating under Java5 and Java6, both Oracle JVMs on Windows.

          I'll look into the generator to see how to fix the output order.

          Show
          Steve Rowe added a comment - I'm not sure about Javacc, but I've seen JFlex reorder cases in switch statements, even when there are no .jflex source changes, when run under different JVM versions. I recall seeing this specifically when generating under Java5 and Java6, both Oracle JVMs on Windows. I'll look into the generator to see how to fix the output order.
          Hide
          Michael McCandless added a comment -

          Small patch to have createLevAutomata.py do the hg checkout of moman under build/core/... instead of into the source tree.

          Show
          Michael McCandless added a comment - Small patch to have createLevAutomata.py do the hg checkout of moman under build/core/... instead of into the source tree.
          Michael McCandless made changes -
          Field Original Value New Value
          Attachment LUCENE-4335.patch [ 12542911 ]
          Hide
          Steve Rowe added a comment -

          I've seen JFlex reorder cases in switch statements, even when there are no .jflex source changes, when run under different JVM versions. I recall seeing this specifically when generating under Java5 and Java6, both Oracle JVMs on Windows.

          Dawid sent me a patch to use LinkedHashMaps instead of HashMaps for the data structures emitted as switch cases, so insertion order will be the emit order. I committed Dawid's patch to JFlex trunk r614. Please update and rebuild to get the change.

          Show
          Steve Rowe added a comment - I've seen JFlex reorder cases in switch statements, even when there are no .jflex source changes, when run under different JVM versions. I recall seeing this specifically when generating under Java5 and Java6, both Oracle JVMs on Windows. Dawid sent me a patch to use LinkedHashMaps instead of HashMaps for the data structures emitted as switch cases, so insertion order will be the emit order. I committed Dawid's patch to JFlex trunk r614. Please update and rebuild to get the change.
          Hide
          Robert Muir added a comment -

          is there a possibility of a jflex release in the future? It would be nice to regenerate it via IVY like javacc.

          Show
          Robert Muir added a comment - is there a possibility of a jflex release in the future? It would be nice to regenerate it via IVY like javacc.
          Hide
          Steve Rowe added a comment -

          is there a possibility of a jflex release in the future? It would be nice to regenerate it via IVY like javacc.

          I agree, it would be nice for several reasons to make JFlex downloadable via IVY.

          I had planned on working toward a JFlex release this summer, but haven't done any work on it yet.

          Show
          Steve Rowe added a comment - is there a possibility of a jflex release in the future? It would be nice to regenerate it via IVY like javacc. I agree, it would be nice for several reasons to make JFlex downloadable via IVY. I had planned on working toward a JFlex release this summer, but haven't done any work on it yet.
          Hide
          Robert Muir added a comment -

          Cool, i think it would be convenient for the long term: in the short
          term I think we can still try to make some progress here towards something
          along the lines of an 'ant regenerate'.

          I am thinking the easiest way is to just add a 'regenerate' task to
          common-build.xml that is a no-op by default, and then each module can do
          what it needs to do?

          Then we would just call it across the build.

          I think we should try to regenerate as much as possible (data too) when
          we do this: e.g. kuromoji would call 'build-dict', icu module would
          regenerate its stuff, and so on.

          Show
          Robert Muir added a comment - Cool, i think it would be convenient for the long term: in the short term I think we can still try to make some progress here towards something along the lines of an 'ant regenerate'. I am thinking the easiest way is to just add a 'regenerate' task to common-build.xml that is a no-op by default, and then each module can do what it needs to do? Then we would just call it across the build. I think we should try to regenerate as much as possible (data too) when we do this: e.g. kuromoji would call 'build-dict', icu module would regenerate its stuff, and so on.
          Hide
          Steve Rowe added a comment -

          Generation of content that's static should be fine, but generation based on external content we don't control, e.g. rules matching TLDs generated for UAX29URLEmailTokenizer, shouldn't be a regular part of the build.

          Show
          Steve Rowe added a comment - Generation of content that's static should be fine, but generation based on external content we don't control, e.g. rules matching TLDs generated for UAX29URLEmailTokenizer, shouldn't be a regular part of the build.
          Hide
          Robert Muir added a comment -

          Right: in cases like that we should just not include it in 'ant regenerate' I think,
          unless we can find a way to make it static.

          root TLDs have a registration date in the IANA database, and they don't ever get deleted
          right?

          Show
          Robert Muir added a comment - Right: in cases like that we should just not include it in 'ant regenerate' I think, unless we can find a way to make it static. root TLDs have a registration date in the IANA database, and they don't ever get deleted right?
          Hide
          Steve Rowe added a comment -

          Sorry, I don't know how the IANA database works. I'll look into it though.

          Show
          Steve Rowe added a comment - Sorry, I don't know how the IANA database works. I'll look into it though.
          Hide
          Robert Muir added a comment -

          I'm not an expert either: I could be wrong about how it works.

          But still I think we can make progress, excluding things like this from 'regenerate'
          and just try to have regenerate "regenerate as much as possible", hooking
          this into the nightly build or something like that.

          Show
          Robert Muir added a comment - I'm not an expert either: I could be wrong about how it works. But still I think we can make progress, excluding things like this from 'regenerate' and just try to have regenerate "regenerate as much as possible", hooking this into the nightly build or something like that.
          Hide
          Commit Tag Bot added a comment -

          [branch_4x commit] Michael McCandless
          http://svn.apache.org/viewvc?view=revision&revision=1381702

          LUCENE-4335: checkout Moman under build dir

          Show
          Commit Tag Bot added a comment - [branch_4x commit] Michael McCandless http://svn.apache.org/viewvc?view=revision&revision=1381702 LUCENE-4335 : checkout Moman under build dir
          Michael McCandless made changes -
          Assignee Michael McCandless [ mikemccand ]
          Hide
          Michael McCandless added a comment -

          First cut at top-level "ant regenerate"...

          Something is still wrong w/ my ant changes because a top-level "ant
          regenerate" hits this:

          BUILD FAILED
          /l/trunk/lucene/build.xml:614: The following error occurred while executing this line:
          /l/trunk/lucene/common-build.xml:1902: The following error occurred while executing this line:
          /l/trunk/lucene/analysis/build.xml:139: The following error occurred while executing this line:
          /l/trunk/lucene/analysis/build.xml:38: The following error occurred while executing this line:
          Target "regenerate" does not exist in the project "analyzers-morfologik". 
          

          But some of the generators make "harmless" mods to the sources,
          e.g. JavaCC does this:

          Index: lucene/queryparser/src/java/org/apache/lucene/queryparser/flexible/standard/parser/CharStream.java
          ===================================================================
          --- lucene/queryparser/src/java/org/apache/lucene/queryparser/flexible/standard/parser/CharStream.java	(revision 1506176)
          +++ lucene/queryparser/src/java/org/apache/lucene/queryparser/flexible/standard/parser/CharStream.java	(working copy)
          @@ -112,4 +112,4 @@
             void Done();
           
           }
          -/* JavaCC - OriginalChecksum=c95f1720d9b38046dc5d294b741c44cb (do not edit this line) */
          +/* JavaCC - OriginalChecksum=53b2ec7502d50e2290e86187a6c01270 (do not edit this line) */
          

          JFlex does this:

          Index: lucene/analysis/common/src/java/org/apache/lucene/analysis/standard/ClassicTokenizerImpl.java
          ===================================================================
          --- lucene/analysis/common/src/java/org/apache/lucene/analysis/standard/ClassicTokenizerImpl.java	(revision 1506176)
          +++ lucene/analysis/common/src/java/org/apache/lucene/analysis/standard/ClassicTokenizerImpl.java	(working copy)
          @@ -1,4 +1,4 @@
          -/* The following code was generated by JFlex 1.5.0-SNAPSHOT on 9/19/12 6:23 PM */
          +/* The following code was generated by JFlex 1.5.0-SNAPSHOT on 7/23/13 3:22 PM */
          @@ -33,8 +33,8 @@
           /**
            * This class is a scanner generated by 
            * <a href="http://www.jflex.de/">JFlex</a> 1.5.0-SNAPSHOT
          - * on 9/19/12 6:23 PM from the specification file
          - * <tt>C:/svn/lucene/dev/trunk/lucene/analysis/common/src/java/org/apache/lucene/analysis/standard/ClassicTokenizerImpl.jflex</tt>
          + * on 7/23/13 3:22 PM from the specification file
          + * <tt>/l/trunk/lucene/analysis/common/src/java/org/apache/lucene/analysis/standard/ClassicTokenizerImpl.jflex</tt>
            */
           class ClassicTokenizerImpl implements StandardTokenizerInterface {
          

          I was able to remove some timestamps from our own gen tools in
          analysis/icu/src/tools (thanks Rob for the pointers!)...

          Also, there seem to be some real cases where the generated code was
          changed but not the generator, e.g. packed ints sources show real
          diffs (and won't compile after regeneration... I haven't dug into this
          yet), and JFlex seemed to lose some @Overrides...

          Show
          Michael McCandless added a comment - First cut at top-level "ant regenerate"... Something is still wrong w/ my ant changes because a top-level "ant regenerate" hits this: BUILD FAILED /l/trunk/lucene/build.xml:614: The following error occurred while executing this line: /l/trunk/lucene/common-build.xml:1902: The following error occurred while executing this line: /l/trunk/lucene/analysis/build.xml:139: The following error occurred while executing this line: /l/trunk/lucene/analysis/build.xml:38: The following error occurred while executing this line: Target "regenerate" does not exist in the project "analyzers-morfologik" . But some of the generators make "harmless" mods to the sources, e.g. JavaCC does this: Index: lucene/queryparser/src/java/org/apache/lucene/queryparser/flexible/standard/parser/CharStream.java =================================================================== --- lucene/queryparser/src/java/org/apache/lucene/queryparser/flexible/standard/parser/CharStream.java (revision 1506176) +++ lucene/queryparser/src/java/org/apache/lucene/queryparser/flexible/standard/parser/CharStream.java (working copy) @@ -112,4 +112,4 @@ void Done(); } -/* JavaCC - OriginalChecksum=c95f1720d9b38046dc5d294b741c44cb ( do not edit this line) */ +/* JavaCC - OriginalChecksum=53b2ec7502d50e2290e86187a6c01270 ( do not edit this line) */ JFlex does this: Index: lucene/analysis/common/src/java/org/apache/lucene/analysis/standard/ClassicTokenizerImpl.java =================================================================== --- lucene/analysis/common/src/java/org/apache/lucene/analysis/standard/ClassicTokenizerImpl.java (revision 1506176) +++ lucene/analysis/common/src/java/org/apache/lucene/analysis/standard/ClassicTokenizerImpl.java (working copy) @@ -1,4 +1,4 @@ -/* The following code was generated by JFlex 1.5.0-SNAPSHOT on 9/19/12 6:23 PM */ +/* The following code was generated by JFlex 1.5.0-SNAPSHOT on 7/23/13 3:22 PM */ @@ -33,8 +33,8 @@ /** * This class is a scanner generated by * <a href= "http: //www.jflex.de/" >JFlex</a> 1.5.0-SNAPSHOT - * on 9/19/12 6:23 PM from the specification file - * <tt>C:/svn/lucene/dev/trunk/lucene/analysis/common/src/java/org/apache/lucene/analysis/standard/ClassicTokenizerImpl.jflex</tt> + * on 7/23/13 3:22 PM from the specification file + * <tt>/l/trunk/lucene/analysis/common/src/java/org/apache/lucene/analysis/standard/ClassicTokenizerImpl.jflex</tt> */ class ClassicTokenizerImpl implements StandardTokenizerInterface { I was able to remove some timestamps from our own gen tools in analysis/icu/src/tools (thanks Rob for the pointers!)... Also, there seem to be some real cases where the generated code was changed but not the generator, e.g. packed ints sources show real diffs (and won't compile after regeneration... I haven't dug into this yet), and JFlex seemed to lose some @Overrides...
          Michael McCandless made changes -
          Attachment LUCENE-4335.patch [ 12593766 ]
          Hide
          ASF subversion and git services added a comment -

          Commit 1506234 from Michael McCandless in branch 'dev/branches/lucene4335'
          [ https://svn.apache.org/r1506234 ]

          LUCENE-4335: make branch

          Show
          ASF subversion and git services added a comment - Commit 1506234 from Michael McCandless in branch 'dev/branches/lucene4335' [ https://svn.apache.org/r1506234 ] LUCENE-4335 : make branch
          Hide
          ASF subversion and git services added a comment -

          Commit 1506240 from Michael McCandless in branch 'dev/branches/lucene4335'
          [ https://svn.apache.org/r1506240 ]

          LUCENE-4335: commit current patch

          Show
          ASF subversion and git services added a comment - Commit 1506240 from Michael McCandless in branch 'dev/branches/lucene4335' [ https://svn.apache.org/r1506240 ] LUCENE-4335 : commit current patch
          Hide
          Michael McCandless added a comment -

          OK I made a branch https://svn.apache.org/repos/asf/lucene/dev/branches/lucene4335 and committed the last (broken, but a starting point) patch ...

          Show
          Michael McCandless added a comment - OK I made a branch https://svn.apache.org/repos/asf/lucene/dev/branches/lucene4335 and committed the last (broken, but a starting point) patch ...
          Hide
          ASF subversion and git services added a comment -

          Commit 1506248 from Michael McCandless in branch 'dev/branches/lucene4335'
          [ https://svn.apache.org/r1506248 ]

          LUCENE-4335: add empty target in common-build.xml

          Show
          ASF subversion and git services added a comment - Commit 1506248 from Michael McCandless in branch 'dev/branches/lucene4335' [ https://svn.apache.org/r1506248 ] LUCENE-4335 : add empty target in common-build.xml
          Hide
          ASF subversion and git services added a comment -

          Commit 1506258 from Michael McCandless in branch 'dev/branches/lucene4335'
          [ https://svn.apache.org/r1506258 ]

          LUCENE-4335: fix generators to match recent code changes to the gen'd files

          Show
          ASF subversion and git services added a comment - Commit 1506258 from Michael McCandless in branch 'dev/branches/lucene4335' [ https://svn.apache.org/r1506258 ] LUCENE-4335 : fix generators to match recent code changes to the gen'd files
          Hide
          ASF subversion and git services added a comment -

          Commit 1506281 from Michael McCandless in branch 'dev/branches/lucene4335'
          [ https://svn.apache.org/r1506281 ]

          LUCENE-4335: add -r 623 to instructions for checking out jflex

          Show
          ASF subversion and git services added a comment - Commit 1506281 from Michael McCandless in branch 'dev/branches/lucene4335' [ https://svn.apache.org/r1506281 ] LUCENE-4335 : add -r 623 to instructions for checking out jflex
          Hide
          Robert Muir added a comment -

          Cool Mike: regenerate seems to be working!

          But now I think we need to edit Uwe Schindler's groovy script to be a macro that fails also if any files were modified.
          We should use this for verifying the regenerated sources have not changed.
          I think we should also use this in jenkins after running tests.

          The precommit test can keep it off as it does now, but jenkins can be more strict.

          Show
          Robert Muir added a comment - Cool Mike: regenerate seems to be working! But now I think we need to edit Uwe Schindler 's groovy script to be a macro that fails also if any files were modified. We should use this for verifying the regenerated sources have not changed. I think we should also use this in jenkins after running tests. The precommit test can keep it off as it does now, but jenkins can be more strict.
          Hide
          ASF subversion and git services added a comment -

          Commit 1506284 from Michael McCandless in branch 'dev/branches/lucene4335'
          [ https://svn.apache.org/r1506284 ]

          LUCENE-4335: don't regenerate for precommit

          Show
          ASF subversion and git services added a comment - Commit 1506284 from Michael McCandless in branch 'dev/branches/lucene4335' [ https://svn.apache.org/r1506284 ] LUCENE-4335 : don't regenerate for precommit
          Hide
          Robert Muir added a comment -
          regenerateAndCheck:
          
          BUILD SUCCESSFUL
          Total time: 57 seconds
          
          Show
          Robert Muir added a comment - regenerateAndCheck: BUILD SUCCESSFUL Total time: 57 seconds
          Hide
          Uwe Schindler added a comment -

          But now I think we need to edit Uwe Schindler's groovy script to be a macro that fails also if any files were modified.

          If we change the top-level task that runs on ant validate, the problem with that is that you are then no longer be able to run validate on a modified checkout before committing. But I think you are thinking of running this check only on the generated files?

          We could create a separate svnkit macro that does a before/after check. I am thinking about a groovy script that runs a check for modified files, saves that information in a Set<?>, then calls a subant with the regenerate task and runs the macro code again, this time adding to a different set. If the Sets are not identical something has changed

          I can assist with that!

          Show
          Uwe Schindler added a comment - But now I think we need to edit Uwe Schindler's groovy script to be a macro that fails also if any files were modified. If we change the top-level task that runs on ant validate, the problem with that is that you are then no longer be able to run validate on a modified checkout before committing. But I think you are thinking of running this check only on the generated files? We could create a separate svnkit macro that does a before/after check. I am thinking about a groovy script that runs a check for modified files, saves that information in a Set<?>, then calls a subant with the regenerate task and runs the macro code again, this time adding to a different set. If the Sets are not identical something has changed I can assist with that!
          Hide
          Robert Muir added a comment -

          Uwe take a look at the branch.

          I didn't change the top-level task that runs on validate. i only changed the jenkins task.
          precommit still does the same checks as before.

          jenkins should not have modified files in any way...

          Show
          Robert Muir added a comment - Uwe take a look at the branch. I didn't change the top-level task that runs on validate. i only changed the jenkins task. precommit still does the same checks as before. jenkins should not have modified files in any way...
          Hide
          Robert Muir added a comment -

          The thing stopping us from merging this branch to trunk right now is the jenkins configuration. To run regenerate, jenkins needs to have the correct versions installed of:

          • mercurial (hg)
          • javacc
          • jflex
          • icu4c
          Show
          Robert Muir added a comment - The thing stopping us from merging this branch to trunk right now is the jenkins configuration. To run regenerate, jenkins needs to have the correct versions installed of: mercurial (hg) javacc jflex icu4c
          Hide
          Uwe Schindler added a comment -

          Hi Robert, that was an alternattive way to fix the precommit Task by a more intelligent approach.

          In general my current problem is:
          I don't want to setup a fixed JFlex on Jenkins, I want to download it with IVY, so before resolving this issue we should have a JFlex version available. If Steve Rowe is not able to relaese the version on Maven, we should maybe fork jflex on Google Code and make a release including the ANT task.

          The second problem in: Moman - I dont want to have Mercurial (hg) on Jenkins, this makes the setup much worse. Python on Windows is terrible already.

          Ideally, the sources generated by Python should be converted to a ant <script> task using jython. This would make setup easier.

          Show
          Uwe Schindler added a comment - Hi Robert, that was an alternattive way to fix the precommit Task by a more intelligent approach. In general my current problem is: I don't want to setup a fixed JFlex on Jenkins, I want to download it with IVY, so before resolving this issue we should have a JFlex version available. If Steve Rowe is not able to relaese the version on Maven, we should maybe fork jflex on Google Code and make a release including the ANT task. The second problem in: Moman - I dont want to have Mercurial (hg) on Jenkins, this makes the setup much worse. Python on Windows is terrible already. Ideally, the sources generated by Python should be converted to a ant <script> task using jython. This would make setup easier.
          Hide
          Uwe Schindler added a comment -
          • javacc

          JavaCC is already downloaded from IVY in ANT

          Show
          Uwe Schindler added a comment - javacc JavaCC is already downloaded from IVY in ANT
          Hide
          Robert Muir added a comment -

          Uwe: right, well then we can disable the jenkins task and merge this to trunk without the check.

          I dont think we should block this issue on shit like jflex releases (I look at my mailing list, last discussion about this was in 2009).... and this isnt my fault.

          Ill back out the jenkins check, merge this to trunk, and open a new issue.

          Show
          Robert Muir added a comment - Uwe: right, well then we can disable the jenkins task and merge this to trunk without the check. I dont think we should block this issue on shit like jflex releases (I look at my mailing list, last discussion about this was in 2009 ).... and this isnt my fault. Ill back out the jenkins check, merge this to trunk, and open a new issue.
          Hide
          Uwe Schindler added a comment -

          Ill back out the jenkins check, merge this to trunk, and open a new issue.

          Can you provide a patch here, I just wanted to have a quick look! Otherwise if we leave out jenkins from the game at the moment and fix that in later issues and talk with Steve Rowe about releasing or forking JFlex. The Moan stuff should maybe downloaded as a ZIP file from the specific HG version (e.g. from Bitbucket as ZIP file using the commit hash) and unzipped.

          Show
          Uwe Schindler added a comment - Ill back out the jenkins check, merge this to trunk, and open a new issue. Can you provide a patch here, I just wanted to have a quick look! Otherwise if we leave out jenkins from the game at the moment and fix that in later issues and talk with Steve Rowe about releasing or forking JFlex. The Moan stuff should maybe downloaded as a ZIP file from the specific HG version (e.g. from Bitbucket as ZIP file using the commit hash) and unzipped.
          Hide
          Robert Muir added a comment -

          That can be in a new issue too. Its unrelated to what we are doing here.

          Show
          Robert Muir added a comment - That can be in a new issue too. Its unrelated to what we are doing here.
          Hide
          Robert Muir added a comment -

          Here is the patch.

          because it adds a new target thats new and hurts no one, i plan to commit soon.

          Show
          Robert Muir added a comment - Here is the patch. because it adds a new target thats new and hurts no one, i plan to commit soon.
          Robert Muir made changes -
          Attachment LUCENE-4335.patch [ 12593930 ]
          Hide
          Uwe Schindler added a comment -

          +1 looks good
          We can fix the remaing stuff on trunk.

          I am currently working on removing the HG clone.

          Show
          Uwe Schindler added a comment - +1 looks good We can fix the remaing stuff on trunk. I am currently working on removing the HG clone.
          Hide
          Michael McCandless added a comment -

          +1, patch looks great!

          Show
          Michael McCandless added a comment - +1, patch looks great!
          Hide
          ASF subversion and git services added a comment -

          Commit 1506516 from Robert Muir in branch 'dev/trunk'
          [ https://svn.apache.org/r1506516 ]

          LUCENE-4335: ant regenerate

          Show
          ASF subversion and git services added a comment - Commit 1506516 from Robert Muir in branch 'dev/trunk' [ https://svn.apache.org/r1506516 ] LUCENE-4335 : ant regenerate
          Hide
          ASF subversion and git services added a comment -

          Commit 1506533 from Uwe Schindler in branch 'dev/trunk'
          [ https://svn.apache.org/r1506533 ]

          LUCENE-4335: Make moman not use HG anymore. Just download as ZIP from bitbucket and unzip

          Show
          ASF subversion and git services added a comment - Commit 1506533 from Uwe Schindler in branch 'dev/trunk' [ https://svn.apache.org/r1506533 ] LUCENE-4335 : Make moman not use HG anymore. Just download as ZIP from bitbucket and unzip
          Hide
          ASF subversion and git services added a comment -

          Commit 1506542 from Robert Muir in branch 'dev/branches/branch_4x'
          [ https://svn.apache.org/r1506542 ]

          LUCENE-4335: ant regenerate

          Show
          ASF subversion and git services added a comment - Commit 1506542 from Robert Muir in branch 'dev/branches/branch_4x' [ https://svn.apache.org/r1506542 ] LUCENE-4335 : ant regenerate
          Hide
          ASF subversion and git services added a comment -

          Commit 1506548 from Uwe Schindler in branch 'dev/trunk'
          [ https://svn.apache.org/r1506548 ]

          LUCENE-4335: Fix the bug with modifications on the SVN root folder

          Show
          ASF subversion and git services added a comment - Commit 1506548 from Uwe Schindler in branch 'dev/trunk' [ https://svn.apache.org/r1506548 ] LUCENE-4335 : Fix the bug with modifications on the SVN root folder
          Hide
          ASF subversion and git services added a comment -

          Commit 1506549 from Uwe Schindler in branch 'dev/branches/branch_4x'
          [ https://svn.apache.org/r1506549 ]

          Merged revision(s) 1506533 from lucene/dev/trunk:
          LUCENE-4335: Make moman not use HG anymore. Just download as ZIP from bitbucket and unzip
          Merged revision(s) 1506548 from lucene/dev/trunk:
          LUCENE-4335: Fix the bug with modifications on the SVN root folder

          Show
          ASF subversion and git services added a comment - Commit 1506549 from Uwe Schindler in branch 'dev/branches/branch_4x' [ https://svn.apache.org/r1506549 ] Merged revision(s) 1506533 from lucene/dev/trunk: LUCENE-4335 : Make moman not use HG anymore. Just download as ZIP from bitbucket and unzip Merged revision(s) 1506548 from lucene/dev/trunk: LUCENE-4335 : Fix the bug with modifications on the SVN root folder
          Hide
          ASF subversion and git services added a comment -

          Commit 1506549 from Uwe Schindler in branch 'dev/branches/branch_4x'
          [ https://svn.apache.org/r1506549 ]

          Merged revision(s) 1506533 from lucene/dev/trunk:
          LUCENE-4335: Make moman not use HG anymore. Just download as ZIP from bitbucket and unzip
          Merged revision(s) 1506548 from lucene/dev/trunk:
          LUCENE-4335: Fix the bug with modifications on the SVN root folder

          Show
          ASF subversion and git services added a comment - Commit 1506549 from Uwe Schindler in branch 'dev/branches/branch_4x' [ https://svn.apache.org/r1506549 ] Merged revision(s) 1506533 from lucene/dev/trunk: LUCENE-4335 : Make moman not use HG anymore. Just download as ZIP from bitbucket and unzip Merged revision(s) 1506548 from lucene/dev/trunk: LUCENE-4335 : Fix the bug with modifications on the SVN root folder
          Hide
          Steve Rowe added a comment -

          I don't want to setup a fixed JFlex on Jenkins, I want to download it with IVY, so before resolving this issue we should have a JFlex version available. If Steve Rowe is not able to relaese the version on Maven, we should maybe fork jflex on Google Code and make a release including the ANT task.

          I can't promise I'll release JFlex anytime soon, sorry. If you want to fork, you can certainly do that. FYI, Gerwin Klein, the JFlex founder, has done some work (maybe all that needs to be done? not sure at this point) to convert JFlex to a BSD license. I'll review the source and see what state that effort is in - BSD licensing should simplify forking, I think.

          Show
          Steve Rowe added a comment - I don't want to setup a fixed JFlex on Jenkins, I want to download it with IVY, so before resolving this issue we should have a JFlex version available. If Steve Rowe is not able to relaese the version on Maven, we should maybe fork jflex on Google Code and make a release including the ANT task. I can't promise I'll release JFlex anytime soon, sorry. If you want to fork, you can certainly do that. FYI, Gerwin Klein, the JFlex founder, has done some work (maybe all that needs to be done? not sure at this point) to convert JFlex to a BSD license. I'll review the source and see what state that effort is in - BSD licensing should simplify forking, I think.
          Hide
          ASF subversion and git services added a comment -

          Commit 1540187 from Ryan Ernst in branch 'dev/trunk'
          [ https://svn.apache.org/r1540187 ]

          LUCENE-4335: Add Namespaces to Expressions Javascript Compiler

          Show
          ASF subversion and git services added a comment - Commit 1540187 from Ryan Ernst in branch 'dev/trunk' [ https://svn.apache.org/r1540187 ] LUCENE-4335 : Add Namespaces to Expressions Javascript Compiler
          Steve Rowe made changes -
          Link This issue is blocked by LUCENE-5552 [ LUCENE-5552 ]
          Hide
          Steve Rowe added a comment -

          As of the JFlex 1.5.1 upgrade (LUCENE-5552), the only changes I see after running ant regenerate at the top level are in the queryparser module:

          M       lucene/queryparser/src/java/org/apache/lucene/queryparser/classic/CharStream.java
          M       lucene/queryparser/src/java/org/apache/lucene/queryparser/classic/ParseException.java
          M       lucene/queryparser/src/java/org/apache/lucene/queryparser/classic/QueryParser.java
          M       lucene/queryparser/src/java/org/apache/lucene/queryparser/classic/Token.java
          M       lucene/queryparser/src/java/org/apache/lucene/queryparser/classic/TokenMgrError.java
          M       lucene/queryparser/src/java/org/apache/lucene/queryparser/flexible/standard/parser/CharStream.java
          M       lucene/queryparser/src/java/org/apache/lucene/queryparser/flexible/standard/parser/ParseException.java
          M       lucene/queryparser/src/java/org/apache/lucene/queryparser/flexible/standard/parser/StandardSyntaxParser.java
          M       lucene/queryparser/src/java/org/apache/lucene/queryparser/flexible/standard/parser/Token.java
          M       lucene/queryparser/src/java/org/apache/lucene/queryparser/flexible/standard/parser/TokenMgrError.java
          M       lucene/queryparser/src/java/org/apache/lucene/queryparser/surround/parser/CharStream.java
          M       lucene/queryparser/src/java/org/apache/lucene/queryparser/surround/parser/ParseException.java
          M       lucene/queryparser/src/java/org/apache/lucene/queryparser/surround/parser/QueryParser.java
          M       lucene/queryparser/src/java/org/apache/lucene/queryparser/surround/parser/Token.java
          M       lucene/queryparser/src/java/org/apache/lucene/queryparser/surround/parser/TokenMgrError.java
          

          Most of these are diamond operator issues: the generated source was manually converted to use the diamond operator, but the corresponding .jj files were not. I removed the appropriate explicit types in the .jj files and ran ant regenerate, but JavaCC 5.0 doesn't like it:

          javacc-QueryParser:
             [javacc] Java Compiler Compiler Version 5.0 (Parser Generator)
             [javacc] (type "javacc" with no arguments for help)
             [javacc] Reading from file /Users/sarowe/svn/lucene/dev/trunk7/lucene/queryparser/src/java/org/apache/lucene/queryparser/classic/QueryParser.jj . . .
             [javacc] org.javacc.parser.ParseException: Encountered " ">" "> "" at line 225, column 47.
             [javacc] Was expecting one of:
             [javacc]     "boolean" ...
             [javacc]     "byte" ...
             [javacc]     "char" ...
             [javacc]     "double" ...
             [javacc]     "float" ...
             [javacc]     "int" ...
             [javacc]     "long" ...
             [javacc]     "short" ...
             [javacc]     "?" ...
             [javacc]     <IDENTIFIER> ...
             [javacc]     
             [javacc] Detected 1 errors and 0 warnings.
          

          I see JavaCC 6.0 was recently released - maybe it can handle the diamond operator?

          One other problem with some JavaCC-generated sources: the checksum seems to have somehow changed, even though nothing else has? - e.g. for the classic queryparser's CharStream.java:

          Index: lucene/queryparser/src/java/org/apache/lucene/queryparser/classic/CharStream.java
          ===================================================================
          --- lucene/queryparser/src/java/org/apache/lucene/queryparser/classic/CharStream.java   (revision 1580832)
          +++ lucene/queryparser/src/java/org/apache/lucene/queryparser/classic/CharStream.java   (working copy)
          @@ -112,4 +112,4 @@
             void Done();
           
           }
          -/* JavaCC - OriginalChecksum=c847dd1920bf7901125a7244125682ad (do not edit this line) */
          +/* JavaCC - OriginalChecksum=30b94cad7b10d0d81e3a59a1083939d0 (do not edit this line) */
          

          One last thing: I accidentally ran ant regenerate using Java8, and the supplementary character jflex macro files output by the icu module changed, and this caused the JFlex-generated scanner classes to change too. On cursory inspection, some lines are reordered, but I wouldn't think that would trigger scanner class changes. At a minimum, the output should be changed to have a fixed ordering.

          Show
          Steve Rowe added a comment - As of the JFlex 1.5.1 upgrade ( LUCENE-5552 ), the only changes I see after running ant regenerate at the top level are in the queryparser module: M lucene/queryparser/src/java/org/apache/lucene/queryparser/classic/CharStream.java M lucene/queryparser/src/java/org/apache/lucene/queryparser/classic/ParseException.java M lucene/queryparser/src/java/org/apache/lucene/queryparser/classic/QueryParser.java M lucene/queryparser/src/java/org/apache/lucene/queryparser/classic/Token.java M lucene/queryparser/src/java/org/apache/lucene/queryparser/classic/TokenMgrError.java M lucene/queryparser/src/java/org/apache/lucene/queryparser/flexible/standard/parser/CharStream.java M lucene/queryparser/src/java/org/apache/lucene/queryparser/flexible/standard/parser/ParseException.java M lucene/queryparser/src/java/org/apache/lucene/queryparser/flexible/standard/parser/StandardSyntaxParser.java M lucene/queryparser/src/java/org/apache/lucene/queryparser/flexible/standard/parser/Token.java M lucene/queryparser/src/java/org/apache/lucene/queryparser/flexible/standard/parser/TokenMgrError.java M lucene/queryparser/src/java/org/apache/lucene/queryparser/surround/parser/CharStream.java M lucene/queryparser/src/java/org/apache/lucene/queryparser/surround/parser/ParseException.java M lucene/queryparser/src/java/org/apache/lucene/queryparser/surround/parser/QueryParser.java M lucene/queryparser/src/java/org/apache/lucene/queryparser/surround/parser/Token.java M lucene/queryparser/src/java/org/apache/lucene/queryparser/surround/parser/TokenMgrError.java Most of these are diamond operator issues: the generated source was manually converted to use the diamond operator, but the corresponding .jj files were not. I removed the appropriate explicit types in the .jj files and ran ant regenerate , but JavaCC 5.0 doesn't like it: javacc-QueryParser: [javacc] Java Compiler Compiler Version 5.0 (Parser Generator) [javacc] (type "javacc" with no arguments for help) [javacc] Reading from file /Users/sarowe/svn/lucene/dev/trunk7/lucene/queryparser/src/java/org/apache/lucene/queryparser/classic/QueryParser.jj . . . [javacc] org.javacc.parser.ParseException: Encountered " ">" "> "" at line 225, column 47. [javacc] Was expecting one of: [javacc] "boolean" ... [javacc] "byte" ... [javacc] "char" ... [javacc] "double" ... [javacc] "float" ... [javacc] "int" ... [javacc] "long" ... [javacc] "short" ... [javacc] "?" ... [javacc] <IDENTIFIER> ... [javacc] [javacc] Detected 1 errors and 0 warnings. I see JavaCC 6.0 was recently released - maybe it can handle the diamond operator? One other problem with some JavaCC-generated sources: the checksum seems to have somehow changed, even though nothing else has? - e.g. for the classic queryparser's CharStream.java : Index: lucene/queryparser/src/java/org/apache/lucene/queryparser/classic/CharStream.java =================================================================== --- lucene/queryparser/src/java/org/apache/lucene/queryparser/classic/CharStream.java (revision 1580832) +++ lucene/queryparser/src/java/org/apache/lucene/queryparser/classic/CharStream.java (working copy) @@ -112,4 +112,4 @@ void Done(); } -/* JavaCC - OriginalChecksum=c847dd1920bf7901125a7244125682ad (do not edit this line) */ +/* JavaCC - OriginalChecksum=30b94cad7b10d0d81e3a59a1083939d0 (do not edit this line) */ One last thing: I accidentally ran ant regenerate using Java8, and the supplementary character jflex macro files output by the icu module changed, and this caused the JFlex-generated scanner classes to change too. On cursory inspection, some lines are reordered, but I wouldn't think that would trigger scanner class changes. At a minimum, the output should be changed to have a fixed ordering.
          Hide
          Uwe Schindler added a comment -

          One other problem with some JavaCC-generated sources: the checksum seems to have somehow changed, even though nothing else has? - e.g. for the classic queryparser's CharStream.java:

          This is because the checksum is generated on the binary input file. As I regenerated the files the last time and I have Windows CR-LF as line separator, the checksum was different. If you run JavaCC on Linux afterwards, the file checksum changes, unfortunately. I know about this problem, but I have no idea how to fix. I would remove the checkum from the files completely after regenerating (using a regex). We already have many regex replaces, this is just one more.

          I see JavaCC 6.0 was recently released - maybe it can handle the diamond operator?

          I would simply let JavaCC use old-style generics. We have no must to use diamonds. If generated code uses conventional declarations, it is no problem at all.

          If we want to upgrade to JavaCC 6.0, we should carefully compare its output. If its identical, I have no problem with upgrading (if its available in Maven Central).

          Show
          Uwe Schindler added a comment - One other problem with some JavaCC-generated sources: the checksum seems to have somehow changed, even though nothing else has? - e.g. for the classic queryparser's CharStream.java: This is because the checksum is generated on the binary input file. As I regenerated the files the last time and I have Windows CR-LF as line separator, the checksum was different. If you run JavaCC on Linux afterwards, the file checksum changes, unfortunately. I know about this problem, but I have no idea how to fix. I would remove the checkum from the files completely after regenerating (using a regex). We already have many regex replaces, this is just one more. I see JavaCC 6.0 was recently released - maybe it can handle the diamond operator? I would simply let JavaCC use old-style generics. We have no must to use diamonds. If generated code uses conventional declarations, it is no problem at all. If we want to upgrade to JavaCC 6.0, we should carefully compare its output. If its identical, I have no problem with upgrading (if its available in Maven Central).
          Hide
          Uwe Schindler added a comment -

          One last thing: I accidentally ran ant regenerate using Java8, and the supplementary character jflex macro files output by the icu module changed, and this caused the JFlex-generated scanner classes to change too. On cursory inspection, some lines are reordered, but I wouldn't think that would trigger scanner class changes. At a minimum, the output should be changed to have a fixed ordering.

          Java 8 has a different hashing algorithm for string keys... The usual problem.

          Show
          Uwe Schindler added a comment - One last thing: I accidentally ran ant regenerate using Java8, and the supplementary character jflex macro files output by the icu module changed, and this caused the JFlex-generated scanner classes to change too. On cursory inspection, some lines are reordered, but I wouldn't think that would trigger scanner class changes. At a minimum, the output should be changed to have a fixed ordering. Java 8 has a different hashing algorithm for string keys... The usual problem.

            People

            • Assignee:
              Michael McCandless
              Reporter:
              Michael McCandless
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:

                Development