mobile/android/build.mk - Issue 4540363985387520: Embed Adblock Plus

Delta Between Two Patch Sets: mobile/android/build.mk

Issue 4540363985387520: Embed Adblock Plus (Closed)

Left Patch Set: Created Nov. 19, 2014, 3:54 p.m.

Right Patch Set: Install as application scope extension Created Jan. 23, 2015, 11:20 p.m.

Left:
Right:

Use n/p to move between diff chunks; N/P to move between comments.

Jump to:

Right: Side by side diff | Download

LEFT	RIGHT
(no file at all)
1 # This Source Code Form is subject to the terms of the Mozilla Public	1 # This Source Code Form is subject to the terms of the Mozilla Public

2 # License, v. 2.0. If a copy of the MPL was not distributed with this	2 # License, v. 2.0. If a copy of the MPL was not distributed with this

3 # file, You can obtain one at http://mozilla.org/MPL/2.0/.	3 # file, You can obtain one at http://mozilla.org/MPL/2.0/.

4	4

5 include $(topsrcdir)/toolkit/mozapps/installer/package-name.mk	5 include $(topsrcdir)/toolkit/mozapps/installer/package-name.mk

6	6

7 installer:	7 installer:

8 @$(MAKE) -C mobile/android/installer installer	8 @$(MAKE) -C mobile/android/installer installer

9	9

10 package:	10 abp_source_dir=$(topsrcdir)/adblockplus

	11 abp_xpi_file=\{d10d0bf8-f5b5-c8b4-a8b2-2b9879e08c5d\}.xpi

	12 package_abp: $(abp_source_dir)

	13 » cd $(abp_source_dir); ./build.py build $(abp_xpi_file)
	Wladimir Palant 2015/02/18 17:54:01 No need to change directory, build.py can be calle No need to change directory, build.py can be called from anywhere: $(abp_source_dir)/build.py build $(DIST)/bin/extensions/$(abp_xpi_file) Felix Dahlke 2015/02/19 10:54:14 Unfortunately not, the build process expects some Show quoted text On 2015/02/18 17:54:01, Wladimir Palant wrote: > No need to change directory, build.py can be called from anywhere: > > $(abp_source_dir)/build.py build $(DIST)/bin/extensions/$(abp_xpi_file) Unfortunately not, the build process expects some stuff in the cwd (e.g. the dependencies file and metadata.gecko). Seems a bit like a bug to me actually, but I wasn't sure if it's supposed to work. That'd allow us to simplify the code here quite a bit, but I'd rather not have this blocked. Wladimir Palant 2015/02/19 18:03:32 Yes, it is indeed a bug - while all the code works Show quoted text On 2015/02/19 10:54:14, Felix H. Dahlke wrote: > Unfortunately not, the build process expects some stuff in the cwd (e.g. the > dependencies file and metadata.gecko). Seems a bit like a bug to me actually, Yes, it is indeed a bug - while all the code works with any directory, our build.py boilerplate passes in '.' as basedir :-( Felix Dahlke 2015/02/19 20:32:09 Here we go :) https://issues.adblockplus.org/tick Show quoted text On 2015/02/19 18:03:32, Wladimir Palant wrote: > On 2015/02/19 10:54:14, Felix H. Dahlke wrote: > > Unfortunately not, the build process expects some stuff in the cwd (e.g. the > > dependencies file and metadata.gecko). Seems a bit like a bug to me actually, > > Yes, it is indeed a bug - while all the code works with any directory, our > build.py boilerplate passes in '.' as basedir :-( Here we go :) https://issues.adblockplus.org/ticket/2020
	14 » $(INSTALL_DATA) $(abp_source_dir)/$(abp_xpi_file) $(DIST)/bin/extensions

	15

	16 package: package_abp
	Wladimir Palant 2015/02/18 17:54:01 Do we have to mess up this makefile? Won't creatin Do we have to mess up this makefile? Won't creating a directory under /extensions with our own build instructions work as well? --enable-extensions=adblockplus needs to be specified but that should be fine. Felix Dahlke 2015/02/19 10:54:14 /extensions doesn't have anything to do with the e Show quoted text On 2015/02/18 17:54:01, Wladimir Palant wrote: > Do we have to mess up this makefile? Won't creating a directory under > /extensions with our own build instructions work as well? > --enable-extensions=adblockplus needs to be specified but that should be fine. /extensions doesn't have anything to do with the extensions directory that ends up in the APK - the former has native extensions (not add ons), definitely the wrong place for us IMO. Secondly, modifying that makefile is the only way I've found to add this to the package phase, rather than the build phase. I think it belongs there, and it gives us a faster turnaround. Wladimir Palant 2015/02/19 18:03:32 I know - it's build system extensions, meant for c Show quoted text On 2015/02/19 10:54:14, Felix H. Dahlke wrote: > /extensions doesn't have anything to do with the extensions directory that ends > up in the APK - the former has native extensions (not add ons), definitely the > wrong place for us IMO. I know - it's build system extensions, meant for custom build steps. And here we have one. Show quoted text > Secondly, modifying that makefile is the only way I've found to add this to the > package phase, rather than the build phase. I think it belongs there, and it > gives us a faster turnaround. While technically this might be true, it will also make updates more complicated. But that's up to you.
11 @$(MAKE) -C mobile/android/installer	17 @$(MAKE) -C mobile/android/installer

12	18

13 ifeq ($(OS_TARGET),Android)	19 ifeq ($(OS_TARGET),Android)

14 ifneq ($(MOZ_ANDROID_INSTALL_TARGET),)	20 ifneq ($(MOZ_ANDROID_INSTALL_TARGET),)

15 ANDROID_SERIAL = $(MOZ_ANDROID_INSTALL_TARGET)	21 ANDROID_SERIAL = $(MOZ_ANDROID_INSTALL_TARGET)

16 endif	22 endif

17 ifneq ($(ANDROID_SERIAL),)	23 ifneq ($(ANDROID_SERIAL),)

18 export ANDROID_SERIAL	24 export ANDROID_SERIAL

19 else	25 else

20 # Determine if there's more than one device connected	26 # Determine if there's more than one device connected

(...skipping 33 matching lines...) Expand 10 before \| Expand all \| Expand 10 after Loading...
54 $(CHECK_TEST_ERROR)	60 $(CHECK_TEST_ERROR)

55	61

56 mochitest:: mochitest-browser-chrome	62 mochitest:: mochitest-browser-chrome

57	63

58 .PHONY: mochitest-browser-chrome	64 .PHONY: mochitest-browser-chrome

59 endif	65 endif

60	66

61 ifeq ($(OS_TARGET),Linux)	67 ifeq ($(OS_TARGET),Linux)

62 deb: installer	68 deb: installer

63 endif	69 endif

LEFT	RIGHT