-
Notifications
You must be signed in to change notification settings - Fork 5
Use system integration options API #10
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
base: main
Are you sure you want to change the base?
Use system integration options API #10
Conversation
README.md
Outdated
By default libssh2 will dynamically link against the crypto backend. | ||
This can be changed through system integration options, i.e. `-fsys` and `-fno-sys` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
System Integration Options do not control whether something is statically or dynamically linked, it's just controlling whether or not we should use the system library. That said, your changes below do make sense as it allows the user to provide/link their own vendored version if they want while still providing a sane default where it "just works"
build.zig
Outdated
// TODO: Add lazy dependency to build.zig.zon for linking statically. | ||
// For now it's the users resposibility to compile and statically link against library |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As mentioned, this has nothing to do with static/dynamic linking just means we don't try to link the system library for folks automatically
We can provide an error message here maybe:
// TODO: Add lazy dependency to build.zig.zon for linking statically. | |
// For now it's the users resposibility to compile and statically link against library | |
@panic("mbedtls not provided. If you pass --no-sys=mbedtls you must link your own mbedtls to the final app."); |
There was a problem hiding this comment.
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 a nice error message can be provided, because using @panic
here will make it always show the message.
Currently the error you will get when it isn't linked is this:
/home/rokas/.cache/zig/p/N-V-__8AAATRLQBu_rNy4X2UK6RtcdYK_yAzkV6OcCqgo5aK/src/mbedtls.h:59:10: error: 'mbedtls/version.h' file not found
#include <mbedtls/version.h>
^~~~~~~~~~~~~~~~~~~~
/home/rokas/.cache/zig/p/N-V-__8AAATRLQBu_rNy4X2UK6RtcdYK_yAzkV6OcCqgo5aK/src/agent.c:42:10: note: in file included from /home/rokas/.cache/zig/p/N-V-__8AAATRLQBu_rNy4X2UK6RtcdYK_yAzkV6OcCqgo5aK/src/agent.c:42:
#include "libssh2_priv.h"
^
/home/rokas/.cache/zig/p/N-V-__8AAATRLQBu_rNy4X2UK6RtcdYK_yAzkV6OcCqgo5aK/src/libssh2_priv.h:187:10: note: in file included from /home/rokas/.cache/zig/p/N-V-__8AAATRLQBu_rNy4X2UK6RtcdYK_yAzkV6OcCqgo5aK/src/libssh2_priv.h:187:
#include "crypto.h"
^
/home/rokas/.cache/zig/p/N-V-__8AAATRLQBu_rNy4X2UK6RtcdYK_yAzkV6OcCqgo5aK/src/crypto.h:49:10: note: in file included from /home/rokas/.cache/zig/p/N-V-__8AAATRLQBu_rNy4X2UK6RtcdYK_yAzkV6OcCqgo5aK/src/crypto.h:49:
#include "mbedtls.h"
// ...
build.zig
Outdated
// TODO: Add lazy dependency to build.zig.zon for linking statically. | ||
// For now it's the users resposibility to compile and statically link against library |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here, let's add an error message
ssh2_lib.linkSystemLibrary2("bcrypt", .{}); | ||
ssh2_lib.linkSystemLibrary2("ncrypt", .{}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might as well use linkSystemLibrary
since we aren't passing any options
build.zig
Outdated
// TODO: Add lazy dependency to build.zig.zon for linking statically. | ||
// For now it's the users resposibility to compile and statically link against library |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here, add an error message
Resolves #7
Here is an example for how the build.zig needs to look if you are using
-fno-sys=mbedtls
: