Skip to content

vimPlugins.vim-colorschemes: add overlay to address hash issue - #157622

Merged
jonringer merged 1 commit into
NixOS:masterfrom
malob:fix-vim-colorschemes-hash
Mar 18, 2022
Merged

jonringer merged 1 commit into
NixOS:masterfrom
malob:fix-vim-colorschemes-hash

Conversation

@malob

@malob malob commented Feb 1, 2022

Copy link
Copy Markdown
Member
Motivation for this change

As discussed in #157609, the source for vim-colorschemes contains two files with who's name only differ by the case:

  • colors/darkBlue.vim
  • colors/darkblue.vim

This results in update.py generating a different hash for the plugin depending on whether the file system of the system it's run on is case-sensitive or not. This mainly comes up on Darwin systems.

There's an issue on the upstream repo (flazz/vim-colorschemes#186) as well as a PR to fix the issue (flazz/vim-colorschemes#181).

Unfortunately the owner of the repo hasn't been active for over a year, so waiting for the PR to land probably isn't a viable solution :(

This PR is a proposed hack to address the issue.

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • 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/)
  • 22.05 Release Notes (or backporting 21.11 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
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@malob
malob requested a review from jonringer as a code owner February 1, 2022 01:08
@github-actions github-actions Bot added the 6.topic: vim Advanced text editor label Feb 1, 2022
@malob

malob commented Feb 1, 2022

Copy link
Copy Markdown
Member Author

Result of nixpkgs-review pr 157622 run on aarch64-darwin 1

1 package built:
  • spacevim

Comment thread pkgs/misc/vim-plugins/overrides.nix Outdated
@malob

malob commented Feb 1, 2022

Copy link
Copy Markdown
Member Author

Result of nixpkgs-review pr 157622 run on x86_64-linux 1

1 package built:
  • spacevim

@ofborg ofborg Bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Feb 1, 2022
@malob
malob requested a review from jonringer February 2, 2022 18:12
@malob
malob force-pushed the fix-vim-colorschemes-hash branch 2 times, most recently from d4b39d7 to 1346094 Compare March 14, 2022 22:47
@malob

malob commented Mar 14, 2022

Copy link
Copy Markdown
Member Author

Result of nixpkgs-review pr 157622 run on aarch64-darwin 1

1 package built:
  • spacevim

@malob

malob commented Mar 14, 2022

Copy link
Copy Markdown
Member Author

Result of nixpkgs-review pr 157622 run on x86_64-linux 1

1 package built:
  • spacevim

@malob
malob requested a review from SuperSandro2000 March 14, 2022 23:11

@jonringer jonringer left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think the hash would need to be recomputed as well

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
postFetch = srcOld.postFetch + lib.optionalString (!stdenv.isDarwin) ''
postFetch = (srcOld.postFetch or "") + lib.optionalString (!stdenv.isDarwin) ''

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@jonringer, I updated the PR with the change you suggested.

I initially thought the same thing about the hash, however, the current hash in nixpkgs is based off the source being downloaded on a darwin system. Given the override does nothing on darwin systems, the hash is the same. When I was testing this to ensure the hashes were the same on both systems I set the sha265 hash for the vim-colorschemes source to "" in generated.nix, to see what hashes Nix expected on each system.

On my Mac:

error: hash mismatch in fixed-output derivation '/nix/store/6yy77dsf8j3dmsia15w1xmbmxh2w1742-source.drv':
         specified: sha256-AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=
            got:    sha256-5Y29LUC1cddrX/mwLQoMTWxRcvXHYrgRo+s5IKRx+k4=

On my NixOS machine:

error: hash mismatch in fixed-output derivation '/nix/store/7hzr62y9l08j1c37flskx890p0h489dk-source.drv':
         specified: sha256-AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=
            got:    sha256-5Y29LUC1cddrX/mwLQoMTWxRcvXHYrgRo+s5IKRx+k4=

The expected hash is the same as the current hash in generated.nix:

nix hash to-sri --type sha256 0kpsf6j20fgblc8vhqn7ymr52v2d1h52vc7rbxmxfwdm80nvv3g5
sha256-5Y29LUC1cddrX/mwLQoMTWxRcvXHYrgRo+s5IKRx+k4=

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.

The or "" is only added to prevent evaluation errors if the overwriten Derivation for some reason no longer contain postFetch.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

That's what I assumed yeah.

@malob
malob force-pushed the fix-vim-colorschemes-hash branch from 1346094 to b348e05 Compare March 15, 2022 17:52
@malob
malob requested a review from jonringer March 15, 2022 18:22
@malob
malob force-pushed the fix-vim-colorschemes-hash branch from b348e05 to c332788 Compare March 15, 2022 18:23

@jonringer jonringer left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

Result of nixpkgs-review pr 157622 run on x86_64-linux 1

2 packages built:
  • spacevim
  • tests.vim.test_vim_with_vim_nix

@malob
malob deleted the fix-vim-colorschemes-hash branch March 18, 2022 19:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: vim Advanced text editor 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants