890182 - Build option for app build to specify full zip contents

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::E-Mail, defect)

Description

[James Burke [:jrburke]]

With #886446, the email app is doing more of its own optimization, and it would be ideal if the Gaia build system had a way for an app to specify that it will generate the full directory contents to be used for the zipfile/final webapp install.

I started a prototype in this branch:

https://github.com/jrburke/gaia/tree/app-zip

Compare view:

https://github.com/jrburke/gaia/compare/app-zip

The main idea, if there is a gaia_build.json file in the app directory, and that JSON has a "dir" property, then that directory will be used for all the gaia build steps after running the app-specific Makefile.

Still lots to do for the prototype, namely:

* maybe move built up a directory, out of the app dir, perhaps a build_stage directory as a sibling to build. This would avoid the build contents showing up in code scans by the developer's IDE from matching things in the build directory.
* parse through all JS files for shared deps, and in css, then trim shared folder appropriately.
* Test DEBUG=1 builds, does dev save/refresh work?
* Test on windows

but mostly wanted to get some sketch to start the conversation.

Comment 1

[James Burke [:jrburke]]

Attachment: Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/10868

Pointer to Github pull-request

Comment 2

[James Burke [:jrburke]]

Comment on attachment 772708 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/10868

This changeset touches some common build files as well as some email files. This was done to test out the the common build changes and show that they are working.

## Common changes

* build/utils.js: sets a buildDirectoryFile for each webapp, set initially to sourceDirectoryFile. If the app has a gaia_build.json file, it will read that and set the "dir" property in there to buildDirectoryFile.
* The build/webapp-*.js files use this buildDirectoryFile when dealing with what directory to use for build steps.
* build/webapp-zip.js now reads a buildDirectoryFile + 'gaia_shared.json" file to get shared resources that should be included in the zip, as well as do the existing HTML scanning it does before this changeset.

## Email changes

* Specifies a gaia_build.json setting the "dir" for buildDirectoryFile to be gaia/build_stage/email.
* Runs a "full project" optimization inside build_stage/email. This sets the stage for doing other optimized layers that depend on each other.
* Adds email/build/make_gaia_shared.js that generates the gaia_shared.json for a build by scanning JS dependencies for "shared/*" dependencies and CSS for @import calls that reference shared CSS files.
* html_cache_restore.js is more generic, reading data- attributes for its work. Also only loads a loader when in dev (make DEBUG=1) so that the source can run as-is in desktop browsers (data-loader attribute removed in the build process)
* Removed some unneeded files.

Comment 3

[James Burke [:jrburke]]

Comment on attachment 772708 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/10868

Asking :ochameau for a review of the common build/ changes. If I chose incorrectly, just let me know who might be a better person to ask. I was going off the Modules listed here: https://wiki.mozilla.org/Modules/FirefoxOS#Gaia

Comment 4

[James Burke [:jrburke]]

Comment on attachment 772708 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/10868

Asking :asuth for a review of the email-specific changes.

Comment 5

[Alexandre Poirot [:ochameau]]

Comment on attachment 772708 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/10868

Sorry for getting back to you so late!

For various reason, the first one being localization, we desperatly need a stage dir for building apps before zipping them. This patch gives me the feeling that we are working around it instead of just getting to it. I'd really like to see all apps being build in such `build_staged/$(app)`, not only email!

If you feel tackling this more general goal, that would be really great. Otherwise your patch looks quite trivial, so we can go for it in the meantime. Just keep in mind, that at some point, we will have stage dir for all apps and we will most likely get rid of this `dir` property of gaia_build.json, and may be drop completely this json file.

For now, I'm reseting the r? to get your feedback about this and also because of the exception I'm seeing.

Comment 6

[James Burke [:jrburke]]

:ochameau -- I updated the pull request:

* Fixed the '..' error with the approach you mentioned.
* Fixed the other error that happened on Windows.
* Merged latest master and rebased.

> Shouldn't we delete gaia_build.json here instead of in each app Makefile?

My initial thought is if the app Makefile specifies that it wants to do its own thing for assembling the directory used for the build, it is indicating it will do any cleanup it wants in the directory.

Since the attachment/pointer to the pull request did not change, just the contents, then I will ask for review again on the same attachment.

On using `build_stage/$(app)` for all apps:

I suppose that would work out, but it still seems like something like a gaia_build.json may be needed for apps that want to handle the copying of files for the start of the staged dir, which is what email is doing in this pull request. Here is an attempt to specify a general `build_stage/$(app)` approach:

For each app:

