346214 - Firefox fails to build when using --enable-official-branding, with error RC2176 : old DIB in ../../dist/branding/firefox.ico

Status: RESOLVED FIXED

(Firefox Build System :: General, defect, P2)

Description

[Bob Clary [:bc] (inactive)]

I began seeing this error this morning on windows xp and windows server 2003. It prevents me from doing automated tests on Firefox 2. I do clobber builds using vc6 for testing purposes.

Comment 1

[Bob Clary [:bc] (inactive)]

Attachment: shell script to set environment

Comment 2

[Bob Clary [:bc] (inactive)]

Attachment: mozconfig

Comment 3

[:Gavin Sharp [email: [e-mail]]]

This must be due to bug 340976.

Comment 4

[:Gavin Sharp [email: [e-mail]]]

Hrm, according to http://www.axialis.com/tutorials/vistaicons.html there is a problem with VC6 and Vista icons.

Comment 5

[:Gavin Sharp [email: [e-mail]]]

More information:
http://www.rw-designer.com/compile-vista-icon

Seems like there isn't a solution yet.

Comment 6

[:Gavin Sharp [email: [e-mail]]]

This didn't affect the Firefox Windows 1.8 tinderbox because it doesn't use --enable-official-branding.

Comment 7

[Mike Beltzner [:beltzner, not reading bugmail]]

Well, we obviously can't spin beta2 up without fixing this. Unless there's a clear path to victory here, I'd recommend that we back out bug 340976 and save it for a later release. Our expected Vista install base will be pretty low prior to Fx 2.0.0.1, I'd imagine.

Comment 8

[Mike Beltzner [:beltzner, not reading bugmail]]

Ooops, sorry for the bugspam. Wrong tab.

Comment 9

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I failed to build on trunk with VC++8/VC++7.1, this bug is not only with VC6.
# I cannot test migration tools that have the code that built if official branding.

Comment 10

[:Gavin Sharp [email: [e-mail]]]

My vc7.1 build doesn't have this problem. Since it's a problem with the resource compiler and not the c++ compiler, I guess saying that it's a "vc6 problem" is technically incorrect. Could it be that an older rc.exe is first in your path?

Comment 11

[Bob Clary [:bc] (inactive)]

(In reply to comment #10)
>

Yes, I set up the vc6 build environment completely separate from the vc8 build environment and could change that so that vc8's version of rc.exe is pulled from vc8's Common7/Tools/Bin or SDK/v2.0/Bin which would be acceptable to me.

Comment 12

[:Gavin Sharp [email: [e-mail]]]

My comment was actually directed to Masayuki, who said he was having this problem with vc7/vc8 :)

I think we need to just revert to the old icon for now.

Comment 13

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

VC7.1++ was uninstalled. My environment only has VC8.
I search the rc.exe in my system folder, I found 4 files in VS8 folder.
One file is for 64 bit compiler, this is not related.
Other files are same version that is 5.1.3609.0. Is this old version?

Comment 14

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #13)
> VC7.1++ was uninstalled. My environment only has VC8.
> I search the rc.exe in my system folder, I found 4 files in VS8 folder.
> One file is for 64 bit compiler, this is not related.
> Other files are same version that is 5.1.3609.0. Is this old version?

Looks like it. My copy of Visual Studio .NET 2003 comes with rc.exe version 5.2.3668.0. Are you using the free vc8 express, or the full version? Either way, I think I'll just back out the new icon.

Comment 15

[:Gavin Sharp [email: [e-mail]]]

I backed out the new icon from the branch, for now.
mozilla/other-licenses/branding/firefox/firefox.ico 	1.2.18.3

Comment 16

[u49640]

