Linker bug with useSourceMaps

[Alex Epshteyn]

I'm trying to fix a bug ( https://code.google.com/p/google-web-toolkit/issues/detail?id=8100
), and I could use some expert advice. I'm a long time GWT user (7
years), but only recently started hacking the internals, and I need
help on this one.

Background:

SelectionScriptLinker.splitPrimaryJavaScript attempts to check whether
the property "compiler.useSourceMaps" is "true" and if it is, the
method avoids splitting the javascript into chunks (which would
destroy the accuracy of the line numbers in the source map).

The Problem:

This method tries to get the property value from
LinkerContext.getProperties, which is not a PropertyOracle, and hence
it can't return the correct property value for the current
permutation. In fact, it will return a null value unless the property
has a static value for all permutations (e.g. <set-property
name="compiler.useSourceMaps" value="true"/>)

However, something like

<set-property name="compiler.useSourceMaps" value="true">
<when-property-is name="user.agent" value="safari"/>
</set-property>

will not work because SelectionScriptLinker.splitPrimaryJavaScript can
only read static property values.

Could someone advise me on how to fix this bug?

Here's is there relevant source code for quick reference:

public static String splitPrimaryJavaScript(StatementRanges ranges,
String js,
int charsPerChunk, String scriptChunkSeparator, LinkerContext
context) {
boolean useSourceMaps = false;
for (SelectionProperty prop : context.getProperties()) {
if (USE_SOURCE_MAPS_PROPERTY.equals(prop.getName())) {
String str = prop.tryGetValue();
useSourceMaps = str == null ? false :
Boolean.parseBoolean(str);
break;
}
}

// TODO(cromwellian) enable chunking with sourcemaps
if (charsPerChunk < 0 || ranges == null || useSourceMaps) {
return js;
}
...

[Alex Epshteyn]

Okay, I found a pretty good solution. Solved the problem by adding a
method isSourceMapsEnabled to CompilationResult, which gets passed all
the way from Link to SelectionScriptLinker. The value is set in Link
via the permutation's property oracles. I tested this and it works
well!

Now, how can I contribute this patch? Long time user, first time
contributor. I signed the CLA many years ago, but haven't submitted
anything until now.

Are the instructions on https://developers.google.com/web-toolkit/makinggwtbetter
still valid?

I'm a little confused because that document says "Upload the patch to
the Rietveld instance at http://gwt-code-reviews.appspot.com/" whereas
I'm seeing all the messages in this group coming from a system called
Gerrit at https://gwt-review.googlesource.com/



On Apr 10, 2:39 am, Alex Epshteyn <alexander.epsht...@gmail.com>
wrote:
> I'm trying to fix a bug (https://code.google.com/p/google-web-toolkit/issues/detail?id=8100

[Thomas Broyer]

On Wednesday, April 10, 2013 8:03:41 PM UTC+2, Alex Epshteyn wrote:

Okay, I found a pretty good solution. Solved the problem by adding a
method isSourceMapsEnabled to CompilationResult, which gets passed all
the way from Link to SelectionScriptLinker.  The value is set in Link
via the permutation's property oracles.  I tested this and it works
well!

Now, how can I contribute this patch?  Long time user, first time
contributor.  I signed the CLA many years ago, but haven't submitted
anything until now.

Are the instructions on https://developers.google.com/web-toolkit/makinggwtbetter
still valid?

Apart from the Rietveld/Gerrit part, they're OK.

 

I'm a little confused because that document says "Upload the patch to
the Rietveld instance at http://gwt-code-reviews.appspot.com/" whereas
I'm seeing all the messages in this group coming from a system called
Gerrit at https://gwt-review.googlesource.com/

Yes, we're switching from SVN and Rietveld to Git and Gerrit, so if you can use Git, please do.

Feel free to ask for guidance here if needed. Setting up your Git for use with Gerrit requires a few special invocations so if you have a problem don't hesitate to ask (we'll then use that in our new docs).

[Alex Epshteyn]

Thanks Thomas!  Is it possible to still use SVN by any chance? I haven't made the leap over to git yet as I'm still using an older version of IntelliJ without git support. If not, could you help me out with some command-line examples of how to submit my patch using git and Gerrit.

Alex

--
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
---
You received this message because you are subscribed to a topic in the Google Groups "Google Web Toolkit Contributors" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/google-web-toolkit-contributors/QaHTo59D_GY/unsubscribe?hl=en.
To unsubscribe from this group and all its topics, send an email to google-web-toolkit-co...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
 
 

[Matthew Dempsky]

On Wed, Apr 10, 2013 at 12:22 PM, Alex Epshteyn <alexander...@gmail.com> wrote:

Thanks Thomas!  Is it possible to still use SVN by any chance?

We'll continue accepting patches from Rietveld, just it's a bit more work for us to merge them.

[Alex Epshteyn]

Gotcha.  I will certainly have more than one patch to contribute now that I've really dived into the GWT compiler internals, so I might as well start working with the new git-based system to make things easier for everyone.  I've already made quite a few other enhancements and bug fixes to the whole "web mode exceptions" reporting toolchain, which are going into the next release of my product, TypeRacer, and I'm eager to contribute those back to GWT.  I'll probably write up a blog post about my experiments and enhancements in the coming weeks.

Anyways, could you guys provide me with some command line examples of the whole process using git, starting from checking out the code to submitting a patch for review?  Thanks in advance.  I'm eager to get started!

--

[Thomas Broyer]

On Wednesday, April 10, 2013 9:22:18 PM UTC+2, Alex Epshteyn wrote:

Thanks Thomas!  Is it possible to still use SVN by any chance? I haven't made the leap over to git yet as I'm still using an older version of IntelliJ without git support. If not, could you help me out with some command-line examples of how to submit my patch using git and Gerrit.

It's actually rather easy (this is for Linux/Mac, adapt for Windows if needed):

1. svn checkout http://google-web-toolkit.googlecode.com/svn/tools/ tools/           # dependencies are here until we move to Maven
2. git clone https://gwt.googlesource.com/gwt
   
3. cd gwt
4. One-time setup specific to Gerrit: download https://gwt-review.googlesource.com/tools/hooks/commit-msg into .git/hooks/commit-msg and make sure the file is executable (chmod +x)
5. You're ready to hack; below is "standard Git":
1. git checkout -b feature-branche master               # create a branch off of master for your feature
   You'll then use "git branch" to list your branches, and "git checkout my-branch" to switch branches.
2. …hack hack hack…
3. Use git status, git add and git commit to commit your files (add --help for help); or try "git gui" or "git citool" for a GUI (that's what I use)
   With Gerrit, each commit will be reviewed independently, so unless you really mean to have 2 commits, you'll "git commit --amend" your one and only commit for a given feature/fix (first create a commit with "git commit", then if you need to update it, use "git commit --amend"). Only use this locally (before you "git push", see below) or when working with commits under review in Gerrit. Updating a review is just a matter of amending the commit and pushing it again.
6. To push your commit for review: git push origin feature-branch:refs/for/master
   If you're on the feature-branch, your can also use "git push origin HEAD:refs/for/master" (HEAD corresponds to the currently checked out commit); refs/for/master means "this is intended to be merged into master"
   Note: you'll have to generate a password from the Gerrit web UI to be able to push.
7. Last, but not least, to stay up-to-date:
1. git checkout master
2. git pull
3. Note: do only ever "git pull" when on the "master" branch.
4. If you need update a branch to a newer "master" (because there's been changes that would conflict with your changes)
1. git checkout my-branch
2. git rebase master
   Git will tell you if there are conflicts and should tell you what to do. Hint: "git mergetool" to resolve conflicts, then "git rebase --continue" when done.
8. Once your change has been merged (or if you want to delete your work):
1. git branch -D my-branch    # generally done from master

[Alex Epshteyn]

Thanks Thomas!  I appreciate it.  I'll give it a try!

--

[Jens]

If you are on Windows or Mac OS I would recommend http://www.sourcetreeapp.com/ as git frontend (its free).

-- J.

[Ray Cromwell]

Good catch. The real solution IMHO to these issues is to do linker processing on a JS AST, not on text. Most of the current linker API relies on string operations which make designing a non-brittle sourcemap difficult. If for example, we read the JS for the fragment via the Closure/Rhino AST, then we could mutate it in all sorts of interesting ways, and let the serialization of the AST take care of updating the source maps. 

The class ClosureJsAstTranslator already contains the logic neccessary. What we could do is use it to translate the GWT JsAst to a Closure Ast, and serialize that in the CompilationResult. The linker API could then get the fragments as strings, but could also get the AST if it needed. Manipulating the AST to do things like splitting, wrapping, et al would preserve the sourcemap.

-Ray

--
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
---

You received this message because you are subscribed to the Google Groups "Google Web Toolkit Contributors" group.
To unsubscribe from this group and stop receiving emails from it, send an email to google-web-toolkit-co...@googlegroups.com.