Closed Bug 965151 Opened 6 years ago Closed 4 years ago

Valgrind builds should build with tooltool gcc like other builds

Categories

(Release Engineering :: General, defect)

x86_64
Linux
defect
Not set

Tracking

(firefox42 fixed)

RESOLVED FIXED
Tracking Status
firefox42 --- fixed

People

(Reporter: jesup, Assigned: glandium)

References

Details

(Whiteboard: [kanban:engops:https://mozilla.kanbanize.com/ctrl_board/6/2690] )

Attachments

(4 files, 3 obsolete files)

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).
Summary: Valgrind builds on linux using gcc 4.7.3 instead of gcc from mozconfig → Valgrind builds should build with tooltool gcc like other builds
Depends on: 971586
Component: Build Config → General Automation
Product: Core → Release Engineering
QA Contact: catlee
Version: Trunk → unspecified
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.
Flags: needinfo?(catlee)
Enable patch for when we get the build slave side fixed.
Attachment #8497818 - Flags: review?(mh+mozilla)
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.
Attachment #8497818 - Flags: review?(mh+mozilla) → review+
Duplicate of this bug: 1090632
Could you also test this on staging?
Attachment #8513150 - Flags: review?(nthomas)
Assignee: nobody → mh+mozilla
Status: NEW → ASSIGNED
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 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.
Flags: needinfo?(catlee)
Attachment #8513150 - Flags: review?(nthomas) → review+
And of course it blows up because there's no toolsdir property.
Valgrind builds hidden on trunk trees until this gets redeployed with a fix or a backout.
Attachment #8514627 - Flags: review?(bugspam.Callek)
Attachment #8514627 - Flags: review?(bugspam.Callek) → review+
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.
Valgrind unhidden since things are passing.
how about moving hgtool out of valgrind.sh?
Flags: needinfo?(nthomas)
Try needs a similar patch to set toolsdir. Working on it now.
(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.
(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.
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 ?
Flags: needinfo?(nthomas)
Whiteboard: [kanban:engops:https://mozilla.kanbanize.com/ctrl_board/6/2683]
Whiteboard: [kanban:engops:https://mozilla.kanbanize.com/ctrl_board/6/2683] → [kanban:engops:https://mozilla.kanbanize.com/ctrl_board/6/2690]
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
No longer blocks: 1175359
Flags: needinfo?(ajones)
(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.
great candidate for migration to TC
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.
(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.
(In reply to Mike Hommey [:glandium] from comment #25)
> That's a comment for bug 1175359, not this one.

Sorry. My bad.
Flags: needinfo?(ajones)
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.
Attachment #8637716 - Flags: review?(jlund)
Attachment #8637716 - Flags: review?(jlund) → review+
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.
Attachment #8497818 - Attachment is obsolete: true
Attachment #8513150 - Attachment is obsolete: true
Attachment #8514627 - Attachment is obsolete: true
Attachment #8637853 - Flags: review?
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.
Attachment #8637855 - Flags: review?(mshal)
Attachment #8637853 - Flags: review? → review?(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.
Attachment #8637853 - Flags: review?(mshal) → review+
Attachment #8637855 - Flags: review?(mshal) → review+
Attachment #8637856 - Flags: review?(mshal) → review+
Component: General Automation → General
You need to log in before you can comment on or make changes to this bug.