Issue 2735 - Use ensure_dependencies.py in adblockplusie

I don't have a strong opinion on this, but I have actually removed the buildtools directory from adblockplusie, since we can just use libadblockplus/adblockplus/buildtools.

[sergei, 2015-06-30 09:51:06]

https://codereview.adblockplus.org/29321227/diff/29321235/createsolution.bat
File createsolution.bat (right):

https://codereview.adblockplus.org/29321227/diff/29321235/createsolution.bat#...
createsolution.bat:9: python ensure_dependencies.py
It means that git users won't be able even to generate solution.

[sergei, 2015-06-30 10:49:12]

LGTM however I will need to figure out an issue with the availability of git.

[Eric, 2015-07-01 16:58:27]

We should be running ensure_dependencies at the beginning of build_release.py,
in order to allow weaker checking (or no checking at all) in createsolution.bat.

A new user who tries to compile has to know to run createsolution.bat at the
beginning. It's essentially the same thing to tell them they need to run two
scripts at the outset.

https://codereview.adblockplus.org/29321227/diff/29321235/createsolution.bat
File createsolution.bat (right):

https://codereview.adblockplus.org/29321227/diff/29321235/createsolution.bat#...
createsolution.bat:9: python ensure_dependencies.py
The way this is written, it means mandatory network activity every time you
create solutions. I am very much against that. It's OK as default behavior,
because newcomers to the code won't know to run this script.

But I don't want the delay every time I fiddle with the gyp file. For example,
in the last week I've been switching tool sets back and forth on one project for
testing purposes. I'm running createsolutions all the time. I really don't want
to be constantly checking repositories for things that aren't changing.

So we need an option here to suppress checking dependencies. Alternately, we
could run ensure_dependencies with an option that does only a sanity check, and
prints out an error message if that fails so that a new user can run
ensure_dependencies manually.

[Felix Dahlke, 2015-07-02 10:02:06]

I agree that integrating this call into createsolution.bat is better than to
integrate it into the build, the way I did it with libadblockplus. People have
to execute this step after every pull anyway. We could in theory later consider
calling THAT one from the build, CMake-style, but that's a different topic.

Note however, that we should then do it the same way in libadblockplus IMHO,
because currently the libadblockplus build will still invoke
ensure_dependencies.py.

https://codereview.adblockplus.org/29321227/diff/29321235/createsolution.bat
File createsolution.bat (right):

https://codereview.adblockplus.org/29321227/diff/29321235/createsolution.bat#...
createsolution.bat:9: python ensure_dependencies.py
On 2015/07/01 16:58:27, Eric wrote:
> The way this is written, it means mandatory network activity every time you
> create solutions. I am very much against that. It's OK as default behavior,
> because newcomers to the code won't know to run this script.
> 
> But I don't want the delay every time I fiddle with the gyp file. For example,
> in the last week I've been switching tool sets back and forth on one project
for
> testing purposes. I'm running createsolutions all the time. I really don't
want
> to be constantly checking repositories for things that aren't changing.

While I agree that this is not too straightforward, there wouldn't actually be
mandatory network activity: The script does nothing if you're already on the
desired revision (i.e. the one from the dependencies file). It only pulls if
necessary.

> So we need an option here to suppress checking dependencies. Alternately, we
> could run ensure_dependencies with an option that does only a sanity check,
and
> prints out an error message if that fails so that a new user can run
> ensure_dependencies manually.

That's what the SKIP_ENSURE_DEPENDENCIES environment variable does (in newer
versions of the script). If set, the script will only check if the user is on
the desired revision and print a warning if he isn't, but it won't attempt any
downloads.

This still is problematic for people that don't have hg/git in the path. But I'd
prefer to tackle this issue as a follow-up (we should file a defect for that,
because it's a common situation for MSYS Git users).

Should be OK the way it's integrated here, the script should continue even if
ensure_dependencies.py fails, for whatever reason.

https://codereview.adblockplus.org/29321227/diff/29321235/ensure_dependencies.py
File ensure_dependencies.py (right):

https://codereview.adblockplus.org/29321227/diff/29321235/ensure_dependencies...
ensure_dependencies.py:37: SKIP_DEPENDENCY_UPDATES = os.environ.get(
This is a newer version of the script then what's in buildtools. Since it is
being fetched from libadblockplus/adblockplus/buildtools, we need to commit the
same version that's in there.

[sergei, 2015-07-02 10:51:06]

LGTM.
However, I would add or change the commit message. /buildtools are removed
because we are not using them.

[Felix Dahlke, 2015-07-02 13:17:37]

LGTM, BUT, I think this review/commit should be linked to
https://issues.adblockplus.org/ticket/2732 which is about the dependency update.
Strictly speaking, integrating ensure_dependencies.py is a side effect of the
dependency update. #2735 can be closed then.

[Eric, 2015-07-04 16:46:46]

LGTM

https://codereview.adblockplus.org/29321227/diff/29321235/createsolution.bat
File createsolution.bat (right):

https://codereview.adblockplus.org/29321227/diff/29321235/createsolution.bat#...
createsolution.bat:9: python ensure_dependencies.py
On 2015/07/02 10:02:06, Felix Dahlke wrote:
> The script does nothing if you're already on the
> desired revision (i.e. the one from the dependencies file).

OK. That's not obvious from the code. (I didn't read it long enough to determine
this.) A description of this behavior really ought to be part of the
documentation for the script.