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

shell_i: Add a FinderSync-based implementation #2340 #3347

Merged
merged 14 commits into from
Jul 2, 2015

Conversation

jturcotte
Copy link
Member

Here what I have already working, I'll go through the checkmarks at #2340 (comment) meanwhile.

cc: @guruz, @dragotin

This prepares the switch to the official FinderSync API on Yosemite
which requires the extension to run in a sandbox. This complicates
the usage of a local socket to communicate with a non-sandboxed GUI
client. An NSConnection is easier to use in this case, which we can
use as long as the server name (i.e. Mach port registered name) is
prefixed with the code signing Team Identifier.

A placeholder server implementation is also added to the client's
SocketApi which basically reproduces the interface of a QLocalSocket.
Most of the references to individual sockets we're only using
QIODevice methods so the type was simply reduced. A typedef to
replace the QLocalServer was the only other part needed.
This uses the new official API to show overlay icons and add our
custom context menu entry instead of hooking directly into the
Finder process and intercept drawind routines.

A dummy desktopclient target is also in the project to allow debugging
directly in Xcode while the official client can be started from the
command line. Otherwise Xcode won't allow attaching to the debugee.

Dummy icon files have been added while we get proper icon produced.
We can't use the old icons since what we use for the legacy shell
integration is already padded according to where the badge should
appear on the full icon.
@jturcotte
Copy link
Member Author

Also please don't merge the request until we can try to build it without an official ownCloud signing key. I'll try at home but if you try to build it that should also do it.

The archive buildaction causes this. Use the default build while
forcing the Release configuration instead.

In both cases the result will end up in SYMROOT.
Since GIT_SHA1 would need to be updated in config.h, all files
including it would be rebuilt by make.

Reduce the number of files to rebuild by moving this variable
to version.h instead.
It was only used on OS X and couldn't be used by the FinderSync
extension since that one runs in a sandbox. So use the same system
to load images in the legacy extension by shipping them in the
extension bundle instead of the owncloud.app bundle.

This is also given that the legacy extension needs padded icons
while the FinderSync one needs unpadded icons.
Since we use that class to lookup the NSBundle using bundleForClass
use a more specific name to avoid any clash with any othe liferay
extension. I couldn't figure out from the documentation if that is
only resolved using the class name, but found some warnings on
stackoverflow and better be safe than sorry for what it costs.
Don't use absolute paths for resources
This allows developers to build and run the extension by default.
Official packages bundles will be re-signed after the build, we

The SocketApi prefix can be set at configure time through cmake and
should match the key that will be used to sign the whole .app bundle
(including the embedded FindexSync .appex bundle).
This tries to catch error at build time instead of having
to check the OS X console for errors afterward.
@jturcotte jturcotte force-pushed the shell_integration_findersync branch from 9153f06 to 18efc5e Compare June 22, 2015 12:03
@jturcotte
Copy link
Member Author

Ok everything should be working fine, I think that the pull request is ready to be merged once you're OK with the code.

@@ -1,12 +1,18 @@
#!/bin/sh -xe
Copy link
Contributor

Choose a reason for hiding this comment

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

@danimo needs to comment on this :-)

@guruz
Copy link
Contributor

guruz commented Jun 22, 2015

Code wise i think it looks good :)

Will try it out now on my 10.9

@guruz
Copy link
Contributor

guruz commented Jun 29, 2015

It works nicely in 10.9. Great job @jturcotte

@danimo
As discussed this needs changes in the build server on signing:
https://github.com/owncloud/client/pull/3347/files#diff-af3b638bc2a3e6c650974192a53c7291R122
https://github.com/owncloud/client/pull/3347/files#diff-d451e3098afdc01086f319f531109038R3

We can merge when @danimo gives the OK for this..

@jturcotte
Copy link
Member Author

We can also start by first enabling 2.0 builds without this branch.

Then I can fix rotor myself, merge the branch and make adjustments right away if anything isn't working as expected.

danimo added a commit that referenced this pull request Jul 2, 2015
shell_i: Add a FinderSync-based implementation #2340
@danimo danimo merged commit 0610d3e into owncloud:master Jul 2, 2015
@danimo
Copy link
Contributor

danimo commented Jul 2, 2015

👍

@jturcotte jturcotte deleted the shell_integration_findersync branch June 27, 2016 16:15
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.

3 participants