Closed Bug 1338099 Opened 8 years ago Closed 5 years ago

Investigate why automation is producing an empty "sourcestamp.txt" file

Categories

(Release Engineering :: Release Automation: Other, defect, P1)

Product:
Release Engineering
Component:
Release Automation: Other
Type:
defect

Tracking

(firefox51 wontfix, firefox52 fixed, firefox-esr52 fixed, firefox-esr68 fixed, firefox53 fixed, firefox54 fixed, firefox69 fixed)

Status:
RESOLVED FIXED
Tracking Flags:
Tracking Status
firefox51 --- wontfix
firefox52 --- fixed
firefox-esr52 --- fixed
firefox-esr68 --- fixed
firefox53 --- fixed
firefox54 --- fixed
firefox69 --- fixed

People

(Reporter: Dexter, Assigned: mshal)

References

Details

(Whiteboard: [measurement:client:tracking])

Attachments

(4 files, 4 obsolete files)

bug1338099.patch
(deleted), patch
Dexter
: review+
gchang
: approval-mozilla-aurora+
jcristau
: approval-mozilla-beta+
Details | Diff | Splinter Review
proposed patch (v2)
(deleted), patch
ted
: review+
Details | Diff | Splinter Review
Bug 1338099 - Ensure sourcestamp.txt has a buildid; r?#firefox-build-system-reviewers
(deleted), text/x-phabricator-request
RyanVM
: approval-mozilla-esr68+
Details
Bug 1338099 - Export MOZILLA_OFFICIAL in the source build for sourcestamp.txt; r?#firefox-build-system-reviewers
(deleted), text/x-phabricator-request
RyanVM
: approval-mozilla-esr68+
Details
In bug 1282782 we added a sourcestamp.txt file to the source tarballs generated along with out releases. However, it seems that the generated file is empty. From a quick look to logs of the source generation task [1], this error stands out: > 21:32:36 INFO - make make-sourcestamp-file > 21:32:36 INFO - Traceback (most recent call last): > 21:32:36 INFO - File "/home/worker/workspace/build/src/config/printconfigsetting.py", line 16, in <module> > 21:32:36 INFO - with open(file) as fh: > 21:32:36 INFO - IOError: [Errno 2] No such file or directory: '../../dist/bin/platform.ini' [1] - https://tools.taskcluster.net/task-group-inspector/#/RzFfClH5SHm4O5dTctODMw/KEuwUwa2SSa7faFf12NBnQ?_k=tt7cgx
Whiteboard: [measurement:client:tracking]
Ted, it looks like the changes from bug 1282782 don't work on a clobbered tree. It looks like the machines that package the source code just issue: > ./mach configure and don't do a ./mach build before calling ./mach build source-package. I was able to reproduce the problem locally: > /mozilla-central$ ./mach build source-package > 0:00.12 /usr/bin/make -C /home/dexter/mozilla-central/objdir-ff-debug -j4 -s backend > 0:00.15 /usr/bin/make -j4 -s source-package > 0:00.19 Generate the sourcestamp file > 0:00.21 source-repo.h > 0:00.62 Traceback (most recent call last): > 0:00.62 File "/home/dexter/mozilla-central/config/printconfigsetting.py", line 16, in <module> > 0:00.62 with open(file) as fh: > 0:00.62 IOError: [Errno 2] No such file or directory: '../../dist/bin/platform.ini' > 0:00.62 make[2]: ../../config/nsinstall: Command not found > 0:00.62 /home/dexter/mozilla-central/toolkit/mozapps/installer/packager.mk:103: recipe for target 'make-sourcestamp-file' failed > 0:00.62 make[2]: *** [make-sourcestamp-file] Error 127 > 0:00.62 /home/dexter/mozilla-central/toolkit/mozapps/installer/packager.mk:162: recipe for target 'source-package' failed > 0:00.62 make[1]: *** [source-package] Error 2 > 0:00.62 /home/dexter/mozilla-central/browser/build.mk:27: recipe for target 'source-package' failed > 0:00.62 make: *** [source-package] Error 2 Calling "./mach build" before packaging the source makes it work. However, I noticed one thing: if I quickly kill the build process right after starting it (1 second or 2) and try to package the source after that, the process doesn't fail and the sourcestamp.txt file gets correctly generated (at least locally): > 0:00.11 /usr/bin/make -C /home/dexter/mozilla-central/objdir-ff-debug -j4 -s backend > 0:00.15 /usr/bin/make -j4 -s source-package > 0:00.18 Generate the sourcestamp file > 0:00.20 source-repo.h > 0:00.59 Traceback (most recent call last): > 0:00.59 File "/home/dexter/mozilla-central/config/printconfigsetting.py", line 16, in <module> > 0:00.59 with open(file) as fh: > 0:00.59 IOError: [Errno 2] No such file or directory: '../../dist/bin/platform.ini' > 0:00.60 Packaging source tarball... This produces, at least locally, a sourcestamp.txt file without the build ID, but with the source revision. @Ted, a few questions for you (given your experience with the build tools, but feel free to tell me who to ping if it's not you!): 1) How bad would be to do a "./mach build" for packaging the source-code? IMHO, it seems like a waste of resources, considering the time it takes to complete. 2) Is there a way to just do the build steps that we need in order to get the BuildId and the MOZ_SOURCE_URL? [1] 2a) What are the required steps there? :) [1] - http://searchfox.org/mozilla-central/rev/d3307f19d5dac31d7d36fc206b00b686de82eee4/toolkit/mozapps/installer/packager.mk#103
Flags: needinfo?(ted)
(In reply to Alessio Placitelli [:Dexter] from comment #1) > 1) How bad would be to do a "./mach build" for packaging the source-code? > IMHO, it seems like a waste of resources, considering the time it takes to > complete. Yeah, let's not do that. > 2) Is there a way to just do the build steps that we need in order to get > the BuildId and the MOZ_SOURCE_URL? [1] > 2a) What are the required steps there? :) So we were somewhat aware of this when you wrote your patch, as you made the `source-package` target run make to generate source-repo.h, but apparently we missed a spot. It took me a little while to figure out what's going on here, to be honest! The `make-sourcestamp-file` target has this innocuous-looking rule: https://dxr.mozilla.org/mozilla-central/rev/25a94c1047e793ef096d8556fa3c26dd72bd37d7/toolkit/mozapps/installer/packager.mk#104 ...but it turns out that $(BUILDID) is defined as a $(shell) command here: https://dxr.mozilla.org/mozilla-central/rev/25a94c1047e793ef096d8556fa3c26dd72bd37d7/toolkit/mozapps/installer/package-name.mk#152 So that's trying to print a value out of application.ini or platform.ini, and those don't get generated unless we build in toolkit/xre: https://dxr.mozilla.org/mozilla-central/rev/25a94c1047e793ef096d8556fa3c26dd72bd37d7/toolkit/xre/moz.build#210 That's pretty silly. So there are two things you could do to fix this: 1) Stick a `$(MAKE) -C $(DEPTH)/toolkit/xre/ misc` line before `$(MAKE) make-sourcestamp-file`, to generate platform.ini so that $(BUILDID) definition works. 2) Change the BUILDID definition to instead be like `$(shell awk '$$2 == "MOZ_BUILDID" {print $$3}' buildid.h)` to fetch the value directly from buildid.h instead of in the roundabout way from application.ini/platform.ini. I guess ideally we'd just write the build ID to a plain file instead of only a header so we could just make that $(shell cat $(DEPTH)/buildid) or something simple like that, but that might be slightly more work. Either way that should unstick this. Sorry for not catching that in the first place, it's sort of a ridiculous situation.
Flags: needinfo?(ted)
Thanks and don't worry for the review, Ted! Unfortunately both options fail locally, at least for me. The first, complains with > 0:00.49 Makefile:21: *** Variable MOZ_BUILDID does not contain a value. Stop. And the second with > 0:00.60 awk: cannot open buildid.h (No such file or directory) Sorry for the trouble :(
Flags: needinfo?(ted)
Oh, right, for #2 that needs to be $(DEPTH)/buildid.h. I don't know what's going wrong in #1, but #2 seems like a simple fix.
Flags: needinfo?(ted)
Attached patch test_sourcestamp.patch (obsolete) (deleted) — — Splinter Review
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #4) > Oh, right, for #2 that needs to be $(DEPTH)/buildid.h. I don't know what's > going wrong in #1, but #2 seems like a simple fix. So, I tried that, but it's still not working. Moreover, if I don't remove the $(NSINSTALL) line, it still would fail with the infamous error: > File "/home/dexter/mozilla-central/config/printconfigsetting.py", line 16, in <module> > 0:00.57 with open(file) as fh: > 0:00.57 IOError: [Errno 2] No such file or directory: '../../dist/bin/platform.ini' While, removing the line (as in the attached patch) and changing the buildid line to $(DEPTH)/buildid.h, fails in this different way. > ./mach build source-package > 0:00.22 /usr/bin/make -C /home/dexter/mozilla-central/objdir-ff-debug -j4 -s backend > 0:00.29 /usr/bin/make -j4 -s source-package > 0:00.31 Generate the sourcestamp file > 0:00.33 source-repo.h > 0:00.76 awk: cannot open ../../buildid.h (No such file or directory) > 0:00.76 /home/dexter/mozilla-central/toolkit/mozapps/installer/packager.mk:103: recipe for target 'make-sourcestamp-file' failed > 0:00.76 make[2]: *** [make-sourcestamp-file] Error 2 > 0:00.76 /home/dexter/mozilla-central/toolkit/mozapps/installer/packager.mk:160: recipe for target 'source-package' failed > 0:00.76 make[1]: *** [source-package] Error 2 > 0:00.76 /home/dexter/mozilla-central/browser/build.mk:27: recipe for target 'source-package' failed > 0:00.76 make: *** [source-package] Error 2 Sorry to keep bothering you Ted, but I have no clue on how to fix this properly :-/
Flags: needinfo?(ted)
(In reply to Alessio Placitelli [:Dexter] from comment #6) > (In reply to Ted Mielczarek [:ted.mielczarek] from comment #4) > > Oh, right, for #2 that needs to be $(DEPTH)/buildid.h. I don't know what's > > going wrong in #1, but #2 seems like a simple fix. > > So, I tried that, but it's still not working. Moreover, if I don't remove > the $(NSINSTALL) line, it still would fail with the infamous error: I'm really not sure what's happening here. That error should only be coming from the original definition of $(BUILDID) where it invokes printconfigsetting.py. > While, removing the line (as in the attached patch) and changing the buildid > line to $(DEPTH)/buildid.h, fails in this different way. Ahh! We're not actually making buildid.h anywhere in the source-package rule! Change this line: http://searchfox.org/mozilla-central/rev/d3307f19d5dac31d7d36fc206b00b686de82eee4/toolkit/mozapps/installer/packager.mk#164 to be $(MAKE) -C $(DEPTH) source-repo.h buildid.h I think that ought to fix that bit.
Flags: needinfo?(ted)
Attached patch bug1338099.patch (obsolete) (deleted) — — Splinter Review
Thanks Ted! This works perfectly (on my system :-D). That's how I tested it, from my HG repo: > ./mach clobber > ./mach configure > ./mach build source-package The output of the last step is: > 0:00.14 /usr/bin/make -C /home/dexter/mozilla-central/objdir-ff-debug -j4 -s backend > 0:00.21 /usr/bin/make -j4 -s source-package > 0:00.25 Generate the sourcestamp file > 0:00.27 source-repo.h > 0:00.27 buildid.h > 0:00.68 Packaging source tarball... > ... And the sourcestamp.txt file contains: > $cat sourcestamp.txt > 20170221123556 > https://hg.mozilla.org/mozilla-central/rev/c65f6e42304cda4c18ddc5808f5f82ef69dd6cb3 So we should be good. Is there any way to test (other than a push to try) that removing the "$(NSINSTALL) -D $(DIST)/$(PKG_PATH)" line doesn't introduce any regression?
Assignee: nobody → alessio.placitelli
Attachment #8839060 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8839428 - Flags: review?(ted)
(In reply to Alessio Placitelli [:Dexter] from comment #8) > So we should be good. Is there any way to test (other than a push to try) > that removing the "$(NSINSTALL) -D $(DIST)/$(PKG_PATH)" line doesn't > introduce any regression? I think that's not used for anything anymore. I would bet it is leftover from the prettynames support, which was just removed in bug 1329355. I checked and yeah, it was set in that case: https://hg.mozilla.org/mozilla-central/rev/318f1bcd336e#l4.75 Now it's just always set to the empty string: https://dxr.mozilla.org/mozilla-central/rev/d0462b0948e0b1147dcce615bddcc46379bdadb2/toolkit/mozapps/installer/package-name.mk#55 So that line was just trying to effectively `mkdir $(DIST)` which is not very useful. Removing it is fine. I'm going to file a bug to remove PKG_PATH entirely.
Comment on attachment 8839428 [details] [diff] [review] bug1338099.patch Review of attachment 8839428 [details] [diff] [review]: ----------------------------------------------------------------- ::: toolkit/mozapps/installer/packager.mk @@ +98,5 @@ > > GARBAGE += make-package > > make-sourcestamp-file:: > + @awk '$$2 == "MOZ_BUILDID" {print $$3}' $(DEPTH)/buildid.h >> $(MOZ_SOURCESTAMP_FILE) You should use > and not >> here, so that if this gets run twice it doesn't wind up with the contents of the file doubled.
Attachment #8839428 - Flags: review?(ted) → review+
Attached patch bug1338099.patch (deleted) — — Splinter Review
Thanks Ted!
Attachment #8839428 - Attachment is obsolete: true
Attachment #8839474 - Flags: review+
This was manually tested as per comment 8, doesn't need a try push (that wouldn't be helpful anyway).
status-firefox51: --- → wontfix
status-firefox52: --- → affected
status-firefox53: --- → affected
Keywords: checkin-needed
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/8d2a0d1f2231 Fix the source packager to produce a non empty sourcestamp.txt file. r=ted
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/8d2a0d1f2231
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox54: --- → fixed
Resolution: --- → FIXED
Comment on attachment 8839474 [details] [diff] [review] bug1338099.patch Approval Request Comment [Feature/Bug causing the regression]: bug 1282782 [User impact if declined]: It won't be possible to enable Telemetry on 3rd party Firefox for Linux builds. [Is this code covered by automated tests?]: No [Has the fix been verified in Nightly?]: This was manually tested as reported in comment 8. [Needs manual test from QE? If yes, steps to reproduce]: No [List of other uplifts needed for the feature/fix]: None [Is the change risky?]: Low to no risk [Why is the change risky/not risky?]: this fixes the build system to correctly output revision information to sourcestamp.txt when the repository has not been build. This only affects FF source archive generation when releasing a new version on Firefox. Worst that can happen, the newly added 'sourcestamp.txt' file is still empty. [String changes made/needed]: None
Attachment #8839474 - Flags: approval-mozilla-beta?
Attachment #8839474 - Flags: approval-mozilla-aurora?
Comment on attachment 8839474 [details] [diff] [review] bug1338099.patch Fix the build system to correctly output revision information and was verified. Aurora53+.
Attachment #8839474 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment on attachment 8839474 [details] [diff] [review] bug1338099.patch fix sourcestamp.txt generation, beta52+
Attachment #8839474 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
(In reply to Ryan VanderMeulen [:RyanVM] from comment #19) > https://hg.mozilla.org/releases/mozilla-beta/rev/484c20783a1b nthomas points to this bug for a failed thunderbird_source step yielding... builder: release-comm-beta-thunderbird_source slave: bld-linux64-spot-327 starttime: 1487904997.44 results: failure (2) buildid: 20170223185626 builduid: 004bf4ba9da24d8e8477ec142f559c4e revision: e3e36f9e7694 ========= Started make source-package failed (results: 2, elapsed: 0 secs) (at 2017-02-23 19:06:26.675059) ========= mock_mozilla -r mozilla-centos6-x86_64 --cwd /builds/slave/tb-rel-c-beta-tb_source-000000/build/comm-beta/objdir-tb --unpriv --shell '/usr/bin/env HG_SHARE_BASE_DIR="/builds/hg-shared" LC_ALL="C" no_tooltool="1" MOZILLA_REV="THUNDERBIRD_52_0b4_RELEASE" MOZ_AUTOMATION="1" MOZ_SIGN_CMD="python /builds/slave/tb-rel-c-beta-tb_source-000000/tools/release/signing/signtool.py --cachedir /builds/slave/tb-rel-c-beta-tb_source-000000/signing_cache -t /builds/slave/tb-rel-c-beta-tb_source-000000/token -n /builds/slave/tb-rel-c-beta-tb_source-000000/nonce -c /builds/slave/tb-rel-c-beta-tb_source-000000/tools/release/signing/host.cert -H ... # Make sure to have repository information available and then generate the # sourcestamp file. make -C ../.. 'source-repo.h' 'buildid.h' source-repo.h make[2]: Entering directory `/builds/slave/tb-rel-c-beta-tb_source-000000/build/comm-beta/objdir-tb' /builds/slave/tb-rel-c-beta-tb_source-000000/build/comm-beta/objdir-tb/_virtualenv/bin/python -m mozbuild.action.file_generate /builds/slave/tb-rel-c-beta-tb_source-000000/comm-beta/mozilla/build/variables.py source_repo_header source-repo.h .deps/source-repo.h.pp buildid.h /builds/slave/tb-rel-c-beta-tb_source-000000/build/comm-beta/objdir-tb/_virtualenv/bin/python -m mozbuild.action.file_generate /builds/slave/tb-rel-c-beta-tb_source-000000/comm-beta/mozilla/build/variables.py buildid_header buildid.h .deps/buildid.h.pp make[2]: Leaving directory `/builds/slave/tb-rel-c-beta-tb_source-000000/build/comm-beta/objdir-tb' make make-sourcestamp-file make[2]: Entering directory `/builds/slave/tb-rel-c-beta-tb_source-000000/build/comm-beta/objdir-tb/mail/installer' /bin/sh: ../../dist/linux-x86_64/en-US//thunderbird-52.0b4.txt: No such file or directory make[2]: *** [make-sourcestamp-file] Error 1 make[2]: Leaving directory `/builds/slave/tb-rel-c-beta-tb_source-000000/build/comm-beta/objdir-tb/mail/installer' make[1]: *** [source-package] Error 2 make[1]: Leaving directory `/builds/slave/tb-rel-c-beta-tb_source-000000/build/comm-beta/objdir-tb/mail/installer' make: *** [source-package] Error 2 State Changed: unlock buildroot program finished with exit code 2 elapsedTime=0.840202
(In reply to Wayne Mery (:wsmwk, NI for questions) from comment #21) > (In reply to Ryan VanderMeulen [:RyanVM] from comment #19) > > https://hg.mozilla.org/releases/mozilla-beta/rev/484c20783a1b > > nthomas points to this bug for a failed thunderbird_source step yielding... Could you please file a bug for that?
Depends on: 1342383
I've just checked the Firefox 52b9 source [1]. Gah, it turns out this isn't printing the revision once again. It goes a bit forward: the sourcestamp.txt file contains the build id (and only that). The log from the packager task doesn't really output any useful information: there's no error [2]. Ted, any idea why this could be happening? [1] - http://ftp.mozilla.org/pub/firefox/releases/52.0b9/source/ [2] - https://taskcluster.github.io/unified-logviewer/?url=https%3A%2F%2Fqueue.taskcluster.net%2Fv1%2Ftask%2FxFBI4WcCQueLdxwz30Vl4A%2Fruns%2F0%2Fartifacts%2Fpublic%2Flogs%2Flive.log&highlightStart=&highlightEnd=&lineNumber=&wrapLines=false&showLineNumbers=true&jumpToHighlight=true&followLog=false&lineHeight=19&asText=false [3] - https://public-artifacts.taskcluster.net/xFBI4WcCQueLdxwz30Vl4A/0/public/logs/live_backing.log
Flags: needinfo?(ted)
Thank you Tomcat!
Attached patch proposed updated patch (obsolete) (deleted) — — Splinter Review
At the beginning of the make-sourcstamp-file part, $(DIST)/$(PKG_PATH) doesn't exist, so NSINSTALL must be used. Also, since PKG_PATH already has the trailing /, I didn't feel it was necessary to add another one at the end of PKG_PATH for MOZ_SOURCESTAMP_FILE etc..
Attachment #8843826 - Flags: review?(ted)
Assignee: alessio.placitelli → nobody
(In reply to Edmund Wong (:ewong) from comment #26) > Created attachment 8843826 [details] [diff] [review] > proposed updated patch Compared to the previous patch this is also missing making buildid.h in the source-package target. Is that intentional? > Also, since PKG_PATH already has the trailing /, I didn't feel it was > necessary to add another one at the end of PKG_PATH for MOZ_SOURCESTAMP_FILE > etc.. Is it really a good idea to touch more lines than necessary here? We already had unexpected breakage from the last patch. (In reply to Alessio Placitelli [:Dexter] from comment #23) > I've just checked the Firefox 52b9 source [1]. Gah, it turns out this isn't > printing the revision once again. It goes a bit forward: the sourcestamp.txt > file contains the build id (and only that). Maybe MOZILLA_OFFICIAL is missing?
To clarify, the repo URL is only written (to either source-repo.h or sourcestamp.txt) if `MOZ_INCLUDE_SOURCE_INFO` is set, and configure only sets that if `MOZILLA_OFFICIAL` is set.
(In reply to Jan Steffens from comment #27) > (In reply to Edmund Wong (:ewong) from comment #26) > > Created attachment 8843826 [details] [diff] [review] > > proposed updated patch > > Compared to the previous patch this is also missing making buildid.h in the > source-package target. Is that intentional? Unintentional mistake. > > > Also, since PKG_PATH already has the trailing /, I didn't feel it was > > necessary to add another one at the end of PKG_PATH for MOZ_SOURCESTAMP_FILE > > etc.. > > Is it really a good idea to touch more lines than necessary here? We already > had unexpected breakage from the last patch. > True, and I doubt it would make any difference with having // or /, I just felt it would look less unwieldy to have just one /. I'll update the patch.
Attached patch proposed patch (v2) (obsolete) (deleted) — — Splinter Review
Attachment #8843826 - Attachment is obsolete: true
Attachment #8843826 - Flags: review?(ted)
Attachment #8844283 - Flags: review?(ted)
Attached patch proposed patch (v2) (deleted) — — Splinter Review
note to self: qrefresh THEN upload patch.
Attachment #8844285 - Flags: review?(ted)
Attachment #8844283 - Attachment is obsolete: true
Attachment #8844283 - Flags: review?(ted)
Comment on attachment 8844285 [details] [diff] [review] proposed patch (v2) Review of attachment 8844285 [details] [diff] [review]: ----------------------------------------------------------------- If it works then it's fine with me.
Attachment #8844285 - Flags: review?(ted) → review+
Unfortunately, testing this locally isn't so useful (as I found out the hard way :-( ). To be absolutely certain that nothing weird happens around merge day, we should test this patch using the same job that packages the source code on the merge day. And that's where I got stuck.
(In reply to Alessio Placitelli [:Dexter] from comment #33) > Unfortunately, testing this locally isn't so useful (as I found out the hard > way :-( ). > > To be absolutely certain that nothing weird happens around merge day, we > should test this patch using the same job that packages the source code on > the merge day. > > And that's where I got stuck. I tried it on a SeaMonkey linux64 builder using the same process as given in the source step log and it seems to do what it intends to do. Perhaps someone with access to the TB Linux64 builders can try it out, if it makes any difference?
Can we talk to RelEng and figure out how to get their source package task into the in-tree task definitions so you could at least run it on try? It already runs on taskcluster so I can't imagine it'd be terribly hard to wire up.
Flags: needinfo?(ted)
While looking into something else today I found myself looking at the build+release tasks for 53.0b6. The source package task is defined here currently: https://github.com/mozilla/releasetasks/blob/master/releasetasks/templates/firefox/source.yml.tmpl I asked rail on IRC if we could move that into m-c so we could trigger it on try and he indicated that the only thing stopping it was someone finding time to do it.
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #36) > While looking into something else today I found myself looking at the > build+release tasks for 53.0b6. The source package task is defined here > currently: > https://github.com/mozilla/releasetasks/blob/master/releasetasks/templates/ > firefox/source.yml.tmpl > > I asked rail on IRC if we could move that into m-c so we could trigger it on > try and he indicated that the only thing stopping it was someone finding > time to do it. Thanks for keeping up the attention on this, Ted! I filed bug 1352106 for the Try integration and added it as a dependency of this bug.
Depends on: 1352106
Dexter, do you mind if I assign the bug to you?
Priority: -- → P1
(In reply to Rail Aliiev [:rail] ⌚️ET from comment #38) > Dexter, do you mind if I assign the bug to you? I don't mind, but I can't promise to get to it shortly :-(

Are there any news on this?
Neither the firefox-60.6.3esr nor firefox-66.0.5 package has content in sourcestamp.txt

(In reply to msirringhaus from comment #40)

Are there any news on this?
Neither the firefox-60.6.3esr nor firefox-66.0.5 package has content in sourcestamp.txt

Not that I know of, I transitioned to a different project now :(

Rail, any chance someone with some build system knowledge could tackle this on your end? I don't think I'll have time :(

Flags: needinfo?(rail)

Now that we moved all release graphs to gecko, it should be straight forward to iterate on this.

After a quick search, I found that the source tarball is generated by this definition:
https://searchfox.org/mozilla-central/source/taskcluster/ci/release-source/kind.yml

which uses this mozharness config:
https://searchfox.org/mozilla-central/source/testing/mozharness/configs/builds/releng_sub_linux_configs/64_source.py

which uses the source-package target, which is defined in:
https://searchfox.org/mozilla-central/source/toolkit/mozapps/installer/packager.mk#168

Something tells me that the make-sourcestamp-file targe is never called:
https://searchfox.org/mozilla-central/source/toolkit/mozapps/installer/packager.mk#121

We can either try adding the make-sourcestamp-file target to the mozharness config file before the source-package, or add a dependency on make-sourcestamp-file in packager.mk.

Not sure which way is preferable.

Mike, maybe you have better ideas?

Flags: needinfo?(rail) → needinfo?(mshal)

Oh, I was wrong, the source-package target explicitly calls $(MAKE) make-sourcestamp-file. It's something else that prevents it from being generated. Maybe it doesn't like the fact that we package sources without compiling the binaries (missing ini files in dist) or something related to --disable-compile-environment in .mozconfig.

Maybe we should be running this in a normal build environment with all build deps installed?

All mozharness tasks in taskcluster have MOZ_SOURCE_CHANGESET set to the correct value (here), which is the same as what gets put in application.ini (via here and here). So, at least in automation, when generating the source directly, we should be able to bypass looking at application.ini for the value.

From the packager.mk code, there are two values that are supposed to go into sourcestamp.txt - the buildid, and the MOZ_SOURCE_URL, which is something like https://hg.mozilla.org/mozilla-unified/rev/123abc...

The buildid part is failing with the platform.ini error, which is what the patches attached to this bug are intended to fix. Using ewong's r+'d patch seems to work locally for me, but I can't test on try (#c35).

The source url isn't getting written, and doesn't look like it was ever written, because we don't set MOZILLA_OFFICIAL=1 in the mozconfig (#c27, #c28). That should be a quick change to the linux64/source mozconfig:

diff --git a/browser/config/mozconfigs/linux64/source b/browser/config/mozconfigs/linux64/source
index 8bc6af77a7276..8ad4faf6b3fe7 100644
--- a/browser/config/mozconfigs/linux64/source
+++ b/browser/config/mozconfigs/linux64/source
@@ -3,3 +3,4 @@
 # extra dependencies on specific toolchains, e.g. gtk3.
 ac_add_options --disable-compile-environment
 ac_add_options --disable-nodejs
+export MOZILLA_OFFICIAL=1

With ewong's patch and the mozconfig patch, I get a sourcestamp.txt file that looks like this:

$ cat sourcestamp.txt
20190523160757
https://hg.mozilla.org/mozilla-unified/rev/c50b4c90d56a76d96c138adfc3562cc5f135257a

Is this what we want it to look like in the end? And rail, do you have a way to test those out in automation?

Flags: needinfo?(mshal) → needinfo?(rail)

mach try fuzzy --full -q '^release-source !sign'

Oh, nice! Thanks for the help, Mike!

Shall we just land this then?

Flags: needinfo?(rail)
Assignee: nobody → mshal

This is the same as ewong's patch in bug 1338099, just rebased and now
in phabricator.

The $(BUILDID) make variable tries to use either application.ini or
platform.ini to fetch the buildid, and if those don't exist then we end
up with an empty buildid. An error message is displayed, but it doesn't
cause the make invocation to fail because it happens inside a $(shell).
Instead we can fetch the buildid similar to the source url, by
generating buildid.h and grabbing the value via awk.

MOZ_INCLUDE_SOURCE_INFO is needed for us to write out the MOZ_SOURCE_URL
in sourcestamp.txt, and MOZ_INCLUDE_SOURCE_INFO is set in configure only
for MOZILLA_OFFICIAL=1 builds.

Depends on D32518

Attachment #9067415 - Attachment description: Bug 1338099 - Ensure sourcestmap.txt has a buildid; r?#firefox-build-system-reviewers → Bug 1338099 - Ensure sourcestamp.txt has a buildid; r?#firefox-build-system-reviewers
Pushed by mshal@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/802132d21228 Ensure sourcestamp.txt has a buildid; r=firefox-build-system-reviewers,chmanchester https://hg.mozilla.org/integration/autoland/rev/b556436df7df Export MOZILLA_OFFICIAL in the source build for sourcestamp.txt; r=nalexander

tyvm!pw-wspx.org

I meant "thank you very much", but somehow my selection clipboard from one the commits also ended up here :D

https://hg.mozilla.org/mozilla-central/rev/802132d21228
https://hg.mozilla.org/mozilla-central/rev/b556436df7df

Status: REOPENED → RESOLVED
Closed: 8 years ago → 5 years ago
status-firefox69: --- → fixed
Resolution: --- → FIXED

This is also a problem on esr68, should it be uplifted there?

Flags: needinfo?(mshal)

Comment on attachment 9067415 [details]
Bug 1338099 - Ensure sourcestamp.txt has a buildid; r?#firefox-build-system-reviewers

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration: This patch is needed to produce a non-empty sourcestamp.txt file
  • User impact if declined: sourcestamp.txt in firefox source tarballs will be empty, making it hard to identify the origin commit and repository.
  • Fix Landed on Version:
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): This change only affects a text file, not the browser itself.
  • String or UUID changes made by this patch:
Flags: needinfo?(mshal)
Attachment #9067415 - Flags: approval-mozilla-esr68?
Attachment #9067416 - Flags: approval-mozilla-esr68?

Comment on attachment 9067415 [details]
Bug 1338099 - Ensure sourcestamp.txt has a buildid; r?#firefox-build-system-reviewers

NPOTB for Firefox. Approved for 68.5esr.

Attachment #9067415 - Flags: approval-mozilla-esr68? → approval-mozilla-esr68+
Attachment #9067416 - Flags: approval-mozilla-esr68? → approval-mozilla-esr68+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: