Proposal for new module format

[MikeSamuel, 2009-01-21 19:32:21]

http://codereview.appspot.com/11948/diff/1/2
File doc/html/compiledModuleFormat/index.html (right):

http://codereview.appspot.com/11948/diff/1/2#newcode2
Line 2: -- Copyright (C) 2008 Google Inc.
2009

http://codereview.appspot.com/11948/diff/1/2#newcode25
Line 25: <script type="text/javascript" src="../common/prettify.js"></script>
Do we have two version of prettify?  There's one under third_party/js/prettify

http://codereview.appspot.com/11948/diff/1/2#newcode103
Line 103: code: function(___, IMPORTS___) { ... },
Can we name this using a verb as per method naming conventions?

http://codereview.appspot.com/11948/diff/1/2#newcode107
Line 107: manifest: { ...},
- trailing ,

http://codereview.appspot.com/11948/diff/1/2#newcode118
Line 118: <td>The anonymous function representing the module, as before.</td>
Does the signature change in any way?  Is the return value significant?
Does it depend differently on any cajita.js machinery?

http://codereview.appspot.com/11948/diff/1/2#newcode122
Line 122: <td><code class="prettyprint">sha1sum</code></td>
Do we want the name of a particular hashing algorithm in the property name? 
SHA-1 will be replaced at some point by a new hash algo.

http://codereview.appspot.com/11948/diff/1/2#newcode133
Line 133: applications may only check it for equality.</td>
Comparable to what?  Specifically, is it comparable to any value exported by
cajita.js?

http://codereview.appspot.com/11948/diff/1/2#newcode141
Line 141: absent.</td>
This sounds like it has the potential to be larger than all the rest of the data
in the module definition.  Can we give containers enough room to be able to
lazily fetch it?

http://codereview.appspot.com/11948/diff/1/2#newcode149
Line 149: the <code>cajita.manifest()</code> function.</td>
What is in the manifest?  Is the cajita.manifest method new?

[MarkM, 2009-01-21 19:36:27]

Good to see this!

http://codereview.appspot.com/11948/diff/1/2
File doc/html/compiledModuleFormat/index.html (right):

http://codereview.appspot.com/11948/diff/1/2#newcode111
Line 111: <p>The following are the standard keys:</p>
Should also include an enumeration of those varnames that the module obtains
from IMPORTS___ (which will be those vars used freely in the original source).

Perhaps this should include the assumed-safe static path info that we're
currently passing to readImport?

http://codereview.appspot.com/11948/diff/1/2#newcode123
Line 123: <td><em>required</em></td>
The sha1sum should not be required. We also shouldn't use sha1. I do not yet
know of a consensus of what to use instead.

http://codereview.appspot.com/11948/diff/1/2#newcode132
Line 132: translator which emitted this module. This is an opaque value;
Strings are not opaque, and it seems meaningless to require apps to check them
only for equality. I also think equality is more severe than you want anyway.

[Jasvir, 2009-01-21 19:43:40]

http://codereview.appspot.com/11948/diff/1/2
File doc/html/compiledModuleFormat/index.html (right):

http://codereview.appspot.com/11948/diff/1/2#newcode107
Line 107: manifest: { ...},
Other standard keys to consider:
date cajoled, original source

http://codereview.appspot.com/11948/diff/1/2#newcode122
Line 122: <td><code class="prettyprint">sha1sum</code></td>
On 2009/01/21 19:32:22, MikeSamuel wrote:
> Do we want the name of a particular hashing algorithm in the property name? 
> SHA-1 will be replaced at some point by a new hash algo.

When the algorithm changes, we probably want old signatures to break too.  We
can split the checksum into two keys, one that names the algorithm and one that
gives it a value but I am not sure how that's better than this.

[MikeSamuel, 2009-01-21 19:55:54]

http://codereview.appspot.com/11948/diff/1/2
File doc/html/compiledModuleFormat/index.html (right):

