FWD » Core Development » Conversion Tools

Multiple customers have noted that using the FWD conversion tools on an existing 4GL project is quite messy because:

• We assume the 4GL project structure must live inside our conversion project structure.
• We emit intermediate artifacts throughout the project structure and distressingly (to customers), alongside the original 4GL source code/schemata.

As customers move to develop 4GL code and test/debug/deploy in FWD, we need to make it much easier to do. One of the first steps is to eliminate the pain caused by our messiness. Plan:

1. Artifacts that are shared project-wide should be moved into the cvtdb. For example, the registry.xml and name_map.xml should be in the database instead of as .xml in the file system.
2. All per-source or per-schema file artifacts (e.g. .cache, .ast, .jast, .schema, .dict, .p2o...) should be emitted into a conversion artifact directory. This would be a configurable location which has a directory tree that mirrors the input structure of the project. In other words, if we are processing abl/myapplication/some/path/prog.p, then we would store related artifacts in <conversion_artifact_directory>/myapplication/some/path/prog.p.*.
3. Some conversion artifacts can be dropped by default. I'm thinking about the .parser and .lexer files here. They are of limited value in most circumstances and we can always enable them in an environment in the rare case they are needed. This will save some disk space in the common scenario. If these do get enabled, they should be emitted into the new conversion artifact directory.

Eventually, even the file-level artifacts will be in the database but for now I don't want to complicate the solution for this task.

I have found that prefixing inside com.goldencode.p2j.uast.JavaPatternWorker#generateJavaFilename(java.lang.String) the output directory does in fact move atleast the .jast artifacts for now and I've compared the contents and they are identical.

      return sb.insert(0, "cvt/").append(JAST_POSTFIX).toString();

It is a good test. Please note that while we do default to cvt/, we do allow this directory to be configured in p2j.cfg.xml. Make sure we honor the user's configuration if it is there.

See Configuration.getConversionFolder() which was implemented in #6203.

Today I've looked through possible implications this kind of change might have. I have reached TransformDriver and wondered, since here

   protected void convertSourceNamesToAstNames()
   {
      List<String> astFiles = new ArrayList<>();

      //TODO: change source to output or something
      astFiles.addAll(Arrays.asList(source.withSuffix(".ast")));

Greg Shah wrote:

It is a good test. Please note that while we do default to cvt/, we do allow this directory to be configured in p2j.cfg.xml. Make sure we honor the user's configuration if it is there.

See Configuration.getConversionFolder() which was implemented in #6203.

I kinda took this into consideration that we would want this to be configured and managed to reach something like:

   private static File createUniqueFile(String filename, String postfix)
   {
      Properties prop = new Properties();
      try
      {
         String src;
         String out;
         String buildPropertiesPath = Utils.findFileInPath("build.properties", new String[]{"."}).getAbsolutePath();
         prop.load(new FileInputStream(buildPropertiesPath));
         if (prop.getProperty("app.4gl.src") != null)
         {
            src = prop.getProperty("app.4gl.src");
         }
         else
         {
            src = "abl";
         }

         if (prop.getProperty("cvt.home") != null)
         {
            out = prop.getProperty("cvt.home");
         }
         else
         {
            out = "cvt";
         }
         filename = filename.replace(src, out);
      }
      catch (IOException e)
      {
      }

      return new File(filename + postfix);
   }

In the FWD code, we should not use any properties. So all of the build.properties stuff like app.4gl.src or cvt.home should not be used. These are things that:

1. Are outside of FWD. They are "conventions" but are not documented features of FWD. Instead we just happen to use them in our project templates. But customers can and do use anything other than these conventions.
2. Are things I'd like to kill MURDER in preference to explicit configuration in p2j.cfg.xml.

Only use configuration from p2j.cfg.xml. This means using Configuration.getConversionFolder() and the basepath from p2j.cfg.xml. If we need something else in p2j.cfg.xml, let's discuss it here and add it as needed.

Understood, will look on how to adjust to p2j.cfg.xml.

Managed to finish middle part of the conversion with the new output directory but I'm facing issues, meaning that temporary-tables are not found when com.goldencode.p2j.schema.P2OLookup#javaPropertyName(java.lang.String, boolean) is called for appsrv-tt.partition historic name.

Inside annotations/annotations my conversion fails when calling [java] javaname = p2o.javaPropertyName(getNoteString("schemaname"), false), while converting ./abl/common/adecomm/as-utils.w

Found the issue, forgot to put the suffix-adding code inside com.goldencode.p2j.schema.P2OLookup#loadTemporarySchema

String src;
      String out;
      src = Configuration.getParameter("basepath");
      try
      {
         out = Configuration.getConversionFolder();
      }
      catch (ConfigurationException e)
      {
         LOG.log(
         Level.SEVERE,
         "Failed to locate the \"cvtpath\" parameter in " + Configuration.DEF_CFG_FILE,
         e
         );
         throw new RetryUnwindException(e);
      }
      if (filename.contains(src))
      {
         filename = out + filename.split(src)[1];
      }
      filename += P2O_POSTFIX;

I've found inside frame_generator.xml the following function:

<function name="gen_fname">
         <parameter name="cname" type="java.lang.String" />
         <return name="fname"    init="''" />

         <rule>true
            <action>
               fname = file.substring(0, file.lastIndexOf(fileSep))
            </action>

            <action>fname = sprintf("%s%s%s", fname, fileSep, cname)</action>
         </rule>
      </function>

The method referenced in #5586-15 looks like this:

         filename = generateJavaFilename(filename);

         return persistJavaAst(filename, context.get().root);

Finalized conversion but now I get some errors when starting the client.

After conversion I'm missing these functions:

   @LegacySignature(type = Type.FUNCTION, name = "fwdGetMainFrame", returns = "HANDLE")
   public handle fwdGetMainFrame()
   {
      return function(this, "fwdGetMainFrame", handle.class, new Block((Body) () -> 
      {
         storeReturnValue(ftreeviewparentFrame.asWidgetHandle());
      }));
   }

   @LegacySignature(type = Type.FUNCTION, name = "fwdGetADMVersion", returns = "CHARACTER")
   public character fwdGetAdmversion()
   {
      return function(this, "fwdGetADMVersion", character.class, new Block((Body) () -> 
      {
         storeReturnValue("ADM2.2");
      }));
   }

Almost fixed the previous issue, now the code converts and i can start hotel_gui but i still have some comments left in the converted code:

 /* _UIB-CODE-BLOCK-END */   
   /* ********************  Preprocessor Definitions  ******************** */   
   /* Db-Required definitions. */   
   /* Custom List Definitions                                              */   
   /* List-1,List-2,List-3,List-4,List-5,List-6                            */   
   /* _UIB-PREPROCESSOR-BLOCK-END */   
   /************************  Function Prototypes**********************/   

   /* Db-Required definitions. */   
   /* Custom List Definitions                                              */   
   /* List-1,List-2,List-3,List-4,List-5,List-6                            */   

Greg, please let me know if you have any ideas about where should i look to fix this issue with the comments.

Stefan Vieru wrote:

Greg, please let me know if you have any ideas about where should i look to fix this issue with the comments.

I've found that, for example, hotel_gui has an existing file named directory.hints.
This type of file is generated inside of my output directory(will refer to it as cvt), but since there's no 4gl file named directory I don't have this file inside cvt.
I was wondering if I should look at any of those files if they exist already inside abl (basepath) and just copy them to cvt. What are some other files extensions I would look for to copy?

After copying directory.hints now conversion is almost the same, tested on hotel_gui, outside 2 differences:
FwdEmbeddedDriver.java

            Buffer.delete(pageToolbar, "upper(pname) = ?", toUpperCase(fwdLastWindow));

            Buffer.delete(pageToolbar, "upper(pageToolbar.pname) = ?", toUpperCase(fwdLastWindow));

         Buffer.delete(ttdcombo, "hviewer = ?", targetProcedure());
         Buffer.delete(ttlookup, "hviewer = ?", targetProcedure());

         Buffer.delete(ttdcombo, "ttdcombo.hviewer = ?", targetProcedure());
         Buffer.delete(ttlookup, "ttlookup.hviewer = ?", targetProcedure());

These are temp-tables.

Stefan Vieru wrote:

After copying directory.hints now conversion is almost the same, tested on hotel_gui, outside 2 differences:

This is a recent change in trunk. Try rebasing your branch.

.hints files are configuration files which are similar to "source" inputs. These should still reside in the source directories and they probably will be checked-in to the project itself. We don't want to copy these, but should read them from their original location.

Constantin Asofiei wrote:

Stefan Vieru wrote:

After copying directory.hints now conversion is almost the same, tested on hotel_gui, outside 2 differences:

This is a recent change in trunk. Try rebasing your branch.

Thanks, conversion now is identical for what I'm testing.
Will exclude the .hints files from my changes.

I created this function inside Configuration

   public static String getPathToConversionFolder(String filename, String postfix)
   {
      if (filename == null)
      {
         return null;
      }
      String basepath = Configuration.getParameter("basepath");
      if (filename != null && filename.contains(basepath))
      {
         String out;
         try
         {
            out = Configuration.normalizeFilename(Configuration.getConversionFolder());
         }
         catch (ConfigurationException e)
         {
            LOG.log(Level.SEVERE,
                    "Failed to locate the \"cvtpath\" parameter in " + Configuration.DEF_CFG_FILE,
                    e);
            throw new RetryUnwindException(e);
         }
         filename = filename.replace(basepath, out);
      }

      return filename + postfix;
   }

Good.

I created a branch 5586a and commited my changes at revision 15767.

Most of my changes revolve around utilizing the function i created which takes the path: ./<basepath>/filename and replaces basepath with the configured conversion output folder and a optional postfix, resulting in ./<cvt>/filename<postfix>. If basepath doesn't exist in the path, then the path is returned back as is.

Code Review Task Branch 5586a Revision 15767

1. In getPathToConversionFolder(String filename, String postfix) please check if postfix is null or "" (empty string) and if so, bypass the concatenation.

2. In getConversionFolder(), it seems like we can return a cvtpath (local var) that is different from cfg.cvtpath. I know you didn't change this but I want to make sure there is no bug here.

3. I don't think the change in TransformDriver.convertSourceNamesToAstNames() is OK. It doesn't change the existing list, it will build a new list based on the .ast files in the filesystem. But that isn't guaranteed to be the right set of ASTs. If the user only specified a subset of the project to convert, they will be surprised when all ASTs are now selected and processed.

4. The RootNodeList changes should be reverted. That file is a configuration file, not something that is generated. Normally we find it in the cfg/ directory.

I have made the suggested changes (rev 15768). About getConversionFolder() it searches for a key in a map and defaults to cvt and then builds the path from home and creates any needed folders, from what i've seen it works as expected, aside from the fact that it defaults to cvt and it doesn't take the path from p2j.cfg.xml.
Also i have a question about ant callgraph, does it run on hotel/hotel_gui as expected? I didn't manage to run it and check some of my changes there.

Managed to finish running callgraph without any errors on hotel with the following change:

=== modified file 'src/com/goldencode/p2j/uast/CallGraphHelper.java'
@@ -437,6 +438,17 @@
          // discard the .ast suffix
          if (filename.endsWith(".ast"))
          {
+            String basepath = Configuration.getParameter("basepath");
+            String cvtpath = null;
+            try
+            {
+               cvtpath = Configuration.normalizeFilename(Configuration.getConversionFolder());
+            }
+            catch (ConfigurationException e)
+            {
+               e.printStackTrace();
+            }
+            filename = filename.replace(cvtpath, basepath);
             filename = filename.substring(0, filename.length() - ".ast".length());
          }
          else

aside from the fact that it defaults to cvt and it doesn't take the path from p2j.cfg.xml

That will need to be fixed.

Callgraph works in Hotel GUI but may not in Hotel ChUI. Please also check analytics, including using custom searches.

I tried building the graph again but i am getting am error and can't seem to get past it:

    [java] Done adding include-files in 00:00:01
     [java] ./abl/activate-rooms-dialog.w
     [java] Use default relation delimiter: -
     [java] WARNING: creating a new node for the include file ./abl/common/get, which was not found in initial include-file list.
     [java] WARNING: creating a new node for the include file ./abl/common/set, which was not found in initial include-file list.
     [java] WARNING: creating a new node for the include file ./abl/common/fn, which was not found in initial include-file list.
     [java] Evicted [137@c0a802df20058-sv1] from cache but waiting too long for transactions to close. Stale transaction alert on: [standardjanusgraphtx[0x010e4cc6]]
     [java] Evicted [136@c0a802df20058-sv1] from cache but waiting too long for transactions to close. Stale transaction alert on: [standardjanusgraphtx[0x010e4cc6]]
     [java] EXPRESSION EXECUTION ERROR:
     [java] ---------------------------
     [java] thisV = cgw.createAstNode(container, this, prog.kw_on, true)
     [java] Elapsed job time:  00:06:10.169
     [java]             ^  { Invalid vertex provided: null }
     [java] ---------------------------
     [java] 
     [java] com.goldencode.p2j.pattern.TreeWalkException: ERROR!  Active Rule:
     [java] -----------------------
     [java]       RULE REPORT      
     [java] -----------------------
     [java] Rule Type :   WALK
     [java] Source AST:  [ ON ] BLOCK/STATEMENT/KW_ON/ @3875:1 {12884913811}
     [java] Copy AST  :  [ ON ] BLOCK/STATEMENT/KW_ON/ @3875:1 {12884913811}
     [java] Condition :  thisV = cgw.createAstNode(container, this, prog.kw_on, true)
     [java] Loop      :  false
     [java] --- END RULE REPORT ---
     [java] 
     [java] 
     [java] 
     [java]     at com.goldencode.p2j.pattern.PatternEngine.run(PatternEngine.java:1101)
     [java]     at com.goldencode.p2j.uast.CallGraphGenerator.generateGraphs(CallGraphGenerator.java:184)
     [java]     at com.goldencode.p2j.uast.CallGraphGenerator.main(CallGraphGenerator.java:410)
     [java] Caused by: com.goldencode.expr.ExpressionException: Expression execution error @1:13 [KW_ON id=12884913811]
     [java]     at com.goldencode.p2j.pattern.AstWalker.walk(AstWalker.java:275)
     [java]     at com.goldencode.p2j.pattern.AstWalker.walk(AstWalker.java:210)
     [java]     at com.goldencode.p2j.pattern.PatternEngine.apply(PatternEngine.java:1693)
     [java]     at com.goldencode.p2j.pattern.PatternEngine.processAst(PatternEngine.java:1579)
     [java]     at com.goldencode.p2j.pattern.PatternEngine.processAst(PatternEngine.java:1512)
     [java]     at com.goldencode.p2j.pattern.PatternEngine.run(PatternEngine.java:1064)
     [java]     ... 2 more
     [java] Caused by: com.goldencode.expr.ExpressionException: Expression execution error @1:13
     [java]     at com.goldencode.expr.Expression.execute(Expression.java:495)
     [java]     at com.goldencode.p2j.pattern.Rule.apply(Rule.java:500)
     [java]     at com.goldencode.p2j.pattern.Rule.executeActions(Rule.java:751)
     [java]     at com.goldencode.p2j.pattern.Rule.coreProcessing(Rule.java:717)
     [java]     at com.goldencode.p2j.pattern.Rule.apply(Rule.java:537)
     [java]     at com.goldencode.p2j.pattern.RuleContainer.apply(RuleContainer.java:597)
     [java]     at com.goldencode.p2j.pattern.RuleSet.apply(RuleSet.java:98)
     [java]     at com.goldencode.p2j.pattern.AstWalker.walk(AstWalker.java:262)
     [java]     ... 7 more
     [java] Caused by: java.lang.IllegalArgumentException: Invalid vertex provided: null
     [java]     at com.google.common.base.Preconditions.checkArgument(Preconditions.java:220)
     [java]     at org.janusgraph.graphdb.vertices.AbstractVertex.addEdge(AbstractVertex.java:178)
     [java]     at org.janusgraph.graphdb.vertices.AbstractVertex.addEdge(AbstractVertex.java:43)
     [java]     at com.goldencode.p2j.uast.CallGraphWorker$CallGraph.createEdge(CallGraphWorker.java:1391)
     [java]     at com.goldencode.p2j.uast.CallGraphWorker$CallGraph.createAstNode(CallGraphWorker.java:1224)
     [java]     at com.goldencode.expr.CE98.execute(Unknown Source)
     [java]     at com.goldencode.expr.Expression.execute(Expression.java:398)
     [java]     ... 14 more
     [java] 

I wonder if the call graph was actually being created on prior runs. You'll have to debug through that code unfortunately. The error itself doesn't give use much in the way of a clue. I guess I'd be looking for some code that is trying to load an AST and can't find it.

Callgraph command on hotel GUI with the following rootlist.xml :

<?xml version="1.0"?>

<!-- Root Node List -->
<roots>

  <node filename="./cvt/start.p.ast" />
  <node filename="./cvt/emain.p.ast" />
  <node filename="./cvt/ehotel.p.ast" />
  <node filename="./cvt/fwd-embedded-driver.p.ast" />
  <node filename="./cvt/ladder-safety-gui.p.ast" />

</roots>

     [java] Caused by: com.goldencode.p2j.pattern.UserGeneratedException: The target for call-site-key KW_LOADCTRL from 180388639180 is null! [COM_PARAMETER id <180388639182> 0:0]
     [java]     at com.goldencode.p2j.pattern.CommonAstSupport$Library.throwException(CommonAstSupport.java:3745)
     [java]     at com.goldencode.p2j.pattern.CommonAstSupport$Library.throwException(CommonAstSupport.java:3730)
     [java]     at com.goldencode.expr.CE2034.execute(Unknown Source)
     [java]     at com.goldencode.expr.Expression.execute(Expression.java:398)
     [java]     ... 33 more

But when running callgraph with only:

<?xml version="1.0"?>

<!-- Root Node List -->
<roots>

  <node filename="./cvt/emain.p.ast" />
  <node filename="./cvt/ehotel.p.ast" />
  <node filename="./cvt/ladder-safety-gui.p.ast" />

</roots>

While debugging today I have found that what's failing right now the callgraph is this :

guests-spreadsheet.w@292:   chCtrlFrame:F1Book:LoadControls("", "spreadsheet":U).

Caused by: com.goldencode.p2j.pattern.UserGeneratedException: The target for      call-site-key KW_LOADCTRL from 180388639180 is null! [COM_PARAMETER id <180388639182> 0:0     ]    

Stefan Vieru wrote:

While debugging today I have found that what's failing right now the callgraph is this :
[...]
It fails because LoadCOntrols as a first argument it takes a file name but here is empty, leading to this Exception during callgraph generation:

Try instead of throwException to just use printf.

Constantin Asofiei wrote:

Stefan Vieru wrote:

While debugging today I have found that what's failing right now the callgraph is this :
[...]
It fails because LoadCOntrols as a first argument it takes a file name but here is empty, leading to this Exception during callgraph generation:

Try instead of throwException to just use printf.

Now i get this error:

     [java] Caused by: java.lang.RuntimeException: More than one node found with key [filename] and value [null]!
     [java]     at com.goldencode.p2j.uast.CallGraphHelper.findUniqueNode(CallGraphHelper.java:278)
     [java]     at com.goldencode.p2j.uast.CallGraphWorker$CallGraph.findUniqueNode(CallGraphWorker.java:2247)
     [java]     at com.goldencode.expr.CE1730.execute(Unknown Source)
     [java]     at com.goldencode.expr.Expression.execute(Expression.java:398)
     [java]     ... 30 more

Since i have not a print not a throw, it goes further and target doesnt have the name of the file:

         <rule>target == null or target.trim().length() == 0
            <action>
               printf("The target for call-site-key %s from %d is null!",
                                      this.lookupTokenName(callSiteKey), callSite.id))
            </action>
         </rule>

         <rule>nodeKey = #(java.lang.String) execLib("node_key", callSiteKey)</rule>

         <rule>nodeKey.equals("filename")
            <action>target = cgw.prepareFilename(target)</action>
         </rule>
         <rule>in = cgw.findUniqueNode(nodeKey, target)</rule>

I tried fixing #5586-41 and during debugging I've set the argument for prepareFilename from "" to "./abl/guests-spreadsheet.w" and callgraph finished. I'm not sure that's how this issue should be handled.

I've modified this in callgraph_lib.rules

         <rule>target == null or target.trim().length() == 0
            <action>
               printf("The target for call-site-key %s from %d is null!",
                                      this.lookupTokenName(callSiteKey), callSite.id))
            </action>
            <action>
               target = this.filename
            </action>
         </rule>

Greg Shah wrote:

1. Artifacts that are shared project-wide should be moved into the cvtdb. For example, the registry.xml and name_map.xml should be in the database instead of as .xml in the file system.

I would like to ask how should i handle this. From what I understand I'll need to create some tables inside the db in cvtdb to store the details from registry.xml and name_map.xml. Are there some things to look out for when structuring those tables?

registry.xml seems like the structures for mappings contains: flags, id, next, treename
name_map.xml has path-mapping, class-mapping, method-mapping, parameter

I see that in XmlFilePlugin, where we create the registry.xml file, we have

      for (Long current : fids.keySet())
      {
         Pair pair = fids.get(current);

         Element map = dom.createElement(ELEM_MAP);
         map.setAttribute(ATTR_ID, current.toString());
         map.setAttribute(ATTR_TREE, pair.name);
         map.setAttribute(ATTR_NEXT, Long.toString(pair.next));
         if (pair.flags != 0)
         {
            map.setAttribute(ATTR_FLAGS, Long.toString(pair.flags)); //default is 0
         }

         root.appendChild(map);
      }

After looking in name_map i though about the following tables:

CREATE TABLE PATH_MAPPING (
   path_name    VARCHAR(255)
);

CREATE TABLE PARAMETER (
    id          VARCHAR(64) NOT NULL IDENTITY PRIMARY KEY,
    jname       VARCHAR(64),
    mode        VARCHAR(64),
    pname       VARCHAR(64),
    table_      VARCHAR(64),
    buffer      VARCHAR(64),
    for_        VARCHAR(64), 
    type        VARCHAR(64)
);

CREATE TABLE METHOD_MAPPING (
    id          VARCHAR(64) NOT NULL IDENTITY PRIMARY KEY,
    in_super    VARCHAR(64),
    jname       VARCHAR(64),
    libname     VARCHAR(64),
    persistent  VARCHAR(64),
    pname       VARCHAR(64),
    private     VARCHAR(64),
    returns     VARCHAR(64),
    type        VARCHAR(64)
);

CREATE TABLE CLASS_MAPPING (
    in_super    VARCHAR(64),
    jname       VARCHAR(64) PRIMARY KEY,
    pname       VARCHAR(64),
    libname     VARCHAR(64),
    persistent  VARCHAR(64),
    FOREIGN KEY (pname) REFERENCES PATH_MAPPING(path_name)
);

1. Artifacts that are shared project-wide should be moved into the cvtdb. For example, the registry.xml and name_map.xml should be in the database instead of as .xml in the file system.

Please create a separate task for this. I don't want to work on this part yet. We are rewriting our AST classes and some of this configuration will potentially change anyway.

Okay, will let this aside then. Will look into dropping .lexer and .parser.
I see that those filed are enabled here

   public static void scan(FileList files, boolean silent, boolean abortOnError, int threads)
   throws Exception
   {
      scan(files, true, true, true, true, true, silent, abortOnError, threads);
   }

Lexer and Parser are 4th and 5th. So changing to

      scan(files, true, true, false, false, true, silent, abortOnError, threads);

Made the following change:

   public static void scan(FileList files, boolean silent, boolean abortOnError, int threads)
   throws Exception
   {
      boolean generateLex = Configuration.getParameter("generate-lexer", false);
      boolean generateParse = Configuration.getParameter("generate-parser", false);
      scan(files, true, true, generateLex, generateParse, true, silent, abortOnError, threads);
   }

