Closed Bug 1105439 Opened 5 years ago Closed 5 years ago

Decision tasks for gecko

Categories

(Taskcluster :: General, defect)

x86
macOS
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: jlal, Assigned: jlal)

References

Details

Attachments

(8 files, 1 obsolete file)

39 bytes, text/x-review-board-request
garndt
: review+
wcosta
: review+
Details
39 bytes, text/x-review-board-request
garndt
: review+
wcosta
: review+
Details
39 bytes, text/x-review-board-request
garndt
: review+
wcosta
: review+
Details
39 bytes, text/x-review-board-request
garndt
: review+
wcosta
: review+
Details
39 bytes, text/x-review-board-request
garndt
: review+
wcosta
: review+
Details
39 bytes, text/x-review-board-request
garndt
: review+
wcosta
: review+
Details
39 bytes, text/x-review-board-request
garndt
: review+
wcosta
: review+
Details
39 bytes, text/x-review-board-request
garndt
: review+
wcosta
: review+
Details
Decision task logic for gecko deals with pulling initial repository and running the initial commit logic.
Blocks: 1080265
Attachment #8531105 - Flags: review?(wcosta)
Attachment #8531105 - Flags: review?(garndt)
/r/1093 - Bug 1105439 - Part 1 New gaia_props.py util for getting gaia version needed by the gecko r=garndt, wcosta
/r/1095 - Bug 1105439 - Part 2 Update builder to use tc-vcs for b2g-desktop/mulet (and related repos) r=wcosta, garndt
/r/1097 - Bug 1105439 -Part 3  Initial decision task logic for gecko r=wcosta, garndt
/r/1099 - Bug 1105439 - Part 4 Framework for inheriting one task from another. r=wcosta, garndt
/r/1101 - Bug 1105439 - Part 5 Use inheritance instead of additional-properties r=wcosta, garndt
/r/1103 - Bug 1105439 - Part 7 Update scopes and debug properties for emulator builds r=wcosta, garndt
/r/1105 - Bug 1105439 - First pass at speeding up emulators r=wcosta, garndt
/r/1107 - Bug 1105439 - Finalize decision task and simplify emulator caching logic r=wcosta, garndt

Pull down these commits:

hg pull review -r 63a74503ea583e7fdd3120f133448b5c27ea61ad
Note this is based off alder _not_ mozilla-central or b2g-inbound.
https://reviewboard.mozilla.org/r/1091/#review653

::: testing/docker/builder/Dockerfile
(Diff revision 1)
> +ADD bin /home/worker/bin

Any particular reason why we don't use a setup script anymore?

::: testing/docker/builder/git.env
(Diff revision 1)
> +GECKO_HEAD_REPOSITORY=https://github.com/mozilla/gecko-dev

Do we use github ot git.mozilla.org for builds?

::: testing/taskcluster/taskcluster_graph/templates.py
(Diff revision 1)
> +    for key in source:

I think it is better to do:

```
for key, value in source.items():
```

Since source[key] is done a lot inside the for loop.
Comment on attachment 8531105 [details]
MozReview Request: bz://1105439/lightsofapollo

lgtm, just some minor comments
Attachment #8531105 - Flags: review?(wcosta) → review+
https://reviewboard.mozilla.org/r/1091/#review655

::: testing/taskcluster/tests/test_templates.py
(Diff revision 1)
> +

nit: extra line

::: testing/taskcluster/mach_commands.py
(Diff revision 1)
> +class InheritTryme(object):

What is this used for?

::: testing/taskcluster/mach_commands.py
(Diff revision 1)
> +        help='URL for "base" repository to clone')

help text is the same as --base-repository.  Won't this be the repostiroty where the commit is?

