When I try to build a debug build with "ac_add_options --enable-debug" set in my mozconfig, artifact install downloads an opt build instead of a debug build. https://queue.taskcluster.net/v1/task/HMR3odtuSJWzKoSNwF6mlw/artifacts/public%2Fbuild%2Ffirefox-47.0a1.en-US.linux-x86_64.tar.bz2

I assume this would also apply to other kind of builds like ASAN etc. So maybe this should be more generic? Otherwise other bugs should be filed.

Follow-on to Bug 1243041. Might be a good mentor bug.

Relevant code is at https://dxr.mozilla.org/mozilla-central/source/python/mozbuild/mozbuild/artifacts.py#753. We'd need to generalize 'linux' at https://dxr.mozilla.org/mozilla-central/source/python/mozbuild/mozbuild/artifacts.py#396 to one of 'linux', 'linux-debug', since that appears to be how to choose between opt and debug. Shouldn't be rocket science!

Review commit: https://reviewboard.mozilla.org/r/48059/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/48059/

(In reply to Nick Alexander :nalexander from comment #3) > Relevant code is at > https://dxr.mozilla.org/mozilla-central/source/python/mozbuild/mozbuild/ > artifacts.py#753. We'd need to generalize 'linux' at > https://dxr.mozilla.org/mozilla-central/source/python/mozbuild/mozbuild/ > artifacts.py#396 to one of 'linux', 'linux-debug', since that appears to be > how to choose between opt and debug. > > Shouldn't be rocket science! Hi Nick, I wanted this feature so badly, that I could not resist to the temptation to prepare a draft patch for it ("no rocket science was harmed" during this patch preparation :-)) The current patch (Attachment 8743834 [details]) is probably far from perfect (it is the first time I look into the building system internals, and so I tried to find where the pieces should go, but I could easily be wrong). Anyway this patch was sitting in my hg bookmarks for a while now, so I thought it is time to attaching it here, as the first step to make it a real thing. NOTE: I've currently tested this patch only on linux (with the linux64 jobs), but theoretically it should work correctly on osx and windows too (the current version doesn't cover android, because I wasn't sure if it follows the exactly same pattern of the other platforms)

Comment on attachment 8743834 [details] MozReview Request: Bug 1253697 - Support downloading debug artifact builds. r=nalexander Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48059/diff/1-2/

https://reviewboard.mozilla.org/r/48059/#review45583 Thanks for the patch! However, we don't want to add new flags. Rather than add a new flag, let's inspect `MOZ_DEBUG`. See the example around https://dxr.mozilla.org/mozilla-central/source/python/mozbuild/mozbuild/artifacts.py#748. Thanks again!

Comment on attachment 8743834 [details] MozReview Request: Bug 1253697 - Support downloading debug artifact builds. r=nalexander Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48059/diff/2-3/

(In reply to Nick Alexander :nalexander from comment #7) > https://reviewboard.mozilla.org/r/48059/#review45583 > > Thanks for the patch! However, we don't want to add new flags. Rather than > add a new flag, let's inspect `MOZ_DEBUG`. Thanks Nick! In the last version pushed to mozreview, I'm now checking the MOZ_DEBUG flag (and I removed from moz.configure the code which used to define the new options). As a side note: using "ac_add_options --enable-debug" doesn't seem to enable the MOZ_DEBUG flag if combined with "ac_add_options --enable-artifact-builds" (my guess is that it is due to "--enable-artifact-builds" which implies "--disable-compile-environment") for the above reason this is the mozconfig file that I'm using with the last patch version to enable the debug artifact build: export MOZ_DEBUG=1 ac_add_options --enable-artifact-builds

Comment on attachment 8743834 [details] MozReview Request: Bug 1253697 - Support downloading debug artifact builds. r=nalexander https://reviewboard.mozilla.org/r/48059/#review45837 Looking better! I have a few nits, and then we're good to go. ::: python/mozbuild/mozbuild/artifacts.py (Diff revision 3) > - self._job = job or self._guess_artifact_job() > self._log = log > self._hg = hg > self._git = git > self._cache_dir = cache_dir > self._skip_cache = skip_cache > > + ## NOTE: _guest_artifact_job needs _log. > + self._job = job or self._guess_artifact_job() > + nit: Revert these changes; they're not needed. ::: python/mozbuild/mozbuild/artifacts.py:773 (Diff revision 3) > > + target_suffix = '' > + > + # Add the "-debug" suffix to the guessed artifact job name > + # if MOZ_DEBUG is enabled. > + if buildconfig.substs.get('MOZ_DEBUG', '') == '1': Just `if buildconfig.substs.get('MOZ_DEBUG'):`. Empty strings are false-y in Python, and we don't care if we get exactly '1'.

Comment on attachment 8743834 [details] MozReview Request: Bug 1253697 - Support downloading debug artifact builds. r=nalexander Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48059/diff/3-4/

https://reviewboard.mozilla.org/r/48059/#review45837 > nit: Revert these changes; they're not needed. The above change has been reverted in the last push to mozreview (and the related logged messages in the _guess_artifact_job removed accordingly) > Just `if buildconfig.substs.get('MOZ_DEBUG'):`. Empty strings are false-y in Python, and we don't care if we get exactly '1'. Agree, done in the last push to mozreview (and my apologies for my "rusty" python)

(In reply to Nick Alexander :nalexander from comment #10) > Comment on attachment 8743834 [details] > MozReview Request: Bug 1253697 - Download debug artifact builds on > ac_add_options --debug-artifact. r?nalexander > > https://reviewboard.mozilla.org/r/48059/#review45837 > > Looking better! I have a few nits, and then we're good to go. That's Great! I just pushed to mozreview the tweaks related to the nits described above. Thanks a lot, Nick.

Comment on attachment 8743834 [details] MozReview Request: Bug 1253697 - Support downloading debug artifact builds. r=nalexander https://reviewboard.mozilla.org/r/48059/#review46101

Comment on attachment 8743834 [details] MozReview Request: Bug 1253697 - Support downloading debug artifact builds. r=nalexander Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48059/diff/4-5/

I've just pushed an update which tweaks the commit message (because the old commit message makes reference to the --debug-artifact option which was added in the first version of the patch and it doesn't exist anymore) can checkin-needed be used to schedule this patch for landing? (My apologies for the above question, but this is the first time that I'm landing a mozbuild patch, and without the possibility to push it to try first, and so I thought it worth a double-check before proceeding)

(In reply to Luca Greco [:rpl] from comment #16) > I've just pushed an update which tweaks the commit message > (because the old commit message makes reference to the --debug-artifact > option which was added in the first version of the patch and it doesn't > exist anymore) > > can checkin-needed be used to schedule this patch for landing? > (My apologies for the above question, but this is the first time that I'm > landing a mozbuild patch, and without the possibility to push it to try > first, and so I thought it worth a double-check before proceeding) Yep, checkin-needed is good. I'm setting it now.