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

Add 'removeOldOutput' option to clear all previously generated sources #92

Closed
joelittlejohn opened this issue Jun 23, 2013 · 20 comments
Closed

Comments

@joelittlejohn
Copy link
Owner

Original author: marst...@gmail.com (March 05, 2013 22:18:53)

What steps will reproduce the problem?

  1. Create a schema, e.g. a type with some enum values
  2. Generate POJO
  3. Modify schema, eg. add new value to enum
  4. Re-Generate POJO

What is the expected output?
The Java source now contains the new enum value.

What do you see instead?
The Java source, generated in step 2, remains unchanged.

What version of the tool are you using? On what Java version? (On what
version of Maven/Ant if applicable?)
Use maven and latest build 0.3.5

If I remove the Java files before step 3, then the Java file is properly generated with the new enum value.

Would it be possible to include a "clean" option to the generate target so that if set to true, all files already present in the generation folder specified would be deleted before starting the generation process (equivalent of deleting manually the already generated files). This is especially important, since you typically do not check-in generate sources, and as such, you expect no sources to be already generated (note that you may have .class reused if found in your class-path, e.g. a jar). Cleaning up source folder before the source generation process therefore makes sense.

I don't know what causes the class not to be refreshed when schema change. Is there any logic that says if the Java (not class) file is present do not regenerate? If the algo (which I believe is the one used) is "if class file exists, do not generate/overwrite source", then I would need to somehow clean not the Java source files but actually the Java (generated) class files from the folder the (maven) project use to store compiled classes, following Java source generation.

Let me know if the description is unclear. I tried my best.

Original issue: http://code.google.com/p/jsonschema2pojo/issues/detail?id=92

@joelittlejohn
Copy link
Owner Author

From joelittl...@gmail.com on March 05, 2013 22:44:57
Philippe, Yes this is by design. This allows you to create schemas that reference existing types from the classpath without fear that new conflicting types will be generated. This allows you to reference types that are part of your project, and also types from jars on the classpath.

The 'clean' option would certainly be possible, but it seems counter-intuitive for a maven plugin to provide such a property, since this is what the clean life-cycle is for. I'm not sure it's such a good thing to encourage this kind of 'extra-curricular' cleaning.

I agree that in most cases, you should not be storing derived resources (such as generated .java files) in source control. I don't understand how this relates to problem though. Maybe an example is best:

  1. You keep some schema documents under source control

  2. You clone this repo and run a maven build:

    mvn compile

    new sources are added into e.g. /target/java-gen, new classes into /target/classes

  3. You make changes to the schema document, you want to clean the existing sources so you do this in the normal fashion:

    mvn clean compile

  4. You make other changes in the project, which should be added to source control

  5. You add and commit all changes - anything in /target (derived resources) is ignored

@joelittlejohn
Copy link
Owner Author

From marst...@gmail.com on March 05, 2013 23:37:50
Hi Joe,
my use case is simple: I have a class generated in java-gen (and
compiled as .class in classes). If I modify the schema, I would expect
the java source to be refreshed without having to know to clean
folders.

The java file is present in java-gen, therefore was at initial
generation identified as not being available in classpath. In other
words, any file in java-gen/classes should not be considered in
classpath (only those from referenced class folders or jars, excluding
the target generated class folder).

If dev forgets to clean, he won't notice java files are out of sync with schema.

That really sounds to me as a design issue, if considered as such by design.

Philippe

Le 2013-03-05 � 5:45 PM, "jsonschema2pojo@googlecode.com"
jsonschema2pojo@googlecode.com a �crit :

@joelittlejohn
Copy link
Owner Author

From joelittl...@gmail.com on March 13, 2013 00:20:18
Hmm, I don't think it's possible to distinguish between classes compiled from generated sources and classes compiled from hand written sources.

Remember that nothing goes in to /java-gen/classes. Sources come from:

src/main/java (hand written)

target/java-gen (generated)

Both of these are source roots, so Maven compiles their contents and adds classes to

/target/classes

It's essential that the directory containing generated types is included as a source root, otherwise Maven wont compile your types. It's also essential that /target/classes is considered in the classpath.

I think some kind of 'clean' option like you propose (that empties out the generated sources directory) probably is the only option. At least this can be added to the pom to guarantee it wont be forgotten. I note that a similar option exists for this jaxb plugin (removeOldOutput):

http://static.highsource.org/mjiip/maven-jaxb2-plugin/generate-mojo.html

@joelittlejohn
Copy link
Owner Author

