186036 - Allow PPEmbed to build under Mach-O [ProjectBuilder]

Status: RESOLVED FIXED

(Core Graveyard :: Embedding: Mac, defect)

Description

[Conrad Carlen (not reading bugmail)]

Since CFM is going away and embeddors will be using this with Mach-O.

Comment 1

[Conrad Carlen (not reading bugmail)]

Attachment: PPEmbed patch 1.0

A few things about the patch:

* This PPEmbed project depends on PowerPlant being built as a framework with
Project Builder. I'll be adding that patch here soon.
* The build copies files from dist/Embed, rather then directly from the tree.
That means that building in embedding/config is required first.
* All resource files are now text. They will be used by the CFM build as well
and the binary .rsrc files will be cvs removed.
* The gutting of UMacUnicode wasn't exactly part of Mach-O adaptation. With the
old code, I would have had to add access paths (depend on) non-frozen parts of
mozilla for Unicode conversion. I took this opportunity to get rid of that
code, using code I wrote for nsLocalFileMac a while back.

Comment 2

[Conrad Carlen (not reading bugmail)]

Attachment: patch v2

Copies PowerPlant framework into itself as an internal framework, some other
cleanup like removing absolute paths, got rid of a lot of compiler warnings,
etc.

Comment 3

[Conrad Carlen (not reading bugmail)]

Attachment: CW project diffs

Changes the CW project to build with the same .r resource files as the Project
Builder project, instead of binary .rsrc and .ppob files.

Comment 4

[Conrad Carlen (not reading bugmail)]

Attachment: PowerPlant framework project

This is the Powerplant project which builds the framework used by PPEmbed. The
path /Developer/PowerPlant is a symlink to the actual PP folder with the
Metrowerks installation. That this symlink has to be made and that slight mods
need to be made to the PowerPlant src is why this is not turned on in the
build.

Comment 5

[Conrad Carlen (not reading bugmail)]

Can I get a r/sr of this? It will be NOT PART OF THE BUILD for now, but worth
having somebody take a look.

Comment 6

[Mike Pinkerton (not reading bugmail)]

why are the resource files derez'd to .r? Makes them more or less useless. I
sure can't read binary and they'd have to be rez'd to be editable by any of the
tools we use, right?

Comment 7

[Conrad Carlen (not reading bugmail)]

> why are the resource files derez'd to .r?

(1) To keep them as text, data fork only files. Cmd-line cvs doesn't handle
binary .rsrc files. If they were binary, you'd still be stuck with running
asdecode in order to edit with any of the tools we use.
(2) Also, for many types of resources, it's easy to edit the DeRez'd text.

Binary is bad, text is good.

Comment 8

[Mike Pinkerton (not reading bugmail)]

-  return NS_FAILED(res) ? eventNotHandledErr : noErr;
+  if (NS_FAILED(res))
+    return eventNotHandledErr;
+  return noErr;

seems like an unnecessary change in all the places you make it. *shrug*

+  if (!mIconRef)
+    return false;
+    
 	Rect	iconRect;
 	CalcLocalFrameRect(iconRect);

should these files just be detabbed while you're at it?

+		F5A63FA003A5643301202892 = {
+			isa = PBXFileReference;
+			name = PowerPlant.r;
+			path = "/Developer/PowerPlant/PowerPlant Resources/PowerPlant.r";
+			refType = 0;

is that where powerplant always lives with pro8?

other than that looks pretty good. r=pink

Comment 9

[Conrad Carlen (not reading bugmail)]

> seems like an unnecessary change in all the places you make it. *shrug*

Without the change, each of those statements gives this warning:
warning: enumeral mismatch in conditional expression: `<anonymous enum>' vs
`<anonymous enum>'

Many of the changes to this code were to quiet warnings which were new with gcc.

> should these files just be detabbed while you're at it?

Yes, this would be a good time to do that - Will do.

> is that where powerplant always lives with pro8?

No, see comment 4. I'll make a README which has a patch for PowerPlant and
explains making the symlink.

> r=pink

Thanks.

Comment 10

[Conrad Carlen (not reading bugmail)]

Attachment: ReadMe for embedding/browser/powerplant

Comment 11

[Conrad Carlen (not reading bugmail)]

Comment on attachment 111067 [details] [diff] [review]
patch v2

Bryner, so you don't get 3 sr requests, can you sr all patches?

Comment 12

[Brian Ryner (not reading)]

One concern I have is that this won't work with objdir builds.  However, I think
there may be a solution to this.  You can manually specify where the built
object files should go in PB, and you can use environment variables in the path.
 So, if we use make to invoke PB, it can set the appropriate variables from our
build system (topsrcdir, DIST, etc) as environment variables before invoking
pbxbuild.  Of course, that would make building from the PB GUI hard.  Maybe
there's a way we can provide default values for these variables in PB that can
be overridden by the environment.

Other than that, it looks fine.

Comment 13

[Conrad Carlen (not reading bugmail)]

Specifying where the object code goes would be possible - see
/Developer/Documentation/ReleaseNotes/PBBuildSettings.html. But, that won't
solve the problem. Files from dist/Embed have to be in the file list of the
project and those, I've found, can't be indirected through a build variable.

In an objdir build, would it be possible to have the makefile create a symlink
to "dist" in the same location relative to the project as a normal build?

Comment 14

[Brian Ryner (not reading)]

No.  You can have multiple objdirs off of the same srcdir; making a symlink in
the srcdir to just one of them is kind of evil.

Another possibility, if it would help, is that we could copy the project file
into the objdir as part of the build.  Then the relative path to dist/ would be
the same.

Comment 15

[Conrad Carlen (not reading bugmail)]

> You can have multiple objdirs off of the same srcdir; making a symlink in
the srcdir to just one of them is kind of evil.

But, the build would update that symlink each time, so as long as you built it
from the Makefile each time...

The problem with copying the project is then what about the relative paths to
PPEmbed's own src?

If supporting objdir builds is important, could make a makefile for building
PPEmbed which would work in objdir builds? Then we'd have the mess of building
an app bundle without PB :-/

Comment 16

[Brian Ryner (not reading)]

Comment on attachment 111067 [details] [diff] [review]
patch v2

(just clearing the review flag; conrad is investigating objdir support)

Comment 17

[Conrad Carlen (not reading bugmail)]

Attachment: v3, part1

Comment 18

[Conrad Carlen (not reading bugmail)]

Attachment: v3, part2

I had to divide the v3 full patch into parts because of size.

Comment 19

[Conrad Carlen (not reading bugmail)]

v3 patch works in objdir build. Bryner, can you sr= both?

Comment 20

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 113705 [details] [diff] [review]
v3, part1

r=pink

Comment 21

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 113707 [details] [diff] [review]
v3, part2

r=pink

Comment 22

[Conrad Carlen (not reading bugmail)]

Comment on attachment 113705 [details] [diff] [review]
v3, part1

I would like to get this in so that it's available to Mach-O embeddors ASAP. It
poses no risk because it's not turned on in the build.

Comment 23

[Conrad Carlen (not reading bugmail)]

Comment on attachment 113707 [details] [diff] [review]
v3, part2

I would like to get this in so that it's available to Mach-O embeddors ASAP. It
poses no risk because it's not turned on in the build.

Comment 24

[Asa Dotzler [:asa]]

Comment on attachment 113705 [details] [diff] [review]
v3, part1

a=asa for not part of the build changes.

Comment 25

[Asa Dotzler [:asa]]

Comment on attachment 113707 [details] [diff] [review]
v3, part2

a=asa for not part of the build changes.

Comment 26

[Conrad Carlen (not reading bugmail)]

Checked in - except for the change to allmakefiles.sh because I couldn't then
say "not part of the build." That can happen when I make it so that this can be
optionally turned on in the build.