::: testing/taskcluster/mach_commands.py
(Diff revision 1)
>      @CommandArgument('--owner',

Should owner be marked as required here as well since it was changed up above?

::: testing/taskcluster/mach_commands.py
(Diff revision 1)
> -    @CommandArgument('--b2g-config',
> +        help='URL for "base" repository to clone')

Same comment about help text above.
https://reviewboard.mozilla.org/r/1091/#review657

::: testing/docker/builder/git.env
(Diff revision 1)
> +GECKO_BASE_REPOSITORY=https://github.com/mozilla/gecko-dev

Where are these env files used or are they just there for defaults if someone wants to setup their environment variables prior to running the mach commands?
https://reviewboard.mozilla.org/r/1091/#review659

::: testing/taskcluster/entrypoints/try.yml
(Diff revision 1)
> +        image: ubuntu:13.10

This is based off of ubuntu image that does not have the entrypoint scripts and gecko like the decision docker image that's built elsewhere.

Also noticed that this is doing something similar to taskcluster/tasks/decision/try.yml, which uses decision:0.0.3

Not sure if both should exist or one should have been removed.

::: testing/taskcluster/tasks/decision/try.yml
(Diff revision 1)
> +    - 'docker-worker:cache:tc-vcs-public-sources'

Do we need to cache gecko?  Entrypoint looks to see if it exists and if not, will do a clone.  Or will we just rely on it pulling the s3 archive each time?
Comment on attachment 8531105 [details]
MozReview Request: bz://1105439/lightsofapollo

r+ with addressing the issues raised in reviewboard, specifically about multiple try.yml definitions and prototype code that should be removed.
Attachment #8531105 - Flags: review?(garndt) → review+
Blocks: 1085631
(In reply to Wander Lairson Costa [:wcosta] from comment #4)
> https://reviewboard.mozilla.org/r/1091/#review653
> 
> ::: testing/docker/builder/Dockerfile
> (Diff revision 1)
> > +ADD bin /home/worker/bin
> 
> Any particular reason why we don't use a setup script anymore?

Putting the commands directly in the Dockerfile greatly increases the chance you can hit the cache which docker uses which means _much_ faster docker build times.

> 
> ::: testing/docker/builder/git.env
> (Diff revision 1)
> > +GECKO_HEAD_REPOSITORY=https://github.com/mozilla/gecko-dev
> 
> Do we use github ot git.mozilla.org for builds?

We do this file is just for testing the docker image file as needed.

> 
> ::: testing/taskcluster/taskcluster_graph/templates.py
> (Diff revision 1)
> > +    for key in source:
> 
> I think it is better to do:
> 
> ```
> for key, value in source.items():
> ```
> 
> Since source[key] is done a lot inside the for loop.

Done
https://reviewboard.mozilla.org/r/1091/#review673

> This is based off of ubuntu image that does not have the entrypoint scripts and gecko like the decision docker image that's built elsewhere.
> 
> Also noticed that this is doing something similar to taskcluster/tasks/decision/try.yml, which uses decision:0.0.3
> 
> Not sure if both should exist or one should have been removed.

Removed this was a bad idea !

> Do we need to cache gecko?  Entrypoint looks to see if it exists and if not, will do a clone.  Or will we just rely on it pulling the s3 archive each time?

We now cache the underlying "sources" (tarballs of the clones) in all images so this will clone the revision each time without caching gecko (somewhat intentionally) so decision tasks always are in as clean of state as possible.
https://reviewboard.mozilla.org/r/1091/#review671

> help text is the same as --base-repository.  Won't this be the repostiroty where the commit is?

Actually "base" revision is what we but the "head" is where the revision is (this is taken from how github describes this type of workflow not sure how right the names are otherwise)
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Attachment #8531105 - Attachment is obsolete: true
Attachment #8618733 - Flags: review+
Attachment #8618734 - Flags: review+
Attachment #8618735 - Flags: review+
Attachment #8618736 - Flags: review+
Attachment #8618737 - Flags: review+
Attachment #8618738 - Flags: review+
Attachment #8618739 - Flags: review+
Attachment #8618740 - Flags: review+
Component: TaskCluster → General
Product: Testing → Taskcluster
Target Milestone: --- → mozilla41
Version: unspecified → Trunk
Resetting Version and Target Milestone that accidentally got changed...
Target Milestone: mozilla41 → ---
Version: Trunk → unspecified
You need to log in before you can comment on or make changes to this bug.