Closed Bug 1417421 Opened 7 years ago Closed 6 years ago

Removing sccache toolchain entries doesn't disable sccache on win32

Categories

(Firefox Build System :: Task Configuration, task)

task
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: rillian, Unassigned)

Details

I was trying to do a try build without sccache investigating bug 1417268. On the docker side we have a task transform which attaches a 'needs-sccache' attribute if a task has a toolchain entry ending in 'sccache'. Then we set USE_SCCACHE or SCCACHE_DISABLE based on this attribute. This appears not to work for windows builds, however. CCACHE still ends up defined and without SCCACHE_DISABLE configure expects to it to be present.
Assignee: nobody → wcosta
Assignee: wcosta → pmoore
Hi Ralph, Is this a bug in the task generation, i.e. inside https://hg.mozilla.org/mozilla-central/file/default/taskcluster that means the wrong env variables are set in the task definition? Or is this bug that the task definition is correct (env vars, commands, etc) but the worker does not behave as expected? If it is the first option above, that should probably be filed in gecko, rather than TaskCluster, as a task generation bug. Thanks!
Flags: needinfo?(giles)
Flags: needinfo?(giles)
Component: Generic-Worker → Integration
Assignee: pmoore → nobody
(In reply to Mike Hommey [:glandium] from comment #2) > The problem is that this setup code: > https://dxr.mozilla.org/mozilla-central/rev/ > 739484451a6399c7f156a0d960335606aa6c1221/taskcluster/taskgraph/transforms/ > task.py#794-803 > > is in a docker-worker-only section. Thanks Mike! Looking at this more closely, I think the docker-worker implementation here isn't ideal either. If I'm reading this correctly, this code will operate on all tasks generated by the decision task, which means all tasks will have either SCCACHE_DISABLE or USE_SCCACHE env vars set, even if the task is not a build and does not care about sccache (e.g. tests, l10n, ...). Probably it would be better to clean the docker-worker implementation up at the same time as implementing for generic-worker, and make sure only tasks that care about these two environment variables (SCCACHE_DISABLE, USE_SCCACHE) actually have them set in the task payload, and maybe this should even be in an independent function which can be called by both docker-worker and generic-worker payload builders, for those tasks that care about them.
... and maybe the best way to achieve that, would be for the setting of those env vars to be put in a transform of its own (under /taskcluster/taskgraph/transforms), which can be added to the various kind.yml files under subdirectories of /taskcluster/ci. As this is a little outside my area of expertise, needinfo'ing dustin to see if he agrees with this analysis! :-)
Flags: needinfo?(dustin)
(the transform would *something* look like this: > @transforms.add > def enable_disable_sccache(config, tasks): > for task in tasks: > if task.get('needs-sccache'): > task['worker']['env']['USE_SCCACHE'] = '1' > else: > task['worker']['env']['SCCACHE_DISABLE'] = '1' > yield task )
The intent of `task['needs-sccache']` in a task description is that other transforms can indicate "this task needs the setup for sccache" without getting into the worker-specific details of what that means. For some worker implementations, sccache is not available and causes an exception. There's a little wrinkle in that, on windows, sccache is handled at the mozharness level and doesn't affect anything worker-specific (no caches, proxies, scopes, etc.). I think the original idea was to only modify the task if needs-sccache was true. Where needs-sccache is false, the resulting task would not refer to SCCACHE at all. But the SCCACHE_DISABLE env var means there are now *three* possible dispositions for a task with respect to sccache: use it, ignore it, or disable it. So, on the reasoning that it's best to model the situation as it stands, perhaps `needs-sccache` should become a three-value flag: `Any(None, False, True)`, with None (the default) meaning "don't mention SCCACHE at all", False meaning SCCACHE_DISABLE=1, and True meaning USE_SCCACHE=1 plus any necessary scopes, proxies, etc. With that done, only the build tasks would set need-sccache -- it would be left at None for the rest.
Flags: needinfo?(dustin)
The USE_SCCACHE/SCCACHE_DISABLE dance was because of how things evolved from buildbot to tc, and because now some things (well, exactly one) opt-in to sccache, while firefox builds opt-out of it. We can surely change the firefox builds to opt-in instead.
Component: Integration → Task Configuration
Product: Taskcluster → Firefox Build System

This was fixed in bug 1439258.

Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.