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

Get started on better implementation handling #25

Closed
wants to merge 2 commits into from

Conversation

coastalwhite
Copy link
Collaborator

Move to better cross implementation handling.

@1wilkens
Copy link
Owner

1wilkens commented Nov 1, 2023

Hey sorry for the very late reply..

I like the direction your PR is going, cleanly separating linux-pam and openpam besides some rudimentary ifdefs in wrapper.h is clearly the way to go.

However, I am not yet sure if we want to add that many C macros and overall content in the headers. Ideally I would like to leave the majority of work to bindgen.

I'll check your PR more in detail in the next weeks and give a more nuanced opinion :)

EDIT: I should have read #18 again before reading this PR. With this added context, I'll come back to this PR and see what we can do. I really would like to release 1.0 ideally still this year :) Thank you for all your hard work so far!

@coastalwhite
Copy link
Collaborator Author

Hi Florian,

I would still be happy to help. This PR is pretty stuck on the linker + bindgen issue. The issue is essentially as follows.

We can either put the OpenPAM and Linux-PAM bindings in the pam-sys crate (1) or introduce two separate crates (2).

For (1), we would need access to both the OpenPAM and Linux-PAM dynamic libraries and headers at compile time since we need them generate the bindings and documentation. Since an OS only ever includes one of the two, this mean we have to include precompiled dynamic libraries, which is definitely suboptimal.

For (2), cargo and the linker have no way of knowing which of the bindings is going to be used when they are invoked which leads to symbol errors and shared cargo links. I don't think this approach is possible at all with the current state of cargo.

I am not sure how to continue that from here. Since both OpenPAM and Linux-PAM are very stable, I would not be against not using bindgen, but I definitely understand that that comes with clear downsides. This would solve the problems that we are facing here though.

@1wilkens. What would you like to see?

@coastalwhite
Copy link
Collaborator Author

Maybe as a compromise solution, that uses the best of both worlds, we can use the bindgen command line. We generate the OpenPAM and Linux-PAM bindings from the CLI using bindgen into separate files and then select which one to use at build time.

@1wilkens
Copy link
Owner

Maybe as a compromise solution, that uses the best of both worlds, we can use the bindgen command line. We generate the OpenPAM and Linux-PAM bindings from the CLI using bindgen into separate files and then select which one to use at build time.

That sounds like a great idea, however this would still require us to include the precompiled binaries as both bindings would be generated right?

@coastalwhite
Copy link
Collaborator Author

This has been migrated to #28.

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