You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@flex.apache.org by Tigran Najaryan <ti...@gmail.com> on 2013/04/08 13:27:42 UTC
FalconJX: bugs in external dependencies (Google Closure Compiler), what to do?
I debugged the problem that I reported earlier (generating incorrect paths
in deps.js) and looks like it is due to a bug in
com.google.javascript.jscomp.deps.PathUtil.isAbsolute. It is implemented by
simply checking that the path starts by a forward slash which of course is
incorrect under Windows.
So what is the procedure when the bug is in an external dependency? It is
not as if it is a single call that can be avoided. PathUtil.isAbsolute is
called a few levels below in the call stack from the
com.google.javascript.jscomp.deps.DepsGenerator.computeDependencyCalls,
which is supposed to compute and return the correct text for dependencies
file.
I will debug this further but just wanted to understand if I should I try to
work around the external bug when I meet one or I need to try something
else?
Tigran.
P.S. Did anyone actually try running FalconJX on Windows or everybody is
developing on a Mac?
Re: FalconJX: bugs in external dependencies (Google Closure
Compiler), what to do?
Posted by Erik de Bruin <er...@ixsoftware.nl>.
Ah, and a good way to start would be to create a JIRA ticket about the
need for a better README and to attach a patch for that issue with the
content of what you wrote in your original email. That should get you
(and us) going on the road of coding and patching and will go some way
towards getting you to committer-ship ;-)
EdB
On Tue, Apr 9, 2013 at 6:46 AM, Erik de Bruin <er...@ixsoftware.nl> wrote:
>> The other approach is to copy the DepsGenerator and PathUtil classes to
>> FalconJX code base and fix it there. Technically this approach works, I
>> tried it. The fix to PathUtil is trivial to do, only a few lines must be
>> modified to make it handle Windows paths correctly. I did not need to touch
>> DepsGenerator except to change the import to use 'our' fixed PathUtil. After
>> the fix is applied compiling FlexJSTest_again works and produces correct
>> deps.js on Windows and the example then runs fine in the browser.
>
> Forking the Google code to our project doesn't seem like the best
> option. We may consider donating your solution to the Closure project,
> which is Open Source...
>
>> Please let me know if what I am doing makes any sense or I should scrap what
>> I did so far and wait for someone smarter than me to work on this.
>
> Keep what you have, please, but don't push it to the 'develop' branch.
> You're probably the smartest person that will work on it, but we need
> to take care that we don't end up with a dependency on a fork of the
> closure tools where we actually want to be able to use their releases.
>
>> Is there any ToDo list for FalconJX so that I can go ahead and try to work
>> on some outstanding issues? I now have Falcon projects in my Eclipse, git is
>> configured and am now ready for a bit more serious work.
>
> For FalconJX the main goal is to write tests for both MXML and AS
> parsing. At the moment there are some areas that are written
> specifically to allow for the correct parsing of the FlexJSTest_again
> example. This obviously is not "ideal" ;-) We need to make sure that
> all code is generic (or as much as possible) and that all output is
> properly tested so we don't kill any existing output when refactoring
> or adding code. Right now, there is only 2 tests specific for FlexJS
> MXML output, and those are "@Ignore"d.
>
> But first, just look around, review what's there and see if what I/we
> did makes sense.
>
> EdB
>
>
>
> --
> Ix Multimedia Software
>
> Jan Luykenstraat 27
> 3521 VB Utrecht
>
> T. 06-51952295
> I. www.ixsoftware.nl
--
Ix Multimedia Software
Jan Luykenstraat 27
3521 VB Utrecht
T. 06-51952295
I. www.ixsoftware.nl
Re: FalconJX: bugs in external dependencies (Google Closure
Compiler), what to do?
Posted by Erik de Bruin <er...@ixsoftware.nl>.
org.apache.flex.compiler.internal.codegen.mxml.flexjs.MXMLFlexJSEmitter.java
is a good place to start :-)
EdB
On Tue, Apr 9, 2013 at 10:13 AM, Tigran Najaryan <ti...@gmail.com> wrote:
>> Also, the current FlexJS implementation for both MXML and AS contains
>> several "giant" methods that simply beg to be broken up into more
>> manageable sets of support functions which maybe even share some
>> common code (like the emission of JSDoc type headers)... That might also
>> be a good way to get your feet wet without needing to change any of the
>> APIs.
>
> Can you list a few file names with those "giant" methods so that I can have
> a look?
>
--
Ix Multimedia Software
Jan Luykenstraat 27
3521 VB Utrecht
T. 06-51952295
I. www.ixsoftware.nl
RE: FalconJX: bugs in external dependencies (Google Closure Compiler), what to do?
Posted by Tigran Najaryan <ti...@gmail.com>.
> Also, the current FlexJS implementation for both MXML and AS contains
> several "giant" methods that simply beg to be broken up into more
> manageable sets of support functions which maybe even share some
> common code (like the emission of JSDoc type headers)... That might also
> be a good way to get your feet wet without needing to change any of the
> APIs.
Can you list a few file names with those "giant" methods so that I can have
a look?
Re: FalconJX: bugs in external dependencies (Google Closure
Compiler), what to do?
Posted by Erik de Bruin <er...@ixsoftware.nl>.
Also, the current FlexJS implementation for both MXML and AS contains
several "giant" methods that simply beg to be broken up into more
manageable sets of support functions which maybe even share some
common code (like the emission of JSDoc type headers)... That might
also be a good way to get your feet wet without needing to change any
of the APIs.
EdB
On Tue, Apr 9, 2013 at 7:11 AM, Tigran Najaryan <ti...@gmail.com> wrote:
>> Forking the Google code to our project doesn't seem like the best option.
> Yes, that what concerns me too.
>
>> We may consider donating your solution to the Closure project, which is
>> Open Source...
> That may be a solution in the long term but I am not sure that helps to fix
> the bug in Falcon compiler right now.
>
> Anyway submitting a patch to Closure will require more work on my part.
> Although the bug fix I propose is generic for PathUtil I still did not
> examine all the dependencies from that class in the Closure itself and am
> not sure my patch is generic enough and correct for the Closure itself.
> Honestly I would rather not digress and stay concentrated on contributing to
> Flex.
>
>
>> Keep what you have, please, but don't push it to the 'develop' branch.
>> You're probably the smartest person that will work on it, but we need to
>> take care that we don't end up with a dependency on a fork of the closure
>> tools where we actually want to be able to use their releases.
> I guess that means I will keep my patch for myself only until either
> somebody else needs it or a better way to fix this bug is found.
>
>
>> For FalconJX the main goal is to write tests for both MXML and AS parsing.
> At
>> the moment there are some areas that are written specifically to allow for
>> the correct parsing of the FlexJSTest_again example. This obviously is not
>> "ideal" ;-) We need to make sure that all code is generic (or as much as
>> possible) and that all output is properly tested so we don't kill any
> existing
>> output when refactoring or adding code. Right now, there is only 2 tests
>> specific for FlexJS MXML output, and those are "@Ignore"d.
>>
>>
>> But first, just look around, review what's there and see if what I/we did
>> makes sense.
> OK, I will spend a few more days just getting familiar with the code base
> and see if I can find and fix some other bugs before trying to make
> functional changes to the compiler.
>
> Tigran.
>
--
Ix Multimedia Software
Jan Luykenstraat 27
3521 VB Utrecht
T. 06-51952295
I. www.ixsoftware.nl
RE: FalconJX: bugs in external dependencies (Google Closure Compiler), what to do?
Posted by Tigran Najaryan <ti...@gmail.com>.
> Forking the Google code to our project doesn't seem like the best option.
Yes, that what concerns me too.
> We may consider donating your solution to the Closure project, which is
> Open Source...
That may be a solution in the long term but I am not sure that helps to fix
the bug in Falcon compiler right now.
Anyway submitting a patch to Closure will require more work on my part.
Although the bug fix I propose is generic for PathUtil I still did not
examine all the dependencies from that class in the Closure itself and am
not sure my patch is generic enough and correct for the Closure itself.
Honestly I would rather not digress and stay concentrated on contributing to
Flex.
> Keep what you have, please, but don't push it to the 'develop' branch.
> You're probably the smartest person that will work on it, but we need to
> take care that we don't end up with a dependency on a fork of the closure
> tools where we actually want to be able to use their releases.
I guess that means I will keep my patch for myself only until either
somebody else needs it or a better way to fix this bug is found.
> For FalconJX the main goal is to write tests for both MXML and AS parsing.
At
> the moment there are some areas that are written specifically to allow for
> the correct parsing of the FlexJSTest_again example. This obviously is not
> "ideal" ;-) We need to make sure that all code is generic (or as much as
> possible) and that all output is properly tested so we don't kill any
existing
> output when refactoring or adding code. Right now, there is only 2 tests
> specific for FlexJS MXML output, and those are "@Ignore"d.
>
>
> But first, just look around, review what's there and see if what I/we did
> makes sense.
OK, I will spend a few more days just getting familiar with the code base
and see if I can find and fix some other bugs before trying to make
functional changes to the compiler.
Tigran.
Re: FalconJX: bugs in external dependencies (Google Closure
Compiler), what to do?
Posted by Erik de Bruin <er...@ixsoftware.nl>.
> The other approach is to copy the DepsGenerator and PathUtil classes to
> FalconJX code base and fix it there. Technically this approach works, I
> tried it. The fix to PathUtil is trivial to do, only a few lines must be
> modified to make it handle Windows paths correctly. I did not need to touch
> DepsGenerator except to change the import to use 'our' fixed PathUtil. After
> the fix is applied compiling FlexJSTest_again works and produces correct
> deps.js on Windows and the example then runs fine in the browser.
Forking the Google code to our project doesn't seem like the best
option. We may consider donating your solution to the Closure project,
which is Open Source...
> Please let me know if what I am doing makes any sense or I should scrap what
> I did so far and wait for someone smarter than me to work on this.
Keep what you have, please, but don't push it to the 'develop' branch.
You're probably the smartest person that will work on it, but we need
to take care that we don't end up with a dependency on a fork of the
closure tools where we actually want to be able to use their releases.
> Is there any ToDo list for FalconJX so that I can go ahead and try to work
> on some outstanding issues? I now have Falcon projects in my Eclipse, git is
> configured and am now ready for a bit more serious work.
For FalconJX the main goal is to write tests for both MXML and AS
parsing. At the moment there are some areas that are written
specifically to allow for the correct parsing of the FlexJSTest_again
example. This obviously is not "ideal" ;-) We need to make sure that
all code is generic (or as much as possible) and that all output is
properly tested so we don't kill any existing output when refactoring
or adding code. Right now, there is only 2 tests specific for FlexJS
MXML output, and those are "@Ignore"d.
But first, just look around, review what's there and see if what I/we
did makes sense.
EdB
--
Ix Multimedia Software
Jan Luykenstraat 27
3521 VB Utrecht
T. 06-51952295
I. www.ixsoftware.nl
RE: FalconJX: bugs in external dependencies (Google Closure Compiler), what to do?
Posted by Tigran Najaryan <ti...@gmail.com>.
Unfortunately I could not find a workaround to this. Played with the paths
trying to fool DepsGenerator but it didn't work.
The other approach is to copy the DepsGenerator and PathUtil classes to
FalconJX code base and fix it there. Technically this approach works, I
tried it. The fix to PathUtil is trivial to do, only a few lines must be
modified to make it handle Windows paths correctly. I did not need to touch
DepsGenerator except to change the import to use 'our' fixed PathUtil. After
the fix is applied compiling FlexJSTest_again works and produces correct
deps.js on Windows and the example then runs fine in the browser.
However I do not know what are full implications and if you practice such
approach anyway. Looks like OK to me from legal point of view since the
Closure Compiler is Apache licensed.
Please let me know if what I am doing makes any sense or I should scrap what
I did so far and wait for someone smarter than me to work on this.
Is there any ToDo list for FalconJX so that I can go ahead and try to work
on some outstanding issues? I now have Falcon projects in my Eclipse, git is
configured and am now ready for a bit more serious work.
Tigran.
-----Original Message-----
From: Erik de Bruin [mailto:erik@ixsoftware.nl]
Sent: Monday, April 08, 2013 4:37 PM
To: dev@flex.apache.org
Subject: Re: FalconJX: bugs in external dependencies (Google Closure
Compiler), what to do?
Tigran,
Both Alex and I are on a Mac, so that is probably why it escaped notice
until now :-) Also, the "publisher" side of the code is very much a work in
progress, which deserves all the attention you can give it. I'll answer your
other email some time tonight, jetlag permitting.
On this one: might manipulation the path(s) before they are fed into the
'depswriter' - working from memory, might be missing the point - help avoid
this low level problem?
EdB
On Mon, Apr 8, 2013 at 6:27 AM, Tigran Najaryan <ti...@gmail.com> wrote:
> I debugged the problem that I reported earlier (generating incorrect
> paths in deps.js) and looks like it is due to a bug in
> com.google.javascript.jscomp.deps.PathUtil.isAbsolute. It is
> implemented by simply checking that the path starts by a forward slash
> which of course is incorrect under Windows.
>
> So what is the procedure when the bug is in an external dependency? It
> is not as if it is a single call that can be avoided.
> PathUtil.isAbsolute is called a few levels below in the call stack
> from the
> com.google.javascript.jscomp.deps.DepsGenerator.computeDependencyCalls
> , which is supposed to compute and return the correct text for
> dependencies file.
>
> I will debug this further but just wanted to understand if I should I
> try to work around the external bug when I meet one or I need to try
> something else?
>
> Tigran.
>
> P.S. Did anyone actually try running FalconJX on Windows or everybody
> is developing on a Mac?
>
>
--
Ix Multimedia Software
Jan Luykenstraat 27
3521 VB Utrecht
T. 06-51952295
I. www.ixsoftware.nl
RE: FalconJX: bugs in external dependencies (Google Closure Compiler), what to do?
Posted by Tigran Najaryan <ti...@gmail.com>.
Erik,
I already tried manipulating the paths before they are fed to
computeDependencyCalls, particularly I tried passing relative paths instead
of absolute ones but that does not work either. One way or another the
computeDependencyCalls() passes the path to PathUtil.makeAbsolute()and it
gets corrupt.
If you look at the source code you will see that the entire
com.google.javascript.jscomp.deps.PathUtil class uses hardcoded forward
slash "/" as path separator in multiple functions and they work incorrect
under Windows.
I will look further to find a solution.
Tigran.
-----Original Message-----
From: Erik de Bruin [mailto:erik@ixsoftware.nl]
Sent: Monday, April 08, 2013 4:37 PM
To: dev@flex.apache.org
Subject: Re: FalconJX: bugs in external dependencies (Google Closure
Compiler), what to do?
Tigran,
Both Alex and I are on a Mac, so that is probably why it escaped notice
until now :-) Also, the "publisher" side of the code is very much a work in
progress, which deserves all the attention you can give it. I'll answer your
other email some time tonight, jetlag permitting.
On this one: might manipulation the path(s) before they are fed into the
'depswriter' - working from memory, might be missing the point - help avoid
this low level problem?
EdB
On Mon, Apr 8, 2013 at 6:27 AM, Tigran Najaryan <ti...@gmail.com> wrote:
> I debugged the problem that I reported earlier (generating incorrect
> paths in deps.js) and looks like it is due to a bug in
> com.google.javascript.jscomp.deps.PathUtil.isAbsolute. It is
> implemented by simply checking that the path starts by a forward slash
> which of course is incorrect under Windows.
>
> So what is the procedure when the bug is in an external dependency? It
> is not as if it is a single call that can be avoided.
> PathUtil.isAbsolute is called a few levels below in the call stack
> from the
> com.google.javascript.jscomp.deps.DepsGenerator.computeDependencyCalls
> , which is supposed to compute and return the correct text for
> dependencies file.
>
> I will debug this further but just wanted to understand if I should I
> try to work around the external bug when I meet one or I need to try
> something else?
>
> Tigran.
>
> P.S. Did anyone actually try running FalconJX on Windows or everybody
> is developing on a Mac?
>
>
--
Ix Multimedia Software
Jan Luykenstraat 27
3521 VB Utrecht
T. 06-51952295
I. www.ixsoftware.nl
Re: FalconJX: bugs in external dependencies (Google Closure
Compiler), what to do?
Posted by Erik de Bruin <er...@ixsoftware.nl>.
Tigran,
Both Alex and I are on a Mac, so that is probably why it escaped
notice until now :-) Also, the "publisher" side of the code is very
much a work in progress, which deserves all the attention you can give
it. I'll answer your other email some time tonight, jetlag permitting.
On this one: might manipulation the path(s) before they are fed into
the 'depswriter' - working from memory, might be missing the point -
help avoid this low level problem?
EdB
On Mon, Apr 8, 2013 at 6:27 AM, Tigran Najaryan <ti...@gmail.com> wrote:
> I debugged the problem that I reported earlier (generating incorrect paths
> in deps.js) and looks like it is due to a bug in
> com.google.javascript.jscomp.deps.PathUtil.isAbsolute. It is implemented by
> simply checking that the path starts by a forward slash which of course is
> incorrect under Windows.
>
> So what is the procedure when the bug is in an external dependency? It is
> not as if it is a single call that can be avoided. PathUtil.isAbsolute is
> called a few levels below in the call stack from the
> com.google.javascript.jscomp.deps.DepsGenerator.computeDependencyCalls,
> which is supposed to compute and return the correct text for dependencies
> file.
>
> I will debug this further but just wanted to understand if I should I try to
> work around the external bug when I meet one or I need to try something
> else?
>
> Tigran.
>
> P.S. Did anyone actually try running FalconJX on Windows or everybody is
> developing on a Mac?
>
>
--
Ix Multimedia Software
Jan Luykenstraat 27
3521 VB Utrecht
T. 06-51952295
I. www.ixsoftware.nl