FWD » Core Development » Conversion Tools

Allow incremental conversion to detect program or class files which have been deleted, and cleanup after it (including .ast/.lexer or other artifacts, its .java generated code, name_map.xml, etc).

Dănuț Filimon wrote:

For #6738, I looked into the code and found JavaPatternWorker$WorkArea.pfiles, this helps us to track the persisted java asts and there's also PatternEngine.processAst() which takes a filename (ast name) which is the file being parsed. The issue is making them aware of each other as a starting point, which I am having trouble with. I can use the same finding for #7793, but this time I need to know which schema I am parsing (from the filename, or from loading the schema dict and checking the annotations) and which table is it for.

• Where the .java file is created, enable tracking.
• When all files are done being created, load the .ast and check for the relative-name annotation of each file (we also need to append the basepath to get the correct filename).
• Save them to the conversion database, we know both the original filename and the name of the resulted java file.

I totally forgot that I have this.filename for asts and that the name of the parsed file is always known.

• The procedure name from name_map.xml
• The source file from registry.xml
• The .jast file of the source file from the conversion folder.
• The conversion output file (.java file)
• Conversion artifacts (ast, p2o, schema, dict, jast, lexer, parser, cache, pphints)

I have an implementation that is partially working at the moment and I am working on handling the name_map.xml and registry.xml updates.

Committed 6738a/16145. Added support for incremental conversion when deleting a program.

• p2j.cfg.xml: The name_map.xml is hardcoded in the project (rules and java code), so I added

  <parameter name="name-map"         value="name_map.xml" />

  to retrieve it from the configuration.
• In Configuration.java, I added getParameterAsPath which I took from 6083a.
• ConversionDriver.java, enabled tracking to know when to save into the conversion database table file_signature
• NameMappingWorker.java, added deleteClassMappings() which works similar to the init and post rules of annotations.xml (had to add a few helpers and expose some methods for it to work)
• ConversionData.java, cleanupDeletedFiles() method is the most important since it deletes all associated resources with the deleted procedures.

Since I used a bit of code from 6083a and the changes are related to the recent incremental work, should I port the changes from 6738a to 6083a and test everything at once?

Code Review Task Branch 6738a Revision 16145

1. In ConversionData.cleanupDeletedFiles(), I wonder if the mgr.save(); should be moved outside of the loop, for performance reasons.

2. Shouldn't we move the name_map.xml into the cvt/ directory? I know that is a "breaking change", but it seems like the right thing to do.

Dănuț Filimon wrote:

Since I used a bit of code from 6083a and the changes are related to the recent incremental work, should I port the changes from 6738a to 6083a and test everything at once?

I have no objection to this.

I ported the changes from 6738a/16145 to 6083a/16155.

Greg Shah wrote:

Code Review Task Branch 6738a Revision 16145

1. In ConversionData.cleanupDeletedFiles(), I wonder if the mgr.save(); should be moved outside of the loop, for performance reasons.

I agree, there's also a mgr.removeTree(mgr.getTreeId(cvtFilePath)); call in 6083a which doesn't require a save as I noticed it is done a bit later.

2. Shouldn't we move the name_map.xml into the cvt/ directory? I know that is a "breaking change", but it seems like the right thing to do.

The name_map.xml is present is both the project root and the src/<pkgroot>. I agree that it should only be available in one place, but I am not sure why it needed to be duplicated in the first place. I will make the change and see if there are any issues.

Committed 6083a/16156. Added support for having the name_map.xml in the cfg folder (not cvt).

Greg, I think that the name_map.xml should be bundled with registry.xml and p2j.cfg.xml instead of being placed in the cvt folder.

• p2j.cfg.xml:

  <parameter name="name-map"         value="./cfg/name_map.xml" />

