If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Status

Testing
General
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: gwagner, Assigned: aus)

Tracking

(Blocks: 1 bug)

Version 3
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

Comment hidden (empty)
(Reporter)

Updated

2 years ago
Blocks: 1239808, 1245091
(Reporter)

Comment 1

2 years ago
Created attachment 8728343 [details] [diff] [review]
fixx.diff
(Reporter)

Comment 2

2 years ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=2ebede86e5f3&exclusion_profile=false&filter-tier=1&filter-tier=2&filter-tier=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
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
(Assignee)

Comment 5

2 years ago
(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.
Flags: needinfo?(hskupin)
(Assignee)

Updated

2 years ago
Assignee: nobody → aus
Maybe I was wrong. Chris should better know the answer to this.
Flags: needinfo?(hskupin) → needinfo?(cmanchester)
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.
Flags: needinfo?(cmanchester)
(Assignee)

Updated

2 years ago
Blocks: 1252482
(Assignee)

Comment 8

2 years ago
https://hg.mozilla.org/projects/pine/rev/6d747274271fde3dca53f33aa57dde147ce06591
Bug 1254604 - Use MOZ_SIMPLE_PACKAGE_NAME when building Mulet. r=chmanchester
(Reporter)

Comment 9

2 years ago
Created attachment 8728829 [details] [diff] [review]
patch
Attachment #8728343 - Attachment is obsolete: true
(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
Flags: needinfo?(aus)
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
(Assignee)

Comment 12

2 years ago
(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.
Flags: needinfo?(aus)
(Assignee)

Comment 13

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

Comment 14

2 years ago
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
Flags: needinfo?(hskupin)
Flags: needinfo?(cmanchester)
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.
Flags: needinfo?(cmanchester)
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
Flags: needinfo?(hskupin)
(Assignee)

Comment 17

2 years ago
https://hg.mozilla.org/projects/pine/rev/f330cc11e37d8794338bcacb2e6d84a87a3232c1
Bug 1254604 - Update build location to reflect use of MOZ_SIMPLE_PACKAGE_NAME.
(Assignee)

Comment 18

2 years ago
Created attachment 8730957 [details] [diff] [review]
Patch - v1 - Sort out build location issues
Attachment #8728829 - Attachment is obsolete: true
Attachment #8730957 - Flags: review?(cmanchester)
(Assignee)

Comment 19

2 years ago
Created attachment 8730960 [details] [diff] [review]
Patch - v1 - Sort out build location issues
Attachment #8730960 - Flags: review?(cmanchester)
(Assignee)

Comment 20

2 years ago
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.
Attachment #8730957 - Attachment is obsolete: true
Attachment #8730957 - Flags: review?(cmanchester)
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.
Attachment #8730960 - Flags: review?(cmanchester) → review+
(Assignee)

Comment 22

2 years ago
(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.
(Assignee)

Comment 23

2 years ago
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
(Assignee)

Updated

2 years ago
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
(Reporter)

Comment 24

2 years ago
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
Flags: needinfo?(aus)
(Assignee)

Comment 25

2 years ago
Clearing ni? as the second build was good.
Flags: needinfo?(aus)
You need to log in before you can comment on or make changes to this bug.