Bug 957911 removed mozilla/config/configObj.py which printconfigsettings.py require. A followup patch was pushed to fix the sync issue between c-c and m-c (http://hg.mozilla.org/comm-central/rev/eecce9d6df78). Now, both TB and SM are busted when trying to get the build ID and app version. ../../../config/nsinstall -R -m 755 'turn_unittest' '../../../dist/bin' ../../../config/nsinstall -R -m 755 'sctp_unittest' '../../../dist/bin' make[5]: Leaving directory `/builds/slave/tb-c-cen-lx-d-0000000000000000/build/objdir-tb/media/mtransport/test' make[5]: Entering directory `/builds/slave/tb-c-cen-lx-d-0000000000000000/build/objdir-tb/mail/app' mkdir -p '.deps/' nsMailApp.o Traceback (most recent call last): File "/builds/slave/tb-c-cen-lx-d-0000000000000000/build/mozilla/config/printconfigsetting.py", line 16, in <module> with open(file) as fh: IOError: [Errno 2] No such file or directory: '/builds/slave/tb-c-cen-lx-d-0000000000000000/build/objdir-tb/dist/bin/platform.ini' Traceback (most recent call last): File "/builds/slave/tb-c-cen-lx-d-0000000000000000/build/mozilla/config/printconfigsetting.py", line 16, in <module> with open(file) as fh: IOError: [Errno 2] No such file or directory: '/builds/slave/tb-c-cen-lx-d-0000000000000000/build/objdir-tb/dist/bin/platform.ini' /usr/bin/ccache /builds/slave/tb-c-cen-lx-d-0000000000000000/build/gcc/bin/g++ -m32 -march=pentiumpro -o nsMailApp.o -c -I../../dist/system_wrappers -include /builds/slave/tb-c-cen-lx-d-0000000000000000/build/mozilla/config/gcc_hidden.h -DMOZ_SOURCE_STAMP="eecce9d6df78" -DMOZ_SOURCE_REPO="https://hg.mozilla.org/comm-central" -DTHUNDERBIRD_ICO='"../../dist/branding/thunderbird.ico"' -DGRE_MILESTONE= -DGRE_BUILDID= -DXPCOM_GLUE -DAPP_VERSION='44.0a1' -DMOZILLA_OFFICIAL -DAB_CD=en-US -DNO_NSPR_10_SUPPORT -I/builds/slave/tb-c-cen-lx-d-0000000000000000/build/mail/app -I. -I/builds/slave/tb-c-cen-lx-d-0000000000000000/build/objdir-tb/build -I/builds/slave/tb-c-cen-lx-d-0000000000000000/build/mozilla/toolkit/xre -I/builds/slave/tb-c-cen-lx-d-0000000000000000/build/mozilla/xpcom/base -I/builds/slave/tb-c-cen-lx-d-0000000000000000/build/mozilla/xpcom/build -I../../dist/include -I/builds/slave/tb-c-cen-lx-d-0000000000000000/build/objdir-tb/dist/include/nspr -I/builds/slave/tb-c-cen-lx-d-0000000000000000/build/objdir-tb/dist/include/nss -fPIC -DMOZILLA_CLIENT -include ../../mozilla-config.h -MD -MP -MF .deps/nsMailApp.o.pp -Wall -Wempty-body -Woverloaded-virtual -Wsign-compare -Wwrite-strings -Wno-invalid-offsetof -Wcast-align -fno-exceptions -fno-strict-aliasing -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -fno-math-errno -std=gnu++0x -pthread -D_GLIBCXX_USE_CXX11_ABI=0 -pipe -DDEBUG -DTRACING -g -freorder-blocks -Os -fno-omit-frame-pointer /builds/slave/tb-c-cen-lx-d-0000000000000000/build/mail/app/nsMailApp.cpp thunderbird Furthering this problem is the additional issue of having a new zip file in the tinderbox-build dir (mozharness.zip) and this plays havok with the checks in http://hg.mozilla.org/build/buildbotcustom/file/f6abee7c5667/process/factory.py#l200. (Since mozharness.zip passes through all those checks, it ends up being selected as the packageUrl). This is how far I understood this issue. Just filing this bug.

I propose the following changes: 1) backout https://hg.mozilla.org/comm-central/rev/eecce9d6df78 2) add configobj.py to the check-sync-exceptions 3) modify SM/TB's build steps to use the c-c's printconfigsettings.py instead of m-c's for #1 and #2, I need a build config peer r+ (selecting :jcranmer) for #3, I need a releng peer

(In reply to Edmund Wong (:ewong) from comment #1) > I propose the following changes: > > 1) backout https://hg.mozilla.org/comm-central/rev/eecce9d6df78 > 2) add configobj.py to the check-sync-exceptions > 3) modify SM/TB's build steps to use the c-c's printconfigsettings.py instead > of m-c's > > for #1 and #2, I need a build config peer r+ (selecting :jcranmer) rs+ for those two steps from me.

Comment on attachment 8672445 [details] [diff] [review] [buildbotcustom] proposed patch(v1) The rationale for this patch is to allow TB's automation to use the c-c's config/printconfigsettings.py and Firefox's automation to use the m-c one. The existence of the mozilla/ dir means TB is being built; otherwise, it's Firefox. (This test breaks when Firefox adds a mozilla/ dir in the source tree.)

This is the check-sync-exceptions patch that jcranmer rs+

(In reply to Joshua Cranmer [:jcranmer] from comment #2) > (In reply to Edmund Wong (:ewong) from comment #1) > > I propose the following changes: > > > > 1) backout https://hg.mozilla.org/comm-central/rev/eecce9d6df78 > > 2) add configobj.py to the check-sync-exceptions > > 3) modify SM/TB's build steps to use the c-c's printconfigsettings.py instead > > of m-c's > > > > for #1 and #2, I need a build config peer r+ (selecting :jcranmer) > > rs+ for those two steps from me. Pushed to comm-central: #1) https://hg.mozilla.org/comm-central/rev/b869e1ed82db #2) https://hg.mozilla.org/comm-central/rev/fb83f130e2d5

Comment on attachment 8672445 [details] [diff] [review] [buildbotcustom] proposed patch(v1) I won't be able to look at this for a few days, feel free to switch the reviewer if you need it before then.

(In reply to Nick Thomas [:nthomas] from comment #7) > Comment on attachment 8672445 [details] [diff] [review] > [buildbotcustom] proposed patch(v1) > > I won't be able to look at this for a few days, feel free to switch the > reviewer if you need it before then. Ah ok. Thanks for the info. Switched to jlund.

Comment on attachment 8672445 [details] [diff] [review] [buildbotcustom] proposed patch(v1) or whoever can review the earliest.

If you go this route, bug 1213839 may not be required for now.

Comment on attachment 8672445 [details] [diff] [review] [buildbotcustom] proposed patch(v1) Review of attachment 8672445 [details] [diff] [review]: ----------------------------------------------------------------- ::: process/factory.py @@ +1649,5 @@ > else: > + if self.mozillaDir: > + useConfDir = 'build/config' > + else: > + useConfDir = 'build%s/config' % self.origSrcDir The previous condition uses: useConfigDir = '%s/config' % self.origSrcDir Is it any different than this one? If yes, can you add a comment why?

(In reply to Rail Aliiev [:rail] from comment #11) > Comment on attachment 8672445 [details] [diff] [review] > [buildbotcustom] proposed patch(v1) > > Review of attachment 8672445 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: process/factory.py > @@ +1649,5 @@ > > else: > > + if self.mozillaDir: > > + useConfDir = 'build/config' > > + else: > > + useConfDir = 'build%s/config' % self.origSrcDir > > The previous condition uses: > > useConfigDir = '%s/config' % self.origSrcDir > > Is it any different than this one? If yes, can you add a comment why? It really should be 'build%s/config' % self.origSrcDir for the reason being that it's the source directory we need (either the absolute version or the truncated one (relative?)). The previous condition needs to be fixed. My concern/issue is whether it needs to be absolute or relative. i.e. '%s/config' % self.absMozillaSrcDir or 'build%s/config' % self.origSrcDir

There should be a high bar and specific reasons why a build fork like this should be created for c-c. Can anyone explain what they are? Otherwise it is a very poor strategy to not follow what m-c is doing. Conversely, the road aleth took, syncing in bug 957911, and trying to figure out new fails due to an older python ver in bug 1213839, seems a far better approach.

(In reply to alta88 from comment #13) > There should be a high bar and specific reasons why a build fork like this > should be created for c-c. Can anyone explain what they are? Mozilla-central uses mozharness-based builds at this point. I don't think they're even running this code at all (hence why it broke c-c and not m-c).

Comment on attachment 8672445 [details] [diff] [review] [buildbotcustom] proposed patch(v1) Review of attachment 8672445 [details] [diff] [review]: ----------------------------------------------------------------- I suppose the other option is to patch buildbot to use mach's python interpreter which has python/configobj (the new config/configobj). this way you do not need to use comm-central at all iiuc right? iow - a buildbot patch that is similar to https://bugzil.la/957951

(In reply to alta88 from comment #13) > There should be a high bar and specific reasons why a build fork like this > should be created for c-c. Can anyone explain what they are? > > Otherwise it is a very poor strategy to not follow what m-c is doing. > Conversely, the road aleth took, syncing in bug 957911, and trying to figure > out new fails due to an older python ver in bug 1213839, seems a far better > approach. In order to follow what m-c is doing, TB needs to get onto the mozharness setup. Right now m-c is building using mozharness: http://hg.mozilla.org/mozilla-central/file/2387ada86428/testing/mozharness and http://hg.mozilla.org/mozilla-central/file/2387ada86428/testing/mozharness/mozharness/mozilla has the necessary 'stuff' for Firefox to be built. To get TB working, (from what I gather), the following needs to be done: 1) thunderbird_desktop_{build,unittest}.py needs to be added to : http://hg.mozilla.org/mozilla-central/file/2387ada86428/testing/mozharness/scripts 2) thunderbird's buildbot-config/buildbotcustom code will need to be changed to use #1 instead of the current stuff. Now the questions are: 1) Are we 'allowed' to add a set of ?.py files to the mozharness' repo that will allow TB to build? [This question exists due to the obstacles we're facing with the c-c merge bug; that is, will they allow c-c based stuff in mozharness?] 2) Is this what everyone wants TB to have in order to continue building?

From what I gather, we cannot move to mozharness at the moment because of the extra client.py checkout step which is not covered by mozharness. I'd appreciate if we could get this bug fixed without mozharness to get nightly builds back. If there is an easy way to move to mozharness afterwards we should do that in a followup, but I think we'll have to rediscuss the c-c/m-c merge to do that without too much pain.

(In reply to Edmund Wong (:ewong) from comment #12) > The previous condition needs to be fixed. My concern/issue is whether > it needs to be absolute or relative. > > i.e. > '%s/config' % self.absMozillaSrcDir > or > 'build%s/config' % self.origSrcDir To take some illusions about those variable names, the abs* variables in process/factory.py are not actually absolute. For example, absMozillaSrcDir is usually put together from self.baseWorkDir and self.mozillaSrcDir. self.baseWorkDir defaults to "build" and is never set to anything different in the calling code, therefore "build" is sometimes hardcoded. self.origSrcDir is additionally usually set from self.branchName, which is not actually the branch but e.g. "comm-central". The workdir these paths are relative to is e.g. /builds/slave/tb-c-cen-m64-ntly-000000000000/. So finally for the objdir you get a path like /builds/slave/tb-c-cen-m64-ntly-000000000000/build/comm-central/objdir-tb. To add to that, mozillaDir is mostly left over from times where we had the dist/ directory in objdir-tb/mozilla/dist, where we needed to run some targets in objdir/mozilla. Thunderbird doesn't use this variable anymore, but only sets mozillaSrcDir and absMozillaSrcDir differently, e.g. comm-central/mozilla. Therefore %s/config with absMozillaSrcDir should get you to build/comm-central/mozilla/config and build%s/config with self.origSrcDir should get you to build/comm-central/config. Please see also my patch in bug 1195442 where I am putting together a truely absolute directory e.g. using WithProperties('%(basedir)s/' + self.absObjDir). It is different per factory.

Comment on attachment 8672445 [details] [diff] [review] [buildbotcustom] proposed patch(v1) It'd be hard to verify this. Let's try it after Firefox 41.0.2 ships (thu/fri).

Comment on attachment 8672445 [details] [diff] [review] [buildbotcustom] proposed patch(v1) http://hg.mozilla.org/build/buildbotcustom/rev/e58fb77dc27f (default)

Comment on attachment 8672445 [details] [diff] [review] [buildbotcustom] proposed patch(v1) Review of attachment 8672445 [details] [diff] [review]: ----------------------------------------------------------------- backed this out as it broke my dev master today. ::: process/factory.py @@ +1367,5 @@ > + # Thunderbird uses the c-c's config/printconfigsetting.py due to > + # the removal of configobj.py (bug 957911) > + useConfigDir = 'build/config' > + else: > + useConfigDir = '%s/config' % self.origSrcDir I have little context but isn't self.origSrcDir only defined in BaseRepackFactory and SingleSourceFactory ? @@ +1372,3 @@ > self.addStep(SetProperty( > command=[ > + 'python', '%s/printconfigsetting.py' % useConfDir, you have defined useConfigDir above but are using useConfDir @@ +1380,5 @@ > descriptionDone=['got', 'buildid'], > )) > self.addStep(SetProperty( > command=[ > + 'python', '%s/printconfigsetting.py' % useConfDir, here again, s/useConfDir/useConfigDir @@ +1649,5 @@ > else: > + if self.mozillaDir: > + useConfDir = 'build/config' > + else: > + useConfDir = 'build%s/config' % self.origSrcDir same with here. I don't think self.origSrcDir is defined in MercurialBuildFactory @@ +3583,5 @@ > property_name='inipath', > workdir=self.absMozillaObjDir, > haltOnFailure=True, > )) > + if mozillaDir: did you mean self.mozillaDir here?

Comment on attachment 8675467 [details] [diff] [review] [buildbotcustom] proposed patch (v2) Review of attachment 8675467 [details] [diff] [review]: ----------------------------------------------------------------- ::: process/factory.py @@ +3584,5 @@ > workdir=self.absMozillaObjDir, > haltOnFailure=True, > )) > + if self.mozillaDir: > + useConfigDir = 'build/config' To not make hardcoding worse than it already is, I'd suggest using self.baseWorkDir instead of build.

Basically changed the build/config to using self.baseWorkDir.

Comment on attachment 8675966 [details] [diff] [review] [buildbotcustom] proposed patch (v3) Review of attachment 8675966 [details] [diff] [review]: ----------------------------------------------------------------- this is looking good bar one line in question still. ::: process/factory.py @@ +3586,5 @@ > )) > + if self.mozillaDir: > + useConfigDir = '%s/config' % self.baseWorkDir > + else: > + useConfigDir = 'build%s/config' % self.mozillaSrcDir should this be: useConfigDir = '%s/config' % self.absMozillaSrcDir

(In reply to Jordan Lund (:jlund) from comment #25) > Comment on attachment 8675966 [details] [diff] [review] > [buildbotcustom] proposed patch (v3) > > Review of attachment 8675966 [details] [diff] [review]: > ----------------------------------------------------------------- > > this is looking good bar one line in question still. > > ::: process/factory.py > @@ +3586,5 @@ > > )) > > + if self.mozillaDir: > > + useConfigDir = '%s/config' % self.baseWorkDir > > + else: > > + useConfigDir = 'build%s/config' % self.mozillaSrcDir > > should this be: > useConfigDir = '%s/config' % self.absMozillaSrcDir Makes sense. Should I change the others to do the same thing? @@ -1358,29 +1358,35 @@ class MercurialBuildFactory(MozillaBuild mock_workdir_prefix=None, )) def addBuildInfoSteps(self): """Helper function for getting build information into properties. Looks for self._gotBuildInfo to make sure we only run this set of steps once.""" if not getattr(self, '_gotBuildInfo', False): + if self.mozillaDir: + # Thunderbird uses the c-c's config/printconfigsetting.py due to + # the removal of configobj.py (bug 957911) + useConfigDir = '%s/config' % self.baseWorkDir + else: + useConfigDir = 'build%s/config' % self.mozillaSrcDir Change that last line to: useConfigDir = '%s/config' % self.absMozillaSrcDir ?

(In reply to Edmund Wong (:ewong) from comment #26) > (In reply to Jordan Lund (:jlund) from comment #25) > > Comment on attachment 8675966 [details] [diff] [review] > > [buildbotcustom] proposed patch (v3) > > > > Review of attachment 8675966 [details] [diff] [review]: > > ----------------------------------------------------------------- > > > > this is looking good bar one line in question still. > > > > ::: process/factory.py > > @@ +3586,5 @@ > > > )) > > > + if self.mozillaDir: > > > + useConfigDir = '%s/config' % self.baseWorkDir > > > + else: > > > + useConfigDir = 'build%s/config' % self.mozillaSrcDir > > > > should this be: > > useConfigDir = '%s/config' % self.absMozillaSrcDir > > Makes sense. Should I change the others to do the same thing? > > > @@ -1358,29 +1358,35 @@ class MercurialBuildFactory(MozillaBuild > mock_workdir_prefix=None, > )) > > def addBuildInfoSteps(self): > """Helper function for getting build information into properties. > Looks for self._gotBuildInfo to make sure we only run this set of > steps > once.""" > if not getattr(self, '_gotBuildInfo', False): > + if self.mozillaDir: > + # Thunderbird uses the c-c's config/printconfigsetting.py > due to > + # the removal of configobj.py (bug 957911) > + useConfigDir = '%s/config' % self.baseWorkDir > + else: > + useConfigDir = 'build%s/config' % self.mozillaSrcDir > > Change that last line to: > > useConfigDir = '%s/config' % self.absMozillaSrcDir > > ? I don't *think* so. I think the one I pointed to was the only one that differed to the original (under NightlyRepackFactory): before: - command=['python', '%s/config/printconfigsetting.py' % self.absMozillaSrcDir, after (your patch): + useConfigDir = 'build%s/config' % self.mozillaSrcDir + command=['python', '%s/printconfigsetting.py' % useConfigDir, do you agree?

(In reply to Jordan Lund (:jlund) from comment #27) > (In reply to Edmund Wong (:ewong) from comment #26) > > (In reply to Jordan Lund (:jlund) from comment #25) > > > Comment on attachment 8675966 [details] [diff] [review] > > > [buildbotcustom] proposed patch (v3) > > > > > > Review of attachment 8675966 [details] [diff] [review]: > > > ----------------------------------------------------------------- > > > > > > this is looking good bar one line in question still. > > > > > > ::: process/factory.py > > > @@ +3586,5 @@ > > > > )) > > > > + if self.mozillaDir: > > > > + useConfigDir = '%s/config' % self.baseWorkDir > > > > + else: > > > > + useConfigDir = 'build%s/config' % self.mozillaSrcDir > > > > > > should this be: > > > useConfigDir = '%s/config' % self.absMozillaSrcDir > > > > Makes sense. Should I change the others to do the same thing? > > > > > > @@ -1358,29 +1358,35 @@ class MercurialBuildFactory(MozillaBuild > > mock_workdir_prefix=None, > > )) > > > > def addBuildInfoSteps(self): > > """Helper function for getting build information into properties. > > Looks for self._gotBuildInfo to make sure we only run this set of > > steps > > once.""" > > if not getattr(self, '_gotBuildInfo', False): > > + if self.mozillaDir: > > + # Thunderbird uses the c-c's config/printconfigsetting.py > > due to > > + # the removal of configobj.py (bug 957911) > > + useConfigDir = '%s/config' % self.baseWorkDir > > + else: > > + useConfigDir = 'build%s/config' % self.mozillaSrcDir > > > > Change that last line to: > > > > useConfigDir = '%s/config' % self.absMozillaSrcDir > > > > ? > > I don't *think* so. I think the one I pointed to was the only one that > differed to the original (under NightlyRepackFactory): > > before: > - command=['python', '%s/config/printconfigsetting.py' % > self.absMozillaSrcDir, > > after (your patch): > + useConfigDir = 'build%s/config' % self.mozillaSrcDir > + command=['python', '%s/printconfigsetting.py' % useConfigDir, > > do you agree? Right. so I'll just change it to using the absMozillaSrcDir.

Comment on attachment 8677802 [details] [diff] [review] [buildbotcustom] proposed patch (v4) [backed-out] Review of attachment 8677802 [details] [diff] [review]: ----------------------------------------------------------------- lgtm :)

https://hg.mozilla.org/build/buildbotcustom/rev/bd49c6b5b405fb01718adc4722b4ecead8582394 Bug 1213715 - Change steps to use c-c's printconfigsettings.py. r=jlund

Comment on attachment 8677802 [details] [diff] [review] [buildbotcustom] proposed patch (v4) [backed-out] Please do not merge this patch into production yet. I'm having some slight doubts as to whether this will work. I'm going to back this out until I have a clearer understanding how TB's config works. Backed out the patch: https://hg.mozilla.org/build/buildbotcustom/rev/eaf21fa8996e

We should not be checking self.mozillaDir (in theory, this should be removed from MercurialBuildFactory's __init__ since mozillaDir points to the mozilla/ factory within the objdir, but this is no longer used when the objdir was flattened). We should be checking the existence of mozilla_srcdir ( self.mozillaSrcDir) as the presence of this value means Thunderbird is being built. (Firefox doesn't have the mozilla_srcdir.)

Comment on attachment 8678633 [details] [diff] [review] [buildbotcustom] proposed patch (v5) Review of attachment 8678633 [details] [diff] [review]: ----------------------------------------------------------------- ::: process/factory.py @@ +2162,5 @@ > name='set_previous_buildid', > description=['set', 'previous', 'buildid'], > doStepIf=self.previousMarExists, > command=['python', > '%s/config/printconfigsetting.py' % self.absMozillaSrcDir, did you want to change this line?

dropping prior so it doesn't page. A contributor is actively working on it, and it has a positive review, will land when said contributor is online and deploy (likely) tomorrow. p.s. Thanks Ewong!

Comment on attachment 8679187 [details] [diff] [review] [buildbotcustom] proposed patch (v6) (fixed missing change from (v5)) Pushed to buildbotcustom: (default) https://hg.mozilla.org/build/buildbotcustom/rev/be7927bbaa31

In production: https://hg.mozilla.org/build/buildbotcustom/rev/be7927bbaa31