390072 - [SoC] Scripting Dictionary and Build System

Status: RESOLVED FIXED

(Camino Graveyard :: OS Integration, defect)

Description

[Peter Jaros (:peeja)]

Attachment: Branch version

These patches will add Camino.sdef, which defines the scripting dictionary, to the tree.  They will also update the build system to build a scriptable Camino:

Branch: Adds a Run Script build phase which compiles the sdef to a .r, then Rezes the .r to Resources/Camino.rsrc.  This is a little ugly, but appears to be the only way to get things to work on 10.3.  This is because we need to adapt some of the terms in the Standard Suite (which is common practice), but if we use a .scriptSuite/.scriptTerminology pair Cocoa Scripting registers the default version of the Standard Suite for us, which means we get two versions of it in the dictionary.  AETE-resource-based dictionaries don't have this problem.  Luckily, this issue will sunset at 2.0 when we go 10.4+-only.

Trunk: As of Tiger we can use an sdef directly.  This version of the patch adds Camino.sdef to the Copy Resources phase and adds a key to the Info.plist pointing to it.

Comment 1

[Peter Jaros (:peeja)]

Attachment: Trunk version

Trunk version.

Simon, I've targeted you since you probably know must about these things.  Please let me know if you're short on time or I should retarget for any other reason.  Thanks. :)

Comment 2

[Peter Jaros (:peeja)]

Comment on attachment 274388 [details] [diff] [review]
Branch version

Thanks, Simon.

Chris?

Comment 3

[Peter Jaros (:peeja)]

Comment on attachment 274388 [details] [diff] [review]
Branch version

Er, retargeting to the right cl.  Sorry [e-mail]... :)

Comment 4

[Peter Jaros (:peeja)]

Attachment: Branch version, take 2

Fixing a couple things Smokey pointed out:

* Camino.sdef didn't end with a newline.
* Branch version project file called itself "CaminoBranch" in one place, due to the way I was juggling two versions of the project file.

Comment 5

[Peter Jaros (:peeja)]

Attachment: Trunk version, take 2

Comment 6

[Peter Jaros (:peeja)]

Attachment: Correct branch version, part 1 (project file)

Oy.  The branch version was an edited trunk project file, and those changes won't merge properly.  Here's a patch that will actually make the branch changes to the branch version.  Due to my working directory setup, I couldn't add the sdef in the same patch; part 2 will be just adding the sdef.

Comment 7

[Peter Jaros (:peeja)]

Attachment: Correct branch version, part 2 (sdef file)

Comment 8

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 274449 [details] [diff] [review]
Trunk version, take 2

sr=pink

Comment 9

[Peter Jaros (:peeja)]

Stuart: Pink wondered if you wanted to look over the sdef before it's checked in.  Otherwise, this is ready to land.

Comment 10

[Stuart Morgan]

Comment on attachment 274471 [details] [diff] [review]
Correct branch version, part 1 (project file)

There are some issues with the branch res script:

> # Complies the .scriptSuite and .scriptTerminology file from the sdef using sdp.

Needs updating, since that's not what it does anymore.

> sdef_path = "#{ENV['SRCROOT']}/resources/application/Camino.sdef"

You may as well check for this file's existence explicitly before trying to feed it to sdp

> # The output directory
> output_dir = "#{ENV['DERIVED_FILES_DIR']}/aete"

I'd like to see a sanity check that the derived files environment variable is set, since otherwise the script will run around the drive root.

> version = "10.3"

According to sdp's man page, -V isn't necessary for the resource format.

> exit 1 unless system "mkdir -p #{output_dir}"

Use ruby's mkdir_p

> exit 1 unless system "touch #{output_dir}/foobar"		# rm gets grumpy if it can't delete something.
> exit 1 unless system "rm #{output_dir}/*"

At the very least the argument needs to be quoted, as the path may have spaces, but it would be much safer to use Dir.glob to find any .r files and then use ruby's unlink on each of them, bypassing the shell entirely.

> exit 1 unless system "/Developer/Tools/Rez #{ENV['DERIVED_FILES_DIR']}/aete/* ...

Again, at the very least the arguments need to be quoted, but I'd much rather you find the path to the .r file and then use the multi-argument version of |system| so you aren't going through the shell.

Comment 11

[Stuart Morgan]

(I'll try to get the trunk stuff landed tomorrow, BTW.)

Comment 12

[Peter Jaros (:peeja)]

Attachment: New "Compile Scripting Files" Script

New version of the sdp/Rez script.  I discovered that you can, in fact, have sdp output to a specific file, not just a directory.  (That gets messy if you're outputting multiple files, but now we're just making the one .r file.)  So:

1. Comment at the top fixed.
2. Existence of Sdef checked.
3. DERIVED_FILES_DIR checked.  (Fails if not set.  Could provide a fallback, but that should be unnecessary.)
4. -output_dir = "#{ENV['DERIVED_FILES_DIR']}/aete"
   +output_file = "#{ENV['DERIVED_FILES_DIR']}/CaminoScripting.r"
5. -V argument removed.
6. System calls dealing with 'aete' directory removed, since we don't even use it anymore.
7. Rez command is passed arguments directly, not through a shell.
8. Rez command is not exec'd rather than system'd so that Rez can provide error reporting directly to Xcode.  Before, we simply exited 1 if Rez exited nonzero.

Stuart, I know you're manually applying a bunch of project changes, so I'm posting this as a Ruby script rather than a project patch.  The contents can be pasted over the old script in the project.

Comment 13

[Stuart Morgan]

Comment on attachment 275328 [details]
New "Compile Scripting Files" Script

r=me. There's still a small possibility of issues with the sdp command, if someone has a path with a |'| in it, but given that that's unlikely, the command isn't particularly dangerous, and this is code that will all go away in the foreseeable future, I'm fine with leaving that until it actually causes a problem for someone.

Comment 14

[Stuart Morgan]

Comment on attachment 275328 [details]
New "Compile Scripting Files" Script

I was a bit too quick there; per IRC discussion, this needs another spin to mkdir_p the output directory, or the build will fail.

Comment 15

[Stuart Morgan]

Attachment: combo trunk patch [landed]

Combined patch for this and bug 385989, as landed on trunk. Leaving open for branch.

Comment 16

[Peter Jaros (:peeja)]

Attachment: Shell script, now mkdir_p'ing the DERIVED_FILES_DIR

As Stuart said, the DERIVED_FILES_DIR may not exist when the script is run. The script now ensures that the output directory for the .r ($DERIVED_FILES_DIR) exists.

Also moved the declaration of output_file out of the conditional.

Comment 17

[Stuart Morgan]

Comment on attachment 275369 [details]
Shell script, now mkdir_p'ing the DERIVED_FILES_DIR

It turns out aete is only used for SE display in Cocoa; we need the .rsrc for SE cleanness, and the .script* stuff for making it actally work.

Comment 18

[Peter Jaros (:peeja)]

Attachment: Sdp script: Now builds .rsrc & .script*

This one looks like it's building right, but Script Editor's not even opening a dictionary (or giving an error) when I test.  Hoping it's local, since the build looks right on inspection.  Stuart, have at it.  :)

Comment 19

[Stuart Morgan]

Comment on attachment 275523 [details]
Sdp script: Now builds .rsrc & .script*

Builds look good this time, r=me for hopefully the last time ;)

I'll try to get this landed tonight if I have time.

Comment 20

[Stuart Morgan]

Attachment: combo branch patch [landed]

Combined patch for this and bug 385989, as landed on MOZILLA_1_8_BRANCH.