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.

Sign up for GitHub

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

Jump to bottom

Enable embedding sources inside the sourcemaps json as sourcesContent #9936

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Enable embedding sources inside the sourcemaps json as sourcesContent #9936

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
26428f4
Enable embedding sources inside the sourcemaps json as sourcesContent
vegegoku Mar 10, 2024
a776cd8
revert the removal of saveSources check and add the embed sources check
vegegoku May 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion .github/workflows/full-check.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ jobs:
- name: Checkout GWT tools into a sibling directory
uses: actions/checkout@v4
with:
repository: 'gwtproject/tools'
repository: 'vegegoku/tools'
ref: 'source-maps'
path: 'tools'
- name: Set up JDK ${{ matrix.java-version }}
# GWT requires Java 11+ to build
Expand Down
3 changes: 2 additions & 1 deletion .github/workflows/quick-check.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ jobs:
- name: Checkout GWT tools into a sibling directory
uses: actions/checkout@v4
with:
repository: 'gwtproject/tools'
repository: 'vegegoku/tools'
ref: 'source-maps'
path: 'tools'
- name: Set up JDK ${{ matrix.java-version }}
# GWT presently requires Java 11+ to build
Expand Down
16 changes: 10 additions & 6 deletions dev/build.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,12 @@
<include name="objectweb/asm-9.6/asm-9.6.jar"/>
<include name="objectweb/asm-9.6/asm-commons-9.6.jar"/>
<include name="objectweb/asm-9.6/asm-util-9.6.jar"/>
<include name="guava/guava-19.0/guava-19.0-rebased.jar"/>
<include name="guava/guava-33.0/guava-33.0.0-jre-rebased.jar"/>
<include name="guava/guava-33.0/failureaccess-1.0.2-rebased.jar"/>
<include name="icu4j/63.1/icu4j.jar"/>
<include name="jetty/jetty-9.4.44.v20210927/jetty-all-9.4.44.v20210927.jar"/>
<include name="gson/gson-2.6.2.jar"/>
<include name="jscomp/20160315/sourcemap-rebased.jar"/>
<include name="jscomp/20231112/sourcemap-rebased.jar"/>
<include name="jsr305/jsr305.jar"/>
<include name="protobuf/protobuf-2.5.0/protobuf-java-rebased-2.5.0.jar"/>
<!-- dependencies needed for JSP support in DevMode: BEGIN -->
Expand Down Expand Up @@ -129,7 +130,8 @@
src="${gwt.tools.lib}/eclipse/org.eclipse.jdt.core_3.32.0.v20221108-1853.jar"/>
<zipfileset
src="${gwt.tools.lib}/eclipse/jdtCompilerAdapter_3.32.0.v20221108-1853.jar"/>
vegegoku marked this conversation as resolved.
Show resolved Hide resolved
<zipfileset src="${gwt.tools.lib}/guava/guava-19.0/guava-19.0-rebased.jar"/>
<zipfileset src="${gwt.tools.lib}/guava/guava-33.0/guava-33.0.0-jre-rebased.jar"/>
<zipfileset src="${gwt.tools.lib}/guava/guava-33.0/failureaccess-1.0.2-rebased.jar" />
<zipfileset src="${gwt.tools.lib}/icu4j/63.1/icu4j.jar"/>
<zipfileset
src="${gwt.tools.lib}/jetty/jetty-9.4.44.v20210927/jetty-all-9.4.44.v20210927.jar">
Expand All @@ -140,7 +142,7 @@
<exclude name="META-INF/services/org.eclipse.jetty.security.Authenticator$Factory"/>
</zipfileset>
<zipfileset src="${gwt.tools.lib}/gson/gson-2.6.2.jar"/>
<zipfileset src="${gwt.tools.lib}/jscomp/20160315/sourcemap-rebased.jar"/>
<zipfileset src="${gwt.tools.lib}/jscomp/20231112/sourcemap-rebased.jar"/>
<zipfileset src="${gwt.tools.lib}/jsr305/jsr305.jar"/>
<zipfileset
src="${gwt.tools.lib}/protobuf/protobuf-2.5.0/protobuf-java-rebased-2.5.0.jar"/>
Expand Down Expand Up @@ -227,9 +229,11 @@
<pathelement
location="${gwt.tools.lib}/eclipse/jdtCompilerAdapter_3.32.0.v20221108-1853.jar"/>
<pathelement
location="${gwt.tools.lib}/guava/guava-19.0/guava-19.0-rebased.jar"/>
location="${gwt.tools.lib}/guava/guava-33.0/guava-33.0.0-jre-rebased.jar"/>
<pathelement
location="${gwt.tools.lib}/guava/guava-33.0/failureaccess-1.0.2-rebased.jar"/>
<pathelement location="${gwt.tools.lib}/gson/gson-2.6.2.jar"/>
<pathelement location="${gwt.tools.lib}/jscomp/20160315/sourcemap-rebased.jar"/>
<pathelement location="${gwt.tools.lib}/jscomp/20231112/sourcemap-rebased.jar"/>
<pathelement location="${gwt.tools.lib}/json/android-sdk-19.1/json-android-rebased.jar"/>
<pathelement location="${gwt.tools.lib}/jsr305/jsr305.jar"/>
<pathelement
Expand Down
6 changes: 6 additions & 0 deletions dev/core/src/com/google/gwt/core/ext/linker/impl/StandardLinkerContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ public int compare(SelectionProperty o1, SelectionProperty o2) {
private ResourceOracle publicResourceOracle;

private final SortedSet<SelectionProperty> selectionProperties;
private final ModuleDef module;

public StandardLinkerContext(TreeLogger logger, ModuleDef module,
ResourceOracle publicResourceOracle, JsOutputOption outputOption)
Expand All @@ -143,6 +144,7 @@ public StandardLinkerContext(TreeLogger logger, ModuleDef module,
this.moduleLastModified = module.lastModified();
this.publicResourceOracle = publicResourceOracle;
this.outputOption = outputOption;
this.module = module;

// Sort the linkers into the order they should actually run.
linkerClasses = new ArrayList<Class<? extends Linker>>();
Expand Down Expand Up @@ -537,6 +539,10 @@ public void produceOutput(TreeLogger logger, ArtifactSet artifacts,
}
}

public ModuleDef getModule() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As we discussed when first working out how to plumb this, I don't really like exposing this here. I don't have a good enough answer to ask you to change it, so just documenting my thoughts in case something strikes one of us (or someone sees this PR in a few years and wants to know why on earth we made this change).

One option is to make a specific getter, shouldEmbedSourcesInModuleSourcemaps() sort of thing that just does return module.isEmbedSourcesContent(). That would better match getModuleName()/getModuleFunctionName()/getModuleLastModified() - though those three are part of LinkerContext itself. I don't think this should be added to LinkerContext.

I dislike this option the least. However, it does suggest that we should do the same in CompilerContext - but CompilerContext has a bunch of other calls to getModule() already, so it feels a bit inconsistent.

Another option would be to remove the other three getters, and have all the callers instead call getModule().getName() and so on. This provides access to too much of ModuleDef I think, and it isn't immutable, so I really don't care for this.

Separately, I don't love the name isEmbedSourcesContent as a method on ModuleDef, nor do I love that the method exists there at all. No other properties exist on ModuleDef itself, so it seems silly to have this one be special enough to be there. That said, it is needed by both the various pre/compile stages and linkers too, so it either needs to be somewhere very accessible, or needs to be duplicated - so I think I'm okay with it staying in ModuleDef or pushing it down into Properties (so you have to call getModule().getProperties.isBlahBlah() - that does happen in other places, so it wouldn't be terrible). Renaming it: how about we get SourceMaps in the name somehow - shouldEmbedSourceMapContents() perhaps? sourceMapContentsEmbedEnabled()?

return module;
}

/**
* (Re)instantiate all linkers.
*/
Expand Down
97 changes: 88 additions & 9 deletions dev/core/src/com/google/gwt/core/linker/SymbolMapsLinker.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@
*/
package com.google.gwt.core.linker;

import static java.util.Objects.isNull;
import static java.util.Objects.nonNull;

import com.google.gwt.core.ext.LinkerContext;
import com.google.gwt.core.ext.TreeLogger;
import com.google.gwt.core.ext.UnableToCompleteException;
Expand All @@ -31,17 +34,27 @@
import com.google.gwt.core.ext.linker.SoftPermutation;
import com.google.gwt.core.ext.linker.SymbolData;
import com.google.gwt.core.ext.linker.SyntheticArtifact;
import com.google.gwt.core.ext.linker.impl.StandardLinkerContext;
import com.google.gwt.dev.cfg.ResourceLoader;
import com.google.gwt.dev.cfg.ResourceLoaders;
import com.google.gwt.dev.util.Util;
import com.google.gwt.dev.util.collect.HashMap;
import com.google.gwt.dev.util.log.speedtracer.CompilerEventType;
import com.google.gwt.dev.util.log.speedtracer.SpeedTracerLogger;
import com.google.gwt.dev.util.log.speedtracer.SpeedTracerLogger.Event;
import com.google.gwt.thirdparty.debugging.sourcemap.SourceMapConsumerV3;
import com.google.gwt.thirdparty.debugging.sourcemap.SourceMapGeneratorV3;
import com.google.gwt.thirdparty.debugging.sourcemap.SourceMapGeneratorV3.ExtensionMergeAction;
import com.google.gwt.thirdparty.debugging.sourcemap.SourceMapParseException;
import com.google.gwt.thirdparty.guava.common.collect.Maps;
import com.google.gwt.util.tools.Utility;

import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.InputStream;
import java.io.PrintWriter;
import java.io.StringWriter;
import java.net.URISyntaxException;
import java.net.URL;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -162,7 +175,8 @@ public static class SourceMapArtifact extends SyntheticArtifact {
private final String sourceRoot;

public SourceMapArtifact(int permutationId, int fragment, byte[] js, String sourceRoot) {
super(SymbolMapsLinker.class, permutationId + '/' + sourceMapFilenameForFragment(fragment), js);
super(SymbolMapsLinker.class, permutationId + '/' +
sourceMapFilenameForFragment(fragment), js);
this.permutationId = permutationId;
this.fragment = fragment;
this.js = js;
Expand Down Expand Up @@ -277,6 +291,7 @@ public ArtifactSet link(TreeLogger logger, LinkerContext context,

Event writeSourceMapsEvent =
SpeedTracerLogger.start(CompilerEventType.WRITE_SOURCE_MAPS);
StandardLinkerContext stdContext = (StandardLinkerContext) context;
for (SourceMapArtifact se : artifacts.find(SourceMapArtifact.class)) {
// filename is permutation_id/sourceMap<fragmentNumber>.json
String sourceMapString = Util.readStreamAsString(se.getContents(logger));
Expand Down Expand Up @@ -312,14 +327,16 @@ public ArtifactSet link(TreeLogger logger, LinkerContext context,
totalPrefixLines += op.getNumLines();
}
}

// TODO(cromwellian): apply insert and remove edits
sourceMapGenerator.mergeMapSection(totalPrefixLines, 0, sourceMapString,
new ExtensionMergeAction() {
@Override
public Object merge(String extKey, Object oldVal, Object newVal) {
return newVal;
}
});
if (stdContext.getModule().shouldEmbedSourceMapContents()) {
embedSourcesInSourceMaps(logger, stdContext, artifacts, sourceMapGenerator,
totalPrefixLines, sourceMapString, partialPath);
} else {
sourceMapGenerator.mergeMapSection(totalPrefixLines, 0, sourceMapString,
(extKey, oldVal, newVal) -> newVal);
}

StringWriter stringWriter = new StringWriter();
sourceMapGenerator.appendTo(stringWriter, "sourceMap");
emArt = emitSourceMapString(logger, stringWriter.toString(), partialPath);
Expand All @@ -335,6 +352,68 @@ public Object merge(String extKey, Object oldVal, Object newVal) {
return artifacts;
}

private static void embedSourcesInSourceMaps(TreeLogger logger, StandardLinkerContext context,
ArtifactSet artifacts,
SourceMapGeneratorV3 sourceMapGenerator,
int totalPrefixLines, String sourceMapString,
String partialPath)
throws SourceMapParseException {
sourceMapGenerator.setStartingPosition(totalPrefixLines, 0);
SourceMapConsumerV3 section = new SourceMapConsumerV3();
section.parse(sourceMapString);
section.visitMappings(sourceMapGenerator::addMapping);

for (Entry<String, Object> entry : section.getExtensions().entrySet()) {
String extensionKey = entry.getKey();
sourceMapGenerator.addExtension(extensionKey, entry.getValue());
}

ResourceLoader resourceLoader = ResourceLoaders.fromContextClassLoader();

Map<String, EmittedArtifact> generatedSources = Maps.newHashMap();
artifacts.find(EmittedArtifact.class)
.forEach(emittedArtifact -> {
if (Visibility.Source == emittedArtifact.getVisibility()) {
generatedSources.put(emittedArtifact.getPartialPath(), emittedArtifact);
}
});

for (String sourceFileName : section.getOriginalSources()) {
String content;
InputStream cis = null;
try {
cis = loadSource(logger, sourceFileName, generatedSources,
resourceLoader);
if (isNull(cis)) {
cis = context.getModule().findSourceFile(sourceFileName).openContents();
}
content = Util.readStreamAsString(cis);
sourceMapGenerator.addSourcesContent(sourceFileName, content);
} catch (UnableToCompleteException | URISyntaxException | IOException e) {
logger.log(TreeLogger.Type.WARN, "Can't write source map " +
partialPath, e);
} finally {
Utility.close(cis);
}
}
}

private static InputStream loadSource(TreeLogger logger, String sourceFileName,
Map<String, EmittedArtifact> generatedSources,
ResourceLoader resourceLoader)
throws UnableToCompleteException, URISyntaxException, IOException {
if (generatedSources.containsKey(sourceFileName)) {
return generatedSources.get(sourceFileName).getContents(logger);
} else {
// ask the resourceOracle for the file contents and add it
URL resource = resourceLoader.getResource(sourceFileName);
if (nonNull(resource)) {
return resource.openStream();
}
}
return null;
}

/**
* Override to change the manner in which the symbol map is emitted.
*/
Expand Down
6 changes: 5 additions & 1 deletion dev/core/src/com/google/gwt/dev/CompilePerms.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -380,8 +380,12 @@ private boolean precompileAndCompile(TreeLogger logger, String moduleName,
if (precompilation == null) {
return false;
}

boolean embedSourcesContent = compilerContext.getModule()
.shouldEmbedSourceMapContents();

// TODO: move to precompile() after params are refactored
if (!options.shouldSaveSource()) {
if (!options.shouldSaveSource() && !embedSourcesContent) {
precompilation.removeSourceArtifacts(logger);
}

Expand Down
5 changes: 4 additions & 1 deletion dev/core/src/com/google/gwt/dev/Compiler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,8 +193,11 @@ public static boolean compile(
if (precompilation == null) {
return false;
}

boolean embedSourcesContent = compilerContext.getModule()
.shouldEmbedSourceMapContents();
// TODO: move to precompile() after params are refactored
if (!options.shouldSaveSource()) {
if (!options.shouldSaveSource() && !embedSourcesContent) {
precompilation.removeSourceArtifacts(branch);
}

Expand Down
5 changes: 4 additions & 1 deletion dev/core/src/com/google/gwt/dev/Link.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -415,7 +415,10 @@ private static void doProduceOutput(TreeLogger logger, ArtifactSet artifacts,
linkerContext.produceOutput(logger, artifacts, Visibility.Private,
extraFileSet);

if (saveSources) {
boolean embedSourcesContent = linkerContext.getModule()
.shouldEmbedSourceMapContents();

if (saveSources && !embedSourcesContent) {
// Assume that all source code is available in the compiler's classpath.
// (This will have to be adjusted to work with Super Dev Mode.)
ResourceLoader loader = ResourceLoaders.fromContextClassLoader();
Expand Down
2 changes: 1 addition & 1 deletion dev/core/src/com/google/gwt/dev/Precompile.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -443,7 +443,7 @@ public boolean run(TreeLogger logger) throws UnableToCompleteException {
return false;
}
// TODO: move to precompile() after params are refactored
if (!options.shouldSaveSource()) {
if (!options.shouldSaveSource() && !module.shouldEmbedSourceMapContents()) {
precompilation.removeSourceArtifacts(logger);
}
Util.writeObjectAsFile(logger, precompilationFile, precompilation);
Expand Down
5 changes: 4 additions & 1 deletion dev/core/src/com/google/gwt/dev/PrecompileOnePerm.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,8 +233,11 @@ private boolean precompilePermutation(TreeLogger logger,
return false;
}

boolean embedSourcesContent = compilerContext.getModule()
.shouldEmbedSourceMapContents();

// TODO: precompile should do this after we get the parameter passing refactored.
if (!options.shouldSaveSource()) {
if (!options.shouldSaveSource() && embedSourcesContent) {
precompilation.removeSourceArtifacts(logger);
}

Expand Down
19 changes: 19 additions & 0 deletions dev/core/src/com/google/gwt/dev/cfg/ModuleDef.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,9 @@
* XML for unit tests.
*/
public class ModuleDef implements DepsInfoProvider {

public static final String EMBED_SOURCE_MAPS = "compiler.embedSourceMaps";

private static final ResourceFilter NON_JAVA_RESOURCES = new ResourceFilter() {
@Override
public boolean allows(String path) {
Expand Down Expand Up @@ -628,6 +631,22 @@ public synchronized void setNameOverride(String nameOverride) {
this.nameOverride = nameOverride;
}

/**
* Checks if embedding sources content inside sourceMaps json is enabled or not.
* @return the boolean value true/false of <code>compiler.embedSourceMaps</code>
* configuration property
*/
public boolean shouldEmbedSourceMapContents() {
return getProperties()
.getConfigurationProperties()
.stream()
.filter(configurationProperty -> EMBED_SOURCE_MAPS.equals(
configurationProperty.getName()))
.findFirst()
.map(configurationProperty -> Boolean.valueOf(configurationProperty.getValue()))
.orElse(false);
}

void addBindingPropertyDefinedValue(BindingProperty bindingProperty, String token) {
bindingProperty.addDefinedValue(bindingProperty.getRootCondition(), token);
}
Expand Down
1 change: 1 addition & 0 deletions dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1201,6 +1201,7 @@ private String buildEntryMethodHolder(StandardGeneratorContext context,
}

EntryMethodHolderGenerator entryMethodHolderGenerator = new EntryMethodHolderGenerator();
context.setCurrentGenerator(EntryMethodHolderGenerator.class);
String entryMethodHolderTypeName =
entryMethodHolderGenerator.generate(logger, context, module.getCanonicalName());
context.finish(logger);
Expand Down
2 changes: 1 addition & 1 deletion eclipse/dev/.classpath
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
<classpathentry kind="var" path="GWT_TOOLS/lib/htmlunit/htmlunit-2.19/htmlunit-2.19.jar"/>
<classpathentry kind="var" path="GWT_TOOLS/lib/htmlunit/htmlunit-2.19/htmlunit-core-js-2.15.jar"/>
<classpathentry kind="var" path="GWT_TOOLS/lib/protobuf/protobuf-2.5.0/protobuf-java-rebased-2.5.0.jar"/>
<classpathentry kind="var" path="GWT_TOOLS/lib/guava/guava-19.0/guava-19.0-rebased.jar"/>
<classpathentry kind="var" path="GWT_TOOLS/lib/guava/guava-33.0/guava-33.0.0-jre-rebased.jar"/>
<classpathentry kind="var" path="GWT_TOOLS/lib/gson/gson-2.6.2.jar"/>
<classpathentry kind="var" path="GWT_TOOLS/lib/jscomp/20160315/sourcemap-rebased.jar"/>
<classpathentry kind="var" path="GWT_TOOLS/lib/json/android-sdk-19.1/json-android-rebased.jar"/>
Expand Down
2 changes: 1 addition & 1 deletion eclipse/dev/codeserver/.classpath
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
<classpathentry kind="src" path="codeserver/java"/>
<classpathentry kind="src" path="codeserver/javatests"/>
<classpathentry kind="con" path="org.eclipse.jdt.launching.JRE_CONTAINER"/>
<classpathentry kind="var" path="GWT_TOOLS/lib/guava/guava-19.0/guava-19.0-rebased.jar"/>
<classpathentry kind="var" path="GWT_TOOLS/lib/guava/guava-33.0/guava-33.0.0-jre-rebased.jar"/>
<classpathentry kind="var" path="GWT_TOOLS/lib/gson-2.6.2.jar"/>
<classpathentry kind="var" path="GWT_TOOLS/lib/jscomp/20160315/sourcemap-rebased.jar"/>
<classpathentry kind="var" path="GWT_TOOLS/lib/junit/junit-4.8.2.jar" sourcepath="/GWT_TOOLS/lib/junit/junit-4.8.2-src.zip"/>
Expand Down
2 changes: 1 addition & 1 deletion eclipse/user/.classpath
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
<classpathentry kind="var" path="GWT_TOOLS/lib/apache/log4j/log4j-1.2.17.jar"/>
<classpathentry kind="var" path="GWT_TOOLS/lib/slf4j/slf4j-api/slf4j-api-1.7.12.jar"/>
<classpathentry kind="var" path="GWT_TOOLS/lib/slf4j/slf4j-log4j12/slf4j-log4j12-1.7.12.jar"/>
<classpathentry kind="var" path="GWT_TOOLS/lib/guava/guava-19.0/guava-19.0-rebased.jar"/>
<classpathentry kind="var" path="GWT_TOOLS/lib/guava/guava-33.0/guava-33.0.0-jre-rebased.jar"/>
<classpathentry kind="var" path="GWT_TOOLS/lib/streamhtmlparser/streamhtmlparser-jsilver-r10/streamhtmlparser-jsilver-r10-1.5-rebased.jar"/>
<classpathentry kind="var" path="GWT_TOOLS/lib/icu4j/50.1.1/icu4j.jar"/>
<classpathentry kind="var" path="GWT_TOOLS/lib/javax/activation/activation-1.1.jar"/>
Expand Down