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

chore: fix tempfile and uuid version due to including breaking change in getrandom #1669

Merged
merged 5 commits into from
Feb 6, 2025

Conversation

KolbyML
Copy link
Member

@KolbyML KolbyML commented Feb 6, 2025

What was wrong?

tempfile 3.16.0 uses getrandom 3 which is a breaking change, which was breaking our deployment script

How was it fixed?

fix tempfile to 3.15

@KolbyML KolbyML requested a review from carver February 6, 2025 21:01
@KolbyML KolbyML self-assigned this Feb 6, 2025
@KolbyML KolbyML force-pushed the fix-tempfile branch 4 times, most recently from fc3fce6 to b37e62c Compare February 6, 2025 21:05
@carver
Copy link
Collaborator

carver commented Feb 6, 2025

I'm still getting the same error which this fix when I try to run a cross-compile locally with:

env PROFILE=release make build-x86_64-unknown-linux-gnu

[cross] warning: Found conflicting cross configuration in /home/carver/.cargo/registry/src/index.crates.io-6f17d22bba15001f/getrandom-0.3.1/Cargo.toml, use [workspace.metadata.cross] in the workspace manifest instead.
Currently only using configuration from /home/carver/.cargo/registry/src/index.crates.io-6f17d22bba15001f/getrandom-0.2.15/Cargo.toml

@carver
Copy link
Collaborator

carver commented Feb 6, 2025

My error was because I tried making the change locally, which didn't include the fact that this PR explicitly downgrades other updates that happened in the Cargo.lock in the previous commit. After checking out directly to this commit, the configuration warning from cross went away.

@carver
Copy link
Collaborator

carver commented Feb 6, 2025

Unfortunately, the pin didn't actually keep it from upgrading in a cargo update. So it's still something we'll have to watch for on the weekly update. Maybe we can fix with a pin of r2d2_sqlite?

│   ├── trin-storage v0.1.0 (/home/carver/code/trin/crates/storage)
│   │   ├── alloy v0.4.2 (*)
│   │   ├── discv5 v0.9.1 (*)
│   │   ├── ethportal-api v0.4.1 (/home/carver/code/trin/crates/ethportal-api) (*)
│   │   ├── r2d2 v0.8.10
│   │   │   ├── log v0.4.25
│   │   │   ├── parking_lot v0.12.3 (*)
│   │   │   └── scheduled-thread-pool v0.2.7
│   │   │       └── parking_lot v0.12.3 (*)
│   │   ├── r2d2_sqlite v0.24.0
│   │   │   ├── r2d2 v0.8.10 (*)
│   │   │   ├── rusqlite v0.31.0
│   │   │   │   ├── bitflags v2.8.0 (*)
│   │   │   │   ├── fallible-iterator v0.3.0
│   │   │   │   ├── fallible-streaming-iterator v0.1.9
│   │   │   │   ├── hashlink v0.9.1 (*)
│   │   │   │   ├── libsqlite3-sys v0.28.0
│   │   │   │   │   [build-dependencies]
│   │   │   │   │   ├── cc v1.2.12 (*)
│   │   │   │   │   ├── pkg-config v0.3.31
│   │   │   │   │   └── vcpkg v0.2.15
│   │   │   │   └── smallvec v1.13.2
│   │   │   └── uuid v1.13.1
│   │   │       ├── getrandom v0.3.1
│   │   │       │   ├── cfg-if v1.0.0
│   │   │       │   └── libc v0.2.169
│   │   │       └── rand v0.9.0
│   │   │           ├── rand_chacha v0.9.0
│   │   │           │   ├── ppv-lite86 v0.2.20 (*)
│   │   │           │   └── rand_core v0.9.0
│   │   │           │       ├── getrandom v0.3.1 (*)
│   │   │           │       └── zerocopy v0.8.17
│   │   │           ├── rand_core v0.9.0 (*)
│   │   │           └── zerocopy v0.8.17

@KolbyML
Copy link
Member Author

KolbyML commented Feb 6, 2025

Unfortunately, the pin didn't actually keep it from upgrading in a cargo update. So it's still something we'll have to watch for on the weekly update. Maybe we can fix with a pin of r2d2_sqlite?

