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

smassh: 3.1.4 -> 3.1.6 #345631

Merged
merged 4 commits into from
Oct 1, 2024
Merged

smassh: 3.1.4 -> 3.1.6 #345631

merged 4 commits into from
Oct 1, 2024

Conversation

khaneliman
Copy link
Contributor

@khaneliman khaneliman commented Oct 1, 2024

Follow up to #345228 with similar fix since #344813 broke it.

EDIT: Also bumping versions released during PR

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@khaneliman
Copy link
Contributor Author

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 345631


x86_64-linux

✅ 2 packages built:
  • smassh
  • smassh.dist

x86_64-darwin

✅ 2 packages built:
  • smassh
  • smassh.dist

aarch64-darwin

✅ 2 packages built:
  • smassh
  • smassh.dist

@kraanzu
Copy link
Contributor

kraanzu commented Oct 1, 2024

Hi @khaneliman, The dooit PR had some tests issue right due to textual version mismatch? Instead of pinning textual version, I can update the repo instead

@khaneliman
Copy link
Contributor Author

Hi @khaneliman, The dooit PR had some tests issue right due to textual version mismatch? Instead of pinning textual version, I can update the repo instead

dooit and smassh both got broken at runtime with the textual 81 update.

File "/nix/store/y40ddcl2k6d5lqsdhh14d9xylqy16ayf-python3.11-textual-0.81.0/lib/python3.11/site-packages/textual/message_pump.py", line 109, in __new__
    class_obj = super().__new__(cls, name, bases, class_dict, **kwargs)
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/nix/store/y40ddcl2k6d5lqsdhh14d9xylqy16ayf-python3.11-textual-0.81.0/lib/python3.11/site-packages/textual/app.py", line 785, in __init_subclass__
    raise ValueError(
ValueError: SCREENS should contain a Screen type or callable, not an instance (got instance of MainScreen for 'main')

@khaneliman
Copy link
Contributor Author

It can be updated upstream to support a newer version of textual, but we just need to unbreak it with the current version of the package here, too.

@kraanzu
Copy link
Contributor

kraanzu commented Oct 1, 2024

Yep. I'm working on it (will push an update in a few minutes), I see that on nixos stable, the textual version is 0.53 but on unstable its the latest (0.81) this will be handled automatically.

@kraanzu
Copy link
Contributor

kraanzu commented Oct 1, 2024

but we just need to unbreak it with the current version of the package here, too.

Ah I see. Sure

I also pushed a newer version so we can also add an update for that

@khaneliman khaneliman changed the title smassh: fix textual version smassh: 3.1.4 -> 3.1.5 Oct 1, 2024
@khaneliman
Copy link
Contributor Author

@kraanzu figured you want to be a maintainer for this so i added you

@kraanzu
Copy link
Contributor

kraanzu commented Oct 1, 2024

Thanks @khaneliman, just one more change, you need to replace appdirs with platformfirs since appdirs is not maintained anymore

@khaneliman khaneliman force-pushed the smassh branch 2 times, most recently from b03ce1a to 70155e6 Compare October 1, 2024 04:01
@khaneliman
Copy link
Contributor Author

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 345631


x86_64-linux

❌ 2 packages failed to build:
  • smassh
  • smassh.dist

x86_64-darwin

❌ 2 packages failed to build:
  • smassh
  • smassh.dist

aarch64-darwin

❌ 2 packages failed to build:
  • smassh
  • smassh.dist

@kraanzu
Copy link
Contributor

kraanzu commented Oct 1, 2024

Hmmm...what went wrong here

@kraanzu
Copy link
Contributor

kraanzu commented Oct 1, 2024

Hi @khaneliman , I think I found the issue and fixed I. You'll need to modify the version to 3.1.6 and change the hash accordingly. Other stuff should be fine

@khaneliman
Copy link
Contributor Author

Hmmm...what went wrong here

sorry, made a tweak before going to bed and had a typo in the relaxedDeps

@khaneliman
Copy link
Contributor Author

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 345631


x86_64-linux

✅ 2 packages built:
  • smassh
  • smassh.dist

x86_64-darwin

✅ 2 packages built:
  • smassh
  • smassh.dist

aarch64-darwin

✅ 2 packages built:
  • smassh
  • smassh.dist

@khaneliman
Copy link
Contributor Author

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 345631


x86_64-linux

✅ 2 packages built:
  • smassh
  • smassh.dist

x86_64-darwin

✅ 2 packages built:
  • smassh
  • smassh.dist

aarch64-darwin

✅ 2 packages built:
  • smassh
  • smassh.dist

@khaneliman khaneliman changed the title smassh: 3.1.4 -> 3.1.5 smassh: 3.1.4 -> 3.1.6 Oct 1, 2024
@kraanzu
Copy link
Contributor

kraanzu commented Oct 1, 2024

Awesome. It works now !

@h7x4 h7x4 merged commit ab9554b into NixOS:master Oct 1, 2024
31 of 32 checks passed
@khaneliman khaneliman deleted the smassh branch October 1, 2024 15:30
@khaneliman khaneliman mentioned this pull request Oct 1, 2024
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants