New issue
Advanced search Search tips

Issue 684950 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Bug



Sign in to add a comment

some component updater files are missing build/build_config.h

Project Member Reported by sorin@chromium.org, Jan 25 2017

Issue description

In order to use conditional compilation, the files must include  "build/build_config.h".

Otherwise, code such as the one below will always take the #else branch since OS_WIN is not defined.

bool IsPerUserInstall() {
#if defined(OS_WIN)
  base::FilePath exe_path;
  PathService::Get(base::FILE_EXE, &exe_path);
  return InstallUtil::IsPerUserInstall(exe_path);
#else
  return true;
#endif 
 

Comment 1 by grt@chromium.org, Jan 25 2017

Ah, that would explain it!

Comment 3 by sorin@chromium.org, Jan 26 2017

Status: Fixed (was: Assigned)

Comment 4 by sorin@chromium.org, Jan 26 2017

Labels: Merge-Request-56 Merge-Request-57
Project Member

Comment 5 by sheriffbot@chromium.org, Jan 26 2017

Labels: -Merge-Request-57 Hotlist-Merge-Approved Merge-Approved-57
Your change meets the bar and is auto-approved for M57. Please go ahead and merge the CL to branch 2987 manually. Please contact milestone owner if you have questions.
Owners: amineer@(clank), cmasso@(bling), ketakid@(cros), govind@(desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 6 by sheriffbot@chromium.org, Jan 26 2017

Labels: -Merge-Request-56 Merge-Review-56 Hotlist-Merge-Review
This bug requires manual review: We are only 4 days from stable.
Please contact the milestone owner if you have questions.
Owners: amineer@(clank), cmasso@(bling), gkihumba@(cros), bustamante@(desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 7 by sheriffbot@chromium.org, Jan 26 2017

Labels: -Merge-Request-57 Hotlist-Merge-Approved Merge-Approved-57
Your change meets the bar and is auto-approved for M57. Please go ahead and merge the CL to branch 2987 manually. Please contact milestone owner if you have questions.
Owners: amineer@(clank), cmasso@(bling), ketakid@(cros), govind@(desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 8 by bugdroid1@chromium.org, Jan 27 2017

Labels: -merge-approved-57 merge-merged-2987
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/02cb3e5d0fddfbe903e2a4dd8f8f010d516f8b89

commit 02cb3e5d0fddfbe903e2a4dd8f8f010d516f8b89
Author: Sorin Jianu <sorin@chromium.org>
Date: Fri Jan 27 17:05:02 2017

Merged to M57.

Include build/build_config.h in all component updater files.

All files using conditional compilation must include the build_config,
otherwise the platform symbols are not defined.

BUG= 684950 

Review-Url: https://codereview.chromium.org/2655813002
Cr-Commit-Position: refs/heads/master@{#446050}
(cherry picked from commit 96d71e0c6c152a39ee9b452c96428af7cc4520bc)

Review-Url: https://codereview.chromium.org/2656713009 .
Cr-Commit-Position: refs/branch-heads/2987@{#148}
Cr-Branched-From: ad51088c0e8776e8dcd963dbe752c4035ba6dab6-refs/heads/master@{#444943}

[modify] https://crrev.com/02cb3e5d0fddfbe903e2a4dd8f8f010d516f8b89/chrome/browser/component_updater/chrome_component_updater_configurator.cc
[modify] https://crrev.com/02cb3e5d0fddfbe903e2a4dd8f8f010d516f8b89/chrome/browser/component_updater/component_updater_utils.cc
[modify] https://crrev.com/02cb3e5d0fddfbe903e2a4dd8f8f010d516f8b89/components/update_client/update_checker_unittest.cc
[modify] https://crrev.com/02cb3e5d0fddfbe903e2a4dd8f8f010d516f8b89/components/update_client/updater_state.cc
[modify] https://crrev.com/02cb3e5d0fddfbe903e2a4dd8f8f010d516f8b89/components/update_client/updater_state_unittest.cc

Labels: -Merge-Review-56 Merge-Approved-56
LGTM for merging into M56
Project Member

Comment 10 by bugdroid1@chromium.org, Jan 27 2017

Labels: -merge-approved-56 merge-merged-2924
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fb55e5c0df12ced401f5ccb1309bd3ac7fde0e65

commit fb55e5c0df12ced401f5ccb1309bd3ac7fde0e65
Author: Sorin Jianu <sorin@chromium.org>
Date: Fri Jan 27 18:28:30 2017

Include build/build_config.h in all component updater files.

All files using conditional compilation must include the build_config,
otherwise the platform symbols are not defined.

Merged to M56.

BUG= 684950 

Review-Url: https://codereview.chromium.org/2655813002
Cr-Commit-Position: refs/heads/master@{#446050}
(cherry picked from commit 96d71e0c6c152a39ee9b452c96428af7cc4520bc)

Review-Url: https://codereview.chromium.org/2658133003 .
Cr-Commit-Position: refs/branch-heads/2924@{#878}
Cr-Branched-From: 3a87aecc31cd1ffe751dd72c04e5a96a1fc8108a-refs/heads/master@{#433059}

[modify] https://crrev.com/fb55e5c0df12ced401f5ccb1309bd3ac7fde0e65/chrome/browser/component_updater/chrome_component_updater_configurator.cc
[modify] https://crrev.com/fb55e5c0df12ced401f5ccb1309bd3ac7fde0e65/chrome/browser/component_updater/component_updater_utils.cc
[modify] https://crrev.com/fb55e5c0df12ced401f5ccb1309bd3ac7fde0e65/components/update_client/update_checker_unittest.cc
[modify] https://crrev.com/fb55e5c0df12ced401f5ccb1309bd3ac7fde0e65/components/update_client/updater_state.cc
[modify] https://crrev.com/fb55e5c0df12ced401f5ccb1309bd3ac7fde0e65/components/update_client/updater_state_unittest.cc

Sign in to add a comment