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

Circular dependencies on java_cup/javacup #271

Closed
wltjr opened this issue Dec 13, 2017 · 25 comments
Closed

Circular dependencies on java_cup/javacup #271

wltjr opened this issue Dec 13, 2017 · 25 comments
Assignees
Labels
question More information is needed
Milestone

Comments

@wltjr
Copy link

wltjr commented Dec 13, 2017

java_cup/javacup needs jflex to build. jflex needs java_cup/javacup to build. Both have circular dependencies on each other requiring bootstrap. No clean way to build either from source without using a pre-made binary. Ideally this should change and allow pieces to be built. Such that either can be built fully from source without requiring a pre-built jar. This must have been the case initially before either was made. Chicken and egg situation. Java seems to have lots of issues with such, jaxen/jdom, antlr/stringtemplate, and jflex/javacup. Of course the JDK itself, though open jdk can technically be built from source going back to 1.5 gcj and gnu-classpath.

Hopefully that is possible here, some old version of either javacup or jflex does not need the other and can be the first step in a building from source solution. At the present time each is having to use binaries which is not preferred. Thank you for your consideration in addressing this circular dependency issue.

@lsf37
Copy link
Member

lsf37 commented Dec 13, 2017

It's a true dependency, I don't think there is much that can be done about it. Both tools contain a generated lexer and a generated parser.

In fact, it's even worse: there is a binary boot-strap dependency of JFlex on JFlex. That's been there since the second working version of JFlex (before it ever was released). The very first bootstrap was a manually-written lexer which was buggy and incomplete, just enough to get it off the ground.

Current JFlex should always be compilable with the previous version of JFlex, though. I.e. it's not circular. In addition, the JFlex release package includes the generated files (at least I think it still does, if not we should add them back in) so you can break the dependency when you build from source there.

The situation with Cup is similar. It should always compile with the previous version of JFlex. If you really want to break that cycle you could go several versions back in each tool until you hit the Cup version that still had a manual lexer (I don't remember which one that was, but there were public releases that didn't need JFlex yet).

@wltjr
Copy link
Author

wltjr commented Dec 13, 2017

Ok so there maybe a way if I go back to old enough versions. I have found that to be the case with antlr and stringtemplate. But sadly in newer versions its lost and I got stuck. Not sure if I will run into the same here. There is also a jlex project/package. Not sure if that could be used for early versions to help. I think I saw the JFlex needs JFlex.Hopefully that stuff is no core and can be built after the initial jflex. Running into that with javacc as well, I have an open issue there.

Just trying to figure out to how to package java_cup and jflex. Without stuffing the other into anothers package, java_cup into jfex, or jflex into java_cup. That is how it is presently done. Jflex checks for javacup on system, if there use that, if not go with bundled. With java_cup having a regular dependency on Jflex. so sorta breaks the cycle.

Really not fun to package and sadly other stuff needs this to build. Not really optional to package. I will go looking for older javacups without jflex and see if I can start there. Thank you for that information!

@wltjr
Copy link
Author

wltjr commented Dec 13, 2017

I am able to build java_cup v10k without jflex. That seems to be the newest/old version without jflex dependency. Sadly it cannot parse the LexParse.cup. Seems I need a newer version of java_cup... Maybe it will build an older version of JFlex. Seems I will have some fun here in daisy chaining versions :) Hopefully will end up with some path to build from source.

I think the files are pre-generated in maven sources. I guess I could switch to that. Tend to prefer to build stuff from scratch vs using pre-generated sources. That way live packages work, checkout code from git and build the same as packaged. Anyway maybe some hope. If I find a working path I will make that known here. If I do not I will also make that known :)

@wltjr
Copy link
Author

wltjr commented Dec 14, 2017

