Closed Bug 535614 Opened 15 years ago Closed 14 years ago

Sparkle doesn't have the right paths for vcs-info

Categories

(Camino Graveyard :: General, defect)

Product:
Camino Graveyard
Component:
General
Platform:
All
macOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: alqahira, Assigned: stuart.morgan+bugzilla)

References

(
URL
)

Details

Attachments

(1 file)

Swap framework reference [landed]
3.74 KB, patch
alqahira
: review+
Details | Diff | Splinter Review 
I was poking 2.0 crashes to see if they were still declining and noticed a crash in objc_msgSend | _CFApplicationPreferencesCreateValueForKey, which ultimately had Sparkle on the stack.  However, we don't have the right paths for our Sparkle files, so they don't get linked on crash-stats:

0|18|Sparkle|-[SUUIBasedUpdateDriver didFindValidUpdate]|/builds/tinderbox/Cm2.0/Darwin_8.11.1_Depend/build/i386/camino/sparkle/SUUIBasedUpdateDriver.m|36|0x14 

rather than something like 

0|33|Camino|main|cvs:cvs.mozilla.org/cvsroot:mozilla/camino/src/application/main.m:1.9.4.1|86|0x11 

I'm not sure why that is; Breakpad (save some relative paths) and Growl are both fine.
One other thing I noticed (not sure if it's related or not): when we're linking Camino, we're linking Sparkle with a full path (and without -framework):

Ld /builds/tinderbox/Cm2-M1.9/Darwin_8.11.1_Depend/build/ppc/camino/build/Deployment/Camino.app/Contents/MacOS/Camino normal ppc
    cd /builds/tinderbox/Cm2-M1.9/Darwin_8.11.1_Depend/build/ppc/camino
    /Developer/usr/bin/g++-4.0 -o /builds/tinderbox/Cm2-M1.9/Darwin_8.11.1_Depend/build/ppc/camino/build/Deployment/Camino.app/Contents/MacOS/Camino 
[...]
-lsqlite3 /builds/tinderbox/Cm2-M1.9/Darwin_8.11.1_Depend/build/ppc/camino/sparkle/build/Release/Sparkle.framework/Sparkle -lnssutil3 -limgicon -framework Growl -lcaminosafebrowsing -framework Breakpad -arch ppc -Wl,-Y,1455 -mmacosx-version-min=10.4 -Wl,-dead_strip -lpthread -lm -Wl,-executable_path,build/Deployment/Camino.app/Contents/MacOS -isysroot /Developer/SDKs/MacOSX10.4u.sdk
(In reply to comment #1)
> One other thing I noticed (not sure if it's related or not): when we're linking
> Camino, we're linking Sparkle with a full path (and without -framework):

My guess is that this is because Sparkle is an orphan in the "Link with Frameworks" phase; it doesn't have a disclosure triangle, nor does "Get Info" show any info, nor does "Reveal in Group Tree" reveal anything, unlike Breakpad and Growl.  It's been disconnected from its source, somewhow.

Unless that's related to the "Change Sparkle Load ID" phase, I'm guessing that's a bug.  Maybe not the vcs-info bug, but some sort of bug. :P
It looks like the link step is referring to the framework as referenced through the Sparkle.xcodeproj inclusion, rather than the manual entry we made for the product (the one listed directly in Linked Frameworks). That was probably just an oversight on my part, as I can't think of any reason we wouldn't want to be consistent. It would be worth a try to change it and seeing if the paths look happier.

(Although if we ever do the Debug/Release renaming in our project, we may be able to get rid of our manual references for built frameworks and just use the linked ones; I'm not 100% sure, but IIRC they are just there because the mismatched build style names meant Xcode couldn't find the right output.)
Here's a patch to switch it; I haven't checked to see what effect it has.
Attachment #433752 - Flags: feedback?(alqahira)
Comment on attachment 433752 [details] [diff] [review]
Swap framework reference [landed]

Well, it fixes the link stuff, which is good enough reason to me to land it.

Unfortunately for the subject of this bug, it seems to have no effect on the path problem :-(
Attachment #433752 - Flags: feedback?(alqahira)
Comment on attachment 433752 [details] [diff] [review]
Swap framework reference [landed]

Stuart, let's go ahead and land this patch, since it does fix the linking issue in comment 1.
Attachment #433752 - Flags: review+
Comment on attachment 433752 [details] [diff] [review]
Swap framework reference [landed]

Landed on CVS trunk.
Attachment #433752 - Attachment description: Swap framework reference → Swap framework reference [landed]
(In reply to comment #7)
> (From update of attachment 433752 [details] [diff] [review])
> Landed on CVS trunk.

And on the CAMINO_2_0_BRANCH to keep project differences low.
(In reply to comment #8)
> (In reply to comment #7)
> > (From update of attachment 433752 [details] [diff] [review] [details])
> > Landed on CVS trunk.
> 
> And on the CAMINO_2_0_BRANCH to keep project differences low.

Although I realized after the fact that the reference in the General prefPane target is still doing the goofy full-path-no--framework during linking.
Comment 7 was imported into the test repo and thence to the final repo as http://hg.mozilla.org/camino/rev/0e8991a735f2
One other thing I've noticed is that Sparkle (and Growl) seems to end up with a corrupt .dSYM on rebuilds; it's like the dSYM is being recreated against a (stripped) framework every run, but the framework isn't being rebuilt.

Maybe this only happens because I'm always running "make buildcaminosymbols" well after my initial build, but it never seems to happen to any of Breakpad's products, or our products, or Core.
(In reply to comment #11)
> One other thing I've noticed is that Sparkle (and Growl) seems to end up with a
> corrupt .dSYM on rebuilds; it's like the dSYM is being recreated against a
> (stripped) framework every run, but the framework isn't being rebuilt.

http://mxr.mozilla.org/camino/source/sparkle/Sparkle.xcodeproj/project.pbxproj#1178, I bet.

Growl doesn't seem to have that set for the framework target (or higher up), so I'm not sure what's happening there.
That's a problem I've encountered with Xcode and dSYM before; my conclusion was that Xcode isn't smart enough to skip doing dSYM generation if the build step was a no-op. So the first build does:
1) Generate binary
2) Generate dSYM
3) Strip binary

But then subsequent builds without changes do:
1) Leave the current (stripped) binary because nothing has changed
2) Destroy the dSYM file by generating a new one from the stripped binary
With the patch in bug 553142, this is fixed:
FILE 22 hg:hg.mozilla.org/camino:sparkle/SUUIBasedUpdateDriver.m:6d17314d18f2
It's probably not worth fixing this for CVS.
And, thanks to your 553142 comment 11, I now know why: Breakpad and Growl both put sourcefiles in sub-folders (src and friends), whereas Sparkle was just a mess of files in the root.  When we symlinked contents/child folders of foo, we excluded CVS, so anywhere where the files are directly in the folder we're symlinking (rather than in child folders), we lost the CVS folder.

I tend to agree it's not worth fixing for CVS, given the number of Sparkle crashes we have.
Assignee: nobody → stuart.morgan+bugzilla
Depends on: 553142
In that case, fixed by the checkin for bug 553142.
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: