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

Treat xcrun --sdk macosx swift as a single command #16

Closed
wants to merge 1 commit into from

Conversation

mkhl
Copy link
Member

@mkhl mkhl commented Aug 9, 2016

When running a script containing a correct hashbang, like

#!/usr/bin/env xcrun --sdk macosx swift

the TextMate::Executor would replace the first element of command with the parsed hashbang command.

This results in TextMate trying to execute commands like

/usr/bin/env xcrun --sdk macosx swift --sdk macosx swift

which fails because swift(1) doesn’t accept an --sdk parameter.

This change specifies the whole xcrun … swift command as a single element, which it is anyway conceptually, and which fixes this problem.

@mkhl
Copy link
Member Author

mkhl commented Aug 9, 2016

…of course I forgot to test on a script without shebang. Such a script results in this error:

Can't find “xcrun --sdk macosx swift” on PATH.

A workaround might be disabling hashbang parsing for Swift files, otherwise I fear this’ll require changes to TM::Executor.

  • Fix this for scripts without shebang lines

@mkhl
Copy link
Member Author

mkhl commented Aug 9, 2016

I believe this has the same cause as #12

When running a script containing a correct hashbang, like

    #!/usr/bin/env xcrun --sdk macosx swift

the TextMate::Executor would replace the *first* element of `command`
with the parsed hashbang command.

This results in TextMate trying to execute commands like

    /usr/bin/env xcrun --sdk macosx swift --sdk macosx swift

which fails because swift(1) doesn’t accept an `--sdk` parameter.

This change specifies the whole `xcrun … swift` command as a single
element, which it is anyway conceptually, and which fixes this problem.
@mkhl
Copy link
Member Author

mkhl commented Aug 9, 2016

So, the simplest thing that works:
Wrap xcrun … swift in a script inside Support/bin and call that by default.

This change now works for files with and without shebangs.

@sorbits
Copy link
Member

sorbits commented Aug 9, 2016

I fear this’ll require changes to TM::Executor

I assume you’re implying that TM::Executor would have to call shellsplit on the interpreter argument?

This is a change that I think would actually be good in general because many run commands will pass ENV['TM_«interpreter»'] to TM::Executor and here we may want the flexibility to include arguments or go via a shim.

Though it does cause an issue when there is a space in the path to the interpreter.

@jtbandes
Copy link
Collaborator

jtbandes commented Aug 9, 2016

@sorbits
Copy link
Member

sorbits commented Aug 9, 2016

@jtbandes Yes

@mkhl
Copy link
Member Author

mkhl commented Aug 9, 2016

I assume you’re implying that TM::Executor would have to call shellsplit on the interpreter argument?

No, because of spaces in filenames.
I was thinking about either declaring default options separately, such that they won’t duplicate options present in the shebang, or letting the first argument be an Array (like after parse_hashbang) that can be replaced wholesale.

@infininight
Copy link
Member

I just tested the swift command and unless I'm mistaken the whole xcrun part is outdated now? Just running this in terminal works just fine:

#!/usr/bin/env swift

import Cocoa
print("Hello, world!")

So it seems that the macOS SDK is default now, at least with the latest (release not beta) version of Xcode. Not to short circuit talk of improving TM::Executor. :)

@infininight
Copy link
Member

@mkhl I've got a commit here with a simplified Run command:

infininight@8545ed0

Any problems running your code with this change?

@mkhl
Copy link
Member Author

mkhl commented Aug 11, 2016

@infininight That works just fine, thanks.

If we merge that, should we update the shebang snippets to match?
(To my understanding, you need to run xcrun --toolchain … swift if you have development versions of swift installed, but that’s probably too rare to affect the default snippet.)

@infininight
Copy link
Member

Added as b3cb437.

@mkhl mkhl deleted the fix/run branch August 25, 2016 13:30
mkhl added a commit to mkhl/bundle-support.tmbundle that referenced this pull request Feb 13, 2017
Some “Run Script” commands supply arguments to the executable that may lead to errors, which caused for example textmate/swift.tmbundle#16.

This change adds a `default_args` option to `#run` which is appended to the command line when there is *no* hashbang line (or parsing them is disabled).

“Run Script” type commands in other bundles will need updating to take advantage of this change.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants