Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Issue 29670555: Issue 6264 - Can't prepare libadblockplus dependencies (Closed)

Created:
Jan. 16, 2018, 5:29 a.m. by anton
Modified:
Jan. 19, 2018, 7:05 a.m.
CC:
Felix Dahlke
Visibility:
Public.

Description

Issue 6264 - Can't prepare libadblockplus dependencies Now i just delete "libadblockplus/third_party" to let 'ensure_dependencies' check out the sources from our repo (it's replaced with Chromium's repo while replacing our deps to Chromium deps - see issue ticket).

Patch Set 1 #

Total comments: 4

Patch Set 2 : Formatted with respect to eyeo python code style and tested with flake8 #

Total comments: 5

Patch Set 3 : Renamed "clear_cache.py" #

Total comments: 2

Patch Set 4 : Replaced 'clear_dependencies.py' with 'delete_dir.py' #

Patch Set 5 : Deleting NDK directory before extracting. Formatted with respect to eyeo python code style #

Total comments: 7

Patch Set 6 : Renamed variable #

Unified diffs Side-by-side diffs Delta from patch set Stats (+93 lines, -49 lines) Patch
M DEPS View 1 2 3 1 chunk +12 lines, -0 lines 0 comments Download
A third_party/libadblockplus/delete_dir.py View 1 2 3 1 chunk +19 lines, -0 lines 0 comments Download
M third_party/libadblockplus/download_ndk.py View 1 2 3 4 5 1 chunk +41 lines, -29 lines 0 comments Download
M third_party/libadblockplus_android/BUILD.gn View 1 2 2 chunks +2 lines, -2 lines 0 comments Download
D third_party/libadblockplus_android/clear.py View 1 chunk +0 lines, -18 lines 0 comments Download
A third_party/libadblockplus_android/delete_dir.py View 1 2 1 chunk +19 lines, -0 lines 0 comments Download

Messages

Total messages: 32
anton
https://codereview.adblockplus.org/29670555/diff/29670556/third_party/libadblockplus_android/BUILD.gn File third_party/libadblockplus_android/BUILD.gn (right): https://codereview.adblockplus.org/29670555/diff/29670556/third_party/libadblockplus_android/BUILD.gn#newcode68 third_party/libadblockplus_android/BUILD.gn:68: script = "clear_cache.py" "clear.py" is renamed to "clear_cache" just ...
Jan. 16, 2018, 5:35 a.m. (2018-01-16 05:35:38 UTC) #1
anton
Jan. 16, 2018, 5:35 a.m. (2018-01-16 05:35:44 UTC) #2
shoniko
https://codereview.adblockplus.org/29670555/diff/29670556/third_party/libadblockplus/clear_dependencies.py File third_party/libadblockplus/clear_dependencies.py (right): https://codereview.adblockplus.org/29670555/diff/29670556/third_party/libadblockplus/clear_dependencies.py#newcode14 third_party/libadblockplus/clear_dependencies.py:14: v8_gyp_launcher_src = os.path.join(libadblockplus_third_party, 'v8_gyp_launcher') Nit: We usually rely on ...
Jan. 16, 2018, 7:14 a.m. (2018-01-16 07:14:14 UTC) #3
anton
https://codereview.adblockplus.org/29670555/diff/29670556/third_party/libadblockplus/clear_dependencies.py File third_party/libadblockplus/clear_dependencies.py (right): https://codereview.adblockplus.org/29670555/diff/29670556/third_party/libadblockplus/clear_dependencies.py#newcode14 third_party/libadblockplus/clear_dependencies.py:14: v8_gyp_launcher_src = os.path.join(libadblockplus_third_party, 'v8_gyp_launcher') On 2018/01/16 07:14:14, shoniko wrote: ...
Jan. 16, 2018, 7:18 a.m. (2018-01-16 07:18:36 UTC) #4
Oleksandr
On 2018/01/16 07:18:36, anton wrote: > https://codereview.adblockplus.org/29670555/diff/29670556/third_party/libadblockplus/clear_dependencies.py > File third_party/libadblockplus/clear_dependencies.py (right): > > https://codereview.adblockplus.org/29670555/diff/29670556/third_party/libadblockplus/clear_dependencies.py#newcode14 > ...
Jan. 16, 2018, 7:22 a.m. (2018-01-16 07:22:17 UTC) #5
anton
On 2018/01/16 07:22:17, Oleksandr wrote: > On 2018/01/16 07:18:36, anton wrote: > > > https://codereview.adblockplus.org/29670555/diff/29670556/third_party/libadblockplus/clear_dependencies.py ...
Jan. 16, 2018, 8:42 a.m. (2018-01-16 08:42:55 UTC) #6
Oleksandr
LGTM
Jan. 16, 2018, 11:23 a.m. (2018-01-16 11:23:48 UTC) #7
sergei
It can be a solution but I think there is a better way, in particular ...
Jan. 16, 2018, 11:51 a.m. (2018-01-16 11:51:00 UTC) #8
anton
On 2018/01/16 11:51:00, sergei wrote: > It can be a solution but I think there ...
Jan. 16, 2018, 12:10 p.m. (2018-01-16 12:10:27 UTC) #9
anton
On 2018/01/16 11:51:00, sergei wrote: > If we removed only libadblockplus/third_party/v8 then we would not ...
Jan. 16, 2018, 12:13 p.m. (2018-01-16 12:13:49 UTC) #10
sergei
On 2018/01/16 12:10:27, anton wrote: > On 2018/01/16 11:51:00, sergei wrote: > > It can ...
Jan. 16, 2018, 12:30 p.m. (2018-01-16 12:30:40 UTC) #11
anton
On 2018/01/16 12:30:40, sergei wrote: > On 2018/01/16 12:10:27, anton wrote: > > On 2018/01/16 ...
Jan. 16, 2018, 1 p.m. (2018-01-16 13:00:44 UTC) #12
sergei
On 2018/01/16 13:00:44, anton wrote: > I got your point - googletest and gyp are ...
Jan. 16, 2018, 2:53 p.m. (2018-01-16 14:53:45 UTC) #13
sergei
> JIC, what about just a dependencies file instead of copying/linking many V8 > files? ...
Jan. 16, 2018, 5:28 p.m. (2018-01-16 17:28:54 UTC) #14
sergei
https://codereview.adblockplus.org/29670555/diff/29670566/third_party/libadblockplus_android/clear_cache.py File third_party/libadblockplus_android/clear_cache.py (right): https://codereview.adblockplus.org/29670555/diff/29670566/third_party/libadblockplus_android/clear_cache.py#newcode1 third_party/libadblockplus_android/clear_cache.py:1: import os Currently it looks like platform independent `rm ...
Jan. 16, 2018, 5:29 p.m. (2018-01-16 17:29:08 UTC) #15
anton
https://codereview.adblockplus.org/29670555/diff/29670566/third_party/libadblockplus_android/clear_cache.py File third_party/libadblockplus_android/clear_cache.py (right): https://codereview.adblockplus.org/29670555/diff/29670566/third_party/libadblockplus_android/clear_cache.py#newcode1 third_party/libadblockplus_android/clear_cache.py:1: import os On 2018/01/16 17:29:08, sergei wrote: > Currently ...
Jan. 17, 2018, 6 a.m. (2018-01-17 06:00:08 UTC) #16
anton
On 2018/01/16 17:29:08, sergei wrote: > Currently it looks like platform independent `rm -rf {}`, ...
Jan. 17, 2018, 6:11 a.m. (2018-01-17 06:11:23 UTC) #17
sergei
https://codereview.adblockplus.org/29670555/diff/29671555/DEPS File DEPS (right): https://codereview.adblockplus.org/29670555/diff/29671555/DEPS#newcode1240 DEPS:1240: 'src/third_party/libadblockplus/clear_dependencies.py' Why not to call here python delete_dir.py src/third_party/libadblockplus/third_party/v8?
Jan. 17, 2018, 9:49 a.m. (2018-01-17 09:49:03 UTC) #18
anton
https://codereview.adblockplus.org/29670555/diff/29671555/DEPS File DEPS (right): https://codereview.adblockplus.org/29670555/diff/29671555/DEPS#newcode1240 DEPS:1240: 'src/third_party/libadblockplus/clear_dependencies.py' On 2018/01/17 09:49:03, sergei wrote: > Why not ...
Jan. 17, 2018, 11:05 a.m. (2018-01-17 11:05:49 UTC) #19
anton
On 2018/01/17 09:49:03, sergei wrote: > https://codereview.adblockplus.org/29670555/diff/29671555/DEPS > File DEPS (right): > > https://codereview.adblockplus.org/29670555/diff/29671555/DEPS#newcode1240 > ...
Jan. 17, 2018, 11:15 a.m. (2018-01-17 11:15:11 UTC) #20
anton
On 2018/01/17 11:15:11, anton wrote: > On 2018/01/17 09:49:03, sergei wrote: > > https://codereview.adblockplus.org/29670555/diff/29671555/DEPS > ...
Jan. 17, 2018, 1:04 p.m. (2018-01-17 13:04:35 UTC) #21
sergei
LGTM, though it seems that we could avoid copying of delete_dir.py if put it one ...
Jan. 17, 2018, 2:39 p.m. (2018-01-17 14:39:10 UTC) #22
anton
https://codereview.adblockplus.org/29670555/diff/29673555/third_party/libadblockplus/download_ndk.py File third_party/libadblockplus/download_ndk.py (right): https://codereview.adblockplus.org/29670555/diff/29673555/third_party/libadblockplus/download_ndk.py#newcode26 third_party/libadblockplus/download_ndk.py:26: ndk_dir = os.path.join(libadblockplus_root, 'android-ndk-r12b') Previously we deleted whole 'third_party' ...
Jan. 18, 2018, 8:25 a.m. (2018-01-18 08:25:46 UTC) #23
anton
On 2018/01/17 14:39:10, sergei wrote: > LGTM, though it seems that we could avoid copying ...
Jan. 18, 2018, 8:44 a.m. (2018-01-18 08:44:02 UTC) #24
anton
On 2018/01/18 08:44:02, anton wrote: Since i've uploaded patch set #5 (because of found problem ...
Jan. 18, 2018, 8:46 a.m. (2018-01-18 08:46:33 UTC) #25
sergei
If it takes too long to test the changes then I think it can be ...
Jan. 18, 2018, 9:16 a.m. (2018-01-18 09:16:51 UTC) #26
anton
https://codereview.adblockplus.org/29670555/diff/29673555/third_party/libadblockplus/download_ndk.py File third_party/libadblockplus/download_ndk.py (right): https://codereview.adblockplus.org/29670555/diff/29673555/third_party/libadblockplus/download_ndk.py#newcode15 third_party/libadblockplus/download_ndk.py:15: 'libadblockplus', 'third_party') On 2018/01/18 09:16:50, sergei wrote: > Why ...
Jan. 18, 2018, 9:49 a.m. (2018-01-18 09:49:12 UTC) #27
anton
https://codereview.adblockplus.org/29670555/diff/29673555/third_party/libadblockplus/download_ndk.py File third_party/libadblockplus/download_ndk.py (right): https://codereview.adblockplus.org/29670555/diff/29673555/third_party/libadblockplus/download_ndk.py#newcode13 third_party/libadblockplus/download_ndk.py:13: libadblockplus_root = os.path.join(cwd, On 2018/01/18 09:16:51, sergei wrote: > ...
Jan. 18, 2018, 12:20 p.m. (2018-01-18 12:20:27 UTC) #28
diegocarloslima
On 2018/01/18 09:49:12, anton wrote: > https://codereview.adblockplus.org/29670555/diff/29673555/third_party/libadblockplus/download_ndk.py > File third_party/libadblockplus/download_ndk.py (right): > > https://codereview.adblockplus.org/29670555/diff/29673555/third_party/libadblockplus/download_ndk.py#newcode15 > ...
Jan. 18, 2018, 12:23 p.m. (2018-01-18 12:23:12 UTC) #29
anton
On 2018/01/18 12:23:12, diegocarloslima wrote: > On 2018/01/18 09:49:12, anton wrote: > > > https://codereview.adblockplus.org/29670555/diff/29673555/third_party/libadblockplus/download_ndk.py ...
Jan. 18, 2018, 12:25 p.m. (2018-01-18 12:25:34 UTC) #30
diegocarloslima
On 2018/01/18 12:25:34, anton wrote: > On 2018/01/18 12:23:12, diegocarloslima wrote: > > On 2018/01/18 ...
Jan. 18, 2018, 12:27 p.m. (2018-01-18 12:27:15 UTC) #31
sergei
Jan. 18, 2018, 12:38 p.m. (2018-01-18 12:38:15 UTC) #32
LGTM then

Powered by Google App Engine
This is Rietveld