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

new file: conf-pip-rdkit/conf-pip-rdkit.1.0.0/opam #24633

Closed
wants to merge 1 commit into from

Conversation

UnixJunkie
Copy link
Contributor

this is the modern way to install rdkit, on most distros and operating systems

@UnixJunkie
Copy link
Contributor Author

the section x-ci-accept-failures in the opam file is left there in case the CI environment starts to annoy us

@mseri
Copy link
Member

mseri commented Oct 16, 2023

This cannot work, the sandbox does not allow internet access or the modification of external folders :(

@UnixJunkie
Copy link
Contributor Author

Well, this is a valid opam package !

@UnixJunkie
Copy link
Contributor Author

For a very famous dependency in my field...

@mseri
Copy link
Member

mseri commented Oct 17, 2023

Yes but it does not to what it is meant to be doing. The package should install and check that rdkit is installed. We can skip the installation if it cannot be done, like in this case, since pip would escape opam sandbox and fail, but should have the check that the package is present. So in this form it is not really fulfilling its purpose.

To support pip depext, you could suggest the feature on the ocaml/opam repository, as it was recently done for npm. Once that is in opam, we can let opam install rdkit as well in this conf package.

@UnixJunkie
Copy link
Contributor Author

Ok, I see.
Currently, the important thing for me is to install the depext automatically.
I can add test of the proper install to this package though.

@mseri
Copy link
Member

mseri commented Oct 17, 2023

Thanks!

Python is such a common requirement that I think there could be value in integrating pip with opam fwiw. Feel free to open an issue about it in the ocaml/opam repository, perhaps mention that something similar is being done for npm and could also be reasonable to think of it broader (including R and rust for example).

UPDATE: I only now see you already opened the issue. Thanks!

@mseri
Copy link
Member

mseri commented Mar 15, 2024

I am going to close this for now since it cannot work with the current sandbox limitation. I am happy to reopen-revisit this once the relevant support exists in opam

@mseri mseri closed this Mar 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants