macOS: Let atom.sh find itself when run from an .app #13042

Merged
merged 2 commits into from Dec 8, 2016

Projects

None yet

4 participants

@steakknife
Contributor
steakknife commented Oct 21, 2016 edited

Problem This patch is needed because node-gyp doesn't work with a space in Atom Beta.app -> renamed to Atom-Beta.app. This error occurs:

Cannot locate Atom.app, it is usually located in /Applications. Set the ATOM_PATH environment variable to the directory containing Atom.app.

Use case Developers/others want to multiple versions of Atom side-by-side.

Solution When launched from a Mac .app, a valid, symlinked atom.sh -> atom/atom-beta will always be contained in it's .app. This also works when a different atom-something symlink points to another atom.sh in an .app.

@steakknife steakknife atom.sh: fix for renamed .app
Needed because `node-gyp` doesn't work with a space in `Atom Beta.app`, and developers/others want to multiple versions of Atom side-by-side.
b957b46
atom.sh
@@ -55,11 +55,17 @@ if [ $EXPECT_OUTPUT ]; then
fi
if [ $OS == 'Mac' ]; then
+ ATOM_APP="$(dirname "$(dirname "$(dirname "$(dirname "$(readlink "$0")")")")")"
@steakknife
steakknife Oct 24, 2016 edited Contributor
/Applications/Atom-Beta.app/Contents/Resources/app/atom.sh
        5            4          3        2      1    0
@steakknife steakknife atom.sh: fix direct execute
25650e8
@steakknife steakknife referenced this pull request in Fred-Barclay/Termination Nov 1, 2016
Closed

1.12 beta5 broken #4

@ungb
ungb commented Dec 5, 2016 edited

hey @steakknife,

I tried the scenario above on current beta and wasn't able to see the error you're seeing. could this possibly be fixed already or maybe I'm doing something wrong? Here's a gif of my repro.

test

@ungb
ungb commented Dec 6, 2016

Ah, I was finally able to repro this issue. it's when you open atom from the terminal. I'll test your pr later today and see if it fixes the issue.

Thanks,

Bryant

@ungb
ungb commented Dec 7, 2016

I was able to build and test his changes on beta build.

@ungb ungb removed the needs-testing label Dec 7, 2016
@iolsen iolsen merged commit bb68969 into atom:master Dec 8, 2016

3 checks passed

ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@steakknife steakknife deleted the steakknife:fix-renamed-mac-app branch Dec 10, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment