initial screensaver work

[jedahan]

First trial of screensaver work.

Manual testing: on a new user, symlinking to ~/Library/Screen Savers works, but only after a logout and login.

However I don't know how to test different versions of brew-cask. For some reason brew install brew-cask still installs phinze/homebrew-cask even though I edited the brew-cask.rb formula in my taps...

[rolandwalker]

Hi! I'll test this tomorrow and give any help you need until we merge.

You can make brew cask invoke your private code by following the recipe in HACKING.md. It's actually totally different than the recipe in CONTRIBUTING.md, which might should be fixed.

[jedahan]

Install works for me without having to logout/login, but travis CI fails because audit fails because it is recommended to use no_checksum for anything with version 'latest'

These screensavers (and I expect most) will probably not be updated that often (on the order of once every 2-3 years if that)

Is it better to replace the checksum with no_checksum or make up a version string, perhaps based on the year it was released? Both seem less than ideal...

Perhaps its a good time to revisit the idea of 'latest' having no_checksum being a hard requirement for Travis CI to succeed - either ignore audit warnings for travis CI or remove the warning for checksums with version 'latest'

[voanhduy1512]

If it will probably not be updated, or even updated but keep the same url, then no_checksum is all good. The url always points to latest version, then why need to checksum it, right?

[rolandwalker]

Cool -- thanks a lot for the new artifact!

The checksums are a separate issue and I'd love to discuss them in a separate PR or on IRC. The standing rule is that the sha1 stanza must be replaced with no_checksum for these to be admitted.

Your Ruby code is mostly ready now, but there are a few tweaks needed to get it into shape if you are willing.

Quibble 1: One thing that got left out of #2416 is creating the target directory. Let's put it into this PR so that this PR will be a better template for the next new artifact. cask.rb is probably not the best place for it, but that's where it is done currently:

     qlplugindir.mkpath unless qlplugindir.exist?
+    screen_saverdir.mkpath unless screen_saverdir.exist?
   end

Quibble 2: the new files you created lack trailing newlines

Quibble 3: it would be better to use Cask::Artifact::ScreenSaver (with CamelCase) as I believe you did on the first revision. This means you have to change to screen_saver in other places. I realize the CLI option --screen_saverdir looks dopey, but I plan to merge all of those options into a single --librarydir soon.

I also verified that login/logout is not needed. Your Ruby code worked perfectly with the existing Cask ios7-screensaver.rb. However, I had an issue with your new Casks, which I'm going to comment on separately.

[rolandwalker]

The three new Casks included in this PR are all from the same source, presstube.com, and they seem to have some common dependency. When I install chunkulus, for example, the screensaver appears under a generic name "ScreenTime Screen Saver Engine 4.0" and the screensaver preview shows an unfortunately truncated error message.

I see some swf files inside chunkulus so I tried installing Flash, but that was not sufficient. We don't have the dependency support we want inside the Cask definition (yet!) so the best thing would be to figure out the dependency according to the vendor site and add a caveats section to your Casks advising the user what they need to install.

I know the Ruby part of your PR is solid, because I modified existing Cask ios-screensaver.rb as follows and it worked perfectly:

screensaver 'Install iOS 7 screensaver.app/Contents/Resources/iOS 7 lockscreen by bodysoulspirit.qtz'

Below is a screenshot of the symptom which appears to be a dependency issue. Thanks again!

[jedahan]

The saver name always being the same is an issue I am getting as well, but the dependency issue is new to me. I will try on a fresh install of mavericks to see what it needs.

[jedahan]

OK I was able to fix the name using

after_install do
    system "/usr/libexec/PlistBuddy -c \"Set :CFBundleName Chunkulus (Presstube)\" #{self.destination_path}/presstube-chunkulus.app/Contents/Resources/Presstube\\ -\\ Chunkulus.saver/Contents/Info.plist"
end

Figuring out the dependencies will be a bit trickier.

[jedahan]

Should I split the presstube casks to a separate branch, so the screensaver branch can be merged sooner?

[rolandwalker]

Should I split the presstube casks to a separate branch

Sure, then we can proceed on this PR easily. Nice plist hacking, tho. As for the dependency, I just thought of Shockwave, I should have tried that in addition to Flash.

[muescha]

Its realy the goal of brew cask to patch files before they work?

[jedahan]

@muescha The installer that is provided by the screensaver manually patches the plist. I hate when package maintainers modify stuff from upstream, however I found no way to invoke the .swf installer from the commandline. If we can figure out how to do that I would gladly remove the plist change.

[rolandwalker]

@jedahan there's nothing wrong with it. We do what we gotta do to make an install work for the end user. Abstracting out what is buried in the custom installer script makes it more transparent as well.

If you grep 'defaults.write' in the Casks directory, you will see several more such cases of plist hacking.

[rolandwalker]

THANK YOU, especially for the revisions!