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

Cannot register a singleton wrapped in Arc<_> #131

Closed
m1guelpf opened this issue Dec 24, 2023 · 3 comments · Fixed by #132
Closed

Cannot register a singleton wrapped in Arc<_> #131

m1guelpf opened this issue Dec 24, 2023 · 3 comments · Fixed by #132

Comments

@m1guelpf
Copy link
Contributor

m1guelpf commented Dec 24, 2023

Use-case

In order to register a Singleton, the base type must implement Clone. To prevent unnecessary cloning, it makes sense to use a pointer instead.

Repro

I've created a new repo with a single commit on top of the base pavex new installation, showcasing the issue: https://github.com/m1guelpf/pavex_arc_repro/commit/f4b5abc51b8f166a24ff5523da15a1b1420847ed. Running cargo px run on the root of this repo fails with the following error:

error[E0433]: failed to resolve: use of undeclared crate or module `alloc`
  --> arc_repro_server_sdk/src/lib.rs:97:13
   |
97 |         v1: alloc::sync::Arc<arc_repro::ExampleDependency>,
   |             ^^^^^ use of undeclared crate or module `alloc`
   |
   = help: add `extern crate alloc` to use the `alloc` crate
help: consider importing one of these items
   |
95 +     use crate::alloc::sync;
   |
95 +     use std::sync;
   |
help: if you import `sync`, refer to it directly
   |
97 -         v1: alloc::sync::Arc<arc_repro::ExampleDependency>,
97 +         v1: sync::Arc<arc_repro::ExampleDependency>,
   |

error[E0433]: failed to resolve: use of undeclared crate or module `alloc`
   --> arc_repro_server_sdk/src/lib.rs:109:13
    |
109 |         v0: alloc::sync::Arc<arc_repro::ExampleDependency>,
    |             ^^^^^ use of undeclared crate or module `alloc`
    |
    = help: add `extern crate alloc` to use the `alloc` crate
help: consider importing one of these items
    |
95  +     use crate::alloc::sync;
    |
95  +     use std::sync;
    |
help: if you import `sync`, refer to it directly
    |
109 -         v0: alloc::sync::Arc<arc_repro::ExampleDependency>,
109 +         v0: sync::Arc<arc_repro::ExampleDependency>,
    |

error[E0433]: failed to resolve: use of undeclared crate or module `alloc`
   --> arc_repro_server_sdk/src/lib.rs:118:14
    |
118 |         s_0: alloc::sync::Arc<arc_repro::ExampleDependency>,
    |              ^^^^^ use of undeclared crate or module `alloc`
    |
    = help: add `extern crate alloc` to use the `alloc` crate
help: consider importing one of these items
    |
95  +     use crate::alloc::sync;
    |
95  +     use std::sync;
    |
help: if you import `sync`, refer to it directly
    |
118 -         s_0: alloc::sync::Arc<arc_repro::ExampleDependency>,
118 +         s_0: sync::Arc<arc_repro::ExampleDependency>,
    |

error[E0433]: failed to resolve: use of undeclared crate or module `alloc`
   --> arc_repro_server_sdk/src/lib.rs:119:18
    |
119 |         next: fn(alloc::sync::Arc<arc_repro::ExampleDependency>) -> T,
    |                  ^^^^^ use of undeclared crate or module `alloc`
    |
    = help: add `extern crate alloc` to use the `alloc` crate
help: consider importing one of these items
    |
95  +     use crate::alloc::sync;
    |
95  +     use std::sync;
    |
help: if you import `sync`, refer to it directly
    |
119 -         next: fn(alloc::sync::Arc<arc_repro::ExampleDependency>) -> T,
119 +         next: fn(sync::Arc<arc_repro::ExampleDependency>) -> T,
@m1guelpf
Copy link
Contributor Author

Updated to the latest commit and this seems to still be an issue: https://github.com/m1guelpf/pavex_arc_repro/commit/673e0ce698e35ffb18fa3a84a8a24efdb3dd5885

@LukeMathWalker
Copy link
Owner

I can't reproduce 🤔 If I git pull and cargo px c on that commit it works just fine. Did you regenerate the SDK via a cargo px command?

@m1guelpf
Copy link
Contributor Author

For future reference, I had forgotten to update the CLI to the latest commit 😅

LukeMathWalker added a commit that referenced this issue Jan 16, 2024
# What's wrong

For Pavex to work as expected, the version of the CLI used when
compiling a project with `cargo px [...]` must match the version of the
`pavex` library crate used in the project.
Currently, there's no mechanism to ensure this is the case. As a result,
you'll get version mismatches, resulting in confusing errors (see
@m1guelpf in #131).
The problem is even worse if you have multiple Pavex projects, each
using a different library version: you'd have to juggle multiple CLI
versions on your own since they can't all be in the `PATH`.

# The solution

To eliminate the problem, this PR reworks the `pavex` CLI: it is now a
_version manager_.
All the meaty functionality (server SDK generation, project generation)
is moved into another binary, `pavexc` (**pavex** **c**ompiler).
`pavex` introduces the concept of toolchains, in the same vein of
`rustup`. There can be multiple toolchains installed and it'll pick the
most appropriate one for each command:

- For `new`, it'll check the default toolchain. If none has been
defined, it'll use the corresponding version of `pavexc`.
- For `generate`, it'll match the version of `pavex` (the library) used
in the project. If the corresponding `pavexc` toolchain is not
installed, it'll be installed on the fly.

## Missing pieces

The `pavex` CLI needs to grow a few commands to manage toolchains:
`pavex toolchain default set`, `pavex toolchain default show`, `pavex
toolchain list` and something for removal.
Since they are only relevant for `pavex new`, that's deferred to a later
PR.
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 a pull request may close this issue.

2 participants