1010088 - STAGE_DIR isn´t used consistently on the build proccess

Status: RESOLVED WONTFIX

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

Description

[Carmen Jimenez Cabezas]

STR:

1.- Being GAIA_DIR=directory where gaia lives, execute 
cd $GAIA_DIR
rm -rf build_stage
PRODUCTION=1 STAGE_DIR=/any/Absolute/Path/buildStage make profile

Expected:
The /any/Absolute/Path/buildStage has been created and contains all the build stage temporary files. The $GAIA_DIR/build_stage directory doesn't exist.

Actual:
The /any/Absolute/Path/buildStage has been created and contains *some* of the build stage temporary files. The $GAIA_DIR/build_stage directory has also been created and contains the rest of the build stage temporary files. The build process fails for some apps because some build_stage temporary files aren't found during the build process of some apps (they're on one of the build stage directory created and they're searched for on the other),


The problem is that some of the build processes don't use the STAGE_DIR variable (which on the other hand is always declared on the global Makefile), but instead have the GAIA_DIR/build_stage hardcoded.

Comment 1

[Carmen Jimenez Cabezas]

Attachment: Proposed patch v1

Comment 2

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

Carmen, I didn't start to review this pull request, but as my experience this pull request definitely should be tested on windows, so I sent this pr to try server first.

https://tbpl.mozilla.org/?tree=Try&rev=4d8db578a30d

Comment 3

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

Comment on attachment 8422264 [details] [review]
Proposed patch v1

Carmen, I didn't imagine this variable can be replaced by environment variable before so that's why you got trouble when you try to replace this variable -- but it is still good to have this feature.

I'm wondering what is your use case? when do we have to use this environment variable to change default build_stage directory? I would love to know the motivation of this issue.

for this pull request, r.js has been patched on this pr which I don't think that's a good idea since it came from external library and we should find another way to get same thing done

and please write a integration test for |STAGE_DIR=<STAGE_DIR> make| in GAIA_DIR/build/test/integration/stage.test.js or build.test.js if it's short, at least we should verify file existing in one app which using r.js for optimization and one app without using r.js.

needinfo me if any questions, and you also can find me on irc #gaia channel.

Comment 4

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

James, could we access environment variable in r.js config file for optimization? if we can access STAGE_DIR environment variable when we can change the default STAGE_DIR to any place.

Comment 5

[James Burke [:jrburke]]

The r.js build config file is considered a static, declarative structure, but almost any build config can be passed to the r.js command line. So in the case of email for example, the Makefile could be changed to this line for the r.js command:

    	$(XULRUNNERSDK) $(XPCSHELLSDK) ../../build/r.js -o build/email.build.js optimize=$(GAIA_EMAIL_MINIFY) dir=$(STAGE_DIR)/email

Then the "dir" line from email/build/email.build.js could be removed. email/build/make_gaia_shared.js would also need to be modified to use STAGE_DIR too, but that is just a plain script run by xpcshell, so should be a way to get that to work too.

Comment 6

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

Thanks James!

so Carmen, let's use the way which is provided on comment 5 to change stage directory!

Comment 7

[Carmen Jimenez Cabezas]

There are two issues with this. One is that the environment variable already existed, but wasn't used consistently in all the build process. So you could define STAGE_DIR expecting all the build stage files to go to that directory, and that wouldn't be the case.

And the second motivation is that  I'm writing a script to make it easier to distribute the personalization files from OBs to the OEMs (from the people that define for example how the homescreen should look to the people that actually have to get that configuration on the actual devices, in other words). In this process we need to distribute the cache for the single variant apps (between other things) because in some cases the machine where the OEMs run the configuration lacks connectivity.

Writing that script I was using STAGE_DIR to keep all the configuration files together, and that when I discovered that some parts of the process didn't honor the variable.

(In reply to Yuren [:yurenju] from comment #3)
> Comment on attachment 8422264 [details] [review]
> Proposed patch v1
> 
> Carmen, I didn't imagine this variable can be replaced by environment
> variable before so that's why you got trouble when you try to replace this
> variable -- but it is still good to have this feature.
> 
> I'm wondering what is your use case? when do we have to use this environment
> variable to change default build_stage directory? I would love to know the
> motivation of this issue.
> 
> for this pull request, r.js has been patched on this pr which I don't think
> that's a good idea since it came from external library and we should find
> another way to get same thing done
> 
> and please write a integration test for |STAGE_DIR=<STAGE_DIR> make| in
> GAIA_DIR/build/test/integration/stage.test.js or build.test.js if it's
> short, at least we should verify file existing in one app which using r.js
> for optimization and one app without using r.js.
> 
> needinfo me if any questions, and you also can find me on irc #gaia channel.

Comment 8

[Carmen Jimenez Cabezas]

Yuren, I've done the changes that you requested, 
please can you take a look?

Comment 9

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

please rebase it.

Comment 10

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

retriggered try server only for Windows.

https://tbpl.mozilla.org/?tree=Try&rev=4d8db578a30d

Comment 11

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

Comment on attachment 8422264 [details] [review]
Proposed patch v1

awesome! thank you Carmen for fixing this issue, that's great!

Comment 12

[Carmen Jimenez Cabezas]

https://github.com/mozilla-b2g/gaia/commit/3c2beda8a8d7958eb5a74c8f157fe93694cbd834
r=yurenju

Comment 13

[Carsten Book [:Tomcat]]

backed this change out to reopen the trees and hopefully fix the build failures from bug 1015658

Comment 15

[BMO Automation]

Firefox OS is not being worked on