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

Issue 10945039: Switch to gyp (Closed)

Created:
June 20, 2013, 3:14 p.m. by Wladimir Palant
Modified:
June 25, 2013, 11:06 a.m.
Reviewers:
Felix Dahlke, Oleksandr
Visibility:
Public.

Description

This is work in progress, almost working. There is still an issue with linking to the tlb file from the rc file. Also, I didn`t check yet whether any of the plugin compile options need to be taken over. And obviously, the end result hasn`t been tested yet.

Patch Set 1 #

Patch Set 2 : This version actually compiles, needs some testing however. build_release.py and installer still ne… #

Total comments: 3

Patch Set 3 : Final version #

Unified diffs Side-by-side diffs Delta from patch set Stats (+269 lines, -1282 lines) Patch
R AdblockPlus.sln View 1 2 1 chunk +0 lines, -58 lines 0 comments Download
R AdblockPlusEngine.vcxproj View 1 2 1 chunk +0 lines, -372 lines 0 comments Download
R AdblockPlusPlugin.vcxproj View 1 2 1 chunk +0 lines, -804 lines 0 comments Download
M adblockplus.gyp View 1 2 1 chunk +154 lines, -8 lines 0 comments Download
M build_release.py View 1 2 1 chunk +6 lines, -27 lines 0 comments Download
M createsolution.bat View 1 2 1 chunk +9 lines, -2 lines 0 comments Download
A defaults.gypi View 1 2 1 chunk +78 lines, -0 lines 0 comments Download
M installer/Makefile View 1 2 2 chunks +2 lines, -2 lines 0 comments Download
M installer/adblockplusie.wxs View 1 2 5 chunks +5 lines, -5 lines 0 comments Download
M src/plugin/AdblockPlus.rc View 1 1 chunk +5 lines, -2 lines 0 comments Download
M src/plugin/Plugin.h View 1 chunk +5 lines, -1 line 0 comments Download
M src/plugin/Plugin.cpp View 1 chunk +5 lines, -1 line 0 comments Download

Messages

Total messages: 7
Wladimir Palant
June 20, 2013, 3:14 p.m. (2013-06-20 15:14:27 UTC) #1
Wladimir Palant
This version actually compiles, needs some testing however. build_release.py and installer still need adjustments.
June 21, 2013, 7:30 a.m. (2013-06-21 07:30:19 UTC) #2
Felix Dahlke
http://codereview.adblockplus.org/10945039/diff/3001/src/plugin/Plugin.cpp File src/plugin/Plugin.cpp (right): http://codereview.adblockplus.org/10945039/diff/3001/src/plugin/Plugin.cpp#newcode5 src/plugin/Plugin.cpp:5: #include "../../build/x64/AdblockPlus_i.c" What exactly is this? Worth a comment ...
June 21, 2013, 2:01 p.m. (2013-06-21 14:01:00 UTC) #3
Wladimir Palant
Final version uploaded, installer packages are created correctly with this one and work. Only missing ...
June 21, 2013, 2:23 p.m. (2013-06-21 14:23:53 UTC) #4
Felix Dahlke
LGTM, as far as I'm concerned.
June 21, 2013, 9:12 p.m. (2013-06-21 21:12:19 UTC) #5
Wladimir Palant
I added a minimal README file and pushed: https://hg.adblockplus.org/adblockplusie/rev/043002ac2a8c
June 22, 2013, 8:42 a.m. (2013-06-22 08:42:48 UTC) #6
Oleksandr
June 25, 2013, 11:06 a.m. (2013-06-25 11:06:43 UTC) #7
http://codereview.adblockplus.org/10945039/diff/3001/src/plugin/Plugin.cpp
File src/plugin/Plugin.cpp (right):

http://codereview.adblockplus.org/10945039/diff/3001/src/plugin/Plugin.cpp#ne...
src/plugin/Plugin.cpp:5: #include "../../build/x64/AdblockPlus_i.c"
It is the output of MIDL compiler.
http://msdn.microsoft.com/en-us/library/windows/desktop/aa367091(v=vs.85).aspx 
In short, it is a way to define marshaling in the RPC calls without going and
into much specifics. It is a standard of defining COM components.
  
On 2013/06/21 14:23:54, Wladimir Palant wrote:
> On 2013/06/21 14:01:01, Felix H. Dahlke wrote:
> > What exactly is this? Worth a comment I think.
> 
> It's some code generated automatically from AdblockPlus.idl (our interface
> definition). As to what's exactly in it and why we need to include it in such
an
> adventurous way - Oleksandr should be the right person to answer, I have no
> idea.
> 
> > Also, can't we move that logic into gyp?
> 
> I wouldn't want to do it without really understanding what this is about.

Powered by Google App Engine
This is Rietveld