Skip to content

chore: drop oldNixpkgs from shell.nix#6897

Open
M1nd3r wants to merge 1 commit into
mainfrom
m1nd3r/drop-oldNixpkgs
Open

chore: drop oldNixpkgs from shell.nix#6897
M1nd3r wants to merge 1 commit into
mainfrom
m1nd3r/drop-oldNixpkgs

Conversation

@M1nd3r
Copy link
Copy Markdown
Contributor

@M1nd3r M1nd3r commented May 10, 2026

Importing oldNixpkgs is no longer needed, as SDL2 was replaced by SDL3, and they are not used anywhere else.

SDL2 libraries moved to fullDeps nix-shell to be used with upgrade tests.

Follows: #6858

@M1nd3r M1nd3r added the no-QA On PR-merge, automatically transition status in the "Firmware" project to "Done (no QA)" state. label May 10, 2026
@trezor-bot trezor-bot Bot added this to Firmware May 10, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 10, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 8afa238a-33a9-4f2e-bf02-1e15b3af2e5e

📥 Commits

Reviewing files that changed from the base of the PR and between 5faec34 and e5aae86.

📒 Files selected for processing (3)
  • docs/tests/upgrade-tests.md
  • shell.nix
  • tests/download_emulators.sh
✅ Files skipped from review due to trivial changes (1)
  • docs/tests/upgrade-tests.md
🚧 Files skipped from review as they are similar to previous changes (2)
  • tests/download_emulators.sh
  • shell.nix

Walkthrough

Updates shell.nix to import a pinned nixpkgs with rustOverlay via overlays, enable unfree packages, and set segger-jlink.acceptLicense from TREZOR_FIRMWARE_ACCEPT_JLINK_LICENSE; removes oldNixpkgs and migrates SDL2 references to sdl2-compat/SDL2_image in fullDeps. Changes docs/tests/upgrade-tests.md to recommend running nix-shell --arg fullDeps true and adds SDL troubleshooting for older emulators. Updates tests/download_emulators.sh to call nix-shell --arg fullDeps true when available.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • mmilata
  • matejcik
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: removing the oldNixpkgs import from shell.nix, which is the core objective of this PR.
Description check ✅ Passed The description provides clear context for the change and references the related PR, but lacks the recommended template sections like development status and project assignments for core developers.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch m1nd3r/drop-oldNixpkgs

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-project-automation github-project-automation Bot moved this to 🔎 Needs review in Firmware May 10, 2026
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 10, 2026

en main(all)

model device_test click_test persistence_test
T2T1 test(all) main(all) test(all) main(all) test(all) main(all)
T3B1 test(all) main(all) test(all) main(all) test(all) main(all)
T3T1 test(all) main(all) test(all) main(all) test(all) main(all)
T3W1 test(all) main(all) test(all) main(all) test(all) main(all)

Latest CI run: 25911049278

@M1nd3r M1nd3r force-pushed the m1nd3r/drop-oldNixpkgs branch 3 times, most recently from d2e7101 to 85d52f9 Compare May 10, 2026 15:35
@M1nd3r M1nd3r marked this pull request as ready for review May 10, 2026 16:53
@M1nd3r M1nd3r requested a review from obrusvit as a code owner May 10, 2026 16:53
@M1nd3r M1nd3r requested a review from mmilata May 10, 2026 16:53
Copy link
Copy Markdown
Member

@mmilata mmilata left a comment

Choose a reason for hiding this comment

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

Sorry, missed the comment in the original PR. We can postpone this until later.

Comment thread tests/download_emulators.sh Outdated
Comment on lines +19 to +20
SDL2="$(nix-build '<nixpkgs>' -A SDL2 --no-out-link)/lib"
SDL2_IMAGE="$(nix-build '<nixpkgs>' -A SDL2_image --no-out-link)/lib"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Oh, did not realize it's needed for old emulators. This is going to use SDL2 and SDL2_image from user's current nixpkgs channel, not what's in shell.nix, which is not good for reproducibility.

I propose to do one of the following:

  1. Put SDL2 SDL2_image (or sdl2-compat SDL2_image, same thing) under the fullDeps section in shell.nix. Add a note that --arg fullDeps true is needed and the SDL_VIDEODRIVER=x11 workaround in doc/tests/upgrade-tests.md.
  2. Put oldNixpkgs.SDL2 oldNixpkgs.SDL2_image in there and only the first note in upgrade-tests.md

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I've implemented the variant 1 8631e69

@M1nd3r M1nd3r force-pushed the m1nd3r/drop-oldNixpkgs branch 2 times, most recently from 5faec34 to 8631e69 Compare May 14, 2026 14:21
@M1nd3r M1nd3r requested a review from mmilata May 14, 2026 14:22
@M1nd3r M1nd3r added ci Continuous Integration (CI) related docs Firmware documentation related https://docs.trezor.io/trezor-firmware/index.html labels May 14, 2026
@obrusvit obrusvit self-requested a review May 15, 2026 09:24
Importing `oldNixpkgs` is no longer needed, as SDL2 was replaced by SDL3, and they are not used anywhere else.

[no changelog]
@M1nd3r M1nd3r force-pushed the m1nd3r/drop-oldNixpkgs branch from 8631e69 to e5aae86 Compare May 15, 2026 09:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci Continuous Integration (CI) related docs Firmware documentation related https://docs.trezor.io/trezor-firmware/index.html no-QA On PR-merge, automatically transition status in the "Firmware" project to "Done (no QA)" state.

Projects

Status: 🔎 Needs review

Development

Successfully merging this pull request may close these issues.

3 participants