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

Add SwiftPM support #575

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from
Open

Conversation

cntrump
Copy link

@cntrump cntrump commented Dec 3, 2021

Checklist

  • documentation is added or updated
  • tests are added or updated

Build library with libtommath:

swift build

Run Swift Unit tests:

swift test

Build library with libtommath:

```
swift build
```

Run Swift Unit tests:

```
swift test
```
@cntrump
Copy link
Author

cntrump commented Dec 3, 2021

Depend on libtom/libtommath#518

@@ -0,0 +1,4 @@
module libtomcrypt [extern_c] {
header "../src/headers/tomcrypt.h"
Copy link
Member

Choose a reason for hiding this comment

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

If libtom/libtommath#518 is really the right way to do stuff, it would also mean that this should be changed to allow using ltc in other modules or not?

Copy link
Author

@cntrump cntrump Dec 3, 2021

Choose a reason for hiding this comment

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

it would also mean that this should be changed to allow using ltc in other modules or not?

Yes, should be changed. if a C package depend on libtomcrypt. because C use #include <tomcrypt.h> in source.
No, if a Swift/ObjC package depend on libtomcyrpt. swift/objc use import libtomcrypt in source.

Copy link
Member

Choose a reason for hiding this comment

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

but it would make sense to support both (C and Swift/ObjC packages depending on this) or not?

Copy link
Author

Choose a reason for hiding this comment

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

but it would make sense to support both (C and Swift/ObjC packages depending on this) or not?

Swift and ObjC can import package as module, module name defined in module.modulemap
But C is not support import module, so use #include <xx> syntax import symbols.

Copy link
Author

Choose a reason for hiding this comment

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

Run swift build -v

header search path in log:

-I .build/checkouts/libtommath/modulemap

Open Package.swift in Xcode and build, header search path in log:

-I ~/Library/Developer/Xcode/DerivedData/libtomcrypt-bewymzccinmagxaywggcjblpnfww/SourcePackages/checkouts/libtommath/modulemap

Copy link
Member

@sjaeckel sjaeckel Dec 3, 2021

Choose a reason for hiding this comment

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

https://github.com/libtom/libtommath/blob/2cec6addaf4acbd8250611ec4cecadf1f4d655ee/Package.swift#L29

I'm not sure if this is correct or should instead state publicHeadersPath: "."

Like that you would also not need libtom/libtommath#518

Copy link
Author

Choose a reason for hiding this comment

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

I'm not sure if this is correct or should instead state publicHeadersPath: "."

It is challenged.

Set publicHeadersPath: "." path same as sources: ["."] and use -fmodule-map-file=modulemap/module.modulemap.

It is not work:

$ swift test

....
./libtommath/mtest/mpi.h:128:8: error: conflicting types for 'mp_mul'
mp_err mp_mul(mp_int *a, mp_int *b, mp_int *c);
       ^
./libtommath/tommath.h:353:8: note: previous declaration is here
mp_err mp_mul(const mp_int *a, const mp_int *b, mp_int *c) MP_WUR;
       ^
<unknown>:0: error: too many errors emitted, stopping now
./libtommath/demo/tommath_tests.swift:2:8: error: could not build Objective-C module 'libtommath'
import libtommath
       ^

error: fatalError

Copy link
Member

Choose a reason for hiding this comment

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

OK, but you shouldn't build mtest in the first place ... also let's move this discussion back to libtom/libtommath#518

DavidSouthgate added a commit to DavidSouthgate/TomCryptSPM that referenced this pull request Jun 18, 2023
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.

2 participants