Skip to content

Comments

Update salsa#18212

Merged
MichaReiser merged 1 commit intomainfrom
micha/salsa-update
May 20, 2025
Merged

Update salsa#18212
MichaReiser merged 1 commit intomainfrom
micha/salsa-update

Conversation

@MichaReiser
Copy link
Member

@MichaReiser MichaReiser commented May 20, 2025

Summary

Update salsa.

The biggest change is that salsa no longer automatically derives PartialOrd and Ord for salsa-structs. Instead, an explicit #[derive(PartialOrd, Ord)] is required
to opt in to the ID based ordering.

Test Plan

cargo test

@MichaReiser MichaReiser added the internal An internal refactor or improvement label May 20, 2025
@github-actions
Copy link
Contributor

github-actions bot commented May 20, 2025 •
edited
Loading

mypy_primer results

No ecosystem changes detected ✅

@github-actions
Copy link
Contributor

github-actions bot commented May 20, 2025 •
edited
Loading

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Formatter (stable)

✅ ecosystem check detected no format changes.

Formatter (preview)

✅ ecosystem check detected no format changes.

sharkdp
sharkdp approved these changes May 20, 2025
View reviewed changes
Copy link
Contributor

@sharkdp sharkdp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! (also for making that change in salsa)

@MichaReiser MichaReiser force-pushed the micha/salsa-update branch from 4c341d0 to 66ad41b Compare May 20, 2025 07:10
@MichaReiser MichaReiser merged commit 3b56c7c into main May 20, 2025
35 checks passed
@MichaReiser MichaReiser deleted the micha/salsa-update branch May 20, 2025 07:19
@ognevny
Copy link
Contributor

ognevny commented May 24, 2025 •
edited
Loading

this change caused a regression with cargo fetch --locked command. build ran in GitHub Actions machine

Updating crates.io index
      Updating git repository `[https://github.com/salsa-rs/salsa.git`](https://github.com/salsa-rs/salsa.git%60)
  error: failed to get `salsa` as a dependency of package `ruff_db v0.0.0 (D:\_\B\src\ruff\crates\ruff_db)`
  
  Caused by:
    failed to load source for dependency `salsa`
  
  Caused by:
    Unable to update https://github.com/salsa-rs/salsa.git?rev=4818b15f3b7516555d39f5a41cb75970448bee4c#4818b15f
  
  Caused by:
    failed to clone into: C:\Users\runneradmin\.cargo\git\db\salsa-e6f3bb7c2a062968
  
  Caused by:
    revision 4818b15f3b7516555d39f5a41cb75970448bee4c not found
  
  Caused by:
    network failure seems to have happened
    if a proxy or similar is necessary `net.git-fetch-with-cli` may help here
    https://doc.rust-lang.org/cargo/reference/config.html#netgit-fetch-with-cli
  
  Caused by:
    the SSL certificate is invalid; class=Ssl (16)

@MichaReiser
Copy link
Member Author

MichaReiser commented May 24, 2025

This looks like a network issue rather than a problem with the pr itself

@ognevny
Copy link
Contributor

ognevny commented May 24, 2025

This looks like a network issue rather than a problem with the pr itself

I reproduced it on my Windows machine too. also retries didn't help

@MichaReiser
Copy link
Member Author

MichaReiser commented May 24, 2025

Any chance you're behind a proxy? Or have otherwise limited network access? Can you open the url in your browser?

@ognevny
Copy link
Contributor

ognevny commented May 24, 2025

Any chance you're behind a proxy? Or have otherwise limited network access? Can you open the url in your browser?

yes, I can. there shouldn't be a proxy on Windows machine...

anyway I'll open a PR now which updates salsa to stable version

@ognevny ognevny mentioned this pull request May 24, 2025
Closed
@MichaReiser
Copy link
Member Author

MichaReiser commented May 24, 2025

We prefer to keep the git dependency as we're still very actively developing salsa

@AlexWaygood AlexWaygood mentioned this pull request May 27, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sharkdp sharkdp sharkdp approved these changes

@carljm carljm Awaiting requested review from carljm carljm is a code owner

@AlexWaygood AlexWaygood Awaiting requested review from AlexWaygood AlexWaygood is a code owner

@dcreager dcreager Awaiting requested review from dcreager dcreager is a code owner

Assignees

No one assigned

Labels

internal An internal refactor or improvement

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@MichaReiser @ognevny @sharkdp