Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Mac packaging to work without platypus #18522

Merged
merged 5 commits into from
Sep 25, 2016
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 27 additions & 19 deletions contrib/mac/app/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -11,37 +11,45 @@ ifeq ($(JULIA_PKGDIR),)
endif

# X.Y.Z or X.Y.Z-hash
DMG_VERSION_SUFFIX:=$(shell [ $$(git describe --tags --exact-match 2>/dev/null) ] && echo $(JULIA_VERSION) || echo $(JULIA_VERSION)-$(JULIA_COMMIT))
JULIA_VERSION_OPT_COMMIT:=$(shell [ $$(git describe --tags --exact-match 2>/dev/null) ] && echo $(JULIA_VERSION) || echo $(JULIA_VERSION)-$(JULIA_COMMIT))

# X.Y
APP_VERSION_SUFFIX:=$(shell echo $(JULIA_VERSION) | grep -o '^[0-9]\+.[0-9]\+')
JULIA_VERSION_MAJOR_MINOR:=$(shell echo $(JULIA_VERSION) | grep -o '^[0-9]\+.[0-9]\+')

DMG_NAME:=Julia-$(DMG_VERSION_SUFFIX).dmg
APP_NAME:=Julia-$(APP_VERSION_SUFFIX).app
VOL_NAME:=Julia-$(DMG_VERSION_SUFFIX)
DMG_NAME:=Julia-$(JULIA_VERSION_OPT_COMMIT).dmg
APP_NAME:=Julia-$(JULIA_VERSION_MAJOR_MINOR).app
VOL_NAME:=Julia-$(JULIA_VERSION_OPT_COMMIT)

all: clean $(DMG_NAME)

$(DMG_NAME): dmg/$(APP_NAME)
-cp -f julia.icns dmg/.VolumeIcon.icns
-ln -fs /Applications ./dmg/Applications
-chmod 755 ./dmg/$(APP_NAME)/Contents/MacOS/Julia
-chmod 755 $</Contents/MacOS/applet
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would expect osacompile to set the permissions on the applet correctly. Is this necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so. The background for this is #9010, and as I said in that issue it's a pretty weird edge case which most apps don't bother with. Maybe we should take this opportunity to get rid of it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I say we see if we can get rid of it as we iterate on this PR. I'd like to simplify as much as possible, and the spirit of #9010 was the chown on line 30 anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've removed this now.

@echo "We're going to chown the .app file to root:admin now, which requires sudo. You may be asked for your password:"
sudo chown -R root:admin ./dmg/$(APP_NAME)
sudo hdiutil create $(DMG_NAME) -size 500m -ov -volname "$(VOL_NAME)" -imagekey zlib-level=9 -srcfolder dmg

