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

blender: add shim script for CLI use #18891

Closed
wants to merge 1 commit into from
Closed

Conversation

psibre
Copy link
Contributor

@psibre psibre commented Feb 14, 2016

  • allows blender installed with cask to be used in external toolchains
  • simply adding binary 'blender-#{version}-OSX_10.6-x86_64/blender.app/Contents/MacOS/blender' is insufficient, since blender needs a custom PYTHONPATH pointing to its bundled python library
  • wrapper shim script is generated during preflight, then linked with binary

@psibre
Copy link
Contributor Author

psibre commented Feb 14, 2016

supersedes #9101

@psibre
Copy link
Contributor Author

psibre commented Feb 14, 2016

shim script could be tested upon installation, e.g.,

  # verify binary
  postflight do
    require 'tempfile'
    tempfile = Tempfile.new('tmp.py')
    tempfile.write 'import bpy\nprint(bpy.app.version_string)'
    cmd = "blender --background --python #{tempfile.path}"
    raise "Cannot run command `#{cmd}`" unless system cmd
  end

but did not stage that hunk, pending a proper solution (see #18830)

end
end

binary shimscript, :target => 'blender'
Copy link
Member

Choose a reason for hiding this comment

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

Should be target: 'blender' instead.

@psibre
Copy link
Contributor Author

psibre commented Feb 14, 2016

OK, commits are piling up. I'll be happy to squash if and when it's acceptable... =)

File.open(shimscript, 'w') do |f|
f.puts '#!/bin/bash'
f.puts "export PYTHONHOME=#{staged_path}/blender-#{version}-OSX_10.6-x86_64/blender.app/Contents/Resources/#{version}/python/lib/python#{pythonversion}"
f.puts "#{staged_path}/blender-#{version}-OSX_10.6-x86_64/blender.app/Contents/MacOS/blender $*"
Copy link
Member

Choose a reason for hiding this comment

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

It should be "$@" (quoted as it is), here. Using $* will break in many cases (see differences).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

indeed, thanks for the hint!

Copy link
Member

Choose a reason for hiding this comment

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

After this change, guess you can squash.

@vitorgalvao vitorgalvao added the awaiting user reply Issue needs response from a user. label Feb 14, 2016
@psibre psibre force-pushed the blender branch 2 times, most recently from 67b065a to 4dbcaa9 Compare February 14, 2016 20:54
@psibre
Copy link
Contributor Author

psibre commented Feb 14, 2016

OK, I just replaced all the incremental commits by a single new one.

@vitorgalvao
Copy link
Member

Just realised something. Travis will break here for sure (wrong stanza order).

Instead of

app
app

shimscript

preflight

binary

try

app
app
shimscript
binary

preflight

And run brew cask style {{your_cask}} after (if there are problems, it’ll find them).

@psibre
Copy link
Contributor Author

psibre commented Feb 14, 2016

Thanks, I did notice all the CI failures and was wondering what was going on!

@psibre
Copy link
Contributor Author

psibre commented Feb 14, 2016

but I don't understand the failures on travis...

@vitorgalvao
Copy link
Member

I’ve seen this Travis hanging before, in otherwise correct changes (like #18879). Can you shed some light, @jawshooah?

@@ -9,4 +9,17 @@

app "blender-#{version}-OSX_10.6-x86_64/blender.app"
app "blender-#{version}-OSX_10.6-x86_64/blenderplayer.app"
# shim script
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you mean actually inserting the full URL of #18809?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

never mind, already fixed in 40754ec. Thanks @adidalal !

@adidalal
Copy link
Contributor

Thank you for the contribution. It needed some fixes, so they were made in 40754ec. Your contribution is still included (and still credited to you), with the appropriate modifications. Please feel free to ask about any of the changes.

@adidalal adidalal closed this Feb 16, 2016
@adidalal adidalal removed the awaiting user reply Issue needs response from a user. label Feb 16, 2016
@Homebrew Homebrew locked and limited conversation to collaborators May 8, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants