Closed Bug 982928 Opened 6 years ago Closed 4 years ago

build/win32/mozconfig.vs2010 is Old And Busted

Categories

(Firefox Build System :: General, defect)

defect
Not set

Tracking

(firefox48 fixed)

RESOLVED FIXED
mozilla48
Tracking Status
firefox48 --- fixed

People

(Reporter: jgilbert, Assigned: jgilbert)

References

Details

Attachments

(3 files, 2 obsolete files)

Allegedly, this file is unused, since we're just building on 64-bit machines now. It should either be fixed or removed. (It can be added back in later, if we find we need it again)
If this is true and it's not used in production, then rs=me to remove it.
Let's give it a week. If no one yells, let's axe it.
Attached patch patch (obsolete) — Splinter Review
Assignee: nobody → jgilbert
Attachment #8390941 - Flags: superreview?(ted)
Comment on attachment 8390941 [details] [diff] [review]
patch

Review of attachment 8390941 [details] [diff] [review]:
-----------------------------------------------------------------

I'm not an official build peer, but its used for XULRunner, SeaMonkey and TB at present:

http://mxr.mozilla.org/comm-central/search?string=mozconfig.vs2010
Attachment #8390941 - Flags: review-
(In reply to Justin Wood (:Callek) from comment #4)
> I'm not an official build peer, but its used for XULRunner, SeaMonkey and TB
> at present:
> 
> http://mxr.mozilla.org/comm-central/search?string=mozconfig.vs2010

They will refer the c-c copy (<http://mxr.mozilla.org/comm-central/source/build/win32/mozconfig.vs2010>) rather than the m-c one (<http://mxr.mozilla.org/comm-central/source/mozilla/build/win32/mozconfig.vs2010>), no?
Flags: needinfo?(bugspam.Callek)
yes and no....

Do to the way configure[.in] expands the mozconfigs any included file like that will actually include the c-c copy from within c-c and the m-c copy when the m-c configure runs.

Again its also used in xulrunner, which is an m-c product.

In this case its not really hurting anything [imo] to leave it in
Flags: needinfo?(bugspam.Callek)
(In reply to Justin Wood (:Callek) from comment #6)
> yes and no....
> 
> Do to the way configure[.in] expands the mozconfigs any included file like
> that will actually include the c-c copy from within c-c and the m-c copy
> when the m-c configure runs.
> 
> Again its also used in xulrunner, which is an m-c product.
> 
> In this case its not really hurting anything [imo] to leave it in

How about we document this? It was briefly confusing when we were investigating where the Windows SDK files live on these machines.
Comment on attachment 8390941 [details] [diff] [review]
patch

Review of attachment 8390941 [details] [diff] [review]:
-----------------------------------------------------------------

I'm fine with removing this. If something in c-c is using it then you can check a copy in there. As it is it does have a negative impact right now--it confuses the issue of what mozconfig is in use for official Firefox builds.
Attachment #8390941 - Flags: superreview?(ted) → superreview+
Ted griped about a reference to build/win32/mozconfig.vs2010 in bug 1186060 and asked me to file a follow-up to track killing it. I found this existing bug instead.
Depends on: vs2015
My actual gripe there was about `mk_export_correct_style`, maybe that wasn't clear.
It looks like we have users again. Maybe they're spurious, but that's more work to determine.

However, win32/mozconfig.vs2010-win64 and win64/mozconfig.vs2010 *are* unused in the tree.
(In reply to Jeff Gilbert [:jgilbert] from comment #11)
> However, win32/mozconfig.vs2010-win64 and win64/mozconfig.vs2010 *are*
> unused in the tree.

Also these files are not used by comm-central!
Attachment #8390941 - Attachment is obsolete: true
Attachment #8741122 - Flags: superreview?(ted)
From c0f7452ac3626070d2f20d2206a2f536bcd7fa70 Mon Sep 17 00:00:00 2001
---
 build/win32/mozconfig.vs2010-win64 | 31 -----------------------------
 build/win64/mozconfig.vs2010       | 40 --------------------------------------
 2 files changed, 71 deletions(-)
 delete mode 100644 build/win32/mozconfig.vs2010-win64
 delete mode 100644 build/win64/mozconfig.vs2010

Review commit: https://reviewboard.mozilla.org/r/46213/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/46213/
Attachment #8741129 - Flags: review?(gps)
From 83f416e08db0cb2884bc29f6d1dd8912181fecfc Mon Sep 17 00:00:00 2001
 r?sfink
---
 js/src/devtools/automation/winbuildenv.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Review commit: https://reviewboard.mozilla.org/r/46215/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/46215/
From bf0bc86e81005811235d8f011080c4384a8965f9 Mon Sep 17 00:00:00 2001
---
 b2g/config/mozconfigs/win32_gecko/debug              | 2 +-
 b2g/config/mozconfigs/win32_gecko/nightly            | 2 +-
 b2g/graphene/config/horizon-mozconfigs/win32/debug   | 2 +-
 b2g/graphene/config/horizon-mozconfigs/win32/nightly | 2 +-
 b2g/graphene/config/mozconfigs/win32/debug           | 2 +-
 b2g/graphene/config/mozconfigs/win32/nightly         | 2 +-
 browser/config/mozconfigs/win32/l10n-mozconfig       | 2 +-
 7 files changed, 7 insertions(+), 7 deletions(-)

Review commit: https://reviewboard.mozilla.org/r/46217/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/46217/
Comment on attachment 8741122 [details] [diff] [review]
0001-Bug-982928-Remove-unused-build-mozconfigs.-sr-ted.mi.patch

It's multiple patches, so let's use MozReview.
Attachment #8741122 - Attachment is obsolete: true
Attachment #8741122 - Flags: superreview?(ted)
Attachment #8741127 - Flags: superreview?(ted)
Attachment #8741128 - Flags: review?(sphink)
Comment on attachment 8741127 [details]
MozReview Request: Bug 982928 - r=ted.mielczarek - Remove unused build mozconfigs.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/46213/diff/1-2/
Attachment #8741127 - Attachment description: MozReview Request: Bug 982928 - Remove unused build mozconfigs. - sr=ted.mielczarek → MozReview Request: Bug 982928 - r=ted.mielczarek - Remove unused build mozconfigs.
Attachment #8741128 - Attachment description: MozReview Request: js/src/devtools/automation/winbuildenv.sh should use vs2015. - → MozReview Request: r?sfink - js/src/devtools/automation/winbuildenv.sh should use
Attachment #8741129 - Attachment description: MozReview Request: Uses of mozconfig.vs2010 should be upgraded. - r?gps → MozReview Request: r?gps - Uses of mozconfig.vs2010 should be upgraded.
Attachment #8741127 - Flags: review?(ted)
Attachment #8741128 - Flags: review?(sphink)
Comment on attachment 8741128 [details]
MozReview Request: r?sfink - js/src/devtools/automation/winbuildenv.sh should use

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/46215/diff/1-2/
Comment on attachment 8741129 [details]
MozReview Request: r?gps - Uses of mozconfig.vs2010 should be upgraded.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/46217/diff/1-2/
Sorry for the noise, just trying to the reviewers to parse. 'close enough' for now.
Attachment #8741128 - Flags: review?(sphink)
Comment on attachment 8741128 [details]
MozReview Request: r?sfink - js/src/devtools/automation/winbuildenv.sh should use

https://reviewboard.mozilla.org/r/46215/#review42749

Totally fine with this.
Attachment #8741128 - Flags: review?(sphink) → review+
Comment on attachment 8741127 [details]
MozReview Request: Bug 982928 - r=ted.mielczarek - Remove unused build mozconfigs.

https://reviewboard.mozilla.org/r/46213/#review42755
Attachment #8741127 - Flags: review+
Comment on attachment 8741129 [details]
MozReview Request: r?gps - Uses of mozconfig.vs2010 should be upgraded.

https://reviewboard.mozilla.org/r/46217/#review42757

Oh look, in-tree mozconfigs that are still building with VS2013. These should be upgraded to VS2015. Let's get a bug on file for that.
Attachment #8741129 - Flags: review?(gps) → review+
Blocks: 1264499
Comment on attachment 8741127 [details]
MozReview Request: Bug 982928 - r=ted.mielczarek - Remove unused build mozconfigs.

https://reviewboard.mozilla.org/r/46213/#review42933
Attachment #8741127 - Flags: review?(ted) → review+
Attachment #8741127 - Flags: superreview?(ted) → superreview+
https://hg.mozilla.org/mozilla-central/rev/1411186a9603
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.