1757 - Integrate ensure_dependencies.py with the adblockpluschrome repository

In #170 we introduced our custom dependency mechanism, this is the patch to use that mechanism in this repo

[Sebastian Noack, 2015-01-13 17:21:52]

http://codereview.adblockplus.org/5710085762318336/diff/5629499534213120/buil...
File build.py (right):

http://codereview.adblockplus.org/5710085762318336/diff/5629499534213120/buil...
build.py:8: if os.path.isfile(DEPENDENCY_SCRIPT):
Why do you check whether the script exists? It's there. You are adding it with
the same patch.

http://codereview.adblockplus.org/5710085762318336/diff/5629499534213120/depe...
File dependencies (right):

http://codereview.adblockplus.org/5710085762318336/diff/5629499534213120/depe...
dependencies:5: adblockplustests = adblockplustests hg:30c3c8f9f80c git:282d46e
Apparently the patch is outdated and doesn't consider the adblockplusui
dependency, which were recently added.

[paco, 2015-01-14 11:40:49]

http://codereview.adblockplus.org/5710085762318336/diff/5629499534213120/buil...
File build.py (right):

http://codereview.adblockplus.org/5710085762318336/diff/5629499534213120/buil...
build.py:8: if os.path.isfile(DEPENDENCY_SCRIPT):
On 2015/01/13 17:21:53, Sebastian Noack wrote:
> Why do you check whether the script exists? It's there. You are adding it with
> the same patch.
As requested by Wladimir, we should call the script only if the file actually
exists.

[Sebastian Noack, 2015-01-14 11:58:45]

http://codereview.adblockplus.org/5710085762318336/diff/5629499534213120/buil...
File build.py (right):

http://codereview.adblockplus.org/5710085762318336/diff/5629499534213120/buil...
build.py:8: if os.path.isfile(DEPENDENCY_SCRIPT):
On 2015/01/14 11:40:49, paco wrote:
> On 2015/01/13 17:21:53, Sebastian Noack wrote:
> > Why do you check whether the script exists? It's there. You are adding it
with
> > the same patch.
> As requested by Wladimir, we should call the script only if the file actually
> exists.

I am not sure whether I agree. The only scenario where the script won't exist,
is if somebody (accidentally) deleted it. In that case I'd certainly prefer to
see an error, rather than silently ignoring it. I put Wladimir into CC. So maybe
he can give some background, why he thinks this check makes sense?

[Wladimir Palant, 2015-01-14 13:11:16]

http://codereview.adblockplus.org/5710085762318336/diff/5629499534213120/buil...
File build.py (right):

http://codereview.adblockplus.org/5710085762318336/diff/5629499534213120/buil...
build.py:8: if os.path.isfile(DEPENDENCY_SCRIPT):
On 2015/01/14 11:58:46, Sebastian Noack wrote:
> I am not sure whether I agree. The only scenario where the script won't exist,
> is if somebody (accidentally) deleted it.

The idea was actually that this build script is to be used everywhere unchanged
- yet some repositories might now have dependencies, so ensure_dependencies.py
won't exist. However, given that build.py itself depends on buildtools we can
probably ignore that scenario and just run ensure_dependencies.py
unconditionally.

http://codereview.adblockplus.org/5710085762318336/diff/5685265389584384/.hgsub
File .hgsub (left):

http://codereview.adblockplus.org/5710085762318336/diff/5685265389584384/.hgs...
.hgsub:3: adblockplustests = ../adblockplustests
Why is adblockplusui missing here? That patch won't apply to the latest
repository state.

http://codereview.adblockplus.org/5710085762318336/diff/5685265389584384/buil...
File build.py (right):

http://codereview.adblockplus.org/5710085762318336/diff/5685265389584384/buil...
build.py:10: subprocess.check_call(["python", DEPENDENCY_SCRIPT])
Only realized now - this shouldn't be "python" but rather sys.interpreter.

http://codereview.adblockplus.org/5710085762318336/diff/5685265389584384/depe...
File dependencies (right):

http://codereview.adblockplus.org/5710085762318336/diff/5685265389584384/depe...
dependencies:4: adblockplus = adblockplus hg:816bc28239cd git:b6b012f
The git revision doesn't match the hg revision. Should be:
db38cd181823db47931231f39f6026ddb6dc1c42

[paco, 2015-01-14 16:00:36]

http://codereview.adblockplus.org/5710085762318336/diff/5629499534213120/buil...
File build.py (right):

http://codereview.adblockplus.org/5710085762318336/diff/5629499534213120/buil...
build.py:8: if os.path.isfile(DEPENDENCY_SCRIPT):
On 2015/01/14 13:11:17, Wladimir Palant wrote:
> On 2015/01/14 11:58:46, Sebastian Noack wrote:
> > I am not sure whether I agree. The only scenario where the script won't
exist,
> > is if somebody (accidentally) deleted it.
> 
> The idea was actually that this build script is to be used everywhere
unchanged
> - yet some repositories might now have dependencies, so ensure_dependencies.py
> won't exist. However, given that build.py itself depends on buildtools we can
> probably ignore that scenario and just run ensure_dependencies.py
> unconditionally.

Will remove `if` in next patch

http://codereview.adblockplus.org/5710085762318336/diff/5685265389584384/.hgsub
File .hgsub (left):

http://codereview.adblockplus.org/5710085762318336/diff/5685265389584384/.hgs...
.hgsub:3: adblockplustests = ../adblockplustests
On 2015/01/14 13:11:17, Wladimir Palant wrote:
> Why is adblockplusui missing here? That patch won't apply to the latest
> repository state.

Done

http://codereview.adblockplus.org/5710085762318336/diff/5685265389584384/buil...
File build.py (right):

http://codereview.adblockplus.org/5710085762318336/diff/5685265389584384/buil...
build.py:10: subprocess.check_call(["python", DEPENDENCY_SCRIPT])
On 2015/01/14 13:11:17, Wladimir Palant wrote:
> Only realized now - this shouldn't be "python" but rather sys.interpreter.

As discussed on IRC, this should be `sys.executable`, will appear on next patch

http://codereview.adblockplus.org/5710085762318336/diff/5685265389584384/depe...
File dependencies (right):

http://codereview.adblockplus.org/5710085762318336/diff/5685265389584384/depe...
dependencies:4: adblockplus = adblockplus hg:816bc28239cd git:b6b012f
On 2015/01/14 13:11:17, Wladimir Palant wrote:
> The git revision doesn't match the hg revision. Should be:
> db38cd181823db47931231f39f6026ddb6dc1c42

Right.

[Sebastian Noack, 2015-01-14 16:34:13]

http://codereview.adblockplus.org/5710085762318336/diff/5634387206995968/buil...
File build.py (right):

http://codereview.adblockplus.org/5710085762318336/diff/5634387206995968/buil...
build.py:13: 
Maybe exit with a non-0 status code?

[Sebastian Noack, 2015-01-14 17:28:36]

LGTM

http://codereview.adblockplus.org/5710085762318336/diff/5634387206995968/buil...
File build.py (right):

http://codereview.adblockplus.org/5710085762318336/diff/5634387206995968/buil...
build.py:13: 
On 2015/01/14 16:34:13, Sebastian Noack wrote:
> Maybe exit with a non-0 status code?

Thinking again about it, I think it's OK to just show a warning continuing with
the build when dependencies failed to update. Otherwise you also won't be able
to work offline.

[Wladimir Palant, 2015-01-14 17:42:45]

LGTM

http://codereview.adblockplus.org/5710085762318336/diff/5634387206995968/buil...
File build.py (right):

http://codereview.adblockplus.org/5710085762318336/diff/5634387206995968/buil...
build.py:13: 
On 2015/01/14 16:34:13, Sebastian Noack wrote:
> Maybe exit with a non-0 status code?

Making dependency issues non-fatal was one of the reasons we wanted to get rid
of Mercurial dependencies...

[Sebastian Noack, 2015-01-15 11:28:57]

Two more things:

1. Please make ensure_dependencies.py executable (according to Wladimir it is
executable in all the other repos).
2. Update the buildtools dependency and contents of ensure_dependency.py
(Wladimir just pushed a bunch of new changes there)

[Sebastian Noack, 2015-01-15 13:59:13]

LGTM