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.

Sign up for GitHub

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

Jump to bottom

fix OfBorg CI #81334

Merged
merged 2 commits into from Feb 29, 2020
Merged

fix OfBorg CI #81334

merged 2 commits into from Feb 29, 2020

Conversation

jonringer
Copy link
Contributor

@jonringer jonringer commented Feb 28, 2020 •
edited

Motivation for this change

fixes: #81325

EDIT: also, the haskell update also broke stack, which is listed as a necessary package for release.nix

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@jonringer
Copy link
Contributor Author

jonringer commented Feb 28, 2020 •
edited

If this builds, and fixes ofborg eval, I'm pushing to master so i can fix all the broken PR gates

@jonringer jonringer requested a review from peti February 28, 2020 23:24
@jonringer
Copy link
Contributor Author

@peti sorry if this creates some work for you, but I think restoring PR gates is a pretty high priority

@jonringer
Copy link
Contributor Author

cross reference: koalaman/shellcheck#1778

This should be fixed in next release

@cole-h
Copy link
Member

cole-h commented Feb 28, 2020

I was working on a patch that looks slightly different (and also enables manpage generation), if you want to lift anything from it before merge:

patch 
diff --git a/pkgs/development/haskell-modules/configuration-common.nix b/pkgs/development/haskell-modules/configuration-common.nix
index d0af64c5986..efb6e6d850e 100644
--- a/pkgs/development/haskell-modules/configuration-common.nix
+++ b/pkgs/development/haskell-modules/configuration-common.nix
@@ -634,6 +634,33 @@ self: super: {
     '';
   });
 
+  ShellCheck = overrideCabal super.ShellCheck (drv: {
+    buildTools = [ pkgs.pandoc ];
+    patches = [
+      # ShellCheck 0.7.0 is incompatible with Cabal 3.x. Use the patch that
+      # added support to master, remove the broken Setup.hs, and generate
+      # manpages.
+      # https://github.com/koalaman/shellcheck/issues/1778
+      (pkgs.fetchpatch {
+        url =
+          "https://github.com/koalaman/shellcheck/commit/2c026f1ec7c205c731ff2a0ccd85365f37245758.patch";
+        sha256 = "16rscpl8ddly1pzv745wdl3b5m2v5c1yvcbdvzbpmksar7mnqz7h";
+        includes = [ "ShellCheck.cabal" ];
+      })
+    ];
+    postPatch = ''
+      rm Setup.hs
+    '';
+    # fetchpatch cannot create files, so fetch the `manpage` shell script, in
+    # order to generate manpages.
+    postBuild = (pkgs.fetchurl {
+      url =
+        "https://raw.githubusercontent.com/koalaman/shellcheck/2c026f1ec7c205c731ff2a0ccd85365f37245758/manpage";
+      sha256 = "1yg8i4xhkh0nh5m9753lqna93a8irk8azwyl0yjyq1rs72w8zsfr";
+      executable = true;
+    });
+  });
+
   # Build the latest git version instead of the official release. This isn't
   # ideal, but Chris doesn't seem to make official releases any more.
   structured-haskell-mode = overrideCabal super.structured-haskell-mode (drv: {

Otherwise, LGTM:

[3 built, 0.0 MiB DL]
https://github.com/NixOS/nixpkgs/pull/81334
1 package marked as broken and skipped:
shellinabox

2 package built:
nix-info-tested shellcheck

@jonringer
Copy link
Contributor Author

I guess I could just do the includes for mine

@jonringer
Copy link
Contributor Author

@GrahamcOfBorg eval

@jonringer
Copy link
Contributor Author

jonringer commented Feb 28, 2020 •
edited

well, this is annoying... darwin fails due to large out path

EDIT: actually, it was just the warning, the actual error was that stack is marked as broken, but also listed as a required job for release.nix

@jonringer
Copy link
Contributor Author

I'll try to fix darwin another time, but the original nixpkgs-unstable-jobset gate is now passing

@jonringer
Copy link
Contributor Author

also learned that passing options fetchpatch also means that you need to invalidate the sha256

@jonringer
Copy link
Contributor Author

@GrahamcOfBorg eval

@jonringer
Copy link
Contributor Author

@GrahamcOfBorg build nix-info-tested shellcheck

@jonringer
Copy link
Contributor Author 

[2 built, 0.0 MiB DL]
https://github.com/NixOS/nixpkgs/pull/81334
1 package marked as broken and skipped:
shellinabox

2 package built:
nix-info-tested shellcheck

@jonringer
Copy link
Contributor Author

@GrahamcOfBorg build stack

@jonringer
Copy link
Contributor Author

sigh....... stack is listed as a necessary build in release.nix for darwin

@jonringer jonringer changed the title shellcheck: fix build fix OfBorg CI Feb 29, 2020
@jonringer
Copy link
Contributor Author

@GrahamcOfBorg eval

@jonringer
Copy link
Contributor Author

I really don't like touching release.nix, but it's odd that stack is only listed in the darwin outputs. All other platforms don't have it.

@jonringer jonringer merged commit add741e into NixOS:master Feb 29, 2020
@jonringer jonringer deleted the fix-ofborg branch February 29, 2020 00:22
cdepillabout
cdepillabout reviewed Feb 29, 2020
View reviewed changes
pkgs/development/haskell-modules/hackage-packages.nix
@@ -17899,7 +17899,6 @@ self: {
description = "Shell script analysis tool";
license = stdenv.lib.licenses.gpl3;
hydraPlatforms = stdenv.lib.platforms.none;
Copy link
Member

Choose a reason for hiding this comment

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

@jonringer
I believe this hydraPlatforms = line has to be removed as well, otherwise hydra won't build this derivation.

However, this line should be removed automatically the next time the haskell packages are updated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's probably the proper thing to do to get haskellPackages.ShellCheck, however, I just need it so that pkgs.shellcheck ran successfully

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cdepillabout cdepillabout cdepillabout left review comments

@cole-h cole-h cole-h approved these changes

@Ma27 Ma27 Ma27 approved these changes

@infinisil infinisil Awaiting requested review from infinisil

@peti peti Awaiting requested review from peti

Assignees
No one assigned
Labels
6.topic: haskell 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

OfBorg eval broken
4 participants
@jonringer @cole-h @cdepillabout @Ma27