Closed Bug 1507094 Opened 2 years ago Closed 2 years ago

Use tarballs for gtest instead of zip files

Categories

(Firefox Build System :: General, enhancement)

enhancement
Not set
normal

Tracking

(firefox65 fixed)

RESOLVED FIXED
mozilla65
Tracking Status
firefox65 --- fixed

People

(Reporter: marco, Assigned: decoder)

References

Details

Attachments

(1 file)

This would be a longer term solution for bugs such as bug 1484073 and bug 1504489.

:gps, could you help with this?
Flags: needinfo?(gps)
I don't have the bandwidth for this at this time.

However, the change /might/ be trivial!

testing/testsuite-targets.mk has TEST_PKGS_TARGZ and TEST_PKGS_ZIP variables. One simply needs to move "gtest" to TEST_PKGS_TARGZ to produce a tar.gz.

What I'm not sure of is what all is referencing the .zip extension. Bug 733530 may shed some light: it converted every archive except for gtest. Looking at https://hg.mozilla.org/mozilla-central/rev/6f2020891542fbc0010d555812a289038095a4ef may give some clues as to what files to look in.

Also, once gtest is not a zip file, TEST_PKGS_ZIP will be unused and can be deleted from testsuite-targets.mk. We should also be able to remove zip support from python/mozbuild/mozbuild/action/test_archive.py and possibly other downstream code as well. That will be a glorious code cleanup!
Depends on: 733530
Flags: needinfo?(gps)
(In reply to Gregory Szorc [:gps] from comment #1)

> 
> testing/testsuite-targets.mk has TEST_PKGS_TARGZ and TEST_PKGS_ZIP
> variables. One simply needs to move "gtest" to TEST_PKGS_TARGZ to produce a
> tar.gz.

I did exactly that and made a try push, all green (and had I read your comment first, I wouldn't have spent 5 minutes to find this spot in the tree ;)).

> 
> What I'm not sure of is what all is referencing the .zip extension. Bug
> 733530 may shed some light: it converted every archive except for gtest.
> Looking at
> https://hg.mozilla.org/mozilla-central/rev/
> 6f2020891542fbc0010d555812a289038095a4ef may give some clues as to what
> files to look in.

I did an extensive grep in some of these directories and found absolutely zero references to the gtest zip file. What I did find though was a reference to the gtest tar.gz file (?) which we don't have yet.

It is here: https://searchfox.org/mozilla-central/rev/647b9eb1099e373e079e16f147f74f3d1d65eed3/toolkit/mozapps/installer/package-name.mk#104

And that was actually changed by the patch you referenced, maybe in error :) So I assume this should work out just fine.


Chris, do you know about any other potential references of the gtest zip file outside of the tree? Like in release engineering automation or something like that?
Flags: needinfo?(catlee)
I really doubt that anything outside of the tree is using the gtest archive. The test zips are produced solely for the benefit of CI.
(In reply to Gregory Szorc [:gps] from comment #1)

> We should also be able to remove zip
> support from python/mozbuild/mozbuild/action/test_archive.py and possibly
> other downstream code as well. That will be a glorious code cleanup!

Unfortunately that is not the case :/ I did some try runs and it looks like other code (e.g. mozharness packaging) still depends on the zip support. So I left it in and only removed the TEST_PKGS_ZIP logic from the Makefile. This is green on try and according to comment 3 there shouldn't be many external consumers of this.
Flags: needinfo?(catlee)
triaging, assigning to :decoder since he attached patches
Assignee: nobody → choller
(In reply to Christian Holler (:decoder) from comment #2)
> Chris, do you know about any other potential references of the gtest zip
> file outside of the tree? Like in release engineering automation or
> something like that?

Not off the top of my head. Maybe mozregression?
Pushed by gszorc@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1d500eb59823
Use tar.gz for gtest archive instead of zip. r=gps
https://hg.mozilla.org/mozilla-central/rev/1d500eb59823
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
You need to log in before you can comment on or make changes to this bug.