From marst...@gmail.com on April 04, 2013 11:47:29
Hi Joe, do you expect to add that one soon?

@joelittlejohn
Copy link
Owner Author

From joelittl...@gmail.com on April 10, 2013 22:01:43
Philippe. A few weeks ago I made various changes to add this property, I've just pushed my work-in-progress here:

https://code.google.com/p/jsonschema2pojo/source/browse/?name=issue92%2Fremoveoldoutput

However no matter how I try to tackle this, I simply cannot find a way to create the behaviour you desire.

The fundamental problem is the one we've discussed, and that you originally mentioned here:

"...then I would need to somehow clean not the Java source files but actually the Java (generated) class files from the folder the (maven) project use to store compiled classes"

Although generated sources can be sent to their own directory (and we can easily remove these before we begin type generation), all the project's compiled classes are thrown into the same directory when compiled, and this directory is necessarily added to the classpath. The only way to guarantee that the classes previously compiled do not appear on the classpath is to use maven's clean lifecycle.

Unfortunately, a 'removeOldOutput' property of the kind mentioned here:

http://static.highsource.org/mjiip/maven-jaxb2-plugin/generate-mojo.html

simply doesn't address this problem. Fundamentally, the behaviour you need conflicts the ability to re-use existing classes. Of course we have the option implement another configuration property here, but I'm really not interested in complicating the plugin with a lot of these strange behavioural switches.

I think the only option here, if you want to clean the workspace of derived content, is to use the clean lifecycle as part of your routine, e.g.

mvn clean package

In my experience, everyone should/does do this regularly (at least before they commit their changes). However, I can sympathise with the problem of developers forgetting to clean. One option is to add a defaultGoal to the build section of your pom:

clean package

but I realise that this hardly helps.

I welcome any further ideas you have here, but I admit I'm inclined to resolve this as WontFix.

@joelittlejohn
Copy link
Owner Author

From marst...@gmail.com on April 17, 2013 11:48:02
Ok. I thought about it a bit; I understand that cleaning generated source folder is a no brainer. At least this option should be added. Regarding the .class I don't quite get why you couldn't explicitly remove/ignore any class that was loaded from the current project target class folder. You do know where classes get generated, don't you? As such, if a class is found, but a file check existence is made on this target class folder, you could easily identify the class being generated comes from this project (and not from another project/jar you have dependency on). If the clean flag was specified, you would ignore the existing class and regenerate the java class source. A compile thereafter would regenerate the .class now up to date with the schema.

Honestly, I don't see this as a nice to have. We do use the clean goal before any build, but having to manually call that target whenever you change a schema does not sound very seamless for our devs. Think productivity, any additional step makes our devs unhappy. There is a reason why jaxb offered the option. Let me know if I can contribute in any way to make this happen. I believe it really lies in the algo to generate or not the java file. If you could dissociate "where" the class comes from (and determine it is a "local" class) that would do the trick, I believe.

@joelittlejohn
Copy link
Owner Author

From joelittl...@gmail.com on April 17, 2013 15:44:44
I think you're making an unsafe assumption here that when you reference an existing type, that type will be from another module, library, project, etc. In practice, a typical setup is to have a Maven module that generates some types but uses hand-coded types for parts of the content (where the generated types, for whatever reason, are too dumb to represent some complex structure). Ignoring classes that come from the current module breaks this.

I think you're right we should probably add this 'removeOldOutput' flag for src. Another example where this is useful is when you change your schema in such a way that some types no longer exist - it's good to purge these source files (even though, once again, the types will remain until you tell Maven to clean).

@joelittlejohn
Copy link
Owner Author

From joelittl...@gmail.com on April 19, 2013 18:19:22
Philippe, if we added a config option 'ignoreClasspathTypes' which caused the plugin to never reuse any types from the classpath (no matter where they came from), would this work for you?

@joelittlejohn
Copy link
Owner Author

From marst...@gmail.com on April 19, 2013 20:23:39
No, my pojo will extends pojos from other modules and must not
regenerate these java/classes. They will likely use a different
package name though.

Only classes physically found in the target classes folder of the
current module should be ignored. With a class found in classpath,
would you be able to identify which file name/folder they're in?

Envoy� de mon iPhone

Le 2013-04-19 � 2:19 PM, "jsonschema2pojo@googlecode.com"
jsonschema2pojo@googlecode.com a �crit :

@joelittlejohn
Copy link
Owner Author

From marst...@gmail.com on May 02, 2013 20:31:14
could you consider the option to delete all .java in source paths that matches a specific package name pattern, with include/exclude regex? That would be sufficient to address our need.

@joelittlejohn
Copy link
Owner Author

From joelittl...@gmail.com on May 03, 2013 11:27:31
Could you explain a bit more about this?

My intention here was to add removeOldOutput, that if true, would remove all files found in the outputDirectory before proceeding. Would you want to include any/all Maven 'source root' directories? Are you currently using /src/main/java as your outputDirectory and you want to keep some of the files there?

It would be useful if you could give me a quick example of what you're trying to achieve.

@joelittlejohn
Copy link
Owner Author

From marst...@gmail.com on May 05, 2013 19:50:49
Yes, we are generating the files under /src/main/java (output directory). I would propose you send me a test version that include that change, and I provide you feedback whether it suits my needs. It sounds complex to explain in very details in such a post.

The way we work is we always create a dedicated maven module for the POJOs being generated by jsonschema2pojo. We do not maintain any other source file in this module but those generated by the tool. We tried generating the code into a target/subfolders, but this did not work well with our old eclipse environment. We would have to create a m2e plugin to make things work smoothly (such as the jaxb one).

The JSON schemas are stored in the /src/main/resources/schemas folder, and MAY refer to POJOs (extends, complex types $refs) that exist in ANOTHER module (or project). This allows reusing POJOs being generated elsewhere. For this reason, we leverage the current ability to NOT regenerate a POJO java file if a class is found in classloader that matches the java file that would otherwise be generated.

Hope this clarifies.

@joelittlejohn
Copy link
Owner Author

From marst...@gmail.com on May 05, 2013 19:52:32
Nota: while we generate code in /src/main/java, we do NOT commit these files (folder added to svn:ignore). So that a clean checkout will always regenerate code from tool.

@joelittlejohn
Copy link
Owner Author

From joelittl...@gmail.com on May 05, 2013 19:55:51
Thanks for the explanation. I think you can probably already build a test version from the branch I mentioned above:

https://code.google.com/p/jsonschema2pojo/source/browse/?name=issue92%2Fremoveoldoutput

I'm still not clear though why you need this:

"...delete all .java in source paths that matches a specific package name pattern, with include/exclude regex"

If you have a dedicated Maven module, why do you need to also supply include/exclude patterns?

@joelittlejohn
Copy link
Owner Author

From marst...@gmail.com on May 05, 2013 20:11:36
No we don't need it if you can get the outputDirectory content - in
our case /src/main/java - to be removed completely.

Not that the folder itself should not be deleted, only all its
subfolders and files.

@joelittlejohn
Copy link
Owner Author

From joelittl...@gmail.com on May 05, 2013 20:12:51
Okay, I think the contents of that branch should allow you to test this out.

@joelittlejohn
Copy link
Owner Author

From marst...@gmail.com on May 07, 2013 02:54:06
Hi Joe,
after about 2 hours playing around, I have sadly to report this is not working with Eclipse and m2e. Eclipse and m2e just screw up the whole building process.

Your tool does what it says, it delete outputFolder, as expected, then generate code. But then as a result, eclipse triggers a change in the workspace which triggers an incremental build. Now the generated files appears generated (yeah!) and it seems, no compile error; I was about to say "it works!"... when I tried opening one of the generated file. Eclipse says "out of sync", so I hit F5 (refresh)... and all generated files got suddendly deleted (without rebuilding) and my project complains about missing files...

This being said, I would keep the changes you've done, since at least, a mvn install will at least always regenerate the code properly, which is just fine. I just can use the option while building within Eclipse. Since we have an automated build process that runs maven plain, I will configure the maven profile to use the option turned on, and will keep the option off for "Eclipse" building, which is what our 100+ developers use daily.

Thanks for the time spent on this. I am just frustrated against m2e and its integration with Eclipse. I know they improved with latest m2e builds, but we cannot move to a newer eclipse and m2e version without recertifying all our existing plugins, so we need to stick by it for now (Eclipse Indigo SR1 with m2e 1.0.2).

Phil

@joelittlejohn
Copy link
Owner Author

From rzh...@demandforce.com on June 05, 2013 21:35:43
Have the same issue that Phil running into.

@joelittlejohn
Copy link
Owner Author

From joelittl...@gmail.com on June 07, 2013 06:39:46
Ray, could you possibly build a snapshot from the current HEAD and try setting the new removeOldOutput config property to 'true'? Does this help in your case?

@joelittlejohn
Copy link
Owner Author

From joelittl...@gmail.com on June 19, 2013 23:35:07
The new removeOldOutput config property will be available in 0.3.7.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant