Closed Bug 1115219 Opened 5 years ago Closed 5 years ago

Windows builds are going to burn when Gecko 36 merges to Beta

Categories

(Core :: Networking: DNS, defect, critical)

All
Windows 8.1
defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla37
Tracking Status
firefox36 + verified
firefox37 + verified

People

(Reporter: RyanVM, Assigned: RyanVM)

References

Details

Attachments

(1 file)

[Tracking Requested - why for this release]: Build bustage on the next merge.

Regression from bug 1084645 maybe? Merge day is 12-Jan.

https://treeherder.mozilla.org/logviewer.html#?job_id=3872683&repo=try

16:14:47 INFO - nsHostResolver.obj : error LNK2001: unresolved external symbol "enum tag_nsresult __cdecl mozilla::net::GetAddrInfo(char const *,unsigned short,unsigned short,class mozilla::net::AddrInfo * *,bool)" (?GetAddrInfo@net@mozilla@@YA?AW4tag_nsresult@@PBDGGPAPAVAddrInfo@12@_N@Z)
Flags: needinfo?(sworkman)
weird. That's a cool test ryan.

This is the patch from comment 0
https://hg.mozilla.org/try/raw-rev/4fc0b394ba49

steve - will you be able to sort this out in time?
Blocks: 1084645
That patch is actually built on top of a couple others, so I'd suggest using this one instead and debugging on top of Aurora (where it also reproduces).
https://hg.mozilla.org/try/raw-rev/916922a5cf5a
I can reproduce this locally too, so at least it's not some weird build config issue.
I think this should be fine. There's a RELEASE_BUILD flag in GetAddrInfo.cpp that can be removed (and should have been removed earlier - oops).

I'm back in the office Jan 5th - I think that should be soon enough, but if not, it should be (famous last words) an easy fix).
Flags: needinfo?(sworkman)
Like this? I can confirm that it fixes the failure locally :)

I'd like to get this landed ASAP so I can see what tests on Windows look like on Aurora36 soonish, so thanks in advance!
Assignee: nobody → ryanvm
Status: NEW → ASSIGNED
Attachment #8541059 - Flags: review?(sworkman)
Comment on attachment 8541059 [details] [diff] [review]
remove RELEASE_BUILD from GetAddrInfo.h

Throwing this up for whoever gets to it first :)
Attachment #8541059 - Flags: review?(mcmanus)
Important issue for the merge. Tracking
Comment on attachment 8541059 [details] [diff] [review]
remove RELEASE_BUILD from GetAddrInfo.h

Verified working on my most recent Try simulations.
Attachment #8541059 - Flags: feedback+
Comment on attachment 8541059 [details] [diff] [review]
remove RELEASE_BUILD from GetAddrInfo.h

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

this is obviously correct - so sorry I was afk
Attachment #8541059 - Flags: review?(sworkman)
Attachment #8541059 - Flags: review?(mcmanus)
Attachment #8541059 - Flags: review+
Comment on attachment 8541059 [details] [diff] [review]
remove RELEASE_BUILD from GetAddrInfo.h

https://hg.mozilla.org/integration/mozilla-inbound/rev/9689b122bc76

Approval Request Comment
[Feature/regressing bug #]: Bug 1084645
[User impact if declined]: Windows build bustage when Gecko 36 is uplifted to Beta. Would have to backout bug 1084645 instead.
[Describe test coverage new/current, TBPL]: Green on Try and locally both on top of trunk and Aurora
[Risks and why]: Minimal, just makes a change that should have landed with bug 1084645 anyway.
[String/UUID change made/needed]: None
Attachment #8541059 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/9689b122bc76
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
Attachment #8541059 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(In reply to Patrick McManus [:mcmanus] from comment #9)
> Comment on attachment 8541059 [details] [diff] [review]
> remove RELEASE_BUILD from GetAddrInfo.h
> 
> Review of attachment 8541059 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> this is obviously correct - so sorry I was afk

Ditto. Glad it was an easy fix! Thanks folks!
No occurrences in my latest Try runs, thanks :)
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.