965151 - Valgrind builds should build with tooltool gcc like other builds

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[Randell Jesup [:jesup] (needinfo me)]

https://tbpl.mozilla.org/?tree=Try&rev=bb13e178c7ef is an example where this fails (it has a gcc patch that enables AVX2 inline asm and should work).

Comment 1

[Mike Hommey [:glandium]]

The buildbot config makes no sense to me. Valgrind builds are not really like the others, but despite that, i don't see why they wouldn't use tooltool: They are copying tooltool in the mock environment through mock_copyin_files, which, considering the other files being copied in, means the config is takes from linux or linux64, which makes sense, since valgrind builds are declared to run on platform linux64. And the linux64 platform has tooltool_manifest_src set which should trigger the tooltool download, but it doesn't.

Comment 2

[Ralph Giles (:rillian)]

Attachment: Part 2: Enable tooltool on linux64 valgrind builds

Enable patch for when we get the build slave side fixed.

Comment 3

[Mike Hommey [:glandium]]

Comment on attachment 8497818 [details] [diff] [review]
Part 2: Enable tooltool on linux64 valgrind builds

Review of attachment 8497818 [details] [diff] [review]:
-----------------------------------------------------------------

Obviously can't land without a part 1 being landed on the buildbot or mozharness side.

Comment 5

[Mike Hommey [:glandium]]

Attachment: Setup tooltool in valgrind builds

Could you also test this on staging?

Comment 6

[Mike Hommey [:glandium]]

Note I /think/ the valgrind builds will use the tooltool manifests for normal linux builds (as in, I think they inherit their tooltool buildbot-config).

Comment 7

[Nick Thomas [:nthomas] (UTC+12)]

Comment on attachment 8513150 [details] [diff] [review]
Setup tooltool in valgrind builds

I think this is good to land as is. It's adding steps like this to 14 valgrind builders:

C {'command': ['sh',  <WithProperties "%(toolsdir)s/scripts/tooltool/tooltool_wrapper.sh">,  'browser/config/tooltool-manifests/linux64/releng.manifest',  'http://runtime-binaries.pvt.build.mozilla.org/tooltool',  'setup.sh',  '/builds/tooltool.py'], 'description': None, 'descriptionDone': None, 'env': {'CCACHE_COMPRESS': '1',  'CCACHE_DIR': '/builds/ccache',  'CCACHE_UMASK': '002',  'DISPLAY': ':2',  'HG_SHARE_BASE_DIR': '/builds/hg-shared',  'LC_ALL': 'C',  'LD_LIBRARY_PATH': '/tools/gcc-4.3.3/installed/lib64',  'MOZ_AUTOMATION': '1',  'MOZ_CRASHREPORTER_NO_REPORT': '1',  'MOZ_OBJDIR': 'obj-firefox',  'MOZ_SYMBOLS_EXTRA_BUILDID': 'linux64',  'PATH': '/tools/buildbot/bin:/usr/local/bin:/usr/lib64/ccache:/bin:/usr/bin:/usr/local/sbin:/usr/sbin:/sbin:/tools/git/bin:/tools/python27/bin:/tools/python27-mercurial/bin:/home/cltbld/bin',  'POST_SYMBOL_UPLOAD_CMD': '/usr/local/bin/post-symbol-upload.py',  'PROPERTIES_FILE': <WithProperties "%(basedir)s/buildprops.json">,  'REVISION': <WithProperties "%(revision)s">,  'SYMBOL_SERVER_HOST': 'symbolpush.mozilla.org',  'SYMBOL_SERVER_PATH': '/mnt/netapp/breakpad/symbols_ffx/',  'SYMBOL_SERVER_SSH_KEY': '/home/mock_mozilla/.ssh/ffxbld_rsa',  'SYMBOL_SERVER_USER': 'ffxbld',  'TINDERBOX_OUTPUT': '1',  'TOOLTOOL_CACHE': '/builds/tooltool_cache',  'TOOLTOOL_HOME': '/builds'}, 'haltOnFailure': True, 'log_eval_func': None, 'logfiles': {}, 'name': 'run_tooltool', 'usePTY': 'slave-config', 'workdir': None} {'HG_SHARE_BASE_DIR': '/builds/hg-shared', 'MOZ_CRASHREPORTER_NO_REPORT': '1', 'SYMBOL_SERVER_HOST': 'symbolpush.mozilla.org', 'MOZ_AUTOMATION': '1', 'PATH': '/tools/buildbot/bin:/usr/local/bin:/usr/lib64/ccache:/bin:/usr/bin:/usr/local/sbin:/usr/sbin:/sbin:/tools/git/bin:/tools/python27/bin:/tools/python27-mercurial/bin:/home/cltbld/bin', 'TINDERBOX_OUTPUT': '1', 'TOOLTOOL_CACHE': '/builds/tooltool_cache', 'MOZ_OBJDIR': 'obj-firefox', 'LC_ALL': 'C', 'SYMBOL_SERVER_USER': 'ffxbld', 'TOOLTOOL_HOME': '/builds', 'DISPLAY': ':2', 'CCACHE_UMASK': '002', 'MOZ_SYMBOLS_EXTRA_BUILDID': 'linux64', 'POST_SYMBOL_UPLOAD_CMD': '/usr/local/bin/post-symbol-upload.py', 'SYMBOL_SERVER_SSH_KEY': '/home/mock_mozilla/.ssh/ffxbld_rsa', 'LD_LIBRARY_PATH': '/tools/gcc-4.3.3/installed/lib64', 'SYMBOL_SERVER_PATH': '/mnt/netapp/breakpad/symbols_ffx/', 'CCACHE_DIR': '/builds/ccache', 'CCACHE_COMPRESS': '1', 'REVISION': <WithProperties "%(revision)s">}

