Merge pull request #9949 from github/edoardo/trap-cache-config

JS: Change how TRAP cache is configured

Author: edoardopirovano

javascript/extractor/src/com/semmle/js/extractor/AutoBuild.java

@@ -86,8 +86,6 @@
  *       <code>XML</code> is also supported
  *   <li><code>LGTM_INDEX_XML_MODE</code>: whether to extract XML files
  *   <li><code>LGTM_THREADS</code>: the maximum number of files to extract in parallel
- *   <li><code>LGTM_TRAP_CACHE</code>: the path of a directory to use for trap caching
- *   <li><code>LGTM_TRAP_CACHE_BOUND</code>: the size to bound the trap cache to
  * </ul>
  *
  * <p>It extracts the following:
@@ -220,7 +218,7 @@ public AutoBuild() {
     this.LGTM_SRC = toRealPath(getPathFromEnvVar("LGTM_SRC"));
     this.SEMMLE_DIST = Paths.get(EnvironmentVariables.getExtractorRoot());
     this.outputConfig = new ExtractorOutputConfig(LegacyLanguage.JAVASCRIPT);
-    this.trapCache = mkTrapCache();
+    this.trapCache = ITrapCache.fromExtractorOptions();
     this.typeScriptMode =
         getEnumFromEnvVar("LGTM_INDEX_TYPESCRIPT", TypeScriptMode.class, TypeScriptMode.FULL);
     this.defaultEncoding = getEnvVar("LGTM_INDEX_DEFAULT_ENCODING");
@@ -281,28 +279,6 @@ private Path toRealPath(Path p) {
     }
   }
 
-  /**
-   * Set up TRAP cache based on environment variables <code>LGTM_TRAP_CACHE</code> and <code>
-   * LGTM_TRAP_CACHE_BOUND</code>.
-   */
-  private ITrapCache mkTrapCache() {
-    ITrapCache trapCache;
-    String trapCachePath = getEnvVar("LGTM_TRAP_CACHE");
-    if (trapCachePath != null) {
-      Long sizeBound = null;
-      String trapCacheBound = getEnvVar("LGTM_TRAP_CACHE_BOUND");
-      if (trapCacheBound != null) {
-        sizeBound = DefaultTrapCache.asFileSize(trapCacheBound);
-        if (sizeBound == null)
-          throw new UserError("Invalid TRAP cache size bound: " + trapCacheBound);
-      }
-      trapCache = new DefaultTrapCache(trapCachePath, sizeBound, Main.EXTRACTOR_VERSION);
-    } else {
-      trapCache = new DummyTrapCache();
-    }
-    return trapCache;
-  }
-
   private void setupFileTypes() {
     for (String spec : Main.NEWLINE.split(getEnvVar("LGTM_INDEX_FILETYPES", ""))) {
       spec = spec.trim();
@@ -513,14 +489,13 @@ private void extractExterns() throws IOException {
           SEMMLE_DIST.resolve(".cache").resolve("trap-cache").resolve("javascript");
       if (Files.isDirectory(trapCachePath)) {
         trapCache =
-            new DefaultTrapCache(trapCachePath.toString(), null, Main.EXTRACTOR_VERSION) {
+            new DefaultTrapCache(trapCachePath.toString(), null, Main.EXTRACTOR_VERSION, false) {
               boolean warnedAboutCacheMiss = false;
 
               @Override
               public File lookup(String source, ExtractorConfig config, FileType type) {
                 File f = super.lookup(source, config, type);
-                // only return `f` if it exists; this has the effect of making the cache read-only
-                if (f.exists()) return f;
+                if (f != null) return f;
                 // warn on first failed lookup
                 if (!warnedAboutCacheMiss) {
                   warn("Trap cache lookup for externs failed.");

javascript/extractor/src/com/semmle/js/extractor/ExtractorOptionsUtil.java

@@ -0,0 +1,12 @@
+package com.semmle.js.extractor;
+
+import com.semmle.util.process.Env;
+
+public class ExtractorOptionsUtil {
+    public static String readExtractorOption(String... option) {
+        StringBuilder name = new StringBuilder("CODEQL_EXTRACTOR_JAVASCRIPT_OPTION");
+        for (String segment : option)
+            name.append("_").append(segment.toUpperCase());
+        return Env.systemEnv().getNonEmpty(name.toString());
+    }
+}

javascript/extractor/src/com/semmle/js/extractor/Main.java

@@ -15,8 +15,6 @@
 import com.semmle.js.extractor.ExtractorConfig.Platform;
 import com.semmle.js.extractor.ExtractorConfig.SourceType;
 import com.semmle.js.extractor.FileExtractor.FileType;
-import com.semmle.js.extractor.trapcache.DefaultTrapCache;
-import com.semmle.js.extractor.trapcache.DummyTrapCache;
 import com.semmle.js.extractor.trapcache.ITrapCache;
 import com.semmle.js.parser.ParsedProject;
 import com.semmle.ts.extractor.TypeExtractor;
@@ -61,8 +59,6 @@ public class Main {
   private static final String P_PLATFORM = "--platform";
   private static final String P_QUIET = "--quiet";
   private static final String P_SOURCE_TYPE = "--source-type";
-  private static final String P_TRAP_CACHE = "--trap-cache";
-  private static final String P_TRAP_CACHE_BOUND = "--trap-cache-bound";
   private static final String P_TYPESCRIPT = "--typescript";
   private static final String P_TYPESCRIPT_FULL = "--typescript-full";
   private static final String P_TYPESCRIPT_RAM = "--typescript-ram";
@@ -112,22 +108,7 @@ public void run(String[] args) {
     ap.parse();
 
     extractorConfig = parseJSOptions(ap);
-    ITrapCache trapCache;
-    if (ap.has(P_TRAP_CACHE)) {
-      Long sizeBound = null;
-      if (ap.has(P_TRAP_CACHE_BOUND)) {
-        String tcb = ap.getString(P_TRAP_CACHE_BOUND);
-        sizeBound = DefaultTrapCache.asFileSize(tcb);
-        if (sizeBound == null) ap.error("Invalid TRAP cache size bound: " + tcb);
-      }
-      trapCache = new DefaultTrapCache(ap.getString(P_TRAP_CACHE), sizeBound, EXTRACTOR_VERSION);
-    } else {
-      if (ap.has(P_TRAP_CACHE_BOUND))
-        ap.error(
-            P_TRAP_CACHE_BOUND + " should only be specified together with " + P_TRAP_CACHE + ".");
-      trapCache = new DummyTrapCache();
-    }
-    fileExtractor = new FileExtractor(extractorConfig, extractorOutputConfig, trapCache);
+    fileExtractor = new FileExtractor(extractorConfig, extractorOutputConfig, ITrapCache.fromExtractorOptions());
 
     setupMatchers(ap);
 
@@ -432,12 +413,6 @@ private ArgsParser addArgs(ArgsParser argsParser) {
     argsParser.addToleratedFlag(P_TOLERATE_PARSE_ERRORS, 0);
     argsParser.addFlag(
         P_ABORT_ON_PARSE_ERRORS, 0, "Abort extraction if a parse error is encountered.");
-    argsParser.addFlag(P_TRAP_CACHE, 1, "Use the given directory as the TRAP cache.");
-    argsParser.addFlag(
-        P_TRAP_CACHE_BOUND,
-        1,
-        "A (soft) upper limit on the size of the TRAP cache, "
-            + "in standard size units (e.g., 'g' for gigabytes).");
     argsParser.addFlag(P_DEFAULT_ENCODING, 1, "The encoding to use; default is UTF-8.");
     argsParser.addFlag(P_TYPESCRIPT, 0, "Enable basic TypesScript support.");
     argsParser.addFlag(

javascript/extractor/src/com/semmle/js/extractor/trapcache/DefaultTrapCache.java

@@ -26,9 +26,15 @@ public class DefaultTrapCache implements ITrapCache {
    */
   private final String extractorVersion;
 
-  public DefaultTrapCache(String trapCache, Long sizeBound, String extractorVersion) {
+  /**
+  * Whether this cache supports write operations.
+  */
+  private final boolean writeable;
+
+  public DefaultTrapCache(String trapCache, Long sizeBound, String extractorVersion, boolean writeable) {
     this.trapCache = new File(trapCache);
     this.extractorVersion = extractorVersion;
+    this.writeable = writeable;
     try {
       initCache(sizeBound);
     } catch (ResourceError | SecurityException e) {
@@ -135,6 +141,8 @@ public File lookup(String source, ExtractorConfig config, FileType type) {
     digestor.write(type.toString());
     digestor.write(config);
     digestor.write(source);
-    return new File(trapCache, digestor.getDigest() + ".trap.gz");
+    File result = new File(trapCache, digestor.getDigest() + ".trap.gz");
+    if (!writeable && !result.exists()) return null; // If the cache isn't writable, only return the file if it exists
+    return result;
   }
 }

javascript/extractor/src/com/semmle/js/extractor/trapcache/ITrapCache.java

@@ -1,7 +1,11 @@
 package com.semmle.js.extractor.trapcache;
 
+import static com.semmle.js.extractor.ExtractorOptionsUtil.readExtractorOption;
+
 import com.semmle.js.extractor.ExtractorConfig;
 import com.semmle.js.extractor.FileExtractor;
+import com.semmle.js.extractor.Main;
+import com.semmle.util.exception.UserError;
 import java.io.File;
 
 /** Generic TRAP cache interface. */
@@ -18,4 +22,29 @@ public interface ITrapCache {
    *     cached information), or does not yet exist (and should be populated by the extractor)
    */
   public File lookup(String source, ExtractorConfig config, FileExtractor.FileType type);
+
+  /**
+  * Build a TRAP cache as defined by the extractor options, which are read from the corresponding
+  * environment variables as defined in
+  * https://github.com/github/codeql-core/blob/main/design/spec/codeql-extractors.md
+  *
+  * @return a TRAP cache
+  */
+  public static ITrapCache fromExtractorOptions() {
+    String trapCachePath = readExtractorOption("trap", "cache", "dir");
+    if (trapCachePath != null) {
+      Long sizeBound = null;
+      String trapCacheBound = readExtractorOption("trap", "cache", "bound");
+      if (trapCacheBound != null) {
+        sizeBound = DefaultTrapCache.asFileSize(trapCacheBound);
+        if (sizeBound == null)
+          throw new UserError("Invalid TRAP cache size bound: " + trapCacheBound);
+      }
+      boolean writeable = true;
+      String trapCacheWrite = readExtractorOption("trap", "cache", "write");
+      if (trapCacheWrite != null) writeable = trapCacheWrite.equalsIgnoreCase("TRUE");
+      return new DefaultTrapCache(trapCachePath, sizeBound, Main.EXTRACTOR_VERSION, writeable);
+    }
+    return new DummyTrapCache();
+  }
 }