Looks like up till 1.5 the sources did have pre-generated files. Seems to have stopped after 1.4.3, last release to contain them pre-generated. Thus far seems like 1.4.3 will be the one needed to get things moving along. I can use JFlex 1.4.3 to build the latest javacup which I can then use for the latest jflex. Though seems like 1.4.3 cannot build 1.6.1, so need 1.5.1. Which I will also use javacup 10k, as it does not need jflex, so I can use that for the newer javacup. And then use that for the latest JFlex... :)
Lame ascii flow chart

javacup 10k
           \
             -> javacup latest -> jflex 1.5.1 ->  jflex latest
           /
jflex 1.4.3

I liked the pre-generation of them in the release. That was something being discussed for javacc as well. Maybe even something that can be committed after any project change. Any project using its own to bootstrap, when updated can run what ever to generate Java files and commit to vc.

@wltjr
Copy link
Author

wltjr commented Dec 14, 2017

Turns out I do not need 1.5. There was some option that was in 1.5 and 1.6 %inputstreamctor false that 1.4 did not like. Without that 1.4 works for both 1.5 and 1.6, thus no need for 1.5. Though I run into other errors. I have seen this before with 1.6 as well.

* Compiling ...
src/main/java/LexScan.java:3697: error: orphaned default
          default:
          ^
src/main/java/LexScan.java:3626: error: 'else' without 'if'
      else {
      ^
2 errors

I have seen this in qdox as well using 1.6. Not sure if its due to Java 9, or what. I tried to fix but it causes even more errors. Not sure exactly where its off.

@lsf37
Copy link
Member

lsf37 commented Dec 14, 2017

Hm. That compile error should not happen. "orphaned default" could be a Java 9 issue (we haven't tested Java 9 yet), the else without if seems weird. Will see if I can test Java 9 tomorrow to check if that is the cause or if the JFlex versions are too far apart (it's theoretically possible that the custom lexer source skeleton doesn't fit with what earlier versions expect).

@wltjr
Copy link
Author

wltjr commented Dec 14, 2017

Yeah not sure, I just build 1.4.3 under 9 as --release 8. I can try 7. Then was building 1.6.1 the same, and got the same error. Basically same error with using jflex for qdox. Maybe a 9 issue. I can see about testing under 1.8. Its fine for qdox 2.0, its 1.x that has a similar issue. qdox 2.0 builds find with jflex 1.4.3 or 1.6.1 under Java 9. Trying to replicate but I know I ran into it from the following sed

#       sed -i -e '2050d' "${S}"/src/java/JFlexLexer.java \
#               || die "Failed to delete crazy default and else {"
#       sed -i -e '1357d' "${S}"/src/java/JFlexLexer.java \
#               || die "Failed to delete crazy default and else {"

That seems to be the exact issue, orphaned default and left over else {. Just using the sed to delete those lines. I was trying to add in missing brackets without luck just the same. Seems the same with 1.4.3 and 1.6.1. Very well could be Java 9, not sure. I have run into odd problems in my env that others have not before. Usually an occasional picky generics or something.

@wltjr
Copy link
Author

wltjr commented Dec 14, 2017

By the way, I liked the generated javacup and jflex files in vc for 1.4.3. Not sure if that was intentionally stopped for >= 1.5 or can resume. It was mentioned in an earlier comment. I like that it is friendly to packagers and resolves most any bootstrapping issues. At least aides in them greatly :)

@lsf37
Copy link
Member

lsf37 commented Dec 14, 2017

There are pros and cons for these. Generally it's nicer to not have generated files in VC, but I do see your point. @regisd, @sarowe any strong opinions?

@wltjr
Copy link
Author

wltjr commented Dec 14, 2017

This is the qdox 1.2.1 flex files that generate the same issue as the jflex ones. Just for comparison/testing, beyond jflex own flex files.

I would normally agree about not putting generated code in vc. Though it does not really change, short of a new version of say jflex generating a new syntax/file, or modifications to the source flex that would change the generated output. Otherwise I think the generated output is always the same, so not sure it matters as much.

Like take this one from 1.4.3 the javacup file. I wonder if I diff that with my own if it would be any different. Given both are coming from javacup 10k. Maybe only the date would differ :)

At the same time it can help others see what version and when the code was generated. Which can give them an idea as to if they may need an older version to generate said file. Or if itwas done under and older jdk/jvm. That newer may have issues. Maybe like I am seeing, but its not consistent. Seems more to do with the syntax of the flex files than the parser. Like in qdox 1.x vs 2.x. Qdox 2 has more to generate and no issues like I have with 1.x. The same issue with jflex on its own jflex files :)

Anyway thanks for your consideration and information provided. Working on the bootstrapping with a few things. Though I doubt I can get kotlin to commit a generated java version of itself :P That one is really bad... Maybe the worst bootstrap yet. Though antlr + stringtemplate is not to far off. This is not to bad short of the odd bug/issue.

@regisd
Copy link
Member

regisd commented Dec 14, 2017

Yes, the generated files were intentionnaly removed from SCM.

There are two reasons:

  1. they can be generated by the previous version of the maven-jflex-plugin (so it's not stricto sensus a circular dep, otherwise it wouldn't build at all). Hence they are not necessary.

  2. Since they are generated, these files lying under SCM can mislead someone into thinking they are used during build. But they are not used: the generated file is used. Of course, they should be identical copies, but this isn't true anymore if the grammar is changed. Or inversely, modifying these copies would be a no-op for the build.

So, I'm generally not in favour of having these files under SCM. If you want to see these generated files, maybe we could publish them in some way via Travis.

I only skimmed through the thread, but I'm not sure I understand what problem you are trying to solve.

@wltjr
Copy link
Author

wltjr commented Dec 14, 2017

@regisd in brief I am building from source not using maven. Its part of packaging on Gentoo, things build from source and do not use per-built binaries etc. Maven and gradle conflict with Gentoo's packaging system and dependency resolution. In the past people have done hackish things to get it to build, using binaries etc. I am trying to build from pure source, no binaries.

One problem with having the files generated is the difficulty in doing such. Like the present issue I am having with else { and default. Which is on more than one project that needs jflex to generated needed java files for build. Though even if jflex added those generated files, other projects may not. This was mostly to address the bootstrapping. If 1.4.3 did not have those generated files it would be much more difficult. That 1.4.3 has that, I can start with that as a base and go up from there.

@wltjr
Copy link
Author

wltjr commented Dec 14, 2017

I did some testing under 1.8, built jflex under 1.8, and then under 9. Ran under each it was built under. I did not see any differences with the generated files. I did run into the qdox error again like I had with jflex. However jflex is not having that problem, now that qdox is again with jflex 1.6.1.

qdox under 1.6.1 bootstrapped via binary

 * Compiling ...
src/java/JFlexLexer.java:1357: error: orphaned default
        default:
        ^
src/java/JFlexLexer.java:2051: error: 'else' without 'if'
          else {
          ^
2 errors
 * ERROR: dev-java/qdox-1.12.1-r10::os-xtoo failed (compile phase):

Now I have an observation but it is a strange one. I get the above error with both jflex 1.5.1 and 1.6.1 if built with jflex 1.4.3. If I generate that file, LexScan.java via fjlex 1.6.1. The above issue with else/default never occurs. Which makes it seems like at times for qdox, 1.6.1 is writing older syntax. I can clearly see the header saying 1.6.1. Diffing all files built under 8 or 9 I see no difference.

So to recap, with jflex 1.4.3, I get the error: orphaned default and error: 'else' without 'if' for jflex 1.5.1 and jflex 1.6.1, but not qdox 1.12.1. That fails with a different error. If I switch to jflex 1.6.1, built using binary bootstrap, then jflex 1.6.1 builds fine, against its same version, assuming jflex 1.5.1 would as well. But qdox fails with the default/else issues under jflex 1.6.1. Regardless of what it was built under or as. Seems odd that 1.4.3 and 1.6.1 both would have the same issue. It does not seem as much JDK specific as something with jflex itself.

I have not tried to rebuild 1.4.3 under Java 8 and see if that makes a difference. It did not seem to matter for 1.6.1, under 8 or 9. Generated files were the same, diff showed no output.

This is under 1.4.3, under 1.6.1 bootstrapped via binary it builds fine

Writing code to "java/LexScan.java"
 * Compiling ...
src/main/java/LexScan.java:3697: error: orphaned default
          default:
          ^
src/main/java/LexScan.java:3626: error: 'else' without 'if'
      else {
      ^
2 errors

Something odd is going on and does not seem consistent. If both had the issue under 1.4.3, then it would be that version. It seems that version has issues with the 1.6.1 LexScan.flex. Which for what ever reason it does not have with older qdox. jflex 1.4.3 seems to handle older qdox better. Since jflex 1.6.1 generates the same issue I get with building jflex 1.6.1 under 1.4.3. Its crazy :)

These are slotted so I know I am using one version over another. Bootstrapped 1.6.1 overwrites and replaces 1.4.3 entirely. So it is only one or the other not both at any time. I cannot explain why I see the same problem under different versions. For different versions of other packages.

For qdox 1.12.1 under jflex 1.4.3 I get this error. Which is different from the 1.6.1 first shown.

Writing code to "src/java/JFlexLexer.java"
byaccj: 5 shift/reduce conflicts.
 * Compiling ...
src/java/com/thoughtworks/qdox/parser/impl/Parser.java:36: error: cannot find symbol
             implements CommentHandler
                        ^
  symbol: class CommentHandler

That means I am missing something else, but its not talking about orphaned default or broken else. This is really strange...

@wltjr
Copy link
Author

wltjr commented Dec 14, 2017

That qdox CommentHandler error was a left over byaccj bit I copied from qdox 2. This is strange as qdox 1.12.1 builds fine under jflex 1.4.3. If I try to use bootstrapped jflex 1.6.1, I get the else/default errors. Which is like I get with jflex 1.6.1 built under jflex 1.4.3.

qdox 1.12.1 builds with 1.4.3, but not 1.6.1
qdox 2 builds with 1.4.3 and 1.6.1
jflex 1.6.1 builds with 1.6.1 but not 1.4.3

The qdox 1.12.1 + jflex 1.6.1 produces same error as jflex 1.6.1 + jflex 1.4.3. While qdox 2 builds fine under both jflex 1.4.3 and 1.6.1.

No clue what is going on, or why one version works and another does not. Nor why completely different versions built differently can produce same error on different projects :P

@wltjr
Copy link
Author

wltjr commented Dec 14, 2017

For the time being I am going to use 1.4.3 and not 1.6.1, till I can build that or 1.5.3. I mostly need this for qdox, if not solely for qdox. Since I can build 1.4.3 from source and it builds both version of qdox 1.12.1 and 2.0. Just going to stick with that till issues with building 1.6.1 can be worked out, and the code it generates in qdox's 1.12.1 case. Worse case I just stick with 1.4.3. Its only needed to build qdox at this time. If I have other packages that need 1.6.1 then maybe worth while. I will be available to help test and move forward.

Even if the files were pre-generated in 1.6.1, I am not sure that would make any difference for qdox 1.12.1. Likely the error would still remain. Not sure what causes that but seems to exist across a few versions, and not sure if its jdk or syntax specific. You all would have a better idea there :)

Thanks for the feedback, sorry for the noise. Let me know if I need to test anything out.

@wltjr
Copy link
Author

wltjr commented Dec 14, 2017

Sorry final comment. I do not think the jdk version matters. I am building and running 1.4.3 under Java 9 and its working fine for both versions of qdox I need it for 1.12.1 and 2.0. Thus I do not believe Java 9 is causing the else/default errors. That seems more syntax of the files and the how its being parsed by jflex.

@wltjr
Copy link
Author

wltjr commented Dec 15, 2017

I filed a new issue #273 since this one is a bit convoluted now. I have the bootstrap/circular deps resolved as stated. At least up to latest javacup. Which resolves the initial issue I reported.

javacup 10k
           \
             -> javacup latest
           /
jflex 1.4.3

Thanks to all and mostly @lsf37 for the initial mention of older versions. That helped with both javacup and jflex :)

@wltjr wltjr closed this as completed Dec 15, 2017
@regisd regisd added the question More information is needed label Dec 15, 2017
@regisd
Copy link
Member

regisd commented Sep 15, 2018

Sorry to answer late here.

Generally it's nicer to not have generated files in VC, but I do see your point. @regisd, @sarowe any strong opinions?

As I said previously, I definitely prefer not to have generated code under source control.

However, the mvn source:aggregated action generates a jar with all java sources, including generated sources. I think that's what we want to release (and this new build artifact could be done in CI by Travis)

@lsf37
Copy link
Member

lsf37 commented Sep 15, 2018

Would it make sense for the release script to produce these and leave them in the source tree such that a from-source distro build could continue from there?

@regisd
Copy link
Member

regisd commented Sep 16, 2018

  • I really see them as a build artefacts, and you could publish them on http://jflex.de as part of the release process (inversely, I'm not sure what is the point of the jflex.zip which is more or less what github already offers for free in the "clone or download" button).
  • Alternatively, they could be build by Travis and published on a specialised branch in github – we can't push on the master branch, as this would cause an infinite Travis triggering. In the release process, we also need to tag this branch correctly.

@regisd
Copy link
Member

regisd commented Sep 16, 2018

Ok, let's reopen. I'll try to implement the second option, which actually has a couple of benefits:

  • it allows to see how the generated source code evolves (mainly as a result of skeleton updates, but we could also detect bugs in the Emitter)
  • it's more portable, and would retain history if we move out of github or Travis

@regisd regisd reopened this Sep 16, 2018
@regisd regisd added this to the 1.7.0 milestone Sep 16, 2018
@lsf37
Copy link
Member

lsf37 commented Sep 16, 2018

I'm Ok with either option (I think :-) -- not 100% sure how option 2 will look like, but happy to leave it up to you).

The point of the zip/tar release is that it has all of this in it, including fully built binaries and documentation. The github download gives you source only, which cannot be built without a bootstrap binary, and the deployed jars would not count as a convenient "standard" (non-Java) distribution formation.

So having the generated sources somewhere is definitely going to be convenient.

regisd added a commit to regisd/jflex that referenced this issue Sep 16, 2018
regisd pushed a commit that referenced this issue Sep 16, 2018
…anch

- Run `mvn source:aggregate`
  This creates a jar containing all java sources accross all projects, _incl. generated-sources_
- Checkout the **aggregated-java-sources** branch in a `repo` directory
- git rm all
- explode the jflex-parent-VERSION-sources.jar
- remove unrelated sources (e.g. plugins)
- git add all
- git commit
- If running in **master**, git push

Fix #271
@regisd regisd self-assigned this Sep 16, 2018
@regisd regisd added this to TODO in Continuous integration via automation Sep 16, 2018
@regisd
Copy link
Member

regisd commented Sep 16, 2018

@wltjr I have good news for you. You can download all java sources (including generated sources) from

Also, I have set up Travis to update the branch
https://github.com/jflex-de/jflex/commits/aggregated-java-sources

@lsf37 We need to tag this new branch in the release process. I've used sources-vX.Y.Z for the naming convention of the tag.

@regisd regisd closed this as completed in 111da23 Sep 16, 2018
Continuous integration automation moved this from TODO to Done Sep 16, 2018
@wltjr
Copy link
Author

wltjr commented Sep 16, 2018

Thank you very much!

@lsf37
Copy link
Member

lsf37 commented Sep 16, 2018

Perfect, this looks very nice!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question More information is needed
Projects
Development

No branches or pull requests

3 participants