http://codereview.appspot.com/11948/diff/1/2#newcode122
Line 122: <td><code class="prettyprint">sha1sum</code></td>
On 2009/01/21 19:43:40, jasvir wrote:
> On 2009/01/21 19:32:22, MikeSamuel wrote:
> > Do we want the name of a particular hashing algorithm in the property name? 
> > SHA-1 will be replaced at some point by a new hash algo.
> 
> When the algorithm changes, we probably want old signatures to break too.  We
> can split the checksum into two keys, one that names the algorithm and one
that
> gives it a value but I am not sure how that's better than this.

Fair enough.
What's the point of having a hash anyway?
Aren't we more likely to want to verify a signature?
So the container JS has one or more DSA public keys baked in, and it can then
verify content from one of a set of trusted gadget providers.

[ihab.awad, 2009-01-22 17:02:05]

Ok folks, check it out now.

http://codereview.appspot.com/11948/diff/1/2
File doc/html/compiledModuleFormat/index.html (right):

http://codereview.appspot.com/11948/diff/1/2#newcode2
Line 2: -- Copyright (C) 2008 Google Inc.
On 2009/01/21 19:32:22, MikeSamuel wrote:
> 2009

Fixed.

http://codereview.appspot.com/11948/diff/1/2#newcode25
Line 25: <script type="text/javascript" src="../common/prettify.js"></script>
On 2009/01/21 19:32:22, MikeSamuel wrote:
> Do we have two version of prettify?  There's one under third_party/js/prettify

Well then I guess we *are* schlepping around 2 versions. :/ But the one in 3pty
does not seem to have prettify.css. What's to do?

http://codereview.appspot.com/11948/diff/1/2#newcode103
Line 103: code: function(___, IMPORTS___) { ... },
On 2009/01/21 19:32:22, MikeSamuel wrote:
> Can we name this using a verb as per method naming conventions?

Good point. Fixed.

http://codereview.appspot.com/11948/diff/1/2#newcode107
Line 107: manifest: { ...},
On 2009/01/21 19:32:22, MikeSamuel wrote:
> - trailing ,

Fixed.

http://codereview.appspot.com/11948/diff/1/2#newcode107
Line 107: manifest: { ...},
On 2009/01/21 19:43:40, jasvir wrote:
> Other standard keys to consider:
> date cajoled, original source

Added date field.

For the original source, this is messy: how would we represent the entire set of
files? I added a field with my best guess (a simple file name -> string literal
map) and made it optional.

http://codereview.appspot.com/11948/diff/1/2#newcode111
Line 111: <p>The following are the standard keys:</p>
On 2009/01/21 19:36:27, MarkM wrote:
> Should also include an enumeration of those varnames that the module obtains
> from IMPORTS___ (which will be those vars used freely in the original source).

Fixed.

> Perhaps this should include the assumed-safe static path info that we're
> currently passing to readImport?

Good point. Added a TODO.

http://codereview.appspot.com/11948/diff/1/2#newcode118
Line 118: <td>The anonymous function representing the module, as before.</td>
On 2009/01/21 19:32:22, MikeSamuel wrote:
> Does the signature change in any way?  Is the return value significant?
> Does it depend differently on any cajita.js machinery?

No, no and, _modulo_ the fact that the cajita.js machinery would just need to
dig one level to call module.execute(imports) rather than module(imports),
nothing new need happen.

http://codereview.appspot.com/11948/diff/1/2#newcode122
Line 122: <td><code class="prettyprint">sha1sum</code></td>
On 2009/01/21 19:55:55, MikeSamuel wrote:
> What's the point of having a hash anyway?

Bobbitted.

http://codereview.appspot.com/11948/diff/1/2#newcode123
Line 123: <td><em>required</em></td>
On 2009/01/21 19:36:27, MarkM wrote:
> The sha1sum should not be required. We also shouldn't use sha1. I do not yet
> know of a consensus of what to use instead.

See above.

http://codereview.appspot.com/11948/diff/1/2#newcode132
Line 132: translator which emitted this module. This is an opaque value;
On 2009/01/21 19:36:27, MarkM wrote:
> Strings are not opaque, and it seems meaningless to require apps to check them
> only for equality. I also think equality is more severe than you want anyway.

Ok, reworded.

http://codereview.appspot.com/11948/diff/1/2#newcode133
Line 133: applications may only check it for equality.</td>
On 2009/01/21 19:32:22, MikeSamuel wrote:
> Comparable to what?  Specifically, is it comparable to any value exported by
> cajita.js?

Fixed; see current text.

http://codereview.appspot.com/11948/diff/1/2#newcode141
Line 141: absent.</td>
On 2009/01/21 19:32:22, MikeSamuel wrote:
> This sounds like it has the potential to be larger than all the rest of the
data
> in the module definition.  Can we give containers enough room to be able to
> lazily fetch it?

Right ... well ....

First of all, this would only be provided if a module is cajoled using debug
mode so, in the common (end-user) case, this is not an issue.

Making it possible to fetch that information separately means we need to define
2 formats instead of 1, and worry about the sort of contract a container would
have with its server.

Given the constraints, I would say this is not a biggie for now. We can always
add something if it becomes a bottleneck later.

http://codereview.appspot.com/11948/diff/1/2#newcode149
Line 149: the <code>cajita.manifest()</code> function.</td>
On 2009/01/21 19:32:22, MikeSamuel wrote:
> What is in the manifest?  Is the cajita.manifest method new?

It's been in the works for a long time but we never really took it seriously.
This just allocates a slot for it.

As envisioned so far, cajita.manifest() is something that a module could call,
passing a JSON object. This JSON gets treated as module "metadata" and is
available to the runtime (and to any of a variety of downstream tools,
conceivably, even ones that treat the module as text) for whatever bookkeeping
needs to be done. In that respect, it is very much like a MANIFEST.MF.

[MarkM, 2009-01-22 17:39:27]

http://codereview.appspot.com/11948/diff/8/9
File doc/html/compiledModuleFormat/index.html (right):

http://codereview.appspot.com/11948/diff/8/9#newcode118
Line 118: <td><code class="prettyprint">execute</code></td>
Based on reading the html without the css, you should add a 
   valign="top"
to your left column headings. I don't know if this is really needed given your
css.

http://codereview.appspot.com/11948/diff/8/9#newcode135
Line 135: <td><code class="prettyprint">cajoledDate</code></td>
Why is the cajoledDate required? I think it should be optional.

http://codereview.appspot.com/11948/diff/8/9#newcode160
Line 160: literal containing the text of the file. The text is escaped such that
Even initially, I think we should somehow make an allowance for the cajoler to
instead provide URLs from which the original source can be fetched, rather than
shipping the original source directly.

http://codereview.appspot.com/11948/diff/8/9#newcode180
Line 180: the <code>cajita.manifest()</code> function.</td>
calling the caja.manifest() function [with a static JSON literal].

It would not work to call it with an expression that needs to be evaluated,
since the whole point is to enable the manifest to be obtained without (or prior
to) calling the module function.

http://codereview.appspot.com/11948/diff/8/9#newcode185
Line 185: <div class="note">TODO: Move support for the "assumed-safe static
path" information
The right place for this is in the variable metadata above, where you already
say "corresponding value is some JSON data providing meta-information about the
module's expectations regarding that variable." The assumed-safe static path
info is such an expectation.

[ihab.awad, 2009-01-22 19:55:16]

http://codereview.appspot.com/11948/diff/8/9
File doc/html/compiledModuleFormat/index.html (right):

http://codereview.appspot.com/11948/diff/8/9#newcode118
Line 118: <td><code class="prettyprint">execute</code></td>
On 2009/01/22 17:39:28, MarkM wrote:
> Based on reading the html without the css, you should add a 
>    valign="top"
> to your left column headings. I don't know if this is really needed given your
> css.

