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

Issue 632141 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Aug 2016
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 2
Type: Bug

Blocked on:
issue 633650



Sign in to add a comment

[GN][Windows][Host] Rebuilding package components does not regenerate an MSI

Project Member Reported by joedow@chromium.org, Jul 27 2016

Issue description


Repro steps:
1.) Create a GN output directory (i.e. gn args out\host):
  <My current args>
  is_debug = true
  is_chrome_branded = true
  target_cpu = "x86
2.) Build the host components (lazy was is 'ninja -C out\host remoting_all')
3.) Note that a chromoting.msi file exists in out\host
4.) Change a remote security key or it2me/me2me native messaging host file
5.) Rebuild
6.) Check date of chromoting.msi file in out\host (it should still be the same file as in step #3)

I can delete out\host\chromoting.msi before building and a new one will be generated, however I think that editing a component that exists in the MSI package should trigger a rebuild as well.
 
Owner: zijiehe@chromium.org
Status: Assigned (was: Untriaged)
Joe, what's the exact component are you talking about? I usually rebuild with ninja -C out/GNRelease remoting_host_installation, which always rebuild chromoting.msi as I expected. Since we may change different files, so I believe one or several dependencies are not correctly set in remoting_me2me_host_archive target.

p.s. You always need to add is_component_build=false in debug mode. Since our package does not include components such as base.dll. I have talked with related people, but they do not want to disable component build for GN debug build.
Try rebuilding remote_security_key or the it2Me NMH using the steps I included in comment #1.  Since remoting_all is an inclusive target, my assumptiuon is that it should rebuild any dependency which has changed.

I can talk to you offline about is_component_build but I've yet to have problems with GN without that flag.
FWIW non-component debug builds are not going to be supported in the future (see https://groups.google.com/a/chromium.org/d/topic/chromium-dev/LWRR3fMlvQ4/discussion ). So we should either make MSI work with component builds or disabled MSI for Release.
Thanks Sergeyu@, I've filed 633368 to track the component_build issue you raised.
A quick update, there are two issues so far.
1. {target}.dll.lib won't be updated if some sources files are changed. I believe this is expected, though I cannot quite understand why remoting_core.dll.lib got updated after changing basic_desktop_environment.cc file. Since our remoting_core.dll.lib is almost an empty lib, all the implementations are in libs it depends on.
2. GN should depend on {target}.dll instead of {target}.dll.lib for shared_library. In our scenario, remoting_core.dll has been updated, but remoting_core.dll.lib won't. So the following build procedure has been stopped as expected.

I will follow up with GN team regarding to this issue.
Blockedon: 633650
Project Member

Comment 8 by bugdroid1@chromium.org, Aug 3 2016

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

commit 059a4c3e886ca394ac67703851d7e17022fedef5
Author: zijiehe <zijiehe@chromium.org>
Date: Wed Aug 03 04:00:16 2016

[Chromoting] [GN Build] Force remoting_host_me2me_archive to depend on the files it requires

In GN build, rebuilding certain package components does not regenerate
chromoting.msi. This is due to a GN build issue discribed in
http://crbug.com/633650. Before the issue has been correctly addressed, a
workaround is to actively add inputs to hint action target to depend on the
correct input files.

BUG= 632141 

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

[modify] https://crrev.com/059a4c3e886ca394ac67703851d7e17022fedef5/remoting/host/BUILD.gn

Status: Fixed (was: Assigned)
This issue has been fixed.

Sign in to add a comment