New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 639853 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 3
Type: Bug-Regression

Blocking:
issue 636399
issue 82385



Sign in to add a comment

Several win/clang bots are failing compile due to disable_outdated_build_detector having build warnings

Project Member Reported by thakis@chromium.org, Aug 22 2016

Issue description

https://build.chromium.org/p/chromium.fyi/builders/CrWinAsan/builds/3976/steps/compile/logs/stdio

FAILED: obj/chrome/tools/disable_outdated_build_detector/lib/constants.obj 
../../third_party/llvm-build/Release+Asserts/bin/clang-cl.exe /nologo /showIncludes /FC @obj/chrome/tools/disable_outdated_build_detector/lib/constants.obj.rsp /c ../../chrome/tools/disable_outdated_build_detector/constants.cc /Foobj/chrome/tools/disable_outdated_build_detector/lib/constants.obj /Fd"obj/chrome/tools/disable_outdated_build_detector/lib_cc.pdb"
In file included from <built-in>:322:
<command line>(54,9):  error: 'NTDDI_VERSION' macro redefined [-Werror,-Wmacro-redefined]
#define NTDDI_VERSION 0x05010200
        ^
<command line>(42,9):  note: previous definition is here
#define NTDDI_VERSION 0x0A000000
        ^


This is due to https://codereview.chromium.org/2255783003
 

Comment 1 by thakis@chromium.org, Aug 22 2016

Summary: Several win/clang bots are failing compile due to disable_outdated_build_detector having build warnings (was: CrWinAsan is failing compile due to disable_outdated_build_detector having build warnings)
Normal official win/clang bots too: https://build.chromium.org/p/chromium.fyi/builders/CrWinClang/builds/11963

Comment 2 by thakis@chromium.org, Aug 22 2016

Blocking: 636399

Comment 3 by thakis@chromium.org, Aug 22 2016

I think this is a good fix:

C:\src\chrome\src>git diff
diff --git a/build/config/win/BUILD.gn b/build/config/win/BUILD.gn
index 5e621ba..a65997b 100644
--- a/build/config/win/BUILD.gn
+++ b/build/config/win/BUILD.gn
@@ -180,7 +180,6 @@ config("runtime_library") {
     "_ATL_NO_OPENGL",
     "_WINDOWS",
     "CERT_CHAIN_PARA_HAS_EXTRA_FIELDS",
-    "NTDDI_VERSION=0x0A000000",
     "PSAPI_VERSION=1",
     "WIN32",
     "_SECURE_ATL",
@@ -224,6 +223,7 @@ config("runtime_library") {
 # targets need to manually override it for their compiles.
 config("winver") {
   defines = [
+    "NTDDI_VERSION=0x0A000000",
     "_WIN32_WINNT=0x0A00",
     "WINVER=0x0A00",
   ]


It'd mean that NTDDI_VERSION stops being defined while building usrsctp, but it's only redefined there because of XP support, and we don't support XP any more.

Comment 4 by thakis@chromium.org, Aug 22 2016

Cc: grt@chromium.org
Owner: thakis@chromium.org
Status: Started (was: Untriaged)
 https://codereview.chromium.org/2265013003 
Project Member

Comment 5 by bugdroid1@chromium.org, Aug 22 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1768309c444850128578325a51afb21599cdcd21

commit 1768309c444850128578325a51afb21599cdcd21
Author: thakis <thakis@chromium.org>
Date: Mon Aug 22 18:38:09 2016

win: Move NTDDI_VERSION into winver, stop setting it for usrsctp

If people want to change WINVER, they likely want to change NTDDI_VERSION as
well, so it make sense to have both in the same config.

The one other use of :winver, usrsctp, didn't change NTDDI_VERSION, but
it looks like it doesn't need to change WINVER anymore anyhow (it did that for
XP support), so drop it there.

No intended behavior change.

BUG= 639853 

Review-Url: https://codereview.chromium.org/2265013003
Cr-Commit-Position: refs/heads/master@{#413480}

[modify] https://crrev.com/1768309c444850128578325a51afb21599cdcd21/build/config/win/BUILD.gn
[modify] https://crrev.com/1768309c444850128578325a51afb21599cdcd21/third_party/usrsctp/BUILD.gn
[modify] https://crrev.com/1768309c444850128578325a51afb21599cdcd21/third_party/usrsctp/usrsctp.gyp

Comment 6 by grt@chromium.org, Aug 22 2016

Thanks for taking care of this!

Comment 7 by thakis@chromium.org, Aug 22 2016

Status: Fixed (was: Started)

Sign in to add a comment