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

Project loading should happen asynchronously #6128

Closed
mkaput opened this issue Aug 2, 2024 · 2 comments · Fixed by software-mansion/cairols#2
Closed

Project loading should happen asynchronously #6128

mkaput opened this issue Aug 2, 2024 · 2 comments · Fixed by software-mansion/cairols#2
Assignees

Comments

@mkaput
Copy link
Member

mkaput commented Aug 2, 2024

Primarily, calling scarb metadata should be an asynchronous process that does not block project model updates like it does currently.

Requirements

  • This is a good opportunity to drop the detect_crates_for method and replace it with a proper abstraction. See the description for old issue New Project Model MVP #6124 for more clues.
  • Implement this in such a way, that when doing Rewrite from tower-lsp to lsp-server #6132 we won't have to change a lot here. Primarily, this means that this system should escape async world as early as possible. Use spawn_blocking like we attempt to do in with_db method.
  • Avoid calling project refreshing process (scarb metadata, cairo_project.toml parsing) if another instance is already running (but if one is running: always schedule at most 1 extra run to pick any changes that could happen while the previous run was running). DISCLAIMER: it wasn't done as it wouldn't result in any meaningful benefit after LS: Get rid of needless scarb metadata calls #6689
  • Gracefully deal with projects which loading fails. In such cases, avoid clearing relevant salsa inputs to preserve as much cached queries as possible. For example, a syntax error in the manifest file (this will make scarb metadata fail) shouldn't cause wiping of all analysis state. Check out what RA does here. DISCLAIMER: this is already done in the old project model, just make sure it works with a new one
  • Handle detached files well (i.e. one without matching Scarb.toml/cairo_project.toml well). starkware-libs/cairols-issues#16
@mkaput mkaput added this to CairoLS Aug 2, 2024
@mkaput mkaput converted this from a draft issue Aug 2, 2024
@mkaput mkaput added the ide label Aug 2, 2024
@mkaput mkaput added this to the LS: The Great Rewrite milestone Aug 2, 2024
@mkaput mkaput moved this from Triage to Todo in CairoLS Aug 2, 2024
@mkaput mkaput moved this from Todo to Backlog in CairoLS Aug 5, 2024
@mkaput mkaput moved this from Backlog to Todo in CairoLS Aug 20, 2024
@piotmag769 piotmag769 moved this from Todo to In Progress in CairoLS Nov 5, 2024
@piotmag769
Copy link
Collaborator

Gracefully deal with projects which loading fails. In such cases, avoid clearing relevant salsa inputs to preserve as much cached queries as possible. For example, a syntax error in the manifest file (this will make scarb metadata fail) shouldn't cause wiping of all analysis state

@mkaput this is no longer the case, no? We just try to init unmanaged corelib in case metadata fails now

@mkaput
Copy link
Member Author

mkaput commented Nov 19, 2024

This is more about recovering from breakage when metadata did succeed already. Take a following scenario as an example:

  1. You open OpenZeppelin/cairo-contracts
  2. It analyses successfully.
  3. You open Scarb.toml and make a syntax error.
  4. scarb metadata will now refresh and it will fail

I don't want to lose all the analysis information due to this problem because it is easily recoverable: if we know something about the code already, and this information is reliable, let's persist that information instead of wiping it.

Mind that with detect_crate_for this is already working as intended, but it's a consequence of luck rather than deliberate engineering.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Archived in project
3 participants