Opened on 05/27/2015 at 03:35:49 PM
Closed on 12/03/2015 at 01:16:59 PM
Last modified on 11/28/2016 at 01:51:11 AM
#2599 closed defect (fixed)
Adblock Plus for IE build is broken for x64
Reported by: | eric@adblockplus.org | Assignee: | |
---|---|---|---|
Priority: | P1 | Milestone: | Adblock-Plus-for-Internet-Explorer-1.6 |
Module: | Adblock-Plus-for-Internet-Explorer | Keywords: | build |
Cc: | oleksandr | Blocked By: | |
Blocking: | Platform: | Internet Explorer | |
Ready: | no | Confidential: | no |
Tester: | Rraceanu | Verified working: | no |
Review URL(s): |
Description
Environment
This is a build error. Environment is one with Visual Studio Express 2012 installed but _without_ Visual Studio Professional or higher also installed. The error goes away if Pro is installed, but Pro is not our designated, completely-no-cost build environment.
How to reproduce
Build the x64 solution in the above environment. The following link error appears:
error LNK2001: unresolved external symbol CComStdCallThunkHelper
First version to exhibit the defect
66c71b83b1057808261031ad628557e92846b9bb
Issue 1672 - Subscribe and listen COM events using ATL::IDispEventImpl and BEGIN_SINK_MAP
Commentary
The reason for the error is an ATL version mismatch. Our designated ATL for building is one from an older version of the Windows Driver Development Kit (WinDDK). This kit does not seem to be available as such any more; the replacement is Windows Driver Kit (WDK). Visual Studio Pro (and above) 2012 come with a _different_ version installed, apparently a slightly later one. Code introduced in the change set referenced above added code that works with the Pro version but not with the WinDDK version.
The problem had been masked because our build system had the Pro version directories ahead of the WinDDK ones, so they took priority. This review changes the priority order, avoiding future masking problems.
The offending directory is "$(VCInstallDir)atlmfc". If that directory is (temporarily) renamed, it acts as if Pro isn't installed (insofar as this defect is concerned); it does this without changing the build.
Attachments (0)
Change History (6)
comment:1 Changed on 05/27/2015 at 03:42:45 PM by eric@adblockplus.org
comment:2 Changed on 06/09/2015 at 02:06:52 PM by oleksandr
- Blocked By 1197 added
We have decided that it would be best just to upgrade to Visual Studio 2013, where we can use the Community Edition, which has ATL. Since the only problem that is blocking the move is libadblockplus incopatibility - I am adding #1197 as blocker.
comment:3 Changed on 06/19/2015 at 12:52:21 PM by oleksandr
- Blocked By 1087 added
comment:4 Changed on 12/03/2015 at 01:16:59 PM by sergz
- Blocked By 1087, 1197 removed
- Resolution set to fixed
- Status changed from new to closed
- Tester set to Unknown
comment:5 Changed on 11/21/2016 at 10:48:01 AM by oleksandr
- Milestone set to Adblock-Plus-for-Internet-Explorer-Next
comment:6 Changed on 11/28/2016 at 01:51:11 AM by rraceanu
- Tester changed from Unknown to Rraceanu
The question remains about how to fix the problem with the build. There are a few options.
1) If we can find a distribution of ATL that matches the one in Visual Studio 2012 Pro, that should work. It's not known whether there was ever such a distribution.
2) We can use the ATL distribution in Visual Studio Community 2013. That, however, requires us to use the 2013 tool set, which is not at present compatible with the version of v8 in libadblockplus.
There was an engine upgrade, but to a version required by Maxthon, not one that's adequate for VS 2013. Does Maxthon still require that specific version?
This option requires upgrading v8 for all platforms, forking our v8 repository to fix the compile problem (it's simple; just some duplicate definitions for C99 math functions), or forking libadblockplus for products that require a particular, older version.
3) We can revert the change that caused the problem entirely. It was made to simplify code, not add a feature or correct a defect. This is difficult, in part because the offending change is pretty far back in the version history at this point.
4) We can rewrite the change that caused the problem to avoid use of the ATL libraries. This seems feasible, since the main change in the offending change set was to use an ATL implementation class for the interface "IDispatch", which pulls in a bunch of ATL infrastructure. We could go back to using a manually written dispatch function, calling out the separate implementation functions. Splitting out these functions from the old dispatch routine was also part of the change set, but there's no inherent problem with those changes.