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

godu: init at 1.3.0 #96282

Merged
merged 2 commits into from Sep 2, 2020
Merged

godu: init at 1.3.0 #96282

merged 2 commits into from Sep 2, 2020

Conversation

rople380
Copy link
Contributor

Motivation for this change

godu: init at 1.3.0

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.

danieldk
danieldk suggested changes Aug 25, 2020
View reviewed changes
Copy link
Contributor

@danieldk danieldk left a comment

Choose a reason for hiding this comment

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

Thank you for your first contribution! 👍

I have added some comment to improve the derivation.

pkgs/tools/misc/godu/default.nix Outdated Show resolved Hide resolved
pkgs/tools/misc/godu/default.nix Outdated Show resolved Hide resolved
pkgs/tools/misc/godu/default.nix Show resolved Hide resolved
pkgs/tools/misc/godu/default.nix Outdated Show resolved Hide resolved
pkgs/tools/misc/godu/default.nix Outdated Show resolved Hide resolved
@rople380
Copy link
Contributor Author

First pass refactor to buildGoModule has some issues on build:

nix-build -A godu
   these derivations will be built:
  /nix/store/z1bv6zv4x7vdpm4f5h3jchpgz76y6z4w-godu-1.3.0.drv
building '/nix/store/z1bv6zv4x7vdpm4f5h3jchpgz76y6z4w-godu-1.3.0.drv'...
unpacking sources
unpacking source archive /nix/store/2z4g0z26rrcj5y87w0bgi098jw6j4fag-source
source root is source
patching sources
configuring
building
Building subPackage .
godu.go:13:2: cannot find package "." in:
        /build/source/vendor/github.com/gdamore/tcell
state_presenter.go:7:2: cannot find package "." in:
        /build/source/vendor/github.com/gdamore/tcell/views
godu.go:14:2: cannot find package "." in:
        /build/source/vendor/github.com/gosuri/uilive
builder for '/nix/store/z1bv6zv4x7vdpm4f5h3jchpgz76y6z4w-godu-1.3.0.drv' failed with exit code 1
error: build of '/nix/store/z1bv6zv4x7vdpm4f5h3jchpgz76y6z4w-godu-1.3.0.drv' failed

Seems related to: #91374 and #88909.

Not too sure how buildGoModule works, but believe has something to do with vendoring of dependencies and sub package setup.

@ofborg ofborg bot requested a review from kalbasit August 31, 2020 16:56
@danieldk
Copy link
Contributor

Not too sure how buildGoModule works, but believe has something to do with vendoring of dependencies and sub package setup.

Since the sources do not contain a vendor directory, you need to set a hash for vendorSha256:

  vendorSha256 = "1zq7b0zn24cbrjssk4g03i90szp1ms7ila4khwcm7hp9n1py245s";

It seems though that upstream's go.mod is not up to date:

go: inconsistent vendoring in /build/source:
        github.com/mattn/go-isatty@v0.0.12: is marked as explicit in vendor/modules.txt, but not explicitly required in go.mod

Please file an issue upstream to fix this. In the meanwhile you can modify the derivation like this:

diff --git a/pkgs/tools/misc/godu/default.nix b/pkgs/tools/misc/godu/default.nix
index f25f483fc60..77f5a6eee4a 100644
--- a/pkgs/tools/misc/godu/default.nix
+++ b/pkgs/tools/misc/godu/default.nix
@@ -11,7 +11,9 @@ buildGoModule rec {
     sha256 = "1fp8iq4x0qiswksznnd6qh7c6g5pwglzz6ga11a7vgic0201wsvb";
   };
 
-  vendorSha256 = null;
+  patches = [ ./go-mod.patch ];
+
+  vendorSha256 = "1zq7b0zn24cbrjssk4g03i90szp1ms7ila4khwcm7hp9n1py245s";
 
   doCheck = false;
 