* If no gaia_build.json, or if "copy_stage" is not true in gaia_build.json, copy the app to build_stage/$(app)

* If there is a gaia_build.json with "copy_stage" set to true, run the a "copy_stage" Makefile invocation in the app's source directory (and the email app would be modified to take this path).

* After copying to the stage is done (either by main Makefile or by app's Makefile) if the app has a custom Makefile in build_stage/$(app), run it (to match the custom app Makefile behavior that exists today).

* Do rest of build tasks using build_stage/$(app).

What I am unsure about is how the DEBUG=1 pathway should work, as that seems to point to the source directories, even though today, all app Makefiles are run.

The more I think about it, it seems like the more general work for the build needs more thought, and probably is worth starting a separate ticket on it to work out the design. I would prefer to see the pull request in this changeset land as it sets up the pathway for email to do further build changes as mentioned in 892069.

I can start a new bugzilla ticket, starting with the sketch above for the general change if you think that is helpful, and I can help with any change as I expect I will need to validate that the email app still does the right thing with any new approach.

Comment 7

[Alexandre Poirot [:ochameau]]

Comment on attachment 772708 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/10868

Looks good now.

Comment 8

[Alexandre Poirot [:ochameau]]

(In reply to James Burke [:jrburke] from comment #6)
> On using `build_stage/$(app)` for all apps:
> 
> I suppose that would work out, but it still seems like something like a
> gaia_build.json may be needed for apps that want to handle the copying of
> files for the start of the staged dir, which is what email is doing in this
> pull request. Here is an attempt to specify a general `build_stage/$(app)`
> approach:
> 
> For each app:
> 
> * If no gaia_build.json, or if "copy_stage" is not true in gaia_build.json,
> copy the app to build_stage/$(app)
> 
> * If there is a gaia_build.json with "copy_stage" set to true, run the a
> "copy_stage" Makefile invocation in the app's source directory (and the
> email app would be modified to take this path).

We can always call the copy_stage rule and it will be up to the app to implement one or not.

> 
> * After copying to the stage is done (either by main Makefile or by app's
> Makefile) if the app has a custom Makefile in build_stage/$(app), run it (to
> match the custom app Makefile behavior that exists today).
> 
> * Do rest of build tasks using build_stage/$(app).
> 
> What I am unsure about is how the DEBUG=1 pathway should work, as that seems
> to point to the source directories, even though today, all app Makefiles are
> run.

You are right, it can easily break DEBUG mode, it is currently very clunky and not all resources can be edited live. I think shared/ resources stay to the version when we ran DEBUG=1 make.

> 
> The more I think about it, it seems like the more general work for the build
> needs more thought, and probably is worth starting a separate ticket on it
> to work out the design. I would prefer to see the pull request in this
> changeset land as it sets up the pathway for email to do further build
> changes as mentioned in 892069.

WFM

> 
> I can start a new bugzilla ticket, starting with the sketch above for the
> general change if you think that is helpful, and I can help with any change
> as I expect I will need to validate that the email app still does the right
> thing with any new approach.

I'm not sure the current DEBUG feature as it exists today is really sustainable as we add more and more build-time features.
I was wondering if it makes sense to run make for a single APP anytime we reload the page on Firefox, or anytime httpd receive a request for launch_path, or something alike.
At the end, I'd really like to see httpd being a dump http server without any gaia specific other than serving app objdir for each app domain.
But we are clearly having a strong build step behavior by doing such and it can easily break live editing behavior.
I'll give some more thoughts about that.

Comment 9

[Andrew Sutherland [:asuth] (he/him)]

Comment on attachment 772708 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/10868

Files that end up in the zip file that shouldn't:
- build.txt (put it in the source tree or )
- gaia_share.json (minor but seems unavoidable given the contract)

My only other comment is that it would be great if you could put some of your comments from comment 2 and comment 6 on this bug into either comments in parts of the build mechanism or in a markdown readme file.  I know when I've previously dealt with the build machinery for Gaia I have been both confused and frustrated because it's not obvious what the intent was.

Comment 10

[James Burke [:jrburke]]

I removed the build.txt file, and added some build comments to Email's README. gaia_shared.json doe need to stay, but it is similar to having some commented out HTML for the previous shared approach, and now at least it is not in the runtime HTML.

I opened bug 897352 to track a possible change to generically move to a staged directory for all apps.

I rebased and will be merging into gaia master next.

Comment 11

[James Burke [:jrburke]]

Merged into Gaia master:

https://github.com/mozilla-b2g/gaia/commit/9b2952800c6a347415f497a8dd6a424ea4e6252d