1254604 - Followup for bug 1239808

Status: RESOLVED FIXED

(Testing :: General, defect)

Comment 1

[Gregor Wagner [:gwagner]]

Attachment: fixx.diff

Comment 2

[Gregor Wagner [:gwagner]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=2ebede86e5f3&exclusion_profile=false&filter-tier=1&filter-tier=2&filter-tier=3

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+2]]

I think the proposed fix is a hack. It would be better to already let the build system produce the correct prefix for the test packages. IMHO it should be doable by updating the build config for activating MOZ_SIMPLE_PACKAGE_NAME:

https://dxr.mozilla.org/mozilla-central/source/toolkit/mozapps/installer/package-name.mk#57

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+2]]

So you might have to add the following lines to the mulet and other build scripts:
https://dxr.mozilla.org/mozilla-central/source/testing/taskcluster/scripts/builder/build-linux.sh#36

Comment 5

[Aus Lacroix [:aus]]

(In reply to Henrik Skupin (:whimboo) from comment #3)
> I think the proposed fix is a hack. It would be better to already let the
> build system produce the correct prefix for the test packages. IMHO it
> should be doable by updating the build config for activating
> MOZ_SIMPLE_PACKAGE_NAME:
> 
> https://dxr.mozilla.org/mozilla-central/source/toolkit/mozapps/installer/
> package-name.mk#57

This makes no sense to me. The JSON file will still be filled with incorrect data. Is it our goal to deprecate the JSON file soon? Otherwise I don't think this fix is a hack, unless there is a better place to REWRITE the data or ensure the data in the file is output correctly.

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Maybe I was wrong. Chris should better know the answer to this.

Comment 7

[Chris Manchester (limited bugmail, email directly)]

Henrik's suggestion looks reasonable, setting MOZ_SIMPLE_PACKAGE_NAME will impact the contents of test_packages.json, but I can't tell from this bug which jobs we're talking about.

Comment 8

[Aus Lacroix [:aus]]

https://hg.mozilla.org/projects/pine/rev/6d747274271fde3dca53f33aa57dde147ce06591
Bug 1254604 - Use MOZ_SIMPLE_PACKAGE_NAME when building Mulet. r=chmanchester

Comment 9

[Gregor Wagner [:gwagner]]

Attachment: patch

Comment 10

[:gerard-majax]

(In reply to Aus Lacroix [:aus] from comment #8)
> https://hg.mozilla.org/projects/pine/rev/
> 6d747274271fde3dca53f33aa57dde147ce06591
> Bug 1254604 - Use MOZ_SIMPLE_PACKAGE_NAME when building Mulet. r=chmanchester

That was not working at all when I tried pushing pine to try :/ https://treeherder.mozilla.org/#/jobs?repo=try&revision=916082500a8d&filter-tier=1&filter-tier=2&filter-tier=3&selectedJob=17876993

Comment 11

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Well, read the build log. As shown you will have to update/remove those lines too:

https://dxr.mozilla.org/mozilla-central/source/testing/taskcluster/scripts/builder/build-mulet-linux.sh#31

Comment 12

[Aus Lacroix [:aus]]

(In reply to Henrik Skupin (:whimboo) from comment #11)
> Well, read the build log. As shown you will have to update/remove those
> lines too:
> 
> https://dxr.mozilla.org/mozilla-central/source/testing/taskcluster/scripts/
> builder/build-mulet-linux.sh#31

That will just ensure that the artifacts we want to save don't get moved into artifacts. Which is really what we want. So no, removing that line won't help. What I need to remove is the line above that deletes everything that was just built.

Comment 13

[Aus Lacroix [:aus]]

https://hg.mozilla.org/projects/pine/rev/87cbf278e3bb6aec47e1c64e957adc3257b24811
Bug 1254604 - Use MOZ_SIMPLE_PACKAGE_NAME. Don't delete build targets! r=chmanchester

Comment 14

[Aus Lacroix [:aus]]

Using MOZ_SIMPLE_PACKAGE_NAME does _NOT_ affect the contents of the JSON file.

23:32:54     INFO - Downloading https://queue.taskcluster.net/v1/task/byiZghP1RTOe2zdD_z_Z8Q/artifacts/public/build/targ
et.linux-x86_64.tar.bz2 to /home/worker/workspace/build/target.linux-x86_64.tar.bz2                                     
23:32:54     INFO - retry: Calling _download_file with args: (), kwargs: {'url': 'https://queue.taskcluster.net/v1/task/
byiZghP1RTOe2zdD_z_Z8Q/artifacts/public/build/target.linux-x86_64.tar.bz2', 'file_name': '/home/worker/workspace/build/t
arget.linux-x86_64.tar.bz2'}, attempt #1                                                                                

https://treeherder.mozilla.org/#/jobs?repo=pine&revision=e9e94e7cb6ad&filter-tier=1&filter-tier=2&filter-tier=3

Comment 15

[Chris Manchester (limited bugmail, email directly)]

Then I'm not sure which JSON file we're referring to. That quoted log is referring to a packaged build, I thought this was an issue with test packages.

Comment 16

[Henrik Skupin [:whimboo][⌚️UTC+2]]

So we are talking about this task:
https://tools.taskcluster.net/task-inspector/#byiZghP1RTOe2zdD_z_Z8Q/0

The produced artifacts are those:

public/build/target.mochitest.tests.zip
public/build/target.mozinfo.json
public/build/target.reftest.tests.zip
public/build/target.talos.tests.zip
public/build/target.tar.bz2
public/build/target.test_packages.json

Checking test_packages.json:
https://public-artifacts.taskcluster.net/byiZghP1RTOe2zdD_z_Z8Q/0/public/build/target.test_packages.json

I see:

{
    "mochitest": [
        "target.common.tests.zip", 
        "target.mochitest.tests.zip"
    ], 
    "reftest": [
        "target.common.tests.zip", 
        "target.reftest.tests.zip"
    ], 
[..]

So everything is getting created correctly.

The problem you are referring to now is not the test package but the installer URL.

The log file of a test task gets a command like:

exec sudo -E -u worker bash /home/worker/bin/test.sh --application=firefox --installer-url=https://queue.taskcluster.net/v1/task/byiZghP1RTOe2zdD_z_Z8Q/artifacts/public/build/target.linux-x86_64.tar.bz2 --test-packages-url=https://queue.taskcluster.net/v1/task/byiZghP1RTOe2zdD_z_Z8Q/artifacts/public/build/target.test_packages.json

That means the wrong installer URL is getting passed into those jobs. So I come back to my comment 11 where disagreed on my proposal:

https://dxr.mozilla.org/mozilla-central/source/testing/taskcluster/scripts/builder/build-mulet-linux.sh#31

Those two lines are still wrong. So a fix like the following should do it:

mv *.linux-x86_64.tar.bz2       $HOME/artifacts/target.tar.bz2
mv *.linux-x86_64.json          $HOME/artifacts/target.json

Comment 17

[Aus Lacroix [:aus]]

https://hg.mozilla.org/projects/pine/rev/f330cc11e37d8794338bcacb2e6d84a87a3232c1
Bug 1254604 - Update build location to reflect use of MOZ_SIMPLE_PACKAGE_NAME.

Comment 18

[Aus Lacroix [:aus]]

Attachment: Patch - v1 - Sort out build location issues

Comment 19

[Aus Lacroix [:aus]]

Attachment: Patch - v1 - Sort out build location issues

Comment 20

[Aus Lacroix [:aus]]

Comment on attachment 8730957 [details] [diff] [review]
Patch - v1 - Sort out build location issues

Had a revision with unrelated changes, provided new patch without these changes.

Comment 21

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8730960 [details] [diff] [review]
Patch - v1 - Sort out build location issues

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

::: testing/mozharness/scripts/gaia_integration.py
@@ -60,5 @@
>          ]
>  
> -        # for Mulet
> -        if 'firefox' in self.binary_path:
> -            cmd += ['RUNTIME=%s' % self.binary_path]

I'm not sure how this change fits in with the rest of the patch.

Comment 22

[Aus Lacroix [:aus]]

(In reply to Chris Manchester (:chmanchester) from comment #21)
> Comment on attachment 8730960 [details] [diff] [review]
> Patch - v1 - Sort out build location issues
> 
> Review of attachment 8730960 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: testing/mozharness/scripts/gaia_integration.py
> @@ -60,5 @@
> >          ]
> >  
> > -        # for Mulet
> > -        if 'firefox' in self.binary_path:
> > -            cmd += ['RUNTIME=%s' % self.binary_path]
> 
> I'm not sure how this change fits in with the rest of the patch.

Not strictly necessary for this fix, I'll take it out before merging to b2g-i.

Comment 23

[Aus Lacroix [:aus]]

https://hg.mozilla.org/integration/b2g-inbound/rev/41c382b7adf3f87ab674f91355a5c9b6273a10b0
Bug 1254604 - Use MOZ_SIMPLE_PACKAGE_NAME when building Mulet. Update build location in taskcluster task. r=chmanchester

Comment 24

[Gregor Wagner [:gwagner]]

Hm mulet turned orange when I merged b2g-inbound with mozilla-central. Do we have to back out this patch on m-c?

https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&revision=d9f50aa0a1aaf90499b85c31e0f329b762e80fdd

Comment 25

[Aus Lacroix [:aus]]

Clearing ni? as the second build was good.