diff --git a/pkgs/tools/misc/godu/go-mod.patch b/pkgs/tools/misc/godu/go-mod.patch
new file mode 100644
index 00000000000..c74ad98e9ff
--- /dev/null
+++ b/pkgs/tools/misc/godu/go-mod.patch
@@ -0,0 +1,33 @@
+diff --git a/go.mod b/go.mod
+index cf8f2fb..e405e03 100644
+--- a/go.mod
++++ b/go.mod
+@@ -5,5 +5,6 @@ go 1.14
+ require (
+ 	github.com/gdamore/tcell v1.1.1
+ 	github.com/gosuri/uilive v0.0.0-20170323041506-ac356e6e42cd
++	github.com/mattn/go-isatty v0.0.12 // indirect
+ 	github.com/stretchr/testify v1.3.0
+ )
+diff --git a/go.sum b/go.sum
+index 23c1232..e25c87e 100644
+--- a/go.sum
++++ b/go.sum
+@@ -8,6 +8,8 @@ github.com/gosuri/uilive v0.0.0-20170323041506-ac356e6e42cd h1:1e+0Z+T4t1mKL5xxv
+ github.com/gosuri/uilive v0.0.0-20170323041506-ac356e6e42cd/go.mod h1:qkLSc0A5EXSP6B04TrN4oQoxqFI7A8XvoXSlJi8cwk8=
+ github.com/lucasb-eyer/go-colorful v0.0.0-20181028223441-12d3b2882a08 h1:5MnxBC15uMxFv5FY/J/8vzyaBiArCOkMdFT9Jsw78iY=
+ github.com/lucasb-eyer/go-colorful v0.0.0-20181028223441-12d3b2882a08/go.mod h1:NXg0ArsFk0Y01623LgUqoqcouGDB+PwCCQlrwrG6xJ4=
++github.com/mattn/go-isatty v0.0.12 h1:wuysRhFDzyxgEmMf5xjvJ2M9dZoWAXNNr5LSBS7uHXY=
++github.com/mattn/go-isatty v0.0.12/go.mod h1:cbi8OIDigv2wuxKPP5vlRcQ1OAZbq2CE4Kysco4FUpU=
+ github.com/mattn/go-runewidth v0.0.4 h1:2BvfKmzob6Bmd4YsL0zygOqfdFnK7GR4QL06Do4/p7Y=
+ github.com/mattn/go-runewidth v0.0.4/go.mod h1:LwmH8dsx7+W8Uxz3IHJYH5QSwggIsqBzpuz5H//U1FU=
+ github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
+@@ -16,6 +18,8 @@ github.com/stretchr/objx v0.1.0 h1:4G4v2dO3VZwixGIRoQ5Lfboy6nUhCyYzaqnIAPPhYs4=
+ github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
+ github.com/stretchr/testify v1.3.0 h1:TivCn/peBQ7UY8ooIcPgZFpTNSz0Q2U6UrFlUfqbe0Q=
+ github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
++golang.org/x/sys v0.0.0-20200116001909-b77594299b42 h1:vEOn+mP2zCOVzKckCZy6YsCtDblrpj/w7B9nxGNELpg=
++golang.org/x/sys v0.0.0-20200116001909-b77594299b42/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
+ golang.org/x/text v0.3.0 h1:g61tztE5qeGQ89tm6NTjjM9VPIm088od1l6aSorWRWg=
+ golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
+ gopkg.in/DATA-DOG/go-sqlmock.v1 v1.3.0 h1:FVCohIoYO7IJoDDVpV2pdq7SgrMH6wHnuTyrdrxJNoY=

@rople380
Copy link
Contributor Author

rople380 commented Sep 1, 2020

Since the sources do not contain a vendor directory, you need to set a hash for vendorSha256:

Confused as to how you got this hash, can you show me the workflow?

Added the patch and vendoring deltas, can squash for final pull in if required.

@danieldk
Copy link
Contributor

danieldk commented Sep 1, 2020

Since the sources do not contain a vendor directory, you need to set a hash for vendorSha256:

Confused as to how you got this hash, can you show me the workflow?

Added the patch and vendoring deltas, can squash for final pull in if required.

You can use lib.fakeSha256 or stdenv.lib.fakeSha256 as the hash and build the derivation. It will then complain of a hash mismatch and give you the correct hash.

@danieldk
Copy link
Contributor

danieldk commented Sep 1, 2020

Added the patch and vendoring deltas, can squash for final pull in if required.

Yep, please squash the commits into two. One one where you add yourself to the maintainers list with the message

maintainers: add rople380

The other where you add the derivation with the message (at least the first line):

godu: init at 1.3.0

@rople380
Copy link
Contributor Author

rople380 commented Sep 2, 2020

Rebased and squashed commits as specified.

rople380 and others added 2 commits September 2, 2020 08:11
@rople380 @danieldk
maintainers: add rople380
8511890
@rople380 @danieldk
godu: init at 1.3.0
128e0ec 
Co-authored-by: Daniël de Kok <me@github.danieldk.eu>
danieldk
danieldk approved these changes Sep 2, 2020
View reviewed changes
Copy link
Contributor

@danieldk danieldk left a comment

Choose a reason for hiding this comment

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

Thanks for making the suggested changes! LGTM.

Result of nixpkgs-review pr 96282 1

1 package built:
  • godu

I made some small changes:

  • Since pretty much all platforms were enabled, removed platforms to enable the platforms supported by Go.
  • Enabled tests, since they just work.
  • Shortened the description.
  • Cleaned up the commit message a bit.

@danieldk danieldk merged commit f20daaa into NixOS:master Sep 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@danieldk danieldk danieldk approved these changes

@kalbasit kalbasit Awaiting requested review from kalbasit

Assignees
No one assigned
Labels
8.has: package (new) 10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 10.rebuild-linux: 1-10 10.rebuild-linux: 1 11.by: package-maintainer 12. first-time contribution
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@rople380 @danieldk @Janik-Haag