1159411 - Preload Marketplace and hosted-online apps into B2G's Gaia build using preload-app-toolkit

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Build, defect, P1)

Description

[Doug Sherk (:drs) (inactive)]

In bug 1158367, we'll be loading some additional apps from source into the Gaia tree, using the repo tool. However, this doesn't allow us to pre-load apps from Marketplace or other online sources.

We currently have a specialized build system that we will have to abandon in order to get all of our Spark apps into master. It can be found here:
https://github.com/fxos/lightsaber

As part of abandoning this build system, we'll have to figure out a way to be able to pre-load apps from Marketplace and online. :dhylands and I discussed over IRC how we could do this. Here's the transcript:

> 17:00 drs: dhylands: hey, I'm working on moving all of our specialized build code from the lightsaber repo into the B2G repo. I'd like your input on part of that. for apps that are on Marketplace that we want to preload, we're currently using preload-app-toolkit with a list of those apps as part of our make script, but I'm not sure where that would go in B2G. what do you think?
> 17:00 dhylands: drs: If its specific to apps, I would think that it should go into gaia
> 17:00 drs: I was thinking of forking preload-app-toolkit, including our app list in that fork, as well as an Android.mk that actually runs it and copies the files over to ../gaia/outoftree_apps/, but that seems really fragile
> 17:01 drs: hm. I didn't think including preload-app-toolkit into Gaia was a good idea
> 17:01 dhylands: drs: But if its got a git repository, then it probably shouldn't in gaia
> 17:02 drs: dhylands: https://github.com/fxos/lightsaber/blob/master/get-external-apps.sh
> 17:02 drs: https://github.com/fxos/lightsaber/blob/master/list
> 17:02 dhylands: We have most of the the stuff in external
> 17:02 drs: https://github.com/fxos/lightsaber/blob/master/Makefile#L62-L63
> 17:02 dhylands: external is bascially a collection of git repositories that each have an Android.mk file in them
> 17:03 drs: yeah, these are webapps though, not libraries
> 17:03 dhylands: Where and when they get built will depend on the dependencies that each particular Android.mk file sets up
> 17:03 dhylands: For example, external/init_sh is where I put the Z3/Z3C recovery setup stiff
> 17:03 dhylands: stuff
> 17:04 dhylands: Stuff in external isn't necessarily libraries
> 17:04 dhylands: They're a combination of libraries, apps, utiltities, etc
> 17:04 drs: I'm not sure how this could be structured sanely
> 17:05 drs: how could I make gaia depend on something in external?
> 17:05 drs: e.g. once I pull the apps, I want them to be copied into B2G/gaia/outoftree_apps/
> 17:05 dhylands: Put a dependency in gaia's Android.mk or put it in the something's Android.mk
> 17:06 dhylands: All of the Android/mk files from the entire tree are read and processed before any building happens
> 17:06 drs: are there any good examples of this, that are doing something similar to what I'm looking for?
> 17:06 dhylands: So you can setup dependencies pretty much any where you want
> 17:06 drs: also, how would you suggest maintaining the list of apps that we want to preload, and the code that actually calls preload-app-toolkit with that list?
> 17:07 drs: maybe put that in another repo, and pull in preload-app-toolkit as a separate dependency?
> 17:08 dhylands: So init_sh adds dependencies on busybox, init and extract_elf_ramdisk
> 17:09 drs: is that `LOCAL_REQUIRED_MODULES`?
> 17:09 dhylands: gaia has various files for the list of apps. Look in gaia/build/config/tablet or phone and there is a file for various types of builds
> 17:09 dhylands: Yeah - The modules listed in LOCAL_REQUIRED_MODULES will be build before this module is built
> 17:10 drs: dhylands: the gaia list isn't the same as what I'm looking for, since I want to preload the apps from Marketplace
> 17:10 dhylands: I guess its kind of arbitrary
> 17:10 drs: e.g. during build, we fetch the apps from Marketplace, then put them into the Gaia build in the outoftree_apps folder
> 17:10 dhylands: You can do whatever you feel like. It really depends on what different types of configurations you'll want
> 17:11 drs: ok
> 17:11 dhylands: I'd probably create a file that has one URL per line or something like that
> 17:11 drs: yeah: https://github.com/fxos/lightsaber/blob/master/list
> 17:11 dhylands: And read that file line by line and do a wget
> 17:12 drs: preload-app-toolkit does all this for us, we just need to call it with a list
> 17:12 dhylands: Write a script which does what you want and call the script from the Android.mk file
> 17:13 dhylands: Or put the script inline in the Makefile if its simple enough
> 17:13 drs: ok, I see. so I can pull something into external that depends on preload-app-toolkit which has this script. let's call it gaia-preload. and then I can make gaia depend on gaia-preload?
> 17:14 drs: is there a way I can make gaia-preload get built before gaia, without modifying Gaia's Android.mk?
> 17:14 drs: or does it not matter? is Android.mk only used for the B2G repo?
> 17:15 drs: also, how would I then safely copy the apps that I download into gaia/outoftree_apps/? is it ok to just use '../../gaia/outoftree_apps/'?
> 17:34 dhylands: Well the B2G repo has almost nothing in it
> 17:35 dhylands: All of the Android.mk files from the entire B2G tree are processed
> 17:36 dhylands: To make gaia depend on something you need to figure out the proper name of the gaia target
> 17:45 dhylands: drs: Hmm. You may need to put the dependency in gaia's Android.mk
> 17:47 dhylands: So you might need to do it by having something set externally and create a new variable like OPTIONAL_GAIA_REQUIRED_MODULES which is empty for a normal build and set to preload-app-toolkit's #LOCALMODULE when doing an ignite build
> 18:23 drs: dhylands: yuck
> 18:27 drs: dhylands: is there any better way to do this? that sounds pretty bad
> 18:29 dhylands: drs: I don't think so. The problem is that we can't control the order the Android.mk files are processed and the Makefile uses :=
> 18:29 drs: okay

This bug is for implementing the gaia-preload package and the gaia repo's optional dependencies on it, so that we can pre-load Marketplace and online hosted apps.

Here are some helpful links.

Current pre-loaded apps in our specialized build system:
https://github.com/fxos/lightsaber/blob/master/list

Our script that calls preload-app-toolkit as part of our specialized build system:
https://github.com/fxos/lightsaber/blob/master/get-external-apps.sh

The place in our build where the get-external-apps.sh script is run:
https://github.com/fxos/lightsaber/blob/master/Makefile#L62-L63

In bug 1158367, we'll be loading apps directly into gaia/outoftree_apps, like so:
<project path="gaia/outoftree_apps/sharing" name="sharing" remote="fxos" revision="gh-pages" />

Thus, we should also copy these apps into the gaia/outoftree_apps folder.

Comment 1

[Doug Sherk (:drs) (inactive)]

Dave, could you take a look at this?

Comment 2

[Doug Sherk (:drs) (inactive)]

We should also pre-load the Foxfooding app. It's hosted at:
https://fxos.github.io/foxfooding (with a manifest.webapp in the root)

Comment 3

[Dale Harvey (:daleharvey)]

Why not add preload.py to /tools/ - https://github.com/mozilla-b2g/gaia/tree/master/tools

and add support for recognising a url to .webapp in https://github.com/mozilla-b2g/gaia/blob/master/build/config/phone/apps-dogfood.list and running preload on it.

Comment 4

[Dale Harvey (:daleharvey)]

Sorry for having to leave the call, I had to leave for dinner, but to summarise what I think.

This is a gaia build issue, having it inside the B2G tree is confusing, Having |./flash.sh gaia| running very differently from |make install-gaia| is also confusing, and telling developers "you want to test spark, start using B2G + repo instead of gaia is fairly inaccessible.