• build.xml:

         <!-- Copy p2j.cfg.xml* from src to equivalent location in build -->
         <copy todir="${build.home}/classes/cfg">
  -         <fileset dir="${cfg.home}" includes="p2j.cfg.xml* registry.xml" />
  +         <fileset dir="${cfg.home}" includes="p2j.cfg.xml* registry.xml name_map.xml" />
         </copy>
  

  and added

  <delete file="cfg/name_map.xml"/>
  

  to the clean.convert target.

Is there a good reason why we should have this file in the cvt folder and not cfg?

I discussed with Alex and he mentioned that name_map.xml should be placed in the cvt folder because it is an artifact generated during conversion, but registry.xml is also generated during conversion so shouldn't it be moved to the cvt folder as well? We should create a separate task to bring all files created during conversion in the cvt folder e.g. txt, log, xml

• p2j.cfg.xml:
  

   <parameter name="name-map"         value="./cvt/name_map.xml" />
  

• build.xml:
  Similar to registry.xml, added

  +      <copy todir="${build.home}/classes/cvt">
  +         <fileset dir="${cvt.home}" includes="name_map.xml" />
  +      </copy>
  

  In the target "jar", added cvt/*.xml and deleted <delete file="name_map.xml"/> from the clean.convert target.

Works the same with latest 6083a.

Let's have the name map default to ./cvt/name_map.xml and get the build.xml change put into all projects.

It makes no sense to have any artifacts of conversion in the project root (or in cfg/). All artifacts generated by conversion should be in cvt/. This includes the registry.

I discussed with Alex and he mentioned that name_map.xml should be placed in the cvt folder because it is an artifact generated during conversion, but registry.xml is also generated during conversion so shouldn't it be moved to the cvt folder as well?

Yes

We should create a separate task to bring all files created during conversion in the cvt folder e.g. txt, log, xml

That task was #5586.

Committed 6083a/16157. Added a default value for the name-map parameter, it can also be used in the rules.

Code Review Task Branch 6083a Revisions 16156 and 16157

The changes are fine except I don't think we want to have cvt/ added to the name map path in the jar. Just leave the jar location unchanged.

Greg Shah wrote:

Code Review Task Branch 6083a Revisions 16156 and 16157

The changes are fine except I don't think we want to have cvt/ added to the name map path in the jar. Just leave the jar location unchanged.

In this case the configuration will be:

+      <copy todir="${build.home}/classes">
+         <fileset dir="${cvt.home}" includes="name_map.xml" />
+      </copy>

How should I look for the name_map.xml in the jar if the expected path is ./cvt/name_map.xml?

I think this needs a change to SourceNameMapper.NAME_MAP_FILE_PATH, assigning Configuration.DEF_NAME_MAP_FILE instead of Configuration.getNameMapAsPath(). In this way, the name_map.xml will be found in the jar.

How should I look for the name_map.xml in the jar if the expected path is ./cvt/name_map.xml?

What I'm saying is that the expected path should not be ./cvt/name_map.xml. At runtime, we would not have any evidence of the cvt/ directory.

Greg Shah wrote:

How should I look for the name_map.xml in the jar if the expected path is ./cvt/name_map.xml?

What I'm saying is that the expected path should not be ./cvt/name_map.xml. At runtime, we would not have any evidence of the cvt/ directory.

I fixed this in 6738a/16158.

Code Review Task Branch 6083a Revisions 16158

I'm good with the change.

1. Go to the project.
2. Check #10536 for the corresponding configuration changes of the project.
3. Go the a .df and define a new table. - For #7793
4. Go to the sources and create a new procedure file with a small piece of code (a message or anything else is fine, as long as it will convert properly) - For #6738
5. Convert the application (so that the conversion database is populated with the right data)
6. Check the name_map.xml in build/classes/, app jar and ./cvt folder.
7. Run incremental conversion once (no changes, nothing should happen)
8. Go the the df and change a label for a field of any table defined. - For #6083
9. Run incremental conversion
10. Go to the df and delete the table that was added initially - For #7793
11. Run incremental conversion
12. Go to the project sources and delete the file that was added originally - For #6738
13. Run incremental conversion

I'll be testing classes and dependencies for #6738 separately.