│   ├── trin-storage v0.1.0 (/home/carver/code/trin/crates/storage)
│   │   ├── alloy v0.4.2 (*)
│   │   ├── discv5 v0.9.1 (*)
│   │   ├── ethportal-api v0.4.1 (/home/carver/code/trin/crates/ethportal-api) (*)
│   │   ├── r2d2 v0.8.10
│   │   │   ├── log v0.4.25
│   │   │   ├── parking_lot v0.12.3 (*)
│   │   │   └── scheduled-thread-pool v0.2.7
│   │   │       └── parking_lot v0.12.3 (*)
│   │   ├── r2d2_sqlite v0.24.0
│   │   │   ├── r2d2 v0.8.10 (*)
│   │   │   ├── rusqlite v0.31.0
│   │   │   │   ├── bitflags v2.8.0 (*)
│   │   │   │   ├── fallible-iterator v0.3.0
│   │   │   │   ├── fallible-streaming-iterator v0.1.9
│   │   │   │   ├── hashlink v0.9.1 (*)
│   │   │   │   ├── libsqlite3-sys v0.28.0
│   │   │   │   │   [build-dependencies]
│   │   │   │   │   ├── cc v1.2.12 (*)
│   │   │   │   │   ├── pkg-config v0.3.31
│   │   │   │   │   └── vcpkg v0.2.15
│   │   │   │   └── smallvec v1.13.2
│   │   │   └── uuid v1.13.1
│   │   │       ├── getrandom v0.3.1
│   │   │       │   ├── cfg-if v1.0.0
│   │   │       │   └── libc v0.2.169
│   │   │       └── rand v0.9.0
│   │   │           ├── rand_chacha v0.9.0
│   │   │           │   ├── ppv-lite86 v0.2.20 (*)
│   │   │           │   └── rand_core v0.9.0
│   │   │           │       ├── getrandom v0.3.1 (*)
│   │   │           │       └── zerocopy v0.8.17
│   │   │           ├── rand_core v0.9.0 (*)
│   │   │           └── zerocopy v0.8.17

I will try using a patch instead

@carver
Copy link
Collaborator

carver commented Feb 6, 2025

https://github.com/uuid-rs/uuid/releases/tag/1.13.0
did the breaking change (it should have been a major bump)

@carver
Copy link
Collaborator

carver commented Feb 6, 2025

So I think pinning uuid might do it. I'm trying

@KolbyML
Copy link
Member Author

KolbyML commented Feb 6, 2025

So I think pinning uuid might do it. I'm trying

ok I updated the PR to do that

@carver
Copy link
Collaborator

carver commented Feb 6, 2025

It didn't work for me. A cargo update with the uuid pinned still allows r2d2 to upgrade uuid higher as its sub-dependency.

@KolbyML
Copy link
Member Author

KolbyML commented Feb 6, 2025

It didn't work for me. A cargo update with the uuid pinned still allows r2d2 to upgrade uuid higher as its sub-dependency.

same

@KolbyML
Copy link
Member Author

KolbyML commented Feb 6, 2025

@carver what do you think of doing

# todo: remove this when our other dependencies update to getrandom 0.3 as it is a breaking change
[patch.crates-io]
uuid = { git = "https://github.com/uuid-rs/uuid", tag = "1.12.1" }

for the time being up to you

@carver
Copy link
Collaborator

carver commented Feb 6, 2025

Taking a look... I'm okay with the approach, just want to make sure it works as planned

@carver
Copy link
Collaborator

carver commented Feb 6, 2025

Yeah, I'm good with this. In fact, I think we should revert the tempfile pin and patch it the same way.

@KolbyML
Copy link
Member Author

KolbyML commented Feb 6, 2025

Yeah, I'm good with this. In fact, I think we should revert the tempfile pin and patch it the same way.

ok

@KolbyML KolbyML changed the title chore: fix tempfile to 3.15.0 chore: fix tempfile and uuid version due to including breaking change in getrandom Feb 6, 2025
Copy link
Collaborator

@carver carver left a comment

Choose a reason for hiding this comment

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

🚢 when CI is green

@KolbyML KolbyML merged commit 9870e99 into ethereum:master Feb 6, 2025
11 checks passed
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