(In reply to comment #14)
> Looks like it. My copy of Visual Studio .NET 2003 comes with rc.exe version
> 5.2.3668.0. Are you using the free vc8 express, or the full version? Either
> way, I think I'll just back out the new icon.
> 

my VS2005 (Prof. edition) came with rc.exe Version 5.1.3609.0 (Lab01_N.020211-2000)

Comment 17

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I'm using professional edition too.

Comment 18

[Masatoshi Kimura [:emk]]

I couldn't also build with --enable-official-branding.
My VS2005(Std.) came with rc.exe 5.2.3690.0. (Why Std. have a newer version than Pro.?)
I tried also Srv2003 SP1 sdk came with rc.exe 5.2.3734.0, still no luck.

Comment 19

[Masatoshi Kimura [:emk]]

I saw a version from "rc /?". A file version from explorer was 5.1.3609.0.
However Srv2003 SP1 sdk has a rc.exe whose file version is also 5.2.3734.0.

Comment 20

[u49640]

completly OT:
I found a Version 5.2.3734.0, but this is in C:\Programme\Microsoft Visual Studio 8\VC\PlatformSDK\Bin\win64\AMD64 so it looks like the x64 Version

the older versions seems to be the 32Bit binaries

Comment 21

[Masatoshi Kimura [:emk]]

I could build with --enable-official-branding using rc.exe came from Windows SDK or WDK for Vista.
gerv:
Did you install Windows SDK?

Comment 22

[Peter Kasting]

Resummarizing slightly.

AFAICT, the affected set of compilers is "every version except VC8 express".

On IRC, Gavin suggested I could back out bug 340976 on trunk, but I am reluctant to do this.

Comment 23

[Peter Kasting]

Installing http://www.microsoft.com/downloads/details.aspx?FamilyId=0BAF2B35-C656-4969-ACE8-E4C0C0716ADB&displaylang=en seems to have made my build work, although I'm not 100% sure.

Comment 24

[Doug Turner (:dougt)]

I am not having this problem.  rob, are you seeing this?

Comment 25

[Robert Strong (they/them - no direct email)]

(In reply to comment #24)
> I am not having this problem.  rob, are you seeing this?
I reproduced this about a week ago

Comment 26

[Ronny Perinke]

(In reply to comment #25)
> I reproduced this about a week ago
> 

same here on TRUNK
Platform SDK Version 5.2.3790.20.75.51 (Server 2003 R2)
VC8 express edition

Comment 27

[Craig Hawco]

Attachment: winable.h -> winuser.h

Comment 28

[Craig Hawco]

I encountered this bug earlier this week - looks like it was an issue with rc.exe from VS2005 and older not being compatible with Vista icons. Upgrading the windows SDK from 2003 to Vista resolved the issue, though I ran into problems with nsKeyboardLayout.cpp looking for winable.h. Apparently winable.h has been removed, and the declarations moved into winuser.h. Once I updated the include everything built fine.

I'm unsure what the proper way forward is here - modifying the include now would undoubtably break builds using older SDKs. Anyone have any suggestions?

Comment 29

[Craig Hawco]

Attachment: patch v2

Better attempt - include winuser.h then check if we pick up the WINABLEAPI define, and if not include winable.h.

Comment 30

[:Gavin Sharp [email: [e-mail]]]

Craig: please file a new bug on that issue, and attach your patch there. This bug is specific to the icon problem.

Comment 31

[Craig Hawco]

I've created bug 364831 for the build issue with the Vista SDK. 

Comment 32

[Justin Wood (:Callek)]

Appears to only be targetted at 2.0 branch, but I was just hit on it for trunk and MSVC8 express.

Comment 33

[Reed Loden [:reed]]

The builds for Firefox 3 Beta 1 are failing with this error.

Comment 34

[Reed Loden [:reed]]

As per mconnor, I backed out the firefox.ico change from bug 340976 on GECKO190_20071106_RELBRANCH (not the trunk).

Checking in firefox.ico;
/cvsroot/mozilla/other-licenses/branding/firefox/firefox.ico,v  <--  firefox.ico
new revision: 1.4.8.1; previous revision: 1.4
done

This needs to be a blocker for M10, though, if we want to use this new image.

Comment 35

[(not currently active) Ted Mielczarek]

Installing a newer Platform SDK on the tinderbox won't fix this, since MozillaBuild is not setup to use VC8 + a PSDK.  It's done this way since VC8 includes a PSDK, and until recently it was newer than the other available SDKs.

http://lxr.mozilla.org/mozilla/source/tools/build-environment/win32/start-msvc8.bat#42

Comment 36

[Mike Connor [:mconnor]]

So can we change MozillaBuild to include the latest platform SDK and update the refplatform that way?  We need to fix this.

Comment 37

[(not currently active) Ted Mielczarek]

We can change MozillaBuild to use newer PSDKs alongside VC8, and update the refplatform tools to include a newer SDK (Vista SDK?).  Will have to coordinate with build to get the refplatform updated though.

Comment 38

[[e-mail] (:bhearsum)]

If Beta 2 is be done on automation this bug needs to be resolved, or the old icon re-landed on trunk.

Comment 39

[Mike Schroepfer]

Bumping to P1 based on comment 38 - Ted can you take this?

Comment 40

[(not currently active) Ted Mielczarek]

I don't think this is feasible for b2 if we're going to code freeze tomorrow.  We'll need to update MozillaBuild and then get that + an updated PSDK on the tinderbox.  I think that's too much to do in the timeframe.  If our release tinderbox can't handle the new icon, we should back that out until we get it to that state.

Comment 41

[[e-mail] (:bhearsum)]

(In reply to comment #40)
> I don't think this is feasible for b2 if we're going to code freeze tomorrow. 
> We'll need to update MozillaBuild and then get that + an updated PSDK on the
> tinderbox.  I think that's too much to do in the timeframe.  If our release
> tinderbox can't handle the new icon, we should back that out until we get it to
> that state.
> 

I agree. It'll be a lot of work to deploy these things on the build machines. I think the safer/quicker route is backing it out.

Comment 42

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

I agree with Ted. Getting a new MozBuild and then deploying it onto the production machines is not something we could do before tomorrow. 

Can we backout this icon for M10? 

Comment 43

[Reed Loden [:reed]]

(In reply to comment #42)
> Can we backout this icon for M10? 

For M9, we just backed it out on the RELBRANCH. I think doing the same thing would be good here, as the trunk should keep the new icon since it's still wanted for Firefox 3, and probability states it's hard to remember to reland things like that. Could you just document it in the M10 build plans to back it out on the RELBRANCH before building win32?

Comment 44

[[e-mail] (:bhearsum)]

(In reply to comment #43)
> (In reply to comment #42)
> > Can we backout this icon for M10? 
> 
> For M9, we just backed it out on the RELBRANCH. I think doing the same thing
> would be good here, as the trunk should keep the new icon since it's still
> wanted for Firefox 3, and probability states it's hard to remember to reland
> things like that. Could you just document it in the M10 build plans to back it
> out on the RELBRANCH before building win32?
> 

Hrm. That would be doable. A new idea just came to mind though. Why don't we just tag the old version with release/relbranch tags manually before the win32 build. Does that makes sense to people?

Comment 45

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

(In reply to comment #43)
> (In reply to comment #42)
> > Can we backout this icon for M10? 
> For M9, we just backed it out on the RELBRANCH. I think doing the same thing
> would be good here, as the trunk should keep the new icon since it's still
> wanted for Firefox 3, and probability states it's hard to remember to reland
> things like that. Could you just document it in the M10 build plans to back it
> out on the RELBRANCH before building win32?

We *could* manually back the icon out on the RELBRANCH, after tagging and branch were done. However, as automation will do the tag+branch, this means either:
- wait for automation to fail out during the windows build. Patch in correct icon. Cleanup and restart win32 build
or
- interrupt automation after tag+branch, but before the win32 build or srctarball steps start. Patch in correct icon. Cleanup and restart win32 build. 



This would be our first release with automation on trunk. If at all possible, I'd prefer to not start with code that we *know* wont compile. This forces us to knowingly break automation and do manual intervention and cleanup. The workaround of backing out the icon seems easier and safer, and something we can do in a safer relaxed manner, so I would prefer to do that. At least then we would know when we start that the code will build.

Open to alternatives, just my $0.02.

Comment 46

[[e-mail] (:bhearsum)]

> - interrupt automation after tag+branch, but before the win32 build or
> srctarball steps start. Patch in correct icon. Cleanup and restart win32 build. 
> 

This should be really easy to do. Run the Tag step manually (so source/build won't fire), tag the working firefox.ico, then restart automation without the Tag step. It's a little messier but not a huge inconvenience. Of course, my preference is not to do anything like this at all...but if keeping the current icon on trunk is what makes the most sense I think we can deal with it.

Comment 47

[(not currently active) Ted Mielczarek]

(In reply to comment #43)
> (In reply to comment #42)
> > Can we backout this icon for M10? 
> 
> For M9, we just backed it out on the RELBRANCH. I think doing the same thing
> would be good here, as the trunk should keep the new icon since it's still
> wanted for Firefox 3, and probability states it's hard to remember to reland
> things like that. Could you just document it in the M10 build plans to back it
> out on the RELBRANCH before building win32?

I don't see why it would be so bad to back out the icon, reopen bug 340976, and mark it as blocking with an appropriate priority.  I don't think that bug should ever have landed, actually, since our win32 tinderbox can't build it!  It makes no sense to have that icon on trunk if we can't build official builds with it yet.  If it's that important, then we'll have to make sure we get the prerequisite things fixed to make it happen, but since they haven't, that bug isn't really fixed anyway.

Comment 48

[Mike Schroepfer]

At FF3/Gecko 1.9 Meeting today we said we'll back-out the icon to get B2 out the door - so we need to revert bug 340976.

bug 402849 covers updating the build machines so we can re-land.  This will be a b3 blocker.

Comment 49

[Mike Connor [:mconnor]]

Downrevved the icon for now.

I think John owns getting the machines up to snuff, assigning provisionally to him, if there's a better owner please pass it along.

Comment 50

[Mike Connor [:mconnor]]

reopened the original bug to track landing the icon (and get matching icons for minefield/generic builds).  Moving deps there, and resolving this.

Comment 51

[Tony Chung [:tchung]]

hi bob, can you verify this bug now? 

Comment 52

[Bob Clary [:bc] (inactive)]

(In reply to comment #51)
> hi bob, can you verify this bug now? 

Sorry. Dead Dell == No Windows build environment. The win32 vms at the colo are tied up doing buildbot runs now.

Comment 53

[(not currently active) Ted Mielczarek]

Build should be able to verify this, since the beta 3 rcs were built with official branding.