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

docs: add info about openssl, pkg-config deps #339

Merged
merged 4 commits into from
Jun 14, 2021

Conversation

shantanoo-desai
Copy link
Contributor

@shantanoo-desai shantanoo-desai commented Jun 9, 2021

  • Without previous installation of openssl-dev, pkg-config,
    cargo install cargo-generate will fail.
  • Potential solution is to install the respective dependencies beforehand

Tested On: Windows10 Pro + WSL2 (Ubuntu-20.04 LTS) + rustc > v1.51
Possible Solution found on: sfackler/rust-openssl#951

Signed-off-by: Shan Desai shantanoo.desai@gmail.com

What this PR does / why we need it:

Additional Information in Documentation for creating a custom discovery handler.

Special notes for your reviewer:

If applicable:

  • this PR contains documentation
  • this PR contains unit tests
  • added code adheres to standard Rust formatting (cargo fmt)
  • code builds properly (cargo build)
  • code is free of common mistakes (cargo clippy)
  • all Akri tests succeed (cargo test)
  • inline documentation builds (cargo doc)
  • version has been updated appropriately (./version.sh)

- Without previous installation of `openssl-dev`, `pkg-config`,
  `cargo install cargo-generate` will fail.
- Potential solution is to install the respective dependencies beforehand

Tested On: Windows10 Pro + WSL2 (Ubuntu-20.04 LTS) + rustc > v1.51
Possible Solution found on: sfackler/rust-openssl#951

Signed-off-by: Shan Desai <shantanoo.desai@gmail.com>
@ghost
Copy link

ghost commented Jun 9, 2021

CLA assistant check
All CLA requirements met.

@kate-goldenring
Copy link
Contributor

Hi Shan! Thanks for finding this. Would it make sense to use the --features vendored-openssl to avoid having to install openssl? vendored-openssl does require other packages, though, so its a trade off. What do you think?

@shantanoo-desai
Copy link
Contributor Author

@kate-goldenring fair point. From a trade-off perspective cargo install cargo-generate is extremely resource intensive (my observation) and not sure if --features vendored-openssl actually makes it easier or not.

Better approach would be to give the reader the choice, either dependencies installation or enable the features flag.

@kate-goldenring
Copy link
Contributor

@shantanoo-desai, is there a way to point out to their documentation so that they have the option, and if their dependencies change we don't need to update our docs?

@shantanoo-desai
Copy link
Contributor Author

shantanoo-desai commented Jun 9, 2021

I was trying to make some sense of the Akri MQTT integration and it turns out The Eclipse Paho Group which is responsible for providing MQTT clients in all programming languages do use the --features flag
Since they essentially are using a rust wrapper on the C library which consists of SSL bindings, this might just be a viable option.

I am not sure why the rust opc-ua client doesn't have such bindings for SSL Certificates.

paho.mqtt.rust Linking OpenSSL Statically

@kate-goldenring
Copy link
Contributor

I am not sure why the rust opc-ua client doesn't have such bindings for SSL Certificates.

Which rust opc ua client are you referring to? Akri's OPC UA discovery handler or a specific crate?

For this PR, i think it would be best to do remove cargo install cargo-generate and replace it with a line that links to their install instructions, such as: "... install cargo-generate ..."

How does that sound?

@shantanoo-desai
Copy link
Contributor Author

Which rust opc ua client are you referring to? Akri's OPC UA discovery handler or a specific crate?

The opcua-client crate used in Akri's OPC-UA Discovery Handler Implementation.

So for this PR it should look something like the following:

-> NOTE: `cargo-generate` depends on `openssl-dev` and `pkg-config` and need to be installed beforehand
-
-__GNU/Linux__:
-
-```sh
-apt install -y openssl-dev pkg-config
-```
-
-__macOS__:
+Install [`cargo-generate`](https://github.com/cargo-generate/cargo-generate#installation) and specify the name of the project with the `--name` parameter.

@kate-goldenring
Copy link
Contributor

The opcua-client crate used in Akri's OPC-UA Discovery Handler Implementation.

Akri's OPC UA discovery handler is simply querying opc ua server's discovery endpoints (which do not require authentication). The client is not creating credentials for itself which may be why no bindings for ssl certs are required.

- Reason: `cargo install cargo-generate` provides `--features` flag to
  enable SSL binding which can add dependencies and remove the `openssl-dev`
  and `pkg-config` standalone installations.
- Approach: Give user the choice to either install these deps accordingly by
  pointing to the Installtion Instruction for `cargo-generate`

Signed-off-by: Shan Desai <shantanoo.desai@gmail.com>
Signed-off-by: Shan Desai <shantanoo.desai@gmail.com>
@kate-goldenring kate-goldenring added the documentation Improvements or additions to documentation label Jun 9, 2021
@kate-goldenring
Copy link
Contributor

@shantanoo-desai can you add your mqtt proposal from commit 00d0393 to a different PR?

@kate-goldenring kate-goldenring merged commit 428a23f into project-akri:main Jun 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants