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

tmux-fingers: 2.1.1 -> 2.2.2 #336863

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

davidsierradz
Copy link
Contributor

Description of changes

First time writing a Crystal derivation, let me know any changes or improvements I should do!

Friendly ping: @Mic92 @WilliamHsieh

tmux-fingers/CHANGELOG.md at master · Morantron/tmux-fingers

From use cling shard to handle arg parsing · Morantron/tmux-fingers@97d1ddd, tmux-fingers need shards to build.

Things done

  1. Used crystal2nix to generate shards.nix
  2. Built fingers binary
  3. Added patch to substitute upstream binary location to fingers' binary

I mostly copied the build pattern from nixpkgs/pkgs/misc/tmux-plugins/tmux-thumbs/default.nix at master · NixOS/nixpkgs

  • 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.

@WilliamHsieh
Copy link
Contributor

The binary output LGTM!

@WilliamHsieh
Copy link
Contributor

Also in the previous PR the reviewer suggested that tests and source code can be removed.
#280173 (comment)

@davidsierradz
Copy link
Contributor Author

Also in the previous PR the reviewer suggested that tests and source code can be removed.
#280173 (comment)

Thanks, done

Comment on lines +29 to +31
doCheck = false;
doInstallCheck = false;
Copy link
Member

Choose a reason for hiding this comment

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

Maybe add a comment why tests won't work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I'm getting the following error with doCheck:

@nix { "action": "setPhase", "phase": "checkPhase" }
Unhandled exception: Missing ENV key: "TMUX" (KeyError)
  from /nix/store/1cjf5grfqsjbjvqadsdf8l1nhinj474s-crystal-1.11.2-lib/crystal/env.cr:57:7 in 'fetch'
  from /nix/store/1cjf5grfqsjbjvqadsdf8l1nhinj474s-crystal-1.11.2-lib/crystal/env.cr:22:5 in '[]'
  from src/fingers/dirs.cr:5:15 in '~Fingers::Dirs::TMUX_PID:init'
  from src/fingers/dirs.cr:6:3 in '__crystal_main'
  from /nix/store/1cjf5grfqsjbjvqadsdf8l1nhinj474s-crystal-1.11.2-lib/crystal/crystal/main.cr:129:5 in 'main_user_code'
  from /nix/store/1cjf5grfqsjbjvqadsdf8l1nhinj474s-crystal-1.11.2-lib/crystal/crystal/main.cr:115:7 in 'main'
  from /nix/store/1cjf5grfqsjbjvqadsdf8l1nhinj474s-crystal-1.11.2-lib/crystal/crystal/main.cr:141:3 in 'main'
  from /nix/store/wlffq5p6mxxgfap10sav3ij936jzqm59-glibc-2.39-52/lib/libc.so.6 in '??'
  from /nix/store/wlffq5p6mxxgfap10sav3ij936jzqm59-glibc-2.39-52/lib/libc.so.6 in '__libc_start_main'
  from /build/source/.crystal/crystal-run-spec.tmp in '_start'
  from ???

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.

3 participants