It isn't. But added anyway for standalone readability. TD valign defaults are a
constant pain in the neck for me. :(

http://codereview.appspot.com/11948/diff/8/9#newcode135
Line 135: <td><code class="prettyprint">cajoledDate</code></td>
On 2009/01/22 17:39:28, MarkM wrote:
> Why is the cajoledDate required? I think it should be optional.

At this point we're taking a best guess at the fields ahead of time, so my first
inclination is to just say "ok" to all feedback. I'm really most interested in
getting the format and keeping things simple.

That all said, I guess making it optional means a cajoler can be implemented in
a secure sandbox where it does not have access to the date, thus helping ensure
that it is deterministic. So => changed to 'optional'.

http://codereview.appspot.com/11948/diff/8/9#newcode160
Line 160: literal containing the text of the file. The text is escaped such that
On 2009/01/22 17:39:28, MarkM wrote:
> Even initially, I think we should somehow make an allowance for the cajoler to
> instead provide URLs from which the original source can be fetched, rather
than
> shipping the original source directly.

This all depends on what the "original source" is used for. Typically, it is
used for debugging.  Given the size of the line-by-line debugging information,
it's not clear to me that the original source will cause enough of a blowup of
the module size to be worth bothering. Plus, including the literal original
source ensures that the server does not depend on the stability of outside URLs
for correctness of the debugging.

So really => we are debugging (so to speak) the *debugging* experience. 

Do you still think this is worth pursuing any further?

http://codereview.appspot.com/11948/diff/8/9#newcode180
Line 180: the <code>cajita.manifest()</code> function.</td>
On 2009/01/22 17:39:28, MarkM wrote:
> calling the caja.manifest() function [with a static JSON literal].
> 
> It would not work to call it with an expression that needs to be evaluated,
> since the whole point is to enable the manifest to be obtained without (or
prior
> to) calling the module function.

I assume the alternative you propose is for the cajita.manifest() argument to be
a string literal?

Since we parse the program, we can hoist the parse tree generated by the
argument of cajita.manifest() one level of instantiation upwards into the module
envelope (after checking that it is JSON, which includes making sure that it has
no references to external variables). Making it part of the parsed program
allows us to do this checking more easily, and allows programmers to use their
native syntax highlighters and what not to write the stuff.

http://codereview.appspot.com/11948/diff/8/9#newcode185
Line 185: <div class="note">TODO: Move support for the "assumed-safe static
path" information
On 2009/01/22 17:39:28, MarkM wrote:
> The right place for this is in the variable metadata above, where you already
> say "corresponding value is some JSON data providing meta-information about
the
> module's expectations regarding that variable." The assumed-safe static path
> info is such an expectation.

Done.

[ihab.awad, 2009-01-28 23:58:33]

Ready for more review.

[felix8a, 2009-02-02 20:52:21]

http://codereview.appspot.com/11948/diff/402/403
File doc/html/compiledModuleFormat/index.html (right):

http://codereview.appspot.com/11948/diff/402/403#newcode207
Line 207: it will be bit-identical to the original source <em>when parsed as a
"bit-identical" seems wrong since js strings are defined as utf-16 while much of
the web is encoded as utf-8.  maybe "char-identical"?

http://codereview.appspot.com/11948/diff/402/403#newcode271
Line 271: source location(s) corresponding to each character position in the
do you really need to specify source location for every char of the output?  it
seems to me this would make debugging info about 100x larger than the source.  I
think I'd be happy with just source range for each line of the output, since
it's relatively cheap to add artificial line splits in the output.

[MikeSamuel, 2009-02-02 21:05:03]

http://codereview.appspot.com/11948/diff/402/403
File doc/html/compiledModuleFormat/index.html (right):

http://codereview.appspot.com/11948/diff/402/403#newcode207
Line 207: it will be bit-identical to the original source <em>when parsed as a
On 2009/02/02 20:52:21, felix8a wrote:
> "bit-identical" seems wrong since js strings are defined as utf-16 while much
of
> the web is encoded as utf-8.  maybe "char-identical"?

Java and python are both UTF-16 and most UTF-8 text out there is really a UTF-8
encoding of UTF-16 code-units, so there seems to be an emerging consensus around
UTF-16 at some level.

Maybe "UTF-16 code-unit identical" would be ideal.

I dislike the use of file names since those are platform specific.