Elm's nsUpdateDriver is out of date

RESOLVED FIXED

Status

()

RESOLVED FIXED
6 years ago
4 years ago

People

(Reporter: jimm, Unassigned)

Tracking

Trunk
x86_64
Windows 8.1
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

6 years ago
Tried to do a merge today and found that nsUpdateDriver.cpp is pretty heavily out of sync on Elm. I wasn't sure which changes I should merge over and which are unique to elm due to the new packaging code.

bbondy, curious if you can help me here when you get back. I really don't feel comfortable merging these two together. I tried a build without merging any changes and came close, but ended up with an unresolved external in the final link - 

nsAppRunner.obj : error LNK2019: unresolved external symbol "enum tag_nsresult __cdecl ProcessUpdates(class nsIFile *,class nsIFile *,int,char * *,char const *,bool,bool,class nsIFile *,void * *)" (?ProcessUpdates@@YA?AW4tag_
nsresult@@PAVnsIFile@@0HPAPADPBD_N30PAPAX@Z) referenced in function "public: int __thiscall XREMain::XRE_mainStartup(bool *)" (?XRE_mainStartup@XREMain@@QAEHPA_N@Z)
(Reporter)

Comment 1

6 years ago
Created attachment 669203 [details] [diff] [review]
elm fixes

webrtc changes are bug 799121
(Reporter)

Comment 2

6 years ago
Note, a bunch of webrtc build fixes have landed on mc, so we should also try with webrtc enabled.
(Reporter)

Comment 3

6 years ago
Also, nsIdleService is "newer" on elm. I just pushed the same improvements to inbound - 

https://hg.mozilla.org/integration/mozilla-inbound/rev/56bc99190c79
Tried to get this done tonight but I want to try the merge again tomorrow morning. Also I don't want to push it until tomorrow morning so I can fire off some updates manually and verify that I didn't break updates.  I'll work on this first thing tomorrow morning.
Created attachment 670427 [details] [diff] [review]
Brian's merge patch

Sorry I thought I posted this yesterday, and trying it again now I see what the error was, patch too big (5mb).  I pulled out the updater part of the patch.

Basically I couldn't get the build to work and ran out of time, so here is the merge I did for you to take over.   Sorry for the delay.

Basically you want to choose local but you also want to  make sure these parameters exist bool isOSUpdate, nsIFile *osApplyToDi just before ProcessType *outpid.
Jim I think you landed this merge already right? Can this be resolved?
(Reporter)

Comment 7

6 years ago
yep!
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
(Assignee)

Updated

4 years ago
OS: Windows 8 Metro → Windows 8.1
You need to log in before you can comment on or make changes to this bug.