My favorites | Sign in
Project Home Downloads Wiki Issues Code Search
New issue   Search
for
  Advanced search   Search tips   Subscriptions
Issue 35878: Remove circular relationships between .gyp files
8 people starred this issue and may be notified of changes. Back to list
 
Project Member Reported by mark@chromium.org, Feb 16, 2010
There aren't any circular relationships between .gyp files on the Mac, but 
there are on other platforms.

Google Chrome XP: gyp.input.CircularException: Some files not reachable, 
cycle in .gyp file dependency graph detected involving some or all of: 
src\chrome\chrome.gyp src\chrome_frame\chrome_frame.gyp 
src\webkit\default_plugin\default_plugin.gyp 
src\chrome\installer\mini_installer.gyp 
src\chrome\app\locales\locales.gyp src\build\all.gyp 
src\chrome\installer\installer.gyp src\webkit\webkit.gyp

Google Chrome Linux: gyp.input.CircularException: Some files not 
reachable, cycle in .gyp file dependency graph detected involving some or 
all of: /b/slave/google-chrome-rel-
linux/build/src/chrome/installer/installer.gyp /b/slave/google-chrome-
rel-linux/build/src/build/all.gyp /b/slave/google-chrome-rel-
linux/build/src/chrome/chrome.gyp

Google Chrome Linux x64: gyp.input.CircularException: Some files not 
reachable, cycle in .gyp file dependency graph detected involving some or 
all of: /b/slave/google-chrome-rel-
linux_64/build/src/chrome/chrome.gyp /b/slave/google-chrome-rel-
linux_64/build/src/build/all.gyp /b/slave/google-chrome-rel-
linux_64/build/src/chrome/installer/installer.gyp

Feb 16, 2010
#1 bugdroid1@gmail.com
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=39128 

------------------------------------------------------------------------
r39128 | mark@chromium.org | 2010-02-16 12:14:26 -0800 (Tue, 16 Feb 2010) | 13 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/DEPS?r1=39128&r2=39127
   M http://src.chromium.org/viewvc/chrome/trunk/src/build/gyp_chromium?r1=39128&r2=39127

Circular relationships between .gyp files should be errors.  Make them errors,
but currently only on the Mac.

These relationships should be errors on all platforms, but some currently
exist on non-Mac platforms.  See http://crbug.com/35878.  Because the Mac is
the only platform where a circular dependency between .gyp files is known to
cause tangible problems, the portions of Chromium's .gyp files that are used
by Macs have been fixed to remove these relationships, and the check is left
enabled on the Mac to ensure that no new ones are created.

BUG=35308
TEST=none
Review URL: http://codereview.chromium.org/600151
------------------------------------------------------------------------

May 18, 2010
#2 tony@chromium.org
(No comment was entered for this change.)
Blockedon: 44538
Jul 8, 2010
#3 tfar...@chromium.org
(No comment was entered for this change.)
Labels: Area-BuildTools
Mar 22, 2011
#5 tony@chromium.org
(No comment was entered for this change.)
Blockedon: 77100
Mar 9, 2013
#6 bugdro...@chromium.org
(No comment was entered for this change.)
Labels: -Area-Internals -Internals-Core -Area-Build Cr-Internals Build Cr-Internals-Core
Blockedon: -chromium:44538 -chromium:77100 chromium:44538 chromium:77100
Blocking: -chromium:35308 chromium:35308
Aug 15, 2013
#7 phajdan.jr@chromium.org
(No comment was entered for this change.)
Cc: phajdan.jr@chromium.org
Labels: TaskForce-GreenTree
Aug 19, 2013
#8 bugdro...@chromium.org
------------------------------------------------------------------------
r218334 | phajdan.jr@chromium.org | 2013-08-19T22:04:53.112530Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/tools/generate_library_loader/generate_library_loader.py?r1=218334&r2=218333&pathrev=218334
   M http://src.chromium.org/viewvc/chrome/trunk/src/build/linux/system.gyp?r1=218334&r2=218333&pathrev=218334

Linux: untangle circular dependencies between .gyp files.

Not enabling the check yet because ChromeOS still has cycles.

BUG=35878
R=mark@chromium.org

Review URL: https://codereview.chromium.org/22825011
------------------------------------------------------------------------
Jan 22, 2014
#9 tapted@chromium.org
Attaching
 - loops that currently exist with use_aura = 1,
 - a python script I hacked together to make sense of things
And, generated with `cat aura_and_views_loops.txt | sed -e 's/ -> /\n/g; s/Cycle: //g' | sort -u` as input)
 - aura.png: graph of loops that exist around aura
 - views.png: graph of loops that exist around views
aura_and_views_loops.txt
12.7 KB   View   Download
find_depcycles.py
3.2 KB   View   Download
aura.png
290 KB   View   Download
views.png
111 KB   View   Download
Jan 30, 2014
#10 tapted@chromium.org
Attaching result of splitting aura.gyp into aura.gyp and aura_tests.gyp (https://codereview.chromium.org/131843004/)
after_aura_tests.png
126 KB   View   Download
Feb 6, 2014
#11 bugdro...@chromium.org
------------------------------------------------------------------------
r249568 | tfarina@chromium.org | 2014-02-07T00:57:53.043875Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/ui/ui_unittests.gyp?r1=249568&r2=249567&pathrev=249568
   M http://src.chromium.org/viewvc/chrome/trunk/src/ui/shell_dialogs/shell_dialogs.gyp?r1=249568&r2=249567&pathrev=249568

Add unit test target for shell_dialogs module.

This is necessary to avoid a circular dependency that on Mac is forbidden:

ui_unittests -> shell_dialogs -> aura -> ui_unittests

The down side is that momentarily we lose automatic test coverage on the bots for the shell_dialogs.
This can be fixed by teaching the buildbot scripts to run it after this lands.

BUG=331669, 35878, 299841
TEST=shell_dialogs_unittests
R=ben@chromium.org

Review URL: https://codereview.chromium.org/151253004
------------------------------------------------------------------------
Mar 6, 2014
#12 bugdro...@chromium.org
------------------------------------------------------------------------
r255512 | tapted@chromium.org | 2014-03-07T03:41:36.610793Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/ui/base/strings/ui_strings.gyp?r1=255512&r2=255511&pathrev=255512
   A http://src.chromium.org/viewvc/chrome/trunk/src/ui/base/test/framework-Info.plist?r1=255512&r2=255511&pathrev=255512
   M http://src.chromium.org/viewvc/chrome/trunk/src/ui/base/test/run_all_unittests.cc?r1=255512&r2=255511&pathrev=255512
   M http://src.chromium.org/viewvc/chrome/trunk/src/ui/base/l10n/time_format_unittest.cc?r1=255512&r2=255511&pathrev=255512
   M http://src.chromium.org/viewvc/chrome/trunk/src/ui/ui_unittests.gyp?r1=255512&r2=255511&pathrev=255512
   A http://src.chromium.org/viewvc/chrome/trunk/src/ui/ui_unittests_bundle.gypi?r1=255512&r2=255511&pathrev=255512

Introduce a mock ui_unittests Framework for loading resources.

This allows ui_unittests to stop depending on the chrome framework.

On Mac, this creates (e.g.)
- out/ui_unittests Framework.framework/
  +-- Resources -> Versions/A/Resources
  \-- Versions
      \-- A
          \-- Resources
              +-- Info.plist
              +-- am.lproj
              |   \-- locale.pak
              +-- ...
              +-- chrome_100_percent.pak -> ui_test.pak
              +-- ...
              +-- en.lproj
              |   \-- locale.pak
              +-- ...

On other platforms, out/ui_test.pak is loaded directly and
out/ui_unittests_strings/ is set as the locale folder (for tests that
load en-US.pak from there). ui_unittests currently depends on
out/locales/ which is only created when Chrome is built.

Note that ui_unittests does not currently succeed in a clobber build
(crbug.com/347851), so that missing dependency is fixed by this change
as well.

BUG=331669, 35878, 347851
TEST=ui_unittests should build and run after clobbering the build folder

Review URL: https://codereview.chromium.org/152543005
------------------------------------------------------------------------
Mar 6, 2014
#13 bugdro...@chromium.org
------------------------------------------------------------------------
r255528 | tapted@chromium.org | 2014-03-07T04:42:24.204488Z

Changed paths:
   D http://src.chromium.org/viewvc/chrome/trunk/src/ui/base/test/framework-Info.plist?r1=255528&r2=255527&pathrev=255528
   M http://src.chromium.org/viewvc/chrome/trunk/src/ui/base/test/run_all_unittests.cc?r1=255528&r2=255527&pathrev=255528
   M http://src.chromium.org/viewvc/chrome/trunk/src/ui/base/l10n/time_format_unittest.cc?r1=255528&r2=255527&pathrev=255528
   M http://src.chromium.org/viewvc/chrome/trunk/src/ui/ui_unittests.gyp?r1=255528&r2=255527&pathrev=255528
   D http://src.chromium.org/viewvc/chrome/trunk/src/ui/ui_unittests_bundle.gypi?r1=255528&r2=255527&pathrev=255528
   M http://src.chromium.org/viewvc/chrome/trunk/src/ui/base/strings/ui_strings.gyp?r1=255528&r2=255527&pathrev=255528

Revert 255512 "Introduce a mock ui_unittests Framework for loadi..."

All the chromium.mac bots were happy, but the official builder didn't like it.

> Introduce a mock ui_unittests Framework for loading resources.
> 
> This allows ui_unittests to stop depending on the chrome framework.
> 
> On Mac, this creates (e.g.)
> - out/ui_unittests Framework.framework/
>   +-- Resources -> Versions/A/Resources
>   \-- Versions
>       \-- A
>           \-- Resources
>               +-- Info.plist
>               +-- am.lproj
>               |   \-- locale.pak
>               +-- ...
>               +-- chrome_100_percent.pak -> ui_test.pak
>               +-- ...
>               +-- en.lproj
>               |   \-- locale.pak
>               +-- ...
> 
> On other platforms, out/ui_test.pak is loaded directly and
> out/ui_unittests_strings/ is set as the locale folder (for tests that
> load en-US.pak from there). ui_unittests currently depends on
> out/locales/ which is only created when Chrome is built.
> 
> Note that ui_unittests does not currently succeed in a clobber build
> (crbug.com/347851), so that missing dependency is fixed by this change
> as well.
> 
> BUG=331669, 35878, 347851
> TEST=ui_unittests should build and run after clobbering the build folder
> 
> Review URL: https://codereview.chromium.org/152543005

TBR=tapted@chromium.org

Review URL: https://codereview.chromium.org/190133003
------------------------------------------------------------------------
Mar 11, 2014
#14 bugdro...@chromium.org
------------------------------------------------------------------------
r256419 | tapted@chromium.org | 2014-03-12T04:33:56.475665Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/ui/base/strings/ui_strings.gyp?r1=256419&r2=256418&pathrev=256419
   A http://src.chromium.org/viewvc/chrome/trunk/src/ui/base/test/framework-Info.plist?r1=256419&r2=256418&pathrev=256419
   M http://src.chromium.org/viewvc/chrome/trunk/src/ui/base/test/run_all_unittests.cc?r1=256419&r2=256418&pathrev=256419
   M http://src.chromium.org/viewvc/chrome/trunk/src/ui/base/l10n/time_format_unittest.cc?r1=256419&r2=256418&pathrev=256419
   M http://src.chromium.org/viewvc/chrome/trunk/src/ui/ui_unittests.gyp?r1=256419&r2=256418&pathrev=256419
   A http://src.chromium.org/viewvc/chrome/trunk/src/ui/ui_unittests_bundle.gypi?r1=256419&r2=256418&pathrev=256419

Introduce a mock ui_unittests Framework for loading resources.

This allows ui_unittests to stop depending on the chrome framework.

On Mac, this creates (e.g.)
- out/ui_unittests Framework.framework/
  +-- Resources -> Versions/A/Resources
  \-- Versions
      \-- A
          \-- Resources
              +-- Info.plist
              +-- am.lproj
              |   \-- locale.pak
              +-- ...
              +-- chrome_100_percent.pak -> ui_test.pak
              +-- ...
              +-- en.lproj
              |   \-- locale.pak
              +-- ...

On other platforms, out/ui_test.pak is loaded directly and
out/ui_unittests_strings/ is set as the locale folder (for tests that
load en-US.pak from there). ui_unittests currently depends on
out/locales/ which is only created when Chrome is built.

Note that ui_unittests does not currently succeed in a clobber build
(crbug.com/347851), so that missing dependency is fixed by this change
as well.

BUG=331669, 35878, 347851
TEST=ui_unittests should build and run after clobbering the build folder

Previously Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=255512

Review URL: https://codereview.chromium.org/152543005
------------------------------------------------------------------------
Mar 22, 2014
#15 bugdro...@chromium.org
------------------------------------------------------------------
r258758 | tfarina@chromium.org | 2014-03-22T03:11:55.443910Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/build/all.gyp?r1=258758&r2=258757&pathrev=258758
   M http://src.chromium.org/viewvc/chrome/trunk/src/ui/views/views.gyp?r1=258758&r2=258757&pathrev=258758
   A http://src.chromium.org/viewvc/chrome/trunk/src/ui/views/controls/webview/webview_tests.gyp?r1=258758&r2=258757&pathrev=258758
   M http://src.chromium.org/viewvc/chrome/trunk/src/ash/ash.gyp?r1=258758&r2=258757&pathrev=258758
   A http://src.chromium.org/viewvc/chrome/trunk/src/ui/views/examples/examples.gyp?r1=258758&r2=258757&pathrev=258758

Break cycles between views, content and webview.

When running gyp_chromium with the following diff:

diff --git a/build/gyp_chromium b/build/gyp_chromium
index 63e8671..ca9b6a4 100755
--- a/build/gyp_chromium
+++ b/build/gyp_chromium
@@ -509,8 +509,6 @@ if __name__ == '__main__':
   # option.  http://crbug.com/35878.
   # TODO(tc): Fix circular dependencies in ChromiumOS then add linux2
   # list.
-  if sys.platform not in ('darwin',):
-    args.append('--no-circular-check')

These cycles are found:

gyp: Cycles in .gyp file dependency graph detected:
Cycle: content/content_shell_and_tests.gyp ->
ui/views/controls/webview/webview.gyp -> ui/views/views.gyp ->
content/content_shell_and_tests.gyp
Cycle: ui/views/controls/webview/webview.gyp -> ui/views/views.gyp ->
content/content_shell_and_tests.gyp ->
ui/views/controls/webview/webview.gyp
Cycle: ui/views/views.gyp -> content/content_shell_and_tests.gyp ->
ui/views/controls/webview/webview.gyp -> ui/views/views.gyp
Cycle: ui/views/views.gyp -> content/content_shell_and_tests.gyp ->
ui/views/views.gyp
Cycle: ui/views/controls/webview/webview.gyp -> ui/views/views.gyp ->
ui/views/controls/webview/webview.gyp
Cycle: ui/views/views.gyp -> ui/views/controls/webview/webview.gyp ->
ui/views/views.gyp
Cycle: content/content_shell_and_tests.gyp -> ui/views/views.gyp ->
content/content_shell_and_tests.gyp

By moving '*examples*' targets from views.gyp to examples.gyp we break
most of these cycles.

Then it remains the cycle:
Cycle: content/content_shell_and_tests.gyp -> ui/views/controls/webview/webview.gyp -> content/content_shell_and_tests.gyp

To fix that we introduced a webview_tests.gyp to which we moved the include of
content_shell_and_tests.gyp from webview.gyp, and thus breaking that
cycle and fixing all the circlar dependencies found above.

BUG=331669,35878
TEST=run gyp_chromium with the above diff, gyp should not throw any
cycles output.

R=ben@chromium.org, harrym@chromium.org, tapted@chromium.org

Review URL: https://codereview.chromium.org/201093002
-----------------------------------------------------------------
Yesterday (32 hours ago)
#16 tapted@chromium.org
(No comment was entered for this change.)
Blockedon: chromium:418455
Sign in to add a comment

Powered by Google Project Hosting