Splitting library, shared_library and wasm library to different crates.

[bakaq]

(Continuing from #2465 (comment), split to discussion because that PR is already too long and this is relevant to other parts of Scryer Prolog)

What about spitting the c/wasm library/interface of into a separate crate?

This is the obvious thing to do actually, but I think we should maybe go even further. I was trying to deal with the issue of visibility for the Rust library interface (see #2490) and I got stuck because of the extensive use of #[macro_use] in Scryer Prolog. That makes a lot of macros that we probably want to be private (like atom!()) to be public (and global!) without a good way to turn it off. I was turning removing every #[macro_use] and doing manual imports instead, but that is a lot of work and a lot of the code depends heavily on these macros being globally public.

Unrelated to that (but also relevant to splitting crates), cargo does versions per package, and not per crate. A package can have multiple crates, and that is what Scryer Prolog does currently. We have a lib and a bin crate in the same scryer-prolog package. This means that they have the same version, which I don't think makes much sense. If we decide to rename something in the public interface in the library side it would be a breaking change and need a major version for crates.io, but then the binary would also get a major version even though that change was completely irrelevant. The converse can also happen. I've pondered a bit about that, and I think the path of least resistance with most gain is to do the following:

• Have this repo be a cargo workspace with multiple packages.
• Leave most of the code where it is in the root package of the workspace called scryer-prolog-core. Ideally we would have a flat workspace where every package has equal status in the workspace, but there were concerns of invalidating current links to the source code the previous time we tried something like that¹. Doing it like this, most of the code (including the Prolog libraries) would stay where it is.
• Have a subdirectory with the other packages of the workspace.
  • scryer-prolog-lib would depend on scryer-prolog-core and have the current Rust library interface. This would easily resolve the visibility issues and give more semver freedom for both packages.
  • scryer-prolog-wasm² and scryer-prolog-sys (calling C interface crates -sys seems to be a convention for Rust) would depend on scryer-prolog-lib (not scryer-prolog-core) and contain the Wasm and C interfaces respectively.
  • scryer-prolog-bin could probably also just depend on scryer-prolog-lib if we do it right, and would contain the binary (and probably literally just 5 lines of code). If we put it in the default-members field we could even cargo run in the workspace without specifying this package and it would just work as it does now.

I'm not sure which of scryer-prolog-bin and scryer-prolog-lib should keep the blessed scryer-prolog package name. I feel that the binary keeping the name makes a bit more sense.

• Pros:
  • Separate versioning of the libraries between themselves and the binary.
  • Deals with visibility in the library interface without refactoring the whole crate to get rid of #[macro_rules].
  • Would make it easier to split scryer-prolog-core further for compilation times and memory usage in the future if we decide to do that.
• Cons:
  • The workflow for some things would change by having to specify -p <package> for some commands.
  • More stuff to keep track of and to publish simultaneously to crates.io.
  • Would make the scryer-prolog package on crates.io lose either the binary or the library, as it currently provides both.

Pinging because you were in the PR thread and will probably want to comment on this @Skgland @jjtolton @triska

Footnotes

1. I personally think that it is worth it, but it's not that big of a deal. If you link to a file on Github you should always use a permalink and/or make it clear that what you are saying is "at time of writing". If your non-permalink breaks it's kind of your fault. ↩

2. I think that with a good enough Rust interface we will not even need a separate wasm package actually, but I need to investigate this a bit more. ↩

[jjtolton]

Edit: posted this in the wrong place, moved to #2490

[Skgland]

I think this is a good idea.

I would suggested keeping the library in the root under the current name and move the binary to a scryer-prolog-cli package but with the same binary name, that should prevent most links from breaking and keep the cli interface the same other that for cargo install one would now need to install scryer-prolog-cli instead of scryer-prolog.

This might also help with #2336

[Skgland]

Ok, I think I missed the part about splinting the current lib crate into scryer-prolog-core and scryer-prolog-lib and would say in that case scryer-prolog-lib should stay as scryer-prolog wherever it ends up.

Though I am not convinced that the split between scryer-prolog-lib and scryer-prolog-core is worth it or helpful.

[bakaq]

It's helpful to not make the macros a semver hazard, and also most of the macros really aren't useful outside of the internals of Scryer Prolog. Making them private isn't as simple as just doing pub(crate), because of #[macro_rules]. I was doing the migration to normal scoped macros but doing the transition is very painful and involves the whole crate because everything assumes the macros are global. Doing a separate crate for the user side would be the easiest way to have a minimal and clean public interface that we can document (#![deny(missing_docs)]) and maintain really well.

[Skgland]

It's helpful to not make the macros a semver hazard, and also most of the macros really aren't useful outside of the internals of Scryer Prolog. Making them private isn't as simple as just doing pub(crate), because of #[macro_rules]. I was doing the migration to normal scoped macros but doing the transition is very painful and involves the whole crate because everything assumes the macros are global.

While I agree that fixing the macros to not use #[macro_export]/#[macro_use] is painful, (i.e. applying the "new method" from https://stackoverflow.com/a/31749071/7847252 to every macro and removing the #[macro_export]/#[macro_use] annotations and adding new import at every use-site), I don't think the split would help much as you would still have that hazard at the interface of scryer-prolog-core and this should be fixed properly as I assume that some future edition will hopefully fix this mess at some point and require the changes (without the workaround) anyways.

Doing a separate crate for the user side would be the easiest way to have a minimal and clean public interface that we can document (#![deny(missing_docs)]) and maintain really well.

We can have #![deny(missing_docs)] anyways as it only applies to the public interface and not internal/private items to my knowledge.

[Skgland]

#![deny(missing_docs)] will even help in a way as it will error if an undocumented function is accidentally public, notifying us if we accidentally make an internal function public.

[bakaq]

We could split everything except scryer-prolog-core and scryer-prolog-lib and decide on that later. I would like to have all breaking changes in the public Rust interface in one go, and doing it in a new crate would be the easiest way to do that, but we can just sit and deal with it too I guess. Splitting the binary and the shared lib still makes sense though.

[Skgland]

Back at a PC so I will now respond in more detail.

I was trying to deal with the issue of visibility for the Rust library interface (see #2490) and I got stuck because of the extensive use of #[macro_use] in Scryer Prolog. That makes a lot of macros that we probably want to be private (like atom!()) to be public (and global!) without a good way to turn it off. I was turning removing every #[macro_use] and doing manual imports instead, but that is a lot of work and a lot of the code depends heavily on these macros being globally public.

I think it would be good to fix this either way to have macros refere to themselves and other items/macros via $crate rather than relying on them being accessible globally/through imports.

Unrelated to that (but also relevant to splitting crates), cargo does versions per package, and not per crate. A package can have multiple crates, and that is what Scryer Prolog does currently. We have a lib and a bin crate in the same scryer-prolog package. This means that they have the same version, which I don't think makes much sense. If we decide to rename something in the public interface in the library side it would be a breaking change and need a major version for crates.io, but then the binary would also get a major version even though that change was completely irrelevant.

As I said in another comment this might also help resolve the warning in #2336 that the scryer-prolog library and executable end up with the same debug symbol file filename.

The converse can also happen. I've pondered a bit about that, and I think the path of least resistance with most gain is to do the following:

• Have this repo be a cargo workspace with multiple packages.
• Leave most of the code where it is in the root package of the workspace called scryer-prolog-core. Ideally we would have a flat workspace where every package has equal status in the workspace, but there were concerns of invalidating current links to the source code the previous time we tried something like that1. Doing it like this, most of the code (including the Prolog libraries) would stay where it is.

👍

• Have a subdirectory with the other packages of the workspace.

👍
I generally like to place by crates into a crates/ folder so that I can simply define the workspace members in Cargo.toml as

[workspace]
members = ["crates/*"]

instead of having to list them one-by-one.

I name the folder crates rather than packages as most are confused about/don't know the difference and I usually only have only one crate per package.

• scryer-prolog-lib would depend on scryer-prolog-core and have the current Rust library interface. This would easily resolve the visibility issues and give more semver freedom for both packages.

I don't think the split between lib and core is worth it as we would need to have core expose internal that lib uses instead of keeping them internal in one crate. Basically don't expose the internal core -> lib interface at all by keeping them in one library.

• scryer-prolog-wasm2 and scryer-prolog-sys (calling C interface crates -sys seems to be a convention for Rust) would depend on scryer-prolog-lib (not scryer-prolog-core) and contain the Wasm and C interfaces respectively.

I know -sys is used for rust bindings of c-libraries i.e. crates that define the link key in Cargo.toml
as cargo only allows one crate with a given link value per dependency tree.
There is also usually a matchin crate exposing a safe interface with the same name with without the -sys suffic.
I don't know if this is also commonly used for the other direction.

• scryer-prolog-bin could probably also just depend on scryer-prolog-lib if we do it right, and would contain the binary (and probably literally just 5 lines of code). If we put it in the default-members field we could even cargo run in the workspace without specifying this package and it would just work as it does now.

We don't need to specify default-members if we only have one binary in the workspace.
and the binary package rust code would just need to be the file currently under src/bin/scryer-prolog.rs

I'm not sure which of scryer-prolog-bin and scryer-prolog-lib should keep the blessed scryer-prolog package name. I feel that the binary keeping the name makes a bit more sense.

I think the package scryer-prolog should be the library as that is the one most would be affected by if it changed.
The executable name also doesn't need to match the package name so we can have a scryer-prolog-cli package that contains a scryer-prolog[.exe] executable.

• Pros:

  • Separate versioning of the libraries between themselves and the binary.
  • Deals with visibility in the library interface without refactoring the whole crate to get rid of #[macro_rules].
  • Would make it easier to split scryer-prolog-core further for compilation times and memory usage in the future if we decide to do that.

• Cons:

  • The workflow for some things would change by having to specify -p <package> for some commands.

The main one that I know of that currently has problems with workspaces is cargo publish

• More stuff to keep track of and to publish simultaneously to crates.io.

At least you can't forget to publish a dependency as crates.io enforces that you can't push a package with un-met dependencies

• Would make the scryer-prolog package on crates.io lose either the binary or the library, as it currently provides both.

I think its common to have a -cli package for the cli.

Footnotes

1. I personally think that it is worth it, but it's not that big of a deal. If you link to a file on Github you should always use a permalink and/or make it clear that what you are saying is "at time of writing". If your non-permalink breaks it's kind of your fault. ↩

I would agree.

2. I think that with a good enough Rust interface we will not even need a separate wasm package actually, but I need to investigate this a bit more. ↩

We probably want a separate interface that we can annotate with #[wasm-bindgen] to auto-generate the bindings for using the wasm module from javascript.

[bakaq]

I think it would be good to fix this either way to have macros refere to themselves and other items/macros via $crate rather than relying on them being accessible globally/through imports.

I was doing that, but it's a lot of stuff to change. Doing another crate would be easier, but maybe we should really just sit and do this.

I generally like to place by crates into a crates/ folder so that I can simply define the workspace members in Cargo.toml [...]

Yes, that's what I meant by "subdirectory". I also use "crates" usually, because "packages" usually only have one crate anyway.

I don't think the split between lib and core is worth it as we would need to have core expose internal that lib uses instead of keeping them internal in one crate. Basically don't expose the internal core -> lib interface at all by keeping them in one library.

Many crates have this pattern of having a -core crate that has a bunch of internal nonsense, and then a crate that wraps it in a nice interface. I think wgpu-core. The current Rust interface doesn't actually use that much currently private stuff from what would be -core, but I see what you mean.

I don't know if this is also commonly used for the other direction.

Fair enough. The only example in the direction we are doing here that really comes to mind is wgpu-native, and they don't use the -sys suffix. -native there is in contrast to browser which is where you would usually use WebGPU.

[Skgland]

-native sounds good/better to me 👍

[triska]

Thank you a lot for this overview and the many useful suggestions and considerations!

I second the suggestion to take the input of @mthom into account especially for synchronizing all proposed changes with developments that are currently ongoing. It is my understanding that extensive changes are pending and it would be great to base all further developments on the latest improvements once they are published.

The only remaining suggestion I have is that, if possible, the existing src/lib/ directory be kept as the location of Prolog libraries. Unless a truly compelling argument against keeping this is brought up, I think it would be nice for the libraries to keep their location also to keep existing links and mentions working, and also to make the libraries easy to find at all times. The libraries will grow in the future while the Rust engine may even shrink (because more parts can be moved to Prolog), and I find it nice that the libraries are available in a rather prominent and easy to reach location in the repository.

[bakaq]

I second the suggestion to take the input of @mthom into account especially for synchronizing all proposed changes with developments that are currently ongoing.

Definitely! I didn't ping him because I thought he would see this eventually, but I guess I should have. This has large consequences, but mostly for the workflow. Everything I proposed here doesn't move or affect most of the code, so rebasing/merging even something as crazy as #2424 wouldn't be much harder than if we just leave things as they are. Splitting deeper parts of the crate is something out of scope here but that we might want to consider in the future for compilation speed and memory usage.

The only remaining suggestion I have is that, if possible, the existing src/lib/ directory be kept as the location of Prolog libraries.

I don't find this as important to maintain (especially if it would be the blocker to have a flat workspace instead of having a root package), but I can't really give a good reason to break it. Everything I proposed here would keep the Prolog libraries in the same place.

[bakaq]

I made a draft PR for solving visibility with the approach of removing the #[macro_use]s instead of separating into crates: #2523. The rest of this discussion is still relevant for separating the binary, shared lib and Wasm interfaces.