This can get deployed in buildbot and it will run tooltool but not use the compiler yet. Then you can verify it's working by pushing the other patch to try. Also looks like
 http://mxr.mozilla.org/mozilla-central/source/build/unix/mozconfig.linux#10
should be updated.

Comment 8

[Mike Hommey [:glandium]]

https://hg.mozilla.org/build/buildbotcustom/rev/0fefa828e5f9

This requires a reconfig, right?

Comment 9

[Nick Thomas [:nthomas] (UTC+12)]

And of course it blows up because there's no toolsdir property.

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Valgrind builds hidden on trunk trees until this gets redeployed with a fix or a backout.

Comment 11

[Nick Thomas [:nthomas] (UTC+12)]

Attachment: [buildbotcustom] Set toolsdir property

Comment 12

[Chris Cooper [:coop] (he/him)]

In production: https://hg.mozilla.org/build/buildbotcustom/rev/0fefa828e5f9

Comment 13

[Nick Thomas [:nthomas] (UTC+12)]

Comment on attachment 8514627 [details] [diff] [review]
[buildbotcustom] Set toolsdir property

https://hg.mozilla.org/build/buildbotcustom/rev/d230027f84a3
https://hg.mozilla.org/build/buildbotcustom/rev/7ec5f5899e53

Deployed by buildbot reconfig.

Comment 14

[Nick Thomas [:nthomas] (UTC+12)]

Bustage is fixed, but it reveals that tooltool won't work at that point in the sequence:
 browser/config/tooltool-manifests/linux64/releng.manifest doesn't exist, skipping...

It's being run prior to the gecko source being checked out. Ideally this would all be in mozharness, but tools/scripts/valgrind/valgrind.sh in the current location. We should back out the two buildbotcustom patches.

Comment 15

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Valgrind unhidden since things are passing.

Comment 16

[Mike Hommey [:glandium]]

how about moving hgtool out of valgrind.sh?

Comment 17

[Chris Cooper [:coop] (he/him)]

Try needs a similar patch to set toolsdir. Working on it now.

Comment 18

[Chris Cooper [:coop] (he/him)]

(In reply to Chris Cooper [:coop] from comment #17)
> Try needs a similar patch to set toolsdir. Working on it now.

Actually, no. nthomas only reconfiged the build masters last night, so the try masters just need to be reconfiged as well.

Comment 19

[Chris Cooper [:coop] (he/him)]

(In reply to Chris Cooper [:coop] from comment #18)
> Actually, no. nthomas only reconfiged the build masters last night, so the
> try masters just need to be reconfiged as well.

Try masters have been reconfig-ed now.

Comment 20

[Nick Thomas [:nthomas] (UTC+12)]

Oops sorry.

(In reply to Mike Hommey [:glandium] from comment #16)
> how about moving hgtool out of valgrind.sh?

(from memory) that would require setting up a new factory or adding another special case. Why can't we just convert this to a mozharness script ?

Comment 21

[Ralph Giles (:rillian)]

I looked at this some more last week, since it's blocking enabling rust for linux64 integration builds. Without calling tooltool there's no way to make the rust toolchain available at all. But I ran out of time without producing anything useful.

valgrind.sh doesn't call tooltool, or tooltool_wrapper.sh. I tools like the script might already be present in the environment, in which case we can just add it now. The valgrind.sh script itself comes from https://hg.mozilla.org/build/tools.

https://mxr.mozilla.org/build/source/tools/scripts/valgrind/valgrind.sh

Comment 22

[(not currently active) Ted Mielczarek]

(In reply to Nick Thomas [:nthomas] from comment #20)
> (from memory) that would require setting up a new factory or adding another
> special case. Why can't we just convert this to a mozharness script ?

There's no reason we can't, it's a matter of someone finding the time to do the work.

Comment 23

[Chris AtLee [:catlee]]

great candidate for migration to TC

Comment 24

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

We can simply disable the rust code for valgrind until someone has time to fix valgrind. There is no value in blocking the non-valgrind builds on Linux.

Comment 25

[Mike Hommey [:glandium]]

(In reply to Anthony Jones (:kentuckyfriedtakahe, :k17e) from comment #24)
> We can simply disable the rust code for valgrind until someone has time to
> fix valgrind. There is no value in blocking the non-valgrind builds on Linux.

That's a comment for bug 1175359, not this one.

Comment 26

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

(In reply to Mike Hommey [:glandium] from comment #25)
> That's a comment for bug 1175359, not this one.

Sorry. My bad.

Comment 27

[Mike Hommey [:glandium]]

Attachment: Use an in-tree valgrind.sh script if present

Change in strategy, let's start with a simple change that makes valgrind builds pick an in-tree script if it exists instead of running the build. That script is run after the tree is cloned. That will allow faster iterations and, allow to test changes on try, without having to do drastic changes like switching to TC.

Comment 28

[Mike Hommey [:glandium]]

https://hg.mozilla.org/build/tools/rev/bfab0c78196c

Comment 29

[Mike Hommey [:glandium]]

Attachment: part 1 - Move relevant parts of valgrind.sh from https://hg.mozilla.org/build/tools in-tree

This copies the parts from valgrind.sh after the changes from attachment 8637716 [details] [diff] [review] to the right location to be used during builds.

Comment 30

[Mike Hommey [:glandium]]

Attachment: part 2 - Use tooltool in valgrind jobs

By not using tooltool, valgrind builds are still using gtk+2 at the moment, so we need to be explicit to keep them that way, because gtk+3 builds don't pass valgrind yet... which means they need not to grab gtk3 from tooltool, thus the new manifests. When switching to gtk+3, those manifests can go away.

Comment 31

[Mike Hommey [:glandium]]

Attachment: bonus - Remove unnecessary LD_LIBRARY_PATH in valgrind driver script

Comment 32

[Michael Shal [:mshal]]

Comment on attachment 8637853 [details] [diff] [review]
part 1 - Move relevant parts of valgrind.sh from https://hg.mozilla.org/build/tools in-tree

These all look fine to me.

Comment 33

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/bc6399f93e5e
https://hg.mozilla.org/integration/mozilla-inbound/rev/9d7d1f357b58
https://hg.mozilla.org/integration/mozilla-inbound/rev/416c25e59d87

Comment 34

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/11d7c6271926

Comment 35

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/bc6399f93e5e
https://hg.mozilla.org/mozilla-central/rev/9d7d1f357b58
https://hg.mozilla.org/mozilla-central/rev/416c25e59d87
https://hg.mozilla.org/mozilla-central/rev/11d7c6271926