dmg/$(APP_NAME): julia.icns
make -C ../../.. binary-dist
tar zxf ../../../julia-*.tar.gz
mv julia-* julia
-mkdir -p ./julia/libexec ./julia/share
rm -f julia/lib/*.{a,la}
-mkdir dmg
platypus -a Julia -p /bin/bash -V $(JULIA_VERSION) -R -u "The Julia Project" -i julia.icns -Q julia.icns -o "None" -I org.julialang.julia -x -f julia script ./dmg/$(APP_NAME)
-codesign -s "AFB379C0B4CBD9DB9A762797FC2AB5460A2B0DBE" --deep ./dmg/$(APP_NAME)
sudo chown -R root:admin $<
sudo hdiutil create $@ -size 500m -ov -volname "$(VOL_NAME)" -imagekey zlib-level=9 -srcfolder dmg


dmg/$(APP_NAME): startup.applescript julia.icns
-mkdir -p dmg
osacompile -o $@ startup.applescript
rm $@/Contents/Resources/applet.icns
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are these created in the tarball now?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They seem to be. Also, I think these were leftover from the days when we included Tk and Cairo libs.

cp julia.icns $@/Contents/Resources/
plutil -replace CFBundleName -string "Julia" $@/Contents/Info.plist
plutil -replace CFBundleIconFile -string "julia.icns" $@/Contents/Info.plist
plutil -insert CFBundleVersion -string "$(JULIA_VERSION_OPT_COMMIT)" $@/Contents/Info.plist
Copy link
Contributor

@slarew slarew Sep 15, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CFBundleVersion: "build version number should be a string comprised of three non-negative, period-separated integers with the first integer being greater than zero—for example, 3.1.2" ... "While developing a new version of your app, you can include a suffix after the number that is being updated; for example 3.1.3a1"

I believe the JULIA_VERSION_OPT_COMMIT variable does not always precisely follow these rules. Maybe it doesn't matter too much in the case of nightlies which are not tagged commits.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should always be of the form X.Y.Z, X.Y.Z-aaa or X.Y.Z-aaa-hash. At the moment the first integer is zero, but there's not much we can do about that.

For reference, platypus would use X.Y.Z or X.Y.Z-aaa.

plutil -insert CFBundleShortVersionString -string "$(JULIA_VERSION)" $@/Contents/Info.plist
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For release versions (e.g. 0.5.0), JULIA_VERSION is probably fine for CFBundleShortVersionString, which should just be three period separated integers. Not sure how important it is to strictly adhere to docs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Me either. It will be of that form for release versions, but for nightlies it can be 0.5.0-dev

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be fixed now.

plutil -insert CFBundleIdentifier -string "org.julialang.julia" $@/Contents/Info.plist
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This app identifier might be too generic. The prefix org.julialang is good. But the suffix of .julia is too generic. I would suggest something like org.julialang.julia.terminallauncherapp.

There are two related issues I'm thinking of here.

  1. Bundle/app identifiers should be unique across the system. Suppose later on there are multiple apps published that all related to Julia in some way. Maybe one app just launches Terminal and the repl like we have here. But another app launches a native Cocoa repl with inline graphics support. Both apps need a unique bundle ID. So it doesn't hurt to be specific with the bundle ID in order to leave future changes/growth open.
  2. Code signing ties the bundle/app ID to the code signature. And code signing really wants unique code signing IDs.

I'd suggest changing the app id to something more specific now while the app doesn't do anything of substance than have to deal with chaning the app id in the future when it could be more difficult.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had just recently changed it for this reason (it used to be just org.julialang), but we can be more specific here.

Does this mean that we can't later switch the code signer for the same ID?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIK, as of now, there are two points where the code signature comes into play:

  1. GateKeeper checks the identity of the signer, which by default must be Apple or a registered Developer ID. If the signing identity doesn't pass GateKeeper checks, the code isn't allowed to execute. So as long as the signing identity is a Developer ID or Apple, it can be changed in the future without affecting the default behavior.
  2. The macOS application (network) firewall records its rules base on the code signature. I have not tested this, but I think that if the signing identity changes, then the firewall rules may need to be updated. This is probably not a big deal.

plutil -insert NSHumanReadableCopyright -string "© 2016 The Julia Project" $@/Contents/Info.plist
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpicking: Most NSHumanReadableCopyright statements I see begin with the actual word "copyright" including the default one Xcode generates.

IANAL, but I think I read once that international copyright conventions/rules stipulate that the word "copyright" must be used. Of course, maybe I have it backwards...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nevermind, Wikipedia says just using the symbol should be fine.

-mkdir -p $@/Contents/Resources/julia
make -C $(JULIAHOME) binary-dist
tar zxf ../../../julia-*.tar.gz -C $@/Contents/Resources/julia --strip-components 1
Copy link
Contributor

@slarew slarew Sep 15, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe strict adherence to code signing rules would disallow putting executable code (e.g. julia executable and deps libraries) in the Resources directory. But this is a whole different issue...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to the updated Resource Rules, it looks like all files are signed, there is no distinction made between "Resources" and "Binaries" anymore, so the "security" side of this distinction should now be moot.

As far as adherence to protocol goes, Apple states:

...putting code into other places will cause it to be sealed as data (resource) files. This causes this code to be sealed twice; once in the nested code, and once in the outer signature. This wastes both signing and verification time and storage space. Also, this can break the outer signature of apps that use their own update mechanisms to replace nested code. If this nested code is being treated as resources, the outer signature doesn't know that this nested content is actually code.

Always put code and data into their proper places. This applies to all signed code and is enforced by the code signing machinery regardless of how the code is distributed.

They don't threaten us with codesigning breaking if we don't follow the rules, but they do say that the Mac App Store may reject submissions that don't follow these rules without warning:

Note: While strict compliance with these rules may not affect your app today, anything that doesn't meet these requirements note may be rejected by code signing verification (and the Mac App Store validator, in the case of Mac App Store apps) at any point in the future without notice.

All this being said, I'm not at all against moving things around, so if you have a better layout in mind I'd love to hear it. We can move things around pretty flexibly now by setting make flags such as these to place our binaries in the proper location, and our non-binaries in others.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The existing structure is just the same as what we did with platypus, but there's no real reason we can't change it. As a first step, it would probably make sense to structure it as a app-local framework.

I don't think we need to move individual files around though. The R and Python frameworks both just keep the usual unix directory structure buried somewhere, and then symlink everything as necessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That said, maybe this would be better as a different pull request.

rm -f $@/Contents/Resources/julia/lib/*.{a,la}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we get rid of this line?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ViralBShah this was your contribution: is this still necessary? Do we still build .a or .la files?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

they get built by deps and we don't want to install them

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, I didn't see any in my build, that's why I was wondering.

Copy link
Contributor

@tkelman tkelman Sep 15, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh there might not be any under lib/julia at the moment misread

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this line is necessary anymore. I recently checked and noticed that we do not install the .a files.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I misread that path initially, but yeah we should be removing static libs and libtool junk during make install and/or make binary-dist which is before it gets here

-codesign -s "AFB379C0B4CBD9DB9A762797FC2AB5460A2B0DBE" --deep $@

clean:
@echo "We have to use sudo here to clean out folders owned by root. You may be asked for your password"
sudo rm -fr julia dmg *.dmg
sudo rm -fr dmg *.dmg

.PHONY: clean all
7 changes: 0 additions & 7 deletions contrib/mac/app/README

This file was deleted.

11 changes: 11 additions & 0 deletions contrib/mac/app/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
Julia OS X packaging
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OS X -> macOS

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically it's still OS X up to Sierra, which is still unreleased

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if Apple provides official marketing guidelines on the OS X vs macOS naming issue.

====================

This builds the Julia OS X application bundle (.app folder), and stores it in a disk image (.dmg file).

Run `make` to build these files.

Other files in this directory

* `startup.applescript` is compiled to an applet which launches Julia in Terminal.app.
* `julia.icns` is the Julia icon file.
26 changes: 0 additions & 26 deletions contrib/mac/app/run-install-name-tool-change.sh

This file was deleted.

9 changes: 0 additions & 9 deletions contrib/mac/app/script

This file was deleted.

5 changes: 5 additions & 0 deletions contrib/mac/app/startup.applescript
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
set RootPath to POSIX path of (path to me)
tell application "Terminal"
do script ("exec '" & RootPath & "Contents/Resources/julia/bin/julia'")
activate
end tell