Add version info to the eventlog_provider.dll.

Add version info to the eventlog_provider.dll.

BUG=none
TEST=none

Review-Url: https://codereview.chromium.org/2687783003
Cr-Commit-Position: refs/heads/master@{#449586}
Committed: https://chromium.googlesource.com/chromium/src/+/fca8ec3cb1f629bdbcb5aafb42f53449bc3939f3

[pastarmovj, 2017-02-10 08:46:15]

Hi Greg,

please take a look when you have time. This CL adds the version info to the dll.
It is not a big deal but can be useful if issues shall ever arise.

Best,
Julian

[pastarmovj, 2017-02-10 08:56:27]

pastarmovj@chromium.org changed reviewers:
+ grt@chromium.org

[pastarmovj, 2017-02-10 08:56:28]

Hi Greg,

please take a look when you have time. This CL adds the version info to the dll.
It is not a big deal but can be useful if issues shall ever arise.

Best,
Julian

[grt (UTC plus 2), 2017-02-10 09:56:55]

lgtm w/ fix below

https://codereview.chromium.org/2687783003/diff/1/base/win/BUILD.gn
File base/win/BUILD.gn (right):

https://codereview.chromium.org/2687783003/diff/1/base/win/BUILD.gn#newcode46
base/win/BUILD.gn:46: output = "$root_gen_dir/base/win/eventlog_provider.rc"
"$target_gen_dir/eventlog_provider_dll_version.rc" for consistency with other
uses of the template.

[pastarmovj, 2017-02-10 10:04:30]

https://codereview.chromium.org/2687783003/diff/1/base/win/BUILD.gn
File base/win/BUILD.gn (right):

https://codereview.chromium.org/2687783003/diff/1/base/win/BUILD.gn#newcode46
base/win/BUILD.gn:46: output = "$root_gen_dir/base/win/eventlog_provider.rc"
On 2017/02/10 09:56:55, grt (UTC plus 1) wrote:
> "$target_gen_dir/eventlog_provider_dll_version.rc" for consistency with other
> uses of the template.

Done.

[pastarmovj, 2017-02-10 10:04:35]

The CQ bit was checked by pastarmovj@chromium.org

[pastarmovj, 2017-02-10 10:04:35]

The patchset sent to the CQ was uploaded after l-g-t-m from grt@chromium.org
Link to the patchset: https://codereview.chromium.org/2687783003/#ps20001
(title: "Address comment.")

[commit-bot: I haz the power, 2017-02-10 10:04:49]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-10 11:02:45]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1486721075283480,
"parent_rev": "080936f5193590e65409c60127a510f71e910d73", "commit_rev":
"fca8ec3cb1f629bdbcb5aafb42f53449bc3939f3"}

[commit-bot: I haz the power, 2017-02-10 11:03:16]

Description was changed from

==========
Add version info to the eventlog_provider.dll.

BUG=none
TEST=none
==========

to

==========
Add version info to the eventlog_provider.dll.

BUG=none
TEST=none

Review-Url: https://codereview.chromium.org/2687783003
Cr-Commit-Position: refs/heads/master@{#449586}
Committed:
https://chromium.googlesource.com/chromium/src/+/fca8ec3cb1f629bdbcb5aafb42f5...
==========

[commit-bot: I haz the power, 2017-02-10 11:03:18]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/fca8ec3cb1f629bdbcb5aafb42f5...

[veranika, 2017-02-17 19:17:20]

veranika@chromium.org changed reviewers:
+ veranika@chromium.org

[veranika, 2017-02-17 19:17:20]

https://codereview.chromium.org/2687783003/diff/20001/base/win/BUILD.gn
File base/win/BUILD.gn (right):

https://codereview.chromium.org/2687783003/diff/20001/base/win/BUILD.gn#newcode8
base/win/BUILD.gn:8: import("//chrome/process_version_rc_template.gni")
This change makes //base/win depend on //chrome, which doesn't look right to me.
Is there any way to refactor process_version_rc_template.gni out of //chrome?

[grt (UTC plus 2), 2017-02-20 09:26:21]

https://codereview.chromium.org/2687783003/diff/20001/base/win/BUILD.gn
File base/win/BUILD.gn (right):

https://codereview.chromium.org/2687783003/diff/20001/base/win/BUILD.gn#newcode8
base/win/BUILD.gn:8: import("//chrome/process_version_rc_template.gni")
On 2017/02/17 19:17:20, veranika wrote:
> This change makes //base/win depend on //chrome, which doesn't look right to
me.
> Is there any way to refactor process_version_rc_template.gni out of //chrome?

Agreed. Sorry I missed that. Since logging::SetEventSourceName is called from
chrome/common to fire up the provider for Chrome, maybe the right thing is to
move the provider dll out of base and into chrome itself?

[pastarmovj, 2017-02-20 09:47:01]

https://codereview.chromium.org/2687783003/diff/20001/base/win/BUILD.gn
File base/win/BUILD.gn (right):

https://codereview.chromium.org/2687783003/diff/20001/base/win/BUILD.gn#newcode8
base/win/BUILD.gn:8: import("//chrome/process_version_rc_template.gni")
On 2017/02/20 09:26:21, grt (UTC plus 1) wrote:
> On 2017/02/17 19:17:20, veranika wrote:
> > This change makes //base/win depend on //chrome, which doesn't look right to
> me.
> > Is there any way to refactor process_version_rc_template.gni out of
//chrome?
> 
> Agreed. Sorry I missed that. Since logging::SetEventSourceName is called from
> chrome/common to fire up the provider for Chrome, maybe the right thing is to
> move the provider dll out of base and into chrome itself?

I contemplated this for a while when I was adding this in here but I decided the
layering did not stretch to using build tools defined in another layer.

Moving the dll generation to Chrome can be a good choice although it really is
related to the SYSLOG macro defined in base then specific to Chrome.

[grt (UTC plus 2), 2017-02-20 09:53:47]

https://codereview.chromium.org/2687783003/diff/20001/base/win/BUILD.gn
File base/win/BUILD.gn (right):

https://codereview.chromium.org/2687783003/diff/20001/base/win/BUILD.gn#newcode8
base/win/BUILD.gn:8: import("//chrome/process_version_rc_template.gni")
On 2017/02/20 09:47:01, pastarmovj wrote:
> On 2017/02/20 09:26:21, grt (UTC plus 1) wrote:
> > On 2017/02/17 19:17:20, veranika wrote:
> > > This change makes //base/win depend on //chrome, which doesn't look right
to
> > me.
> > > Is there any way to refactor process_version_rc_template.gni out of
> //chrome?
> > 
> > Agreed. Sorry I missed that. Since logging::SetEventSourceName is called
from
> > chrome/common to fire up the provider for Chrome, maybe the right thing is
to
> > move the provider dll out of base and into chrome itself?
> 
> I contemplated this for a while when I was adding this in here but I decided
the
> layering did not stretch to using build tools defined in another layer.
> 
> Moving the dll generation to Chrome can be a good choice although it really is
> related to the SYSLOG macro defined in base then specific to Chrome.

Hmm. Well, another way to look at it is that adding version info to a DLL or EXE
is a build-ish thing more than a Chrome-ish thing. Can the version template
stuff be moved into src/build without moving Chrome-isms into there?