So as above, I propose 'spark' becomes a custom build of gaia, we give it a custom app list in the customisations folder, we add support for preloading those built apps with preload.py. We can debate whether that customisation is enabled by default later, as far as I can tell there is a large amount of work needed in order to properly distribute firefox os (something we have never dont properly) and happy to kick off that discussion on dev-internal.

If everyone is happy with that, I can kick off the work

Comment 5

[Doug Sherk (:drs) (inactive)]

There's no need for debate about whether or not the customizations will be default as it has already been decided that they will be. Any solution that makes it harder for us to build with them, such as requiring build-time flags other than |PRODUCTION=1| and whatever other flags we build production builds with, is unsuitable.

Here is what would be acceptable, given our constraints, though:

1. We use the customizations infrastructure, as you suggested.
2. We ship a default set of customizations, which vendors and developers can disable if they don't want. The important part here is that it's opt-out, rather than opt-in, at least for now.
3. We include the preload.py script into the Gaia tree, and some method of preloading apps using it.

Doing this will require significantly more work -- as well as more dealing with pushback -- than the method that Dave is working on using the B2G tree. So even if we follow this customizations path, we run the risk of it not being ready in time for the Spark launch. Thus we should continue with this bug as was originally planned, and roll out your proposal if it satisfies our requirements and is ready in time.

It's important to note that this isn't meant to be a permanent solution.

Comment 6

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(In reply to Doug Sherk (:drs) (use needinfo?) from comment #5)
> 1. We use the customizations infrastructure, as you suggested.
> 2. We ship a default set of customizations, which vendors and developers can
> disable if they don't want. The important part here is that it's opt-out,
> rather than opt-in, at least for now.
> 3. We include the preload.py script into the Gaia tree, and some method of
> preloading apps using it.

I've already said we can't trigger downloading of an arbitrary version of external resource either |./build.sh| or |make|. Our buildbots can't handle it (unless there were some impovement unknown to me around TaskCluster). Our QA can't lock an regression window on these apps. Our downstream vendors trapped in Chinese Intranet or in locked-down server farm can't work with them. Julien who build code on a commute train can't work with it. Our community friends on 56K modem in remote part of the world can't use them. Again, this is not of my personal taste or preferences, this is just an external constraint we must honor.

At the very least we would need to pin them with a hash, either with submodule or custom logic like what's down with module.tar.

> It's important to note that this isn't meant to be a permanent solution.

Permanent is a relative concept, <insert dramatic example here> wasn't meant to be permanent but it stayed. I am supportive in terms of meeting a deadline set in MoCo, but it's important to acknowledge we are lowering the quality of Mozilla Open Source Project to do what you want here, and you are ultimately responsible of fixing them.

Comment 7

[Dale Harvey (:daleharvey)]

> I've already said we can't trigger downloading of an arbitrary version of 
> external resource either |./build.sh| or |make|. Our buildbots can't handle it 
> (unless there were some impovement unknown to me around TaskCluster). 
> Our QA can't lock an regression window on these apps. Our downstream vendors trapped 
> in Chinese Intranet or in locked-down server farm can't work with them. Julien 
> who build code on a commute train can't work with it. Our community friends on 
> 56K modem in remote part of the world can't use them. Again, this is not of 
> my personal taste or preferences, this is just an external constraint we must honor.

I entirely agree, and this is why I propose it being a non default customization. 

It may be easier for the purposes of the discussion to not think of the customizations folder as customizations but as distributions. Gaia as is is a development environment and when we want to distribute it in a particular way with particular applications we create a distribution, much in the same way our partners already do and these distributions do not require the same limitations and requirements as core.

> At the very least we would need to pin them with a hash, either with submodule or custom logic 
> like what's down with module.tar.

I dont think it makes sense to do that, a lot of the applications we do not control the source to and pinning their versions is not possible (twitter, facebook, loop). 

> Doing this will require significantly more work -- as well as more dealing 
> with pushback -- than the method that Dave is working on using the B2G tree.

This is wrong, all the tools to add this to the customisation already exist they just need plumbing, as previously mentioning pinning the versions of apps we do not control is not possible and requires extra work for applications we do control and that is not mentioning the extra confusion that is entailed.

I think the main reason there is pushback is because there has been a huge lack of communication about what it planned and a lot of "there doesnt need to be a discussion, I have decided", the lack of collaboration is extremely concerning since I believe there is a good chance we want the same vision / end goal.

Comment 8

[Dave Hylands [:dhylands]]

After further discussion, we decided not to implement it this way, since this gives unreproducible builds.

What we'll do instead, is take whatever this would have done and check that result into a git repository. We'll then add that git repository to the gaia.xml manifest file (ala bug 1158367).

On some type of periodic basis (could be automatic, manual, whatever) the git repository will get new versions. But at least we can go back to a known set of hashes and get the same build.

Comment 9

[Doug Sherk (:drs) (inactive)]

Though comment 8 seemed like a good solution to me because it solved the reproducibility constraint and wasn't a huge commitment of resources, we can't do it anymore. Since Dave and I discussed this, and he wrote comment 8, our requirements have changed, so I'm reopening this. I found out that we may be distributing master relatively soon to partners, and they may or may not want Spark to be part of it. Thus we have but one choice that I can see, which is to augment and use the customizations infrastructure as Dale has proposed. To be clear, my opposition to this was due to our time constraints, and that only. Since this is now the only solution that solves our problems, we'll have to make it work, and get it done in time.

Here's my new proposal, which I think everyone will be happier with, though it introduces the repo tool into Gaia, which Tim didn't agree with:

1. We make the Spark changes, such as wallpapers, Homescreen layout, and new apps, all part of the customizations infrastructure via a new "spark" customization that we ship, but don't enable by default.
2. To satisfy reproducibility, Spark apps will be pulled in via the repo tool, but only as part of the "spark" customization. We will have to add the repo tool to the Gaia repo to do this.
  a. We can't use preload-app-toolkit, since it doesn't satisfy reproducibility.
  b. The "spark" customization can't be used by default because it's pulling in content from outside during a build step.
3. We will create a series of proxy repos that house just manifests and app caches for apps that we don't control, such as Facebook, Twitter, etc.
4. For apps that we do control, we will serve pre-built versions of those apps on their gh-pages branches, which the Gaia repo tool will pull in. Since these branches will be versioned, they can be pinned to specific hashes.
5. We get the A-team to start doing all master builds with the |CUSTOMIZATION=spark| build-time flag set.

Side notes: I don't think the Homescreen customizations work anymore, so we will likely have to fix that. It seems to me from my experimentation with customizations that it has generally fallen into disrepair, but I could be wrong.

(In reply to Dale Harvey (:daleharvey) from comment #7)
> I think the main reason there is pushback is because there has been a huge
> lack of communication about what it planned and a lot of "there doesnt need
> to be a discussion, I have decided", the lack of collaboration is extremely
> concerning since I believe there is a good chance we want the same vision /
> end goal.

This wasn't my decision, it was a MoCo decision.

I'm actually very unopinionated on how this build system should look at the end of the day, and I'd prefer to leave this to the build peers and v3 arch people who are doing more work on hosted apps than we are. But we need something in the interim, and soon. I'd be happy to be part of the effort to make this better in the future, too.

I agree with you, based on my understanding of your position, that we have the same end goal.

Dale, can you help drive this as you offered to do? I'd be happy to provide any support needed once we reach an agreement on how we're going to do this. Also needinfoing Tim for discussion.

Comment 10

[Dale Harvey (:daleharvey)]

While storing appcaches may make reproducable builds in some circumstances it requires more work and doesnt take into account dynamically generated appcaches nor service workers. It also requires more maintenance and will likely lead to the same breakage as just not doing the preload at all.

Builds reproducibility should not be an issue, we are not shipping twitter we are shipping a distribution with links to external applications, the preload doesnt even have to happen during 'build' time it can happen afterwards in a conceptually 'pre runtime', if it doesnt run the build still works. I entirely agree with Tim's worries about requiring a connection to have a default gaia build to run, we are already doing a bad job of enabling offline development and our build is already too slow. I would very much like to see that improve, not get worse.

My proposed solution still seems the best way forward, add preload.py to tools, preload apps when we see a url to a .webapp in the application list, the default build changes in no way whatsoever and having support for adding arbitrary web application to a custom gaia profile seems like a very uncontroversial feature. Tim would you agree?

I am however not comfortable with putting my time and effort into this project in the current way collaboration is happening, I will follow up with that via email.

Comment 11

[Tim Guan-tin Chien [:timdream] (please needinfo)]

I would like us stick to what's already being work on in bug 1158367. It's the less complex solution and put the existing repo tool in B2G to work.

Comment 12

[Dale Harvey (:daleharvey)]

I dont agree its the less complex solution, I should be able to have this up and running by today. To state my thoughts:

We want to add some applications that we may or may not have control over the source over into firefox os that is distributed to firefox os users. Ideally these applications have their cache preloaded so when they are first used without a connection they still load.

https://bugzilla.mozilla.org/show_bug.cgi?id=1158367 is proposing to fetch the appcache of these applications (if they have one) and create repositories for their source files which is fetched as part of the build process in the B2G repo.

In this patch I propose we add a /distribution/spark to /gaia and run preload on these external web applications

Upsides to doing it in B2G: 
 * We have reproducible builds that work offline

Downsides to B2G
 * We have to keep the source repositories up to date manually
 * Even doing so we will get outdated precaches
 * It is a large amount more work, both initially and for maintenance
 * It is inaccessible to developers using the gaia repository
 * We diverge the code path a large amount from developers using the gaia repo vs the b2g repo
 * Will only ever work for appcache, Service worker application priming will have to work completely differently

Downsides to doing it in gaia distribution
 * Developers wanting to CUSTOMISATION=spark make install-gaia require a connection
 * We may need to add flags to the automation tools that generate the distribution

Upsides to doing it in a gaia distribution 
 * Default build is unchanged
 * We already have all the code setup for customisations, just need to add preload
 * accessible and clear to developers using the /gaia repo
 * Is a smaller change to the workflow so extending to support service workers is more feasible.

Tim mentioned as an engineer regression ranges for the additional apps was his primary concern, I dont think we need to be concerned about that at the moment, we are essentially adding bookmarks (and precaching) some content here, we will not have automated tests failing the build for our preinstalled twitter app.

Comment 13

[Dale Harvey (:daleharvey)]

So Doug mentioned on a call that he was also worried about the ability to bisect similiarly to Tims concern. 

For a certain subset of the applications that may be an issue, in the past we want the ability to find a regression range because gecko changes broke the application since our applications were heavily tied to gecko. I dont expect the new applications to require this level of debugging as they dont heavily use gecko api's as well as a lot being external where we really dont need the ability to do regression ranges at all. In future we will likely need an entirely different way to precache and version pin these applications due to service workers so I would rather not expect a lot of effort into version pinning our bookmarks

Will do this via existing customizations for now and if we do want the ability to pin specific versions it can be an enhancement (likely with service workers in mind)

Comment 14

[Doug Sherk (:drs) (inactive)]

After speaking with Dale, I generally with him, though I'm disappointed that this solution will make bisecting and regression checking much more difficult. We can mitigate this by continuing with our plans to host apps on the gh-pages branches of each app repo, and maintain a history there.

Thanks for taking this, Dale. I'm putting this in Gaia::Build for now, but Gaia might be more appropriate since we're actually working with the customizations framework.

Comment 15

[Doug Sherk (:drs) (inactive)]

I found out that 'preload.py' turns the apps into hosted apps, which is OK except for certified apps, which can't be hosted. Thus we need another solution for our certified apps. From what I see at this point, our options are one of:

1. We check them directly into the Gaia repo, perhaps guarding them behind the 'spark' Customization, similar to the TV apps that are in the repo now.
2. We pull them in using the B2G preloading tool that we discussed in bug 1158367. We could actually combine this with option 1.

Dale, is your work on this going to just re-use the 'preload.py' script? If so, it won't be sufficient for our purposes.

Any suggestions?

Comment 16

[Dale Harvey (:daleharvey)]

Yup I am just using preload.py, I think putting them in the repo and having a custom `list` for spark will work fine here

Comment 17

[GitHub Autolander Bot]

Attachment: [gaia] daleharvey:1159411 > mozilla-b2g:master

Comment 18

[Dale Harvey (:daleharvey)]

Comment on attachment 8604611 [details] [review]
[gaia] daleharvey:1159411 > mozilla-b2g:master

Default build is untouched and we have the ability to specify a list of arbitrary remote web apps to include in a build.

Wasnt certain about the best folder layout, I would prefer to move the current 'customizations' folder so its alongside the `spark` one. 

Also if we changed it to build the remote apps into `outoftree_apps` instead of `distros/spark/outoftree_apps` we wouldnt need to have our own `apps.list`, but I felt like we may not want to interfere with a possibly already existing `outoftree_apps` folder.

Few of the applications wont work out of the box with this (hackerplace etc) at the least they need web components but that will be a seperate bug

Comment 19

[Doug Sherk (:drs) (inactive)]

Comment on attachment 8604611 [details] [review]
[gaia] daleharvey:1159411 > mozilla-b2g:master

(In reply to Dale Harvey (:daleharvey) from comment #16)
> Yup I am just using preload.py, I think putting them in the repo and having
> a custom `list` for spark will work fine here

I'm fine with this too, though it's unfortunate.

(In reply to Dale Harvey (:daleharvey) from comment #18)
> Comment on attachment 8604611 [details] [review]
> [gaia] daleharvey:1159411 > mozilla-b2g:master
> 
> Default build is untouched and we have the ability to specify a list of
> arbitrary remote web apps to include in a build.
> 
> Wasnt certain about the best folder layout, I would prefer to move the
> current 'customizations' folder so its alongside the `spark` one. 

I agree with this. We could call it "example", or something like that.

> Also if we changed it to build the remote apps into `outoftree_apps` instead
> of `distros/spark/outoftree_apps` we wouldnt need to have our own
> `apps.list`, but I felt like we may not want to interfere with a possibly
> already existing `outoftree_apps` folder.

Agreed.

> Few of the applications wont work out of the box with this (hackerplace etc)
> at the least they need web components but that will be a seperate bug

We can use bug 1158367 for this.

Comment 20

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Sorry for the delay. I was trapped in a rather complex bug.

So a Spark build, leveraging the feature here, will be pulled daily when the daily build are generated. It will make regression range hard to pin down if any Gecko changes break them. We would also not be able know that ahead of time before the Nightly users updates their phones.

If people working on Spark are happy with the (lack of) protection this setup implies, I don't have strong opinion about it.

Comment 21

[Ricky Chien [:rickychien] (inactive)]

Comment on attachment 8604611 [details] [review]
[gaia] daleharvey:1159411 > mozilla-b2g:master

Great job! I'm glad to see default build script is untouched. Patch looks good to me and I don't have further opinion on it.

Comment 22

[Dale Harvey (:daleharvey)]

Thanks, addressed feedback from tim and waited on a green run (https://treeherder.mozilla.org/#/jobs?repo=gaia&revision=c2f0e65654cae4ea3b0119296701d2fe9c8a9300)

Landed in https://github.com/mozilla-b2g/gaia/commit/ecc112a7eb5502eae0f822a6cd7e6edeb0dea2fd

Comment 23

[Ryan VanderMeulen [:RyanVM]]

Reverted for Windows B2G Desktop build bustage.
Master: https://github.com/mozilla-b2g/gaia/commit/84d77c1f3cbe06b91d4a5acac02a6c829e9a7488

https://treeherder.mozilla.org/logviewer.html#?job_id=1934807&repo=b2g-inbound

Comment 24

[Dale Harvey (:daleharvey)]

So

  var remoteList;
  try {
    remoteList = utils.getFile(opts.GAIA_DISTRIBUTION_DIR, 'remote.list');
  } catch(e) {}

  if (!remoteList || !remoteList.exists()) {
    return;
  }

Is the only code that should run on a default build or one that doesnt contain remote.list, trying to reproduce but Ricky you have any ideas what could be causing this?

Comment 25

[Dale Harvey (:daleharvey)]

Ryan also is there any chance with you being familiar with the tests what test this is actually running, looks like its doing a mach build of some sorts?

Downloading and installing windows now...

Comment 26

[Ryan VanderMeulen [:RyanVM]]

I know nothing about B2G builds, sorry.

Comment 27

[Ricky Chien [:rickychien] (inactive)]

The problem is clear at https://github.com/mozilla-b2g/gaia/pull/30023/files#diff-0f2db5e0ccabe0666b0c2dd3214bf668R26, windows buildbot doesn't provide sh.exe in their environment. Try to invoke `"python " + preload` directly without using any sh command on windows.

Comment 28

[Ricky Chien [:rickychien] (inactive)]

Remember to fix all suggestions on Github before landing this patch thanks!

Comment 29

[Dale Harvey (:daleharvey)]

Thanks, I was / am confused because at no point should the windows try build ever get to that point, it should early return because there is no test being run with a remote.list defined in GAIA_DISTRIBUTION_DIR.

All the examples of us running a command in the gaia build automatically go through sh.run(['-c' which automatically calls /sh/, how do I run a command without that and in a way that will work on windows? I could also do 

      return sh.run(['-c', '(cd ' + appFolder.path + '; python ' + preload + ')']);


but imagine that will have the same problems

Comment 30

[Ricky Chien [:rickychien] (inactive)]

If it is unnecessary to cd into appFolder.path, why don't we try to use python directly

var python = new utils.Commander('python');
python.initPath(utils.getEnvPath());

...

return sh.run([preload]);

I haven't verify it on my machine, but I hope preload.py can work properly in this case =)

Comment 31

[Dale Harvey (:daleharvey)]

Its necessary to cd, but I can patch that in preload.py and add an argument for the outdir, testing that now cheers

Comment 32

[Dale Harvey (:daleharvey)]

Attachment: https://github.com/mozilla-b2g/preload-app-toolkit/pull/75

This avoids us having to change directory from where the call is invoked. 

I cant find any reviews in bugzilla for this, please forward it on if needed

Comment 33

[Kevin Grandon :kgrandon]

Comment on attachment 8607085 [details] [review]
https://github.com/mozilla-b2g/preload-app-toolkit/pull/75

It looks like Ricky or Fred would be the best reviewer for this.

Comment 34

[Ricky Chien [:rickychien] (inactive)]

Comment on attachment 8607085 [details] [review]
https://github.com/mozilla-b2g/preload-app-toolkit/pull/75

Cancel review because Fred is the best person to review the preload-app-toolkit.

Comment 35

[[e-mail]]

Comment on attachment 8607085 [details] [review]
https://github.com/mozilla-b2g/preload-app-toolkit/pull/75

Thanks for provide the patch to switch root folder. I leave some comments on github.

And please make sure basic tests are passed, or the vendors will cry https://github.com/mozilla-b2g/preload-app-toolkit#setup-unit-test

Comment 36

[Dale Harvey (:daleharvey)]

Comment on attachment 8607085 [details] [review]
https://github.com/mozilla-b2g/preload-app-toolkit/pull/75

Thanks a lot Fred, addressed the comments you had and should be good to review, I ensured all the tests were passing.

Comment 37

[Dale Harvey (:daleharvey)]

Ok, update to preload is waiting for review @ https://github.com/mozilla-b2g/preload-app-toolkit/pull/75

Have updated https://github.com/mozilla-b2g/gaia/pull/30023 and running a try run @ https://treeherder.mozilla.org/#/jobs?repo=try&revision=03997080b66f

Comment 38

[[e-mail]]

Comment on attachment 8607085 [details] [review]
https://github.com/mozilla-b2g/preload-app-toolkit/pull/75

lgtm

Comment 39

[Doug Sherk (:drs) (inactive)]

Comment on attachment 8604611 [details] [review]
[gaia] daleharvey:1159411 > mozilla-b2g:master

Dale, I updated the app list here:
https://github.com/fxos/lightsaber/blob/master/list

Please update the list here before you land it.

Comment 40

[Dale Harvey (:daleharvey)]

Will do, cheers, it looks like there is still some very strange looking windows issues and at a conference, should get it definitely green and landed tomorrow

Comment 41

[Dale Harvey (:daleharvey)]

Landed the patch to preload @ https://github.com/mozilla-b2g/preload-app-toolkit/commit/37248465c8a1cef742b0c191571de26c1a0c3dff

Comment 42

[Dale Harvey (:daleharvey)]

Hrm, new windows b2g desktop build failure

07:40:06     INFO -  [svoperapps.js] PROFILE_DIR, GAIA_DISTRIBUTION_DIR, VARIANT_PATH are all required
07:40:06     INFO -  Exception: Error: LOCALES_FILE doesn't exists: c:\builds\moz2_slave\try-w32_g-00000000000000000000\build\src\gaia\c:\builds\moz2_slave\try-w32_g-00000000000000000000\build\src\gaia\locales\languages_all.json
07:40:06     INFO -  WebappShared.prototype.setOptions@resource://gre/modules/commonjs/toolkit/loader.js -> file:///c://builds//moz2_slave//try-w32_g-00000000000000000000//build//src//gaia/build/webapp-shared.js:24:5
07:40:06     INFO -  WebappShared.prototype.execute@resource://gre/modules/commonjs/toolkit/loader.js -> file:///c://builds//moz2_slave//try-w32_g-00000000000000000000//build//src//gaia/build/webapp-shared.js:368:3
07:40:06     INFO -  execute/<@resource://gre/modules/commonjs/toolkit/loader.js -> file:///c://builds//moz2_slave//try-w32_g-00000000000000000000//build//src//gaia/build/webapp-shared.js:376:1
07:40:06     INFO -  execute@resource://gre/modules/commonjs/toolkit/loader.js -> file:///c://builds//moz2_slave//try-w32_g-00000000000000000000//build//src//gaia/build/webapp-shared.js:374:3
07:40:06     INFO -  execute@resource://gre/modules/commonjs/toolkit/loader.js -> file:///c://builds//moz2_slave//try-w32_g-00000000000000000000//build//src//gaia/build/pre-app.js:28:3
07:40:06     INFO -  CommonjsRunner.prototype.run@c:\builds\moz2_slave\try-w32_g-00000000000000000000\build\src\gaia/build/xpcshell-commonjs.js:73:5

again I am really confused as we should barely be changing any code that is run through a default build but able to debug this now

Comment 43

[Dale Harvey (:daleharvey)]

Ok was based on an unrelated broken windows b2g build, all green now, nits addressed and relanded in https://github.com/mozilla-b2g/gaia/commit/a68746f8fdce1d4c37751f72140dd9f2694debca

Doug I will address the app.list in https://bugzilla.mozilla.org/show_bug.cgi?id=1159457