griff / metaz

Two letters better than MetaX
https://metaz.maven-group.org/
MIT License
415 stars 56 forks source link

Solve plugin script codesign fail by compiling scripts in project and… #221

Closed jmcintyre closed 4 years ago

jmcintyre commented 4 years ago

… creating symlinks from the plugin directory to a location in the resource directory.

jmcintyre commented 4 years ago

This is one way to try to deal with the fact that the applescripts that are in the plugin directory fail signing in Xcode 11.

griff commented 4 years ago

Well the travis build failed and how du you solve the chicken and egg problem of needing a compiled MetaZ to compile the scripts and the compiled scripts to build MetaZ?

jmcintyre commented 4 years ago

Looks like I broke the Growl part of the build in this branch. I'll try to track that down in the next couple of days. Unfortunately, because Growl signing fails, and I don't have a workaround for that yet, I have to manually delete that part of the build, and re-add before I check it in. Apparently I missed something on this branch.

I haven't run into any issues compiling the applescripts, including with a completely deleted build directory. I saw the note in the readme, but I haven't had a compile failure in my testing. Was the issue something other than a failure to compile?

jmcintyre commented 4 years ago

So, that replicates the compile issue on the server. Not sure why it isn't replicating locally. I'll have to take a look at it closer later this week.

griff commented 4 years ago

The applescript compile issue was something I ran into while setting up travis ci. The osacompile command needs to have a version of MetaZ installed in /Application to compile the scripts. It needs MetaZ because it needs the applescript definition to be able to translate the event names used in the scripts to the two FourCC apple event codes.

griff commented 4 years ago

In essence it needs this to translate queue started processing to MZqustar

jmcintyre commented 4 years ago

Looking at the difference between the build logs on the server and locally, it appears that Xcode 11 is smart enough to link and create the binary before compiling the AppleScripts, but 9.3 isn't. I probably shouldn't look at it any more than that this evening.

jmcintyre commented 4 years ago

As a side note, if you have time to look at the other two pull requests, it would save me a lot of time if those can be applied. I'm juggling a lot of different build issues as I try to get this all woking.

jmcintyre commented 4 years ago

I’m trying compiling on Xcode 11.3, but now I’m having issues with Carthage.

griff commented 4 years ago

Yeah it seems there is a symlink bug in carthage

griff commented 4 years ago

Carthage/Carthage#2726 Carthage/Carthage#2445

jmcintyre commented 4 years ago

Looks that way, though it is supposed to have been fixed. I find it interesting that the issue only happens in release builds (which is why I originally didn't notice it here).

jmcintyre commented 4 years ago

Specifically, in release, the sparkle symlink in the root of the framework directory isn't a symlink it should be (and is in debug).

bdbaddog commented 4 years ago

Will this build in xcode 11.3?

jmcintyre commented 4 years ago

It will build in debug in 11.3 for me with the recent changes that have gone in, but not release due to issues with how Carthage is copying Sparkle. If you are building locally, you could remove Sparle or try just using a debug build.

There is also an issue with a check in I made recently that I discovered tonight related to symbolic links. I’m going to try to fix that tomorrow.

griff commented 4 years ago

@jmcintyre should we just take out carthage and replace it with something else? It is only used to update Sparkle and so it should not be that hard to replace it with something else and at this point it hurts more than it helps since it prevents you from helping out more.

jmcintyre commented 4 years ago

I guess we could put sparkle in directly until they fix whatever the issue is. I'm not sure there is really a better solution for the build server.

griff commented 4 years ago

@jmcintyre lets just do that for now so that we get you up and running.

I am also currently working on creating some test VM's with Catalina and other versions of macOS so that I have an easier time testing and hopefully fixing some of the issues that are OS version specific, including this one.

And I have some prototypes of TheTVDB and TheMovieDb plugins rewritten in Swift and using URLSession that I am testing at the moment.

jmcintyre commented 4 years ago

Those updates sound great. I’ll change the pull request to include sparkle and temporarily disable Carthage. I’ll also create a branch with Carthage still in for testing. Probably can’t get to it today, but hopefully soon. I have quite a few things I’d like to add.

griff commented 4 years ago

@jmcintyre I have looked some more the issues in this PR and two things have become apparent.

  1. The AppleScripts need to be precompiled.
  2. Carthage clearly has a bug in their code when stripping frameworks as part of copy-frameworks. I have basically reopened your issue Carthage/carthage#2948 but bottom line we can work around the issue by setting COPY_PHASE_STRIP to NO

I say that we need to precompile AppleScripts because very soon we won't be able to compile them all on a single machine. The 3 current scripts that deal with iTunes can't compile on my freshly installed Catalina since that doesn't have iTunes. And soon if I can port the iTunes functionality to the TV app we will have scripts that only compile on Catalina. And lastly it looks to me from the latest build failure on the PR that the chicken and egg problem of compiling MetaZ without an existing version of the app installed is still there.

I have tried to make the changes you needed (code signing that works) in a PR of my own #231 which uses precompiled scripts and just re-signs them as part of the build. If those changes work for you I think we should just go with those and close this PR.

jmcintyre commented 4 years ago

This isn't necessary not that griff has other changes.