Skip to content

Conversation

@r-arias
Copy link

@r-arias r-arias commented Aug 13, 2014

Addresses #4
After posting my issue I saw that @iltercengiz has already forked this project here and created a podspec for it. He also fixed a resulting compiling problem (that comes from not including files as "sodium/xyz.h" but as <sodium/xyz.h>). He also made some other changes I did not quite understand and don't think are necessary.
I took your current repository, added his podspec and include fixes, and then changed his Podspec to point to TabbedOut's original repository and hope that this might be useful for someone. Installing SodiumObjc in my project was just adding

pod "SodiumObjc", :git => https://github.com/r-arias/SodiumObjc.git

to the podfile for me and should work using your repository, once you decide to merge this in.
You should check whether the version "1.1" is correct.
Credit for this definitely should go to @iltercengiz.

@iltercengiz
Copy link

Hey, thanks for the credit.
I first did some discovery and those unnecessary commits were them, I think. :)
Thank you for your contribution to this repo, as I was thinking to do it but had no time recently. 👍

@r-arias
Copy link
Author

r-arias commented Aug 18, 2014

People seem not to be available.
@iltercengiz The commit I am referring to is iltercengiz@e31ea08. Could you check again whether anything in there seems relevant to you?

@iltercengiz
Copy link

I've recompiled the lib and they're the results of it.
I'm not sure if recompilation is necessary, but at that time I came across to a problem about invalid archs and thought it was because of compilation of libsodium for other archs.

@r-arias
Copy link
Author

r-arias commented Aug 18, 2014

I stumbled across a similar issue and fixed it (I think) by removing the NAChloride Pod that was still dangling around in my workspace/Project. I got duplicate libsodium binaries through that.

@heydamianc
Copy link
Contributor

Hi guys,

First off, thanks for your contributions. I am not familiar enough with CocoaPods to accept these changes verbatim, but I see some things I think should be changed. Let's see if we can figure this out together.

In the project, there are two top-level groups:

  • private consists of header files that should not leak into the integrating project
  • public consists of header files that should be exposed to the integrating project

Also, no details of the underlying libsodium library should leak out. The intent is that the Objective-C wrapper fully encapsulates all of the implementation details of encrypting/decrypting/signing using idiomatic Objective-C constructs and naming schemes. Blindly exposing all headers violates my initial intent to keep these separate and that's probably the reason why the header includes need to be changed in this revision.

I am opposed to changing that particular header because it is not intended to be exposed and it is copied directly from the libsodium project. Every time that library is rebuilt, the maintainer of SodiumObjc will need to copy that header and manipulate it in order to successfully integrate libsodium.

The following headers are what I intended to be public:

  • SodiumObjc.h
  • public
    • NACL.h
    • NACLNonce.h
    • NSData+NACL.h
    • NSString+NACL.h
    • Public-Key Cryptography
      • NACLAsymmetricKeyPair.h
      • NACLAsymmetricPrivateKey.h
      • NACLAsymmetricPublicKey.h
    • Private-Key Cryptography
      • NACLSymmetricPrivateKey.h
    • Signatures
      • NACLSigningKeyPair.h
      • NACLSigningPrivateKey.h
      • NACLSigningPublicKey.h

I'm not sure how you'd specify this in the PodSpec, however.

@iltercengiz
Copy link

It'll be enough to adjust this line: https://github.com/iltercengiz/SodiumObjc/blob/master/SodiumObjc.podspec#L17

As SodiumObjc.h only imports the header you've mentioned, it'll be enough to mention SodiumObjc.h as the only public header. I'll try to give it a shot asap.

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