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: Mention required openssl dep for rust-script #1216

Merged
merged 2 commits into from Oct 20, 2021

Conversation

mbargull
Copy link
Contributor

@mbargull mbargull commented Oct 12, 2021

Description

https://github.com/snakemake/snakemake/blob/v6.9.1/snakemake/script.py#L1142 needs the json_typegen Crate which pulls in ( https://github.com/evestera/json_typegen/blob/v0.7.0/json_typegen/Cargo.toml#L19 ) its own library/Crate json_typegen_shared and that one by its default features pulls in reqwest ( https://github.com/evestera/json_typegen/blob/v0.7.0/json_typegen_shared/Cargo.toml#L24 ) which further down the road depends on OpenSSL (see cargo tree for json_typegen for the dependency tree).
So, OpenSSL is not a rust-script dependency here, but one that Snakemake-generated Rust "scripts" need.

We should also ask (i.e., issue or if possible PR) the json_typegen maintainers to propagate json_typegen_shared feature selection to json_typegen, i.e., ask to add a feature to json_typegen to deactivate json_typegen_shared's remote-samples feature (possibly also the other default features local-samples, option-parsing then; but those don't concern the OpenSSL problem here).

QC

  • The PR contains a test case for the changes or the changes are already covered by an existing test case.
  • The documentation (docs/) is updated to reflect the changes or this is not necessary (e.g. if the change does neither modify the language nor the behavior or functionalities of Snakemake).

@mbargull mbargull requested a review from johanneskoester as a code owner Oct 12, 2021
@mbargull
Copy link
Contributor Author

mbargull commented Oct 12, 2021

Fails with

error: /home/runner/.cache/rust-script/binaries/release/deps/libjson_typegen-e57fc97d05e82a5a.so: undefined symbol: FIPS_mode_set

which smells like custom Red Hat OpenSSL ... No idea where it picked that one up O_o

@mbargull mbargull marked this pull request as draft Oct 20, 2021
@mbargull mbargull force-pushed the rust-script-openssl branch 3 times, most recently from d56f2c3 to 5b6f7b8 Compare Oct 20, 2021
docs/snakefiles/rules.rst Outdated Show resolved Hide resolved
mbargull added 2 commits Oct 20, 2021
OpenSSL is added indirectly via dependencies pulled in from the
json_typegen=0.7 Crate. The compilation needs OpenSSL sources from the
conda-forge::openssl package and a C compiler that uses the Conda
prefix's path to look for headers/libraries which is provided via
conda-forge::c-compiler.
Help text output changed with Python 3.10 due to renamed optionals
group; ref: https://github.com/python/cpython/pull/23858/files
@sonarcloud
Copy link

sonarcloud bot commented Oct 20, 2021

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@mbargull
Copy link
Contributor Author

mbargull commented Oct 20, 2021

If evestera/json_typegen#34 lands in a json_typegen release, we should be set with only rust-script, depend on the json_typegen crate with default-features = false and could then remove these doc/test changes again.

Though we could then think about what to do to make this more convenient for the -- probably not that uncommon -- cases that use/depend/build C libraries on-the-fly (e.g., @FelixMoelder already encountered this with a rust-script that used rust-htslib at snakemake-workflows/dna-seq-varlociraptor#80 ). For the Conda environment-based workflows we could make a (meta) package snakemake-rust-script-or-similarly-named that just carries these dependencies and then put in the docs not rust-script but snakemake-rust-script as the dependency that users should put into their env.yml files.

@mbargull mbargull marked this pull request as ready for review Oct 20, 2021
@johanneskoester johanneskoester merged commit fc8c5f6 into snakemake:main Oct 20, 2021
6 checks passed
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.

None yet

3 participants