Skip to content

kitty: make possible to override goModules#241707

Merged
Artturin merged 1 commit intoNixOS:masterfrom
Artturin:kittyoverridable
Jul 6, 2023
Merged

kitty: make possible to override goModules#241707
Artturin merged 1 commit intoNixOS:masterfrom
Artturin:kittyoverridable

Conversation

@Artturin
Copy link
Copy Markdown
Member

@Artturin Artturin commented Jul 5, 2023

go-modules name had to be changed to goModules so it becomes a env var (bash vars can't contain -)

enables overriding the build without overriding configurePhase

pkgs.kitty.overrideAttrs (finalAttrs: previousAttrs: {
  name = "kitty-master";
  version = "master";
  src = pkgs.fetchFromGitHub {
    owner = "kovidgoyal";
    repo = "kitty";
    rev = "66a2efd191f98a1a5b3c2d4fdc9569204b445a2f";
    sha256 = "sha256-pJuhnJOjGBflHXOOYKrg9/e3c+VGEESZtfka84ARCd4=";
  };
  goModules = (pkgs.buildGoModule {
    pname = "kitty-go-modules";
    inherit (finalAttrs) src version;
    vendorHash = "sha256-eAovZ/7JnvXlnlFyt0skhYJ1Nrzy2XCpRlQvSmWCyaY=";
  }).go-modules;
})
Description of changes
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/)
  • 23.11 Release Notes (or backporting 23.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.

go-modules name had to be changed to goModules so it becomes a env
var (bash vars can't contain -)

enables overriding the build without overriding configurePhase

```
pkgs.kitty.overrideAttrs (finalAttrs: previousAttrs: {
  name = "kitty-master";
  version = "master";
  src = pkgs.fetchFromGitHub {
    owner = "kovidgoyal";
    repo = "kitty";
    rev = "66a2efd191f98a1a5b3c2d4fdc9569204b445a2f";
    sha256 = "sha256-pJuhnJOjGBflHXOOYKrg9/e3c+VGEESZtfka84ARCd4=";
  };
  goModules = (pkgs.buildGoModule {
    pname = "kitty-go-modules";
    inherit (finalAttrs) src version;
    vendorHash = "sha256-eAovZ/7JnvXlnlFyt0skhYJ1Nrzy2XCpRlQvSmWCyaY=";
  }).go-modules;
})
```
@ofborg ofborg bot requested review from Luflosi, adamcstephens, rvolosatovs and tex July 5, 2023 16:09
@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. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. labels Jul 5, 2023
Copy link
Copy Markdown
Contributor

@adamcstephens adamcstephens left a comment

Choose a reason for hiding this comment

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

Built successfully on all supported platforms.

@adamcstephens adamcstephens added the 12.approvals: 1 This PR was reviewed and approved by one person. label Jul 6, 2023
@Artturin Artturin merged commit 0ad0cc5 into NixOS:master Jul 6, 2023
@Artturin Artturin deleted the kittyoverridable branch July 6, 2023 17:09
@adamcstephens
Copy link
Copy Markdown
Contributor

Sadly this seems to have broken the update script's vendorHash updating. :(

@Artturin
Copy link
Copy Markdown
Member Author

Artturin commented Jul 11, 2023

Checking out if we can just rename all go-modules to goModules and change nix-update
adding go-modules = goModules; should work as a workaround

@adamcstephens
Copy link
Copy Markdown
Contributor

adamcstephens commented Jul 11, 2023

I've included this fix in the version bump #242896

Thanks for the quick response!

@Artturin Artturin mentioned this pull request Jul 11, 2023
12 tasks
@adamcstephens adamcstephens mentioned this pull request Jul 12, 2023
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. 12.approvals: 1 This PR was reviewed and approved by one person.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants