Investigate why automation is producing an empty "sourcestamp.txt" file
Categories
(Release Engineering :: Release Automation: Other, defect, P1)
Tracking
(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)
1.96 KB,
patch
|
Dexter
:
review+
gchang
:
approval-mozilla-aurora+
jcristau
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
1.87 KB,
patch
|
ted
:
review+
|
Details | Diff | Splinter Review |
47 bytes,
text/x-phabricator-request
|
RyanVM
:
approval-mozilla-esr68+
|
Details | Review |
47 bytes,
text/x-phabricator-request
|
RyanVM
:
approval-mozilla-esr68+
|
Details | Review |
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
Reporter | ||
Updated•7 years ago
|
Reporter | ||
Comment 1•7 years ago
|
||
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
Comment 2•7 years ago
|
||
(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.
Reporter | ||
Comment 3•7 years ago
|
||
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 :(
Comment 4•7 years ago
|
||
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.
Reporter | ||
Comment 5•7 years ago
|
||
Reporter | ||
Comment 6•7 years ago
|
||
(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 :-/
Comment 7•7 years ago
|
||
(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.
Reporter | ||
Comment 8•7 years ago
|
||
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?
Comment 9•7 years ago
|
||
(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 10•7 years ago
|
||
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.
Reporter | ||
Comment 11•7 years ago
|
||
Thanks Ted!
Reporter | ||
Comment 12•7 years ago
|
||
This was manually tested as per comment 8, doesn't need a try push (that wouldn't be helpful anyway).
Comment 13•7 years ago
|
||
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
Comment 14•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/8d2a0d1f2231
Reporter | ||
Comment 15•7 years ago
|
||
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
Comment 16•7 years ago
|
||
Comment on attachment 8839474 [details] [diff] [review] bug1338099.patch Fix the build system to correctly output revision information and was verified. Aurora53+.
Comment 17•7 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-aurora/rev/ba9e4cf76c3e
Comment 18•7 years ago
|
||
Comment on attachment 8839474 [details] [diff] [review] bug1338099.patch fix sourcestamp.txt generation, beta52+
Comment 19•7 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/484c20783a1b
Comment 20•7 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-esr52/rev/484c20783a1b
Comment 21•7 years ago
|
||
(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
Reporter | ||
Comment 22•7 years ago
|
||
(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?
Reporter | ||
Comment 23•7 years ago
|
||
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
Comment 24•7 years ago
|
||
backed out from release in https://hg.mozilla.org/releases/mozilla-release/rev/0f434b9df5ff46fd478968354c3bb1e72b36b04f beta: https://hg.mozilla.org/releases/mozilla-beta/rev/615b84cd4c5f5923b78ffa1d335e51a6a74001f5 esr 52 https://hg.mozilla.org/releases/mozilla-esr52/rev/15d9d940341f02ef6dcad96899d284619d3d48db aurora: https://hg.mozilla.org/releases/mozilla-aurora/rev/0d297906afe978a54991613faacbb8e7f99fa88f central: https://hg.mozilla.org/mozilla-central/rev/9732cd019a8b94c49a275661320c1b742635a3d6
Reporter | ||
Comment 25•7 years ago
|
||
Thank you Tomcat!
Comment 26•7 years ago
|
||
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..
Reporter | ||
Updated•7 years ago
|
Comment 27•7 years ago
|
||
(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?
Comment 28•7 years ago
|
||
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.
Comment 29•7 years ago
|
||
(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.
Comment 30•7 years ago
|
||
Comment 31•7 years ago
|
||
note to self: qrefresh THEN upload patch.
Updated•7 years ago
|
Comment 32•7 years ago
|
||
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.
Reporter | ||
Comment 33•7 years ago
|
||
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.
Comment 34•7 years ago
|
||
(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?
Comment 35•7 years ago
|
||
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.
Comment 36•7 years ago
|
||
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.
Reporter | ||
Comment 37•7 years ago
|
||
(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.
Reporter | ||
Comment 39•7 years ago
|
||
(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 :-(
Comment 40•5 years ago
|
||
Are there any news on this?
Neither the firefox-60.6.3esr
nor firefox-66.0.5
package has content in sourcestamp.txt
Reporter | ||
Comment 41•5 years ago
|
||
(In reply to msirringhaus from comment #40)
Are there any news on this?
Neither thefirefox-60.6.3esr
norfirefox-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 :(
Comment 42•5 years ago
|
||
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?
Comment 43•5 years ago
|
||
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?
Comment 44•5 years ago
|
||
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.
Assignee | ||
Comment 45•5 years ago
•
|
||
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?
Comment 46•5 years ago
|
||
mach try fuzzy --full -q '^release-source !sign'
Assignee | ||
Comment 47•5 years ago
|
||
Comment 48•5 years ago
|
||
Oh, nice! Thanks for the help, Mike!
Shall we just land this then?
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Comment 49•5 years ago
|
||
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.
Assignee | ||
Comment 50•5 years ago
|
||
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
Updated•5 years ago
|
Comment 51•5 years ago
|
||
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
Comment 52•5 years ago
|
||
tyvm!pw-wspx.org
Comment 53•5 years ago
|
||
I meant "thank you very much", but somehow my selection clipboard from one the commits also ended up here :D
Comment 54•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/802132d21228
https://hg.mozilla.org/mozilla-central/rev/b556436df7df
Comment 55•4 years ago
|
||
This is also a problem on esr68, should it be uplifted there?
Assignee | ||
Comment 56•4 years ago
|
||
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:
Assignee | ||
Updated•4 years ago
|
Comment 57•4 years ago
|
||
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.
Updated•4 years ago
|
Comment 58•4 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-esr68/rev/a5bfe5e0d388
https://hg.mozilla.org/releases/mozilla-esr68/rev/e9a7f943da2a
Description
•