Updated this, ref 15808.
Added some changes that basically checks if:
basepath is at the beginning of the filename and replace it with cvtpath
Example:
basepath = "." filename="start.p" it can replace it properly now, resulting in ./cvt/start.p
basepath = "./abl" filename="abl/start.p this case aswell.

Also, P2OLookup.loadTemporarySchema in the past when it encountered the filename argument as null the function used it anyway, resulting in null.p2o, so i had to adapt to this behavior.

Code Review Task Branch 5586a Revisions 15804 through 15808

1. In Configuration.getPathToConversionFolder(), I don't understand the use of RetryUnwindException. That is a 4GL legacy compatibility class only used at runtime. It seems inappropriate for this usage which is at conversion time.

2. In Configuration.withFileProfile(), won't we get an NPE on the String.replace() when a ConfigurationException is thrown in getConversionFolder()? There is the same issue in PreprocessorHints.loadHints(), CallGraphHelper.loadFileNodes(). I would prefer if these used common helper code AND if it would never fail to provide a safe default.

3. In CallGraphGenerator.generateGraphs(), shouldn't the code be pe = new PatternEngine(basePath + "data/namespace", "*.schema"); instead of pe = new PatternEngine(basePath, "*.schema");?

I'm looking into 3 but I can't find an example where namespace dir is used. I see in testcases that it is referenced in build.xml under clean.convert but other than that, in p2j.cfg we have the namespaces there, but all imports have data/name_of_db.

My point here is that the generated schema artifacts should also reside inside the configured convert directory (or in cvt/data/name_of_db/ by default). On the other hand, the schema inputs (e.g. .df files@ should be wherever the p2j.cfg.xml namespace configuration specifies.

Well, the changes on the branch already generate the .schema, .p2o and .dict files inside cvt/data, with the schema input files being wherever p2j.cfg indicates.
I can't understand the namespace part in data/namespace. I saw #4105-10 and #4105-26 which have namespace in there.

Well, the changes on the branch already generate the .schema, .p2o and .dict files inside cvt/data, with the schema input files being wherever p2j.cfg indicates.

That's good, but the callgraph needs to ensure it loads those files from wherever they are generated.

I can't understand the namespace part in data/namespace. I saw #4105-10 and #4105-26 which have namespace in there.

Historically we have placed the .df files in data/ and the .dict/.schema files were generated in data/namespace/. When they were moved to honor the cvt/ directory (or its configured location), we just left the namespace part in place.

• provided safe default options in case of exceptions
• adding getPathToSourceFileFromConversionFolder()
• the patternEngine call is done with basePath + "data/namespace"
• fix situation when preprocessorHints would have a NPE

Greg Shah wrote:

Code Review Task Branch 5586a Revisions 15809 through 15811

The changes are good.

Minor change: please create a static member of Configuration for DEFAULT_CVT and assign it "." + File.separator + "cvt" + File.separator. Then use this in getPathToConversionFolder() and getPathToSourceFileFromConversionFolder().

Rebase the branch and it needs to be conversion tested with multiple applications.

I created a commit and rebased the branch. 5586a/15867

While converting big customer application I have an error related to a temp-table, unexpected token error.

After some heavy debugging I've found that this temp table ("tt_kolom") is not found in SchemaDictionary.scopeMap.
It seems that the issue is somehow related to SchemaDictionary and this temp table not being added there.

If this error only happens in 5586a then it is a real regression.

Adding the temp-table to the schema dictionary is done during parsing, when the temp-table is defined (the DEFINE TEMP-TABLE statement).

I have found the issue, inside SchemaWorker

    public static SchemaException loadFromPersistence(SchemaDictionary dictionary, String file)
    {
-      String fname = adjustFilename(file, AstGenerator.DICT_POSTFIX);
+      String fname = Configuration.getPathToConversionFolder(file, AstGenerator.DICT_POSTFIX);
       File   fil   = new File(fname);
       if (fil.exists())

I will also add this change:

-      if (postfix != null && !postfix.isEmpty())
+      if (postfix != null && !postfix.isEmpty() && !filename.endsWith(postfix))

It seems that some .rules files are using %s.ast and that string parameter is the source file of the ast, meaning that .rules file passed to a function a .ast file that's not in conversion folder.
I have adjusted this issue.

After some changes I am now facing this issue:

 ./abl/src/--/src/msc/apps/mapwhereclause.cls
     [java] EXPRESSION EXECUTION ERROR:
     [java] ---------------------------
     [java] javaname = p2o.javaPropertyName(getNoteString("schemaname"), false)
     [java]                ^  { No java name found for legacy name:  tt_token.tokentype }
     [java] ---------------------------
     [java] Elapsed job time:  00:01:32.854
     [java] ERROR:
     [java] com.goldencode.p2j.pattern.TreeWalkException: ERROR!  Active Rule:
     [java] -----------------------
     [java]       RULE REPORT      
     [java] -----------------------
     [java] Rule Type :   WALK
     [java] Source AST:  [ tt_token.tokentype ] BLOCK/CLASS_DEF/BLOCK/METHOD_DEF/BLOCK/INNER_BLOCK/KW_FOR/RECORD_PHRASE/KW_WHERE/EXPRESSION/EQUALS/FIELD_CHAR/ @46:33 {43967580209480}
     [java] Copy AST  :  [ tt_token.tokentype ] BLOCK/CLASS_DEF/BLOCK/METHOD_DEF/BLOCK/INNER_BLOCK/KW_FOR/RECORD_PHRASE/KW_WHERE/EXPRESSION/EQUALS/FIELD_CHAR/ @46:33 {43967580209480}
     [java] Condition :  javaname = p2o.javaPropertyName(getNoteString("schemaname"), false)

[java] ./abl/src/---/src/sys/pitstop/pitstop_engine.cls
[java] ERROR: can't emit parameter for parameter [PARAMETER]:58836756988051 @0:0
[java]       (isoo=false, type=1771, parmtype=706)
[java]    logical [KW_LOGICAL]:58836756988052 @24:60

Seems that #5586-68 occurs even when the build is passing so this issue wasn't introduced with this branch.

• on trunk we have the following:
  

        filename += P2O_POSTFIX;
        filename = Configuration.normalizeFilename(filename);
  

  
  at the beginning of the function, which results in the filename being ./cvt/data/hotel.p2o.p2o
• with my changes we get the following: ./cvt/data/hotel.p2o which results then inside P2OLookup.dynamicGet() the if to be executed

What would be the correct thing to do in this situation?

At Generate Data Model Objects (database DMOs) step, inside loadTemporarySchema when the filename is ./cvt/data/hotel.p2o

• on trunk we have the following:
  [...]
  at the beginning of the function, which results in the filename being ./cvt/data/hotel.p2o.p2o

This name is certainly wrong, but it probably accidentally "works" because ./cvt/data/hotel.p2o.p2o doesn't exist and thus it will be bypassed when P2OWorker.loadTemporarySchema() calls P2OLookup.dynamicGet().

• with my changes we get the following: ./cvt/data/hotel.p2o which results then inside P2OLookup.dynamicGet() the if to be executed
  What would be the correct thing to do in this situation?

I think that P2OWorker.loadTemporarySchema() should not be called for the non-temp P2O files.

Eric/Ovidiu: How do we fix this latent bug?

Eric/Ovidiu: How do we fix this latent bug?

To me it looks like the P2OWorker is used from the rules at Generate Data Model Objects phase. This P2OWorker was meant to load the temporary schema of the current file, but in this phase, the current file is the actual p2o. From my POV, it is correct to append a P2O_POSTFIX and search for the file. In fact, it shouldn't exist, as there is no p2o generated for p2o files. From my POV, normalizeFilename should keep the double p2o extension.

The alternative is not calling loadTemporarySchema if the target AST didn't generate a p2o. But in other terms, this is synonym with "there is no .p2o" file associated. So I can't tell if there is in fact an alternative way to handle this rather than actual doing a file-system check.

The double p2o extension is not a bug, but a mechanism to search for a p20 file which in this specific case should fail (aka there is no temporary schema defined in hotel.p2o).

This is not a latent bug; it is a feature that was removed in #5586.

The hotel.p2o is the persistent database and should not be loaded by loadTemporarySchema() because that code is specific to temp-tables. To do otherwise is confusing, especially since it relies upon manipulating the filename directly. This is not something that we want to retain.

Greg Shah wrote:

The hotel.p2o is the persistent database and should not be loaded by loadTemporarySchema() because that code is specific to temp-tables. To do otherwise is confusing, especially since it relies upon manipulating the filename directly. This is not something that we want to retain.

• unhook it from non-procedure ASTs OR
• check if the current AST is a p2o AST and avoid calling loadTemporarySchema for it.

The question is now - why would we need a P2OWorker when we process a p2o?

check if the current AST is a p2o AST and avoid calling loadTemporarySchema for it.

This would work, though:

• it is not as obvious as checking to see if this is a persistent database schema
• with #7180 we are moving away from direct knowledge of the filenames being processed, so we will still have to deal with this issue again later if we don't move away from the knowledge of the .p2o now

Addressing this .p2o issue, in the hopes that I understood correctly, my changes to visitAst would be:

-      String fileName = ast.getFilename();
-      P2OLookup.loadTemporarySchema(fileName, clsDef);
+      if (!ast.isAnnotation("permanent") || !ast.getAnnotation("permanent").equals(true)))
+      {
+         String fileName = ast.getFilename();
+         P2OLookup.loadTemporarySchema(fileName, clsDef);
+      }

Let me know if this change is what's intended or not.

Question about .jast files: should they be moved aswell into artifact directory, or deleted afterwards if they're not needed after conversion?
they are currently generated at the root level in the same path that would be the java package (for hotel com.goldencode.hotel)

Stefan Vieru wrote:

Question about .jast files: should they be moved aswell into artifact directory, or deleted afterwards if they're not needed after conversion?
they are currently generated at the root level in the same path that would be the java package (for hotel com.goldencode.hotel)

There are .jast files generated for procedures which are already moved. I want to clarify that this is about DMO *.jast that are created artificially (without a source file).

Stefan Vieru wrote:

Addressing this .p2o issue, in the hopes that I understood correctly, my changes to visitAst would be:
[...]
I'm moving away from checking the file name and also only entering here when we have a non-persistent database.
This is the case for both com.goldencode.p2j.schema.P2OAccessWorker#visitAst and com.goldencode.p2j.convert.IndexSelectionWorker#visitAst.

Let me know if this change is what's intended or not.

Yes, it looks good.

Stefan Vieru wrote:

Question about .jast files: should they be moved aswell into artifact directory,

Yes. Being able to see these is very useful for debugging. Customers might even find it useful by writing TRPL rules to analyze or transform the Java ASTs.

or deleted afterwards if they're not needed after conversion?

No.

they are currently generated at the root level in the same path that would be the java package (for hotel com.goldencode.hotel)

Yes, this is the wrong place for .jast files. Likewise for any .jast files that go with 4GL code (which appear alongside the 4GL code today). They should be with the rest of the artifacts in the "convert" directory (e.g. usually ./cvt/).

• every .ast, .cache, ... string reference I made is now replaced by it's counterpart from AstGenerator: AstGenerator.AST_POSTFIX, AstGenerator.CACHE_POSTFIX, ...(except for JAST since it's not in AstGenerator)
• missed a spot where an ast was looking for the inherited parent's properties(for temp-tables and such) and corrected it
• moved the generated .jast files into the artifacts directory (previously they we're generated in the root directory)

Greg, I will put this back into review, let me know if there's anything I need to change.

I tested this with the two customer applications. Both have passed with no issues during deploy.

Have you re-tested Hotel GUI (including analytics and callgraph)?

Please make sure to test conversion of the ChUI regression suite.

I already checked callgraph and it works.
Regarding analytics I had an issue where it would look for rootnodes inside ./deploy/server/cvt ... due to normalizeFilename inside getPathToSourceFileFromConversionFolder.
To fix this I just added getPathToSourceFileFromConversionFolder(String filename, String ext, boolean relative) which normalizes the file if the relative flag is false.

Will look into the last point.

New revno: 15902

I used the regression suite, should i upload the results here for any support? i have 6 fails
I built the tests and ran them with jdk17.

• [...]

You can post in #7823 for help on that test suite.

I have looked at 5586a / r15973.

While I agree with using the constant suffixes for file types, I do not think we should involve Configuration in these two classes. The wa.dynQueryFile, wa.jastFile, wa.astFile, wa.p2oFile and wa.dictFile are virtual files. They are not actually written to disk. The suffixes are also irrelevant from this point of view. In fact, these file names are just keys in a map of InMemoryRegistryPlugin. As opposed to static conversion where the resource files are indeed written/read from the storage by XmlFilePlugin. Remember that code in these classes execute at runtime; normally, there is no static conversion environment defined in the cfg/p2j.cfg.xml.

A second issue, I do not encourage usage of static imports of method. It is difficult for a human reader to quickly identify where the method belongs to; the natural instinct is to assume the current class, which is incorrect in this case. So my choice is Configuration.getPathToConversionFolder(file, suffix).

Ovidiu Maxiniuc wrote:

I have looked at 5586a / r15973.

While I agree with using the constant suffixes for file types, I do not think we should involve Configuration in these two classes. The wa.dynQueryFile, wa.jastFile, wa.astFile, wa.p2oFile and wa.dictFile are virtual files. They are not actually written to disk. The suffixes are also irrelevant from this point of view. In fact, these file names are just keys in a map of InMemoryRegistryPlugin. As opposed to static conversion where the resource files are indeed written/read from the storage by XmlFilePlugin. Remember that code in these classes execute at runtime; normally, there is no static conversion environment defined in the cfg/p2j.cfg.xml.

A second issue, I do not encourage usage of static imports of method. It is difficult for a human reader to quickly identify where the method belongs to; the natural instinct is to assume the current class, which is incorrect in this case. So my choice is Configuration.getPathToConversionFolder(file, suffix).

Okay, considering this I reverted the changes and added the following check inside com.goldencode.p2j.cfg.Configuration#getPathToConversionFolder(java.lang.String, java.lang.String)

      if (filename.startsWith("./in-mem-p2j-query") || filename.startsWith("./in-mem-valexp"))
      {
         return filename + postfix;
      }

And on a local test with a dynamic query it works. Please let me know what you think of this approach.

I think you should simply ignore these locations. These "in-mem" files are volatile resources, not related to static conversion / not producing any conversion artifact.

I do not see any update on branch, the last revision is still r15973.

Ovidiu Maxiniuc wrote:

I think you should simply ignore these locations. These "in-mem" files are volatile resources, not related to static conversion / not producing any conversion artifact.

I do not see any update on branch, the last revision is still r15973.

I didn't create a commit since I wasn't sure if it was the right approach. Will commit now with only a startsWith("./in-mem") since I guess all the virtual/volatile resources start with in-mem from what I'm understanding.

Created r15974 with the following changes:

=== modified file 'src/com/goldencode/p2j/cfg/Configuration.java'
@@ -668,6 +669,10 @@
       {
          return null + postfix;
       }
+      if (filename.startsWith("." + File.separator + "in-mem"))
+      {
+         return filename + postfix;
+      }
       filename = normalizeFilename(filename);
       String basepath = normalizeFilename(getParameter("basepath")) + File.separator;
       String cvtpath = DEFAULT_CVT;

=== modified file 'src/com/goldencode/p2j/persist/DynamicQueryHelper.java'
@@ -397,10 +397,10 @@
          wa.buffers      = allBuffers;
          wa.queryId      = queryId;
          wa.dynQueryFile = "." + File.separator + "in-mem-p2j-query" + queryId + ".p";
-         wa.jastFile     = getPathToConversionFolder(wa.dynQueryFile, JavaPatternWorker.JAST_POSTFIX);
-         wa.astFile      = getPathToConversionFolder(wa.dynQueryFile, AstGenerator.AST_POSTFIX);
-         wa.p2oFile      = getPathToConversionFolder(wa.dynQueryFile, P2OLookup.P2O_POSTFIX);
-         wa.dictFile     = getPathToConversionFolder(wa.dynQueryFile, AstGenerator.DICT_POSTFIX);
+         wa.jastFile     = wa.dynQueryFile + JavaPatternWorker.JAST_POSTFIX;
+         wa.astFile      = wa.dynQueryFile + AstGenerator.AST_POSTFIX;
+         wa.p2oFile      = wa.dynQueryFile + P2OLookup.P2O_POSTFIX;
+         wa.dictFile     = wa.dynQueryFile + AstGenerator.DICT_POSTFIX;

=== modified file 'src/com/goldencode/p2j/persist/DynamicValidationHelper.java'
@@ -196,10 +195,10 @@
          wa.dynExprFile = "." + File.separator + "in-mem-valexp" + valExprId + ".p";
-         wa.jastFile     = getPathToConversionFolder(wa.dynExprFile, JavaPatternWorker.JAST_POSTFIX);
-         wa.astFile      = getPathToConversionFolder(wa.dynExprFile, AstGenerator.AST_POSTFIX);
-         wa.p2oFile      = getPathToConversionFolder(wa.dynExprFile, P2OLookup.P2O_POSTFIX);
-         wa.dictFile     = getPathToConversionFolder(wa.dynExprFile, AstGenerator.DICT_POSTFIX);
+         wa.jastFile     = wa.dynExprFile + JavaPatternWorker.JAST_POSTFIX;
+         wa.astFile      = wa.dynExprFile + AstGenerator.AST_POSTFIX;
+         wa.p2oFile      = wa.dynExprFile + P2OLookup.P2O_POSTFIX;
+         wa.dictFile     = wa.dynExprFile + AstGenerator.DICT_POSTFIX;

Also added r15975 which removes a redundant null check from getPathToConversionFolder. This check has been moved in P2OAccessWorker, and IndexSelectionWorker respectively, since temp tables are calling visitAst with a null ast.filename.

Stefan, I mentioned in our weekly meeting that I plan to run a conversion on a customer project. At the moment, the instance is down and I can't run this conversion.

Created r15974 with the following changes:

Please add comments to these code locations to explain that // these in-memory use cases don't access the filesystem; we intentionally bypass normal artifact management.

While testing the latest revision I have found that there are many places that might call getPathToConversionFolder with a null filename.
I will revert the change, and it will look as follows:

      if (filename == null)
      {
         // filename argument is null, so we keep the post convention of adding the postfix to null
         // will also print the method which appends this in case of debugging
         LOG.log(Level.WARNING, "WARNING: getPathToConversionFolder has been called with a null filename.");
         StackTraceElement[] stackTrace = Thread.currentThread().getStackTrace();
         for (StackTraceElement element : stackTrace) {
            LOG.log(Level.WARNING, element.toString());
         }

         return "null" + postfix;
      }

I have found that there are many places that might call getPathToConversionFolder with a null filename

This seems like a bug. Even if it is intentional, returning "null" + postfix is confusing. What is the list of locations that do this?

The functions in #5586-77, the visitAst methods.
In the case of com.goldencode.p2j.uast.SymbolResolver#loadClassDefinition(com.goldencode.p2j.uast.SymbolResolver, java.lang.String) we have:

      String file = ConversionData.findClassFilename(qname);

In those cases, we should not call getPathToConversionFolder(). Just bypass it when the input is null. In other words, the callers need to be smarter instead of passing back a non-null string that makes no sense.

Okay, will check for null calls and make sure these are bypassed. I kept the null checks in visitAst which have been discovered until now.
Should I still let the logging in #5586-101 for catching those cases in the future when they appear? When that occurs, the call would need to be updated with the bypass. I will change in #5586-101 to just return null.

Or maybe enable this logging when in p2j.cfg.xml there is a "debugging" parameter set on true.

Returning null will likely result in an NPE and we will see the stack trace already.

Got it, will remove this logging and will retain the null checks that have been found until now. I will create a commit and I have this converting on two customer applications.

I have updated: r15976

Stefan, is there anything else you need to add to the branch? I want to pick this up and run a customer conversion.

You can start, I have nothing to add for now.

The project was converted and built successfully.

Greg, all regression testing passed. 5586a can be merged.

Please remind me: is there any requirement for scripting or configuration changes in the project cfg?

From what I remember no, in p2j.cfg the cvtpath can be configured, but by default it has cvt.

But don't we need to change the project-level build.xml to process things like clean in a different location?

Greg Shah wrote:

But don't we need to change the project-level build.xml to process things like clean in a different location?

• archive.incremental
• clean.convert
• rpt-no-front

These need changing but the clean removes the cvt directory, so all those artifacts are deleted, and from basepath nothing is deleted. So I think clean is fine.
How will we proceed?

Since we must provide guidance to customers on this shift, it is important that we prepare documentation and examples of the changes that need to be made.

For those projects where we maintain the configuration/scripts, we can make the changes directly and just point the customers to those changes which need to be picked up when the associated trunk revision is reached.

For those projects where the customer maintains the confg/scripts, we must offer the documentation and examples to explain the change they must implement when the associated trunk revision is reached.

Where will this documentation be? In this issue or on a different page?
I will look into which ant commands are impacted by this change.

Start here and we will see where we move it.

• rpt-no-front - this ant command uses ReportDriver, which in turn may take a list of .ast files or directory as arguments;
• achive-debug - the output of the command is an archive with multiple artifacts useful for debugging, we have to adjust where to take the .cache and .pphints files
• archive.incremental - same as the previous command, but here .ast, .jast, .schema and .p2o files are added
• rpt.dist - we need to add the artifacts directory here, because these files are used at the report generation stage
• clean.convert - this contains a fileset with all artifacts, since these have been moved, the fileset root needs to be changed

The artifacts directory can be configured from p2j.cfg.xml by setting the parameter cvtpath. FWD does have a safe default in this sense, having cvt as a default with no configuration needed in p2j.cfg.xml.
The change in build.xml would be replacing the {app.4gl.src} (or whatever is the source of the abl code) from the above commands, but also other commands that might use the artifacts mentioned in #5586-1, with the path configured earlier in p2j.cfg.xml.

As an example:
p2j.cfg.xml (this is optional if we want cvt directory by default)

<cfg>
<global>
...
      <!-- conversion output -->
      <parameter name="output-root"      value="./src" />
      <parameter name="pkgroot"          value="com.goldencode.hotel" /> 
+     <parameter name="cvtpath"          value="./cvt" />
...

build.xml

<target name="clean.convert" ...>
...
      <delete>
-        <fileset dir="${app.4gl.src}" 
+        <fileset dir="./cvt" 
                  includes="**/*.pphints 
                            **/*.cache
                            **/*.lexer 
                            **/*.parser 
                            **/*.ast
                            **/*.ast.original
                            **/*.jast 
                            **/*.dict
                            **/*.p2o
                            **/*.schema
                            **/*.log" />
      </delete>
...
</target>

Now the generation of .lexer and .parser files is disabled by default, enabling it must be done from p2j.cfg.xml:

      <parameter name="generate-lexer"   value="true" />
      <parameter name="generate-parser"   value="true" />

Greg, please let me know if I need to do anything else on this issue and if it's ready to be merged.

Stefan Vieru wrote:

Greg, please let me know if I need to do anything else on this issue and if it's ready to be merged.

Please complete the changes for #10189, then we can merge.

I had to update a small part of the getPathToConversionFolder method:

=== modified file 'src/com/goldencode/p2j/cfg/Configuration.java'
--- old/src/com/goldencode/p2j/cfg/Configuration.java    2025-06-17 14:10:02 +0000
+++ new/src/com/goldencode/p2j/cfg/Configuration.java    2025-06-24 10:41:30 +0000
@@ -673,7 +673,15 @@
       }

       filename = normalizeFilename(filename);
-      String basepath = normalizeFilename(getParameter("basepath")) + File.separator;
+      String base = normalizeFilename(getParameter("basepath")) + File.separator;
+      // we have to get the first directory from base, for example:
+      // we have getParameter(basepath)="abl/src", we have to move the artifacts to "cvt/src" 
+      String basepath = "." + File.separator;
+      if (!base.equals(basepath))
+      {
+         // we make sure we have something after "./" to use split
+         basepath = basepath + base.split(String.valueOf(File.separatorChar))[1] + File.separator;
+      }
       String cvtpath = DEFAULT_CVT;
       try
       {