Can you do the following?
1) Make a new CL for just build/install-build-deps.sh.
2) Land that CL.
3) Announce the update on the chromium-dev mailing list and ask developers to re-run build/install-build-deps.sh to get the new font packages?
This is assuming simply installing the new font package won't break anything in Chromium. OTOH, in PDFium our bots got magically updated per bug 665934 and now we have mysterious failing tests and I finally tracked it down.
Linking this bug to Issue 665693 to document the cause and effect relationship.
The build dependencies are clearly incomplete -- the blink_tests target doesn't understand that an update to install-build-deps may require a run of webkit_tests. From the earlier CL:
https://codereview.chromium.org/2500063003
The tryjob for linux_chromium_rel_ng:
https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_ng/builds/340638
didn't attempt to run webkit_tests. If it had, it would have failed, preventing the bad patch from reaching the tree in the first place.
Fixing the missing dependency is the most important action to be taken on this bug, before the actual font changes are made. Otherwise the risk will be run of breaking the tree again.
Thank you for your help!
I'm trying to understand why the change to fontconfig_util_linux.cc didn't trigger blink_tests?
I assumed that content_shell depends on fontconfig_util_linux.cc (I think it was rebuilt when I changed fontconfig_util_linux.cc)
and I assume blink_tests depends on content_shell?
(content_shell failed to start without the new font files I added to fontconfig_util_linux.cc, breaking the tree.)
Is it reasonable for a change to fontconfig_util_linux.cc to trigger blink_tests or am I way off course?
I assume that if I edit fontconfig_util_linux.cc and run ninja blink_tests, I should see more than the following?
> $ edit ui/gfx/test/fontconfig_util_linux.cc
> $ ninja -C out/Default -n blink_tests
> ninja: Entering directory `out/Default'
> [9/9] STAMP obj/blink_tests.stamp
> $
Changing install_build_deps.sh should probably just recompile the world (via the whitelist in //testing/buildbot/trybot_analyze_config.json).
I don't think we have a mechanism at the moment for GN to detect when linux packages have been updated, and so the normal mechanisms for analyze wouldn't work right (I filed bug 667380 to double-check/confirm this).
Since the layout tests do not yet run under swarming, we intentionally do not run them on every change that affects them (unfortunately). So, that part of the system behaved as I'd expect (see bug 524758 for the work getting them to be swarmed).
As to the results in comment #9, given the path shown in comment #8, I'd expect touching fontconfig_util_linux to do something, and in comment #9, you can see that it does something, but not quite what :). It's possible that if we recompiled one file in a shared library it might only trigger the 9 steps shown. I don't have an up-to-date linux build in front of me to confirm at the moment.
My mistake, ninja blink_tests did recompile content_shell, as it should.
I now understand that *running* the layout tests is another matter, unconnected with GN and Ninja?
I think that fontconfig_util_linux.cc, not install-build-deps.sh, is significant to this issue because:
If changing install-build-deps.sh recompiled the world, but I commit the change to install-build-deps.sh first [1], then the layout tests will still pass and the patch will reach the tree.
When I subsequently commit fontconfig_util_linux.cc, if the tests aren't run at that time, then that patch will reach the tree and break it, because subsequent layout tests runs will fail.
So I think this issue is that we intentionally don't run the layout tests on every change that affects them, and issue 524758 fixes that?
In the meantime, I should have anticipated this problem and manually requested layout tests runs on the Linux bots with:
> git cl try \
> -b linux_precise_blink_rel \
> -b linux_trusty_blink_rel \
Is this right?
[1] https://codereview.chromium.org/2519793002
> So I think this issue is that we intentionally don't run the layout tests on every
> change that affects them
That's not my understanding -- I thought that if a test target was affected by a CL, then it's run. In my understanding linux_chromium_rel_ng should have run webkit_tests against your CL. dpranke@, tansell@, qyearsley@ -- is this correct?
in comment #11, jack@nottheoilrig.com wrote:
> So I think this issue is that we intentionally don't run the layout tests
> on every change that affects them, and issue 524758 fixes that?
Correct.
> In the meantime, I should have anticipated this problem and manually
> requested layout tests runs on the Linux bots with:
>
> > git cl try \
> > -b linux_precise_blink_rel \
> > -b linux_trusty_blink_rel \
>
> Is this right?
Actually, I wouldn't have expected you to anticipate that. I didn't think of it, either :). I put this problem more in the "accidental bad thing happened, let's figure out how to improve things in the future" bucket.
in comment #12, kbr@ wrote:
> > So I think this issue is that we intentionally don't run the layout tests on every
> > change that affects them
>
> That's not my understanding -- I thought that if a test target was affected by a CL,
> then it's run. In my understanding linux_chromium_rel_ng should have run
> webkit_tests against your CL. dpranke@, tansell@, qyearsley@ -- is this correct?
No, that's not correct, and your understanding is wrong :). What I wrote in comment #10 (we intentionally did not run them for this change) is correct.
Thanks Dirk, I wasn't aware of that. What rules govern whether the layout tests will be run for a given CL? Clearly linux_chromium_rel_ng runs them sometimes. Issue 524758 doesn't describe the rules.
The rules are not well-established, and set in a mostly ad-hoc way by mostly jam@ w/ some arguing with others :).
We run the tryjobs for changes to files in a given set of directories, as set by this list:
https://cs.chromium.org/chromium/build/scripts/slave/recipe_modules/chromium_tests/api.py?rcl=0&l=36
which is basically "stuff in blink" plus stuff that is really really closely tied to blink and also doesn't take too long to run. That last part is important, and is why we don't run the tests over everything in content, let alone //net, //base, etc.
We very much want to get rid of these ad-hoc rules, but we need swarmed layout tests for that.
dpranke@ Don't you use gn build rules to figure this out for normal unit tests? Could we switch to using that here now and just filter out anything which we think might cause to much triggering at the moment?
@tansell - the fonts aren't actually being tracked in-tree; they're just assumed to be installed on the system, and there's no reference to them in the build anywhere.
I expect that you'll suggest that we should be checking the fonts in and tracking and referencing them via GN that way (and not depend on the system packages at all), and if you do suggest that, I'll agree with you :).
content_shell.log only exists in some cases, depending on how you're running it. In the webkit_tests case, the output is captured as part of the test run.
Where do I find the content_shell output? e.g. in this run [1] I see:
> 16:53:48.821 13886 Failed to start the content_shell process:
> 16:53:48.822 13886 content_shell took too long to startup.
> 16:53:48.857 13886 worker/0 http/tests/accessibility/slow-document-load.html crashed, (no stderr)
> 16:53:48.866 13806 [1/43088] http/tests/accessibility/slow-document-load.html failed unexpectedly (content_shell crashed [pid=13894])
> 16:53:48.858 13886 worker/0 killing primary driver
> 16:53:48.858 13886 worker/0 killing secondary driver
> 16:53:48.858 13886 worker/0 http/tests/accessibility/slow-document-load.html failed:
> 16:53:48.858 13886 worker/0 content_shell crashed [pid=13894]
> 16:53:48.859 13886 worker/0 http/tests/activedomobject/media.html started
What I'm looking for is:
> [...:ERROR:fontconfig_util_linux.cc(88)] You are missing /usr/share/fonts/opentype/ipafont-gothic/ipag.ttf. Try re-running build/install-build-deps.sh. Also see https://chromium.googlesource.com/chromium/src/+/master/docs/layout_tests_linux.md
(It's written to content_shell.log when I run the tests locally.)
[1] https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux%20Trusty/builds/19980
Comment 1 by j...@nottheoilrig.com
, Nov 15 2016